Hi Quentin, On 2024-04-09 18:02, Quentin Schulz wrote: > Hi Jonas, > > On 4/9/24 17:58, Jonas Karlman wrote: >> Hi Quentin, >> >> On 2024-04-09 17:27, Quentin Schulz wrote: >>> Hi Jonas, >>> >>> On 4/8/24 23:06, Jonas Karlman wrote: >>>> eMMC nodes in linux device tree files typically only contain a mmc-hs400 >>>> prop to signal support for both HS400 and HS200. However, U-Boot require >>>> an explicit mmc-hs200 prop to signal support for the HS200 mode. >>>> > Fix this by follow linux and imply HS200 cap when HS400 cap is signaled >>>> using a mmc-hs400 prop. >>>> >>> >>> Technically speaking, the DT binding should be the one and only source >>> of truth and should be implementation-agnostic. >>> >>> There it says: >>> """ >>> mmc-hs400-1_2v: >>> $ref: /schemas/types.yaml#/definitions/flag >>> description: >>> eMMC HS400 mode (1.2V I/O) is supported. >>> >>> mmc-hs400-1_8v: >>> $ref: /schemas/types.yaml#/definitions/flag >>> description: >>> eMMC HS400 mode (1.8V I/O) is supported. >>> """ >>> >>> So I'd say, the DTs should be fixed to add mmc-hs200 as well wherever it >>> makes sense. >>> >>> The point of the DT/DT binding is to be system-agnostic and >>> representative of the **HW** implementation. At least that's what the DT >>> people want it to be. >>> >>> If the eMMC standard doesn't allow to have HS400 without HS200, then I >>> think this change is acceptable as is, because it is the reality of the >>> HW standard. Couldn't find this implied in the standard though (but I >>> just skimmed through). >>> >>> It's also quite surprising, as it's not because the eMMC works with >>> HS400 that it necessarily does with HS200 or that it's desired (EMI, >>> signal integrity/stability, etc...)? >>> >>> Now, it wouldn't be the first time U-Boot follows whatever is done in >>> Linux, so... up to you/the maintainers :) >> >> Agree that implying HS200 does not fully make sense, however it was part >> of the original Linux binding when HS400 was added in v3.16-rc1 [1] so I >> think that this is the expected behavior and changing it may be an ABI >> breakage. >> > > I'm not advocating undoing the kernel "hack", but rather make it so that > we add hs200 to DTs where it's actually supported instead of doing the > same hack the kernel does. In that case, we wouldn't need the hack anymore.
I will add a patch that adds the missing mmc-hs200 props to affected rk3588 boards, nanopc-t4 and quartzpro64 in v2 of the "rockchip: rk35xx: Miscellaneous fixes and updates" series. Also turns out the issue with those boards was because of my other "mmc: rockchip_sdhci: Revert 4 blocks PIO mode read limit for RK35xx" patch, so will need to rework that revert some more before posting a v2 of that patch. For this patch it is fully up to the maintainers if U-Boot wants to mimic Linux kernel or not. Regards, Jonas > > (well maybe it isn't a hack per-se, but for lack of more info on that, I > call the kernel implementation this :) ) > > Cheers, > Quentin