[PATCH v2 0/8] RZN1 USB Host support

[PATCH v2 0/8] RZN1 USB Host support

[Herve Codina]

Hi,

This series add support for the USB Host controllers available on
RZN1 (r9a06g032) SOC.

These USB Host controllers are PCI OHCI/EHCI controllers located
behind a bridge.

Regards,
Herve

Changes v2:
- Convert bindings to json-schema
- Update clocks description
- Remove unneeded '.compatible = "renesas,pci-r9a06g032"'

Herve Codina (8):
  PCI: rcar-gen2: Add support for clocks
  dt-bindings: PCI: renesas-pci-usb: Convert bindings to json-schema
  dt-bindings: PCI: renesas-pci-usb: Allow multiple clocks
  dt-bindings: PCI: renesas-pci-usb: Add device tree support for
    r9a06g032
  PCI: rcar-gen2: Add R9A06G032 support
  ARM: dts: r9a06g032: Add internal PCI bridge node
  ARM: dts: r9a06g032: Add USB PHY DT support
  ARM: dts: r9a06g032: Link the PCI USB devices to the USB PHY

 .../devicetree/bindings/pci/pci-rcar-gen2.txt |  84 -----------
 .../bindings/pci/renesas,pci-usb.yaml         | 139 ++++++++++++++++++
 arch/arm/boot/dts/r9a06g032.dtsi              |  46 ++++++
 drivers/pci/controller/pci-rcar-gen2.c        |  29 +++-
 4 files changed, 212 insertions(+), 86 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt
 create mode 100644 Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml

-- 
2.35.1

[PATCH v2 1/8] PCI: rcar-gen2: Add support for clocks

[Herve Codina]

The PCI rcar-gen2 does not call any clk_prepare_enable().
This lead to an access failure when the driver tries to access
the IP (at least on a RZ/N1D platform).

Prepare and enable clocks using the bulk version of
clk_prepare_enable() in order to prepare and enable all clocks
attached to this device.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 drivers/pci/controller/pci-rcar-gen2.c | 28 ++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/pci-rcar-gen2.c b/drivers/pci/controller/pci-rcar-gen2.c
index 35804ea394fd..528bc3780e01 100644
--- a/drivers/pci/controller/pci-rcar-gen2.c
+++ b/drivers/pci/controller/pci-rcar-gen2.c
@@ -8,6 +8,7 @@
  * Author: Valentine Barshak <valentine.barshak@cogentembedded.com>
  */
 
+#include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
@@ -99,6 +100,8 @@ struct rcar_pci {
 	struct resource mem_res;
 	struct resource *cfg_res;
 	int irq;
+	struct clk_bulk_data *clocks;
+	int nclocks;
 };
 
 /* PCI configuration space operations */
@@ -282,6 +285,7 @@ static int rcar_pci_probe(struct platform_device *pdev)
 	struct rcar_pci *priv;
 	struct pci_host_bridge *bridge;
 	void __iomem *reg;
+	int ret;
 
 	bridge = devm_pci_alloc_host_bridge(dev, sizeof(*priv));
 	if (!bridge)
@@ -305,13 +309,25 @@ static int rcar_pci_probe(struct platform_device *pdev)
 	priv->mem_res = *mem_res;
 	priv->cfg_res = cfg_res;
 
+	ret = devm_clk_bulk_get_all(dev, &priv->clocks);
+	if (ret < 0) {
+		dev_err(dev, "failed to get clocks %d\n", ret);
+		return ret;
+	}
+	priv->nclocks = ret;
+
+	ret = clk_bulk_prepare_enable(priv->nclocks, priv->clocks);
+	if (ret)
+		return ret;
+
 	priv->irq = platform_get_irq(pdev, 0);
 	priv->reg = reg;
 	priv->dev = dev;
 
 	if (priv->irq < 0) {
 		dev_err(dev, "no valid irq found\n");
-		return priv->irq;
+		ret = priv->irq;
+		goto disable_clocks;
 	}
 
 	bridge->ops = &rcar_pci_ops;
@@ -320,7 +336,15 @@ static int rcar_pci_probe(struct platform_device *pdev)
 
 	rcar_pci_setup(priv);
 
-	return pci_host_probe(bridge);
+	ret = pci_host_probe(bridge);
+	if (ret < 0)
+		goto disable_clocks;
+
+	return 0;
+
+disable_clocks:
+	clk_bulk_disable_unprepare(priv->nclocks, priv->clocks);
+	return ret;
 }
 
 static const struct of_device_id rcar_pci_of_match[] = {
-- 
2.35.1

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

[Herve Codina]

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.
+
+  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";
+            };
+        };
+    };
-- 
2.35.1

[PATCH v2 3/8] dt-bindings: PCI: renesas-pci-usb: Allow multiple clocks

[Herve Codina]

Define that multiple clocks can be present at clocks property.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml b/Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml
index 3f8d79b746c7..5b85be751b88 100644
--- a/Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml
+++ b/Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml
@@ -54,7 +54,8 @@ properties:
       Standard property that helps to define the interrupt mapping.
 
   clocks:
