2021-01-26 19:29:43

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH RFC 2/2] dt: pci: kirin-pcie.txt: convert it to yaml

On Tue, Jan 26, 2021 at 1:35 AM Mauro Carvalho Chehab
<[email protected]> wrote:
>
> Convert the file into a JSON description at the yaml format.
>
> Signed-off-by: Mauro Carvalho Chehab <[email protected]>
> ---
> .../bindings/pci/hisilicon,kirin-pcie.yaml | 98 +++++++++++++++++++
> .../devicetree/bindings/pci/kirin-pcie.txt | 50 ----------
> MAINTAINERS | 2 +-
> 3 files changed, 99 insertions(+), 51 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
> delete mode 100644 Documentation/devicetree/bindings/pci/kirin-pcie.txt
>
> diff --git a/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml b/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
> new file mode 100644
> index 000000000000..8d8112b2aca0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
> @@ -0,0 +1,98 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/hisilicon,kirin-pcie.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: HiSilicon Kirin SoCs PCIe host DT description
> +
> +maintainers:
> + - Mauro Carvalho Chehab <[email protected]>
> +
> +description: |
> + Kirin PCIe host controller is based on the Synopsys DesignWare PCI core.
> + It shares common functions with the PCIe DesignWare core driver and
> + inherits common properties defined in
> + Documentation/devicetree/bindings/pci/designware-pcie.yaml.

Drop this and move the $ref to here.

> +
> +properties:
> + compatible:
> + const: hisilicon,kirin960-pcie
> +
> + reg:
> + description: |
> + Should contain rc_dbi, apb, phy, config registers location and length.
> +
> + reg-names:
> + description: |
> + Must include the following entries:
> + "dbi": controller configuration registers;
> + "apb": apb Ctrl register defined by Kirin;
> + "phy": apb PHY register defined by Kirin;
> + "config": PCIe configuration space registers.

That needs to be a schema listing the entries.

> +
> + "#address-cells":
> + const: 3
> +
> + "#size-cells":
> + const: 2

Covered by pci-bus.yaml.

> +
> + reset-gpios:
> + description: The GPIO to generate PCIe PERST# assert and deassert signal.
> + maxItems: 1
> +
> +allOf:
> + - $ref: "designware,pcie.yaml#"
> +
> +required:
> + - compatible
> + - reg
> + - reg-names
> + - reset-gpios
> + - "#address-cells"
> + - "#size-cells"
> + - device_type
> + - ranges
> + - "#interrupt-cells"
> + - interrupt-map-mask
> + - interrupt-map

pci-bus.yaml covers most of these.

> +
> +additionalProperties: false

This will cause the example to fail (some reason these didn't get
picked up by PW). You need to use 'unevaluatedProperties: false' here.

