[PATCH] dt-bindings: mmc: sdhci-omap: convert text based binding to json schema

[PATCH] dt-bindings: mmc: sdhci-omap: convert text based binding to json schema

[Charan Pedumuru]

Convert TI OMAP SDHCI Controller binding to YAML format.
Changes during Conversion:
- Add patternProperties for pinctrl-<n>.
- Define new properties like "ti,hwmods", "ti,needs-special-reset"
  "ti,needs-special-hs-handling", "cap-mmc-dual-data-rate"
  and "pbias-supply".
- Remove "ti,hwmods", "pinctrl-names" and "pinctrl-<n>"
  from required as they are not necessary for all DTS files.
- Add missing strings like "default-rev11", "sdr12-rev11", "sdr25-rev11",
  "hs-rev11", "sdr25-rev11" and "sleep" to pinctrl-names string array.

Signed-off-by: Charan Pedumuru <charan.pedumuru@gmail.com>
---
 .../devicetree/bindings/mmc/sdhci-omap.txt         |  43 ------
 .../devicetree/bindings/mmc/sdhci-omap.yaml        | 155 +++++++++++++++++++++
 2 files changed, 155 insertions(+), 43 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/sdhci-omap.txt b/Documentation/devicetree/bindings/mmc/sdhci-omap.txt
deleted file mode 100644
index f91e341e6b36c410275e6f993dd08400be3fc1f8..0000000000000000000000000000000000000000
--- a/Documentation/devicetree/bindings/mmc/sdhci-omap.txt
+++ /dev/null
@@ -1,43 +0,0 @@
-* TI OMAP SDHCI Controller
-
-Refer to mmc.txt for standard MMC bindings.
-
-For UHS devices which require tuning, the device tree should have a "cpu_thermal" node which maps to the appropriate thermal zone. This is used to get the temperature of the zone during tuning.
-
-Required properties:
-- compatible: Should be "ti,omap2430-sdhci" for omap2430 controllers
-	      Should be "ti,omap3-sdhci" for omap3 controllers
-	      Should be "ti,omap4-sdhci" for omap4 and ti81 controllers
-	      Should be "ti,omap5-sdhci" for omap5 controllers
-	      Should be "ti,dra7-sdhci" for DRA7 and DRA72 controllers
-	      Should be "ti,k2g-sdhci" for K2G
-	      Should be "ti,am335-sdhci" for am335x controllers
-	      Should be "ti,am437-sdhci" for am437x controllers
-- ti,hwmods: Must be "mmc<n>", <n> is controller instance starting 1
-	     (Not required for K2G).
-- pinctrl-names: Should be subset of "default", "hs", "sdr12", "sdr25", "sdr50",
-		 "ddr50-rev11", "sdr104-rev11", "ddr50", "sdr104",
-		 "ddr_1_8v-rev11", "ddr_1_8v" or "ddr_3_3v", "hs200_1_8v-rev11",
-		 "hs200_1_8v",
-- pinctrl-<n> : Pinctrl states as described in bindings/pinctrl/pinctrl-bindings.txt
-
-Optional properties:
-- dmas:		List of DMA specifiers with the controller specific format as described
-		in the generic DMA client binding. A tx and rx specifier is required.
-- dma-names:	List of DMA request names. These strings correspond 1:1 with the
-		DMA specifiers listed in dmas. The string naming is to be "tx"
-		and "rx" for TX and RX DMA requests, respectively.
-
-Deprecated properties:
-- ti,non-removable: Compatible with the generic non-removable property
-
-Example:
-	mmc1: mmc@4809c000 {
-		compatible = "ti,dra7-sdhci";
-		reg = <0x4809c000 0x400>;
-		ti,hwmods = "mmc1";
-		bus-width = <4>;
-		vmmc-supply = <&vmmc>; /* phandle to regulator node */
-		dmas = <&sdma 61 &sdma 62>;
-		dma-names = "tx", "rx";
-	};
diff --git a/Documentation/devicetree/bindings/mmc/sdhci-omap.yaml b/Documentation/devicetree/bindings/mmc/sdhci-omap.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..e707837bc242b055bbc497ed893a91c9b24f2dde
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/sdhci-omap.yaml
@@ -0,0 +1,155 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mmc/sdhci-omap.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TI OMAP SDHCI Controller
+
+maintainers:
+  - Ulf Hansson <ulf.hansson@linaro.org>
+
+description:
+  For UHS devices which require tuning, the device tree should have a
+  cpu_thermal node which maps to the appropriate thermal zone. This
+  is used to get the temperature of the zone during tuning.
+
+allOf:
+  - $ref: sdhci-common.yaml#
+
+properties:
+  compatible:
+    enum:
+      - ti,omap2430-sdhci
+      - ti,omap3-sdhci
+      - ti,omap4-sdhci
+      - ti,omap5-sdhci
+      - ti,dra7-sdhci
+      - ti,k2g-sdhci
+      - ti,am335-sdhci
+      - ti,am437-sdhci
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  pinctrl-names:
+    $ref: /schemas/types.yaml#/definitions/string-array
+    minItems: 1
+    maxItems: 19
+    items:
+      enum:
+        - default
+        - default-rev11
+        - hs
+        - sdr12
+        - sdr12-rev11
+        - sdr25
+        - sdr25-rev11
+        - sdr50
+        - ddr50-rev11
+        - sdr104-rev11
+        - ddr50
+        - sdr104
+        - ddr_1_8v-rev11
+        - ddr_1_8v
+        - ddr_3_3v
+        - hs-rev11
+        - hs200_1_8v-rev11
+        - hs200_1_8v
+        - sleep
+
+  dmas:
+    maxItems: 2
+
+  dma-names:
+    items:
+      - const: tx
+      - const: rx
+
+  ti,hwmods:
+    $ref: /schemas/types.yaml#/definitions/string
+    description:
+      This field is used to fetch the information such as
+      address range, irq lines, dma lines, interconnect, PRCM register,
+      clock domain, input clocks associated with MMC.
+    pattern: "^mmc[0-9]+$"
+
+  ti,needs-special-reset:
+    description:
+      It indicates that a specific soft reset sequence is required for
+      certain Texas Instruments devices, particularly those with
+      HSMMC (High-Speed MultiMediaCard) controllers.
+    type: boolean
+
+  ti,needs-special-hs-handling:
+    description:
+      It's presence in an MMC controller's DT node signals to the Linux kernel's
+      omap_hsmmc driver that this particular IP block requires special software
+      handling or workarounds to correctly manage High-Speed (HS) modes like
+      SDR25, SDR50, SDR104, DDR50.
+    type: boolean
+
+  pbias-supply:
+    description:
+      It is used to specify the voltage regulator that provides the bias
+      voltage for certain analog or I/O pads.
+
+  cap-mmc-dual-data-rate:
+    description:
+      A characteristic or capability associated with MultiMediaCard (MMC)
+      interfaces, specifically indicating that the MMC controller
+      supports Dual Data Rate (DDR) mode
+    type: boolean
+
+  ti,non-removable:
+    description:
+      It indicates that a component is not meant to be easily removed or
+      replaced by the user, such as an embedded battery or a non-removable
+      storage slot like eMMC.
+    type: boolean
+    deprecated: true
+
+  vmmmc-supply:
+    description:
+      It is used to specify the power supply (regulator) for the MMC/SD card's
+      main operating voltage (VCC/VDD).
+
+  clock-frequency:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      It is used to specify the frequency of a clock in Hertz (Hz). It's a
+      fundamental property for communicating hardware clocking information from
+      the Device Tree to the Linux kernel.
+
+patternProperties:
+  "^pinctrl-[0-9]+$":
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description:
+      Phandles to pinctrl states. The numeric suffix determines the
+      state index corresponding to entries in the pinctrl-names array.
+    minItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    mmc@4809c000 {
+        compatible = "ti,omap2430-sdhci";
+        reg = <0x4809c000 0x400>;
+        interrupts = <GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>;
+        ti,hwmods = "mmc1";
+        bus-width = <4>;
+        vmmc-supply = <&vmmc>; /* phandle to regulator node */
+        dmas = <&sdma 61>, <&sdma 62>;
+        dma-names = "tx", "rx";
+    };
+...

