[PATCH v2 1/3] rockchip: rk35xx: Remove use of eMMC DDR52 mode

[Jonas Karlman]

On 2024-02-05 14:51, Dragan Simic wrote:
> On 2024-02-05 14:45, Quentin Schulz wrote:
>> On 2/5/24 14:40, Dragan Simic wrote:
>>> On 2024-02-05 14:24, Jonas Karlman wrote:
>>>> On 2024-02-05 10:40, Quentin Schulz wrote:
>>>>> Shouldn't we also remove cap-mmc-highspeed?
>>>>>
>>>>> I assume this is for MMC_HS? Which we discovered doesn't work
>>>>> reliably?
>>>>
>>>> Writing in lower speeds on RK356x does work better then on RK3588,
>>>> however HS200 still seem to be more reliable overall and give more
>>>> speed.
>>>>
>>>> For RK3588 they could possible be removed, but since reading does 
>>>> work
>>>> and similar issue also existed in the fallback MMC legacy mode, I did
>>>> not see that cleanup as important, or maybe more correctly it did not
>>>> cross my mind to also remove these props :-), the fully broken ddr52
>>>> felt more important.
>>>>
>>>>> Since it could be in a different patch if you wanted to, for the
>>>>> changes in this patch:
>>>>
>>>> Agree, a possible cleanup of cap-mmc-highspeed props could be done in 
>>>> a
>>>> separate patch.
>>>>
>>>> Alternatively any missing and appropriate modes currently in
>>>> u-boot.dtsi
>>>> should be added to linux DT and they can then be synced back to 
>>>> U-Boot
>>>> and any override/addition dropped from u-boot.dtsi files.
>>>
>>> How about waiting until I fix the mode selection in the Linux kernel
>>> drivers?  As I noted already, that's already on my TODO list, and
>>> perhaps it would be good to test a bit again under Linux with those
>>> fixes applied, before actually removing the buggy modes from the
>>> kernel's RK356x and RK3588(s) SoC dtsi files.
>>
>> While I appreciate there's some work that can be done in the Linux
>> kernel for those low-speed modes, the properties in question are
>> removed only from U-Boot's DTS and your future patches for the kernel
>> wouldn't impact the bootloader anyway.
>>
>> The issue right now is that U-Boot is currently broken because of
>> those properties, which are only in U-Boot's DTS.
>>
>> Not sure exactly why we should waiting on Linux kernel patches for
>> merging work-arounds for U-Boot? What exactly are you suggesting we
>> wait for? What can improve the situation?
> 
> Ah, sorry for the confusion, let me clarify a bit...
> 
> I just replied to the Jonas's remark about the need to eventually
> have the same fixes and mode removals in the Linux kernel's SoC dtsi
> files.  In other words, all I wrote was about those changes, not
> about the changes on the U-Boot side.
> 
> Of course, there are no reasons for delaying these U-Boot fixes.

Short history about the added eMMC modes in u-boot.dtsi on RK35xx boards:

U-Boot defaults to not support HS200+ modes unless explicitly enabled in
Kconfig options and on top of that the driver require DT to have a
corresponding mode prop to make use of any mode faster than MMC legacy.

When I worked on eMMC fixes for RK3568 and support for RK3588 [1] I only
tested reading data using all modes.

Because reading data in all modes seem to work on the boards I tested on,
I eventually added the "missing" props to board u-boot.dtsi-files, so
that U-Boot could make use of faster modes. However, I opted to only
enable the HS200 Kconfig option on boards with embedded eMMC.

This has resulted in that most RK35xx boards now default to using the
fastest none HS200+ mode, i.e. DDR52 mode. They would also try to use a
faster mode as long as a HS200+ Kconfig option is enabled.

With the removal of mmc-ddr-1_8v prop in this series, at least the most
broken mode is avoided for all RK35xx boards. And with the HS200 Kconfig
option enabled by default, the required tuning in HS200+ mode should
automatically happen as long as eMMC module support HS200 mode. After
that writing in any supported lower speed seem to work OK.

In the case where HS200 is supported any read/write in lower speed seem
to work, so the modes provided by cap-mmc-highspeed prop is OK.

However, if HS200+ is not supported it does not really matter, first
write will fail in any remaining mode, the slowest legacy or the modes
provided by cap-mmc-highspeed prop.

Keeping the cap-mmc-highspeed prop at least make reading data faster
possible, and writing data is still semi-broken in any remaining mode.

In conclusion I suggest:

- We keep the cap-mmc-highspeed props in board u-boot.dtsi for now.

- The mmc mode props that has been added in board u-boot.dtsi and have
  been validated working with both U-Boot and linux should be added to
  board DT. e.g HS200+ props is missing in some board DT.

- When an updated board DT has been synced from linux, any override in
  board u-boot.dtsi should be dropped.

I am also fine with completely dropping cap-mmc-highspeed props now if
they cause issues not solved by the HS200 Kconfig option being enabled.

[1] https://patchwork.ozlabs.org/cover/1764606/

Regards,
Jonas