[PATCH v2 0/7] dts: sun8i: add extra uart/i2c to H3

[PATCH v2 0/7] dts: sun8i: add extra uart/i2c to H3

[jorik]

From: Jorik Jonker <jorik@kippendief.biz>

This is v2 of my patch series. Just after sending I noticed the interrupts
of i2c1 were too liberally copy/paste'd. This is fixed in v2.

This series is a follow-up to Martin Ayotte's efforts to add additional UARTS
and I2C ports to the AllWinner H3 SoC DTS files. In this series I explicitly
leave out i2c2, since this is not broken out on my boards, so I cannot verify if
it actually works.

After the patch, one enables for example uart1 on a opi+ like this from u-boot:
 => fdt set /soc/serial@01c28400 status okay

Regards,
Jorik

Jorik Jonker (7):
  dts: sun8i-h3: add pinmux definitions for uart2/uart3
  dts: sun8i-h3: add pinmux definitions for i2c0/i2c1
  dts: sun8i-h3: add i2c0/i2c1 SoC peripherals
  dts: sun8i-h3: add UART1-3 to Orange Pi Plus
  dts: sun8i-h3: add UART1-3 to Orange Pi PC
  dts: sun8i-h3: add I2C0-1 to Orange Pi Plus
  dts: sun8i-h3: add I2C0-1 to Orange Pi PC

 arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts   | 25 ++++++++++++++
 arch/arm/boot/dts/sun8i-h3-orangepi-plus.dts | 25 ++++++++++++++
 arch/arm/boot/dts/sun8i-h3.dtsi              | 50 ++++++++++++++++++++++++++++
 3 files changed, 100 insertions(+)

-- 
2.7.4

[PATCH v2 1/7] dts: sun8i-h3: add pinmux definitions for uart2/uart3

[jorik]

From: Jorik Jonker <jorik@kippendief.biz>

This adds pinmux definitions for uarts 2 and 3 on H3 boards

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 fdf9fdb..7740748 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_pins_a: uart2@0 {
+				allwinner,pins = "PA0", "PA1";
+				allwinner,function = "uart2";
+				allwinner,drive = <SUN4I_PINCTRL_10_MA>;
+				allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
+			};
+
+			uart3_pins_a: uart3@0 {
+				allwinner,pins = "PA13", "PA14";
+				allwinner,function = "uart3";
+				allwinner,drive = <SUN4I_PINCTRL_10_MA>;
+				allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
+			};
 		};
 
 		timer@01c20c00 {
-- 
2.7.4

[PATCH v2 2/7] dts: sun8i-h3: add pinmux definitions for i2c0/i2c1

[jorik]

From: Jorik Jonker <jorik@kippendief.biz>

This adds proper pinmux definitions for i2c0 and i2c1. Although H3 has a third
i2c controller, these are not exposed on my boards. If someone actually has a
H3 board with an exposed i2c2, they could add the third.

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 7740748..0637b95 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -327,6 +327,20 @@
 			interrupt-controller;
 			#interrupt-cells = <3>;
 
+			i2c0_pins_a: i2c0@0 {
+				allwinner,pins = "PA11", "PA12";
+				allwinner,function = "i2c0";
+				allwinner,drive = <SUN4I_PINCTRL_10_MA>;
+				allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
+			};
+
+			i2c1_pins_a: i2c1@0 {
+				allwinner,pins = "PA18", "PA19";
+				allwinner,function = "i2c1";
+				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 v2 3/7] dts: sun8i-h3: add i2c0/i2c1 SoC peripherals

[jorik]

From: Jorik Jonker <jorik@kippendief.biz>

This enables the i2c0/i2c1 peripherals of the SoC. There is actually a third
controller, but I do not have a board on hands on which i2c2 is exposed in such
a way that I can verify that it works.

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

diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
index 0637b95..b4ce925 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -469,6 +469,28 @@
 			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>;
+			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>;
+			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

[PATCH v2 4/7] dts: sun8i-h3: add UART1-3 to Orange Pi Plus

[jorik]

From: Jorik Jonker <jorik@kippendief.biz>

This board has UARTS1-3 exposed on its expansion header

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

diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-plus.dts b/arch/arm/boot/dts/sun8i-h3-orangepi-plus.dts
index b0cb417..7a17839 100644
--- a/arch/arm/boot/dts/sun8i-h3-orangepi-plus.dts
+++ b/arch/arm/boot/dts/sun8i-h3-orangepi-plus.dts
@@ -90,6 +90,21 @@
 	};
 };
 
