linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/6] [media] c8sectpfe: Various fixups
@ 2015-08-28 17:52 Peter Griffin
  2015-08-28 17:52 ` [PATCH v3 1/6] ARM: DT: STi: stihxxx-b2120: Add pulse-width properties to ssc2 & ssc3 Peter Griffin
                   ` (5 more replies)
  0 siblings, 6 replies; 20+ messages in thread
From: Peter Griffin @ 2015-08-28 17:52 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Mauro,

This series includes a couple of fixes for the c8sectpfe Linux dvb driver.

One was caused by omitting a patch from the original c8sectpfe series which
defined the ssc2 and ssc3 dt nodes, which was then used by the later DT patch.
This patch is included, along with the original patch which you reverted.

Also Valentin Rothberg spotted LIBELF32 Kconfig symbol I was selecting in the
Kconfig, this isn't required upstream and is left over legacy so I've removed
it.

Also included are some fixups spotted by Lee Jones.

Changes since v2:
 - Add some "\n" and other formatting fixes
 - 'Suggested by' should be in chronological order
 - Simplify load_slim_core_fw loop

Changes since v1:
 - Various formating patches to DT node
 - Update to reset-gpios
 - Use GPIO_ACTIVE_HIGH, GIC_SPI and IRQ_TYPE_NONE defines

kind regards,

Peter.

Peter Griffin (6):
  ARM: DT: STi: stihxxx-b2120: Add pulse-width properties to ssc2 & ssc3
  ARM: DT: STi: STiH407: Add c8sectpfe LinuxDVB DT node.
  [media] c8sectpfe: Remove select on undefined LIBELF_32
  [media] c8sectpfe: Update binding to reset-gpios
  [media] c8sectpfe: Update DT binding doc with some minor fixes
  [media] c8sectpfe: Simplify for loop in load_slim_core_fw

 .../bindings/media/stih407-c8sectpfe.txt           | 20 +++++-----
 arch/arm/boot/dts/stihxxx-b2120.dtsi               | 45 +++++++++++++++++++++-
 drivers/media/platform/sti/c8sectpfe/Kconfig       |  1 -
 .../media/platform/sti/c8sectpfe/c8sectpfe-core.c  |  9 +++--
 4 files changed, 58 insertions(+), 17 deletions(-)

-- 
1.9.1

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v3 1/6] ARM: DT: STi: stihxxx-b2120: Add pulse-width properties to ssc2 & ssc3
  2015-08-28 17:52 [PATCH v3 0/6] [media] c8sectpfe: Various fixups Peter Griffin
@ 2015-08-28 17:52 ` Peter Griffin
  2015-08-28 17:52 ` [PATCH v3 2/6] ARM: DT: STi: STiH407: Add c8sectpfe LinuxDVB DT node Peter Griffin
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 20+ messages in thread
From: Peter Griffin @ 2015-08-28 17:52 UTC (permalink / raw)
  To: linux-arm-kernel

Adding these properties makes the I2C bus to the demodulators much
more reliable, and we no longer suffer from I2C errors when tuning.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/boot/dts/stihxxx-b2120.dtsi | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/stihxxx-b2120.dtsi b/arch/arm/boot/dts/stihxxx-b2120.dtsi
index f589fe4..62994ae 100644
--- a/arch/arm/boot/dts/stihxxx-b2120.dtsi
+++ b/arch/arm/boot/dts/stihxxx-b2120.dtsi
@@ -27,12 +27,18 @@
 			};
 		};
 
