[PATCH] dt-bindings: mmc: xenon: Convert to JSON schema

[PATCH] dt-bindings: mmc: xenon: Convert to JSON schema

[Chris Packham]

Convert the marvell,xenon-sdhci binding to JSON schema. This is a fairly
direct conversion so there are some requirements that are documented in
prose but not currently enforced.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
 .../bindings/mmc/marvell,xenon-sdhci.txt      | 173 ------------
 .../bindings/mmc/marvell,xenon-sdhci.yaml     | 252 ++++++++++++++++++
 2 files changed, 252 insertions(+), 173 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
 create mode 100644 Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml

diff --git a/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
deleted file mode 100644
index c51a62d751dc..000000000000
--- a/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
+++ /dev/null
@@ -1,173 +0,0 @@
-Marvell Xenon SDHCI Controller device tree bindings
-This file documents differences between the core mmc properties
-described by mmc.txt and the properties used by the Xenon implementation.
-
-Multiple SDHCs might be put into a single Xenon IP, to save size and cost.
-Each SDHC is independent and owns independent resources, such as register sets,
-clock and PHY.
-Each SDHC should have an independent device tree node.
-
-Required Properties:
-- compatible: should be one of the following
-  - "marvell,armada-3700-sdhci": For controllers on Armada-3700 SoC.
-  Must provide a second register area and marvell,pad-type.
-  - "marvell,armada-ap806-sdhci": For controllers on Armada AP806.
-  - "marvell,armada-ap807-sdhci": For controllers on Armada AP807.
-  - "marvell,armada-cp110-sdhci": For controllers on Armada CP110.
-
-- clocks:
-  Array of clocks required for SDHC.
-  Require at least input clock for Xenon IP core. For Armada AP806 and
-  CP110, the AXI clock is also mandatory.
-
-- clock-names:
-  Array of names corresponding to clocks property.
-  The input clock for Xenon IP core should be named as "core".
-  The input clock for the AXI bus must be named as "axi".
-
-- reg:
-  * For "marvell,armada-3700-sdhci", two register areas.
-    The first one for Xenon IP register. The second one for the Armada 3700 SoC
-    PHY PAD Voltage Control register.
-    Please follow the examples with compatible "marvell,armada-3700-sdhci"
-    in below.
-    Please also check property marvell,pad-type in below.
-
-  * For other compatible strings, one register area for Xenon IP.
-
-Optional Properties:
-- marvell,xenon-sdhc-id:
-  Indicate the corresponding bit index of current SDHC in
-  SDHC System Operation Control Register Bit[7:0].
-  Set/clear the corresponding bit to enable/disable current SDHC.
-  If Xenon IP contains only one SDHC, this property is optional.
-
-- marvell,xenon-phy-type:
-  Xenon support multiple types of PHYs.
-  To select eMMC 5.1 PHY, set:
-  marvell,xenon-phy-type = "emmc 5.1 phy"
-  eMMC 5.1 PHY is the default choice if this property is not provided.
-  To select eMMC 5.0 PHY, set:
-  marvell,xenon-phy-type = "emmc 5.0 phy"
-
-  All those types of PHYs can support eMMC, SD and SDIO.
-  Please note that this property only presents the type of PHY.
-  It doesn't stand for the entire SDHC type or property.
-  For example, "emmc 5.1 phy" doesn't mean that this Xenon SDHC only
-  supports eMMC 5.1.
-
-- marvell,xenon-phy-znr:
-  Set PHY ZNR value.
-  Only available for eMMC PHY.
-  Valid range = [0:0x1F].
-  ZNR is set as 0xF by default if this property is not provided.
-
-- marvell,xenon-phy-zpr:
-  Set PHY ZPR value.
-  Only available for eMMC PHY.
-  Valid range = [0:0x1F].
-  ZPR is set as 0xF by default if this property is not provided.
-
-- marvell,xenon-phy-nr-success-tun:
-  Set the number of required consecutive successful sampling points
-  used to identify a valid sampling window, in tuning process.
-  Valid range = [1:7].
-  Set as 0x4 by default if this property is not provided.
-
-- marvell,xenon-phy-tun-step-divider:
-  Set the divider for calculating TUN_STEP.
-  Set as 64 by default if this property is not provided.
-
-- marvell,xenon-phy-slow-mode:
-  If this property is selected, transfers will bypass PHY.
-  Only available when bus frequency lower than 55MHz in SDR mode.
-  Disabled by default. Please only try this property if timing issues
-  always occur with PHY enabled in eMMC HS SDR, SD SDR12, SD SDR25,
-  SD Default Speed and HS mode and eMMC legacy speed mode.
-
-- marvell,xenon-tun-count:
-  Xenon SDHC SoC usually doesn't provide re-tuning counter in
-  Capabilities Register 3 Bit[11:8].
-  This property provides the re-tuning counter.
-  If this property is not set, default re-tuning counter will
-  be set as 0x9 in driver.
-
-- marvell,pad-type:
-  Type of Armada 3700 SoC PHY PAD Voltage Controller register.
-  Only valid when "marvell,armada-3700-sdhci" is selected.
-  Two types: "sd" and "fixed-1-8v".
-  If "sd" is selected, SoC PHY PAD is set as 3.3V at the beginning and is
-  switched to 1.8V when later in higher speed mode.
-  If "fixed-1-8v" is selected, SoC PHY PAD is fixed 1.8V, such as for eMMC.
-  Please follow the examples with compatible "marvell,armada-3700-sdhci"
-  in below.
-
-Example:
-- For eMMC:
-
-	sdhci@aa0000 {
-		compatible = "marvell,armada-ap806-sdhci";
-		reg = <0xaa0000 0x1000>;
-		interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>
-		clocks = <&emmc_clk>,<&axi_clk>;
-		clock-names = "core", "axi";
-		bus-width = <4>;
-		marvell,xenon-phy-slow-mode;
-		marvell,xenon-tun-count = <11>;
-		non-removable;
-		no-sd;
-		no-sdio;
-
-		/* Vmmc and Vqmmc are both fixed */
-	};
-
-- For SD/SDIO:
-
-	sdhci@ab0000 {
-		compatible = "marvell,armada-cp110-sdhci";
-		reg = <0xab0000 0x1000>;
-		interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>
-		vqmmc-supply = <&sd_vqmmc_regulator>;
-		vmmc-supply = <&sd_vmmc_regulator>;
-		clocks = <&sdclk>, <&axi_clk>;
-		clock-names = "core", "axi";
-		bus-width = <4>;
-		marvell,xenon-tun-count = <9>;
-	};
-
-- For eMMC with compatible "marvell,armada-3700-sdhci":
-
-	sdhci@aa0000 {
-		compatible = "marvell,armada-3700-sdhci";
-		reg = <0xaa0000 0x1000>,
-		      <phy_addr 0x4>;
-		interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>
-		clocks = <&emmcclk>;
-		clock-names = "core";
-		bus-width = <8>;
-		mmc-ddr-1_8v;
-		mmc-hs400-1_8v;
-		non-removable;
-		no-sd;
-		no-sdio;
-
-		/* Vmmc and Vqmmc are both fixed */
-
-		marvell,pad-type = "fixed-1-8v";
-	};
-
-- For SD/SDIO with compatible "marvell,armada-3700-sdhci":
-
-	sdhci@ab0000 {
-		compatible = "marvell,armada-3700-sdhci";
-		reg = <0xab0000 0x1000>,
-		      <phy_addr 0x4>;
-		interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>
-		vqmmc-supply = <&sd_regulator>;
-		/* Vmmc is fixed */
-		clocks = <&sdclk>;
-		clock-names = "core";
-		bus-width = <4>;
-
-		marvell,pad-type = "sd";
-	};
diff --git a/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml
new file mode 100644
index 000000000000..22d5cbf28042
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml
@@ -0,0 +1,252 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mmc/marvell,xenon-sdhci.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Marvell Xenon SDHCI Controller device tree bindings
+
+description: |
+  This file documents differences between the core mmc properties described by
+  mmc-controller.yaml and the properties used by the Xenon implementation.
+
+  Multiple SDHCs might be put into a single Xenon IP, to save size and cost.
+  Each SDHC is independent and owns independent resources, such as register
+  sets, clock and PHY.
+
+  Each SDHC should have an independent device tree node.
+
+maintainers:
+  - Ulf Hansson <ulf.hansson@linaro.org>
+
+patternProperties:
+  "^sdhci@[0-9a-f]+$":
+    type: object
+    $ref: mmc-controller.yaml
+
+    properties:
+      compatible:
+        oneOf:
+          - const: marvell,armada-3700-sdhci
+            description: |
+              Must provide a second register area and marvell,pad-type
+          - const: marvell,armada-ap806-sdhci
+          - const: marvell,armada-ap807-sdhci
+          - const: marvell,armada-cp110-sdhci
+          - const: marvell,sdhci-xenon
+          - items:
+            - const: marvell,armada-3700-sdhci
+            - const: marvell,sdhci-xenon
+          - items:
+            - const: marvell,armada-ap807-sdhci
+            - const: marvell,armada-ap806-sdhci
+
+      reg:
+        minItems: 1
+        maxItems: 2
+        description: |
+          For "marvell,armada-3700-sdhci", two register areas.  The first one
+          for Xenon IP register. The second one for the Armada 3700 SoC PHY PAD
+          Voltage Control register.  Please follow the examples with compatible
+          "marvell,armada-3700-sdhci" in below.
+          Please also check property marvell,pad-type in below.
+
+          For other compatible strings, one register area for Xenon IP.
+
+      clocks:
+        minItems: 1
+        maxItems: 2
+
+      clock-names:
+        minItems: 1
+        items:
+          - const: core
+          - const: axi
+
+      marvell,xenon-sdhc-id:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 0
+        maximum: 7
+        description: |
+          Indicate the corresponding bit index of current SDHC in SDHC System
+          Operation Control Register Bit[7:0].  Set/clear the corresponding bit to
+          enable/disable current SDHC.  If Xenon IP contains only one SDHC, this
+          property is optional.
+
+      marvell,xenon-phy-type:
+        enum:
+          - "emmc 5.1 phy"
+          - "emmc 5.0 phy"
+        description: |
+          Xenon support multiple types of PHYs. To select eMMC 5.1 PHY, set:
+          marvell,xenon-phy-type = "emmc 5.1 phy" eMMC 5.1 PHY is the default
+          choice if this property is not provided.  To select eMMC 5.0 PHY, set:
+          marvell,xenon-phy-type = "emmc 5.0 phy"
+
+          All those types of PHYs can support eMMC, SD and SDIO. Please note that
+          this property only presents the type of PHY.  It doesn't stand for the
+          entire SDHC type or property.  For example, "emmc 5.1 phy" doesn't mean
+          that this Xenon SDHC only supports eMMC 5.1.
+
+      marvell,xenon-phy-znr:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 0
+        maximum: 0x1f
+        default: 0xf
+        description: |
+          Set PHY ZNR value.
+          Only available for eMMC PHY.
+          Valid range = [0:0x1F].
+          ZNR is set as 0xF by default if this property is not provided.
+
+      marvell,xenon-phy-zpr:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 0
+        maximum: 0x1f
+        default: 0xf
+        description: |
+          Set PHY ZPR value.
+          Only available for eMMC PHY.
+          Valid range = [0:0x1F].
+          ZPR is set as 0xF by default if this property is not provided.
+
+      marvell,xenon-phy-nr-success-tun:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 1
+        maximum: 7
+        default: 0x4
+        description: |
+          Set the number of required consecutive successful sampling points
+          used to identify a valid sampling window, in tuning process.
+          Valid range = [1:7].
+          Set as 0x4 by default if this property is not provided.
+
+      marvell,xenon-phy-tun-step-divider:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description: |
+          Set the divider for calculating TUN_STEP.
+          Set as 64 by default if this property is not provided.
+
+      marvell,xenon-phy-slow-mode:
+        type: boolean
+        description: |
+          If this property is selected, transfers will bypass PHY.
+          Only available when bus frequency lower than 55MHz in SDR mode.
+          Disabled by default. Please only try this property if timing issues
+          always occur with PHY enabled in eMMC HS SDR, SD SDR12, SD SDR25,
+          SD Default Speed and HS mode and eMMC legacy speed mode.
+
+      marvell,xenon-tun-count:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description: |
+          Xenon SDHC SoC usually doesn't provide re-tuning counter in
+          Capabilities Register 3 Bit[11:8].
+          This property provides the re-tuning counter.
+          If this property is not set, default re-tuning counter will
+          be set as 0x9 in driver.
+
+      marvell,pad-type:
+        enum:
+          - sd
+          - fixed-1-8v
+        description: |
+          Type of Armada 3700 SoC PHY PAD Voltage Controller register.
+          Only valid when "marvell,armada-3700-sdhci" is selected.
+          Two types: "sd" and "fixed-1-8v".
+          If "sd" is selected, SoC PHY PAD is set as 3.3V at the beginning and is
+          switched to 1.8V when later in higher speed mode.
+          If "fixed-1-8v" is selected, SoC PHY PAD is fixed 1.8V, such as for eMMC.
+          Please follow the examples with compatible "marvell,armada-3700-sdhci"
+          in below.
+
+    required:
+      - compatible
+      - reg
+      - clocks
+      - clock-names
+
+    unevaluatedProperties: false
+
+additionalProperties: false
+
+examples:
+  - |
+    // For eMMC
+
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    sdhci@aa0000 {
+      compatible = "marvell,armada-ap807-sdhci", "marvell,armada-ap806-sdhci";
+      reg = <0xaa0000 0x1000>;
+      interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>;
+      clocks = <&emmc_clk 0>, <&axi_clk 0>;
+      clock-names = "core", "axi";
+      bus-width = <4>;
+      marvell,xenon-phy-slow-mode;
+      marvell,xenon-tun-count = <11>;
+      non-removable;
+      no-sd;
+      no-sdio;
+
+      /* Vmmc and Vqmmc are both fixed */
+    };
+
+  - |
+    // For SD/SDIO
+
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    sdhci@ab0000 {
+      compatible = "marvell,armada-cp110-sdhci";
+      reg = <0xab0000 0x1000>;
+      interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>;
+      vqmmc-supply = <&sd_vqmmc_regulator>;
+      vmmc-supply = <&sd_vmmc_regulator>;
+      clocks = <&sdclk 0>, <&axi_clk 0>;
+      clock-names = "core", "axi";
+      bus-width = <4>;
+      marvell,xenon-tun-count = <9>;
+    };
+
+  - |
+    // For eMMC with compatible "marvell,armada-3700-sdhci":
+
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    sdhci@aa0000 {
+      compatible = "marvell,armada-3700-sdhci";
+      reg = <0xaa0000 0x1000>,
+            <0x17808 0x4>;
+      interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>;
+      clocks = <&emmcclk 0>;
+      clock-names = "core";
+      bus-width = <8>;
+      mmc-ddr-1_8v;
+      mmc-hs400-1_8v;
+      non-removable;
+      no-sd;
+      no-sdio;
+
+      /* Vmmc and Vqmmc are both fixed */
+
+      marvell,pad-type = "fixed-1-8v";
+    };
+
+  - |
+    // For SD/SDIO with compatible "marvell,armada-3700-sdhci":
+
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    sdhci@ab0000 {
+      compatible = "marvell,armada-3700-sdhci";
+      reg = <0xab0000 0x1000>,
+            <0x17808 0x4>;
+      interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>;
+      vqmmc-supply = <&sd_regulator>;
+      /* Vmmc is fixed */
+      clocks = <&sdclk 0>;
+      clock-names = "core";
+      bus-width = <4>;
+
+      marvell,pad-type = "sd";
+    };
-- 
2.35.1

