diff mbox series

[v3,02/24] dt-bindings: memory: mediatek: Convert SMI to DT schema

Message ID 20200930070647.10188-3-yong.wu@mediatek.com (mailing list archive)
State New, archived
Headers show
Series MT8192 IOMMU support | expand

Commit Message

Yong Wu (吴勇) Sept. 30, 2020, 7:06 a.m. UTC
Convert MediaTek SMI to DT schema.

Signed-off-by: Yong Wu <yong.wu@mediatek.com>
---
 .../mediatek,smi-common.txt                   |  49 ---------
 .../mediatek,smi-common.yaml                  | 100 ++++++++++++++++++
 .../memory-controllers/mediatek,smi-larb.txt  |  49 ---------
 .../memory-controllers/mediatek,smi-larb.yaml |  91 ++++++++++++++++
 4 files changed, 191 insertions(+), 98 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt
 create mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
 delete mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.txt
 create mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml

Comments

Krzysztof Kozlowski Oct. 2, 2020, 11:04 a.m. UTC | #1
On Wed, Sep 30, 2020 at 03:06:25PM +0800, Yong Wu wrote:
> Convert MediaTek SMI to DT schema.
> 
> Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> ---
>  .../mediatek,smi-common.txt                   |  49 ---------
>  .../mediatek,smi-common.yaml                  | 100 ++++++++++++++++++
>  .../memory-controllers/mediatek,smi-larb.txt  |  49 ---------
>  .../memory-controllers/mediatek,smi-larb.yaml |  91 ++++++++++++++++
>  4 files changed, 191 insertions(+), 98 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt
>  create mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
>  delete mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.txt
>  create mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml
> 
> diff --git a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt
> deleted file mode 100644
> index b64573680b42..000000000000
> --- a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt
> +++ /dev/null
> @@ -1,49 +0,0 @@
> -SMI (Smart Multimedia Interface) Common
> -
> -The hardware block diagram please check bindings/iommu/mediatek,iommu.txt
> -
> -Mediatek SMI have two generations of HW architecture, here is the list
> -which generation the SoCs use:
> -generation 1: mt2701 and mt7623.
> -generation 2: mt2712, mt6779, mt8173 and mt8183.
> -
> -There's slight differences between the two SMI, for generation 2, the
> -register which control the iommu port is at each larb's register base. But
> -for generation 1, the register is at smi ao base(smi always on register
> -base). Besides that, the smi async clock should be prepared and enabled for
> -SMI generation 1 to transform the smi clock into emi clock domain, but that is
> -not needed for SMI generation 2.
> -
> -Required properties:
> -- compatible : must be one of :
> -	"mediatek,mt2701-smi-common"
> -	"mediatek,mt2712-smi-common"
> -	"mediatek,mt6779-smi-common"
> -	"mediatek,mt7623-smi-common", "mediatek,mt2701-smi-common"
> -	"mediatek,mt8173-smi-common"
> -	"mediatek,mt8183-smi-common"
> -- reg : the register and size of the SMI block.
> -- power-domains : a phandle to the power domain of this local arbiter.
> -- clocks : Must contain an entry for each entry in clock-names.
> -- clock-names : must contain 3 entries for generation 1 smi HW and 2 entries
> -  for generation 2 smi HW as follows:
> -  - "apb" : Advanced Peripheral Bus clock, It's the clock for setting
> -	    the register.
> -  - "smi" : It's the clock for transfer data and command.
> -	    They may be the same if both source clocks are the same.
> -  - "async" : asynchronous clock, it help transform the smi clock into the emi
> -	      clock domain, this clock is only needed by generation 1 smi HW.
> -  and these 2 option clocks for generation 2 smi HW:
> -  - "gals0": the path0 clock of GALS(Global Async Local Sync).
> -  - "gals1": the path1 clock of GALS(Global Async Local Sync).
> -  Here is the list which has this GALS: mt6779 and mt8183.
> -
> -Example:
> -	smi_common: smi@14022000 {
> -		compatible = "mediatek,mt8173-smi-common";
> -		reg = <0 0x14022000 0 0x1000>;
> -		power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
> -		clocks = <&mmsys CLK_MM_SMI_COMMON>,
> -			 <&mmsys CLK_MM_SMI_COMMON>;
> -		clock-names = "apb", "smi";
> -	};
> diff --git a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
> new file mode 100644
> index 000000000000..76ecc7205438
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
> @@ -0,0 +1,100 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)

You relicense existing GPLv2 work. Please CC all contributors and
collect their acks/SoB.

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/memory-controllers/mediatek,smi-common.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: SMI (Smart Multimedia Interface) Common
> +
> +maintainers:
> +  - Yong Wu <yong.wu@mediatek.com>
> +
> +description: |+
> +  The hardware block diagram please check bindings/iommu/mediatek,iommu.yaml
> +
> +  MediaTek SMI have two generations of HW architecture, here is the list
> +  which generation the SoCs use:
> +  generation 1: mt2701 and mt7623.
> +  generation 2: mt2712, mt6779, mt8173 and mt8183.
> +
> +  There's slight differences between the two SMI, for generation 2, the
> +  register which control the iommu port is at each larb's register base. But
> +  for generation 1, the register is at smi ao base(smi always on register
> +  base). Besides that, the smi async clock should be prepared and enabled for
> +  SMI generation 1 to transform the smi clock into emi clock domain, but that is
> +  not needed for SMI generation 2.
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - enum:
> +          - mediatek,mt2701-smi-common
> +          - mediatek,mt2712-smi-common
> +          - mediatek,mt6779-smi-common
> +          - mediatek,mt8173-smi-common
> +          - mediatek,mt8183-smi-common
> +
> +      - description: for mt7623
> +        items:
> +          - const: mediatek,mt7623-smi-common
> +          - const: mediatek,mt2701-smi-common
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    description: |
> +      apb and smi are mandatory. the async is only for generation 1 smi HW.
> +      gals(global async local sync) also is optional, here is the list which
> +      require gals: mt6779 and mt8183.
> +    minItems: 2
> +    maxItems: 4
> +    items:
> +      - description: apb is Advanced Peripheral Bus clock, It's the clock for
> +          setting the register.
> +      - description: smi is the clock for transfer data and command.
> +      - description: async is asynchronous clock, it help transform the smi clock
> +          into the emi clock domain.
> +      - description: gals0 is the path0 clock of gals.
> +      - description: gals1 is the path1 clock of gals.

You put here five items, but max are four. Does it really work as
intended?

