[PATCH 00/04 v2] ARM: shmobile: r7s72100 and Genmai GPIO / PINCTRL V2

[PATCH 00/04 v2] ARM: shmobile: r7s72100 and Genmai GPIO / PINCTRL V2

[Magnus Damm]

ARM: shmobile: r7s72100 and Genmai GPIO / PINCTRL V2

[PATCH 01/04] ARM: shmobile: r7s72100 GPIO and PINCTRL device nodes
[PATCH 02/04] ARM: shmobile: Genmai SCIF2 PINCTRL configuration
[PATCH 03/04] ARM: shmobile: Genmai LED1 and LED2 support
[PATCH 04/04 v2] ARM: shmobile: Genmai I2C-over-GPIO support

Add various device nodes to the r7s72100 SoC and the Genmai board,
nothing special just basic support for LEDs and I2C-over-GPIO.

Changes since V1:
 - 2/4 and 3/4 now includes acks, thanks Laurent!
 - 4/4 has been updated, thanks to Sergei and Wolfram!

Signed-off-by: Magnus Damm <damm@opensource.se>
---

 Written against renesas.git tag renesas-devel-v3.13-rc3-20131214v2
 [PATCH v3] gpio: Renesas RZ GPIO driver V3
 [PATCH 00/05 v2] pinctrl: sh-pfc: r7s72100 support V2

 arch/arm/boot/dts/r7s72100-genmai-reference.dts |   41 +++++-
 arch/arm/boot/dts/r7s72100.dtsi                 |  154 +++++++++++++++++++++++
 2 files changed, 194 insertions(+), 1 deletion(-)

[PATCH 00/04 v2] ARM: shmobile: r7s72100 and Genmai GPIO / PINCTRL V2

[Magnus Damm]

ARM: shmobile: r7s72100 and Genmai GPIO / PINCTRL V2

[PATCH 01/04] ARM: shmobile: r7s72100 GPIO and PINCTRL device nodes
[PATCH 02/04] ARM: shmobile: Genmai SCIF2 PINCTRL configuration
[PATCH 03/04] ARM: shmobile: Genmai LED1 and LED2 support
[PATCH 04/04 v2] ARM: shmobile: Genmai I2C-over-GPIO support

Add various device nodes to the r7s72100 SoC and the Genmai board,
nothing special just basic support for LEDs and I2C-over-GPIO.

Changes since V1:
 - 2/4 and 3/4 now includes acks, thanks Laurent!
 - 4/4 has been updated, thanks to Sergei and Wolfram!

Signed-off-by: Magnus Damm <damm@opensource.se>
---

 Written against renesas.git tag renesas-devel-v3.13-rc3-20131214v2
 [PATCH v3] gpio: Renesas RZ GPIO driver V3
 [PATCH 00/05 v2] pinctrl: sh-pfc: r7s72100 support V2

 arch/arm/boot/dts/r7s72100-genmai-reference.dts |   41 +++++-
 arch/arm/boot/dts/r7s72100.dtsi                 |  154 +++++++++++++++++++++++
 2 files changed, 194 insertions(+), 1 deletion(-)

[PATCH 01/04] ARM: shmobile: r7s72100 GPIO and PINCTRL device nodes

[Magnus Damm]

From: Magnus Damm <damm@opensource.se>

Add support for r7s72100 PFC and GPIO device nodes port0 -> port11
and jtagport0.

Signed-off-by: Magnus Damm <damm@opensource.se>
---

 arch/arm/boot/dts/r7s72100.dtsi |  154 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 154 insertions(+)

--- 0001/arch/arm/boot/dts/r7s72100.dtsi
+++ work/arch/arm/boot/dts/r7s72100.dtsi	2013-11-27 16:06:36.000000000 +0900
@@ -14,6 +14,22 @@
 	#address-cells = <1>;
 	#size-cells = <1>;
 
+	aliases {
+		gpio0 = &port0;
+		gpio1 = &port1;
+		gpio2 = &port2;
+		gpio3 = &port3;
+		gpio4 = &port4;
+		gpio5 = &port5;
+		gpio6 = &port6;
+		gpio7 = &port7;
+		gpio8 = &port8;
+		gpio9 = &port9;
+		gpio10 = &port10;
+		gpio11 = &port11;
+		gpio12 = &jtagport0;
+	};
+
 	cpus {
 		#address-cells = <1>;
 		#size-cells = <0>;
@@ -33,4 +49,142 @@
 		reg = <0xe8201000 0x1000>,
 			<0xe8202000 0x1000>;
 	};
+
+	pfc: pfc@fcfe3300 {
+		compatible = "renesas,pfc-r7s72100";
+		reg = <0xfcfe3300 0x400>, /* PM, PMC, PFC, PFCE */
+		  	  <0xfcfe3a00 0x100>, /* PFCAE */
+			  <0xfcfe7000 0x300>, /* PIBC, PBDC, PIPC */
+			  <0xfcfe7b40 0x04>, /* JPMC */
+			  <0xfcfe7b90 0x04>, /* JPMCSR */
+			  <0xfcfe7f00 0x04>; /* JPIBC */
+	};
+
+	port0: gpio@fcfe3100 {
+		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
+		reg = <0xfcfe3100 0x4>, /* PSR */
+		  	  <0xfcfe3200 0x2>, /* PPR */
+			  <0xfcfe3800 0x4>; /* PMSR */
+		#gpio-cells = <2>;
+		gpio-controller;
+		gpio-ranges = <&pfc 0 0 6>;
+	};
+
+	port1: gpio@fcfe3104 {
+		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
+		reg = <0xfcfe3104 0x4>, /* PSR */
+		  	  <0xfcfe3204 0x2>, /* PPR */
+			  <0xfcfe3804 0x4>; /* PMSR */
+		#gpio-cells = <2>;
+		gpio-controller;
+		gpio-ranges = <&pfc 0 16 16>;
+	};
+
+	port2: gpio@fcfe3108 {
+		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
+		reg = <0xfcfe3108 0x4>, /* PSR */
+		  	  <0xfcfe3208 0x2>, /* PPR */
+			  <0xfcfe3808 0x4>; /* PMSR */
+		#gpio-cells = <2>;
+		gpio-controller;
+		gpio-ranges = <&pfc 0 32 16>;
+	};
+
+	port3: gpio@fcfe310c {
+		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
+		reg = <0xfcfe310c 0x4>, /* PSR */
+		  	  <0xfcfe320c 0x2>, /* PPR */
+			  <0xfcfe380c 0x4>; /* PMSR */
+		#gpio-cells = <2>;
+		gpio-controller;
+		gpio-ranges = <&pfc 0 48 16>;
+	};
+
+	port4: gpio@fcfe3110 {
+		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
+		reg = <0xfcfe3110 0x4>, /* PSR */
+		  	  <0xfcfe3210 0x2>, /* PPR */
+			  <0xfcfe3810 0x4>; /* PMSR */
+		#gpio-cells = <2>;
+		gpio-controller;
+		gpio-ranges = <&pfc 0 64 16>;
+	};
+
+	port5: gpio@fcfe3114 {
+		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
+		reg = <0xfcfe3114 0x4>, /* PSR */
+		  	  <0xfcfe3214 0x2>, /* PPR */
+			  <0xfcfe3814 0x4>; /* PMSR */
+		#gpio-cells = <2>;
+		gpio-controller;
+		gpio-ranges = <&pfc 0 80 11>;
+	};
+
+	port6: gpio@fcfe3118 {
+		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
+		reg = <0xfcfe3118 0x4>, /* PSR */
+		  	  <0xfcfe3218 0x2>, /* PPR */
+			  <0xfcfe3818 0x4>; /* PMSR */
+		#gpio-cells = <2>;
+		gpio-controller;
+		gpio-ranges = <&pfc 0 96 16>;
+	};
+
+	port7: gpio@fcfe311c {
+		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
+		reg = <0xfcfe311c 0x4>, /* PSR */
+		  	  <0xfcfe321c 0x2>, /* PPR */
+			  <0xfcfe381c 0x4>; /* PMSR */
+		#gpio-cells = <2>;
+		gpio-controller;
+		gpio-ranges = <&pfc 0 112 16>;
+	};
+
+	port8: gpio@fcfe3120 {
+		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
+		reg = <0xfcfe3120 0x4>, /* PSR */
+		  	  <0xfcfe3220 0x2>, /* PPR */
+			  <0xfcfe3820 0x4>; /* PMSR */
+		#gpio-cells = <2>;
+		gpio-controller;
+		gpio-ranges = <&pfc 0 128 16>;
+	};
+
+	port9: gpio@fcfe3124 {
+		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
+		reg = <0xfcfe3124 0x4>, /* PSR */
+		  	  <0xfcfe3224 0x2>, /* PPR */
+			  <0xfcfe3824 0x4>; /* PMSR */
+		#gpio-cells = <2>;
+		gpio-controller;
+		gpio-ranges = <&pfc 0 144 8>;
+	};
+
+	port10: gpio@fcfe3128 {
+		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
+		reg = <0xfcfe3128 0x4>, /* PSR */
+		  	  <0xfcfe3228 0x2>, /* PPR */
+			  <0xfcfe3828 0x4>; /* PMSR */
+		#gpio-cells = <2>;
+		gpio-controller;
+		gpio-ranges = <&pfc 0 160 16>;
+	};
+
+	port11: gpio@fcfe312c {
+		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
+		reg = <0xfcfe312c 0x4>, /* PSR */
+		  	  <0xfcfe322c 0x2>, /* PPR */
+			  <0xfcfe382c 0x4>; /* PMSR */
+		#gpio-cells = <2>;
+		gpio-controller;
+		gpio-ranges = <&pfc 0 176 16>;
+	};
+
+	jtagport0: gpio@fcfe7b20 {
+		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
+		reg = <0xfcfe7b20 0x2>; /* JPPR */
+		#gpio-cells = <2>;
+		gpio-controller;
+		gpio-ranges = <&pfc 0 192 2>;
+	};
 };

[PATCH 01/04] ARM: shmobile: r7s72100 GPIO and PINCTRL device nodes

[Magnus Damm]

From: Magnus Damm <damm@opensource.se>

Add support for r7s72100 PFC and GPIO device nodes port0 -> port11
and jtagport0.

Signed-off-by: Magnus Damm <damm@opensource.se>
---

 arch/arm/boot/dts/r7s72100.dtsi |  154 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 154 insertions(+)

--- 0001/arch/arm/boot/dts/r7s72100.dtsi
+++ work/arch/arm/boot/dts/r7s72100.dtsi	2013-11-27 16:06:36.000000000 +0900
@@ -14,6 +14,22 @@
 	#address-cells = <1>;
 	#size-cells = <1>;
 
+	aliases {
+		gpio0 = &port0;
+		gpio1 = &port1;
+		gpio2 = &port2;
+		gpio3 = &port3;
+		gpio4 = &port4;
+		gpio5 = &port5;
+		gpio6 = &port6;
+		gpio7 = &port7;
+		gpio8 = &port8;
+		gpio9 = &port9;
+		gpio10 = &port10;
+		gpio11 = &port11;
+		gpio12 = &jtagport0;
+	};
+
 	cpus {
 		#address-cells = <1>;
 		#size-cells = <0>;
@@ -33,4 +49,142 @@
 		reg = <0xe8201000 0x1000>,
 			<0xe8202000 0x1000>;
 	};