---
base-commit: ed61cb3d78d585209ec775933078e268544fe9a4
change-id: 20250519-ti-sdhci-omap-907f847f7530

Best regards,
-- 
Charan Pedumuru <charan.pedumuru@gmail.com>

Re: [PATCH] dt-bindings: mmc: sdhci-omap: convert text based binding to json schema

[Krzysztof Kozlowski]

On 23/05/2025 19:05, Charan Pedumuru wrote:
> Convert TI OMAP SDHCI Controller binding to YAML format.
> Changes during Conversion:
> - Add patternProperties for pinctrl-<n>.
> - Define new properties like "ti,hwmods", "ti,needs-special-reset"
>   "ti,needs-special-hs-handling", "cap-mmc-dual-data-rate"
>   and "pbias-supply".

Why? commit should answer this.

> - Remove "ti,hwmods", "pinctrl-names" and "pinctrl-<n>"

Why? You just added ti,hwmods, so how can you remove it from required?

>   from required as they are not necessary for all DTS files.
> - Add missing strings like "default-rev11", "sdr12-rev11", "sdr25-rev11",
>   "hs-rev11", "sdr25-rev11" and "sleep" to pinctrl-names string array.
> 
> Signed-off-by: Charan Pedumuru <charan.pedumuru@gmail.com>
> ---
>  .../devicetree/bindings/mmc/sdhci-omap.txt         |  43 ------
>  .../devicetree/bindings/mmc/sdhci-omap.yaml        | 155 +++++++++++++++++++++


Filename: ti,omap-sdhci.yaml or one of the compatibles (or anything else
following convention that it should match compatible).


"ti,needs-special-hs-handling" is already documented in other binding


>  2 files changed, 155 insertions(+), 43 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-omap.txt b/Documentation/devicetree/bindings/mmc/sdhci-omap.txt
> deleted file mode 100644
> index f91e341e6b36c410275e6f993dd08400be3fc1f8..0000000000000000000000000000000000000000
> --- a/Documentation/devicetree/bindings/mmc/sdhci-omap.txt
> +++ /dev/null
> @@ -1,43 +0,0 @@
> -* TI OMAP SDHCI Controller


...


> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-omap.yaml b/Documentation/devicetree/bindings/mmc/sdhci-omap.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..e707837bc242b055bbc497ed893a91c9b24f2dde
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/sdhci-omap.yaml
> @@ -0,0 +1,155 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mmc/sdhci-omap.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TI OMAP SDHCI Controller
> +
> +maintainers:
> +  - Ulf Hansson <ulf.hansson@linaro.org>

This is supposed to be someone caring about this device. Eventually
platform maintainer.

> +
> +description:
> +  For UHS devices which require tuning, the device tree should have a
> +  cpu_thermal node which maps to the appropriate thermal zone. This
> +  is used to get the temperature of the zone during tuning.
> +
> +allOf:
> +  - $ref: sdhci-common.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - ti,omap2430-sdhci
> +      - ti,omap3-sdhci
> +      - ti,omap4-sdhci
> +      - ti,omap5-sdhci
> +      - ti,dra7-sdhci
> +      - ti,k2g-sdhci
> +      - ti,am335-sdhci
> +      - ti,am437-sdhci
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  pinctrl-names:
> +    $ref: /schemas/types.yaml#/definitions/string-array
> +    minItems: 1
> +    maxItems: 19
> +    items:
> +      enum:
> +        - default
> +        - default-rev11
> +        - hs
> +        - sdr12
> +        - sdr12-rev11
> +        - sdr25
> +        - sdr25-rev11
> +        - sdr50
> +        - ddr50-rev11
> +        - sdr104-rev11
> +        - ddr50
> +        - sdr104
> +        - ddr_1_8v-rev11
> +        - ddr_1_8v
> +        - ddr_3_3v
> +        - hs-rev11
> +        - hs200_1_8v-rev11
> +        - hs200_1_8v
> +        - sleep
> +
> +  dmas:
> +    maxItems: 2
> +
> +  dma-names:
> +    items:
> +      - const: tx
> +      - const: rx
> +
> +  ti,hwmods:
> +    $ref: /schemas/types.yaml#/definitions/string
> +    description:
> +      This field is used to fetch the information such as
> +      address range, irq lines, dma lines, interconnect, PRCM register,
> +      clock domain, input clocks associated with MMC.
> +    pattern: "^mmc[0-9]+$"
> +
> +  ti,needs-special-reset:

I don't understand why you added this. There is no user of it.

> +    description:
> +      It indicates that a specific soft reset sequence is required for
> +      certain Texas Instruments devices, particularly those with
> +      HSMMC (High-Speed MultiMediaCard) controllers.
> +    type: boolean
> +
> +  ti,needs-special-hs-handling:

I don't understand why you added this. There is no user of it.


> +    description:
> +      It's presence in an MMC controller's DT node signals to the Linux kernel's
> +      omap_hsmmc driver that this particular IP block requires special software
> +      handling or workarounds to correctly manage High-Speed (HS) modes like
> +      SDR25, SDR50, SDR104, DDR50.
> +    type: boolean
> +
> +  pbias-supply:
> +    description:
> +      It is used to specify the voltage regulator that provides the bias
> +      voltage for certain analog or I/O pads.
> +
> +  cap-mmc-dual-data-rate:
> +    description:
> +      A characteristic or capability associated with MultiMediaCard (MMC)
> +      interfaces, specifically indicating that the MMC controller
> +      supports Dual Data Rate (DDR) mode

Drop the property. We have standard properties for this and there is no
ABI for it anyway.

> +    type: boolean
> +
> +  ti,non-removable:
> +    description:
> +      It indicates that a component is not meant to be easily removed or
> +      replaced by the user, such as an embedded battery or a non-removable
> +      storage slot like eMMC.
> +    type: boolean
> +    deprecated: true
> +
> +  vmmmc-supply:
> +    description:
> +      It is used to specify the power supply (regulator) for the MMC/SD card's
> +      main operating voltage (VCC/VDD).
> +
> +  clock-frequency:

Why is it here? Nothing in commit msg explained adding it.

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      It is used to specify the frequency of a clock in Hertz (Hz). It's a
> +      fundamental property for communicating hardware clocking information from
> +      the Device Tree to the Linux kernel.

Redundant description. It is not a fundamental property. It is a legacy
property.

> +
> +patternProperties:
> +  "^pinctrl-[0-9]+$":
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    description:
> +      Phandles to pinctrl states. The numeric suffix determines the
> +      state index corresponding to entries in the pinctrl-names array.
> +    minItems: 1

Why exactly do you need these?

> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +unevaluatedProperties: false
> +
Best regards,
Krzysztof

Re: [PATCH] dt-bindings: mmc: sdhci-omap: convert text based binding to json schema

[Charan Pedumuru]

On 28-05-2025 13:30, Krzysztof Kozlowski wrote:
> On 23/05/2025 19:05, Charan Pedumuru wrote:
>> Convert TI OMAP SDHCI Controller binding to YAML format.
>> Changes during Conversion:
>> - Add patternProperties for pinctrl-<n>.
>> - Define new properties like "ti,hwmods", "ti,needs-special-reset"
>>   "ti,needs-special-hs-handling", "cap-mmc-dual-data-rate"
>>   and "pbias-supply".
> 
> Why? commit should answer this.

The above properties are not documented in the text binding, so I defined them to resolve DTB_CHECK, I will write the reason in next revision.

> 
>> - Remove "ti,hwmods", "pinctrl-names" and "pinctrl-<n>"
> 
> Why? You just added ti,hwmods, so how can you remove it from required?

