diff mbox

[3/4] gpio: dt-bindings: Declare gpio-hog optional for GPIO subnodes

Message ID 1456214089-13954-3-git-send-email-mpa@pengutronix.de
State Changes Requested, archived
Headers show

Commit Message

Markus Pargmann Feb. 23, 2016, 7:54 a.m. UTC
Reuse the GPIO hogging DT bindings for more generic specifications of
the GPIO. Therefore gpio-hog is declared optional.

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
 Documentation/devicetree/bindings/gpio/gpio.txt | 28 ++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

Comments

Linus Walleij Feb. 23, 2016, 1:37 p.m. UTC | #1
On Tue, Feb 23, 2016 at 8:54 AM, Markus Pargmann <mpa@pengutronix.de> wrote:

> Reuse the GPIO hogging DT bindings for more generic specifications of
> the GPIO. Therefore gpio-hog is declared optional.
>
> Signed-off-by: Markus Pargmann <mpa@pengutronix.de>

I like this binding for names since it reuse what we already have,
but will allow some time for others to comment.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rob Herring Feb. 23, 2016, 11:25 p.m. UTC | #2
On Tue, Feb 23, 2016 at 08:54:48AM +0100, Markus Pargmann wrote:
> Reuse the GPIO hogging DT bindings for more generic specifications of
> the GPIO. Therefore gpio-hog is declared optional.

Before extending this binding further, we need to consider other cases 
that have been discussed recently such as gpio switch binding and 
whether it should fit in with this.

My biggest issue with this binding is whether the DT changes because 
today we don't have a driver for some GPIO line and tomorrow we do.

That said, some specific comments below:

> Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> ---
>  Documentation/devicetree/bindings/gpio/gpio.txt | 28 ++++++++++++++++++-------
>  1 file changed, 21 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/gpio/gpio.txt b/Documentation/devicetree/bindings/gpio/gpio.txt
> index 069cdf6f9dac..820898e0649c 100644
> --- a/Documentation/devicetree/bindings/gpio/gpio.txt
> +++ b/Documentation/devicetree/bindings/gpio/gpio.txt
> @@ -155,13 +155,25 @@ gpio-controller@00000000 {
>  	ngpios = <18>;
>  }
>  
> -The GPIO chip may contain GPIO hog definitions. GPIO hogging is a mechanism
> -providing automatic GPIO request and configuration as part of the
> -gpio-controller's driver probe function.
> +The GPIO chip may contain subnodes describing specific GPIOs of this
> +controller. These GPIO nodes can be used to define GPIO hogging or define GPIO
> +names.
> +GPIO hogging is a mechanism providing automatic GPIO request and configuration
> +as part of the gpio-controller's driver probe function.

Linux driver details don't belong in binding descriptions.

> -Each GPIO hog definition is represented as a child node of the GPIO controller.
> +Each GPIO node consists of the following properties:
>  Required properties:
> +- gpios:      Store the GPIO information (id, flags, ...). Shall contain the
> +	      number of cells specified in its parent node (GPIO controller
> +	      node).
> +
> +Optional properties:
> +- line-name:  The GPIO name. If not present the node name is used.

I'd like to deprecate line-name in favor of the standard property 
for user visible names "label".

>  - gpio-hog:   A property specifying that this child node represent a GPIO hog.
> +
> +For GPIO hogging which is specified by the "gpio-hog" property the following
> +additional properties are required.
> +Required properties for GPIO hogging:
>  - gpios:      Store the GPIO information (id, flags, ...). Shall contain the

This doesn't make sense. gpios is always required and already listed 
above.