-		i2c at 9842000 {
+		ssc2: i2c at 9842000 {
 			status = "okay";
+			clock-frequency = <100000>;
+			st,i2c-min-scl-pulse-width-us = <0>;
+			st,i2c-min-sda-pulse-width-us = <5>;
 		};
 
-		i2c at 9843000 {
+		ssc3: i2c at 9843000 {
 			status = "okay";
+			clock-frequency = <100000>;
+			st,i2c-min-scl-pulse-width-us = <0>;
+			st,i2c-min-sda-pulse-width-us = <5>;
 		};
 
 		i2c at 9844000 {
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 2/6] ARM: DT: STi: STiH407: Add c8sectpfe LinuxDVB DT node.
  2015-08-28 17:52 [PATCH v3 0/6] [media] c8sectpfe: Various fixups Peter Griffin
  2015-08-28 17:52 ` [PATCH v3 1/6] ARM: DT: STi: stihxxx-b2120: Add pulse-width properties to ssc2 & ssc3 Peter Griffin
@ 2015-08-28 17:52 ` Peter Griffin
  2015-09-01  7:59   ` Lee Jones
  2015-08-28 17:52 ` [PATCH v3 3/6] [media] c8sectpfe: Remove select on undefined LIBELF_32 Peter Griffin
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 20+ messages in thread
From: Peter Griffin @ 2015-08-28 17:52 UTC (permalink / raw)
  To: linux-arm-kernel

This patch adds in the required DT node for the c8sectpfe
Linux DVB demux driver which allows the tsin channels
to be used on an upstream kernel.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 arch/arm/boot/dts/stihxxx-b2120.dtsi | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/arch/arm/boot/dts/stihxxx-b2120.dtsi b/arch/arm/boot/dts/stihxxx-b2120.dtsi
index 62994ae..f9fca10 100644
--- a/arch/arm/boot/dts/stihxxx-b2120.dtsi
+++ b/arch/arm/boot/dts/stihxxx-b2120.dtsi
@@ -6,6 +6,9 @@
  * 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/media/c8sectpfe.h>
 / {
 	soc {
 		sbc_serial0: serial at 9530000 {
@@ -85,5 +88,37 @@
 			status = "okay";
 		};
 
+		demux at 08a20000 {
+			compatible	= "st,stih407-c8sectpfe";
+			status		= "okay";
+			reg		= <0x08a20000 0x10000>,
+					  <0x08a00000 0x4000>;
+			reg-names	= "c8sectpfe", "c8sectpfe-ram";
+			interrupts	= <GIC_SPI 34 IRQ_TYPE_NONE>,
+					  <GIC_SPI 35 IRQ_TYPE_NONE>;
+			interrupt-names	= "c8sectpfe-error-irq",
+					  "c8sectpfe-idle-irq";
+			pinctrl-0	= <&pinctrl_tsin0_serial>;
+			pinctrl-1	= <&pinctrl_tsin0_parallel>;
+			pinctrl-2	= <&pinctrl_tsin3_serial>;
+			pinctrl-3	= <&pinctrl_tsin4_serial_alt3>;
+			pinctrl-4	= <&pinctrl_tsin5_serial_alt1>;
+			pinctrl-names	= "tsin0-serial",
+					  "tsin0-parallel",
+					  "tsin3-serial",
+					  "tsin4-serial",
+					  "tsin5-serial";
+			clocks		= <&clk_s_c0_flexgen CLK_PROC_STFE>;
+			clock-names	= "c8sectpfe";
+
+			/* tsin0 is TSA on NIMA */
+			tsin0: port at 0 {
+				tsin-num	= <0>;
+				serial-not-parallel;
+				i2c-bus		= <&ssc2>;
+				rst-gpio	= <&pio15 4 GPIO_ACTIVE_HIGH>;
+				dvb-card	= <STV0367_TDA18212_NIMA_1>;
+			};
+		};
 	};
 };
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 3/6] [media] c8sectpfe: Remove select on undefined LIBELF_32
  2015-08-28 17:52 [PATCH v3 0/6] [media] c8sectpfe: Various fixups Peter Griffin
  2015-08-28 17:52 ` [PATCH v3 1/6] ARM: DT: STi: stihxxx-b2120: Add pulse-width properties to ssc2 & ssc3 Peter Griffin
  2015-08-28 17:52 ` [PATCH v3 2/6] ARM: DT: STi: STiH407: Add c8sectpfe LinuxDVB DT node Peter Griffin
@ 2015-08-28 17:52 ` Peter Griffin
  2015-08-28 17:52 ` [PATCH v3 4/6] [media] c8sectpfe: Update binding to reset-gpios Peter Griffin
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 20+ messages in thread
From: Peter Griffin @ 2015-08-28 17:52 UTC (permalink / raw)
  To: linux-arm-kernel

LIBELF_32 is not defined in Kconfig, and is left over legacy
which is not required in the upstream driver, so remove it.

Suggested-by: Valentin Rothberg <valentinrothberg@gmail.com>
Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/media/platform/sti/c8sectpfe/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/media/platform/sti/c8sectpfe/Kconfig b/drivers/media/platform/sti/c8sectpfe/Kconfig
index d1bfd4c..b9ec667 100644
--- a/drivers/media/platform/sti/c8sectpfe/Kconfig
+++ b/drivers/media/platform/sti/c8sectpfe/Kconfig
@@ -1,7 +1,6 @@
 config DVB_C8SECTPFE
 	tristate "STMicroelectronics C8SECTPFE DVB support"
 	depends on DVB_CORE && I2C && (ARCH_STI || ARCH_MULTIPLATFORM)
-	select LIBELF_32
 	select FW_LOADER
 	select FW_LOADER_USER_HELPER_FALLBACK
 	select DEBUG_FS
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 4/6] [media] c8sectpfe: Update binding to reset-gpios
  2015-08-28 17:52 [PATCH v3 0/6] [media] c8sectpfe: Various fixups Peter Griffin
                   ` (2 preceding siblings ...)
  2015-08-28 17:52 ` [PATCH v3 3/6] [media] c8sectpfe: Remove select on undefined LIBELF_32 Peter Griffin
