Re: [PATCH v2 3/3] dt-bindings: mmc: convert arasan sdhci bindings to yaml

[Rob Herring <robh@kernel.org>]

On Tue, May 26, 2020 at 02:27:58PM +0800, Wan Ahmad Zainie wrote:
> Convert arasan,sdhci.txt file to yaml. The new file arasan,sdhci.yaml
> will inherit properties from mmc-controller.yaml. 'sdhci' is no longer
> a valid name for node and should be changed to 'mmc'.
> 
> Suggested-by: Ulf Hansson <ulf.hansson@linaro.org>
> Signed-off-by: Wan Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@intel.com>
> ---
>  .../devicetree/bindings/mmc/arasan,sdhci.txt  | 192 ------------
>  .../devicetree/bindings/mmc/arasan,sdhci.yaml | 293 ++++++++++++++++++
>  2 files changed, 293 insertions(+), 192 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
>  create mode 100644 Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml


> diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml b/Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml
> new file mode 100644
> index 000000000000..927e2f13958b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml
> @@ -0,0 +1,293 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/mmc/arasan,sdhci.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Device Tree Bindings for the Arasan SDHCI Controller
> +
> +allOf:
> +  - $ref: "mmc-controller.yaml#"
> +
> +maintainers:
> +  - Adrian Hunter <adrian.hunter@intel.com>
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - const: arasan,sdhci-8.9a                # generic Arasan SDHCI 8.9a PHY
> +      - const: arasan,sdhci-4.9a                # generic Arasan SDHCI 4.9a PHY
> +      - const: arasan,sdhci-5.1                 # generic Arasan SDHCI 5.1 PHY
> +      - items:
> +          - const: rockchip,rk3399-sdhci-5.1    # rk3399 eMMC PHY
> +          - const: arasan,sdhci-5.1
> +        description: |

Can drop '|' as formatting isn't important.

> +          For this device it is strongly suggested to include
> +          arasan,soc-ctl-syscon.
> +      - items:
> +          - const: xlnx,zynqmp-8.9a             # ZynqMP SDHCI 8.9a PHY
> +          - const: arasan,sdhci-8.9a
> +        description: |
> +          For this device it is strongly suggested to include
> +          clock-output-names and '#clock-cells'.

Sounds like a constraint. It's either optional or required though. There 
is no suggested.

> +      - items:
> +          - const: xlnx,versal-8.9a             # Versal SDHCI 8.9a PHY
> +          - const: arasan,sdhci-8.9a
> +        description: |
> +          For this device it is strongly suggested to include
> +          clock-output-names and '#clock-cells'.
> +      - items:
> +          - const: intel,lgm-sdhci-5.1-emmc     # Intel LGM eMMC PHY
> +          - const: arasan,sdhci-5.1
> +        description: |
> +          For this device it is strongly suggested to include
> +          arasan,soc-ctl-syscon.
> +      - items:
> +          - const: intel,lgm-sdhci-5.1-sdxc     # Intel LGM SDXC PHY
> +          - const: arasan,sdhci-5.1
> +        description: |
> +          For this device it is strongly suggested to include
> +          arasan,soc-ctl-syscon.
> +      - items:
> +          - const: intel,keembay-sdhci-5.1-emmc # Intel Keem Bay eMMC PHY
> +          - const: arasan,sdhci-5.1
> +        description: |
> +          For this device it is strongly suggested to include
> +          arasan,soc-ctl-syscon.
> +      - const: intel,keembay-sdhci-5.1-sd       # Intel Keem Bay SD controller
> +        description: |
> +          For this device it is strongly suggested to include
> +          arasan,soc-ctl-syscon.
> +      - const: intel,keembay-sdhci-5.1-sdio     # Intel Keem Bay SDIO controller
> +        description: |
> +          For this device it is strongly suggested to include
> +          arasan,soc-ctl-syscon.
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    minItems: 2
> +    maxItems: 3
> +
> +  clock-names:
> +    minItems: 2
> +    items:
> +      - const: clk_xin
> +      - const: clk_ahb
> +      - const: gate
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  phys:
> +    maxItems: 1
> +
> +  phy-names:
> +    const: phy_arasan
> +
> +  arasan,soc-ctl-syscon:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: |
> +      A phandle to a syscon device (see ../mfd/syscon.txt) used to access
> +      core corecfg registers. Offsets of registers in this syscon are
> +      determined based on the main compatible string for the device.
> +
> +  clock-output-names:
> +    description: |
> +      If specified, this will be the name of the card clock which will
> +      be exposed by this device. Required if '#clock-cells' is specified.