-    description: The reference to the device clock.
+    description:
+      The references to the device clocks (several clocks can be referenced).
 
   bus-range:
     description: |
-- 
2.35.1

[PATCH v2 4/8] dt-bindings: PCI: renesas-pci-usb: Add device tree support for r9a06g032

[Herve Codina]

Add internal PCI bridge support for the r9a06g032 SoC. The Renesas
RZ/N1D (R9A06G032) internal PCI bridge is compatible with the one
present in the R-Car Gen2 family.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml b/Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml
index 5b85be751b88..5637f5d7cf05 100644
--- a/Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml
+++ b/Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml
@@ -32,6 +32,10 @@ properties:
               - renesas,pci-r8a7793      # R-Car M2-N
               - renesas,pci-r8a7794      # R-Car E2
           - const: renesas,pci-rcar-gen2 # R-Car Gen2 and RZ/G1
+      - items:
+          - enum:
+              - renesas,pci-r9a06g032     # RZ/N1D
+          - const: renesas,pci-rzn1       # RZ/N1
 
   reg:
     description: |
-- 
2.35.1

[PATCH v2 5/8] PCI: rcar-gen2: Add R9A06G032 support

[Herve Codina]

Add Renesas R9A06G032 SoC support to the Renesas R-Car gen2 PCI
bridge driver.
The Renesas RZ/N1D (R9A06G032) internal PCI bridge is compatible
with the one available in the R-Car Gen2 family.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 drivers/pci/controller/pci-rcar-gen2.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pci/controller/pci-rcar-gen2.c b/drivers/pci/controller/pci-rcar-gen2.c
index 528bc3780e01..c190d1a030e4 100644
--- a/drivers/pci/controller/pci-rcar-gen2.c
+++ b/drivers/pci/controller/pci-rcar-gen2.c
@@ -352,6 +352,7 @@ static const struct of_device_id rcar_pci_of_match[] = {
 	{ .compatible = "renesas,pci-r8a7791", },
 	{ .compatible = "renesas,pci-r8a7794", },
 	{ .compatible = "renesas,pci-rcar-gen2", },
+	{ .compatible = "renesas,pci-rzn1", },
 	{ },
 };
 
-- 
2.35.1

[PATCH v2 6/8] ARM: dts: r9a06g032: Add internal PCI bridge node

[Herve Codina]

Add the device node for the r9a06g032 internal PCI bridge device.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 arch/arm/boot/dts/r9a06g032.dtsi | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/arch/arm/boot/dts/r9a06g032.dtsi b/arch/arm/boot/dts/r9a06g032.dtsi
index 636a6ab31c58..848dc034bb8c 100644
--- a/arch/arm/boot/dts/r9a06g032.dtsi
+++ b/arch/arm/boot/dts/r9a06g032.dtsi
@@ -211,6 +211,34 @@ gic: interrupt-controller@44101000 {
 			interrupts =
 				<GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_HIGH)>;
 		};
+
+		pci_usb: pci@40030000 {
+			compatible = "renesas,pci-r9a06g032", "renesas,pci-rzn1";
+			device_type = "pci";
+			clocks = <&sysctrl R9A06G032_HCLK_USBH>,
+				 <&sysctrl R9A06G032_HCLK_USBPM>,
+				 <&sysctrl R9A06G032_CLK_PCI_USB>;
+			clock-names = "hclk_usbh", "hclk_usbpm", "clk_pci_usb";
+			reg = <0x40030000 0xc00>,
+			      <0x40020000 0x1100>;
+			interrupts = <GIC_SPI 79 IRQ_TYPE_LEVEL_HIGH>;
+			status = "disabled";
+
+			bus-range = <0 0>;
+			#address-cells = <3>;
+			#size-cells = <2>;
+			#interrupt-cells = <1>;
+			ranges = <0x02000000 0 0x40020000 0x40020000 0 0x00010000>;
+			/* Should map all possible DDR as inbound ranges, but
+			 * the IP only supports a 256MB, 512MB, or 1GB window.
+			 * flags, PCI addr (64-bit), CPU addr, PCI size (64-bit)
+			 */
+			dma-ranges = <0x42000000 0 0x80000000 0x80000000 0 0x40000000>;
+			interrupt-map-mask = <0xf800 0 0 0x7>;
+			interrupt-map = <0x0000 0 0 1 &gic GIC_SPI 79 IRQ_TYPE_LEVEL_HIGH
+					 0x0800 0 0 1 &gic GIC_SPI 79 IRQ_TYPE_LEVEL_HIGH
+					 0x1000 0 0 2 &gic GIC_SPI 79 IRQ_TYPE_LEVEL_HIGH>;
+		};
 	};
 
 	timer {
-- 
2.35.1

[PATCH v2 7/8] ARM: dts: r9a06g032: Add USB PHY DT support

[Herve Codina]

Define the r9a06g032 generic part of the USB PHY device node.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 arch/arm/boot/dts/r9a06g032.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/r9a06g032.dtsi b/arch/arm/boot/dts/r9a06g032.dtsi
index 848dc034bb8c..2f7236e3eff0 100644
--- a/arch/arm/boot/dts/r9a06g032.dtsi
+++ b/arch/arm/boot/dts/r9a06g032.dtsi
@@ -59,6 +59,12 @@ ext_rtc_clk: extrtcclk {
 		clock-frequency = <0>;
 	};
 
+	usbphy: usbphy {
+		#phy-cells = <0>;
+		compatible = "usb-nop-xceiv";
+		status = "disabled";
+	};
+
 	soc {
 		compatible = "simple-bus";
 		#address-cells = <1>;
-- 
2.35.1

[PATCH v2 8/8] ARM: dts: r9a06g032: Link the PCI USB devices to the USB PHY

[Herve Codina]

Describe the PCI USB devices that are behind the PCI bridge, adding
necessary links to the USB PHY device.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 arch/arm/boot/dts/r9a06g032.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm/boot/dts/r9a06g032.dtsi b/arch/arm/boot/dts/r9a06g032.dtsi
index 2f7236e3eff0..f0007b0447ca 100644
--- a/arch/arm/boot/dts/r9a06g032.dtsi
+++ b/arch/arm/boot/dts/r9a06g032.dtsi
@@ -244,6 +244,18 @@ pci_usb: pci@40030000 {
 			interrupt-map = <0x0000 0 0 1 &gic GIC_SPI 79 IRQ_TYPE_LEVEL_HIGH
 					 0x0800 0 0 1 &gic GIC_SPI 79 IRQ_TYPE_LEVEL_HIGH
 					 0x1000 0 0 2 &gic GIC_SPI 79 IRQ_TYPE_LEVEL_HIGH>;
+
+			usb@1,0 {
+				reg = <0x800 0 0 0 0>;
+				phys = <&usbphy 0>;
+				phy-names = "usb";
+			};
+
+			usb@2,0 {
+				reg = <0x1000 0 0 0 0>;
+				phys = <&usbphy 0>;
+				phy-names = "usb";
+			};
 		};
 	};
 
-- 
2.35.1

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

[Miquel Raynal]

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

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

[Geert Uytterhoeven]

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

Re: [PATCH v2 3/8] dt-bindings: PCI: renesas-pci-usb: Allow multiple clocks

[Geert Uytterhoeven]

Hi Hervé,

On Thu, Apr 14, 2022 at 9:40 AM Herve Codina <herve.codina@bootlin.com> wrote:
> Define that multiple clocks can be present at clocks property.
>
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>

Thanks for your patch!

> --- a/Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml
> +++ b/Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml
> @@ -54,7 +54,8 @@ properties:
>        Standard property that helps to define the interrupt mapping.
>
>    clocks:
> -    description: The reference to the device clock.
> +    description:
> +      The references to the device clocks (several clocks can be referenced).

Please describe the clocks, and add the missing "clock-names" property.

>
>    bus-range:
>      description: |

I think it would be better to combine this with [PATCH v2 4/8], as the
additional clocks are only present on RZ/N1.

Then you can easily add json-schema logic to enforce the correct
number of clocks, depending on the compatible value.

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

Re: [PATCH v2 1/8] PCI: rcar-gen2: Add support for clocks

[Geert Uytterhoeven]

Hi Hervé,

Thanks for your patch!

On Thu, Apr 14, 2022 at 9:40 AM Herve Codina <herve.codina@bootlin.com> wrote:
> The PCI rcar-gen2 does not call any clk_prepare_enable().

Correct, this driver manages the clocks indirectly through Runtime PM.

> This lead to an access failure when the driver tries to access
> the IP (at least on a RZ/N1D platform).

I expect adding

    power-domans = <&sysctrl>;

to the pci_usb node makes this patch redundant.

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

Re: [PATCH v2 1/8] PCI: rcar-gen2: Add support for clocks

[Herve Codina]

Hi Geert,

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

> Hi Hervé,
> 
> Thanks for your patch!
> 
> On Thu, Apr 14, 2022 at 9:40 AM Herve Codina <herve.codina@bootlin.com> wrote:
> > The PCI rcar-gen2 does not call any clk_prepare_enable().  
> 
> Correct, this driver manages the clocks indirectly through Runtime PM.
> 
> > This lead to an access failure when the driver tries to access
> > the IP (at least on a RZ/N1D platform).  
> 
> I expect adding
> 
>     power-domans = <&sysctrl>;
> 
> to the pci_usb node makes this patch redundant.

Seems not enough.
I tried what you suggest :
 - Added 'power-domains = <&systrl>;' to the pci_usb node
 - Added missing '#power-domain-cells = <0>;' to sysctrl node
 - Reverted my patch.

The system crashed at boot:
--- 8< ---
...
[    0.705309] loop: module loaded
[    0.709597] ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver
[    0.716804] ehci-pci: EHCI PCI platform driver
[    0.721716] ohci_hcd: USB 1.1 'Open' Host Controller (OHCI) Driver
[    0.728522] ohci-pci: OHCI PCI platform driver
[    0.733581] usbcore: registered new interface driver usb-storage
[    0.740458] ThumbEE CPU extension supported.
[    0.745093] Registering SWP/SWPB emulation handler
[    0.797518] rzn1-pinctrl 40067000.pinctrl: probed
[    0.803311] pci-rcar-gen2 40030000.pci: host bridge /soc/pci@40030000 ranges:
[    0.811255] pci-rcar-gen2 40030000.pci:      MEM 0x0040020000..0x004002ffff -> 0x0040020000
[    0.820373] pci-rcar-gen2 40030000.pci:   IB MEM 0x0080000000..0x00bfffffff -> 0x0080000000
[    0.829609] 8<--- cut here ---
[    0.832958] Unhandled fault: external abort on non-linefetch (0x1008) at 0x90b5f848
[    0.841259] [90b5f848] *pgd=82149811, *pte=40030653, *ppte=40030453
[    0.848093] Internal error: : 1008 [#1] SMP ARM
[    0.853024] Modules linked in:
[    0.856398] CPU: 0 PID: 31 Comm: kworker/u4:1 Not tainted 5.18.0-rc2-00009-g803ee9fd9fa5-dirty #5
[    0.865998] Hardware name: Generic DT based system
[    0.871176] Workqueue: events_unbound deferred_probe_work_func
[    0.877539] PC is at rcar_pci_probe+0x15c/0x2f8
[    0.882454] LR is at _raw_spin_unlock_irqrestore+0x24/0x2c
[    0.888434] pc : [<803ea428>]    lr : [<804dc9b0>]    psr: 60000013
[    0.895193] sp : 90aa5e40  ip : 8217c4e0  fp : 00000000
[    0.900857] r10: 80e7bd30  r9 : 80000000  r8 : 40000000
[    0.906532] r7 : 80000000  r6 : 8217c410  r5 : 821d3400  r4 : 90b5f000
[    0.913580] r3 : 00000009  r2 : 5c120fb6  r1 : 60000013  r0 : 00000000
[    0.920646] Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
[    0.928365] Control: 10c5387d  Table: 8000406a  DAC: 00000051
...
--- 8< ---

I also added a trace printk in r9a06g032-clocks.c and
r9a06g032_attach_dev() was never called.

Did I miss to set something ?

Regards,
Hervé

Re: [PATCH v2 1/8] PCI: rcar-gen2: Add support for clocks

[Geert Uytterhoeven]

Hi Hervé,

On Thu, Apr 14, 2022 at 1:29 PM Herve Codina <herve.codina@bootlin.com> wrote:
> On Thu, 14 Apr 2022 10:45:54 +0200
> Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Thu, Apr 14, 2022 at 9:40 AM Herve Codina <herve.codina@bootlin.com> wrote:
> > > The PCI rcar-gen2 does not call any clk_prepare_enable().
> >
> > Correct, this driver manages the clocks indirectly through Runtime PM.
> >
> > > This lead to an access failure when the driver tries to access
> > > the IP (at least on a RZ/N1D platform).
> >
> > I expect adding
> >
> >     power-domans = <&sysctrl>;
> >
> > to the pci_usb node makes this patch redundant.
>
> Seems not enough.
> I tried what you suggest :
>  - Added 'power-domains = <&systrl>;' to the pci_usb node
>  - Added missing '#power-domain-cells = <0>;' to sysctrl node
>  - Reverted my patch.
>
> The system crashed at boot:

> [    0.832958] Unhandled fault: external abort on non-linefetch (0x1008) at 0x90b5f848

That's indeed a typical symptom of accessing a module's registers
while the module's clock is disabled.

> I also added a trace printk in r9a06g032-clocks.c and
> r9a06g032_attach_dev() was never called.
>
> Did I miss to set something ?

Do you have CONFIG_PM and CONFIG_PM_GENERIC_DOMAINS
enabled?
Apparently ARCH_RZN1 does not select these options yet.

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

Re: [PATCH v2 1/8] PCI: rcar-gen2: Add support for clocks

[Herve Codina]

Hi Geert,

On Thu, 14 Apr 2022 13:48:22 +0200
Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> Hi Hervé,
> 
> On Thu, Apr 14, 2022 at 1:29 PM Herve Codina <herve.codina@bootlin.com> wrote:
> > On Thu, 14 Apr 2022 10:45:54 +0200
> > Geert Uytterhoeven <geert@linux-m68k.org> wrote:  
> > > On Thu, Apr 14, 2022 at 9:40 AM Herve Codina <herve.codina@bootlin.com> wrote:  
> > > > The PCI rcar-gen2 does not call any clk_prepare_enable().  
> > >
> > > Correct, this driver manages the clocks indirectly through Runtime PM.
> > >  
> > > > This lead to an access failure when the driver tries to access
> > > > the IP (at least on a RZ/N1D platform).  
> > >
> > > I expect adding
> > >
> > >     power-domans = <&sysctrl>;
> > >
> > > to the pci_usb node makes this patch redundant.  
> >
> > Seems not enough.
> > I tried what you suggest :
> >  - Added 'power-domains = <&systrl>;' to the pci_usb node
> >  - Added missing '#power-domain-cells = <0>;' to sysctrl node
> >  - Reverted my patch.
> >
> > The system crashed at boot:  
> 
> > [    0.832958] Unhandled fault: external abort on non-linefetch (0x1008) at 0x90b5f848  
> 
> That's indeed a typical symptom of accessing a module's registers
> while the module's clock is disabled.
> 
> > I also added a trace printk in r9a06g032-clocks.c and
> > r9a06g032_attach_dev() was never called.
> >
> > Did I miss to set something ?  
> 
> Do you have CONFIG_PM and CONFIG_PM_GENERIC_DOMAINS
> enabled?
> Apparently ARCH_RZN1 does not select these options yet.
> 

Thanks a lot for pointing this.

I added select CONFIG_PM and CONFIG_PM_GENERIC_DOMAINS
in ARCH_RZN1 and it works.

I will remove my patch calling clk_bulk_prepare_enable() and
add some new patches to enable power domains in the v3 series.

Regards,
Hervé

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

[Rob Herring]

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
> 
> 

Re: [PATCH v2 6/8] ARM: dts: r9a06g032: Add internal PCI bridge node

[Sergey Shtylyov]

Hello!

On 4/14/22 10:40 AM, Herve Codina wrote:

> Add the device node for the r9a06g032 internal PCI bridge device.
> 
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---
>  arch/arm/boot/dts/r9a06g032.dtsi | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/r9a06g032.dtsi b/arch/arm/boot/dts/r9a06g032.dtsi
> index 636a6ab31c58..848dc034bb8c 100644
> --- a/arch/arm/boot/dts/r9a06g032.dtsi
> +++ b/arch/arm/boot/dts/r9a06g032.dtsi
> @@ -211,6 +211,34 @@ gic: interrupt-controller@44101000 {
>  			interrupts =
>  				<GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_HIGH)>;
>  		};
> +
> +		pci_usb: pci@40030000 {
> +			compatible = "renesas,pci-r9a06g032", "renesas,pci-rzn1";
> +			device_type = "pci";
> +			clocks = <&sysctrl R9A06G032_HCLK_USBH>,
> +				 <&sysctrl R9A06G032_HCLK_USBPM>,
> +				 <&sysctrl R9A06G032_CLK_PCI_USB>;
> +			clock-names = "hclk_usbh", "hclk_usbpm", "clk_pci_usb";
> +			reg = <0x40030000 0xc00>,
> +			      <0x40020000 0x1100>;
> +			interrupts = <GIC_SPI 79 IRQ_TYPE_LEVEL_HIGH>;
> +			status = "disabled";
> +
> +			bus-range = <0 0>;
> +			#address-cells = <3>;
> +			#size-cells = <2>;
> +			#interrupt-cells = <1>;

   Really? I don't think this PCI bridge is also an interrupt controller...

> +			ranges = <0x02000000 0 0x40020000 0x40020000 0 0x00010000>;
> +			/* Should map all possible DDR as inbound ranges, but
> +			 * the IP only supports a 256MB, 512MB, or 1GB window.
> +			 * flags, PCI addr (64-bit), CPU addr, PCI size (64-bit)
> +			 */
> +			dma-ranges = <0x42000000 0 0x80000000 0x80000000 0 0x40000000>;
> +			interrupt-map-mask = <0xf800 0 0 0x7>;
> +			interrupt-map = <0x0000 0 0 1 &gic GIC_SPI 79 IRQ_TYPE_LEVEL_HIGH
> +					 0x0800 0 0 1 &gic GIC_SPI 79 IRQ_TYPE_LEVEL_HIGH
> +					 0x1000 0 0 2 &gic GIC_SPI 79 IRQ_TYPE_LEVEL_HIGH>;
> +		};
>  	};
>  
>  	timer {

MBR, Sergey

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

[Herve Codina]

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é

-- 
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

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

[Herve Codina]

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é

-- 
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v2 3/8] dt-bindings: PCI: renesas-pci-usb: Allow multiple clocks