@ 2015-08-28 17:52 ` Peter Griffin
  2015-08-31 15:25   ` Rob Herring
  2015-09-01  8:32   ` Javier Martinez Canillas
  2015-08-28 17:52 ` [PATCH v3 5/6] [media] c8sectpfe: Update DT binding doc with some minor fixes Peter Griffin
  2015-08-28 17:52 ` [PATCH v3 6/6] [media] c8sectpfe: Simplify for loop in load_slim_core_fw Peter Griffin
  5 siblings, 2 replies; 20+ messages in thread
From: Peter Griffin @ 2015-08-28 17:52 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 5/6] [media] c8sectpfe: Update DT binding doc with some minor fixes
  2015-08-28 17:52 [PATCH v3 0/6] [media] c8sectpfe: Various fixups Peter Griffin
                   ` (3 preceding siblings ...)
  2015-08-28 17:52 ` [PATCH v3 4/6] [media] c8sectpfe: Update binding to reset-gpios Peter Griffin
@ 2015-08-28 17:52 ` Peter Griffin
  2015-08-28 17:52 ` [PATCH v3 6/6] [media] c8sectpfe: Simplify for loop in load_slim_core_fw Peter Griffin
  5 siblings, 0 replies; 20+ messages in thread
From: Peter Griffin @ 2015-08-28 17:52 UTC (permalink / raw)
  To: linux-arm-kernel

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
 .../devicetree/bindings/media/stih407-c8sectpfe.txt        | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt b/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
index e70d840..cc51b1f 100644
--- a/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
+++ b/Documentation/devicetree/bindings/media/stih407-c8sectpfe.txt
@@ -55,20 +55,20 @@ Example:
 		status = "okay";
 		reg = <0x08a20000 0x10000>, <0x08a00000 0x4000>;
 		reg-names = "stfe", "stfe-ram";
-		interrupts = <0 34 0>, <0 35 0>;
+		interrupts = <GIC_SPI 34 IRQ_TYPE_NONE>, <GIC_SPI 35 IRQ_TYPE_NONE>;
 		interrupt-names = "stfe-error-irq", "stfe-idle-irq";
-
-		pinctrl-names	= "tsin0-serial", "tsin0-parallel", "tsin3-serial",
-				"tsin4-serial", "tsin5-serial";
-
 		pinctrl-0	= <&pinctrl_tsin0_serial>;
 		pinctrl-1	= <&pinctrl_tsin0_parallel>;
 		pinctrl-2	= <&pinctrl_tsin3_serial>;
 		pinctrl-3	= <&pinctrl_tsin4_serial_alt3>;
 		pinctrl-4	= <&pinctrl_tsin5_serial_alt1>;
-
+		pinctrl-names	= "tsin0-serial",
+				  "tsin0-parallel",
+				  "tsin3-serial",
+				  "tsin4-serial",
+				  "tsin5-serial";
 		clocks = <&clk_s_c0_flexgen CLK_PROC_STFE>;
-		clock-names = "stfe";
+		clock-names = "c8sectpfe";
 
 		/* tsin0 is TSA on NIMA */
 		tsin0: port at 0 {
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 6/6] [media] c8sectpfe: Simplify for loop in load_slim_core_fw
  2015-08-28 17:52 [PATCH v3 0/6] [media] c8sectpfe: Various fixups Peter Griffin
                   ` (4 preceding siblings ...)
  2015-08-28 17:52 ` [PATCH v3 5/6] [media] c8sectpfe: Update DT binding doc with some minor fixes Peter Griffin
@ 2015-08-28 17:52 ` Peter Griffin
  5 siblings, 0 replies; 20+ messages in thread
