Re: [PATCH] ARM: dts: rockchip: move uart2 pinctrl settings to each dts for rk3288

From: Katsuhiro Suzuki
Date: Thu Mar 19 2020 - 14:32:00 EST

Hello Robin,

Thank you for reply.

On 2020/03/20 1:29, Robin Murphy wrote:
On 2020-03-19 3:58 pm, Katsuhiro Suzuki wrote:
Hello Heiko,

On 2020/03/20 0:25, Heiko StÃbner wrote:
Hi,

Am Donnerstag, 19. MÃrz 2020, 16:13:39 CET schrieb Katsuhiro Suzuki:
This patch removes conflicted pinctrl settings uart2 and pwm 2/3
from common rk3288.dtsi and moves exist uart2 pinctrl settings
into each rk3288*.dts files.

 - pwm2_pin : use GPIO7_C6
 - pwm3_pin : use GPIO7_C7
ÂÂ - uart2_xfer: use GPIO7_C6, GPIO7_C7

Board files only ever get to enable either pwm2/3 or uart2,
not both at once - so I'm not sure where you see conflicts.


At first, I think so too. But I've saw this message when booting.

----------
[ÂÂÂ 2.435504] rockchip-pinctrl pinctrl: pin gpio7-22 already requested by ff680020.pwm; cannot claim for ff690000.serial
[ÂÂÂ 2.447506] rockchip-pinctrl pinctrl: pin-238 (ff690000.serial) status -22
[ 2.455198] rockchip-pinctrl pinctrl: could not request pin 238 (gpio7-22) from group uart2-xfer on device rockchip-pinctrl
----------

And it seems that uart2(ttyS2) is not working correctly.

----------
# cat /dev/ttyS2
cat: /dev/ttyS2: Input/output error
----------

I'm using newest linux-next, make defconfig and CONFIG_MODULES = n setting
with TinkerBoard. Can I resolve this issue in other ways?

Do you perhaps have a DT overlay or bootloader script enabling pwm2? (try `cat /sys/firmware/devicetree/base/pwm@ff680020/status` for a sanity check). FWIW I don't recall ever seeing this on my RK3288 box.

(or of course maybe there's just some pinctrl bug in -next that's claiming configs for disabled devices)

Ah, cat /sys/firmware/devicetree/base/pwm@ff680020/status is 'okay'.
So bootloader or someone enables pwm2 on booting.

I'll check settings of bootloader of my board. Thanks a lot!

Best Regards,
Katsuhiro Suzuki


Robin.


Best Regards,
Katsuhiro Suzuki


And of course there are alternative pins to use, if you need uart2
you need both pins in uart-pinmux and if you need either as pwm, then
the board by design just can't use them as uart2.

So pin setting should stay where they are, as there really is no conflict.


Heiko



Currently uart2 rk3288 user is the following:

ÂÂ - rk3288-evb.dtsi:&uart2 {
ÂÂ - rk3288-firefly-reload.dts:&uart2 {
ÂÂ - rk3288-firefly.dtsi:&uart2 {
ÂÂ - rk3288-miqi.dts:&uart2 {
ÂÂ - rk3288-phycore-rdk.dts:&uart2 {
ÂÂ - rk3288-popmetal.dts:&uart2 {
ÂÂ - rk3288-r89.dts:&uart2 {
ÂÂ - rk3288-rock2-square.dts:&uart2 {
ÂÂ - rk3288-tinker.dtsi:&uart2 {
ÂÂ - rk3288-veyron.dtsi:&uart2 {
ÂÂ - rk3288-vyasa.dts:&uart2 {

And no one is using pwm2 nor pwm3.

Signed-off-by: Katsuhiro Suzuki <katsuhiro@xxxxxxxxxxxxx>
---
 arch/arm/boot/dts/rk3288-evb.dtsi | 2 ++
 arch/arm/boot/dts/rk3288-firefly-reload.dts | 2 ++
 arch/arm/boot/dts/rk3288-firefly.dtsi | 2 ++
 arch/arm/boot/dts/rk3288-miqi.dts | 2 ++
 arch/arm/boot/dts/rk3288-phycore-rdk.dts | 2 ++
 arch/arm/boot/dts/rk3288-popmetal.dts | 2 ++
 arch/arm/boot/dts/rk3288-r89.dts | 2 ++
 arch/arm/boot/dts/rk3288-rock2-square.dts | 2 ++
 arch/arm/boot/dts/rk3288-tinker.dtsi | 2 ++
 arch/arm/boot/dts/rk3288-veyron.dtsi | 2 ++
 arch/arm/boot/dts/rk3288-vyasa.dts | 2 ++
 arch/arm/boot/dts/rk3288.dtsi | 6 ------
 12 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/arch/arm/boot/dts/rk3288-evb.dtsi b/arch/arm/boot/dts/rk3288-evb.dtsi
index 018802df4c0e..74091f831ecf 100644
--- a/arch/arm/boot/dts/rk3288-evb.dtsi
+++ b/arch/arm/boot/dts/rk3288-evb.dtsi
@@ -285,6 +285,8 @@ &uart1 {
 };
 &uart2 {
+ÂÂÂ pinctrl-names = "default";
+ÂÂÂ pinctrl-0 = <&uart2_xfer>;
ÂÂÂÂÂ status = "okay";
 };
diff --git a/arch/arm/boot/dts/rk3288-firefly-reload.dts b/arch/arm/boot/dts/rk3288-firefly-reload.dts
index 8c38bda21a7c..b0c976c8e35b 100644
--- a/arch/arm/boot/dts/rk3288-firefly-reload.dts
+++ b/arch/arm/boot/dts/rk3288-firefly-reload.dts
@@ -283,6 +283,8 @@ &uart1 {
 };
 &uart2 {
+ÂÂÂ pinctrl-names = "default";
+ÂÂÂ pinctrl-0 = <&uart2_xfer>;
ÂÂÂÂÂ status = "okay";
 };
diff --git a/arch/arm/boot/dts/rk3288-firefly.dtsi b/arch/arm/boot/dts/rk3288-firefly.dtsi
index 5e0a19004e46..1632cc083c12 100644
--- a/arch/arm/boot/dts/rk3288-firefly.dtsi
+++ b/arch/arm/boot/dts/rk3288-firefly.dtsi
@@ -532,6 +532,8 @@ &uart1 {
 };
 &uart2 {
+ÂÂÂ pinctrl-names = "default";
+ÂÂÂ pinctrl-0 = <&uart2_xfer>;
ÂÂÂÂÂ status = "okay";
 };
diff --git a/arch/arm/boot/dts/rk3288-miqi.dts b/arch/arm/boot/dts/rk3288-miqi.dts
index c41d012c8850..2c0ed37fde80 100644
--- a/arch/arm/boot/dts/rk3288-miqi.dts
+++ b/arch/arm/boot/dts/rk3288-miqi.dts
@@ -379,6 +379,8 @@ &tsadc {
 };
 &uart2 {
+ÂÂÂ pinctrl-names = "default";
+ÂÂÂ pinctrl-0 = <&uart2_xfer>;
ÂÂÂÂÂ status = "okay";
 };
diff --git a/arch/arm/boot/dts/rk3288-phycore-rdk.dts b/arch/arm/boot/dts/rk3288-phycore-rdk.dts
index 1e33859de484..6532c1ac43cd 100644
--- a/arch/arm/boot/dts/rk3288-phycore-rdk.dts
+++ b/arch/arm/boot/dts/rk3288-phycore-rdk.dts
@@ -244,6 +244,8 @@ &uart0 {
 };
 &uart2 {
+ÂÂÂ pinctrl-names = "default";
+ÂÂÂ pinctrl-0 = <&uart2_xfer>;
ÂÂÂÂÂ status = "okay";
 };
diff --git a/arch/arm/boot/dts/rk3288-popmetal.dts b/arch/arm/boot/dts/rk3288-popmetal.dts
index 6a51940398b5..f18306bd9e6e 100644
--- a/arch/arm/boot/dts/rk3288-popmetal.dts
+++ b/arch/arm/boot/dts/rk3288-popmetal.dts
@@ -481,6 +481,8 @@ &uart1 {
 };
 &uart2 {
+ÂÂÂ pinctrl-names = "default";
+ÂÂÂ pinctrl-0 = <&uart2_xfer>;
ÂÂÂÂÂ status = "okay";
 };
diff --git a/arch/arm/boot/dts/rk3288-r89.dts b/arch/arm/boot/dts/rk3288-r89.dts
index a258c7ae5329..02d2f5cfe201 100644
--- a/arch/arm/boot/dts/rk3288-r89.dts
+++ b/arch/arm/boot/dts/rk3288-r89.dts
@@ -340,6 +340,8 @@ &uart1 {
 };
 &uart2 {
+ÂÂÂ pinctrl-names = "default";
+ÂÂÂ pinctrl-0 = <&uart2_xfer>;
ÂÂÂÂÂ status = "okay";
 };
diff --git a/arch/arm/boot/dts/rk3288-rock2-square.dts b/arch/arm/boot/dts/rk3288-rock2-square.dts
index cdcdc921ee09..a44290e882be 100644
--- a/arch/arm/boot/dts/rk3288-rock2-square.dts
+++ b/arch/arm/boot/dts/rk3288-rock2-square.dts
@@ -264,6 +264,8 @@ &spdif {
 };
 &uart2 {
+ÂÂÂ pinctrl-names = "default";
+ÂÂÂ pinctrl-0 = <&uart2_xfer>;
ÂÂÂÂÂ status = "okay";
 };
diff --git a/arch/arm/boot/dts/rk3288-tinker.dtsi b/arch/arm/boot/dts/rk3288-tinker.dtsi
index acfaa12ec239..0327119f71b4 100644
--- a/arch/arm/boot/dts/rk3288-tinker.dtsi
+++ b/arch/arm/boot/dts/rk3288-tinker.dtsi
@@ -500,6 +500,8 @@ &uart1 {
 };
 &uart2 {
+ÂÂÂ pinctrl-names = "default";
+ÂÂÂ pinctrl-0 = <&uart2_xfer>;
ÂÂÂÂÂ status = "okay";
 };
diff --git a/arch/arm/boot/dts/rk3288-veyron.dtsi b/arch/arm/boot/dts/rk3288-veyron.dtsi
index 54a6838d73f5..baa44d00e49a 100644
--- a/arch/arm/boot/dts/rk3288-veyron.dtsi
+++ b/arch/arm/boot/dts/rk3288-veyron.dtsi
@@ -412,6 +412,8 @@ &uart1 {
 };
 &uart2 {
+ÂÂÂ pinctrl-names = "default";
+ÂÂÂ pinctrl-0 = <&uart2_xfer>;
ÂÂÂÂÂ status = "okay";
 };
diff --git a/arch/arm/boot/dts/rk3288-vyasa.dts b/arch/arm/boot/dts/rk3288-vyasa.dts
index 385dd59393e1..aa50cdc7f839 100644
--- a/arch/arm/boot/dts/rk3288-vyasa.dts
+++ b/arch/arm/boot/dts/rk3288-vyasa.dts
@@ -398,6 +398,8 @@ &tsadc {
 };
 &uart2 {
+ÂÂÂ pinctrl-names = "default";
+ÂÂÂ pinctrl-0 = <&uart2_xfer>;
ÂÂÂÂÂ status = "okay";
 };
diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
index 0cd88774db95..4c1f8cabb5eb 100644
--- a/arch/arm/boot/dts/rk3288.dtsi
+++ b/arch/arm/boot/dts/rk3288.dtsi
@@ -450,8 +450,6 @@ uart2: serial@ff690000 {
ÂÂÂÂÂÂÂÂÂ reg-io-width = <4>;
ÂÂÂÂÂÂÂÂÂ clocks = <&cru SCLK_UART2>, <&cru PCLK_UART2>;
ÂÂÂÂÂÂÂÂÂ clock-names = "baudclk", "apb_pclk";
-ÂÂÂÂÂÂÂ pinctrl-names = "default";
-ÂÂÂÂÂÂÂ pinctrl-0 = <&uart2_xfer>;
ÂÂÂÂÂÂÂÂÂ status = "disabled";
ÂÂÂÂÂ };
@@ -706,8 +704,6 @@ pwm2: pwm@ff680020 {
ÂÂÂÂÂÂÂÂÂ compatible = "rockchip,rk3288-pwm";
ÂÂÂÂÂÂÂÂÂ reg = <0x0 0xff680020 0x0 0x10>;
ÂÂÂÂÂÂÂÂÂ #pwm-cells = <3>;
-ÂÂÂÂÂÂÂ pinctrl-names = "default";
-ÂÂÂÂÂÂÂ pinctrl-0 = <&pwm2_pin>;
ÂÂÂÂÂÂÂÂÂ clocks = <&cru PCLK_RKPWM>;
ÂÂÂÂÂÂÂÂÂ clock-names = "pwm";
ÂÂÂÂÂÂÂÂÂ status = "disabled";
@@ -717,8 +713,6 @@ pwm3: pwm@ff680030 {
ÂÂÂÂÂÂÂÂÂ compatible = "rockchip,rk3288-pwm";
ÂÂÂÂÂÂÂÂÂ reg = <0x0 0xff680030 0x0 0x10>;
ÂÂÂÂÂÂÂÂÂ #pwm-cells = <3>;
-ÂÂÂÂÂÂÂ pinctrl-names = "default";
-ÂÂÂÂÂÂÂ pinctrl-0 = <&pwm3_pin>;
ÂÂÂÂÂÂÂÂÂ clocks = <&cru PCLK_RKPWM>;
ÂÂÂÂÂÂÂÂÂ clock-names = "pwm";
ÂÂÂÂÂÂÂÂÂ status = "disabled";








_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel