diff mbox series

[v2] arm64: dts: rockchip: Fix SD card init on rk3399-nanopi4

Message ID A9634366-A012-43D2-B253-8BB9BF6005C7@kohlschutter.com (mailing list archive)
State New, archived
Headers show
Series [v2] arm64: dts: rockchip: Fix SD card init on rk3399-nanopi4 | expand

Commit Message

Christian Kohlschütter July 14, 2022, 4:26 p.m. UTC
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(-)

Comments

Christian Loehle July 14, 2022, 4:44 p.m. UTC | #1
I only briefly skimmed the discussion, but does this mean that a soft-reset (CMD0) of a UHS (post-voltage-switch) will not work?
(As the card/spec requires a power-cycle by the host which will not come, right?)
Can you try this real quick? I can give you a mmc-utils snippet if you have trouble issuing one.
If that does indeed not work I think the general approach is to disable uhs in the dts or at least document that.
Regards,
Christian

-----Original Message-----
From: Christian Kohlschütter <christian@kohlschutter.com> 
Sent: Donnerstag, 14. Juli 2022 18:27
To: Robin Murphy <robin.murphy@arm.com>; Markus Reichl <m.reichl@fivetechno.de>; Heiko Stübner <heiko@sntech.de>; linux-arm-kernel@lists.infradead.org; linux-rockchip@lists.infradead.org; linux-kernel@vger.kernel.org; Linux MMC List <linux-mmc@vger.kernel.org>
Subject: [PATCH v2] arm64: dts: rockchip: Fix SD card init on rk3399-nanopi4

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
 		pinctrl-names = "default";
 		pinctrl-0 = <&sdmmc0_pwr_h>;
 		regulator-always-on;
Chen-Yu Tsai July 14, 2022, 5:02 p.m. UTC | #2
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
Christian Kohlschütter July 14, 2022, 5:20 p.m. UTC | #3
> Am 14.07.2022 um 18:44 schrieb Christian Loehle <CLoehle@hyperstone.com>:
> 
> I only briefly skimmed the discussion, but does this mean that a soft-reset (CMD0) of a UHS (post-voltage-switch) will not work?
> (As the card/spec requires a power-cycle by the host which will not come, right?)
> Can you try this real quick? I can give you a mmc-utils snippet if you have trouble issuing one.
> If that does indeed not work I think the general approach is to disable uhs in the dts or at least document that.
> Regards,
> Christian

I tried disabling UHS in the DTS, but that would still cause mmc detection issues.

"rmmod dw_mmc_rockchip" followed by "modprobe dw_mmc_rockchip" still detects the card:
> [ 4481.141764] mmc1: card 0001 removed
> [ 4488.133398] dwmmc_rockchip fe320000.mmc: IDMAC supports 32-bit address mode.
> [ 4488.133462] dwmmc_rockchip fe320000.mmc: Using internal DMA controller.
> [ 4488.133484] dwmmc_rockchip fe320000.mmc: Version ID is 270a
> [ 4488.133541] dwmmc_rockchip fe320000.mmc: DW MMC controller at irq 32,32 bit host data width,256 deep fifo
> [ 4488.134320] dwmmc_rockchip fe320000.mmc: Got CD GPIO
> [ 4488.147329] mmc_host mmc1: Bus speed (slot 0) = 400000Hz (slot req 400000Hz, actual 400000HZ div = 0)
> [ 4488.218364] mmc_host mmc1: Bus speed (slot 0) = 148500000Hz (slot req 150000000Hz, actual 148500000HZ div = 0)
> [ 4488.678181] dwmmc_rockchip fe320000.mmc: Successfully tuned phase to 214
> [ 4488.678239] mmc1: new ultra high speed SDR104 SDHC card at address 0001
> [ 4488.680315] mmcblk1: mmc1:0001 ASTC 14.6 GiB 
> [ 4488.684871]  mmcblk1: p1 p2