Re: [PATCH] dt-bindings: mmc: xenon: Convert to JSON schema

[Krzysztof Kozlowski]

On 18/03/2022 04:35, Chris Packham wrote:
> Convert the marvell,xenon-sdhci binding to JSON schema. This is a fairly
> direct conversion so there are some requirements that are documented in
> prose but not currently enforced.
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>

Thanks for the change. Several comments below.

> ---
>  .../bindings/mmc/marvell,xenon-sdhci.txt      | 173 ------------
>  .../bindings/mmc/marvell,xenon-sdhci.yaml     | 252 ++++++++++++++++++

Invalid path in maintainers, please update the maintainers file.

>  2 files changed, 252 insertions(+), 173 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
>  create mode 100644 Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
> deleted file mode 100644
> index c51a62d751dc..000000000000
> --- a/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
> +++ /dev/null
> @@ -1,173 +0,0 @@
> -Marvell Xenon SDHCI Controller device tree bindings
> -This file documents differences between the core mmc properties
> -described by mmc.txt and the properties used by the Xenon implementation.
> -
> -Multiple SDHCs might be put into a single Xenon IP, to save size and cost.
> -Each SDHC is independent and owns independent resources, such as register sets,
> -clock and PHY.
> -Each SDHC should have an independent device tree node.
> -
> -Required Properties:
> -- compatible: should be one of the following
> -  - "marvell,armada-3700-sdhci": For controllers on Armada-3700 SoC.
> -  Must provide a second register area and marvell,pad-type.
> -  - "marvell,armada-ap806-sdhci": For controllers on Armada AP806.
> -  - "marvell,armada-ap807-sdhci": For controllers on Armada AP807.
> -  - "marvell,armada-cp110-sdhci": For controllers on Armada CP110.
> -
> -- clocks:
> -  Array of clocks required for SDHC.
> -  Require at least input clock for Xenon IP core. For Armada AP806 and
> -  CP110, the AXI clock is also mandatory.
> -
> -- clock-names:
> -  Array of names corresponding to clocks property.
> -  The input clock for Xenon IP core should be named as "core".
> -  The input clock for the AXI bus must be named as "axi".
> -
> -- reg:
> -  * For "marvell,armada-3700-sdhci", two register areas.
> -    The first one for Xenon IP register. The second one for the Armada 3700 SoC
> -    PHY PAD Voltage Control register.
> -    Please follow the examples with compatible "marvell,armada-3700-sdhci"
> -    in below.
> -    Please also check property marvell,pad-type in below.
> -
> -  * For other compatible strings, one register area for Xenon IP.
> -
> -Optional Properties:
> -- marvell,xenon-sdhc-id:
> -  Indicate the corresponding bit index of current SDHC in
> -  SDHC System Operation Control Register Bit[7:0].
> -  Set/clear the corresponding bit to enable/disable current SDHC.
> -  If Xenon IP contains only one SDHC, this property is optional.
> -
> -- marvell,xenon-phy-type:
> -  Xenon support multiple types of PHYs.
> -  To select eMMC 5.1 PHY, set:
> -  marvell,xenon-phy-type = "emmc 5.1 phy"
> -  eMMC 5.1 PHY is the default choice if this property is not provided.
> -  To select eMMC 5.0 PHY, set:
> -  marvell,xenon-phy-type = "emmc 5.0 phy"
> -
> -  All those types of PHYs can support eMMC, SD and SDIO.
> -  Please note that this property only presents the type of PHY.
> -  It doesn't stand for the entire SDHC type or property.
> -  For example, "emmc 5.1 phy" doesn't mean that this Xenon SDHC only
> -  supports eMMC 5.1.
> -
> -- marvell,xenon-phy-znr:
> -  Set PHY ZNR value.
> -  Only available for eMMC PHY.
> -  Valid range = [0:0x1F].
> -  ZNR is set as 0xF by default if this property is not provided.
> -
> -- marvell,xenon-phy-zpr:
> -  Set PHY ZPR value.
> -  Only available for eMMC PHY.
> -  Valid range = [0:0x1F].
> -  ZPR is set as 0xF by default if this property is not provided.
> -
> -- marvell,xenon-phy-nr-success-tun:
> -  Set the number of required consecutive successful sampling points
> -  used to identify a valid sampling window, in tuning process.
> -  Valid range = [1:7].
> -  Set as 0x4 by default if this property is not provided.
> -
> -- marvell,xenon-phy-tun-step-divider:
> -  Set the divider for calculating TUN_STEP.
> -  Set as 64 by default if this property is not provided.
> -
> -- marvell,xenon-phy-slow-mode:
> -  If this property is selected, transfers will bypass PHY.
> -  Only available when bus frequency lower than 55MHz in SDR mode.
> -  Disabled by default. Please only try this property if timing issues
> -  always occur with PHY enabled in eMMC HS SDR, SD SDR12, SD SDR25,
> -  SD Default Speed and HS mode and eMMC legacy speed mode.
> -
> -- marvell,xenon-tun-count:
> -  Xenon SDHC SoC usually doesn't provide re-tuning counter in
> -  Capabilities Register 3 Bit[11:8].
> -  This property provides the re-tuning counter.
> -  If this property is not set, default re-tuning counter will
> -  be set as 0x9 in driver.
> -
> -- marvell,pad-type:
> -  Type of Armada 3700 SoC PHY PAD Voltage Controller register.
> -  Only valid when "marvell,armada-3700-sdhci" is selected.
> -  Two types: "sd" and "fixed-1-8v".
> -  If "sd" is selected, SoC PHY PAD is set as 3.3V at the beginning and is
> -  switched to 1.8V when later in higher speed mode.
> -  If "fixed-1-8v" is selected, SoC PHY PAD is fixed 1.8V, such as for eMMC.
> -  Please follow the examples with compatible "marvell,armada-3700-sdhci"
> -  in below.
> -
> -Example:
> -- For eMMC:
> -
> -	sdhci@aa0000 {
> -		compatible = "marvell,armada-ap806-sdhci";
> -		reg = <0xaa0000 0x1000>;
> -		interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>
> -		clocks = <&emmc_clk>,<&axi_clk>;
> -		clock-names = "core", "axi";
> -		bus-width = <4>;
> -		marvell,xenon-phy-slow-mode;
> -		marvell,xenon-tun-count = <11>;
> -		non-removable;
> -		no-sd;
> -		no-sdio;
> -
> -		/* Vmmc and Vqmmc are both fixed */
> -	};
> -
> -- For SD/SDIO:
> -
> -	sdhci@ab0000 {
> -		compatible = "marvell,armada-cp110-sdhci";
> -		reg = <0xab0000 0x1000>;
> -		interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>
> -		vqmmc-supply = <&sd_vqmmc_regulator>;
> -		vmmc-supply = <&sd_vmmc_regulator>;
> -		clocks = <&sdclk>, <&axi_clk>;
> -		clock-names = "core", "axi";
> -		bus-width = <4>;
> -		marvell,xenon-tun-count = <9>;
> -	};
> -
> -- For eMMC with compatible "marvell,armada-3700-sdhci":
> -
> -	sdhci@aa0000 {
> -		compatible = "marvell,armada-3700-sdhci";
> -		reg = <0xaa0000 0x1000>,
> -		      <phy_addr 0x4>;
> -		interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>
> -		clocks = <&emmcclk>;
> -		clock-names = "core";
> -		bus-width = <8>;
> -		mmc-ddr-1_8v;
> -		mmc-hs400-1_8v;
> -		non-removable;
> -		no-sd;
> -		no-sdio;
> -
> -		/* Vmmc and Vqmmc are both fixed */
> -
> -		marvell,pad-type = "fixed-1-8v";
> -	};
> -
> -- For SD/SDIO with compatible "marvell,armada-3700-sdhci":
> -
> -	sdhci@ab0000 {
> -		compatible = "marvell,armada-3700-sdhci";
> -		reg = <0xab0000 0x1000>,
> -		      <phy_addr 0x4>;
> -		interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>
> -		vqmmc-supply = <&sd_regulator>;
> -		/* Vmmc is fixed */
> -		clocks = <&sdclk>;
> -		clock-names = "core";
> -		bus-width = <4>;
> -
> -		marvell,pad-type = "sd";
> -	};
> diff --git a/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml
> new file mode 100644
> index 000000000000..22d5cbf28042
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml
> @@ -0,0 +1,252 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mmc/marvell,xenon-sdhci.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Marvell Xenon SDHCI Controller device tree bindings

Drop "device tree bindings". Title is about hardware.

> +
> +description: |
> +  This file documents differences between the core mmc properties described by

s/mmc/MMC/

> +  mmc-controller.yaml and the properties used by the Xenon implementation.
> +
> +  Multiple SDHCs might be put into a single Xenon IP, to save size and cost.
> +  Each SDHC is independent and owns independent resources, such as register
> +  sets, clock and PHY.
> +
> +  Each SDHC should have an independent device tree node.
> +
> +maintainers:
> +  - Ulf Hansson <ulf.hansson@linaro.org>
> +
> +patternProperties:
> +  "^sdhci@[0-9a-f]+$":
> +    type: object
> +    $ref: mmc-controller.yaml

This is unusual schema... What are you matching here? Are these children
of this device?

Looks like you wanted allOf. See some existing examples, like:
Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml

> +
> +    properties:
> +      compatible:
> +        oneOf:
> +          - const: marvell,armada-3700-sdhci
> +            description: |
> +              Must provide a second register area and marvell,pad-type
> +          - const: marvell,armada-ap806-sdhci
> +          - const: marvell,armada-ap807-sdhci

This looks wrong. Either these can be standalone properties or in a list
like in your last items below.

> +          - const: marvell,armada-cp110-sdhci
> +          - const: marvell,sdhci-xenon


This did not exist before. Separate patches please for additions (with
explanation why). Maybe some DTS lists this, but then it should be
individually judged whether the DTS is correct.

> +          - items:
> +            - const: marvell,armada-3700-sdhci
> +            - const: marvell,sdhci-xenon
> +          - items:
> +            - const: marvell,armada-ap807-sdhci
> +            - const: marvell,armada-ap806-sdhci

> +
> +      reg:
> +        minItems: 1
> +        maxItems: 2
> +        description: |
> +          For "marvell,armada-3700-sdhci", two register areas.  The first one
> +          for Xenon IP register. The second one for the Armada 3700 SoC PHY PAD
> +          Voltage Control register.  Please follow the examples with compatible
> +          "marvell,armada-3700-sdhci" in below.
> +          Please also check property marvell,pad-type in below.

For this condition and similar one in clocks/clock-names, you need
if:then:" inside allOf. See for example:
https://elixir.bootlin.com/linux/v5.17-rc8/source/Documentation/devicetree/bindings/clock/samsung,exynos850-clock.yaml#L56

> +
> +          For other compatible strings, one register area for Xenon IP.
> +
> +      clocks:
> +        minItems: 1
> +        maxItems: 2
> +
> +      clock-names:
> +        minItems: 1
> +        items:
> +          - const: core
> +          - const: axi
> +
> +      marvell,xenon-sdhc-id:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        minimum: 0
> +        maximum: 7
> +        description: |
> +          Indicate the corresponding bit index of current SDHC in SDHC System
> +          Operation Control Register Bit[7:0].  Set/clear the corresponding bit to
> +          enable/disable current SDHC.  If Xenon IP contains only one SDHC, this
> +          property is optional.

Skip all the "this property is optional" because it is obvious from
"required:" part.

> +
> +      marvell,xenon-phy-type:
> +        enum:
> +          - "emmc 5.1 phy"
> +          - "emmc 5.0 phy"

ref: string.

> +        description: |
> +          Xenon support multiple types of PHYs. To select eMMC 5.1 PHY, set:
> +          marvell,xenon-phy-type = "emmc 5.1 phy" eMMC 5.1 PHY is the default
> +          choice if this property is not provided.  To select eMMC 5.0 PHY, set:
> +          marvell,xenon-phy-type = "emmc 5.0 phy"
> +
> +          All those types of PHYs can support eMMC, SD and SDIO. Please note that
> +          this property only presents the type of PHY.  It doesn't stand for the
> +          entire SDHC type or property.  For example, "emmc 5.1 phy" doesn't mean
> +          that this Xenon SDHC only supports eMMC 5.1.
> +
> +      marvell,xenon-phy-znr:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        minimum: 0
> +        maximum: 0x1f
> +        default: 0xf
> +        description: |
> +          Set PHY ZNR value.
> +          Only available for eMMC PHY.
> +          Valid range = [0:0x1F].

Skip "valid range". It's obvious. Same in all other places. In general,
trim the description from any parts which are now defined in the
bindings. Previously (in TXT) this has to be mentioned in description,
but now we have better way - through DT schema.

> +          ZNR is set as 0xF by default if this property is not provided.
> +
> +      marvell,xenon-phy-zpr:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        minimum: 0
> +        maximum: 0x1f
> +        default: 0xf
> +        description: |
> +          Set PHY ZPR value.
> +          Only available for eMMC PHY.
> +          Valid range = [0:0x1F].
> +          ZPR is set as 0xF by default if this property is not provided.
> +
> +      marvell,xenon-phy-nr-success-tun:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        minimum: 1
> +        maximum: 7
> +        default: 0x4
> +        description: |
> +          Set the number of required consecutive successful sampling points
> +          used to identify a valid sampling window, in tuning process.
> +          Valid range = [1:7].
> +          Set as 0x4 by default if this property is not provided.
> +
> +      marvell,xenon-phy-tun-step-divider:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: |
> +          Set the divider for calculating TUN_STEP.
> +          Set as 64 by default if this property is not provided.

default: 64

> +
> +      marvell,xenon-phy-slow-mode:
> +        type: boolean
> +        description: |
> +          If this property is selected, transfers will bypass PHY.
> +          Only available when bus frequency lower than 55MHz in SDR mode.
> +          Disabled by default. Please only try this property if timing issues
> +          always occur with PHY enabled in eMMC HS SDR, SD SDR12, SD SDR25,
> +          SD Default Speed and HS mode and eMMC legacy speed mode.
> +
> +      marvell,xenon-tun-count:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: |
> +          Xenon SDHC SoC usually doesn't provide re-tuning counter in
> +          Capabilities Register 3 Bit[11:8].
> +          This property provides the re-tuning counter.
> +          If this property is not set, default re-tuning counter will
> +          be set as 0x9 in driver.> +
> +      marvell,pad-type:
> +        enum:
> +          - sd
> +          - fixed-1-8v
> +        description: |
> +          Type of Armada 3700 SoC PHY PAD Voltage Controller register.
> +          Only valid when "marvell,armada-3700-sdhci" is selected.
> +          Two types: "sd" and "fixed-1-8v".
> +          If "sd" is selected, SoC PHY PAD is set as 3.3V at the beginning and is
> +          switched to 1.8V when later in higher speed mode.
> +          If "fixed-1-8v" is selected, SoC PHY PAD is fixed 1.8V, such as for eMMC.
> +          Please follow the examples with compatible "marvell,armada-3700-sdhci"
> +          in below.
> +
> +    required:
> +      - compatible
> +      - reg
> +      - clocks
> +      - clock-names
> +
> +    unevaluatedProperties: false
> +
> +additionalProperties: false

This will be gone once you remove this incorrect patternProperties

> +
> +examples:
> +  - |
> +    // For eMMC
> +

Blank line rather after includes, not before.

> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    sdhci@aa0000 {
> +      compatible = "marvell,armada-ap807-sdhci", "marvell,armada-ap806-sdhci";
> +      reg = <0xaa0000 0x1000>;
> +      interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>;
> +      clocks = <&emmc_clk 0>, <&axi_clk 0>;
> +      clock-names = "core", "axi";
> +      bus-width = <4>;
> +      marvell,xenon-phy-slow-mode;
> +      marvell,xenon-tun-count = <11>;
> +      non-removable;
> +      no-sd;
> +      no-sdio;
> +
> +      /* Vmmc and Vqmmc are both fixed */
> +    };
> +
> +  - |
> +    // For SD/SDIO
> +
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    sdhci@ab0000 {
> +      compatible = "marvell,armada-cp110-sdhci";
> +      reg = <0xab0000 0x1000>;
> +      interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>;
> +      vqmmc-supply = <&sd_vqmmc_regulator>;
> +      vmmc-supply = <&sd_vmmc_regulator>;
> +      clocks = <&sdclk 0>, <&axi_clk 0>;
> +      clock-names = "core", "axi";
> +      bus-width = <4>;
> +      marvell,xenon-tun-count = <9>;
> +    };
> +
> +  - |
> +    // For eMMC with compatible "marvell,armada-3700-sdhci":
> +
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    sdhci@aa0000 {
> +      compatible = "marvell,armada-3700-sdhci";
> +      reg = <0xaa0000 0x1000>,
> +            <0x17808 0x4>;
> +      interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>;
> +      clocks = <&emmcclk 0>;
> +      clock-names = "core";
> +      bus-width = <8>;
> +      mmc-ddr-1_8v;
> +      mmc-hs400-1_8v;
> +      non-removable;
> +      no-sd;
> +      no-sdio;
> +
> +      /* Vmmc and Vqmmc are both fixed */
> +
> +      marvell,pad-type = "fixed-1-8v";
> +    };
> +
> +  - |
> +    // For SD/SDIO with compatible "marvell,armada-3700-sdhci":
> +
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    sdhci@ab0000 {
> +      compatible = "marvell,armada-3700-sdhci";
> +      reg = <0xab0000 0x1000>,
> +            <0x17808 0x4>;
> +      interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>;
> +      vqmmc-supply = <&sd_regulator>;
> +      /* Vmmc is fixed */
> +      clocks = <&sdclk 0>;
> +      clock-names = "core";
> +      bus-width = <4>;
> +
> +      marvell,pad-type = "sd";

It looks the same as previous example for SD. Maybe just remove it?

> +    };


Best regards,
Krzysztof

Re: [PATCH] dt-bindings: mmc: xenon: Convert to JSON schema

[Rob Herring]

On Fri, 18 Mar 2022 16:35:21 +1300, Chris Packham wrote:
> Convert the marvell,xenon-sdhci binding to JSON schema. This is a fairly
> direct conversion so there are some requirements that are documented in
> prose but not currently enforced.
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
>  .../bindings/mmc/marvell,xenon-sdhci.txt      | 173 ------------
>  .../bindings/mmc/marvell,xenon-sdhci.yaml     | 252 ++++++++++++++++++
>  2 files changed, 252 insertions(+), 173 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
>  create mode 100644 Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml:38:13: [warning] wrong indentation: expected 14 but found 12 (indentation)
./Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml:41:13: [warning] wrong indentation: expected 14 but found 12 (indentation)

dtschema/dtc warnings/errors:

doc reference errors (make refcheckdocs):
MAINTAINERS: Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt

See https://patchwork.ozlabs.org/patch/1606868

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [PATCH] dt-bindings: mmc: xenon: Convert to JSON schema

[Chris Packham]

