Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1215481 > unrolled thread

[PATCH v3 4/6] [media] c8sectpfe: Update binding to reset-gpios

Started byPeter Griffin <peter.griffin@linaro.org>
First post2015-08-28 20:00 +0200
Last post2015-09-02 17:10 +0200
Articles 13 — 4 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v3 4/6] [media] c8sectpfe: Update binding to reset-gpios Peter Griffin <peter.griffin@linaro.org> - 2015-08-28 20:00 +0200
    Re: [PATCH v3 4/6] [media] c8sectpfe: Update binding to reset-gpios Rob Herring <robherring2@gmail.com> - 2015-08-31 17:30 +0200
      Re: [PATCH v3 4/6] [media] c8sectpfe: Update binding to reset-gpios Lee Jones <lee.jones@linaro.org> - 2015-09-01 10:00 +0200
      Re: [PATCH v3 4/6] [media] c8sectpfe: Update binding to reset-gpios Peter Griffin <peter.griffin@linaro.org> - 2015-09-01 12:20 +0200
    Re: [PATCH v3 4/6] [media] c8sectpfe: Update binding to reset-gpios Javier Martinez Canillas <javier@dowhile0.org> - 2015-09-01 10:40 +0200
      Re: [PATCH v3 4/6] [media] c8sectpfe: Update binding to reset-gpios Lee Jones <lee.jones@linaro.org> - 2015-09-01 11:10 +0200
        Re: [PATCH v3 4/6] [media] c8sectpfe: Update binding to reset-gpios Javier Martinez Canillas <javier@dowhile0.org> - 2015-09-01 11:20 +0200
          Re: [PATCH v3 4/6] [media] c8sectpfe: Update binding to reset-gpios Lee Jones <lee.jones@linaro.org> - 2015-09-01 11:30 +0200
          Re: [PATCH v3 4/6] [media] c8sectpfe: Update binding to reset-gpios Peter Griffin <peter.griffin@linaro.org> - 2015-09-01 12:40 +0200
            Re: [PATCH v3 4/6] [media] c8sectpfe: Update binding to reset-gpios Lee Jones <lee.jones@linaro.org> - 2015-09-01 14:20 +0200
      Re: [PATCH v3 4/6] [media] c8sectpfe: Update binding to reset-gpios Rob Herring <robherring2@gmail.com> - 2015-09-01 14:00 +0200
        Re: [PATCH v3 4/6] [media] c8sectpfe: Update binding to reset-gpios Javier Martinez Canillas <javier@dowhile0.org> - 2015-09-01 14:40 +0200
        Re: [PATCH v3 4/6] [media] c8sectpfe: Update binding to reset-gpios Peter Griffin <peter.griffin@linaro.org> - 2015-09-02 17:10 +0200

#1215481 — [PATCH v3 4/6] [media] c8sectpfe: Update binding to reset-gpios

FromPeter Griffin <peter.griffin@linaro.org>
Date2015-08-28 20:00 +0200
Subject[PATCH v3 4/6] [media] c8sectpfe: Update binding to reset-gpios
Message-ID<q2w6t-21Z-7@gated-at.bofh.it>
gpio.txt documents that GPIO properties should be named
"[<name>-]gpios", with <name> being the purpose of this
GPIO for the device.

This change has been done as one atomic commit.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
 Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt | 6 +++---
 arch/arm/boot/dts/stihxxx-b2120.dtsi                          | 4 ++--
 drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c         | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt b/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
index d4def76..e70d840 100644
--- a/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
+++ b/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
@@ -35,7 +35,7 @@ Required properties (tsin (child) node):
 
 - tsin-num	: tsin id of the InputBlock (must be between 0 to 6)
 - i2c-bus	: phandle to the I2C bus DT node which the demodulators & tuners on this tsin channel are connected.
-- rst-gpio	: reset gpio for this tsin channel.
+- reset-gpios	: reset gpio for this tsin channel.
 
 Optional properties (tsin (child) node):
 
@@ -75,7 +75,7 @@ Example:
 			tsin-num		= <0>;
 			serial-not-parallel;
 			i2c-bus			= <&ssc2>;
-			rst-gpio		= <&pio15 4 0>;
+			reset-gpios		= <&pio15 4 GPIO_ACTIVE_HIGH>;
 			dvb-card		= <STV0367_TDA18212_NIMA_1>;
 		};
 
@@ -83,7 +83,7 @@ Example:
 			tsin-num		= <3>;
 			serial-not-parallel;
 			i2c-bus			= <&ssc3>;