The property is defined but is not required by all DTS files and the old binding says it is required for all boards, I will add this reason to the commit message.

> 
>>   from required as they are not necessary for all DTS files.
>> - Add missing strings like "default-rev11", "sdr12-rev11", "sdr25-rev11",
>>   "hs-rev11", "sdr25-rev11" and "sleep" to pinctrl-names string array.
>>
>> Signed-off-by: Charan Pedumuru <charan.pedumuru@gmail.com>
>> ---
>>  .../devicetree/bindings/mmc/sdhci-omap.txt         |  43 ------
>>  .../devicetree/bindings/mmc/sdhci-omap.yaml        | 155 +++++++++++++++++++++
> 
> 
> Filename: ti,omap-sdhci.yaml or one of the compatibles (or anything else
> following convention that it should match compatible).

Sure, I was following the name format of other files from the same directory here, but will change it to the compatible in next revision.

> 
> 
> "ti,needs-special-hs-handling" is already documented in other binding

Well, I didn't see this property defined in any common.yaml in mmc directory.

> 
> 
>>  2 files changed, 155 insertions(+), 43 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-omap.txt b/Documentation/devicetree/bindings/mmc/sdhci-omap.txt
>> deleted file mode 100644
>> index f91e341e6b36c410275e6f993dd08400be3fc1f8..0000000000000000000000000000000000000000
>> --- a/Documentation/devicetree/bindings/mmc/sdhci-omap.txt
>> +++ /dev/null
>> @@ -1,43 +0,0 @@
>> -* TI OMAP SDHCI Controller
> 
> 
> ...
> 
> 
>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-omap.yaml b/Documentation/devicetree/bindings/mmc/sdhci-omap.yaml
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..e707837bc242b055bbc497ed893a91c9b24f2dde
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-omap.yaml
>> @@ -0,0 +1,155 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/mmc/sdhci-omap.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: TI OMAP SDHCI Controller
>> +
>> +maintainers:
>> +  - Ulf Hansson <ulf.hansson@linaro.org>
> 
> This is supposed to be someone caring about this device. Eventually
> platform maintainer.

Sure, I will change that, I was following the names of MAINTAINERS from the list I got from the command, "./scripts/get_maintainer.pl Documentation/dev
icetree/bindings/mmc/sdhci-omap.txt"

> 
>> +
>> +description:
>> +  For UHS devices which require tuning, the device tree should have a
>> +  cpu_thermal node which maps to the appropriate thermal zone. This
>> +  is used to get the temperature of the zone during tuning.
>> +
>> +allOf:
>> +  - $ref: sdhci-common.yaml#
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - ti,omap2430-sdhci
>> +      - ti,omap3-sdhci
>> +      - ti,omap4-sdhci
>> +      - ti,omap5-sdhci
>> +      - ti,dra7-sdhci
>> +      - ti,k2g-sdhci
>> +      - ti,am335-sdhci
>> +      - ti,am437-sdhci
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  pinctrl-names:
>> +    $ref: /schemas/types.yaml#/definitions/string-array
>> +    minItems: 1
>> +    maxItems: 19
>> +    items:
>> +      enum:
>> +        - default
>> +        - default-rev11
>> +        - hs
>> +        - sdr12
>> +        - sdr12-rev11
>> +        - sdr25
>> +        - sdr25-rev11
>> +        - sdr50
>> +        - ddr50-rev11
>> +        - sdr104-rev11
>> +        - ddr50
>> +        - sdr104
>> +        - ddr_1_8v-rev11
>> +        - ddr_1_8v
>> +        - ddr_3_3v
>> +        - hs-rev11
>> +        - hs200_1_8v-rev11
>> +        - hs200_1_8v
>> +        - sleep
>> +
>> +  dmas:
>> +    maxItems: 2
>> +
>> +  dma-names:
>> +    items:
>> +      - const: tx
>> +      - const: rx
>> +
>> +  ti,hwmods:
>> +    $ref: /schemas/types.yaml#/definitions/string
>> +    description:
>> +      This field is used to fetch the information such as
>> +      address range, irq lines, dma lines, interconnect, PRCM register,
>> +      clock domain, input clocks associated with MMC.
>> +    pattern: "^mmc[0-9]+$"
>> +
>> +  ti,needs-special-reset:
> 
> I don't understand why you added this. There is no user of it.


May be, but the DTB_CHECK failed for some boards when not defined it here.

> 
>> +    description:
>> +      It indicates that a specific soft reset sequence is required for
>> +      certain Texas Instruments devices, particularly those with
>> +      HSMMC (High-Speed MultiMediaCard) controllers.
>> +    type: boolean
>> +
>> +  ti,needs-special-hs-handling:
> 
> I don't understand why you added this. There is no user of it.

...

> 
> 
>> +    description:
>> +      It's presence in an MMC controller's DT node signals to the Linux kernel's
>> +      omap_hsmmc driver that this particular IP block requires special software
>> +      handling or workarounds to correctly manage High-Speed (HS) modes like
>> +      SDR25, SDR50, SDR104, DDR50.
>> +    type: boolean
>> +
>> +  pbias-supply:
>> +    description:
>> +      It is used to specify the voltage regulator that provides the bias
>> +      voltage for certain analog or I/O pads.
>> +
>> +  cap-mmc-dual-data-rate:
>> +    description:
>> +      A characteristic or capability associated with MultiMediaCard (MMC)
>> +      interfaces, specifically indicating that the MMC controller
>> +      supports Dual Data Rate (DDR) mode
> 
> Drop the property. We have standard properties for this and there is no
> ABI for it anyway.
> 

