Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1691132 > unrolled thread

Re: [PATCH 1/3] mfd: Add support for FTDI FT232H devices

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 1/3] mfd: Add support for FTDI FT232H devices Johan Hovold <johan@kernel.org> - 2017-07-19 11:00 +0200
    Re: [PATCH 1/3] mfd: Add support for FTDI FT232H devices Anatolij Gustschin <agust@denx.de> - 2017-07-19 15:00 +0200
      Re: [PATCH 1/3] mfd: Add support for FTDI FT232H devices Johan Hovold <johan@kernel.org> - 2017-07-25 14:00 +0200
        Re: [PATCH 1/3] mfd: Add support for FTDI FT232H devices Anatolij Gustschin <agust@denx.de> - 2017-07-25 14:20 +0200

#1691132 — Re: [PATCH 1/3] mfd: Add support for FTDI FT232H devices

On Mon, Jul 10, 2017 at 02:52:10PM +0200, Johan Hovold wrote:
> On Thu, Jul 06, 2017 at 10:49:16PM +0200, Anatolij Gustschin wrote:
> > Add USB part with common functions for USB-GPIO/I2C/SPI master
> > adapters. These allow communication with chip's control, transmit
> > and receive endpoints and will be used by various FT232H drivers.
> 
> > +static const struct mfd_cell ftdi_cells[] = {
> > +	{ .name = "ftdi-cbus-gpio", },
> > +	{ .name = "ftdi-mpsse-i2c", },
> > +	{ .name = "ftdi-mpsse-spi", },
> > +	{ .name = "ftdi-fifo-fpp-mgr", },
> > +};
> 
> Correct me if I'm wrong, but aren't these modes really mutually
> exclusive, possibly with exception of cbus-gpio (some pins are at least
> available as GPIOs in MPSSE mode)? Then MFD is not is not the right fit
> here either.

You never replied to this, and I'm afraid there are more issue with this
series.

> And as David Laight already pointed out, your ftdi-fifo-fpp-mgr driver
> seems too application specific for a generic chip like this.

Of which this is one is one of the major.

In short, your driver is much to application specific and is probably
something that needs to be implemented in userspace using libftdi.

Speaking of libftdi, you seem to have copied or borrowed a lot of code
and protocol from libftdi and this should have been mentioned in commit
messages and file headers (not just in a comment to one specific
function).

These chips can be used for a many different applications (also in FIFO
mode) so you cannot tie a driver to it exposing just a specific
interface for programming a certain class of FPGAs.

Johan

[toc] | [next] | [standalone]

#1691653

On Wed, 19 Jul 2017 10:59:34 +0200
Johan Hovold johan@kernel.org wrote:
...
>> > +static const struct mfd_cell ftdi_cells[] = {
>> > +	{ .name = "ftdi-cbus-gpio", },
>> > +	{ .name = "ftdi-mpsse-i2c", },
>> > +	{ .name = "ftdi-mpsse-spi", },
>> > +	{ .name = "ftdi-fifo-fpp-mgr", },
>> > +};  
>> 
>> Correct me if I'm wrong, but aren't these modes really mutually
>> exclusive, possibly with exception of cbus-gpio (some pins are at least
>> available as GPIOs in MPSSE mode)? Then MFD is not is not the right fit
>> here either.  
>
>You never replied to this, and I'm afraid there are more issue with this
>series.

Sorry, unfortunately I'm too busy with other stuff. Will try to find
time to rework.

>> And as David Laight already pointed out, your ftdi-fifo-fpp-mgr driver
>> seems too application specific for a generic chip like this.  
>
>Of which this is one is one of the major.

Thanks all for feedback. I'm still pondering how to interface the
fpga manager driver to FTDI FIFO driver.

>In short, your driver is much to application specific and is probably
>something that needs to be implemented in userspace using libftdi.

I have a requirement to use the fpga manager framework, therefore the
kernel driver is needed. Our usage scenario is a multi stage fpga
configuration process, the first stage is a pre-configuration via
FTDI SPI/FIFO, all subsequent stages are also done by other fpga
manager drivers. libftdi based driver already existed for hardware
bring-up, now I need similar functionality as kernel fpga manager.

>Speaking of libftdi, you seem to have copied or borrowed a lot of code
>and protocol from libftdi and this should have been mentioned in commit
>messages and file headers (not just in a comment to one specific
>function).

I'll mention this in next patch series.

>These chips can be used for a many different applications (also in FIFO
>mode) so you cannot tie a driver to it exposing just a specific
>interface for programming a certain class of FPGAs.

Agreed.

Anatolij

[toc] | [prev] | [next] | [standalone]

#1695657

On Wed, Jul 19, 2017 at 02:59:00PM +0200, Anatolij Gustschin wrote:
> On Wed, 19 Jul 2017 10:59:34 +0200
> Johan Hovold johan@kernel.org wrote:

> >> And as David Laight already pointed out, your ftdi-fifo-fpp-mgr driver
> >> seems too application specific for a generic chip like this.  
> >
> >Of which this is one is one of the major.
> 
> Thanks all for feedback. I'm still pondering how to interface the
> fpga manager driver to FTDI FIFO driver.
> 
> >In short, your driver is much to application specific and is probably
> >something that needs to be implemented in userspace using libftdi.
> 
> I have a requirement to use the fpga manager framework, therefore the
> kernel driver is needed. Our usage scenario is a multi stage fpga
> configuration process, the first stage is a pre-configuration via
> FTDI SPI/FIFO, all subsequent stages are also done by other fpga
> manager drivers. libftdi based driver already existed for hardware
> bring-up, now I need similar functionality as kernel fpga manager.

How are you even accessing these fpga-managers from userspace? I thought
current interface was for in-kernel use only?

Thanks,
Johan

[toc] | [prev] | [next] | [standalone]

#1695677

On Tue, 25 Jul 2017 13:52:35 +0200
Johan Hovold johan@kernel.org wrote:

>On Wed, Jul 19, 2017 at 02:59:00PM +0200, Anatolij Gustschin wrote:
>> On Wed, 19 Jul 2017 10:59:34 +0200
>> Johan Hovold johan@kernel.org wrote:  
>
>> >> And as David Laight already pointed out, your ftdi-fifo-fpp-mgr driver
>> >> seems too application specific for a generic chip like this.    
>> >
>> >Of which this is one is one of the major.  
>> 
>> Thanks all for feedback. I'm still pondering how to interface the
>> fpga manager driver to FTDI FIFO driver.
>>   
>> >In short, your driver is much to application specific and is probably
>> >something that needs to be implemented in userspace using libftdi.  
>> 
>> I have a requirement to use the fpga manager framework, therefore the
>> kernel driver is needed. Our usage scenario is a multi stage fpga
>> configuration process, the first stage is a pre-configuration via
>> FTDI SPI/FIFO, all subsequent stages are also done by other fpga
>> manager drivers. libftdi based driver already existed for hardware
>> bring-up, now I need similar functionality as kernel fpga manager.  
>
>How are you even accessing these fpga-managers from userspace? I thought
>current interface was for in-kernel use only?

We have different possibilities to access to these fpga-managers from
userspace. On platforms with device-tree support there is a DT-Overlay
configfs interface for inserting a blob with configuration data and
fpga device description. After inserting the blob the fpga manager is
invoked automatically. There are also fpga-mgr debugfs config interface
patches in Altera linux-socfpga git tree for accessing from userspace.
I used them for initial testing, but now I'm using additional kernel
module with a single configuration interface for different managers.

Thanks,
Anatolij

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web