[PATCH 1/2] pinctrl: sunxi: fix uart1 CTS/RTS pins at PG on A23/A33

PG8, PG9 is said to be the CTS/RTS pins for UART1 according to the A23/33
datasheets. However, the function is wrongly named "uart2" in the pinctrl
driver. This patch fixes this by modifying them to be named "uart1".

Signed-off-by: Icenowy Zheng <redacted>
---
 drivers/pinctrl/sunxi/pinctrl-sun8i-a23.c | 4 ++--
 drivers/pinctrl/sunxi/pinctrl-sun8i-a33.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/sunxi/pinctrl-sun8i-a23.c b/drivers/pinctrl/sunxi/pinctrl-sun8i-a23.c
index ce483b0..f9d661e 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sun8i-a23.c
+++ b/drivers/pinctrl/sunxi/pinctrl-sun8i-a23.c

@@ -485,12 +485,12 @@ static const struct sunxi_desc_pin sun8i_a23_pins[] = {
 	SUNXI_PIN(SUNXI_PINCTRL_PIN(G, 8),
 		  SUNXI_FUNCTION(0x0, "gpio_in"),
 		  SUNXI_FUNCTION(0x1, "gpio_out"),
-		  SUNXI_FUNCTION(0x2, "uart2"),		/* RTS */
+		  SUNXI_FUNCTION(0x2, "uart1"),		/* RTS */
 		  SUNXI_FUNCTION_IRQ_BANK(0x4, 2, 8)),	/* PG_EINT8 */
 	SUNXI_PIN(SUNXI_PINCTRL_PIN(G, 9),
 		  SUNXI_FUNCTION(0x0, "gpio_in"),
 		  SUNXI_FUNCTION(0x1, "gpio_out"),
-		  SUNXI_FUNCTION(0x2, "uart2"),		/* CTS */
+		  SUNXI_FUNCTION(0x2, "uart1"),		/* CTS */
 		  SUNXI_FUNCTION_IRQ_BANK(0x4, 2, 9)),	/* PG_EINT9 */
 	SUNXI_PIN(SUNXI_PINCTRL_PIN(G, 10),
 		  SUNXI_FUNCTION(0x0, "gpio_in"),

diff --git a/drivers/pinctrl/sunxi/pinctrl-sun8i-a33.c b/drivers/pinctrl/sunxi/pinctrl-sun8i-a33.c
index 3040abe..3131cac 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sun8i-a33.c
+++ b/drivers/pinctrl/sunxi/pinctrl-sun8i-a33.c

@@ -407,12 +407,12 @@ static const struct sunxi_desc_pin sun8i_a33_pins[] = {
 	SUNXI_PIN(SUNXI_PINCTRL_PIN(G, 8),
 		  SUNXI_FUNCTION(0x0, "gpio_in"),
 		  SUNXI_FUNCTION(0x1, "gpio_out"),
-		  SUNXI_FUNCTION(0x2, "uart2"),		/* RTS */
+		  SUNXI_FUNCTION(0x2, "uart1"),		/* RTS */
 		  SUNXI_FUNCTION_IRQ_BANK(0x4, 1, 8)),	/* PG_EINT8 */
 	SUNXI_PIN(SUNXI_PINCTRL_PIN(G, 9),
 		  SUNXI_FUNCTION(0x0, "gpio_in"),
 		  SUNXI_FUNCTION(0x1, "gpio_out"),
-		  SUNXI_FUNCTION(0x2, "uart2"),		/* CTS */
+		  SUNXI_FUNCTION(0x2, "uart1"),		/* CTS */
 		  SUNXI_FUNCTION_IRQ_BANK(0x4, 1, 9)),	/* PG_EINT9 */
 	SUNXI_PIN(SUNXI_PINCTRL_PIN(G, 10),
 		  SUNXI_FUNCTION(0x0, "gpio_in"),

-- 
2.9.3

A23/33 have 4 UART controllers outside PRCM. However, the devicetree used
to mentioned only UART0's pinmux settings. Some extra UART controllers have
RTS/CTS, and is suitable for using as bluetooth UART controller.

Add the pinmux settings for the UART controller, to make use of the
bluetooth function of some tablets.

Signed-off-by: Icenowy Zheng <redacted>
---
 arch/arm/boot/dts/sun8i-a23-a33.dtsi | 56 ++++++++++++++++++++++++++++++++++++
 arch/arm/boot/dts/sun8i-a23.dtsi     |  7 +++++
 2 files changed, 63 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a23-a33.dtsi b/arch/arm/boot/dts/sun8i-a23-a33.dtsi
index 7e05e09..89ea479 100644
--- a/arch/arm/boot/dts/sun8i-a23-a33.dtsi
+++ b/arch/arm/boot/dts/sun8i-a23-a33.dtsi

@@ -361,6 +361,62 @@
 				allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
 			};
 
+			uart1_pins_a: uart1 at 0 {
+				allwinner,pins = "PD10", "PD11";
+				allwinner,function = "uart1";
+				allwinner,drive = <SUN4I_PINCTRL_10_MA>;
+				allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
+			};
+
+			uart1_pins_cts_rts_a: uart1-cts-rts at 1 {
+				allwinner,pins = "PD12", "PD13";
+				allwinner,function = "uart1";
+				allwinner,drive = <SUN4I_PINCTRL_10_MA>;
+				allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
+			};
+
+			uart1_pins_b: uart1 at 1 {
+				allwinner,pins = "PG6", "PG7";
+				allwinner,function = "uart1";
+				allwinner,drive = <SUN4I_PINCTRL_10_MA>;
+				allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
+			};
+
+			uart1_pins_cts_rts_b: uart1-cts-rts at 0 {
+				allwinner,pins = "PG8", "PG9";
+				allwinner,function = "uart1";
+				allwinner,drive = <SUN4I_PINCTRL_10_MA>;
+				allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
+			};
+
+			uart2_pins_a: uart2 at 0 {
+				allwinner,pins = "PB0", "PB1";
+				allwinner,function = "uart2";
+				allwinner,drive = <SUN4I_PINCTRL_10_MA>;
+				allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
+			};
+
+			uart2_pins_cts_rts_a: uart2-cts-rts at 1 {
+				allwinner,pins = "PB2", "PB3";
+				allwinner,function = "uart2";
+				allwinner,drive = <SUN4I_PINCTRL_10_MA>;
+				allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
+			};
+
+			uart3_pins_a: uart3 at 0 {
+				allwinner,pins = "PH6", "PH7";
+				allwinner,function = "uart3";
+				allwinner,drive = <SUN4I_PINCTRL_10_MA>;
+				allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
+			};
+
+			uart3_pins_cts_rts_a: uart3-cts-rts at 1 {
+				allwinner,pins = "PH8", "PH9";
+				allwinner,function = "uart3";
+				allwinner,drive = <SUN4I_PINCTRL_10_MA>;
+				allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
+			};
+
 			mmc0_pins_a: mmc0 at 0 {
 				allwinner,pins = "PF0", "PF1", "PF2",
 						 "PF3", "PF4", "PF5";

diff --git a/arch/arm/boot/dts/sun8i-a23.dtsi b/arch/arm/boot/dts/sun8i-a23.dtsi
index 92e6616..6b76580 100644
--- a/arch/arm/boot/dts/sun8i-a23.dtsi
+++ b/arch/arm/boot/dts/sun8i-a23.dtsi

@@ -123,4 +123,11 @@
 	interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>,
 		     <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>,
 		     <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;
+
+	uart3_pins_b: uart3 at 0 {
+		allwinner,pins = "PD8", "PD9";
+		allwinner,function = "uart3";
+		allwinner,drive = <SUN4I_PINCTRL_10_MA>;
+		allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
+	};
 };

-- 
2.9.3

23.08.2016, 14:47, "Maxime Ripard" [off-list ref]:

Hi,

On Tue, Aug 23, 2016 at 01:58:25PM +0800, Icenowy Zheng wrote:

quoted

?PG8, PG9 is said to be the CTS/RTS pins for UART1 according to the A23/33
?datasheets. However, the function is wrongly named "uart2" in the pinctrl
?driver. This patch fixes this by modifying them to be named "uart1".

?Signed-off-by: Icenowy Zheng [off-list ref]

This looks good, but could you send it to stable?

Yes... But currently in mainline kernel, no dt file for A23/33 uses UART1.
(Lots of the tablets is so low-end that they do not ship with BT support, and
then no support for extra UART is needed)

I found the problem when running mainline kernel on my iNet D978 Rev2 board
tablet. It come with a RTL8723BS.

Thanks,
Maxime

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

Hi,

On Tue, Aug 23, 2016 at 01:58:25PM +0800, Icenowy Zheng wrote:

PG8, PG9 is said to be the CTS/RTS pins for UART1 according to the A23/33
datasheets. However, the function is wrongly named "uart2" in the pinctrl
driver. This patch fixes this by modifying them to be named "uart1".

Signed-off-by: Icenowy Zheng <redacted>

This looks good, but could you send it to stable?

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160823/f03c83a4/attachment.sig>

On Tue, Aug 23, 2016 at 8:47 AM, Maxime Ripard
[off-list ref] wrote:

On Tue, Aug 23, 2016 at 01:58:25PM +0800, Icenowy Zheng wrote:

quoted

PG8, PG9 is said to be the CTS/RTS pins for UART1 according to the A23/33
datasheets. However, the function is wrongly named "uart2" in the pinctrl
driver. This patch fixes this by modifying them to be named "uart1".

Signed-off-by: Icenowy Zheng <redacted>

This looks good, but could you send it to stable?

If that is an ACK I can tag it for stable when applying.

Yours,
Linus Walleij

On Tue, Aug 23, 2016 at 11:27:10AM +0200, Linus Walleij wrote:

On Tue, Aug 23, 2016 at 8:47 AM, Maxime Ripard
[off-list ref] wrote:

quoted

On Tue, Aug 23, 2016 at 01:58:25PM +0800, Icenowy Zheng wrote:

quoted

PG8, PG9 is said to be the CTS/RTS pins for UART1 according to the A23/33
datasheets. However, the function is wrongly named "uart2" in the pinctrl
driver. This patch fixes this by modifying them to be named "uart1".

Signed-off-by: Icenowy Zheng <redacted>

This looks good, but could you send it to stable?

If that is an ACK I can tag it for stable when applying.

Yes, it's an ACK. Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160823/f6ea9a5f/attachment.sig>

On Tue, Aug 23, 2016 at 7:58 AM, Icenowy Zheng [off-list ref] wrote:

PG8, PG9 is said to be the CTS/RTS pins for UART1 according to the A23/33
datasheets. However, the function is wrongly named "uart2" in the pinctrl
driver. This patch fixes this by modifying them to be named "uart1".

Signed-off-by: Icenowy Zheng <redacted>

Applied for stable with Maxime's ACK.

Yours,
Linus Walleij

Hi,

On Tue, Aug 23, 2016 at 01:58:26PM +0800, Icenowy Zheng wrote:

A23/33 have 4 UART controllers outside PRCM. However, the devicetree used
to mentioned only UART0's pinmux settings. Some extra UART controllers have
RTS/CTS, and is suitable for using as bluetooth UART controller.

Add the pinmux settings for the UART controller, to make use of the
bluetooth function of some tablets.

Signed-off-by: Icenowy Zheng <redacted>

Our policy is that we avoid adding unused pinctrl nodes to not cripple
the DT for no particular reason.

So please add only the nodes you'll need in your tablet.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160823/6b013030/attachment.sig>

23.08.2016, 19:42, "Maxime Ripard" [off-list ref]:

Hi,

On Tue, Aug 23, 2016 at 01:58:26PM +0800, Icenowy Zheng wrote:

quoted

?A23/33 have 4 UART controllers outside PRCM. However, the devicetree used
?to mentioned only UART0's pinmux settings. Some extra UART controllers have
?RTS/CTS, and is suitable for using as bluetooth UART controller.

?Add the pinmux settings for the UART controller, to make use of the
?bluetooth function of some tablets.

?Signed-off-by: Icenowy Zheng [off-list ref]

Our policy is that we avoid adding unused pinctrl nodes to not cripple
the DT for no particular reason.

So please add only the nodes you'll need in your tablet.

Then... if what I needs is the PG group of uart1, should I name it as
uart1_pins_a or uart1_pins_b?

Thanks!
Maxime

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

Hi,

On Tue, Aug 23, 2016 at 09:51:05PM +0800, Icenowy Zheng wrote:

23.08.2016, 19:42, "Maxime Ripard" [off-list ref]:

quoted

Hi,

On Tue, Aug 23, 2016 at 01:58:26PM +0800, Icenowy Zheng wrote:

quoted

?A23/33 have 4 UART controllers outside PRCM. However, the devicetree used
?to mentioned only UART0's pinmux settings. Some extra UART controllers have
?RTS/CTS, and is suitable for using as bluetooth UART controller.

?Add the pinmux settings for the UART controller, to make use of the
?bluetooth function of some tablets.

?Signed-off-by: Icenowy Zheng [off-list ref]

Our policy is that we avoid adding unused pinctrl nodes to not cripple
the DT for no particular reason.

So please add only the nodes you'll need in your tablet.

Then... if what I needs is the PG group of uart1, should I name it as
uart1_pins_a or uart1_pins_b?

We don't have any rules here, whichever comes first wins.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160826/e190ebe2/attachment-0001.sig>