> +
> +examples:
> + - |
> + #include <dt-bindings/clock/hi3660-clock.h>
> +
> + soc {
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + pcie@f4000000 {
> + compatible = "hisilicon,kirin960-pcie";
> + reg = <0x0 0xf4000000 0x0 0x1000>,
> + <0x0 0xff3fe000 0x0 0x1000>,
> + <0x0 0xf3f20000 0x0 0x40000>,
> + <0x0 0xF4000000 0 0x2000>;
> + reg-names = "dbi","apb","phy", "config";
> + bus-range = <0x0 0x1>;
> + #address-cells = <3>;
> + #size-cells = <2>;
> + device_type = "pci";
> + ranges = <0x02000000 0x0 0x00000000 0x0 0xf5000000 0x0 0x2000000>;
> + num-lanes = <1>;
> + #interrupt-cells = <1>;
> + interrupt-map-mask = <0xf800 0 0 7>;
> + interrupt-map = <0x0 0 0 1 &gic 0 0 0 282 4>,
> + <0x0 0 0 2 &gic 0 0 0 283 4>,
> + <0x0 0 0 3 &gic 0 0 0 284 4>,
> + <0x0 0 0 4 &gic 0 0 0 285 4>;
> + clocks = <&crg_ctrl HI3660_PCIEPHY_REF>,
> + <&crg_ctrl HI3660_CLK_GATE_PCIEAUX>,
> + <&crg_ctrl HI3660_PCLK_GATE_PCIE_PHY>,
> + <&crg_ctrl HI3660_PCLK_GATE_PCIE_SYS>,
> + <&crg_ctrl HI3660_ACLK_GATE_PCIE>;
> + clock-names = "pcie_phy_ref", "pcie_aux", "pcie_apb_phy",
> + "pcie_apb_sys", "pcie_aclk";
> + reset-gpios = <&gpio11 1 0 >;
> + };
> + };
> diff --git a/Documentation/devicetree/bindings/pci/kirin-pcie.txt b/Documentation/devicetree/bindings/pci/kirin-pcie.txt
> deleted file mode 100644
> index 8e4fe7fc50f9..000000000000
> --- a/Documentation/devicetree/bindings/pci/kirin-pcie.txt
> +++ /dev/null
> @@ -1,50 +0,0 @@
> -HiSilicon Kirin SoCs PCIe host DT description
> -
> -Kirin PCIe host controller is based on the Synopsys DesignWare PCI core.
> -It shares common functions with the PCIe DesignWare core driver and
> -inherits common properties defined in
> -Documentation/devicetree/bindings/pci/designware,pcie.yaml.
> -
> -Additional properties are described here:
> -
> -Required properties
> -- compatible:
> - "hisilicon,kirin960-pcie" for PCIe of Kirin960 SoC
> -- reg: Should contain rc_dbi, apb, phy, config registers location and length.
> -- reg-names: Must include the following entries:
> - "dbi": controller configuration registers;
> - "apb": apb Ctrl register defined by Kirin;
> - "phy": apb PHY register defined by Kirin;
> - "config": PCIe configuration space registers.
> -- reset-gpios: The GPIO to generate PCIe PERST# assert and deassert signal.
> -
> -Optional properties:
> -
> -Example based on kirin960:
> -
> - pcie@f4000000 {
> - compatible = "hisilicon,kirin-pcie";
> - reg = <0x0 0xf4000000 0x0 0x1000>, <0x0 0xff3fe000 0x0 0x1000>,
> - <0x0 0xf3f20000 0x0 0x40000>, <0x0 0xF4000000 0 0x2000>;
> - reg-names = "dbi","apb","phy", "config";
> - bus-range = <0x0 0x1>;
> - #address-cells = <3>;
> - #size-cells = <2>;
> - device_type = "pci";
> - ranges = <0x02000000 0x0 0x00000000 0x0 0xf5000000 0x0 0x2000000>;
> - num-lanes = <1>;
> - #interrupt-cells = <1>;
> - interrupt-map-mask = <0xf800 0 0 7>;
> - interrupt-map = <0x0 0 0 1 &gic 0 0 0 282 4>,
> - <0x0 0 0 2 &gic 0 0 0 283 4>,
> - <0x0 0 0 3 &gic 0 0 0 284 4>,
> - <0x0 0 0 4 &gic 0 0 0 285 4>;
> - clocks = <&crg_ctrl HI3660_PCIEPHY_REF>,
> - <&crg_ctrl HI3660_CLK_GATE_PCIEAUX>,
> - <&crg_ctrl HI3660_PCLK_GATE_PCIE_PHY>,
> - <&crg_ctrl HI3660_PCLK_GATE_PCIE_SYS>,
> - <&crg_ctrl HI3660_ACLK_GATE_PCIE>;
> - clock-names = "pcie_phy_ref", "pcie_aux",
> - "pcie_apb_phy", "pcie_apb_sys", "pcie_aclk";
> - reset-gpios = <&gpio11 1 0 >;
> - };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3bb3233830ec..2b98a4763724 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -13611,7 +13611,7 @@ M: Xiaowei Song <[email protected]>
> M: Binghui Wang <[email protected]>
> L: [email protected]
> S: Maintained
> -F: Documentation/devicetree/bindings/pci/kirin-pcie.txt
> +F: Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
> F: drivers/pci/controller/dwc/pcie-kirin.c
>
> PCIE DRIVER FOR HISILICON STB
> --
> 2.29.2
>


2021-02-02 09:49:23

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: Re: [PATCH RFC 2/2] dt: pci: kirin-pcie.txt: convert it to yaml

Em Tue, 26 Jan 2021 09:49:18 -0600
Rob Herring <[email protected]> escreveu:

> On Tue, Jan 26, 2021 at 1:35 AM Mauro Carvalho Chehab
> <[email protected]> wrote:
> >
> > Convert the file into a JSON description at the yaml format.
> >
> > Signed-off-by: Mauro Carvalho Chehab <[email protected]>
> > ---
> > .../bindings/pci/hisilicon,kirin-pcie.yaml | 98 +++++++++++++++++++
> > .../devicetree/bindings/pci/kirin-pcie.txt | 50 ----------
> > MAINTAINERS | 2 +-
> > 3 files changed, 99 insertions(+), 51 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
> > delete mode 100644 Documentation/devicetree/bindings/pci/kirin-pcie.txt
> >
> > diff --git a/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml b/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
> > new file mode 100644
> > index 000000000000..8d8112b2aca0
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
> > @@ -0,0 +1,98 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/pci/hisilicon,kirin-pcie.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: HiSilicon Kirin SoCs PCIe host DT description
> > +
> > +maintainers:
> > + - Mauro Carvalho Chehab <[email protected]>
> > +
> > +description: |
> > + Kirin PCIe host controller is based on the Synopsys DesignWare PCI core.
> > + It shares common functions with the PCIe DesignWare core driver and
> > + inherits common properties defined in
> > + Documentation/devicetree/bindings/pci/designware-pcie.yaml.
>
> Drop this and move the $ref to here.

That doesn't pass at dt_binding_check. If I do either:

allOf:
- $ref: snps,pcie.yaml#

or:

allOf:
- $ref: /schemas/pci/pci-bus.yaml#
- $ref: snps,pcie.yaml#

Then dt-binding-check starts to think that this DT is for a pinctrl:

make CROSS_COMPILE=aarch64-linux-gnu- ARCH=arm64 dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
LINT Documentation/devicetree/bindings
DTEX Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.example.dts
./Documentation/devicetree/bindings/sound/mt8192-mt6359-rt1015-rt5682.yaml:10:4: [warning] wrong indentation: expected 2 but found 3 (indentation)
./Documentation/devicetree/bindings/pinctrl/microchip,sparx5-sgpio.yaml:102:10: [warning] wrong indentation: expected 10 but found 9 (indentation)
CHKDT Documentation/devicetree/bindings/processed-schema-examples.json
SCHEMA Documentation/devicetree/bindings/processed-schema-examples.json
DTC Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.example.dt.yaml
CHECK Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.example.dt.yaml
.../Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.example.dt.yaml: pcie@f4000000: '#address-cells', '#interrupt-cells', '#size-cells', 'bus-range', 'device_type', 'interrupt-map', 'interrupt-map-mask', 'interrupt-names', 'interrupts', 'num-lanes', 'ranges', 'reg-names', 'reset-gpios' do not match any of the regexes: 'pinctrl-[0-9]+'
From schema: .../Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml

No idea why. Perhaps something broken at pinctrl schema?

Thanks,
Mauro

2021-02-09 18:10:25

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH RFC 2/2] dt: pci: kirin-pcie.txt: convert it to yaml

On Tue, Feb 02, 2021 at 10:45:37AM +0100, Mauro Carvalho Chehab wrote:
> Em Tue, 26 Jan 2021 09:49:18 -0600
> Rob Herring <[email protected]> escreveu:
>
> > On Tue, Jan 26, 2021 at 1:35 AM Mauro Carvalho Chehab
> > <[email protected]> wrote:
> > >
> > > Convert the file into a JSON description at the yaml format.
> > >
> > > Signed-off-by: Mauro Carvalho Chehab <[email protected]>
> > > ---
> > > .../bindings/pci/hisilicon,kirin-pcie.yaml | 98 +++++++++++++++++++
> > > .../devicetree/bindings/pci/kirin-pcie.txt | 50 ----------
> > > MAINTAINERS | 2 +-
> > > 3 files changed, 99 insertions(+), 51 deletions(-)
> > > create mode 100644 Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
> > > delete mode 100644 Documentation/devicetree/bindings/pci/kirin-pcie.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml b/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
> > > new file mode 100644
> > > index 000000000000..8d8112b2aca0
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
> > > @@ -0,0 +1,98 @@
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/pci/hisilicon,kirin-pcie.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: HiSilicon Kirin SoCs PCIe host DT description
> > > +
> > > +maintainers:
> > > + - Mauro Carvalho Chehab <[email protected]>
> > > +
> > > +description: |
> > > + Kirin PCIe host controller is based on the Synopsys DesignWare PCI core.
> > > + It shares common functions with the PCIe DesignWare core driver and
> > > + inherits common properties defined in
> > > + Documentation/devicetree/bindings/pci/designware-pcie.yaml.
> >
> > Drop this and move the $ref to here.
>
> That doesn't pass at dt_binding_check. If I do either:
>
> allOf:
> - $ref: snps,pcie.yaml#
>
> or:
>
> allOf:
> - $ref: /schemas/pci/pci-bus.yaml#
> - $ref: snps,pcie.yaml#
>
> Then dt-binding-check starts to think that this DT is for a pinctrl:
>
> make CROSS_COMPILE=aarch64-linux-gnu- ARCH=arm64 dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
> LINT Documentation/devicetree/bindings
> DTEX Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.example.dts
> ./Documentation/devicetree/bindings/sound/mt8192-mt6359-rt1015-rt5682.yaml:10:4: [warning] wrong indentation: expected 2 but found 3 (indentation)
> ./Documentation/devicetree/bindings/pinctrl/microchip,sparx5-sgpio.yaml:102:10: [warning] wrong indentation: expected 10 but found 9 (indentation)
> CHKDT Documentation/devicetree/bindings/processed-schema-examples.json
> SCHEMA Documentation/devicetree/bindings/processed-schema-examples.json
> DTC Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.example.dt.yaml
> CHECK Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.example.dt.yaml
> .../Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.example.dt.yaml: pcie@f4000000: '#address-cells', '#interrupt-cells', '#size-cells', 'bus-range', 'device_type', 'interrupt-map', 'interrupt-map-mask', 'interrupt-names', 'interrupts', 'num-lanes', 'ranges', 'reg-names', 'reset-gpios' do not match any of the regexes: 'pinctrl-[0-9]+'
> From schema: .../Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
>
> No idea why. Perhaps something broken at pinctrl schema?

You'll need to use unevaluatedProperties instead of
additionalProperties.

Rob