Re: [PATCH v6 1/5] dt-bindings: mmc: Add dll-presets values for HS400 and HS200 modes

[Krzysztof Kozlowski <krzk@kernel.org>]

On 16/12/2025 13:51, Ram Prakash Gupta wrote:
> 
> On 12/15/2025 5:41 PM, Krzysztof Kozlowski wrote:
>> On 15/12/2025 13:06, Krzysztof Kozlowski wrote:
>>> On 15/12/2025 13:00, Ram Prakash Gupta wrote:
>>>> From: Sachin Gupta <quic_sachgupt@quicinc.com>
>>>>
>>>> Document the 'dll-presets' property for MMC device tree bindings.
>>>> The 'dll-presets' property defines the DLL configurations for HS400
>>>> and HS200 modes.
>>>>
>>>> QC SoCs can have 0 to 4 SDHCI instances, and each one may need
>>>> different tuning.
>>>>
>>>> Signed-off-by: Sachin Gupta <quic_sachgupt@quicinc.com>
>>>> Signed-off-by: Ram Prakash Gupta <quic_rampraka@quicinc.com>
>>>> ---
>>>>  Documentation/devicetree/bindings/mmc/sdhci-msm.yaml | 5 +++++
>>>>  1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml b/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
>>>> index 938be8228d66..a1a16e6e12ce 100644
>>>> --- a/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
>>>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
>>>> @@ -140,6 +140,11 @@ properties:
>>>>      $ref: /schemas/types.yaml#/definitions/uint32
>>>>      description: platform specific settings for DLL_CONFIG reg.
>>>>  
>>>> +  qcom,dll-presets:
>>>> +    maxItems: 10
>>>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>>>> +    description: platform specific settings for DLL registers.
>>>
>>> So look at your DTS - this is clearly incomplete now. So obvious. I
>>> don't understand why you were hiding the DTS change, you just make
>>> review more difficult.
>>>
>>> Anyway, your DTS says you replace some other DLL properties and nothing
>>> in the binding or commit msg explains that. Neither the reason nor the
>>> impact.
> 
> Sorry dt change was not pushed earlier, will add dt change as well
> going forward.
> 
> The existing qcom,dll-config or qcom-ddr-config are not deprecated,
> but certainly I will add below in commit regarding the issue as well
> which is observed without this change as below:

No.

You need schema for this. Actually for both cases - replacing properties
and defining the list.

> 
> "Document the 'dll-presets' property for MMC device tree bindings.
> The 'dll-presets' property defines the DLL configurations for HS400
> and HS200 modes.
> 
> It have 2 arrays of 5 elements each for HS400 and HS200 mode.
> The 5 elements are DLL_CONFIG, DLL_CONFIG_2,DLL_CONFIG_3, DLL_USER_CTRL
> and DDR_CONFIG.
> 
> dll-presets fixes instances of CRC error observed with targets having
> artanis dll.
> 
> QC SoCs can have 0 to 4 SDHCI instances, and each one may need
> different tuning using dll-presets."
> 
>> Plus it looks like you are adding some meaning to the entries, judging
>> by the DTS. DTS suggested something here is e.g. DLL_CONFIG, so that
>> meaning - including different modes - needs description in the schema in
>> items.

Here I wrote it already. Don't ask for repeating.

>>
>> Best regards,
>> Krzysztof
> 
> I will update the schema description as well with the 2 modes and 5 elements

I did not say description. I said you need schema.


Best regards,
Krzysztof