+&uart1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart1_pins_a>;
+};
+
+&uart2 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart2_pins_a>;
+};
+
+&uart3 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart3_pins_a>;
+};
+
 &usbphy {
 	usb3_vbus-supply = <&reg_usb3_vbus>;
 };
-- 
2.7.4

[PATCH v2 5/7] dts: sun8i-h3: add UART1-3 to Orange Pi PC

[jorik]

From: Jorik Jonker <jorik@kippendief.biz>

This board has UARTS1-3 exposed on its expansion header

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

diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
index daf50b9..602b012 100644
--- a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
+++ b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
@@ -161,6 +161,21 @@
 	status = "okay";
 };
 
+&uart1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart1_pins_a>;
+};
+
+&uart2 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart2_pins_a>;
+};
+
+&uart3 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart3_pins_a>;
+};
+
 &usbphy {
 	/* USB VBUS is always on */
 	status = "okay";
-- 
2.7.4

[PATCH v2 6/7] dts: sun8i-h3: add I2C0-1 to Orange Pi Plus

[jorik]

From: Jorik Jonker <jorik@kippendief.biz>

This board has I2C0-2 exposed on its expansion header.

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

diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-plus.dts b/arch/arm/boot/dts/sun8i-h3-orangepi-plus.dts
index 7a17839..fcacafa 100644
--- a/arch/arm/boot/dts/sun8i-h3-orangepi-plus.dts
+++ b/arch/arm/boot/dts/sun8i-h3-orangepi-plus.dts
@@ -64,6 +64,16 @@
 	status = "okay";
 };
 
+&i2c0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c0_pins_a>;
+};
+
+&i2c1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c1_pins_a>;
+};
+
 &mmc2 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&mmc2_8bit_pins>;
-- 
2.7.4

[PATCH v2 7/7] dts: sun8i-h3: add I2C0-1 to Orange Pi PC

[jorik]

From: Jorik Jonker <jorik@kippendief.biz>

This board has I2C0-I2C1 exposed on its expansion header.

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

diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
index 602b012..6102e37 100644
--- a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
+++ b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
@@ -108,6 +108,16 @@
 	status = "okay";
 };
 
+&i2c0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c0_pins_a>;
+};
+
+&i2c1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c1_pins_a>;
+};
+
 &mmc0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&mmc0_pins_a>, <&mmc0_cd_pin>;
-- 
2.7.4

Re: [PATCH v2 3/7] dts: sun8i-h3: add i2c0/i2c1 SoC peripherals

[Chen-Yu Tsai]

On Thu, Sep 1, 2016 at 3:30 AM,  <jorik@kippendief.biz> wrote:
> From: Jorik Jonker <jorik@kippendief.biz>
>
> This enables the i2c0/i2c1 peripherals of the SoC. There is actually a third
> controller, but I do not have a board on hands on which i2c2 is exposed in such
> a way that I can verify that it works.

If they are listed in the manual, and the interrupts, clocks, resets, pins
all exist, that is good enough for me.

>
> Signed-off-by: Jorik Jonker <jorik@kippendief.biz>

Acked-by: Chen-Yu Tsai <wens@csie.org>

Re: [PATCH v2 2/7] dts: sun8i-h3: add pinmux definitions for i2c0/i2c1

[Chen-Yu Tsai]