+
+	pfc: pfc at fcfe3300 {
+		compatible = "renesas,pfc-r7s72100";
+		reg = <0xfcfe3300 0x400>, /* PM, PMC, PFC, PFCE */
+		  	  <0xfcfe3a00 0x100>, /* PFCAE */
+			  <0xfcfe7000 0x300>, /* PIBC, PBDC, PIPC */
+			  <0xfcfe7b40 0x04>, /* JPMC */
+			  <0xfcfe7b90 0x04>, /* JPMCSR */
+			  <0xfcfe7f00 0x04>; /* JPIBC */
+	};
+
+	port0: gpio at fcfe3100 {
+		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
+		reg = <0xfcfe3100 0x4>, /* PSR */
+		  	  <0xfcfe3200 0x2>, /* PPR */
+			  <0xfcfe3800 0x4>; /* PMSR */
+		#gpio-cells = <2>;
+		gpio-controller;
+		gpio-ranges = <&pfc 0 0 6>;
+	};
+
+	port1: gpio at fcfe3104 {
+		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
+		reg = <0xfcfe3104 0x4>, /* PSR */
+		  	  <0xfcfe3204 0x2>, /* PPR */
+			  <0xfcfe3804 0x4>; /* PMSR */
+		#gpio-cells = <2>;
+		gpio-controller;
+		gpio-ranges = <&pfc 0 16 16>;
+	};
+
+	port2: gpio at fcfe3108 {
+		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
+		reg = <0xfcfe3108 0x4>, /* PSR */
+		  	  <0xfcfe3208 0x2>, /* PPR */
+			  <0xfcfe3808 0x4>; /* PMSR */
+		#gpio-cells = <2>;
+		gpio-controller;
+		gpio-ranges = <&pfc 0 32 16>;
+	};
+
+	port3: gpio at fcfe310c {
+		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
+		reg = <0xfcfe310c 0x4>, /* PSR */
+		  	  <0xfcfe320c 0x2>, /* PPR */
+			  <0xfcfe380c 0x4>; /* PMSR */
+		#gpio-cells = <2>;
+		gpio-controller;
+		gpio-ranges = <&pfc 0 48 16>;
+	};
+
+	port4: gpio at fcfe3110 {
+		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
+		reg = <0xfcfe3110 0x4>, /* PSR */
+		  	  <0xfcfe3210 0x2>, /* PPR */
+			  <0xfcfe3810 0x4>; /* PMSR */
+		#gpio-cells = <2>;
+		gpio-controller;
+		gpio-ranges = <&pfc 0 64 16>;
+	};
+
+	port5: gpio at fcfe3114 {
+		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
+		reg = <0xfcfe3114 0x4>, /* PSR */
+		  	  <0xfcfe3214 0x2>, /* PPR */
+			  <0xfcfe3814 0x4>; /* PMSR */
+		#gpio-cells = <2>;
+		gpio-controller;
+		gpio-ranges = <&pfc 0 80 11>;
+	};
+
+	port6: gpio at fcfe3118 {
+		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
+		reg = <0xfcfe3118 0x4>, /* PSR */
+		  	  <0xfcfe3218 0x2>, /* PPR */
+			  <0xfcfe3818 0x4>; /* PMSR */
+		#gpio-cells = <2>;
+		gpio-controller;
+		gpio-ranges = <&pfc 0 96 16>;
+	};
+
+	port7: gpio at fcfe311c {
+		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
+		reg = <0xfcfe311c 0x4>, /* PSR */
+		  	  <0xfcfe321c 0x2>, /* PPR */
+			  <0xfcfe381c 0x4>; /* PMSR */
+		#gpio-cells = <2>;
+		gpio-controller;
+		gpio-ranges = <&pfc 0 112 16>;
+	};
+
+	port8: gpio at fcfe3120 {
+		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
+		reg = <0xfcfe3120 0x4>, /* PSR */
+		  	  <0xfcfe3220 0x2>, /* PPR */
+			  <0xfcfe3820 0x4>; /* PMSR */
+		#gpio-cells = <2>;
+		gpio-controller;
+		gpio-ranges = <&pfc 0 128 16>;
+	};
+
+	port9: gpio at fcfe3124 {
+		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
+		reg = <0xfcfe3124 0x4>, /* PSR */
+		  	  <0xfcfe3224 0x2>, /* PPR */
+			  <0xfcfe3824 0x4>; /* PMSR */
+		#gpio-cells = <2>;
+		gpio-controller;
+		gpio-ranges = <&pfc 0 144 8>;
+	};
+
+	port10: gpio at fcfe3128 {
+		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
+		reg = <0xfcfe3128 0x4>, /* PSR */
+		  	  <0xfcfe3228 0x2>, /* PPR */
+			  <0xfcfe3828 0x4>; /* PMSR */
+		#gpio-cells = <2>;
+		gpio-controller;
+		gpio-ranges = <&pfc 0 160 16>;
+	};
+
+	port11: gpio at fcfe312c {
+		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
+		reg = <0xfcfe312c 0x4>, /* PSR */
+		  	  <0xfcfe322c 0x2>, /* PPR */
+			  <0xfcfe382c 0x4>; /* PMSR */
+		#gpio-cells = <2>;
+		gpio-controller;
+		gpio-ranges = <&pfc 0 176 16>;
+	};
+
+	jtagport0: gpio at fcfe7b20 {
+		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
+		reg = <0xfcfe7b20 0x2>; /* JPPR */
+		#gpio-cells = <2>;
+		gpio-controller;
+		gpio-ranges = <&pfc 0 192 2>;
+	};
 };

Re: [PATCH 01/04] ARM: shmobile: r7s72100 GPIO and PINCTRL device nodes

[Laurent Pinchart]

Hi Magnus,

Thank you for the patch.

On Tuesday 17 December 2013 14:02:42 Magnus Damm wrote:
> From: Magnus Damm <damm@opensource.se>
> 
> Add support for r7s72100 PFC and GPIO device nodes port0 -> port11
> and jtagport0.
> 
> Signed-off-by: Magnus Damm <damm@opensource.se>
> ---
> 
>  arch/arm/boot/dts/r7s72100.dtsi |  154 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 154 insertions(+)
> 
> --- 0001/arch/arm/boot/dts/r7s72100.dtsi
> +++ work/arch/arm/boot/dts/r7s72100.dtsi	2013-11-27 16:06:36.000000000 
+0900
> @@ -14,6 +14,22 @@
>  	#address-cells = <1>;
>  	#size-cells = <1>;
> 
> +	aliases {
> +		gpio0 = &port0;
> +		gpio1 = &port1;
> +		gpio2 = &port2;
> +		gpio3 = &port3;
> +		gpio4 = &port4;
> +		gpio5 = &port5;
> +		gpio6 = &port6;
> +		gpio7 = &port7;
> +		gpio8 = &port8;
> +		gpio9 = &port9;
> +		gpio10 = &port10;
> +		gpio11 = &port11;
> +		gpio12 = &jtagport0;
> +	};
> +
>  	cpus {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
> @@ -33,4 +49,142 @@
>  		reg = <0xe8201000 0x1000>,
>  			<0xe8202000 0x1000>;
>  	};
> +
> +	pfc: pfc@fcfe3300 {
> +		compatible = "renesas,pfc-r7s72100";
> +		reg = <0xfcfe3300 0x400>, /* PM, PMC, PFC, PFCE */
> +		  	  <0xfcfe3a00 0x100>, /* PFCAE */
> +			  <0xfcfe7000 0x300>, /* PIBC, PBDC, PIPC */
> +			  <0xfcfe7b40 0x04>, /* JPMC */
> +			  <0xfcfe7b90 0x04>, /* JPMCSR */
> +			  <0xfcfe7f00 0x04>; /* JPIBC */
> +	};
> +
> +	port0: gpio@fcfe3100 {
> +		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
> +		reg = <0xfcfe3100 0x4>, /* PSR */
> +		  	  <0xfcfe3200 0x2>, /* PPR */
> +			  <0xfcfe3800 0x4>; /* PMSR */
> +		#gpio-cells = <2>;
> +		gpio-controller;
> +		gpio-ranges = <&pfc 0 0 6>;
> +	};
> +
> +	port1: gpio@fcfe3104 {
> +		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
> +		reg = <0xfcfe3104 0x4>, /* PSR */
> +		  	  <0xfcfe3204 0x2>, /* PPR */
> +			  <0xfcfe3804 0x4>; /* PMSR */
> +		#gpio-cells = <2>;
> +		gpio-controller;
> +		gpio-ranges = <&pfc 0 16 16>;

As P0 has 6 pins only this should ideally be 

		gpio-ranges = <&pfc 0 6 16>;

Otherwise the PFC driver will expose pins that don't exist. However, that 
would require computing the pin numbers in the PFC driver differently, as we 
currently just use the bank * 16 + index formula. Given that we only have 
three ports with less than 16 pins we could come up with a not overly complex 
formula that can be evaluated at compile time. Something like this should do.

#define RZ_PORT_PIN(bank, pin) \
	(bank) < 1 ? (pin) : \
	(bank) < 6 ? 6 + (((bank) - 1) * 16) + (pin)) : \
	(bank) < 10 ? 6 + 11 + 4 * 16 + (((bank) - 6) * 16) + (pin)) : \
	6 + 11 + 8 + 7 * 16 + (((bank) - 10) * 16) + (pin))

> +	};
> +
> +	port2: gpio@fcfe3108 {
> +		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
> +		reg = <0xfcfe3108 0x4>, /* PSR */
> +		  	  <0xfcfe3208 0x2>, /* PPR */
> +			  <0xfcfe3808 0x4>; /* PMSR */
> +		#gpio-cells = <2>;
> +		gpio-controller;
> +		gpio-ranges = <&pfc 0 32 16>;
> +	};
> +
> +	port3: gpio@fcfe310c {
> +		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
> +		reg = <0xfcfe310c 0x4>, /* PSR */
> +		  	  <0xfcfe320c 0x2>, /* PPR */
> +			  <0xfcfe380c 0x4>; /* PMSR */
> +		#gpio-cells = <2>;
> +		gpio-controller;
> +		gpio-ranges = <&pfc 0 48 16>;
> +	};
> +
> +	port4: gpio@fcfe3110 {
> +		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
> +		reg = <0xfcfe3110 0x4>, /* PSR */
> +		  	  <0xfcfe3210 0x2>, /* PPR */
> +			  <0xfcfe3810 0x4>; /* PMSR */
> +		#gpio-cells = <2>;
> +		gpio-controller;
> +		gpio-ranges = <&pfc 0 64 16>;
> +	};
> +
> +	port5: gpio@fcfe3114 {
> +		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
> +		reg = <0xfcfe3114 0x4>, /* PSR */
> +		  	  <0xfcfe3214 0x2>, /* PPR */
> +			  <0xfcfe3814 0x4>; /* PMSR */
> +		#gpio-cells = <2>;
> +		gpio-controller;
> +		gpio-ranges = <&pfc 0 80 11>;
> +	};
> +
> +	port6: gpio@fcfe3118 {
> +		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
> +		reg = <0xfcfe3118 0x4>, /* PSR */
> +		  	  <0xfcfe3218 0x2>, /* PPR */
> +			  <0xfcfe3818 0x4>; /* PMSR */
> +		#gpio-cells = <2>;
> +		gpio-controller;
> +		gpio-ranges = <&pfc 0 96 16>;
> +	};
> +
> +	port7: gpio@fcfe311c {
> +		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
> +		reg = <0xfcfe311c 0x4>, /* PSR */
> +		  	  <0xfcfe321c 0x2>, /* PPR */
> +			  <0xfcfe381c 0x4>; /* PMSR */
> +		#gpio-cells = <2>;
> +		gpio-controller;
> +		gpio-ranges = <&pfc 0 112 16>;
> +	};
> +
> +	port8: gpio@fcfe3120 {
> +		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
> +		reg = <0xfcfe3120 0x4>, /* PSR */
> +		  	  <0xfcfe3220 0x2>, /* PPR */
> +			  <0xfcfe3820 0x4>; /* PMSR */
> +		#gpio-cells = <2>;
> +		gpio-controller;
> +		gpio-ranges = <&pfc 0 128 16>;
> +	};
> +
> +	port9: gpio@fcfe3124 {
> +		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
> +		reg = <0xfcfe3124 0x4>, /* PSR */
> +		  	  <0xfcfe3224 0x2>, /* PPR */
> +			  <0xfcfe3824 0x4>; /* PMSR */
> +		#gpio-cells = <2>;
> +		gpio-controller;
> +		gpio-ranges = <&pfc 0 144 8>;
> +	};
> +
> +	port10: gpio@fcfe3128 {
> +		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
> +		reg = <0xfcfe3128 0x4>, /* PSR */
> +		  	  <0xfcfe3228 0x2>, /* PPR */
> +			  <0xfcfe3828 0x4>; /* PMSR */
> +		#gpio-cells = <2>;
> +		gpio-controller;
> +		gpio-ranges = <&pfc 0 160 16>;
> +	};
> +
> +	port11: gpio@fcfe312c {
> +		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
> +		reg = <0xfcfe312c 0x4>, /* PSR */
> +		  	  <0xfcfe322c 0x2>, /* PPR */
> +			  <0xfcfe382c 0x4>; /* PMSR */
> +		#gpio-cells = <2>;
> +		gpio-controller;
> +		gpio-ranges = <&pfc 0 176 16>;
> +	};
> +
> +	jtagport0: gpio@fcfe7b20 {
> +		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
> +		reg = <0xfcfe7b20 0x2>; /* JPPR */
> +		#gpio-cells = <2>;
> +		gpio-controller;
> +		gpio-ranges = <&pfc 0 192 2>;
> +	};
>  };
-- 
Regards,

Laurent Pinchart

[PATCH 01/04] ARM: shmobile: r7s72100 GPIO and PINCTRL device nodes

[Laurent Pinchart]

Hi Magnus,

Thank you for the patch.

On Tuesday 17 December 2013 14:02:42 Magnus Damm wrote:
> From: Magnus Damm <damm@opensource.se>
> 
> Add support for r7s72100 PFC and GPIO device nodes port0 -> port11
> and jtagport0.
> 
> Signed-off-by: Magnus Damm <damm@opensource.se>
> ---
> 
>  arch/arm/boot/dts/r7s72100.dtsi |  154 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 154 insertions(+)
> 
> --- 0001/arch/arm/boot/dts/r7s72100.dtsi
> +++ work/arch/arm/boot/dts/r7s72100.dtsi	2013-11-27 16:06:36.000000000 
+0900
> @@ -14,6 +14,22 @@
>  	#address-cells = <1>;
>  	#size-cells = <1>;
> 
> +	aliases {
> +		gpio0 = &port0;
> +		gpio1 = &port1;
> +		gpio2 = &port2;
> +		gpio3 = &port3;
> +		gpio4 = &port4;
> +		gpio5 = &port5;
> +		gpio6 = &port6;
> +		gpio7 = &port7;
> +		gpio8 = &port8;
> +		gpio9 = &port9;
> +		gpio10 = &port10;
> +		gpio11 = &port11;
> +		gpio12 = &jtagport0;
> +	};
> +
>  	cpus {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
> @@ -33,4 +49,142 @@
>  		reg = <0xe8201000 0x1000>,
>  			<0xe8202000 0x1000>;
>  	};
> +
> +	pfc: pfc at fcfe3300 {
> +		compatible = "renesas,pfc-r7s72100";
> +		reg = <0xfcfe3300 0x400>, /* PM, PMC, PFC, PFCE */
> +		  	  <0xfcfe3a00 0x100>, /* PFCAE */
> +			  <0xfcfe7000 0x300>, /* PIBC, PBDC, PIPC */
> +			  <0xfcfe7b40 0x04>, /* JPMC */
> +			  <0xfcfe7b90 0x04>, /* JPMCSR */
> +			  <0xfcfe7f00 0x04>; /* JPIBC */
> +	};
> +
> +	port0: gpio at fcfe3100 {
> +		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
> +		reg = <0xfcfe3100 0x4>, /* PSR */
> +		  	  <0xfcfe3200 0x2>, /* PPR */
> +			  <0xfcfe3800 0x4>; /* PMSR */
> +		#gpio-cells = <2>;
> +		gpio-controller;
> +		gpio-ranges = <&pfc 0 0 6>;
> +	};
> +
> +	port1: gpio at fcfe3104 {
> +		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
> +		reg = <0xfcfe3104 0x4>, /* PSR */
> +		  	  <0xfcfe3204 0x2>, /* PPR */
> +			  <0xfcfe3804 0x4>; /* PMSR */
> +		#gpio-cells = <2>;
> +		gpio-controller;
> +		gpio-ranges = <&pfc 0 16 16>;

As P0 has 6 pins only this should ideally be 

		gpio-ranges = <&pfc 0 6 16>;

Otherwise the PFC driver will expose pins that don't exist. However, that 
would require computing the pin numbers in the PFC driver differently, as we 
currently just use the bank * 16 + index formula. Given that we only have 
three ports with less than 16 pins we could come up with a not overly complex 
formula that can be evaluated at compile time. Something like this should do.

#define RZ_PORT_PIN(bank, pin) \
	(bank) < 1 ? (pin) : \
	(bank) < 6 ? 6 + (((bank) - 1) * 16) + (pin)) : \
	(bank) < 10 ? 6 + 11 + 4 * 16 + (((bank) - 6) * 16) + (pin)) : \
	6 + 11 + 8 + 7 * 16 + (((bank) - 10) * 16) + (pin))

