diff mbox series

[v2,2/8] dt-bindings: PCI: renesas-pci-usb: Convert bindings to json-schema

Message ID 20220414074011.500533-3-herve.codina@bootlin.com (mailing list archive)
State Superseded
Delegated to: Geert Uytterhoeven
Headers show
Series RZN1 USB Host support | expand

Commit Message

Herve Codina April 14, 2022, 7:40 a.m. UTC
Convert Renesas PCI bridge bindings documentation to json-schema.
Also name it 'renesas,pci-usb' as it is specifically used to
connect the PCI USB controllers to AHB bus.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 .../devicetree/bindings/pci/pci-rcar-gen2.txt |  84 -----------
 .../bindings/pci/renesas,pci-usb.yaml         | 134 ++++++++++++++++++
 2 files changed, 134 insertions(+), 84 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt
 create mode 100644 Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml

Comments

Miquel Raynal April 14, 2022, 8:08 a.m. UTC | #1
Hi Hervé,

herve.codina@bootlin.com wrote on Thu, 14 Apr 2022 09:40:05 +0200:

> Convert Renesas PCI bridge bindings documentation to json-schema.
> Also name it 'renesas,pci-usb' as it is specifically used to
> connect the PCI USB controllers to AHB bus.
> 
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---
>  .../devicetree/bindings/pci/pci-rcar-gen2.txt |  84 -----------
>  .../bindings/pci/renesas,pci-usb.yaml         | 134 ++++++++++++++++++
>  2 files changed, 134 insertions(+), 84 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt
>  create mode 100644 Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml
> 
> diff --git a/Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt b/Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt
> deleted file mode 100644
> index aeba38f0a387..000000000000
> --- a/Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt
> +++ /dev/null
> @@ -1,84 +0,0 @@
> -Renesas AHB to PCI bridge
> --------------------------
> -
> -This is the bridge used internally to connect the USB controllers to the
> -AHB. There is one bridge instance per USB port connected to the internal
> -OHCI and EHCI controllers.
> -
> -Required properties:
> -- compatible: "renesas,pci-r8a7742" for the R8A7742 SoC;
> -	      "renesas,pci-r8a7743" for the R8A7743 SoC;
> -	      "renesas,pci-r8a7744" for the R8A7744 SoC;
> -	      "renesas,pci-r8a7745" for the R8A7745 SoC;
> -	      "renesas,pci-r8a7790" for the R8A7790 SoC;
> -	      "renesas,pci-r8a7791" for the R8A7791 SoC;
> -	      "renesas,pci-r8a7793" for the R8A7793 SoC;
> -	      "renesas,pci-r8a7794" for the R8A7794 SoC;
> -	      "renesas,pci-rcar-gen2" for a generic R-Car Gen2 or
> -				      RZ/G1 compatible device.
> -
> -
> -	      When compatible with the generic version, nodes must list the
> -	      SoC-specific version corresponding to the platform first
> -	      followed by the generic version.
> -
> -- reg:	A list of physical regions to access the device: the first is
> -	the operational registers for the OHCI/EHCI controllers and the
> -	second is for the bridge configuration and control registers.
> -- interrupts: interrupt for the device.
> -- clocks: The reference to the device clock.
> -- bus-range: The PCI bus number range; as this is a single bus, the range
> -	     should be specified as the same value twice.
> -- #address-cells: must be 3.
> -- #size-cells: must be 2.
> -- #interrupt-cells: must be 1.
> -- interrupt-map: standard property used to define the mapping of the PCI
> -  interrupts to the GIC interrupts.
> -- interrupt-map-mask: standard property that helps to define the interrupt
> -  mapping.
> -
> -Optional properties:
> -- dma-ranges: a single range for the inbound memory region. If not supplied,
> -  defaults to 1GiB at 0x40000000. Note there are hardware restrictions on the
> -  allowed combinations of address and size.
> -
> -Example SoC configuration:
> -
> -	pci0: pci@ee090000  {
> -		compatible = "renesas,pci-r8a7790", "renesas,pci-rcar-gen2";
> -		clocks = <&mstp7_clks R8A7790_CLK_EHCI>;
> -		reg = <0x0 0xee090000 0x0 0xc00>,
> -		      <0x0 0xee080000 0x0 0x1100>;
> -		interrupts = <0 108 IRQ_TYPE_LEVEL_HIGH>;
> -		status = "disabled";
> -
> -		bus-range = <0 0>;
> -		#address-cells = <3>;
> -		#size-cells = <2>;
> -		#interrupt-cells = <1>;
> -		dma-ranges = <0x42000000 0 0x40000000 0 0x40000000 0 0x40000000>;
> -		interrupt-map-mask = <0xff00 0 0 0x7>;
> -		interrupt-map = <0x0000 0 0 1 &gic 0 108 IRQ_TYPE_LEVEL_HIGH
> -				 0x0800 0 0 1 &gic 0 108 IRQ_TYPE_LEVEL_HIGH
> -				 0x1000 0 0 2 &gic 0 108 IRQ_TYPE_LEVEL_HIGH>;
> -
> -		usb@1,0 {
> -			reg = <0x800 0 0 0 0>;
> -			phys = <&usb0 0>;
> -			phy-names = "usb";
> -		};
> -
> -		usb@2,0 {
> -			reg = <0x1000 0 0 0 0>;
> -			phys = <&usb0 0>;
> -			phy-names = "usb";
> -		};
> -	};
> -
> -Example board setup:
> -
> -&pci0 {
> -	status = "okay";
> -	pinctrl-0 = <&usb0_pins>;
> -	pinctrl-names = "default";
> -};
> diff --git a/Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml b/Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml
> new file mode 100644
> index 000000000000..3f8d79b746c7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml
> @@ -0,0 +1,134 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/renesas,pci-usb.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas AHB to PCI bridge
> +
> +maintainers:
> +  - Marek Vasut <marek.vasut+renesas@gmail.com>
> +  - Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> +
> +description: |
> +  This is the bridge used internally to connect the USB controllers to the
> +  AHB. There is one bridge instance per USB port connected to the internal
> +  OHCI and EHCI controllers.
> +
> +allOf:
> +  - $ref: /schemas/pci/pci-bus.yaml#
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - items:
> +          - enum:
> +              - renesas,pci-r8a7742      # RZ/G1H
> +              - renesas,pci-r8a7743      # RZ/G1M
> +              - renesas,pci-r8a7744      # RZ/G1N
> +              - renesas,pci-r8a7745      # RZ/G1E
> +              - renesas,pci-r8a7790      # R-Car H2
> +              - renesas,pci-r8a7791      # R-Car M2-W
> +              - renesas,pci-r8a7793      # R-Car M2-N
> +              - renesas,pci-r8a7794      # R-Car E2
> +          - const: renesas,pci-rcar-gen2 # R-Car Gen2 and RZ/G1
> +
> +  reg:
> +    description: |
> +      A list of physical regions to access the device. The first is
> +      the operational registers for the OHCI/EHCI controllers and the
> +      second is for the bridge configuration and control registers.
> +    minItems: 2
> +    maxItems: 2
> +
> +  interrupts:
> +    description: Interrupt for the device.