> +
> +  clock-names:
> +    oneOf:
> +      - items:
> +          - const: apb
> +          - const: smi
> +      - items:
> +          - const: apb
> +          - const: smi
> +          - const: async
> +      - items:
> +          - const: apb
> +          - const: smi
> +          - const: gals0
> +          - const: gals1
> +
> +  power-domains:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - power-domains
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/mt8173-clk.h>
> +    #include <dt-bindings/power/mt8173-power.h>
> +
> +    smi_common: smi@14022000 {
> +            compatible = "mediatek,mt8173-smi-common";
> +            reg = <0x14022000 0x1000>;
> +            power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
> +            clocks = <&mmsys CLK_MM_SMI_COMMON>,
> +                     <&mmsys CLK_MM_SMI_COMMON>;
> +            clock-names = "apb", "smi";
> +    };
> diff --git a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.txt b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.txt
> deleted file mode 100644
> index 8f19dfe7d80e..000000000000
> --- a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.txt
> +++ /dev/null
> @@ -1,49 +0,0 @@
> -SMI (Smart Multimedia Interface) Local Arbiter
> -
> -The hardware block diagram please check bindings/iommu/mediatek,iommu.txt
> -
> -Required properties:
> -- compatible : must be one of :
> -		"mediatek,mt2701-smi-larb"
> -		"mediatek,mt2712-smi-larb"
> -		"mediatek,mt6779-smi-larb"
> -		"mediatek,mt7623-smi-larb", "mediatek,mt2701-smi-larb"
> -		"mediatek,mt8173-smi-larb"
> -		"mediatek,mt8183-smi-larb"
> -- reg : the register and size of this local arbiter.
> -- mediatek,smi : a phandle to the smi_common node.
> -- power-domains : a phandle to the power domain of this local arbiter.
> -- clocks : Must contain an entry for each entry in clock-names.
> -- clock-names: must contain 2 entries, as follows:
> -  - "apb" : Advanced Peripheral Bus clock, It's the clock for setting
> -	    the register.
> -  - "smi" : It's the clock for transfer data and command.
> -  and this optional clock name:
> -  - "gals": the clock for GALS(Global Async Local Sync).
> -  Here is the list which has this GALS: mt8183.
> -
> -Required property for mt2701, mt2712, mt6779 and mt7623:
> -- mediatek,larb-id :the hardware id of this larb.
> -
> -Example:
> -	larb1: larb@16010000 {
> -		compatible = "mediatek,mt8173-smi-larb";
> -		reg = <0 0x16010000 0 0x1000>;
> -		mediatek,smi = <&smi_common>;
> -		power-domains = <&scpsys MT8173_POWER_DOMAIN_VDEC>;
> -		clocks = <&vdecsys CLK_VDEC_CKEN>,
> -			 <&vdecsys CLK_VDEC_LARB_CKEN>;
> -		clock-names = "apb", "smi";
> -	};
> -
> -Example for mt2701:
> -	larb0: larb@14010000 {
> -		compatible = "mediatek,mt2701-smi-larb";
> -		reg = <0 0x14010000 0 0x1000>;
> -		mediatek,smi = <&smi_common>;
> -		mediatek,larb-id = <0>;
> -		clocks = <&mmsys CLK_MM_SMI_LARB0>,
> -			 <&mmsys CLK_MM_SMI_LARB0>;
> -		clock-names = "apb", "smi";
> -		power-domains = <&scpsys MT2701_POWER_DOMAIN_DISP>;
> -	};
> diff --git a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml
> new file mode 100644
> index 000000000000..50793a0e6759
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml
> @@ -0,0 +1,91 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)

The same - you need to collect licensing change agreements.

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/memory-controllers/mediatek,smi-larb.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: SMI (Smart Multimedia Interface) Local Arbiter
> +
> +maintainers:
> +  - Yong Wu <yong.wu@mediatek.com>
> +
> +description: |+
> +  The hardware block diagram please check bindings/iommu/mediatek,iommu.yaml
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - enum:
> +          - mediatek,mt2701-smi-larb
> +          - mediatek,mt2712-smi-larb
> +          - mediatek,mt6779-smi-larb
> +          - mediatek,mt8173-smi-larb
> +          - mediatek,mt8183-smi-larb
> +
> +      - description: for mt7623
> +        items:
> +          - const: mediatek,mt7623-smi-larb
> +          - const: mediatek,mt2701-smi-larb
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    description: |
> +      apb and smi are mandatory. gals(global async local sync) is optional,
> +      here is the list which require gals: mt8183.
> +    minItems: 2
> +    maxItems: 3
> +    items:
> +       - description: apb is Advanced Peripheral Bus clock, It's the clock for
> +           setting the register.
> +       - description: smi is the clock for transfer data and command.
> +       - description: the clock for gals.
> +
> +  clock-names:
> +    oneOf:
> +      - items:
> +         - const: apb
> +         - const: smi
> +      - items:
> +         - const: apb
> +         - const: smi
> +         - const: gals
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  mediatek,smi:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    description: a phandle to the smi_common node.
> +
> +  mediatek,larb-id:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0
> +    maximum: 15
> +    description: the hardware id of this larb.
> +      Required property for mt2701, mt2712, mt6779 and mt7623.

You need if-then-required for this. See
Documentation/devicetree/bindings/example-schema.yaml for example.

Best regards,
Krzysztof
Krzysztof Kozlowski Oct. 2, 2020, 11:08 a.m. UTC | #2
On Wed, Sep 30, 2020 at 03:06:25PM +0800, Yong Wu wrote:
> Convert MediaTek SMI to DT schema.
> 
> Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> ---
>  .../mediatek,smi-common.txt                   |  49 ---------
>  .../mediatek,smi-common.yaml                  | 100 ++++++++++++++++++
>  .../memory-controllers/mediatek,smi-larb.txt  |  49 ---------
>  .../memory-controllers/mediatek,smi-larb.yaml |  91 ++++++++++++++++
>  4 files changed, 191 insertions(+), 98 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt
>  create mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
>  delete mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.txt
>  create mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml
> 
> diff --git a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt
> deleted file mode 100644
> index b64573680b42..000000000000
> --- a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt
> +++ /dev/null
> @@ -1,49 +0,0 @@
> -SMI (Smart Multimedia Interface) Common
> -
> -The hardware block diagram please check bindings/iommu/mediatek,iommu.txt
> -
> -Mediatek SMI have two generations of HW architecture, here is the list
> -which generation the SoCs use:
> -generation 1: mt2701 and mt7623.
> -generation 2: mt2712, mt6779, mt8173 and mt8183.
> -
> -There's slight differences between the two SMI, for generation 2, the
> -register which control the iommu port is at each larb's register base. But
> -for generation 1, the register is at smi ao base(smi always on register
> -base). Besides that, the smi async clock should be prepared and enabled for
> -SMI generation 1 to transform the smi clock into emi clock domain, but that is
> -not needed for SMI generation 2.
> -
> -Required properties:
> -- compatible : must be one of :
> -	"mediatek,mt2701-smi-common"
> -	"mediatek,mt2712-smi-common"
> -	"mediatek,mt6779-smi-common"
> -	"mediatek,mt7623-smi-common", "mediatek,mt2701-smi-common"
> -	"mediatek,mt8173-smi-common"
> -	"mediatek,mt8183-smi-common"
> -- reg : the register and size of the SMI block.
> -- power-domains : a phandle to the power domain of this local arbiter.
> -- clocks : Must contain an entry for each entry in clock-names.
> -- clock-names : must contain 3 entries for generation 1 smi HW and 2 entries
> -  for generation 2 smi HW as follows:
> -  - "apb" : Advanced Peripheral Bus clock, It's the clock for setting
> -	    the register.
> -  - "smi" : It's the clock for transfer data and command.
> -	    They may be the same if both source clocks are the same.
> -  - "async" : asynchronous clock, it help transform the smi clock into the emi
> -	      clock domain, this clock is only needed by generation 1 smi HW.
> -  and these 2 option clocks for generation 2 smi HW:
> -  - "gals0": the path0 clock of GALS(Global Async Local Sync).
> -  - "gals1": the path1 clock of GALS(Global Async Local Sync).
> -  Here is the list which has this GALS: mt6779 and mt8183.
> -
> -Example:
> -	smi_common: smi@14022000 {
> -		compatible = "mediatek,mt8173-smi-common";
> -		reg = <0 0x14022000 0 0x1000>;
> -		power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
> -		clocks = <&mmsys CLK_MM_SMI_COMMON>,
> -			 <&mmsys CLK_MM_SMI_COMMON>;
> -		clock-names = "apb", "smi";
> -	};
> diff --git a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
> new file mode 100644
> index 000000000000..76ecc7205438
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
> @@ -0,0 +1,100 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/memory-controllers/mediatek,smi-common.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: SMI (Smart Multimedia Interface) Common
> +
> +maintainers:
> +  - Yong Wu <yong.wu@mediatek.com>
> +
> +description: |+
> +  The hardware block diagram please check bindings/iommu/mediatek,iommu.yaml
> +
> +  MediaTek SMI have two generations of HW architecture, here is the list
> +  which generation the SoCs use:
> +  generation 1: mt2701 and mt7623.
> +  generation 2: mt2712, mt6779, mt8173 and mt8183.
> +
> +  There's slight differences between the two SMI, for generation 2, the
> +  register which control the iommu port is at each larb's register base. But
> +  for generation 1, the register is at smi ao base(smi always on register
> +  base). Besides that, the smi async clock should be prepared and enabled for
> +  SMI generation 1 to transform the smi clock into emi clock domain, but that is
> +  not needed for SMI generation 2.
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - enum:
> +          - mediatek,mt2701-smi-common
> +          - mediatek,mt2712-smi-common
> +          - mediatek,mt6779-smi-common
> +          - mediatek,mt8173-smi-common
> +          - mediatek,mt8183-smi-common
> +
> +      - description: for mt7623
> +        items:
> +          - const: mediatek,mt7623-smi-common
> +          - const: mediatek,mt2701-smi-common
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    description: |
> +      apb and smi are mandatory. the async is only for generation 1 smi HW.
> +      gals(global async local sync) also is optional, here is the list which
> +      require gals: mt6779 and mt8183.
> +    minItems: 2
> +    maxItems: 4
> +    items:
> +      - description: apb is Advanced Peripheral Bus clock, It's the clock for
> +          setting the register.
> +      - description: smi is the clock for transfer data and command.
> +      - description: async is asynchronous clock, it help transform the smi clock
> +          into the emi clock domain.
> +      - description: gals0 is the path0 clock of gals.
> +      - description: gals1 is the path1 clock of gals.
> +
> +  clock-names:
> +    oneOf:
> +      - items:
> +          - const: apb
> +          - const: smi
> +      - items:
> +          - const: apb
> +          - const: smi
> +          - const: async
> +      - items:
> +          - const: apb
> +          - const: smi
> +          - const: gals0
> +          - const: gals1

