[PATCH/RFC 0/3] i2c: gpio: Add support for named gpios in DT

[PATCH/RFC 0/3] i2c: gpio: Add support for named gpios in DT

[Geert Uytterhoeven]

	Hi all,

The current i2c-gpio DT bindings use a single unnamed "gpios" property
to refer to the SDA and SCL signal lines by index.  This is error-prone
for the casual DT writer and reviewer, as one has to look up the order
in the DT bindings, and leads to the addition of comments in DTS files,
like

	gpios = <&pfc 208 GPIO_ACTIVE_HIGH /* sda */
		 &pfc 91 GPIO_ACTIVE_HIGH /* scl */
		>;

This patch series:
  - Amends the DT bindings to use named gpios,
  - Adds support for the new bindings to the driver,
  - Converts the armadillo800eva to use the new bindings, as an example.

Thanks for your comments!

Geert Uytterhoeven (3):
  dt-bindings: i2c: i2c-gpio: Add support for named gpios
  i2c: gpio: Add support for named gpios in DT
  ARM: dts: armadillo800eva: Convert to named i2c-gpio bindings

 Documentation/devicetree/bindings/i2c/i2c-gpio.txt | 5 +++--
 arch/arm/boot/dts/r8a7740-armadillo800eva.dts      | 5 ++---
 drivers/i2c/busses/i2c-gpio.c                      | 9 +++++++++
 3 files changed, 14 insertions(+), 5 deletions(-)

-- 
2.7.4

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds
--
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

[PATCH/RFC 1/3] dt-bindings: i2c: i2c-gpio: Add support for named gpios

[Geert Uytterhoeven]

The current i2c-gpio DT bindings use a single unnamed "gpios" property
to refer to the SDA and SCL signal lines by index.  This is error-prone
for the casual DT writer and reviewer, as one has to look up the order
in the DT bindings.

Fix this by amending the DT bindings to use two separate named gpios
properties, and deprecate the old unnamed variant.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 Documentation/devicetree/bindings/i2c/i2c-gpio.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-gpio.txt b/Documentation/devicetree/bindings/i2c/i2c-gpio.txt
index 4f8ec947c6bd9cad..61033f73577dd70e 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-gpio.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-gpio.txt
@@ -2,8 +2,9 @@ Device-Tree bindings for i2c gpio driver
 
 Required properties:
 	- compatible = "i2c-gpio";
-	- gpios: sda and scl gpio
-
+	- sda-gpios: gpio used for the sda signal
+	- scl-gpios: gpio used for the scl signal
+	- gpios: sda and scl gpio, alternative for {sda,scl}-gpios (deprecated)
 
 Optional properties:
 	- i2c-gpio,sda-open-drain: sda as open drain
-- 
2.7.4

[PATCH/RFC 2/3] i2c: gpio: Add support for named gpios in DT

[Geert Uytterhoeven]

Add support for the new DT bindings using named gpios, which are less
error-prone than unnamed gpios.  The latter are still supported as a
fallback.

Signed-off-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
---
 drivers/i2c/busses/i2c-gpio.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c
index 0ef8fcc6ac3aca1e..45b7ec99dc210bbe 100644
--- a/drivers/i2c/busses/i2c-gpio.c
+++ b/drivers/i2c/busses/i2c-gpio.c
@@ -88,6 +88,15 @@ static int i2c_gpio_getscl(void *data)
 static int of_i2c_gpio_get_pins(struct device_node *np,
 				unsigned int *sda_pin, unsigned int *scl_pin)
 {
+	*sda_pin = of_get_named_gpio(np, "sda-gpios", 0);
+	*scl_pin = of_get_named_gpio(np, "scl-gpios", 0);
+	if (*sda_pin == -EPROBE_DEFER || *scl_pin == -EPROBE_DEFER)
+		return -EPROBE_DEFER;
+
+	if (gpio_is_valid(*sda_pin) && gpio_is_valid(*scl_pin))
+		return 0;
+
+	/* Try deprecated unnamed gpios */
 	if (of_gpio_count(np) < 2)
 		return -ENODEV;
 
-- 
2.7.4

--
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

[PATCH/RFC 3/3] ARM: dts: armadillo800eva: Convert to named i2c-gpio bindings

[Geert Uytterhoeven]

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/arm/boot/dts/r8a7740-armadillo800eva.dts | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/r8a7740-armadillo800eva.dts b/arch/arm/boot/dts/r8a7740-armadillo800eva.dts
index 153609c1a5d83923..d62d22c36da1c9b9 100644
--- a/arch/arm/boot/dts/r8a7740-armadillo800eva.dts
+++ b/arch/arm/boot/dts/r8a7740-armadillo800eva.dts
@@ -131,9 +131,8 @@
 		#address-cells = <1>;
 		#size-cells = <0>;
 		compatible = "i2c-gpio";
-		gpios = <&pfc 208 GPIO_ACTIVE_HIGH /* sda */
-			 &pfc 91 GPIO_ACTIVE_HIGH /* scl */
-			>;
+		sda-gpios = <&pfc 208 GPIO_ACTIVE_HIGH>;
+		scl-gpios = <&pfc 91 GPIO_ACTIVE_HIGH>;
 		i2c-gpio,delay-us = <5>;
 	};
 