-			rst-gpio		= <&pio15 7 0>;
+			reset-gpios		= <&pio15 7 GPIO_ACTIVE_HIGH>;
 			dvb-card		= <STV0367_TDA18212_NIMB_1>;
 		};
 	};
diff --git a/arch/arm/boot/dts/stihxxx-b2120.dtsi b/arch/arm/boot/dts/stihxxx-b2120.dtsi
index f9fca10..0b7592e 100644
--- a/arch/arm/boot/dts/stihxxx-b2120.dtsi
+++ b/arch/arm/boot/dts/stihxxx-b2120.dtsi
@@ -6,8 +6,8 @@
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
  */
-
 #include <dt-bindings/clock/stih407-clks.h>
+#include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/media/c8sectpfe.h>
 / {
 	soc {
@@ -116,7 +116,7 @@
 				tsin-num	= <0>;
 				serial-not-parallel;
 				i2c-bus		= <&ssc2>;
-				rst-gpio	= <&pio15 4 GPIO_ACTIVE_HIGH>;
+				reset-gpios	= <&pio15 4 GPIO_ACTIVE_HIGH>;
 				dvb-card	= <STV0367_TDA18212_NIMA_1>;
 			};
 		};
diff --git a/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c b/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
index 3a91093..c691e13 100644
--- a/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
+++ b/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
@@ -822,7 +822,7 @@ static int c8sectpfe_probe(struct platform_device *pdev)
 		}
 		of_node_put(i2c_bus);
 