Similarly to my comment to other properties, this requirement per
compatible should be part of the schema within 'if-then'.

Best regards,
Krzysztof
Yong Wu (吴勇) Oct. 6, 2020, 4:27 a.m. UTC | #3
On Fri, 2020-10-02 at 13:08 +0200, Krzysztof Kozlowski wrote:
> On Wed, Sep 30, 2020 at 03:06:25PM +0800, Yong Wu wrote:
> > Convert MediaTek SMI to DT schema.
> > 
> > Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> > ---
> >  .../mediatek,smi-common.txt                   |  49 ---------
> >  .../mediatek,smi-common.yaml                  | 100 ++++++++++++++++++
> >  .../memory-controllers/mediatek,smi-larb.txt  |  49 ---------
> >  .../memory-controllers/mediatek,smi-larb.yaml |  91 ++++++++++++++++
> >  4 files changed, 191 insertions(+), 98 deletions(-)
> >  delete mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt
> >  create mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
> >  delete mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.txt
> >  create mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml
...
> > +properties:
> > +  compatible:
> > +    oneOf:
> > +      - enum:
> > +          - mediatek,mt2701-smi-common
> > +          - mediatek,mt2712-smi-common
> > +          - mediatek,mt6779-smi-common
> > +          - mediatek,mt8173-smi-common
> > +          - mediatek,mt8183-smi-common
> > +
> > +      - description: for mt7623
> > +        items:
> > +          - const: mediatek,mt7623-smi-common
> > +          - const: mediatek,mt2701-smi-common
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    description: |
> > +      apb and smi are mandatory. the async is only for generation 1 smi HW.
> > +      gals(global async local sync) also is optional, here is the list which
> > +      require gals: mt6779 and mt8183.
> > +    minItems: 2
> > +    maxItems: 4
> > +    items:
> > +      - description: apb is Advanced Peripheral Bus clock, It's the clock for
> > +          setting the register.
> > +      - description: smi is the clock for transfer data and command.
> > +      - description: async is asynchronous clock, it help transform the smi clock
> > +          into the emi clock domain.
> > +      - description: gals0 is the path0 clock of gals.
> > +      - description: gals1 is the path1 clock of gals.
> > +
> > +  clock-names:
> > +    oneOf:
> > +      - items:
> > +          - const: apb
> > +          - const: smi
> > +      - items:
> > +          - const: apb
> > +          - const: smi
> > +          - const: async
> > +      - items:
> > +          - const: apb
> > +          - const: smi
> > +          - const: gals0
> > +          - const: gals1
> 
> Similarly to my comment to other properties, this requirement per
> compatible should be part of the schema within 'if-then'.

I'm not so familiar with this format. Do this has "if-then-'else
if'-then-else"?

I tried below instead of the clocks segment above:

===================================
if:
  properties:
    compatible:
      enum:
        - mediatek,mt6779-smi-common
        - mediatek,mt8183-smi-common

then:
  properties:
    clock:
      items:
        - description: apb is the clock for setting the register..
        - description: smi is the clock for transfer data and command.
        - description: gals0 is the path0 clock of gals(global async
local sync).
        - description: gals1 is the path1 clock of gals.
    clock-names:
      items:
        - const: apb
        - const: smi
        - const: gals0
        - const: gals1
else:
  if:
    properties:
      compatible:
        contains:
          enum:
            - mediatek,mt2701-smi-common

  then:
    properties:
      clocks:
        items:
          - description: apb is the clock for setting the register.
          - description: smi is the clock for transfer data and command.
          - description: async is asynchronous clock, it help transform
the smi clock
              into the emi clock domain.
      clock-names:
        items:
          - const: apb
          - const: smi
          - const: async
  else:
    properties:
      clocks:
        items:
          - description: apb is the clock for setting the register.
          - description: smi is the clock for transfer data and
command.    
      clock-names:
        items:
          - const: apb
          - const: smi
================================

But I got a warning when dt_binding_check:

CHKDT
Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
  SCHEMA
Documentation/devicetree/bindings/processed-schema-examples.yaml
  DTC
Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.example.dt.yaml
  CHECK
Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.example.dt.yaml
.../Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.example.dt.yaml: smi@14022000: 'clock-names', 'clocks' do not match any of the regexes: 'pinctrl-[0-9]+'
    From
schema: .../Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml

Any suggestion about this?
Thanks.

> 
> Best regards,
> Krzysztof
Krzysztof Kozlowski Oct. 6, 2020, 7:15 a.m. UTC | #4
On Tue, 6 Oct 2020 at 06:27, Yong Wu <yong.wu@mediatek.com> wrote:
>
> On Fri, 2020-10-02 at 13:08 +0200, Krzysztof Kozlowski wrote:
> > On Wed, Sep 30, 2020 at 03:06:25PM +0800, Yong Wu wrote:
> > > Convert MediaTek SMI to DT schema.
> > >
> > > Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> > > ---
> > >  .../mediatek,smi-common.txt                   |  49 ---------
> > >  .../mediatek,smi-common.yaml                  | 100 ++++++++++++++++++
> > >  .../memory-controllers/mediatek,smi-larb.txt  |  49 ---------
> > >  .../memory-controllers/mediatek,smi-larb.yaml |  91 ++++++++++++++++
> > >  4 files changed, 191 insertions(+), 98 deletions(-)
> > >  delete mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt
> > >  create mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
> > >  delete mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.txt
> > >  create mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml
> ...
> > > +properties:
> > > +  compatible:
> > > +    oneOf:
> > > +      - enum:
> > > +          - mediatek,mt2701-smi-common
> > > +          - mediatek,mt2712-smi-common
> > > +          - mediatek,mt6779-smi-common
> > > +          - mediatek,mt8173-smi-common
> > > +          - mediatek,mt8183-smi-common
> > > +
> > > +      - description: for mt7623
> > > +        items:
> > > +          - const: mediatek,mt7623-smi-common
> > > +          - const: mediatek,mt2701-smi-common
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  clocks:
> > > +    description: |
> > > +      apb and smi are mandatory. the async is only for generation 1 smi HW.
> > > +      gals(global async local sync) also is optional, here is the list which
> > > +      require gals: mt6779 and mt8183.
> > > +    minItems: 2
> > > +    maxItems: 4
> > > +    items:
> > > +      - description: apb is Advanced Peripheral Bus clock, It's the clock for
> > > +          setting the register.
> > > +      - description: smi is the clock for transfer data and command.
> > > +      - description: async is asynchronous clock, it help transform the smi clock
> > > +          into the emi clock domain.
> > > +      - description: gals0 is the path0 clock of gals.
> > > +      - description: gals1 is the path1 clock of gals.
> > > +
> > > +  clock-names:
> > > +    oneOf:
> > > +      - items:
> > > +          - const: apb
> > > +          - const: smi
> > > +      - items:
> > > +          - const: apb
> > > +          - const: smi
> > > +          - const: async
> > > +      - items:
> > > +          - const: apb
> > > +          - const: smi
> > > +          - const: gals0
> > > +          - const: gals1
> >
> > Similarly to my comment to other properties, this requirement per
> > compatible should be part of the schema within 'if-then'.
>
> I'm not so familiar with this format. Do this has "if-then-'else
> if'-then-else"?

These are mutually exclusive conditions, so you can skip else:
 - if-then
 - if-then
 - if-then
It will be more readable then stacking 'if' under 'else'

>
> I tried below instead of the clocks segment above:
>
> ===================================
> if:
>   properties:
>     compatible:

Missing contains. Just take an example from some existing schema.

>       enum:
>         - mediatek,mt6779-smi-common
>         - mediatek,mt8183-smi-common
>
> then:
>   properties:
>     clock:
>       items:
>         - description: apb is the clock for setting the register..
>         - description: smi is the clock for transfer data and command.
>         - description: gals0 is the path0 clock of gals(global async
> local sync).
>         - description: gals1 is the path1 clock of gals.
>     clock-names:
>       items:
>         - const: apb
>         - const: smi
>         - const: gals0
>         - const: gals1
> else:
>   if:
>     properties:
>       compatible:
>         contains:
>           enum:
>             - mediatek,mt2701-smi-common
>
>   then:
>     properties:
>       clocks:
>         items:
>           - description: apb is the clock for setting the register.
>           - description: smi is the clock for transfer data and command.
>           - description: async is asynchronous clock, it help transform
> the smi clock
>               into the emi clock domain.
>       clock-names:
>         items:
>           - const: apb
>           - const: smi
>           - const: async
>   else:
>     properties:
>       clocks:
>         items:
>           - description: apb is the clock for setting the register.
>           - description: smi is the clock for transfer data and
> command.
>       clock-names:
>         items:
>           - const: apb
>           - const: smi
> ================================
>
> But I got a warning when dt_binding_check:
>
> CHKDT
> Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
>   SCHEMA
> Documentation/devicetree/bindings/processed-schema-examples.yaml
>   DTC
> Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.example.dt.yaml
>   CHECK
> Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.example.dt.yaml
> .../Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.example.dt.yaml: smi@14022000: 'clock-names', 'clocks' do not match any of the regexes: 'pinctrl-[0-9]+'

There are several files which already choose different clocks based on
compatible, simple grep shows them:
Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml
Documentation/devicetree/bindings/clock/idt,versaclock5.yaml

Best regards,
Krzysztof
Yong Wu (吴勇) Oct. 10, 2020, 6:18 a.m. UTC | #5
On Tue, 2020-10-06 at 09:15 +0200, Krzysztof Kozlowski wrote:
> On Tue, 6 Oct 2020 at 06:27, Yong Wu <yong.wu@mediatek.com> wrote:
> >
> > On Fri, 2020-10-02 at 13:08 +0200, Krzysztof Kozlowski wrote:
> > > On Wed, Sep 30, 2020 at 03:06:25PM +0800, Yong Wu wrote:
> > > > Convert MediaTek SMI to DT schema.
> > > >
> > > > Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> > > > ---
> > > >  .../mediatek,smi-common.txt                   |  49 ---------
> > > >  .../mediatek,smi-common.yaml                  | 100 ++++++++++++++++++
> > > >  .../memory-controllers/mediatek,smi-larb.txt  |  49 ---------
> > > >  .../memory-controllers/mediatek,smi-larb.yaml |  91 ++++++++++++++++
> > > >  4 files changed, 191 insertions(+), 98 deletions(-)
> > > >  delete mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt
> > > >  create mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
> > > >  delete mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.txt
> > > >  create mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml
> > ...
> > > > +properties:
> > > > +  compatible:
> > > > +    oneOf:
> > > > +      - enum:
> > > > +          - mediatek,mt2701-smi-common
> > > > +          - mediatek,mt2712-smi-common
> > > > +          - mediatek,mt6779-smi-common
> > > > +          - mediatek,mt8173-smi-common
> > > > +          - mediatek,mt8183-smi-common
> > > > +
> > > > +      - description: for mt7623
> > > > +        items:
> > > > +          - const: mediatek,mt7623-smi-common
> > > > +          - const: mediatek,mt2701-smi-common
> > > > +
> > > > +  reg:
> > > > +    maxItems: 1
> > > > +
> > > > +  clocks:
> > > > +    description: |
> > > > +      apb and smi are mandatory. the async is only for generation 1 smi HW.
> > > > +      gals(global async local sync) also is optional, here is the list which
> > > > +      require gals: mt6779 and mt8183.
> > > > +    minItems: 2
> > > > +    maxItems: 4
> > > > +    items:
> > > > +      - description: apb is Advanced Peripheral Bus clock, It's the clock for
> > > > +          setting the register.
> > > > +      - description: smi is the clock for transfer data and command.
> > > > +      - description: async is asynchronous clock, it help transform the smi clock
> > > > +          into the emi clock domain.
> > > > +      - description: gals0 is the path0 clock of gals.
> > > > +      - description: gals1 is the path1 clock of gals.
> > > > +
> > > > +  clock-names:
> > > > +    oneOf:
> > > > +      - items:
> > > > +          - const: apb
> > > > +          - const: smi
> > > > +      - items:
> > > > +          - const: apb
> > > > +          - const: smi
> > > > +          - const: async
> > > > +      - items:
> > > > +          - const: apb
> > > > +          - const: smi
> > > > +          - const: gals0
> > > > +          - const: gals1
> > >
> > > Similarly to my comment to other properties, this requirement per
> > > compatible should be part of the schema within 'if-then'.
> >
> > I'm not so familiar with this format. Do this has "if-then-'else
> > if'-then-else"?
> 
> These are mutually exclusive conditions, so you can skip else:
>  - if-then
>  - if-then
>  - if-then
> It will be more readable then stacking 'if' under 'else'

Thanks. I will use something like this:

 anyOf:
   - if: #gen1 hw
     then:
       use apb/smi/async clocks

   - if: #gen2 hw that has gals.
     then:
       use apb/smi/gals0/gals1 clocks
     else: # gen2 hw that doesn't have gals.
       use apb/smi clocks.

> 
> >
> > I tried below instead of the clocks segment above:
> >
> > ===================================
> > if:
> >   properties:
> >     compatible:
> 
> Missing contains. Just take an example from some existing schema.


Like the example you gave below
(Documentation/devicetree/bindings/clock/idt,versaclock5.yaml), It also
doesn't have "contains" in "if". I guess it is unnecessary if there is
only one compatible string. it may be necessary when it has backward
compatible string.

> 
> >       enum:
> >         - mediatek,mt6779-smi-common
> >         - mediatek,mt8183-smi-common
> >
> > then:
> >   properties:
> >     clock:
> >       items:
> >         - description: apb is the clock for setting the register..
> >         - description: smi is the clock for transfer data and command.
> >         - description: gals0 is the path0 clock of gals(global async
> > local sync).
> >         - description: gals1 is the path1 clock of gals.
> >     clock-names:
> >       items:
> >         - const: apb
> >         - const: smi
> >         - const: gals0
> >         - const: gals1
> > else:
> >   if:
> >     properties:
> >       compatible:
> >         contains:
> >           enum:
> >             - mediatek,mt2701-smi-common
> >
> >   then:
> >     properties:
> >       clocks:
> >         items:
> >           - description: apb is the clock for setting the register.
> >           - description: smi is the clock for transfer data and command.
> >           - description: async is asynchronous clock, it help transform
> > the smi clock
> >               into the emi clock domain.
> >       clock-names:
> >         items:
> >           - const: apb
> >           - const: smi
> >           - const: async
> >   else:
> >     properties:
> >       clocks:
> >         items:
> >           - description: apb is the clock for setting the register.
> >           - description: smi is the clock for transfer data and
> > command.
> >       clock-names:
> >         items:
> >           - const: apb
> >           - const: smi
> > ================================
> >
> > But I got a warning when dt_binding_check:
> >
> > CHKDT
> > Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
> >   SCHEMA
> > Documentation/devicetree/bindings/processed-schema-examples.yaml
> >   DTC
> > Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.example.dt.yaml
> >   CHECK
> > Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.example.dt.yaml
> > .../Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.example.dt.yaml: smi@14022000: 'clock-names', 'clocks' do not match any of the regexes: 'pinctrl-[0-9]+'
> 
> There are several files which already choose different clocks based on
> compatible, simple grep shows them:
> Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml
> Documentation/devicetree/bindings/clock/idt,versaclock5.yaml

