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 |
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> >
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,
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, >> > >
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.
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, >>>> >>> >>> >> > >
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 --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
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,