> +	};
> +
> +	port2: gpio at fcfe3108 {
> +		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
> +		reg = <0xfcfe3108 0x4>, /* PSR */
> +		  	  <0xfcfe3208 0x2>, /* PPR */
> +			  <0xfcfe3808 0x4>; /* PMSR */
> +		#gpio-cells = <2>;
> +		gpio-controller;
> +		gpio-ranges = <&pfc 0 32 16>;
> +	};
> +
> +	port3: gpio at fcfe310c {
> +		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
> +		reg = <0xfcfe310c 0x4>, /* PSR */
> +		  	  <0xfcfe320c 0x2>, /* PPR */
> +			  <0xfcfe380c 0x4>; /* PMSR */
> +		#gpio-cells = <2>;
> +		gpio-controller;
> +		gpio-ranges = <&pfc 0 48 16>;
> +	};
> +
> +	port4: gpio at fcfe3110 {
> +		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
> +		reg = <0xfcfe3110 0x4>, /* PSR */
> +		  	  <0xfcfe3210 0x2>, /* PPR */
> +			  <0xfcfe3810 0x4>; /* PMSR */
> +		#gpio-cells = <2>;
> +		gpio-controller;
> +		gpio-ranges = <&pfc 0 64 16>;
> +	};
> +
> +	port5: gpio at fcfe3114 {
> +		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
> +		reg = <0xfcfe3114 0x4>, /* PSR */
> +		  	  <0xfcfe3214 0x2>, /* PPR */
> +			  <0xfcfe3814 0x4>; /* PMSR */
> +		#gpio-cells = <2>;
> +		gpio-controller;
> +		gpio-ranges = <&pfc 0 80 11>;
> +	};
> +
> +	port6: gpio at fcfe3118 {
> +		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
> +		reg = <0xfcfe3118 0x4>, /* PSR */
> +		  	  <0xfcfe3218 0x2>, /* PPR */
> +			  <0xfcfe3818 0x4>; /* PMSR */
> +		#gpio-cells = <2>;
> +		gpio-controller;
> +		gpio-ranges = <&pfc 0 96 16>;
> +	};
> +
> +	port7: gpio at fcfe311c {
> +		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
> +		reg = <0xfcfe311c 0x4>, /* PSR */
> +		  	  <0xfcfe321c 0x2>, /* PPR */
> +			  <0xfcfe381c 0x4>; /* PMSR */
> +		#gpio-cells = <2>;
> +		gpio-controller;
> +		gpio-ranges = <&pfc 0 112 16>;
> +	};
> +
> +	port8: gpio at fcfe3120 {
> +		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
> +		reg = <0xfcfe3120 0x4>, /* PSR */
> +		  	  <0xfcfe3220 0x2>, /* PPR */
> +			  <0xfcfe3820 0x4>; /* PMSR */
> +		#gpio-cells = <2>;
> +		gpio-controller;
> +		gpio-ranges = <&pfc 0 128 16>;
> +	};
> +
> +	port9: gpio at fcfe3124 {
> +		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
> +		reg = <0xfcfe3124 0x4>, /* PSR */
> +		  	  <0xfcfe3224 0x2>, /* PPR */
> +			  <0xfcfe3824 0x4>; /* PMSR */
> +		#gpio-cells = <2>;
> +		gpio-controller;
> +		gpio-ranges = <&pfc 0 144 8>;
> +	};
> +
> +	port10: gpio at fcfe3128 {
> +		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
> +		reg = <0xfcfe3128 0x4>, /* PSR */
> +		  	  <0xfcfe3228 0x2>, /* PPR */
> +			  <0xfcfe3828 0x4>; /* PMSR */
> +		#gpio-cells = <2>;
> +		gpio-controller;
> +		gpio-ranges = <&pfc 0 160 16>;
> +	};
> +
> +	port11: gpio at fcfe312c {
> +		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
> +		reg = <0xfcfe312c 0x4>, /* PSR */
> +		  	  <0xfcfe322c 0x2>, /* PPR */
> +			  <0xfcfe382c 0x4>; /* PMSR */
> +		#gpio-cells = <2>;
> +		gpio-controller;
> +		gpio-ranges = <&pfc 0 176 16>;
> +	};
> +
> +	jtagport0: gpio at fcfe7b20 {
> +		compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
> +		reg = <0xfcfe7b20 0x2>; /* JPPR */
> +		#gpio-cells = <2>;
> +		gpio-controller;
> +		gpio-ranges = <&pfc 0 192 2>;
> +	};
>  };
-- 
Regards,

Laurent Pinchart

Re: [PATCH 01/04] ARM: shmobile: r7s72100 GPIO and PINCTRL device nodes

[Magnus Damm]

Hi Laurent,

On Wed, Dec 18, 2013 at 1:29 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Magnus,
>
> Thank you for the patch.
>
> On Tuesday 17 December 2013 14:02:42 Magnus Damm wrote:
>> From: Magnus Damm <damm@opensource.se>
>>
>> Add support for r7s72100 PFC and GPIO device nodes port0 -> port11
>> and jtagport0.
>>
>> Signed-off-by: Magnus Damm <damm@opensource.se>
>> ---
>>
>>  arch/arm/boot/dts/r7s72100.dtsi |  154 ++++++++++++++++++++++++++++++++++++
>>  1 file changed, 154 insertions(+)
>>
>> --- 0001/arch/arm/boot/dts/r7s72100.dtsi
>> +++ work/arch/arm/boot/dts/r7s72100.dtsi      2013-11-27 16:06:36.000000000
> +0900
>> @@ -14,6 +14,22 @@
>>       #address-cells = <1>;
>>       #size-cells = <1>;
>>
>> +     aliases {
>> +             gpio0 = &port0;
>> +             gpio1 = &port1;
>> +             gpio2 = &port2;
>> +             gpio3 = &port3;
>> +             gpio4 = &port4;
>> +             gpio5 = &port5;
>> +             gpio6 = &port6;
>> +             gpio7 = &port7;
>> +             gpio8 = &port8;
>> +             gpio9 = &port9;
>> +             gpio10 = &port10;
>> +             gpio11 = &port11;
>> +             gpio12 = &jtagport0;
>> +     };
>> +
>>       cpus {
>>               #address-cells = <1>;
>>               #size-cells = <0>;
>> @@ -33,4 +49,142 @@
>>               reg = <0xe8201000 0x1000>,
>>                       <0xe8202000 0x1000>;
>>       };
>> +
>> +     pfc: pfc@fcfe3300 {
>> +             compatible = "renesas,pfc-r7s72100";
>> +             reg = <0xfcfe3300 0x400>, /* PM, PMC, PFC, PFCE */
>> +                       <0xfcfe3a00 0x100>, /* PFCAE */
>> +                       <0xfcfe7000 0x300>, /* PIBC, PBDC, PIPC */
>> +                       <0xfcfe7b40 0x04>, /* JPMC */
>> +                       <0xfcfe7b90 0x04>, /* JPMCSR */
>> +                       <0xfcfe7f00 0x04>; /* JPIBC */
>> +     };
>> +
>> +     port0: gpio@fcfe3100 {
>> +             compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
>> +             reg = <0xfcfe3100 0x4>, /* PSR */
>> +                       <0xfcfe3200 0x2>, /* PPR */
>> +                       <0xfcfe3800 0x4>; /* PMSR */
>> +             #gpio-cells = <2>;
>> +             gpio-controller;
>> +             gpio-ranges = <&pfc 0 0 6>;
>> +     };
>> +
>> +     port1: gpio@fcfe3104 {
>> +             compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
>> +             reg = <0xfcfe3104 0x4>, /* PSR */
>> +                       <0xfcfe3204 0x2>, /* PPR */
>> +                       <0xfcfe3804 0x4>; /* PMSR */
>> +             #gpio-cells = <2>;
>> +             gpio-controller;
>> +             gpio-ranges = <&pfc 0 16 16>;
>
> As P0 has 6 pins only this should ideally be
>
>                 gpio-ranges = <&pfc 0 6 16>;
>
> Otherwise the PFC driver will expose pins that don't exist. However, that
> would require computing the pin numbers in the PFC driver differently, as we
> currently just use the bank * 16 + index formula. Given that we only have
> three ports with less than 16 pins we could come up with a not overly complex
> formula that can be evaluated at compile time. Something like this should do.
>
> #define RZ_PORT_PIN(bank, pin) \
>         (bank) < 1 ? (pin) : \
>         (bank) < 6 ? 6 + (((bank) - 1) * 16) + (pin)) : \
>         (bank) < 10 ? 6 + 11 + 4 * 16 + (((bank) - 6) * 16) + (pin)) : \
>         6 + 11 + 8 + 7 * 16 + (((bank) - 10) * 16) + (pin))

Uhm, well, you can make the mapping more compact yes, but I'm not sure
if I agree that it becomes any better. Isn't it better to simply
follow the per-port setup that the manual defines? Is there an actual
problem with having unused GPIOs?

Actually, I prefer going in the opposite direction so I would like to
share the simple version of RZ_PORT_PIN() in a header file like we do
with RCAR_GP_PIN() in <linux/platform_data/gpio-rcar.h>. This because
we would like to use the same macro in the GPIO driver and in the
current PFC code (and potentially more PFCs using the same GPIO
driver).

Please let me know what you think.

Cheers,

/ magnus

[PATCH 01/04] ARM: shmobile: r7s72100 GPIO and PINCTRL device nodes

[Magnus Damm]

Hi Laurent,

On Wed, Dec 18, 2013 at 1:29 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Magnus,
>
> Thank you for the patch.
>
> On Tuesday 17 December 2013 14:02:42 Magnus Damm wrote:
>> From: Magnus Damm <damm@opensource.se>
>>
>> Add support for r7s72100 PFC and GPIO device nodes port0 -> port11
>> and jtagport0.
>>
>> Signed-off-by: Magnus Damm <damm@opensource.se>
>> ---
>>
>>  arch/arm/boot/dts/r7s72100.dtsi |  154 ++++++++++++++++++++++++++++++++++++
>>  1 file changed, 154 insertions(+)
>>
>> --- 0001/arch/arm/boot/dts/r7s72100.dtsi
>> +++ work/arch/arm/boot/dts/r7s72100.dtsi      2013-11-27 16:06:36.000000000
> +0900
>> @@ -14,6 +14,22 @@
>>       #address-cells = <1>;
>>       #size-cells = <1>;
>>
>> +     aliases {
>> +             gpio0 = &port0;
>> +             gpio1 = &port1;
>> +             gpio2 = &port2;
>> +             gpio3 = &port3;
>> +             gpio4 = &port4;
>> +             gpio5 = &port5;
>> +             gpio6 = &port6;
>> +             gpio7 = &port7;
>> +             gpio8 = &port8;
>> +             gpio9 = &port9;
>> +             gpio10 = &port10;
>> +             gpio11 = &port11;
>> +             gpio12 = &jtagport0;
>> +     };
>> +
>>       cpus {
>>               #address-cells = <1>;
>>               #size-cells = <0>;
>> @@ -33,4 +49,142 @@
>>               reg = <0xe8201000 0x1000>,
>>                       <0xe8202000 0x1000>;
>>       };
>> +
>> +     pfc: pfc at fcfe3300 {
>> +             compatible = "renesas,pfc-r7s72100";
>> +             reg = <0xfcfe3300 0x400>, /* PM, PMC, PFC, PFCE */
>> +                       <0xfcfe3a00 0x100>, /* PFCAE */
>> +                       <0xfcfe7000 0x300>, /* PIBC, PBDC, PIPC */
>> +                       <0xfcfe7b40 0x04>, /* JPMC */
>> +                       <0xfcfe7b90 0x04>, /* JPMCSR */
>> +                       <0xfcfe7f00 0x04>; /* JPIBC */
>> +     };
>> +
>> +     port0: gpio at fcfe3100 {
>> +             compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
>> +             reg = <0xfcfe3100 0x4>, /* PSR */
>> +                       <0xfcfe3200 0x2>, /* PPR */
>> +                       <0xfcfe3800 0x4>; /* PMSR */
>> +             #gpio-cells = <2>;
>> +             gpio-controller;
>> +             gpio-ranges = <&pfc 0 0 6>;
>> +     };
>> +
>> +     port1: gpio at fcfe3104 {
>> +             compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
>> +             reg = <0xfcfe3104 0x4>, /* PSR */
>> +                       <0xfcfe3204 0x2>, /* PPR */
>> +                       <0xfcfe3804 0x4>; /* PMSR */
>> +             #gpio-cells = <2>;
>> +             gpio-controller;
>> +             gpio-ranges = <&pfc 0 16 16>;
>
> As P0 has 6 pins only this should ideally be
>
>                 gpio-ranges = <&pfc 0 6 16>;
>
> Otherwise the PFC driver will expose pins that don't exist. However, that
> would require computing the pin numbers in the PFC driver differently, as we
> currently just use the bank * 16 + index formula. Given that we only have
> three ports with less than 16 pins we could come up with a not overly complex
> formula that can be evaluated at compile time. Something like this should do.
>
> #define RZ_PORT_PIN(bank, pin) \
>         (bank) < 1 ? (pin) : \
>         (bank) < 6 ? 6 + (((bank) - 1) * 16) + (pin)) : \
>         (bank) < 10 ? 6 + 11 + 4 * 16 + (((bank) - 6) * 16) + (pin)) : \
>         6 + 11 + 8 + 7 * 16 + (((bank) - 10) * 16) + (pin))