On Thu, Sep 1, 2016 at 3:30 AM,  <jorik@kippendief.biz> wrote:
> From: Jorik Jonker <jorik@kippendief.biz>
>
> This adds proper pinmux definitions for i2c0 and i2c1. Although H3 has a third
> i2c controller, these are not exposed on my boards. If someone actually has a
> H3 board with an exposed i2c2, they could add the third.
>
> 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 7740748..0637b95 100644
> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> @@ -327,6 +327,20 @@
>                         interrupt-controller;
>                         #interrupt-cells = <3>;
>
> +                       i2c0_pins_a: i2c0@0 {
> +                               allwinner,pins = "PA11", "PA12";
> +                               allwinner,function = "i2c0";
> +                               allwinner,drive = <SUN4I_PINCTRL_10_MA>;
> +                               allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
> +                       };
> +
> +                       i2c1_pins_a: i2c1@0 {

These pinmuxes are the only ones possible for each peripheral.
Please drop the _a suffix and the @0 address for both of them.

ChenYu

> +                               allwinner,pins = "PA18", "PA19";
> +                               allwinner,function = "i2c1";
> +                               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
>

Re: [PATCH v2 3/7] dts: sun8i-h3: add i2c0/i2c1 SoC peripherals

[Jorik Jonker]

Hi,

(trying again in plaintext, sorry for the HTML spam...)

A bit tricky to reply to two mails in one, as I think my reply relates
to both, but here it goes.

On 1 September 2016 at 04:42, Chen-Yu Tsai <wens@csie.org> wrote:
> On Thu, Sep 1, 2016 at 3:30 AM,  <jorik@kippendief.biz> wrote:
>> From: Jorik Jonker <jorik@kippendief.biz>
>>
>> This enables the i2c0/i2c1 peripherals of the SoC. There is actually a third
>> controller, but I do not have a board on hands on which i2c2 is exposed in such
>> a way that I can verify that it works.
>
> If they are listed in the manual, and the interrupts, clocks, resets, pins
> all exist, that is good enough for me.

That sounds sensible, I will do that in v3.

> These pinmuxes are the only ones possible for each peripheral.
> Please drop the _a suffix and the @0 address for both of them.

Agreed. But: I think the same goes for UARTs 0-3, of which 0-1 have a pinmux
following the _a / @0 syntax in current kernel. There are not really options
here, except leaving out RTS/CTS on uarts1-3, which one could do in a
board-specific pinmux.

Moreover, I could put all the pinmux-peripheral associations for H3 in the DTSI,
removing them from the DTS files (including already existing), as the
associations themselves are not really board specific, right?

Best,
Jorik

Re: [PATCH v2 3/7] dts: sun8i-h3: add i2c0/i2c1 SoC peripherals

[Chen-Yu Tsai]

On Thu, Sep 1, 2016 at 2:31 PM, Jorik Jonker <jorik@kippendief.biz> wrote:
> Hi,
>
> A bit tricky to reply to two mails in one, as I think my reply relates to
> both, but here it goes.
>
> On 1 September 2016 at 04:42, Chen-Yu Tsai <wens@csie.org> wrote:
>>
>> On Thu, Sep 1, 2016 at 3:30 AM,  <jorik@kippendief.biz> wrote:
>> > From: Jorik Jonker <jorik@kippendief.biz>
>> >
>> > This enables the i2c0/i2c1 peripherals of the SoC. There is actually a
>> > third
>> > controller, but I do not have a board on hands on which i2c2 is exposed
>> > in such
>> > a way that I can verify that it works.
>>
>> If they are listed in the manual, and the interrupts, clocks, resets, pins
>> all exist, that is good enough for me.
>
>
> That sounds sensible, I will do that in v3.
>
>> These pinmuxes are the only ones possible for each peripheral.
>> Please drop the _a suffix and the @0 address for both of them.
>
> Agreed. But: I think the same goes for UARTs 0-3, of which 0-1 have a pinmux
> following the _a / @0 syntax in current kernel. There are not really options
> here, except leaving out RTS/CTS on uarts1-3, which one could do in a
> board-specific pinmux.

That would work. You could also do the RTS/CTS pins and call them
"uartX_rts_cts_pins: uartX-rts-cts { ... }". Of course doing all these
is dependent on some board actually using them. Let's see what Maxime
has to say about your other patches. :)