Same here, the DTB_CHECK failed, so had to define it here

>> +    type: boolean
>> +
>> +  ti,non-removable:
>> +    description:
>> +      It indicates that a component is not meant to be easily removed or
>> +      replaced by the user, such as an embedded battery or a non-removable
>> +      storage slot like eMMC.
>> +    type: boolean
>> +    deprecated: true
>> +
>> +  vmmmc-supply:
>> +    description:
>> +      It is used to specify the power supply (regulator) for the MMC/SD card's
>> +      main operating voltage (VCC/VDD).
>> +
>> +  clock-frequency:
> 
> Why is it here? Nothing in commit msg explained adding it.

I will add this change to commit message along with the reason.

> 
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    description:
>> +      It is used to specify the frequency of a clock in Hertz (Hz). It's a
>> +      fundamental property for communicating hardware clocking information from
>> +      the Device Tree to the Linux kernel.
> 
> Redundant description. It is not a fundamental property. It is a legacy
> property.
> 

Sure, will change the description.

>> +
>> +patternProperties:
>> +  "^pinctrl-[0-9]+$":
>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>> +    description:
>> +      Phandles to pinctrl states. The numeric suffix determines the
>> +      state index corresponding to entries in the pinctrl-names array.
>> +    minItems: 1
> 
> Why exactly do you need these?

Some boards have this property with multiple pincontrol states, so had to define a pattern property to recognize all the defined pinctrl properties.

> 
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - interrupts
>> +
>> +unevaluatedProperties: false
>> +
> Best regards,
> Krzysztof

-- 
Best Regards,
Charan.

Re: [PATCH] dt-bindings: mmc: sdhci-omap: convert text based binding to json schema

[Krzysztof Kozlowski]

On 06/09/2025 10:43, Charan Pedumuru wrote:
> 
> 
> On 28-05-2025 13:30, Krzysztof Kozlowski wrote:
>> On 23/05/2025 19:05, Charan Pedumuru wrote:
>>> Convert TI OMAP SDHCI Controller binding to YAML format.
>>> Changes during Conversion:
>>> - Add patternProperties for pinctrl-<n>.
>>> - Define new properties like "ti,hwmods", "ti,needs-special-reset"
>>>   "ti,needs-special-hs-handling", "cap-mmc-dual-data-rate"
>>>   and "pbias-supply".
>>
>> Why? commit should answer this.
> 
> The above properties are not documented in the text binding, so I defined them to resolve DTB_CHECK, I will write the reason in next revision.

You revive discussion from 3 months ago...

Anyway, explain in the commit msg that properties are already used in
the DTS.

> 
>>
>>> - Remove "ti,hwmods", "pinctrl-names" and "pinctrl-<n>"
>>
>> Why? You just added ti,hwmods, so how can you remove it from required?
> 
> The property is defined but is not required by all DTS files and the old binding says it is required for all boards, I will add this reason to the commit message.
> 
>>
>>>   from required as they are not necessary for all DTS files.
>>> - Add missing strings like "default-rev11", "sdr12-rev11", "sdr25-rev11",
>>>   "hs-rev11", "sdr25-rev11" and "sleep" to pinctrl-names string array.
>>>
>>> Signed-off-by: Charan Pedumuru <charan.pedumuru@gmail.com>
>>> ---
>>>  .../devicetree/bindings/mmc/sdhci-omap.txt         |  43 ------
>>>  .../devicetree/bindings/mmc/sdhci-omap.yaml        | 155 +++++++++++++++++++++
>>
>>
>> Filename: ti,omap-sdhci.yaml or one of the compatibles (or anything else
>> following convention that it should match compatible).
> 
> Sure, I was following the name format of other files from the same directory here, but will change it to the compatible in next revision.
> 
>>
>>
>> "ti,needs-special-hs-handling" is already documented in other binding
> 
> Well, I didn't see this property defined in any common.yaml in mmc directory.
> 
>>
>>
>>>  2 files changed, 155 insertions(+), 43 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-omap.txt b/Documentation/devicetree/bindings/mmc/sdhci-omap.txt
>>> deleted file mode 100644
>>> index f91e341e6b36c410275e6f993dd08400be3fc1f8..0000000000000000000000000000000000000000
>>> --- a/Documentation/devicetree/bindings/mmc/sdhci-omap.txt
>>> +++ /dev/null
>>> @@ -1,43 +0,0 @@
>>> -* TI OMAP SDHCI Controller
>>
>>
>> ...
>>
>>
>>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-omap.yaml b/Documentation/devicetree/bindings/mmc/sdhci-omap.yaml
>>> new file mode 100644
>>> index 0000000000000000000000000000000000000000..e707837bc242b055bbc497ed893a91c9b24f2dde
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-omap.yaml
>>> @@ -0,0 +1,155 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/mmc/sdhci-omap.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: TI OMAP SDHCI Controller
>>> +
>>> +maintainers:
>>> +  - Ulf Hansson <ulf.hansson@linaro.org>
>>
>> This is supposed to be someone caring about this device. Eventually
>> platform maintainer.
> 
> Sure, I will change that, I was following the names of MAINTAINERS from the list I got from the command, "./scripts/get_maintainer.pl Documentation/dev
> icetree/bindings/mmc/sdhci-omap.txt"
> 
>>
>>> +
>>> +description:
>>> +  For UHS devices which require tuning, the device tree should have a
>>> +  cpu_thermal node which maps to the appropriate thermal zone. This
>>> +  is used to get the temperature of the zone during tuning.
>>> +
>>> +allOf:
>>> +  - $ref: sdhci-common.yaml#
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - ti,omap2430-sdhci
>>> +      - ti,omap3-sdhci
>>> +      - ti,omap4-sdhci
>>> +      - ti,omap5-sdhci
>>> +      - ti,dra7-sdhci
>>> +      - ti,k2g-sdhci
>>> +      - ti,am335-sdhci
>>> +      - ti,am437-sdhci
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +  interrupts:
>>> +    maxItems: 1
>>> +
>>> +  pinctrl-names:
>>> +    $ref: /schemas/types.yaml#/definitions/string-array
>>> +    minItems: 1
>>> +    maxItems: 19
>>> +    items:
>>> +      enum:
>>> +        - default
>>> +        - default-rev11
>>> +        - hs
>>> +        - sdr12
>>> +        - sdr12-rev11
>>> +        - sdr25
>>> +        - sdr25-rev11
>>> +        - sdr50
>>> +        - ddr50-rev11
>>> +        - sdr104-rev11
>>> +        - ddr50
>>> +        - sdr104
>>> +        - ddr_1_8v-rev11
>>> +        - ddr_1_8v
>>> +        - ddr_3_3v
>>> +        - hs-rev11
>>> +        - hs200_1_8v-rev11
>>> +        - hs200_1_8v
>>> +        - sleep
>>> +
>>> +  dmas:
>>> +    maxItems: 2
>>> +
>>> +  dma-names:
>>> +    items:
>>> +      - const: tx
>>> +      - const: rx
>>> +
>>> +  ti,hwmods:
>>> +    $ref: /schemas/types.yaml#/definitions/string
>>> +    description:
>>> +      This field is used to fetch the information such as
>>> +      address range, irq lines, dma lines, interconnect, PRCM register,
>>> +      clock domain, input clocks associated with MMC.
>>> +    pattern: "^mmc[0-9]+$"
>>> +
>>> +  ti,needs-special-reset:
>>
>> I don't understand why you added this. There is no user of it.
> 
> 
> May be, but the DTB_CHECK failed for some boards when not defined it here.

Then maybe should be dropped from DTS?

> 
>>
>>> +    description:
>>> +      It indicates that a specific soft reset sequence is required for
>>> +      certain Texas Instruments devices, particularly those with
>>> +      HSMMC (High-Speed MultiMediaCard) controllers.
>>> +    type: boolean
>>> +
>>> +  ti,needs-special-hs-handling:
>>
>> I don't understand why you added this. There is no user of it.
> 
> ...
> 
>>
>>
>>> +    description:
>>> +      It's presence in an MMC controller's DT node signals to the Linux kernel's
>>> +      omap_hsmmc driver that this particular IP block requires special software
>>> +      handling or workarounds to correctly manage High-Speed (HS) modes like
>>> +      SDR25, SDR50, SDR104, DDR50.
>>> +    type: boolean
>>> +
>>> +  pbias-supply:
>>> +    description:
>>> +      It is used to specify the voltage regulator that provides the bias
>>> +      voltage for certain analog or I/O pads.
>>> +
>>> +  cap-mmc-dual-data-rate:
>>> +    description:
>>> +      A characteristic or capability associated with MultiMediaCard (MMC)
>>> +      interfaces, specifically indicating that the MMC controller
>>> +      supports Dual Data Rate (DDR) mode
>>
>> Drop the property. We have standard properties for this and there is no
>> ABI for it anyway.
>>
> 
> Same here, the DTB_CHECK failed, so had to define it here
> 
>>> +    type: boolean
>>> +
>>> +  ti,non-removable:
>>> +    description:
>>> +      It indicates that a component is not meant to be easily removed or
>>> +      replaced by the user, such as an embedded battery or a non-removable
>>> +      storage slot like eMMC.
>>> +    type: boolean
>>> +    deprecated: true
>>> +
>>> +  vmmmc-supply:
>>> +    description:
>>> +      It is used to specify the power supply (regulator) for the MMC/SD card's
>>> +      main operating voltage (VCC/VDD).
>>> +
>>> +  clock-frequency:
>>
>> Why is it here? Nothing in commit msg explained adding it.
> 
> I will add this change to commit message along with the reason.
> 
>>
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>> +    description:
>>> +      It is used to specify the frequency of a clock in Hertz (Hz). It's a
>>> +      fundamental property for communicating hardware clocking information from
>>> +      the Device Tree to the Linux kernel.
>>
>> Redundant description. It is not a fundamental property. It is a legacy
>> property.
>>
> 
> Sure, will change the description.
> 
>>> +
>>> +patternProperties:
>>> +  "^pinctrl-[0-9]+$":
>>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>>> +    description:
>>> +      Phandles to pinctrl states. The numeric suffix determines the
>>> +      state index corresponding to entries in the pinctrl-names array.
>>> +    minItems: 1
>>
>> Why exactly do you need these?
> 
> Some boards have this property with multiple pincontrol states, so had to define a pattern property to recognize all the defined pinctrl properties.

