| Message ID | 20210503210526.43455-1-u.kleine-koenig@pengutronix.de |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] dt-bindings: gpio: introduce hog properties with less ambiguity | expand |
On Mon, May 03, 2021 at 11:05:26PM +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). > +1 on clarifying the naming. > 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. > You use active/inactive here, but then asserted/deasserted in the patch. My preference would be the active/inactive, which has more of a level feel, over the asserted/deasserted which feels more like an edge. And you still use active/inactive in the descriptions, so now we have all three naming schemes in the mix. What made you change? Cheers, Kent. > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- > Hello, > > I already sent this patch back in July and Linus (Walleij) liked the > patch but asked for an implementation. For that I added the second patch > now. > > Best regards > Uwe > > Documentation/devicetree/bindings/gpio/gpio.txt | 15 ++++++++++----- > 1 file changed, 10 insertions(+), 5 deletions(-) > > diff --git a/Documentation/devicetree/bindings/gpio/gpio.txt b/Documentation/devicetree/bindings/gpio/gpio.txt > index a8895d339bfe..1061c346a619 100644 > --- a/Documentation/devicetree/bindings/gpio/gpio.txt > +++ b/Documentation/devicetree/bindings/gpio/gpio.txt > @@ -196,11 +196,16 @@ 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-deasserted: 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-asserted: A property specifying to set the GPIO direction as output > + with the active value. > + > +For backwards compatibility "output-low" and "output-high" should be supported > +as aliases for "output-deasserted" and "output-asserted" 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. > -- > 2.30.2 >
Hello, On Tue, May 04, 2021 at 10:55:46AM +0800, Kent Gibson wrote: > On Mon, May 03, 2021 at 11:05:26PM +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). > > +1 on clarifying the naming. > > > 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. > > > > You use active/inactive here, but then asserted/deasserted in the patch. oops, this is an oversight. > My preference would be the active/inactive, which has more of a level > feel, over the asserted/deasserted which feels more like an edge. > > And you still use active/inactive in the descriptions, so now we have all > three naming schemes in the mix. > > What made you change? I had active/inactive first, but Linux Walleij requested asserted/deasserted: https://lore.kernel.org/r/CACRpkdbccHbhYcCyPiSoA7+zGXBtbL_LwLkPB3vQDyOqkTA7EQ@mail.gmail.com While I like active/inactive better than asserted/deasserted, the latter is still way better than high/low, so I didn't discuss. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ |
On Tue, May 04, 2021 at 11:14:59AM +0200, Uwe Kleine-König wrote: > Hello, > > On Tue, May 04, 2021 at 10:55:46AM +0800, Kent Gibson wrote: > > On Mon, May 03, 2021 at 11:05:26PM +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). > > > > +1 on clarifying the naming. > > > > > 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. > > > > > > > You use active/inactive here, but then asserted/deasserted in the patch. > > oops, this is an oversight. > > > My preference would be the active/inactive, which has more of a level > > feel, over the asserted/deasserted which feels more like an edge. > > > > And you still use active/inactive in the descriptions, so now we have all > > three naming schemes in the mix. > > > > What made you change? > > I had active/inactive first, but Linux Walleij requested > asserted/deasserted: > > https://lore.kernel.org/r/CACRpkdbccHbhYcCyPiSoA7+zGXBtbL_LwLkPB3vQDyOqkTA7EQ@mail.gmail.com > Thanks - I'd missed that. I don't suppose you happen to have a link to the gpiod_set_value() discussion that Linus mentions? > While I like active/inactive better than asserted/deasserted, the latter > is still way better than high/low, so I didn't discuss. > As a native English speaker, I find deasserted to be awkward - though it is the appropriate negative of asserted in this context. And there is no escaping the naming of the active-low, so I'm curious to know if there is a good reason not to go with active/inactive. Cheers, Kent.
Hello, On Tue, May 04, 2021 at 06:24:54PM +0800, Kent Gibson wrote: > On Tue, May 04, 2021 at 11:14:59AM +0200, Uwe Kleine-König wrote: > > On Tue, May 04, 2021 at 10:55:46AM +0800, Kent Gibson wrote: > > > On Mon, May 03, 2021 at 11:05:26PM +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). > > > > > > +1 on clarifying the naming. > > > > > > > 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. > > > > > > > > > > You use active/inactive here, but then asserted/deasserted in the patch. > > > > oops, this is an oversight. > > > > > My preference would be the active/inactive, which has more of a level > > > feel, over the asserted/deasserted which feels more like an edge. > > > > > > And you still use active/inactive in the descriptions, so now we have all > > > three naming schemes in the mix. > > > > > > What made you change? > > > > I had active/inactive first, but Linux Walleij requested > > asserted/deasserted: > > > > https://lore.kernel.org/r/CACRpkdbccHbhYcCyPiSoA7+zGXBtbL_LwLkPB3vQDyOqkTA7EQ@mail.gmail.com > > Thanks - I'd missed that. > > I don't suppose you happen to have a link to the gpiod_set_value() > discussion that Linus mentions? I found https://lore.kernel.org/linux-gpio/CACRpkdZAm5AML6cfrX_VrzyADASj1rsVXC3zwtfdo+aRSgX7fQ@mail.gmail.com/ but not that other thread Linus mentions there. I would have expected https://lore.kernel.org/linux-gpio/?q=GPIO_OUT_ASSERTED to find it, but it doesn't. > > While I like active/inactive better than asserted/deasserted, the latter > > is still way better than high/low, so I didn't discuss. > > > > As a native English speaker, I find deasserted to be awkward - though it > is the appropriate negative of asserted in this context. > > And there is no escaping the naming of the active-low, so I'm curious to Ack, we shouldn't rename that to assert-low :-) > know if there is a good reason not to go with active/inactive. Linus: So we're already 3 out of 3 who would like active/inactive better than asserted/deasserted. I'm curious about your preference, too. Best regards Uwe
On Tue, May 4, 2021 at 12:56 PM Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: but not active > > know if there is a good reason not to go with active/inactive. > > Linus: So we're already 3 out of 3 who would like active/inactive better > than asserted/deasserted. I'm curious about your preference, too. I suppose it depends on where you come from. In electronics the terms asserted/deasserted is commonly used and that is where I'm coming from. Maybe just the materials I've been subjected to, who knows. Yours, Linus Walleij
On Mon, May 3, 2021 at 11:06 PM Uwe Kleine-König <u.kleine-koenig@pengutronix.de> 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> Acked-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
On Thu, May 06, 2021 at 02:35:41PM +0200, Linus Walleij wrote: > On Tue, May 4, 2021 at 12:56 PM Uwe Kleine-König > <u.kleine-koenig@pengutronix.de> wrote: > but not active > > > know if there is a good reason not to go with active/inactive. > > > > Linus: So we're already 3 out of 3 who would like active/inactive better > > than asserted/deasserted. I'm curious about your preference, too. > > I suppose it depends on where you come from. In electronics > the terms asserted/deasserted is commonly used and > that is where I'm coming from. Maybe just the materials > I've been subjected to, who knows. > I also come from electronics and, depending on context, deasserted can also mean the line is set to high impedance. Here we are trying to indicate that the line is actively driven to the inactive state, so using output-deasserted would be more open to misinterpretation than output-inactive, no? Cheers, Kent.
On Mon, May 03, 2021 at 11:05:26PM +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> > --- > Hello, > > I already sent this patch back in July and Linus (Walleij) liked the > patch but asked for an implementation. For that I added the second patch > now. > > Best regards > Uwe > > Documentation/devicetree/bindings/gpio/gpio.txt | 15 ++++++++++----- > 1 file changed, 10 insertions(+), 5 deletions(-) The schema in dtschema will need to be updated too. Really, probably all or most of gpio.txt needs to be moved there if not already. But for now, Acked-by: Rob Herring <robh@kernel.org> > > diff --git a/Documentation/devicetree/bindings/gpio/gpio.txt b/Documentation/devicetree/bindings/gpio/gpio.txt > index a8895d339bfe..1061c346a619 100644 > --- a/Documentation/devicetree/bindings/gpio/gpio.txt > +++ b/Documentation/devicetree/bindings/gpio/gpio.txt > @@ -196,11 +196,16 @@ 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-deasserted: 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-asserted: A property specifying to set the GPIO direction as output > + with the active value. > + > +For backwards compatibility "output-low" and "output-high" should be supported > +as aliases for "output-deasserted" and "output-asserted" 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. > -- > 2.30.2 >
diff --git a/Documentation/devicetree/bindings/gpio/gpio.txt b/Documentation/devicetree/bindings/gpio/gpio.txt index a8895d339bfe..1061c346a619 100644 --- a/Documentation/devicetree/bindings/gpio/gpio.txt +++ b/Documentation/devicetree/bindings/gpio/gpio.txt @@ -196,11 +196,16 @@ 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-deasserted: 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-asserted: A property specifying to set the GPIO direction as output + with the active value. + +For backwards compatibility "output-low" and "output-high" should be supported +as aliases for "output-deasserted" and "output-asserted" 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> --- Hello, I already sent this patch back in July and Linus (Walleij) liked the patch but asked for an implementation. For that I added the second patch now. Best regards Uwe Documentation/devicetree/bindings/gpio/gpio.txt | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)