From: Peter Griffin @ 2015-08-28 17:52 UTC (permalink / raw)
  To: linux-arm-kernel

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c b/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
index c691e13..ce72ffb 100644
--- a/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
+++ b/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
@@ -1096,7 +1096,7 @@ static int load_slim_core_fw(const struct firmware *fw, void *context)
 	Elf32_Ehdr *ehdr;
 	Elf32_Phdr *phdr;
 	u8 __iomem *dst;
-	int err, i;
+	int err = 0, i;
 
 	if (!fw || !context)
 		return -EINVAL;
@@ -1105,7 +1105,7 @@ static int load_slim_core_fw(const struct firmware *fw, void *context)
 	phdr = (Elf32_Phdr *)(fw->data + ehdr->e_phoff);
 
 	/* go through the available ELF segments */
-	for (i = 0; i < ehdr->e_phnum && !err; i++, phdr++) {
+	for (i = 0; i < ehdr->e_phnum; i++, phdr++) {
 
 		/* Only consider LOAD segments */
 		if (phdr->p_type != PT_LOAD)
@@ -1118,7 +1118,7 @@ static int load_slim_core_fw(const struct firmware *fw, void *context)
 			dev_err(fei->dev,
 				"Segment %d is outside of firmware file\n", i);
 			err = -EINVAL;
-			break;
+			goto err;
 		}
 
 		/*
@@ -1146,6 +1146,7 @@ static int load_slim_core_fw(const struct firmware *fw, void *context)
 		}
 	}
 
+err:
 	release_firmware(fw);
 	return err;
 }
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v3 4/6] [media] c8sectpfe: Update binding to reset-gpios
  2015-08-28 17:52 ` [PATCH v3 4/6] [media] c8sectpfe: Update binding to reset-gpios Peter Griffin
@ 2015-08-31 15:25   ` Rob Herring
  2015-09-01  7:58     ` Lee Jones
  2015-09-01 10:12     ` Peter Griffin
  2015-09-01  8:32   ` Javier Martinez Canillas
  1 sibling, 2 replies; 20+ messages in thread
From: Rob Herring @ 2015-08-31 15:25 UTC (permalink / raw)
  To: linux-arm-kernel

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 at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v3 4/6] [media] c8sectpfe: Update binding to reset-gpios
  2015-08-31 15:25   ` Rob Herring
@ 2015-09-01  7:58     ` Lee Jones
  2015-09-01 10:12     ` Peter Griffin
  1 sibling, 0 replies; 20+ messages in thread
From: Lee Jones @ 2015-09-01  7:58 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v3 2/6] ARM: DT: STi: STiH407: Add c8sectpfe LinuxDVB DT node.
  2015-08-28 17:52 ` [PATCH v3 2/6] ARM: DT: STi: STiH407: Add c8sectpfe LinuxDVB DT node Peter Griffin
@ 2015-09-01  7:59   ` Lee Jones
  0 siblings, 0 replies; 20+ messages in thread
From: Lee Jones @ 2015-09-01  7:59 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, 28 Aug 2015, Peter Griffin wrote:

> This patch adds in the required DT node for the c8sectpfe
> Linux DVB demux driver which allows the tsin channels
> to be used on an upstream kernel.
> 
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> ---
>  arch/arm/boot/dts/stihxxx-b2120.dtsi | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)

Acked-by: Lee Jones <lee.jones@linaro.org>

> diff --git a/arch/arm/boot/dts/stihxxx-b2120.dtsi b/arch/arm/boot/dts/stihxxx-b2120.dtsi
> index 62994ae..f9fca10 100644
> --- a/arch/arm/boot/dts/stihxxx-b2120.dtsi
> +++ b/arch/arm/boot/dts/stihxxx-b2120.dtsi
> @@ -6,6 +6,9 @@
>   * 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/media/c8sectpfe.h>
>  / {
>  	soc {
>  		sbc_serial0: serial at 9530000 {
> @@ -85,5 +88,37 @@
>  			status = "okay";
>  		};
>  
> +		demux at 08a20000 {
> +			compatible	= "st,stih407-c8sectpfe";
> +			status		= "okay";
> +			reg		= <0x08a20000 0x10000>,
> +					  <0x08a00000 0x4000>;
> +			reg-names	= "c8sectpfe", "c8sectpfe-ram";
> +			interrupts	= <GIC_SPI 34 IRQ_TYPE_NONE>,
> +					  <GIC_SPI 35 IRQ_TYPE_NONE>;
> +			interrupt-names	= "c8sectpfe-error-irq",
> +					  "c8sectpfe-idle-irq";
> +			pinctrl-0	= <&pinctrl_tsin0_serial>;
> +			pinctrl-1	= <&pinctrl_tsin0_parallel>;
> +			pinctrl-2	= <&pinctrl_tsin3_serial>;
> +			pinctrl-3	= <&pinctrl_tsin4_serial_alt3>;
> +			pinctrl-4	= <&pinctrl_tsin5_serial_alt1>;
> +			pinctrl-names	= "tsin0-serial",
> +					  "tsin0-parallel",
> +					  "tsin3-serial",
> +					  "tsin4-serial",
> +					  "tsin5-serial";
> +			clocks		= <&clk_s_c0_flexgen CLK_PROC_STFE>;
> +			clock-names	= "c8sectpfe";
> +
> +			/* tsin0 is TSA on NIMA */
> +			tsin0: port at 0 {
> +				tsin-num	= <0>;
> +				serial-not-parallel;
> +				i2c-bus		= <&ssc2>;
> +				rst-gpio	= <&pio15 4 GPIO_ACTIVE_HIGH>;
> +				dvb-card	= <STV0367_TDA18212_NIMA_1>;
> +			};
> +		};
>  	};
>  };

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v3 4/6] [media] c8sectpfe: Update binding to reset-gpios
  2015-08-28 17:52 ` [PATCH v3 4/6] [media] c8sectpfe: Update binding to reset-gpios Peter Griffin
  2015-08-31 15:25   ` Rob Herring