[Herve Codina]

Hi Geert, Rob,

On Thu, 14 Apr 2022 10:35:07 +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:
> > Define that multiple clocks can be present at clocks property.
> >
> > Signed-off-by: Herve Codina <herve.codina@bootlin.com>  
> 
> Thanks for your patch!
> 
> > --- a/Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml
> > +++ b/Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml
> > @@ -54,7 +54,8 @@ properties:
> >        Standard property that helps to define the interrupt mapping.
> >
> >    clocks:
> > -    description: The reference to the device clock.
> > +    description:
> > +      The references to the device clocks (several clocks can be referenced).  
> 
> Please describe the clocks, and add the missing "clock-names" property.
> 
> >
> >    bus-range:
> >      description: |  
> 
> I think it would be better to combine this with [PATCH v2 4/8], as the
> additional clocks are only present on RZ/N1.
> 
> Then you can easily add json-schema logic to enforce the correct
> number of clocks, depending on the compatible value.

Sure.

Is there a way to have the clocks description depending on the compatible value.
I mean something like:
--- 8< ---
properties:
  clocks:
    maxItems: 1

if:
  properties:
    compatible:
      contains:
        enum:
          - renesas,pci-r9a06g032
          - renesas,pci-rzn1

then:
  properties:
    clocks:
      items:
        - description: Internal bus clock (AHB) for HOST
        - description: Internal bus clock (AHB) Power Management
        - description: PCI clock for USB subsystem
      minItems: 3
      maxItems: 3