Uhm, well, you can make the mapping more compact yes, but I'm not sure
if I agree that it becomes any better. Isn't it better to simply
follow the per-port setup that the manual defines? Is there an actual
problem with having unused GPIOs?

Actually, I prefer going in the opposite direction so I would like to
share the simple version of RZ_PORT_PIN() in a header file like we do
with RCAR_GP_PIN() in <linux/platform_data/gpio-rcar.h>. This because
we would like to use the same macro in the GPIO driver and in the
current PFC code (and potentially more PFCs using the same GPIO
driver).

Please let me know what you think.

Cheers,

/ magnus

Re: [PATCH 01/04] ARM: shmobile: r7s72100 GPIO and PINCTRL device nodes

[Laurent Pinchart]

Hi Magnus,

On Wednesday 18 December 2013 07:41:57 Magnus Damm wrote:
> On Wed, Dec 18, 2013 at 1:29 AM, Laurent Pinchart wrote:
> > On Tuesday 17 December 2013 14:02:42 Magnus Damm wrote:
> >> From: Magnus Damm <damm@opensource.se>
> >> 
> >> Add support for r7s72100 PFC and GPIO device nodes port0 -> port11
> >> and jtagport0.
> >> 
> >> Signed-off-by: Magnus Damm <damm@opensource.se>
> >> ---
> >> 
> >>  arch/arm/boot/dts/r7s72100.dtsi |  154 +++++++++++++++++++++++++++++++++
> >>  1 file changed, 154 insertions(+)
> >> 
> >> --- 0001/arch/arm/boot/dts/r7s72100.dtsi
> >> +++ work/arch/arm/boot/dts/r7s72100.dtsi      2013-11-27
> >> 16:06:36.000000000 +0900

[snip]

> >> +
> >> +     port0: gpio@fcfe3100 {
> >> +             compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
> >> +             reg = <0xfcfe3100 0x4>, /* PSR */
> >> +                       <0xfcfe3200 0x2>, /* PPR */
> >> +                       <0xfcfe3800 0x4>; /* PMSR */
> >> +             #gpio-cells = <2>;
> >> +             gpio-controller;
> >> +             gpio-ranges = <&pfc 0 0 6>;
> >> +     };
> >> +
> >> +     port1: gpio@fcfe3104 {
> >> +             compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
> >> +             reg = <0xfcfe3104 0x4>, /* PSR */
> >> +                       <0xfcfe3204 0x2>, /* PPR */
> >> +                       <0xfcfe3804 0x4>; /* PMSR */
> >> +             #gpio-cells = <2>;
> >> +             gpio-controller;
> >> +             gpio-ranges = <&pfc 0 16 16>;
> > 
> > As P0 has 6 pins only this should ideally be
> > 
> >                 gpio-ranges = <&pfc 0 6 16>;
> > 
> > Otherwise the PFC driver will expose pins that don't exist. However, that
> > would require computing the pin numbers in the PFC driver differently, as
> > we currently just use the bank * 16 + index formula. Given that we only
> > have three ports with less than 16 pins we could come up with a not
> > overly complex formula that can be evaluated at compile time. Something
> > like this should do.
> > 
> > #define RZ_PORT_PIN(bank, pin) \
> > 
> >         (bank) < 1 ? (pin) : \
> >         (bank) < 6 ? 6 + (((bank) - 1) * 16) + (pin)) : \
> >         (bank) < 10 ? 6 + 11 + 4 * 16 + (((bank) - 6) * 16) + (pin)) : \
> >         6 + 11 + 8 + 7 * 16 + (((bank) - 10) * 16) + (pin))
> 
> Uhm, well, you can make the mapping more compact yes, but I'm not sure
> if I agree that it becomes any better. Isn't it better to simply
> follow the per-port setup that the manual defines? Is there an actual
> problem with having unused GPIOs?

If I'm not mistaken it's unused pins, not unused GPIOs. They waste memory in 
data tables, although by a relatively small amount. Oh, and of course, it's 
not clean ;-)

Speaking of data tables, I'm thinking about simplifying them. The RZ/A1H is a 
good candidate for that, as each pin is handled individually, and several 
registers could be handled to with a small amount of code instead of large 
data tables. It's just a thought for now, I have more urgent tasks to work on.

> Actually, I prefer going in the opposite direction so I would like to
> share the simple version of RZ_PORT_PIN() in a header file like we do
> with RCAR_GP_PIN() in <linux/platform_data/gpio-rcar.h>. This because
> we would like to use the same macro in the GPIO driver and in the
> current PFC code (and potentially more PFCs using the same GPIO
> driver).

What do you need it for in the GPIO driver ?

> Please let me know what you think.

-- 
Regards,

Laurent Pinchart

[PATCH 01/04] ARM: shmobile: r7s72100 GPIO and PINCTRL device nodes

[Laurent Pinchart]

Hi Magnus,

On Wednesday 18 December 2013 07:41:57 Magnus Damm wrote:
> On Wed, Dec 18, 2013 at 1:29 AM, Laurent Pinchart wrote:
> > On Tuesday 17 December 2013 14:02:42 Magnus Damm wrote:
> >> From: Magnus Damm <damm@opensource.se>
> >> 
> >> Add support for r7s72100 PFC and GPIO device nodes port0 -> port11
> >> and jtagport0.
> >> 
> >> Signed-off-by: Magnus Damm <damm@opensource.se>
> >> ---
> >> 
> >>  arch/arm/boot/dts/r7s72100.dtsi |  154 +++++++++++++++++++++++++++++++++
> >>  1 file changed, 154 insertions(+)
> >> 
> >> --- 0001/arch/arm/boot/dts/r7s72100.dtsi
> >> +++ work/arch/arm/boot/dts/r7s72100.dtsi      2013-11-27
> >> 16:06:36.000000000 +0900

[snip]

> >> +
> >> +     port0: gpio at fcfe3100 {
> >> +             compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
> >> +             reg = <0xfcfe3100 0x4>, /* PSR */
> >> +                       <0xfcfe3200 0x2>, /* PPR */
> >> +                       <0xfcfe3800 0x4>; /* PMSR */
> >> +             #gpio-cells = <2>;
> >> +             gpio-controller;
> >> +             gpio-ranges = <&pfc 0 0 6>;
> >> +     };
> >> +
> >> +     port1: gpio at fcfe3104 {
> >> +             compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
> >> +             reg = <0xfcfe3104 0x4>, /* PSR */
> >> +                       <0xfcfe3204 0x2>, /* PPR */
> >> +                       <0xfcfe3804 0x4>; /* PMSR */
> >> +             #gpio-cells = <2>;
> >> +             gpio-controller;
> >> +             gpio-ranges = <&pfc 0 16 16>;
> > 
> > As P0 has 6 pins only this should ideally be
> > 
> >                 gpio-ranges = <&pfc 0 6 16>;
> > 
> > Otherwise the PFC driver will expose pins that don't exist. However, that
> > would require computing the pin numbers in the PFC driver differently, as
> > we currently just use the bank * 16 + index formula. Given that we only
> > have three ports with less than 16 pins we could come up with a not
> > overly complex formula that can be evaluated at compile time. Something
> > like this should do.
> > 
> > #define RZ_PORT_PIN(bank, pin) \
> > 
> >         (bank) < 1 ? (pin) : \
> >         (bank) < 6 ? 6 + (((bank) - 1) * 16) + (pin)) : \
> >         (bank) < 10 ? 6 + 11 + 4 * 16 + (((bank) - 6) * 16) + (pin)) : \
> >         6 + 11 + 8 + 7 * 16 + (((bank) - 10) * 16) + (pin))
> 
> Uhm, well, you can make the mapping more compact yes, but I'm not sure
> if I agree that it becomes any better. Isn't it better to simply
> follow the per-port setup that the manual defines? Is there an actual
> problem with having unused GPIOs?

If I'm not mistaken it's unused pins, not unused GPIOs. They waste memory in 
data tables, although by a relatively small amount. Oh, and of course, it's 
not clean ;-)

Speaking of data tables, I'm thinking about simplifying them. The RZ/A1H is a 
good candidate for that, as each pin is handled individually, and several 
registers could be handled to with a small amount of code instead of large 
data tables. It's just a thought for now, I have more urgent tasks to work on.

> Actually, I prefer going in the opposite direction so I would like to
> share the simple version of RZ_PORT_PIN() in a header file like we do
> with RCAR_GP_PIN() in <linux/platform_data/gpio-rcar.h>. This because
> we would like to use the same macro in the GPIO driver and in the
> current PFC code (and potentially more PFCs using the same GPIO
> driver).

What do you need it for in the GPIO driver ?

> Please let me know what you think.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 01/04] ARM: shmobile: r7s72100 GPIO and PINCTRL device nodes

[Magnus Damm]

Hi Laurent,

On Wed, Dec 18, 2013 at 9:40 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Magnus,
>
> On Wednesday 18 December 2013 07:41:57 Magnus Damm wrote:
>> On Wed, Dec 18, 2013 at 1:29 AM, Laurent Pinchart wrote:
>> > On Tuesday 17 December 2013 14:02:42 Magnus Damm wrote:
>> >> From: Magnus Damm <damm@opensource.se>
>> >>
>> >> Add support for r7s72100 PFC and GPIO device nodes port0 -> port11
>> >> and jtagport0.
>> >>
>> >> Signed-off-by: Magnus Damm <damm@opensource.se>
>> >> ---
>> >>
>> >>  arch/arm/boot/dts/r7s72100.dtsi |  154 +++++++++++++++++++++++++++++++++
>> >>  1 file changed, 154 insertions(+)
>> >>
>> >> --- 0001/arch/arm/boot/dts/r7s72100.dtsi
>> >> +++ work/arch/arm/boot/dts/r7s72100.dtsi      2013-11-27
>> >> 16:06:36.000000000 +0900
>
> [snip]
>
>> >> +
>> >> +     port0: gpio@fcfe3100 {
>> >> +             compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
>> >> +             reg = <0xfcfe3100 0x4>, /* PSR */
>> >> +                       <0xfcfe3200 0x2>, /* PPR */
>> >> +                       <0xfcfe3800 0x4>; /* PMSR */
>> >> +             #gpio-cells = <2>;
>> >> +             gpio-controller;
>> >> +             gpio-ranges = <&pfc 0 0 6>;
>> >> +     };
>> >> +
>> >> +     port1: gpio@fcfe3104 {
>> >> +             compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
>> >> +             reg = <0xfcfe3104 0x4>, /* PSR */
>> >> +                       <0xfcfe3204 0x2>, /* PPR */
>> >> +                       <0xfcfe3804 0x4>; /* PMSR */
>> >> +             #gpio-cells = <2>;
>> >> +             gpio-controller;
>> >> +             gpio-ranges = <&pfc 0 16 16>;
>> >
>> > As P0 has 6 pins only this should ideally be
>> >
>> >                 gpio-ranges = <&pfc 0 6 16>;
>> >
>> > Otherwise the PFC driver will expose pins that don't exist. However, that
>> > would require computing the pin numbers in the PFC driver differently, as
>> > we currently just use the bank * 16 + index formula. Given that we only
>> > have three ports with less than 16 pins we could come up with a not
>> > overly complex formula that can be evaluated at compile time. Something
>> > like this should do.
>> >
>> > #define RZ_PORT_PIN(bank, pin) \
>> >
>> >         (bank) < 1 ? (pin) : \
>> >         (bank) < 6 ? 6 + (((bank) - 1) * 16) + (pin)) : \
>> >         (bank) < 10 ? 6 + 11 + 4 * 16 + (((bank) - 6) * 16) + (pin)) : \
>> >         6 + 11 + 8 + 7 * 16 + (((bank) - 10) * 16) + (pin))
>>
>> Uhm, well, you can make the mapping more compact yes, but I'm not sure
>> if I agree that it becomes any better. Isn't it better to simply
>> follow the per-port setup that the manual defines? Is there an actual
>> problem with having unused GPIOs?
>
> If I'm not mistaken it's unused pins, not unused GPIOs. They waste memory in
> data tables, although by a relatively small amount. Oh, and of course, it's
> not clean ;-)

Yes, you are correct about pins vs GPIOs. Regarding how to implement
RZ_PORT_PIN(), I believe the only way not to shoot yourself in the
foot is to keep things simple. I also think that some level of
redundancy is an acceptable tradeoff if it keeps things simpler. So I
suppose cleanliness is a matter of taste. =)

> Speaking of data tables, I'm thinking about simplifying them. The RZ/A1H is a
> good candidate for that, as each pin is handled individually, and several
> registers could be handled to with a small amount of code instead of large
> data tables. It's just a thought for now, I have more urgent tasks to work on.

Incremental patches to improve the state is always nice, thanks.