-		tsin->rst_gpio = of_get_named_gpio(child, "rst-gpio", 0);
+		tsin->rst_gpio = of_get_named_gpio(child, "reset-gpios", 0);
 
 		ret = gpio_is_valid(tsin->rst_gpio);
 		if (!ret) {
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1216267

FromRob Herring <robherring2@gmail.com>
Date2015-08-31 17:30 +0200
Message-ID<q3zbY-3fy-25@gated-at.bofh.it>
In reply to#1215481
On Fri, Aug 28, 2015 at 12:52 PM, Peter Griffin
<peter.griffin@linaro.org> wrote:
> gpio.txt documents that GPIO properties should be named
> "[<name>-]gpios", with <name> being the purpose of this
> GPIO for the device.
>
> This change has been done as one atomic commit.

dtb and kernel updates are not necessarily atomic, so you are breaking
compatibility with older dtbs. You should certainly highlight that in
the commit message. I only point this out. I'll leave it to platform
maintainers whether or not this breakage is acceptable.

Rob

>
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> Acked-by: Lee Jones <lee.jones@linaro.org>
> ---
>  Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt | 6 +++---
>  arch/arm/boot/dts/stihxxx-b2120.dtsi                          | 4 ++--
>  drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c         | 2 +-
>  3 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt b/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
> index d4def76..e70d840 100644
> --- a/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
> +++ b/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
> @@ -35,7 +35,7 @@ Required properties (tsin (child) node):
>
>  - tsin-num     : tsin id of the InputBlock (must be between 0 to 6)
>  - i2c-bus      : phandle to the I2C bus DT node which the demodulators & tuners on this tsin channel are connected.
> -- rst-gpio     : reset gpio for this tsin channel.
> +- reset-gpios  : reset gpio for this tsin channel.
>
>  Optional properties (tsin (child) node):
>
> @@ -75,7 +75,7 @@ Example:
>                         tsin-num                = <0>;
>                         serial-not-parallel;
>                         i2c-bus                 = <&ssc2>;
> -                       rst-gpio                = <&pio15 4 0>;
> +                       reset-gpios             = <&pio15 4 GPIO_ACTIVE_HIGH>;
>                         dvb-card                = <STV0367_TDA18212_NIMA_1>;
>                 };
>
> @@ -83,7 +83,7 @@ Example:
>                         tsin-num                = <3>;
>                         serial-not-parallel;
>                         i2c-bus                 = <&ssc3>;
> -                       rst-gpio                = <&pio15 7 0>;
> +                       reset-gpios             = <&pio15 7 GPIO_ACTIVE_HIGH>;
>                         dvb-card                = <STV0367_TDA18212_NIMB_1>;
>                 };
>         };
> diff --git a/arch/arm/boot/dts/stihxxx-b2120.dtsi b/arch/arm/boot/dts/stihxxx-b2120.dtsi
> index f9fca10..0b7592e 100644
> --- a/arch/arm/boot/dts/stihxxx-b2120.dtsi
> +++ b/arch/arm/boot/dts/stihxxx-b2120.dtsi
> @@ -6,8 +6,8 @@
>   * it under the terms of the GNU General Public License version 2 as
>   * published by the Free Software Foundation.
>   */
> -
>  #include <dt-bindings/clock/stih407-clks.h>
> +#include <dt-bindings/gpio/gpio.h>
>  #include <dt-bindings/media/c8sectpfe.h>
>  / {
>         soc {
> @@ -116,7 +116,7 @@
>                                 tsin-num        = <0>;
>                                 serial-not-parallel;
>                                 i2c-bus         = <&ssc2>;
> -                               rst-gpio        = <&pio15 4 GPIO_ACTIVE_HIGH>;
> +                               reset-gpios     = <&pio15 4 GPIO_ACTIVE_HIGH>;
>                                 dvb-card        = <STV0367_TDA18212_NIMA_1>;
>                         };
>                 };
> diff --git a/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c b/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
> index 3a91093..c691e13 100644
> --- a/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
> +++ b/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
> @@ -822,7 +822,7 @@ static int c8sectpfe_probe(struct platform_device *pdev)
>                 }
>                 of_node_put(i2c_bus);
>
> -               tsin->rst_gpio = of_get_named_gpio(child, "rst-gpio", 0);
> +               tsin->rst_gpio = of_get_named_gpio(child, "reset-gpios", 0);
>
>                 ret = gpio_is_valid(tsin->rst_gpio);
>                 if (!ret) {
> --
> 1.9.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1216655

FromLee Jones <lee.jones@linaro.org>
Date2015-09-01 10:00 +0200
Message-ID<q3OE2-iJ-17@gated-at.bofh.it>
In reply to#1216267
On Mon, 31 Aug 2015, Rob Herring wrote:

> On Fri, Aug 28, 2015 at 12:52 PM, Peter Griffin
> <peter.griffin@linaro.org> wrote:
> > gpio.txt documents that GPIO properties should be named
> > "[<name>-]gpios", with <name> being the purpose of this
> > GPIO for the device.
> >
> > This change has been done as one atomic commit.
> 
> dtb and kernel updates are not necessarily atomic, so you are breaking
> compatibility with older dtbs. You should certainly highlight that in
> the commit message.

Good idea.

> I only point this out. I'll leave it to platform
> maintainers whether or not this breakage is acceptable.

This driver is new.  The 'original' bindings are in next.  So this
binding is not even close to being ABI.

[...]

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1216749

FromPeter Griffin <peter.griffin@linaro.org>
Date2015-09-01 12:20 +0200
Message-ID<q3QPv-3BB-15@gated-at.bofh.it>
In reply to#1216267
Hi Rob,

On Mon, 31 Aug 2015, Rob Herring wrote:

> On Fri, Aug 28, 2015 at 12:52 PM, Peter Griffin
> <peter.griffin@linaro.org> wrote:
> > gpio.txt documents that GPIO properties should be named
> > "[<name>-]gpios", with <name> being the purpose of this
> > GPIO for the device.
> >
> > This change has been done as one atomic commit.
> 
> dtb and kernel updates are not necessarily atomic, so you are breaking
> compatibility with older dtbs. You should certainly highlight that in
> the commit message. I only point this out. I'll leave it to platform
> maintainers whether or not this breakage is acceptable.

Ok I will highlight that in the commit message of the next version.

regards,

Peter.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1216678

FromJavier Martinez Canillas <javier@dowhile0.org>
Date2015-09-01 10:40 +0200
Message-ID<q3PgK-1hp-11@gated-at.bofh.it>
In reply to#1215481
Hello Peter,

On Fri, Aug 28, 2015 at 7:52 PM, Peter Griffin <peter.griffin@linaro.org> wrote:
> gpio.txt documents that GPIO properties should be named
> "[<name>-]gpios", with <name> being the purpose of this
> GPIO for the device.
>
> This change has been done as one atomic commit.
>
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> Acked-by: Lee Jones <lee.jones@linaro.org>
> ---
>  Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt | 6 +++---
>  arch/arm/boot/dts/stihxxx-b2120.dtsi                          | 4 ++--
>  drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c         | 2 +-
>  3 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt b/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
> index d4def76..e70d840 100644
> --- a/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
> +++ b/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
> @@ -35,7 +35,7 @@ Required properties (tsin (child) node):
>
>  - tsin-num     : tsin id of the InputBlock (must be between 0 to 6)
>  - i2c-bus      : phandle to the I2C bus DT node which the demodulators & tuners on this tsin channel are connected.
> -- rst-gpio     : reset gpio for this tsin channel.
> +- reset-gpios  : reset gpio for this tsin channel.

The documentation is a bit outdated, the GPIO subsystem supports both
-gpio and -gpios, see commit:

dd34c37aa3e8 ("gpio: of: Allow -gpio suffix for property names")

So it makes sense to me to use -gpio instead of -gpios in this case
since is a single GPIO. Also rst is already a descriptive name since
that's how many datasheets name a reset pin. I'm not saying I'm
against this patch, just pointing out since the commit message is a
bit misleading.

Best regards,
Javier
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1216708

FromLee Jones <lee.jones@linaro.org>
Date2015-09-01 11:10 +0200
Message-ID<q3PJM-24L-11@gated-at.bofh.it>
In reply to#1216678
On Tue, 01 Sep 2015, Javier Martinez Canillas wrote:
> On Fri, Aug 28, 2015 at 7:52 PM, Peter Griffin <peter.griffin@linaro.org> wrote:
> > gpio.txt documents that GPIO properties should be named
> > "[<name>-]gpios", with <name> being the purpose of this
> > GPIO for the device.
> >
> > This change has been done as one atomic commit.
> >
> > Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> > Acked-by: Lee Jones <lee.jones@linaro.org>
> > ---
> >  Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt | 6 +++---
> >  arch/arm/boot/dts/stihxxx-b2120.dtsi                          | 4 ++--
> >  drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c         | 2 +-
> >  3 files changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt b/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
> > index d4def76..e70d840 100644
> > --- a/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
> > +++ b/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
> > @@ -35,7 +35,7 @@ Required properties (tsin (child) node):
> >
> >  - tsin-num     : tsin id of the InputBlock (must be between 0 to 6)
> >  - i2c-bus      : phandle to the I2C bus DT node which the demodulators & tuners on this tsin channel are connected.
> > -- rst-gpio     : reset gpio for this tsin channel.
> > +- reset-gpios  : reset gpio for this tsin channel.
> 
> The documentation is a bit outdated, the GPIO subsystem supports both
> -gpio and -gpios, see commit:
> 
> dd34c37aa3e8 ("gpio: of: Allow -gpio suffix for property names")
> 
> So it makes sense to me to use -gpio instead of -gpios in this case
> since is a single GPIO. Also rst is already a descriptive name since
> that's how many datasheets name a reset pin. I'm not saying I'm
> against this patch, just pointing out since the commit message is a
> bit misleading.

As I suggested this patch, I feel I must comment.

My order of preference would be:

 reset-gpio
 reset-gpios
 rst-gpio
 rst-gpios

This current patch is No2, so it's okay to stay IMHO.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1216712

FromJavier Martinez Canillas <javier@dowhile0.org>
Date2015-09-01 11:20 +0200
Message-ID<q3PTr-2g5-13@gated-at.bofh.it>
In reply to#1216708
Hello Lee,

On Tue, Sep 1, 2015 at 11:09 AM, Lee Jones <lee.jones@linaro.org> wrote:
> On Tue, 01 Sep 2015, Javier Martinez Canillas wrote:
>> On Fri, Aug 28, 2015 at 7:52 PM, Peter Griffin <peter.griffin@linaro.org> wrote:
>> > gpio.txt documents that GPIO properties should be named
>> > "[<name>-]gpios", with <name> being the purpose of this
>> > GPIO for the device.
>> >
>> > This change has been done as one atomic commit.
>> >
>> > Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
>> > Acked-by: Lee Jones <lee.jones@linaro.org>
>> > ---
>> >  Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt | 6 +++---
>> >  arch/arm/boot/dts/stihxxx-b2120.dtsi                          | 4 ++--
>> >  drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c         | 2 +-
>> >  3 files changed, 6 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt b/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
>> > index d4def76..e70d840 100644
>> > --- a/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
>> > +++ b/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
>> > @@ -35,7 +35,7 @@ Required properties (tsin (child) node):
>> >
>> >  - tsin-num     : tsin id of the InputBlock (must be between 0 to 6)
>> >  - i2c-bus      : phandle to the I2C bus DT node which the demodulators & tuners on this tsin channel are connected.
>> > -- rst-gpio     : reset gpio for this tsin channel.
>> > +- reset-gpios  : reset gpio for this tsin channel.
>>
>> The documentation is a bit outdated, the GPIO subsystem supports both
>> -gpio and -gpios, see commit:
>>
>> dd34c37aa3e8 ("gpio: of: Allow -gpio suffix for property names")
>>
>> So it makes sense to me to use -gpio instead of -gpios in this case
>> since is a single GPIO. Also rst is already a descriptive name since
>> that's how many datasheets name a reset pin. I'm not saying I'm
>> against this patch, just pointing out since the commit message is a
>> bit misleading.
>
> As I suggested this patch, I feel I must comment.
>
> My order of preference would be:
>
>  reset-gpio
>  reset-gpios
>  rst-gpio
>  rst-gpios
>
> This current patch is No2, so it's okay to stay IMHO.
>

If the property is being changed anyways, why not going with No1 then?

As I said, I'm not against the patch but I think the commit message
has to be reworded since implies that the problem is that the -gpio
sufix is being used instead of -gpios. But since both are supported by
the GPIO subsystem, the commit should mention that "reset" is more
clear and easier to read than "rst" or something along those lines.

BTW, I just posted a patch for the GPIO doc to mention that -gpio is
also supported:

https://patchwork.kernel.org/patch/7103761/

> --
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog

Best regards,
Javier
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1216716

FromLee Jones <lee.jones@linaro.org>
Date2015-09-01 11:30 +0200
Message-ID<q3Q38-2rk-21@gated-at.bofh.it>
In reply to#1216712
On Tue, 01 Sep 2015, Javier Martinez Canillas wrote:

> Hello Lee,
> 
> On Tue, Sep 1, 2015 at 11:09 AM, Lee Jones <lee.jones@linaro.org> wrote:
> > On Tue, 01 Sep 2015, Javier Martinez Canillas wrote:
> >> On Fri, Aug 28, 2015 at 7:52 PM, Peter Griffin <peter.griffin@linaro.org> wrote:
> >> > gpio.txt documents that GPIO properties should be named
> >> > "[<name>-]gpios", with <name> being the purpose of this
> >> > GPIO for the device.
> >> >
> >> > This change has been done as one atomic commit.
> >> >
> >> > Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> >> > Acked-by: Lee Jones <lee.jones@linaro.org>
> >> > ---
> >> >  Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt | 6 +++---
> >> >  arch/arm/boot/dts/stihxxx-b2120.dtsi                          | 4 ++--
> >> >  drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c         | 2 +-
> >> >  3 files changed, 6 insertions(+), 6 deletions(-)
> >> >
> >> > diff --git a/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt b/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
> >> > index d4def76..e70d840 100644
> >> > --- a/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
> >> > +++ b/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
> >> > @@ -35,7 +35,7 @@ Required properties (tsin (child) node):
> >> >
> >> >  - tsin-num     : tsin id of the InputBlock (must be between 0 to 6)
> >> >  - i2c-bus      : phandle to the I2C bus DT node which the demodulators & tuners on this tsin channel are connected.
> >> > -- rst-gpio     : reset gpio for this tsin channel.
> >> > +- reset-gpios  : reset gpio for this tsin channel.
> >>
> >> The documentation is a bit outdated, the GPIO subsystem supports both
> >> -gpio and -gpios, see commit:
> >>
> >> dd34c37aa3e8 ("gpio: of: Allow -gpio suffix for property names")
> >>
> >> So it makes sense to me to use -gpio instead of -gpios in this case
> >> since is a single GPIO. Also rst is already a descriptive name since
> >> that's how many datasheets name a reset pin. I'm not saying I'm
> >> against this patch, just pointing out since the commit message is a
> >> bit misleading.
> >
> > As I suggested this patch, I feel I must comment.
> >
> > My order of preference would be:
> >
> >  reset-gpio
> >  reset-gpios
> >  rst-gpio
> >  rst-gpios
> >
> > This current patch is No2, so it's okay to stay IMHO.
> >
> 
> If the property is being changed anyways, why not going with No1 then?
> 
> As I said, I'm not against the patch but I think the commit message
> has to be reworded since implies that the problem is that the -gpio
> sufix is being used instead of -gpios. But since both are supported by
> the GPIO subsystem, the commit should mention that "reset" is more
> clear and easier to read than "rst" or something along those lines.
> 
> BTW, I just posted a patch for the GPIO doc to mention that -gpio is
> also supported:
> 
> https://patchwork.kernel.org/patch/7103761/

Noted.  Thanks for the heads-up.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1216753

FromPeter Griffin <peter.griffin@linaro.org>
Date2015-09-01 12:40 +0200
Message-ID<q3R8R-3Y0-1@gated-at.bofh.it>
In reply to#1216712
Hi Javier,

On Tue, 01 Sep 2015, Javier Martinez Canillas wrote:

> Hello Lee,
> 
> On Tue, Sep 1, 2015 at 11:09 AM, Lee Jones <lee.jones@linaro.org> wrote:
> > On Tue, 01 Sep 2015, Javier Martinez Canillas wrote:
> >> On Fri, Aug 28, 2015 at 7:52 PM, Peter Griffin <peter.griffin@linaro.org> wrote:
> >> > gpio.txt documents that GPIO properties should be named
> >> > "[<name>-]gpios", with <name> being the purpose of this
> >> > GPIO for the device.
> >> >
> >> > This change has been done as one atomic commit.
> >> >
> >> > Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> >> > Acked-by: Lee Jones <lee.jones@linaro.org>
> >> > ---
> >> >  Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt | 6 +++---
> >> >  arch/arm/boot/dts/stihxxx-b2120.dtsi                          | 4 ++--
> >> >  drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c         | 2 +-
> >> >  3 files changed, 6 insertions(+), 6 deletions(-)
> >> >
> >> > diff --git a/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt b/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
> >> > index d4def76..e70d840 100644
> >> > --- a/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
> >> > +++ b/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
> >> > @@ -35,7 +35,7 @@ Required properties (tsin (child) node):
> >> >
> >> >  - tsin-num     : tsin id of the InputBlock (must be between 0 to 6)
> >> >  - i2c-bus      : phandle to the I2C bus DT node which the demodulators & tuners on this tsin channel are connected.
> >> > -- rst-gpio     : reset gpio for this tsin channel.
> >> > +- reset-gpios  : reset gpio for this tsin channel.
> >>
> >> The documentation is a bit outdated, the GPIO subsystem supports both
> >> -gpio and -gpios, see commit:
> >>
> >> dd34c37aa3e8 ("gpio: of: Allow -gpio suffix for property names")
> >>
> >> So it makes sense to me to use -gpio instead of -gpios in this case
> >> since is a single GPIO. Also rst is already a descriptive name since
> >> that's how many datasheets name a reset pin. I'm not saying I'm
> >> against this patch, just pointing out since the commit message is a
> >> bit misleading.

Ok thanks for pointing that out. It's nice to know the original binding was
actually OK.

> >
> > As I suggested this patch, I feel I must comment.
> >
> > My order of preference would be:
> >
> >  reset-gpio
> >  reset-gpios
> >  rst-gpio
> >  rst-gpios
> >
> > This current patch is No2, so it's okay to stay IMHO.
> >
> 
> If the property is being changed anyways, why not going with No1 then?

I've changed to No1 in v4.

> 
> As I said, I'm not against the patch but I think the commit message
> has to be reworded since implies that the problem is that the -gpio
> sufix is being used instead of -gpios. But since both are supported by
> the GPIO subsystem, the commit should mention that "reset" is more
> clear and easier to read than "rst" or something along those lines.

I've re-worded the commit message like you suggest in v4

> 
> BTW, I just posted a patch for the GPIO doc to mention that -gpio is
> also supported:
> 
> https://patchwork.kernel.org/patch/7103761/

Ok thanks for the pointer.

regards,

Peter.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1216807

FromLee Jones <lee.jones@linaro.org>
Date2015-09-01 14:20 +0200
Message-ID<q3SHD-6h9-3@gated-at.bofh.it>
In reply to#1216753
On Tue, 01 Sep 2015, Peter Griffin wrote:
> On Tue, 01 Sep 2015, Javier Martinez Canillas wrote:
> > On Tue, Sep 1, 2015 at 11:09 AM, Lee Jones <lee.jones@linaro.org> wrote:
> > > On Tue, 01 Sep 2015, Javier Martinez Canillas wrote:
> > >> On Fri, Aug 28, 2015 at 7:52 PM, Peter Griffin <peter.griffin@linaro.org> wrote:
> > >> > gpio.txt documents that GPIO properties should be named
> > >> > "[<name>-]gpios", with <name> being the purpose of this
> > >> > GPIO for the device.
> > >> >
> > >> > This change has been done as one atomic commit.
> > >> >
> > >> > Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> > >> > Acked-by: Lee Jones <lee.jones@linaro.org>
> > >> > ---
> > >> >  Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt | 6 +++---
> > >> >  arch/arm/boot/dts/stihxxx-b2120.dtsi                          | 4 ++--
> > >> >  drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c         | 2 +-
> > >> >  3 files changed, 6 insertions(+), 6 deletions(-)
> > >> >
> > >> > diff --git a/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt b/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
> > >> > index d4def76..e70d840 100644
> > >> > --- a/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
> > >> > +++ b/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
> > >> > @@ -35,7 +35,7 @@ Required properties (tsin (child) node):
> > >> >
> > >> >  - tsin-num     : tsin id of the InputBlock (must be between 0 to 6)
> > >> >  - i2c-bus      : phandle to the I2C bus DT node which the demodulators & tuners on this tsin channel are connected.
> > >> > -- rst-gpio     : reset gpio for this tsin channel.
> > >> > +- reset-gpios  : reset gpio for this tsin channel.
> > >>
> > >> The documentation is a bit outdated, the GPIO subsystem supports both
> > >> -gpio and -gpios, see commit:
> > >>
> > >> dd34c37aa3e8 ("gpio: of: Allow -gpio suffix for property names")
> > >>
> > >> So it makes sense to me to use -gpio instead of -gpios in this case
> > >> since is a single GPIO. Also rst is already a descriptive name since
> > >> that's how many datasheets name a reset pin. I'm not saying I'm
> > >> against this patch, just pointing out since the commit message is a
> > >> bit misleading.
> 
> Ok thanks for pointing that out. It's nice to know the original binding was
> actually OK.

By luck, rather than judgement. ;)