Ejecting/re-inserting the card also works:
> [ 4607.521119] mmc1: card 0001 removed
> [ 4608.517343] mmc_host mmc1: Bus speed (slot 0) = 400000Hz (slot req 400000Hz, actual 400000HZ div = 0)
> [ 4608.632987] mmc_host mmc1: Bus speed (slot 0) = 148500000Hz (slot req 150000000Hz, actual 148500000HZ div = 0)
> [ 4609.065445] dwmmc_rockchip fe320000.mmc: Successfully tuned phase to 213
> [ 4609.065535] mmc1: new ultra high speed SDR104 SDHC card at address 0001
> [ 4609.067942] mmcblk1: mmc1:0001 ASTC 14.6 GiB 
> [ 4609.073521]  mmcblk1: p1 p2

and so is changing the clock back and forth:
> echo 400000 > /sys/kernel/debug/mmc1/clock; echo 150000000 > kernel/debug/mmc1/clock; fdisk -l /dev/mmcblk1

> [ 4817.829078] mmc_host mmc1: Bus speed (slot 0) = 400000Hz (slot req 400000Hz, actual 400000HZ div = 0)
> [ 4820.063457] mmc_host mmc1: Bus speed (slot 0) = 148500000Hz (slot req 150000000Hz, actual 148500000HZ div = 0)
> [ 4835.305419] dwmmc_rockchip fe320000.mmc: Successfully tuned phase to 213
> [ 4836.346928]  mmcblk1: p1 p2

Swapping with a "highspeed" (non-UHS) card also seems to work
> [ 5733.702083] mmc1: card 0001 removed
> [ 5738.858439] mmc_host mmc1: Bus speed (slot 0) = 400000Hz (slot req 400000Hz, actual 400000HZ div = 0)
> [ 5739.378487] mmc_host mmc1: Bus speed (slot 0) = 50000000Hz (slot req 50000000Hz, actual 50000000HZ div = 0)
> [ 5739.378627] mmc1: new high speed SD card at address 21bb
> [ 5739.380491] mmcblk1: mmc1:21bb APPSD 480 MiB 
> [ 5739.382795] debugfs: Directory 'mmcblk1' with parent 'block' already present!
> [ 5739.385096]  mmcblk1: p1
> [ 5774.386536] FAT-fs (mmcblk1p1): utf8 is not a recommended IO charset for FAT filesystems, filesystem will be case sensitive!
> [ 5795.486365] mmc1: card 21bb removed
> [ 5801.302688] mmc_host mmc1: Bus speed (slot 0) = 400000Hz (slot req 400000Hz, actual 400000HZ div = 0)
> [ 5801.447128] mmc_host mmc1: Bus speed (slot 0) = 148500000Hz (slot req 150000000Hz, actual 148500000HZ div = 0)
> [ 5801.880374] dwmmc_rockchip fe320000.mmc: Successfully tuned phase to 211
> [ 5801.880440] mmc1: new ultra high speed SDR104 SDHC card at address 0001
> [ 5801.882253] mmcblk1: mmc1:0001 ASTC 14.6 GiB 
> [ 5801.884145] debugfs: Directory 'mmcblk1' with parent 'block' already present!
> [ 5801.886558]  mmcblk1: p1 p2
> 