-- 
2.7.4

Re: [PATCH/RFC 1/3] dt-bindings: i2c: i2c-gpio: Add support for named gpios

[Simon Horman]

On Thu, Aug 24, 2017 at 11:21:12AM +0200, Geert Uytterhoeven wrote:
> The current i2c-gpio DT bindings use a single unnamed "gpios" property
> to refer to the SDA and SCL signal lines by index.  This is error-prone
> for the casual DT writer and reviewer, as one has to look up the order
> in the DT bindings.
> 
> Fix this by amending the DT bindings to use two separate named gpios
> properties, and deprecate the old unnamed variant.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/i2c/i2c-gpio.txt | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-gpio.txt b/Documentation/devicetree/bindings/i2c/i2c-gpio.txt
> index 4f8ec947c6bd9cad..61033f73577dd70e 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-gpio.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-gpio.txt
> @@ -2,8 +2,9 @@ Device-Tree bindings for i2c gpio driver
>  
>  Required properties:
>  	- compatible = "i2c-gpio";
> -	- gpios: sda and scl gpio
> -
> +	- sda-gpios: gpio used for the sda signal
> +	- scl-gpios: gpio used for the scl signal
> +	- gpios: sda and scl gpio, alternative for {sda,scl}-gpios (deprecated)
>  
>  Optional properties:
>  	- i2c-gpio,sda-open-drain: sda as open drain

Maybe it would be best to update the example too?
--
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

Re: [PATCH/RFC 1/3] dt-bindings: i2c: i2c-gpio: Add support for named gpios

[Rob Herring]

On Thu, Aug 24, 2017 at 11:21:12AM +0200, Geert Uytterhoeven wrote:
> The current i2c-gpio DT bindings use a single unnamed "gpios" property
> to refer to the SDA and SCL signal lines by index.  This is error-prone
> for the casual DT writer and reviewer, as one has to look up the order
> in the DT bindings.
> 
> Fix this by amending the DT bindings to use two separate named gpios
> properties, and deprecate the old unnamed variant.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  Documentation/devicetree/bindings/i2c/i2c-gpio.txt | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-gpio.txt b/Documentation/devicetree/bindings/i2c/i2c-gpio.txt
> index 4f8ec947c6bd9cad..61033f73577dd70e 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-gpio.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-gpio.txt
> @@ -2,8 +2,9 @@ Device-Tree bindings for i2c gpio driver
>  
>  Required properties:
>  	- compatible = "i2c-gpio";
> -	- gpios: sda and scl gpio
> -
> +	- sda-gpios: gpio used for the sda signal
> +	- scl-gpios: gpio used for the scl signal
> +	- gpios: sda and scl gpio, alternative for {sda,scl}-gpios (deprecated)
>  
>  Optional properties:
>  	- i2c-gpio,sda-open-drain: sda as open drain

Well, if we're going to change things, we should drop these open-drain 
properties. We have open-drain flags in gpio cells, so we don't need 
these I think.

Rob

Re: [PATCH/RFC 1/3] dt-bindings: i2c: i2c-gpio: Add support for named gpios

[Geert Uytterhoeven]

Hi Rob,