>  	      number of cells specified in its parent node (GPIO controller
>  	      node).
> @@ -175,9 +187,6 @@ configuration.
>  - output-high A property specifying to set the GPIO direction as output with
>  	      the value high.
>  
> -Optional properties:
> -- line-name:  The GPIO label name. If not present the node name is used.
> -
>  Example of two SOC GPIO banks defined as gpio-controller nodes:
>  
>  	qe_pio_a: gpio-controller@1400 {
> @@ -192,6 +201,11 @@ Example of two SOC GPIO banks defined as gpio-controller nodes:
>  			output-low;
>  			line-name = "foo-bar-gpio";
>  		};
> +
> +		line_c {
> +			gpios = <7 0>;
> +			line-name = "SCHEMATIC_LINE_NAME";
> +		};
>  	};
>  
>  	qe_pio_e: gpio-controller@1460 {
> -- 
> 2.7.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Markus Pargmann Feb. 24, 2016, 7:01 a.m. UTC | #3
Hi,

On Tuesday, February 23, 2016 05:25:34 PM Rob Herring wrote:
> On Tue, Feb 23, 2016 at 08:54:48AM +0100, Markus Pargmann wrote:
> > Reuse the GPIO hogging DT bindings for more generic specifications of
> > the GPIO. Therefore gpio-hog is declared optional.
> 
> Before extending this binding further, we need to consider other cases 
> that have been discussed recently such as gpio switch binding and 
> whether it should fit in with this.

I tried to discuss my ideas and issues with the gpio-switch bindings
already some time ago, unfortunately the discussion didn't proceed:
	http://thread.gmane.org/gmane.linux.kernel/2100574

> 
> My biggest issue with this binding is whether the DT changes because 
> today we don't have a driver for some GPIO line and tomorrow we do.

This patch is just about defining that we can give GPIO lines names. In
the best case this name refers to the line name on the schematic. In the
worst case it is some arbitrary name. In both cases it should not
influence any driver that is trying to use this GPIO.

gpio-hogging indeed could cause problems for future drivers as these
GPIOs are not available for any drivers. But gpio-hogging is already
defined for quite some time in the DT bindings.
Also I would assume that for most new drivers new DT bindings are
created so that it already requires a DT update together with a kernel
update.

> 
> That said, some specific comments below:
> 
> > Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> > ---
> >  Documentation/devicetree/bindings/gpio/gpio.txt | 28 ++++++++++++++++++-------
> >  1 file changed, 21 insertions(+), 7 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/gpio/gpio.txt b/Documentation/devicetree/bindings/gpio/gpio.txt
> > index 069cdf6f9dac..820898e0649c 100644
> > --- a/Documentation/devicetree/bindings/gpio/gpio.txt
> > +++ b/Documentation/devicetree/bindings/gpio/gpio.txt
> > @@ -155,13 +155,25 @@ gpio-controller@00000000 {
> >  	ngpios = <18>;
> >  }
> >  
> > -The GPIO chip may contain GPIO hog definitions. GPIO hogging is a mechanism
> > -providing automatic GPIO request and configuration as part of the
> > -gpio-controller's driver probe function.
> > +The GPIO chip may contain subnodes describing specific GPIOs of this
> > +controller. These GPIO nodes can be used to define GPIO hogging or define GPIO
> > +names.
> > +GPIO hogging is a mechanism providing automatic GPIO request and configuration
> > +as part of the gpio-controller's driver probe function.
> 
> Linux driver details don't belong in binding descriptions.

This last sentence was only moved a bit. So perhaps this should be
changed in a separate patch.

> 
> > -Each GPIO hog definition is represented as a child node of the GPIO controller.
> > +Each GPIO node consists of the following properties:
> >  Required properties:
> > +- gpios:      Store the GPIO information (id, flags, ...). Shall contain the
> > +	      number of cells specified in its parent node (GPIO controller
> > +	      node).
> > +
> > +Optional properties:
> > +- line-name:  The GPIO name. If not present the node name is used.
> 
> I'd like to deprecate line-name in favor of the standard property 
> for user visible names "label".
> 
> >  - gpio-hog:   A property specifying that this child node represent a GPIO hog.
> > +
> > +For GPIO hogging which is specified by the "gpio-hog" property the following
> > +additional properties are required.
> > +Required properties for GPIO hogging:
> >  - gpios:      Store the GPIO information (id, flags, ...). Shall contain the
> 
> This doesn't make sense. gpios is always required and already listed 
> above.

Thanks, will remove it.

Best Regards,

Markus Pargmann
Rob Herring March 2, 2016, 3:41 p.m. UTC | #4
On Wed, Feb 24, 2016 at 08:01:47AM +0100, Markus Pargmann wrote:
> Hi,
> 
> On Tuesday, February 23, 2016 05:25:34 PM Rob Herring wrote:
> > On Tue, Feb 23, 2016 at 08:54:48AM +0100, Markus Pargmann wrote:
> > > Reuse the GPIO hogging DT bindings for more generic specifications of
> > > the GPIO. Therefore gpio-hog is declared optional.
> > 
> > Before extending this binding further, we need to consider other cases 
> > that have been discussed recently such as gpio switch binding and 
> > whether it should fit in with this.
> 
> I tried to discuss my ideas and issues with the gpio-switch bindings
> already some time ago, unfortunately the discussion didn't proceed:
> 	http://thread.gmane.org/gmane.linux.kernel/2100574

We need to revive that. I do plan to discuss this with Linus F2F next 
week.

> > My biggest issue with this binding is whether the DT changes because 
> > today we don't have a driver for some GPIO line and tomorrow we do.
> 
> This patch is just about defining that we can give GPIO lines names. In
> the best case this name refers to the line name on the schematic. In the
> worst case it is some arbitrary name. In both cases it should not
> influence any driver that is trying to use this GPIO.

No issues with adding this information. The question is just how should 
all this fit together (line names, simple functions (e.g. switches), and 
initialization). The overall structure needs to extendable. As Linus 
said, we need a proposal that addresses all these issues.

> gpio-hogging indeed could cause problems for future drivers as these
> GPIOs are not available for any drivers. But gpio-hogging is already
> defined for quite some time in the DT bindings.

That doesn't mean we can't decide some bindings were a bad idea and move 
towards something new.

> Also I would assume that for most new drivers new DT bindings are
> created so that it already requires a DT update together with a kernel
> update.

Frequently that may be right. However, I think having a temporary 
binding only encourages to not fully define their h/w. Think of the case 
where you have a single DTB and support multiple OSs (or different 
versions). The DTB would have to work with both as one OS may require 
gpio-hogs and the other may require a specific driver. That should be 
possible to do. Requiring removing the gpio-hog to add a specific 
binding is a problem. AIUI, a hog will prevent any other user in the 
kernel. I think that is backwards in that a hog should only provide some 
default configuration/use. 

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Walleij March 9, 2016, 3:53 a.m. UTC | #5
On Wed, Feb 24, 2016 at 6:25 AM, Rob Herring <robh@kernel.org> wrote:
> On Tue, Feb 23, 2016 at 08:54:48AM +0100, Markus Pargmann wrote:

>> +Optional properties:
>> +- line-name:  The GPIO name. If not present the node name is used.
>
> I'd like to deprecate line-name in favor of the standard property
> for user visible names "label".

This causes some unfortunate confusion wrt internal linux naming.

GPIO lines have two "names"

- The "name" is what the line (pin) is named
  on the chip, or the name of the rail going out on the board, from the chip,
  so it is a producer name.

- Then the GPIO consumers set a second name, called "label"
  (confusingly) so we can see in e.g. debugfs and now also the userspace
  ABI who is using it.

I named these "name" and "consumer" in the userspace ABI, I think
I should take a round and rename it from "label" to "consumer"
also inside the kernel to avoid confusion with this, because "label"
in DT is going to be converted to "name" in the GPIOlib and
then it also has something named "label" and that is another thing,
argh!

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Walleij March 9, 2016, 4:07 a.m. UTC | #6
On Wed, Mar 2, 2016 at 10:41 PM, Rob Herring <robh@kernel.org> wrote:

> Requiring removing the gpio-hog to add a specific
> binding is a problem. AIUI, a hog will prevent any other user in the
> kernel. I think that is backwards in that a hog should only provide some
> default configuration/use.

I think there is some misunderstanding here.

- HOGS are done to hook a line up in a way so that
  it is *used* for something, such as biasing a MMC CD line
  in this example from arch/arm/boot/dts/ste-nomadik-s8815.dts:

        gpio3: gpio@101e7000 {
                /* This hog will bias the MMC/SD card detect line */
                mmcsd-gpio {
                        gpio-hog;
                        gpios = <16 0x0>;
                        output-low;
                        line-name = "card detect bias";
                };
        };

   So it's not like this line is "unused" or something.

- For the case when we want to just set up some default values
  in some registers for a GPIO line and just leave it for whatever
  consumer may turn up (whether a driver or e.g. userspace), we
  have had proposals for
  setting up initial values from the DT, using essentially the hog
  syntax but with another bool than "gpio-hog" obviously,
  "gpio-initval" or just leaving it blank was suggested in the former
  thread. (See switch discussion.)

So my view is if we want just initial configuration of lines, then handle
that separately, do not try to change the semantic meaning of the
hogs to be something else, let hogs be hogs.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rob Herring March 9, 2016, 4:27 a.m. UTC | #7
On Tue, Mar 8, 2016 at 9:53 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Wed, Feb 24, 2016 at 6:25 AM, Rob Herring <robh@kernel.org> wrote:
>> On Tue, Feb 23, 2016 at 08:54:48AM +0100, Markus Pargmann wrote:
>
>>> +Optional properties:
>>> +- line-name:  The GPIO name. If not present the node name is used.
>>
>> I'd like to deprecate line-name in favor of the standard property
>> for user visible names "label".
>
> This causes some unfortunate confusion wrt internal linux naming.

Yes.

> GPIO lines have two "names"
>
> - The "name" is what the line (pin) is named
>   on the chip, or the name of the rail going out on the board, from the chip,
>   so it is a producer name.

Who/what uses this?

> - Then the GPIO consumers set a second name, called "label"
>   (confusingly) so we can see in e.g. debugfs and now also the userspace
>   ABI who is using it.
>
> I named these "name" and "consumer" in the userspace ABI, I think
> I should take a round and rename it from "label" to "consumer"
> also inside the kernel to avoid confusion with this, because "label"
> in DT is going to be converted to "name" in the GPIOlib and
> then it also has something named "label" and that is another thing,
> argh!

There is still confusion because DT label should be the consumer side,
not the SoC pin name.

Then of course you could have 3 levels of names needed if you have SoC
pin, board connector pin, and mezzanine consumer.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Walleij March 9, 2016, 5:56 a.m. UTC | #8
On Wed, Mar 9, 2016 at 11:27 AM, Rob Herring <robh@kernel.org> wrote:
> On Tue, Mar 8, 2016 at 9:53 PM, Linus Walleij <linus.walleij@linaro.org> wrote:

>> - The "name" is what the line (pin) is named
>>   on the chip, or the name of the rail going out on the board, from the chip,
>>   so it is a producer name.
>
> Who/what uses this?

The names[] in struct gpio_chip has been used for years to name
the lines of the GPIO chips.

The requirement coming from the community for userspace GPIO
is about being able to name the lines on the producer side. One example
is 96boards that want to consistently name lines on a header from
the producer side across a set of boards, no matter what SoC is
used. In their case the actual consumer is some one-off peripheral
driven from userspace, so there (I expect) userspace will set up the
label/consumer name eventually, if any.

>> I named these "name" and "consumer" in the userspace ABI, I think
>> I should take a round and rename it from "label" to "consumer"
>> also inside the kernel to avoid confusion with this, because "label"
>> in DT is going to be converted to "name" in the GPIOlib and
>> then it also has something named "label" and that is another thing,
>> argh!
>
> There is still confusion because DT label should be the consumer side,
> not the SoC pin name.

Oh I see what you mean. That is actually how it works with
the hogs, it is setting the consumer side of things through gpiod_get().

> Then of course you could have 3 levels of names needed if you have SoC
> pin, board connector pin, and mezzanine consumer.

I don't think the distinction between pin and board connector is
needed, if it is then the connector should have its own DT node
and I suspect that would be overkill. I am atleast optimistically
assuming that "name" will cover pin/rail/header name - whatever
makes most sense - for a GPIO line producer.

Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/gpio/gpio.txt b/Documentation/devicetree/bindings/gpio/gpio.txt
index 069cdf6f9dac..820898e0649c 100644
--- a/Documentation/devicetree/bindings/gpio/gpio.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio.txt
@@ -155,13 +155,25 @@  gpio-controller@00000000 {
 	ngpios = <18>;
 }
 
-The GPIO chip may contain GPIO hog definitions. GPIO hogging is a mechanism
-providing automatic GPIO request and configuration as part of the
-gpio-controller's driver probe function.
+The GPIO chip may contain subnodes describing specific GPIOs of this
+controller. These GPIO nodes can be used to define GPIO hogging or define GPIO
+names.
+GPIO hogging is a mechanism providing automatic GPIO request and configuration
+as part of the gpio-controller's driver probe function.
 
-Each GPIO hog definition is represented as a child node of the GPIO controller.
+Each GPIO node consists of the following properties:
 Required properties:
+- gpios:      Store the GPIO information (id, flags, ...). Shall contain the
+	      number of cells specified in its parent node (GPIO controller
+	      node).
+
+Optional properties:
+- line-name:  The GPIO name. If not present the node name is used.
 - gpio-hog:   A property specifying that this child node represent a GPIO hog.
+
+For GPIO hogging which is specified by the "gpio-hog" property the following
+additional properties are required.
+Required properties for GPIO hogging:
 - gpios:      Store the GPIO information (id, flags, ...). Shall contain the
 	      number of cells specified in its parent node (GPIO controller
 	      node).
@@ -175,9 +187,6 @@  configuration.
 - output-high A property specifying to set the GPIO direction as output with
 	      the value high.
 
-Optional properties:
-- line-name:  The GPIO label name. If not present the node name is used.
-
 Example of two SOC GPIO banks defined as gpio-controller nodes:
 
 	qe_pio_a: gpio-controller@1400 {
@@ -192,6 +201,11 @@  Example of two SOC GPIO banks defined as gpio-controller nodes:
 			output-low;
 			line-name = "foo-bar-gpio";
 		};
+
+		line_c {
+			gpios = <7 0>;
+			line-name = "SCHEMATIC_LINE_NAME";
+		};
 	};
 
 	qe_pio_e: gpio-controller@1460 {