ARM: rockchip: rk3188: enable pull-ups on UART inputs
PATCH 1/1 ARM: rockchip: rk3188: enable pull-ups on UART inputs
Enable integrated pull-ups on the UART RX pins of Rockchip RK3188.
This fixes UART receive on the radxa Rock board, which has an input diode preventing the RX pin from being raised from the outside.
--- arch/arm/boot/dts/rk3188.dtsi | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
arch/arm/boot/dts/rk3188.dtsi +4 -4
@@ -149,7 +149,7 @@ uart0 { uart0_xfer: uart0-xfer { - rockchip,pins = <RK_GPIO1 0 RK_FUNC_1 &pcfg_pull_none>, + rockchip,pins = <RK_GPIO1 0 RK_FUNC_1 &pcfg_pull_up>, <RK_GPIO1 1 RK_FUNC_1 &pcfg_pull_none>; };
@@ -164,7 +164,7 @@ uart1 { uart1_xfer: uart1-xfer { - rockchip,pins = <RK_GPIO1 4 RK_FUNC_1 &pcfg_pull_none>, + rockchip,pins = <RK_GPIO1 4 RK_FUNC_1 &pcfg_pull_up>, <RK_GPIO1 5 RK_FUNC_1 &pcfg_pull_none>; };
@@ -179,7 +179,7 @@ uart2 { uart2_xfer: uart2-xfer { - rockchip,pins = <RK_GPIO1 8 RK_FUNC_1 &pcfg_pull_none>, + rockchip,pins = <RK_GPIO1 8 RK_FUNC_1 &pcfg_pull_up>, <RK_GPIO1 9 RK_FUNC_1 &pcfg_pull_none>; }; /* no rts / cts for uart2 */
@@ -187,7 +187,7 @@ uart3 { uart3_xfer: uart3-xfer { - rockchip,pins = <RK_GPIO1 10 RK_FUNC_1 &pcfg_pull_none>, + rockchip,pins = <RK_GPIO1 10 RK_FUNC_1 &pcfg_pull_up>, <RK_GPIO1 11 RK_FUNC_1 &pcfg_pull_none>; };
Signature · 1 line
1.8.3.2
PATCH 1/1 ARM: rockchip: rk3188: enable pull-ups on UART inputs
Hi Max,
Am Sonntag, 9. M?rz 2014, 20:43:11 schrieb Max Schwarz:
Quoted 6 lines
Enable integrated pull-ups on the UART RX pins of Rockchip RK3188.
This fixes UART receive on the radxa Rock board, which has an input diode preventing the RX pin from being raised from the outside.
Signed-off-by Max Schwarz <[email protected]>
Your patch changes the default behaviour for all rk3188 uarts, while the patch description suggests a Radxa Rock specific problem.
On my preproduction Radxa Rock the issue is not present, but I think I remember reading somewhere, that the diodes where added in the mass-production model.
So I've dug a bit through different sources:
- the schematics of another rk3188 based device, did not use diodes for the uarts like the Radxa, but
- the default values for pull configuration supports your observation:
gpio1-0: pull-up
gpio1-1: pull down [currently also none]
gpio1-4: pull up
gpio1-5: pull-down
and so on for the two other pin-groups.
- and rockchip vendor kernels do not seem to change the uart pull configs
I've tested both your patch + enabling the pull down for the txd pin too. Both variants work on my radxa.
So I agree with you but would like to determine if we should also set the txd to pull down in one go, to restore the default pull setting of these pins or should leave them as is.
Thoughts?
Heiko
arch/arm/boot/dts/rk3188.dtsi line 152
Quoted 13 lines
--- arch/arm/boot/dts/rk3188.dtsi | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)arch/arm/boot/dts/rk3188.dtsi +0
-1@@ -149,7 +149,7 @@ uart0 { uart0_xfer: uart0-xfer { - rockchip,pins = <RK_GPIO1 0 RK_FUNC_1
&pcfg_pull_none>,
arch/arm/boot/dts/rk3188.dtsi line 167
Quoted 9 lines
arch/arm/boot/dts/rk3188.dtsi +1
-1+ rockchip,pins = <RK_GPIO1 0 RK_FUNC_1 &pcfg_pull_up>, <RK_GPIO1 1 RK_FUNC_1 &pcfg_pull_none>; };@@ -164,7 +164,7 @@ uart1 { uart1_xfer: uart1-xfer { - rockchip,pins = <RK_GPIO1 4 RK_FUNC_1
&pcfg_pull_none>,
arch/arm/boot/dts/rk3188.dtsi line 182
Quoted 9 lines
arch/arm/boot/dts/rk3188.dtsi +1
-1+ rockchip,pins = <RK_GPIO1 4 RK_FUNC_1 &pcfg_pull_up>, <RK_GPIO1 5 RK_FUNC_1 &pcfg_pull_none>; };@@ -179,7 +179,7 @@ uart2 { uart2_xfer: uart2-xfer { - rockchip,pins = <RK_GPIO1 8 RK_FUNC_1
&pcfg_pull_none>,
arch/arm/boot/dts/rk3188.dtsi line 190
Quoted 9 lines
arch/arm/boot/dts/rk3188.dtsi +1
-1+ rockchip,pins = <RK_GPIO1 8 RK_FUNC_1 &pcfg_pull_up>, <RK_GPIO1 9 RK_FUNC_1 &pcfg_pull_none>; }; /* no rts / cts for uart2 */@@ -187,7 +187,7 @@ uart3 { uart3_xfer: uart3-xfer { - rockchip,pins = <RK_GPIO1 10 RK_FUNC_1
&pcfg_pull_none>,
Quoted 3 lines
arch/arm/boot/dts/rk3188.dtsi +1
-0+ rockchip,pins = <RK_GPIO1 10 RK_FUNC_1 &pcfg_pull_up>, <RK_GPIO1 11 RK_FUNC_1 &pcfg_pull_none>; };
PATCH 1/1 ARM: rockchip: rk3188: enable pull-ups on UART inputs
Hello Heiko,
Quoted 4 lines
So I agree with you but would like to determine if we should also set the txd to pull down in one go, to restore the default pull setting of these pins or should leave them as is.
Thoughts?
As soon as the UART is enabled, that TX pull-down does not do anything because the UART will actively drive the pin. I don't know what happens if the UART is suspended through runtime PM, though. I skimmed over the 8250_dw and saw support for that.
On the radxa board, there is even an external pull-up on the TX pin. The UART idle level is high, so that makes sense. If we wanted to pull the pin somewhere, I guess it should be up, not down.
My vote would be to keep the patch as it is. In any case, it's an improvement of the status quo and does not change TX behavior.
Cheers,
Max
PATCH 1/1 ARM: rockchip: rk3188: enable pull-ups on UART inputs
Hi Max,
Am Donnerstag, 13. M?rz 2014, 20:25:43 schrieb Max Schwarz:
Quoted 12 lines
Hello Heiko,
Quoted 4 lines
So I agree with you but would like to determine if we should also set the txd to pull down in one go, to restore the default pull setting of these pins or should leave them as is.
Thoughts?As soon as the UART is enabled, that TX pull-down does not do anything because the UART will actively drive the pin. I don't know what happens if the UART is suspended through runtime PM, though. I skimmed over the 8250_dw and saw support for that.
Quoted 6 lines
On the radxa board, there is even an external pull-up on the TX pin. The UART idle level is high, so that makes sense. If we wanted to pull the pin somewhere, I guess it should be up, not down.
My vote would be to keep the patch as it is. In any case, it's an improvement of the status quo and does not change TX behavior.
ok, but as we change the default behaviour of the pin - and as we discussed above rightfully so, the commit message should reflect this and not describe a Radxa-Rock specific change. So would you be ok with something like:
---------- 8< --------------------
Subject: [PATCH] ARM: rockchip: rk3188: enable pull-ups on UART RX pins
The default behaviour of the uart-rx pins on the rk3188 is to be pulled up and a lot of designs use diodes to even prevent them from being raised from the outside.
Therefore change the rx-pin settings accordingly.
This also fixes a uart receive problem on mass production Radxa Rock boards. ---------- 8< --------------------
Heiko
PATCH 1/1 ARM: rockchip: rk3188: enable pull-ups on UART inputs
Hi Heiko,
Quoted 7 lines
ok, but as we change the default behaviour of the pin - and as we discussed above rightfully so, the commit message should reflect this and not describe a Radxa-Rock specific change. So would you be ok with something like:
---------- 8< --------------------
Subject: [PATCH] ARM: rockchip: rk3188: enable pull-ups on UART RX pins
Quoted 8 lines
The default behaviour of the uart-rx pins on the rk3188 is to be pulled up and a lot of designs use diodes to even prevent them from being raised from the outside.
Therefore change the rx-pin settings accordingly.
This also fixes a uart receive problem on mass production Radxa Rock boards. ---------- 8< --------------------
Yes, that sounds good to me. Thanks!
Cheers,
Max