linux-kernel - Re: [PATCH 1/3] arm64: dts: rockchip: Add supported UHS-I rates to sdmmc0 on rock-3b

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: <e9ff5387-b3c1-41a7-b251-a12789d25a70@kwiboo.se>
Date: Tue, 12 Nov 2024 23:37:14 +0100
From: Jonas Karlman <jonas@...boo.se>
To: Tamás Szűcs <tszucs@...ux.com>
Cc: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Heiko Stuebner <heiko@...ech.de>,
 FUKAUMI Naoki <naoki@...xa.com>, Dragan Simic <dsimic@...jaro.org>,
 Chukun Pan <amadeus@....edu.cn>, devicetree@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-rockchip@...ts.infradead.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3] arm64: dts: rockchip: Add supported UHS-I rates to
 sdmmc0 on rock-3b

Hi Tamás,

On 2024-11-12 15:36, Tamás Szűcs wrote:
> Hi Jonas,
> 
> Thank you for pointing this out! I haven't noticed this before. I've
> done some testing and I believe I am able to reproduce the issue you
> described, although I cannot confirm the reason.
> The only occasion I encounter any problems is when a UHS SD card or
> SDIO device is connected to sdmmc0 during bootup. Sometimes the device
> is recognized as HS only. Obviously no tuning value reported. Also,
> sdmmc2 cuts out completely. I'm booting from eMMC and when the SD card
> is removed in this state I lose my rootfs. Certainly, this needs more
> attention but it seems to be unrelated to the changes here.
> 
> I need more time to check but are you sure this SD card during bootup
> issue is gone with UHS-I disabled?

Yes, the issue is that the io voltage domain must be configured to match
the io signal voltage used, and to use uhs the voltage changes from 3v3
to 1v8. Causing a miss-match between io voltage domain config and the
regulator voltage used during initial probe, unless io-domain driver
happens to be fully loaded before mmc devices are probed.

> 
> Also, in every other case, when you connect any device to sdmmc0 after
> bootup, performance and stability is perfect.
> Interestingly I also don't experience this behavior with an eMMC
> device and / or an SDIO device connected to sdmmc2 during bootup. Only
> sdmmc0 is problematic and only during bootup.

Yes, as you have discovered, inserting the sd-card after system has
booted and io-domain driver has been loaded, everything can work as
expected with uhs speeds.

Until this probe race condition has been solved booting with a sd-card
inserted may or may not result in wrong tuning or other related issues.

Because of this I advice not to enable uhs mode for sdmmc0 at this time.

Regards,
Jonas

> 
> Any more thoughts on this are very welcome.
> 
> Kind regards,
> Tamas
> 
> 
> 
> Tamás Szűcs
> tszucs@...ux.com
> 
> On Mon, Nov 11, 2024 at 8:00 PM Jonas Karlman <jonas@...boo.se> wrote:
>>
>> Hi Tamás,
>>
>> On 2024-11-11 19:17, Tamás Szűcs wrote:
>>> Add all supported UHS-I rates to sdmmc0 and allow 200 MHz maximum clock to
>>> benefit modern SD cards.
>>>
>>> Signed-off-by: Tamás Szűcs <tszucs@...ux.com>
>>> ---
>>>  arch/arm64/boot/dts/rockchip/rk3568-rock-3b.dts | 6 ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-rock-3b.dts b/arch/arm64/boot/dts/rockchip/rk3568-rock-3b.dts
>>> index 3d0c1ccfaa79..242af5337cdf 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3b.dts
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3b.dts
>>> @@ -670,8 +670,14 @@ &sdmmc0 {
>>>       bus-width = <4>;
>>>       cap-sd-highspeed;
>>>       disable-wp;
>>> +     max-frequency = <200000000>;
>>>       pinctrl-names = "default";
>>>       pinctrl-0 = <&sdmmc0_bus4 &sdmmc0_clk &sdmmc0_cmd &sdmmc0_det>;
>>> +     sd-uhs-sdr12;
>>> +     sd-uhs-sdr25;
>>> +     sd-uhs-sdr50;
>>> +     sd-uhs-sdr104;
>>> +     sd-uhs-ddr50;
>>
>> There is an issue with io-domain driver not always being probed before
>> mmc driver, this typically result in io-domain being configured wrong,
>> and mmc tuning happen before io-domain is correctly configured.
>>
>> You can usually observe this by looking at the tuning value during boot
>> and comparing it to the tuning value after removing and re-insering a
>> sd-card.
>>
>> Because of this uhs modes was left out from initial DT submission, some
>> cards will work others wont, sd-uhs-sdr50 is known to be working with
>> most cards even with the probe order issue.
>>
>> Also I thought that lower speeds where implied?
>>
>> Regards,
>> Jonas
>>
>>>       vmmc-supply = <&vcc3v3_sd>;
>>>       vqmmc-supply = <&vccio_sd>;
>>>       status = "okay";
>>