Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Content-Type: text/plain;
        charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3696.100.31\))
Subject: Re: [PATCH] arm64: dts: rockchip: Fix SD card init on rk3399-nanopi4
From:   =?utf-8?Q?Christian_Kohlsch=C3=BCtter?= 
        <christian@kohlschutter.com>
In-Reply-To: <103b714c-b07c-f016-1062-84bd94786b22@arm.com>
Date:   Thu, 14 Jul 2022 14:14:04 +0200
Cc:     =?utf-8?Q?Heiko_St=C3=BCbner?= <heiko@sntech.de>,
        linux-arm-kernel@lists.infradead.org,
        linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org
Content-Transfer-Encoding: quoted-printable
Message-Id: <9AF1E75F-5947-49B0-887D-82C426527B99@kohlschutter.com>
References: <C639AD88-77A1-4485-BAEA-2FF8FC15A844@kohlschutter.com>
 <12878108.O9o76ZdvQC@diego> <103b714c-b07c-f016-1062-84bd94786b22@arm.com>
To:     Robin Murphy <robin.murphy@arm.com>
Precedence: bulk

> Am 14.07.2022 um 13:41 schrieb Robin Murphy <robin.murphy@arm.com>:
>=20
> On 2022-07-14 00:41, Heiko St=C3=BCbner wrote:
>> Hi Christian,
>> Am Donnerstag, 14. Juli 2022, 00:22:23 CEST schrieb Christian =
Kohlsch=C3=BCtter:
>>> mmc/SD-card initialization may sometimes fail on NanoPi r4s with
>>> "mmc1: problem reading SD Status register" /
>>> "mmc1: error -110 whilst initialising SD card"
>>>=20
>>> Moreover, rebooting would also sometimes hang.
>>>=20
>> Nit: here the commit message should continue with something like:
>> -----
>> This is caused by the vcc3v0-sd regulator referencing the wrong gpio.
>> Fix the regulator to use the correct pin and drop the always-on =
property.
>> -----
>>> Signed-off-by: Christian Kohlsch=C3=BCtter =
<christian@kohlschutter.com>
>>> ---
>>>  arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi | 6 +++---
>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>=20
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi =
b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
>>> index 8c0ff6c96e03..91789801ab03 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
>>> @@ -67,10 +67,10 @@ vcc1v8_s3: vcc1v8-s3 {
>>>  	vcc3v0_sd: vcc3v0-sd {
>>>  		compatible =3D "regulator-fixed";
>>>  		enable-active-high;
>>> -		gpio =3D <&gpio0 RK_PA1 GPIO_ACTIVE_HIGH>;
>>> +		gpio =3D <&gpio0 RK_PD6 GPIO_ACTIVE_HIGH>;
>> The interesting question would be how nano-pi-specific that gpio is.
>> I.e. this is the rk3399-nanopi4.dtsi that is shared by multiple board =
types,
>> so can you check in schematics if gpio0-d6 is always used on all of =
them?
>=20
> On the R4S schematic, this is GPIO0_A1 same as the others. GPIO0 =
doesn't even have a bank D on RK3399, it only goes up to B5 :/
>=20
>> Thanks
>> Heiko
>>>  		pinctrl-names =3D "default";
>>>  		pinctrl-0 =3D <&sdmmc0_pwr_h>;
>>> -		regulator-always-on;
>>> +		regulator-boot-on;
>>>  		regulator-min-microvolt =3D <3000000>;
>>>  		regulator-max-microvolt =3D <3000000>;
>>>  		regulator-name =3D "vcc3v0_sd";
>>> @@ -580,7 +580,7 @@ wifi_reg_on_h: wifi-reg_on-h {
>>>    	sdmmc {
>>>  		sdmmc0_det_l: sdmmc0-det-l {
>>> -			rockchip,pins =3D <0 RK_PA7 RK_FUNC_GPIO =
&pcfg_pull_up>;
>>> +			rockchip,pins =3D <0 RK_PD6 RK_FUNC_GPIO =
&pcfg_pull_up>;
>=20
> ...and claiming the card detect is on the same non-existent pin as the =
regulator enable is clearly even more wrong.
>=20
> Is this another case where a UHS card is involved, such that VCC_SDIO =
gets left at 1.8V after a reboot, so subsequent attempts to do the =
initial handshake where the card is expecting 3V logic levels fail? (AKA =
the Tinkerboard problem). Hobbling the regulator such that Linux can =
never actually turn VCC3V0_SD off, thus the card never gets reset, might =
appear to "work", but isn't the right thing to do.
>=20
> Robin.

Right, this is all very strange.=20

Indeed, I have a UHS card and the problem you describe.
I've actually looked into the other RK3399 dts files to come up with =
that patch (e.g. rk3399-roc-pc.dtsi, which does this; also see [1])

I understand that we should do the right thing, but I am by no means =
sure what the "right" thing for this problem is.
That said, I wished someone with expertise and authority in the =
mmc/rockchip community would fix this for good.

There are several patches around this 1.8V/3V voltage dance that do =
work, get used by several distributions but didn't get merged to =
mainline because, well, there could be a better right thing to do (like =
[2], which is also still needed when using mainline u-boot).

Given the state of this issue, at least aligning the code to match =
another board's fix =E2=80=94 which already is in the mainline kernel =
=E2=80=94 seems a sensible approach to me. It unblocks users like me who =
would perhaps otherwise just give up on using these devices.

[1] =
https://lore.kernel.org/linux-mmc/3364813.APbW32NlgJ@phil/T/#m1b23cd2c43b6=
00b5a6f084461ae53e482ad65316
[2] =
https://patchwork.kernel.org/project/linux-mmc/patch/AM3PR03MB09664161A7FA=
2BD68B2800A7AC620@AM3PR03MB0966.eurprd03.prod.outlook.com/