| Message ID | 20230530151946.2317748-2-u.kleine-koenig@pengutronix.de |
|---|---|
| State | New |
| Headers | show |
| Series | gpio: introduce hog properties with less ambiguity | expand |
Hey Uwe, On Tue, May 30, 2023 at 05:19:45PM +0200, Uwe Kleine-König wrote: > For active low lines the semantic of output-low and output-high is hard > to grasp because there is a double negation involved and so output-low > is actually a request to drive the line high (aka inactive). > > So introduce output-inactive and output-active with the same semantic as > output-low and output-high respectively have today, but with a more > sensible name. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- > Documentation/devicetree/bindings/gpio/gpio.txt | 16 +++++++++++----- > 1 file changed, 11 insertions(+), 5 deletions(-) > > diff --git a/Documentation/devicetree/bindings/gpio/gpio.txt b/Documentation/devicetree/bindings/gpio/gpio.txt > index d82c32217fff..2f037bbd3ffa 100644 > --- a/Documentation/devicetree/bindings/gpio/gpio.txt > +++ b/Documentation/devicetree/bindings/gpio/gpio.txt > @@ -209,15 +209,21 @@ Required properties: > - gpios: Store the GPIO information (id, flags, ...) for each GPIO to > affect. Shall contain an integer multiple of the number of cells > specified in its parent node (GPIO controller node). > + > Only one of the following properties scanned in the order shown below. > This means that when multiple properties are present they will be searched > in the order presented below and the first match is taken as the intended > configuration. > -- input: A property specifying to set the GPIO direction as input. > -- output-low A property specifying to set the GPIO direction as output with > - the value low. > -- output-high A property specifying to set the GPIO direction as output with > - the value high. > +- input: A property specifying to set the GPIO direction as input. > +- output-inactive: A property specifying to set the GPIO direction as output > + with the inactive value (depending on the line's polarity, > + which is active-high by default) > +- output-active: A property specifying to set the GPIO direction as output > + with the active value. > + > +For backwards compatibility "output-low" and "output-high" are supported as > +aliases for "output-inactive" and "output-active" respectively. Their usage is > +misleading for active-low outputs, so their use is discouraged. Seems like an improvement to me, Acked-by: Conor Dooley <conor.dooley@microchip.com> Rob did note that gpio-hog.yaml in dt-schema would need to be updated, but he's not around right now to approve anything there. Cheers, Conor.
Hi, Am Dienstag, 30. Mai 2023, 17:19:45 CEST schrieb Uwe Kleine-König: > For active low lines the semantic of output-low and output-high is hard > to grasp because there is a double negation involved and so output-low > is actually a request to drive the line high (aka inactive). > > So introduce output-inactive and output-active with the same semantic as > output-low and output-high respectively have today, but with a more > sensible name. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- > Documentation/devicetree/bindings/gpio/gpio.txt | 16 +++++++++++----- > 1 file changed, 11 insertions(+), 5 deletions(-) > > diff --git a/Documentation/devicetree/bindings/gpio/gpio.txt > b/Documentation/devicetree/bindings/gpio/gpio.txt index > d82c32217fff..2f037bbd3ffa 100644 > --- a/Documentation/devicetree/bindings/gpio/gpio.txt > +++ b/Documentation/devicetree/bindings/gpio/gpio.txt > @@ -209,15 +209,21 @@ Required properties: > - gpios: Store the GPIO information (id, flags, ...) for each GPIO to > affect. Shall contain an integer multiple of the number of cells > specified in its parent node (GPIO controller node). > + > Only one of the following properties scanned in the order shown below. > This means that when multiple properties are present they will be searched > in the order presented below and the first match is taken as the intended > configuration. > -- input: A property specifying to set the GPIO direction as input. > -- output-low A property specifying to set the GPIO direction as output > with - the value low. > -- output-high A property specifying to set the GPIO direction as output > with - the value high. > +- input: A property specifying to set the GPIO direction as > input. +- output-inactive: A property specifying to set the GPIO > direction as output + with the inactive value (depending on the > line's polarity, + which is active-high by default) > +- output-active: A property specifying to set the GPIO direction as > output + with the active value. I know this is essentially just renaming currently existing properties. But these mutual exclusive (boolean) properties make it impossible to change them in DT overlay. Any ideas how to support changing the output level onGPIO hogs in DT overlay? Despite that, this change looks sensible to me. Best regards, Alexander > +For backwards compatibility "output-low" and "output-high" are supported as > +aliases for "output-inactive" and "output-active" respectively. Their > usage is +misleading for active-low outputs, so their use is discouraged. > > Optional properties: > - line-name: The GPIO label name. If not present the node name is used.
On Wed, May 31, 2023 at 08:18:14AM +0200, Alexander Stein wrote: > Hi, > > Am Dienstag, 30. Mai 2023, 17:19:45 CEST schrieb Uwe Kleine-König: > > For active low lines the semantic of output-low and output-high is hard > > to grasp because there is a double negation involved and so output-low > > is actually a request to drive the line high (aka inactive). > > > > So introduce output-inactive and output-active with the same semantic as > > output-low and output-high respectively have today, but with a more > > sensible name. > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > --- > > Documentation/devicetree/bindings/gpio/gpio.txt | 16 +++++++++++----- > > 1 file changed, 11 insertions(+), 5 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/gpio/gpio.txt > > b/Documentation/devicetree/bindings/gpio/gpio.txt index > > d82c32217fff..2f037bbd3ffa 100644 > > --- a/Documentation/devicetree/bindings/gpio/gpio.txt > > +++ b/Documentation/devicetree/bindings/gpio/gpio.txt > > @@ -209,15 +209,21 @@ Required properties: > > - gpios: Store the GPIO information (id, flags, ...) for each GPIO to > > affect. Shall contain an integer multiple of the number of > cells > > specified in its parent node (GPIO controller node). > > + > > Only one of the following properties scanned in the order shown below. > > This means that when multiple properties are present they will be searched > > in the order presented below and the first match is taken as the intended > > configuration. > > -- input: A property specifying to set the GPIO direction as input. > > -- output-low A property specifying to set the GPIO direction as output > > with - the value low. > > -- output-high A property specifying to set the GPIO direction as output > > with - the value high. > > +- input: A property specifying to set the GPIO direction as > > input. +- output-inactive: A property specifying to set the GPIO > > direction as output + with the inactive value > (depending on the > > line's polarity, + which is active-high by default) > > +- output-active: A property specifying to set the GPIO direction as > > output + with the active value. > > I know this is essentially just renaming currently existing properties. > But these mutual exclusive (boolean) properties make it impossible to change > them in DT overlay. Any ideas how to support changing the output level onGPIO > hogs in DT overlay? The universal way to address this would be to make it possible in dtbos to delete properties (and nodes). Best regards Uwe
Hello Conor, On Tue, May 30, 2023 at 11:20:38PM +0100, Conor Dooley wrote: > On Tue, May 30, 2023 at 05:19:45PM +0200, Uwe Kleine-König wrote: > > For active low lines the semantic of output-low and output-high is hard > > to grasp because there is a double negation involved and so output-low > > is actually a request to drive the line high (aka inactive). > > > > So introduce output-inactive and output-active with the same semantic as > > output-low and output-high respectively have today, but with a more > > sensible name. > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > --- > > Documentation/devicetree/bindings/gpio/gpio.txt | 16 +++++++++++----- > > 1 file changed, 11 insertions(+), 5 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/gpio/gpio.txt b/Documentation/devicetree/bindings/gpio/gpio.txt > > index d82c32217fff..2f037bbd3ffa 100644 > > --- a/Documentation/devicetree/bindings/gpio/gpio.txt > > +++ b/Documentation/devicetree/bindings/gpio/gpio.txt > > @@ -209,15 +209,21 @@ Required properties: > > - gpios: Store the GPIO information (id, flags, ...) for each GPIO to > > affect. Shall contain an integer multiple of the number of cells > > specified in its parent node (GPIO controller node). > > + > > Only one of the following properties scanned in the order shown below. > > This means that when multiple properties are present they will be searched > > in the order presented below and the first match is taken as the intended > > configuration. > > -- input: A property specifying to set the GPIO direction as input. > > -- output-low A property specifying to set the GPIO direction as output with > > - the value low. > > -- output-high A property specifying to set the GPIO direction as output with > > - the value high. > > +- input: A property specifying to set the GPIO direction as input. > > +- output-inactive: A property specifying to set the GPIO direction as output > > + with the inactive value (depending on the line's polarity, > > + which is active-high by default) > > +- output-active: A property specifying to set the GPIO direction as output > > + with the active value. > > + > > +For backwards compatibility "output-low" and "output-high" are supported as > > +aliases for "output-inactive" and "output-active" respectively. Their usage is > > +misleading for active-low outputs, so their use is discouraged. > > Seems like an improvement to me, > Acked-by: Conor Dooley <conor.dooley@microchip.com> Thanks! > Rob did note that gpio-hog.yaml in dt-schema would need to be updated, This is a followup-change in a separate repository once the change under discussion is in mainline, right? Best regards Uwe
On Wed, May 31, 2023 at 09:03:51AM +0200, Uwe Kleine-König wrote: > > Rob did note that gpio-hog.yaml in dt-schema would need to be updated, > > This is a followup-change in a separate repository once the change > under discussion is in mainline, right? Yeah, https://github.com/devicetree-org/dt-schema Cheers, Conor.
On Tue, May 30, 2023 at 05:19:45PM +0200, Uwe Kleine-König wrote: > For active low lines the semantic of output-low and output-high is hard > to grasp because there is a double negation involved and so output-low > is actually a request to drive the line high (aka inactive). > > So introduce output-inactive and output-active with the same semantic as > output-low and output-high respectively have today, but with a more > sensible name. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- > Documentation/devicetree/bindings/gpio/gpio.txt | 16 +++++++++++----- > 1 file changed, 11 insertions(+), 5 deletions(-) Please add this to the schemas in dtschema. Everything in gpio.txt should be removed. If that hasn't been done, it's because what's here needs to be relicensed. Looks like you need TI's and Geert's permission for the hog part. Anything from Grant, Linaro or NVIDIA I've gotten permission for as well. Rob
Hi Alexander, On Wed, May 31, 2023 at 8:19 AM Alexander Stein <alexander.stein@ew.tq-group.com> wrote: > Am Dienstag, 30. Mai 2023, 17:19:45 CEST schrieb Uwe Kleine-König: > > For active low lines the semantic of output-low and output-high is hard > > to grasp because there is a double negation involved and so output-low > > is actually a request to drive the line high (aka inactive). > > > > So introduce output-inactive and output-active with the same semantic as > > output-low and output-high respectively have today, but with a more > > sensible name. > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > --- > > Documentation/devicetree/bindings/gpio/gpio.txt | 16 +++++++++++----- > > 1 file changed, 11 insertions(+), 5 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/gpio/gpio.txt > > b/Documentation/devicetree/bindings/gpio/gpio.txt index > > d82c32217fff..2f037bbd3ffa 100644 > > --- a/Documentation/devicetree/bindings/gpio/gpio.txt > > +++ b/Documentation/devicetree/bindings/gpio/gpio.txt > > @@ -209,15 +209,21 @@ Required properties: > > - gpios: Store the GPIO information (id, flags, ...) for each GPIO to > > affect. Shall contain an integer multiple of the number of > cells > > specified in its parent node (GPIO controller node). > > + > > Only one of the following properties scanned in the order shown below. > > This means that when multiple properties are present they will be searched > > in the order presented below and the first match is taken as the intended > > configuration. > > -- input: A property specifying to set the GPIO direction as input. > > -- output-low A property specifying to set the GPIO direction as output > > with - the value low. > > -- output-high A property specifying to set the GPIO direction as output > > with - the value high. > > +- input: A property specifying to set the GPIO direction as > > input. +- output-inactive: A property specifying to set the GPIO > > direction as output + with the inactive value > (depending on the > > line's polarity, + which is active-high by default) > > +- output-active: A property specifying to set the GPIO direction as > > output + with the active value. > > I know this is essentially just renaming currently existing properties. > But these mutual exclusive (boolean) properties make it impossible to change > them in DT overlay. Any ideas how to support changing the output level onGPIO > hogs in DT overlay? That's a good point. And despite it not working, people do try to stick e.g. /delete-node/ in .dtso files... I assume you can sort of remove the existing hog subnode by adding status = "disabled" in the DT overlay, and adding a new subnode to configure the new output level? Gr{oetje,eeting}s, Geert
diff --git a/Documentation/devicetree/bindings/gpio/gpio.txt b/Documentation/devicetree/bindings/gpio/gpio.txt index d82c32217fff..2f037bbd3ffa 100644 --- a/Documentation/devicetree/bindings/gpio/gpio.txt +++ b/Documentation/devicetree/bindings/gpio/gpio.txt @@ -209,15 +209,21 @@ Required properties: - gpios: Store the GPIO information (id, flags, ...) for each GPIO to affect. Shall contain an integer multiple of the number of cells specified in its parent node (GPIO controller node). + Only one of the following properties scanned in the order shown below. This means that when multiple properties are present they will be searched in the order presented below and the first match is taken as the intended configuration. -- input: A property specifying to set the GPIO direction as input. -- output-low A property specifying to set the GPIO direction as output with - the value low. -- output-high A property specifying to set the GPIO direction as output with - the value high. +- input: A property specifying to set the GPIO direction as input. +- output-inactive: A property specifying to set the GPIO direction as output + with the inactive value (depending on the line's polarity, + which is active-high by default) +- output-active: A property specifying to set the GPIO direction as output + with the active value. + +For backwards compatibility "output-low" and "output-high" are supported as +aliases for "output-inactive" and "output-active" respectively. Their usage is +misleading for active-low outputs, so their use is discouraged. Optional properties: - line-name: The GPIO label name. If not present the node name is used.
For active low lines the semantic of output-low and output-high is hard to grasp because there is a double negation involved and so output-low is actually a request to drive the line high (aka inactive). So introduce output-inactive and output-active with the same semantic as output-low and output-high respectively have today, but with a more sensible name. Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- Documentation/devicetree/bindings/gpio/gpio.txt | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)