Re: [PATCH] USB: ftdi_sio: add GPIO support
On Sat, Jul 4, 2015 at 12:13 AM, Grant Likely wrote: > On Tue, Jun 2, 2015 at 2:18 PM, Linus Walleij > wrote: >> On Sat, May 30, 2015 at 10:29 PM, Grant Likely >> wrote: >>> On Mon, Jul 7, 2014 at 6:31 PM, Greg Kroah-Hartman >>> wrote: >> > However is the MFD cell approach acceptable? Yes it is. >>> >>> Going back to this old conversation... Actually, I disagree. There is >>> absolutely no need to go the MFD approach for this driver. That just >>> adds layers of abstraction for no purpose. GPIOLIB is an interface, >>> and it is completely fine for a driver to hook up to the GPIOLIB >>> interface at the same time as exposing a serial port. MFD doesn't buy >>> the driver anything useful here. >> >> What is buys is centralizing code into the proper drivers/gpio >> folder of the kernel. So more of a maintenance point than a >> mechanics/performance point. >> >> We do have GPIO drivers scattered all over the kernel so one >> more or less wouldn't matter so much... > > Yeah, I would say that's a non-reason. When it comes to a single > device, it is far better in my opinion to have the entire driver > located together rather than splitting it up into parts so that each > part lives with it's subsystem. We've got tools for find users of > interfaces, whereas spliting a driver up can make maintenance a lot > more complicated. Yeah I already gave up on this in some other thread :D Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] USB: ftdi_sio: add GPIO support
On Sat, Jul 4, 2015 at 12:13 AM, Grant Likely grant.lik...@secretlab.ca wrote: On Tue, Jun 2, 2015 at 2:18 PM, Linus Walleij linus.wall...@linaro.org wrote: On Sat, May 30, 2015 at 10:29 PM, Grant Likely grant.lik...@secretlab.ca wrote: On Mon, Jul 7, 2014 at 6:31 PM, Greg Kroah-Hartman gre...@linuxfoundation.org wrote: However is the MFD cell approach acceptable? Yes it is. Going back to this old conversation... Actually, I disagree. There is absolutely no need to go the MFD approach for this driver. That just adds layers of abstraction for no purpose. GPIOLIB is an interface, and it is completely fine for a driver to hook up to the GPIOLIB interface at the same time as exposing a serial port. MFD doesn't buy the driver anything useful here. What is buys is centralizing code into the proper drivers/gpio folder of the kernel. So more of a maintenance point than a mechanics/performance point. We do have GPIO drivers scattered all over the kernel so one more or less wouldn't matter so much... Yeah, I would say that's a non-reason. When it comes to a single device, it is far better in my opinion to have the entire driver located together rather than splitting it up into parts so that each part lives with it's subsystem. We've got tools for find users of interfaces, whereas spliting a driver up can make maintenance a lot more complicated. Yeah I already gave up on this in some other thread :D Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] USB: ftdi_sio: add GPIO support
On Tue, Jun 2, 2015 at 2:18 PM, Linus Walleij wrote: > On Sat, May 30, 2015 at 10:29 PM, Grant Likely > wrote: >> On Mon, Jul 7, 2014 at 6:31 PM, Greg Kroah-Hartman >> wrote: > However is the MFD cell approach acceptable? >>> >>> Yes it is. >> >> Going back to this old conversation... Actually, I disagree. There is >> absolutely no need to go the MFD approach for this driver. That just >> adds layers of abstraction for no purpose. GPIOLIB is an interface, >> and it is completely fine for a driver to hook up to the GPIOLIB >> interface at the same time as exposing a serial port. MFD doesn't buy >> the driver anything useful here. > > What is buys is centralizing code into the proper drivers/gpio > folder of the kernel. So more of a maintenance point than a > mechanics/performance point. > > We do have GPIO drivers scattered all over the kernel so one > more or less wouldn't matter so much... Yeah, I would say that's a non-reason. When it comes to a single device, it is far better in my opinion to have the entire driver located together rather than splitting it up into parts so that each part lives with it's subsystem. We've got tools for find users of interfaces, whereas spliting a driver up can make maintenance a lot more complicated. g. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] USB: ftdi_sio: add GPIO support
On Tue, Jun 2, 2015 at 2:18 PM, Linus Walleij linus.wall...@linaro.org wrote: On Sat, May 30, 2015 at 10:29 PM, Grant Likely grant.lik...@secretlab.ca wrote: On Mon, Jul 7, 2014 at 6:31 PM, Greg Kroah-Hartman gre...@linuxfoundation.org wrote: However is the MFD cell approach acceptable? Yes it is. Going back to this old conversation... Actually, I disagree. There is absolutely no need to go the MFD approach for this driver. That just adds layers of abstraction for no purpose. GPIOLIB is an interface, and it is completely fine for a driver to hook up to the GPIOLIB interface at the same time as exposing a serial port. MFD doesn't buy the driver anything useful here. What is buys is centralizing code into the proper drivers/gpio folder of the kernel. So more of a maintenance point than a mechanics/performance point. We do have GPIO drivers scattered all over the kernel so one more or less wouldn't matter so much... Yeah, I would say that's a non-reason. When it comes to a single device, it is far better in my opinion to have the entire driver located together rather than splitting it up into parts so that each part lives with it's subsystem. We've got tools for find users of interfaces, whereas spliting a driver up can make maintenance a lot more complicated. g. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] USB: ftdi_sio: add GPIO support
On Sat, May 30, 2015 at 10:29 PM, Grant Likely wrote: > On Mon, Jul 7, 2014 at 6:31 PM, Greg Kroah-Hartman > wrote: >>> However is the MFD cell approach acceptable? >> >> Yes it is. > > Going back to this old conversation... Actually, I disagree. There is > absolutely no need to go the MFD approach for this driver. That just > adds layers of abstraction for no purpose. GPIOLIB is an interface, > and it is completely fine for a driver to hook up to the GPIOLIB > interface at the same time as exposing a serial port. MFD doesn't buy > the driver anything useful here. What is buys is centralizing code into the proper drivers/gpio folder of the kernel. So more of a maintenance point than a mechanics/performance point. We do have GPIO drivers scattered all over the kernel so one more or less wouldn't matter so much... Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] USB: ftdi_sio: add GPIO support
On Sat, May 30, 2015 at 10:29 PM, Grant Likely grant.lik...@secretlab.ca wrote: On Mon, Jul 7, 2014 at 6:31 PM, Greg Kroah-Hartman gre...@linuxfoundation.org wrote: However is the MFD cell approach acceptable? Yes it is. Going back to this old conversation... Actually, I disagree. There is absolutely no need to go the MFD approach for this driver. That just adds layers of abstraction for no purpose. GPIOLIB is an interface, and it is completely fine for a driver to hook up to the GPIOLIB interface at the same time as exposing a serial port. MFD doesn't buy the driver anything useful here. What is buys is centralizing code into the proper drivers/gpio folder of the kernel. So more of a maintenance point than a mechanics/performance point. We do have GPIO drivers scattered all over the kernel so one more or less wouldn't matter so much... Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] USB: ftdi_sio: add GPIO support
On Mon, Jul 7, 2014 at 6:31 PM, Greg Kroah-Hartman wrote: > On Mon, Jul 07, 2014 at 12:44:28PM +0200, Linus Walleij wrote: >> On Fri, Jun 13, 2014 at 8:31 PM, Greg Kroah-Hartman >> wrote: >> > On Fri, Jun 13, 2014 at 09:25:07AM +0200, Linus Walleij wrote: >> >> >> But I also want to bring the device model into question: normally >> >> when a mother device spawns children across different subsystems >> >> we model them as MFD devices (drivers/mfd) that instantiate >> >> children for the different subsystems. So you could spawn a >> >> serial and a GPIO device from a USB-based hub device there. >> >> >> >> I do not know if that is really apropriate in this case. It seems the >> >> device is first and foremost FTDI. >> >> >> >> But it could still spawn a child platform device for the GPIO stuff >> >> so that this can live as a separate driver under drivers/gpio/gpio-ftdi.c >> >> or similar. >> >> >> >> You could then use something like: >> >> >> >> struct platform_device *gdev; >> > >> > Ick, no, it's a USB device, do not abuse the platform_device code any >> > more than it currently is (note, I HATE the platform device code, >> > someday I'll delete it entirely... Well, I can dream...) >> >> Haha yeah :-) >> >> However is the MFD cell approach acceptable? > > Yes it is. Going back to this old conversation... Actually, I disagree. There is absolutely no need to go the MFD approach for this driver. That just adds layers of abstraction for no purpose. GPIOLIB is an interface, and it is completely fine for a driver to hook up to the GPIOLIB interface at the same time as exposing a serial port. MFD doesn't buy the driver anything useful here. g. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] USB: ftdi_sio: add GPIO support
On Mon, Jul 7, 2014 at 6:31 PM, Greg Kroah-Hartman gre...@linuxfoundation.org wrote: On Mon, Jul 07, 2014 at 12:44:28PM +0200, Linus Walleij wrote: On Fri, Jun 13, 2014 at 8:31 PM, Greg Kroah-Hartman gre...@linuxfoundation.org wrote: On Fri, Jun 13, 2014 at 09:25:07AM +0200, Linus Walleij wrote: But I also want to bring the device model into question: normally when a mother device spawns children across different subsystems we model them as MFD devices (drivers/mfd) that instantiate children for the different subsystems. So you could spawn a serial and a GPIO device from a USB-based hub device there. I do not know if that is really apropriate in this case. It seems the device is first and foremost FTDI. But it could still spawn a child platform device for the GPIO stuff so that this can live as a separate driver under drivers/gpio/gpio-ftdi.c or similar. You could then use something like: struct platform_device *gdev; Ick, no, it's a USB device, do not abuse the platform_device code any more than it currently is (note, I HATE the platform device code, someday I'll delete it entirely... Well, I can dream...) Haha yeah :-) However is the MFD cell approach acceptable? Yes it is. Going back to this old conversation... Actually, I disagree. There is absolutely no need to go the MFD approach for this driver. That just adds layers of abstraction for no purpose. GPIOLIB is an interface, and it is completely fine for a driver to hook up to the GPIOLIB interface at the same time as exposing a serial port. MFD doesn't buy the driver anything useful here. g. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] USB: ftdi_sio: add GPIO support
On Mon, Jul 07, 2014 at 12:44:28PM +0200, Linus Walleij wrote: > On Fri, Jun 13, 2014 at 8:31 PM, Greg Kroah-Hartman > wrote: > > On Fri, Jun 13, 2014 at 09:25:07AM +0200, Linus Walleij wrote: > > >> But I also want to bring the device model into question: normally > >> when a mother device spawns children across different subsystems > >> we model them as MFD devices (drivers/mfd) that instantiate > >> children for the different subsystems. So you could spawn a > >> serial and a GPIO device from a USB-based hub device there. > >> > >> I do not know if that is really apropriate in this case. It seems the > >> device is first and foremost FTDI. > >> > >> But it could still spawn a child platform device for the GPIO stuff > >> so that this can live as a separate driver under drivers/gpio/gpio-ftdi.c > >> or similar. > >> > >> You could then use something like: > >> > >> struct platform_device *gdev; > > > > Ick, no, it's a USB device, do not abuse the platform_device code any > > more than it currently is (note, I HATE the platform device code, > > someday I'll delete it entirely... Well, I can dream...) > > Haha yeah :-) > > However is the MFD cell approach acceptable? Yes it is. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] USB: ftdi_sio: add GPIO support
On Fri, Jun 13, 2014 at 8:31 PM, Greg Kroah-Hartman wrote: > On Fri, Jun 13, 2014 at 09:25:07AM +0200, Linus Walleij wrote: >> But I also want to bring the device model into question: normally >> when a mother device spawns children across different subsystems >> we model them as MFD devices (drivers/mfd) that instantiate >> children for the different subsystems. So you could spawn a >> serial and a GPIO device from a USB-based hub device there. >> >> I do not know if that is really apropriate in this case. It seems the >> device is first and foremost FTDI. >> >> But it could still spawn a child platform device for the GPIO stuff >> so that this can live as a separate driver under drivers/gpio/gpio-ftdi.c >> or similar. >> >> You could then use something like: >> >> struct platform_device *gdev; > > Ick, no, it's a USB device, do not abuse the platform_device code any > more than it currently is (note, I HATE the platform device code, > someday I'll delete it entirely... Well, I can dream...) Haha yeah :-) However is the MFD cell approach acceptable? Now MFD cells are platform_devices mind you, but it's in principle just how that got implemented, MFD cells could be device-something else. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] USB: ftdi_sio: add GPIO support
On Fri, Jun 13, 2014 at 8:31 PM, Greg Kroah-Hartman gre...@linuxfoundation.org wrote: On Fri, Jun 13, 2014 at 09:25:07AM +0200, Linus Walleij wrote: But I also want to bring the device model into question: normally when a mother device spawns children across different subsystems we model them as MFD devices (drivers/mfd) that instantiate children for the different subsystems. So you could spawn a serial and a GPIO device from a USB-based hub device there. I do not know if that is really apropriate in this case. It seems the device is first and foremost FTDI. But it could still spawn a child platform device for the GPIO stuff so that this can live as a separate driver under drivers/gpio/gpio-ftdi.c or similar. You could then use something like: struct platform_device *gdev; Ick, no, it's a USB device, do not abuse the platform_device code any more than it currently is (note, I HATE the platform device code, someday I'll delete it entirely... Well, I can dream...) Haha yeah :-) However is the MFD cell approach acceptable? Now MFD cells are platform_devices mind you, but it's in principle just how that got implemented, MFD cells could be device-something else. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] USB: ftdi_sio: add GPIO support
On Mon, Jul 07, 2014 at 12:44:28PM +0200, Linus Walleij wrote: On Fri, Jun 13, 2014 at 8:31 PM, Greg Kroah-Hartman gre...@linuxfoundation.org wrote: On Fri, Jun 13, 2014 at 09:25:07AM +0200, Linus Walleij wrote: But I also want to bring the device model into question: normally when a mother device spawns children across different subsystems we model them as MFD devices (drivers/mfd) that instantiate children for the different subsystems. So you could spawn a serial and a GPIO device from a USB-based hub device there. I do not know if that is really apropriate in this case. It seems the device is first and foremost FTDI. But it could still spawn a child platform device for the GPIO stuff so that this can live as a separate driver under drivers/gpio/gpio-ftdi.c or similar. You could then use something like: struct platform_device *gdev; Ick, no, it's a USB device, do not abuse the platform_device code any more than it currently is (note, I HATE the platform device code, someday I'll delete it entirely... Well, I can dream...) Haha yeah :-) However is the MFD cell approach acceptable? Yes it is. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] USB: ftdi_sio: add GPIO support
Hi, Would this patch interfere with adding support for using the CBUS pins as GPIOs while operating in normal UART mode? Most interesting question! Care was taken to prevent using GPIOs if the serial device is in use and vice versa. What about CBUS GPIO support? Ok, so we're not extending the serial driver with support for controlling some unused pins (e.g. CBUS) but rather implementing support for a mutually exclusive mode. For bitbang mode (on the data lines) - yes. But for bitbang on CBUS - no. I have sent a proposal a few weeks ago but Greg did not like special sysfs stuff :-P He asked me to use the GPIO subsystem for the CBUS pins. I am out of time until july. But I still want and need CBUS support in parallel with serial/FIFO operation. Regards Philipp -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] USB: ftdi_sio: add GPIO support
Hi, Would this patch interfere with adding support for using the CBUS pins as GPIOs while operating in normal UART mode? Most interesting question! Care was taken to prevent using GPIOs if the serial device is in use and vice versa. What about CBUS GPIO support? Ok, so we're not extending the serial driver with support for controlling some unused pins (e.g. CBUS) but rather implementing support for a mutually exclusive mode. For bitbang mode (on the data lines) - yes. But for bitbang on CBUS - no. I have sent a proposal a few weeks ago but Greg did not like special sysfs stuff :-P He asked me to use the GPIO subsystem for the CBUS pins. I am out of time until july. But I still want and need CBUS support in parallel with serial/FIFO operation. Regards Philipp -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] USB: ftdi_sio: add GPIO support
On Fri, Jun 13, 2014 at 09:25:07AM +0200, Linus Walleij wrote: > On Mon, Jun 9, 2014 at 3:21 PM, Sascha Silbe wrote: > > > The chips can operate either in regular or in bitbang mode. Care was > > taken to prevent using GPIOs if the serial device is in use and vice > > versa. > > Very interesting patch! I've seen USB-based GPIO things before > but never a dual-mode thing. > > There was already a comment to move the implementation to a > separate file, which I won't repeat. > > But I also want to bring the device model into question: normally > when a mother device spawns children across different subsystems > we model them as MFD devices (drivers/mfd) that instantiate > children for the different subsystems. So you could spawn a > serial and a GPIO device from a USB-based hub device there. > > I do not know if that is really apropriate in this case. It seems the > device is first and foremost FTDI. > > But it could still spawn a child platform device for the GPIO stuff > so that this can live as a separate driver under drivers/gpio/gpio-ftdi.c > or similar. > > You could then use something like: > > struct platform_device *gdev; Ick, no, it's a USB device, do not abuse the platform_device code any more than it currently is (note, I HATE the platform device code, someday I'll delete it entirely... Well, I can dream...) greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] USB: ftdi_sio: add GPIO support
On Mon, Jun 9, 2014 at 3:21 PM, Sascha Silbe wrote:
> The chips can operate either in regular or in bitbang mode. Care was
> taken to prevent using GPIOs if the serial device is in use and vice
> versa.
Very interesting patch! I've seen USB-based GPIO things before
but never a dual-mode thing.
There was already a comment to move the implementation to a
separate file, which I won't repeat.
But I also want to bring the device model into question: normally
when a mother device spawns children across different subsystems
we model them as MFD devices (drivers/mfd) that instantiate
children for the different subsystems. So you could spawn a
serial and a GPIO device from a USB-based hub device there.
I do not know if that is really apropriate in this case. It seems the
device is first and foremost FTDI.
But it could still spawn a child platform device for the GPIO stuff
so that this can live as a separate driver under drivers/gpio/gpio-ftdi.c
or similar.
You could then use something like:
struct platform_device *gdev;
gdev = platform_device_alloc("gpio-ftdi", PLATFORM_DEVID_AUTO);
/* pdata contains communication cookie for callbacks etc */
ret = platform_device_add_data(gdev, pdata, sizeof(*pdata));
ret = platform_device_add(gdev);
Then we can probe that device normally in the GPIO subsystem
like any other driver, just that it needs some
header or similar to define the function
calls to communicate with the FTDI driver.
However Greg is device core maintainer and may have better
ideas about this!
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] USB: ftdi_sio: add GPIO support
On Mon, Jun 9, 2014 at 3:21 PM, Sascha Silbe x-li...@infra-silbe.de wrote: The chips can operate either in regular or in bitbang mode. Care was taken to prevent using GPIOs if the serial device is in use and vice versa. Very interesting patch! I've seen USB-based GPIO things before but never a dual-mode thing. There was already a comment to move the implementation to a separate file, which I won't repeat. But I also want to bring the device model into question: normally when a mother device spawns children across different subsystems we model them as MFD devices (drivers/mfd) that instantiate children for the different subsystems. So you could spawn a serial and a GPIO device from a USB-based hub device there. I do not know if that is really apropriate in this case. It seems the device is first and foremost FTDI. But it could still spawn a child platform device for the GPIO stuff so that this can live as a separate driver under drivers/gpio/gpio-ftdi.c or similar. You could then use something like: struct platform_device *gdev; gdev = platform_device_alloc(gpio-ftdi, PLATFORM_DEVID_AUTO); /* pdata contains communication cookie for callbacks etc */ ret = platform_device_add_data(gdev, pdata, sizeof(*pdata)); ret = platform_device_add(gdev); Then we can probe that device normally in the GPIO subsystem like any other driver, just that it needs some linux/usb/ftdi.h header or similar to define the function calls to communicate with the FTDI driver. However Greg is device core maintainer and may have better ideas about this! Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] USB: ftdi_sio: add GPIO support
On Fri, Jun 13, 2014 at 09:25:07AM +0200, Linus Walleij wrote: On Mon, Jun 9, 2014 at 3:21 PM, Sascha Silbe x-li...@infra-silbe.de wrote: The chips can operate either in regular or in bitbang mode. Care was taken to prevent using GPIOs if the serial device is in use and vice versa. Very interesting patch! I've seen USB-based GPIO things before but never a dual-mode thing. There was already a comment to move the implementation to a separate file, which I won't repeat. But I also want to bring the device model into question: normally when a mother device spawns children across different subsystems we model them as MFD devices (drivers/mfd) that instantiate children for the different subsystems. So you could spawn a serial and a GPIO device from a USB-based hub device there. I do not know if that is really apropriate in this case. It seems the device is first and foremost FTDI. But it could still spawn a child platform device for the GPIO stuff so that this can live as a separate driver under drivers/gpio/gpio-ftdi.c or similar. You could then use something like: struct platform_device *gdev; Ick, no, it's a USB device, do not abuse the platform_device code any more than it currently is (note, I HATE the platform device code, someday I'll delete it entirely... Well, I can dream...) greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] USB: ftdi_sio: add GPIO support
On Mon, Jun 09, 2014 at 03:21:55PM +0200, Sascha Silbe wrote:
> Most FTDI USB serial / parallel adapter chips support an asynchronous
> "bitbang" mode. Make this functionality available as a GPIO controller
> so they can be used with the Linux GPIO API instead of requiring
> special user space software that only works with this particular
> family of chips.
Finally a patch that implements this using gpiolib rather than driver-
specific ioctl or sysfs-entries. :)
> The chips can operate either in regular or in bitbang mode.
Ok, so we're not extending the serial driver with support for
controlling some unused pins (e.g. CBUS) but rather implementing support
for a mutually exclusive mode.
Given that these pins will always be configured as regular UART pins at
power-on and what you mention below about toggling, I'm a bit sceptic
about how useful this device will be as a gpio controller. I also
understand there's already support for this in libftdi?
Would this patch interfere with adding support for using the CBUS pins
as GPIOs while operating in normal UART mode?
> Care was taken to prevent using GPIOs if the serial device is in use
> and vice versa.
There are still some issues related to this that need to be addressed,
see comments below.
> At least the FT245RL will toggle several of its pins multiple times
> during initialisation (even if connected to a dumb USB power supply,
> so not a driver issue). Using synchronous bitbang mode might help
> coping with this if the hardware connected to the FTDI chip supports
> it. However, as synchronous mode isn't supported by the hardware used
> for testing (SainSmart 8-channel USB relay card) and is not as
> straightforward to use (reading the inputs requires the outputs to be
> written first) it's left as a potential future enhancement.
>
> Serial mode and GPIO mode were tested on FT232RL using a simple
> loopback setup (TXD connected to RXD). In addition, GPIO output mode
> was tested on FT245RL (8 channels).
>
> Signed-off-by: Sascha Silbe
> ---
> Is making the GPIO support conditional on CONFIG_GPIOLIB enough or
> should I introduce a new configuration option USB_SERIAL_FTDI_SIO_GPIO
> that depends on CONFIG_GPIOLIB?
A new option seems appropriate.
> Signed-off-by: Sascha Silbe
Second Signed-off-by?
> ---
> drivers/usb/serial/ftdi_sio.c | 264
> +-
> drivers/usb/serial/ftdi_sio.h | 26 +
> 2 files changed, 289 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
> index edf3b12..92f5c14 100644
> --- a/drivers/usb/serial/ftdi_sio.c
> +++ b/drivers/usb/serial/ftdi_sio.c
> @@ -33,6 +33,9 @@
>
> #include
> #include
> +#ifdef CONFIG_GPIOLIB
> +#include
> +#endif
> #include
> #include
> #include
> @@ -53,6 +56,7 @@
>
> struct ftdi_private {
> enum ftdi_chip_type chip_type;
> + struct usb_serial_port *port;
> /* type of device, either SIO or FT8U232AM */
> int baud_base; /* baud base clock for divisor setting */
> int custom_divisor; /* custom_divisor kludge, this is for
> @@ -77,6 +81,15 @@ struct ftdi_private {
> unsigned int latency; /* latency setting in use */
> unsigned short max_packet_size;
> struct mutex cfg_lock; /* Avoid mess by parallel calls of config
> ioctl() and change_speed() */
> +#ifdef CONFIG_GPIOLIB
> + struct gpio_chip gc;
> + bool serial_open;
> + unsigned char open_gpios;
> + unsigned int bitmode;
> + unsigned char gpio_direction; /* data direction in bitbang
> +* mode, 0=in / 1=out */
Can you shorten the comment to just the essential /* 0=in / 1=out */?
> + unsigned char gpo_values; /* GPIO output values */
> +#endif
> };
>
> /* struct ftdi_sio_quirk is used by devices requiring special attention. */
> @@ -973,6 +986,7 @@ static int ftdi_sio_probe(struct usb_serial *serial,
> static int ftdi_sio_port_probe(struct usb_serial_port *port);
> static int ftdi_sio_port_remove(struct usb_serial_port *port);
> static int ftdi_open(struct tty_struct *tty, struct usb_serial_port *port);
> +static void ftdi_close(struct usb_serial_port *port);
> static void ftdi_dtr_rts(struct usb_serial_port *port, int on);
> static void ftdi_process_read_urb(struct urb *urb);
> static int ftdi_prepare_write_buffer(struct usb_serial_port *port,
> @@ -996,6 +1010,15 @@ static __u32 ftdi_232bm_baud_to_divisor(int baud);
> static __u32 ftdi_2232h_baud_base_to_divisor(int baud, int base);
> static __u32 ftdi_2232h_baud_to_divisor(int baud);
>
> +#ifdef CONFIG_GPIOLIB
> +static int ftdi_gpio_get(struct gpio_chip *gc, unsigned int gpio);
> +static void ftdi_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val);
> +static int ftdi_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio);
> +static int ftdi_gpio_dir_out(struct gpio_chip *gc, unsigned int gpio, int
> val);
>
Re: [PATCH] USB: ftdi_sio: add GPIO support
On Mon, Jun 09, 2014 at 03:21:55PM +0200, Sascha Silbe wrote:
Most FTDI USB serial / parallel adapter chips support an asynchronous
bitbang mode. Make this functionality available as a GPIO controller
so they can be used with the Linux GPIO API instead of requiring
special user space software that only works with this particular
family of chips.
Finally a patch that implements this using gpiolib rather than driver-
specific ioctl or sysfs-entries. :)
The chips can operate either in regular or in bitbang mode.
Ok, so we're not extending the serial driver with support for
controlling some unused pins (e.g. CBUS) but rather implementing support
for a mutually exclusive mode.
Given that these pins will always be configured as regular UART pins at
power-on and what you mention below about toggling, I'm a bit sceptic
about how useful this device will be as a gpio controller. I also
understand there's already support for this in libftdi?
Would this patch interfere with adding support for using the CBUS pins
as GPIOs while operating in normal UART mode?
Care was taken to prevent using GPIOs if the serial device is in use
and vice versa.
There are still some issues related to this that need to be addressed,
see comments below.
At least the FT245RL will toggle several of its pins multiple times
during initialisation (even if connected to a dumb USB power supply,
so not a driver issue). Using synchronous bitbang mode might help
coping with this if the hardware connected to the FTDI chip supports
it. However, as synchronous mode isn't supported by the hardware used
for testing (SainSmart 8-channel USB relay card) and is not as
straightforward to use (reading the inputs requires the outputs to be
written first) it's left as a potential future enhancement.
Serial mode and GPIO mode were tested on FT232RL using a simple
loopback setup (TXD connected to RXD). In addition, GPIO output mode
was tested on FT245RL (8 channels).
Signed-off-by: Sascha Silbe x-li...@infra-silbe.de
---
Is making the GPIO support conditional on CONFIG_GPIOLIB enough or
should I introduce a new configuration option USB_SERIAL_FTDI_SIO_GPIO
that depends on CONFIG_GPIOLIB?
A new option seems appropriate.
Signed-off-by: Sascha Silbe x-li...@infra-silbe.de
Second Signed-off-by?
---
drivers/usb/serial/ftdi_sio.c | 264
+-
drivers/usb/serial/ftdi_sio.h | 26 +
2 files changed, 289 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index edf3b12..92f5c14 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -33,6 +33,9 @@
#include linux/kernel.h
#include linux/errno.h
+#ifdef CONFIG_GPIOLIB
+#include linux/gpio.h
+#endif
#include linux/slab.h
#include linux/tty.h
#include linux/tty_driver.h
@@ -53,6 +56,7 @@
struct ftdi_private {
enum ftdi_chip_type chip_type;
+ struct usb_serial_port *port;
/* type of device, either SIO or FT8U232AM */
int baud_base; /* baud base clock for divisor setting */
int custom_divisor; /* custom_divisor kludge, this is for
@@ -77,6 +81,15 @@ struct ftdi_private {
unsigned int latency; /* latency setting in use */
unsigned short max_packet_size;
struct mutex cfg_lock; /* Avoid mess by parallel calls of config
ioctl() and change_speed() */
+#ifdef CONFIG_GPIOLIB
+ struct gpio_chip gc;
+ bool serial_open;
+ unsigned char open_gpios;
+ unsigned int bitmode;
+ unsigned char gpio_direction; /* data direction in bitbang
+* mode, 0=in / 1=out */
Can you shorten the comment to just the essential /* 0=in / 1=out */?
+ unsigned char gpo_values; /* GPIO output values */
+#endif
};
/* struct ftdi_sio_quirk is used by devices requiring special attention. */
@@ -973,6 +986,7 @@ static int ftdi_sio_probe(struct usb_serial *serial,
static int ftdi_sio_port_probe(struct usb_serial_port *port);
static int ftdi_sio_port_remove(struct usb_serial_port *port);
static int ftdi_open(struct tty_struct *tty, struct usb_serial_port *port);
+static void ftdi_close(struct usb_serial_port *port);
static void ftdi_dtr_rts(struct usb_serial_port *port, int on);
static void ftdi_process_read_urb(struct urb *urb);
static int ftdi_prepare_write_buffer(struct usb_serial_port *port,
@@ -996,6 +1010,15 @@ static __u32 ftdi_232bm_baud_to_divisor(int baud);
static __u32 ftdi_2232h_baud_base_to_divisor(int baud, int base);
static __u32 ftdi_2232h_baud_to_divisor(int baud);
+#ifdef CONFIG_GPIOLIB
+static int ftdi_gpio_get(struct gpio_chip *gc, unsigned int gpio);
+static void ftdi_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val);
+static int ftdi_gpio_dir_in(struct gpio_chip *gc, unsigned int gpio);
+static int ftdi_gpio_dir_out(struct gpio_chip *gc,
Re: [PATCH] USB: ftdi_sio: add GPIO support
Hello.
On 06/09/2014 06:23 PM, One Thousand Gnomes wrote:
#include
#include
+#ifdef CONFIG_GPIOLIB
+#include
+#endif
Please create a new struct, a new file and put all the GPIO stuff in
there rather than #if bombing the driver.
You can then declare blank methods for the gpio stuff if GPIO is not
compiled in - ie in the headers
#ifdef CONFIG_GPIOLIB
extern int ftdi_gpio_open(struct ftdi_private *priv);
etc...
#else
extern inline int ftdi_gpio_open(struct ftdi_private *priv) { return 0 };
I guess you meant *static* instead of *extern* here?
#endif
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] USB: ftdi_sio: add GPIO support
> #include
> #include
> +#ifdef CONFIG_GPIOLIB
> +#include
> +#endif
Please create a new struct, a new file and put all the GPIO stuff in
there rather than #if bombing the driver.
You can then declare blank methods for the gpio stuff if GPIO is not
compiled in - ie in the headers
#ifdef CONFIG_GPIOLIB
extern int ftdi_gpio_open(struct ftdi_private *priv);
etc...
#else
extern inline int ftdi_gpio_open(struct ftdi_private *priv) { return 0 };
#endif
that keeps the code itself clean and easy to read and also ensures all
the types are checked everywhere we want.
Functionality wise nothing stands out as a problem. I would expect -EBUSY
not -ENXIO if the port was being used for GPIO so could not be used for
tty.
I would favour the new config option.
Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] USB: ftdi_sio: add GPIO support
#include linux/kernel.h
#include linux/errno.h
+#ifdef CONFIG_GPIOLIB
+#include linux/gpio.h
+#endif
Please create a new struct, a new file and put all the GPIO stuff in
there rather than #if bombing the driver.
You can then declare blank methods for the gpio stuff if GPIO is not
compiled in - ie in the headers
#ifdef CONFIG_GPIOLIB
extern int ftdi_gpio_open(struct ftdi_private *priv);
etc...
#else
extern inline int ftdi_gpio_open(struct ftdi_private *priv) { return 0 };
#endif
that keeps the code itself clean and easy to read and also ensures all
the types are checked everywhere we want.
Functionality wise nothing stands out as a problem. I would expect -EBUSY
not -ENXIO if the port was being used for GPIO so could not be used for
tty.
I would favour the new config option.
Alan
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] USB: ftdi_sio: add GPIO support
Hello.
On 06/09/2014 06:23 PM, One Thousand Gnomes wrote:
#include linux/kernel.h
#include linux/errno.h
+#ifdef CONFIG_GPIOLIB
+#include linux/gpio.h
+#endif
Please create a new struct, a new file and put all the GPIO stuff in
there rather than #if bombing the driver.
You can then declare blank methods for the gpio stuff if GPIO is not
compiled in - ie in the headers
#ifdef CONFIG_GPIOLIB
extern int ftdi_gpio_open(struct ftdi_private *priv);
etc...
#else
extern inline int ftdi_gpio_open(struct ftdi_private *priv) { return 0 };
I guess you meant *static* instead of *extern* here?
#endif
WBR, Sergei
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