Thanks for the review. I will send the next version after v5.10.

> 
> Best regards,
> Krzysztof
Krzysztof Kozlowski Oct. 12, 2020, 7:18 a.m. UTC | #6
On Sat, Oct 10, 2020 at 02:18:11PM +0800, Yong Wu wrote:
> On Tue, 2020-10-06 at 09:15 +0200, Krzysztof Kozlowski wrote:
> > On Tue, 6 Oct 2020 at 06:27, Yong Wu <yong.wu@mediatek.com> wrote:
> > >
> > > On Fri, 2020-10-02 at 13:08 +0200, Krzysztof Kozlowski wrote:
> > > > On Wed, Sep 30, 2020 at 03:06:25PM +0800, Yong Wu wrote:
> > > > > Convert MediaTek SMI to DT schema.
> > > > >
> > > > > Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> > > > > ---
> > > > >  .../mediatek,smi-common.txt                   |  49 ---------
> > > > >  .../mediatek,smi-common.yaml                  | 100 ++++++++++++++++++
> > > > >  .../memory-controllers/mediatek,smi-larb.txt  |  49 ---------
> > > > >  .../memory-controllers/mediatek,smi-larb.yaml |  91 ++++++++++++++++
> > > > >  4 files changed, 191 insertions(+), 98 deletions(-)
> > > > >  delete mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt
> > > > >  create mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
> > > > >  delete mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.txt
> > > > >  create mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml
> > > ...
> > > > > +properties:
> > > > > +  compatible:
> > > > > +    oneOf:
> > > > > +      - enum:
> > > > > +          - mediatek,mt2701-smi-common
> > > > > +          - mediatek,mt2712-smi-common
> > > > > +          - mediatek,mt6779-smi-common
> > > > > +          - mediatek,mt8173-smi-common
> > > > > +          - mediatek,mt8183-smi-common
> > > > > +
> > > > > +      - description: for mt7623
> > > > > +        items:
> > > > > +          - const: mediatek,mt7623-smi-common
> > > > > +          - const: mediatek,mt2701-smi-common
> > > > > +
> > > > > +  reg:
> > > > > +    maxItems: 1
> > > > > +
> > > > > +  clocks:
> > > > > +    description: |
> > > > > +      apb and smi are mandatory. the async is only for generation 1 smi HW.
> > > > > +      gals(global async local sync) also is optional, here is the list which
> > > > > +      require gals: mt6779 and mt8183.
> > > > > +    minItems: 2
> > > > > +    maxItems: 4
> > > > > +    items:
> > > > > +      - description: apb is Advanced Peripheral Bus clock, It's the clock for
> > > > > +          setting the register.
> > > > > +      - description: smi is the clock for transfer data and command.
> > > > > +      - description: async is asynchronous clock, it help transform the smi clock
> > > > > +          into the emi clock domain.
> > > > > +      - description: gals0 is the path0 clock of gals.
> > > > > +      - description: gals1 is the path1 clock of gals.
> > > > > +
> > > > > +  clock-names:
> > > > > +    oneOf:
> > > > > +      - items:
> > > > > +          - const: apb
> > > > > +          - const: smi
> > > > > +      - items:
> > > > > +          - const: apb
> > > > > +          - const: smi
> > > > > +          - const: async
> > > > > +      - items:
> > > > > +          - const: apb
> > > > > +          - const: smi
> > > > > +          - const: gals0
> > > > > +          - const: gals1
> > > >
> > > > Similarly to my comment to other properties, this requirement per
> > > > compatible should be part of the schema within 'if-then'.
> > >
> > > I'm not so familiar with this format. Do this has "if-then-'else
> > > if'-then-else"?
> > 
> > These are mutually exclusive conditions, so you can skip else:
> >  - if-then
> >  - if-then
> >  - if-then
> > It will be more readable then stacking 'if' under 'else'
> 
> Thanks. I will use something like this:
> 
>  anyOf:

Then it should be oneOf as only one condition can be valid.

Best regards,
Krzysztof

>    - if: #gen1 hw
>      then:
>        use apb/smi/async clocks
> 
>    - if: #gen2 hw that has gals.
>      then:
>        use apb/smi/gals0/gals1 clocks
>      else: # gen2 hw that doesn't have gals.
>        use apb/smi clocks.
Yong Wu (吴勇) Oct. 12, 2020, 12:01 p.m. UTC | #7
On Mon, 2020-10-12 at 09:18 +0200, Krzysztof Kozlowski wrote:
> On Sat, Oct 10, 2020 at 02:18:11PM +0800, Yong Wu wrote:
> > On Tue, 2020-10-06 at 09:15 +0200, Krzysztof Kozlowski wrote:
> > > On Tue, 6 Oct 2020 at 06:27, Yong Wu <yong.wu@mediatek.com> wrote:
> > > >
> > > > On Fri, 2020-10-02 at 13:08 +0200, Krzysztof Kozlowski wrote:
> > > > > On Wed, Sep 30, 2020 at 03:06:25PM +0800, Yong Wu wrote:
> > > > > > Convert MediaTek SMI to DT schema.
> > > > > >
> > > > > > Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> > > > > > ---
> > > > > >  .../mediatek,smi-common.txt                   |  49 ---------
> > > > > >  .../mediatek,smi-common.yaml                  | 100 ++++++++++++++++++
> > > > > >  .../memory-controllers/mediatek,smi-larb.txt  |  49 ---------
> > > > > >  .../memory-controllers/mediatek,smi-larb.yaml |  91 ++++++++++++++++
> > > > > >  4 files changed, 191 insertions(+), 98 deletions(-)
> > > > > >  delete mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt
> > > > > >  create mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
> > > > > >  delete mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.txt
> > > > > >  create mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml
> > > > ...
> > > > > > +properties:
> > > > > > +  compatible:
> > > > > > +    oneOf:
> > > > > > +      - enum:
> > > > > > +          - mediatek,mt2701-smi-common
> > > > > > +          - mediatek,mt2712-smi-common
> > > > > > +          - mediatek,mt6779-smi-common
> > > > > > +          - mediatek,mt8173-smi-common
> > > > > > +          - mediatek,mt8183-smi-common
> > > > > > +
> > > > > > +      - description: for mt7623
> > > > > > +        items:
> > > > > > +          - const: mediatek,mt7623-smi-common
> > > > > > +          - const: mediatek,mt2701-smi-common
> > > > > > +
> > > > > > +  reg:
> > > > > > +    maxItems: 1
> > > > > > +
> > > > > > +  clocks:
> > > > > > +    description: |
> > > > > > +      apb and smi are mandatory. the async is only for generation 1 smi HW.
> > > > > > +      gals(global async local sync) also is optional, here is the list which
> > > > > > +      require gals: mt6779 and mt8183.
> > > > > > +    minItems: 2
> > > > > > +    maxItems: 4
> > > > > > +    items:
> > > > > > +      - description: apb is Advanced Peripheral Bus clock, It's the clock for
> > > > > > +          setting the register.
> > > > > > +      - description: smi is the clock for transfer data and command.
> > > > > > +      - description: async is asynchronous clock, it help transform the smi clock
> > > > > > +          into the emi clock domain.
> > > > > > +      - description: gals0 is the path0 clock of gals.
> > > > > > +      - description: gals1 is the path1 clock of gals.
> > > > > > +
> > > > > > +  clock-names:
> > > > > > +    oneOf:
> > > > > > +      - items:
> > > > > > +          - const: apb
> > > > > > +          - const: smi
> > > > > > +      - items:
> > > > > > +          - const: apb
> > > > > > +          - const: smi
> > > > > > +          - const: async
> > > > > > +      - items:
> > > > > > +          - const: apb
> > > > > > +          - const: smi
> > > > > > +          - const: gals0
> > > > > > +          - const: gals1
> > > > >
> > > > > Similarly to my comment to other properties, this requirement per
> > > > > compatible should be part of the schema within 'if-then'.
> > > >
> > > > I'm not so familiar with this format. Do this has "if-then-'else
> > > > if'-then-else"?
> > > 
> > > These are mutually exclusive conditions, so you can skip else:
> > >  - if-then
> > >  - if-then
> > >  - if-then
> > > It will be more readable then stacking 'if' under 'else'
> > 
> > Thanks. I will use something like this:
> > 
> >  anyOf:
> 
> Then it should be oneOf as only one condition can be valid.

