linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Robin Murphy <robin.murphy@arm.com>
To: "Christian Kohlschütter" <christian@kohlschutter.com>,
	wens@kernel.org, "Heiko Stübner" <heiko@sntech.de>,
	"Markus Reichl" <m.reichl@fivetechno.de>
Cc: 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 v4] arm64: dts: rockchip: Fix SD card init on rk3399-nanopi4
Date: Fri, 15 Jul 2022 19:57:52 +0100	[thread overview]
Message-ID: <9405b97a-6758-ad4e-ccff-eed072096539@arm.com> (raw)
In-Reply-To: <17a4c6f6-d79c-a7b2-860f-e5944b778f9f@arm.com>

[-- Attachment #1: Type: text/plain, Size: 3771 bytes --]

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.

> 
> If you just whack something like an mdelay(500) at around that point in 
> set_machine_constraints(), without the DT property, does it have the 
> same effect?
> 
> Robin.
> 
> [1] 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/regulator/core.c#n1521 
> 
> 
>>> Am 15.07.2022 um 19:12 schrieb Christian Kohlschütter 
>>> <christian@kohlschutter.com>:
>>>
>>> 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 <christian@kohlschutter.com>
>>> ---
>>> 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
>>
> 
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

[-- Attachment #2: SDS00002.png --]
[-- Type: image/png, Size: 21500 bytes --]

[-- Attachment #3: Type: text/plain, Size: 176 bytes --]

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

  parent reply	other threads:[~2022-07-15 18:59 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-13 22:22 [PATCH] arm64: dts: rockchip: Fix SD card init on rk3399-nanopi4 Christian Kohlschütter
2022-07-13 23:41 ` Heiko Stübner
2022-07-14 11:41   ` Robin Murphy
2022-07-14 12:14     ` Christian Kohlschütter
2022-07-14 13:14       ` Markus Reichl
2022-07-14 13:50       ` Robin Murphy
2022-07-14 16:24         ` Christian Kohlschütter
2022-07-14 16:26           ` [PATCH v2] " Christian Kohlschütter
2022-07-14 16:44             ` Christian Loehle
2022-07-14 17:20               ` Christian Kohlschütter
2022-07-15 17:02                 ` Christian Loehle
2022-07-14 17:02             ` Chen-Yu Tsai
2022-07-14 17:35               ` Robin Murphy
2022-07-14 17:57                 ` Christian Kohlschütter
2022-07-14 23:44                 ` Robin Murphy
2022-07-15 17:01                   ` [PATCH v3] " Christian Kohlschütter
2022-07-15 17:12                     ` [PATCH v4] " Christian Kohlschütter
2022-07-15 17:16                       ` Christian Kohlschütter
2022-07-15 18:11                         ` Robin Murphy
2022-07-15 18:57                           ` Christian Kohlschütter
2022-07-15 18:57                           ` Robin Murphy [this message]
2022-07-15 19:04                             ` Christian Kohlschütter
2022-07-15 19:38                               ` Robin Murphy
2022-07-15 22:33                                 ` Christian Kohlschütter
2022-07-16  0:24                                   ` Christian Kohlschütter
2022-07-16 19:43                                     ` [PATCH v5] " Christian Kohlschütter
2022-07-18 12:04                                       ` [PATCH v6] " Christian Kohlschütter
2022-07-18 12:05                                         ` Christian Kohlschütter
2022-07-18 21:04                                           ` Christian Kohlschütter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9405b97a-6758-ad4e-ccff-eed072096539@arm.com \
    --to=robin.murphy@arm.com \
    --cc=christian@kohlschutter.com \
    --cc=heiko@sntech.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mmc@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=m.reichl@fivetechno.de \
    --cc=wens@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).