| Message ID | 20241219-mmc-slot-v1-1-dfc747a3d3fb@microchip.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | dt-bindings: mmc: move compatible property to its specific binding | expand |
On Thu, Dec 19, 2024 at 09:40:41AM +0530, Dharma Balasubiramani wrote: > Move the `compatible` property into its specific binding to make the MMC > slot more generic and modular. This makes no sense, as presented. What's the real reason for this change? You want to ref mmc-slot.yaml but the compatible is causing a driver to probe? Otherwise, if this is just to avoid having to fix up some devicetree source files, I don't think we should do this. Thanks, Conor. > > Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com> > --- > Documentation/devicetree/bindings/mmc/amlogic,meson-mx-sdio.yaml | 4 ++++ > Documentation/devicetree/bindings/mmc/mmc-slot.yaml | 7 +------ > 2 files changed, 5 insertions(+), 6 deletions(-) > > diff --git a/Documentation/devicetree/bindings/mmc/amlogic,meson-mx-sdio.yaml b/Documentation/devicetree/bindings/mmc/amlogic,meson-mx-sdio.yaml > index 022682a977c6..7600a4988eca 100644 > --- a/Documentation/devicetree/bindings/mmc/amlogic,meson-mx-sdio.yaml > +++ b/Documentation/devicetree/bindings/mmc/amlogic,meson-mx-sdio.yaml > @@ -54,6 +54,10 @@ patternProperties: > A node for each slot provided by the MMC controller > > properties: > + compatible: > + items: > + - const: mmc-slot > + > reg: > enum: [0, 1, 2] > > diff --git a/Documentation/devicetree/bindings/mmc/mmc-slot.yaml b/Documentation/devicetree/bindings/mmc/mmc-slot.yaml > index 1f0667828063..84c4605658e0 100644 > --- a/Documentation/devicetree/bindings/mmc/mmc-slot.yaml > +++ b/Documentation/devicetree/bindings/mmc/mmc-slot.yaml > @@ -20,19 +20,15 @@ properties: > $nodename: > pattern: "^slot(@.*)?$" > > - compatible: > - const: mmc-slot > - > reg: > description: > the slot (or "port") ID > maxItems: 1 > > required: > - - compatible > - reg > > -unevaluatedProperties: false > +additionalProperties: true > > examples: > - | > @@ -40,7 +36,6 @@ examples: > #address-cells = <1>; > #size-cells = <0>; > slot@0 { > - compatible = "mmc-slot"; > reg = <0>; > bus-width = <4>; > }; > > --- > base-commit: 7fa366f1b6e376c38966faa42da7f0f2e013fdab > change-id: 20241219-mmc-slot-0574889daea3 > > Best regards, > -- > Dharma Balasubiramani <dharma.b@microchip.com> >
Hi Dharma, On Tue, Jan 7, 2025 at 4:34 AM <Dharma.B@microchip.com> wrote: > > On 20/12/24 1:41 am, Conor Dooley wrote: > > On Thu, Dec 19, 2024 at 09:40:41AM +0530, Dharma Balasubiramani wrote: > >> Move the `compatible` property into its specific binding to make the MMC > >> slot more generic and modular. > > This makes no sense, as presented. What's the real reason for this > > change? You want to ref mmc-slot.yaml but the compatible is causing a > > driver to probe? > > We don’t have the configuration for that driver enabled. Wouldn’t > including the compatible in the DTS files without the actual driver be > redundant? > > Is it the correct approach to add the compatible just to fix the dt > binding errors? Let me try to summarize what I understand so far: - your are trying to convert the dt-binding of atmel-hsmci from .txt to .yaml - while doing so Rob asked to reference the mmc-slot schema - after referencing the mmc-slot schema you now get warnings in when validating the .dts because your .dts doesn't specify compatible = "mmc-slot" Is that correct? There aren't many MMC controllers with multiple slot support out there. When I wrote the dt-bindings for amlogic,meson-mx-sdio I *think* (it's been some years) Ulf pointed out another dt-binding (Documentation/devicetree/bindings/mmc/cavium-mmc.txt) and driver (drivers/mmc/host/cavium-thunderx.c) that already used the mmc-slot compatible string. > related discussion: > https://lore.kernel.org/lkml/63473475-f29e-4a65-a0aa-1f1e4112b57d@microchip.com/ Rob has suggested two approaches in that thread: - don't mark the "compatible" property as required (in Documentation/devicetree/bindings/mmc/mmc-slot.yaml) - add the compatible string where needed (I attached a diff with an example where I picked one random Atmel board and added the compatible string) Your patch is different from these suggestions as it forbids the compatible property in the generic mmc-slot binding. What's the problem with Rob's suggestions? If they cannot be implemented then please document why that is. Best regards, Martin
Hi Dharma, On Wed, Jan 8, 2025 at 4:11 AM <Dharma.B@microchip.com> wrote: [...] > "One issue is 'compatible' is required. Either that would have to be > dropped as required." > > Instead of just dropping it from "required:", I removed the property > itself and moved it to another binding. > > I will send a v2 by removing it from the required, will it be fine? For me this is fine. My understanding is that if we drop the compatible property completely then any compatible string will be allowed (for example: compatible = "random,name"). This is because mmc-slot.yaml inherits the properties from mmc-controller-common.yaml which itself has "additionalProperties: true". However, if we allow it but make it optional it means that there's only two valid states: - no compatible property (on the Atmel / Microchip SoCs) - a compatible property with the value "mmc-slot" (as used on Amlogic Meson and Cavium Thunder SoCs) - (anything else is considered invalid) Rob, Conor: can confirm this or correct me wherever I got something wrong. I hope that your feedback will help Dharma write a good patch description for v2. Best regards, Martin
diff --git a/Documentation/devicetree/bindings/mmc/amlogic,meson-mx-sdio.yaml b/Documentation/devicetree/bindings/mmc/amlogic,meson-mx-sdio.yaml index 022682a977c6..7600a4988eca 100644 --- a/Documentation/devicetree/bindings/mmc/amlogic,meson-mx-sdio.yaml +++ b/Documentation/devicetree/bindings/mmc/amlogic,meson-mx-sdio.yaml @@ -54,6 +54,10 @@ patternProperties: A node for each slot provided by the MMC controller properties: + compatible: + items: + - const: mmc-slot + reg: enum: [0, 1, 2] diff --git a/Documentation/devicetree/bindings/mmc/mmc-slot.yaml b/Documentation/devicetree/bindings/mmc/mmc-slot.yaml index 1f0667828063..84c4605658e0 100644 --- a/Documentation/devicetree/bindings/mmc/mmc-slot.yaml +++ b/Documentation/devicetree/bindings/mmc/mmc-slot.yaml @@ -20,19 +20,15 @@ properties: $nodename: pattern: "^slot(@.*)?$" - compatible: - const: mmc-slot - reg: description: the slot (or "port") ID maxItems: 1 required: - - compatible - reg -unevaluatedProperties: false +additionalProperties: true examples: - | @@ -40,7 +36,6 @@ examples: #address-cells = <1>; #size-cells = <0>; slot@0 { - compatible = "mmc-slot"; reg = <0>; bus-width = <4>; };
Move the `compatible` property into its specific binding to make the MMC slot more generic and modular. Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com> --- Documentation/devicetree/bindings/mmc/amlogic,meson-mx-sdio.yaml | 4 ++++ Documentation/devicetree/bindings/mmc/mmc-slot.yaml | 7 +------ 2 files changed, 5 insertions(+), 6 deletions(-) --- base-commit: 7fa366f1b6e376c38966faa42da7f0f2e013fdab change-id: 20241219-mmc-slot-0574889daea3 Best regards,