>> Actually, I prefer going in the opposite direction so I would like to
>> share the simple version of RZ_PORT_PIN() in a header file like we do
>> with RCAR_GP_PIN() in <linux/platform_data/gpio-rcar.h>. This because
>> we would like to use the same macro in the GPIO driver and in the
>> current PFC code (and potentially more PFCs using the same GPIO
>> driver).
>
> What do you need it for in the GPIO driver ?

Well, I thought I needed it but it turns out that I'm wrong. =)

Initially I had the following two in the header file:
+#define RZ_GPIOS_PER_PORT 16
+#define RZ_PORT_PIN(bank, pin) (((bank) * RZ_GPIOS_PER_PORT) + (pin))

RZ_GPIOS_PER_PORT was used in both the GPIO driver and
RZ_PORT_PIN() was used in the PFC driver

On a second though, I don't mind duplicating them.

I do however think your version of the RZ_PORT_PIN() is overly
complex. And that needs to be matched with updated gpio-ranges that
together seem quite error prone to me.

How would you like me to proceed?

Cheers,

/ magnus

[PATCH 01/04] ARM: shmobile: r7s72100 GPIO and PINCTRL device nodes

[Magnus Damm]

Hi Laurent,

On Wed, Dec 18, 2013 at 9:40 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Magnus,
>
> On Wednesday 18 December 2013 07:41:57 Magnus Damm wrote:
>> On Wed, Dec 18, 2013 at 1:29 AM, Laurent Pinchart wrote:
>> > On Tuesday 17 December 2013 14:02:42 Magnus Damm wrote:
>> >> From: Magnus Damm <damm@opensource.se>
>> >>
>> >> Add support for r7s72100 PFC and GPIO device nodes port0 -> port11
>> >> and jtagport0.
>> >>
>> >> Signed-off-by: Magnus Damm <damm@opensource.se>
>> >> ---
>> >>
>> >>  arch/arm/boot/dts/r7s72100.dtsi |  154 +++++++++++++++++++++++++++++++++
>> >>  1 file changed, 154 insertions(+)
>> >>
>> >> --- 0001/arch/arm/boot/dts/r7s72100.dtsi
>> >> +++ work/arch/arm/boot/dts/r7s72100.dtsi      2013-11-27
>> >> 16:06:36.000000000 +0900
>
> [snip]
>
>> >> +
>> >> +     port0: gpio at fcfe3100 {
>> >> +             compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
>> >> +             reg = <0xfcfe3100 0x4>, /* PSR */
>> >> +                       <0xfcfe3200 0x2>, /* PPR */
>> >> +                       <0xfcfe3800 0x4>; /* PMSR */
>> >> +             #gpio-cells = <2>;
>> >> +             gpio-controller;
>> >> +             gpio-ranges = <&pfc 0 0 6>;
>> >> +     };
>> >> +
>> >> +     port1: gpio at fcfe3104 {
>> >> +             compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
>> >> +             reg = <0xfcfe3104 0x4>, /* PSR */
>> >> +                       <0xfcfe3204 0x2>, /* PPR */
>> >> +                       <0xfcfe3804 0x4>; /* PMSR */
>> >> +             #gpio-cells = <2>;
>> >> +             gpio-controller;
>> >> +             gpio-ranges = <&pfc 0 16 16>;
>> >
>> > As P0 has 6 pins only this should ideally be
>> >
>> >                 gpio-ranges = <&pfc 0 6 16>;
>> >
>> > Otherwise the PFC driver will expose pins that don't exist. However, that
>> > would require computing the pin numbers in the PFC driver differently, as
>> > we currently just use the bank * 16 + index formula. Given that we only
>> > have three ports with less than 16 pins we could come up with a not
>> > overly complex formula that can be evaluated at compile time. Something
>> > like this should do.
>> >
>> > #define RZ_PORT_PIN(bank, pin) \
>> >
>> >         (bank) < 1 ? (pin) : \
>> >         (bank) < 6 ? 6 + (((bank) - 1) * 16) + (pin)) : \
>> >         (bank) < 10 ? 6 + 11 + 4 * 16 + (((bank) - 6) * 16) + (pin)) : \
>> >         6 + 11 + 8 + 7 * 16 + (((bank) - 10) * 16) + (pin))
>>
>> Uhm, well, you can make the mapping more compact yes, but I'm not sure
>> if I agree that it becomes any better. Isn't it better to simply
>> follow the per-port setup that the manual defines? Is there an actual
>> problem with having unused GPIOs?
>
> If I'm not mistaken it's unused pins, not unused GPIOs. They waste memory in
> data tables, although by a relatively small amount. Oh, and of course, it's
> not clean ;-)

Yes, you are correct about pins vs GPIOs. Regarding how to implement
RZ_PORT_PIN(), I believe the only way not to shoot yourself in the
foot is to keep things simple. I also think that some level of
redundancy is an acceptable tradeoff if it keeps things simpler. So I
suppose cleanliness is a matter of taste. =)

> Speaking of data tables, I'm thinking about simplifying them. The RZ/A1H is a
> good candidate for that, as each pin is handled individually, and several
> registers could be handled to with a small amount of code instead of large
> data tables. It's just a thought for now, I have more urgent tasks to work on.

Incremental patches to improve the state is always nice, thanks.

>> Actually, I prefer going in the opposite direction so I would like to
>> share the simple version of RZ_PORT_PIN() in a header file like we do
>> with RCAR_GP_PIN() in <linux/platform_data/gpio-rcar.h>. This because
>> we would like to use the same macro in the GPIO driver and in the
>> current PFC code (and potentially more PFCs using the same GPIO
>> driver).
>
> What do you need it for in the GPIO driver ?

Well, I thought I needed it but it turns out that I'm wrong. =)

Initially I had the following two in the header file:
+#define RZ_GPIOS_PER_PORT 16
+#define RZ_PORT_PIN(bank, pin) (((bank) * RZ_GPIOS_PER_PORT) + (pin))

RZ_GPIOS_PER_PORT was used in both the GPIO driver and
RZ_PORT_PIN() was used in the PFC driver

On a second though, I don't mind duplicating them.

I do however think your version of the RZ_PORT_PIN() is overly
complex. And that needs to be matched with updated gpio-ranges that
together seem quite error prone to me.

How would you like me to proceed?

Cheers,

/ magnus

Re: [PATCH 01/04] ARM: shmobile: r7s72100 GPIO and PINCTRL device nodes

[Laurent Pinchart]

Hi Magnus (and Linus, as there's a question for your below),

On Wednesday 18 December 2013 11:07:15 Magnus Damm wrote:
> On Wed, Dec 18, 2013 at 9:40 AM, Laurent Pinchart wrote:
> > On Wednesday 18 December 2013 07:41:57 Magnus Damm wrote:
> >> On Wed, Dec 18, 2013 at 1:29 AM, Laurent Pinchart wrote:
> >> > On Tuesday 17 December 2013 14:02:42 Magnus Damm wrote:
> >> >> From: Magnus Damm <damm@opensource.se>
> >> >> 
> >> >> Add support for r7s72100 PFC and GPIO device nodes port0 -> port11
> >> >> and jtagport0.
> >> >> 
> >> >> Signed-off-by: Magnus Damm <damm@opensource.se>
> >> >> ---
> >> >> 
> >> >>  arch/arm/boot/dts/r7s72100.dtsi |  154 ++++++++++++++++++++++++++++++
> >> >>  1 file changed, 154 insertions(+)
> >> >> 
> >> >> --- 0001/arch/arm/boot/dts/r7s72100.dtsi
> >> >> +++ work/arch/arm/boot/dts/r7s72100.dtsi      2013-11-27
> >> >> 16:06:36.000000000 +0900
> > 
> > [snip]
> > 
> >> >> +
> >> >> +     port0: gpio@fcfe3100 {
> >> >> +             compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
> >> >> +             reg = <0xfcfe3100 0x4>, /* PSR */
> >> >> +                       <0xfcfe3200 0x2>, /* PPR */
> >> >> +                       <0xfcfe3800 0x4>; /* PMSR */
> >> >> +             #gpio-cells = <2>;
> >> >> +             gpio-controller;
> >> >> +             gpio-ranges = <&pfc 0 0 6>;
> >> >> +     };
> >> >> +
> >> >> +     port1: gpio@fcfe3104 {
> >> >> +             compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
> >> >> +             reg = <0xfcfe3104 0x4>, /* PSR */
> >> >> +                       <0xfcfe3204 0x2>, /* PPR */
> >> >> +                       <0xfcfe3804 0x4>; /* PMSR */
> >> >> +             #gpio-cells = <2>;
> >> >> +             gpio-controller;
> >> >> +             gpio-ranges = <&pfc 0 16 16>;
> >> > 
> >> > As P0 has 6 pins only this should ideally be
> >> > 
> >> >                 gpio-ranges = <&pfc 0 6 16>;
> >> > 
> >> > Otherwise the PFC driver will expose pins that don't exist. However,
> >> > that would require computing the pin numbers in the PFC driver
> >> > differently, as we currently just use the bank * 16 + index formula.
> >> > Given that we only have three ports with less than 16 pins we could
> >> > come up with a not overly complex formula that can be evaluated at
> >> > compile time. Something like this should do.
> >> > 
> >> > #define RZ_PORT_PIN(bank, pin) \
> >> > 
> >> >         (bank) < 1 ? (pin) : \
> >> >         (bank) < 6 ? 6 + (((bank) - 1) * 16) + (pin)) : \
> >> >         (bank) < 10 ? 6 + 11 + 4 * 16 + (((bank) - 6) * 16) + (pin)) :
> >> >         \
> >> >         6 + 11 + 8 + 7 * 16 + (((bank) - 10) * 16) + (pin))
> >> 
> >> Uhm, well, you can make the mapping more compact yes, but I'm not sure
> >> if I agree that it becomes any better. Isn't it better to simply
> >> follow the per-port setup that the manual defines? Is there an actual
> >> problem with having unused GPIOs?
> > 
> > If I'm not mistaken it's unused pins, not unused GPIOs. They waste memory
> > in data tables, although by a relatively small amount. Oh, and of course,
> > it's not clean ;-)
> 
> Yes, you are correct about pins vs GPIOs. Regarding how to implement
> RZ_PORT_PIN(), I believe the only way not to shoot yourself in the foot is
> to keep things simple. I also think that some level of redundancy is an
> acceptable tradeoff if it keeps things simpler. So I suppose cleanliness is
> a matter of taste. =)

Absolutely, and there's no universal reason why my cleanliness would be better 
than yours :-)

> > Speaking of data tables, I'm thinking about simplifying them. The RZ/A1H
> > is a good candidate for that, as each pin is handled individually, and
> > several registers could be handled to with a small amount of code instead
> > of large data tables. It's just a thought for now, I have more urgent
> > tasks to work on.
>
> Incremental patches to improve the state is always nice, thanks.

You're welcome.

> >> Actually, I prefer going in the opposite direction so I would like to
> >> share the simple version of RZ_PORT_PIN() in a header file like we do
> >> with RCAR_GP_PIN() in <linux/platform_data/gpio-rcar.h>. This because
> >> we would like to use the same macro in the GPIO driver and in the
> >> current PFC code (and potentially more PFCs using the same GPIO
> >> driver).
> > 
> > What do you need it for in the GPIO driver ?
> 
> Well, I thought I needed it but it turns out that I'm wrong. =)
> 
> Initially I had the following two in the header file:
> +#define RZ_GPIOS_PER_PORT 16
> +#define RZ_PORT_PIN(bank, pin) (((bank) * RZ_GPIOS_PER_PORT) + (pin))
> 
> RZ_GPIOS_PER_PORT was used in both the GPIO driver and
> RZ_PORT_PIN() was used in the PFC driver
> 
> On a second though, I don't mind duplicating them.

I agree here, I don't think we need to share RZ_GPIOS_PER_PORT between the two 
drivers.

> I do however think your version of the RZ_PORT_PIN() is overly complex. And
> that needs to be matched with updated gpio-ranges that together seem quite
> error prone to me.
> 
> How would you like me to proceed?

I have mixed feelings about this, and understand your concern about 
complexity. I usually tend to favor correctness over complexity (without 
reaching the overcomplexity level). In this case I'd like to hear Linus' point 
of view. If he's fine with your version of the code I'll be fine as well. Is 
that OK ?

-- 
Regards,

Laurent Pinchart

[PATCH 01/04] ARM: shmobile: r7s72100 GPIO and PINCTRL device nodes

[Laurent Pinchart]