> Moreover, I could put all the pinmux-peripheral associations for H3 in the
> DTSI, removing them from the DTS files (including already existing), as the
> associations themselves are not really board specific, right?

I've done so for a few peripherals, such as RSB and IR, where there are
only 1 set of pins that make sense. For 2/4/8 pin UARTs, MMC, LCDs, etc,
I wouldn't do it.

ChenYu

Re: [PATCH v2 4/7] dts: sun8i-h3: add UART1-3 to Orange Pi Plus

[Maxime Ripard]

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

Hi,

On Wed, Aug 31, 2016 at 09:30:55PM +0200, jorik@kippendief.biz wrote:
> From: Jorik Jonker <jorik@kippendief.biz>
> 
> This board has UARTS1-3 exposed on its expansion header
> 
> Signed-off-by: Jorik Jonker <jorik@kippendief.biz>
> ---
>  arch/arm/boot/dts/sun8i-h3-orangepi-plus.dts | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-plus.dts b/arch/arm/boot/dts/sun8i-h3-orangepi-plus.dts
> index b0cb417..7a17839 100644
> --- a/arch/arm/boot/dts/sun8i-h3-orangepi-plus.dts
> +++ b/arch/arm/boot/dts/sun8i-h3-orangepi-plus.dts
> @@ -90,6 +90,21 @@
>  	};
>  };
>  
> +&uart1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&uart1_pins_a>;
> +};
> +
> +&uart2 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&uart2_pins_a>;
> +};
> +
> +&uart3 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&uart3_pins_a>;
> +};
> +

Unfortunately, these pins can be used for other purposes as well, so
we cannot make force that decision down to our users.

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 v2 4/7] dts: sun8i-h3: add UART1-3 to Orange Pi Plus

[Jorik Jonker]

On Fri, Sep 02, 2016 at 09:04:25AM +0200, Maxime Ripard wrote:
>Unfortunately, these pins can be used for other purposes as well, so
>we cannot make force that decision down to our users.

Yes, but since the associated peripheral is disabled, the users are free 
to configure other functions/peripherals, right? I mean something like 
this in pseudo-DT:

 /soc/pio: pinctrl@01c20800/uart1_pins:
   allwinner,pins = "PG6, PG7";
 /soc/pio: pinctrl@01c20800/foo0_pins:
   allwinner,pins = "PG6, PG7";
   ..
 /soc/uart1: serial@serial@01c28400:
   pinctrl-0 = <&uart1_pins>;
   status = "disabled";
 /soc/bar:
   pinctrl-0 = <&uart1_pins>;
   status = "disabled";

Assuming Linux/DT allows this, this would force nothing, only offer 
choice and ease of use.

Best,

Jorik

Re: [PATCH v2 4/7] dts: sun8i-h3: add UART1-3 to Orange Pi Plus

[Maxime Ripard]

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

Hi Jorik,

On Sat, Sep 03, 2016 at 02:09:32PM +0200, Jorik Jonker wrote:
> On Fri, Sep 02, 2016 at 09:04:25AM +0200, Maxime Ripard wrote:
> >Unfortunately, these pins can be used for other purposes as well, so
> >we cannot make force that decision down to our users.
> 
> Yes, but since the associated peripheral is disabled, the users are free to
> configure other functions/peripherals, right? I mean something like this in
> pseudo-DT:
> 
> /soc/pio: pinctrl@01c20800/uart1_pins:
>   allwinner,pins = "PG6, PG7";
> /soc/pio: pinctrl@01c20800/foo0_pins:
>   allwinner,pins = "PG6, PG7";
>   ..
> /soc/uart1: serial@serial@01c28400:
>   pinctrl-0 = <&uart1_pins>;
>   status = "disabled";
> /soc/bar:
>   pinctrl-0 = <&uart1_pins>;
>   status = "disabled";
> 
> Assuming Linux/DT allows this, this would force nothing, only offer choice
> and ease of use.

Hmm, sorry, I went over your patches too quickly...

That's a great compromise I think. Chen-Yu, any opinion on this?