> > > As I suggested this patch, I feel I must comment.
> > >
> > > My order of preference would be:
> > >
> > >  reset-gpio
> > >  reset-gpios
> > >  rst-gpio
> > >  rst-gpios
> > >
> > > This current patch is No2, so it's okay to stay IMHO.
> > >
> > 
> > If the property is being changed anyways, why not going with No1 then?
> 
> I've changed to No1 in v4.

Nice!  Thanks for fixing up.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1216793

FromRob Herring <robherring2@gmail.com>
Date2015-09-01 14:00 +0200
Message-ID<q3Soi-5Fd-1@gated-at.bofh.it>
In reply to#1216678
On Tue, Sep 1, 2015 at 3:32 AM, Javier Martinez Canillas
<javier@dowhile0.org> wrote:
> Hello Peter,
>
> On Fri, Aug 28, 2015 at 7:52 PM, Peter Griffin <peter.griffin@linaro.org> wrote:
>> gpio.txt documents that GPIO properties should be named
>> "[<name>-]gpios", with <name> being the purpose of this
>> GPIO for the device.
>>
>> This change has been done as one atomic commit.
>>
>> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
>> Acked-by: Lee Jones <lee.jones@linaro.org>
>> ---
>>  Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt | 6 +++---
>>  arch/arm/boot/dts/stihxxx-b2120.dtsi                          | 4 ++--
>>  drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c         | 2 +-
>>  3 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt b/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
>> index d4def76..e70d840 100644
>> --- a/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
>> +++ b/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
>> @@ -35,7 +35,7 @@ Required properties (tsin (child) node):
>>
>>  - tsin-num     : tsin id of the InputBlock (must be between 0 to 6)
>>  - i2c-bus      : phandle to the I2C bus DT node which the demodulators & tuners on this tsin channel are connected.
>> -- rst-gpio     : reset gpio for this tsin channel.
>> +- reset-gpios  : reset gpio for this tsin channel.
>
> The documentation is a bit outdated, the GPIO subsystem supports both
> -gpio and -gpios, see commit:
>
> dd34c37aa3e8 ("gpio: of: Allow -gpio suffix for property names")