I did do this at the beginning. But I get a warning log when
dt_binding_check.


Below is my schema and the detailed warning log:
//=======================================
  clocks:
    description: |
      xxxxx
    minItems: 2
    maxItems: 4
    items:
      - description: apb is the clock for setting the register.
      - description: smi is the clock for transfer data and command.
      - description: async is asynchronous clock.
      - description: gals0 is the path0 clock of gals.
      - description: gals1 is the path1 clock of gals.    

  clock-names:
    minItems: 2
    maxItems: 4 

required:
  - compatible
  - reg
  - power-domains
  - clocks
  - clock-names

oneOf:
  - if: #only for gen1 HW
      properties:
        compatible:
          contains:
            enum:
              - mediatek,mt2701-smi-common
    then:
       properties:
         clock:
           items:
             minItems: 3
             maxItems: 3
         clock-names:
           items:
             - const: apb
             - const: smi
             - const: async

  - if: #for gen2 HW that have gals
      properties:
        compatible:
            enum:
              - mediatek,mt6779-smi-common
              - mediatek,mt8183-smi-common
              - mediatek,mt8192-smi-common

    then:
      properties:
        clock:
          items:
            minItems: 4
            maxItems: 4
        clock-names:
          items:
            - const: apb
            - const: smi
            - const: gals0
            - const: gals1

    else: #for gen2 HW that don't have gals
      properties:
        clock:
          items:
            minItems: 2
            maxItems: 2
        clock-names:
          items:
            - const: apb
            - const: smi

additionalProperties: false 


//===========warning log=====================

Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.example.dt.yaml
.../Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.example.dt.yaml: smi@14022000: {'compatible': ['mediatek,mt8173-smi-common'], 'reg': [[335683584, 4096]], 'power-domains': [[4294967295, 3]], 'clocks': [[4294967295, 1], [4294967295, 1]], 'clock-names': ['apb', 'smi'], '$nodename': ['smi@14022000']} is valid under each of {'else': {'properties': {'clock': {'items': {'maxItems': 2, 'minItems': 2}, 'type': 'array'}, 'clock-names': {'additionalItems': False, 'items': [{'const': 'apb'}, {'const': 'smi'}], 'maxItems': 2, 'minItems': 2, 'type': 'array'}}}, 'if': {'properties': {'compatible': {'additionalItems': False, 'items': [{'enum': ['mediatek,mt6779-smi-common', 'mediatek,mt8183-smi-common', 'mediatek,mt8192-smi-common']}], 'maxItems': 1, 'minItems': 1, 'type': 'array'}}}, 'then': {'properties': {'clock': {'items': {'maxItems': 4, 'minItems': 4}, 'type': 'array'}, 'clock-names': {'additionalItems': False, 'items': [{'const': 'apb'}, {'const': 'smi'}, {'const'
 : 'gals0'}, {'const': 'gals1'}], 'maxItems': 4, 'minItems': 4, 'type': 'array'}}}}, {'if': {'properties': {'compatible': {'contains': {'enum': ['mediatek,mt2701-smi-common']}}}}, 'then': {'properties': {'clock': {'items': {'maxItems': 3, 'minItems': 3}, 'type': 'array'}, 'clock-names': {'additionalItems': False, 'items': [{'const': 'apb'}, {'const': 'smi'}, {'const': 'async'}], 'maxItems': 3, 'minItems': 3, 'type': 'array'}}}}
{'$filename':
'.../Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml',
 '$id':
'http://devicetree.org/schemas/memory-controllers/mediatek,smi-common.yaml#',
 '$schema': 'http://devicetree.org/meta-schemas/core.yaml#',
 '$select_validator': <jsonschema.validators.create.<locals>.Validator
object at 0x7fb3df378908>,
 'additionalProperties': False,
 'oneOf': [{'if': {'properties': {'compatible': {'contains': {'enum':
['mediatek,mt2701-smi-common']}}}},
            'then': {'properties': {'clock': {'items': {'maxItems': 3,
                                                        'minItems': 3},
                                              'type': 'array'},
                                    'clock-names': {'additionalItems':
False,
                                                    'items': [{'const':
'apb'},
                                                              {'const':
'smi'},
                                                              {'const':
'async'}],
                                                    'maxItems': 3,
                                                    'minItems': 3,
                                                    'type': 'array'}}}},
           {'else': {'properties': {'clock': {'items': {'maxItems': 2,
                                                        'minItems': 2},
                                              'type': 'array'},
                                    'clock-names': {'additionalItems':
False,
                                                    'items': [{'const':
'apb'},
                                                              {'const':
'smi'}],
                                                    'maxItems': 2,
                                                    'minItems': 2,
                                                    'type': 'array'}}},
            'if': {'properties': {'compatible': {'additionalItems':
False,
                                                 'items': [{'enum':
['mediatek,mt6779-smi-common',

'mediatek,mt8183-smi-common',

'mediatek,mt8192-smi-common']}],
                                                 'maxItems': 1,
                                                 'minItems': 1,
                                                 'type': 'array'}}},
            'then': {'properties': {'clock': {'items': {'maxItems': 4,
                                                        'minItems': 4},
                                              'type': 'array'},
                                    'clock-names': {'additionalItems':
False,
                                                    'items': [{'const':
'apb'},
                                                              {'const':
'smi'},
                                                              {'const':
'gals0'},
                                                              {'const':
'gals1'}],
                                                    'maxItems': 4,
                                                    'minItems': 4,
                                                    'type':
'array'}}}}],
 'patternProperties': {'pinctrl-[0-9]+': True},
 'properties': {'$nodename': True,
                'clock-names': {'maxItems': 4, 'minItems': 2},
                'clocks': {'additionalItems': False,
                           'items': [{}, {}, {}, {}, {}],
                           'maxItems': 4,
                           'minItems': 2,
                           'type': 'array'},
                'compatible': {'oneOf': [{'additionalItems': False,
                                          'items': [{'enum':
['mediatek,mt2701-smi-common',

'mediatek,mt2712-smi-common',

'mediatek,mt6779-smi-common',

'mediatek,mt8173-smi-common',

'mediatek,mt8183-smi-common',

'mediatek,mt8192-smi-common']}],
                                          'maxItems': 1,
                                          'minItems': 1,
                                          'type': 'array'},
                                         {'additionalItems': False,
                                          'items': [{'const':
'mediatek,mt7623-smi-common'},
                                                    {'const':
'mediatek,mt2701-smi-common'}],
                                          'maxItems': 2,
                                          'minItems': 2,
                                          'type': 'array'}]},
                'phandle': True,
                'pinctrl-names': True,
                'power-domains': {'maxItems': 1, 'minItems': 1},
                'reg': {'maxItems': 1, 'minItems': 1},
                'status': True},
 'required': ['compatible',
              'reg',
              'power-domains',
              'clocks',
              'clock-names'],
 'select': {'properties': {'compatible': {'contains': {'enum':
['mediatek,mt2701-smi-common',

'mediatek,mt2712-smi-common',

'mediatek,mt6779-smi-common',

'mediatek,mt7623-smi-common',

'mediatek,mt8173-smi-common',

'mediatek,mt8183-smi-common',

'mediatek,mt8192-smi-common']}}},
            'required': ['compatible']},
 'title': 'SMI (Smart Multimedia Interface) Common'}
	From
