2024-03-27 19:27:08

by Folker Schwesinger

[permalink] [raw]
Subject: [PATCH 0/2] Enable internal strobe pulldown on Radxa ROCK 4 SBCs

Various RK3399 boards, including the ROCK Pi 4 series showed instabilities
with some eMMC modules when operating in HS400 mode [1]. Later, a missing
pull-down resistor on the eMMC strobe line was identified as the root
cause of these instabilities [2].

This series enables the internal pull-down on the eMMC PHY strobe line
for all ROCK 4 series boards as they all lack an external strobe
pull-down resistor.

Furthermore, HS400 mode is re-enabled for these boards. Previously, to
workaround the instabilities until the issue was investigated further,
HS400 mode was replaced with HS200 mode [1].

[1]: https://lore.kernel.org/all/[email protected]/
[2]: https://lore.kernel.org/all/[email protected]/


Folker Schwesinger (2):
arm64: dts: rockchip: Add enable-strobe-pulldown to emmc phy on ROCK
Pi 4
arm64: dts: rockchip: Add enable-strobe-pulldown to emmc phy on ROCK
4C+

arch/arm64/boot/dts/rockchip/rk3399-rock-4c-plus.dts | 4 +++-
arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi | 4 +++-
2 files changed, 6 insertions(+), 2 deletions(-)


base-commit: 4cece764965020c22cff7665b18a012006359095
--
2.44.0



2024-03-27 19:29:11

by Folker Schwesinger

[permalink] [raw]
Subject: [PATCH 1/2] arm64: dts: rockchip: Add enable-strobe-pulldown to emmc phy on ROCK Pi 4

Commit 8b5c2b45b8f0 disabled the internal pull-down for the strobe line
causing I/O errors in HS400 mode for various eMMC modules.

Enable the internal strobe pull-down for ROCK Pi 4 boards. Also re-enable
HS400 mode, that was replaced with HS200 mode as a workaround for the
stability issues in:
cee572756aa2 ("arm64: dts: rockchip: Disable HS400 for eMMC on ROCK Pi 4").

This was tested on ROCK 4SE and ROCK Pi 4B+.

Fixes: 8b5c2b45b8f0 ("phy: rockchip: set pulldown for strobe line in dts")
Signed-off-by: Folker Schwesinger <[email protected]>
---
arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi
index 281a12180703..b9d6284bb804 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi
@@ -194,6 +194,7 @@ &cpu_b1 {
};

&emmc_phy {
+ rockchip,enable-strobe-pulldown;
status = "okay";
};