Thanks,
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 v2 4/7] dts: sun8i-h3: add UART1-3 to Orange Pi Plus

[Chen-Yu Tsai]

On Tue, Sep 6, 2016 at 3:31 AM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Hi Jorik,
>
> On Sat, Sep 03, 2016 at 02:09:32PM +0200, Jorik Jonker wrote:
>> On Fri, Sep 02, 2016 at 09:04:25AM +0200, Maxime Ripard wrote:
>> >Unfortunately, these pins can be used for other purposes as well, so
>> >we cannot make force that decision down to our users.
>>
>> Yes, but since the associated peripheral is disabled, the users are free to
>> configure other functions/peripherals, right? I mean something like this in
>> pseudo-DT:
>>
>> /soc/pio: pinctrl@01c20800/uart1_pins:
>>   allwinner,pins = "PG6, PG7";
>> /soc/pio: pinctrl@01c20800/foo0_pins:
>>   allwinner,pins = "PG6, PG7";
>>   ..
>> /soc/uart1: serial@serial@01c28400:
>>   pinctrl-0 = <&uart1_pins>;
>>   status = "disabled";
>> /soc/bar:
>>   pinctrl-0 = <&uart1_pins>;
>>   status = "disabled";
>>
>> Assuming Linux/DT allows this, this would force nothing, only offer choice
>> and ease of use.
>
> Hmm, sorry, I went over your patches too quickly...
>
> That's a great compromise I think. Chen-Yu, any opinion on this?

In short, I'm ok with it. But please put an explicit

    status = "disabled";

and probably a comment about how/where the peripheral can be
used in the board dts.

I intended to do this for the Banana Pis. Though my original plan
was to enable Raspberry Pi compatible peripherals by default, and
list the other peripherals that are defined by the vendor as
"disabled".

"Defined by the vendor" means that the vendor has some sort of
document associating the gpio header pins with the peripherals,
as shown in:

    http://www.orangepi.org/Docs/Pindefinition.html#CON3_Definition

This should make it easier for the average user to enable the
peripherals. I'm not sure we should list _all_ possible ones
though. That would make the list very large, and some might
end up never being used.


Regards
ChenYu

Re: [PATCH v2 4/7] dts: sun8i-h3: add UART1-3 to Orange Pi Plus

[Jorik Jonker]

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

On Tue, Sep 06, 2016 at 11:04:38AM +0800, Chen-Yu Tsai wrote:
>In short, I'm ok with it. But please put an explicit
>
>    status = "disabled";
>
>and probably a comment about how/where the peripheral can be
>used in the board dts.

Allright, I will do that in v3.

>I intended to do this for the Banana Pis. Though my original plan
>was to enable Raspberry Pi compatible peripherals by default, and
>list the other peripherals that are defined by the vendor as
>"disabled".
>
>"Defined by the vendor" means that the vendor has some sort of
>document associating the gpio header pins with the peripherals,
>as shown in:
>
>    http://www.orangepi.org/Docs/Pindefinition.html#CON3_Definition
>
>This should make it easier for the average user to enable the
>peripherals. I'm not sure we should list _all_ possible ones
>though. That would make the list very large, and some might
>end up never being used.

This is exactly what I was thinking, albeit I wanted to go a bit 
'bigger': move all unambiguous pinmux/peripheral associations from the 
H3 DTS files to the DTSI, complete them with known associations for 
which there are drivers, and have all of these disabled. Reduce the 
existing blocks in the DTS files to just "status = okay".

Peripherals requiring board-specific things (like emac, USB, voltage 
regulators, displays) stay put in the DTS.

It will explodes the scope of my proposed change a bit, but if you are 
OK with this, I will gladly do it.

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

Best,

Jorik

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

Re: [PATCH v2 4/7] dts: sun8i-h3: add UART1-3 to Orange Pi Plus

[Maxime Ripard]

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

