linux-kernel - Re: [PATCH v4] arm64: dts: rockchip: Fix SD card init on rk3399-nanopi4

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <daf3b61c-d886-98eb-0443-de233d742041@arm.com>
Date:   Fri, 15 Jul 2022 20:38:19 +0100
From:   Robin Murphy <robin.murphy@....com>
To:     Christian Kohlschütter 
        <christian@...lschutter.com>
Cc:     wens@...nel.org, Heiko Stübner <heiko@...ech.de>,
        Markus Reichl <m.reichl@...etechno.de>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        "open list:ARM/Rockchip SoC..." <linux-rockchip@...ts.infradead.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Linux MMC List <linux-mmc@...r.kernel.org>
Subject: Re: [PATCH v4] arm64: dts: rockchip: Fix SD card init on
 rk3399-nanopi4

On 2022-07-15 20:04, Christian Kohlschütter wrote:
> Am 15.07.2022 um 20:57 schrieb Robin Murphy <robin.murphy@....com>:
>>
>> 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.
Download attachment "SDS00003.png" of type "image/png" (22276 bytes)