[PATCH 1/2] mmc: Imply HS200 cap with mmc-hs400 prop to match linux

[Dragan Simic]

Hello Jonas,

On 2024-04-09 18:30, Jonas Karlman wrote:
> On 2024-04-09 18:02, Quentin Schulz wrote:
>> On 4/9/24 17:58, Jonas Karlman wrote:
>>> On 2024-04-09 17:27, Quentin Schulz wrote:
>>>> 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.

I think that the logic used in the Linux kernel should be followed,
because one of the goals should be to add as few "touches" to the
upstream DT files in U-Boot as possible.

If the kernel binding patch that I mentioned in my earlier email [1]
becomes accepted, that should be another reason to do so.

[1] 
https://lore.kernel.org/u-boot/9b62bbedf2c6d52b76a8ce1ce57dd35d@manjaro.org/