The last sentence can be a 'dependencies' schema.

Are there defined names for this?

> +
> +  '#clock-cells':
> +    enum: [0, 1]
> +    description: |
> +      With this property in place we will export one or two clocks
> +      representing the Card Clock. These clocks are expected to be
> +      consumed by our PHY.
> +
> +  xlnx,fails-without-test-cd:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description: |
> +      When present, the controller doesn't work when the CD line is not
> +      connected properly, and the line is not connected properly.
> +      Test mode can be used to force the controller to function.
> +
> +  xlnx,int-clock-stable-broken:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description: |
> +      When present, the controller always reports that the internal clock
> +      is stable even when it is not.
> +
> +  xlnx,mio-bank:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: |
> +      When specified, this will indicate the MIO bank number in which
> +      the command and data lines are configured. If not specified, driver
> +      will assume this as 0.

default: 0

Is there a range of valid values?

> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +
> +if:
> +  properties:
> +    compatible:
> +      contains:
> +        const: arasan,sdhci-5.1
> +then:
> +  required:
> +    - phys
> +    - phy-names

Add: unevaluatedProperties: false

> +
> +examples:
> +  - |
> +    mmc@e0100000 {
> +          compatible = "arasan,sdhci-8.9a";
> +          reg = <0xe0100000 0x1000>;
> +          clock-names = "clk_xin", "clk_ahb";
> +          clocks = <&clkc 21>, <&clkc 32>;
> +          interrupt-parent = <&gic>;
> +          interrupts = <0 24 4>;
> +    };
> +
> +  - |
> +    mmc@e2800000 {
> +          compatible = "arasan,sdhci-5.1";
> +          reg = <0xe2800000 0x1000>;
> +          clock-names = "clk_xin", "clk_ahb";
> +          clocks = <&cru 8>, <&cru 18>;
> +          interrupt-parent = <&gic>;
> +          interrupts = <0 24 4>;
> +          phys = <&emmc_phy>;
> +          phy-names = "phy_arasan";
> +    };
> +
> +  - |
> +    #include <dt-bindings/clock/rk3399-cru.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    mmc@fe330000 {
> +          compatible = "rockchip,rk3399-sdhci-5.1", "arasan,sdhci-5.1";
> +          reg = <0x0 0xfe330000 0x0 0x10000>;

Examples default to a single cell each for size and address.