On Tue, Sep 06, 2016 at 10:52:02AM +0200, Jorik Jonker wrote:
> On Tue, Sep 06, 2016 at 11:04:38AM +0800, Chen-Yu Tsai wrote:
> >In short, I'm ok with it. But please put an explicit
> >
> >   status = "disabled";
> >
> >and probably a comment about how/where the peripheral can be
> >used in the board dts.
> 
> Allright, I will do that in v3.
> 
> >I intended to do this for the Banana Pis. Though my original plan
> >was to enable Raspberry Pi compatible peripherals by default, and
> >list the other peripherals that are defined by the vendor as
> >"disabled".
> >
> >"Defined by the vendor" means that the vendor has some sort of
> >document associating the gpio header pins with the peripherals,
> >as shown in:
> >
> >   http://www.orangepi.org/Docs/Pindefinition.html#CON3_Definition
> >
> >This should make it easier for the average user to enable the
> >peripherals. I'm not sure we should list _all_ possible ones
> >though. That would make the list very large, and some might
> >end up never being used.
> 
> This is exactly what I was thinking, albeit I wanted to go a bit 'bigger':
> move all unambiguous pinmux/peripheral associations from the H3 DTS files to
> the DTSI, complete them with known associations for which there are drivers,
> and have all of these disabled. Reduce the existing blocks in the DTS files
> to just "status = okay".
> 
> Peripherals requiring board-specific things (like emac, USB, voltage
> regulators, displays) stay put in the DTS.
> 
> It will explodes the scope of my proposed change a bit, but if you are OK
> with this, I will gladly do it.
> 
> Maxime, Chen-Yu: what do you think of this?

I'm not exactly sure what you have in mind. Please send a patch, and
we'll see then :)

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 v2 4/7] dts: sun8i-h3: add UART1-3 to Orange Pi Plus

[Maxime Ripard]

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

On Tue, Sep 06, 2016 at 11:04:38AM +0800, Chen-Yu Tsai wrote:
> On Tue, Sep 6, 2016 at 3:31 AM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > Hi Jorik,
> >
> > On Sat, Sep 03, 2016 at 02:09:32PM +0200, Jorik Jonker wrote:
> >> On Fri, Sep 02, 2016 at 09:04:25AM +0200, Maxime Ripard wrote:
> >> >Unfortunately, these pins can be used for other purposes as well, so
> >> >we cannot make force that decision down to our users.
> >>
> >> Yes, but since the associated peripheral is disabled, the users are free to
> >> configure other functions/peripherals, right? I mean something like this in
> >> pseudo-DT:
> >>
> >> /soc/pio: pinctrl@01c20800/uart1_pins:
> >>   allwinner,pins = "PG6, PG7";
> >> /soc/pio: pinctrl@01c20800/foo0_pins:
> >>   allwinner,pins = "PG6, PG7";
> >>   ..
> >> /soc/uart1: serial@serial@01c28400:
> >>   pinctrl-0 = <&uart1_pins>;
> >>   status = "disabled";
> >> /soc/bar:
> >>   pinctrl-0 = <&uart1_pins>;
> >>   status = "disabled";
> >>
> >> Assuming Linux/DT allows this, this would force nothing, only offer choice
> >> and ease of use.
> >
> > Hmm, sorry, I went over your patches too quickly...
> >
> > That's a great compromise I think. Chen-Yu, any opinion on this?
> 
> In short, I'm ok with it. But please put an explicit
> 
>     status = "disabled";
> 
> and probably a comment about how/where the peripheral can be
> used in the board dts.
> 
> I intended to do this for the Banana Pis. Though my original plan
> was to enable Raspberry Pi compatible peripherals by default, and
> list the other peripherals that are defined by the vendor as
> "disabled".
> 
> "Defined by the vendor" means that the vendor has some sort of
> document associating the gpio header pins with the peripherals,
> as shown in:
> 
>     http://www.orangepi.org/Docs/Pindefinition.html#CON3_Definition
> 
> This should make it easier for the average user to enable the
> peripherals. I'm not sure we should list _all_ possible ones
> though. That would make the list very large, and some might
> end up never being used.

Having a clear limit on what we can put and what we can't isn't very
easy to do though. Any suggestion on how we can solve that?

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