else:
  properties:
    items:
       - description: Device clock
    clocks:
      minItems: 1
      maxItems: 1
--- 8< ---

In fact, I would like to describe the 3 clocks only for the r9a06g032 SOC
and the rzn1 family and have an other description for the other 'compatible'.

I cannot succeed to do it.

The only thing I can do is to leave the description of the 3 clocks out of the
conditional part. This leads to :

--- 8< ---
properties:
  clocks:
    items:
      - description: Internal bus clock (AHB) for HOST
      - description: Internal bus clock (AHB) Power Management
      - description: PCI clock for USB subsystem
    minItems: 1

if:
  properties:
    compatible:
      contains:
        enum:
          - renesas,pci-r9a06g032
          - renesas,pci-rzn1

then:
  properties:
    clocks:
      minItems: 3
      maxItems: 3

else:
  properties:
    clocks:
      minItems: 1
      maxItems: 1
--- 8< ---

Also the clock-names items can be different depending on the
compatible value.

Regards,
Hervé

-- 
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

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

[Rob Herring]

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

Re: [PATCH v2 6/8] ARM: dts: r9a06g032: Add internal PCI bridge node

[Herve Codina]

Hi Sergey,

On Mon, 18 Apr 2022 12:02:52 +0300
Sergey Shtylyov <s.shtylyov@omp.ru> wrote:

> Hello!
> 
> On 4/14/22 10:40 AM, Herve Codina wrote:
> 
> > Add the device node for the r9a06g032 internal PCI bridge device.
> > 
> > Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> > ---
> >  arch/arm/boot/dts/r9a06g032.dtsi | 28 ++++++++++++++++++++++++++++
> >  1 file changed, 28 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/r9a06g032.dtsi b/arch/arm/boot/dts/r9a06g032.dtsi
> > index 636a6ab31c58..848dc034bb8c 100644
> > --- a/arch/arm/boot/dts/r9a06g032.dtsi
> > +++ b/arch/arm/boot/dts/r9a06g032.dtsi
> > @@ -211,6 +211,34 @@ gic: interrupt-controller@44101000 {
> >  			interrupts =
> >  				<GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_HIGH)>;
> >  		};
> > +
> > +		pci_usb: pci@40030000 {
> > +			compatible = "renesas,pci-r9a06g032", "renesas,pci-rzn1";
> > +			device_type = "pci";
> > +			clocks = <&sysctrl R9A06G032_HCLK_USBH>,
> > +				 <&sysctrl R9A06G032_HCLK_USBPM>,
> > +				 <&sysctrl R9A06G032_CLK_PCI_USB>;
> > +			clock-names = "hclk_usbh", "hclk_usbpm", "clk_pci_usb";
> > +			reg = <0x40030000 0xc00>,
> > +			      <0x40020000 0x1100>;
> > +			interrupts = <GIC_SPI 79 IRQ_TYPE_LEVEL_HIGH>;
> > +			status = "disabled";
> > +
> > +			bus-range = <0 0>;
> > +			#address-cells = <3>;
> > +			#size-cells = <2>;
> > +			#interrupt-cells = <1>;  
> 
>    Really? I don't think this PCI bridge is also an interrupt controller...

The #interrupt-cells property is required in the binding.
The #interrupt-cells is needed when we use interrupt-map property.

At least from 'make dtbindings_check':
	properties: '#interrupt-cells' is a dependency of 'interrupt-map'
	from schema $id: http://devicetree.org/meta-schemas/interrupts.yaml#

Do I miss something ?

Regards,
Hervé

> 
> > +			ranges = <0x02000000 0 0x40020000 0x40020000 0 0x00010000>;
> > +			/* Should map all possible DDR as inbound ranges, but
> > +			 * the IP only supports a 256MB, 512MB, or 1GB window.
> > +			 * flags, PCI addr (64-bit), CPU addr, PCI size (64-bit)
> > +			 */
> > +			dma-ranges = <0x42000000 0 0x80000000 0x80000000 0 0x40000000>;
> > +			interrupt-map-mask = <0xf800 0 0 0x7>;
> > +			interrupt-map = <0x0000 0 0 1 &gic GIC_SPI 79 IRQ_TYPE_LEVEL_HIGH
> > +					 0x0800 0 0 1 &gic GIC_SPI 79 IRQ_TYPE_LEVEL_HIGH
> > +					 0x1000 0 0 2 &gic GIC_SPI 79 IRQ_TYPE_LEVEL_HIGH>;
> > +		};
> >  	};
> >  
> >  	timer {  
> 
> MBR, Sergey



-- 
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v2 3/8] dt-bindings: PCI: renesas-pci-usb: Allow multiple clocks