> +          interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
> +          clocks = <&cru SCLK_EMMC>, <&cru ACLK_EMMC>;
> +          clock-names = "clk_xin", "clk_ahb";
> +          arasan,soc-ctl-syscon = <&grf>;
> +          assigned-clocks = <&cru SCLK_EMMC>;
> +          assigned-clock-rates = <200000000>;
> +          clock-output-names = "emmc_cardclock";
> +          phys = <&emmc_phy>;
> +          phy-names = "phy_arasan";
> +          #clock-cells = <0>;
> +    };
> +
> +  - |
> +    mmc@ff160000 {
> +          compatible = "xlnx,zynqmp-8.9a", "arasan,sdhci-8.9a";
> +          interrupt-parent = <&gic>;
> +          interrupts = <0 48 4>;
> +          reg = <0x0 0xff160000 0x0 0x1000>;

Same here.

> +          clocks = <&clk200>, <&clk200>;
> +          clock-names = "clk_xin", "clk_ahb";
> +          clock-output-names = "clk_out_sd0", "clk_in_sd0";
> +          #clock-cells = <1>;
> +          clk-phase-sd-hs = <63 72>;
> +    };
> +
> +  - |
> +    mmc@f1040000 {
> +          compatible = "xlnx,versal-8.9a", "arasan,sdhci-8.9a";
> +          interrupt-parent = <&gic>;
> +          interrupts = <0 126 4>;
> +          reg = <0x0 0xf1040000 0x0 0x10000>;
> +          clocks = <&clk200>, <&clk200>;
> +          clock-names = "clk_xin", "clk_ahb";
> +          clock-output-names = "clk_out_sd0", "clk_in_sd0";
> +          #clock-cells = <1>;
> +          clk-phase-sd-hs = <132>, <60>;
> +    };
> +
> +  - |
> +    #define LGM_CLK_EMMC5
> +    #define LGM_CLK_NGI
> +    #define LGM_GCLK_EMMC
> +    mmc@ec700000 {
> +          compatible = "intel,lgm-sdhci-5.1-emmc", "arasan,sdhci-5.1";
> +          reg = <0xec700000 0x300>;
> +          interrupt-parent = <&ioapic1>;
> +          interrupts = <44 1>;
> +          clocks = <&cgu0 LGM_CLK_EMMC5>, <&cgu0 LGM_CLK_NGI>,
> +                   <&cgu0 LGM_GCLK_EMMC>;
> +          clock-names = "clk_xin", "clk_ahb", "gate";
> +          clock-output-names = "emmc_cardclock";
> +          #clock-cells = <0>;
> +          phys = <&emmc_phy>;
> +          phy-names = "phy_arasan";
> +          arasan,soc-ctl-syscon = <&sysconf>;
> +    };
> +
> +  - |
> +    #define LGM_CLK_SDIO
> +    #define LGM_GCLK_SDXC
> +    mmc@ec600000 {
> +          compatible = "intel,lgm-sdhci-5.1-sdxc", "arasan,sdhci-5.1";
> +          reg = <0xec600000 0x300>;
> +          interrupt-parent = <&ioapic1>;
> +          interrupts = <43 1>;
> +          clocks = <&cgu0 LGM_CLK_SDIO>, <&cgu0 LGM_CLK_NGI>,
> +                   <&cgu0 LGM_GCLK_SDXC>;
> +          clock-names = "clk_xin", "clk_ahb", "gate";
> +          clock-output-names = "sdxc_cardclock";
> +          #clock-cells = <0>;
> +          phys = <&sdxc_phy>;
> +          phy-names = "phy_arasan";
> +          arasan,soc-ctl-syscon = <&sysconf>;
> +    };
> +
> +  - |
> +    #define KEEM_BAY_PSS_AUX_EMMC
> +    #define KEEM_BAY_PSS_EMMC
> +    mmc@33000000 {
> +          compatible = "intel,keembay-sdhci-5.1-emmc", "arasan,sdhci-5.1";
> +          interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
> +          reg = <0x0 0x33000000 0x0 0x300>;
> +          clock-names = "clk_xin", "clk_ahb";
> +          clocks = <&scmi_clk KEEM_BAY_PSS_AUX_EMMC>,
> +                   <&scmi_clk KEEM_BAY_PSS_EMMC>;
> +          phys = <&emmc_phy>;
> +          phy-names = "phy_arasan";
> +          assigned-clocks = <&scmi_clk KEEM_BAY_PSS_AUX_EMMC>;
> +          assigned-clock-rates = <200000000>;
> +          clock-output-names = "emmc_cardclock";
> +          #clock-cells = <0>;
> +          arasan,soc-ctl-syscon = <&mmc_phy_syscon>;
> +    };
> +
> +  - |
> +    #define KEEM_BAY_PSS_AUX_SD0
> +    #define KEEM_BAY_PSS_SD0
> +    mmc@31000000 {
> +          compatible = "intel,keembay-sdhci-5.1-sd";
> +          interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>;
> +          reg = <0x0 0x31000000 0x0 0x300>;
> +          clock-names = "clk_xin", "clk_ahb";
> +          clocks = <&scmi_clk KEEM_BAY_PSS_AUX_SD0>,
> +                   <&scmi_clk KEEM_BAY_PSS_SD0>;
> +          arasan,soc-ctl-syscon = <&sd0_phy_syscon>;
> +    };
> +
> +  - |
> +    #define KEEM_BAY_PSS_AUX_SD1
> +    #define KEEM_BAY_PSS_SD1
> +    mmc@32000000 {
> +          compatible = "intel,keembay-sdhci-5.1-sdio";
> +          interrupts = <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>;
> +          reg = <0x0 0x32000000 0x0 0x300>;
> +          clock-names = "clk_xin", "clk_ahb";
> +          clocks = <&scmi_clk KEEM_BAY_PSS_AUX_SD1>,
> +                   <&scmi_clk KEEM_BAY_PSS_SD1>;
> +          arasan,soc-ctl-syscon = <&sd1_phy_syscon>;
> +    };

Really need 3 Keem Bay examples?

> -- 
> 2.17.1
>