On Thu, Aug 31, 2017 at 7:55 PM, Rob Herring <robh@kernel.org> wrote:
> On Thu, Aug 24, 2017 at 11:21:12AM +0200, Geert Uytterhoeven wrote:
>> The current i2c-gpio DT bindings use a single unnamed "gpios" property
>> to refer to the SDA and SCL signal lines by index.  This is error-prone
>> for the casual DT writer and reviewer, as one has to look up the order
>> in the DT bindings.
>>
>> Fix this by amending the DT bindings to use two separate named gpios
>> properties, and deprecate the old unnamed variant.
>>
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> ---
>>  Documentation/devicetree/bindings/i2c/i2c-gpio.txt | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/i2c/i2c-gpio.txt b/Documentation/devicetree/bindings/i2c/i2c-gpio.txt
>> index 4f8ec947c6bd9cad..61033f73577dd70e 100644
>> --- a/Documentation/devicetree/bindings/i2c/i2c-gpio.txt
>> +++ b/Documentation/devicetree/bindings/i2c/i2c-gpio.txt
>> @@ -2,8 +2,9 @@ Device-Tree bindings for i2c gpio driver
>>
>>  Required properties:
>>       - compatible = "i2c-gpio";
>> -     - gpios: sda and scl gpio
>> -
>> +     - sda-gpios: gpio used for the sda signal
>> +     - scl-gpios: gpio used for the scl signal
>> +     - gpios: sda and scl gpio, alternative for {sda,scl}-gpios (deprecated)
>>
>>  Optional properties:
>>       - i2c-gpio,sda-open-drain: sda as open drain
>
> Well, if we're going to change things, we should drop these open-drain
> properties. We have open-drain flags in gpio cells, so we don't need
> these I think.

It depends. Some GPIO controllers use #gpio-cells = 1, lacking flags.
Furthermore, the gpio-specifiers are controller-specific, and not all GPIO
controllers supporting flags follow the semi-standard flag definitions in
<dt-bindings/gpio/gpio.h>.

See Documentation/devicetree/bindings/gpio/gpio.txt.

Gr{oetje,eeting}s,

                        Geert

Re: [PATCH/RFC 2/3] i2c: gpio: Add support for named gpios in DT

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 98 bytes --]


> +	/* Try deprecated unnamed gpios */

I think adding "(deprecated use)" or sth would be good.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH/RFC 2/3] i2c: gpio: Add support for named gpios in DT

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 208 bytes --]

On Wed, Sep 13, 2017 at 10:53:33PM +0200, Wolfram Sang wrote:
> 
> > +	/* Try deprecated unnamed gpios */
> 
> I think adding "(deprecated use)" or sth would be good.

Where is my brown paper bag :)


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH/RFC 1/3] dt-bindings: i2c: i2c-gpio: Add support for named gpios

[Rob Herring]

On Thu, Sep 7, 2017 at 6:42 AM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> Hi Rob,
>
> On Thu, Aug 31, 2017 at 7:55 PM, Rob Herring <robh@kernel.org> wrote:
>> On Thu, Aug 24, 2017 at 11:21:12AM +0200, Geert Uytterhoeven wrote:
>>> The current i2c-gpio DT bindings use a single unnamed "gpios" property
>>> to refer to the SDA and SCL signal lines by index.  This is error-prone
>>> for the casual DT writer and reviewer, as one has to look up the order
>>> in the DT bindings.
>>>
>>> Fix this by amending the DT bindings to use two separate named gpios
>>> properties, and deprecate the old unnamed variant.
>>>
>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>> ---
>>>  Documentation/devicetree/bindings/i2c/i2c-gpio.txt | 5 +++--
>>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/i2c/i2c-gpio.txt b/Documentation/devicetree/bindings/i2c/i2c-gpio.txt
>>> index 4f8ec947c6bd9cad..61033f73577dd70e 100644
>>> --- a/Documentation/devicetree/bindings/i2c/i2c-gpio.txt
>>> +++ b/Documentation/devicetree/bindings/i2c/i2c-gpio.txt
>>> @@ -2,8 +2,9 @@ Device-Tree bindings for i2c gpio driver
>>>
>>>  Required properties:
>>>       - compatible = "i2c-gpio";
>>> -     - gpios: sda and scl gpio
>>> -
>>> +     - sda-gpios: gpio used for the sda signal
>>> +     - scl-gpios: gpio used for the scl signal
>>> +     - gpios: sda and scl gpio, alternative for {sda,scl}-gpios (deprecated)
>>>
>>>  Optional properties:
>>>       - i2c-gpio,sda-open-drain: sda as open drain
>>
>> Well, if we're going to change things, we should drop these open-drain
>> properties. We have open-drain flags in gpio cells, so we don't need
>> these I think.
>
> It depends. Some GPIO controllers use #gpio-cells = 1, lacking flags.