On 19/03/22 03:20, Krzysztof Kozlowski wrote:
> On 18/03/2022 04:35, Chris Packham wrote:
>> Convert the marvell,xenon-sdhci binding to JSON schema. This is a fairly
>> direct conversion so there are some requirements that are documented in
>> prose but not currently enforced.
>>
>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> Thanks for the change. Several comments below.
>
>> ---
>>   .../bindings/mmc/marvell,xenon-sdhci.txt      | 173 ------------
>>   .../bindings/mmc/marvell,xenon-sdhci.yaml     | 252 ++++++++++++++++++
> Invalid path in maintainers, please update the maintainers file.
>
>>   2 files changed, 252 insertions(+), 173 deletions(-)
>>   delete mode 100644 Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
>>   create mode 100644 Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
>> deleted file mode 100644
>> index c51a62d751dc..000000000000
>> --- a/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
>> +++ /dev/null
>> @@ -1,173 +0,0 @@
>> -Marvell Xenon SDHCI Controller device tree bindings
>> -This file documents differences between the core mmc properties
>> -described by mmc.txt and the properties used by the Xenon implementation.
>> -
>> -Multiple SDHCs might be put into a single Xenon IP, to save size and cost.
>> -Each SDHC is independent and owns independent resources, such as register sets,
>> -clock and PHY.
>> -Each SDHC should have an independent device tree node.
>> -
>> -Required Properties:
>> -- compatible: should be one of the following
>> -  - "marvell,armada-3700-sdhci": For controllers on Armada-3700 SoC.
>> -  Must provide a second register area and marvell,pad-type.
>> -  - "marvell,armada-ap806-sdhci": For controllers on Armada AP806.
>> -  - "marvell,armada-ap807-sdhci": For controllers on Armada AP807.
>> -  - "marvell,armada-cp110-sdhci": For controllers on Armada CP110.
>> -
>> -- clocks:
>> -  Array of clocks required for SDHC.
>> -  Require at least input clock for Xenon IP core. For Armada AP806 and
>> -  CP110, the AXI clock is also mandatory.
>> -
>> -- clock-names:
>> -  Array of names corresponding to clocks property.
>> -  The input clock for Xenon IP core should be named as "core".
>> -  The input clock for the AXI bus must be named as "axi".
>> -
>> -- reg:
>> -  * For "marvell,armada-3700-sdhci", two register areas.
>> -    The first one for Xenon IP register. The second one for the Armada 3700 SoC
>> -    PHY PAD Voltage Control register.
>> -    Please follow the examples with compatible "marvell,armada-3700-sdhci"
>> -    in below.
>> -    Please also check property marvell,pad-type in below.
>> -
>> -  * For other compatible strings, one register area for Xenon IP.
>> -
>> -Optional Properties:
>> -- marvell,xenon-sdhc-id:
>> -  Indicate the corresponding bit index of current SDHC in
>> -  SDHC System Operation Control Register Bit[7:0].
>> -  Set/clear the corresponding bit to enable/disable current SDHC.
>> -  If Xenon IP contains only one SDHC, this property is optional.
>> -
>> -- marvell,xenon-phy-type:
>> -  Xenon support multiple types of PHYs.
>> -  To select eMMC 5.1 PHY, set:
>> -  marvell,xenon-phy-type = "emmc 5.1 phy"
>> -  eMMC 5.1 PHY is the default choice if this property is not provided.
>> -  To select eMMC 5.0 PHY, set:
>> -  marvell,xenon-phy-type = "emmc 5.0 phy"
>> -
>> -  All those types of PHYs can support eMMC, SD and SDIO.
>> -  Please note that this property only presents the type of PHY.
>> -  It doesn't stand for the entire SDHC type or property.
>> -  For example, "emmc 5.1 phy" doesn't mean that this Xenon SDHC only
>> -  supports eMMC 5.1.
>> -
>> -- marvell,xenon-phy-znr:
>> -  Set PHY ZNR value.
>> -  Only available for eMMC PHY.
>> -  Valid range = [0:0x1F].
>> -  ZNR is set as 0xF by default if this property is not provided.
>> -
>> -- marvell,xenon-phy-zpr:
>> -  Set PHY ZPR value.
>> -  Only available for eMMC PHY.
>> -  Valid range = [0:0x1F].
>> -  ZPR is set as 0xF by default if this property is not provided.
>> -
>> -- marvell,xenon-phy-nr-success-tun:
>> -  Set the number of required consecutive successful sampling points
>> -  used to identify a valid sampling window, in tuning process.
>> -  Valid range = [1:7].
>> -  Set as 0x4 by default if this property is not provided.
>> -
>> -- marvell,xenon-phy-tun-step-divider:
>> -  Set the divider for calculating TUN_STEP.
>> -  Set as 64 by default if this property is not provided.
>> -
>> -- marvell,xenon-phy-slow-mode:
>> -  If this property is selected, transfers will bypass PHY.
>> -  Only available when bus frequency lower than 55MHz in SDR mode.
>> -  Disabled by default. Please only try this property if timing issues
>> -  always occur with PHY enabled in eMMC HS SDR, SD SDR12, SD SDR25,
>> -  SD Default Speed and HS mode and eMMC legacy speed mode.
>> -
>> -- marvell,xenon-tun-count:
>> -  Xenon SDHC SoC usually doesn't provide re-tuning counter in
>> -  Capabilities Register 3 Bit[11:8].
>> -  This property provides the re-tuning counter.
>> -  If this property is not set, default re-tuning counter will
>> -  be set as 0x9 in driver.
>> -
>> -- marvell,pad-type:
>> -  Type of Armada 3700 SoC PHY PAD Voltage Controller register.
>> -  Only valid when "marvell,armada-3700-sdhci" is selected.
>> -  Two types: "sd" and "fixed-1-8v".
>> -  If "sd" is selected, SoC PHY PAD is set as 3.3V at the beginning and is
>> -  switched to 1.8V when later in higher speed mode.
>> -  If "fixed-1-8v" is selected, SoC PHY PAD is fixed 1.8V, such as for eMMC.
>> -  Please follow the examples with compatible "marvell,armada-3700-sdhci"
>> -  in below.
>> -
>> -Example:
>> -- For eMMC:
>> -
>> -	sdhci@aa0000 {
>> -		compatible = "marvell,armada-ap806-sdhci";
>> -		reg = <0xaa0000 0x1000>;
>> -		interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>
>> -		clocks = <&emmc_clk>,<&axi_clk>;
>> -		clock-names = "core", "axi";
>> -		bus-width = <4>;
>> -		marvell,xenon-phy-slow-mode;
>> -		marvell,xenon-tun-count = <11>;
>> -		non-removable;
>> -		no-sd;
>> -		no-sdio;
>> -
>> -		/* Vmmc and Vqmmc are both fixed */
>> -	};
>> -
>> -- For SD/SDIO:
>> -
>> -	sdhci@ab0000 {
>> -		compatible = "marvell,armada-cp110-sdhci";
>> -		reg = <0xab0000 0x1000>;
>> -		interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>
>> -		vqmmc-supply = <&sd_vqmmc_regulator>;
>> -		vmmc-supply = <&sd_vmmc_regulator>;
>> -		clocks = <&sdclk>, <&axi_clk>;
>> -		clock-names = "core", "axi";
>> -		bus-width = <4>;
>> -		marvell,xenon-tun-count = <9>;
>> -	};
>> -
>> -- For eMMC with compatible "marvell,armada-3700-sdhci":
>> -
>> -	sdhci@aa0000 {
>> -		compatible = "marvell,armada-3700-sdhci";
>> -		reg = <0xaa0000 0x1000>,
>> -		      <phy_addr 0x4>;
>> -		interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>
>> -		clocks = <&emmcclk>;
>> -		clock-names = "core";
>> -		bus-width = <8>;
>> -		mmc-ddr-1_8v;
>> -		mmc-hs400-1_8v;
>> -		non-removable;
>> -		no-sd;
>> -		no-sdio;
>> -
>> -		/* Vmmc and Vqmmc are both fixed */
>> -
>> -		marvell,pad-type = "fixed-1-8v";
>> -	};
>> -
>> -- For SD/SDIO with compatible "marvell,armada-3700-sdhci":
>> -
>> -	sdhci@ab0000 {
>> -		compatible = "marvell,armada-3700-sdhci";
>> -		reg = <0xab0000 0x1000>,
>> -		      <phy_addr 0x4>;
>> -		interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>
>> -		vqmmc-supply = <&sd_regulator>;
>> -		/* Vmmc is fixed */
>> -		clocks = <&sdclk>;
>> -		clock-names = "core";
>> -		bus-width = <4>;
>> -
>> -		marvell,pad-type = "sd";
>> -	};
>> diff --git a/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml
>> new file mode 100644
>> index 000000000000..22d5cbf28042
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml
>> @@ -0,0 +1,252 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://scanmail.trustwave.com/?c=20988&d=qZW04jRMZgk_UFKhuPblJizHEw95We1imujRV7EwpA&u=http%3a%2f%2fdevicetree%2eorg%2fschemas%2fmmc%2fmarvell%2cxenon-sdhci%2eyaml%23
>> +$schema: http://scanmail.trustwave.com/?c=20988&d=qZW04jRMZgk_UFKhuPblJizHEw95We1imriGDeBi9w&u=http%3a%2f%2fdevicetree%2eorg%2fmeta-schemas%2fcore%2eyaml%23
>> +
>> +title: Marvell Xenon SDHCI Controller device tree bindings
> Drop "device tree bindings". Title is about hardware.
>
>> +
>> +description: |
>> +  This file documents differences between the core mmc properties described by
> s/mmc/MMC/
>
>> +  mmc-controller.yaml and the properties used by the Xenon implementation.
>> +
>> +  Multiple SDHCs might be put into a single Xenon IP, to save size and cost.
>> +  Each SDHC is independent and owns independent resources, such as register
>> +  sets, clock and PHY.
>> +
>> +  Each SDHC should have an independent device tree node.
>> +
>> +maintainers:
>> +  - Ulf Hansson <ulf.hansson@linaro.org>
>> +
>> +patternProperties:
>> +  "^sdhci@[0-9a-f]+$":
>> +    type: object
>> +    $ref: mmc-controller.yaml
> This is unusual schema... What are you matching here? Are these children
> of this device?
I was going for compatibility with existing uses. The 
mmc-controller.yaml schema expects these nodes to be mmc@... . But all 
of the existing usages of these bindings use sdhci@... as the primary 
node. I could make my example use mmc@ to squash the warning but I was 
hoping to be able to do something that didn't make the existing usages 
invalid.
> Looks like you wanted allOf. See some existing examples, like:
> Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
>
>> +
>> +    properties:
>> +      compatible:
>> +        oneOf:
>> +          - const: marvell,armada-3700-sdhci
>> +            description: |
>> +              Must provide a second register area and marvell,pad-type
>> +          - const: marvell,armada-ap806-sdhci
>> +          - const: marvell,armada-ap807-sdhci
> This looks wrong. Either these can be standalone properties or in a list
> like in your last items below.
I was trying to allow 'compatible = "marvell,armada-ap806-sdhci";' or 
'compatible = "marvell,armada-ap807-sdhci", "marvell,armada-ap806-sdhci";'