schema: .../Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml

=================================

Sorry, I didn't find a example of multi "if-then" in oneOf and don't
know how to fix this.

From [1], "allOf" looks also is allowed... In this case, both allOf and
anyOf have no warning when dt_binding_check. only oneOf has the warning
log above.

Any suggestion is appreciated.

[1]
https://elixir.bootlin.com/linux/v5.9-rc1/source/Documentation/devicetree/bindings/example-schema.yaml#L195

> 
> Best regards,
> Krzysztof
> 
> >    - if: #gen1 hw
> >      then:
> >        use apb/smi/async clocks
> > 
> >    - if: #gen2 hw that has gals.
> >      then:
> >        use apb/smi/gals0/gals1 clocks
> >      else: # gen2 hw that doesn't have gals.
> >        use apb/smi clocks.
Krzysztof Kozlowski Oct. 12, 2020, 1:26 p.m. UTC | #8
On Mon, 12 Oct 2020 at 14:02, Yong Wu <yong.wu@mediatek.com> wrote:
>
> On Mon, 2020-10-12 at 09:18 +0200, Krzysztof Kozlowski wrote:
> > On Sat, Oct 10, 2020 at 02:18:11PM +0800, Yong Wu wrote:
> > > On Tue, 2020-10-06 at 09:15 +0200, Krzysztof Kozlowski wrote:
> > > > On Tue, 6 Oct 2020 at 06:27, Yong Wu <yong.wu@mediatek.com> wrote:
> > > > >
> > > > > On Fri, 2020-10-02 at 13:08 +0200, Krzysztof Kozlowski wrote:
> > > > > > On Wed, Sep 30, 2020 at 03:06:25PM +0800, Yong Wu wrote:
> > > > > > > Convert MediaTek SMI to DT schema.
> > > > > > >
> > > > > > > Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> > > > > > > ---
> > > > > > >  .../mediatek,smi-common.txt                   |  49 ---------
> > > > > > >  .../mediatek,smi-common.yaml                  | 100 ++++++++++++++++++
> > > > > > >  .../memory-controllers/mediatek,smi-larb.txt  |  49 ---------
> > > > > > >  .../memory-controllers/mediatek,smi-larb.yaml |  91 ++++++++++++++++
> > > > > > >  4 files changed, 191 insertions(+), 98 deletions(-)
> > > > > > >  delete mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt
> > > > > > >  create mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
> > > > > > >  delete mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.txt
> > > > > > >  create mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml
> > > > > ...
> > > > > > > +properties:
> > > > > > > +  compatible:
> > > > > > > +    oneOf:
> > > > > > > +      - enum:
> > > > > > > +          - mediatek,mt2701-smi-common
> > > > > > > +          - mediatek,mt2712-smi-common
> > > > > > > +          - mediatek,mt6779-smi-common
> > > > > > > +          - mediatek,mt8173-smi-common
> > > > > > > +          - mediatek,mt8183-smi-common
> > > > > > > +
> > > > > > > +      - description: for mt7623
> > > > > > > +        items:
> > > > > > > +          - const: mediatek,mt7623-smi-common
> > > > > > > +          - const: mediatek,mt2701-smi-common
> > > > > > > +
> > > > > > > +  reg:
> > > > > > > +    maxItems: 1
> > > > > > > +
> > > > > > > +  clocks:
> > > > > > > +    description: |
> > > > > > > +      apb and smi are mandatory. the async is only for generation 1 smi HW.
> > > > > > > +      gals(global async local sync) also is optional, here is the list which
> > > > > > > +      require gals: mt6779 and mt8183.
> > > > > > > +    minItems: 2
> > > > > > > +    maxItems: 4
> > > > > > > +    items:
> > > > > > > +      - description: apb is Advanced Peripheral Bus clock, It's the clock for
> > > > > > > +          setting the register.
> > > > > > > +      - description: smi is the clock for transfer data and command.
> > > > > > > +      - description: async is asynchronous clock, it help transform the smi clock
> > > > > > > +          into the emi clock domain.
> > > > > > > +      - description: gals0 is the path0 clock of gals.
> > > > > > > +      - description: gals1 is the path1 clock of gals.
> > > > > > > +
> > > > > > > +  clock-names:
> > > > > > > +    oneOf:
> > > > > > > +      - items:
> > > > > > > +          - const: apb
> > > > > > > +          - const: smi
> > > > > > > +      - items:
> > > > > > > +          - const: apb
> > > > > > > +          - const: smi
> > > > > > > +          - const: async
> > > > > > > +      - items:
> > > > > > > +          - const: apb
> > > > > > > +          - const: smi
> > > > > > > +          - const: gals0
> > > > > > > +          - const: gals1
> > > > > >
> > > > > > Similarly to my comment to other properties, this requirement per
> > > > > > compatible should be part of the schema within 'if-then'.
> > > > >
> > > > > I'm not so familiar with this format. Do this has "if-then-'else
> > > > > if'-then-else"?
> > > >
> > > > These are mutually exclusive conditions, so you can skip else:
> > > >  - if-then
> > > >  - if-then
> > > >  - if-then
> > > > It will be more readable then stacking 'if' under 'else'
> > >
> > > Thanks. I will use something like this:
> > >
> > >  anyOf:
> >
> > Then it should be oneOf as only one condition can be valid.
>
> I did do this at the beginning. But I get a warning log when
> dt_binding_check.

