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 <[email protected]>
---
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;
--
2.36.1
On Fri, Jul 15, 2022 at 12:27 AM Christian Kohlschütter
<[email protected]> 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 <[email protected]>
> ---
> 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
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 14/07/2022 6:02 pm, Chen-Yu Tsai wrote:
> On Fri, Jul 15, 2022 at 12:27 AM Christian Kohlschütter
> <[email protected]> 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 <[email protected]>
>> ---
>> 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.
Am 14.07.2022 um 19:35 schrieb Robin Murphy <[email protected]>:
>
> On 14/07/2022 6:02 pm, Chen-Yu Tsai wrote:
>> On Fri, Jul 15, 2022 at 12:27 AM Christian Kohlschütter
>> <[email protected]> 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 <[email protected]>
>>> ---
>>> 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
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
>> <[email protected]> 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 <[email protected]>
>>> ---
>>> 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.
mmc/SD-card initialization may fail on NanoPi R4S with
"mmc1: problem reading SD Status register" /
"mmc1: error -110 whilst initialising SD card"
either on cold boot or after a reboot.
Moreover, the system would also sometimes hang upon reboot.
This is prevented by setting an explicit undervoltage protection limit
for the SD-card-specific vcc3v0_sd voltage regulator.
While using a limit of 3V seems to work, an additional safety buffer
should prevent accidental tripping, preventing a system hang.
Set the undervoltage protection limit to 2.7V, which is the minimum
permissible SD card operating voltage.
Signed-off-by: Christian Kohlschütter <[email protected]>
---
arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi | 4 ++++
1 file changed, 4 insertions(+)
mode change 100644 => 100755 arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
old mode 100644
new mode 100755
index 8c0ff6c96e03..669c74ce4d13
--- a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
@@ -73,6 +73,10 @@ vcc3v0_sd: vcc3v0-sd {
regulator-always-on;
regulator-min-microvolt = <3000000>;
regulator-max-microvolt = <3000000>;
+
+ // must be initialized or SD card may fail to initialize / system may hang
+ regulator-uv-protection-microvolt = <2700000>;
+
regulator-name = "vcc3v0_sd";
vin-supply = <&vcc3v3_sys>;
};
--
2.36.1
mmc/SD-card initialization may fail on NanoPi R4S with
"mmc1: problem reading SD Status register" /
"mmc1: error -110 whilst initialising SD card"
either on cold boot or after a reboot.
Moreover, the system would also sometimes hang upon reboot.
This is prevented by setting an explicit undervoltage protection limit
for the SD-card-specific vcc3v0_sd voltage regulator.
Set the undervoltage protection limit to 2.7V, which is the minimum
permissible SD card operating voltage.
Signed-off-by: Christian Kohlschütter <[email protected]>
---
arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi | 4 ++++
1 file changed, 4 insertions(+)
mode change 100644 => 100755 arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
old mode 100644
new mode 100755
index 8c0ff6c96e03..669c74ce4d13
--- a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
@@ -73,6 +73,10 @@ vcc3v0_sd: vcc3v0-sd {
regulator-always-on;
regulator-min-microvolt = <3000000>;
regulator-max-microvolt = <3000000>;
+
+ // must be configured or SD card may fail to initialize occasionally
+ regulator-uv-protection-microvolt = <2700000>;
+
regulator-name = "vcc3v0_sd";
vin-supply = <&vcc3v3_sys>;
};
--
2.36.1
Am 15.07.2022 um 20:57 schrieb Robin Murphy <[email protected]>:
>
> On 2022-07-15 19:11, Robin Murphy wrote:
>> On 2022-07-15 18:16, Christian Kohlschütter wrote:
>>> OK, this took me a while to figure out.
>>>
>>> When no undervoltage limit is configured, I can reliably trigger the initialization bug upon boot.
>>> When the limit is set to 3.0V, it rarely occurs, but just after I send the v3 patch, I was able to reproduce...
>> Well this has to be in the running for "weirdest placebo ever"... :/
>> All it actually seems to achieve is printing an error[1] (this is after all a tiny 5-pin fixed-voltage LDO regulator, not an intelligent PMIC), and if that makes an appreciable difference then there has to be some kind of weird timing condition at play. Maybe regulator_register() ends up turning it off and on again rapidly enough that the card sees a voltage brownout and glitches, and adding more delay by printing to the console somewhere in the middle gives it enough time to act as a proper power cycle with no ill effect?
>
> ...and apparently the answer is yes, it seems to be doing exactly that (see attached). But seemingly my SD cards don't mind, or maybe my T4 board happens to have more capacitance than Christian's R4S so my voltage dip isn't as bad, or both.
>
> So it seems like the solution here might indeed simply be to remove the regulator-always-on which doesn't seem to have any reason to be here anyway. Without that, the enable stays low until the MMC driver probes and claims it, which is then massively longer than the time it takes for VCC3V0_SD to ramp down completely.
>
> Robin.
Removing "regulator-always-on" has the effect that the system freezes upon reboot. There may well be another bug slumbering in the codebase that is circumvented by 1. adding a delay in the code and 2. not turning the regulator off upon shutdown.
On 2022-07-15 20:04, Christian Kohlschütter wrote:
> Am 15.07.2022 um 20:57 schrieb Robin Murphy <[email protected]>:
>>
>> On 2022-07-15 19:11, Robin Murphy wrote:
>>> On 2022-07-15 18:16, Christian Kohlschütter wrote:
>>>> OK, this took me a while to figure out.
>>>>
>>>> When no undervoltage limit is configured, I can reliably trigger the initialization bug upon boot.
>>>> When the limit is set to 3.0V, it rarely occurs, but just after I send the v3 patch, I was able to reproduce...
>>> Well this has to be in the running for "weirdest placebo ever"... :/
>>> All it actually seems to achieve is printing an error[1] (this is after all a tiny 5-pin fixed-voltage LDO regulator, not an intelligent PMIC), and if that makes an appreciable difference then there has to be some kind of weird timing condition at play. Maybe regulator_register() ends up turning it off and on again rapidly enough that the card sees a voltage brownout and glitches, and adding more delay by printing to the console somewhere in the middle gives it enough time to act as a proper power cycle with no ill effect?
>>
>> ...and apparently the answer is yes, it seems to be doing exactly that (see attached). But seemingly my SD cards don't mind, or maybe my T4 board happens to have more capacitance than Christian's R4S so my voltage dip isn't as bad, or both.
>>
>> So it seems like the solution here might indeed simply be to remove the regulator-always-on which doesn't seem to have any reason to be here anyway. Without that, the enable stays low until the MMC driver probes and claims it, which is then massively longer than the time it takes for VCC3V0_SD to ramp down completely.
>>
>> Robin.
>
> Removing "regulator-always-on" has the effect that the system freezes upon reboot.
Ah, right (can we fast-forward to a world where everyone has a reliable
bootloader in SPI flash or similar?). Is that more glitching, or a
firmware bug not resetting the GPIOs to their default state on warm
reset, I wonder.
> There may well be another bug slumbering in the codebase that is circumvented by 1. adding a delay in the code and 2. not turning the regulator off upon shutdown.
Yes, it seems suboptimal that the regulator core allows this glitch
where an always-on regulator which is already on gets turned off at all,
but I guess that's its own problem. In the meantime, off-on-delay-us
sounds like the most likely property to bandage this locally. I'm seeing
a fall time in the order of milliseconds (attached), so we'd probably
want a fair chunk of that to be safe.
Robin.
> Am 15.07.2022 um 21:38 schrieb Robin Murphy <[email protected]>:
>
> On 2022-07-15 20:04, Christian Kohlschütter wrote:
>> Am 15.07.2022 um 20:57 schrieb Robin Murphy <[email protected]>:
>>>
>>> On 2022-07-15 19:11, Robin Murphy wrote:
>>>> On 2022-07-15 18:16, Christian Kohlschütter wrote:
>>>>> OK, this took me a while to figure out.
>>>>>
>>>>> When no undervoltage limit is configured, I can reliably trigger the initialization bug upon boot.
>>>>> When the limit is set to 3.0V, it rarely occurs, but just after I send the v3 patch, I was able to reproduce...
>>>> Well this has to be in the running for "weirdest placebo ever"... :/
>>>> All it actually seems to achieve is printing an error[1] (this is after all a tiny 5-pin fixed-voltage LDO regulator, not an intelligent PMIC), and if that makes an appreciable difference then there has to be some kind of weird timing condition at play. Maybe regulator_register() ends up turning it off and on again rapidly enough that the card sees a voltage brownout and glitches, and adding more delay by printing to the console somewhere in the middle gives it enough time to act as a proper power cycle with no ill effect?
>>>
>>> ...and apparently the answer is yes, it seems to be doing exactly that (see attached). But seemingly my SD cards don't mind, or maybe my T4 board happens to have more capacitance than Christian's R4S so my voltage dip isn't as bad, or both.
>>>
>>> So it seems like the solution here might indeed simply be to remove the regulator-always-on which doesn't seem to have any reason to be here anyway. Without that, the enable stays low until the MMC driver probes and claims it, which is then massively longer than the time it takes for VCC3V0_SD to ramp down completely.
>>>
>>> Robin.
>> Removing "regulator-always-on" has the effect that the system freezes upon reboot.
>
> Ah, right (can we fast-forward to a world where everyone has a reliable bootloader in SPI flash or similar?). Is that more glitching, or a firmware bug not resetting the GPIOs to their default state on warm reset, I wonder.
>
>> There may well be another bug slumbering in the codebase that is circumvented by 1. adding a delay in the code and 2. not turning the regulator off upon shutdown.
>
> Yes, it seems suboptimal that the regulator core allows this glitch where an always-on regulator which is already on gets turned off at all, but I guess that's its own problem. In the meantime, off-on-delay-us sounds like the most likely property to bandage this locally. I'm seeing a fall time in the order of milliseconds (attached), so we'd probably want a fair chunk of that to be safe.
>
> Robin.<SDS00003.png>
I think we have a way where there's no need to pick a delay value that may ultimately not work in all cases.
Following up with "[PATCH] regulator: core: Resolve supply name earlier to prevent double-init" [1]
Thank you so much for helping me getting that far! It would be great if you'd keep following the thread.
Best,
Christian
[1] https://www.spinics.net/lists/kernel/msg4440365.html
>>
>>> There may well be another bug slumbering in the codebase that is circumvented by 1. adding a delay in the code and 2. not turning the regulator off upon shutdown.
>>
>> Yes, it seems suboptimal that the regulator core allows this glitch where an always-on regulator which is already on gets turned off at all, but I guess that's its own problem. In the meantime, off-on-delay-us sounds like the most likely property to bandage this locally. I'm seeing a fall time in the order of milliseconds (attached), so we'd probably want a fair chunk of that to be safe.
>>
>> Robin.<SDS00003.png>
>
> I think we have a way where there's no need to pick a delay value that may ultimately not work in all cases.
> Following up with "[PATCH] regulator: core: Resolve supply name earlier to prevent double-init" [1]
>
> Thank you so much for helping me getting that far! It would be great if you'd keep following the thread.
>
> Best,
> Christian
>
> [1] https://www.spinics.net/lists/kernel/msg4440365.html
@Robin,
oddly enough, setting off-on-delay-us with values of up to a second (1000000 us) still results in failed inits.
I hope we can find another bandage until the regular-core patch gets merged.
mmc/SD-card initialization may fail on NanoPi R4S with
"mmc1: problem reading SD Status register" /
"mmc1: error -110 whilst initialising SD card"
either on cold boot or after a reboot.
Moreover, the system would also sometimes hang upon reboot.
This is caused by vcc3v0-sd's "regulator-always-on", which triggers
an erroneous double-initialization of the regulator. This causes
voltage fluctuations that can, depending on timing, prevent the
SD card from initializing correctly.
Adding some liberal delay via "off-on-delay-us" is ineffective since
that codepath is skipped as long "regulator-always-on" is set.
Removing "regulator-always-on" alone is not sufficient because that
would allow the system to set GPIO0_A1 to LOW upon reboot, which may
cause the system to hang.
In order to allow the system to set GPIO0_A1 to HIGH upon initialization
but prevent it from changing it back to LOW, this patch increases the
usage count of vcc3v0-sd from 1 to 2, whereas the additional reference,
"vcc1v8_s3", is marked as "always-on", causing permanent retention.
As a welcome side-effect, this change allows the SD card voltage to be
set back to 3.0V upon reboot, allowing bootloaders to use the card right
away, obsoleting further patching.
Signed-off-by: Christian Kohlschütter <[email protected]>
---
arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
index 8c0ff6c96e03..38507a6e3046 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
@@ -61,7 +61,17 @@ vcc1v8_s3: vcc1v8-s3 {
regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <1800000>;
regulator-name = "vcc1v8_s3";
- vin-supply = <&vcc_1v8>;
+
+ /*
+ * Workaround to skip setting gpio0 RK_PA1 to LOW upon reboot,
+ * which may freeze the system.
+ *
+ * Adding a reference to vcc3v0_sd increases its num_users
+ * count to 2, preventing deactivation since this regulator is
+ * marked "always-on".
+ */
+ // vin-supply = <&vcc_1v8>; // actual supply
+ vin-supply = <&vcc3v0_sd>;
};
vcc3v0_sd: vcc3v0-sd {
@@ -70,7 +80,6 @@ vcc3v0_sd: vcc3v0-sd {
gpio = <&gpio0 RK_PA1 GPIO_ACTIVE_HIGH>;
pinctrl-names = "default";
pinctrl-0 = <&sdmmc0_pwr_h>;
- regulator-always-on;
regulator-min-microvolt = <3000000>;
regulator-max-microvolt = <3000000>;
regulator-name = "vcc3v0_sd";
--
2.36.1
mmc/SD-card initialization may fail on NanoPi R4S with
"mmc1: problem reading SD Status register" /
"mmc1: error -110 whilst initialising SD card"
either on cold boot or after a reboot.
Moreover, the system would also sometimes hang upon reboot.
This is caused by vcc3v0-sd's "regulator-always-on", which triggers
an erroneous double-initialization of the regulator. This causes
voltage fluctuations that can, depending on timing, prevent the
SD card from initializing correctly.
Adding some liberal delay via "off-on-delay-us" is ineffective since
that codepath is skipped as long "regulator-always-on" is set.
Removing "regulator-always-on" alone is not sufficient because that
would allow the system to set GPIO0_A1 to LOW upon reboot, which may
cause the system to hang.
In order to allow the system to set GPIO0_A1 to HIGH upon initialization
but prevent it from changing it back to LOW, this patch increases the
usage count of vcc3v0-sd from 1 to 2, whereas the additional reference,
"vcc1v8_s3", is marked as "always-on", causing permanent retention.
Signed-off-by: Christian Kohlschütter <[email protected]>
---
arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
index 8c0ff6c96e03..38507a6e3046 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-nanopi4.dtsi
@@ -61,7 +61,17 @@ vcc1v8_s3: vcc1v8-s3 {
regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <1800000>;
regulator-name = "vcc1v8_s3";
- vin-supply = <&vcc_1v8>;
+
+ /*
+ * Workaround to skip setting gpio0 RK_PA1 to LOW upon reboot,
+ * which may freeze the system.
+ *
+ * Adding a reference to vcc3v0_sd increases its num_users
+ * count to 2, preventing deactivation since this regulator is
+ * marked "always-on".
+ */
+ // vin-supply = <&vcc_1v8>; // actual supply
+ vin-supply = <&vcc3v0_sd>;
};
vcc3v0_sd: vcc3v0-sd {
@@ -70,7 +80,6 @@ vcc3v0_sd: vcc3v0-sd {
gpio = <&gpio0 RK_PA1 GPIO_ACTIVE_HIGH>;
pinctrl-names = "default";
pinctrl-0 = <&sdmmc0_pwr_h>;
- regulator-always-on;
regulator-min-microvolt = <3000000>;
regulator-max-microvolt = <3000000>;
regulator-name = "vcc3v0_sd";
--
2.36.1