[Rob Herring]

On Wed, Apr 20, 2022 at 03:07:59PM +0200, Herve Codina wrote:
> Hi Geert, Rob,
> 
> On Thu, 14 Apr 2022 10:35:07 +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:
> > > Define that multiple clocks can be present at clocks property.
> > >
> > > Signed-off-by: Herve Codina <herve.codina@bootlin.com>  
> > 
> > Thanks for your patch!
> > 
> > > --- a/Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml
> > > +++ b/Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml
> > > @@ -54,7 +54,8 @@ properties:
> > >        Standard property that helps to define the interrupt mapping.
> > >
> > >    clocks:
> > > -    description: The reference to the device clock.
> > > +    description:
> > > +      The references to the device clocks (several clocks can be referenced).  
> > 
> > Please describe the clocks, and add the missing "clock-names" property.
> > 
> > >
> > >    bus-range:
> > >      description: |  
> > 
> > I think it would be better to combine this with [PATCH v2 4/8], as the
> > additional clocks are only present on RZ/N1.
> > 
> > Then you can easily add json-schema logic to enforce the correct
> > number of clocks, depending on the compatible value.
> 
> Sure.
> 
> Is there a way to have the clocks description depending on the compatible value.
> I mean something like:
> --- 8< ---
> properties:
>   clocks:
>     maxItems: 1

This would need to cover both cases:

minItems: 1
maxItems: 3

> 
> if:
>   properties:
>     compatible:
>       contains:
>         enum:
>           - renesas,pci-r9a06g032
>           - renesas,pci-rzn1
> 
> then:
>   properties:
>     clocks:
>       items:
>         - description: Internal bus clock (AHB) for HOST
>         - description: Internal bus clock (AHB) Power Management
>         - description: PCI clock for USB subsystem
>       minItems: 3
>       maxItems: 3

Don't need minItems or maxItems here. 3 is the default size based on 
'items' length.

> 
> else:
>   properties:
>     items:

I think you meant for this to be under 'clocks'.

>        - description: Device clock
>     clocks:
>       minItems: 1
>       maxItems: 1

Just 'maxItems' is enough.

> --- 8< ---
> 
> In fact, I would like to describe the 3 clocks only for the r9a06g032 SOC
> and the rzn1 family and have an other description for the other 'compatible'.
> 
> I cannot succeed to do it.
> 
> The only thing I can do is to leave the description of the 3 clocks out of the
> conditional part. This leads to :
> 
> --- 8< ---
> properties:
>   clocks:
>     items:
>       - description: Internal bus clock (AHB) for HOST
>       - description: Internal bus clock (AHB) Power Management
>       - description: PCI clock for USB subsystem
>     minItems: 1
> 
> if:
>   properties:
>     compatible:
>       contains:
>         enum:
>           - renesas,pci-r9a06g032
>           - renesas,pci-rzn1
> 
> then:
>   properties:
>     clocks:
>       minItems: 3
>       maxItems: 3

minItems is enough.

> 
> else:
>   properties:
>     clocks:
>       minItems: 1
>       maxItems: 1

This doesn't seem right as the description of the first clock is wrong 
for this case.

I would go with the first way.

Rob

Re: [PATCH v2 3/8] dt-bindings: PCI: renesas-pci-usb: Allow multiple clocks

[Geert Uytterhoeven]

Hi Hervé,

On Wed, Apr 20, 2022 at 3:08 PM Herve Codina <herve.codina@bootlin.com> wrote:
> Is there a way to have the clocks description depending on the compatible value.

Rob already replied.
For an example, check out the various bindings for RZ/G2L devices,
e.g. Documentation/devicetree/bindings/net/renesas,etheravb.yaml

> I mean something like:
> --- 8< ---
> properties:
>   clocks:
>     maxItems: 1
>
> if:
>   properties:
>     compatible:
>       contains:
>         enum:
>           - renesas,pci-r9a06g032
>           - renesas,pci-rzn1

Checking only for the second compatible value should be sufficient.

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

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

[Herve Codina]

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é

-- 
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v2 3/8] dt-bindings: PCI: renesas-pci-usb: Allow multiple clocks

[Herve Codina]

Hi Rob, Geert,

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

