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]

Date:   Fri, 15 Jul 2022 20:57:24 +0200
From:   Christian Kohlschütter 
        <christian@...lschutter.com>
To:     Robin Murphy <robin.murphy@....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

> Am 15.07.2022 um 20:11 schrieb Robin Murphy <robin.murphy@....com>:
> 
> 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?

That's definitely something between placebo and homeopathy :-)

I can confirm that setting a limit higher than 3.0V still works, which means that the one time incident where it still crashed means that there's indeed a timing issue at play, and adding that undervoltage statement (unlike the ramp-delay configs that I also tried) added just enough of a delay that made it work 99 out of 100 times.

> 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?
Adding a delay for vcc3v0_sd works, which is great! (patch below)

Is there an existing path from device-tree parser to regular/core.c that we can use to specify this delay specifically for this regulator?
Also, what delay should we choose to make sure it works all the time and not just 99 out of 100 times?

Best,
Christian

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index c4d844ffad7a..0e15ec2548f4 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1483,6 +1483,11 @@ static int set_machine_constraints(struct regulator_dev *rdev)
                          "IC does not support requested over voltage limits\n");
        }
 
+if(!strncmp(rdev_get_name(rdev),"vcc3v0_sd",sizeof("vcc3v0_sd"))) {
+       rdev_err(rdev, "DELAY: %s\n", rdev_get_name(rdev));
+       mdelay(500);
+}
+
        if (rdev->constraints->under_voltage_detection)
                ret = handle_notify_limits(rdev,
                                           ops->set_under_voltage_protection,