>
>> +          - const: marvell,armada-cp110-sdhci
>> +          - const: marvell,sdhci-xenon
>
> This did not exist before. Separate patches please for additions (with
> explanation why). Maybe some DTS lists this, but then it should be
> individually judged whether the DTS is correct.
Ah OK. I added it because it was in some DTSes but sure I can add it as 
a follow up.
>
>> +          - items:
>> +            - const: marvell,armada-3700-sdhci
>> +            - const: marvell,sdhci-xenon
>> +          - items:
>> +            - const: marvell,armada-ap807-sdhci
>> +            - const: marvell,armada-ap806-sdhci
>> +
>> +      reg:
>> +        minItems: 1
>> +        maxItems: 2
>> +        description: |
>> +          For "marvell,armada-3700-sdhci", two register areas.  The first one
>> +          for Xenon IP register. The second one for the Armada 3700 SoC PHY PAD
>> +          Voltage Control register.  Please follow the examples with compatible
>> +          "marvell,armada-3700-sdhci" in below.
>> +          Please also check property marvell,pad-type in below.
> For this condition and similar one in clocks/clock-names, you need
> if:then:" inside allOf. See for example:
> https://scanmail.trustwave.com/?c=20988&d=qZW04jRMZgk_UFKhuPblJizHEw95We1imreDVuBupQ&u=https%3a%2f%2felixir%2ebootlin%2ecom%2flinux%2fv5%2e17-rc8%2fsource%2fDocumentation%2fdevicetree%2fbindings%2fclock%2fsamsung%2cexynos850-clock%2eyaml%23L56
I'll give it a try. I did wonder if this was something best left as a 
follow up after the initial conversion.
>
>> +
>> +          For other compatible strings, one register area for Xenon IP.
>> +
>> +      clocks:
>> +        minItems: 1
>> +        maxItems: 2
>> +
>> +      clock-names:
>> +        minItems: 1
>> +        items:
>> +          - const: core
>> +          - const: axi
>> +
>> +      marvell,xenon-sdhc-id:
>> +        $ref: /schemas/types.yaml#/definitions/uint32
>> +        minimum: 0
>> +        maximum: 7
>> +        description: |
>> +          Indicate the corresponding bit index of current SDHC in SDHC System
>> +          Operation Control Register Bit[7:0].  Set/clear the corresponding bit to
>> +          enable/disable current SDHC.  If Xenon IP contains only one SDHC, this
>> +          property is optional.
> Skip all the "this property is optional" because it is obvious from
> "required:" part.
>
>> +
>> +      marvell,xenon-phy-type:
>> +        enum:
>> +          - "emmc 5.1 phy"
>> +          - "emmc 5.0 phy"
> ref: string.
>
>> +        description: |
>> +          Xenon support multiple types of PHYs. To select eMMC 5.1 PHY, set:
>> +          marvell,xenon-phy-type = "emmc 5.1 phy" eMMC 5.1 PHY is the default
>> +          choice if this property is not provided.  To select eMMC 5.0 PHY, set:
>> +          marvell,xenon-phy-type = "emmc 5.0 phy"
>> +
>> +          All those types of PHYs can support eMMC, SD and SDIO. Please note that
>> +          this property only presents the type of PHY.  It doesn't stand for the
>> +          entire SDHC type or property.  For example, "emmc 5.1 phy" doesn't mean
>> +          that this Xenon SDHC only supports eMMC 5.1.
>> +
>> +      marvell,xenon-phy-znr:
>> +        $ref: /schemas/types.yaml#/definitions/uint32
>> +        minimum: 0
>> +        maximum: 0x1f
>> +        default: 0xf
>> +        description: |
>> +          Set PHY ZNR value.
>> +          Only available for eMMC PHY.
>> +          Valid range = [0:0x1F].
> Skip "valid range". It's obvious. Same in all other places. In general,
> trim the description from any parts which are now defined in the
> bindings. Previously (in TXT) this has to be mentioned in description,
> but now we have better way - through DT schema.
>
>> +          ZNR is set as 0xF by default if this property is not provided.
>> +
>> +      marvell,xenon-phy-zpr:
>> +        $ref: /schemas/types.yaml#/definitions/uint32
>> +        minimum: 0
>> +        maximum: 0x1f
>> +        default: 0xf
>> +        description: |
>> +          Set PHY ZPR value.
>> +          Only available for eMMC PHY.
>> +          Valid range = [0:0x1F].
>> +          ZPR is set as 0xF by default if this property is not provided.
>> +
>> +      marvell,xenon-phy-nr-success-tun:
>> +        $ref: /schemas/types.yaml#/definitions/uint32
>> +        minimum: 1
>> +        maximum: 7
>> +        default: 0x4
>> +        description: |
>> +          Set the number of required consecutive successful sampling points
>> +          used to identify a valid sampling window, in tuning process.
>> +          Valid range = [1:7].
>> +          Set as 0x4 by default if this property is not provided.
>> +
>> +      marvell,xenon-phy-tun-step-divider:
>> +        $ref: /schemas/types.yaml#/definitions/uint32
>> +        description: |
>> +          Set the divider for calculating TUN_STEP.
>> +          Set as 64 by default if this property is not provided.
> default: 64
>
>> +
>> +      marvell,xenon-phy-slow-mode:
>> +        type: boolean
>> +        description: |
>> +          If this property is selected, transfers will bypass PHY.
>> +          Only available when bus frequency lower than 55MHz in SDR mode.
>> +          Disabled by default. Please only try this property if timing issues
>> +          always occur with PHY enabled in eMMC HS SDR, SD SDR12, SD SDR25,
>> +          SD Default Speed and HS mode and eMMC legacy speed mode.
>> +
>> +      marvell,xenon-tun-count:
>> +        $ref: /schemas/types.yaml#/definitions/uint32
>> +        description: |
>> +          Xenon SDHC SoC usually doesn't provide re-tuning counter in
>> +          Capabilities Register 3 Bit[11:8].
>> +          This property provides the re-tuning counter.
>> +          If this property is not set, default re-tuning counter will
>> +          be set as 0x9 in driver.> +
>> +      marvell,pad-type:
>> +        enum:
>> +          - sd
>> +          - fixed-1-8v
>> +        description: |
>> +          Type of Armada 3700 SoC PHY PAD Voltage Controller register.
>> +          Only valid when "marvell,armada-3700-sdhci" is selected.
>> +          Two types: "sd" and "fixed-1-8v".
>> +          If "sd" is selected, SoC PHY PAD is set as 3.3V at the beginning and is
>> +          switched to 1.8V when later in higher speed mode.
>> +          If "fixed-1-8v" is selected, SoC PHY PAD is fixed 1.8V, such as for eMMC.
>> +          Please follow the examples with compatible "marvell,armada-3700-sdhci"
>> +          in below.
>> +
>> +    required:
>> +      - compatible
>> +      - reg
>> +      - clocks
>> +      - clock-names
>> +
>> +    unevaluatedProperties: false
>> +
>> +additionalProperties: false
> This will be gone once you remove this incorrect patternProperties
>
>> +
>> +examples:
>> +  - |
>> +    // For eMMC
>> +
> Blank line rather after includes, not before.
>
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +    #include <dt-bindings/interrupt-controller/irq.h>
>> +    sdhci@aa0000 {
>> +      compatible = "marvell,armada-ap807-sdhci", "marvell,armada-ap806-sdhci";
>> +      reg = <0xaa0000 0x1000>;
>> +      interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>;
>> +      clocks = <&emmc_clk 0>, <&axi_clk 0>;
>> +      clock-names = "core", "axi";
>> +      bus-width = <4>;
>> +      marvell,xenon-phy-slow-mode;
>> +      marvell,xenon-tun-count = <11>;
>> +      non-removable;
>> +      no-sd;
>> +      no-sdio;
>> +
>> +      /* Vmmc and Vqmmc are both fixed */
>> +    };
>> +
>> +  - |
>> +    // For SD/SDIO
>> +
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +    #include <dt-bindings/interrupt-controller/irq.h>
>> +    sdhci@ab0000 {
>> +      compatible = "marvell,armada-cp110-sdhci";
>> +      reg = <0xab0000 0x1000>;
>> +      interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>;
>> +      vqmmc-supply = <&sd_vqmmc_regulator>;
>> +      vmmc-supply = <&sd_vmmc_regulator>;
>> +      clocks = <&sdclk 0>, <&axi_clk 0>;
>> +      clock-names = "core", "axi";
>> +      bus-width = <4>;
>> +      marvell,xenon-tun-count = <9>;
>> +    };
>> +
>> +  - |
>> +    // For eMMC with compatible "marvell,armada-3700-sdhci":
>> +
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +    #include <dt-bindings/interrupt-controller/irq.h>
>> +    sdhci@aa0000 {
>> +      compatible = "marvell,armada-3700-sdhci";
>> +      reg = <0xaa0000 0x1000>,
>> +            <0x17808 0x4>;
>> +      interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>;
>> +      clocks = <&emmcclk 0>;
>> +      clock-names = "core";
>> +      bus-width = <8>;
>> +      mmc-ddr-1_8v;
>> +      mmc-hs400-1_8v;
>> +      non-removable;
>> +      no-sd;
>> +      no-sdio;
>> +
>> +      /* Vmmc and Vqmmc are both fixed */
>> +
>> +      marvell,pad-type = "fixed-1-8v";
>> +    };
>> +
>> +  - |
>> +    // For SD/SDIO with compatible "marvell,armada-3700-sdhci":
>> +
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +    #include <dt-bindings/interrupt-controller/irq.h>
>> +    sdhci@ab0000 {
>> +      compatible = "marvell,armada-3700-sdhci";
>> +      reg = <0xab0000 0x1000>,
>> +            <0x17808 0x4>;
>> +      interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>;
>> +      vqmmc-supply = <&sd_regulator>;
>> +      /* Vmmc is fixed */
>> +      clocks = <&sdclk 0>;
>> +      clock-names = "core";
>> +      bus-width = <4>;
>> +
>> +      marvell,pad-type = "sd";
> It looks the same as previous example for SD. Maybe just remove it?
>
>> +    };
>
> Best regards,
> Krzysztof