...
> > Is there a way to have the clocks description depending on the compatible value.
> > I mean something like:
> > --- 8< ---
> > properties:
> >   clocks:
> >     maxItems: 1  
> 
> This would need to cover both cases:
> 
> minItems: 1
> maxItems: 3
> 
> > 
> > if:
> >   properties:
> >     compatible:
> >       contains:
> >         enum:
> >           - renesas,pci-r9a06g032
> >           - renesas,pci-rzn1
> > 
> > then:
> >   properties:
> >     clocks:
> >       items:
> >         - description: Internal bus clock (AHB) for HOST
> >         - description: Internal bus clock (AHB) Power Management
> >         - description: PCI clock for USB subsystem
> >       minItems: 3
> >       maxItems: 3  
> 
> Don't need minItems or maxItems here. 3 is the default size based on 
> 'items' length.
> 
> > 
> > else:
> >   properties:
> >     items:  
> 
> I think you meant for this to be under 'clocks'.
> 
> >        - description: Device clock
> >     clocks:
> >       minItems: 1
> >       maxItems: 1  
> 
> Just 'maxItems' is enough.
> 
> > --- 8< ---
> > 

Thanks to your details and Geert's binding examples,

I have the clocks description and clock-names set depending
on compatible value.

This will be present in v3 series.

Regards,
Hervé

-- 
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v2 3/8] dt-bindings: PCI: renesas-pci-usb: Allow multiple clocks

[Herve Codina]

Hi Geert,

On Wed, 20 Apr 2022 15:32:27 +0200
Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> Hi Hervé,
> 
> On Wed, Apr 20, 2022 at 3:08 PM Herve Codina <herve.codina@bootlin.com> wrote:
> > Is there a way to have the clocks description depending on the compatible value.  
> 
> Rob already replied.
> For an example, check out the various bindings for RZ/G2L devices,
> e.g. Documentation/devicetree/bindings/net/renesas,etheravb.yaml

Yes, thanks.

> 
> > I mean something like:
> > --- 8< ---
> > properties:
> >   clocks:
> >     maxItems: 1
> >
> > if:
> >   properties:
> >     compatible:
> >       contains:
> >         enum:
> >           - renesas,pci-r9a06g032
> >           - renesas,pci-rzn1  
> 
> Checking only for the second compatible value should be sufficient.

Ok, changed.

Regards,
Hervé

Re: [PATCH v2 6/8] ARM: dts: r9a06g032: Add internal PCI bridge node

[Rob Herring]

On Mon, Apr 18, 2022 at 12:02:52PM +0300, Sergey Shtylyov wrote:
> Hello!
> 
> On 4/14/22 10:40 AM, Herve Codina wrote:
> 
> > Add the device node for the r9a06g032 internal PCI bridge device.
> > 
> > Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> > ---
> >  arch/arm/boot/dts/r9a06g032.dtsi | 28 ++++++++++++++++++++++++++++
> >  1 file changed, 28 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/r9a06g032.dtsi b/arch/arm/boot/dts/r9a06g032.dtsi
> > index 636a6ab31c58..848dc034bb8c 100644
> > --- a/arch/arm/boot/dts/r9a06g032.dtsi
> > +++ b/arch/arm/boot/dts/r9a06g032.dtsi
> > @@ -211,6 +211,34 @@ gic: interrupt-controller@44101000 {
> >  			interrupts =
> >  				<GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_HIGH)>;
> >  		};
> > +
> > +		pci_usb: pci@40030000 {
> > +			compatible = "renesas,pci-r9a06g032", "renesas,pci-rzn1";
> > +			device_type = "pci";
> > +			clocks = <&sysctrl R9A06G032_HCLK_USBH>,
> > +				 <&sysctrl R9A06G032_HCLK_USBPM>,
> > +				 <&sysctrl R9A06G032_CLK_PCI_USB>;
> > +			clock-names = "hclk_usbh", "hclk_usbpm", "clk_pci_usb";
> > +			reg = <0x40030000 0xc00>,
> > +			      <0x40020000 0x1100>;
> > +			interrupts = <GIC_SPI 79 IRQ_TYPE_LEVEL_HIGH>;
> > +			status = "disabled";
> > +
> > +			bus-range = <0 0>;
> > +			#address-cells = <3>;
> > +			#size-cells = <2>;
> > +			#interrupt-cells = <1>;
> 
>    Really? I don't think this PCI bridge is also an interrupt controller...

'interrupt-map' depends on '#interrupt-cells'.

> 
> > +			ranges = <0x02000000 0 0x40020000 0x40020000 0 0x00010000>;
> > +			/* Should map all possible DDR as inbound ranges, but
> > +			 * the IP only supports a 256MB, 512MB, or 1GB window.
> > +			 * flags, PCI addr (64-bit), CPU addr, PCI size (64-bit)
> > +			 */
> > +			dma-ranges = <0x42000000 0 0x80000000 0x80000000 0 0x40000000>;
> > +			interrupt-map-mask = <0xf800 0 0 0x7>;
> > +			interrupt-map = <0x0000 0 0 1 &gic GIC_SPI 79 IRQ_TYPE_LEVEL_HIGH
> > +					 0x0800 0 0 1 &gic GIC_SPI 79 IRQ_TYPE_LEVEL_HIGH
> > +					 0x1000 0 0 2 &gic GIC_SPI 79 IRQ_TYPE_LEVEL_HIGH>;
> > +		};
> >  	};
> >  
> >  	timer {
> 
> MBR, Sergey

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

[Rob Herring]

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