diff mbox series

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

Message ID 20250205-mmc-slot-v2-1-da3c5f30e2d9@microchip.com (mailing list archive)
State New
Headers show
Series [RFC,v2] dt-bindings: mmc: mmc-slot: make compatible property optional | expand

Commit Message

Dharma Balasubiramani Feb. 5, 2025, 3:48 a.m. UTC
Remove the compatible property from the list of required properties and
mark it as optional.

Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
---
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@microchip.com
---
 Documentation/devicetree/bindings/mmc/mmc-slot.yaml | 1 -
 1 file changed, 1 deletion(-)


---
base-commit: 40b8e93e17bff4a4e0cc129e04f9fdf5daa5397e
change-id: 20241219-mmc-slot-0574889daea3

Best regards,

Comments

Conor Dooley Feb. 6, 2025, 6:27 p.m. UTC | #1
On Wed, Feb 05, 2025 at 09:18:40AM +0530, 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@microchip.com>
> ---
> 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@microchip.com

Why is this RFC? I don't see any complaints from Rob's bot, so I am
assuming that this actually works and the error you mentioned in the
previously version has been resolved?

> ---
>  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
>  
>  unevaluatedProperties: false
> 
> ---
> base-commit: 40b8e93e17bff4a4e0cc129e04f9fdf5daa5397e
> change-id: 20241219-mmc-slot-0574889daea3
> 
> Best regards,
> -- 
> Dharma Balasubiramani <dharma.b@microchip.com>
>
Neil Armstrong Feb. 7, 2025, 8:55 a.m. UTC | #2
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@microchip.com>
> ---
> 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@microchip.com
> ---
>   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.

Thanks,
Neil

>   
>   unevaluatedProperties: false
> 
> ---
> base-commit: 40b8e93e17bff4a4e0cc129e04f9fdf5daa5397e
> change-id: 20241219-mmc-slot-0574889daea3
> 
> Best regards,
Neil Armstrong Feb. 7, 2025, 9:17 a.m. UTC | #3
On 07/02/2025 10:02, Dharma.B@microchip.com 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@microchip.com>
>>> ---
>>> 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@microchip.com
>>> ---
>>>    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.

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,
>>
> 
>
Conor Dooley Feb. 7, 2025, 4:03 p.m. UTC | #4
On Fri, Feb 07, 2025 at 10:17:29AM +0100, neil.armstrong@linaro.org wrote:
> On 07/02/2025 10:02, Dharma.B@microchip.com 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@microchip.com>
> > > > ---
> > > > 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@microchip.com
> > > > ---
> > > >    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.
> 
> 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 ?

Looks about right, ye.
Neil Armstrong Feb. 10, 2025, 9:51 a.m. UTC | #5
On 10/02/2025 06:28, Dharma.B@microchip.com wrote:
> On 07/02/25 2:47 pm, neil.armstrong@linaro.org 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@microchip.com 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@microchip.com>
>>>>> ---
>>>>> 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@microchip.com
>>>>> ---
>>>>>     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.

It's designed to be reused, the goal was to facilitate migration of
other mmc controllers bindings.

Neil

> 
>>
>> 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,
>>>>
>>>
>>>
>>
> 
>
Rob Herring (Arm) Feb. 11, 2025, 8:12 p.m. UTC | #6
On Mon, Feb 10, 2025 at 05:28:27AM +0000, Dharma.B@microchip.com wrote:
> On 07/02/25 2:47 pm, neil.armstrong@linaro.org 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@microchip.com 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.

The diff tells us that. Please say why 'compatible' being required is a 
problem and needs to not be required.

> >>>>
> >>>> Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
> >>>> ---
> >>>> 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@microchip.com
> >>>> ---
> >>>>    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?

Because mmc-slot.yaml is designed to be complete (hence 
"unevaluatedProperties: false"). There's at least 2 bindings which use 
it (with "mmc-slot" compatible). Leaving it at least prevents folks from 
coming up with their own random compatible strings for mmc-slot.

Rob
diff mbox series

Patch

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
 
 unevaluatedProperties: false