Some debug output: cat /sys/kernel/debug/mmc1/ios
UHC
> clock:		150000000 Hz
> actual clock:	148500000 Hz
> vdd:		18 (3.0 ~ 3.1 V)
> bus mode:	2 (push-pull)
> chip select:	0 (don't care)
> power mode:	2 (on)
> bus width:	2 (4 bits)
> timing spec:	6 (sd uhs SDR104)
> signal voltage:	1 (1.80 V)
> driver type:	0 (driver type B)
non-UHC
> cat /sys/kernel/debug/mmc1/ios
> clock:		50000000 Hz
> vdd:		18 (3.0 ~ 3.1 V)
> bus mode:	2 (push-pull)
> chip select:	0 (don't care)
> power mode:	2 (on)
> bus width:	2 (4 bits)
> timing spec:	2 (sd high-speed)
> signal voltage:	0 (3.30 V)
> driver type:	0 (driver type B)
> 

How do I make sure I specifically send the soft-reset command? I'm happy to help but I'm really a novice here.

Best,
Christian
Robin Murphy July 14, 2022, 5:35 p.m. UTC | #4
On 14/07/2022 6:02 pm, Chen-Yu Tsai wrote:
> 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.

Hmm, that's a good point - by the time we get to Linux, we should have 
control over the VCC_SDIO regulator and the I/O domain as well, so a 
full clean reset should really be no problem :/

The "Tinkerboard problem" I was thinking of is when the SD card is the 
boot medium, VCC_SDIO stays at 1.8V over a reboot but the I/O domain 
gets reset back to 3.3V mode, thus cannot see a logic high on any of the 
I/O lines, thus the boot ROM gives up after failing to detect the card. 
If we're still able to boot as far as Linux, it's probably a different 
thing. Apologies for the confusion.

>>                  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.

Indeed, if it's a slow regulator and we're simply trying to probe the 
card too soon before its supply voltage has actually stabilised, that 
sounds entirely plausible, particularly if the failure is only intermittent.

Thanks,
Robin.
Christian Kohlschütter July 14, 2022, 5:57 p.m. UTC | #5
Am 14.07.2022 um 19:35 schrieb Robin Murphy <robin.murphy@arm.com>:
> 
> On 14/07/2022 6:02 pm, Chen-Yu Tsai wrote:
>> 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.

This is the current situation in the mainline kernel (regulator-always-on is there and unchanged), but yes, you are correct that's a bug.

See below for my results with your proposed changes.

>> 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.
> 
> Hmm, that's a good point - by the time we get to Linux, we should have control over the VCC_SDIO regulator and the I/O domain as well, so a full clean reset should really be no problem :/
> 
> The "Tinkerboard problem" I was thinking of is when the SD card is the boot medium, VCC_SDIO stays at 1.8V over a reboot but the I/O domain gets reset back to 3.3V mode, thus cannot see a logic high on any of the I/O lines, thus the boot ROM gives up after failing to detect the card. If we're still able to boot as far as Linux, it's probably a different thing. Apologies for the confusion.

I believe it's that problem you describe.

In my case, I boot off the mmc card with u-boot mainline v2022.07, which invokes IPXE that boots the kernel (5.18.10 mainline) + rk3399-nanopi-r4s dtb over the network. 
This lets me quickly test kernel changes without juggling sd-cards.

> 
>>>                 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.
> 
> Indeed, if it's a slow regulator and we're simply trying to probe the card too soon before its supply voltage has actually stabilised, that sounds entirely plausible, particularly if the failure is only intermittent.

With
> gpio = <&gpio0 RK_PA1 GPIO_ACTIVE_HIGH>;
> // regulator-always-on;
> regulator-enable-ramp-delay = <50>;
I'm still not getting the reset going. It looks the MMC gets detected reliably upon cold start, but I cannot test reboot/warm start.

It looks like there's one issue upon start (which MAY be fixed by these changes), but there's another issue upon restart ("suspend" in device tree lingo?).
I tried both

> regulator-state-mem {
>         regulator-off-in-suspend;
> };    
and
> regulator-state-mem {
>         regulator-on-in-suspend;
> };
to no avail (no difference; reboot still hangs).

What are we missing?

Best,
Christian
Robin Murphy July 14, 2022, 11:44 p.m. UTC | #6
On 2022-07-14 18:35, Robin Murphy wrote:
> On 14/07/2022 6:02 pm, Chen-Yu Tsai wrote:
>> 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.
> 
> Hmm, that's a good point - by the time we get to Linux, we should have 
> control over the VCC_SDIO regulator and the I/O domain as well, so a 
> full clean reset should really be no problem :/
> 
> The "Tinkerboard problem" I was thinking of is when the SD card is the 
> boot medium, VCC_SDIO stays at 1.8V over a reboot but the I/O domain 
> gets reset back to 3.3V mode, thus cannot see a logic high on any of the 
> I/O lines, thus the boot ROM gives up after failing to detect the card. 
> If we're still able to boot as far as Linux, it's probably a different 
> thing. Apologies for the confusion.
> 
>>>                  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.
> 
> Indeed, if it's a slow regulator and we're simply trying to probe the 
> card too soon before its supply voltage has actually stabilised, that 
> sounds entirely plausible, particularly if the failure is only 
> intermittent.

For giggles, I scoped it on my NanoPC-T4. It actually takes about 60 
microseconds from asserting the enable to reach 3V, but then seems to 
take about another 100 after that to truly stabilise (the load in that 
case was a no-name 8GB high speed micro-SDHC). Thus if I wanted to test 
further I'd probably first try a significantly longer delay in the order 
of milliseconds to see whether that has any effect.

However I also gave it a bit of a stress test by removing 
regulator-always-on then repeatedly unbinding and rebinding the MMC 
driver to cycle the regulator and re-detect the card, and none of the 
cards I have to hand (from a fancy Sandisk Extreme Plus to a full-size 
128MB Lexar from about 2005 hanging off a comedy 60cm adapter cable) 
seemed to mind. So if something is up with R4S, I think it's fair to say 
that it doesn't affect all nanopi4 boards.

Cheers,
Robin.
Christian Loehle July 15, 2022, 5:02 p.m. UTC | #7
Please try my softreset patch for mmc-utils.

If some UHS cards have trouble coming up again then we have a problem on that hardware.

-----Original Message-----
From: Christian Kohlschütter <christian@kohlschutter.com> 
Sent: Donnerstag, 14. Juli 2022 19:21
To: Christian Loehle <CLoehle@hyperstone.com>
Cc: Robin Murphy <robin.murphy@arm.com>; Markus Reichl <m.reichl@fivetechno.de>; Heiko Stübner <heiko@sntech.de>; linux-arm-kernel@lists.infradead.org; linux-rockchip@lists.infradead.org; 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

> Am 14.07.2022 um 18:44 schrieb Christian Loehle <CLoehle@hyperstone.com>:
> 
> I only briefly skimmed the discussion, but does this mean that a soft-reset (CMD0) of a UHS (post-voltage-switch) will not work?
> (As the card/spec requires a power-cycle by the host which will not come, right?)
> Can you try this real quick? I can give you a mmc-utils snippet if you have trouble issuing one.
> If that does indeed not work I think the general approach is to disable uhs in the dts or at least document that.
> Regards,
> Christian

I tried disabling UHS in the DTS, but that would still cause mmc detection issues.

"rmmod dw_mmc_rockchip" followed by "modprobe dw_mmc_rockchip" still detects the card:
> [ 4481.141764] mmc1: card 0001 removed
> [ 4488.133398] dwmmc_rockchip fe320000.mmc: IDMAC supports 32-bit address mode.
> [ 4488.133462] dwmmc_rockchip fe320000.mmc: Using internal DMA controller.
> [ 4488.133484] dwmmc_rockchip fe320000.mmc: Version ID is 270a
> [ 4488.133541] dwmmc_rockchip fe320000.mmc: DW MMC controller at irq 32,32 bit host data width,256 deep fifo
> [ 4488.134320] dwmmc_rockchip fe320000.mmc: Got CD GPIO
> [ 4488.147329] mmc_host mmc1: Bus speed (slot 0) = 400000Hz (slot req 400000Hz, actual 400000HZ div = 0)
> [ 4488.218364] mmc_host mmc1: Bus speed (slot 0) = 148500000Hz (slot req 150000000Hz, actual 148500000HZ div = 0)
> [ 4488.678181] dwmmc_rockchip fe320000.mmc: Successfully tuned phase to 214
> [ 4488.678239] mmc1: new ultra high speed SDR104 SDHC card at address 0001
> [ 4488.680315] mmcblk1: mmc1:0001 ASTC 14.6 GiB 
> [ 4488.684871]  mmcblk1: p1 p2

Ejecting/re-inserting the card also works:
> [ 4607.521119] mmc1: card 0001 removed
> [ 4608.517343] mmc_host mmc1: Bus speed (slot 0) = 400000Hz (slot req 400000Hz, actual 400000HZ div = 0)
> [ 4608.632987] mmc_host mmc1: Bus speed (slot 0) = 148500000Hz (slot req 150000000Hz, actual 148500000HZ div = 0)
> [ 4609.065445] dwmmc_rockchip fe320000.mmc: Successfully tuned phase to 213
> [ 4609.065535] mmc1: new ultra high speed SDR104 SDHC card at address 0001
> [ 4609.067942] mmcblk1: mmc1:0001 ASTC 14.6 GiB 
> [ 4609.073521]  mmcblk1: p1 p2

and so is changing the clock back and forth:
> echo 400000 > /sys/kernel/debug/mmc1/clock; echo 150000000 > kernel/debug/mmc1/clock; fdisk -l /dev/mmcblk1

> [ 4817.829078] mmc_host mmc1: Bus speed (slot 0) = 400000Hz (slot req 400000Hz, actual 400000HZ div = 0)
> [ 4820.063457] mmc_host mmc1: Bus speed (slot 0) = 148500000Hz (slot req 150000000Hz, actual 148500000HZ div = 0)
> [ 4835.305419] dwmmc_rockchip fe320000.mmc: Successfully tuned phase to 213
> [ 4836.346928]  mmcblk1: p1 p2

Swapping with a "highspeed" (non-UHS) card also seems to work
> [ 5733.702083] mmc1: card 0001 removed
> [ 5738.858439] mmc_host mmc1: Bus speed (slot 0) = 400000Hz (slot req 400000Hz, actual 400000HZ div = 0)
> [ 5739.378487] mmc_host mmc1: Bus speed (slot 0) = 50000000Hz (slot req 50000000Hz, actual 50000000HZ div = 0)
> [ 5739.378627] mmc1: new high speed SD card at address 21bb
> [ 5739.380491] mmcblk1: mmc1:21bb APPSD 480 MiB 
> [ 5739.382795] debugfs: Directory 'mmcblk1' with parent 'block' already present!
> [ 5739.385096]  mmcblk1: p1
> [ 5774.386536] FAT-fs (mmcblk1p1): utf8 is not a recommended IO charset for FAT filesystems, filesystem will be case sensitive!
> [ 5795.486365] mmc1: card 21bb removed
> [ 5801.302688] mmc_host mmc1: Bus speed (slot 0) = 400000Hz (slot req 400000Hz, actual 400000HZ div = 0)
> [ 5801.447128] mmc_host mmc1: Bus speed (slot 0) = 148500000Hz (slot req 150000000Hz, actual 148500000HZ div = 0)
> [ 5801.880374] dwmmc_rockchip fe320000.mmc: Successfully tuned phase to 211
> [ 5801.880440] mmc1: new ultra high speed SDR104 SDHC card at address 0001
> [ 5801.882253] mmcblk1: mmc1:0001 ASTC 14.6 GiB 
> [ 5801.884145] debugfs: Directory 'mmcblk1' with parent 'block' already present!
> [ 5801.886558]  mmcblk1: p1 p2
> 

Some debug output: cat /sys/kernel/debug/mmc1/ios
UHC
> clock:		150000000 Hz
> actual clock:	148500000 Hz
> vdd:		18 (3.0 ~ 3.1 V)
> bus mode:	2 (push-pull)
> chip select:	0 (don't care)
> power mode:	2 (on)
> bus width:	2 (4 bits)
> timing spec:	6 (sd uhs SDR104)
> signal voltage:	1 (1.80 V)
> driver type:	0 (driver type B)
non-UHC
> cat /sys/kernel/debug/mmc1/ios
> clock:		50000000 Hz
> vdd:		18 (3.0 ~ 3.1 V)
> bus mode:	2 (push-pull)
> chip select:	0 (don't care)
> power mode:	2 (on)
> bus width:	2 (4 bits)
> timing spec:	2 (sd high-speed)
> signal voltage:	0 (3.30 V)
> driver type:	0 (driver type B)
> 

How do I make sure I specifically send the soft-reset command? I'm happy to help but I'm really a novice here.

Best,
Christian

Hyperstone GmbH | Reichenaustr. 39a  | 78467 Konstanz
Managing Director: Dr. Jan Peter Berns.
Commercial register of local courts: Freiburg HRB381782
diff mbox series

Patch

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
 		pinctrl-names = "default";
 		pinctrl-0 = <&sdmmc0_pwr_h>;
 		regulator-always-on;