No, that's just confusing error from dtschema. Look at other bindings -
no binding defines type and description for pinctrl.

It just means your schema was incomplete.

Best regards,
Krzysztof

Re: [PATCH] dt-bindings: mmc: sdhci-omap: convert text based binding to json schema

[Charan Pedumuru]

On 06-09-2025 14:25, Krzysztof Kozlowski wrote:
> On 06/09/2025 10:43, Charan Pedumuru wrote:
>>
>>
>> On 28-05-2025 13:30, Krzysztof Kozlowski wrote:
>>> On 23/05/2025 19:05, Charan Pedumuru wrote:
>>>> Convert TI OMAP SDHCI Controller binding to YAML format.
>>>> Changes during Conversion:
>>>> - Add patternProperties for pinctrl-<n>.
>>>> - Define new properties like "ti,hwmods", "ti,needs-special-reset"
>>>>   "ti,needs-special-hs-handling", "cap-mmc-dual-data-rate"
>>>>   and "pbias-supply".
>>>
>>> Why? commit should answer this.
>>
>> The above properties are not documented in the text binding, so I defined them to resolve DTB_CHECK, I will write the reason in next revision.
> 
> You revive discussion from 3 months ago...
> 
> Anyway, explain in the commit msg that properties are already used in
> the DTS.

Sure.

> 
>>
>>>
>>>> - Remove "ti,hwmods", "pinctrl-names" and "pinctrl-<n>"
>>>
>>> Why? You just added ti,hwmods, so how can you remove it from required?
>>
>> The property is defined but is not required by all DTS files and the old binding says it is required for all boards, I will add this reason to the commit message.
>>
>>>
>>>>   from required as they are not necessary for all DTS files.
>>>> - Add missing strings like "default-rev11", "sdr12-rev11", "sdr25-rev11",
>>>>   "hs-rev11", "sdr25-rev11" and "sleep" to pinctrl-names string array.
>>>>
>>>> Signed-off-by: Charan Pedumuru <charan.pedumuru@gmail.com>
>>>> ---
>>>>  .../devicetree/bindings/mmc/sdhci-omap.txt         |  43 ------
>>>>  .../devicetree/bindings/mmc/sdhci-omap.yaml        | 155 +++++++++++++++++++++
>>>
>>>
>>> Filename: ti,omap-sdhci.yaml or one of the compatibles (or anything else
>>> following convention that it should match compatible).
>>
>> Sure, I was following the name format of other files from the same directory here, but will change it to the compatible in next revision.
>>
>>>
>>>
>>> "ti,needs-special-hs-handling" is already documented in other binding
>>
>> Well, I didn't see this property defined in any common.yaml in mmc directory.
>>
>>>
>>>
>>>>  2 files changed, 155 insertions(+), 43 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-omap.txt b/Documentation/devicetree/bindings/mmc/sdhci-omap.txt
>>>> deleted file mode 100644
>>>> index f91e341e6b36c410275e6f993dd08400be3fc1f8..0000000000000000000000000000000000000000
>>>> --- a/Documentation/devicetree/bindings/mmc/sdhci-omap.txt
>>>> +++ /dev/null
>>>> @@ -1,43 +0,0 @@
>>>> -* TI OMAP SDHCI Controller
>>>
>>>
>>> ...
>>>
>>>
>>>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-omap.yaml b/Documentation/devicetree/bindings/mmc/sdhci-omap.yaml
>>>> new file mode 100644
>>>> index 0000000000000000000000000000000000000000..e707837bc242b055bbc497ed893a91c9b24f2dde
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-omap.yaml
>>>> @@ -0,0 +1,155 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/mmc/sdhci-omap.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: TI OMAP SDHCI Controller
>>>> +
>>>> +maintainers:
>>>> +  - Ulf Hansson <ulf.hansson@linaro.org>
>>>
>>> This is supposed to be someone caring about this device. Eventually
>>> platform maintainer.
>>
>> Sure, I will change that, I was following the names of MAINTAINERS from the list I got from the command, "./scripts/get_maintainer.pl Documentation/dev
>> icetree/bindings/mmc/sdhci-omap.txt"
>>
>>>
>>>> +
>>>> +description:
>>>> +  For UHS devices which require tuning, the device tree should have a
>>>> +  cpu_thermal node which maps to the appropriate thermal zone. This
>>>> +  is used to get the temperature of the zone during tuning.
>>>> +
>>>> +allOf:
>>>> +  - $ref: sdhci-common.yaml#
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    enum:
>>>> +      - ti,omap2430-sdhci
>>>> +      - ti,omap3-sdhci
>>>> +      - ti,omap4-sdhci
>>>> +      - ti,omap5-sdhci
>>>> +      - ti,dra7-sdhci
>>>> +      - ti,k2g-sdhci
>>>> +      - ti,am335-sdhci
>>>> +      - ti,am437-sdhci
>>>> +
>>>> +  reg:
>>>> +    maxItems: 1
>>>> +
>>>> +  interrupts:
>>>> +    maxItems: 1
>>>> +
>>>> +  pinctrl-names:
>>>> +    $ref: /schemas/types.yaml#/definitions/string-array
>>>> +    minItems: 1
>>>> +    maxItems: 19
>>>> +    items:
>>>> +      enum:
>>>> +        - default
>>>> +        - default-rev11
>>>> +        - hs
>>>> +        - sdr12
>>>> +        - sdr12-rev11
>>>> +        - sdr25
>>>> +        - sdr25-rev11
>>>> +        - sdr50
>>>> +        - ddr50-rev11
>>>> +        - sdr104-rev11
>>>> +        - ddr50
>>>> +        - sdr104
>>>> +        - ddr_1_8v-rev11
>>>> +        - ddr_1_8v
>>>> +        - ddr_3_3v
>>>> +        - hs-rev11
>>>> +        - hs200_1_8v-rev11
>>>> +        - hs200_1_8v
>>>> +        - sleep
>>>> +
>>>> +  dmas:
>>>> +    maxItems: 2
>>>> +
>>>> +  dma-names:
>>>> +    items:
>>>> +      - const: tx
>>>> +      - const: rx
>>>> +
>>>> +  ti,hwmods:
>>>> +    $ref: /schemas/types.yaml#/definitions/string
>>>> +    description:
>>>> +      This field is used to fetch the information such as
>>>> +      address range, irq lines, dma lines, interconnect, PRCM register,
>>>> +      clock domain, input clocks associated with MMC.
>>>> +    pattern: "^mmc[0-9]+$"
>>>> +
>>>> +  ti,needs-special-reset:
>>>
>>> I don't understand why you added this. There is no user of it.
>>
>>
>> May be, but the DTB_CHECK failed for some boards when not defined it here.
> 
> Then maybe should be dropped from DTS?

