Re: [PATCH v6] USB: serial: ftdi_sio: implement GPIO support for FT-X devices

[Johan Hovold]

On Tue, Sep 25, 2018 at 10:46:30AM +0000, Karoly Pados wrote:
> Hi,
> 
> >> +#if defined(CONFIG_GPIOLIB)
> >> +static const char * const ftdi_ftx_gpio_names[] = {
> >> + "CBUS0", "CBUS1", "CBUS2", "CBUS3"
> >> +};
> >> +#endif
> > 
> > We want to keep the ifdeffery to a minimum, so move this inside the
> > gpiolib ifdef below (and possibly even into the function where it is
> > used).
> > 
> > Also note that these names are shared with FT232R, but not with FT232H.
> > 
> 
> What naming do you suggest then?
> 
> My personal preference would be however to leave this name as is, because
> this patch only adds support for the FT-X. Even if support for others can 
> be added relatively trivially after this, there is explicitly no GPIO 
> support for FT232R *yet*. If somebody else adds GPIO support for the FT232R
> in a later patch, he/she should make corresponding adjustments themselves,
> including naming changes. IMHO.

Yes, that's perfectly fine. I was merely pointing it out as background
info which could possibly affect how you choose to address this (e.g.
moving it into the ftx function or not, but also that can be changed
later of course).

> >> + if (priv->gpio_output & BIT(gpio))
> >> + return 0;
> >> + else
> >> + return 1;
> > 
> > This could just simplified using negation (!), but perhaps this is
> > easier to parse as it stands.
> > 
> 
> Sorry, it is not clear what your preferred action here is. 
> So should I leave it as is then or not?

Just do

	res = !(priv->gpio_output & BIT(gpio));

And I think you should add locking here to.

Johan