We should stop allowing that or at least require that anyone using
i2c-gpio have flags. If they are added this new i2c-gpio binding, then
they can add flags too.

> Furthermore, the gpio-specifiers are controller-specific, and not all GPIO
> controllers supporting flags follow the semi-standard flag definitions in
> <dt-bindings/gpio/gpio.h>.

In theory yes, but reality is pretty much everyone follows it. There's
no other way to specify active high vs. low for example. Furthermore,
if someone wanted to do flags in their own custom way, that would
still work. It is still the controller (or GPIO core) that interprets
the flags, not the client.

This is a new binding, so only new DT will have it and we can apply
new standards.

Rob

Re: [PATCH/RFC 1/3] dt-bindings: i2c: i2c-gpio: Add support for named gpios

[Linus Walleij]

On Mon, Sep 18, 2017 at 6:52 PM, Rob Herring <robh@kernel.org> wrote:

> In theory yes, but reality is pretty much everyone follows it. There's
> no other way to specify active high vs. low for example. Furthermore,
> if someone wanted to do flags in their own custom way, that would
> still work. It is still the controller (or GPIO core) that interprets
> the flags, not the client.
>
> This is a new binding, so only new DT will have it and we can apply
> new standards.

I'd like to get a picture of any one-cell GPIO DTS:es and/or drivers
still around.

I would like to deal with them somehow.

When we started the big DT migration this was one of the areas
we made some screwups in, admittedly, but someone just has to
go first, and that was incidentally GPIO controllers.

At this time people were even playing around with DT bindings in
BNF form, which is why the GPIO binding is a bit .. esoteric
at times. I guess I should fix that.

Yours,
Linus Walleij

Re: [PATCH/RFC 1/3] dt-bindings: i2c: i2c-gpio: Add support for named gpios

[Rob Herring]

On Mon, Sep 18, 2017 at 2:45 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Mon, Sep 18, 2017 at 6:52 PM, Rob Herring <robh@kernel.org> wrote:
>
>> In theory yes, but reality is pretty much everyone follows it. There's
>> no other way to specify active high vs. low for example. Furthermore,
>> if someone wanted to do flags in their own custom way, that would
>> still work. It is still the controller (or GPIO core) that interprets
>> the flags, not the client.
>>
>> This is a new binding, so only new DT will have it and we can apply
>> new standards.
>
> I'd like to get a picture of any one-cell GPIO DTS:es and/or drivers
> still around.

$ git grep '#gpio-cells.*1'
Documentation/devicetree/bindings/gpio/gpio.txt:
#gpio-cells = <1>;
Documentation/devicetree/bindings/gpio/spear_spics.txt:  *
#gpio-cells: should be 1 and will mention chip select number
Documentation/devicetree/bindings/interrupt-controller/interrupts.txt:
                 #gpio-cells = <1>;
Documentation/devicetree/bindings/mfd/twl6040.txt:- #gpio-cells = <1>:
twl6040 provides GPO lines.
arch/mips/boot/dts/xilfpga/nexys4ddr.dts:               #gpio-cells = <1>;

...and this one may be out of date. All other instances for Xilinx
gpio use 2 cells.

We also have to look for custom translate functions of which there are
a handful, but they all use standard flags.

>
> I would like to deal with them somehow.

Looks like we already warn in the kernel if <2. Might be worth adding
this to dtc, then I can get the warning without booting a kernel on
platforms I don't have.

> When we started the big DT migration this was one of the areas
> we made some screwups in, admittedly, but someone just has to
> go first, and that was incidentally GPIO controllers.

The move to standardize flags started before the Great ARM Conversion,
so I would have expected some old PPC DTs. But it looks like we are
good.

Rob