Re: [PATCH] dt-bindings: mmc: xenon: Convert to JSON schema

[Chris Packham]

(adding some ARM people, resending hopefully without the html)

On 19/03/22 03:20, Krzysztof Kozlowski wrote:
>> +          - const: marvell,sdhci-xenon
> This did not exist before. Separate patches please for additions (with
> explanation why). Maybe some DTS lists this, but then it should be
> individually judged whether the DTS is correct.
>
On this specifically. I was all ready to add an additional patch to 
document this but then I noticed nothing actually uses the 
"marvell,sdhci-xenon" compatible and it appears nothing ever did. I then 
figured I'd delete the unused compatible string from armada-37xx.dtsi 
but then I remembered that sometimes we add compatible strings to have 
them "just in case" we need them for some SoC specific workaround.

So there's a few things I can do

0. Nothing (easy) although the binding I just submitted will complain 
about the unexpected value
1. Document "marvell,sdhci-xenon" as a valid compatible
1.a. Add "marvell,sdhci-xenon" to the sdhci-xenon.c driver
2. Remove "marvell,sdhci-xenon" from armada-37xx.dtsi

Re: [PATCH] dt-bindings: mmc: xenon: Convert to JSON schema

[Chris Packham]

(adding some ARM people, resending hopefully without the html)

On 19/03/22 03:20, Krzysztof Kozlowski wrote:
>> +          - const: marvell,sdhci-xenon
> This did not exist before. Separate patches please for additions (with
> explanation why). Maybe some DTS lists this, but then it should be
> individually judged whether the DTS is correct.
>
On this specifically. I was all ready to add an additional patch to 
document this but then I noticed nothing actually uses the 
"marvell,sdhci-xenon" compatible and it appears nothing ever did. I then 
figured I'd delete the unused compatible string from armada-37xx.dtsi 
but then I remembered that sometimes we add compatible strings to have 
them "just in case" we need them for some SoC specific workaround.

So there's a few things I can do

0. Nothing (easy) although the binding I just submitted will complain 
about the unexpected value
1. Document "marvell,sdhci-xenon" as a valid compatible
1.a. Add "marvell,sdhci-xenon" to the sdhci-xenon.c driver
2. Remove "marvell,sdhci-xenon" from armada-37xx.dtsi
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] dt-bindings: mmc: xenon: Convert to JSON schema

[Krzysztof Kozlowski]

On 20/03/2022 20:51, Chris Packham wrote:
> 

(...)

>>> +
>>> +patternProperties:
>>> +  "^sdhci@[0-9a-f]+$":
>>> +    type: object
>>> +    $ref: mmc-controller.yaml
>> This is unusual schema... What are you matching here? Are these children
>> of this device?
> I was going for compatibility with existing uses. The 
> mmc-controller.yaml schema expects these nodes to be mmc@... . But all 
> of the existing usages of these bindings use sdhci@... as the primary 
> node. I could make my example use mmc@ to squash the warning but I was 
> hoping to be able to do something that didn't make the existing usages 
> invalid.

Please do not create inconsistent bindings because some DTS are
inconsistent. Change the DTS and align them with generic MMC schema.
Node name should not be considered an ABI, so it can be changed in DTS.
Some systems unfortunately break (usually Android and Chrome like to
encode node names), so then it would have to be individually discussed.

>> Looks like you wanted allOf. See some existing examples, like:
>> Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
>>
>>> +
>>> +    properties:
>>> +      compatible:
>>> +        oneOf:
>>> +          - const: marvell,armada-3700-sdhci
>>> +            description: |
>>> +              Must provide a second register area and marvell,pad-type
>>> +          - const: marvell,armada-ap806-sdhci
>>> +          - const: marvell,armada-ap807-sdhci
>> This looks wrong. Either these can be standalone properties or in a list
>> like in your last items below.
> I was trying to allow 'compatible = "marvell,armada-ap806-sdhci";' or 
> 'compatible = "marvell,armada-ap807-sdhci", "marvell,armada-ap806-sdhci";'

But you have here 807! Both 806 and 807.  So is 807 compatible with 806
or not?

Best regards,
Krzysztof

Re: [PATCH] dt-bindings: mmc: xenon: Convert to JSON schema

[Krzysztof Kozlowski]

On 21/03/2022 01:12, Chris Packham wrote:
> (adding some ARM people, resending hopefully without the html)
> 
> On 19/03/22 03:20, Krzysztof Kozlowski wrote:
>>> +          - const: marvell,sdhci-xenon
>> This did not exist before. Separate patches please for additions (with
>> explanation why). Maybe some DTS lists this, but then it should be
>> individually judged whether the DTS is correct.
>>
> On this specifically. I was all ready to add an additional patch to 
> document this but then I noticed nothing actually uses the 
> "marvell,sdhci-xenon" compatible and it appears nothing ever did. I then 
> figured I'd delete the unused compatible string from armada-37xx.dtsi 
> but then I remembered that sometimes we add compatible strings to have 
> them "just in case" we need them for some SoC specific workaround.
> 
> So there's a few things I can do
> 
> 0. Nothing (easy) although the binding I just submitted will complain 
> about the unexpected value
> 1. Document "marvell,sdhci-xenon" as a valid compatible
> 1.a. Add "marvell,sdhci-xenon" to the sdhci-xenon.c driver
> 2. Remove "marvell,sdhci-xenon" from armada-37xx.dtsi

Option 1 in a new patch, explaining why you are adding it.

Best regards,
Krzysztof

Re: [PATCH] dt-bindings: mmc: xenon: Convert to JSON schema

[Krzysztof Kozlowski]

On 21/03/2022 01:12, Chris Packham wrote:
> (adding some ARM people, resending hopefully without the html)
> 
> On 19/03/22 03:20, Krzysztof Kozlowski wrote:
>>> +          - const: marvell,sdhci-xenon
>> This did not exist before. Separate patches please for additions (with
>> explanation why). Maybe some DTS lists this, but then it should be
>> individually judged whether the DTS is correct.
>>
> On this specifically. I was all ready to add an additional patch to 
> document this but then I noticed nothing actually uses the 
> "marvell,sdhci-xenon" compatible and it appears nothing ever did. I then 
> figured I'd delete the unused compatible string from armada-37xx.dtsi 
> but then I remembered that sometimes we add compatible strings to have 
> them "just in case" we need them for some SoC specific workaround.
> 
> So there's a few things I can do
> 
> 0. Nothing (easy) although the binding I just submitted will complain 
> about the unexpected value
> 1. Document "marvell,sdhci-xenon" as a valid compatible
> 1.a. Add "marvell,sdhci-xenon" to the sdhci-xenon.c driver
> 2. Remove "marvell,sdhci-xenon" from armada-37xx.dtsi

Option 1 in a new patch, explaining why you are adding it.

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel