| Message ID | 4b0bcf4679163c0da53e412a47eecd4bb03849b8.1413966148.git.jiri.prchal@aksignal.cz |
|---|---|
| State | Rejected |
| Headers | show |
On Wed, Oct 22, 2014 at 5:26 PM, Jiri Prchal <jiri.prchal@aksignal.cz> wrote: > This patch adds new driver "gpio-of-helper", witch has possibility to export > gpios defined in dt. It exports them in defined name under > /sysfs/class/gpio/name. > It's rebased from Pantelis Antoniou patch to new kernel. > Usage example: > gpio { > compatible = "gpio-of-helper"; > status = "okay"; > pinctrl-names = "default"; > pinctrl-0 = <&pinctrl_gpio>; > > gsm_on { > gpio-name = "gsm_on"; > gpio = <&pioB 13 GPIO_ACTIVE_HIGH>; > output; > init-low; > }; > }; > > This patch needs Alexey Ignatov [PATCH] gpiolib: allow exporting gpios with > custom names. We will need to see whether the pre-requisite patch can get merged first, but there are a couple of things that are wrong with your patch anyway: - it is missing a bindings documentation - it is using the legacy integer GPIOs instead of the descriptor interface (see include/linux/gpio/consumer.h). Since this is DT-based, there is absolutely no reason to not use the descriptors interface. - it seems quite long for what it needs to do - the MODULE_AUTHOR has not signed-off this patch (?) But what makes me nervous is that this encourages more usage of the sysfs interface, an in a way that is potentially harmful. Also, I don't know if the DT people will be happy with this idea. Since this concerns DT, please also add the devicetree list and get a Acked-by for the bindings you want to push by a DT maintainer. -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Alexandre, > On Oct 22, 2014, at 12:18 PM, Alexandre Courbot <gnurou@gmail.com> wrote: > > On Wed, Oct 22, 2014 at 5:26 PM, Jiri Prchal <jiri.prchal@aksignal.cz> wrote: >> This patch adds new driver "gpio-of-helper", witch has possibility to export >> gpios defined in dt. It exports them in defined name under >> /sysfs/class/gpio/name. >> It's rebased from Pantelis Antoniou patch to new kernel. >> Usage example: >> gpio { >> compatible = "gpio-of-helper"; >> status = "okay"; >> pinctrl-names = "default"; >> pinctrl-0 = <&pinctrl_gpio>; >> >> gsm_on { >> gpio-name = "gsm_on"; >> gpio = <&pioB 13 GPIO_ACTIVE_HIGH>; >> output; >> init-low; >> }; >> }; >> >> This patch needs Alexey Ignatov [PATCH] gpiolib: allow exporting gpios with >> custom names. > > We will need to see whether the pre-requisite patch can get merged > first, but there are a couple of things that are wrong with your patch > anyway: > > - it is missing a bindings documentation > - it is using the legacy integer GPIOs instead of the descriptor > interface (see include/linux/gpio/consumer.h). Since this is DT-based, > there is absolutely no reason to not use the descriptors interface. > - it seems quite long for what it needs to do > - the MODULE_AUTHOR has not signed-off this patch (?) > > But what makes me nervous is that this encourages more usage of the > sysfs interface, an in a way that is potentially harmful. > > Also, I don't know if the DT people will be happy with this idea. > Since this concerns DT, please also add the devicetree list and get a > Acked-by for the bindings you want to push by a DT maintainer. Since I’m the original perpetrator, let me put a few words here for posterity. This patch was meant as a quick and dirty method for doing the automatic export and pinmux configuration from DT on a 3.8 kernel. The kernel APIs have moved on since. It wasn’t meant to be submitted for mainline right as it is. Unfortunately (or fortunately for many people) it does what a lot of people need i.e. configures a board with heavily pinmuxed GPIO right at boot time. I’m open at having a small discussion about how to do what the users of this patch do ‘right’. Regards — Pantelis -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Oct 22, 2014 at 6:24 PM, Pantelis Antoniou <panto@antoniou-consulting.com> wrote: > Hi Alexandre, > >> On Oct 22, 2014, at 12:18 PM, Alexandre Courbot <gnurou@gmail.com> wrote: >> >> On Wed, Oct 22, 2014 at 5:26 PM, Jiri Prchal <jiri.prchal@aksignal.cz> wrote: >>> This patch adds new driver "gpio-of-helper", witch has possibility to export >>> gpios defined in dt. It exports them in defined name under >>> /sysfs/class/gpio/name. >>> It's rebased from Pantelis Antoniou patch to new kernel. >>> Usage example: >>> gpio { >>> compatible = "gpio-of-helper"; >>> status = "okay"; >>> pinctrl-names = "default"; >>> pinctrl-0 = <&pinctrl_gpio>; >>> >>> gsm_on { >>> gpio-name = "gsm_on"; >>> gpio = <&pioB 13 GPIO_ACTIVE_HIGH>; >>> output; >>> init-low; >>> }; >>> }; >>> >>> This patch needs Alexey Ignatov [PATCH] gpiolib: allow exporting gpios with >>> custom names. >> >> We will need to see whether the pre-requisite patch can get merged >> first, but there are a couple of things that are wrong with your patch >> anyway: >> >> - it is missing a bindings documentation >> - it is using the legacy integer GPIOs instead of the descriptor >> interface (see include/linux/gpio/consumer.h). Since this is DT-based, >> there is absolutely no reason to not use the descriptors interface. >> - it seems quite long for what it needs to do >> - the MODULE_AUTHOR has not signed-off this patch (?) >> >> But what makes me nervous is that this encourages more usage of the >> sysfs interface, an in a way that is potentially harmful. >> >> Also, I don't know if the DT people will be happy with this idea. >> Since this concerns DT, please also add the devicetree list and get a >> Acked-by for the bindings you want to push by a DT maintainer. > > Since I’m the original perpetrator, let me put a few words here for posterity. > > This patch was meant as a quick and dirty method for doing the automatic export > and pinmux configuration from DT on a 3.8 kernel. The kernel APIs have moved on > since. It wasn’t meant to be submitted for mainline right as it is. > > Unfortunately (or fortunately for many people) it does what a lot of people need > i.e. configures a board with heavily pinmuxed GPIO right at boot time. > > I’m open at having a small discussion about how to do what the users of this patch > do ‘right’. Sure, although the discussion might turn out to not be that "small". :) Pinmux configuration sounds like a job for pinmux more than GPIO, and exporting potentially many GPIOs to user-space sounds like a work for a proper driver. We need to understand why this is needed, and then see if the DT bindings are acceptable before going any further. -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Alexandre, > On Oct 22, 2014, at 12:41 PM, Alexandre Courbot <gnurou@gmail.com> wrote: > > On Wed, Oct 22, 2014 at 6:24 PM, Pantelis Antoniou > <panto@antoniou-consulting.com> wrote: >> Hi Alexandre, >> >>> On Oct 22, 2014, at 12:18 PM, Alexandre Courbot <gnurou@gmail.com> wrote: >>> >>> On Wed, Oct 22, 2014 at 5:26 PM, Jiri Prchal <jiri.prchal@aksignal.cz> wrote: >>>> This patch adds new driver "gpio-of-helper", witch has possibility to export >>>> gpios defined in dt. It exports them in defined name under >>>> /sysfs/class/gpio/name. >>>> It's rebased from Pantelis Antoniou patch to new kernel. >>>> Usage example: >>>> gpio { >>>> compatible = "gpio-of-helper"; >>>> status = "okay"; >>>> pinctrl-names = "default"; >>>> pinctrl-0 = <&pinctrl_gpio>; >>>> >>>> gsm_on { >>>> gpio-name = "gsm_on"; >>>> gpio = <&pioB 13 GPIO_ACTIVE_HIGH>; >>>> output; >>>> init-low; >>>> }; >>>> }; >>>> >>>> This patch needs Alexey Ignatov [PATCH] gpiolib: allow exporting gpios with >>>> custom names. >>> >>> We will need to see whether the pre-requisite patch can get merged >>> first, but there are a couple of things that are wrong with your patch >>> anyway: >>> >>> - it is missing a bindings documentation >>> - it is using the legacy integer GPIOs instead of the descriptor >>> interface (see include/linux/gpio/consumer.h). Since this is DT-based, >>> there is absolutely no reason to not use the descriptors interface. >>> - it seems quite long for what it needs to do >>> - the MODULE_AUTHOR has not signed-off this patch (?) >>> >>> But what makes me nervous is that this encourages more usage of the >>> sysfs interface, an in a way that is potentially harmful. >>> >>> Also, I don't know if the DT people will be happy with this idea. >>> Since this concerns DT, please also add the devicetree list and get a >>> Acked-by for the bindings you want to push by a DT maintainer. >> >> Since I’m the original perpetrator, let me put a few words here for posterity. >> >> This patch was meant as a quick and dirty method for doing the automatic export >> and pinmux configuration from DT on a 3.8 kernel. The kernel APIs have moved on >> since. It wasn’t meant to be submitted for mainline right as it is. >> >> Unfortunately (or fortunately for many people) it does what a lot of people need >> i.e. configures a board with heavily pinmuxed GPIO right at boot time. >> >> I’m open at having a small discussion about how to do what the users of this patch >> do ‘right’. > > Sure, although the discussion might turn out to not be that "small". :) > Heh ;) > Pinmux configuration sounds like a job for pinmux more than GPIO, and > exporting potentially many GPIOs to user-space sounds like a work for > a proper driver. > I’m afraid that’s not the case. A great many users do not require anything more than setting a pinmux and the GPIO configuration (input/output). They can then do low speed I/O using the sysfs interface, without having to use any complex APIs (shell works just fine). Think of stuff like controlling a sprinkler valve, or something like a mechanical door detection open state. > We need to understand why this is needed, and then see if the DT > bindings are acceptable before going any further. I can take care of the bindings if we get into some kind of agreement on the basic interface. I’m CCing device tree and the DT maintainer. Regards — Pantelis -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi all, Dne 22.10.2014 v 11:58 Pantelis Antoniou napsal(a): > Hi Alexandre, > >> On Oct 22, 2014, at 12:41 PM, Alexandre Courbot <gnurou@gmail.com> wrote: >> >> On Wed, Oct 22, 2014 at 6:24 PM, Pantelis Antoniou >> <panto@antoniou-consulting.com> wrote: >>> Hi Alexandre, >>> >>>> On Oct 22, 2014, at 12:18 PM, Alexandre Courbot <gnurou@gmail.com> wrote: >>>> >>>> On Wed, Oct 22, 2014 at 5:26 PM, Jiri Prchal <jiri.prchal@aksignal.cz> wrote: >>>>> This patch adds new driver "gpio-of-helper", witch has possibility to export >>>>> gpios defined in dt. It exports them in defined name under >>>>> /sysfs/class/gpio/name. >>>>> It's rebased from Pantelis Antoniou patch to new kernel. >>>>> Usage example: >>>>> gpio { >>>>> compatible = "gpio-of-helper"; >>>>> status = "okay"; >>>>> pinctrl-names = "default"; >>>>> pinctrl-0 = <&pinctrl_gpio>; >>>>> >>>>> gsm_on { >>>>> gpio-name = "gsm_on"; >>>>> gpio = <&pioB 13 GPIO_ACTIVE_HIGH>; >>>>> output; >>>>> init-low; >>>>> }; >>>>> }; >>>>> >>>>> This patch needs Alexey Ignatov [PATCH] gpiolib: allow exporting gpios with >>>>> custom names. >>>> >>>> We will need to see whether the pre-requisite patch can get merged >>>> first, but there are a couple of things that are wrong with your patch >>>> anyway: Let's continue discussion that "someone" need gpio_export_name. >>>> >>>> - it is missing a bindings documentation >>>> - it is using the legacy integer GPIOs instead of the descriptor >>>> interface (see include/linux/gpio/consumer.h). Since this is DT-based, >>>> there is absolutely no reason to not use the descriptors interface. >>>> - it seems quite long for what it needs to do >>>> - the MODULE_AUTHOR has not signed-off this patch (?) OK, this was just first try mainly to open discussion. >>>> >>>> But what makes me nervous is that this encourages more usage of the >>>> sysfs interface, an in a way that is potentially harmful. >>>> >>>> Also, I don't know if the DT people will be happy with this idea. >>>> Since this concerns DT, please also add the devicetree list and get a >>>> Acked-by for the bindings you want to push by a DT maintainer. >>> >>> Since I’m the original perpetrator, let me put a few words here for posterity. >>> >>> This patch was meant as a quick and dirty method for doing the automatic export >>> and pinmux configuration from DT on a 3.8 kernel. The kernel APIs have moved on >>> since. It wasn’t meant to be submitted for mainline right as it is. Dirty, but is functional. I'm using something like this from 2.6.38! Than I switched to 3.8, devicetree, find and apply this patch, but for other reasons moved to newer kernels and every time need to rebase this. >>> >>> Unfortunately (or fortunately for many people) it does what a lot of people need >>> i.e. configures a board with heavily pinmuxed GPIO right at boot time. >>> >>> I’m open at having a small discussion about how to do what the users of this patch >>> do ‘right’. >> >> Sure, although the discussion might turn out to not be that "small". :) >> > > Heh ;) > >> Pinmux configuration sounds like a job for pinmux more than GPIO, and >> exporting potentially many GPIOs to user-space sounds like a work for >> a proper driver. >> > > I’m afraid that’s not the case. A great many users do not require anything > more than setting a pinmux and the GPIO configuration (input/output). > They can then do low speed I/O using the sysfs interface, without having to > use any complex APIs (shell works just fine). That is exactly what we need. > > Think of stuff like controlling a sprinkler valve, or something like a mechanical > door detection open state. > >> We need to understand why this is needed, and then see if the DT >> bindings are acceptable before going any further. > > I can take care of the bindings if we get into some kind of agreement on the basic > interface. I’m CCing device tree and the DT maintainer. > > Regards > > — Pantelis > > -- > To unsubscribe from this list: send the line "unsubscribe linux-gpio" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Oct 22, 2014 at 6:58 PM, Pantelis Antoniou <panto@antoniou-consulting.com> wrote: > Hi Alexandre, > >> On Oct 22, 2014, at 12:41 PM, Alexandre Courbot <gnurou@gmail.com> wrote: >> >> On Wed, Oct 22, 2014 at 6:24 PM, Pantelis Antoniou >> <panto@antoniou-consulting.com> wrote: >>> Hi Alexandre, >>> >>>> On Oct 22, 2014, at 12:18 PM, Alexandre Courbot <gnurou@gmail.com> wrote: >>>> >>>> On Wed, Oct 22, 2014 at 5:26 PM, Jiri Prchal <jiri.prchal@aksignal.cz> wrote: >>>>> This patch adds new driver "gpio-of-helper", witch has possibility to export >>>>> gpios defined in dt. It exports them in defined name under >>>>> /sysfs/class/gpio/name. >>>>> It's rebased from Pantelis Antoniou patch to new kernel. >>>>> Usage example: >>>>> gpio { >>>>> compatible = "gpio-of-helper"; >>>>> status = "okay"; >>>>> pinctrl-names = "default"; >>>>> pinctrl-0 = <&pinctrl_gpio>; >>>>> >>>>> gsm_on { >>>>> gpio-name = "gsm_on"; >>>>> gpio = <&pioB 13 GPIO_ACTIVE_HIGH>; >>>>> output; >>>>> init-low; >>>>> }; >>>>> }; >>>>> >>>>> This patch needs Alexey Ignatov [PATCH] gpiolib: allow exporting gpios with >>>>> custom names. >>>> >>>> We will need to see whether the pre-requisite patch can get merged >>>> first, but there are a couple of things that are wrong with your patch >>>> anyway: >>>> >>>> - it is missing a bindings documentation >>>> - it is using the legacy integer GPIOs instead of the descriptor >>>> interface (see include/linux/gpio/consumer.h). Since this is DT-based, >>>> there is absolutely no reason to not use the descriptors interface. >>>> - it seems quite long for what it needs to do >>>> - the MODULE_AUTHOR has not signed-off this patch (?) >>>> >>>> But what makes me nervous is that this encourages more usage of the >>>> sysfs interface, an in a way that is potentially harmful. >>>> >>>> Also, I don't know if the DT people will be happy with this idea. >>>> Since this concerns DT, please also add the devicetree list and get a >>>> Acked-by for the bindings you want to push by a DT maintainer. >>> >>> Since I’m the original perpetrator, let me put a few words here for posterity. >>> >>> This patch was meant as a quick and dirty method for doing the automatic export >>> and pinmux configuration from DT on a 3.8 kernel. The kernel APIs have moved on >>> since. It wasn’t meant to be submitted for mainline right as it is. >>> >>> Unfortunately (or fortunately for many people) it does what a lot of people need >>> i.e. configures a board with heavily pinmuxed GPIO right at boot time. >>> >>> I’m open at having a small discussion about how to do what the users of this patch >>> do ‘right’. >> >> Sure, although the discussion might turn out to not be that "small". :) >> > > Heh ;) > >> Pinmux configuration sounds like a job for pinmux more than GPIO, and >> exporting potentially many GPIOs to user-space sounds like a work for >> a proper driver. >> > > I’m afraid that’s not the case. A great many users do not require anything > more than setting a pinmux and the GPIO configuration (input/output). > They can then do low speed I/O using the sysfs interface, without having to > use any complex APIs (shell works just fine). > > Think of stuff like controlling a sprinkler valve, or something like a mechanical > door detection open state. That sounds like any kind of arbitrary device that can be temporarily connected to a set of GPIOs that will be bit-banged. Is my interpretation correct? In that case, I seriously doubt that this should be part of the DT. Right now the DT is part of the firmware, and an immutable description of the hardware layout of a board - definitely such devices do not fit into that definition. This might change once the Device Tree Overlays are merged, but we are not there yet. If the DT maintainers say this is a valid use-case for overlays, then I will reconsider my position, but in any case it really looks like this could/should be done from user-space. You have almost all the necessary pieces: you can export, configure and manipulate any GPIO from the sysfs interface. The only thing you would be missing is a way to give a name to a GPIO, something that can easily be fixed. Since the devices you want to configure that way are typically removable or usage-specific, why would you want to store that information all the way up into the firmware? A commonly accepted wisdom is that what can be done in user-space should be done in user-space, and it really looks like this applies here. Say you buy a Jetson TK1 to control that sprinkler valve (a good use for all these GPU cores!). The mainline DT has no knowledge about the valve, so using your proposed way you will have to re-build and flash a custom device tree just to be able to use your sprinkler. If you decide to assign your board to something else, you will need another DT. Instead, have a shell script or a systemd unit tmpfile that will perform the correct setup at boot time. Then you can easily switch usages from user-space (systemctl stop sprinkler && systemctl start doordetect). This is much, much more flexible. At least until the Device Tree Overlays are merged, but even then this seems overkill. So I don't see any good reason to accept this patch for now. I'm ready to discuss and work on the required improvements to the sysfs interface though - actually this might be a good opportunity to write a better alternative sysfs interface that does not depend on the integer GPIO space, something I have in mind since some time already. -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dne 23.10.2014 v 07:08 Alexandre Courbot napsal(a): > On Wed, Oct 22, 2014 at 6:58 PM, Pantelis Antoniou > <panto@antoniou-consulting.com> wrote: >> Hi Alexandre, >> >>> On Oct 22, 2014, at 12:41 PM, Alexandre Courbot <gnurou@gmail.com> wrote: >>> >>> On Wed, Oct 22, 2014 at 6:24 PM, Pantelis Antoniou >>> <panto@antoniou-consulting.com> wrote: >>>> Hi Alexandre, >>>> >>>>> On Oct 22, 2014, at 12:18 PM, Alexandre Courbot <gnurou@gmail.com> wrote: >>>>> >>>>> On Wed, Oct 22, 2014 at 5:26 PM, Jiri Prchal <jiri.prchal@aksignal.cz> wrote: >>>>>> This patch adds new driver "gpio-of-helper", witch has possibility to export >>>>>> gpios defined in dt. It exports them in defined name under >>>>>> /sysfs/class/gpio/name. >>>>>> It's rebased from Pantelis Antoniou patch to new kernel. >>>>>> Usage example: >>>>>> gpio { >>>>>> compatible = "gpio-of-helper"; >>>>>> status = "okay"; >>>>>> pinctrl-names = "default"; >>>>>> pinctrl-0 = <&pinctrl_gpio>; >>>>>> >>>>>> gsm_on { >>>>>> gpio-name = "gsm_on"; >>>>>> gpio = <&pioB 13 GPIO_ACTIVE_HIGH>; >>>>>> output; >>>>>> init-low; >>>>>> }; >>>>>> }; >>>>>> >>>>>> This patch needs Alexey Ignatov [PATCH] gpiolib: allow exporting gpios with >>>>>> custom names. >>>>> >>>>> We will need to see whether the pre-requisite patch can get merged >>>>> first, but there are a couple of things that are wrong with your patch >>>>> anyway: >>>>> >>>>> - it is missing a bindings documentation >>>>> - it is using the legacy integer GPIOs instead of the descriptor >>>>> interface (see include/linux/gpio/consumer.h). Since this is DT-based, >>>>> there is absolutely no reason to not use the descriptors interface. >>>>> - it seems quite long for what it needs to do >>>>> - the MODULE_AUTHOR has not signed-off this patch (?) >>>>> >>>>> But what makes me nervous is that this encourages more usage of the >>>>> sysfs interface, an in a way that is potentially harmful. >>>>> >>>>> Also, I don't know if the DT people will be happy with this idea. >>>>> Since this concerns DT, please also add the devicetree list and get a >>>>> Acked-by for the bindings you want to push by a DT maintainer. >>>> >>>> Since I’m the original perpetrator, let me put a few words here for posterity. >>>> >>>> This patch was meant as a quick and dirty method for doing the automatic export >>>> and pinmux configuration from DT on a 3.8 kernel. The kernel APIs have moved on >>>> since. It wasn’t meant to be submitted for mainline right as it is. >>>> >>>> Unfortunately (or fortunately for many people) it does what a lot of people need >>>> i.e. configures a board with heavily pinmuxed GPIO right at boot time. >>>> >>>> I’m open at having a small discussion about how to do what the users of this patch >>>> do ‘right’. >>> >>> Sure, although the discussion might turn out to not be that "small". :) >>> >> >> Heh ;) >> >>> Pinmux configuration sounds like a job for pinmux more than GPIO, and >>> exporting potentially many GPIOs to user-space sounds like a work for >>> a proper driver. >>> >> >> I’m afraid that’s not the case. A great many users do not require anything >> more than setting a pinmux and the GPIO configuration (input/output). >> They can then do low speed I/O using the sysfs interface, without having to >> use any complex APIs (shell works just fine). >> >> Think of stuff like controlling a sprinkler valve, or something like a mechanical >> door detection open state. > > That sounds like any kind of arbitrary device that can be temporarily > connected to a set of GPIOs that will be bit-banged. Is my > interpretation correct? Not only temporarily. For example on/off switch of some hw on board like gsm modem. Or some optocoupler isoleted in/outs for general use from userspace. > > In that case, I seriously doubt that this should be part of the DT. > Right now the DT is part of the firmware, and an immutable description > of the hardware layout of a board - definitely such devices do not fit This is our case of gpio. It's like LEDs, they are hw layout of board, but they are in/out instead of LEDs are only output. > into that definition. This might change once the Device Tree Overlays > are merged, but we are not there yet. If the DT maintainers say this > is a valid use-case for overlays, then I will reconsider my position, > but in any case it really looks like this could/should be done from > user-space. I don't think so as I explained - HW layout of boerd. In other way I'm looking forward for DT overlays. > > You have almost all the necessary pieces: you can export, configure > and manipulate any GPIO from the sysfs interface. The only thing you > would be missing is a way to give a name to a GPIO, something that can > easily be fixed. But isn't it nice to have already exported gpios which are layouted out from board. All other unexported gpios aren't connected to anywhere on board. So user don't have to export or unexport anything. > > Since the devices you want to configure that way are typically > removable or usage-specific, why would you want to store that > information all the way up into the firmware? A commonly accepted > wisdom is that what can be done in user-space should be done in > user-space, and it really looks like this applies here. They are NOT removable. > > Say you buy a Jetson TK1 to control that sprinkler valve (a good use > for all these GPU cores!). The mainline DT has no knowledge about the > valve, so using your proposed way you will have to re-build and flash > a custom device tree just to be able to use your sprinkler. If you > decide to assign your board to something else, you will need another > DT. But if you buy more "real world ready" board with for example 8 power outputs for sprinkler valves, why has user think about what gpio to export, if there could be valve0 - 7 ready. If he will use only valve0 does not mater, all unused gpio can't be used for anything else since they has layout for valves. > > Instead, have a shell script or a systemd unit tmpfile that will > perform the correct setup at boot time. Then you can easily switch > usages from user-space (systemctl stop sprinkler && systemctl start > doordetect). This is much, much more flexible. At least until the > Device Tree Overlays are merged, but even then this seems overkill. I seriously think about it, but I think if LEDs are in DT, GPIOs should be there too. In other way, I don't like exporting without given name. > > So I don't see any good reason to accept this patch for now. I'm ready > to discuss and work on the required improvements to the sysfs > interface though - actually this might be a good opportunity to write > a better alternative sysfs interface that does not depend on the > integer GPIO space, something I have in mind since some time already. > -- > To unsubscribe from this list: send the line "unsubscribe linux-gpio" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Oct 23, 2014 at 3:23 PM, Jiří Prchal <jiri.prchal@aksignal.cz> wrote: > > > Dne 23.10.2014 v 07:08 Alexandre Courbot napsal(a): > >> On Wed, Oct 22, 2014 at 6:58 PM, Pantelis Antoniou >> <panto@antoniou-consulting.com> wrote: >>> >>> Hi Alexandre, >>> >>>> On Oct 22, 2014, at 12:41 PM, Alexandre Courbot <gnurou@gmail.com> >>>> wrote: >>>> >>>> On Wed, Oct 22, 2014 at 6:24 PM, Pantelis Antoniou >>>> <panto@antoniou-consulting.com> wrote: >>>>> >>>>> Hi Alexandre, >>>>> >>>>>> On Oct 22, 2014, at 12:18 PM, Alexandre Courbot <gnurou@gmail.com> >>>>>> wrote: >>>>>> >>>>>> On Wed, Oct 22, 2014 at 5:26 PM, Jiri Prchal <jiri.prchal@aksignal.cz> >>>>>> wrote: >>>>>>> >>>>>>> This patch adds new driver "gpio-of-helper", witch has possibility to >>>>>>> export >>>>>>> gpios defined in dt. It exports them in defined name under >>>>>>> /sysfs/class/gpio/name. >>>>>>> It's rebased from Pantelis Antoniou patch to new kernel. >>>>>>> Usage example: >>>>>>> gpio { >>>>>>> compatible = "gpio-of-helper"; >>>>>>> status = "okay"; >>>>>>> pinctrl-names = "default"; >>>>>>> pinctrl-0 = <&pinctrl_gpio>; >>>>>>> >>>>>>> gsm_on { >>>>>>> gpio-name = "gsm_on"; >>>>>>> gpio = <&pioB 13 GPIO_ACTIVE_HIGH>; >>>>>>> output; >>>>>>> init-low; >>>>>>> }; >>>>>>> }; >>>>>>> >>>>>>> This patch needs Alexey Ignatov [PATCH] gpiolib: allow exporting >>>>>>> gpios with >>>>>>> custom names. >>>>>> >>>>>> >>>>>> We will need to see whether the pre-requisite patch can get merged >>>>>> first, but there are a couple of things that are wrong with your patch >>>>>> anyway: >>>>>> >>>>>> - it is missing a bindings documentation >>>>>> - it is using the legacy integer GPIOs instead of the descriptor >>>>>> interface (see include/linux/gpio/consumer.h). Since this is DT-based, >>>>>> there is absolutely no reason to not use the descriptors interface. >>>>>> - it seems quite long for what it needs to do >>>>>> - the MODULE_AUTHOR has not signed-off this patch (?) >>>>>> >>>>>> But what makes me nervous is that this encourages more usage of the >>>>>> sysfs interface, an in a way that is potentially harmful. >>>>>> >>>>>> Also, I don't know if the DT people will be happy with this idea. >>>>>> Since this concerns DT, please also add the devicetree list and get a >>>>>> Acked-by for the bindings you want to push by a DT maintainer. >>>>> >>>>> >>>>> Since I’m the original perpetrator, let me put a few words here for >>>>> posterity. >>>>> >>>>> This patch was meant as a quick and dirty method for doing the >>>>> automatic export >>>>> and pinmux configuration from DT on a 3.8 kernel. The kernel APIs have >>>>> moved on >>>>> since. It wasn’t meant to be submitted for mainline right as it is. >>>>> >>>>> Unfortunately (or fortunately for many people) it does what a lot of >>>>> people need >>>>> i.e. configures a board with heavily pinmuxed GPIO right at boot time. >>>>> >>>>> I’m open at having a small discussion about how to do what the users of >>>>> this patch >>>>> do ‘right’. >>>> >>>> >>>> Sure, although the discussion might turn out to not be that "small". :) >>>> >>> >>> Heh ;) >>> >>>> Pinmux configuration sounds like a job for pinmux more than GPIO, and >>>> exporting potentially many GPIOs to user-space sounds like a work for >>>> a proper driver. >>>> >>> >>> I’m afraid that’s not the case. A great many users do not require >>> anything >>> more than setting a pinmux and the GPIO configuration (input/output). >>> They can then do low speed I/O using the sysfs interface, without having >>> to >>> use any complex APIs (shell works just fine). >>> >>> Think of stuff like controlling a sprinkler valve, or something like a >>> mechanical >>> door detection open state. >> >> >> That sounds like any kind of arbitrary device that can be temporarily >> connected to a set of GPIOs that will be bit-banged. Is my >> interpretation correct? > > > Not only temporarily. For example on/off switch of some hw on board like gsm > modem. ... which should be handled in-kernel. > Or some optocoupler isoleted in/outs for general use from userspace. ... which should thus be configured from userspace. >> >> >> In that case, I seriously doubt that this should be part of the DT. >> Right now the DT is part of the firmware, and an immutable description >> of the hardware layout of a board - definitely such devices do not fit > > This is our case of gpio. It's like LEDs, they are hw layout of board, > but they are in/out instead of LEDs are only output. >> >> into that definition. This might change once the Device Tree Overlays >> are merged, but we are not there yet. If the DT maintainers say this >> is a valid use-case for overlays, then I will reconsider my position, >> but in any case it really looks like this could/should be done from >> user-space. > > I don't think so as I explained - HW layout of boerd. If it is a static hardware layout that makes the case even clearer - these devices should have a proper kernel driver that configures these GPIOs, and exports a proper interface to userspace, not just raw GPIOs. > In other way I'm looking forward for DT overlays. >> >> >> You have almost all the necessary pieces: you can export, configure >> and manipulate any GPIO from the sysfs interface. The only thing you >> would be missing is a way to give a name to a GPIO, something that can >> easily be fixed. > > But isn't it nice to have already exported gpios which are layouted out > from board. All other unexported gpios aren't connected to anywhere on > board. So user don't have to export or unexport anything. >> >> >> Since the devices you want to configure that way are typically >> removable or usage-specific, why would you want to store that >> information all the way up into the firmware? A commonly accepted >> wisdom is that what can be done in user-space should be done in >> user-space, and it really looks like this applies here. > > They are NOT removable. >> >> >> Say you buy a Jetson TK1 to control that sprinkler valve (a good use >> for all these GPU cores!). The mainline DT has no knowledge about the >> valve, so using your proposed way you will have to re-build and flash >> a custom device tree just to be able to use your sprinkler. If you >> decide to assign your board to something else, you will need another >> DT. > > But if you buy more "real world ready" board with for example 8 power > outputs for sprinkler valves, why has user think about what gpio to > export, if there could be valve0 - 7 ready. If he will use only valve0 > does not mater, all unused gpio can't be used for anything else since > they has layout for valves. The problem is indeed different. If your sprinkler valves are wired on your board and the lines cannot be used for anything else, then what you need is a driver for the valve that will acquire the GPIOs and export its own userspace interface under /sys/. You device tree must reflect the layout of the board ; in this case "here is a valve that uses this GPIO active-low". Saying "this GPIO is output active-low and named valve0" is not a good description of your hardware. >> >> >> Instead, have a shell script or a systemd unit tmpfile that will >> perform the correct setup at boot time. Then you can easily switch >> usages from user-space (systemctl stop sprinkler && systemctl start >> doordetect). This is much, much more flexible. At least until the >> Device Tree Overlays are merged, but even then this seems overkill. > > I seriously think about it, but I think if LEDs are in DT, GPIOs should > be there too. In other way, I don't like exporting without given name. LEDs are actually a perfect example of what you should do. They are a simple device often plugged to a GPIO that controls them. We could very well do what you propose: export GPIOs named "ledXX" and call it a day. Instead we have a driver that exports a proper device node in /sys with the operations that are relevant to a LED. Once your GPIOs are assigned to a given function, they are not "general purpose" anymore. Therefore you should expose the right abstraction to your users, even if that means writing some more code. -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dne 23.10.2014 v 10:53 Alexandre Courbot napsal(a): > On Thu, Oct 23, 2014 at 3:23 PM, Jiří Prchal <jiri.prchal@aksignal.cz> wrote: >> >> >> Dne 23.10.2014 v 07:08 Alexandre Courbot napsal(a): >> >>> On Wed, Oct 22, 2014 at 6:58 PM, Pantelis Antoniou >>> <panto@antoniou-consulting.com> wrote: >>>> >>>> Hi Alexandre, >>>> >>>>> On Oct 22, 2014, at 12:41 PM, Alexandre Courbot <gnurou@gmail.com> >>>>> wrote: >>>>> >>>>> On Wed, Oct 22, 2014 at 6:24 PM, Pantelis Antoniou >>>>> <panto@antoniou-consulting.com> wrote: >>>>>> >>>>>> Hi Alexandre, >>>>>> >>>>>>> On Oct 22, 2014, at 12:18 PM, Alexandre Courbot <gnurou@gmail.com> >>>>>>> wrote: >>>>>>> >>>>>>> On Wed, Oct 22, 2014 at 5:26 PM, Jiri Prchal <jiri.prchal@aksignal.cz> >>>>>>> wrote: >>>>>>>> >>>>>>>> This patch adds new driver "gpio-of-helper", witch has possibility to >>>>>>>> export >>>>>>>> gpios defined in dt. It exports them in defined name under >>>>>>>> /sysfs/class/gpio/name. >>>>>>>> It's rebased from Pantelis Antoniou patch to new kernel. >>>>>>>> Usage example: >>>>>>>> gpio { >>>>>>>> compatible = "gpio-of-helper"; >>>>>>>> status = "okay"; >>>>>>>> pinctrl-names = "default"; >>>>>>>> pinctrl-0 = <&pinctrl_gpio>; >>>>>>>> >>>>>>>> gsm_on { >>>>>>>> gpio-name = "gsm_on"; >>>>>>>> gpio = <&pioB 13 GPIO_ACTIVE_HIGH>; >>>>>>>> output; >>>>>>>> init-low; >>>>>>>> }; >>>>>>>> }; >>>>>>>> >>>>>>>> This patch needs Alexey Ignatov [PATCH] gpiolib: allow exporting >>>>>>>> gpios with >>>>>>>> custom names. >>>>>>> >>>>>>> >>>>>>> We will need to see whether the pre-requisite patch can get merged >>>>>>> first, but there are a couple of things that are wrong with your patch >>>>>>> anyway: >>>>>>> >>>>>>> - it is missing a bindings documentation >>>>>>> - it is using the legacy integer GPIOs instead of the descriptor >>>>>>> interface (see include/linux/gpio/consumer.h). Since this is DT-based, >>>>>>> there is absolutely no reason to not use the descriptors interface. >>>>>>> - it seems quite long for what it needs to do >>>>>>> - the MODULE_AUTHOR has not signed-off this patch (?) >>>>>>> >>>>>>> But what makes me nervous is that this encourages more usage of the >>>>>>> sysfs interface, an in a way that is potentially harmful. >>>>>>> >>>>>>> Also, I don't know if the DT people will be happy with this idea. >>>>>>> Since this concerns DT, please also add the devicetree list and get a >>>>>>> Acked-by for the bindings you want to push by a DT maintainer. >>>>>> >>>>>> >>>>>> Since I’m the original perpetrator, let me put a few words here for >>>>>> posterity. >>>>>> >>>>>> This patch was meant as a quick and dirty method for doing the >>>>>> automatic export >>>>>> and pinmux configuration from DT on a 3.8 kernel. The kernel APIs have >>>>>> moved on >>>>>> since. It wasn’t meant to be submitted for mainline right as it is. >>>>>> >>>>>> Unfortunately (or fortunately for many people) it does what a lot of >>>>>> people need >>>>>> i.e. configures a board with heavily pinmuxed GPIO right at boot time. >>>>>> >>>>>> I’m open at having a small discussion about how to do what the users of >>>>>> this patch >>>>>> do ‘right’. >>>>> >>>>> >>>>> Sure, although the discussion might turn out to not be that "small". :) >>>>> >>>> >>>> Heh ;) >>>> >>>>> Pinmux configuration sounds like a job for pinmux more than GPIO, and >>>>> exporting potentially many GPIOs to user-space sounds like a work for >>>>> a proper driver. >>>>> >>>> >>>> I’m afraid that’s not the case. A great many users do not require >>>> anything >>>> more than setting a pinmux and the GPIO configuration (input/output). >>>> They can then do low speed I/O using the sysfs interface, without having >>>> to >>>> use any complex APIs (shell works just fine). >>>> >>>> Think of stuff like controlling a sprinkler valve, or something like a >>>> mechanical >>>> door detection open state. >>> >>> >>> That sounds like any kind of arbitrary device that can be temporarily >>> connected to a set of GPIOs that will be bit-banged. Is my >>> interpretation correct? >> >> >> Not only temporarily. For example on/off switch of some hw on board like gsm >> modem. > > ... which should be handled in-kernel. How to? It's just connected to ttyS and after apply power it has to be switched on by pulling some pin. > >> Or some optocoupler isoleted in/outs for general use from userspace. > > ... which should thus be configured from userspace. Why not from dt? > >>> >>> >>> In that case, I seriously doubt that this should be part of the DT. >>> Right now the DT is part of the firmware, and an immutable description >>> of the hardware layout of a board - definitely such devices do not fit >> >> This is our case of gpio. It's like LEDs, they are hw layout of board, >> but they are in/out instead of LEDs are only output. >>> >>> into that definition. This might change once the Device Tree Overlays >>> are merged, but we are not there yet. If the DT maintainers say this >>> is a valid use-case for overlays, then I will reconsider my position, >>> but in any case it really looks like this could/should be done from >>> user-space. >> >> I don't think so as I explained - HW layout of boerd. > > If it is a static hardware layout that makes the case even clearer - > these devices should have a proper kernel driver that configures these > GPIOs, and exports a proper interface to userspace, not just raw > GPIOs. But we don't need anything else just raw named gpio. > >> In other way I'm looking forward for DT overlays. >>> >>> >>> You have almost all the necessary pieces: you can export, configure >>> and manipulate any GPIO from the sysfs interface. The only thing you >>> would be missing is a way to give a name to a GPIO, something that can >>> easily be fixed. >> >> But isn't it nice to have already exported gpios which are layouted out >> from board. All other unexported gpios aren't connected to anywhere on >> board. So user don't have to export or unexport anything. >>> >>> >>> Since the devices you want to configure that way are typically >>> removable or usage-specific, why would you want to store that >>> information all the way up into the firmware? A commonly accepted >>> wisdom is that what can be done in user-space should be done in >>> user-space, and it really looks like this applies here. >> >> They are NOT removable. >>> >>> >>> Say you buy a Jetson TK1 to control that sprinkler valve (a good use >>> for all these GPU cores!). The mainline DT has no knowledge about the >>> valve, so using your proposed way you will have to re-build and flash >>> a custom device tree just to be able to use your sprinkler. If you >>> decide to assign your board to something else, you will need another >>> DT. >> >> But if you buy more "real world ready" board with for example 8 power >> outputs for sprinkler valves, why has user think about what gpio to >> export, if there could be valve0 - 7 ready. If he will use only valve0 >> does not mater, all unused gpio can't be used for anything else since >> they has layout for valves. > > The problem is indeed different. > > If your sprinkler valves are wired on your board and the lines cannot > be used for anything else, then what you need is a driver for the > valve that will acquire the GPIOs and export its own userspace > interface under /sys/. > > You device tree must reflect the layout of the board ; in this case > "here is a valve that uses this GPIO active-low". Saying "this GPIO is > output active-low and named valve0" is not a good description of your > hardware. Maybe I didn't give good example. There is a board. It has 8 outputs. They are relay outputs. Let's give them rel0 - 7 name. And user can same of them use for valve, some for light... Ant board has 8 inputs. They are optocoupler isolated. And user can some of them use to get water level, light... > >>> >>> >>> Instead, have a shell script or a systemd unit tmpfile that will >>> perform the correct setup at boot time. Then you can easily switch >>> usages from user-space (systemctl stop sprinkler && systemctl start >>> doordetect). This is much, much more flexible. At least until the >>> Device Tree Overlays are merged, but even then this seems overkill. >> >> I seriously think about it, but I think if LEDs are in DT, GPIOs should >> be there too. In other way, I don't like exporting without given name. > > LEDs are actually a perfect example of what you should do. They are a > simple device often plugged to a GPIO that controls them. We could > very well do what you propose: export GPIOs named "ledXX" and call it > a day. Instead we have a driver that exports a proper device node in > /sys with the operations that are relevant to a LED. > > Once your GPIOs are assigned to a given function, they are not > "general purpose" anymore. Therefore you should expose the right > abstraction to your users, even if that means writing some more code. So making some kind of device "inout" wold be good idea? > -- > To unsubscribe from this list: send the line "unsubscribe linux-gpio" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Alexandre, > On Oct 23, 2014, at 11:53 AM, Alexandre Courbot <gnurou@gmail.com> wrote: > > On Thu, Oct 23, 2014 at 3:23 PM, Jiří Prchal <jiri.prchal@aksignal.cz> wrote: >> >> >> Dne 23.10.2014 v 07:08 Alexandre Courbot napsal(a): >> >>> On Wed, Oct 22, 2014 at 6:58 PM, Pantelis Antoniou >>> <panto@antoniou-consulting.com> wrote: >>>> >>>> Hi Alexandre, >>>> >>>>> On Oct 22, 2014, at 12:41 PM, Alexandre Courbot <gnurou@gmail.com> >>>>> wrote: >>>>> >>>>> On Wed, Oct 22, 2014 at 6:24 PM, Pantelis Antoniou >>>>> <panto@antoniou-consulting.com> wrote: >>>>>> >>>>>> Hi Alexandre, >>>>>> >>>>>>> On Oct 22, 2014, at 12:18 PM, Alexandre Courbot <gnurou@gmail.com> >>>>>>> wrote: >>>>>>> >>>>>>> On Wed, Oct 22, 2014 at 5:26 PM, Jiri Prchal <jiri.prchal@aksignal.cz> >>>>>>> wrote: >>>>>>>> >>>>>>>> This patch adds new driver "gpio-of-helper", witch has possibility to >>>>>>>> export >>>>>>>> gpios defined in dt. It exports them in defined name under >>>>>>>> /sysfs/class/gpio/name. >>>>>>>> It's rebased from Pantelis Antoniou patch to new kernel. >>>>>>>> Usage example: >>>>>>>> gpio { >>>>>>>> compatible = "gpio-of-helper"; >>>>>>>> status = "okay"; >>>>>>>> pinctrl-names = "default"; >>>>>>>> pinctrl-0 = <&pinctrl_gpio>; >>>>>>>> >>>>>>>> gsm_on { >>>>>>>> gpio-name = "gsm_on"; >>>>>>>> gpio = <&pioB 13 GPIO_ACTIVE_HIGH>; >>>>>>>> output; >>>>>>>> init-low; >>>>>>>> }; >>>>>>>> }; >>>>>>>> >>>>>>>> This patch needs Alexey Ignatov [PATCH] gpiolib: allow exporting >>>>>>>> gpios with >>>>>>>> custom names. >>>>>>> >>>>>>> >>>>>>> We will need to see whether the pre-requisite patch can get merged >>>>>>> first, but there are a couple of things that are wrong with your patch >>>>>>> anyway: >>>>>>> >>>>>>> - it is missing a bindings documentation >>>>>>> - it is using the legacy integer GPIOs instead of the descriptor >>>>>>> interface (see include/linux/gpio/consumer.h). Since this is DT-based, >>>>>>> there is absolutely no reason to not use the descriptors interface. >>>>>>> - it seems quite long for what it needs to do >>>>>>> - the MODULE_AUTHOR has not signed-off this patch (?) >>>>>>> >>>>>>> But what makes me nervous is that this encourages more usage of the >>>>>>> sysfs interface, an in a way that is potentially harmful. >>>>>>> >>>>>>> Also, I don't know if the DT people will be happy with this idea. >>>>>>> Since this concerns DT, please also add the devicetree list and get a >>>>>>> Acked-by for the bindings you want to push by a DT maintainer. >>>>>> >>>>>> >>>>>> Since I’m the original perpetrator, let me put a few words here for >>>>>> posterity. >>>>>> >>>>>> This patch was meant as a quick and dirty method for doing the >>>>>> automatic export >>>>>> and pinmux configuration from DT on a 3.8 kernel. The kernel APIs have >>>>>> moved on >>>>>> since. It wasn’t meant to be submitted for mainline right as it is. >>>>>> >>>>>> Unfortunately (or fortunately for many people) it does what a lot of >>>>>> people need >>>>>> i.e. configures a board with heavily pinmuxed GPIO right at boot time. >>>>>> >>>>>> I’m open at having a small discussion about how to do what the users of >>>>>> this patch >>>>>> do ‘right’. >>>>> >>>>> >>>>> Sure, although the discussion might turn out to not be that "small". :) >>>>> >>>> >>>> Heh ;) >>>> >>>>> Pinmux configuration sounds like a job for pinmux more than GPIO, and >>>>> exporting potentially many GPIOs to user-space sounds like a work for >>>>> a proper driver. >>>>> >>>> >>>> I’m afraid that’s not the case. A great many users do not require >>>> anything >>>> more than setting a pinmux and the GPIO configuration (input/output). >>>> They can then do low speed I/O using the sysfs interface, without having >>>> to >>>> use any complex APIs (shell works just fine). >>>> >>>> Think of stuff like controlling a sprinkler valve, or something like a >>>> mechanical >>>> door detection open state. >>> >>> >>> That sounds like any kind of arbitrary device that can be temporarily >>> connected to a set of GPIOs that will be bit-banged. Is my >>> interpretation correct? >> >> >> Not only temporarily. For example on/off switch of some hw on board like gsm >> modem. > > ... which should be handled in-kernel. > Not really. This is part of some kind user-space ABI but which has to do with the mapping of an named GPIO in the board schematics to user-space. >> Or some optocoupler isoleted in/outs for general use from userspace. > > ... which should thus be configured from userspace. > No. It is used from userspace, but the the configuration of such is performed by board specific means in the kernel. >>> >>> >>> In that case, I seriously doubt that this should be part of the DT. >>> Right now the DT is part of the firmware, and an immutable description >>> of the hardware layout of a board - definitely such devices do not fit >> >> This is our case of gpio. It's like LEDs, they are hw layout of board, >> but they are in/out instead of LEDs are only output. >>> >>> into that definition. This might change once the Device Tree Overlays >>> are merged, but we are not there yet. If the DT maintainers say this >>> is a valid use-case for overlays, then I will reconsider my position, >>> but in any case it really looks like this could/should be done from >>> user-space. >> >> I don't think so as I explained - HW layout of boerd. > > If it is a static hardware layout that makes the case even clearer - > these devices should have a proper kernel driver that configures these > GPIOs, and exports a proper interface to userspace, not just raw > GPIOs. Raw GPIOs is what the user-space application wants to use. There is no higher abstraction possible. What these applications need are: 1. A way to map a GPIO to a name that is board invariant. Many boards move GPIOs around but the function typically stays the same. So for instance rev-0 might have GARAGE_DOOR to GPIO_A10, and on subsequent revision rev-1 it might be GARAGE_DOOR to GPIO_B08. The user-space application that controls everything does not want to deal with revisions ideally, just to know that the named GARAGE_DOOR gpio exists. 2. A way to have the pinmux configuration of said GPIO configured without having to do anything explicit. I.e. on am335x the pinmux configuration is decoupled from the GPIO driver; having a GPIO configured as a GPIO does not alter the pinmux settings. 3. The user-space facing API must be simple, preferably file based like the sysfs method right now. It is very simple to use even from within scripts. > >> In other way I'm looking forward for DT overlays. >>> >>> >>> You have almost all the necessary pieces: you can export, configure >>> and manipulate any GPIO from the sysfs interface. The only thing you >>> would be missing is a way to give a name to a GPIO, something that can >>> easily be fixed. >> >> But isn't it nice to have already exported gpios which are layouted out >> from board. All other unexported gpios aren't connected to anywhere on >> board. So user don't have to export or unexport anything. >>> >>> >>> Since the devices you want to configure that way are typically >>> removable or usage-specific, why would you want to store that >>> information all the way up into the firmware? A commonly accepted >>> wisdom is that what can be done in user-space should be done in >>> user-space, and it really looks like this applies here. >> >> They are NOT removable. >>> >>> >>> Say you buy a Jetson TK1 to control that sprinkler valve (a good use >>> for all these GPU cores!). The mainline DT has no knowledge about the >>> valve, so using your proposed way you will have to re-build and flash >>> a custom device tree just to be able to use your sprinkler. If you >>> decide to assign your board to something else, you will need another >>> DT. >> >> But if you buy more "real world ready" board with for example 8 power >> outputs for sprinkler valves, why has user think about what gpio to >> export, if there could be valve0 - 7 ready. If he will use only valve0 >> does not mater, all unused gpio can't be used for anything else since >> they has layout for valves. > > The problem is indeed different. > > If your sprinkler valves are wired on your board and the lines cannot > be used for anything else, then what you need is a driver for the > valve that will acquire the GPIOs and export its own userspace > interface under /sys/. > > You device tree must reflect the layout of the board ; in this case > "here is a valve that uses this GPIO active-low". Saying "this GPIO is > output active-low and named valve0" is not a good description of your > hardware. > There is no higher layer driver API. We don’t want to litter the kernel with ‘valve’ drivers, there’s no such thing. There’s only digital I/Os that’s connected to low speed devices. If we are going for a higher layer API that would be something very thin like a user-space GPIO driver. Do you see the conundrum? >>> >>> >>> Instead, have a shell script or a systemd unit tmpfile that will >>> perform the correct setup at boot time. Then you can easily switch >>> usages from user-space (systemctl stop sprinkler && systemctl start >>> doordetect). This is much, much more flexible. At least until the >>> Device Tree Overlays are merged, but even then this seems overkill. >> >> I seriously think about it, but I think if LEDs are in DT, GPIOs should >> be there too. In other way, I don't like exporting without given name. > > LEDs are actually a perfect example of what you should do. They are a > simple device often plugged to a GPIO that controls them. We could > very well do what you propose: export GPIOs named "ledXX" and call it > a day. Instead we have a driver that exports a proper device node in > /sys with the operations that are relevant to a LED. > > Once your GPIOs are assigned to a given function, they are not > "general purpose" anymore. Therefore you should expose the right > abstraction to your users, even if that means writing some more code. Please, try to understand how our users use our linux kernel drivers. Jiri’s concerns are valid and I have seen his use patterns in countless other cases. Regards — Pantelis -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Added the DT maintainers and fixed the DT mailing-list's address so this discussion becomes visible to them as well. On Thu, Oct 23, 2014 at 8:23 PM, Pantelis Antoniou <panto@antoniou-consulting.com> wrote: > Hi Alexandre, > >> On Oct 23, 2014, at 11:53 AM, Alexandre Courbot <gnurou@gmail.com> wrote: >> >> On Thu, Oct 23, 2014 at 3:23 PM, Jiří Prchal <jiri.prchal@aksignal.cz> wrote: >>> >>> >>> Dne 23.10.2014 v 07:08 Alexandre Courbot napsal(a): >>> >>>> On Wed, Oct 22, 2014 at 6:58 PM, Pantelis Antoniou >>>> <panto@antoniou-consulting.com> wrote: >>>>> >>>>> Hi Alexandre, >>>>> >>>>>> On Oct 22, 2014, at 12:41 PM, Alexandre Courbot <gnurou@gmail.com> >>>>>> wrote: >>>>>> >>>>>> On Wed, Oct 22, 2014 at 6:24 PM, Pantelis Antoniou >>>>>> <panto@antoniou-consulting.com> wrote: >>>>>>> >>>>>>> Hi Alexandre, >>>>>>> >>>>>>>> On Oct 22, 2014, at 12:18 PM, Alexandre Courbot <gnurou@gmail.com> >>>>>>>> wrote: >>>>>>>> >>>>>>>> On Wed, Oct 22, 2014 at 5:26 PM, Jiri Prchal <jiri.prchal@aksignal.cz> >>>>>>>> wrote: >>>>>>>>> >>>>>>>>> This patch adds new driver "gpio-of-helper", witch has possibility to >>>>>>>>> export >>>>>>>>> gpios defined in dt. It exports them in defined name under >>>>>>>>> /sysfs/class/gpio/name. >>>>>>>>> It's rebased from Pantelis Antoniou patch to new kernel. >>>>>>>>> Usage example: >>>>>>>>> gpio { >>>>>>>>> compatible = "gpio-of-helper"; >>>>>>>>> status = "okay"; >>>>>>>>> pinctrl-names = "default"; >>>>>>>>> pinctrl-0 = <&pinctrl_gpio>; >>>>>>>>> >>>>>>>>> gsm_on { >>>>>>>>> gpio-name = "gsm_on"; >>>>>>>>> gpio = <&pioB 13 GPIO_ACTIVE_HIGH>; >>>>>>>>> output; >>>>>>>>> init-low; >>>>>>>>> }; >>>>>>>>> }; >>>>>>>>> >>>>>>>>> This patch needs Alexey Ignatov [PATCH] gpiolib: allow exporting >>>>>>>>> gpios with >>>>>>>>> custom names. >>>>>>>> >>>>>>>> >>>>>>>> We will need to see whether the pre-requisite patch can get merged >>>>>>>> first, but there are a couple of things that are wrong with your patch >>>>>>>> anyway: >>>>>>>> >>>>>>>> - it is missing a bindings documentation >>>>>>>> - it is using the legacy integer GPIOs instead of the descriptor >>>>>>>> interface (see include/linux/gpio/consumer.h). Since this is DT-based, >>>>>>>> there is absolutely no reason to not use the descriptors interface. >>>>>>>> - it seems quite long for what it needs to do >>>>>>>> - the MODULE_AUTHOR has not signed-off this patch (?) >>>>>>>> >>>>>>>> But what makes me nervous is that this encourages more usage of the >>>>>>>> sysfs interface, an in a way that is potentially harmful. >>>>>>>> >>>>>>>> Also, I don't know if the DT people will be happy with this idea. >>>>>>>> Since this concerns DT, please also add the devicetree list and get a >>>>>>>> Acked-by for the bindings you want to push by a DT maintainer. >>>>>>> >>>>>>> >>>>>>> Since I’m the original perpetrator, let me put a few words here for >>>>>>> posterity. >>>>>>> >>>>>>> This patch was meant as a quick and dirty method for doing the >>>>>>> automatic export >>>>>>> and pinmux configuration from DT on a 3.8 kernel. The kernel APIs have >>>>>>> moved on >>>>>>> since. It wasn’t meant to be submitted for mainline right as it is. >>>>>>> >>>>>>> Unfortunately (or fortunately for many people) it does what a lot of >>>>>>> people need >>>>>>> i.e. configures a board with heavily pinmuxed GPIO right at boot time. >>>>>>> >>>>>>> I’m open at having a small discussion about how to do what the users of >>>>>>> this patch >>>>>>> do ‘right’. >>>>>> >>>>>> >>>>>> Sure, although the discussion might turn out to not be that "small". :) >>>>>> >>>>> >>>>> Heh ;) >>>>> >>>>>> Pinmux configuration sounds like a job for pinmux more than GPIO, and >>>>>> exporting potentially many GPIOs to user-space sounds like a work for >>>>>> a proper driver. >>>>>> >>>>> >>>>> I’m afraid that’s not the case. A great many users do not require >>>>> anything >>>>> more than setting a pinmux and the GPIO configuration (input/output). >>>>> They can then do low speed I/O using the sysfs interface, without having >>>>> to >>>>> use any complex APIs (shell works just fine). >>>>> >>>>> Think of stuff like controlling a sprinkler valve, or something like a >>>>> mechanical >>>>> door detection open state. >>>> >>>> >>>> That sounds like any kind of arbitrary device that can be temporarily >>>> connected to a set of GPIOs that will be bit-banged. Is my >>>> interpretation correct? >>> >>> >>> Not only temporarily. For example on/off switch of some hw on board like gsm >>> modem. >> >> ... which should be handled in-kernel. >> > > Not really. This is part of some kind user-space ABI but which has to do > with the mapping of an named GPIO in the board schematics to user-space. > >>> Or some optocoupler isoleted in/outs for general use from userspace. >> >> ... which should thus be configured from userspace. >> > > No. It is used from userspace, but the the configuration of such is performed > by board specific means in the kernel. > >>>> >>>> >>>> In that case, I seriously doubt that this should be part of the DT. >>>> Right now the DT is part of the firmware, and an immutable description >>>> of the hardware layout of a board - definitely such devices do not fit >>> >>> This is our case of gpio. It's like LEDs, they are hw layout of board, >>> but they are in/out instead of LEDs are only output. >>>> >>>> into that definition. This might change once the Device Tree Overlays >>>> are merged, but we are not there yet. If the DT maintainers say this >>>> is a valid use-case for overlays, then I will reconsider my position, >>>> but in any case it really looks like this could/should be done from >>>> user-space. >>> >>> I don't think so as I explained - HW layout of boerd. >> >> If it is a static hardware layout that makes the case even clearer - >> these devices should have a proper kernel driver that configures these >> GPIOs, and exports a proper interface to userspace, not just raw >> GPIOs. > > Raw GPIOs is what the user-space application wants to use. There is no > higher abstraction possible. Why can't the application use something else than raw GPIOs? Don't you have its source code? > > What these applications need are: > > 1. A way to map a GPIO to a name that is board invariant. Many boards > move GPIOs around but the function typically stays the same. So for > instance rev-0 might have GARAGE_DOOR to GPIO_A10, and on subsequent > revision rev-1 it might be GARAGE_DOOR to GPIO_B08. The user-space > application that controls everything does not want to deal with revisions > ideally, just to know that the named GARAGE_DOOR gpio exists. This sounds pretty much like you are asking to register application-specific configuration into the DT. > > 2. A way to have the pinmux configuration of said GPIO configured without > having to do anything explicit. I.e. on am335x the pinmux configuration is > decoupled from the GPIO driver; having a GPIO configured as a GPIO does not > alter the pinmux settings. Shouldn't that be fixed in the pinmux/GPIO kernel drivers? I am not too familiar with pinmux, but IIUC pixmux configuration when GPIOs are requested is something that is commonly done. > > 3. The user-space facing API must be simple, preferably file based like the > sysfs method right now. It is very simple to use even from within scripts. That we definitely agree, and as of today there are many ways to do this: 1) A dedicated driver that exposes exactly the right interface to sysfs. I know it may seem overkill, but we have drivers for other simple things like GPIO-controlled LEDs. 2) If you think writing a driver is not worth the effort, you can already export and configure (and I'm willing to discuss the possibility to add "give names" to that list) GPIOs, from user-space. I understand that you want to keep the GPIO exporting/configuration from the DT to have all the board-specific settings in one place, but my understanding of the device tree is that it is supposed to say "I have a garage door control device connected to this GPIO", not "export this GPIO as input active-low, and name it GARAGE_DOOR". > >> >>> In other way I'm looking forward for DT overlays. >>>> >>>> >>>> You have almost all the necessary pieces: you can export, configure >>>> and manipulate any GPIO from the sysfs interface. The only thing you >>>> would be missing is a way to give a name to a GPIO, something that can >>>> easily be fixed. >>> >>> But isn't it nice to have already exported gpios which are layouted out >>> from board. All other unexported gpios aren't connected to anywhere on >>> board. So user don't have to export or unexport anything. >>>> >>>> >>>> Since the devices you want to configure that way are typically >>>> removable or usage-specific, why would you want to store that >>>> information all the way up into the firmware? A commonly accepted >>>> wisdom is that what can be done in user-space should be done in >>>> user-space, and it really looks like this applies here. >>> >>> They are NOT removable. >>>> >>>> >>>> Say you buy a Jetson TK1 to control that sprinkler valve (a good use >>>> for all these GPU cores!). The mainline DT has no knowledge about the >>>> valve, so using your proposed way you will have to re-build and flash >>>> a custom device tree just to be able to use your sprinkler. If you >>>> decide to assign your board to something else, you will need another >>>> DT. >>> >>> But if you buy more "real world ready" board with for example 8 power >>> outputs for sprinkler valves, why has user think about what gpio to >>> export, if there could be valve0 - 7 ready. If he will use only valve0 >>> does not mater, all unused gpio can't be used for anything else since >>> they has layout for valves. >> >> The problem is indeed different. >> >> If your sprinkler valves are wired on your board and the lines cannot >> be used for anything else, then what you need is a driver for the >> valve that will acquire the GPIOs and export its own userspace >> interface under /sys/. >> >> You device tree must reflect the layout of the board ; in this case >> "here is a valve that uses this GPIO active-low". Saying "this GPIO is >> output active-low and named valve0" is not a good description of your >> hardware. >> > > There is no higher layer driver API. We don’t want to litter the kernel > with ‘valve’ drivers, there’s no such thing. There’s only digital I/Os > that’s connected to low speed devices. > > If we are going for a higher layer API that would be something very thin > like a user-space GPIO driver. Do you see the conundrum? Could you maybe point me to references of the hardware (both the devices connected to the GPIOs and the board itself if the specs are available) so I can understand the issue fully? > >>>> >>>> >>>> Instead, have a shell script or a systemd unit tmpfile that will >>>> perform the correct setup at boot time. Then you can easily switch >>>> usages from user-space (systemctl stop sprinkler && systemctl start >>>> doordetect). This is much, much more flexible. At least until the >>>> Device Tree Overlays are merged, but even then this seems overkill. >>> >>> I seriously think about it, but I think if LEDs are in DT, GPIOs should >>> be there too. In other way, I don't like exporting without given name. >> >> LEDs are actually a perfect example of what you should do. They are a >> simple device often plugged to a GPIO that controls them. We could >> very well do what you propose: export GPIOs named "ledXX" and call it >> a day. Instead we have a driver that exports a proper device node in >> /sys with the operations that are relevant to a LED. >> >> Once your GPIOs are assigned to a given function, they are not >> "general purpose" anymore. Therefore you should expose the right >> abstraction to your users, even if that means writing some more code. > > Please, try to understand how our users use our linux kernel drivers. In that case my problem is precisely that there is no kernel driver. Anyway, the first question to answer is "is it reasonable to export and configure GPIOs from the Device Tree", and it concerns the DT maintainers mostly ; so let's hear what they think about this first. -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi all, Dne 24.10.2014 v 08:23 Alexandre Courbot napsal(a): > Added the DT maintainers and fixed the DT mailing-list's address so > this discussion becomes visible to them as well. > > On Thu, Oct 23, 2014 at 8:23 PM, Pantelis Antoniou > <panto@antoniou-consulting.com> wrote: >> Hi Alexandre, >> >>> On Oct 23, 2014, at 11:53 AM, Alexandre Courbot <gnurou@gmail.com> wrote: >>> >>> On Thu, Oct 23, 2014 at 3:23 PM, Jiří Prchal <jiri.prchal@aksignal.cz> wrote: >>>> >>>> >>>> Dne 23.10.2014 v 07:08 Alexandre Courbot napsal(a): >>>> >>>>> On Wed, Oct 22, 2014 at 6:58 PM, Pantelis Antoniou >>>>> <panto@antoniou-consulting.com> wrote: >>>>>> >>>>>> Hi Alexandre, >>>>>> >>>>>>> On Oct 22, 2014, at 12:41 PM, Alexandre Courbot <gnurou@gmail.com> >>>>>>> wrote: >>>>>>> >>>>>>> On Wed, Oct 22, 2014 at 6:24 PM, Pantelis Antoniou >>>>>>> <panto@antoniou-consulting.com> wrote: >>>>>>>> >>>>>>>> Hi Alexandre, >>>>>>>> >>>>>>>>> On Oct 22, 2014, at 12:18 PM, Alexandre Courbot <gnurou@gmail.com> >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>> On Wed, Oct 22, 2014 at 5:26 PM, Jiri Prchal <jiri.prchal@aksignal.cz> >>>>>>>>> wrote: >>>>>>>>>> >>>>>>>>>> This patch adds new driver "gpio-of-helper", witch has possibility to >>>>>>>>>> export >>>>>>>>>> gpios defined in dt. It exports them in defined name under >>>>>>>>>> /sysfs/class/gpio/name. >>>>>>>>>> It's rebased from Pantelis Antoniou patch to new kernel. >>>>>>>>>> Usage example: >>>>>>>>>> gpio { >>>>>>>>>> compatible = "gpio-of-helper"; >>>>>>>>>> status = "okay"; >>>>>>>>>> pinctrl-names = "default"; >>>>>>>>>> pinctrl-0 = <&pinctrl_gpio>; >>>>>>>>>> >>>>>>>>>> gsm_on { >>>>>>>>>> gpio-name = "gsm_on"; >>>>>>>>>> gpio = <&pioB 13 GPIO_ACTIVE_HIGH>; >>>>>>>>>> output; >>>>>>>>>> init-low; >>>>>>>>>> }; >>>>>>>>>> }; >>>>>>>>>> >>>>>>>>>> This patch needs Alexey Ignatov [PATCH] gpiolib: allow exporting >>>>>>>>>> gpios with >>>>>>>>>> custom names. >>>>>>>>> >>>>>>>>> >>>>>>>>> We will need to see whether the pre-requisite patch can get merged >>>>>>>>> first, but there are a couple of things that are wrong with your patch >>>>>>>>> anyway: >>>>>>>>> >>>>>>>>> - it is missing a bindings documentation >>>>>>>>> - it is using the legacy integer GPIOs instead of the descriptor >>>>>>>>> interface (see include/linux/gpio/consumer.h). Since this is DT-based, >>>>>>>>> there is absolutely no reason to not use the descriptors interface. >>>>>>>>> - it seems quite long for what it needs to do >>>>>>>>> - the MODULE_AUTHOR has not signed-off this patch (?) >>>>>>>>> >>>>>>>>> But what makes me nervous is that this encourages more usage of the >>>>>>>>> sysfs interface, an in a way that is potentially harmful. >>>>>>>>> >>>>>>>>> Also, I don't know if the DT people will be happy with this idea. >>>>>>>>> Since this concerns DT, please also add the devicetree list and get a >>>>>>>>> Acked-by for the bindings you want to push by a DT maintainer. >>>>>>>> >>>>>>>> >>>>>>>> Since I’m the original perpetrator, let me put a few words here for >>>>>>>> posterity. >>>>>>>> >>>>>>>> This patch was meant as a quick and dirty method for doing the >>>>>>>> automatic export >>>>>>>> and pinmux configuration from DT on a 3.8 kernel. The kernel APIs have >>>>>>>> moved on >>>>>>>> since. It wasn’t meant to be submitted for mainline right as it is. >>>>>>>> >>>>>>>> Unfortunately (or fortunately for many people) it does what a lot of >>>>>>>> people need >>>>>>>> i.e. configures a board with heavily pinmuxed GPIO right at boot time. >>>>>>>> >>>>>>>> I’m open at having a small discussion about how to do what the users of >>>>>>>> this patch >>>>>>>> do ‘right’. >>>>>>> >>>>>>> >>>>>>> Sure, although the discussion might turn out to not be that "small". :) >>>>>>> >>>>>> >>>>>> Heh ;) >>>>>> >>>>>>> Pinmux configuration sounds like a job for pinmux more than GPIO, and >>>>>>> exporting potentially many GPIOs to user-space sounds like a work for >>>>>>> a proper driver. >>>>>>> >>>>>> >>>>>> I’m afraid that’s not the case. A great many users do not require >>>>>> anything >>>>>> more than setting a pinmux and the GPIO configuration (input/output). >>>>>> They can then do low speed I/O using the sysfs interface, without having >>>>>> to >>>>>> use any complex APIs (shell works just fine). >>>>>> >>>>>> Think of stuff like controlling a sprinkler valve, or something like a >>>>>> mechanical >>>>>> door detection open state. >>>>> >>>>> >>>>> That sounds like any kind of arbitrary device that can be temporarily >>>>> connected to a set of GPIOs that will be bit-banged. Is my >>>>> interpretation correct? >>>> >>>> >>>> Not only temporarily. For example on/off switch of some hw on board like gsm >>>> modem. >>> >>> ... which should be handled in-kernel. >>> >> >> Not really. This is part of some kind user-space ABI but which has to do >> with the mapping of an named GPIO in the board schematics to user-space. >> >>>> Or some optocoupler isoleted in/outs for general use from userspace. >>> >>> ... which should thus be configured from userspace. >>> >> >> No. It is used from userspace, but the the configuration of such is performed >> by board specific means in the kernel. >> >>>>> >>>>> >>>>> In that case, I seriously doubt that this should be part of the DT. >>>>> Right now the DT is part of the firmware, and an immutable description >>>>> of the hardware layout of a board - definitely such devices do not fit >>>> >>>> This is our case of gpio. It's like LEDs, they are hw layout of board, >>>> but they are in/out instead of LEDs are only output. >>>>> >>>>> into that definition. This might change once the Device Tree Overlays >>>>> are merged, but we are not there yet. If the DT maintainers say this >>>>> is a valid use-case for overlays, then I will reconsider my position, >>>>> but in any case it really looks like this could/should be done from >>>>> user-space. >>>> >>>> I don't think so as I explained - HW layout of boerd. >>> >>> If it is a static hardware layout that makes the case even clearer - >>> these devices should have a proper kernel driver that configures these >>> GPIOs, and exports a proper interface to userspace, not just raw >>> GPIOs. >> >> Raw GPIOs is what the user-space application wants to use. There is no >> higher abstraction possible. > > Why can't the application use something else than raw GPIOs? Don't you > have its source code? > >> >> What these applications need are: >> >> 1. A way to map a GPIO to a name that is board invariant. Many boards >> move GPIOs around but the function typically stays the same. So for >> instance rev-0 might have GARAGE_DOOR to GPIO_A10, and on subsequent >> revision rev-1 it might be GARAGE_DOOR to GPIO_B08. The user-space >> application that controls everything does not want to deal with revisions >> ideally, just to know that the named GARAGE_DOOR gpio exists. > > This sounds pretty much like you are asking to register > application-specific configuration into the DT. Not application specific, it's board hw specific. > >> >> 2. A way to have the pinmux configuration of said GPIO configured without >> having to do anything explicit. I.e. on am335x the pinmux configuration is >> decoupled from the GPIO driver; having a GPIO configured as a GPIO does not >> alter the pinmux settings. > > Shouldn't that be fixed in the pinmux/GPIO kernel drivers? I am not > too familiar with pinmux, but IIUC pixmux configuration when GPIOs are > requested is something that is commonly done. > >> >> 3. The user-space facing API must be simple, preferably file based like the >> sysfs method right now. It is very simple to use even from within scripts. > > That we definitely agree, and as of today there are many ways to do this: > 1) A dedicated driver that exposes exactly the right interface to > sysfs. I know it may seem overkill, but we have drivers for other > simple things like GPIO-controlled LEDs. Let me ask: what is the difference between kernel exported gpios and user space? User-space interface is good enough for usage gpios, so why write other driver doing the same. What only we like is export board layout gpio from kernel using dt. > 2) If you think writing a driver is not worth the effort, you can > already export and configure (and I'm willing to discuss the > possibility to add "give names" to that list) GPIOs, from user-space. > > I understand that you want to keep the GPIO exporting/configuration > from the DT to have all the board-specific settings in one place, but > my understanding of the device tree is that it is supposed to say "I > have a garage door control device connected to this GPIO", not "export > this GPIO as input active-low, and name it GARAGE_DOOR". Exactly we like to export "out0" ... "out7". If user will use some of them for "GARAGE_DOOR" is up to him. But he knows that somewhere on some connector on the board is "out0" and he will find "out0" in "sysfs". Not strange "gpioA25" and set by default as input. Thats all what we need. > >> >>> >>>> In other way I'm looking forward for DT overlays. >>>>> >>>>> >>>>> You have almost all the necessary pieces: you can export, configure >>>>> and manipulate any GPIO from the sysfs interface. The only thing you >>>>> would be missing is a way to give a name to a GPIO, something that can >>>>> easily be fixed. >>>> >>>> But isn't it nice to have already exported gpios which are layouted out >>>> from board. All other unexported gpios aren't connected to anywhere on >>>> board. So user don't have to export or unexport anything. >>>>> >>>>> >>>>> Since the devices you want to configure that way are typically >>>>> removable or usage-specific, why would you want to store that >>>>> information all the way up into the firmware? A commonly accepted >>>>> wisdom is that what can be done in user-space should be done in >>>>> user-space, and it really looks like this applies here. >>>> >>>> They are NOT removable. >>>>> >>>>> >>>>> Say you buy a Jetson TK1 to control that sprinkler valve (a good use >>>>> for all these GPU cores!). The mainline DT has no knowledge about the >>>>> valve, so using your proposed way you will have to re-build and flash >>>>> a custom device tree just to be able to use your sprinkler. If you >>>>> decide to assign your board to something else, you will need another >>>>> DT. >>>> >>>> But if you buy more "real world ready" board with for example 8 power >>>> outputs for sprinkler valves, why has user think about what gpio to >>>> export, if there could be valve0 - 7 ready. If he will use only valve0 >>>> does not mater, all unused gpio can't be used for anything else since >>>> they has layout for valves. >>> >>> The problem is indeed different. >>> >>> If your sprinkler valves are wired on your board and the lines cannot >>> be used for anything else, then what you need is a driver for the >>> valve that will acquire the GPIOs and export its own userspace >>> interface under /sys/. >>> >>> You device tree must reflect the layout of the board ; in this case >>> "here is a valve that uses this GPIO active-low". Saying "this GPIO is >>> output active-low and named valve0" is not a good description of your >>> hardware. >>> >> >> There is no higher layer driver API. We don’t want to litter the kernel >> with ‘valve’ drivers, there’s no such thing. There’s only digital I/Os >> that’s connected to low speed devices. >> >> If we are going for a higher layer API that would be something very thin >> like a user-space GPIO driver. Do you see the conundrum? > > Could you maybe point me to references of the hardware (both the > devices connected to the GPIOs and the board itself if the specs are > available) so I can understand the issue fully? Short example: in13 { gpio-name = "in13"; input; gpio = <&pioC 18 GPIO_ACTIVE_LOW>; }; in14 { gpio-name = "in14"; input; gpio = <&pioC 20 GPIO_ACTIVE_LOW>; }; in17 { gpio-name = "in17"; input; gpio = <&pioC 16 GPIO_ACTIVE_LOW>; }; out18 { gpio-name = "out18"; gpio = <&pioA 30 GPIO_ACTIVE_HIGH>; output; init-low; }; out19 { gpio-name = "out19"; gpio = <&pioA 31 GPIO_ACTIVE_HIGH>; output; init-low; }; out20 { gpio-name = "out20"; gpio = <&pioA 29 GPIO_ACTIVE_HIGH>; output; init-low; }; >> >>>>> >>>>> >>>>> Instead, have a shell script or a systemd unit tmpfile that will >>>>> perform the correct setup at boot time. Then you can easily switch >>>>> usages from user-space (systemctl stop sprinkler && systemctl start >>>>> doordetect). This is much, much more flexible. At least until the >>>>> Device Tree Overlays are merged, but even then this seems overkill. >>>> >>>> I seriously think about it, but I think if LEDs are in DT, GPIOs should >>>> be there too. In other way, I don't like exporting without given name. >>> >>> LEDs are actually a perfect example of what you should do. They are a >>> simple device often plugged to a GPIO that controls them. We could >>> very well do what you propose: export GPIOs named "ledXX" and call it >>> a day. Instead we have a driver that exports a proper device node in >>> /sys with the operations that are relevant to a LED. >>> >>> Once your GPIOs are assigned to a given function, they are not >>> "general purpose" anymore. Therefore you should expose the right >>> abstraction to your users, even if that means writing some more code. >> >> Please, try to understand how our users use our linux kernel drivers. > > In that case my problem is precisely that there is no kernel driver. > > Anyway, the first question to answer is "is it reasonable to export > and configure GPIOs from the Device Tree", and it concerns the DT > maintainers mostly ; so let's hear what they think about this first. > -- > To unsubscribe from this list: send the line "unsubscribe devicetree" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Oct 22, 2014 at 11:58 AM, Pantelis Antoniou <panto@antoniou-consulting.com> wrote: >> On Oct 22, 2014, at 12:41 PM, Alexandre Courbot <gnurou@gmail.com> wrote: >> Pinmux configuration sounds like a job for pinmux more than GPIO, and >> exporting potentially many GPIOs to user-space sounds like a work for >> a proper driver. > > I’m afraid that’s not the case. A great many users (...) How many are many? How long is a piece of string? "Many" may refer to every system on the market or me and my friends. That claim has to be more specific. > do not require anything > more than setting a pinmux Which can be done (from device tree if need be) using pin control hogs. > and the GPIO configuration (input/output). And we have GPIO hogs for that in the pipe. > They can then do low speed I/O using the sysfs interface, without having to > use any complex APIs (shell works just fine). So what is added is export capability, which could be done by supplanting the GPIO hogging mechanism with some linux,export thing. > Think of stuff like controlling a sprinkler valve, or something like a mechanical > door detection open state. OK it's in the department of automatic control, which is arguably the only technical area where userspace GPIO handling actually makes sense. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Oct 24, 2014 at 8:23 AM, Alexandre Courbot <gnurou@gmail.com> wrote: > On Thu, Oct 23, 2014 at 8:23 PM, Pantelis Antoniou > <panto@antoniou-consulting.com> wrote: >> 2. A way to have the pinmux configuration of said GPIO configured without >> having to do anything explicit. I.e. on am335x the pinmux configuration is >> decoupled from the GPIO driver; having a GPIO configured as a GPIO does not >> alter the pinmux settings. > > Shouldn't that be fixed in the pinmux/GPIO kernel drivers? I am not > too familiar with pinmux, but IIUC pixmux configuration when GPIOs are > requested is something that is commonly done. The answer is in Documentation/pinctrl.txt: Pin control interaction with the GPIO subsystem =============================================== Note that the following implies that the use case is to use a certain pin from the Linux kernel using the API in <linux/gpio.h> with gpio_request() and similar functions. There are cases where you may be using something that your datasheet calls "GPIO mode", but actually is just an electrical configuration for a certain device. See the section below named "GPIO mode pitfalls" for more details on this scenario. The public pinmux API contains two functions named pinctrl_request_gpio() and pinctrl_free_gpio(). These two functions shall *ONLY* be called from gpiolib-based drivers as part of their gpio_request() and gpio_free() semantics. Likewise the pinctrl_gpio_direction_[input|output] shall only be called from within respective gpio_direction_[input|output] gpiolib implementation. NOTE that platforms and individual drivers shall *NOT* request GPIO pins to be controlled e.g. muxed in. Instead, implement a proper gpiolib driver and have that driver request proper muxing and other control for its pins. The function list could become long, especially if you can convert every individual pin into a GPIO pin independent of any other pins, and then try the approach to define every pin as a function. In this case, the function array would become 64 entries for each GPIO setting and then the device functions. For this reason there are two functions a pin control driver can implement to enable only GPIO on an individual pin: .gpio_request_enable() and .gpio_disable_free(). This function will pass in the affected GPIO range identified by the pin controller core, so you know which GPIO pins are being affected by the request operation. If your driver needs to have an indication from the framework of whether the GPIO pin shall be used for input or output you can implement the .gpio_set_direction() function. As described this shall be called from the gpiolib driver and the affected GPIO range, pin offset and desired direction will be passed along to this function. Alternatively to using these special functions, it is fully allowed to use named functions for each GPIO pin, the pinctrl_request_gpio() will attempt to obtain the function "gpioN" where "N" is the global GPIO pin number if no special GPIO-handler is registered. (...) >> There is no higher layer driver API. We don’t want to litter the kernel >> with ‘valve’ drivers, there’s no such thing. There’s only digital I/Os >> that’s connected to low speed devices. This is true. But we do want the kernel to handle: LEDs, switches, PWMs, and other simple, yet normally kernel-business things. Here is an example: /* The user LED on the board is set up to be used for heartbeat */ leds { compatible = "gpio-leds"; user-led { label = "user_led"; gpios = <&gpio0 2 0x1>; default-state = "off"; linux,default-trigger = "heartbeat"; pinctrl-names = "default"; pinctrl-0 = <&user_led_default_mode>; }; }; /* User key mapped in as "escape" */ gpio-keys { compatible = "gpio-keys"; user-button { label = "user_button"; gpios = <&gpio0 3 0x1>; linux,code = <1>; /* KEY_ESC */ gpio-key,wakeup; pinctrl-names = "default"; pinctrl-0 = <&user_button_default_mode>; }; }; This is just setting up already existing in-kernel drivers to use GPIOs as backend for a LED and a key (switch). Is someone prefers to handle LEDs or keys in userspace they are plain *WRONG* from our point of view. I agree that valves and doors should not have kernel drivers, that is not the issue. They should probably use sysfs. But for hardware the kernel normally handles and that is not exceptional, no way. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Linus, > On Oct 28, 2014, at 19:09 , Linus Walleij <linus.walleij@linaro.org> wrote: > > On Wed, Oct 22, 2014 at 11:58 AM, Pantelis Antoniou > <panto@antoniou-consulting.com> wrote: >>> On Oct 22, 2014, at 12:41 PM, Alexandre Courbot <gnurou@gmail.com> wrote: > >>> Pinmux configuration sounds like a job for pinmux more than GPIO, and >>> exporting potentially many GPIOs to user-space sounds like a work for >>> a proper driver. >> >> I’m afraid that’s not the case. A great many users (...) > > How many are many? How long is a piece of string? "Many" may > refer to every system on the market or me and my friends. > That claim has to be more specific. > On every embedded product board that I have worked on for a customer, invariably user-space GPIO was required for some odd pieces of hardware. I guess that’s in the dozen range for me, and Jiri that originally posted seems to be doing something similar too. >> do not require anything >> more than setting a pinmux > > Which can be done (from device tree if need be) using pin control > hogs. > >> and the GPIO configuration (input/output). > > And we have GPIO hogs for that in the pipe. > The original proposal was on January, and there’s a RFC posted just a few days back. I will take a look. >> They can then do low speed I/O using the sysfs interface, without having to >> use any complex APIs (shell works just fine). > > So what is added is export capability, which could be done by supplanting > the GPIO hogging mechanism with some linux,export thing. > Yes, that would be nice. >> Think of stuff like controlling a sprinkler valve, or something like a mechanical >> door detection open state. > > OK it's in the department of automatic control, which is arguably the > only technical area where userspace GPIO handling actually makes > sense. > Err, let’s just say that I completely disagree, but whatever. If the new GPIO hogging methods make it in mainline along with the export mechanism it’d be fine by me. > Yours, > Linus Walleij Regards — Pantelis -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi all, Dne 28.10.2014 v 18:28 Pantelis Antoniou napsal(a): > Hi Linus, > >> On Oct 28, 2014, at 19:09 , Linus Walleij <linus.walleij@linaro.org> wrote: >> >> On Wed, Oct 22, 2014 at 11:58 AM, Pantelis Antoniou >> <panto@antoniou-consulting.com> wrote: >>>> On Oct 22, 2014, at 12:41 PM, Alexandre Courbot <gnurou@gmail.com> wrote: >> >>>> Pinmux configuration sounds like a job for pinmux more than GPIO, and >>>> exporting potentially many GPIOs to user-space sounds like a work for >>>> a proper driver. >>> >>> I’m afraid that’s not the case. A great many users (...) >> >> How many are many? How long is a piece of string? "Many" may >> refer to every system on the market or me and my friends. >> That claim has to be more specific. >> > > On every embedded product board that I have worked on for a customer, > invariably user-space GPIO was required for some odd pieces of hardware. > > I guess that’s in the dozen range for me, and Jiri that originally posted > seems to be doing something similar too. Yes. > >>> do not require anything >>> more than setting a pinmux >> >> Which can be done (from device tree if need be) using pin control >> hogs. >> > >>> and the GPIO configuration (input/output). >> >> And we have GPIO hogs for that in the pipe. >> > > The original proposal was on January, and there’s a RFC posted just a few days back. > I will take a look. > >>> They can then do low speed I/O using the sysfs interface, without having to >>> use any complex APIs (shell works just fine). >> >> So what is added is export capability, which could be done by supplanting >> the GPIO hogging mechanism with some linux,export thing. >> > > Yes, that would be nice. That would be nice for me too, I'll try it later, now I have more urgent work. > >>> Think of stuff like controlling a sprinkler valve, or something like a mechanical >>> door detection open state. >> >> OK it's in the department of automatic control, which is arguably the >> only technical area where userspace GPIO handling actually makes >> sense. >> > > Err, let’s just say that I completely disagree, but whatever. > > If the new GPIO hogging methods make it in mainline along with the export > mechanism it’d be fine by me. > >> Yours, >> Linus Walleij > > Regards > > — Pantelis Yours Jiri > > -- > To unsubscribe from this list: send the line "unsubscribe linux-gpio" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 9de1515..3ec127f 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -85,6 +85,20 @@ config GPIO_SYSFS Kernel drivers may also request that a particular GPIO be exported to userspace; this can be useful when debugging. +config GPIO_OF_HELPER + bool "GPIO OF helper device" + depends on OF_GPIO + help + Say Y here to add an GPIO OF helper driver + + Allows you specify a GPIO helper based on OF + which allows simple export of GPIO functionality + in user-space. + + Features include, value set/get, direction control, + interrupt/value change poll support, event counting + and others. + config GPIO_GENERIC tristate diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile index 5d024e3..6708880 100644 --- a/drivers/gpio/Makefile +++ b/drivers/gpio/Makefile @@ -8,6 +8,7 @@ obj-$(CONFIG_GPIOLIB) += gpiolib-legacy.o obj-$(CONFIG_OF_GPIO) += gpiolib-of.o obj-$(CONFIG_GPIO_SYSFS) += gpiolib-sysfs.o obj-$(CONFIG_GPIO_ACPI) += gpiolib-acpi.o +obj-$(CONFIG_GPIO_OF_HELPER) += gpio-of-helper.o # Device drivers. Generally keep list sorted alphabetically obj-$(CONFIG_GPIO_GENERIC) += gpio-generic.o diff --git a/drivers/gpio/gpio-of-helper.c b/drivers/gpio/gpio-of-helper.c new file mode 100644 index 0000000..4e6f2e2 --- /dev/null +++ b/drivers/gpio/gpio-of-helper.c @@ -0,0 +1,423 @@ +/* + * GPIO OF based helper + * + * A simple DT based driver to provide access to GPIO functionality + * to user-space via sysfs. + * + * Copyright (C) 2013 Pantelis Antoniou <panto@antoniou-consulting.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. + */ + +#include <linux/module.h> +#include <linux/kernel.h> +#include <linux/string.h> +#include <linux/timer.h> +#include <linux/errno.h> +#include <linux/init.h> +#include <linux/delay.h> +#include <linux/bitops.h> +#include <linux/err.h> +#include <linux/of.h> +#include <linux/of_device.h> +#include <linux/of_gpio.h> +#include <linux/pinctrl/pinctrl.h> +#include <linux/pinctrl/pinmux.h> +#include <linux/pinctrl/consumer.h> +#include <linux/atomic.h> +#include <linux/clk.h> +#include <linux/interrupt.h> +#include <linux/math64.h> +#include <linux/atomic.h> +#include <linux/idr.h> + +/* fwd decl. */ +struct gpio_of_helper_info; + +enum gpio_type { + GPIO_TYPE_INPUT = 0, + GPIO_TYPE_OUTPUT = 1, +}; + +struct gpio_of_entry { + int id; + struct gpio_of_helper_info *info; + struct device_node *node; + enum gpio_type type; + int gpio; + enum of_gpio_flags gpio_flags; + int irq; + const char *name; + atomic64_t counter; + unsigned int count_flags; +#define COUNT_RISING_EDGE (1 << 0) +#define COUNT_FALLING_EDGE (1 << 1) +}; + +struct gpio_of_helper_info { + struct platform_device *pdev; + struct idr idr; +}; + +static DEFINE_IDR(gpio_of_helper_idr); +static DEFINE_SPINLOCK(gpio_of_helper_lock); + +static const struct of_device_id gpio_of_helper_of_match[] = { + { .compatible = "gpio-of-helper", }, + { }, +}; +MODULE_DEVICE_TABLE(of, gpio_of_helper_of_match); + +static ssize_t gpio_of_helper_show_status(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct platform_device *pdev = to_platform_device(dev); + struct gpio_of_helper_info *info = platform_get_drvdata(pdev); + struct gpio_of_entry *entry; + char *p, *e; + int id, n; + + p = buf; + e = p + PAGE_SIZE; + n = 0; + idr_for_each_entry(&info->idr, entry, id) { + switch (entry->type) { + case GPIO_TYPE_INPUT: + n = snprintf(p, e - p, "%2d %-24s %3d %-3s %llu\n", + entry->id, entry->name, entry->gpio, "IN", + (unsigned long long) + atomic64_read(&entry->counter)); + break; + case GPIO_TYPE_OUTPUT: + n = snprintf(p, e - p, "%2d %-24s %3d %-3s\n", + entry->id, entry->name, entry->gpio, "OUT"); + break; + } + p += n; + } + + return p - buf; +} + +static DEVICE_ATTR(status, S_IRUGO, + gpio_of_helper_show_status, NULL); + +static irqreturn_t gpio_of_helper_handler(int irq, void *ptr) +{ + struct gpio_of_entry *entry = ptr; + + /* caution - low speed interfaces only! */ + atomic64_inc(&entry->counter); + + return IRQ_HANDLED; +} + +static struct gpio_of_entry * +gpio_of_entry_create(struct gpio_of_helper_info *info, struct device_node *node) +{ + struct platform_device *pdev = info->pdev; + struct device *dev = &pdev->dev; + struct gpio_of_entry *entry; + int err, gpio, irq; + unsigned int req_flags, count_flags, irq_flags; + enum gpio_type type; + enum of_gpio_flags gpio_flags; + const char *name; + + /* get the type of the node first */ + if (of_property_read_bool(node, "input")) + type = GPIO_TYPE_INPUT; + else if (of_property_read_bool(node, "output")) + type = GPIO_TYPE_OUTPUT; + else { + dev_err(dev, "Not valid gpio node type\n"); + err = -EINVAL; + goto err_bad_node; + } + + /* get the name */ + err = of_property_read_string(node, "gpio-name", &name); + if (err != 0) { + dev_err(dev, "Failed to get name property\n"); + goto err_bad_node; + } + + err = of_get_named_gpio_flags(node, "gpio", 0, &gpio_flags); + if (IS_ERR_VALUE(err)) { + dev_err(dev, "Failed to get gpio property of '%s'\n", name); + goto err_bad_node; + } + gpio = err; + + if (!gpio_is_valid(gpio)) { + dev_err(dev, "Skipping unavailable gpio %d (%s)\n", + gpio, name); + goto err_bad_node; + } + + req_flags = 0; + count_flags = 0; + + /* set the request flags */ + switch (type) { + case GPIO_TYPE_INPUT: + req_flags = GPIOF_DIR_IN; + if (of_property_read_bool(node, "count-falling-edge")) + count_flags |= COUNT_FALLING_EDGE; + if (of_property_read_bool(node, "count-rising-edge")) + count_flags |= COUNT_RISING_EDGE; + break; + case GPIO_TYPE_OUTPUT: + req_flags = GPIOF_DIR_OUT; + if (of_property_read_bool(node, "init-high")) + req_flags |= GPIOF_OUT_INIT_HIGH; + else if (of_property_read_bool(node, "init-low")) + req_flags |= GPIOF_OUT_INIT_LOW; + break; + } + if (gpio_flags) + req_flags |= GPIOF_ACTIVE_LOW; + + /* request the gpio */ + err = devm_gpio_request_one(dev, gpio, req_flags, name); + if (err != 0) { + dev_err(dev, "Failed to request gpio '%s'\n", name); + goto err_bad_node; + } + + gpio_export_name(gpio, 0, name); + + irq = -1; + irq_flags = 0; + + /* counter mode requested - need an interrupt */ + if (count_flags != 0) { + irq = gpio_to_irq(gpio); + if (IS_ERR_VALUE(irq)) { + dev_err(dev, "Failed to request gpio '%s'\n", name); + goto err_bad_node; + } + + if (count_flags & COUNT_RISING_EDGE) + irq_flags |= IRQF_TRIGGER_RISING; + if (count_flags & COUNT_FALLING_EDGE) + irq_flags |= IRQF_TRIGGER_FALLING; + } + + entry = devm_kzalloc(dev, sizeof(*entry), GFP_KERNEL); + if (entry == NULL) { + dev_err(dev, "Failed to allocate gpio entry of '%s'\n", name); + err = -ENOMEM; + goto err_no_mem; + } + + entry->id = -1; + entry->info = info; + entry->node = of_node_get(node); /* get node reference */ + entry->type = type; + entry->gpio = gpio; + entry->gpio_flags = gpio_flags; + entry->irq = irq; + entry->name = name; + + idr_preload(GFP_KERNEL); + spin_lock(&gpio_of_helper_lock); + err = idr_alloc(&gpio_of_helper_idr, entry, 0, 0, GFP_NOWAIT); + if (err >= 0) + entry->id = err; + spin_unlock(&gpio_of_helper_lock); + idr_preload_end(); + if (err < 0) { + dev_err(dev, "Failed to idr_get_new of '%s'\n", name); + goto err_fail_idr; + } + + /* interrupt enable is last thing done */ + if (irq >= 0) { + atomic64_set(&entry->counter, 0); + entry->count_flags = count_flags; + err = devm_request_irq(dev, irq, gpio_of_helper_handler, + irq_flags, name, entry); + if (err != 0) { + dev_err(dev, "Failed to request irq of '%s'\n", name); + goto err_no_irq; + } + } + + dev_info(dev, "Allocated GPIO id=%d\n", entry->id); + + return entry; + +err_fail_idr: + /* nothing to do */ +err_no_irq: + /* release node ref */ + of_node_put(node); + /* nothing else needs to be done, devres handles it */ +err_no_mem: +err_bad_node: + return ERR_PTR(err); +} + +static int gpio_of_entry_destroy(struct gpio_of_entry *entry) +{ + struct gpio_of_helper_info *info = entry->info; + struct platform_device *pdev = info->pdev; + struct device *dev = &pdev->dev; + + dev_info(dev, "Destroying GPIO id=%d\n", entry->id); + + /* remove from the IDR */ + idr_remove(&info->idr, entry->id); + + /* remove node ref */ + of_node_put(entry->node); + + /* free gpio */ + devm_gpio_free(dev, entry->gpio); + + /* gree irq */ + if (entry->irq >= 0) + devm_free_irq(dev, entry->irq, entry); + + /* and free */ + devm_kfree(dev, entry); + + return 0; +} + +static int gpio_of_helper_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct gpio_of_helper_info *info; + struct gpio_of_entry *entry; + struct device_node *pnode = pdev->dev.of_node; + struct device_node *cnode; + struct pinctrl *pinctrl; + int err; + + /* we only support OF */ + if (pnode == NULL) { + dev_err(&pdev->dev, "No platform of_node!\n"); + return -ENODEV; + } + + pinctrl = devm_pinctrl_get_select_default(&pdev->dev); + if (IS_ERR(pinctrl)) { + /* special handling for probe defer */ + if (PTR_ERR(pinctrl) == -EPROBE_DEFER) + return -EPROBE_DEFER; + + dev_warn(&pdev->dev, + "pins are not configured from the driver\n"); + } + + info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL); + if (info == NULL) { + dev_err(&pdev->dev, "Failed to allocate info\n"); + err = -ENOMEM; + goto err_no_mem; + } + platform_set_drvdata(pdev, info); + info->pdev = pdev; + + idr_init(&info->idr); + + err = device_create_file(dev, &dev_attr_status); + if (err != 0) { + dev_err(dev, "Failed to create status sysfs attribute\n"); + goto err_no_sysfs; + } + + for_each_child_of_node(pnode, cnode) { + + entry = gpio_of_entry_create(info, cnode); + if (IS_ERR_OR_NULL(entry)) { + dev_err(dev, "Failed to create gpio entry\n"); + err = PTR_ERR(entry); + goto err_fail_entry; + } + } + + dev_info(&pdev->dev, "ready\n"); + + return 0; +err_fail_entry: + device_remove_file(&pdev->dev, &dev_attr_status); +err_no_sysfs: +err_no_mem: + return err; +} + +static int gpio_of_helper_remove(struct platform_device *pdev) +{ + struct gpio_of_helper_info *info = platform_get_drvdata(pdev); + struct gpio_of_entry *entry; + int id; + + dev_info(&pdev->dev, "removing\n"); + + device_remove_file(&pdev->dev, &dev_attr_status); + + id = 0; + idr_for_each_entry(&info->idr, entry, id) { + /* destroy each and every one */ + gpio_of_entry_destroy(entry); + } + + return 0; +} + +#ifdef CONFIG_PM +#ifdef CONFIG_PM_RUNTIME +static int gpio_of_helper_runtime_suspend(struct device *dev) +{ + /* place holder */ + return 0; +} + +static int gpio_of_helper_runtime_resume(struct device *dev) +{ + /* place holder */ + return 0; +} +#endif /* CONFIG_PM_RUNTIME */ + +static struct dev_pm_ops gpio_of_helper_pm_ops = { + SET_RUNTIME_PM_OPS(gpio_of_helper_runtime_suspend, + gpio_of_helper_runtime_resume, NULL) +}; +#define GPIO_OF_HELPER_PM_OPS (&gpio_of_helper_pm_ops) +#else +#define GPIO_OF_HELPER_PM_OPS NULL +#endif /* CONFIG_PM */ + +struct platform_driver gpio_of_helper_driver = { + .probe = gpio_of_helper_probe, + .remove = gpio_of_helper_remove, + .driver = { + .name = "gpio-of-helper", + .owner = THIS_MODULE, + .pm = GPIO_OF_HELPER_PM_OPS, + .of_match_table = gpio_of_helper_of_match, + }, +}; + +module_platform_driver(gpio_of_helper_driver); + +MODULE_AUTHOR("Pantelis Antoniou <panto@antoniou-consulting.com>"); +MODULE_DESCRIPTION("GPIO OF Helper driver"); +MODULE_LICENSE("GPL"); +MODULE_ALIAS("platform:gpio-of-helper");
This patch adds new driver "gpio-of-helper", witch has possibility to export gpios defined in dt. It exports them in defined name under /sysfs/class/gpio/name. It's rebased from Pantelis Antoniou patch to new kernel. Usage example: gpio { compatible = "gpio-of-helper"; status = "okay"; pinctrl-names = "default"; pinctrl-0 = <&pinctrl_gpio>; gsm_on { gpio-name = "gsm_on"; gpio = <&pioB 13 GPIO_ACTIVE_HIGH>; output; init-low; }; }; This patch needs Alexey Ignatov [PATCH] gpiolib: allow exporting gpios with custom names. Signed-off-by: Jiri Prchal <jiri.prchal@aksignal.cz> --- drivers/gpio/Kconfig | 14 ++ drivers/gpio/Makefile | 1 + drivers/gpio/gpio-of-helper.c | 423 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 438 insertions(+)