* Add support for monitoring gpio switches @ 2015-12-04 17:31 ` Martyn Welch 0 siblings, 0 replies; 53+ messages in thread From: Martyn Welch @ 2015-12-04 17:31 UTC (permalink / raw) To: Arnd Bergmann, Greg Kroah-Hartman Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Russell King, Kukjin Kim, Krzysztof Kozlowski, Martyn Welch, devicetree, linux-kernel, linux-arm-kernel, linux-samsung-soc, Olof Johansson This driver was written to expose a read only interface to a number of gpios on Chromebooks. These gpios are attached to signals which cause the firmware on Chromebooks to enter alternative modes of operation and/or control other device characteristics (such as write protection on flash devices). It was originally posted as "Add support for monitoring Chrome OS firmware signals". A request was made to make it more generic. In addition this patch series provides the required bindings for this to the peach-pi Chromebook. This is a new binding, but the driver is based (now some what loosely) on functionality in the kernel shipped on Chromebooks. ^ permalink raw reply [flat|nested] 53+ messages in thread
* Add support for monitoring gpio switches @ 2015-12-04 17:31 ` Martyn Welch 0 siblings, 0 replies; 53+ messages in thread From: Martyn Welch @ 2015-12-04 17:31 UTC (permalink / raw) To: linux-arm-kernel This driver was written to expose a read only interface to a number of gpios on Chromebooks. These gpios are attached to signals which cause the firmware on Chromebooks to enter alternative modes of operation and/or control other device characteristics (such as write protection on flash devices). It was originally posted as "Add support for monitoring Chrome OS firmware signals". A request was made to make it more generic. In addition this patch series provides the required bindings for this to the peach-pi Chromebook. This is a new binding, but the driver is based (now some what loosely) on functionality in the kernel shipped on Chromebooks. ^ permalink raw reply [flat|nested] 53+ messages in thread
* Add support for monitoring gpio switches @ 2015-12-04 17:31 ` Martyn Welch 0 siblings, 0 replies; 53+ messages in thread From: Martyn Welch @ 2015-12-04 17:31 UTC (permalink / raw) To: Arnd Bergmann, Greg Kroah-Hartman Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Russell King, Kukjin Kim, Krzysztof Kozlowski, Martyn Welch, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA, Olof Johansson This driver was written to expose a read only interface to a number of gpios on Chromebooks. These gpios are attached to signals which cause the firmware on Chromebooks to enter alternative modes of operation and/or control other device characteristics (such as write protection on flash devices). It was originally posted as "Add support for monitoring Chrome OS firmware signals". A request was made to make it more generic. In addition this patch series provides the required bindings for this to the peach-pi Chromebook. This is a new binding, but the driver is based (now some what loosely) on functionality in the kernel shipped on Chromebooks. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 1/3] Device tree binding documentation for gpio-switch 2015-12-04 17:31 ` Martyn Welch @ 2015-12-04 17:31 ` Martyn Welch -1 siblings, 0 replies; 53+ messages in thread From: Martyn Welch @ 2015-12-04 17:31 UTC (permalink / raw) To: Arnd Bergmann, Greg Kroah-Hartman Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Russell King, Kukjin Kim, Krzysztof Kozlowski, Martyn Welch, devicetree, linux-kernel, linux-arm-kernel, linux-samsung-soc, Olof Johansson This patch adds documentation for the gpio-switch binding. This binding provides a mechanism to bind named links to gpio, with the primary purpose of enabling standardised access to switches that might be standard across a group of devices but implemented differently on each device. Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk> --- .../devicetree/bindings/misc/gpio-switch.txt | 47 ++++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 Documentation/devicetree/bindings/misc/gpio-switch.txt diff --git a/Documentation/devicetree/bindings/misc/gpio-switch.txt b/Documentation/devicetree/bindings/misc/gpio-switch.txt new file mode 100644 index 0000000..13528bd --- /dev/null +++ b/Documentation/devicetree/bindings/misc/gpio-switch.txt @@ -0,0 +1,47 @@ +Device-Tree bindings for gpio attached switches. + +This provides a mechanism to provide a named link to specified gpios. This can +be useful in instances such as when theres a need to monitor a switch, which is +common across a family of devices, but attached to different gpios and even +implemented in different ways on differnet devices. + +Required properties: + - compatible = "gpio-switch"; + +Each signal is represented as a sub-node of "gpio-switch". The naming of +sub-nodes is arbitrary. + +Required sub-node properties: + + - label: Name to be given to gpio switch. + - gpios: OF device-tree gpio specification. + +Optional sub-node properties: + + - read-only: Boolean flag to mark the gpio as read-only, i.e. the line + should not be driven by the host. + +Example nodes: + + gpio-switch { + compatible = "gpio-switch"; + + write-protect { + label = "write-protect"; + gpios = <&gpx3 0 GPIO_ACTIVE_LOW>; + read-only; + }; + + developer-switch { + label = "developer-switch"; + gpios = <&gpx1 3 GPIO_ACTIVE_HIGH>; + read-only; + }; + + recovery-switch { + label = "recovery-switch"; + gpios = <&gpx0 7 GPIO_ACTIVE_LOW>; + read-only; + }; + }; + -- 2.1.4 ^ permalink raw reply related [flat|nested] 53+ messages in thread
* [PATCH 1/3] Device tree binding documentation for gpio-switch @ 2015-12-04 17:31 ` Martyn Welch 0 siblings, 0 replies; 53+ messages in thread From: Martyn Welch @ 2015-12-04 17:31 UTC (permalink / raw) To: linux-arm-kernel This patch adds documentation for the gpio-switch binding. This binding provides a mechanism to bind named links to gpio, with the primary purpose of enabling standardised access to switches that might be standard across a group of devices but implemented differently on each device. Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk> --- .../devicetree/bindings/misc/gpio-switch.txt | 47 ++++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 Documentation/devicetree/bindings/misc/gpio-switch.txt diff --git a/Documentation/devicetree/bindings/misc/gpio-switch.txt b/Documentation/devicetree/bindings/misc/gpio-switch.txt new file mode 100644 index 0000000..13528bd --- /dev/null +++ b/Documentation/devicetree/bindings/misc/gpio-switch.txt @@ -0,0 +1,47 @@ +Device-Tree bindings for gpio attached switches. + +This provides a mechanism to provide a named link to specified gpios. This can +be useful in instances such as when theres a need to monitor a switch, which is +common across a family of devices, but attached to different gpios and even +implemented in different ways on differnet devices. + +Required properties: + - compatible = "gpio-switch"; + +Each signal is represented as a sub-node of "gpio-switch". The naming of +sub-nodes is arbitrary. + +Required sub-node properties: + + - label: Name to be given to gpio switch. + - gpios: OF device-tree gpio specification. + +Optional sub-node properties: + + - read-only: Boolean flag to mark the gpio as read-only, i.e. the line + should not be driven by the host. + +Example nodes: + + gpio-switch { + compatible = "gpio-switch"; + + write-protect { + label = "write-protect"; + gpios = <&gpx3 0 GPIO_ACTIVE_LOW>; + read-only; + }; + + developer-switch { + label = "developer-switch"; + gpios = <&gpx1 3 GPIO_ACTIVE_HIGH>; + read-only; + }; + + recovery-switch { + label = "recovery-switch"; + gpios = <&gpx0 7 GPIO_ACTIVE_LOW>; + read-only; + }; + }; + -- 2.1.4 ^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: [PATCH 1/3] Device tree binding documentation for gpio-switch 2015-12-04 17:31 ` Martyn Welch @ 2015-12-07 17:37 ` Rob Herring -1 siblings, 0 replies; 53+ messages in thread From: Rob Herring @ 2015-12-07 17:37 UTC (permalink / raw) To: Martyn Welch, Linus Walleij Cc: Arnd Bergmann, Greg Kroah-Hartman, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Russell King, Kukjin Kim, Krzysztof Kozlowski, devicetree, linux-kernel, linux-arm-kernel, linux-samsung-soc, Olof Johansson +Linus W On Fri, Dec 04, 2015 at 05:31:13PM +0000, Martyn Welch wrote: > This patch adds documentation for the gpio-switch binding. This binding > provides a mechanism to bind named links to gpio, with the primary > purpose of enabling standardised access to switches that might be standard > across a group of devices but implemented differently on each device. This is good and what I suggested, but it now makes me wonder if switch is generic enough. This boils down to needing to expose single gpio lines to userspace with a defined function/use. IIRC, there's been some discussion about this before along with improving the userspace interface for GPIO in general. So I'd like to get Linus' thoughts on this. > Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk> > --- > .../devicetree/bindings/misc/gpio-switch.txt | 47 ++++++++++++++++++++++ > 1 file changed, 47 insertions(+) > create mode 100644 Documentation/devicetree/bindings/misc/gpio-switch.txt > > diff --git a/Documentation/devicetree/bindings/misc/gpio-switch.txt b/Documentation/devicetree/bindings/misc/gpio-switch.txt > new file mode 100644 > index 0000000..13528bd > --- /dev/null > +++ b/Documentation/devicetree/bindings/misc/gpio-switch.txt > @@ -0,0 +1,47 @@ > +Device-Tree bindings for gpio attached switches. > + > +This provides a mechanism to provide a named link to specified gpios. This can > +be useful in instances such as when theres a need to monitor a switch, which is > +common across a family of devices, but attached to different gpios and even > +implemented in different ways on differnet devices. > + > +Required properties: > + - compatible = "gpio-switch"; > + > +Each signal is represented as a sub-node of "gpio-switch". The naming of > +sub-nodes is arbitrary. > + > +Required sub-node properties: > + > + - label: Name to be given to gpio switch. > + - gpios: OF device-tree gpio specification. > + > +Optional sub-node properties: > + > + - read-only: Boolean flag to mark the gpio as read-only, i.e. the line > + should not be driven by the host. In terms a a switch use, allowing driving it would be an override of the switch. Is that the idea here? > + > +Example nodes: > + > + gpio-switch { > + compatible = "gpio-switch"; Both from a binding and driver perspective, there is no point in grouping these. Each node can simply have this compatible string. > + > + write-protect { > + label = "write-protect"; > + gpios = <&gpx3 0 GPIO_ACTIVE_LOW>; > + read-only; > + }; > + > + developer-switch { > + label = "developer-switch"; > + gpios = <&gpx1 3 GPIO_ACTIVE_HIGH>; > + read-only; > + }; > + > + recovery-switch { > + label = "recovery-switch"; > + gpios = <&gpx0 7 GPIO_ACTIVE_LOW>; > + read-only; > + }; > + }; > + > -- > 2.1.4 > ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 1/3] Device tree binding documentation for gpio-switch @ 2015-12-07 17:37 ` Rob Herring 0 siblings, 0 replies; 53+ messages in thread From: Rob Herring @ 2015-12-07 17:37 UTC (permalink / raw) To: linux-arm-kernel +Linus W On Fri, Dec 04, 2015 at 05:31:13PM +0000, Martyn Welch wrote: > This patch adds documentation for the gpio-switch binding. This binding > provides a mechanism to bind named links to gpio, with the primary > purpose of enabling standardised access to switches that might be standard > across a group of devices but implemented differently on each device. This is good and what I suggested, but it now makes me wonder if switch is generic enough. This boils down to needing to expose single gpio lines to userspace with a defined function/use. IIRC, there's been some discussion about this before along with improving the userspace interface for GPIO in general. So I'd like to get Linus' thoughts on this. > Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk> > --- > .../devicetree/bindings/misc/gpio-switch.txt | 47 ++++++++++++++++++++++ > 1 file changed, 47 insertions(+) > create mode 100644 Documentation/devicetree/bindings/misc/gpio-switch.txt > > diff --git a/Documentation/devicetree/bindings/misc/gpio-switch.txt b/Documentation/devicetree/bindings/misc/gpio-switch.txt > new file mode 100644 > index 0000000..13528bd > --- /dev/null > +++ b/Documentation/devicetree/bindings/misc/gpio-switch.txt > @@ -0,0 +1,47 @@ > +Device-Tree bindings for gpio attached switches. > + > +This provides a mechanism to provide a named link to specified gpios. This can > +be useful in instances such as when theres a need to monitor a switch, which is > +common across a family of devices, but attached to different gpios and even > +implemented in different ways on differnet devices. > + > +Required properties: > + - compatible = "gpio-switch"; > + > +Each signal is represented as a sub-node of "gpio-switch". The naming of > +sub-nodes is arbitrary. > + > +Required sub-node properties: > + > + - label: Name to be given to gpio switch. > + - gpios: OF device-tree gpio specification. > + > +Optional sub-node properties: > + > + - read-only: Boolean flag to mark the gpio as read-only, i.e. the line > + should not be driven by the host. In terms a a switch use, allowing driving it would be an override of the switch. Is that the idea here? > + > +Example nodes: > + > + gpio-switch { > + compatible = "gpio-switch"; Both from a binding and driver perspective, there is no point in grouping these. Each node can simply have this compatible string. > + > + write-protect { > + label = "write-protect"; > + gpios = <&gpx3 0 GPIO_ACTIVE_LOW>; > + read-only; > + }; > + > + developer-switch { > + label = "developer-switch"; > + gpios = <&gpx1 3 GPIO_ACTIVE_HIGH>; > + read-only; > + }; > + > + recovery-switch { > + label = "recovery-switch"; > + gpios = <&gpx0 7 GPIO_ACTIVE_LOW>; > + read-only; > + }; > + }; > + > -- > 2.1.4 > ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 1/3] Device tree binding documentation for gpio-switch 2015-12-07 17:37 ` Rob Herring @ 2015-12-07 21:10 ` Martyn Welch -1 siblings, 0 replies; 53+ messages in thread From: Martyn Welch @ 2015-12-07 21:10 UTC (permalink / raw) To: Rob Herring, Linus Walleij Cc: Arnd Bergmann, Greg Kroah-Hartman, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Russell King, Kukjin Kim, Krzysztof Kozlowski, devicetree, linux-kernel, linux-arm-kernel, linux-samsung-soc, Olof Johansson On 07/12/15 17:37, Rob Herring wrote: > +Linus W > > On Fri, Dec 04, 2015 at 05:31:13PM +0000, Martyn Welch wrote: >> This patch adds documentation for the gpio-switch binding. This binding >> provides a mechanism to bind named links to gpio, with the primary >> purpose of enabling standardised access to switches that might be standard >> across a group of devices but implemented differently on each device. > > This is good and what I suggested, but it now makes me wonder if switch > is generic enough. This boils down to needing to expose single gpio > lines to userspace with a defined function/use. IIRC, there's been some > discussion about this before along with improving the userspace > interface for GPIO in general. So I'd like to get Linus' thoughts on > this. > No problem. Rename gpio-signal? > >> Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk> >> --- >> .../devicetree/bindings/misc/gpio-switch.txt | 47 ++++++++++++++++++++++ >> 1 file changed, 47 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/misc/gpio-switch.txt >> >> diff --git a/Documentation/devicetree/bindings/misc/gpio-switch.txt b/Documentation/devicetree/bindings/misc/gpio-switch.txt >> new file mode 100644 >> index 0000000..13528bd >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/misc/gpio-switch.txt >> @@ -0,0 +1,47 @@ >> +Device-Tree bindings for gpio attached switches. >> + >> +This provides a mechanism to provide a named link to specified gpios. This can >> +be useful in instances such as when theres a need to monitor a switch, which is >> +common across a family of devices, but attached to different gpios and even >> +implemented in different ways on differnet devices. >> + >> +Required properties: >> + - compatible = "gpio-switch"; >> + >> +Each signal is represented as a sub-node of "gpio-switch". The naming of >> +sub-nodes is arbitrary. >> + >> +Required sub-node properties: >> + >> + - label: Name to be given to gpio switch. >> + - gpios: OF device-tree gpio specification. >> + >> +Optional sub-node properties: >> + >> + - read-only: Boolean flag to mark the gpio as read-only, i.e. the line >> + should not be driven by the host. > > In terms a a switch use, allowing driving it would be an override of the > switch. Is that the idea here? > Yeah - since it had become a lot more generic and a lot of switches/signals would probably be implemented with a pull-up resistor of something like that, it seemed to make sense to allow them to be driven as well. >> + >> +Example nodes: >> + >> + gpio-switch { >> + compatible = "gpio-switch"; > > Both from a binding and driver perspective, there is no point in > grouping these. Each node can simply have this compatible string. > True. I did it this way as this is how gpio-keys is implemented. OK, that has one optional parameter (autorepeat) for the block and this has none. Though I can also see that these probably have less in common than the individual lines used in gpio-keys. >> + >> + write-protect { >> + label = "write-protect"; >> + gpios = <&gpx3 0 GPIO_ACTIVE_LOW>; >> + read-only; >> + }; >> + >> + developer-switch { >> + label = "developer-switch"; >> + gpios = <&gpx1 3 GPIO_ACTIVE_HIGH>; >> + read-only; >> + }; >> + >> + recovery-switch { >> + label = "recovery-switch"; >> + gpios = <&gpx0 7 GPIO_ACTIVE_LOW>; >> + read-only; >> + }; >> + }; >> + >> -- >> 2.1.4 >> ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 1/3] Device tree binding documentation for gpio-switch @ 2015-12-07 21:10 ` Martyn Welch 0 siblings, 0 replies; 53+ messages in thread From: Martyn Welch @ 2015-12-07 21:10 UTC (permalink / raw) To: linux-arm-kernel On 07/12/15 17:37, Rob Herring wrote: > +Linus W > > On Fri, Dec 04, 2015 at 05:31:13PM +0000, Martyn Welch wrote: >> This patch adds documentation for the gpio-switch binding. This binding >> provides a mechanism to bind named links to gpio, with the primary >> purpose of enabling standardised access to switches that might be standard >> across a group of devices but implemented differently on each device. > > This is good and what I suggested, but it now makes me wonder if switch > is generic enough. This boils down to needing to expose single gpio > lines to userspace with a defined function/use. IIRC, there's been some > discussion about this before along with improving the userspace > interface for GPIO in general. So I'd like to get Linus' thoughts on > this. > No problem. Rename gpio-signal? > >> Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk> >> --- >> .../devicetree/bindings/misc/gpio-switch.txt | 47 ++++++++++++++++++++++ >> 1 file changed, 47 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/misc/gpio-switch.txt >> >> diff --git a/Documentation/devicetree/bindings/misc/gpio-switch.txt b/Documentation/devicetree/bindings/misc/gpio-switch.txt >> new file mode 100644 >> index 0000000..13528bd >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/misc/gpio-switch.txt >> @@ -0,0 +1,47 @@ >> +Device-Tree bindings for gpio attached switches. >> + >> +This provides a mechanism to provide a named link to specified gpios. This can >> +be useful in instances such as when theres a need to monitor a switch, which is >> +common across a family of devices, but attached to different gpios and even >> +implemented in different ways on differnet devices. >> + >> +Required properties: >> + - compatible = "gpio-switch"; >> + >> +Each signal is represented as a sub-node of "gpio-switch". The naming of >> +sub-nodes is arbitrary. >> + >> +Required sub-node properties: >> + >> + - label: Name to be given to gpio switch. >> + - gpios: OF device-tree gpio specification. >> + >> +Optional sub-node properties: >> + >> + - read-only: Boolean flag to mark the gpio as read-only, i.e. the line >> + should not be driven by the host. > > In terms a a switch use, allowing driving it would be an override of the > switch. Is that the idea here? > Yeah - since it had become a lot more generic and a lot of switches/signals would probably be implemented with a pull-up resistor of something like that, it seemed to make sense to allow them to be driven as well. >> + >> +Example nodes: >> + >> + gpio-switch { >> + compatible = "gpio-switch"; > > Both from a binding and driver perspective, there is no point in > grouping these. Each node can simply have this compatible string. > True. I did it this way as this is how gpio-keys is implemented. OK, that has one optional parameter (autorepeat) for the block and this has none. Though I can also see that these probably have less in common than the individual lines used in gpio-keys. >> + >> + write-protect { >> + label = "write-protect"; >> + gpios = <&gpx3 0 GPIO_ACTIVE_LOW>; >> + read-only; >> + }; >> + >> + developer-switch { >> + label = "developer-switch"; >> + gpios = <&gpx1 3 GPIO_ACTIVE_HIGH>; >> + read-only; >> + }; >> + >> + recovery-switch { >> + label = "recovery-switch"; >> + gpios = <&gpx0 7 GPIO_ACTIVE_LOW>; >> + read-only; >> + }; >> + }; >> + >> -- >> 2.1.4 >> ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 1/3] Device tree binding documentation for gpio-switch 2015-12-04 17:31 ` Martyn Welch (?) @ 2015-12-11 12:39 ` Linus Walleij -1 siblings, 0 replies; 53+ messages in thread From: Linus Walleij @ 2015-12-11 12:39 UTC (permalink / raw) To: Martyn Welch, Alexandre Courbot, Michael Welling, Markus Pargmann Cc: Arnd Bergmann, Greg Kroah-Hartman, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Russell King, Kukjin Kim, Krzysztof Kozlowski, devicetree, linux-kernel, linux-arm-kernel, linux-samsung-soc, Olof Johansson, Johan Hovold On Fri, Dec 4, 2015 at 6:31 PM, Martyn Welch <martyn.welch@collabora.co.uk> wrote: > This patch adds documentation for the gpio-switch binding. This binding > provides a mechanism to bind named links to gpio, with the primary > purpose of enabling standardised access to switches that might be standard > across a group of devices but implemented differently on each device. > > Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk> As mentioned in the comment to the second patch, this solves the following generic problem: Expose a GPIO line to userspace using a specific name That means basically naming GPIO lines and marking them as "not used by the operating system". This is something that has been proposed before, and postponed because the kernel lacks the right infrastructure. Markus Pargmann also did a series that add initial values to hogs, which is the inverse usecase of this, where you want to *output* something by default, then maybe also make it available to userspace. So what we need to see here is a patch series that does all of these things: - Name lines - Sets them to initial values - Mark them as read-only - Mark them as "not used by the operating system" so that they can be default-exported to userspace. Making *USE* of this naming etc inside the Linux kernel > + gpio-switch { > + compatible = "gpio-switch"; > + > + write-protect { > + label = "write-protect"; > + gpios = <&gpx3 0 GPIO_ACTIVE_LOW>; > + read-only; > + }; This should not need new structures and nodes like this. It should be part of Documentation/devicetree/bindings/gpio/gpio.txt and put directly in the gpiochip node. Maybe as an extension of the existing hogs, but that has already been tried. While we can agree on a device tree binding, the kernel still needs major refactoring to actually expose named GPIOs to userspace, and that should be done using the new chardev, not with sysfs links. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 1/3] Device tree binding documentation for gpio-switch @ 2015-12-11 12:39 ` Linus Walleij 0 siblings, 0 replies; 53+ messages in thread From: Linus Walleij @ 2015-12-11 12:39 UTC (permalink / raw) To: linux-arm-kernel On Fri, Dec 4, 2015 at 6:31 PM, Martyn Welch <martyn.welch@collabora.co.uk> wrote: > This patch adds documentation for the gpio-switch binding. This binding > provides a mechanism to bind named links to gpio, with the primary > purpose of enabling standardised access to switches that might be standard > across a group of devices but implemented differently on each device. > > Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk> As mentioned in the comment to the second patch, this solves the following generic problem: Expose a GPIO line to userspace using a specific name That means basically naming GPIO lines and marking them as "not used by the operating system". This is something that has been proposed before, and postponed because the kernel lacks the right infrastructure. Markus Pargmann also did a series that add initial values to hogs, which is the inverse usecase of this, where you want to *output* something by default, then maybe also make it available to userspace. So what we need to see here is a patch series that does all of these things: - Name lines - Sets them to initial values - Mark them as read-only - Mark them as "not used by the operating system" so that they can be default-exported to userspace. Making *USE* of this naming etc inside the Linux kernel > + gpio-switch { > + compatible = "gpio-switch"; > + > + write-protect { > + label = "write-protect"; > + gpios = <&gpx3 0 GPIO_ACTIVE_LOW>; > + read-only; > + }; This should not need new structures and nodes like this. It should be part of Documentation/devicetree/bindings/gpio/gpio.txt and put directly in the gpiochip node. Maybe as an extension of the existing hogs, but that has already been tried. While we can agree on a device tree binding, the kernel still needs major refactoring to actually expose named GPIOs to userspace, and that should be done using the new chardev, not with sysfs links. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 1/3] Device tree binding documentation for gpio-switch @ 2015-12-11 12:39 ` Linus Walleij 0 siblings, 0 replies; 53+ messages in thread From: Linus Walleij @ 2015-12-11 12:39 UTC (permalink / raw) To: Martyn Welch, Alexandre Courbot, Michael Welling, Markus Pargmann Cc: Mark Rutland, devicetree, Krzysztof Kozlowski, linux-samsung-soc, Russell King, Arnd Bergmann, Pawel Moll, Ian Campbell, Greg Kroah-Hartman, linux-kernel, Johan Hovold, Rob Herring, Kukjin Kim, Kumar Gala, Olof Johansson, linux-arm-kernel On Fri, Dec 4, 2015 at 6:31 PM, Martyn Welch <martyn.welch@collabora.co.uk> wrote: > This patch adds documentation for the gpio-switch binding. This binding > provides a mechanism to bind named links to gpio, with the primary > purpose of enabling standardised access to switches that might be standard > across a group of devices but implemented differently on each device. > > Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk> As mentioned in the comment to the second patch, this solves the following generic problem: Expose a GPIO line to userspace using a specific name That means basically naming GPIO lines and marking them as "not used by the operating system". This is something that has been proposed before, and postponed because the kernel lacks the right infrastructure. Markus Pargmann also did a series that add initial values to hogs, which is the inverse usecase of this, where you want to *output* something by default, then maybe also make it available to userspace. So what we need to see here is a patch series that does all of these things: - Name lines - Sets them to initial values - Mark them as read-only - Mark them as "not used by the operating system" so that they can be default-exported to userspace. Making *USE* of this naming etc inside the Linux kernel > + gpio-switch { > + compatible = "gpio-switch"; > + > + write-protect { > + label = "write-protect"; > + gpios = <&gpx3 0 GPIO_ACTIVE_LOW>; > + read-only; > + }; This should not need new structures and nodes like this. It should be part of Documentation/devicetree/bindings/gpio/gpio.txt and put directly in the gpiochip node. Maybe as an extension of the existing hogs, but that has already been tried. While we can agree on a device tree binding, the kernel still needs major refactoring to actually expose named GPIOs to userspace, and that should be done using the new chardev, not with sysfs links. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 1/3] Device tree binding documentation for gpio-switch 2015-12-11 12:39 ` Linus Walleij (?) @ 2015-12-11 14:06 ` Rob Herring -1 siblings, 0 replies; 53+ messages in thread From: Rob Herring @ 2015-12-11 14:06 UTC (permalink / raw) To: Linus Walleij Cc: Martyn Welch, Alexandre Courbot, Michael Welling, Markus Pargmann, Arnd Bergmann, Greg Kroah-Hartman, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Russell King, Kukjin Kim, Krzysztof Kozlowski, devicetree, linux-kernel, linux-arm-kernel, linux-samsung-soc, Olof Johansson, Johan Hovold On Fri, Dec 11, 2015 at 6:39 AM, Linus Walleij <linus.walleij@linaro.org> wrote: > On Fri, Dec 4, 2015 at 6:31 PM, Martyn Welch > <martyn.welch@collabora.co.uk> wrote: > >> This patch adds documentation for the gpio-switch binding. This binding >> provides a mechanism to bind named links to gpio, with the primary >> purpose of enabling standardised access to switches that might be standard >> across a group of devices but implemented differently on each device. >> >> Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk> > > As mentioned in the comment to the second patch, this solves the > following generic problem: > > Expose a GPIO line to userspace using a specific name > > That means basically naming GPIO lines and marking them as > "not used by the operating system". > > This is something that has been proposed before, and postponed > because the kernel lacks the right infrastructure. That doesn't necessarily mean we can't define a binding. > Markus Pargmann also did a series that add initial values to > hogs, which is the inverse usecase of this, where you want to > *output* something by default, then maybe also make it available > to userspace. > > So what we need to see here is a patch series that does all of these > things: > > - Name lines > > - Sets them to initial values > > - Mark them as read-only > > - Mark them as "not used by the operating system" so that they > can be default-exported to userspace. No! This should not be a DT property. Whether I want to control a GPIO in the kernel or userspace is not known and can change over time. It could simply depend on kernel config. There is also the case that a GPIO has no connection or kernel driver until some time later when a DT overlay for an expansion board is applied. Rob ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 1/3] Device tree binding documentation for gpio-switch @ 2015-12-11 14:06 ` Rob Herring 0 siblings, 0 replies; 53+ messages in thread From: Rob Herring @ 2015-12-11 14:06 UTC (permalink / raw) To: linux-arm-kernel On Fri, Dec 11, 2015 at 6:39 AM, Linus Walleij <linus.walleij@linaro.org> wrote: > On Fri, Dec 4, 2015 at 6:31 PM, Martyn Welch > <martyn.welch@collabora.co.uk> wrote: > >> This patch adds documentation for the gpio-switch binding. This binding >> provides a mechanism to bind named links to gpio, with the primary >> purpose of enabling standardised access to switches that might be standard >> across a group of devices but implemented differently on each device. >> >> Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk> > > As mentioned in the comment to the second patch, this solves the > following generic problem: > > Expose a GPIO line to userspace using a specific name > > That means basically naming GPIO lines and marking them as > "not used by the operating system". > > This is something that has been proposed before, and postponed > because the kernel lacks the right infrastructure. That doesn't necessarily mean we can't define a binding. > Markus Pargmann also did a series that add initial values to > hogs, which is the inverse usecase of this, where you want to > *output* something by default, then maybe also make it available > to userspace. > > So what we need to see here is a patch series that does all of these > things: > > - Name lines > > - Sets them to initial values > > - Mark them as read-only > > - Mark them as "not used by the operating system" so that they > can be default-exported to userspace. No! This should not be a DT property. Whether I want to control a GPIO in the kernel or userspace is not known and can change over time. It could simply depend on kernel config. There is also the case that a GPIO has no connection or kernel driver until some time later when a DT overlay for an expansion board is applied. Rob ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 1/3] Device tree binding documentation for gpio-switch @ 2015-12-11 14:06 ` Rob Herring 0 siblings, 0 replies; 53+ messages in thread From: Rob Herring @ 2015-12-11 14:06 UTC (permalink / raw) To: Linus Walleij Cc: Martyn Welch, Alexandre Courbot, Michael Welling, Markus Pargmann, Arnd Bergmann, Greg Kroah-Hartman, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Russell King, Kukjin Kim, Krzysztof Kozlowski, devicetree, linux-kernel, linux-arm-kernel, linux-samsung-soc, Olof Johansson, Johan Hovold On Fri, Dec 11, 2015 at 6:39 AM, Linus Walleij <linus.walleij@linaro.org> wrote: > On Fri, Dec 4, 2015 at 6:31 PM, Martyn Welch > <martyn.welch@collabora.co.uk> wrote: > >> This patch adds documentation for the gpio-switch binding. This binding >> provides a mechanism to bind named links to gpio, with the primary >> purpose of enabling standardised access to switches that might be standard >> across a group of devices but implemented differently on each device. >> >> Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk> > > As mentioned in the comment to the second patch, this solves the > following generic problem: > > Expose a GPIO line to userspace using a specific name > > That means basically naming GPIO lines and marking them as > "not used by the operating system". > > This is something that has been proposed before, and postponed > because the kernel lacks the right infrastructure. That doesn't necessarily mean we can't define a binding. > Markus Pargmann also did a series that add initial values to > hogs, which is the inverse usecase of this, where you want to > *output* something by default, then maybe also make it available > to userspace. > > So what we need to see here is a patch series that does all of these > things: > > - Name lines > > - Sets them to initial values > > - Mark them as read-only > > - Mark them as "not used by the operating system" so that they > can be default-exported to userspace. No! This should not be a DT property. Whether I want to control a GPIO in the kernel or userspace is not known and can change over time. It could simply depend on kernel config. There is also the case that a GPIO has no connection or kernel driver until some time later when a DT overlay for an expansion board is applied. Rob ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 1/3] Device tree binding documentation for gpio-switch 2015-12-11 14:06 ` Rob Herring (?) @ 2015-12-14 14:28 ` Linus Walleij -1 siblings, 0 replies; 53+ messages in thread From: Linus Walleij @ 2015-12-14 14:28 UTC (permalink / raw) To: Rob Herring Cc: Martyn Welch, Alexandre Courbot, Michael Welling, Markus Pargmann, Arnd Bergmann, Greg Kroah-Hartman, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Russell King, Kukjin Kim, Krzysztof Kozlowski, devicetree, linux-kernel, linux-arm-kernel, linux-samsung-soc, Olof Johansson, Johan Hovold On Fri, Dec 11, 2015 at 3:06 PM, Rob Herring <robh+dt@kernel.org> wrote: > On Fri, Dec 11, 2015 at 6:39 AM, Linus Walleij <linus.walleij@linaro.org> wrote: >> On Fri, Dec 4, 2015 at 6:31 PM, Martyn Welch >> <martyn.welch@collabora.co.uk> wrote: >> >>> This patch adds documentation for the gpio-switch binding. This binding >>> provides a mechanism to bind named links to gpio, with the primary >>> purpose of enabling standardised access to switches that might be standard >>> across a group of devices but implemented differently on each device. >>> >>> Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk> >> >> As mentioned in the comment to the second patch, this solves the >> following generic problem: >> >> Expose a GPIO line to userspace using a specific name >> >> That means basically naming GPIO lines and marking them as >> "not used by the operating system". >> >> This is something that has been proposed before, and postponed >> because the kernel lacks the right infrastructure. > > That doesn't necessarily mean we can't define a binding. Yeah that's true. just saying that the other stuff was not merged for this reason, but then we can have a look at Markus' bindings in parallell, Markus can you repost them to this audience? Only the bindings I mean. >> Markus Pargmann also did a series that add initial values to >> hogs, which is the inverse usecase of this, where you want to >> *output* something by default, then maybe also make it available >> to userspace. >> >> So what we need to see here is a patch series that does all of these >> things: >> >> - Name lines >> >> - Sets them to initial values >> >> - Mark them as read-only >> >> - Mark them as "not used by the operating system" so that they >> can be default-exported to userspace. > > No! This should not be a DT property. > > Whether I want to control a GPIO in the kernel or userspace is not > known and can change over time. It could simply depend on kernel > config. There is also the case that a GPIO has no connection or kernel > driver until some time later when a DT overlay for an expansion board > is applied. That's correct. So from a DT point of view, what really matters is to give things a name, and the hogs and initvals syntax already has a structure for this that is in use (from Documentation/devicetree/bindings/gpio/gpio.txt): qe_pio_a: gpio-controller@1400 { compatible = "fsl,qe-pario-bank-a", "fsl,qe-pario-bank"; reg = <0x1400 0x18>; gpio-controller; #gpio-cells = <2>; line_b { gpio-hog; gpios = <6 0>; output-low; line-name = "foo-bar-gpio"; }; }; Markus use this also for initial values. That could easily be used in a budget version like this: line_b { /* Just naming */ gpios = <6 0>; line-name = "foo-bar-gpio"; }; That could grow kind of big though. Or maybe not? How many GPIO lines are actually properly named in a typical system? The problem is that naming is usually imposed by consumers (they are the ones who know how the line is used). And then I do not mean naming it after the pin on the capsule where it comes out as per the datasheet, but naming it after the actual use. Naming it after the hardware specs is something the operating system can do, in Linux' case by the array char *names; inside the struct gpio_chip and should not be part of the bindings IMO. I would rather imagine this is something used in a top-level board file naming it: "header-2-22" for the 22nd pin on some second header on my BeagleBone Black or something like that. And those may not be so vast in numbers so they could be named using this pattern. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 1/3] Device tree binding documentation for gpio-switch @ 2015-12-14 14:28 ` Linus Walleij 0 siblings, 0 replies; 53+ messages in thread From: Linus Walleij @ 2015-12-14 14:28 UTC (permalink / raw) To: linux-arm-kernel On Fri, Dec 11, 2015 at 3:06 PM, Rob Herring <robh+dt@kernel.org> wrote: > On Fri, Dec 11, 2015 at 6:39 AM, Linus Walleij <linus.walleij@linaro.org> wrote: >> On Fri, Dec 4, 2015 at 6:31 PM, Martyn Welch >> <martyn.welch@collabora.co.uk> wrote: >> >>> This patch adds documentation for the gpio-switch binding. This binding >>> provides a mechanism to bind named links to gpio, with the primary >>> purpose of enabling standardised access to switches that might be standard >>> across a group of devices but implemented differently on each device. >>> >>> Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk> >> >> As mentioned in the comment to the second patch, this solves the >> following generic problem: >> >> Expose a GPIO line to userspace using a specific name >> >> That means basically naming GPIO lines and marking them as >> "not used by the operating system". >> >> This is something that has been proposed before, and postponed >> because the kernel lacks the right infrastructure. > > That doesn't necessarily mean we can't define a binding. Yeah that's true. just saying that the other stuff was not merged for this reason, but then we can have a look at Markus' bindings in parallell, Markus can you repost them to this audience? Only the bindings I mean. >> Markus Pargmann also did a series that add initial values to >> hogs, which is the inverse usecase of this, where you want to >> *output* something by default, then maybe also make it available >> to userspace. >> >> So what we need to see here is a patch series that does all of these >> things: >> >> - Name lines >> >> - Sets them to initial values >> >> - Mark them as read-only >> >> - Mark them as "not used by the operating system" so that they >> can be default-exported to userspace. > > No! This should not be a DT property. > > Whether I want to control a GPIO in the kernel or userspace is not > known and can change over time. It could simply depend on kernel > config. There is also the case that a GPIO has no connection or kernel > driver until some time later when a DT overlay for an expansion board > is applied. That's correct. So from a DT point of view, what really matters is to give things a name, and the hogs and initvals syntax already has a structure for this that is in use (from Documentation/devicetree/bindings/gpio/gpio.txt): qe_pio_a: gpio-controller at 1400 { compatible = "fsl,qe-pario-bank-a", "fsl,qe-pario-bank"; reg = <0x1400 0x18>; gpio-controller; #gpio-cells = <2>; line_b { gpio-hog; gpios = <6 0>; output-low; line-name = "foo-bar-gpio"; }; }; Markus use this also for initial values. That could easily be used in a budget version like this: line_b { /* Just naming */ gpios = <6 0>; line-name = "foo-bar-gpio"; }; That could grow kind of big though. Or maybe not? How many GPIO lines are actually properly named in a typical system? The problem is that naming is usually imposed by consumers (they are the ones who know how the line is used). And then I do not mean naming it after the pin on the capsule where it comes out as per the datasheet, but naming it after the actual use. Naming it after the hardware specs is something the operating system can do, in Linux' case by the array char *names; inside the struct gpio_chip and should not be part of the bindings IMO. I would rather imagine this is something used in a top-level board file naming it: "header-2-22" for the 22nd pin on some second header on my BeagleBone Black or something like that. And those may not be so vast in numbers so they could be named using this pattern. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 1/3] Device tree binding documentation for gpio-switch @ 2015-12-14 14:28 ` Linus Walleij 0 siblings, 0 replies; 53+ messages in thread From: Linus Walleij @ 2015-12-14 14:28 UTC (permalink / raw) To: Rob Herring Cc: Martyn Welch, Alexandre Courbot, Michael Welling, Markus Pargmann, Arnd Bergmann, Greg Kroah-Hartman, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Russell King, Kukjin Kim, Krzysztof Kozlowski, devicetree, linux-kernel, linux-arm-kernel, linux-samsung-soc, Olof Johansson, Johan Hovold On Fri, Dec 11, 2015 at 3:06 PM, Rob Herring <robh+dt@kernel.org> wrote: > On Fri, Dec 11, 2015 at 6:39 AM, Linus Walleij <linus.walleij@linaro.org> wrote: >> On Fri, Dec 4, 2015 at 6:31 PM, Martyn Welch >> <martyn.welch@collabora.co.uk> wrote: >> >>> This patch adds documentation for the gpio-switch binding. This binding >>> provides a mechanism to bind named links to gpio, with the primary >>> purpose of enabling standardised access to switches that might be standard >>> across a group of devices but implemented differently on each device. >>> >>> Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk> >> >> As mentioned in the comment to the second patch, this solves the >> following generic problem: >> >> Expose a GPIO line to userspace using a specific name >> >> That means basically naming GPIO lines and marking them as >> "not used by the operating system". >> >> This is something that has been proposed before, and postponed >> because the kernel lacks the right infrastructure. > > That doesn't necessarily mean we can't define a binding. Yeah that's true. just saying that the other stuff was not merged for this reason, but then we can have a look at Markus' bindings in parallell, Markus can you repost them to this audience? Only the bindings I mean. >> Markus Pargmann also did a series that add initial values to >> hogs, which is the inverse usecase of this, where you want to >> *output* something by default, then maybe also make it available >> to userspace. >> >> So what we need to see here is a patch series that does all of these >> things: >> >> - Name lines >> >> - Sets them to initial values >> >> - Mark them as read-only >> >> - Mark them as "not used by the operating system" so that they >> can be default-exported to userspace. > > No! This should not be a DT property. > > Whether I want to control a GPIO in the kernel or userspace is not > known and can change over time. It could simply depend on kernel > config. There is also the case that a GPIO has no connection or kernel > driver until some time later when a DT overlay for an expansion board > is applied. That's correct. So from a DT point of view, what really matters is to give things a name, and the hogs and initvals syntax already has a structure for this that is in use (from Documentation/devicetree/bindings/gpio/gpio.txt): qe_pio_a: gpio-controller@1400 { compatible = "fsl,qe-pario-bank-a", "fsl,qe-pario-bank"; reg = <0x1400 0x18>; gpio-controller; #gpio-cells = <2>; line_b { gpio-hog; gpios = <6 0>; output-low; line-name = "foo-bar-gpio"; }; }; Markus use this also for initial values. That could easily be used in a budget version like this: line_b { /* Just naming */ gpios = <6 0>; line-name = "foo-bar-gpio"; }; That could grow kind of big though. Or maybe not? How many GPIO lines are actually properly named in a typical system? The problem is that naming is usually imposed by consumers (they are the ones who know how the line is used). And then I do not mean naming it after the pin on the capsule where it comes out as per the datasheet, but naming it after the actual use. Naming it after the hardware specs is something the operating system can do, in Linux' case by the array char *names; inside the struct gpio_chip and should not be part of the bindings IMO. I would rather imagine this is something used in a top-level board file naming it: "header-2-22" for the 22nd pin on some second header on my BeagleBone Black or something like that. And those may not be so vast in numbers so they could be named using this pattern. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 1/3] Device tree binding documentation for gpio-switch @ 2015-12-14 15:45 ` Rob Herring 0 siblings, 0 replies; 53+ messages in thread From: Rob Herring @ 2015-12-14 15:45 UTC (permalink / raw) To: Linus Walleij Cc: Martyn Welch, Alexandre Courbot, Michael Welling, Markus Pargmann, Arnd Bergmann, Greg Kroah-Hartman, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Russell King, Kukjin Kim, Krzysztof Kozlowski, devicetree, linux-kernel, linux-arm-kernel, linux-samsung-soc, Olof Johansson, Johan Hovold On Mon, Dec 14, 2015 at 8:28 AM, Linus Walleij <linus.walleij@linaro.org> wrote: > On Fri, Dec 11, 2015 at 3:06 PM, Rob Herring <robh+dt@kernel.org> wrote: >> On Fri, Dec 11, 2015 at 6:39 AM, Linus Walleij <linus.walleij@linaro.org> wrote: >>> On Fri, Dec 4, 2015 at 6:31 PM, Martyn Welch >>> <martyn.welch@collabora.co.uk> wrote: [...] >>> Markus Pargmann also did a series that add initial values to >>> hogs, which is the inverse usecase of this, where you want to >>> *output* something by default, then maybe also make it available >>> to userspace. >>> >>> So what we need to see here is a patch series that does all of these >>> things: >>> >>> - Name lines >>> >>> - Sets them to initial values >>> >>> - Mark them as read-only >>> >>> - Mark them as "not used by the operating system" so that they >>> can be default-exported to userspace. >> >> No! This should not be a DT property. >> >> Whether I want to control a GPIO in the kernel or userspace is not >> known and can change over time. It could simply depend on kernel >> config. There is also the case that a GPIO has no connection or kernel >> driver until some time later when a DT overlay for an expansion board >> is applied. > > That's correct. So from a DT point of view, what really matters is > to give things a name, and the hogs and initvals syntax already > has a structure for this that is in use > (from Documentation/devicetree/bindings/gpio/gpio.txt): > > qe_pio_a: gpio-controller@1400 { > compatible = "fsl,qe-pario-bank-a", "fsl,qe-pario-bank"; > reg = <0x1400 0x18>; > gpio-controller; > #gpio-cells = <2>; > > line_b { > gpio-hog; > gpios = <6 0>; > output-low; > line-name = "foo-bar-gpio"; > }; > }; > > Markus use this also for initial values. That could easily be used in > a budget version like this: > > line_b { > /* Just naming */ > gpios = <6 0>; > line-name = "foo-bar-gpio"; > }; > > That could grow kind of big though. Or maybe not? How many > GPIO lines are actually properly named in a typical system? We should limit it to GPIOs with no connection to another node. For example, I don't want to see a SD card detect in the list as that should be in the SD host node. However, that is hard to enforce and can change over time. Removing it would break userspace potentially. Of course if the kernel starts own a signal that userspace used, then that potentially breaks userspace regardless of the DT changing. OTOH, using GPIOs in userspace is kind of use at your own risk. The only real differences between this and Martyn's proposal are the location of the nodes and having a compatible string. A compatible string may be good to have. It indicates type of signal, but not specific use. Similar to how gpio-key compatible defines the function, but not what key code, or gpio-led does not say what color LED. A compatible here could cover switches, mode/id/revision strapping signals, jumpers, presence detect, etc. > The problem is that naming is usually imposed by consumers (they > are the ones who know how the line is used). > > And then I do not mean naming it after the pin on the capsule > where it comes out as per the datasheet, but > naming it after the actual use. Right. We need a way to query "I need the GPIO that does X" which works across boards. > Naming it after the hardware specs is something the operating > system can do, in Linux' case by the array char *names; inside the > struct gpio_chip and should not be part of the bindings IMO. Agreed. That just came up with STM32 gpio bindings[1]. > I would rather imagine this is something used in a top-level board > file naming it: "header-2-22" for the 22nd pin on some second header > on my BeagleBone Black or something like that. And those may not > be so vast in numbers so they could be named using this pattern. Exactly. This is one of the cases I care about. However, this is not really a function, but it is SOC independent at least. We also have to consider how to handle add-on boards. We probably need a connector node which can remap connector signals to host signals in order to have overlays independent of the host board DT. For GPIOs this is probably a gpio-map property similar to interrupt-map. The complicated part will be connectors that require pinmux setup of the host. Rob [1] https://lkml.org/lkml/2015/12/11/600 ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 1/3] Device tree binding documentation for gpio-switch @ 2015-12-14 15:45 ` Rob Herring 0 siblings, 0 replies; 53+ messages in thread From: Rob Herring @ 2015-12-14 15:45 UTC (permalink / raw) To: linux-arm-kernel On Mon, Dec 14, 2015 at 8:28 AM, Linus Walleij <linus.walleij@linaro.org> wrote: > On Fri, Dec 11, 2015 at 3:06 PM, Rob Herring <robh+dt@kernel.org> wrote: >> On Fri, Dec 11, 2015 at 6:39 AM, Linus Walleij <linus.walleij@linaro.org> wrote: >>> On Fri, Dec 4, 2015 at 6:31 PM, Martyn Welch >>> <martyn.welch@collabora.co.uk> wrote: [...] >>> Markus Pargmann also did a series that add initial values to >>> hogs, which is the inverse usecase of this, where you want to >>> *output* something by default, then maybe also make it available >>> to userspace. >>> >>> So what we need to see here is a patch series that does all of these >>> things: >>> >>> - Name lines >>> >>> - Sets them to initial values >>> >>> - Mark them as read-only >>> >>> - Mark them as "not used by the operating system" so that they >>> can be default-exported to userspace. >> >> No! This should not be a DT property. >> >> Whether I want to control a GPIO in the kernel or userspace is not >> known and can change over time. It could simply depend on kernel >> config. There is also the case that a GPIO has no connection or kernel >> driver until some time later when a DT overlay for an expansion board >> is applied. > > That's correct. So from a DT point of view, what really matters is > to give things a name, and the hogs and initvals syntax already > has a structure for this that is in use > (from Documentation/devicetree/bindings/gpio/gpio.txt): > > qe_pio_a: gpio-controller at 1400 { > compatible = "fsl,qe-pario-bank-a", "fsl,qe-pario-bank"; > reg = <0x1400 0x18>; > gpio-controller; > #gpio-cells = <2>; > > line_b { > gpio-hog; > gpios = <6 0>; > output-low; > line-name = "foo-bar-gpio"; > }; > }; > > Markus use this also for initial values. That could easily be used in > a budget version like this: > > line_b { > /* Just naming */ > gpios = <6 0>; > line-name = "foo-bar-gpio"; > }; > > That could grow kind of big though. Or maybe not? How many > GPIO lines are actually properly named in a typical system? We should limit it to GPIOs with no connection to another node. For example, I don't want to see a SD card detect in the list as that should be in the SD host node. However, that is hard to enforce and can change over time. Removing it would break userspace potentially. Of course if the kernel starts own a signal that userspace used, then that potentially breaks userspace regardless of the DT changing. OTOH, using GPIOs in userspace is kind of use at your own risk. The only real differences between this and Martyn's proposal are the location of the nodes and having a compatible string. A compatible string may be good to have. It indicates type of signal, but not specific use. Similar to how gpio-key compatible defines the function, but not what key code, or gpio-led does not say what color LED. A compatible here could cover switches, mode/id/revision strapping signals, jumpers, presence detect, etc. > The problem is that naming is usually imposed by consumers (they > are the ones who know how the line is used). > > And then I do not mean naming it after the pin on the capsule > where it comes out as per the datasheet, but > naming it after the actual use. Right. We need a way to query "I need the GPIO that does X" which works across boards. > Naming it after the hardware specs is something the operating > system can do, in Linux' case by the array char *names; inside the > struct gpio_chip and should not be part of the bindings IMO. Agreed. That just came up with STM32 gpio bindings[1]. > I would rather imagine this is something used in a top-level board > file naming it: "header-2-22" for the 22nd pin on some second header > on my BeagleBone Black or something like that. And those may not > be so vast in numbers so they could be named using this pattern. Exactly. This is one of the cases I care about. However, this is not really a function, but it is SOC independent at least. We also have to consider how to handle add-on boards. We probably need a connector node which can remap connector signals to host signals in order to have overlays independent of the host board DT. For GPIOs this is probably a gpio-map property similar to interrupt-map. The complicated part will be connectors that require pinmux setup of the host. Rob [1] https://lkml.org/lkml/2015/12/11/600 ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 1/3] Device tree binding documentation for gpio-switch @ 2015-12-14 15:45 ` Rob Herring 0 siblings, 0 replies; 53+ messages in thread From: Rob Herring @ 2015-12-14 15:45 UTC (permalink / raw) To: Linus Walleij Cc: Martyn Welch, Alexandre Courbot, Michael Welling, Markus Pargmann, Arnd Bergmann, Greg Kroah-Hartman, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Russell King, Kukjin Kim, Krzysztof Kozlowski, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-samsung-soc, Olof Johansson, Johan Hovold On Mon, Dec 14, 2015 at 8:28 AM, Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote: > On Fri, Dec 11, 2015 at 3:06 PM, Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote: >> On Fri, Dec 11, 2015 at 6:39 AM, Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote: >>> On Fri, Dec 4, 2015 at 6:31 PM, Martyn Welch >>> <martyn.welch-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org> wrote: [...] >>> Markus Pargmann also did a series that add initial values to >>> hogs, which is the inverse usecase of this, where you want to >>> *output* something by default, then maybe also make it available >>> to userspace. >>> >>> So what we need to see here is a patch series that does all of these >>> things: >>> >>> - Name lines >>> >>> - Sets them to initial values >>> >>> - Mark them as read-only >>> >>> - Mark them as "not used by the operating system" so that they >>> can be default-exported to userspace. >> >> No! This should not be a DT property. >> >> Whether I want to control a GPIO in the kernel or userspace is not >> known and can change over time. It could simply depend on kernel >> config. There is also the case that a GPIO has no connection or kernel >> driver until some time later when a DT overlay for an expansion board >> is applied. > > That's correct. So from a DT point of view, what really matters is > to give things a name, and the hogs and initvals syntax already > has a structure for this that is in use > (from Documentation/devicetree/bindings/gpio/gpio.txt): > > qe_pio_a: gpio-controller@1400 { > compatible = "fsl,qe-pario-bank-a", "fsl,qe-pario-bank"; > reg = <0x1400 0x18>; > gpio-controller; > #gpio-cells = <2>; > > line_b { > gpio-hog; > gpios = <6 0>; > output-low; > line-name = "foo-bar-gpio"; > }; > }; > > Markus use this also for initial values. That could easily be used in > a budget version like this: > > line_b { > /* Just naming */ > gpios = <6 0>; > line-name = "foo-bar-gpio"; > }; > > That could grow kind of big though. Or maybe not? How many > GPIO lines are actually properly named in a typical system? We should limit it to GPIOs with no connection to another node. For example, I don't want to see a SD card detect in the list as that should be in the SD host node. However, that is hard to enforce and can change over time. Removing it would break userspace potentially. Of course if the kernel starts own a signal that userspace used, then that potentially breaks userspace regardless of the DT changing. OTOH, using GPIOs in userspace is kind of use at your own risk. The only real differences between this and Martyn's proposal are the location of the nodes and having a compatible string. A compatible string may be good to have. It indicates type of signal, but not specific use. Similar to how gpio-key compatible defines the function, but not what key code, or gpio-led does not say what color LED. A compatible here could cover switches, mode/id/revision strapping signals, jumpers, presence detect, etc. > The problem is that naming is usually imposed by consumers (they > are the ones who know how the line is used). > > And then I do not mean naming it after the pin on the capsule > where it comes out as per the datasheet, but > naming it after the actual use. Right. We need a way to query "I need the GPIO that does X" which works across boards. > Naming it after the hardware specs is something the operating > system can do, in Linux' case by the array char *names; inside the > struct gpio_chip and should not be part of the bindings IMO. Agreed. That just came up with STM32 gpio bindings[1]. > I would rather imagine this is something used in a top-level board > file naming it: "header-2-22" for the 22nd pin on some second header > on my BeagleBone Black or something like that. And those may not > be so vast in numbers so they could be named using this pattern. Exactly. This is one of the cases I care about. However, this is not really a function, but it is SOC independent at least. We also have to consider how to handle add-on boards. We probably need a connector node which can remap connector signals to host signals in order to have overlays independent of the host board DT. For GPIOs this is probably a gpio-map property similar to interrupt-map. The complicated part will be connectors that require pinmux setup of the host. Rob [1] https://lkml.org/lkml/2015/12/11/600 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 1/3] Device tree binding documentation for gpio-switch 2015-12-14 15:45 ` Rob Herring @ 2015-12-15 9:09 ` Markus Pargmann -1 siblings, 0 replies; 53+ messages in thread From: Markus Pargmann @ 2015-12-15 9:09 UTC (permalink / raw) To: linux-arm-kernel Cc: Rob Herring, Linus Walleij, Mark Rutland, devicetree, Krzysztof Kozlowski, linux-samsung-soc, Pawel Moll, Martyn Welch, Arnd Bergmann, Ian Campbell, Greg Kroah-Hartman, Olof Johansson, linux-kernel, Johan Hovold, Kukjin Kim, Alexandre Courbot, Kumar Gala, Michael Welling, Russell King [-- Attachment #1: Type: text/plain, Size: 6716 bytes --] Hi, On Monday 14 December 2015 09:45:48 Rob Herring wrote: > On Mon, Dec 14, 2015 at 8:28 AM, Linus Walleij <linus.walleij@linaro.org> wrote: > > On Fri, Dec 11, 2015 at 3:06 PM, Rob Herring <robh+dt@kernel.org> wrote: > >> On Fri, Dec 11, 2015 at 6:39 AM, Linus Walleij <linus.walleij@linaro.org> wrote: > >>> On Fri, Dec 4, 2015 at 6:31 PM, Martyn Welch > >>> <martyn.welch@collabora.co.uk> wrote: > > [...] > > >>> Markus Pargmann also did a series that add initial values to > >>> hogs, which is the inverse usecase of this, where you want to > >>> *output* something by default, then maybe also make it available > >>> to userspace. > >>> > >>> So what we need to see here is a patch series that does all of these > >>> things: > >>> > >>> - Name lines > >>> > >>> - Sets them to initial values > >>> > >>> - Mark them as read-only > >>> > >>> - Mark them as "not used by the operating system" so that they > >>> can be default-exported to userspace. > >> > >> No! This should not be a DT property. > >> > >> Whether I want to control a GPIO in the kernel or userspace is not > >> known and can change over time. It could simply depend on kernel > >> config. There is also the case that a GPIO has no connection or kernel > >> driver until some time later when a DT overlay for an expansion board > >> is applied. > > > > That's correct. So from a DT point of view, what really matters is > > to give things a name, and the hogs and initvals syntax already > > has a structure for this that is in use > > (from Documentation/devicetree/bindings/gpio/gpio.txt): > > > > qe_pio_a: gpio-controller@1400 { > > compatible = "fsl,qe-pario-bank-a", "fsl,qe-pario-bank"; > > reg = <0x1400 0x18>; > > gpio-controller; > > #gpio-cells = <2>; > > > > line_b { > > gpio-hog; > > gpios = <6 0>; > > output-low; > > line-name = "foo-bar-gpio"; > > }; > > }; > > > > Markus use this also for initial values. That could easily be used in > > a budget version like this: > > > > line_b { > > /* Just naming */ > > gpios = <6 0>; > > line-name = "foo-bar-gpio"; > > }; > > > > That could grow kind of big though. Or maybe not? How many > > GPIO lines are actually properly named in a typical system? > > We should limit it to GPIOs with no connection to another node. For > example, I don't want to see a SD card detect in the list as that > should be in the SD host node. However, that is hard to enforce and > can change over time. Removing it would break userspace potentially. > Of course if the kernel starts own a signal that userspace used, then > that potentially breaks userspace regardless of the DT changing. OTOH, > using GPIOs in userspace is kind of use at your own risk. I see this a bit differently. I would really like to see the each GPIO having two different names: - GPIO label: This is the name of the GPIO line in the schematic for example - GPIO use (this is the current semantic of the GPIO name): The use of a GPIO, e.g. 'sd-card-detect', 'LED', ... I think it would be good to describe all GPIO labels in gpiochip subnodes as gpio-hogging introduced it. This would offer a use-independent naming. The use of the function could be defined in the device node that is using this gpio. As an example perhaps something like this: &gpiochip { some_interrupt { gpios = <4 0>; line-name = "some_interrupt_line"; }; line_b { gpios = <6 0>; line-name = "line-b"; }; }; randomswitch { compatible = "gpio-switch"; gpios = <&gpiochip 4 0>; use = "action-trigger"; read-only; }; Also this does seem kind of inconsistent with gpio-hogging and the proposed gpio-initval. gpio-hogging is defined in subnodes of the gpio chip while gpio-switches are not. As "gpio-switch" is not really any kind of device it would perhaps make sense to keep this consistent with gpio-hogging as well and define it in the same subnodes? I would be happy about any consistent way. > > The only real differences between this and Martyn's proposal are the > location of the nodes and having a compatible string. A compatible > string may be good to have. It indicates type of signal, but not > specific use. Similar to how gpio-key compatible defines the function, > but not what key code, or gpio-led does not say what color LED. A > compatible here could cover switches, mode/id/revision strapping > signals, jumpers, presence detect, etc. > > > The problem is that naming is usually imposed by consumers (they > > are the ones who know how the line is used). > > > > And then I do not mean naming it after the pin on the capsule > > where it comes out as per the datasheet, but > > naming it after the actual use. > > Right. We need a way to query "I need the GPIO that does X" which > works across boards. > > > Naming it after the hardware specs is something the operating > > system can do, in Linux' case by the array char *names; inside the > > struct gpio_chip and should not be part of the bindings IMO. > > Agreed. That just came up with STM32 gpio bindings[1]. > > > I would rather imagine this is something used in a top-level board > > file naming it: "header-2-22" for the 22nd pin on some second header > > on my BeagleBone Black or something like that. And those may not > > be so vast in numbers so they could be named using this pattern. > > Exactly. This is one of the cases I care about. However, this is not > really a function, but it is SOC independent at least. > > We also have to consider how to handle add-on boards. We probably need > a connector node which can remap connector signals to host signals in > order to have overlays independent of the host board DT. For GPIOs > this is probably a gpio-map property similar to interrupt-map. The > complicated part will be connectors that require pinmux setup of the > host. Also what about hotplugging gpio-chips? Is there any mechanism to let the 'gpio-switch' know that the GPIO is not there anymore? Best Regards, Markus -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 1/3] Device tree binding documentation for gpio-switch @ 2015-12-15 9:09 ` Markus Pargmann 0 siblings, 0 replies; 53+ messages in thread From: Markus Pargmann @ 2015-12-15 9:09 UTC (permalink / raw) To: linux-arm-kernel Hi, On Monday 14 December 2015 09:45:48 Rob Herring wrote: > On Mon, Dec 14, 2015 at 8:28 AM, Linus Walleij <linus.walleij@linaro.org> wrote: > > On Fri, Dec 11, 2015 at 3:06 PM, Rob Herring <robh+dt@kernel.org> wrote: > >> On Fri, Dec 11, 2015 at 6:39 AM, Linus Walleij <linus.walleij@linaro.org> wrote: > >>> On Fri, Dec 4, 2015 at 6:31 PM, Martyn Welch > >>> <martyn.welch@collabora.co.uk> wrote: > > [...] > > >>> Markus Pargmann also did a series that add initial values to > >>> hogs, which is the inverse usecase of this, where you want to > >>> *output* something by default, then maybe also make it available > >>> to userspace. > >>> > >>> So what we need to see here is a patch series that does all of these > >>> things: > >>> > >>> - Name lines > >>> > >>> - Sets them to initial values > >>> > >>> - Mark them as read-only > >>> > >>> - Mark them as "not used by the operating system" so that they > >>> can be default-exported to userspace. > >> > >> No! This should not be a DT property. > >> > >> Whether I want to control a GPIO in the kernel or userspace is not > >> known and can change over time. It could simply depend on kernel > >> config. There is also the case that a GPIO has no connection or kernel > >> driver until some time later when a DT overlay for an expansion board > >> is applied. > > > > That's correct. So from a DT point of view, what really matters is > > to give things a name, and the hogs and initvals syntax already > > has a structure for this that is in use > > (from Documentation/devicetree/bindings/gpio/gpio.txt): > > > > qe_pio_a: gpio-controller at 1400 { > > compatible = "fsl,qe-pario-bank-a", "fsl,qe-pario-bank"; > > reg = <0x1400 0x18>; > > gpio-controller; > > #gpio-cells = <2>; > > > > line_b { > > gpio-hog; > > gpios = <6 0>; > > output-low; > > line-name = "foo-bar-gpio"; > > }; > > }; > > > > Markus use this also for initial values. That could easily be used in > > a budget version like this: > > > > line_b { > > /* Just naming */ > > gpios = <6 0>; > > line-name = "foo-bar-gpio"; > > }; > > > > That could grow kind of big though. Or maybe not? How many > > GPIO lines are actually properly named in a typical system? > > We should limit it to GPIOs with no connection to another node. For > example, I don't want to see a SD card detect in the list as that > should be in the SD host node. However, that is hard to enforce and > can change over time. Removing it would break userspace potentially. > Of course if the kernel starts own a signal that userspace used, then > that potentially breaks userspace regardless of the DT changing. OTOH, > using GPIOs in userspace is kind of use at your own risk. I see this a bit differently. I would really like to see the each GPIO having two different names: - GPIO label: This is the name of the GPIO line in the schematic for example - GPIO use (this is the current semantic of the GPIO name): The use of a GPIO, e.g. 'sd-card-detect', 'LED', ... I think it would be good to describe all GPIO labels in gpiochip subnodes as gpio-hogging introduced it. This would offer a use-independent naming. The use of the function could be defined in the device node that is using this gpio. As an example perhaps something like this: &gpiochip { some_interrupt { gpios = <4 0>; line-name = "some_interrupt_line"; }; line_b { gpios = <6 0>; line-name = "line-b"; }; }; randomswitch { compatible = "gpio-switch"; gpios = <&gpiochip 4 0>; use = "action-trigger"; read-only; }; Also this does seem kind of inconsistent with gpio-hogging and the proposed gpio-initval. gpio-hogging is defined in subnodes of the gpio chip while gpio-switches are not. As "gpio-switch" is not really any kind of device it would perhaps make sense to keep this consistent with gpio-hogging as well and define it in the same subnodes? I would be happy about any consistent way. > > The only real differences between this and Martyn's proposal are the > location of the nodes and having a compatible string. A compatible > string may be good to have. It indicates type of signal, but not > specific use. Similar to how gpio-key compatible defines the function, > but not what key code, or gpio-led does not say what color LED. A > compatible here could cover switches, mode/id/revision strapping > signals, jumpers, presence detect, etc. > > > The problem is that naming is usually imposed by consumers (they > > are the ones who know how the line is used). > > > > And then I do not mean naming it after the pin on the capsule > > where it comes out as per the datasheet, but > > naming it after the actual use. > > Right. We need a way to query "I need the GPIO that does X" which > works across boards. > > > Naming it after the hardware specs is something the operating > > system can do, in Linux' case by the array char *names; inside the > > struct gpio_chip and should not be part of the bindings IMO. > > Agreed. That just came up with STM32 gpio bindings[1]. > > > I would rather imagine this is something used in a top-level board > > file naming it: "header-2-22" for the 22nd pin on some second header > > on my BeagleBone Black or something like that. And those may not > > be so vast in numbers so they could be named using this pattern. > > Exactly. This is one of the cases I care about. However, this is not > really a function, but it is SOC independent at least. > > We also have to consider how to handle add-on boards. We probably need > a connector node which can remap connector signals to host signals in > order to have overlays independent of the host board DT. For GPIOs > this is probably a gpio-map property similar to interrupt-map. The > complicated part will be connectors that require pinmux setup of the > host. Also what about hotplugging gpio-chips? Is there any mechanism to let the 'gpio-switch' know that the GPIO is not there anymore? Best Regards, Markus -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: This is a digitally signed message part. URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20151215/6a2743a5/attachment-0001.sig> ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 1/3] Device tree binding documentation for gpio-switch 2015-12-15 9:09 ` Markus Pargmann (?) @ 2016-03-02 16:03 ` Rob Herring -1 siblings, 0 replies; 53+ messages in thread From: Rob Herring @ 2016-03-02 16:03 UTC (permalink / raw) To: Markus Pargmann Cc: linux-arm-kernel, Linus Walleij, Mark Rutland, devicetree, Krzysztof Kozlowski, linux-samsung-soc, Pawel Moll, Martyn Welch, Arnd Bergmann, Ian Campbell, Greg Kroah-Hartman, Olof Johansson, linux-kernel, Johan Hovold, Kukjin Kim, Alexandre Courbot, Kumar Gala, Michael Welling, Russell King Reviving this thread... On Tue, Dec 15, 2015 at 3:09 AM, Markus Pargmann <mpa@pengutronix.de> wrote: > Hi, > > On Monday 14 December 2015 09:45:48 Rob Herring wrote: >> On Mon, Dec 14, 2015 at 8:28 AM, Linus Walleij <linus.walleij@linaro.org> wrote: >> > On Fri, Dec 11, 2015 at 3:06 PM, Rob Herring <robh+dt@kernel.org> wrote: >> >> On Fri, Dec 11, 2015 at 6:39 AM, Linus Walleij <linus.walleij@linaro.org> wrote: >> >>> On Fri, Dec 4, 2015 at 6:31 PM, Martyn Welch >> >>> <martyn.welch@collabora.co.uk> wrote: >> >> [...] >> >> >>> Markus Pargmann also did a series that add initial values to >> >>> hogs, which is the inverse usecase of this, where you want to >> >>> *output* something by default, then maybe also make it available >> >>> to userspace. >> >>> >> >>> So what we need to see here is a patch series that does all of these >> >>> things: >> >>> >> >>> - Name lines >> >>> >> >>> - Sets them to initial values >> >>> >> >>> - Mark them as read-only >> >>> >> >>> - Mark them as "not used by the operating system" so that they >> >>> can be default-exported to userspace. >> >> >> >> No! This should not be a DT property. >> >> >> >> Whether I want to control a GPIO in the kernel or userspace is not >> >> known and can change over time. It could simply depend on kernel >> >> config. There is also the case that a GPIO has no connection or kernel >> >> driver until some time later when a DT overlay for an expansion board >> >> is applied. >> > >> > That's correct. So from a DT point of view, what really matters is >> > to give things a name, and the hogs and initvals syntax already >> > has a structure for this that is in use >> > (from Documentation/devicetree/bindings/gpio/gpio.txt): >> > >> > qe_pio_a: gpio-controller@1400 { >> > compatible = "fsl,qe-pario-bank-a", "fsl,qe-pario-bank"; >> > reg = <0x1400 0x18>; >> > gpio-controller; >> > #gpio-cells = <2>; >> > >> > line_b { >> > gpio-hog; >> > gpios = <6 0>; >> > output-low; >> > line-name = "foo-bar-gpio"; >> > }; >> > }; >> > >> > Markus use this also for initial values. That could easily be used in >> > a budget version like this: >> > >> > line_b { >> > /* Just naming */ >> > gpios = <6 0>; >> > line-name = "foo-bar-gpio"; >> > }; >> > >> > That could grow kind of big though. Or maybe not? How many >> > GPIO lines are actually properly named in a typical system? >> >> We should limit it to GPIOs with no connection to another node. For >> example, I don't want to see a SD card detect in the list as that >> should be in the SD host node. However, that is hard to enforce and >> can change over time. Removing it would break userspace potentially. >> Of course if the kernel starts own a signal that userspace used, then >> that potentially breaks userspace regardless of the DT changing. OTOH, >> using GPIOs in userspace is kind of use at your own risk. > > I see this a bit differently. I would really like to see the each GPIO having > two different names: I think we are saying the same thing... > - GPIO label: This is the name of the GPIO line in the schematic for example Yes. > - GPIO use (this is the current semantic of the GPIO name): The use of a GPIO, > e.g. 'sd-card-detect', 'LED', ... This should be determined from the compatible string and/or -gpios prefix. This is the what the function is and "label" is which one. > I think it would be good to describe all GPIO labels in gpiochip subnodes as > gpio-hogging introduced it. This would offer a use-independent naming. The use > of the function could be defined in the device node that is using this gpio. I think I agree here. You may have a defined function without any connection to another node. I also think we should encourage simple GPIO bindings like gpio-leds to be child nodes. Having them at the top-level is kind of arbitrary. Of course, allowing for both is required. > As an example perhaps something like this: > > &gpiochip { > some_interrupt { > gpios = <4 0>; > line-name = "some_interrupt_line"; > }; > > line_b { > gpios = <6 0>; > line-name = "line-b"; > }; > }; > > randomswitch { > compatible = "gpio-switch"; > gpios = <&gpiochip 4 0>; > use = "action-trigger"; > read-only; > }; > > Also this does seem kind of inconsistent with gpio-hogging and the proposed > gpio-initval. gpio-hogging is defined in subnodes of the gpio chip while > gpio-switches are not. As "gpio-switch" is not really any kind of device it > would perhaps make sense to keep this consistent with gpio-hogging as well and > define it in the same subnodes? > I would be happy about any consistent way. Yes, as well as gpio leds, keys, etc. bindings. The key is you would need to be able to start with something minimal and extend it with specific compatibles. [...] >> We also have to consider how to handle add-on boards. We probably need >> a connector node which can remap connector signals to host signals in >> order to have overlays independent of the host board DT. For GPIOs >> this is probably a gpio-map property similar to interrupt-map. The >> complicated part will be connectors that require pinmux setup of the >> host. > > Also what about hotplugging gpio-chips? Is there any mechanism to let the > 'gpio-switch' know that the GPIO is not there anymore? There are certainly issues around hotplug and GPIOs. If the gpio-switch device is a child of the gpio controller, then it should be possible. Rob ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 1/3] Device tree binding documentation for gpio-switch @ 2016-03-02 16:03 ` Rob Herring 0 siblings, 0 replies; 53+ messages in thread From: Rob Herring @ 2016-03-02 16:03 UTC (permalink / raw) To: linux-arm-kernel Reviving this thread... On Tue, Dec 15, 2015 at 3:09 AM, Markus Pargmann <mpa@pengutronix.de> wrote: > Hi, > > On Monday 14 December 2015 09:45:48 Rob Herring wrote: >> On Mon, Dec 14, 2015 at 8:28 AM, Linus Walleij <linus.walleij@linaro.org> wrote: >> > On Fri, Dec 11, 2015 at 3:06 PM, Rob Herring <robh+dt@kernel.org> wrote: >> >> On Fri, Dec 11, 2015 at 6:39 AM, Linus Walleij <linus.walleij@linaro.org> wrote: >> >>> On Fri, Dec 4, 2015 at 6:31 PM, Martyn Welch >> >>> <martyn.welch@collabora.co.uk> wrote: >> >> [...] >> >> >>> Markus Pargmann also did a series that add initial values to >> >>> hogs, which is the inverse usecase of this, where you want to >> >>> *output* something by default, then maybe also make it available >> >>> to userspace. >> >>> >> >>> So what we need to see here is a patch series that does all of these >> >>> things: >> >>> >> >>> - Name lines >> >>> >> >>> - Sets them to initial values >> >>> >> >>> - Mark them as read-only >> >>> >> >>> - Mark them as "not used by the operating system" so that they >> >>> can be default-exported to userspace. >> >> >> >> No! This should not be a DT property. >> >> >> >> Whether I want to control a GPIO in the kernel or userspace is not >> >> known and can change over time. It could simply depend on kernel >> >> config. There is also the case that a GPIO has no connection or kernel >> >> driver until some time later when a DT overlay for an expansion board >> >> is applied. >> > >> > That's correct. So from a DT point of view, what really matters is >> > to give things a name, and the hogs and initvals syntax already >> > has a structure for this that is in use >> > (from Documentation/devicetree/bindings/gpio/gpio.txt): >> > >> > qe_pio_a: gpio-controller at 1400 { >> > compatible = "fsl,qe-pario-bank-a", "fsl,qe-pario-bank"; >> > reg = <0x1400 0x18>; >> > gpio-controller; >> > #gpio-cells = <2>; >> > >> > line_b { >> > gpio-hog; >> > gpios = <6 0>; >> > output-low; >> > line-name = "foo-bar-gpio"; >> > }; >> > }; >> > >> > Markus use this also for initial values. That could easily be used in >> > a budget version like this: >> > >> > line_b { >> > /* Just naming */ >> > gpios = <6 0>; >> > line-name = "foo-bar-gpio"; >> > }; >> > >> > That could grow kind of big though. Or maybe not? How many >> > GPIO lines are actually properly named in a typical system? >> >> We should limit it to GPIOs with no connection to another node. For >> example, I don't want to see a SD card detect in the list as that >> should be in the SD host node. However, that is hard to enforce and >> can change over time. Removing it would break userspace potentially. >> Of course if the kernel starts own a signal that userspace used, then >> that potentially breaks userspace regardless of the DT changing. OTOH, >> using GPIOs in userspace is kind of use at your own risk. > > I see this a bit differently. I would really like to see the each GPIO having > two different names: I think we are saying the same thing... > - GPIO label: This is the name of the GPIO line in the schematic for example Yes. > - GPIO use (this is the current semantic of the GPIO name): The use of a GPIO, > e.g. 'sd-card-detect', 'LED', ... This should be determined from the compatible string and/or -gpios prefix. This is the what the function is and "label" is which one. > I think it would be good to describe all GPIO labels in gpiochip subnodes as > gpio-hogging introduced it. This would offer a use-independent naming. The use > of the function could be defined in the device node that is using this gpio. I think I agree here. You may have a defined function without any connection to another node. I also think we should encourage simple GPIO bindings like gpio-leds to be child nodes. Having them at the top-level is kind of arbitrary. Of course, allowing for both is required. > As an example perhaps something like this: > > &gpiochip { > some_interrupt { > gpios = <4 0>; > line-name = "some_interrupt_line"; > }; > > line_b { > gpios = <6 0>; > line-name = "line-b"; > }; > }; > > randomswitch { > compatible = "gpio-switch"; > gpios = <&gpiochip 4 0>; > use = "action-trigger"; > read-only; > }; > > Also this does seem kind of inconsistent with gpio-hogging and the proposed > gpio-initval. gpio-hogging is defined in subnodes of the gpio chip while > gpio-switches are not. As "gpio-switch" is not really any kind of device it > would perhaps make sense to keep this consistent with gpio-hogging as well and > define it in the same subnodes? > I would be happy about any consistent way. Yes, as well as gpio leds, keys, etc. bindings. The key is you would need to be able to start with something minimal and extend it with specific compatibles. [...] >> We also have to consider how to handle add-on boards. We probably need >> a connector node which can remap connector signals to host signals in >> order to have overlays independent of the host board DT. For GPIOs >> this is probably a gpio-map property similar to interrupt-map. The >> complicated part will be connectors that require pinmux setup of the >> host. > > Also what about hotplugging gpio-chips? Is there any mechanism to let the > 'gpio-switch' know that the GPIO is not there anymore? There are certainly issues around hotplug and GPIOs. If the gpio-switch device is a child of the gpio controller, then it should be possible. Rob ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 1/3] Device tree binding documentation for gpio-switch @ 2016-03-02 16:03 ` Rob Herring 0 siblings, 0 replies; 53+ messages in thread From: Rob Herring @ 2016-03-02 16:03 UTC (permalink / raw) To: Markus Pargmann Cc: linux-arm-kernel, Linus Walleij, Mark Rutland, devicetree, Krzysztof Kozlowski, linux-samsung-soc, Pawel Moll, Martyn Welch, Arnd Bergmann, Ian Campbell, Greg Kroah-Hartman, Olof Johansson, linux-kernel, Johan Hovold, Kukjin Kim, Alexandre Courbot, Kumar Gala, Michael Welling, Russell King Reviving this thread... On Tue, Dec 15, 2015 at 3:09 AM, Markus Pargmann <mpa@pengutronix.de> wrote: > Hi, > > On Monday 14 December 2015 09:45:48 Rob Herring wrote: >> On Mon, Dec 14, 2015 at 8:28 AM, Linus Walleij <linus.walleij@linaro.org> wrote: >> > On Fri, Dec 11, 2015 at 3:06 PM, Rob Herring <robh+dt@kernel.org> wrote: >> >> On Fri, Dec 11, 2015 at 6:39 AM, Linus Walleij <linus.walleij@linaro.org> wrote: >> >>> On Fri, Dec 4, 2015 at 6:31 PM, Martyn Welch >> >>> <martyn.welch@collabora.co.uk> wrote: >> >> [...] >> >> >>> Markus Pargmann also did a series that add initial values to >> >>> hogs, which is the inverse usecase of this, where you want to >> >>> *output* something by default, then maybe also make it available >> >>> to userspace. >> >>> >> >>> So what we need to see here is a patch series that does all of these >> >>> things: >> >>> >> >>> - Name lines >> >>> >> >>> - Sets them to initial values >> >>> >> >>> - Mark them as read-only >> >>> >> >>> - Mark them as "not used by the operating system" so that they >> >>> can be default-exported to userspace. >> >> >> >> No! This should not be a DT property. >> >> >> >> Whether I want to control a GPIO in the kernel or userspace is not >> >> known and can change over time. It could simply depend on kernel >> >> config. There is also the case that a GPIO has no connection or kernel >> >> driver until some time later when a DT overlay for an expansion board >> >> is applied. >> > >> > That's correct. So from a DT point of view, what really matters is >> > to give things a name, and the hogs and initvals syntax already >> > has a structure for this that is in use >> > (from Documentation/devicetree/bindings/gpio/gpio.txt): >> > >> > qe_pio_a: gpio-controller@1400 { >> > compatible = "fsl,qe-pario-bank-a", "fsl,qe-pario-bank"; >> > reg = <0x1400 0x18>; >> > gpio-controller; >> > #gpio-cells = <2>; >> > >> > line_b { >> > gpio-hog; >> > gpios = <6 0>; >> > output-low; >> > line-name = "foo-bar-gpio"; >> > }; >> > }; >> > >> > Markus use this also for initial values. That could easily be used in >> > a budget version like this: >> > >> > line_b { >> > /* Just naming */ >> > gpios = <6 0>; >> > line-name = "foo-bar-gpio"; >> > }; >> > >> > That could grow kind of big though. Or maybe not? How many >> > GPIO lines are actually properly named in a typical system? >> >> We should limit it to GPIOs with no connection to another node. For >> example, I don't want to see a SD card detect in the list as that >> should be in the SD host node. However, that is hard to enforce and >> can change over time. Removing it would break userspace potentially. >> Of course if the kernel starts own a signal that userspace used, then >> that potentially breaks userspace regardless of the DT changing. OTOH, >> using GPIOs in userspace is kind of use at your own risk. > > I see this a bit differently. I would really like to see the each GPIO having > two different names: I think we are saying the same thing... > - GPIO label: This is the name of the GPIO line in the schematic for example Yes. > - GPIO use (this is the current semantic of the GPIO name): The use of a GPIO, > e.g. 'sd-card-detect', 'LED', ... This should be determined from the compatible string and/or -gpios prefix. This is the what the function is and "label" is which one. > I think it would be good to describe all GPIO labels in gpiochip subnodes as > gpio-hogging introduced it. This would offer a use-independent naming. The use > of the function could be defined in the device node that is using this gpio. I think I agree here. You may have a defined function without any connection to another node. I also think we should encourage simple GPIO bindings like gpio-leds to be child nodes. Having them at the top-level is kind of arbitrary. Of course, allowing for both is required. > As an example perhaps something like this: > > &gpiochip { > some_interrupt { > gpios = <4 0>; > line-name = "some_interrupt_line"; > }; > > line_b { > gpios = <6 0>; > line-name = "line-b"; > }; > }; > > randomswitch { > compatible = "gpio-switch"; > gpios = <&gpiochip 4 0>; > use = "action-trigger"; > read-only; > }; > > Also this does seem kind of inconsistent with gpio-hogging and the proposed > gpio-initval. gpio-hogging is defined in subnodes of the gpio chip while > gpio-switches are not. As "gpio-switch" is not really any kind of device it > would perhaps make sense to keep this consistent with gpio-hogging as well and > define it in the same subnodes? > I would be happy about any consistent way. Yes, as well as gpio leds, keys, etc. bindings. The key is you would need to be able to start with something minimal and extend it with specific compatibles. [...] >> We also have to consider how to handle add-on boards. We probably need >> a connector node which can remap connector signals to host signals in >> order to have overlays independent of the host board DT. For GPIOs >> this is probably a gpio-map property similar to interrupt-map. The >> complicated part will be connectors that require pinmux setup of the >> host. > > Also what about hotplugging gpio-chips? Is there any mechanism to let the > 'gpio-switch' know that the GPIO is not there anymore? There are certainly issues around hotplug and GPIOs. If the gpio-switch device is a child of the gpio controller, then it should be possible. Rob ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 1/3] Device tree binding documentation for gpio-switch @ 2016-03-07 8:26 ` Markus Pargmann 0 siblings, 0 replies; 53+ messages in thread From: Markus Pargmann @ 2016-03-07 8:26 UTC (permalink / raw) To: Rob Herring Cc: linux-arm-kernel, Linus Walleij, Mark Rutland, devicetree, Krzysztof Kozlowski, linux-samsung-soc, Pawel Moll, Martyn Welch, Arnd Bergmann, Ian Campbell, Greg Kroah-Hartman, Olof Johansson, linux-kernel, Johan Hovold, Kukjin Kim, Alexandre Courbot, Kumar Gala, Michael Welling, Russell King [-- Attachment #1: Type: text/plain, Size: 7898 bytes --] On Wednesday 02 March 2016 10:03:27 Rob Herring wrote: > Reviving this thread... > > On Tue, Dec 15, 2015 at 3:09 AM, Markus Pargmann <mpa@pengutronix.de> wrote: > > Hi, > > > > On Monday 14 December 2015 09:45:48 Rob Herring wrote: > >> On Mon, Dec 14, 2015 at 8:28 AM, Linus Walleij <linus.walleij@linaro.org> wrote: > >> > On Fri, Dec 11, 2015 at 3:06 PM, Rob Herring <robh+dt@kernel.org> wrote: > >> >> On Fri, Dec 11, 2015 at 6:39 AM, Linus Walleij <linus.walleij@linaro.org> wrote: > >> >>> On Fri, Dec 4, 2015 at 6:31 PM, Martyn Welch > >> >>> <martyn.welch@collabora.co.uk> wrote: > >> > >> [...] > >> > >> >>> Markus Pargmann also did a series that add initial values to > >> >>> hogs, which is the inverse usecase of this, where you want to > >> >>> *output* something by default, then maybe also make it available > >> >>> to userspace. > >> >>> > >> >>> So what we need to see here is a patch series that does all of these > >> >>> things: > >> >>> > >> >>> - Name lines > >> >>> > >> >>> - Sets them to initial values > >> >>> > >> >>> - Mark them as read-only > >> >>> > >> >>> - Mark them as "not used by the operating system" so that they > >> >>> can be default-exported to userspace. > >> >> > >> >> No! This should not be a DT property. > >> >> > >> >> Whether I want to control a GPIO in the kernel or userspace is not > >> >> known and can change over time. It could simply depend on kernel > >> >> config. There is also the case that a GPIO has no connection or kernel > >> >> driver until some time later when a DT overlay for an expansion board > >> >> is applied. > >> > > >> > That's correct. So from a DT point of view, what really matters is > >> > to give things a name, and the hogs and initvals syntax already > >> > has a structure for this that is in use > >> > (from Documentation/devicetree/bindings/gpio/gpio.txt): > >> > > >> > qe_pio_a: gpio-controller@1400 { > >> > compatible = "fsl,qe-pario-bank-a", "fsl,qe-pario-bank"; > >> > reg = <0x1400 0x18>; > >> > gpio-controller; > >> > #gpio-cells = <2>; > >> > > >> > line_b { > >> > gpio-hog; > >> > gpios = <6 0>; > >> > output-low; > >> > line-name = "foo-bar-gpio"; > >> > }; > >> > }; > >> > > >> > Markus use this also for initial values. That could easily be used in > >> > a budget version like this: > >> > > >> > line_b { > >> > /* Just naming */ > >> > gpios = <6 0>; > >> > line-name = "foo-bar-gpio"; > >> > }; > >> > > >> > That could grow kind of big though. Or maybe not? How many > >> > GPIO lines are actually properly named in a typical system? > >> > >> We should limit it to GPIOs with no connection to another node. For > >> example, I don't want to see a SD card detect in the list as that > >> should be in the SD host node. However, that is hard to enforce and > >> can change over time. Removing it would break userspace potentially. > >> Of course if the kernel starts own a signal that userspace used, then > >> that potentially breaks userspace regardless of the DT changing. OTOH, > >> using GPIOs in userspace is kind of use at your own risk. > > > > I see this a bit differently. I would really like to see the each GPIO having > > two different names: > > I think we are saying the same thing... > > > - GPIO label: This is the name of the GPIO line in the schematic for example > > Yes. > > > - GPIO use (this is the current semantic of the GPIO name): The use of a GPIO, > > e.g. 'sd-card-detect', 'LED', ... > > This should be determined from the compatible string and/or -gpios > prefix. This is the what the function is and "label" is which one. > > > I think it would be good to describe all GPIO labels in gpiochip subnodes as > > gpio-hogging introduced it. This would offer a use-independent naming. The use > > of the function could be defined in the device node that is using this gpio. > > I think I agree here. You may have a defined function without any > connection to another node. I also think we should encourage simple > GPIO bindings like gpio-leds to be child nodes. Having them at the > top-level is kind of arbitrary. Of course, allowing for both is > required. > > > As an example perhaps something like this: > > > > &gpiochip { > > some_interrupt { > > gpios = <4 0>; > > line-name = "some_interrupt_line"; > > }; > > > > line_b { > > gpios = <6 0>; > > line-name = "line-b"; > > }; > > }; > > > > randomswitch { > > compatible = "gpio-switch"; > > gpios = <&gpiochip 4 0>; > > use = "action-trigger"; > > read-only; > > }; > > > > Also this does seem kind of inconsistent with gpio-hogging and the proposed > > gpio-initval. gpio-hogging is defined in subnodes of the gpio chip while > > gpio-switches are not. As "gpio-switch" is not really any kind of device it > > would perhaps make sense to keep this consistent with gpio-hogging as well and > > define it in the same subnodes? > > I would be happy about any consistent way. > > Yes, as well as gpio leds, keys, etc. bindings. The key is you would > need to be able to start with something minimal and extend it with > specific compatibles. So if I understand you right it would be better to handle most distinctions using compatibles? Something like this? &gpiochip { some_led { compatible = "gpio-leds"; default-state = "on"; gpios = <3 0>; line-name = "leda"; }; some_switch { compatible = "gpio-switch", "gpio-initval"; gpios = <4 0>; line-name = "switch1"; /* This is used by gpio-initval in case gpio-switch is not implemented */ output-low; }; some_interrupt { gpios = <5 0>; line-name = "some_interrupt_line"; }; line_b { gpios = <6 0>; line-name = "line-b"; }; }; randomcomponent { gpios = <&gpiochip 5 0>; read-only; }; In this case there is a gpio-led embedded within the gpiochip node. Also 'some_switch' is a gpio-switch and falls back to being gpio-initval. This way we can replace it at some point with a real driver. But if it doesn't exist we can at least initialize the GPIO properly. As you suggested this would open up the possibilities for all compatibles and drivers for single gpios. Best Regards, Markus > > [...] > > >> We also have to consider how to handle add-on boards. We probably need > >> a connector node which can remap connector signals to host signals in > >> order to have overlays independent of the host board DT. For GPIOs > >> this is probably a gpio-map property similar to interrupt-map. The > >> complicated part will be connectors that require pinmux setup of the > >> host. > > > > Also what about hotplugging gpio-chips? Is there any mechanism to let the > > 'gpio-switch' know that the GPIO is not there anymore? > > There are certainly issues around hotplug and GPIOs. If the > gpio-switch device is a child of the gpio controller, then it should > be possible. > > Rob > -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 1/3] Device tree binding documentation for gpio-switch @ 2016-03-07 8:26 ` Markus Pargmann 0 siblings, 0 replies; 53+ messages in thread From: Markus Pargmann @ 2016-03-07 8:26 UTC (permalink / raw) To: linux-arm-kernel On Wednesday 02 March 2016 10:03:27 Rob Herring wrote: > Reviving this thread... > > On Tue, Dec 15, 2015 at 3:09 AM, Markus Pargmann <mpa@pengutronix.de> wrote: > > Hi, > > > > On Monday 14 December 2015 09:45:48 Rob Herring wrote: > >> On Mon, Dec 14, 2015 at 8:28 AM, Linus Walleij <linus.walleij@linaro.org> wrote: > >> > On Fri, Dec 11, 2015 at 3:06 PM, Rob Herring <robh+dt@kernel.org> wrote: > >> >> On Fri, Dec 11, 2015 at 6:39 AM, Linus Walleij <linus.walleij@linaro.org> wrote: > >> >>> On Fri, Dec 4, 2015 at 6:31 PM, Martyn Welch > >> >>> <martyn.welch@collabora.co.uk> wrote: > >> > >> [...] > >> > >> >>> Markus Pargmann also did a series that add initial values to > >> >>> hogs, which is the inverse usecase of this, where you want to > >> >>> *output* something by default, then maybe also make it available > >> >>> to userspace. > >> >>> > >> >>> So what we need to see here is a patch series that does all of these > >> >>> things: > >> >>> > >> >>> - Name lines > >> >>> > >> >>> - Sets them to initial values > >> >>> > >> >>> - Mark them as read-only > >> >>> > >> >>> - Mark them as "not used by the operating system" so that they > >> >>> can be default-exported to userspace. > >> >> > >> >> No! This should not be a DT property. > >> >> > >> >> Whether I want to control a GPIO in the kernel or userspace is not > >> >> known and can change over time. It could simply depend on kernel > >> >> config. There is also the case that a GPIO has no connection or kernel > >> >> driver until some time later when a DT overlay for an expansion board > >> >> is applied. > >> > > >> > That's correct. So from a DT point of view, what really matters is > >> > to give things a name, and the hogs and initvals syntax already > >> > has a structure for this that is in use > >> > (from Documentation/devicetree/bindings/gpio/gpio.txt): > >> > > >> > qe_pio_a: gpio-controller at 1400 { > >> > compatible = "fsl,qe-pario-bank-a", "fsl,qe-pario-bank"; > >> > reg = <0x1400 0x18>; > >> > gpio-controller; > >> > #gpio-cells = <2>; > >> > > >> > line_b { > >> > gpio-hog; > >> > gpios = <6 0>; > >> > output-low; > >> > line-name = "foo-bar-gpio"; > >> > }; > >> > }; > >> > > >> > Markus use this also for initial values. That could easily be used in > >> > a budget version like this: > >> > > >> > line_b { > >> > /* Just naming */ > >> > gpios = <6 0>; > >> > line-name = "foo-bar-gpio"; > >> > }; > >> > > >> > That could grow kind of big though. Or maybe not? How many > >> > GPIO lines are actually properly named in a typical system? > >> > >> We should limit it to GPIOs with no connection to another node. For > >> example, I don't want to see a SD card detect in the list as that > >> should be in the SD host node. However, that is hard to enforce and > >> can change over time. Removing it would break userspace potentially. > >> Of course if the kernel starts own a signal that userspace used, then > >> that potentially breaks userspace regardless of the DT changing. OTOH, > >> using GPIOs in userspace is kind of use at your own risk. > > > > I see this a bit differently. I would really like to see the each GPIO having > > two different names: > > I think we are saying the same thing... > > > - GPIO label: This is the name of the GPIO line in the schematic for example > > Yes. > > > - GPIO use (this is the current semantic of the GPIO name): The use of a GPIO, > > e.g. 'sd-card-detect', 'LED', ... > > This should be determined from the compatible string and/or -gpios > prefix. This is the what the function is and "label" is which one. > > > I think it would be good to describe all GPIO labels in gpiochip subnodes as > > gpio-hogging introduced it. This would offer a use-independent naming. The use > > of the function could be defined in the device node that is using this gpio. > > I think I agree here. You may have a defined function without any > connection to another node. I also think we should encourage simple > GPIO bindings like gpio-leds to be child nodes. Having them at the > top-level is kind of arbitrary. Of course, allowing for both is > required. > > > As an example perhaps something like this: > > > > &gpiochip { > > some_interrupt { > > gpios = <4 0>; > > line-name = "some_interrupt_line"; > > }; > > > > line_b { > > gpios = <6 0>; > > line-name = "line-b"; > > }; > > }; > > > > randomswitch { > > compatible = "gpio-switch"; > > gpios = <&gpiochip 4 0>; > > use = "action-trigger"; > > read-only; > > }; > > > > Also this does seem kind of inconsistent with gpio-hogging and the proposed > > gpio-initval. gpio-hogging is defined in subnodes of the gpio chip while > > gpio-switches are not. As "gpio-switch" is not really any kind of device it > > would perhaps make sense to keep this consistent with gpio-hogging as well and > > define it in the same subnodes? > > I would be happy about any consistent way. > > Yes, as well as gpio leds, keys, etc. bindings. The key is you would > need to be able to start with something minimal and extend it with > specific compatibles. So if I understand you right it would be better to handle most distinctions using compatibles? Something like this? &gpiochip { some_led { compatible = "gpio-leds"; default-state = "on"; gpios = <3 0>; line-name = "leda"; }; some_switch { compatible = "gpio-switch", "gpio-initval"; gpios = <4 0>; line-name = "switch1"; /* This is used by gpio-initval in case gpio-switch is not implemented */ output-low; }; some_interrupt { gpios = <5 0>; line-name = "some_interrupt_line"; }; line_b { gpios = <6 0>; line-name = "line-b"; }; }; randomcomponent { gpios = <&gpiochip 5 0>; read-only; }; In this case there is a gpio-led embedded within the gpiochip node. Also 'some_switch' is a gpio-switch and falls back to being gpio-initval. This way we can replace it at some point with a real driver. But if it doesn't exist we can at least initialize the GPIO properly. As you suggested this would open up the possibilities for all compatibles and drivers for single gpios. Best Regards, Markus > > [...] > > >> We also have to consider how to handle add-on boards. We probably need > >> a connector node which can remap connector signals to host signals in > >> order to have overlays independent of the host board DT. For GPIOs > >> this is probably a gpio-map property similar to interrupt-map. The > >> complicated part will be connectors that require pinmux setup of the > >> host. > > > > Also what about hotplugging gpio-chips? Is there any mechanism to let the > > 'gpio-switch' know that the GPIO is not there anymore? > > There are certainly issues around hotplug and GPIOs. If the > gpio-switch device is a child of the gpio controller, then it should > be possible. > > Rob > -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: This is a digitally signed message part. URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160307/f8d05808/attachment.sig> ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 1/3] Device tree binding documentation for gpio-switch @ 2016-03-07 8:26 ` Markus Pargmann 0 siblings, 0 replies; 53+ messages in thread From: Markus Pargmann @ 2016-03-07 8:26 UTC (permalink / raw) To: Rob Herring Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Linus Walleij, Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA, Krzysztof Kozlowski, linux-samsung-soc, Pawel Moll, Martyn Welch, Arnd Bergmann, Ian Campbell, Greg Kroah-Hartman, Olof Johansson, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Johan Hovold, Kukjin Kim, Alexandre Courbot, Kumar Gala, Michael Welling, Russell King [-- Attachment #1: Type: text/plain, Size: 7976 bytes --] On Wednesday 02 March 2016 10:03:27 Rob Herring wrote: > Reviving this thread... > > On Tue, Dec 15, 2015 at 3:09 AM, Markus Pargmann <mpa-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> wrote: > > Hi, > > > > On Monday 14 December 2015 09:45:48 Rob Herring wrote: > >> On Mon, Dec 14, 2015 at 8:28 AM, Linus Walleij <linus.walleij@linaro.org> wrote: > >> > On Fri, Dec 11, 2015 at 3:06 PM, Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote: > >> >> On Fri, Dec 11, 2015 at 6:39 AM, Linus Walleij <linus.walleij@linaro.org> wrote: > >> >>> On Fri, Dec 4, 2015 at 6:31 PM, Martyn Welch > >> >>> <martyn.welch-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org> wrote: > >> > >> [...] > >> > >> >>> Markus Pargmann also did a series that add initial values to > >> >>> hogs, which is the inverse usecase of this, where you want to > >> >>> *output* something by default, then maybe also make it available > >> >>> to userspace. > >> >>> > >> >>> So what we need to see here is a patch series that does all of these > >> >>> things: > >> >>> > >> >>> - Name lines > >> >>> > >> >>> - Sets them to initial values > >> >>> > >> >>> - Mark them as read-only > >> >>> > >> >>> - Mark them as "not used by the operating system" so that they > >> >>> can be default-exported to userspace. > >> >> > >> >> No! This should not be a DT property. > >> >> > >> >> Whether I want to control a GPIO in the kernel or userspace is not > >> >> known and can change over time. It could simply depend on kernel > >> >> config. There is also the case that a GPIO has no connection or kernel > >> >> driver until some time later when a DT overlay for an expansion board > >> >> is applied. > >> > > >> > That's correct. So from a DT point of view, what really matters is > >> > to give things a name, and the hogs and initvals syntax already > >> > has a structure for this that is in use > >> > (from Documentation/devicetree/bindings/gpio/gpio.txt): > >> > > >> > qe_pio_a: gpio-controller@1400 { > >> > compatible = "fsl,qe-pario-bank-a", "fsl,qe-pario-bank"; > >> > reg = <0x1400 0x18>; > >> > gpio-controller; > >> > #gpio-cells = <2>; > >> > > >> > line_b { > >> > gpio-hog; > >> > gpios = <6 0>; > >> > output-low; > >> > line-name = "foo-bar-gpio"; > >> > }; > >> > }; > >> > > >> > Markus use this also for initial values. That could easily be used in > >> > a budget version like this: > >> > > >> > line_b { > >> > /* Just naming */ > >> > gpios = <6 0>; > >> > line-name = "foo-bar-gpio"; > >> > }; > >> > > >> > That could grow kind of big though. Or maybe not? How many > >> > GPIO lines are actually properly named in a typical system? > >> > >> We should limit it to GPIOs with no connection to another node. For > >> example, I don't want to see a SD card detect in the list as that > >> should be in the SD host node. However, that is hard to enforce and > >> can change over time. Removing it would break userspace potentially. > >> Of course if the kernel starts own a signal that userspace used, then > >> that potentially breaks userspace regardless of the DT changing. OTOH, > >> using GPIOs in userspace is kind of use at your own risk. > > > > I see this a bit differently. I would really like to see the each GPIO having > > two different names: > > I think we are saying the same thing... > > > - GPIO label: This is the name of the GPIO line in the schematic for example > > Yes. > > > - GPIO use (this is the current semantic of the GPIO name): The use of a GPIO, > > e.g. 'sd-card-detect', 'LED', ... > > This should be determined from the compatible string and/or -gpios > prefix. This is the what the function is and "label" is which one. > > > I think it would be good to describe all GPIO labels in gpiochip subnodes as > > gpio-hogging introduced it. This would offer a use-independent naming. The use > > of the function could be defined in the device node that is using this gpio. > > I think I agree here. You may have a defined function without any > connection to another node. I also think we should encourage simple > GPIO bindings like gpio-leds to be child nodes. Having them at the > top-level is kind of arbitrary. Of course, allowing for both is > required. > > > As an example perhaps something like this: > > > > &gpiochip { > > some_interrupt { > > gpios = <4 0>; > > line-name = "some_interrupt_line"; > > }; > > > > line_b { > > gpios = <6 0>; > > line-name = "line-b"; > > }; > > }; > > > > randomswitch { > > compatible = "gpio-switch"; > > gpios = <&gpiochip 4 0>; > > use = "action-trigger"; > > read-only; > > }; > > > > Also this does seem kind of inconsistent with gpio-hogging and the proposed > > gpio-initval. gpio-hogging is defined in subnodes of the gpio chip while > > gpio-switches are not. As "gpio-switch" is not really any kind of device it > > would perhaps make sense to keep this consistent with gpio-hogging as well and > > define it in the same subnodes? > > I would be happy about any consistent way. > > Yes, as well as gpio leds, keys, etc. bindings. The key is you would > need to be able to start with something minimal and extend it with > specific compatibles. So if I understand you right it would be better to handle most distinctions using compatibles? Something like this? &gpiochip { some_led { compatible = "gpio-leds"; default-state = "on"; gpios = <3 0>; line-name = "leda"; }; some_switch { compatible = "gpio-switch", "gpio-initval"; gpios = <4 0>; line-name = "switch1"; /* This is used by gpio-initval in case gpio-switch is not implemented */ output-low; }; some_interrupt { gpios = <5 0>; line-name = "some_interrupt_line"; }; line_b { gpios = <6 0>; line-name = "line-b"; }; }; randomcomponent { gpios = <&gpiochip 5 0>; read-only; }; In this case there is a gpio-led embedded within the gpiochip node. Also 'some_switch' is a gpio-switch and falls back to being gpio-initval. This way we can replace it at some point with a real driver. But if it doesn't exist we can at least initialize the GPIO properly. As you suggested this would open up the possibilities for all compatibles and drivers for single gpios. Best Regards, Markus > > [...] > > >> We also have to consider how to handle add-on boards. We probably need > >> a connector node which can remap connector signals to host signals in > >> order to have overlays independent of the host board DT. For GPIOs > >> this is probably a gpio-map property similar to interrupt-map. The > >> complicated part will be connectors that require pinmux setup of the > >> host. > > > > Also what about hotplugging gpio-chips? Is there any mechanism to let the > > 'gpio-switch' know that the GPIO is not there anymore? > > There are certainly issues around hotplug and GPIOs. If the > gpio-switch device is a child of the gpio controller, then it should > be possible. > > Rob > -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 2/3] Add support for monitoring gpio switches 2015-12-04 17:31 ` Martyn Welch (?) @ 2015-12-04 17:31 ` Martyn Welch -1 siblings, 0 replies; 53+ messages in thread From: Martyn Welch @ 2015-12-04 17:31 UTC (permalink / raw) To: Arnd Bergmann, Greg Kroah-Hartman Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Russell King, Kukjin Kim, Krzysztof Kozlowski, Martyn Welch, devicetree, linux-kernel, linux-arm-kernel, linux-samsung-soc, Olof Johansson Select Chromebooks have gpio attached to switches used to cause the firmware to enter alternative modes of operation and/or control other device characteristics (such as write protection on flash devices). This patch adds a driver that exposes a read-only interface to allow these signals to be read from user space. This functionality has been generalised to provide support for any device with device tree support which needs to identify a gpio as being used for a specific task. Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk> --- drivers/misc/Kconfig | 11 ++++ drivers/misc/Makefile | 1 + drivers/misc/gpio-switch.c | 160 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 172 insertions(+) create mode 100644 drivers/misc/gpio-switch.c diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig index 22892c7..d24367c 100644 --- a/drivers/misc/Kconfig +++ b/drivers/misc/Kconfig @@ -525,6 +525,17 @@ config VEXPRESS_SYSCFG bus. System Configuration interface is one of the possible means of generating transactions on this bus. +config GPIO_SWITCH + tristate "GPIO Switch driver" + depends on GPIO_SYSFS + ---help--- + Some devices have gpio attached to dedicated switches, an example of + this are chromebooks (where connection to some switches for predefined + purposes are provided on the generic development card). This driver + provides the ability to create consistently named sysfs entries to the + functionally equivalent signals across a range of devices. This + functionality currently requires a device which supports device tree. + source "drivers/misc/c2port/Kconfig" source "drivers/misc/eeprom/Kconfig" source "drivers/misc/cb710/Kconfig" diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile index 537d7f3..7a7e11a 100644 --- a/drivers/misc/Makefile +++ b/drivers/misc/Makefile @@ -56,3 +56,4 @@ obj-$(CONFIG_GENWQE) += genwqe/ obj-$(CONFIG_ECHO) += echo/ obj-$(CONFIG_VEXPRESS_SYSCFG) += vexpress-syscfg.o obj-$(CONFIG_CXL_BASE) += cxl/ +obj-$(CONFIG_GPIO_SWITCH) += gpio-switch.o diff --git a/drivers/misc/gpio-switch.c b/drivers/misc/gpio-switch.c new file mode 100644 index 0000000..1f381d6 --- /dev/null +++ b/drivers/misc/gpio-switch.c @@ -0,0 +1,160 @@ +/* + * Copyright (C) 2015 Collabora Ltd. + * + * based on vendor driver, + * + * Copyright (C) 2011 The Chromium OS Authors + * + * 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. + */ +#include <linux/bcd.h> +#include <linux/gpio.h> +#include <linux/notifier.h> +#include <linux/io.h> +#include <linux/of.h> +#include <linux/of_gpio.h> +#include <linux/platform_device.h> +#include <linux/slab.h> + +struct gpio_switch_gpio_info { + int gpio; + const char *link; +}; + +static int dt_gpio_init(struct platform_device *pdev, struct device_node *child, + struct gpio_switch_gpio_info *gpio) +{ + int err; + enum of_gpio_flags of_flags; + unsigned long flags = GPIOF_DIR_IN | GPIOF_EXPORT; + const char *name; + + err = of_property_read_string(child, "label", &name); + if (err) + return err; + + gpio->gpio = of_get_named_gpio_flags(child, "gpios", 0, &of_flags); + if (!gpio_is_valid(gpio->gpio)) { + err = -EINVAL; + goto err_prop; + } + + if (of_flags & OF_GPIO_ACTIVE_LOW) + flags |= GPIOF_ACTIVE_LOW; + + if (!of_property_read_bool(child, "read-only")) + flags |= GPIOF_EXPORT_CHANGEABLE; + + err = gpio_request_one(gpio->gpio, flags, name); + if (err) + goto err_prop; + + err = gpio_export_link(&pdev->dev, name, gpio->gpio); + if (err) + goto err_gpio; + + gpio->link = name; + + return 0; + +err_gpio: + gpio_free(gpio->gpio); +err_prop: + of_node_put(child); + + return err; +} + +static void gpio_switch_rem(struct device *dev, + struct gpio_switch_gpio_info *gpio) +{ + sysfs_remove_link(&dev->kobj, gpio->link); + + gpio_unexport(gpio->gpio); + + gpio_free(gpio->gpio); +} + +static int gpio_switch_probe(struct platform_device *pdev) +{ + struct gpio_switch_gpio_info *gpios; + struct device_node *child; + struct device_node *np = pdev->dev.of_node; + int ret; + int i; + + i = of_get_child_count(np); + if (i < 1) + return i; + + gpios = devm_kmalloc(&pdev->dev, sizeof(gpios) * i, GFP_KERNEL); + if (!gpios) + return -ENOMEM; + + i = 0; + + for_each_child_of_node(np, child) { + ret = dt_gpio_init(pdev, child, &gpios[i]); + if (ret) { + dev_err(&pdev->dev, "Failed to init child node %d.\n", + i); + goto err; + } + + i++; + } + + platform_set_drvdata(pdev, gpios); + + return 0; + +err: + while (i > 0) { + i--; + gpio_switch_rem(&pdev->dev, &gpios[i]); + } + + return ret; +} + +static int gpio_switch_remove(struct platform_device *pdev) +{ + struct gpio_switch_gpio_info *gpios = platform_get_drvdata(pdev); + struct device_node *np = pdev->dev.of_node; + int i; + + i = of_get_child_count(np); + + while (i > 0) { + i--; + gpio_switch_rem(&pdev->dev, &gpios[i]); + } + + return 0; +} + +static const struct of_device_id gpio_switch_of_match[] = { + { .compatible = "gpio-switch" }, + { } +}; +MODULE_DEVICE_TABLE(of, gpio_switch_of_match); + +static struct platform_driver gpio_switch_driver = { + .probe = gpio_switch_probe, + .remove = gpio_switch_remove, + .driver = { + .name = "gpio_switch", + .of_match_table = gpio_switch_of_match, + }, +}; +module_platform_driver(gpio_switch_driver); + +MODULE_LICENSE("GPL"); -- 2.1.4 ^ permalink raw reply related [flat|nested] 53+ messages in thread
* [PATCH 2/3] Add support for monitoring gpio switches @ 2015-12-04 17:31 ` Martyn Welch 0 siblings, 0 replies; 53+ messages in thread From: Martyn Welch @ 2015-12-04 17:31 UTC (permalink / raw) To: linux-arm-kernel Select Chromebooks have gpio attached to switches used to cause the firmware to enter alternative modes of operation and/or control other device characteristics (such as write protection on flash devices). This patch adds a driver that exposes a read-only interface to allow these signals to be read from user space. This functionality has been generalised to provide support for any device with device tree support which needs to identify a gpio as being used for a specific task. Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk> --- drivers/misc/Kconfig | 11 ++++ drivers/misc/Makefile | 1 + drivers/misc/gpio-switch.c | 160 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 172 insertions(+) create mode 100644 drivers/misc/gpio-switch.c diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig index 22892c7..d24367c 100644 --- a/drivers/misc/Kconfig +++ b/drivers/misc/Kconfig @@ -525,6 +525,17 @@ config VEXPRESS_SYSCFG bus. System Configuration interface is one of the possible means of generating transactions on this bus. +config GPIO_SWITCH + tristate "GPIO Switch driver" + depends on GPIO_SYSFS + ---help--- + Some devices have gpio attached to dedicated switches, an example of + this are chromebooks (where connection to some switches for predefined + purposes are provided on the generic development card). This driver + provides the ability to create consistently named sysfs entries to the + functionally equivalent signals across a range of devices. This + functionality currently requires a device which supports device tree. + source "drivers/misc/c2port/Kconfig" source "drivers/misc/eeprom/Kconfig" source "drivers/misc/cb710/Kconfig" diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile index 537d7f3..7a7e11a 100644 --- a/drivers/misc/Makefile +++ b/drivers/misc/Makefile @@ -56,3 +56,4 @@ obj-$(CONFIG_GENWQE) += genwqe/ obj-$(CONFIG_ECHO) += echo/ obj-$(CONFIG_VEXPRESS_SYSCFG) += vexpress-syscfg.o obj-$(CONFIG_CXL_BASE) += cxl/ +obj-$(CONFIG_GPIO_SWITCH) += gpio-switch.o diff --git a/drivers/misc/gpio-switch.c b/drivers/misc/gpio-switch.c new file mode 100644 index 0000000..1f381d6 --- /dev/null +++ b/drivers/misc/gpio-switch.c @@ -0,0 +1,160 @@ +/* + * Copyright (C) 2015 Collabora Ltd. + * + * based on vendor driver, + * + * Copyright (C) 2011 The Chromium OS Authors + * + * 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. + */ +#include <linux/bcd.h> +#include <linux/gpio.h> +#include <linux/notifier.h> +#include <linux/io.h> +#include <linux/of.h> +#include <linux/of_gpio.h> +#include <linux/platform_device.h> +#include <linux/slab.h> + +struct gpio_switch_gpio_info { + int gpio; + const char *link; +}; + +static int dt_gpio_init(struct platform_device *pdev, struct device_node *child, + struct gpio_switch_gpio_info *gpio) +{ + int err; + enum of_gpio_flags of_flags; + unsigned long flags = GPIOF_DIR_IN | GPIOF_EXPORT; + const char *name; + + err = of_property_read_string(child, "label", &name); + if (err) + return err; + + gpio->gpio = of_get_named_gpio_flags(child, "gpios", 0, &of_flags); + if (!gpio_is_valid(gpio->gpio)) { + err = -EINVAL; + goto err_prop; + } + + if (of_flags & OF_GPIO_ACTIVE_LOW) + flags |= GPIOF_ACTIVE_LOW; + + if (!of_property_read_bool(child, "read-only")) + flags |= GPIOF_EXPORT_CHANGEABLE; + + err = gpio_request_one(gpio->gpio, flags, name); + if (err) + goto err_prop; + + err = gpio_export_link(&pdev->dev, name, gpio->gpio); + if (err) + goto err_gpio; + + gpio->link = name; + + return 0; + +err_gpio: + gpio_free(gpio->gpio); +err_prop: + of_node_put(child); + + return err; +} + +static void gpio_switch_rem(struct device *dev, + struct gpio_switch_gpio_info *gpio) +{ + sysfs_remove_link(&dev->kobj, gpio->link); + + gpio_unexport(gpio->gpio); + + gpio_free(gpio->gpio); +} + +static int gpio_switch_probe(struct platform_device *pdev) +{ + struct gpio_switch_gpio_info *gpios; + struct device_node *child; + struct device_node *np = pdev->dev.of_node; + int ret; + int i; + + i = of_get_child_count(np); + if (i < 1) + return i; + + gpios = devm_kmalloc(&pdev->dev, sizeof(gpios) * i, GFP_KERNEL); + if (!gpios) + return -ENOMEM; + + i = 0; + + for_each_child_of_node(np, child) { + ret = dt_gpio_init(pdev, child, &gpios[i]); + if (ret) { + dev_err(&pdev->dev, "Failed to init child node %d.\n", + i); + goto err; + } + + i++; + } + + platform_set_drvdata(pdev, gpios); + + return 0; + +err: + while (i > 0) { + i--; + gpio_switch_rem(&pdev->dev, &gpios[i]); + } + + return ret; +} + +static int gpio_switch_remove(struct platform_device *pdev) +{ + struct gpio_switch_gpio_info *gpios = platform_get_drvdata(pdev); + struct device_node *np = pdev->dev.of_node; + int i; + + i = of_get_child_count(np); + + while (i > 0) { + i--; + gpio_switch_rem(&pdev->dev, &gpios[i]); + } + + return 0; +} + +static const struct of_device_id gpio_switch_of_match[] = { + { .compatible = "gpio-switch" }, + { } +}; +MODULE_DEVICE_TABLE(of, gpio_switch_of_match); + +static struct platform_driver gpio_switch_driver = { + .probe = gpio_switch_probe, + .remove = gpio_switch_remove, + .driver = { + .name = "gpio_switch", + .of_match_table = gpio_switch_of_match, + }, +}; +module_platform_driver(gpio_switch_driver); + +MODULE_LICENSE("GPL"); -- 2.1.4 ^ permalink raw reply related [flat|nested] 53+ messages in thread
* [PATCH 2/3] Add support for monitoring gpio switches @ 2015-12-04 17:31 ` Martyn Welch 0 siblings, 0 replies; 53+ messages in thread From: Martyn Welch @ 2015-12-04 17:31 UTC (permalink / raw) To: Arnd Bergmann, Greg Kroah-Hartman Cc: Mark Rutland, devicetree, Krzysztof Kozlowski, linux-samsung-soc, Russell King, Martyn Welch, Pawel Moll, Ian Campbell, linux-kernel, Rob Herring, Kukjin Kim, Kumar Gala, Olof Johansson, linux-arm-kernel Select Chromebooks have gpio attached to switches used to cause the firmware to enter alternative modes of operation and/or control other device characteristics (such as write protection on flash devices). This patch adds a driver that exposes a read-only interface to allow these signals to be read from user space. This functionality has been generalised to provide support for any device with device tree support which needs to identify a gpio as being used for a specific task. Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk> --- drivers/misc/Kconfig | 11 ++++ drivers/misc/Makefile | 1 + drivers/misc/gpio-switch.c | 160 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 172 insertions(+) create mode 100644 drivers/misc/gpio-switch.c diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig index 22892c7..d24367c 100644 --- a/drivers/misc/Kconfig +++ b/drivers/misc/Kconfig @@ -525,6 +525,17 @@ config VEXPRESS_SYSCFG bus. System Configuration interface is one of the possible means of generating transactions on this bus. +config GPIO_SWITCH + tristate "GPIO Switch driver" + depends on GPIO_SYSFS + ---help--- + Some devices have gpio attached to dedicated switches, an example of + this are chromebooks (where connection to some switches for predefined + purposes are provided on the generic development card). This driver + provides the ability to create consistently named sysfs entries to the + functionally equivalent signals across a range of devices. This + functionality currently requires a device which supports device tree. + source "drivers/misc/c2port/Kconfig" source "drivers/misc/eeprom/Kconfig" source "drivers/misc/cb710/Kconfig" diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile index 537d7f3..7a7e11a 100644 --- a/drivers/misc/Makefile +++ b/drivers/misc/Makefile @@ -56,3 +56,4 @@ obj-$(CONFIG_GENWQE) += genwqe/ obj-$(CONFIG_ECHO) += echo/ obj-$(CONFIG_VEXPRESS_SYSCFG) += vexpress-syscfg.o obj-$(CONFIG_CXL_BASE) += cxl/ +obj-$(CONFIG_GPIO_SWITCH) += gpio-switch.o diff --git a/drivers/misc/gpio-switch.c b/drivers/misc/gpio-switch.c new file mode 100644 index 0000000..1f381d6 --- /dev/null +++ b/drivers/misc/gpio-switch.c @@ -0,0 +1,160 @@ +/* + * Copyright (C) 2015 Collabora Ltd. + * + * based on vendor driver, + * + * Copyright (C) 2011 The Chromium OS Authors + * + * 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. + */ +#include <linux/bcd.h> +#include <linux/gpio.h> +#include <linux/notifier.h> +#include <linux/io.h> +#include <linux/of.h> +#include <linux/of_gpio.h> +#include <linux/platform_device.h> +#include <linux/slab.h> + +struct gpio_switch_gpio_info { + int gpio; + const char *link; +}; + +static int dt_gpio_init(struct platform_device *pdev, struct device_node *child, + struct gpio_switch_gpio_info *gpio) +{ + int err; + enum of_gpio_flags of_flags; + unsigned long flags = GPIOF_DIR_IN | GPIOF_EXPORT; + const char *name; + + err = of_property_read_string(child, "label", &name); + if (err) + return err; + + gpio->gpio = of_get_named_gpio_flags(child, "gpios", 0, &of_flags); + if (!gpio_is_valid(gpio->gpio)) { + err = -EINVAL; + goto err_prop; + } + + if (of_flags & OF_GPIO_ACTIVE_LOW) + flags |= GPIOF_ACTIVE_LOW; + + if (!of_property_read_bool(child, "read-only")) + flags |= GPIOF_EXPORT_CHANGEABLE; + + err = gpio_request_one(gpio->gpio, flags, name); + if (err) + goto err_prop; + + err = gpio_export_link(&pdev->dev, name, gpio->gpio); + if (err) + goto err_gpio; + + gpio->link = name; + + return 0; + +err_gpio: + gpio_free(gpio->gpio); +err_prop: + of_node_put(child); + + return err; +} + +static void gpio_switch_rem(struct device *dev, + struct gpio_switch_gpio_info *gpio) +{ + sysfs_remove_link(&dev->kobj, gpio->link); + + gpio_unexport(gpio->gpio); + + gpio_free(gpio->gpio); +} + +static int gpio_switch_probe(struct platform_device *pdev) +{ + struct gpio_switch_gpio_info *gpios; + struct device_node *child; + struct device_node *np = pdev->dev.of_node; + int ret; + int i; + + i = of_get_child_count(np); + if (i < 1) + return i; + + gpios = devm_kmalloc(&pdev->dev, sizeof(gpios) * i, GFP_KERNEL); + if (!gpios) + return -ENOMEM; + + i = 0; + + for_each_child_of_node(np, child) { + ret = dt_gpio_init(pdev, child, &gpios[i]); + if (ret) { + dev_err(&pdev->dev, "Failed to init child node %d.\n", + i); + goto err; + } + + i++; + } + + platform_set_drvdata(pdev, gpios); + + return 0; + +err: + while (i > 0) { + i--; + gpio_switch_rem(&pdev->dev, &gpios[i]); + } + + return ret; +} + +static int gpio_switch_remove(struct platform_device *pdev) +{ + struct gpio_switch_gpio_info *gpios = platform_get_drvdata(pdev); + struct device_node *np = pdev->dev.of_node; + int i; + + i = of_get_child_count(np); + + while (i > 0) { + i--; + gpio_switch_rem(&pdev->dev, &gpios[i]); + } + + return 0; +} + +static const struct of_device_id gpio_switch_of_match[] = { + { .compatible = "gpio-switch" }, + { } +}; +MODULE_DEVICE_TABLE(of, gpio_switch_of_match); + +static struct platform_driver gpio_switch_driver = { + .probe = gpio_switch_probe, + .remove = gpio_switch_remove, + .driver = { + .name = "gpio_switch", + .of_match_table = gpio_switch_of_match, + }, +}; +module_platform_driver(gpio_switch_driver); + +MODULE_LICENSE("GPL"); -- 2.1.4 ^ permalink raw reply related [flat|nested] 53+ messages in thread
* [PATCH] fix noderef.cocci warnings 2015-12-04 17:31 ` Martyn Welch (?) @ 2015-12-04 18:14 ` kbuild test robot -1 siblings, 0 replies; 53+ messages in thread From: kbuild test robot @ 2015-12-04 18:14 UTC (permalink / raw) To: Martyn Welch Cc: kbuild-all, Arnd Bergmann, Greg Kroah-Hartman, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Russell King, Kukjin Kim, Krzysztof Kozlowski, Martyn Welch, devicetree, linux-kernel, linux-arm-kernel, linux-samsung-soc, Olof Johansson drivers/misc/gpio-switch.c:98:34-40: ERROR: application of sizeof to pointer sizeof when applied to a pointer typed expression gives the size of the pointer Generated by: scripts/coccinelle/misc/noderef.cocci CC: Martyn Welch <martyn.welch@collabora.co.uk> Signed-off-by: Fengguang Wu <fengguang.wu@intel.com> --- gpio-switch.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) --- a/drivers/misc/gpio-switch.c +++ b/drivers/misc/gpio-switch.c @@ -95,7 +95,7 @@ static int gpio_switch_probe(struct plat if (i < 1) return i; - gpios = devm_kmalloc(&pdev->dev, sizeof(gpios) * i, GFP_KERNEL); + gpios = devm_kmalloc(&pdev->dev, sizeof(*gpios) * i, GFP_KERNEL); if (!gpios) return -ENOMEM; ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH] fix noderef.cocci warnings @ 2015-12-04 18:14 ` kbuild test robot 0 siblings, 0 replies; 53+ messages in thread From: kbuild test robot @ 2015-12-04 18:14 UTC (permalink / raw) To: linux-arm-kernel drivers/misc/gpio-switch.c:98:34-40: ERROR: application of sizeof to pointer sizeof when applied to a pointer typed expression gives the size of the pointer Generated by: scripts/coccinelle/misc/noderef.cocci CC: Martyn Welch <martyn.welch@collabora.co.uk> Signed-off-by: Fengguang Wu <fengguang.wu@intel.com> --- gpio-switch.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) --- a/drivers/misc/gpio-switch.c +++ b/drivers/misc/gpio-switch.c @@ -95,7 +95,7 @@ static int gpio_switch_probe(struct plat if (i < 1) return i; - gpios = devm_kmalloc(&pdev->dev, sizeof(gpios) * i, GFP_KERNEL); + gpios = devm_kmalloc(&pdev->dev, sizeof(*gpios) * i, GFP_KERNEL); if (!gpios) return -ENOMEM; ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH] fix noderef.cocci warnings @ 2015-12-04 18:14 ` kbuild test robot 0 siblings, 0 replies; 53+ messages in thread From: kbuild test robot @ 2015-12-04 18:14 UTC (permalink / raw) Cc: kbuild-all, Arnd Bergmann, Greg Kroah-Hartman, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Russell King, Kukjin Kim, Krzysztof Kozlowski, Martyn Welch, devicetree, linux-kernel, linux-arm-kernel, linux-samsung-soc, Olof Johansson drivers/misc/gpio-switch.c:98:34-40: ERROR: application of sizeof to pointer sizeof when applied to a pointer typed expression gives the size of the pointer Generated by: scripts/coccinelle/misc/noderef.cocci CC: Martyn Welch <martyn.welch@collabora.co.uk> Signed-off-by: Fengguang Wu <fengguang.wu@intel.com> --- gpio-switch.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) --- a/drivers/misc/gpio-switch.c +++ b/drivers/misc/gpio-switch.c @@ -95,7 +95,7 @@ static int gpio_switch_probe(struct plat if (i < 1) return i; - gpios = devm_kmalloc(&pdev->dev, sizeof(gpios) * i, GFP_KERNEL); + gpios = devm_kmalloc(&pdev->dev, sizeof(*gpios) * i, GFP_KERNEL); if (!gpios) return -ENOMEM; ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 2/3] Add support for monitoring gpio switches 2015-12-04 17:31 ` Martyn Welch (?) @ 2015-12-04 18:14 ` kbuild test robot -1 siblings, 0 replies; 53+ messages in thread From: kbuild test robot @ 2015-12-04 18:14 UTC (permalink / raw) To: Martyn Welch Cc: kbuild-all, Arnd Bergmann, Greg Kroah-Hartman, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Russell King, Kukjin Kim, Krzysztof Kozlowski, Martyn Welch, devicetree, linux-kernel, linux-arm-kernel, linux-samsung-soc, Olof Johansson Hi Martyn, [auto build test WARNING on char-misc/char-misc-testing] [also build test WARNING on v4.4-rc3 next-20151203] url: https://github.com/0day-ci/linux/commits/Martyn-Welch/Device-tree-binding-documentation-for-gpio-switch/20151205-014105 coccinelle warnings: (new ones prefixed by >>) >> drivers/misc/gpio-switch.c:98:34-40: ERROR: application of sizeof to pointer Please review and possibly fold the followup patch. --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 2/3] Add support for monitoring gpio switches @ 2015-12-04 18:14 ` kbuild test robot 0 siblings, 0 replies; 53+ messages in thread From: kbuild test robot @ 2015-12-04 18:14 UTC (permalink / raw) To: linux-arm-kernel Hi Martyn, [auto build test WARNING on char-misc/char-misc-testing] [also build test WARNING on v4.4-rc3 next-20151203] url: https://github.com/0day-ci/linux/commits/Martyn-Welch/Device-tree-binding-documentation-for-gpio-switch/20151205-014105 coccinelle warnings: (new ones prefixed by >>) >> drivers/misc/gpio-switch.c:98:34-40: ERROR: application of sizeof to pointer Please review and possibly fold the followup patch. --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 2/3] Add support for monitoring gpio switches @ 2015-12-04 18:14 ` kbuild test robot 0 siblings, 0 replies; 53+ messages in thread From: kbuild test robot @ 2015-12-04 18:14 UTC (permalink / raw) Cc: kbuild-all, Arnd Bergmann, Greg Kroah-Hartman, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Russell King, Kukjin Kim, Krzysztof Kozlowski, Martyn Welch, devicetree, linux-kernel, linux-arm-kernel, linux-samsung-soc, Olof Johansson Hi Martyn, [auto build test WARNING on char-misc/char-misc-testing] [also build test WARNING on v4.4-rc3 next-20151203] url: https://github.com/0day-ci/linux/commits/Martyn-Welch/Device-tree-binding-documentation-for-gpio-switch/20151205-014105 coccinelle warnings: (new ones prefixed by >>) >> drivers/misc/gpio-switch.c:98:34-40: ERROR: application of sizeof to pointer Please review and possibly fold the followup patch. --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 2/3] Add support for monitoring gpio switches 2015-12-04 17:31 ` Martyn Welch @ 2015-12-04 18:57 ` Greg Kroah-Hartman -1 siblings, 0 replies; 53+ messages in thread From: Greg Kroah-Hartman @ 2015-12-04 18:57 UTC (permalink / raw) To: Martyn Welch Cc: Arnd Bergmann, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Russell King, Kukjin Kim, Krzysztof Kozlowski, devicetree, linux-kernel, linux-arm-kernel, linux-samsung-soc, Olof Johansson On Fri, Dec 04, 2015 at 05:31:14PM +0000, Martyn Welch wrote: > Select Chromebooks have gpio attached to switches used to cause the > firmware to enter alternative modes of operation and/or control other > device characteristics (such as write protection on flash devices). This > patch adds a driver that exposes a read-only interface to allow these > signals to be read from user space. > > This functionality has been generalised to provide support for any device > with device tree support which needs to identify a gpio as being used for a > specific task. > > Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk> > --- > drivers/misc/Kconfig | 11 ++++ > drivers/misc/Makefile | 1 + > drivers/misc/gpio-switch.c | 160 +++++++++++++++++++++++++++++++++++++++++++++ Why isn't this in drivers/gpio/ ? why make it a misc driver? thanks, greg k-h ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 2/3] Add support for monitoring gpio switches @ 2015-12-04 18:57 ` Greg Kroah-Hartman 0 siblings, 0 replies; 53+ messages in thread From: Greg Kroah-Hartman @ 2015-12-04 18:57 UTC (permalink / raw) To: linux-arm-kernel On Fri, Dec 04, 2015 at 05:31:14PM +0000, Martyn Welch wrote: > Select Chromebooks have gpio attached to switches used to cause the > firmware to enter alternative modes of operation and/or control other > device characteristics (such as write protection on flash devices). This > patch adds a driver that exposes a read-only interface to allow these > signals to be read from user space. > > This functionality has been generalised to provide support for any device > with device tree support which needs to identify a gpio as being used for a > specific task. > > Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk> > --- > drivers/misc/Kconfig | 11 ++++ > drivers/misc/Makefile | 1 + > drivers/misc/gpio-switch.c | 160 +++++++++++++++++++++++++++++++++++++++++++++ Why isn't this in drivers/gpio/ ? why make it a misc driver? thanks, greg k-h ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 2/3] Add support for monitoring gpio switches 2015-12-04 18:57 ` Greg Kroah-Hartman @ 2015-12-05 10:42 ` Martyn Welch -1 siblings, 0 replies; 53+ messages in thread From: Martyn Welch @ 2015-12-05 10:42 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Arnd Bergmann, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Russell King, Kukjin Kim, Krzysztof Kozlowski, devicetree, linux-kernel, linux-arm-kernel, linux-samsung-soc, Olof Johansson, Linus Walleij, Alexandre Courbot, linux-gpio On 04/12/15 18:57, Greg Kroah-Hartman wrote: > On Fri, Dec 04, 2015 at 05:31:14PM +0000, Martyn Welch wrote: >> Select Chromebooks have gpio attached to switches used to cause the >> firmware to enter alternative modes of operation and/or control other >> device characteristics (such as write protection on flash devices). This >> patch adds a driver that exposes a read-only interface to allow these >> signals to be read from user space. >> >> This functionality has been generalised to provide support for any device >> with device tree support which needs to identify a gpio as being used for a >> specific task. >> >> Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk> >> --- >> drivers/misc/Kconfig | 11 ++++ >> drivers/misc/Makefile | 1 + >> drivers/misc/gpio-switch.c | 160 +++++++++++++++++++++++++++++++++++++++++++++ > > Why isn't this in drivers/gpio/ ? > > why make it a misc driver? > I thought all the drivers in /drivers/gpio were gpio drivers, rather than users of the gpio framework. Is that not the case? Happy to move it if the consensus is that that's the correct place to put it. Martyn > thanks, > > greg k-h > ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 2/3] Add support for monitoring gpio switches @ 2015-12-05 10:42 ` Martyn Welch 0 siblings, 0 replies; 53+ messages in thread From: Martyn Welch @ 2015-12-05 10:42 UTC (permalink / raw) To: linux-arm-kernel On 04/12/15 18:57, Greg Kroah-Hartman wrote: > On Fri, Dec 04, 2015 at 05:31:14PM +0000, Martyn Welch wrote: >> Select Chromebooks have gpio attached to switches used to cause the >> firmware to enter alternative modes of operation and/or control other >> device characteristics (such as write protection on flash devices). This >> patch adds a driver that exposes a read-only interface to allow these >> signals to be read from user space. >> >> This functionality has been generalised to provide support for any device >> with device tree support which needs to identify a gpio as being used for a >> specific task. >> >> Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk> >> --- >> drivers/misc/Kconfig | 11 ++++ >> drivers/misc/Makefile | 1 + >> drivers/misc/gpio-switch.c | 160 +++++++++++++++++++++++++++++++++++++++++++++ > > Why isn't this in drivers/gpio/ ? > > why make it a misc driver? > I thought all the drivers in /drivers/gpio were gpio drivers, rather than users of the gpio framework. Is that not the case? Happy to move it if the consensus is that that's the correct place to put it. Martyn > thanks, > > greg k-h > ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 2/3] Add support for monitoring gpio switches 2015-12-04 17:31 ` Martyn Welch (?) @ 2015-12-11 9:08 ` Linus Walleij -1 siblings, 0 replies; 53+ messages in thread From: Linus Walleij @ 2015-12-11 9:08 UTC (permalink / raw) To: Martyn Welch Cc: Arnd Bergmann, Greg Kroah-Hartman, Mark Rutland, devicetree, Krzysztof Kozlowski, linux-samsung-soc, Russell King, Pawel Moll, Ian Campbell, linux-kernel, Rob Herring, Kukjin Kim, Kumar Gala, Olof Johansson, linux-arm-kernel On Fri, Dec 4, 2015 at 6:31 PM, Martyn Welch <martyn.welch@collabora.co.uk> wrote: > Select Chromebooks have gpio attached to switches used to cause the > firmware to enter alternative modes of operation and/or control other > device characteristics (such as write protection on flash devices). This > patch adds a driver that exposes a read-only interface to allow these > signals to be read from user space. > > This functionality has been generalised to provide support for any device > with device tree support which needs to identify a gpio as being used for a > specific task. > > Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk> If you want to do this thing, also propose a device tree binding document for "gpio-switch". But first (from Documentation/gpio/drivers-on-gpio.txt): - gpio-keys: drivers/input/keyboard/gpio_keys.c is used when your GPIO line can generate interrupts in response to a key press. Also supports debounce. - gpio-keys-polled: drivers/input/keyboard/gpio_keys_polled.c is used when your GPIO line cannot generate interrupts, so it needs to be periodically polled by a timer. - extcon-gpio: drivers/extcon/extcon-gpio.c is used when you need to read an external connector status, such as a headset line for an audio driver or an HDMI connector. It will provide a better userspace sysfs interface than GPIO. So you mean none of these apply for this case? Second: what you want to do is export a number of GPIOs with certain names to userspace. This is something very generic and should be implemented as such, not as something Chromebook-specific. Patches like that has however already been suggested, and I have NACKed them because the GPIO sysfs ABI is insane, and that is why I am refactoring the world to create a proper chardev ABI for GPIO instead. See: http://marc.info/?l=linux-gpio&m=144550276512673&w=2 So for the moment, NACK on this, please participate in creating the *right* ABI for GPIO instead of trying to shoehorn stuff into the dying sysfs ABI. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 2/3] Add support for monitoring gpio switches @ 2015-12-11 9:08 ` Linus Walleij 0 siblings, 0 replies; 53+ messages in thread From: Linus Walleij @ 2015-12-11 9:08 UTC (permalink / raw) To: linux-arm-kernel On Fri, Dec 4, 2015 at 6:31 PM, Martyn Welch <martyn.welch@collabora.co.uk> wrote: > Select Chromebooks have gpio attached to switches used to cause the > firmware to enter alternative modes of operation and/or control other > device characteristics (such as write protection on flash devices). This > patch adds a driver that exposes a read-only interface to allow these > signals to be read from user space. > > This functionality has been generalised to provide support for any device > with device tree support which needs to identify a gpio as being used for a > specific task. > > Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk> If you want to do this thing, also propose a device tree binding document for "gpio-switch". But first (from Documentation/gpio/drivers-on-gpio.txt): - gpio-keys: drivers/input/keyboard/gpio_keys.c is used when your GPIO line can generate interrupts in response to a key press. Also supports debounce. - gpio-keys-polled: drivers/input/keyboard/gpio_keys_polled.c is used when your GPIO line cannot generate interrupts, so it needs to be periodically polled by a timer. - extcon-gpio: drivers/extcon/extcon-gpio.c is used when you need to read an external connector status, such as a headset line for an audio driver or an HDMI connector. It will provide a better userspace sysfs interface than GPIO. So you mean none of these apply for this case? Second: what you want to do is export a number of GPIOs with certain names to userspace. This is something very generic and should be implemented as such, not as something Chromebook-specific. Patches like that has however already been suggested, and I have NACKed them because the GPIO sysfs ABI is insane, and that is why I am refactoring the world to create a proper chardev ABI for GPIO instead. See: http://marc.info/?l=linux-gpio&m=144550276512673&w=2 So for the moment, NACK on this, please participate in creating the *right* ABI for GPIO instead of trying to shoehorn stuff into the dying sysfs ABI. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 2/3] Add support for monitoring gpio switches @ 2015-12-11 9:08 ` Linus Walleij 0 siblings, 0 replies; 53+ messages in thread From: Linus Walleij @ 2015-12-11 9:08 UTC (permalink / raw) To: Martyn Welch Cc: Arnd Bergmann, Greg Kroah-Hartman, Mark Rutland, devicetree, Krzysztof Kozlowski, linux-samsung-soc, Russell King, Pawel Moll, Ian Campbell, linux-kernel, Rob Herring, Kukjin Kim, Kumar Gala, Olof Johansson, linux-arm-kernel On Fri, Dec 4, 2015 at 6:31 PM, Martyn Welch <martyn.welch@collabora.co.uk> wrote: > Select Chromebooks have gpio attached to switches used to cause the > firmware to enter alternative modes of operation and/or control other > device characteristics (such as write protection on flash devices). This > patch adds a driver that exposes a read-only interface to allow these > signals to be read from user space. > > This functionality has been generalised to provide support for any device > with device tree support which needs to identify a gpio as being used for a > specific task. > > Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk> If you want to do this thing, also propose a device tree binding document for "gpio-switch". But first (from Documentation/gpio/drivers-on-gpio.txt): - gpio-keys: drivers/input/keyboard/gpio_keys.c is used when your GPIO line can generate interrupts in response to a key press. Also supports debounce. - gpio-keys-polled: drivers/input/keyboard/gpio_keys_polled.c is used when your GPIO line cannot generate interrupts, so it needs to be periodically polled by a timer. - extcon-gpio: drivers/extcon/extcon-gpio.c is used when you need to read an external connector status, such as a headset line for an audio driver or an HDMI connector. It will provide a better userspace sysfs interface than GPIO. So you mean none of these apply for this case? Second: what you want to do is export a number of GPIOs with certain names to userspace. This is something very generic and should be implemented as such, not as something Chromebook-specific. Patches like that has however already been suggested, and I have NACKed them because the GPIO sysfs ABI is insane, and that is why I am refactoring the world to create a proper chardev ABI for GPIO instead. See: http://marc.info/?l=linux-gpio&m=144550276512673&w=2 So for the moment, NACK on this, please participate in creating the *right* ABI for GPIO instead of trying to shoehorn stuff into the dying sysfs ABI. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 2/3] Add support for monitoring gpio switches 2015-12-11 9:08 ` Linus Walleij (?) @ 2015-12-16 10:11 ` Martyn Welch -1 siblings, 0 replies; 53+ messages in thread From: Martyn Welch @ 2015-12-16 10:11 UTC (permalink / raw) To: Linus Walleij Cc: Arnd Bergmann, Greg Kroah-Hartman, Mark Rutland, devicetree, Krzysztof Kozlowski, linux-samsung-soc, Russell King, Pawel Moll, Ian Campbell, linux-kernel, Rob Herring, Kukjin Kim, Kumar Gala, Olof Johansson, linux-arm-kernel On 11/12/15 09:08, Linus Walleij wrote: > On Fri, Dec 4, 2015 at 6:31 PM, Martyn Welch > <martyn.welch@collabora.co.uk> wrote: > >> Select Chromebooks have gpio attached to switches used to cause the >> firmware to enter alternative modes of operation and/or control other >> device characteristics (such as write protection on flash devices). This >> patch adds a driver that exposes a read-only interface to allow these >> signals to be read from user space. >> >> This functionality has been generalised to provide support for any device >> with device tree support which needs to identify a gpio as being used for a >> specific task. >> >> Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk> > > If you want to do this thing, also propose a device tree binding document > for "gpio-switch". > > But first (from Documentation/gpio/drivers-on-gpio.txt): > > - gpio-keys: drivers/input/keyboard/gpio_keys.c is used when your GPIO line > can generate interrupts in response to a key press. Also supports debounce. > This one generates input events from gpio. I'm not looking to generate events. > - gpio-keys-polled: drivers/input/keyboard/gpio_keys_polled.c is used when your > GPIO line cannot generate interrupts, so it needs to be periodically polled > by a timer. > Ditto (but using a polled mechanism rather than interrupts) > - extcon-gpio: drivers/extcon/extcon-gpio.c is used when you need to read an > external connector status, such as a headset line for an audio driver or an > HDMI connector. It will provide a better userspace sysfs interface than GPIO. > This appears to be exclusively for monitoring insertion events, or am I missing something? > So you mean none of these apply for this case? > No, I'm looking for a mechanism to identify GPIO as connected to a specific signal, which is provided across multiple devices, but which might be implemented subtly differently on different platforms (i.e. active high/low) and on different GPIO lines. > Second: what you want to do is export a number of GPIOs with certain names > to userspace. This is something very generic and should be implemented > as such, not as something Chromebook-specific. > I'd agree that my first implementation was ChromeBook specific, but I'm fairly sure that my last attempt wasn't. I've mentioned ChromeBooks as an example of an existing use case. > Patches like that has however already been suggested, and I have NACKed > them because the GPIO sysfs ABI is insane, and that is why I am refactoring > the world to create a proper chardev ABI for GPIO instead. See: > http://marc.info/?l=linux-gpio&m=144550276512673&w=2 > I can certainly understand the rationale for the changes that you are proposing, though do worry that it does make it a bit tougher to use from scripting languages. I see that the question of how to provide functionality equivalent to the above was raised and no answer was forthcoming. Is there a plan for supporting the identification of a GPIO line serving a specific purpose? What is the status of the mentioned patch series? Martyn > So for the moment, NACK on this, please participate in creating the > *right* ABI for GPIO instead of trying to shoehorn stuff into the dying > sysfs ABI. > > Yours, > Linus Walleij > ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 2/3] Add support for monitoring gpio switches @ 2015-12-16 10:11 ` Martyn Welch 0 siblings, 0 replies; 53+ messages in thread From: Martyn Welch @ 2015-12-16 10:11 UTC (permalink / raw) To: linux-arm-kernel On 11/12/15 09:08, Linus Walleij wrote: > On Fri, Dec 4, 2015 at 6:31 PM, Martyn Welch > <martyn.welch@collabora.co.uk> wrote: > >> Select Chromebooks have gpio attached to switches used to cause the >> firmware to enter alternative modes of operation and/or control other >> device characteristics (such as write protection on flash devices). This >> patch adds a driver that exposes a read-only interface to allow these >> signals to be read from user space. >> >> This functionality has been generalised to provide support for any device >> with device tree support which needs to identify a gpio as being used for a >> specific task. >> >> Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk> > > If you want to do this thing, also propose a device tree binding document > for "gpio-switch". > > But first (from Documentation/gpio/drivers-on-gpio.txt): > > - gpio-keys: drivers/input/keyboard/gpio_keys.c is used when your GPIO line > can generate interrupts in response to a key press. Also supports debounce. > This one generates input events from gpio. I'm not looking to generate events. > - gpio-keys-polled: drivers/input/keyboard/gpio_keys_polled.c is used when your > GPIO line cannot generate interrupts, so it needs to be periodically polled > by a timer. > Ditto (but using a polled mechanism rather than interrupts) > - extcon-gpio: drivers/extcon/extcon-gpio.c is used when you need to read an > external connector status, such as a headset line for an audio driver or an > HDMI connector. It will provide a better userspace sysfs interface than GPIO. > This appears to be exclusively for monitoring insertion events, or am I missing something? > So you mean none of these apply for this case? > No, I'm looking for a mechanism to identify GPIO as connected to a specific signal, which is provided across multiple devices, but which might be implemented subtly differently on different platforms (i.e. active high/low) and on different GPIO lines. > Second: what you want to do is export a number of GPIOs with certain names > to userspace. This is something very generic and should be implemented > as such, not as something Chromebook-specific. > I'd agree that my first implementation was ChromeBook specific, but I'm fairly sure that my last attempt wasn't. I've mentioned ChromeBooks as an example of an existing use case. > Patches like that has however already been suggested, and I have NACKed > them because the GPIO sysfs ABI is insane, and that is why I am refactoring > the world to create a proper chardev ABI for GPIO instead. See: > http://marc.info/?l=linux-gpio&m=144550276512673&w=2 > I can certainly understand the rationale for the changes that you are proposing, though do worry that it does make it a bit tougher to use from scripting languages. I see that the question of how to provide functionality equivalent to the above was raised and no answer was forthcoming. Is there a plan for supporting the identification of a GPIO line serving a specific purpose? What is the status of the mentioned patch series? Martyn > So for the moment, NACK on this, please participate in creating the > *right* ABI for GPIO instead of trying to shoehorn stuff into the dying > sysfs ABI. > > Yours, > Linus Walleij > ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 2/3] Add support for monitoring gpio switches @ 2015-12-16 10:11 ` Martyn Welch 0 siblings, 0 replies; 53+ messages in thread From: Martyn Welch @ 2015-12-16 10:11 UTC (permalink / raw) To: Linus Walleij Cc: Arnd Bergmann, Greg Kroah-Hartman, Mark Rutland, devicetree, Krzysztof Kozlowski, linux-samsung-soc, Russell King, Pawel Moll, Ian Campbell, linux-kernel, Rob Herring, Kukjin Kim, Kumar Gala, Olof Johansson, linux-arm-kernel On 11/12/15 09:08, Linus Walleij wrote: > On Fri, Dec 4, 2015 at 6:31 PM, Martyn Welch > <martyn.welch@collabora.co.uk> wrote: > >> Select Chromebooks have gpio attached to switches used to cause the >> firmware to enter alternative modes of operation and/or control other >> device characteristics (such as write protection on flash devices). This >> patch adds a driver that exposes a read-only interface to allow these >> signals to be read from user space. >> >> This functionality has been generalised to provide support for any device >> with device tree support which needs to identify a gpio as being used for a >> specific task. >> >> Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk> > > If you want to do this thing, also propose a device tree binding document > for "gpio-switch". > > But first (from Documentation/gpio/drivers-on-gpio.txt): > > - gpio-keys: drivers/input/keyboard/gpio_keys.c is used when your GPIO line > can generate interrupts in response to a key press. Also supports debounce. > This one generates input events from gpio. I'm not looking to generate events. > - gpio-keys-polled: drivers/input/keyboard/gpio_keys_polled.c is used when your > GPIO line cannot generate interrupts, so it needs to be periodically polled > by a timer. > Ditto (but using a polled mechanism rather than interrupts) > - extcon-gpio: drivers/extcon/extcon-gpio.c is used when you need to read an > external connector status, such as a headset line for an audio driver or an > HDMI connector. It will provide a better userspace sysfs interface than GPIO. > This appears to be exclusively for monitoring insertion events, or am I missing something? > So you mean none of these apply for this case? > No, I'm looking for a mechanism to identify GPIO as connected to a specific signal, which is provided across multiple devices, but which might be implemented subtly differently on different platforms (i.e. active high/low) and on different GPIO lines. > Second: what you want to do is export a number of GPIOs with certain names > to userspace. This is something very generic and should be implemented > as such, not as something Chromebook-specific. > I'd agree that my first implementation was ChromeBook specific, but I'm fairly sure that my last attempt wasn't. I've mentioned ChromeBooks as an example of an existing use case. > Patches like that has however already been suggested, and I have NACKed > them because the GPIO sysfs ABI is insane, and that is why I am refactoring > the world to create a proper chardev ABI for GPIO instead. See: > http://marc.info/?l=linux-gpio&m=144550276512673&w=2 > I can certainly understand the rationale for the changes that you are proposing, though do worry that it does make it a bit tougher to use from scripting languages. I see that the question of how to provide functionality equivalent to the above was raised and no answer was forthcoming. Is there a plan for supporting the identification of a GPIO line serving a specific purpose? What is the status of the mentioned patch series? Martyn > So for the moment, NACK on this, please participate in creating the > *right* ABI for GPIO instead of trying to shoehorn stuff into the dying > sysfs ABI. > > Yours, > Linus Walleij > ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 2/3] Add support for monitoring gpio switches 2015-12-16 10:11 ` Martyn Welch (?) @ 2015-12-22 9:25 ` Linus Walleij -1 siblings, 0 replies; 53+ messages in thread From: Linus Walleij @ 2015-12-22 9:25 UTC (permalink / raw) To: Martyn Welch Cc: Arnd Bergmann, Greg Kroah-Hartman, Mark Rutland, devicetree, Krzysztof Kozlowski, linux-samsung-soc, Russell King, Pawel Moll, Ian Campbell, linux-kernel, Rob Herring, Kukjin Kim, Kumar Gala, Olof Johansson, linux-arm-kernel On Wed, Dec 16, 2015 at 11:11 AM, Martyn Welch <martyn.welch@collabora.co.uk> wrote: >> Patches like that has however already been suggested, and I have NACKed >> them because the GPIO sysfs ABI is insane, and that is why I am >> refactoring >> the world to create a proper chardev ABI for GPIO instead. See: >> http://marc.info/?l=linux-gpio&m=144550276512673&w=2 > > I can certainly understand the rationale for the changes that you are > proposing, though do worry that it does make it a bit tougher to use from > scripting languages. The idea is to provide commandline tools in the kernel tools/gpio subdir to satisfy the needs of scripting. "lsgpio" today is just one example, nothing stops us from having a tool called just "gpio-sak" (GPIO swiss army knife) that will be tailored for scripting. > I see that the question of how to provide functionality > equivalent to the above was raised and no answer was forthcoming. Is there a > plan for supporting the identification of a GPIO line serving a specific > purpose? Yes by name. > What is the status of the mentioned patch series? They stubled into a few dozen architecture issues in the GPIO subsystem so I am busy refactoring the whole know universe :D But I still intend to persue the series. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 2/3] Add support for monitoring gpio switches @ 2015-12-22 9:25 ` Linus Walleij 0 siblings, 0 replies; 53+ messages in thread From: Linus Walleij @ 2015-12-22 9:25 UTC (permalink / raw) To: linux-arm-kernel On Wed, Dec 16, 2015 at 11:11 AM, Martyn Welch <martyn.welch@collabora.co.uk> wrote: >> Patches like that has however already been suggested, and I have NACKed >> them because the GPIO sysfs ABI is insane, and that is why I am >> refactoring >> the world to create a proper chardev ABI for GPIO instead. See: >> http://marc.info/?l=linux-gpio&m=144550276512673&w=2 > > I can certainly understand the rationale for the changes that you are > proposing, though do worry that it does make it a bit tougher to use from > scripting languages. The idea is to provide commandline tools in the kernel tools/gpio subdir to satisfy the needs of scripting. "lsgpio" today is just one example, nothing stops us from having a tool called just "gpio-sak" (GPIO swiss army knife) that will be tailored for scripting. > I see that the question of how to provide functionality > equivalent to the above was raised and no answer was forthcoming. Is there a > plan for supporting the identification of a GPIO line serving a specific > purpose? Yes by name. > What is the status of the mentioned patch series? They stubled into a few dozen architecture issues in the GPIO subsystem so I am busy refactoring the whole know universe :D But I still intend to persue the series. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 2/3] Add support for monitoring gpio switches @ 2015-12-22 9:25 ` Linus Walleij 0 siblings, 0 replies; 53+ messages in thread From: Linus Walleij @ 2015-12-22 9:25 UTC (permalink / raw) To: Martyn Welch Cc: Arnd Bergmann, Greg Kroah-Hartman, Mark Rutland, devicetree, Krzysztof Kozlowski, linux-samsung-soc, Russell King, Pawel Moll, Ian Campbell, linux-kernel, Rob Herring, Kukjin Kim, Kumar Gala, Olof Johansson, linux-arm-kernel On Wed, Dec 16, 2015 at 11:11 AM, Martyn Welch <martyn.welch@collabora.co.uk> wrote: >> Patches like that has however already been suggested, and I have NACKed >> them because the GPIO sysfs ABI is insane, and that is why I am >> refactoring >> the world to create a proper chardev ABI for GPIO instead. See: >> http://marc.info/?l=linux-gpio&m=144550276512673&w=2 > > I can certainly understand the rationale for the changes that you are > proposing, though do worry that it does make it a bit tougher to use from > scripting languages. The idea is to provide commandline tools in the kernel tools/gpio subdir to satisfy the needs of scripting. "lsgpio" today is just one example, nothing stops us from having a tool called just "gpio-sak" (GPIO swiss army knife) that will be tailored for scripting. > I see that the question of how to provide functionality > equivalent to the above was raised and no answer was forthcoming. Is there a > plan for supporting the identification of a GPIO line serving a specific > purpose? Yes by name. > What is the status of the mentioned patch series? They stubled into a few dozen architecture issues in the GPIO subsystem so I am busy refactoring the whole know universe :D But I still intend to persue the series. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 3/3] ARM: dts: Addition of binding for gpio switches on peach-pi 2015-12-04 17:31 ` Martyn Welch @ 2015-12-04 17:31 ` Martyn Welch -1 siblings, 0 replies; 53+ messages in thread From: Martyn Welch @ 2015-12-04 17:31 UTC (permalink / raw) To: Arnd Bergmann, Greg Kroah-Hartman Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Russell King, Kukjin Kim, Krzysztof Kozlowski, Martyn Welch, devicetree, linux-kernel, linux-arm-kernel, linux-samsung-soc, Olof Johansson The peach pi has a GPIO connected to the firmware write protect, developer mode and recovery mode lines (which are primarily controlled via external switches on developer test board). This patch adds the required nodes to the device tree to configure the pinmuxing and allow these to be read from user space. Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk> --- arch/arm/boot/dts/exynos5800-peach-pi.dts | 46 +++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/arch/arm/boot/dts/exynos5800-peach-pi.dts b/arch/arm/boot/dts/exynos5800-peach-pi.dts index 49a4f43..2937372 100644 --- a/arch/arm/boot/dts/exynos5800-peach-pi.dts +++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts @@ -53,6 +53,31 @@ }; }; + gpio-switch { + compatible = "gpio-switch"; + + pinctrl-names = "default"; + pinctrl-0 = <&wp_gpio &dev_mode &rec_mode>; + + write-protect { + label = "write-protect"; + gpios = <&gpx3 0 GPIO_ACTIVE_LOW>; + read-only; + }; + + developer-switch { + label = "developer-switch"; + gpios = <&gpx1 3 GPIO_ACTIVE_HIGH>; + read-only; + }; + + recovery-switch { + label = "recovery-switch"; + gpios = <&gpx0 7 GPIO_ACTIVE_LOW>; + read-only; + }; + }; + gpio-keys { compatible = "gpio-keys"; @@ -731,6 +756,13 @@ samsung,pin-val = <0>; }; + rec_mode: rec-mode { + samsung,pins = "gpx0-7"; + samsung,pin-function = <0>; + samsung,pin-pud = <0>; + samsung,pin-drv = <0>; + }; + tpm_irq: tpm-irq { samsung,pins = "gpx1-0"; samsung,pin-function = <0>; @@ -752,6 +784,13 @@ samsung,pin-drv = <0>; }; + dev_mode: dev-mode { + samsung,pins = "gpx1-3"; + samsung,pin-function = <0>; + samsung,pin-pud = <3>; + samsung,pin-drv = <0>; + }; + ec_irq: ec-irq { samsung,pins = "gpx1-5"; samsung,pin-function = <0>; @@ -773,6 +812,13 @@ samsung,pin-drv = <0>; }; + wp_gpio: wp_gpio { + samsung,pins = "gpx3-0"; + samsung,pin-function = <0>; + samsung,pin-pud = <0>; + samsung,pin-drv = <0>; + }; + max77802_irq: max77802-irq { samsung,pins = "gpx3-1"; samsung,pin-function = <0>; -- 2.1.4 ^ permalink raw reply related [flat|nested] 53+ messages in thread
* [PATCH 3/3] ARM: dts: Addition of binding for gpio switches on peach-pi @ 2015-12-04 17:31 ` Martyn Welch 0 siblings, 0 replies; 53+ messages in thread From: Martyn Welch @ 2015-12-04 17:31 UTC (permalink / raw) To: linux-arm-kernel The peach pi has a GPIO connected to the firmware write protect, developer mode and recovery mode lines (which are primarily controlled via external switches on developer test board). This patch adds the required nodes to the device tree to configure the pinmuxing and allow these to be read from user space. Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk> --- arch/arm/boot/dts/exynos5800-peach-pi.dts | 46 +++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/arch/arm/boot/dts/exynos5800-peach-pi.dts b/arch/arm/boot/dts/exynos5800-peach-pi.dts index 49a4f43..2937372 100644 --- a/arch/arm/boot/dts/exynos5800-peach-pi.dts +++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts @@ -53,6 +53,31 @@ }; }; + gpio-switch { + compatible = "gpio-switch"; + + pinctrl-names = "default"; + pinctrl-0 = <&wp_gpio &dev_mode &rec_mode>; + + write-protect { + label = "write-protect"; + gpios = <&gpx3 0 GPIO_ACTIVE_LOW>; + read-only; + }; + + developer-switch { + label = "developer-switch"; + gpios = <&gpx1 3 GPIO_ACTIVE_HIGH>; + read-only; + }; + + recovery-switch { + label = "recovery-switch"; + gpios = <&gpx0 7 GPIO_ACTIVE_LOW>; + read-only; + }; + }; + gpio-keys { compatible = "gpio-keys"; @@ -731,6 +756,13 @@ samsung,pin-val = <0>; }; + rec_mode: rec-mode { + samsung,pins = "gpx0-7"; + samsung,pin-function = <0>; + samsung,pin-pud = <0>; + samsung,pin-drv = <0>; + }; + tpm_irq: tpm-irq { samsung,pins = "gpx1-0"; samsung,pin-function = <0>; @@ -752,6 +784,13 @@ samsung,pin-drv = <0>; }; + dev_mode: dev-mode { + samsung,pins = "gpx1-3"; + samsung,pin-function = <0>; + samsung,pin-pud = <3>; + samsung,pin-drv = <0>; + }; + ec_irq: ec-irq { samsung,pins = "gpx1-5"; samsung,pin-function = <0>; @@ -773,6 +812,13 @@ samsung,pin-drv = <0>; }; + wp_gpio: wp_gpio { + samsung,pins = "gpx3-0"; + samsung,pin-function = <0>; + samsung,pin-pud = <0>; + samsung,pin-drv = <0>; + }; + max77802_irq: max77802-irq { samsung,pins = "gpx3-1"; samsung,pin-function = <0>; -- 2.1.4 ^ permalink raw reply related [flat|nested] 53+ messages in thread
end of thread, other threads:[~2016-03-07 8:27 UTC | newest] Thread overview: 53+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-12-04 17:31 Add support for monitoring gpio switches Martyn Welch 2015-12-04 17:31 ` Martyn Welch 2015-12-04 17:31 ` Martyn Welch 2015-12-04 17:31 ` [PATCH 1/3] Device tree binding documentation for gpio-switch Martyn Welch 2015-12-04 17:31 ` Martyn Welch 2015-12-07 17:37 ` Rob Herring 2015-12-07 17:37 ` Rob Herring 2015-12-07 21:10 ` Martyn Welch 2015-12-07 21:10 ` Martyn Welch 2015-12-11 12:39 ` Linus Walleij 2015-12-11 12:39 ` Linus Walleij 2015-12-11 12:39 ` Linus Walleij 2015-12-11 14:06 ` Rob Herring 2015-12-11 14:06 ` Rob Herring 2015-12-11 14:06 ` Rob Herring 2015-12-14 14:28 ` Linus Walleij 2015-12-14 14:28 ` Linus Walleij 2015-12-14 14:28 ` Linus Walleij 2015-12-14 15:45 ` Rob Herring 2015-12-14 15:45 ` Rob Herring 2015-12-14 15:45 ` Rob Herring 2015-12-15 9:09 ` Markus Pargmann 2015-12-15 9:09 ` Markus Pargmann 2016-03-02 16:03 ` Rob Herring 2016-03-02 16:03 ` Rob Herring 2016-03-02 16:03 ` Rob Herring 2016-03-07 8:26 ` Markus Pargmann 2016-03-07 8:26 ` Markus Pargmann 2016-03-07 8:26 ` Markus Pargmann 2015-12-04 17:31 ` [PATCH 2/3] Add support for monitoring gpio switches Martyn Welch 2015-12-04 17:31 ` Martyn Welch 2015-12-04 17:31 ` Martyn Welch 2015-12-04 18:14 ` [PATCH] fix noderef.cocci warnings kbuild test robot 2015-12-04 18:14 ` kbuild test robot 2015-12-04 18:14 ` kbuild test robot 2015-12-04 18:14 ` [PATCH 2/3] Add support for monitoring gpio switches kbuild test robot 2015-12-04 18:14 ` kbuild test robot 2015-12-04 18:14 ` kbuild test robot 2015-12-04 18:57 ` Greg Kroah-Hartman 2015-12-04 18:57 ` Greg Kroah-Hartman 2015-12-05 10:42 ` Martyn Welch 2015-12-05 10:42 ` Martyn Welch 2015-12-11 9:08 ` Linus Walleij 2015-12-11 9:08 ` Linus Walleij 2015-12-11 9:08 ` Linus Walleij 2015-12-16 10:11 ` Martyn Welch 2015-12-16 10:11 ` Martyn Welch 2015-12-16 10:11 ` Martyn Welch 2015-12-22 9:25 ` Linus Walleij 2015-12-22 9:25 ` Linus Walleij 2015-12-22 9:25 ` Linus Walleij 2015-12-04 17:31 ` [PATCH 3/3] ARM: dts: Addition of binding for gpio switches on peach-pi Martyn Welch 2015-12-04 17:31 ` Martyn Welch
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.