@ 2015-09-01  8:32   ` Javier Martinez Canillas
  2015-09-01  9:09     ` Lee Jones
  2015-09-01 11:54     ` Rob Herring
  1 sibling, 2 replies; 20+ messages in thread
From: Javier Martinez Canillas @ 2015-09-01  8:32 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v3 4/6] [media] c8sectpfe: Update binding to reset-gpios
  2015-09-01  8:32   ` Javier Martinez Canillas
@ 2015-09-01  9:09     ` Lee Jones
  2015-09-01  9:16       ` Javier Martinez Canillas
  2015-09-01 11:54     ` Rob Herring
  1 sibling, 1 reply; 20+ messages in thread
From: Lee Jones @ 2015-09-01  9:09 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v3 4/6] [media] c8sectpfe: Update binding to reset-gpios
  2015-09-01  9:09     ` Lee Jones
@ 2015-09-01  9:16       ` Javier Martinez Canillas
  2015-09-01  9:25         ` Lee Jones
  2015-09-01 10:34         ` Peter Griffin
  0 siblings, 2 replies; 20+ messages in thread
From: Javier Martinez Canillas @ 2015-09-01  9:16 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v3 4/6] [media] c8sectpfe: Update binding to reset-gpios
  2015-09-01  9:16       ` Javier Martinez Canillas
@ 2015-09-01  9:25         ` Lee Jones
  2015-09-01 10:34         ` Peter Griffin
  1 sibling, 0 replies; 20+ messages in thread
From: Lee Jones @ 2015-09-01  9:25 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v3 4/6] [media] c8sectpfe: Update binding to reset-gpios
  2015-08-31 15:25   ` Rob Herring
  2015-09-01  7:58     ` Lee Jones