Okay, should I drop the properties ti,needs-special-reset, ti,needs-special-hs-handling and cap-mmc-dual-data-rate from the DTS and send a patch series?

> 
>>
>>>
>>>> +    description:
>>>> +      It indicates that a specific soft reset sequence is required for
>>>> +      certain Texas Instruments devices, particularly those with
>>>> +      HSMMC (High-Speed MultiMediaCard) controllers.
>>>> +    type: boolean
>>>> +
>>>> +  ti,needs-special-hs-handling:
>>>
>>> I don't understand why you added this. There is no user of it.
>>
>> ...
>>
>>>
>>>
>>>> +    description:
>>>> +      It's presence in an MMC controller's DT node signals to the Linux kernel's
>>>> +      omap_hsmmc driver that this particular IP block requires special software
>>>> +      handling or workarounds to correctly manage High-Speed (HS) modes like
>>>> +      SDR25, SDR50, SDR104, DDR50.
>>>> +    type: boolean
>>>> +
>>>> +  pbias-supply:
>>>> +    description:
>>>> +      It is used to specify the voltage regulator that provides the bias
>>>> +      voltage for certain analog or I/O pads.
>>>> +
>>>> +  cap-mmc-dual-data-rate:
>>>> +    description:
>>>> +      A characteristic or capability associated with MultiMediaCard (MMC)
>>>> +      interfaces, specifically indicating that the MMC controller
>>>> +      supports Dual Data Rate (DDR) mode
>>>
>>> Drop the property. We have standard properties for this and there is no
>>> ABI for it anyway.
>>>
>>
>> Same here, the DTB_CHECK failed, so had to define it here
>>
>>>> +    type: boolean
>>>> +
>>>> +  ti,non-removable:
>>>> +    description:
>>>> +      It indicates that a component is not meant to be easily removed or
>>>> +      replaced by the user, such as an embedded battery or a non-removable
>>>> +      storage slot like eMMC.
>>>> +    type: boolean
>>>> +    deprecated: true
>>>> +
>>>> +  vmmmc-supply:
>>>> +    description:
>>>> +      It is used to specify the power supply (regulator) for the MMC/SD card's
>>>> +      main operating voltage (VCC/VDD).
>>>> +
>>>> +  clock-frequency:
>>>
>>> Why is it here? Nothing in commit msg explained adding it.
>>
>> I will add this change to commit message along with the reason.
>>
>>>
>>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>>> +    description:
>>>> +      It is used to specify the frequency of a clock in Hertz (Hz). It's a
>>>> +      fundamental property for communicating hardware clocking information from
>>>> +      the Device Tree to the Linux kernel.
>>>
>>> Redundant description. It is not a fundamental property. It is a legacy
>>> property.
>>>
>>
>> Sure, will change the description.
>>
>>>> +
>>>> +patternProperties:
>>>> +  "^pinctrl-[0-9]+$":
>>>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>>>> +    description:
>>>> +      Phandles to pinctrl states. The numeric suffix determines the
>>>> +      state index corresponding to entries in the pinctrl-names array.
>>>> +    minItems: 1
>>>
>>> Why exactly do you need these?
>>
>> Some boards have this property with multiple pincontrol states, so had to define a pattern property to recognize all the defined pinctrl properties.
> 
> No, that's just confusing error from dtschema. Look at other bindings -
> no binding defines type and description for pinctrl.

Okay, I will remove this pattern property and will define it like normal property following other bindings.

> 
> It just means your schema was incomplete.
> 
> Best regards,
> Krzysztof

-- 
Best Regards,
Charan.