[PATCH v4 0/8] dts: sun8i-h3: complete UART I2C definitions for H3

[PATCH v4 0/8] dts: sun8i-h3: complete UART I2C definitions for H3

[jorik]

From: Jorik Jonker <jorik@kippendief.biz>

Of course, minutes after sending you notice a huge copy/paste error in
the submitted patch. Mae culpa, and here is a new version.

This is my fourth attempt to complete the UART/I2C definitions on H3.
Changes since v3:
 * I was sloppy when I copy/pasted the i2c2 peripheral, so it had to
   share clocks/resets and interrupts with i2c1. This is fixed now.

Changes since v2:
 * I have moved the pinmux/peripheral associations to the H3 DTSI file,
   including already existing associations for UART0 and UART1.
 * I have left out _a and @0 prefixes where there is only one option
 * I have added i2c2, since it is there (although not exposed on my test
   board)

Best,

Jorik


Jorik Jonker (8):
  dts: sun8i-h3: drop _a and address suffix from uart0 pinmux
  dts: sun8i-h3: clarify uart1 pinmux definition name
  dts: sun8i-h3: move uart0 pinmux/peripheral assocation to DSTI
  dts: sun8i-h3: move uart1 pinmux/peripheral assocation to DSTI
  dts: sun8i-h3: add pinmux definitions for UART2-3
  dts: sun8i-h3: associate pinmux/peripherals for UART2-3
  dts: sun8i-h3: add pinmux definitions for I2C0-2
  dts: sun8i-h3: add I2C0-2 peripherals to H3 SOC

 arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts |  4 --
 arch/arm/boot/dts/sun8i-h3-orangepi-2.dts       |  2 -
 arch/arm/boot/dts/sun8i-h3-orangepi-one.dts     |  2 -
 arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts      |  2 -
 arch/arm/boot/dts/sun8i-h3.dtsi                 | 86 ++++++++++++++++++++++++-
 5 files changed, 84 insertions(+), 12 deletions(-)

-- 
2.7.4

[PATCH v4 1/8] dts: sun8i-h3: drop _a and address suffix from uart0 pinmux

[jorik]

From: Jorik Jonker <jorik@kippendief.biz>

These are the only possible pins for UART0 on H3

Signed-off-by: Jorik Jonker <jorik@kippendief.biz>
---
 arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts | 2 +-
 arch/arm/boot/dts/sun8i-h3-orangepi-2.dts       | 2 +-
 arch/arm/boot/dts/sun8i-h3-orangepi-one.dts     | 2 +-
 arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts      | 2 +-
 arch/arm/boot/dts/sun8i-h3.dtsi                 | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts b/arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts
index f3b1d5f..3779280 100644
--- a/arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts
+++ b/arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts
@@ -179,7 +179,7 @@
 
 &uart0 {
 	pinctrl-names = "default";
-	pinctrl-0 = <&uart0_pins_a>;
+	pinctrl-0 = <&uart0_pins>;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-2.dts b/arch/arm/boot/dts/sun8i-h3-orangepi-2.dts
index f93f5d1..6cb29f3 100644
--- a/arch/arm/boot/dts/sun8i-h3-orangepi-2.dts
+++ b/arch/arm/boot/dts/sun8i-h3-orangepi-2.dts
@@ -172,7 +172,7 @@
 
 &uart0 {
 	pinctrl-names = "default";
-	pinctrl-0 = <&uart0_pins_a>;
+	pinctrl-0 = <&uart0_pins>;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-one.dts b/arch/arm/boot/dts/sun8i-h3-orangepi-one.dts
index 0adf932..a1423cc 100644
--- a/arch/arm/boot/dts/sun8i-h3-orangepi-one.dts
+++ b/arch/arm/boot/dts/sun8i-h3-orangepi-one.dts
@@ -135,7 +135,7 @@
 
 &uart0 {
 	pinctrl-names = "default";
-	pinctrl-0 = <&uart0_pins_a>;
+	pinctrl-0 = <&uart0_pins>;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
index daf50b9..772239e 100644
--- a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
+++ b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
@@ -157,7 +157,7 @@
 
 &uart0 {
 	pinctrl-names = "default";
-	pinctrl-0 = <&uart0_pins_a>;
+	pinctrl-0 = <&uart0_pins>;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
index fdf9fdb..ee10004 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -360,7 +360,7 @@
 				allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
 			};
 
-			uart0_pins_a: uart0@0 {
+			uart0_pins: uart0 {
 				allwinner,pins = "PA4", "PA5";
 				allwinner,function = "uart0";
 				allwinner,drive = <SUN4I_PINCTRL_10_MA>;
-- 
2.7.4

Re: [PATCH v4 1/8] dts: sun8i-h3: drop _a and address suffix from uart0 pinmux

[Maxime Ripard]

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

Hi,

On Wed, Sep 07, 2016 at 09:58:54AM +0200, jorik@kippendief.biz wrote:
> From: Jorik Jonker <jorik@kippendief.biz>
> 
> These are the only possible pins for UART0 on H3

That's not true, you can also mux it on PF2 / PF4.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

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

[PATCH v4 2/8] dts: sun8i-h3: clarify uart1 pinmux definition name

[jorik]

From: Jorik Jonker <jorik@kippendief.biz>

Signed-off-by: Jorik Jonker <jorik@kippendief.biz>
---
 arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts | 2 +-
 arch/arm/boot/dts/sun8i-h3.dtsi                 | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts b/arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts
index 3779280..45f623b 100644
--- a/arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts
+++ b/arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts
@@ -185,7 +185,7 @@
 
 &uart1 {
 	pinctrl-names = "default";
-	pinctrl-0 = <&uart1_pins_a>;
+	pinctrl-0 = <&uart1_rts_cts_pins>;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
index ee10004..933d42c 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -367,7 +367,7 @@
 				allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
 			};
 
-			uart1_pins_a: uart1@0 {
+			uart1_rts_cts_pins: uart1_rts_cts {
 				allwinner,pins = "PG6", "PG7", "PG8", "PG9";
 				allwinner,function = "uart1";
 				allwinner,drive = <SUN4I_PINCTRL_10_MA>;
-- 
2.7.4

Re: [PATCH v4 2/8] dts: sun8i-h3: clarify uart1 pinmux definition name

[Maxime Ripard]

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

Hi,

On Wed, Sep 07, 2016 at 09:58:55AM +0200, jorik@kippendief.biz wrote:
> From: Jorik Jonker <jorik@kippendief.biz>
> 
> Signed-off-by: Jorik Jonker <jorik@kippendief.biz>

Commit log?

> ---
>  arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts | 2 +-
>  arch/arm/boot/dts/sun8i-h3.dtsi                 | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts b/arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts
> index 3779280..45f623b 100644
> --- a/arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts
> +++ b/arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts
> @@ -185,7 +185,7 @@
>  
>  &uart1 {
>  	pinctrl-names = "default";
> -	pinctrl-0 = <&uart1_pins_a>;
> +	pinctrl-0 = <&uart1_rts_cts_pins>;
>  	status = "okay";
>  };
>  
> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
> index ee10004..933d42c 100644
> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> @@ -367,7 +367,7 @@
>  				allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
>  			};
>  
> -			uart1_pins_a: uart1@0 {
> +			uart1_rts_cts_pins: uart1_rts_cts {
>  				allwinner,pins = "PG6", "PG7", "PG8", "PG9";

I'd prefer to have one pinctrl node for RX/TX, and one for
RTS/CTS. That way, we don't have to duplicate the nodes for the boards
that want to enable only RX/TX

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

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

[PATCH v4 3/8] dts: sun8i-h3: move uart0 pinmux/peripheral assocation to DSTI

[jorik]

From: Jorik Jonker <jorik@kippendief.biz>

This is the only sensible pinmux/peripheral association for uart0, so I have
moved it from the board specific DTS to the DTSI. It is still up to the boards
to enable it, though.

Signed-off-by: Jorik Jonker <jorik@kippendief.biz>
---
 arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts | 2 --
 arch/arm/boot/dts/sun8i-h3-orangepi-2.dts       | 2 --
 arch/arm/boot/dts/sun8i-h3-orangepi-one.dts     | 2 --
 arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts      | 2 --
 arch/arm/boot/dts/sun8i-h3.dtsi                 | 2 ++
 5 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts b/arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts
index 45f623b..c874270 100644
--- a/arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts
+++ b/arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts
@@ -178,8 +178,6 @@
 };
 
 &uart0 {
-	pinctrl-names = "default";
-	pinctrl-0 = <&uart0_pins>;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-2.dts b/arch/arm/boot/dts/sun8i-h3-orangepi-2.dts
index 6cb29f3..c9c0014 100644
--- a/arch/arm/boot/dts/sun8i-h3-orangepi-2.dts
+++ b/arch/arm/boot/dts/sun8i-h3-orangepi-2.dts
@@ -171,8 +171,6 @@
 };
 
 &uart0 {
-	pinctrl-names = "default";
-	pinctrl-0 = <&uart0_pins>;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-one.dts b/arch/arm/boot/dts/sun8i-h3-orangepi-one.dts
index a1423cc..c830ff9 100644
--- a/arch/arm/boot/dts/sun8i-h3-orangepi-one.dts
+++ b/arch/arm/boot/dts/sun8i-h3-orangepi-one.dts
@@ -134,8 +134,6 @@
 };
 
 &uart0 {
-	pinctrl-names = "default";
-	pinctrl-0 = <&uart0_pins>;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
index 772239e..ae87061 100644
--- a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
+++ b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
@@ -156,8 +156,6 @@
 };
 
 &uart0 {
-	pinctrl-names = "default";
-	pinctrl-0 = <&uart0_pins>;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
index 933d42c..6635f3d 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -399,6 +399,8 @@
 			resets = <&ccu RST_BUS_UART0>;
 			dmas = <&dma 6>, <&dma 6>;
 			dma-names = "rx", "tx";
+			pinctrl-names = "default";
+			pinctrl-0 = <&uart0_pins>;
 			status = "disabled";
 		};
 
-- 
2.7.4

Re: [PATCH v4 3/8] dts: sun8i-h3: move uart0 pinmux/peripheral assocation to DSTI

[Maxime Ripard]

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

On Wed, Sep 07, 2016 at 09:58:56AM +0200, jorik@kippendief.biz wrote:
> From: Jorik Jonker <jorik@kippendief.biz>
> 
> This is the only sensible pinmux/peripheral association for uart0, so I have
> moved it from the board specific DTS to the DTSI. It is still up to the boards
> to enable it, though.
> 
> Signed-off-by: Jorik Jonker <jorik@kippendief.biz>

There's really several options for the UART0 to be muxed, so it's
better if we leave that to the board entirely.

That's useful only for pins where you truely have only a single
option in the entire SoC.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

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

[PATCH v4 4/8] dts: sun8i-h3: move uart1 pinmux/peripheral assocation to DSTI

[jorik]

From: Jorik Jonker <jorik@kippendief.biz>

Users using this UART without RTS/CTS should override the association in
their board specific DTS. All (1) board using this UART uses RTS/CTS, so
this breaks nothing.

Signed-off-by: Jorik Jonker <jorik@kippendief.biz>
---
 arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts | 2 --
 arch/arm/boot/dts/sun8i-h3.dtsi                 | 2 ++
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts b/arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts
index c874270..4c86e45 100644
--- a/arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts
+++ b/arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts
@@ -182,8 +182,6 @@
 };
 
 &uart1 {
-	pinctrl-names = "default";
-	pinctrl-0 = <&uart1_rts_cts_pins>;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
index 6635f3d..685755c 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -414,6 +414,8 @@
 			resets = <&ccu RST_BUS_UART1>;
 			dmas = <&dma 7>, <&dma 7>;
 			dma-names = "rx", "tx";
+			pinctrl-names = "default";
+			pinctrl-0 = <&uart1_rts_cts_pins>;
 			status = "disabled";
 		};
 
-- 
2.7.4

Re: [PATCH v4 4/8] dts: sun8i-h3: move uart1 pinmux/peripheral assocation to DSTI

[Chen-Yu Tsai]

On Wed, Sep 7, 2016 at 3:58 PM,  <jorik@kippendief.biz> wrote:
> From: Jorik Jonker <jorik@kippendief.biz>
>
> Users using this UART without RTS/CTS should override the association in
> their board specific DTS. All (1) board using this UART uses RTS/CTS, so
> this breaks nothing.

I'd prefer we only do this for peripherals that have absolutely one
possible setting, like RSB, or I2C with only one group of pins.
UARTs with 2/4/8 pin configurations don't fall in this category.

ChenYu

Re: [PATCH v4 4/8] dts: sun8i-h3: move uart1 pinmux/peripheral assocation to DSTI

[Maxime Ripard]

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

On Wed, Sep 07, 2016 at 09:58:57AM +0200, jorik@kippendief.biz wrote:
> From: Jorik Jonker <jorik@kippendief.biz>
> 
> Users using this UART without RTS/CTS should override the association in
> their board specific DTS. All (1) board using this UART uses RTS/CTS, so
> this breaks nothing.

Using RTS / CTS is very rare among the boards. Forcing it down the
throat of every user doesn't seem like the right thing to do.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

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

Re: [PATCH v4 4/8] dts: sun8i-h3: move uart1 pinmux/peripheral assocation to DSTI

[Jorik Jonker]

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

Maxime, Chen-Yu: thanks for taking the effort to go through my patches 
again!

On Thu, Sep 08, 2016 at 08:23:17AM +0200, Maxime Ripard wrote:
>On Wed, Sep 07, 2016 at 09:58:57AM +0200, jorik@kippendief.biz wrote:
>> From: Jorik Jonker <jorik@kippendief.biz>
>>
>> Users using this UART without RTS/CTS should override the association in
>> their board specific DTS. All (1) board using this UART uses RTS/CTS, so
>> this breaks nothing.
>
>Using RTS / CTS is very rare among the boards. Forcing it down the
>throat of every user doesn't seem like the right thing to do.

So, I'm going for a v5, with these changes:
- rename uart0_pins to uart0_pa_pins (as there could be a pf)
- associate uart0_pa_pins with uart0 on all H3 board DTS files
- put rts/cts in seperate pinmux sets for uart1 (2,3: see below)
- associate rx/tx for uart1-3 in H3 DTSI (this is the only option)
- associate UART1 rts/cts as pinctrl-1 in sun8i-h3-bananapi-m2-plus
  (to prevent breakage for existing users)

I am a bit in doubt if I should include pinmux definitions for the 
following things, as Chen-Yu said to only include stuff that is actually 
used in a board:

- uart0_pf_pins, since there is no board using it
- uart{2,3}_rts_cts, as I agree RTS/CTS is a bit exotic

Maxime/Chen-Yu: what do you think of this?

Best,
Jorik

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

Re: [PATCH v4 4/8] dts: sun8i-h3: move uart1 pinmux/peripheral assocation to DSTI

[Maxime Ripard]

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

On Thu, Sep 08, 2016 at 10:02:13AM +0200, Jorik Jonker wrote:
> Maxime, Chen-Yu: thanks for taking the effort to go through my patches
> again!
> 
> On Thu, Sep 08, 2016 at 08:23:17AM +0200, Maxime Ripard wrote:
> >On Wed, Sep 07, 2016 at 09:58:57AM +0200, jorik@kippendief.biz wrote:
> >>From: Jorik Jonker <jorik@kippendief.biz>
> >>
> >>Users using this UART without RTS/CTS should override the association in
> >>their board specific DTS. All (1) board using this UART uses RTS/CTS, so
> >>this breaks nothing.
> >
> >Using RTS / CTS is very rare among the boards. Forcing it down the
> >throat of every user doesn't seem like the right thing to do.
> 
> So, I'm going for a v5, with these changes:
> - rename uart0_pins to uart0_pa_pins (as there could be a pf)
> - associate uart0_pa_pins with uart0 on all H3 board DTS files

Please don't. We use that naming scheme everywhere else. Plus, nothing
prevents any one from using one PF pin and one PA pin.

> - put rts/cts in seperate pinmux sets for uart1 (2,3: see below)
> - associate rx/tx for uart1-3 in H3 DTSI (this is the only option)

I'm still a bit skeptical about this. This wouldn't be in any way
consistant. I prefer to have something consistant and a bit duplicated
over something without any duplication but that confuses everyone
about what should be placed where.

> - associate UART1 rts/cts as pinctrl-1 in sun8i-h3-bananapi-m2-plus
>  (to prevent breakage for existing users)

You can also set it in pinctrl-0.

> I am a bit in doubt if I should include pinmux definitions for the following
> things, as Chen-Yu said to only include stuff that is actually used in a
> board:
> 
> - uart0_pf_pins, since there is no board using it
> - uart{2,3}_rts_cts, as I agree RTS/CTS is a bit exotic

Don't

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

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

Re: [PATCH v4 4/8] dts: sun8i-h3: move uart1 pinmux/peripheral assocation to DSTI

[Jorik Jonker]

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

On Thu, Sep 08, 2016 at 11:01:53AM +0200, Maxime Ripard wrote:
>On Thu, Sep 08, 2016 at 10:02:13AM +0200, Jorik Jonker wrote:
>> So, I'm going for a v5, with these changes:
>> - rename uart0_pins to uart0_pa_pins (as there could be a pf)
>> - associate uart0_pa_pins with uart0 on all H3 board DTS files
>
>Please don't. We use that naming scheme everywhere else. Plus, nothing
>prevents any one from using one PF pin and one PA pin.

OK, I will leave uart0 untouched, that's a good point.

>> - put rts/cts in seperate pinmux sets for uart1 (2,3: see below)
>> - associate rx/tx for uart1-3 in H3 DTSI (this is the only option)
>
>I'm still a bit skeptical about this. This wouldn't be in any way
>consistant. I prefer to have something consistant and a bit duplicated
>over something without any duplication but that confuses everyone
>about what should be placed where.
>
>> - associate UART1 rts/cts as pinctrl-1 in sun8i-h3-bananapi-m2-plus
>>  (to prevent breakage for existing users)
>
>You can also set it in pinctrl-0.

OK, sounds reasonable, but also a bit contradictive. One the one hand 
you prefer consistency (so, let uart2-3 follow uart1 and include rts/cts 
in them), on the other hand the common case over the rare (so split off 
rts/cts). What should I do with uarts2-3 and should I do that to uart1 
too?

Moreover, Chen-Yu prefers to drop _a and @0 when they are redundant, 
which does not appear to be the convention, looking at existing 
sun*dsti. What's your opinion on this?

Best,
Jorik

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

Re: [PATCH v4 4/8] dts: sun8i-h3: move uart1 pinmux/peripheral assocation to DSTI

[Maxime Ripard]

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

On Thu, Sep 08, 2016 at 11:51:09AM +0200, Jorik Jonker wrote:
> >>- put rts/cts in seperate pinmux sets for uart1 (2,3: see below)
> >>- associate rx/tx for uart1-3 in H3 DTSI (this is the only option)
> >
> >I'm still a bit skeptical about this. This wouldn't be in any way
> >consistant. I prefer to have something consistant and a bit duplicated
> >over something without any duplication but that confuses everyone
> >about what should be placed where.
> >
> >>- associate UART1 rts/cts as pinctrl-1 in sun8i-h3-bananapi-m2-plus
> >> (to prevent breakage for existing users)
> >
> >You can also set it in pinctrl-0.
> 
> OK, sounds reasonable, but also a bit contradictive. One the one hand you
> prefer consistency (so, let uart2-3 follow uart1 and include rts/cts in
> them)

Hmm, I never said that, quite the opposite actually. Any board might
use either just RX/TX, or RX/TX and RTS/CTS. I don't see why we should
enable RTS/CTS on any board by default.

> , on the other hand the common case over the rare (so split off
> rts/cts). What should I do with uarts2-3 and should I do that to
> uart1 too?

You do the exact same thing in both cases.

My point was that you could just do:

pinctrl-0 = <&uart0_pins_a>, <&uart0_rts_cts_pins_a>;
pinctrl-names = "default";

instead of

pinctrl-0 = <&uart0_pins_a>;
pinctrl-1 = <&uart0_rts_cts_pins_a>;
pinctrl-names = "default", "default";

Since they are the exact same pin state.

> Moreover, Chen-Yu prefers to drop _a and @0 when they are redundant, which
> does not appear to be the convention, looking at existing sun*dsti. What's
> your opinion on this?

AFAIK, he wanted to remove them when they're not relevant (ie, only
one pin state possible).

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

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

[PATCH v4 5/8] dts: sun8i-h3: add pinmux definitions for UART2-3

[jorik]

From: Jorik Jonker <jorik@kippendief.biz>

These are the pinmux definitions for uarts2-3 on H3. These are the only
pins for these UARTS, but one could choose to use reduce them to only
RX/TX.

Signed-off-by: Jorik Jonker <jorik@kippendief.biz>
---
 arch/arm/boot/dts/sun8i-h3.dtsi | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
index 685755c..5cde54c 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -373,6 +373,20 @@
 				allwinner,drive = <SUN4I_PINCTRL_10_MA>;
 				allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
 			};
+
+			uart2_rts_cts_pins: uart2_rts_cts {
+				allwinner,pins = "PA0", "PA1", "PA2", "PA3";
+				allwinner,function = "uart2";
+				allwinner,drive = <SUN4I_PINCTRL_10_MA>;
+				allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
+			};
+
+			uart3_rts_cts_pins: uart3_rts_cts {
+				allwinner,pins = "PA13", "PA14", "PA15", "PA16";
+				allwinner,function = "uart3";
+				allwinner,drive = <SUN4I_PINCTRL_10_MA>;
+				allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
+			};
 		};
 
 		timer@01c20c00 {
-- 
2.7.4

[PATCH v4 6/8] dts: sun8i-h3: associate pinmux/peripherals for UART2-3

[jorik]

From: Jorik Jonker <jorik@kippendief.biz>

Although these peripherals are disabled, they are associated to the
pinmux definitions. Again, these are the full pinmux defs which include
RTS/CTS. One could narrow this down to RX/TX only in a board specific
DTS.

Signed-off-by: Jorik Jonker <jorik@kippendief.biz>
---
 arch/arm/boot/dts/sun8i-h3.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
index 5cde54c..e9528d1 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -443,6 +443,8 @@
 			resets = <&ccu RST_BUS_UART2>;
 			dmas = <&dma 8>, <&dma 8>;
 			dma-names = "rx", "tx";
+			pinctrl-names = "default";
+			pinctrl-0 = <&uart2_rts_cts_pins>;
 			status = "disabled";
 		};
 
@@ -456,6 +458,8 @@
 			resets = <&ccu RST_BUS_UART3>;
 			dmas = <&dma 9>, <&dma 9>;
 			dma-names = "rx", "tx";
+			pinctrl-names = "default";
+			pinctrl-0 = <&uart3_rts_cts_pins>;
 			status = "disabled";
 		};
 
-- 
2.7.4

[PATCH v4 7/8] dts: sun8i-h3: add pinmux definitions for I2C0-2

[jorik]

From: Jorik Jonker <jorik@kippendief.biz>

These are the only possible pins for these peripherals.

Signed-off-by: Jorik Jonker <jorik@kippendief.biz>
---
 arch/arm/boot/dts/sun8i-h3.dtsi | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
index e9528d1..937096a 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -327,6 +327,27 @@
 			interrupt-controller;
 			#interrupt-cells = <3>;
 
+			i2c0_pins: i2c0 {
+				allwinner,pins = "PA11", "PA12";
+				allwinner,function = "i2c0";
+				allwinner,drive = <SUN4I_PINCTRL_10_MA>;
+				allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
+			};
+
+			i2c1_pins: i2c1 {
+				allwinner,pins = "PA18", "PA19";
+				allwinner,function = "i2c1";
+				allwinner,drive = <SUN4I_PINCTRL_10_MA>;
+				allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
+			};
+
+			i2c2_pins: i2c2 {
+				allwinner,pins = "PE12", "PE13";
+				allwinner,function = "i2c2";
+				allwinner,drive = <SUN4I_PINCTRL_10_MA>;
+				allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
+			};
+
 			mmc0_pins_a: mmc0@0 {
 				allwinner,pins = "PF0", "PF1", "PF2", "PF3",
 						 "PF4", "PF5";
-- 
2.7.4

[PATCH v4 8/8] dts: sun8i-h3: add I2C0-2 peripherals to H3 SOC

[jorik]

From: Jorik Jonker <jorik@kippendief.biz>

These peripherals have only possible set of pins, so I have associated
the peripherals with those pinmux definitions.

Signed-off-by: Jorik Jonker <jorik@kippendief.biz>
---
 arch/arm/boot/dts/sun8i-h3.dtsi | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
index 937096a..342183a 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -484,6 +484,45 @@
 			status = "disabled";
 		};
 
+		i2c0: i2c@01c2ac00 {
+			compatible = "allwinner,sun6i-a31-i2c";
+			reg = <0x01c2ac00 0x400>;
+			interrupts = <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&ccu CLK_BUS_I2C0>;
+			resets = <&ccu RST_BUS_I2C0>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&i2c0_pins>;
+			status = "disabled";
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+
+		i2c1: i2c@01c2b000 {
+			compatible = "allwinner,sun6i-a31-i2c";
+			reg = <0x01c2b000 0x400>;
+			interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&ccu CLK_BUS_I2C1>;
+			resets = <&ccu RST_BUS_I2C1>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&i2c1_pins>;
+			status = "disabled";
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+
+		i2c2: i2c@01c2b400 {
+			compatible = "allwinner,sun6i-a31-i2c";
+			reg = <0x01c2b000 0x400>;
+			interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&ccu CLK_BUS_I2C2>;
+			resets = <&ccu RST_BUS_I2C2>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&i2c2_pins>;
+			status = "disabled";
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+
 		gic: interrupt-controller@01c81000 {
 			compatible = "arm,cortex-a7-gic", "arm,cortex-a15-gic";
 			reg = <0x01c81000 0x1000>,
-- 
2.7.4