Hi Magnus (and Linus, as there's a question for your below),

On Wednesday 18 December 2013 11:07:15 Magnus Damm wrote:
> On Wed, Dec 18, 2013 at 9:40 AM, Laurent Pinchart wrote:
> > On Wednesday 18 December 2013 07:41:57 Magnus Damm wrote:
> >> On Wed, Dec 18, 2013 at 1:29 AM, Laurent Pinchart wrote:
> >> > On Tuesday 17 December 2013 14:02:42 Magnus Damm wrote:
> >> >> From: Magnus Damm <damm@opensource.se>
> >> >> 
> >> >> Add support for r7s72100 PFC and GPIO device nodes port0 -> port11
> >> >> and jtagport0.
> >> >> 
> >> >> Signed-off-by: Magnus Damm <damm@opensource.se>
> >> >> ---
> >> >> 
> >> >>  arch/arm/boot/dts/r7s72100.dtsi |  154 ++++++++++++++++++++++++++++++
> >> >>  1 file changed, 154 insertions(+)
> >> >> 
> >> >> --- 0001/arch/arm/boot/dts/r7s72100.dtsi
> >> >> +++ work/arch/arm/boot/dts/r7s72100.dtsi      2013-11-27
> >> >> 16:06:36.000000000 +0900
> > 
> > [snip]
> > 
> >> >> +
> >> >> +     port0: gpio at fcfe3100 {
> >> >> +             compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
> >> >> +             reg = <0xfcfe3100 0x4>, /* PSR */
> >> >> +                       <0xfcfe3200 0x2>, /* PPR */
> >> >> +                       <0xfcfe3800 0x4>; /* PMSR */
> >> >> +             #gpio-cells = <2>;
> >> >> +             gpio-controller;
> >> >> +             gpio-ranges = <&pfc 0 0 6>;
> >> >> +     };
> >> >> +
> >> >> +     port1: gpio at fcfe3104 {
> >> >> +             compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
> >> >> +             reg = <0xfcfe3104 0x4>, /* PSR */
> >> >> +                       <0xfcfe3204 0x2>, /* PPR */
> >> >> +                       <0xfcfe3804 0x4>; /* PMSR */
> >> >> +             #gpio-cells = <2>;
> >> >> +             gpio-controller;
> >> >> +             gpio-ranges = <&pfc 0 16 16>;
> >> > 
> >> > As P0 has 6 pins only this should ideally be
> >> > 
> >> >                 gpio-ranges = <&pfc 0 6 16>;
> >> > 
> >> > Otherwise the PFC driver will expose pins that don't exist. However,
> >> > that would require computing the pin numbers in the PFC driver
> >> > differently, as we currently just use the bank * 16 + index formula.
> >> > Given that we only have three ports with less than 16 pins we could
> >> > come up with a not overly complex formula that can be evaluated at
> >> > compile time. Something like this should do.
> >> > 
> >> > #define RZ_PORT_PIN(bank, pin) \
> >> > 
> >> >         (bank) < 1 ? (pin) : \
> >> >         (bank) < 6 ? 6 + (((bank) - 1) * 16) + (pin)) : \
> >> >         (bank) < 10 ? 6 + 11 + 4 * 16 + (((bank) - 6) * 16) + (pin)) :
> >> >         \
> >> >         6 + 11 + 8 + 7 * 16 + (((bank) - 10) * 16) + (pin))
> >> 
> >> Uhm, well, you can make the mapping more compact yes, but I'm not sure
> >> if I agree that it becomes any better. Isn't it better to simply
> >> follow the per-port setup that the manual defines? Is there an actual
> >> problem with having unused GPIOs?
> > 
> > If I'm not mistaken it's unused pins, not unused GPIOs. They waste memory
> > in data tables, although by a relatively small amount. Oh, and of course,
> > it's not clean ;-)
> 
> Yes, you are correct about pins vs GPIOs. Regarding how to implement
> RZ_PORT_PIN(), I believe the only way not to shoot yourself in the foot is
> to keep things simple. I also think that some level of redundancy is an
> acceptable tradeoff if it keeps things simpler. So I suppose cleanliness is
> a matter of taste. =)

Absolutely, and there's no universal reason why my cleanliness would be better 
than yours :-)

> > Speaking of data tables, I'm thinking about simplifying them. The RZ/A1H
> > is a good candidate for that, as each pin is handled individually, and
> > several registers could be handled to with a small amount of code instead
> > of large data tables. It's just a thought for now, I have more urgent
> > tasks to work on.
>
> Incremental patches to improve the state is always nice, thanks.

You're welcome.

> >> Actually, I prefer going in the opposite direction so I would like to
> >> share the simple version of RZ_PORT_PIN() in a header file like we do
> >> with RCAR_GP_PIN() in <linux/platform_data/gpio-rcar.h>. This because
> >> we would like to use the same macro in the GPIO driver and in the
> >> current PFC code (and potentially more PFCs using the same GPIO
> >> driver).
> > 
> > What do you need it for in the GPIO driver ?
> 
> Well, I thought I needed it but it turns out that I'm wrong. =)
> 
> Initially I had the following two in the header file:
> +#define RZ_GPIOS_PER_PORT 16
> +#define RZ_PORT_PIN(bank, pin) (((bank) * RZ_GPIOS_PER_PORT) + (pin))
> 
> RZ_GPIOS_PER_PORT was used in both the GPIO driver and
> RZ_PORT_PIN() was used in the PFC driver
> 
> On a second though, I don't mind duplicating them.

I agree here, I don't think we need to share RZ_GPIOS_PER_PORT between the two 
drivers.

> I do however think your version of the RZ_PORT_PIN() is overly complex. And
> that needs to be matched with updated gpio-ranges that together seem quite
> error prone to me.
> 
> How would you like me to proceed?

I have mixed feelings about this, and understand your concern about 
complexity. I usually tend to favor correctness over complexity (without 
reaching the overcomplexity level). In this case I'd like to hear Linus' point 
of view. If he's fine with your version of the code I'll be fine as well. Is 
that OK ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH 01/04] ARM: shmobile: r7s72100 GPIO and PINCTRL device nodes

[Magnus Damm]

Hi Laurent,

On Thu, Dec 19, 2013 at 9:15 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Magnus (and Linus, as there's a question for your below),
>
> On Wednesday 18 December 2013 11:07:15 Magnus Damm wrote:
>> On Wed, Dec 18, 2013 at 9:40 AM, Laurent Pinchart wrote:
>> > On Wednesday 18 December 2013 07:41:57 Magnus Damm wrote:
>> >> On Wed, Dec 18, 2013 at 1:29 AM, Laurent Pinchart wrote:
>> >> > On Tuesday 17 December 2013 14:02:42 Magnus Damm wrote:
>> >> >> From: Magnus Damm <damm@opensource.se>
>> >> >>
>> >> >> Add support for r7s72100 PFC and GPIO device nodes port0 -> port11
>> >> >> and jtagport0.
>> >> >>
>> >> >> Signed-off-by: Magnus Damm <damm@opensource.se>
>> >> >> ---
>> >> >>
>> >> >>  arch/arm/boot/dts/r7s72100.dtsi |  154 ++++++++++++++++++++++++++++++
>> >> >>  1 file changed, 154 insertions(+)
>> >> >>
>> >> >> --- 0001/arch/arm/boot/dts/r7s72100.dtsi
>> >> >> +++ work/arch/arm/boot/dts/r7s72100.dtsi      2013-11-27
>> >> >> 16:06:36.000000000 +0900
>> >
>> > [snip]
>> >
>> >> >> +
>> >> >> +     port0: gpio@fcfe3100 {
>> >> >> +             compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
>> >> >> +             reg = <0xfcfe3100 0x4>, /* PSR */
>> >> >> +                       <0xfcfe3200 0x2>, /* PPR */
>> >> >> +                       <0xfcfe3800 0x4>; /* PMSR */
>> >> >> +             #gpio-cells = <2>;
>> >> >> +             gpio-controller;
>> >> >> +             gpio-ranges = <&pfc 0 0 6>;
>> >> >> +     };
>> >> >> +
>> >> >> +     port1: gpio@fcfe3104 {
>> >> >> +             compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
>> >> >> +             reg = <0xfcfe3104 0x4>, /* PSR */
>> >> >> +                       <0xfcfe3204 0x2>, /* PPR */
>> >> >> +                       <0xfcfe3804 0x4>; /* PMSR */
>> >> >> +             #gpio-cells = <2>;
>> >> >> +             gpio-controller;
>> >> >> +             gpio-ranges = <&pfc 0 16 16>;
>> >> >
>> >> > As P0 has 6 pins only this should ideally be
>> >> >
>> >> >                 gpio-ranges = <&pfc 0 6 16>;
>> >> >
>> >> > Otherwise the PFC driver will expose pins that don't exist. However,
>> >> > that would require computing the pin numbers in the PFC driver
>> >> > differently, as we currently just use the bank * 16 + index formula.
>> >> > Given that we only have three ports with less than 16 pins we could
>> >> > come up with a not overly complex formula that can be evaluated at
>> >> > compile time. Something like this should do.
>> >> >
>> >> > #define RZ_PORT_PIN(bank, pin) \
>> >> >
>> >> >         (bank) < 1 ? (pin) : \
>> >> >         (bank) < 6 ? 6 + (((bank) - 1) * 16) + (pin)) : \
>> >> >         (bank) < 10 ? 6 + 11 + 4 * 16 + (((bank) - 6) * 16) + (pin)) :
>> >> >         \
>> >> >         6 + 11 + 8 + 7 * 16 + (((bank) - 10) * 16) + (pin))
>> >>
>> >> Uhm, well, you can make the mapping more compact yes, but I'm not sure
>> >> if I agree that it becomes any better. Isn't it better to simply
>> >> follow the per-port setup that the manual defines? Is there an actual
>> >> problem with having unused GPIOs?
>> >
>> > If I'm not mistaken it's unused pins, not unused GPIOs. They waste memory
>> > in data tables, although by a relatively small amount. Oh, and of course,
>> > it's not clean ;-)
>>
>> Yes, you are correct about pins vs GPIOs. Regarding how to implement
>> RZ_PORT_PIN(), I believe the only way not to shoot yourself in the foot is
>> to keep things simple. I also think that some level of redundancy is an
>> acceptable tradeoff if it keeps things simpler. So I suppose cleanliness is
>> a matter of taste. =)
>
> Absolutely, and there's no universal reason why my cleanliness would be better
> than yours :-)

I think we should count LOC, just to add a rigged metric to my side. =)

>> > Speaking of data tables, I'm thinking about simplifying them. The RZ/A1H
>> > is a good candidate for that, as each pin is handled individually, and
>> > several registers could be handled to with a small amount of code instead
>> > of large data tables. It's just a thought for now, I have more urgent
>> > tasks to work on.
>>
>> Incremental patches to improve the state is always nice, thanks.
>
> You're welcome.

Thanks.

>> >> Actually, I prefer going in the opposite direction so I would like to
>> >> share the simple version of RZ_PORT_PIN() in a header file like we do
>> >> with RCAR_GP_PIN() in <linux/platform_data/gpio-rcar.h>. This because
>> >> we would like to use the same macro in the GPIO driver and in the
>> >> current PFC code (and potentially more PFCs using the same GPIO
>> >> driver).
>> >
>> > What do you need it for in the GPIO driver ?
>>
>> Well, I thought I needed it but it turns out that I'm wrong. =)
>>
>> Initially I had the following two in the header file:
>> +#define RZ_GPIOS_PER_PORT 16
>> +#define RZ_PORT_PIN(bank, pin) (((bank) * RZ_GPIOS_PER_PORT) + (pin))
>>
>> RZ_GPIOS_PER_PORT was used in both the GPIO driver and
>> RZ_PORT_PIN() was used in the PFC driver
>>
>> On a second though, I don't mind duplicating them.
>
> I agree here, I don't think we need to share RZ_GPIOS_PER_PORT between the two
> drivers.

They have separate name spaces for GPIOS and pins anyway, so yes.

>> I do however think your version of the RZ_PORT_PIN() is overly complex. And
>> that needs to be matched with updated gpio-ranges that together seem quite
>> error prone to me.
>>
>> How would you like me to proceed?
>
> I have mixed feelings about this, and understand your concern about
> complexity. I usually tend to favor correctness over complexity (without
> reaching the overcomplexity level). In this case I'd like to hear Linus' point
> of view. If he's fine with your version of the code I'll be fine as well. Is
> that OK ?

Sure, that's fine. I may resend the series as-is and see what happens. =)

/ magnus

[PATCH 01/04] ARM: shmobile: r7s72100 GPIO and PINCTRL device nodes

[Magnus Damm]

Hi Laurent,

On Thu, Dec 19, 2013 at 9:15 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Magnus (and Linus, as there's a question for your below),
>
> On Wednesday 18 December 2013 11:07:15 Magnus Damm wrote:
>> On Wed, Dec 18, 2013 at 9:40 AM, Laurent Pinchart wrote:
>> > On Wednesday 18 December 2013 07:41:57 Magnus Damm wrote:
>> >> On Wed, Dec 18, 2013 at 1:29 AM, Laurent Pinchart wrote:
>> >> > On Tuesday 17 December 2013 14:02:42 Magnus Damm wrote:
>> >> >> From: Magnus Damm <damm@opensource.se>
>> >> >>
>> >> >> Add support for r7s72100 PFC and GPIO device nodes port0 -> port11
>> >> >> and jtagport0.
>> >> >>
>> >> >> Signed-off-by: Magnus Damm <damm@opensource.se>
>> >> >> ---
>> >> >>
>> >> >>  arch/arm/boot/dts/r7s72100.dtsi |  154 ++++++++++++++++++++++++++++++
>> >> >>  1 file changed, 154 insertions(+)
>> >> >>
>> >> >> --- 0001/arch/arm/boot/dts/r7s72100.dtsi
>> >> >> +++ work/arch/arm/boot/dts/r7s72100.dtsi      2013-11-27
>> >> >> 16:06:36.000000000 +0900
>> >
>> > [snip]
>> >
>> >> >> +
>> >> >> +     port0: gpio at fcfe3100 {
>> >> >> +             compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
>> >> >> +             reg = <0xfcfe3100 0x4>, /* PSR */
>> >> >> +                       <0xfcfe3200 0x2>, /* PPR */
>> >> >> +                       <0xfcfe3800 0x4>; /* PMSR */
>> >> >> +             #gpio-cells = <2>;
>> >> >> +             gpio-controller;
>> >> >> +             gpio-ranges = <&pfc 0 0 6>;
>> >> >> +     };
>> >> >> +
>> >> >> +     port1: gpio at fcfe3104 {
>> >> >> +             compatible = "renesas,gpio-r7s72100", "renesas,gpio-rz";
>> >> >> +             reg = <0xfcfe3104 0x4>, /* PSR */
>> >> >> +                       <0xfcfe3204 0x2>, /* PPR */
>> >> >> +                       <0xfcfe3804 0x4>; /* PMSR */
>> >> >> +             #gpio-cells = <2>;
>> >> >> +             gpio-controller;
>> >> >> +             gpio-ranges = <&pfc 0 16 16>;
>> >> >
>> >> > As P0 has 6 pins only this should ideally be
>> >> >
>> >> >                 gpio-ranges = <&pfc 0 6 16>;
>> >> >
>> >> > Otherwise the PFC driver will expose pins that don't exist. However,
>> >> > that would require computing the pin numbers in the PFC driver
>> >> > differently, as we currently just use the bank * 16 + index formula.
>> >> > Given that we only have three ports with less than 16 pins we could
>> >> > come up with a not overly complex formula that can be evaluated at
>> >> > compile time. Something like this should do.
>> >> >
>> >> > #define RZ_PORT_PIN(bank, pin) \
>> >> >
>> >> >         (bank) < 1 ? (pin) : \
>> >> >         (bank) < 6 ? 6 + (((bank) - 1) * 16) + (pin)) : \
>> >> >         (bank) < 10 ? 6 + 11 + 4 * 16 + (((bank) - 6) * 16) + (pin)) :
>> >> >         \
>> >> >         6 + 11 + 8 + 7 * 16 + (((bank) - 10) * 16) + (pin))
>> >>
>> >> Uhm, well, you can make the mapping more compact yes, but I'm not sure
>> >> if I agree that it becomes any better. Isn't it better to simply
>> >> follow the per-port setup that the manual defines? Is there an actual
>> >> problem with having unused GPIOs?
>> >
>> > If I'm not mistaken it's unused pins, not unused GPIOs. They waste memory
>> > in data tables, although by a relatively small amount. Oh, and of course,
>> > it's not clean ;-)
>>
>> Yes, you are correct about pins vs GPIOs. Regarding how to implement
>> RZ_PORT_PIN(), I believe the only way not to shoot yourself in the foot is
>> to keep things simple. I also think that some level of redundancy is an
>> acceptable tradeoff if it keeps things simpler. So I suppose cleanliness is
>> a matter of taste. =)
>
> Absolutely, and there's no universal reason why my cleanliness would be better
> than yours :-)

I think we should count LOC, just to add a rigged metric to my side. =)

>> > Speaking of data tables, I'm thinking about simplifying them. The RZ/A1H
>> > is a good candidate for that, as each pin is handled individually, and
>> > several registers could be handled to with a small amount of code instead
>> > of large data tables. It's just a thought for now, I have more urgent
>> > tasks to work on.
>>
>> Incremental patches to improve the state is always nice, thanks.
>
> You're welcome.

Thanks.

>> >> Actually, I prefer going in the opposite direction so I would like to
>> >> share the simple version of RZ_PORT_PIN() in a header file like we do
>> >> with RCAR_GP_PIN() in <linux/platform_data/gpio-rcar.h>. This because
>> >> we would like to use the same macro in the GPIO driver and in the
>> >> current PFC code (and potentially more PFCs using the same GPIO
>> >> driver).
>> >
>> > What do you need it for in the GPIO driver ?
>>
>> Well, I thought I needed it but it turns out that I'm wrong. =)
>>
>> Initially I had the following two in the header file:
>> +#define RZ_GPIOS_PER_PORT 16
>> +#define RZ_PORT_PIN(bank, pin) (((bank) * RZ_GPIOS_PER_PORT) + (pin))
>>
>> RZ_GPIOS_PER_PORT was used in both the GPIO driver and
>> RZ_PORT_PIN() was used in the PFC driver
>>
>> On a second though, I don't mind duplicating them.
>
> I agree here, I don't think we need to share RZ_GPIOS_PER_PORT between the two
> drivers.

They have separate name spaces for GPIOS and pins anyway, so yes.

>> I do however think your version of the RZ_PORT_PIN() is overly complex. And
>> that needs to be matched with updated gpio-ranges that together seem quite
>> error prone to me.
>>
>> How would you like me to proceed?
>
> I have mixed feelings about this, and understand your concern about
> complexity. I usually tend to favor correctness over complexity (without
> reaching the overcomplexity level). In this case I'd like to hear Linus' point
> of view. If he's fine with your version of the code I'll be fine as well. Is
> that OK ?

Sure, that's fine. I may resend the series as-is and see what happens. =)

/ magnus

Re: [PATCH 01/04] ARM: shmobile: r7s72100 GPIO and PINCTRL device nodes

[Wolfram Sang]

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

On Tue, Dec 17, 2013 at 02:02:42PM +0900, Magnus Damm wrote:
> From: Magnus Damm <damm@opensource.se>
> 
> Add support for r7s72100 PFC and GPIO device nodes port0 -> port11
> and jtagport0.
> 
> Signed-off-by: Magnus Damm <damm@opensource.se>

Got this when applying the patch:

warning: squelched 8 whitespace errors
warning: 13 lines add whitespace errors.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 01/04] ARM: shmobile: r7s72100 GPIO and PINCTRL device nodes

[Wolfram Sang]

On Tue, Dec 17, 2013 at 02:02:42PM +0900, Magnus Damm wrote:
> From: Magnus Damm <damm@opensource.se>
> 
> Add support for r7s72100 PFC and GPIO device nodes port0 -> port11
> and jtagport0.
> 
> Signed-off-by: Magnus Damm <damm@opensource.se>

Got this when applying the patch:

warning: squelched 8 whitespace errors
warning: 13 lines add whitespace errors.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131217/07649155/attachment.sig>

[PATCH 02/04] ARM: shmobile: Genmai SCIF2 PINCTRL configuration

[Magnus Damm]

From: Magnus Damm <damm@opensource.se>

Configure the r7s72100 PINCTRL hardware and select pin function
for the SCIF2 serial console.

Signed-off-by: Magnus Damm <damm@opensource.se>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---

 arch/arm/boot/dts/r7s72100-genmai-reference.dts |   10 ++++++++++
 1 file changed, 10 insertions(+)

--- 0009/arch/arm/boot/dts/r7s72100-genmai-reference.dts
+++ work/arch/arm/boot/dts/r7s72100-genmai-reference.dts	2013-11-27 17:13:43.000000000 +0900
@@ -58,3 +58,13 @@
 	};
 
 };
+
+&pfc {
+	pinctrl-0 = <&scif2_pins>;
+	pinctrl-names = "default";
+
+	scif2_pins: serial2 {
+		renesas,groups = "scif2_txd_p3_0", "scif2_rxd_p3_2";
+		renesas,function = "scif2";
+	};
+};

[PATCH 02/04] ARM: shmobile: Genmai SCIF2 PINCTRL configuration

[Magnus Damm]

From: Magnus Damm <damm@opensource.se>

Configure the r7s72100 PINCTRL hardware and select pin function
for the SCIF2 serial console.

Signed-off-by: Magnus Damm <damm@opensource.se>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---

 arch/arm/boot/dts/r7s72100-genmai-reference.dts |   10 ++++++++++
 1 file changed, 10 insertions(+)

--- 0009/arch/arm/boot/dts/r7s72100-genmai-reference.dts
+++ work/arch/arm/boot/dts/r7s72100-genmai-reference.dts	2013-11-27 17:13:43.000000000 +0900
@@ -58,3 +58,13 @@
 	};
 
 };
+
+&pfc {
+	pinctrl-0 = <&scif2_pins>;
+	pinctrl-names = "default";
+
+	scif2_pins: serial2 {
+		renesas,groups = "scif2_txd_p3_0", "scif2_rxd_p3_2";
+		renesas,function = "scif2";
+	};
+};

[PATCH 03/04] ARM: shmobile: Genmai LED1 and LED2 support

[Magnus Damm]

From: Magnus Damm <damm@opensource.se>

Add support for Genmai board LED1 and LED2 via gpio-leds.

Signed-off-by: Magnus Damm <damm@opensource.se>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---

 arch/arm/boot/dts/r7s72100-genmai-reference.dts |   13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

--- 0001/arch/arm/boot/dts/r7s72100-genmai-reference.dts
+++ work/arch/arm/boot/dts/r7s72100-genmai-reference.dts	2013-11-27 15:49:27.000000000 +0900
@@ -9,7 +9,8 @@
  */
 
 /dts-v1/;
-/include/ "r7s72100.dtsi"
+#include "r7s72100.dtsi"
+#include <dt-bindings/gpio/gpio.h>
 
 / {
 	model = "Genmai";
@@ -28,4 +29,14 @@
 		#address-cells = <1>;
 		#size-cells = <1>;
 	};
+
+	leds {
+		compatible = "gpio-leds";
+		led1 {
+			gpios = <&port4 10 GPIO_ACTIVE_LOW>;
+		};
+		led2 {
+			gpios = <&port4 11 GPIO_ACTIVE_LOW>;
+		};
+	};
 };

[PATCH 03/04] ARM: shmobile: Genmai LED1 and LED2 support

[Magnus Damm]

From: Magnus Damm <damm@opensource.se>

Add support for Genmai board LED1 and LED2 via gpio-leds.

Signed-off-by: Magnus Damm <damm@opensource.se>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---

 arch/arm/boot/dts/r7s72100-genmai-reference.dts |   13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

--- 0001/arch/arm/boot/dts/r7s72100-genmai-reference.dts
+++ work/arch/arm/boot/dts/r7s72100-genmai-reference.dts	2013-11-27 15:49:27.000000000 +0900
@@ -9,7 +9,8 @@
  */
 
 /dts-v1/;
-/include/ "r7s72100.dtsi"
+#include "r7s72100.dtsi"
+#include <dt-bindings/gpio/gpio.h>
 
 / {
 	model = "Genmai";
@@ -28,4 +29,14 @@
 		#address-cells = <1>;
 		#size-cells = <1>;
 	};
+
+	leds {
+		compatible = "gpio-leds";
+		led1 {
+			gpios = <&port4 10 GPIO_ACTIVE_LOW>;
+		};
+		led2 {
+			gpios = <&port4 11 GPIO_ACTIVE_LOW>;
+		};
+	};
 };

[PATCH 04/04 v2] ARM: shmobile: Genmai I2C-over-GPIO support

[Magnus Damm]

From: Magnus Damm <damm@opensource.se>

Add support for the Genmai I2C bus hooked up to P1_5 and P1_4 using
the i2c-gpio driver. On the bus sits a 24c128 EEPRROM.

Signed-off-by: Magnus Damm <damm@opensource.se>
---

 Changes since V1:
 - Use compatible string "renesas,12c128" and set pagesize, thanks Wolfram!
 - Use "flash@50", thanks Sergei!

 arch/arm/boot/dts/r7s72100-genmai-reference.dts |   18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

--- 0011/arch/arm/boot/dts/r7s72100-genmai-reference.dts
+++ work/arch/arm/boot/dts/r7s72100-genmai-reference.dts	2013-12-16 20:08:48.000000000 +0900
@@ -39,6 +39,24 @@
 			gpios = <&port4 11 GPIO_ACTIVE_LOW>;
 		};
 	};
+
+	i2c@0 {
+		compatible = "i2c-gpio";
+		gpios = <&port1 5 GPIO_ACTIVE_HIGH /* sda */
+			 &port1 4 GPIO_ACTIVE_HIGH /* scl */
+			>;
+		i2c-gpio,sda-open-drain;
+		i2c-gpio,scl-open-drain;
+		i2c-gpio,delay-us = <5>;	/* ~100 kHz */
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		flash@50 {
+			compatible = "renesas,24c128";
+			reg = <0x50>;
+			pagesize = <64>;
+		};
+	};
 };
 
 &pfc {

[PATCH 04/04 v2] ARM: shmobile: Genmai I2C-over-GPIO support

[Magnus Damm]

From: Magnus Damm <damm@opensource.se>

Add support for the Genmai I2C bus hooked up to P1_5 and P1_4 using
the i2c-gpio driver. On the bus sits a 24c128 EEPRROM.

Signed-off-by: Magnus Damm <damm@opensource.se>
---

 Changes since V1:
 - Use compatible string "renesas,12c128" and set pagesize, thanks Wolfram!
 - Use "flash at 50", thanks Sergei!

 arch/arm/boot/dts/r7s72100-genmai-reference.dts |   18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

--- 0011/arch/arm/boot/dts/r7s72100-genmai-reference.dts
+++ work/arch/arm/boot/dts/r7s72100-genmai-reference.dts	2013-12-16 20:08:48.000000000 +0900
@@ -39,6 +39,24 @@
 			gpios = <&port4 11 GPIO_ACTIVE_LOW>;
 		};
 	};
+
+	i2c at 0 {
+		compatible = "i2c-gpio";
+		gpios = <&port1 5 GPIO_ACTIVE_HIGH /* sda */
+			 &port1 4 GPIO_ACTIVE_HIGH /* scl */
+			>;
+		i2c-gpio,sda-open-drain;
+		i2c-gpio,scl-open-drain;
+		i2c-gpio,delay-us = <5>;	/* ~100 kHz */
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		flash at 50 {
+			compatible = "renesas,24c128";
+			reg = <0x50>;
+			pagesize = <64>;
+		};
+	};
 };
 
 &pfc {

Re: [PATCH 04/04 v2] ARM: shmobile: Genmai I2C-over-GPIO support

[Wolfram Sang]

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

On Tue, Dec 17, 2013 at 02:03:24PM +0900, Magnus Damm wrote:
> From: Magnus Damm <damm@opensource.se>
> 
> Add support for the Genmai I2C bus hooked up to P1_5 and P1_4 using
> the i2c-gpio driver. On the bus sits a 24c128 EEPRROM.
> 
> Signed-off-by: Magnus Damm <damm@opensource.se>

Acked-by: Wolfram Sang <wsa@the-dreams.de>


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 04/04 v2] ARM: shmobile: Genmai I2C-over-GPIO support

[Wolfram Sang]