@@ -648,7 +649,8 @@ &saradc {
&sdhci {
max-frequency = <150000000>;
bus-width = <8>;
- mmc-hs200-1_8v;
+ mmc-hs400-1_8v;
+ mmc-hs400-enhanced-strobe;
non-removable;
status = "okay";
};
--
2.44.0


2024-03-27 19:56:53

by Folker Schwesinger

[permalink] [raw]
Subject: [PATCH 2/2] arm64: dts: rockchip: Add enable-strobe-pulldown to emmc phy on ROCK 4C+

Commit 8b5c2b45b8f0 disabled the internal pull-down for the strobe line
causing I/O errors in HS400 mode for various eMMC modules.

Enable the internal strobe pull-down for the ROCK 4C+ board. Also re-enable
HS400 mode, that was replaced with HS200 mode as a workaround for the
stability issues in:
2bd1d2dd808c ("arm64: dts: rockchip: Disable HS400 for eMMC on ROCK 4C+").

Fixes: 8b5c2b45b8f0 ("phy: rockchip: set pulldown for strobe line in dts")
Signed-off-by: Folker Schwesinger <[email protected]>
---
arch/arm64/boot/dts/rockchip/rk3399-rock-4c-plus.dts | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rock-4c-plus.dts b/arch/arm64/boot/dts/rockchip/rk3399-rock-4c-plus.dts
index 7baf9d1b22fd..972aea843afd 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-rock-4c-plus.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-rock-4c-plus.dts
@@ -151,6 +151,7 @@ &cpu_b1 {
};

&emmc_phy {
+ rockchip,enable-strobe-pulldown;
status = "okay";
};

@@ -549,7 +550,8 @@ &saradc {
&sdhci {
max-frequency = <150000000>;
bus-width = <8>;
- mmc-hs200-1_8v;
+ mmc-hs400-1_8v;
+ mmc-hs400-enhanced-strobe;
non-removable;
status = "okay";
};
--
2.44.0


2024-03-28 05:39:25

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH 1/2] arm64: dts: rockchip: Add enable-strobe-pulldown to emmc phy on ROCK Pi 4

On Thu, Mar 28, 2024 at 4:33 AM Folker Schwesinger
<[email protected]> wrote:
>
> Commit 8b5c2b45b8f0 disabled the internal pull-down for the strobe line
> causing I/O errors in HS400 mode for various eMMC modules.
>
> Enable the internal strobe pull-down for ROCK Pi 4 boards. Also re-enable
> HS400 mode, that was replaced with HS200 mode as a workaround for the
> stability issues in:
> cee572756aa2 ("arm64: dts: rockchip: Disable HS400 for eMMC on ROCK Pi 4").
>
> This was tested on ROCK 4SE and ROCK Pi 4B+.
>
> Fixes: 8b5c2b45b8f0 ("phy: rockchip: set pulldown for strobe line in dts")
> Signed-off-by: Folker Schwesinger <[email protected]>
> ---
> arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi
> index 281a12180703..b9d6284bb804 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi
> @@ -194,6 +194,7 @@ &cpu_b1 {
> };
>
> &emmc_phy {
> + rockchip,enable-strobe-pulldown;
> status = "okay";
> };
>
> @@ -648,7 +649,8 @@ &saradc {
> &sdhci {
> max-frequency = <150000000>;
> bus-width = <8>;
> - mmc-hs200-1_8v;

Shouldn't this be kept around? The node should list all supported modes,
not just the highest one. Same for the other patch.

ChenYu

> + mmc-hs400-1_8v;
> + mmc-hs400-enhanced-strobe;
> non-removable;
> status = "okay";
> };
> --
> 2.44.0
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

2024-03-28 07:45:02

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH 1/2] arm64: dts: rockchip: Add enable-strobe-pulldown to emmc phy on ROCK Pi 4

On Thu, Mar 28, 2024 at 3:09 PM Folker Schwesinger
<[email protected]> wrote:
>
> On Thu Mar 28, 2024 at 6:39 AM CET, Chen-Yu Tsai wrote:
> > > @@ -648,7 +649,8 @@ &saradc {
> > > &sdhci {
> > > max-frequency <150000000>;
> > > bus-width <8>;
> > > - mmc-hs200-1_8v;
> >
> > Shouldn't this be kept around? The node should list all supported modes,
> > not just the highest one. Same for the other patch.
> >
>
> This is not necessary, mmc-hs400-1_8v implicitly activates the
> corresponding HS200 capability, see drivers/mmc/core/host.c:
>
> if (device_property_read_bool(dev, "mmc-hs200-1_8v"))
> host->caps2 | MMC_CAP2_HS200_1_8V_SDR;
> /* ... */
> if (device_property_read_bool(dev, "mmc-hs400-1_8v"))
> host->caps2 | MMC_CAP2_HS400_1_8V | MMC_CAP2_HS200_1_8V_SDR;

Looking at the initial commit for adding the hs400 properties,
it was also mentioned that hs400 support implies hs200 support.
It just wasn't explicitly spelled out in the bindings.

In that case, I think we're good here for this particular case.

> Kind regards
>
> Folker
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

2024-03-28 16:55:59

by Folker Schwesinger

[permalink] [raw]
Subject: Re: [PATCH 1/2] arm64: dts: rockchip: Add enable-strobe-pulldown to emmc phy on ROCK Pi 4

On Thu Mar 28, 2024 at 6:39 AM CET, Chen-Yu Tsai wrote:
> > @@ -648,7 +649,8 @@ &saradc {
> > &sdhci {
> > max-frequency 150000000>;
> > bus-width 8>;
> > - mmc-hs200-1_8v;
>
> Shouldn't this be kept around? The node should list all supported modes,
> not just the highest one. Same for the other patch.
>

This is not necessary, mmc-hs400-1_8v implicitly activates the
corresponding HS200 capability, see drivers/mmc/core/host.c:

if (device_property_read_bool(dev, "mmc-hs200-1_8v"))
host->caps2 |MC_CAP2_HS200_1_8V_SDR;
/* ... */
if (device_property_read_bool(dev, "mmc-hs400-1_8v"))
host->caps2 |MC_CAP2_HS400_1_8V | MMC_CAP2_HS200_1_8V_SDR;

Kind regards

Folker


Attachments:
signature.asc (273.00 B)

2024-03-31 19:15:20

by Dragan Simic

[permalink] [raw]
Subject: Re: [PATCH 2/2] arm64: dts: rockchip: Add enable-strobe-pulldown to emmc phy on ROCK 4C+

On 2024-03-27 20:26, Folker Schwesinger wrote:
> Commit 8b5c2b45b8f0 disabled the internal pull-down for the strobe line
> causing I/O errors in HS400 mode for various eMMC modules.
>
> Enable the internal strobe pull-down for the ROCK 4C+ board. Also
> re-enable
> HS400 mode, that was replaced with HS200 mode as a workaround for the
> stability issues in:
> 2bd1d2dd808c ("arm64: dts: rockchip: Disable HS400 for eMMC on ROCK
> 4C+").
>
> Fixes: 8b5c2b45b8f0 ("phy: rockchip: set pulldown for strobe line in
> dts")
> Signed-off-by: Folker Schwesinger <[email protected]>

Looking good to me.

Reviewed-by: Dragan Simic <[email protected]>

> ---
> arch/arm64/boot/dts/rockchip/rk3399-rock-4c-plus.dts | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rock-4c-plus.dts
> b/arch/arm64/boot/dts/rockchip/rk3399-rock-4c-plus.dts
> index 7baf9d1b22fd..972aea843afd 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-rock-4c-plus.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-rock-4c-plus.dts
> @@ -151,6 +151,7 @@ &cpu_b1 {
> };
>
> &emmc_phy {
> + rockchip,enable-strobe-pulldown;
> status = "okay";
> };
>
> @@ -549,7 +550,8 @@ &saradc {
> &sdhci {
> max-frequency = <150000000>;
> bus-width = <8>;
> - mmc-hs200-1_8v;
> + mmc-hs400-1_8v;
> + mmc-hs400-enhanced-strobe;
> non-removable;
> status = "okay";
> };

2024-03-31 19:22:53

by Dragan Simic

[permalink] [raw]
Subject: Re: [PATCH 1/2] arm64: dts: rockchip: Add enable-strobe-pulldown to emmc phy on ROCK Pi 4

On 2024-03-27 20:26, Folker Schwesinger wrote:
> Commit 8b5c2b45b8f0 disabled the internal pull-down for the strobe line
> causing I/O errors in HS400 mode for various eMMC modules.
>
> Enable the internal strobe pull-down for ROCK Pi 4 boards. Also
> re-enable
> HS400 mode, that was replaced with HS200 mode as a workaround for the
> stability issues in:
> cee572756aa2 ("arm64: dts: rockchip: Disable HS400 for eMMC on ROCK Pi
> 4").
>
> This was tested on ROCK 4SE and ROCK Pi 4B+.
>
> Fixes: 8b5c2b45b8f0 ("phy: rockchip: set pulldown for strobe line in
> dts")
> Signed-off-by: Folker Schwesinger <[email protected]>

Looking good to me.

Reviewed-by: Dragan Simic <[email protected]>

> ---
> arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi
> b/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi
> index 281a12180703..b9d6284bb804 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-rock-pi-4.dtsi
> @@ -194,6 +194,7 @@ &cpu_b1 {
> };
>
> &emmc_phy {
> + rockchip,enable-strobe-pulldown;
> status = "okay";
> };
>
> @@ -648,7 +649,8 @@ &saradc {
> &sdhci {
> max-frequency = <150000000>;
> bus-width = <8>;
> - mmc-hs200-1_8v;
> + mmc-hs400-1_8v;
> + mmc-hs400-enhanced-strobe;
> non-removable;
> status = "okay";
> };

2024-04-01 21:56:19

by Heiko Stübner

[permalink] [raw]
Subject: Re: [PATCH 0/2] Enable internal strobe pulldown on Radxa ROCK 4 SBCs

On Wed, 27 Mar 2024 20:26:36 +0100, Folker Schwesinger wrote:
> Various RK3399 boards, including the ROCK Pi 4 series showed instabilities
> with some eMMC modules when operating in HS400 mode [1]. Later, a missing
> pull-down resistor on the eMMC strobe line was identified as the root
> cause of these instabilities [2].
>
> This series enables the internal pull-down on the eMMC PHY strobe line
> for all ROCK 4 series boards as they all lack an external strobe
> pull-down resistor.
>
> [...]

Applied, thanks!

[1/2] arm64: dts: rockchip: Add enable-strobe-pulldown to emmc phy on ROCK Pi 4
commit: f720dd9b8b6d8b2160beda789429d5489ce8a099
[2/2] arm64: dts: rockchip: Add enable-strobe-pulldown to emmc phy on ROCK 4C+
commit: c1b1f340dd7db11f273e426e110697551c9f501f

Best regards,
--
Heiko Stuebner <[email protected]>