Yes, because we have lots of them.

> So it makes sense to me to use -gpio instead of -gpios in this case
> since is a single GPIO. Also rst is already a descriptive name since
> that's how many datasheets name a reset pin. I'm not saying I'm
> against this patch, just pointing out since the commit message is a
> bit misleading.

I believe that this has been discussed at length and it was decided
that new bindings should use "-gpios" even for 1. Just like "clocks"
is always plural.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1216814

FromJavier Martinez Canillas <javier@dowhile0.org>
Date2015-09-01 14:40 +0200
Message-ID<q3T11-6Dv-19@gated-at.bofh.it>
In reply to#1216793
[adding GPIO maintainers to cc list]

Hello Rob,

On Tue, Sep 1, 2015 at 1:54 PM, Rob Herring <robherring2@gmail.com> wrote:
> On Tue, Sep 1, 2015 at 3:32 AM, Javier Martinez Canillas
> <javier@dowhile0.org> wrote:
>> Hello Peter,
>>
>> On Fri, Aug 28, 2015 at 7:52 PM, Peter Griffin <peter.griffin@linaro.org> wrote:
>>> gpio.txt documents that GPIO properties should be named
>>> "[<name>-]gpios", with <name> being the purpose of this
>>> GPIO for the device.
>>>
>>> This change has been done as one atomic commit.
>>>
>>> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
>>> Acked-by: Lee Jones <lee.jones@linaro.org>
>>> ---
>>>  Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt | 6 +++---
>>>  arch/arm/boot/dts/stihxxx-b2120.dtsi                          | 4 ++--
>>>  drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c         | 2 +-
>>>  3 files changed, 6 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt b/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
>>> index d4def76..e70d840 100644
>>> --- a/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
>>> +++ b/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
>>> @@ -35,7 +35,7 @@ Required properties (tsin (child) node):
>>>
>>>  - tsin-num     : tsin id of the InputBlock (must be between 0 to 6)
>>>  - i2c-bus      : phandle to the I2C bus DT node which the demodulators & tuners on this tsin channel are connected.
>>> -- rst-gpio     : reset gpio for this tsin channel.
>>> +- reset-gpios  : reset gpio for this tsin channel.
>>
>> The documentation is a bit outdated, the GPIO subsystem supports both
>> -gpio and -gpios, see commit:
>>
>> dd34c37aa3e8 ("gpio: of: Allow -gpio suffix for property names")
>
> Yes, because we have lots of them.
>

