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

[Dragan Simic]

Hello Jonas,

On 2024-02-05 18:01, Jonas Karlman wrote:
> 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:
>>>> 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/

Thank you for taking the time to write this detailed summary!

It's somewhat unexpected that the writes to eMMC fail to work while
the reads work fine.  As we know, the writes don't introduce the delay
between the host-generated clock and the data to be written.  It would
be expected for the reads to be more error-prone.