Mhmm, right, since "if-else" matches in either of arms, then oneOf
will complain as it expects only one of items to match.  Then just go
with allOf. anyOf might match zero of items, so it would not catch
actual errors, I think.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt
deleted file mode 100644
index b64573680b42..000000000000
--- a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt
+++ /dev/null
@@ -1,49 +0,0 @@ 
-SMI (Smart Multimedia Interface) Common
-
-The hardware block diagram please check bindings/iommu/mediatek,iommu.txt
-
-Mediatek SMI have two generations of HW architecture, here is the list
-which generation the SoCs use:
-generation 1: mt2701 and mt7623.
-generation 2: mt2712, mt6779, mt8173 and mt8183.
-
-There's slight differences between the two SMI, for generation 2, the
-register which control the iommu port is at each larb's register base. But
-for generation 1, the register is at smi ao base(smi always on register
-base). Besides that, the smi async clock should be prepared and enabled for
-SMI generation 1 to transform the smi clock into emi clock domain, but that is
-not needed for SMI generation 2.
-
-Required properties:
-- compatible : must be one of :
-	"mediatek,mt2701-smi-common"
-	"mediatek,mt2712-smi-common"
-	"mediatek,mt6779-smi-common"
-	"mediatek,mt7623-smi-common", "mediatek,mt2701-smi-common"
-	"mediatek,mt8173-smi-common"
-	"mediatek,mt8183-smi-common"
-- reg : the register and size of the SMI block.
-- power-domains : a phandle to the power domain of this local arbiter.
-- clocks : Must contain an entry for each entry in clock-names.
-- clock-names : must contain 3 entries for generation 1 smi HW and 2 entries
-  for generation 2 smi HW as follows:
-  - "apb" : Advanced Peripheral Bus clock, It's the clock for setting
-	    the register.
-  - "smi" : It's the clock for transfer data and command.
-	    They may be the same if both source clocks are the same.
-  - "async" : asynchronous clock, it help transform the smi clock into the emi
-	      clock domain, this clock is only needed by generation 1 smi HW.
-  and these 2 option clocks for generation 2 smi HW:
-  - "gals0": the path0 clock of GALS(Global Async Local Sync).
-  - "gals1": the path1 clock of GALS(Global Async Local Sync).
-  Here is the list which has this GALS: mt6779 and mt8183.
-
-Example:
-	smi_common: smi@14022000 {
-		compatible = "mediatek,mt8173-smi-common";
-		reg = <0 0x14022000 0 0x1000>;
-		power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
-		clocks = <&mmsys CLK_MM_SMI_COMMON>,
-			 <&mmsys CLK_MM_SMI_COMMON>;
-		clock-names = "apb", "smi";
-	};
diff --git a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
new file mode 100644
index 000000000000..76ecc7205438
--- /dev/null
+++ b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml
@@ -0,0 +1,100 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/memory-controllers/mediatek,smi-common.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: SMI (Smart Multimedia Interface) Common
+
+maintainers:
+  - Yong Wu <yong.wu@mediatek.com>
+
+description: |+
+  The hardware block diagram please check bindings/iommu/mediatek,iommu.yaml
+
+  MediaTek SMI have two generations of HW architecture, here is the list
+  which generation the SoCs use:
+  generation 1: mt2701 and mt7623.
+  generation 2: mt2712, mt6779, mt8173 and mt8183.
+
+  There's slight differences between the two SMI, for generation 2, the
+  register which control the iommu port is at each larb's register base. But
+  for generation 1, the register is at smi ao base(smi always on register
+  base). Besides that, the smi async clock should be prepared and enabled for
+  SMI generation 1 to transform the smi clock into emi clock domain, but that is
+  not needed for SMI generation 2.
+
+properties:
+  compatible:
+    oneOf:
+      - enum:
+          - mediatek,mt2701-smi-common
+          - mediatek,mt2712-smi-common
+          - mediatek,mt6779-smi-common
+          - mediatek,mt8173-smi-common
+          - mediatek,mt8183-smi-common
+
+      - description: for mt7623
+        items:
+          - const: mediatek,mt7623-smi-common
+          - const: mediatek,mt2701-smi-common
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    description: |
+      apb and smi are mandatory. the async is only for generation 1 smi HW.
+      gals(global async local sync) also is optional, here is the list which
+      require gals: mt6779 and mt8183.
+    minItems: 2
+    maxItems: 4
+    items:
+      - description: apb is Advanced Peripheral Bus clock, It's the clock for
+          setting the register.
+      - description: smi is the clock for transfer data and command.
+      - description: async is asynchronous clock, it help transform the smi clock
+          into the emi clock domain.
+      - description: gals0 is the path0 clock of gals.
+      - description: gals1 is the path1 clock of gals.
+
+  clock-names:
+    oneOf:
+      - items:
+          - const: apb
+          - const: smi
+      - items:
+          - const: apb
+          - const: smi
+          - const: async
+      - items:
+          - const: apb
+          - const: smi
+          - const: gals0
+          - const: gals1
+
+  power-domains:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - power-domains
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/mt8173-clk.h>
+    #include <dt-bindings/power/mt8173-power.h>
+
+    smi_common: smi@14022000 {
+            compatible = "mediatek,mt8173-smi-common";
+            reg = <0x14022000 0x1000>;
+            power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+            clocks = <&mmsys CLK_MM_SMI_COMMON>,
+                     <&mmsys CLK_MM_SMI_COMMON>;
+            clock-names = "apb", "smi";
+    };
diff --git a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.txt b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.txt
deleted file mode 100644
index 8f19dfe7d80e..000000000000
--- a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.txt
+++ /dev/null
@@ -1,49 +0,0 @@ 
-SMI (Smart Multimedia Interface) Local Arbiter
-
-The hardware block diagram please check bindings/iommu/mediatek,iommu.txt
-
-Required properties:
-- compatible : must be one of :
-		"mediatek,mt2701-smi-larb"
-		"mediatek,mt2712-smi-larb"
-		"mediatek,mt6779-smi-larb"
-		"mediatek,mt7623-smi-larb", "mediatek,mt2701-smi-larb"
-		"mediatek,mt8173-smi-larb"
-		"mediatek,mt8183-smi-larb"
-- reg : the register and size of this local arbiter.
-- mediatek,smi : a phandle to the smi_common node.
-- power-domains : a phandle to the power domain of this local arbiter.
-- clocks : Must contain an entry for each entry in clock-names.
-- clock-names: must contain 2 entries, as follows:
-  - "apb" : Advanced Peripheral Bus clock, It's the clock for setting
-	    the register.
-  - "smi" : It's the clock for transfer data and command.
-  and this optional clock name:
-  - "gals": the clock for GALS(Global Async Local Sync).
-  Here is the list which has this GALS: mt8183.
-
-Required property for mt2701, mt2712, mt6779 and mt7623:
-- mediatek,larb-id :the hardware id of this larb.
-
-Example:
-	larb1: larb@16010000 {
-		compatible = "mediatek,mt8173-smi-larb";
-		reg = <0 0x16010000 0 0x1000>;
-		mediatek,smi = <&smi_common>;
-		power-domains = <&scpsys MT8173_POWER_DOMAIN_VDEC>;
-		clocks = <&vdecsys CLK_VDEC_CKEN>,
-			 <&vdecsys CLK_VDEC_LARB_CKEN>;
-		clock-names = "apb", "smi";
-	};
-
-Example for mt2701:
-	larb0: larb@14010000 {
-		compatible = "mediatek,mt2701-smi-larb";
-		reg = <0 0x14010000 0 0x1000>;
-		mediatek,smi = <&smi_common>;
-		mediatek,larb-id = <0>;
-		clocks = <&mmsys CLK_MM_SMI_LARB0>,
-			 <&mmsys CLK_MM_SMI_LARB0>;
-		clock-names = "apb", "smi";
-		power-domains = <&scpsys MT2701_POWER_DOMAIN_DISP>;
-	};
diff --git a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml
new file mode 100644
index 000000000000..50793a0e6759
--- /dev/null
+++ b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml
@@ -0,0 +1,91 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/memory-controllers/mediatek,smi-larb.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: SMI (Smart Multimedia Interface) Local Arbiter
+
+maintainers:
+  - Yong Wu <yong.wu@mediatek.com>
+
+description: |+
+  The hardware block diagram please check bindings/iommu/mediatek,iommu.yaml
+
+properties:
+  compatible:
+    oneOf:
+      - enum:
+          - mediatek,mt2701-smi-larb
+          - mediatek,mt2712-smi-larb
+          - mediatek,mt6779-smi-larb
+          - mediatek,mt8173-smi-larb
+          - mediatek,mt8183-smi-larb
+
+      - description: for mt7623
+        items:
+          - const: mediatek,mt7623-smi-larb
+          - const: mediatek,mt2701-smi-larb
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    description: |
+      apb and smi are mandatory. gals(global async local sync) is optional,
+      here is the list which require gals: mt8183.
+    minItems: 2
+    maxItems: 3
+    items:
+       - description: apb is Advanced Peripheral Bus clock, It's the clock for
+           setting the register.
+       - description: smi is the clock for transfer data and command.
+       - description: the clock for gals.
+
+  clock-names:
+    oneOf:
+      - items:
+         - const: apb
+         - const: smi
+      - items:
+         - const: apb
+         - const: smi
+         - const: gals
+
+  power-domains:
+    maxItems: 1
+
+  mediatek,smi:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description: a phandle to the smi_common node.
+
+  mediatek,larb-id:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 0
+    maximum: 15
+    description: the hardware id of this larb.
+      Required property for mt2701, mt2712, mt6779 and mt7623.
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - power-domains
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/mt8173-clk.h>
+    #include <dt-bindings/power/mt8173-power.h>
+
+    larb1: larb@16010000 {
+      compatible = "mediatek,mt8173-smi-larb";
+      reg = <0x16010000 0x1000>;
+      mediatek,smi = <&smi_common>;
+      power-domains = <&scpsys MT8173_POWER_DOMAIN_VDEC>;
+      clocks = <&vdecsys CLK_VDEC_CKEN>,
+               <&vdecsys CLK_VDEC_LARB_CKEN>;
+      clock-names = "apb", "smi";
+    };