When the description is rather straightforward (like "this is the main
interrupt/clock") I don't think a description is expected. A number
however might be useful, I believe.

> +
> +  interrupt-map:
> +    description: |
> +      Standard property used to define the mapping of the PCI interrupts
> +      to the GIC interrupts.
> +
> +  interrupt-map-mask:
> +    description:
> +      Standard property that helps to define the interrupt mapping.
> +
> +  clocks:
> +    description: The reference to the device clock.

Maybe maxItems: 1 ?

> +
> +  bus-range:
> +    description: |
> +      The PCI bus number range; as this is a single bus, the range
> +      should be specified as the same value twice.
> +
> +  "#address-cells":
> +    const: 3
> +
> +  "#size-cells":
> +    const: 2
> +
> +  "#interrupt-cells":
> +    const: 1
> +
> +  dma-ranges:
> +    description: |
> +      A single range for the inbound memory region. If not supplied,
> +      defaults to 1GiB at 0x40000000. Note there are hardware restrictions on
> +      the allowed combinations of address and size.
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - interrupt-map
> +  - interrupt-map-mask
> +  - clocks
> +  - bus-range
> +  - "#address-cells"
> +  - "#size-cells"
> +  - "#interrupt-cells"
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/r8a7790-cpg-mssr.h>
> +
> +    bus {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        pci0: pci@ee090000  {
> +            compatible = "renesas,pci-r8a7790", "renesas,pci-rcar-gen2";
> +            device_type = "pci";
> +            clocks = <&cpg CPG_MOD 703>;
> +            reg = <0 0xee090000 0 0xc00>,
> +                  <0 0xee080000 0 0x1100>;
> +            interrupts = <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>;
> +            status = "disabled";
> +
> +            bus-range = <0 0>;
> +            #address-cells = <3>;
> +            #size-cells = <2>;
> +            #interrupt-cells = <1>;
> +            ranges = <0x02000000 0 0xee080000 0 0xee080000 0 0x00010000>;
> +            dma-ranges = <0x42000000 0 0x40000000 0 0x40000000 0 0x40000000>;
> +            interrupt-map-mask = <0xf800 0 0 0x7>;
> +            interrupt-map = <0x0000 0 0 1 &gic GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>,
> +                            <0x0800 0 0 1 &gic GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>,
> +                            <0x1000 0 0 2 &gic GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>;
> +
> +            usb@1,0 {
> +                reg = <0x800 0 0 0 0>;
> +                phys = <&usb0 0>;
> +                phy-names = "usb";
> +            };
> +            
> +            usb@2,0 {
> +                reg = <0x1000 0 0 0 0>;
> +                phys = <&usb0 0>;
> +                phy-names = "usb";
> +            };
> +        };
> +    };


Thanks,
Miquèl
Geert Uytterhoeven April 14, 2022, 8:28 a.m. UTC | #2
Hi Hervé,

On Thu, Apr 14, 2022 at 9:40 AM Herve Codina <herve.codina@bootlin.com> wrote:
> Convert Renesas PCI bridge bindings documentation to json-schema.
> Also name it 'renesas,pci-usb' as it is specifically used to
> connect the PCI USB controllers to AHB bus.
>
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>

Thanks a lot for tackling this DT binding file!

> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml
> @@ -0,0 +1,134 @@
> +# SPDX-License-Identifier: GPL-2.0

scripts/checkpatch.pl says:
WARNING: DT binding documents should be licensed (GPL-2.0-only OR BSD-2-Clause)

> +  reg:
> +    description: |
> +      A list of physical regions to access the device. The first is
> +      the operational registers for the OHCI/EHCI controllers and the
> +      second is for the bridge configuration and control registers.
> +    minItems: 2
> +    maxItems: 2

reg:
  items:
    - description: Operational registers for the OHCI/EHCI controllers.
    - description: Bridge configuration and control registers.

> +
> +  interrupts:
> +    description: Interrupt for the device.

maxItems: 1

The description is not needed.

> +
> +  interrupt-map:
> +    description: |
> +      Standard property used to define the mapping of the PCI interrupts
> +      to the GIC interrupts.
> +
> +  interrupt-map-mask:
> +    description:
> +      Standard property that helps to define the interrupt mapping.
> +
> +  clocks:
> +    description: The reference to the device clock.

maxItems: 1

The description is not needed.

Missing "resets" and "power-domains" properties.

Missing description of the child nodes.

> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - interrupt-map
> +  - interrupt-map-mask
> +  - clocks

Missing "resets" and "power-domains".

> +  - bus-range
> +  - "#address-cells"
> +  - "#size-cells"
> +  - "#interrupt-cells"
> +
> +unevaluatedProperties: false

Why doesn't "make dtbs_check" complain about the presence of
e.g. "resets" in the actual DTS files?

> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/r8a7790-cpg-mssr.h>
> +
> +    bus {
> +        #address-cells = <2>;
> +        #size-cells = <2>;

I think you should drop this (and the corresponding high addresses
below).

> +
> +        pci0: pci@ee090000  {
> +            compatible = "renesas,pci-r8a7790", "renesas,pci-rcar-gen2";
> +            device_type = "pci";
> +            clocks = <&cpg CPG_MOD 703>;
> +            reg = <0 0xee090000 0 0xc00>,
> +                  <0 0xee080000 0 0x1100>;
> +            interrupts = <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>;

                        power-domains = <&sysc R8A7790_PD_ALWAYS_ON>;
                        resets = <&cpg 703>;

> +            status = "disabled";
> +
> +            bus-range = <0 0>;
> +            #address-cells = <3>;
> +            #size-cells = <2>;
> +            #interrupt-cells = <1>;
> +            ranges = <0x02000000 0 0xee080000 0 0xee080000 0 0x00010000>;
> +            dma-ranges = <0x42000000 0 0x40000000 0 0x40000000 0 0x40000000>;
> +            interrupt-map-mask = <0xf800 0 0 0x7>;
> +            interrupt-map = <0x0000 0 0 1 &gic GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>,
> +                            <0x0800 0 0 1 &gic GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>,
> +                            <0x1000 0 0 2 &gic GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>;
> +
> +            usb@1,0 {
> +                reg = <0x800 0 0 0 0>;
> +                phys = <&usb0 0>;
> +                phy-names = "usb";
> +            };
> +

ERROR: trailing whitespace
#249: FILE: Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml:127:
+            $

> +            usb@2,0 {
> +                reg = <0x1000 0 0 0 0>;
> +                phys = <&usb0 0>;
> +                phy-names = "usb";
> +            };
> +        };
> +    };

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Rob Herring (Arm) April 14, 2022, 6:15 p.m. UTC | #3
On Thu, Apr 14, 2022 at 09:40:05AM +0200, Herve Codina wrote:
> Convert Renesas PCI bridge bindings documentation to json-schema.
> Also name it 'renesas,pci-usb' as it is specifically used to
> connect the PCI USB controllers to AHB bus.

Please name it based on compatible strings. renesas,pci-rcar-gen2.yaml

> 
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---
>  .../devicetree/bindings/pci/pci-rcar-gen2.txt |  84 -----------
>  .../bindings/pci/renesas,pci-usb.yaml         | 134 ++++++++++++++++++
>  2 files changed, 134 insertions(+), 84 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt
>  create mode 100644 Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml
> 
> diff --git a/Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt b/Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt
> deleted file mode 100644
> index aeba38f0a387..000000000000
> --- a/Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt
> +++ /dev/null
> @@ -1,84 +0,0 @@
> -Renesas AHB to PCI bridge
> --------------------------
> -
> -This is the bridge used internally to connect the USB controllers to the
> -AHB. There is one bridge instance per USB port connected to the internal
> -OHCI and EHCI controllers.
> -
> -Required properties:
> -- compatible: "renesas,pci-r8a7742" for the R8A7742 SoC;
> -	      "renesas,pci-r8a7743" for the R8A7743 SoC;
> -	      "renesas,pci-r8a7744" for the R8A7744 SoC;
> -	      "renesas,pci-r8a7745" for the R8A7745 SoC;
> -	      "renesas,pci-r8a7790" for the R8A7790 SoC;
> -	      "renesas,pci-r8a7791" for the R8A7791 SoC;
> -	      "renesas,pci-r8a7793" for the R8A7793 SoC;
> -	      "renesas,pci-r8a7794" for the R8A7794 SoC;
> -	      "renesas,pci-rcar-gen2" for a generic R-Car Gen2 or
> -				      RZ/G1 compatible device.
> -
> -
> -	      When compatible with the generic version, nodes must list the
> -	      SoC-specific version corresponding to the platform first
> -	      followed by the generic version.
> -
> -- reg:	A list of physical regions to access the device: the first is
> -	the operational registers for the OHCI/EHCI controllers and the
> -	second is for the bridge configuration and control registers.
> -- interrupts: interrupt for the device.
> -- clocks: The reference to the device clock.
> -- bus-range: The PCI bus number range; as this is a single bus, the range
> -	     should be specified as the same value twice.
> -- #address-cells: must be 3.
> -- #size-cells: must be 2.
> -- #interrupt-cells: must be 1.
> -- interrupt-map: standard property used to define the mapping of the PCI
> -  interrupts to the GIC interrupts.
> -- interrupt-map-mask: standard property that helps to define the interrupt
> -  mapping.
> -
> -Optional properties:
> -- dma-ranges: a single range for the inbound memory region. If not supplied,
> -  defaults to 1GiB at 0x40000000. Note there are hardware restrictions on the
> -  allowed combinations of address and size.
> -
> -Example SoC configuration:
> -
> -	pci0: pci@ee090000  {
> -		compatible = "renesas,pci-r8a7790", "renesas,pci-rcar-gen2";
> -		clocks = <&mstp7_clks R8A7790_CLK_EHCI>;
> -		reg = <0x0 0xee090000 0x0 0xc00>,
> -		      <0x0 0xee080000 0x0 0x1100>;
> -		interrupts = <0 108 IRQ_TYPE_LEVEL_HIGH>;
> -		status = "disabled";
> -
> -		bus-range = <0 0>;
> -		#address-cells = <3>;
> -		#size-cells = <2>;
> -		#interrupt-cells = <1>;
> -		dma-ranges = <0x42000000 0 0x40000000 0 0x40000000 0 0x40000000>;
> -		interrupt-map-mask = <0xff00 0 0 0x7>;
> -		interrupt-map = <0x0000 0 0 1 &gic 0 108 IRQ_TYPE_LEVEL_HIGH
> -				 0x0800 0 0 1 &gic 0 108 IRQ_TYPE_LEVEL_HIGH
> -				 0x1000 0 0 2 &gic 0 108 IRQ_TYPE_LEVEL_HIGH>;
> -
> -		usb@1,0 {
> -			reg = <0x800 0 0 0 0>;
> -			phys = <&usb0 0>;
> -			phy-names = "usb";
> -		};
> -
> -		usb@2,0 {
> -			reg = <0x1000 0 0 0 0>;
> -			phys = <&usb0 0>;
> -			phy-names = "usb";
> -		};
> -	};
> -
> -Example board setup:
> -
> -&pci0 {
> -	status = "okay";
> -	pinctrl-0 = <&usb0_pins>;
> -	pinctrl-names = "default";
> -};
> diff --git a/Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml b/Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml
> new file mode 100644
> index 000000000000..3f8d79b746c7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml
> @@ -0,0 +1,134 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/renesas,pci-usb.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas AHB to PCI bridge
> +
> +maintainers:
> +  - Marek Vasut <marek.vasut+renesas@gmail.com>
> +  - Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> +
> +description: |
> +  This is the bridge used internally to connect the USB controllers to the
> +  AHB. There is one bridge instance per USB port connected to the internal
> +  OHCI and EHCI controllers.
> +
> +allOf:
> +  - $ref: /schemas/pci/pci-bus.yaml#
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - items:
> +          - enum:
> +              - renesas,pci-r8a7742      # RZ/G1H
> +              - renesas,pci-r8a7743      # RZ/G1M
> +              - renesas,pci-r8a7744      # RZ/G1N
> +              - renesas,pci-r8a7745      # RZ/G1E
> +              - renesas,pci-r8a7790      # R-Car H2
> +              - renesas,pci-r8a7791      # R-Car M2-W
> +              - renesas,pci-r8a7793      # R-Car M2-N
> +              - renesas,pci-r8a7794      # R-Car E2
> +          - const: renesas,pci-rcar-gen2 # R-Car Gen2 and RZ/G1
> +
> +  reg:
> +    description: |
> +      A list of physical regions to access the device. The first is
> +      the operational registers for the OHCI/EHCI controllers and the
> +      second is for the bridge configuration and control registers.
> +    minItems: 2
> +    maxItems: 2
> +
> +  interrupts:
> +    description: Interrupt for the device.
> +
> +  interrupt-map:
> +    description: |
> +      Standard property used to define the mapping of the PCI interrupts
> +      to the GIC interrupts.
> +
> +  interrupt-map-mask:
> +    description:
> +      Standard property that helps to define the interrupt mapping.
> +
> +  clocks:
> +    description: The reference to the device clock.
> +
> +  bus-range:
> +    description: |
> +      The PCI bus number range; as this is a single bus, the range
> +      should be specified as the same value twice.

items:
  const: 0

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

All these are defined by pci-bus.yaml

> +
> +  dma-ranges:
> +    description: |
> +      A single range for the inbound memory region. If not supplied,
> +      defaults to 1GiB at 0x40000000. Note there are hardware restrictions on
> +      the allowed combinations of address and size.

'a single range' == 'maxItems: 1'
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - interrupt-map
> +  - interrupt-map-mask
> +  - clocks
> +  - bus-range
> +  - "#address-cells"
> +  - "#size-cells"
> +  - "#interrupt-cells"
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/r8a7790-cpg-mssr.h>
> +
> +    bus {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        pci0: pci@ee090000  {
> +            compatible = "renesas,pci-r8a7790", "renesas,pci-rcar-gen2";
> +            device_type = "pci";
> +            clocks = <&cpg CPG_MOD 703>;
> +            reg = <0 0xee090000 0 0xc00>,
> +                  <0 0xee080000 0 0x1100>;
> +            interrupts = <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>;
> +            status = "disabled";

Don't disable your example.

> +
> +            bus-range = <0 0>;
> +            #address-cells = <3>;
> +            #size-cells = <2>;
> +            #interrupt-cells = <1>;
> +            ranges = <0x02000000 0 0xee080000 0 0xee080000 0 0x00010000>;
> +            dma-ranges = <0x42000000 0 0x40000000 0 0x40000000 0 0x40000000>;
> +            interrupt-map-mask = <0xf800 0 0 0x7>;
> +            interrupt-map = <0x0000 0 0 1 &gic GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>,
> +                            <0x0800 0 0 1 &gic GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>,
> +                            <0x1000 0 0 2 &gic GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>;
> +
> +            usb@1,0 {
> +                reg = <0x800 0 0 0 0>;
> +                phys = <&usb0 0>;
> +                phy-names = "usb";
> +            };
> +            
> +            usb@2,0 {
> +                reg = <0x1000 0 0 0 0>;
> +                phys = <&usb0 0>;
> +                phy-names = "usb";
> +            };
> +        };
> +    };
> -- 
> 2.35.1
> 
>
Herve Codina April 19, 2022, 2:41 p.m. UTC | #4
Hi Geert,

On Thu, 14 Apr 2022 10:28:47 +0200
Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> Hi Hervé,
> 
> On Thu, Apr 14, 2022 at 9:40 AM Herve Codina <herve.codina@bootlin.com> wrote:
> > Convert Renesas PCI bridge bindings documentation to json-schema.
> > Also name it 'renesas,pci-usb' as it is specifically used to
> > connect the PCI USB controllers to AHB bus.
> >
> > Signed-off-by: Herve Codina <herve.codina@bootlin.com>  
> 
> Thanks a lot for tackling this DT binding file!
> 
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml
> > @@ -0,0 +1,134 @@
> > +# SPDX-License-Identifier: GPL-2.0  
> 
> scripts/checkpatch.pl says:
> WARNING: DT binding documents should be licensed (GPL-2.0-only OR BSD-2-Clause)

Right, changed to "GPL-2.0-only OR BSD-2-Clause"

> 
> > +  reg:
> > +    description: |
> > +      A list of physical regions to access the device. The first is
> > +      the operational registers for the OHCI/EHCI controllers and the
> > +      second is for the bridge configuration and control registers.
> > +    minItems: 2
> > +    maxItems: 2  
> 
> reg:
>   items:
>     - description: Operational registers for the OHCI/EHCI controllers.
>     - description: Bridge configuration and control registers.

Ok, changed.

> 
> > +
> > +  interrupts:
> > +    description: Interrupt for the device.  
> 
> maxItems: 1
> 
> The description is not needed.

Ok, changed.

> 
> > +
> > +  interrupt-map:
> > +    description: |
> > +      Standard property used to define the mapping of the PCI interrupts
> > +      to the GIC interrupts.
> > +
> > +  interrupt-map-mask:
> > +    description:
> > +      Standard property that helps to define the interrupt mapping.
> > +
> > +  clocks:
> > +    description: The reference to the device clock.  
> 
> maxItems: 1
> 
> The description is not needed.

Ok, changed

> 
> Missing "resets" and "power-domains" properties.
> 
> Missing description of the child nodes.

"resets", "power-domains" dans child nodes added

> 
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - interrupt-map
> > +  - interrupt-map-mask
> > +  - clocks  
> 
> Missing "resets" and "power-domains".

Added

> 
> > +  - bus-range
> > +  - "#address-cells"
> > +  - "#size-cells"
> > +  - "#interrupt-cells"
> > +
> > +unevaluatedProperties: false  
> 
> Why doesn't "make dtbs_check" complain about the presence of
> e.g. "resets" in the actual DTS files?
> 
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    #include <dt-bindings/clock/r8a7790-cpg-mssr.h>
> > +
> > +    bus {
> > +        #address-cells = <2>;
> > +        #size-cells = <2>;  
> 
> I think you should drop this (and the corresponding high addresses
> below).
> 

Ok

> > +
> > +        pci0: pci@ee090000  {
> > +            compatible = "renesas,pci-r8a7790", "renesas,pci-rcar-gen2";
> > +            device_type = "pci";
> > +            clocks = <&cpg CPG_MOD 703>;
> > +            reg = <0 0xee090000 0 0xc00>,
> > +                  <0 0xee080000 0 0x1100>;
> > +            interrupts = <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>;  
> 
>                         power-domains = <&sysc R8A7790_PD_ALWAYS_ON>;
>                         resets = <&cpg 703>;

Ok

> 
> > +            status = "disabled";
> > +
> > +            bus-range = <0 0>;
> > +            #address-cells = <3>;
> > +            #size-cells = <2>;
> > +            #interrupt-cells = <1>;
> > +            ranges = <0x02000000 0 0xee080000 0 0xee080000 0 0x00010000>;
> > +            dma-ranges = <0x42000000 0 0x40000000 0 0x40000000 0 0x40000000>;
> > +            interrupt-map-mask = <0xf800 0 0 0x7>;
> > +            interrupt-map = <0x0000 0 0 1 &gic GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>,
> > +                            <0x0800 0 0 1 &gic GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>,
> > +                            <0x1000 0 0 2 &gic GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>;
> > +
> > +            usb@1,0 {
> > +                reg = <0x800 0 0 0 0>;
> > +                phys = <&usb0 0>;
> > +                phy-names = "usb";
> > +            };
> > +  
> 
> ERROR: trailing whitespace
> #249: FILE: Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml:127:
> +            $

Ok

> 
> > +            usb@2,0 {
> > +                reg = <0x1000 0 0 0 0>;
> > +                phys = <&usb0 0>;
> > +                phy-names = "usb";
> > +            };
> > +        };
> > +    };  
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

Thanks for the review,
Hervé
Herve Codina April 20, 2022, 12:44 p.m. UTC | #5
Hi Rob,

On Thu, 14 Apr 2022 13:15:30 -0500
Rob Herring <robh@kernel.org> wrote:

> On Thu, Apr 14, 2022 at 09:40:05AM +0200, Herve Codina wrote:
> > Convert Renesas PCI bridge bindings documentation to json-schema.
> > Also name it 'renesas,pci-usb' as it is specifically used to
> > connect the PCI USB controllers to AHB bus.  
> 
> Please name it based on compatible strings. renesas,pci-rcar-gen2.yaml

Ok, renamed.

> 
> > 
> > Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> > ---
> >  .../devicetree/bindings/pci/pci-rcar-gen2.txt |  84 -----------
> >  .../bindings/pci/renesas,pci-usb.yaml         | 134 ++++++++++++++++++
> >  2 files changed, 134 insertions(+), 84 deletions(-)
> >  delete mode 100644 Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt
> >  create mode 100644 Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt b/Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt
> > deleted file mode 100644
...
> > diff --git a/Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml b/Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml
> > new file mode 100644
...
> > index 000000000000..3f8d79b746c7
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml
> > @@ -0,0 +1,134 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/pci/renesas,pci-usb.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Renesas AHB to PCI bridge
> > +
> > +maintainers:
> > +  - Marek Vasut <marek.vasut+renesas@gmail.com>
> > +  - Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > +
> > +description: |
> > +  This is the bridge used internally to connect the USB controllers to the
> > +  AHB. There is one bridge instance per USB port connected to the internal
> > +  OHCI and EHCI controllers.
> > +
> > +allOf:
> > +  - $ref: /schemas/pci/pci-bus.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    oneOf:
> > +      - items:
> > +          - enum:
> > +              - renesas,pci-r8a7742      # RZ/G1H
> > +              - renesas,pci-r8a7743      # RZ/G1M
> > +              - renesas,pci-r8a7744      # RZ/G1N
> > +              - renesas,pci-r8a7745      # RZ/G1E
> > +              - renesas,pci-r8a7790      # R-Car H2
> > +              - renesas,pci-r8a7791      # R-Car M2-W
> > +              - renesas,pci-r8a7793      # R-Car M2-N
> > +              - renesas,pci-r8a7794      # R-Car E2
> > +          - const: renesas,pci-rcar-gen2 # R-Car Gen2 and RZ/G1
> > +
> > +  reg:
> > +    description: |
> > +      A list of physical regions to access the device. The first is
> > +      the operational registers for the OHCI/EHCI controllers and the
> > +      second is for the bridge configuration and control registers.
> > +    minItems: 2
> > +    maxItems: 2
> > +
> > +  interrupts:
> > +    description: Interrupt for the device.
> > +
> > +  interrupt-map:
> > +    description: |
> > +      Standard property used to define the mapping of the PCI interrupts
> > +      to the GIC interrupts.
> > +
> > +  interrupt-map-mask:
> > +    description:
> > +      Standard property that helps to define the interrupt mapping.
> > +
> > +  clocks:
> > +    description: The reference to the device clock.
> > +
> > +  bus-range:
> > +    description: |
> > +      The PCI bus number range; as this is a single bus, the range
> > +      should be specified as the same value twice.  
> 
> items:
>   const: 0

Well, some other values are present in some dtsi files such as
'bus_range = <1 1>;' or 'bus_range = <2 2>;' in r8a7742.dtsi.

The constraint is to have the same value twice. Is there a way
to specify this constraint ?

> 
> > +
> > +  "#address-cells":
> > +    const: 3
> > +
> > +  "#size-cells":
> > +    const: 2
> > +
> > +  "#interrupt-cells":
> > +    const: 1  
> 
> All these are defined by pci-bus.yaml

Right.
Replaced by:

"#address-cells": true
"#size-cells": true
"#interrupt-cells": true

Is that correct ?

> 
> > +
> > +  dma-ranges:
> > +    description: |
> > +      A single range for the inbound memory region. If not supplied,
> > +      defaults to 1GiB at 0x40000000. Note there are hardware restrictions on
> > +      the allowed combinations of address and size.  
> 
> 'a single range' == 'maxItems: 1'

Ok, maxItems added.

> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - interrupt-map
> > +  - interrupt-map-mask
> > +  - clocks
> > +  - bus-range
> > +  - "#address-cells"
> > +  - "#size-cells"
> > +  - "#interrupt-cells"
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    #include <dt-bindings/clock/r8a7790-cpg-mssr.h>
> > +
> > +    bus {
> > +        #address-cells = <2>;
> > +        #size-cells = <2>;
> > +
> > +        pci0: pci@ee090000  {
> > +            compatible = "renesas,pci-r8a7790", "renesas,pci-rcar-gen2";
> > +            device_type = "pci";
> > +            clocks = <&cpg CPG_MOD 703>;
> > +            reg = <0 0xee090000 0 0xc00>,
> > +                  <0 0xee080000 0 0x1100>;
> > +            interrupts = <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>;
> > +            status = "disabled";  
> 
> Don't disable your example.

Ok, done


Thanks for the review.
Hervé
Rob Herring (Arm) April 20, 2022, 1:18 p.m. UTC | #6
On Wed, Apr 20, 2022 at 02:44:11PM +0200, Herve Codina wrote:
> Hi Rob,
> 
> On Thu, 14 Apr 2022 13:15:30 -0500
> Rob Herring <robh@kernel.org> wrote:
> 
> > On Thu, Apr 14, 2022 at 09:40:05AM +0200, Herve Codina wrote:
> > > Convert Renesas PCI bridge bindings documentation to json-schema.
> > > Also name it 'renesas,pci-usb' as it is specifically used to
> > > connect the PCI USB controllers to AHB bus.  
> > 
> > Please name it based on compatible strings. renesas,pci-rcar-gen2.yaml
> 
> Ok, renamed.
> 
> > 
> > > 
> > > Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> > > ---
> > >  .../devicetree/bindings/pci/pci-rcar-gen2.txt |  84 -----------
> > >  .../bindings/pci/renesas,pci-usb.yaml         | 134 ++++++++++++++++++
> > >  2 files changed, 134 insertions(+), 84 deletions(-)
> > >  delete mode 100644 Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt
> > >  create mode 100644 Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml
> > > 
> > > diff --git a/Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt b/Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt
> > > deleted file mode 100644
> ...
> > > diff --git a/Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml b/Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml
> > > new file mode 100644
> ...
> > > index 000000000000..3f8d79b746c7
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml
> > > @@ -0,0 +1,134 @@
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/pci/renesas,pci-usb.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Renesas AHB to PCI bridge
> > > +
> > > +maintainers:
> > > +  - Marek Vasut <marek.vasut+renesas@gmail.com>
> > > +  - Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > > +
> > > +description: |
> > > +  This is the bridge used internally to connect the USB controllers to the
> > > +  AHB. There is one bridge instance per USB port connected to the internal
> > > +  OHCI and EHCI controllers.
> > > +
> > > +allOf:
> > > +  - $ref: /schemas/pci/pci-bus.yaml#
> > > +
> > > +properties:
> > > +  compatible:
> > > +    oneOf:
> > > +      - items:
> > > +          - enum:
> > > +              - renesas,pci-r8a7742      # RZ/G1H
> > > +              - renesas,pci-r8a7743      # RZ/G1M
> > > +              - renesas,pci-r8a7744      # RZ/G1N
> > > +              - renesas,pci-r8a7745      # RZ/G1E
> > > +              - renesas,pci-r8a7790      # R-Car H2
> > > +              - renesas,pci-r8a7791      # R-Car M2-W
> > > +              - renesas,pci-r8a7793      # R-Car M2-N
> > > +              - renesas,pci-r8a7794      # R-Car E2
> > > +          - const: renesas,pci-rcar-gen2 # R-Car Gen2 and RZ/G1
> > > +
> > > +  reg:
> > > +    description: |
> > > +      A list of physical regions to access the device. The first is
> > > +      the operational registers for the OHCI/EHCI controllers and the
> > > +      second is for the bridge configuration and control registers.
> > > +    minItems: 2
> > > +    maxItems: 2
> > > +
> > > +  interrupts:
> > > +    description: Interrupt for the device.
> > > +
> > > +  interrupt-map:
> > > +    description: |
> > > +      Standard property used to define the mapping of the PCI interrupts
> > > +      to the GIC interrupts.
> > > +
> > > +  interrupt-map-mask:
> > > +    description:
> > > +      Standard property that helps to define the interrupt mapping.
> > > +
> > > +  clocks:
> > > +    description: The reference to the device clock.
> > > +
> > > +  bus-range:
> > > +    description: |
> > > +      The PCI bus number range; as this is a single bus, the range
> > > +      should be specified as the same value twice.  
> > 
> > items:
> >   const: 0
> 
> Well, some other values are present in some dtsi files such as
> 'bus_range = <1 1>;' or 'bus_range = <2 2>;' in r8a7742.dtsi.
> 
> The constraint is to have the same value twice. Is there a way
> to specify this constraint ?

Yes, but probably not worthwhile. Just drop it as pci-bus.yaml already 
defines it.

> > > +
> > > +  "#address-cells":
> > > +    const: 3
> > > +
> > > +  "#size-cells":
> > > +    const: 2
> > > +
> > > +  "#interrupt-cells":
> > > +    const: 1  
> > 
> > All these are defined by pci-bus.yaml
> 
> Right.
> Replaced by:
> 
> "#address-cells": true
> "#size-cells": true
> "#interrupt-cells": true
> 
> Is that correct ?

You can just drop them completely.

Rob
Herve Codina April 20, 2022, 1:46 p.m. UTC | #7
Hi Rob,

On Wed, 20 Apr 2022 08:18:50 -0500
Rob Herring <robh@kernel.org> wrote:

...

> > > > +  bus-range:
> > > > +    description: |
> > > > +      The PCI bus number range; as this is a single bus, the range
> > > > +      should be specified as the same value twice.    
> > > 
> > > items:
> > >   const: 0  
> > 
> > Well, some other values are present in some dtsi files such as
> > 'bus_range = <1 1>;' or 'bus_range = <2 2>;' in r8a7742.dtsi.
> > 
> > The constraint is to have the same value twice. Is there a way
> > to specify this constraint ?  
> 
> Yes, but probably not worthwhile. Just drop it as pci-bus.yaml already 
> defines it.

Instead of fully dropping the property, don't you think that keeping
the given description here can be a way to express that the same value
is needed twice ?

> 
> > > > +
> > > > +  "#address-cells":
> > > > +    const: 3
> > > > +
> > > > +  "#size-cells":
> > > > +    const: 2
> > > > +
> > > > +  "#interrupt-cells":
> > > > +    const: 1    
> > > 
> > > All these are defined by pci-bus.yaml  
> > 
> > Right.
> > Replaced by:
> > 
> > "#address-cells": true
> > "#size-cells": true
> > "#interrupt-cells": true
> > 
> > Is that correct ?  
> 
> You can just drop them completely.

Ok for #address-cells and #size-cells but not for #interrupt-cells.

Dropping #interrupt-cells makes 'make dtbindings_check' unhappy:
--- 8< ---
$ make dt_binding_check DT_SCHEMA_FILES=renesas,pci-rcar-gen2.yaml
  LINT    Documentation/devicetree/bindings
  CHKDT   Documentation/devicetree/bindings/processed-schema.json
/home/hcodina/xxx/Documentation/devicetree/bindings/pci/renesas,pci-rcar-gen2.yaml: properties: '#interrupt-cells' is a dependency of 'interrupt-map'
	from schema $id: http://devicetree.org/meta-schemas/interrupts.yaml#
  SCHEMA  Documentation/devicetree/bindings/processed-schema.json
/home/hcodina/xxx/Documentation/devicetree/bindings/pci/renesas,pci-rcar-gen2.yaml: ignoring, error in schema: properties
  DTEX    Documentation/devicetree/bindings/pci/renesas,pci-rcar-gen2.example.dts
  DTC     Documentation/devicetree/bindings/pci/renesas,pci-rcar-gen2.example.dtb
  CHECK   Documentation/devicetree/bindings/pci/renesas,pci-rcar-gen2.example.dtb
$ 
--- 8< ---

So I keep 
"#interrupt-cells": true

Regards,
Hervé
Rob Herring (Arm) April 20, 2022, 9:37 p.m. UTC | #8
On Wed, Apr 20, 2022 at 03:46:11PM +0200, Herve Codina wrote:
> Hi Rob,
> 
> On Wed, 20 Apr 2022 08:18:50 -0500
> Rob Herring <robh@kernel.org> wrote:
> 
> ...
> 
> > > > > +  bus-range:
> > > > > +    description: |
> > > > > +      The PCI bus number range; as this is a single bus, the range
> > > > > +      should be specified as the same value twice.    
> > > > 
> > > > items:
> > > >   const: 0  
> > > 
> > > Well, some other values are present in some dtsi files such as
> > > 'bus_range = <1 1>;' or 'bus_range = <2 2>;' in r8a7742.dtsi.
> > > 
> > > The constraint is to have the same value twice. Is there a way
> > > to specify this constraint ?  
> > 
> > Yes, but probably not worthwhile. Just drop it as pci-bus.yaml already 
> > defines it.
> 
> Instead of fully dropping the property, don't you think that keeping
> the given description here can be a way to express that the same value
> is needed twice ?

Yeah, that's fine.


> > > > > +  "#address-cells":
> > > > > +    const: 3
> > > > > +
> > > > > +  "#size-cells":
> > > > > +    const: 2
> > > > > +
> > > > > +  "#interrupt-cells":
> > > > > +    const: 1    
> > > > 
> > > > All these are defined by pci-bus.yaml  
> > > 
> > > Right.
> > > Replaced by:
> > > 
> > > "#address-cells": true
> > > "#size-cells": true
> > > "#interrupt-cells": true
> > > 
> > > Is that correct ?  
> > 
> > You can just drop them completely.
> 
> Ok for #address-cells and #size-cells but not for #interrupt-cells.
> 
> Dropping #interrupt-cells makes 'make dtbindings_check' unhappy:
> --- 8< ---
> $ make dt_binding_check DT_SCHEMA_FILES=renesas,pci-rcar-gen2.yaml
>   LINT    Documentation/devicetree/bindings
>   CHKDT   Documentation/devicetree/bindings/processed-schema.json
> /home/hcodina/xxx/Documentation/devicetree/bindings/pci/renesas,pci-rcar-gen2.yaml: properties: '#interrupt-cells' is a dependency of 'interrupt-map'
> 	from schema $id: http://devicetree.org/meta-schemas/interrupts.yaml#
>   SCHEMA  Documentation/devicetree/bindings/processed-schema.json
> /home/hcodina/xxx/Documentation/devicetree/bindings/pci/renesas,pci-rcar-gen2.yaml: ignoring, error in schema: properties
>   DTEX    Documentation/devicetree/bindings/pci/renesas,pci-rcar-gen2.example.dts
>   DTC     Documentation/devicetree/bindings/pci/renesas,pci-rcar-gen2.example.dtb
>   CHECK   Documentation/devicetree/bindings/pci/renesas,pci-rcar-gen2.example.dtb
> $ 
> --- 8< ---
> 
> So I keep 
> "#interrupt-cells": true

You should also drop 'interrupt-map' and 'interrupt-map-mask'.

Rob
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt b/Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt
deleted file mode 100644
index aeba38f0a387..000000000000
--- a/Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt
+++ /dev/null
@@ -1,84 +0,0 @@ 
-Renesas AHB to PCI bridge
--------------------------
-
-This is the bridge used internally to connect the USB controllers to the
-AHB. There is one bridge instance per USB port connected to the internal
-OHCI and EHCI controllers.
-
-Required properties:
-- compatible: "renesas,pci-r8a7742" for the R8A7742 SoC;
-	      "renesas,pci-r8a7743" for the R8A7743 SoC;
-	      "renesas,pci-r8a7744" for the R8A7744 SoC;
-	      "renesas,pci-r8a7745" for the R8A7745 SoC;
-	      "renesas,pci-r8a7790" for the R8A7790 SoC;
-	      "renesas,pci-r8a7791" for the R8A7791 SoC;
-	      "renesas,pci-r8a7793" for the R8A7793 SoC;
-	      "renesas,pci-r8a7794" for the R8A7794 SoC;
-	      "renesas,pci-rcar-gen2" for a generic R-Car Gen2 or
-				      RZ/G1 compatible device.
-
-
-	      When compatible with the generic version, nodes must list the
-	      SoC-specific version corresponding to the platform first
-	      followed by the generic version.
-
-- reg:	A list of physical regions to access the device: the first is
-	the operational registers for the OHCI/EHCI controllers and the
-	second is for the bridge configuration and control registers.
-- interrupts: interrupt for the device.
-- clocks: The reference to the device clock.
-- bus-range: The PCI bus number range; as this is a single bus, the range
-	     should be specified as the same value twice.
-- #address-cells: must be 3.
-- #size-cells: must be 2.
-- #interrupt-cells: must be 1.
-- interrupt-map: standard property used to define the mapping of the PCI
-  interrupts to the GIC interrupts.
-- interrupt-map-mask: standard property that helps to define the interrupt
-  mapping.
-
-Optional properties:
-- dma-ranges: a single range for the inbound memory region. If not supplied,
-  defaults to 1GiB at 0x40000000. Note there are hardware restrictions on the
-  allowed combinations of address and size.
-
-Example SoC configuration:
-
-	pci0: pci@ee090000  {
-		compatible = "renesas,pci-r8a7790", "renesas,pci-rcar-gen2";
-		clocks = <&mstp7_clks R8A7790_CLK_EHCI>;
-		reg = <0x0 0xee090000 0x0 0xc00>,
-		      <0x0 0xee080000 0x0 0x1100>;
-		interrupts = <0 108 IRQ_TYPE_LEVEL_HIGH>;
-		status = "disabled";
-
-		bus-range = <0 0>;
-		#address-cells = <3>;
-		#size-cells = <2>;
-		#interrupt-cells = <1>;
-		dma-ranges = <0x42000000 0 0x40000000 0 0x40000000 0 0x40000000>;
-		interrupt-map-mask = <0xff00 0 0 0x7>;
-		interrupt-map = <0x0000 0 0 1 &gic 0 108 IRQ_TYPE_LEVEL_HIGH
-				 0x0800 0 0 1 &gic 0 108 IRQ_TYPE_LEVEL_HIGH
-				 0x1000 0 0 2 &gic 0 108 IRQ_TYPE_LEVEL_HIGH>;
-
-		usb@1,0 {
-			reg = <0x800 0 0 0 0>;
-			phys = <&usb0 0>;
-			phy-names = "usb";
-		};
-
-		usb@2,0 {
-			reg = <0x1000 0 0 0 0>;
-			phys = <&usb0 0>;
-			phy-names = "usb";
-		};
-	};
-
-Example board setup:
-
-&pci0 {
-	status = "okay";
-	pinctrl-0 = <&usb0_pins>;
-	pinctrl-names = "default";
-};
diff --git a/Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml b/Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml
new file mode 100644
index 000000000000..3f8d79b746c7
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml
@@ -0,0 +1,134 @@ 
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/renesas,pci-usb.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas AHB to PCI bridge
+
+maintainers:
+  - Marek Vasut <marek.vasut+renesas@gmail.com>
+  - Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
+
+description: |
+  This is the bridge used internally to connect the USB controllers to the
+  AHB. There is one bridge instance per USB port connected to the internal
+  OHCI and EHCI controllers.
+
+allOf:
+  - $ref: /schemas/pci/pci-bus.yaml#
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+          - enum:
+              - renesas,pci-r8a7742      # RZ/G1H
+              - renesas,pci-r8a7743      # RZ/G1M
+              - renesas,pci-r8a7744      # RZ/G1N
+              - renesas,pci-r8a7745      # RZ/G1E
+              - renesas,pci-r8a7790      # R-Car H2
+              - renesas,pci-r8a7791      # R-Car M2-W
+              - renesas,pci-r8a7793      # R-Car M2-N
+              - renesas,pci-r8a7794      # R-Car E2
+          - const: renesas,pci-rcar-gen2 # R-Car Gen2 and RZ/G1
+
+  reg:
+    description: |
+      A list of physical regions to access the device. The first is
+      the operational registers for the OHCI/EHCI controllers and the
+      second is for the bridge configuration and control registers.
+    minItems: 2
+    maxItems: 2
+
+  interrupts:
+    description: Interrupt for the device.
+
+  interrupt-map:
+    description: |
+      Standard property used to define the mapping of the PCI interrupts
+      to the GIC interrupts.
+
+  interrupt-map-mask:
+    description:
+      Standard property that helps to define the interrupt mapping.
+
+  clocks:
+    description: The reference to the device clock.
+
+  bus-range:
+    description: |
+      The PCI bus number range; as this is a single bus, the range
+      should be specified as the same value twice.
+
+  "#address-cells":
+    const: 3
+
+  "#size-cells":
+    const: 2
+
+  "#interrupt-cells":
+    const: 1
+
+  dma-ranges:
+    description: |
+      A single range for the inbound memory region. If not supplied,
+      defaults to 1GiB at 0x40000000. Note there are hardware restrictions on
+      the allowed combinations of address and size.
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - interrupt-map
+  - interrupt-map-mask
+  - clocks
+  - bus-range
+  - "#address-cells"
+  - "#size-cells"
+  - "#interrupt-cells"
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/r8a7790-cpg-mssr.h>
+
+    bus {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        pci0: pci@ee090000  {
+            compatible = "renesas,pci-r8a7790", "renesas,pci-rcar-gen2";
+            device_type = "pci";
+            clocks = <&cpg CPG_MOD 703>;
+            reg = <0 0xee090000 0 0xc00>,
+                  <0 0xee080000 0 0x1100>;
+            interrupts = <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>;
+            status = "disabled";
+
+            bus-range = <0 0>;
+            #address-cells = <3>;
+            #size-cells = <2>;
+            #interrupt-cells = <1>;
+            ranges = <0x02000000 0 0xee080000 0 0xee080000 0 0x00010000>;
+            dma-ranges = <0x42000000 0 0x40000000 0 0x40000000 0 0x40000000>;
+            interrupt-map-mask = <0xf800 0 0 0x7>;
+            interrupt-map = <0x0000 0 0 1 &gic GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>,
+                            <0x0800 0 0 1 &gic GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>,
+                            <0x1000 0 0 2 &gic GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>;
+
+            usb@1,0 {
+                reg = <0x800 0 0 0 0>;
+                phys = <&usb0 0>;
+                phy-names = "usb";
+            };
+            
+            usb@2,0 {
+                reg = <0x1000 0 0 0 0>;
+                phys = <&usb0 0>;
+                phy-names = "usb";
+            };
+        };
+    };