On Tue, Dec 17, 2013 at 02:03:24PM +0900, Magnus Damm wrote:
> From: Magnus Damm <damm@opensource.se>
> 
> Add support for the Genmai I2C bus hooked up to P1_5 and P1_4 using
> the i2c-gpio driver. On the bus sits a 24c128 EEPRROM.
> 
> Signed-off-by: Magnus Damm <damm@opensource.se>

Acked-by: Wolfram Sang <wsa@the-dreams.de>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131217/0538f762/attachment-0001.sig>

Re: [PATCH 04/04 v2] ARM: shmobile: Genmai I2C-over-GPIO support

[Simon Horman]

On Tue, Dec 17, 2013 at 05:34:36PM +0100, Wolfram Sang wrote:
> On Tue, Dec 17, 2013 at 02:03:24PM +0900, Magnus Damm wrote:
> > From: Magnus Damm <damm@opensource.se>
> > 
> > Add support for the Genmai I2C bus hooked up to P1_5 and P1_4 using
> > the i2c-gpio driver. On the bus sits a 24c128 EEPRROM.
> > 
> > Signed-off-by: Magnus Damm <damm@opensource.se>
> 
> Acked-by: Wolfram Sang <wsa@the-dreams.de>

Hi Magnus, Hi Wolfram,

could you clarify the status of this patch for me?

I believe it is reverted by "[PATCH V2 4/5] arm: shmobile: genmai: adapt
dts to use native i2c driver".

I'm fine with applying both patches at some point. But perhaps
we can skip some dependencies by just applying a squashed version?

[PATCH 04/04 v2] ARM: shmobile: Genmai I2C-over-GPIO support

[Simon Horman]

On Tue, Dec 17, 2013 at 05:34:36PM +0100, Wolfram Sang wrote:
> On Tue, Dec 17, 2013 at 02:03:24PM +0900, Magnus Damm wrote:
> > From: Magnus Damm <damm@opensource.se>
> > 
> > Add support for the Genmai I2C bus hooked up to P1_5 and P1_4 using
> > the i2c-gpio driver. On the bus sits a 24c128 EEPRROM.
> > 
> > Signed-off-by: Magnus Damm <damm@opensource.se>
> 
> Acked-by: Wolfram Sang <wsa@the-dreams.de>

Hi Magnus, Hi Wolfram,

could you clarify the status of this patch for me?

I believe it is reverted by "[PATCH V2 4/5] arm: shmobile: genmai: adapt
dts to use native i2c driver".

I'm fine with applying both patches at some point. But perhaps
we can skip some dependencies by just applying a squashed version?

Re: [PATCH 04/04 v2] ARM: shmobile: Genmai I2C-over-GPIO support

[Wolfram Sang]

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


> could you clarify the status of this patch for me?
> 
> I believe it is reverted by "[PATCH V2 4/5] arm: shmobile: genmai: adapt
> dts to use native i2c driver".
> 
> I'm fine with applying both patches at some point. But perhaps
> we can skip some dependencies by just applying a squashed version?

If we keep this patch and my one on top of it, we need the PFC series.
If we skip this patch and use my native I2C right from the beginning, we
can skip the PFC series and have i2c support right away. I'd prefer the
latter. I am about to push a branch to my tree that does the latter
these minutes.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 04/04 v2] ARM: shmobile: Genmai I2C-over-GPIO support

[Wolfram Sang]

> could you clarify the status of this patch for me?
> 
> I believe it is reverted by "[PATCH V2 4/5] arm: shmobile: genmai: adapt
> dts to use native i2c driver".
> 
> I'm fine with applying both patches at some point. But perhaps
> we can skip some dependencies by just applying a squashed version?

If we keep this patch and my one on top of it, we need the PFC series.
If we skip this patch and use my native I2C right from the beginning, we
can skip the PFC series and have i2c support right away. I'd prefer the
latter. I am about to push a branch to my tree that does the latter
these minutes.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140108/b64c65e6/attachment.sig>

Re: [PATCH 04/04 v2] ARM: shmobile: Genmai I2C-over-GPIO support

[Simon Horman]

On Wed, Jan 08, 2014 at 12:23:59PM +0100, Wolfram Sang wrote:
> 
> > could you clarify the status of this patch for me?
> > 
> > I believe it is reverted by "[PATCH V2 4/5] arm: shmobile: genmai: adapt
> > dts to use native i2c driver".
> > 
> > I'm fine with applying both patches at some point. But perhaps
> > we can skip some dependencies by just applying a squashed version?
> 
> If we keep this patch and my one on top of it, we need the PFC series.
> If we skip this patch and use my native I2C right from the beginning, we
> can skip the PFC series and have i2c support right away. I'd prefer the
> latter. I am about to push a branch to my tree that does the latter
> these minutes.

I think I prefer the latter too.

[PATCH 04/04 v2] ARM: shmobile: Genmai I2C-over-GPIO support

[Simon Horman]

On Wed, Jan 08, 2014 at 12:23:59PM +0100, Wolfram Sang wrote:
> 
> > could you clarify the status of this patch for me?
> > 
> > I believe it is reverted by "[PATCH V2 4/5] arm: shmobile: genmai: adapt
> > dts to use native i2c driver".
> > 
> > I'm fine with applying both patches at some point. But perhaps
> > we can skip some dependencies by just applying a squashed version?
> 
> If we keep this patch and my one on top of it, we need the PFC series.
> If we skip this patch and use my native I2C right from the beginning, we
> can skip the PFC series and have i2c support right away. I'd prefer the
> latter. I am about to push a branch to my tree that does the latter
> these minutes.

I think I prefer the latter too.

Re: [PATCH 00/04 v2] ARM: shmobile: r7s72100 and Genmai GPIO / PINCTRL V2

[Geert Uytterhoeven]

Hi Magnus,

On Tue, Dec 17, 2013 at 6:02 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
> [PATCH 02/04] ARM: shmobile: Genmai SCIF2 PINCTRL configuration
> [PATCH 03/04] ARM: shmobile: Genmai LED1 and LED2 support

FWIW,  "[PATCH 02/04] ARM: shmobile: Genmai SCIF2 PINCTRL
configuration" appends to a version of r7s72100-genmai-reference.dts
that's 60 lines long:

--- 0009/arch/arm/boot/dts/r7s72100-genmai-reference.dts
+++ work/arch/arm/boot/dts/r7s72100-genmai-reference.dts
2013-11-27 17:13:43.000000000 +0900
@@ -58,3 +58,13 @@
        };

 };
+
+&pfc {
+       pinctrl-0 = <&scif2_pins>;
+       pinctrl-names = "default";
+
+       scif2_pins: serial2 {
+               renesas,groups = "scif2_txd_p3_0", "scif2_rxd_p3_2";
+               renesas,function = "scif2";
+       };
+};

while "[PATCH 03/04] ARM: shmobile: Genmai LED1 and LED2 support"
inserts into a version that has only 31 lines:

@@ -28,4 +29,14 @@
                #address-cells = <1>;
                #size-cells = <1>;
        };
+
+       leds {
+               compatible = "gpio-leds";
+               led1 {
+                       gpios = <&port4 10 GPIO_ACTIVE_LOW>;
+               };
+               led2 {
+                       gpios = <&port4 11 GPIO_ACTIVE_LOW>;
+               };
+       };
 };

Perhaps you reordered or edited them by hand?

Anyway, I managed to apply them by manually,

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

[PATCH 00/04 v2] ARM: shmobile: r7s72100 and Genmai GPIO / PINCTRL V2

[Geert Uytterhoeven]

Hi Magnus,

On Tue, Dec 17, 2013 at 6:02 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
> [PATCH 02/04] ARM: shmobile: Genmai SCIF2 PINCTRL configuration
> [PATCH 03/04] ARM: shmobile: Genmai LED1 and LED2 support

FWIW,  "[PATCH 02/04] ARM: shmobile: Genmai SCIF2 PINCTRL
configuration" appends to a version of r7s72100-genmai-reference.dts
that's 60 lines long:

--- 0009/arch/arm/boot/dts/r7s72100-genmai-reference.dts
+++ work/arch/arm/boot/dts/r7s72100-genmai-reference.dts
2013-11-27 17:13:43.000000000 +0900
@@ -58,3 +58,13 @@
        };

 };
+
+&pfc {
+       pinctrl-0 = <&scif2_pins>;
+       pinctrl-names = "default";
+
+       scif2_pins: serial2 {
+               renesas,groups = "scif2_txd_p3_0", "scif2_rxd_p3_2";
+               renesas,function = "scif2";
+       };
+};

while "[PATCH 03/04] ARM: shmobile: Genmai LED1 and LED2 support"
inserts into a version that has only 31 lines:

@@ -28,4 +29,14 @@
                #address-cells = <1>;
                #size-cells = <1>;
        };
+
+       leds {
+               compatible = "gpio-leds";
+               led1 {
+                       gpios = <&port4 10 GPIO_ACTIVE_LOW>;
+               };
+               led2 {
+                       gpios = <&port4 11 GPIO_ACTIVE_LOW>;
+               };
+       };
 };

Perhaps you reordered or edited them by hand?

Anyway, I managed to apply them by manually,

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

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

Re: [PATCH 00/04 v2] ARM: shmobile: r7s72100 and Genmai GPIO / PINCTRL V2

[Simon Horman]

On Tue, Dec 17, 2013 at 02:02:32PM +0900, Magnus Damm wrote:
> ARM: shmobile: r7s72100 and Genmai GPIO / PINCTRL V2
> 
> [PATCH 01/04] ARM: shmobile: r7s72100 GPIO and PINCTRL device nodes
> [PATCH 02/04] ARM: shmobile: Genmai SCIF2 PINCTRL configuration
> [PATCH 03/04] ARM: shmobile: Genmai LED1 and LED2 support
> [PATCH 04/04 v2] ARM: shmobile: Genmai I2C-over-GPIO support
>
> Add various device nodes to the r7s72100 SoC and the Genmai board,
> nothing special just basic support for LEDs and I2C-over-GPIO.
> 
> Changes since V1:
>  - 2/4 and 3/4 now includes acks, thanks Laurent!
>  - 4/4 has been updated, thanks to Sergei and Wolfram!
> 
> Signed-off-by: Magnus Damm <damm@opensource.se>

Hi Magnus,

what is the status of this series?

[PATCH 00/04 v2] ARM: shmobile: r7s72100 and Genmai GPIO / PINCTRL V2

[Simon Horman]

On Tue, Dec 17, 2013 at 02:02:32PM +0900, Magnus Damm wrote:
> ARM: shmobile: r7s72100 and Genmai GPIO / PINCTRL V2
> 
> [PATCH 01/04] ARM: shmobile: r7s72100 GPIO and PINCTRL device nodes
> [PATCH 02/04] ARM: shmobile: Genmai SCIF2 PINCTRL configuration
> [PATCH 03/04] ARM: shmobile: Genmai LED1 and LED2 support
> [PATCH 04/04 v2] ARM: shmobile: Genmai I2C-over-GPIO support
>
> Add various device nodes to the r7s72100 SoC and the Genmai board,
> nothing special just basic support for LEDs and I2C-over-GPIO.
> 
> Changes since V1:
>  - 2/4 and 3/4 now includes acks, thanks Laurent!
>  - 4/4 has been updated, thanks to Sergei and Wolfram!
> 
> Signed-off-by: Magnus Damm <damm@opensource.se>

Hi Magnus,

what is the status of this series?

Re: [PATCH 00/04 v2] ARM: shmobile: r7s72100 and Genmai GPIO / PINCTRL V2

[Simon Horman]

On Tue, Dec 17, 2013 at 02:02:32PM +0900, Magnus Damm wrote:
> ARM: shmobile: r7s72100 and Genmai GPIO / PINCTRL V2
> 
> [PATCH 01/04] ARM: shmobile: r7s72100 GPIO and PINCTRL device nodes
> [PATCH 02/04] ARM: shmobile: Genmai SCIF2 PINCTRL configuration
> [PATCH 03/04] ARM: shmobile: Genmai LED1 and LED2 support
> [PATCH 04/04 v2] ARM: shmobile: Genmai I2C-over-GPIO support
> 
> Add various device nodes to the r7s72100 SoC and the Genmai board,
> nothing special just basic support for LEDs and I2C-over-GPIO.

If the series is v2 then it would help me greatly if you included
v2 in the subject all all patches. The reason is that it helps to
group the patches visually in patchwork where cover emails and threading
do not exist.

You can make it clear that there were no changes by adding something
like the following to the changelog. But I generally assume there
were no changes in the absence of information to the contrary in the
changelog.

---
v2
* No change

[PATCH 00/04 v2] ARM: shmobile: r7s72100 and Genmai GPIO / PINCTRL V2

[Simon Horman]

On Tue, Dec 17, 2013 at 02:02:32PM +0900, Magnus Damm wrote:
> ARM: shmobile: r7s72100 and Genmai GPIO / PINCTRL V2
> 
> [PATCH 01/04] ARM: shmobile: r7s72100 GPIO and PINCTRL device nodes
> [PATCH 02/04] ARM: shmobile: Genmai SCIF2 PINCTRL configuration
> [PATCH 03/04] ARM: shmobile: Genmai LED1 and LED2 support
> [PATCH 04/04 v2] ARM: shmobile: Genmai I2C-over-GPIO support
> 
> Add various device nodes to the r7s72100 SoC and the Genmai board,
> nothing special just basic support for LEDs and I2C-over-GPIO.

If the series is v2 then it would help me greatly if you included
v2 in the subject all all patches. The reason is that it helps to
group the patches visually in patchwork where cover emails and threading
do not exist.

You can make it clear that there were no changes by adding something
like the following to the changelog. But I generally assume there
were no changes in the absence of information to the contrary in the
changelog.

---
v2
* No change