@ 2015-09-01 10:12     ` Peter Griffin
  1 sibling, 0 replies; 20+ messages in thread
From: Peter Griffin @ 2015-09-01 10:12 UTC (permalink / raw)
  To: linux-arm-kernel

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.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v3 4/6] [media] c8sectpfe: Update binding to reset-gpios
  2015-09-01  9:16       ` Javier Martinez Canillas
  2015-09-01  9:25         ` Lee Jones
@ 2015-09-01 10:34         ` Peter Griffin
  2015-09-01 12:09           ` Lee Jones
  1 sibling, 1 reply; 20+ messages in thread
From: Peter Griffin @ 2015-09-01 10:34 UTC (permalink / raw)
  To: linux-arm-kernel

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.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v3 4/6] [media] c8sectpfe: Update binding to reset-gpios
  2015-09-01  8:32   ` Javier Martinez Canillas
  2015-09-01  9:09     ` Lee Jones
@ 2015-09-01 11:54     ` Rob Herring
  2015-09-01 12:30       ` Javier Martinez Canillas
  2015-09-02 15:03       ` Peter Griffin
  1 sibling, 2 replies; 20+ messages in thread
From: Rob Herring @ 2015-09-01 11:54 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v3 4/6] [media] c8sectpfe: Update binding to reset-gpios
  2015-09-01 10:34         ` Peter Griffin
@ 2015-09-01 12:09           ` Lee Jones
  0 siblings, 0 replies; 20+ messages in thread
From: Lee Jones @ 2015-09-01 12:09 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v3 4/6] [media] c8sectpfe: Update binding to reset-gpios
  2015-09-01 11:54     ` Rob Herring
@ 2015-09-01 12:30       ` Javier Martinez Canillas
  2015-09-02 15:03       ` Peter Griffin
  1 sibling, 0 replies; 20+ messages in thread
From: Javier Martinez Canillas @ 2015-09-01 12:30 UTC (permalink / raw)
  To: linux-arm-kernel

[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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v3 4/6] [media] c8sectpfe: Update binding to reset-gpios
  2015-09-01 11:54     ` Rob Herring
  2015-09-01 12:30       ` Javier Martinez Canillas
@ 2015-09-02 15:03       ` Peter Griffin
  1 sibling, 0 replies; 20+ messages in thread
From: Peter Griffin @ 2015-09-02 15:03 UTC (permalink / raw)
  To: linux-arm-kernel

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.

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2015-09-02 15:03 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-28 17:52 [PATCH v3 0/6] [media] c8sectpfe: Various fixups Peter Griffin
2015-08-28 17:52 ` [PATCH v3 1/6] ARM: DT: STi: stihxxx-b2120: Add pulse-width properties to ssc2 & ssc3 Peter Griffin
2015-08-28 17:52 ` [PATCH v3 2/6] ARM: DT: STi: STiH407: Add c8sectpfe LinuxDVB DT node Peter Griffin
2015-09-01  7:59   ` Lee Jones
2015-08-28 17:52 ` [PATCH v3 3/6] [media] c8sectpfe: Remove select on undefined LIBELF_32 Peter Griffin
2015-08-28 17:52 ` [PATCH v3 4/6] [media] c8sectpfe: Update binding to reset-gpios Peter Griffin
2015-08-31 15:25   ` Rob Herring
2015-09-01  7:58     ` Lee Jones
2015-09-01 10:12     ` Peter Griffin
2015-09-01  8:32   ` Javier Martinez Canillas
2015-09-01  9:09     ` Lee Jones
2015-09-01  9:16       ` Javier Martinez Canillas
2015-09-01  9:25         ` Lee Jones
2015-09-01 10:34         ` Peter Griffin
2015-09-01 12:09           ` Lee Jones
2015-09-01 11:54     ` Rob Herring
2015-09-01 12:30       ` Javier Martinez Canillas
2015-09-02 15:03       ` Peter Griffin
2015-08-28 17:52 ` [PATCH v3 5/6] [media] c8sectpfe: Update DT binding doc with some minor fixes Peter Griffin
2015-08-28 17:52 ` [PATCH v3 6/6] [media] c8sectpfe: Simplify for loop in load_slim_core_fw Peter Griffin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).