Re: [PATCH] dt-bindings: mmc: move compatible property to its specific binding

[Martin Blumenstingl]

Hi Dharma,

On Tue, Jan 7, 2025 at 4:34 AM <Dharma.B@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx/
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
diff --git a/arch/arm/boot/dts/microchip/at91sam9n12ek.dts b/arch/arm/boot/dts/microchip/at91sam9n12ek.dts
index 643c3b2ab97e..a27fcef28df5 100644
--- a/arch/arm/boot/dts/microchip/at91sam9n12ek.dts
+++ b/arch/arm/boot/dts/microchip/at91sam9n12ek.dts
@@ -80,6 +80,7 @@ &pinctrl_mmc0_slot0_clk_cmd_dat0
 					&pinctrl_mmc0_slot0_dat1_3>;
 				status = "okay";
 				slot@0 {
+					compatible = "mmc-slot";
 					reg = <0>;
 					bus-width = <4>;
 					cd-gpios = <&pioA 7 GPIO_ACTIVE_HIGH>;