Yes, I know that was the motivation for that change.

>> So it makes sense to me to use -gpio instead of -gpios in this case
>> since is a single GPIO. Also rst is already a descriptive name since
>> that's how many datasheets name a reset pin. I'm not saying I'm
>> against this patch, just pointing out since the commit message is a
>> bit misleading.
>
> I believe that this has been discussed at length and it was decided
> that new bindings should use "-gpios" even for 1. Just like "clocks"
> is always plural.
>

The documentation doesn't reflect that decision though. If new
bindings are supposed to be using -gpios rather than -gpio even when a
single GPIO is used, then
Documentation/devicetree/bindings/gpio/gpio.txt and
Documentation/gpio/board.txt should say that <function>-gpio is also
supported for backward compatibility but that is deprecated and should
not be used.

Otherwise when looking the code it seems that is just that the
documentation is outdated and that both -gpio or -gpios can be used.

> Rob

Best regards,
Javier
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1217690

FromPeter Griffin <peter.griffin@linaro.org>
Date2015-09-02 17:10 +0200
Message-ID<q4hPI-bE-19@gated-at.bofh.it>
In reply to#1216793
Hi Rob,

On Tue, 01 Sep 2015, Rob Herring wrote:

> On Tue, Sep 1, 2015 at 3:32 AM, Javier Martinez Canillas
> <javier@dowhile0.org> wrote:
> > Hello Peter,
> >
> > On Fri, Aug 28, 2015 at 7:52 PM, Peter Griffin <peter.griffin@linaro.org> wrote:
> >> gpio.txt documents that GPIO properties should be named
> >> "[<name>-]gpios", with <name> being the purpose of this
> >> GPIO for the device.
> >>
> >> This change has been done as one atomic commit.
> >>
> >> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> >> Acked-by: Lee Jones <lee.jones@linaro.org>
> >> ---
> >>  Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt | 6 +++---
> >>  arch/arm/boot/dts/stihxxx-b2120.dtsi                          | 4 ++--
> >>  drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c         | 2 +-
> >>  3 files changed, 6 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt b/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
> >> index d4def76..e70d840 100644
> >> --- a/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
> >> +++ b/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
> >> @@ -35,7 +35,7 @@ Required properties (tsin (child) node):
> >>
> >>  - tsin-num     : tsin id of the InputBlock (must be between 0 to 6)
> >>  - i2c-bus      : phandle to the I2C bus DT node which the demodulators & tuners on this tsin channel are connected.
> >> -- rst-gpio     : reset gpio for this tsin channel.
> >> +- reset-gpios  : reset gpio for this tsin channel.
> >
> > The documentation is a bit outdated, the GPIO subsystem supports both
> > -gpio and -gpios, see commit:
> >
> > dd34c37aa3e8 ("gpio: of: Allow -gpio suffix for property names")
> 
> Yes, because we have lots of them.
> 
> > So it makes sense to me to use -gpio instead of -gpios in this case
> > since is a single GPIO. Also rst is already a descriptive name since
> > that's how many datasheets name a reset pin. I'm not saying I'm
> > against this patch, just pointing out since the commit message is a
> > bit misleading.
> 
> I believe that this has been discussed at length and it was decided
> that new bindings should use "-gpios" even for 1. Just like "clocks"
> is always plural.

Doh! Ok I will change back again to 'reset-gpios'.

regards,

Peter.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web