lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b172a1d1-fa48-6181-e66e-7dbe54076304@alliedtelesis.co.nz>
Date:   Mon, 21 Mar 2022 20:29:18 +0000
From:   Chris Packham <Chris.Packham@...iedtelesis.co.nz>
To:     "ulf.hansson@...aro.org" <ulf.hansson@...aro.org>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "huziji@...vell.com" <huziji@...vell.com>,
        "gregory.clement@...tlin.com" <gregory.clement@...tlin.com>
CC:     "linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] dt-bindings: mmc: xenon: Convert to JSON schema

Hi Gregory,

Question for you below

On 21/03/22 12:49, Chris Packham wrote:
> Convert the marvell,xenon-sdhci binding to JSON schema. Currently the
> in-tree dts files don't validate because they use sdhci@ instead of mmc@
> as required by the generic mmc-controller schema.
>
> Signed-off-by: Chris Packham <chris.packham@...iedtelesis.co.nz>
> ---
>
> Notes:
>      Changes in v2:
>      - Update MAINTAINERS entry
>      - Incorporate feedback from Krzysztof
>
>   .../bindings/mmc/marvell,xenon-sdhci.txt      | 173 -----------
>   .../bindings/mmc/marvell,xenon-sdhci.yaml     | 272 ++++++++++++++++++
>   MAINTAINERS                                   |   2 +-
>   3 files changed, 273 insertions(+), 174 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@...000 {
> -		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@...000 {
> -		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@...000 {
> -		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@...000 {
> -		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..eae7498686af
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml
> @@ -0,0 +1,272 @@
> +# 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
> +
> +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@...aro.org>
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - enum:
> +          - marvell,armada-3700-sdhci
> +          - marvell,armada-cp110-sdhci
> +          - marvell,armada-ap807-sdhci
> +          - marvell,armada-ap806-sdhci
> +
> +      - 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.
> +
> +  marvell,xenon-phy-type:
> +    $ref: /schemas/types.yaml#/definitions/string
> +    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.
> +
> +  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.
> +
> +  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.
> +
> +  marvell,xenon-phy-tun-step-divider:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    default: 64
> +    description: |
> +      Set the divider for calculating TUN_STEP.
> +
> +  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
> +    default: 0x9
> +    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.
> +
> +allOf:
> +  - $ref: mmc-controller.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: marvell,armada-3700-sdhci
> +
> +    then:
> +      properties:
> +        reg:
> +          items:
> +            - description: Xenon IP registers
> +            - description: Armada 3700 SoC PHY PAD Voltage Control register
> +          minItems: 2
> +
> +        marvell,pad-type:
> +          enum:
> +            - sd
> +            - fixed-1-8v
> +          description: |
> +            Type of Armada 3700 SoC PHY PAD Voltage Controller register.
> +            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:
> +        - marvell,pad-type
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - marvell,armada-cp110-sdhci
> +              - marvell,armada-ap807-sdhci
> +              - marvell,armada-ap806-sdhci

In commit bb16ea1742c8 ("mmc: sdhci-xenon: Fix clock resource by adding 
an optional bus clock") you added a comment that "For Armada AP806 and 
CP110, the AXI clock is also mandatory". The cp110 blocks all supply and 
axi clock but none of the existing ap806/ap807 blocks supply an axi 
clock. Does this mean they're all broken or is the clock actually 
optional for the ap806 (the code seems to treat it as such).

> +
> +    then:
> +      properties:
> +        clocks:
> +          minItems: 2
> +
> +        clock-names:
> +          items:
> +            - const: core
> +            - const: axi
> +
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    // For eMMC
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    mmc@...000 {
> +      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>
> +
> +    mmc@...000 {
> +      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>
> +
> +    mmc@...000 {
> +      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>
> +
> +    mmc@...000 {
> +      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";
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 05fd080b82f3..8e0a19d3b8d5 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11595,7 +11595,7 @@ MARVELL XENON MMC/SD/SDIO HOST CONTROLLER DRIVER
>   M:	Hu Ziji <huziji@...vell.com>
>   L:	linux-mmc@...r.kernel.org
>   S:	Supported
> -F:	Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
> +F:	Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.yaml
>   F:	drivers/mmc/host/sdhci-xenon*
>   
>   MATROX FRAMEBUFFER DRIVER

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