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@linux.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;
 	vmmc-supply = <&vcc3v3_sd>;
 	vqmmc-supply = <&vccio_sd>;
 	status = "okay";
-- 
2.45.2

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@linux.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";

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?

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.

Any more thoughts on this are very welcome.

Kind regards,
Tamas



Tamás Szűcs
tszucs@linux.com

On Mon, Nov 11, 2024 at 8:00 PM Jonas Karlman <jonas@kwiboo.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@linux.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";
>

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@linux.com
> 
> On Mon, Nov 11, 2024 at 8:00 PM Jonas Karlman <jonas@kwiboo.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@linux.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";
>>

Hi Jonas,

On Tue, Nov 12, 2024 at 11:37 PM Jonas Karlman <jonas@kwiboo.se> wrote:
>
> 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.

All right, and thank you for the explanation. My hands are full at the
moment but let me think about this.

>
> Regards,
> Jonas
>
> >
> > Any more thoughts on this are very welcome.
> >
> > Kind regards,
> > Tamas
> >
> >
> >
> > Tamás Szűcs
> > tszucs@linux.com
> >
> > On Mon, Nov 11, 2024 at 8:00 PM Jonas Karlman <jonas@kwiboo.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@linux.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";
> >>
>