Re: [PATCH RFC v2] dt-bindings: mmc: mmc-slot: make compatible property optional

[Dharma.B]

On 07/02/25 2:47 pm, neil.armstrong@xxxxxxxxxx wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know 
> the content is safe
> 
> On 07/02/2025 10:02, Dharma.B@xxxxxxxxxxxxx wrote:
>> On 07/02/25 2:25 pm, Neil Armstrong wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know
>>> the content is safe
>>>
>>> On 05/02/2025 04:48, Dharma Balasubiramani wrote:
>>>> Remove the compatible property from the list of required properties and
>>>> mark it as optional.
>>>>
>>>> Signed-off-by: Dharma Balasubiramani <dharma.b@xxxxxxxxxxxxx>
>>>> ---
>>>> Changes in v2:
>>>> - Instead of moving the compatible string to the other binding, just
>>>> make it
>>>>     optional (remove from required list).
>>>> - Link to v1: https://lore.kernel.org/r/20241219-mmc-slot-v1-1-
>>>> dfc747a3d3fb@xxxxxxxxxxxxx
>>>> ---
>>>>    Documentation/devicetree/bindings/mmc/mmc-slot.yaml | 1 -
>>>>    1 file changed, 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mmc/mmc-slot.yaml b/
>>>> Documentation/devicetree/bindings/mmc/mmc-slot.yaml
>>>> index 1f0667828063..ca3d0114bfc6 100644
>>>> --- a/Documentation/devicetree/bindings/mmc/mmc-slot.yaml
>>>> +++ b/Documentation/devicetree/bindings/mmc/mmc-slot.yaml
>>>> @@ -29,7 +29,6 @@ properties:
>>>>        maxItems: 1
>>>>
>>>>    required:
>>>> -  - compatible
>>>>      - reg
>>>
>>> If you remove it from here then it's still required in Documentation/
>>> devicetree/bindings/mmc/amlogic,meson-mx-sdio.yaml
>>> so please add it.
>>
>> If moving the compatible to its specific binding isn't appropriate (as
>> per Conor),
>> and if removing it from the required list here doesn’t seem reasonable
>> to you,
>> then adding an unnecessary compatible string in our DTS files doesn’t
>> make sense to me.
>>
>> What could be the solution then?
> 
> The solution is right but you modify the meson-mx-sdio bindings, so
> simply add compatible in a required list for the slot node.

Okay, we declare compatible as optional in the generic mmc-slot binding 
but make it required in the meson-mx-sdio binding, which inherits from it.

So why not define the property directly in the meson-mx-sdio binding 
instead?

It feels like the mmc-slot binding itself serves no real purpose.

> 
> Something like:
> ========================================================================
> diff --git a/Documentation/devicetree/bindings/mmc/amlogic,meson-mx- 
> sdio.yaml b/Documentation/devicetree/bindings/mmc/amlogic,meson-mx- 
> sdio.yaml
> index 022682a977c6..0d4d9ca6a8d9 100644
> --- a/Documentation/devicetree/bindings/mmc/amlogic,meson-mx-sdio.yaml
> +++ b/Documentation/devicetree/bindings/mmc/amlogic,meson-mx-sdio.yaml
> @@ -60,6 +60,9 @@ patternProperties:
>         bus-width:
>           enum: [1, 4]
> 
> +    required:
> +      - compatible
> +
>       unevaluatedProperties: false
> 
> ========================================================================
> 
> Conor, Is it right ?
> 
> Neil
> 
>>>
>>> Thanks,
>>> Neil
>>>
>>>>
>>>>    unevaluatedProperties: false
>>>>
>>>> ---
>>>> base-commit: 40b8e93e17bff4a4e0cc129e04f9fdf5daa5397e
>>>> change-id: 20241219-mmc-slot-0574889daea3
>>>>
>>>> Best regards,
>>>
>>
>>
> 


-- 
With Best Regards,
Dharma B.