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

[Quentin Schulz]

Hi Dragan,

On 4/9/24 21:30, Dragan Simic wrote:
> 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.
> 

I was suggesting to fix the upstream DT files as well.

Cheers,
Quentin