Re: [PATCH v2] arm64: dts: rockchip: Fix SD card init on rk3399-nanopi4 - Chen-Yu Tsai

From: Chen-Yu Tsai <wens@kernel.org>
To: "Christian Kohlschütter" <christian@kohlschutter.com>
Cc: "Robin Murphy" <robin.murphy@arm.com>,
	"Markus Reichl" <m.reichl@fivetechno.de>,
	"Heiko Stübner" <heiko@sntech.de>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	"open list:ARM/Rockchip SoC..."
	<linux-rockchip@lists.infradead.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	"Linux MMC List" <linux-mmc@vger.kernel.org>
Subject: Re: [PATCH v2] arm64: dts: rockchip: Fix SD card init on rk3399-nanopi4
Date: Fri, 15 Jul 2022 01:02:02 +0800	[thread overview]
Message-ID: <CAGb2v65Ehbu1wrib2CzF1fDZuD3fHZQDhKfVusyUF9KnxTvi+Q@mail.gmail.com> (raw)
In-Reply-To: <A9634366-A012-43D2-B253-8BB9BF6005C7@kohlschutter.com>

On Fri, Jul 15, 2022 at 12:27 AM Christian Kohlschütter
<christian@kohlschutter.com> wrote:
>
> mmc/SD-card initialization may fail on NanoPi r4s with
> "mmc1: problem reading SD Status register" /
> "mmc1: error -110 whilst initialising SD card"
>
> Moreover, rebooting would also sometimes hang.
>
> This is caused by the gpio entry for the vcc3v0-sd regulator;
> even though it appears to be the correct GPIO pin, the presence
> of the binding causes these errors.
>
> Fix the regulator to drop the gpio binding and add a comment
> to prevent accidental reintroduction of that entry.
>
> Signed-off-by: Christian Kohlschütter <christian@kohlschutter.com>
> ---
>  arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
> index 8c0ff6c96e03..d5f8a62e01be 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
> @@ -67,7 +67,7 @@ vcc1v8_s3: vcc1v8-s3 {
>         vcc3v0_sd: vcc3v0-sd {
>                 compatible = "regulator-fixed";
>                 enable-active-high;
> -               gpio = <&gpio0 RK_PA1 GPIO_ACTIVE_HIGH>;
> +               // gpio = <&gpio0 RK_PA1 GPIO_ACTIVE_HIGH>; // breaks SDHC card support

This change only means that the regulator no longer gets cycled when
it probes. It's not a proper fix. You're leaving the kernel without
any control over SD card power, and with whatever state the bootloader
left the GPIO in. If the bootloader left the GPIO low, then you don't
get to use the SD card, ever.

It cycles because of the lack of regulator-boot-on, so the driver
requests the GPIO with initial low state, and then drives it
high to enable the regulator.

>                 pinctrl-names = "default";
>                 pinctrl-0 = <&sdmmc0_pwr_h>;
>                 regulator-always-on;

I think dropping "regulator-always-on" so that Linux can cycle power
and properly reset the SD card is the proper fix to the card being
stuck in UHS and not responding.

Also, the regulator used is RT9193, according to the schematics. That
chip has an enable delay under 50 micro-seconds. If that needs to be
modeled, then add regulator-enable-ramp-delay.

Regards
ChenYu

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

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