Skip to contentSkip to conversation

RZN1 USB Host support

PATCH Superseded Herve Codina 5 participants 30 messages Apr 14, 2022 – Apr 20, 2022

Subsystems (5)

v2 of 6 Superseded Latest is v6 →

lore
Share & tools
lore
#0 lore raw

v2 cover RZN1 USB Host support

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
Signature · 1 line
2.35.1
#1 lore raw

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

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.

---
 drivers/pci/controller/pci-rcar-gen2.c | 28 ++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)
drivers/pci/controller/pci-rcar-gen2.c +26 -2
index 35804ea394fd..528bc3780e01 100644
@@ -8,6 +8,7 @@
  * Author: Valentine Barshak <[email protected]>
  */
 
+#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[] = {
Signature · 1 line
2.35.1
#2 lore raw

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

Hi Hervé,

Thanks for your patch!

Quoted 1 line · Herve Codina

The PCI rcar-gen2 does not call any clk_prepare_enable().

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

Quoted 2 lines

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 -- [email protected]

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

#3 lore raw

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

Hi Geert,

Quoted 9 lines · On Thu, 14 Apr 2022 10:45:54 +0200 Geert Uytterhoeven <[email protected]>…

Hi Hervé,

Thanks for your patch!

Quoted 1 line · Herve Codina

The PCI rcar-gen2 does not call any clk_prepare_enable().

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

Quoted 8 lines
Quoted 2 lines

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é

#4 lore raw

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

Hi Hervé,

Quoted 22 lines · Herve Codina
Quoted 5 lines · On Thu, 14 Apr 2022 10:45:54 +0200 Geert Uytterhoeven <[email protected]>…
Quoted 1 line · Herve Codina

The PCI rcar-gen2 does not call any clk_prepare_enable().

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

Quoted 8 lines
Quoted 2 lines

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.

Quoted 3 lines

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.

Quoted 4 lines

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 -- [email protected]

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

#5 lore raw

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

Hi Geert,

Quoted 32 lines · On Thu, 14 Apr 2022 13:48:22 +0200 Geert Uytterhoeven <[email protected]>…

Hi Hervé,

Quoted 22 lines · Herve Codina
Quoted 5 lines · On Thu, 14 Apr 2022 10:45:54 +0200 Geert Uytterhoeven <[email protected]>…
Quoted 1 line · Herve Codina

The PCI rcar-gen2 does not call any clk_prepare_enable().

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

Quoted 8 lines
Quoted 2 lines

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.

Quoted 3 lines

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.

Quoted 9 lines
Quoted 4 lines

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é

#6 lore raw

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

In reply to 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.

---
 .../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
Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt deleted +0 -84
deleted file mode 100644
index aeba38f0a387..000000000000
@@ -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";
-};
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +134 -0

134 lines not on this page · raw message

Signature · 1 line
2.35.1
#7 lore raw

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

Hi Hervé,

[email protected] wrote on Thu, 14 Apr 2022 09:40:05 +0200:

Quoted 143 lines

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.

---
 .../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
Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt deleted +0 -84
deleted file mode 100644
index aeba38f0a387..000000000000
@@ -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";
-};
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +35 -0
new file mode 100644
index 000000000000..3f8d79b746c7
@@ -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 <[email protected]>
+  - Yoshihiro Shimoda <[email protected]>
+
+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
+

Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml line 45

Quoted 10 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +10 -0
+  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.

Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml line 57

Quoted 12 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +12 -0
+
+  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 ?

Quoted 67 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +67 -0
+
+  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>;

Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml line 134

Quoted 10 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +10 -0
+                phy-names = "usb";
+            };
+            
+            usb@2,0 {
+                reg = <0x1000 0 0 0 0>;
+                phys = <&usb0 0>;
+                phy-names = "usb";
+            };
+        };
+    };

Thanks,
Miquèl

#8 lore raw

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

In reply to Herve Codina

Hi Hervé,

Quoted 5 lines · 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.

Thanks a lot for tackling this DT binding file!

Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml line 1

Quoted 4 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +1 -0
@@ -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)

Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml line 42

Quoted 7 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +7 -0
+  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.

Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml line 45

Quoted 3 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +3 -0
+
+  interrupts:
+    description: Interrupt for the device.

maxItems: 1

The description is not needed.

Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml line 57

Quoted 12 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +12 -0
+
+  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.

Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml line 85

Quoted 7 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +7 -0
+required:
+  - compatible
+  - reg
+  - interrupts
+  - interrupt-map
+  - interrupt-map-mask
+  - clocks

Missing "resets" and "power-domains".

Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml line 91

Quoted 6 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +6 -0
+  - 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?

Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml line 100

Quoted 9 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +9 -0
+
+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).

Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml line 108

Quoted 8 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +8 -0
+
+        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>;
Quoted 9 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +9 -0
+            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>;

Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml line 127

Quoted 10 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +10 -0
+            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:
+ $

Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml line 134

Quoted 7 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +7 -0
+            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 -- [email protected]

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

#9 lore raw

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

Hi Geert,

Quoted 11 lines · On Thu, 14 Apr 2022 10:28:47 +0200 Geert Uytterhoeven <[email protected]>…

Hi Hervé,

Quoted 5 lines · 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.

Thanks a lot for tackling this DT binding file!

Quoted 7 lines

Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml line 1

Quoted 4 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +1 -0
@@ -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"

Quoted 9 lines

Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml line 42

Quoted 8 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +7 -0
+  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  
Quoted 4 lines

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

Ok, changed.

Quoted 8 lines

Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml line 45

Quoted 4 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +3 -0
+
+  interrupts:
+    description: Interrupt for the device.  

maxItems: 1

The description is not needed.

Ok, changed.

Quoted 16 lines

Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml line 57

Quoted 13 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +12 -0
+
+  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

Quoted 1 line

The description is not needed.

Ok, changed

Quoted 4 lines

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

Missing description of the child nodes.

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

Quoted 10 lines

Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml line 85

Quoted 8 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +7 -0
+required:
+  - compatible
+  - reg
+  - interrupts
+  - interrupt-map
+  - interrupt-map-mask
+  - clocks  

Missing "resets" and "power-domains".

Added

Quoted 11 lines

Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml line 91

Quoted 7 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +6 -0
+  - 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?

Quoted 13 lines

Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml line 100

Quoted 9 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +9 -0
+
+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

Quoted 11 lines

Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml line 108

Quoted 8 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +8 -0
+
+        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

Quoted 16 lines
Quoted 16 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +15 -0
+            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>;
Quoted 8 lines

Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml line 127

Quoted 4 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +4 -0
+                phys = <&usb0 0>;
+                phy-names = "usb";
+            };
+  

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

Ok

Quoted 11 lines

Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml line 134

Quoted 8 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +7 -0
+            usb@2,0 {
+                reg = <0x1000 0 0 0 0>;
+                phys = <&usb0 0>;
+                phy-names = "usb";
+            };
+        };
+    };  

Gr{oetje,eeting}s,

Quoted 8 lines

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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é

Signature · 3 lines
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
maintainer
#10 lore raw

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

In reply to Herve Codina
Quoted 3 lines · 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.

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

Quoted 157 lines
---
 .../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
Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt deleted +0 -84
deleted file mode 100644
index aeba38f0a387..000000000000
@@ -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";
-};
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +52 -0
new file mode 100644
index 000000000000..3f8d79b746c7
@@ -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 <[email protected]>
+  - Yoshihiro Shimoda <[email protected]>
+
+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:

Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml line 62

Quoted 10 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +10 -0
+    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

Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml line 71

Quoted 9 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +9 -0
+
+  "#address-cells":
+    const: 3
+
+  "#size-cells":
+    const: 2
+
+  "#interrupt-cells":
+    const: 1

All these are defined by pci-bus.yaml

Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml line 77

Quoted 6 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +6 -0
+
+  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'

Quoted 22 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +22 -0
+
+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>;

Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml line 109

Quoted 10 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +10 -0
+        #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.

Quoted 29 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +25 -0
+
+            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";
+            };
+        };
+    };
Signature · 1 line
2.35.1
#11 lore raw

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

Hi Rob,

Quoted 6 lines · On Thu, 14 Apr 2022 13:15:30 -0500 Rob Herring <[email protected]> wrote:
Quoted 3 lines · 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.

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

Ok, renamed.

Quoted 12 lines
Quoted 12 lines
---
 .../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
Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt deleted +0 -0
deleted file mode 100644

...

Quoted 2 lines
Quoted 2 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +0 -0
new file mode 100644

...

Quoted 61 lines
Quoted 61 lines

index 000000000000..3f8d79b746c7

Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +57 -0
@@ -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 <[email protected]>
+  - Yoshihiro Shimoda <[email protected]>
+
+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.
Quoted 8 lines

Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml line 62

Quoted 5 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +5 -0
+
+  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 ?

Quoted 12 lines

Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml line 71

Quoted 10 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +9 -0
+
+  "#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 ?

Quoted 9 lines

Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml line 77

Quoted 7 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +6 -0
+
+  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.

Quoted 26 lines
Quoted 26 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +26 -0
+
+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";
Quoted 8 lines

Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml line 109

Quoted 6 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +6 -0
+            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é

Signature · 3 lines
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
maintainer
#12 lore raw

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

Quoted 100 lines · Herve Codina

Hi Rob,

Quoted 6 lines · On Thu, 14 Apr 2022 13:15:30 -0500 Rob Herring <[email protected]> wrote:
Quoted 3 lines · 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.

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

Ok, renamed.

Quoted 12 lines
Quoted 12 lines
---
 .../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
Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt deleted +0 -0
deleted file mode 100644

...

Quoted 2 lines
Quoted 2 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +0 -0
new file mode 100644

...

Quoted 61 lines
Quoted 61 lines

index 000000000000..3f8d79b746c7

Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +57 -0
@@ -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 <[email protected]>
+  - Yoshihiro Shimoda <[email protected]>
+
+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.
Quoted 8 lines

Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml line 62

Quoted 5 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +5 -0
+
+  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

Quoted 5 lines

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.

Quoted 12 lines
Quoted 11 lines

Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml line 71

Quoted 9 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +9 -0
+
+  "#address-cells":
+    const: 3
+
+  "#size-cells":
+    const: 2
+
+  "#interrupt-cells":
+    const: 1  

All these are defined by pci-bus.yaml

Quoted 8 lines

Right.
Replaced by:

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

Is that correct ?

You can just drop them completely.

Rob

#13 lore raw

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

Hi Rob,

On Wed, 20 Apr 2022 08:18:50 -0500
Rob Herring <[email protected]> wrote:

...

Quoted 8 lines
Quoted 8 lines
Quoted 7 lines

Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml line 62

Quoted 4 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +4 -0
+  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

Quoted 8 lines
Quoted 5 lines

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 ?

Quoted 16 lines
Quoted 16 lines
Quoted 12 lines

Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml line 71

Quoted 10 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +9 -0
+
+  "#address-cells":
+    const: 3
+
+  "#size-cells":
+    const: 2
+
+  "#interrupt-cells":
+    const: 1    

All these are defined by pci-bus.yaml

Right.
Replaced by:

Quoted 7 lines
Quoted 5 lines

"#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é

Signature · 3 lines
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
maintainer
#14 lore raw

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

Quoted 21 lines · Herve Codina

Hi Rob,

On Wed, 20 Apr 2022 08:18:50 -0500
Rob Herring <[email protected]> wrote:

...

Quoted 14 lines
Quoted 13 lines
Quoted 7 lines

Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml line 62

Quoted 4 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +4 -0
+  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 ?

Quoted 6 lines
Quoted 2 lines

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.

Quoted 39 lines
Quoted 14 lines
Quoted 14 lines
Quoted 10 lines

Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml line 71

Quoted 8 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml new file +8 -0
+  "#address-cells":
+    const: 3
+
+  "#size-cells":
+    const: 2
+
+  "#interrupt-cells":
+    const: 1    

All these are defined by pci-bus.yaml

Right.
Replaced by:

Quoted 7 lines
Quoted 5 lines

"#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< ---
Quoted 2 lines

So I keep
"#interrupt-cells": true

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

Rob

#15 lore raw

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

In reply to Herve Codina

Define that multiple clocks can be present at clocks property.

---
 Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml +2 -1
index 3f8d79b746c7..5b85be751b88 100644
@@ -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: |
Signature · 1 line
2.35.1
#16 lore raw

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

Hi Hervé,

Quoted 3 lines · Herve Codina

Define that multiple clocks can be present at clocks property.

Thanks for your patch!

Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml properties: · line 58

Quoted 9 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml +2 -1
@@ -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.

Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml properties: · line 61

Quoted 3 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml +0 -0
   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 -- [email protected]

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

#17 lore raw

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

Hi Geert, Rob,

Quoted 22 lines · On Thu, 14 Apr 2022 10:35:07 +0200 Geert Uytterhoeven <[email protected]>…

Hi Hervé,

Quoted 3 lines · Herve Codina

Define that multiple clocks can be present at clocks property.

Thanks for your patch!

Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml properties: · line 58

Quoted 9 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml +2 -1
@@ -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.

Quoted 1 line
Quoted 8 lines
Quoted 2 lines

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é

Signature · 3 lines
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
maintainer
#18 lore raw

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

Quoted 36 lines · Herve Codina

Hi Geert, Rob,

Quoted 22 lines · On Thu, 14 Apr 2022 10:35:07 +0200 Geert Uytterhoeven <[email protected]>…

Hi Hervé,

Quoted 3 lines · Herve Codina

Define that multiple clocks can be present at clocks property.

Thanks for your patch!

Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml properties: · line 58

Quoted 9 lines
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml +2 -1
@@ -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.

Quoted 1 line
Quoted 8 lines
Quoted 2 lines

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.

Quoted 8 lines

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

Quoted 9 lines

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

Quoted 9 lines

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.

Quoted 4 lines

else:
properties:
items:

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

Quoted 4 lines

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

Just 'maxItems' is enough.

Quoted 27 lines

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

Quoted 5 lines

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

minItems is enough.

Quoted 6 lines

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

#19 lore raw

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

Hi Rob, Geert,

On Wed, 20 Apr 2022 08:24:59 -0500
Rob Herring <[email protected]> wrote:

...

Quoted 39 lines
Quoted 6 lines

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

Quoted 9 lines

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

Quoted 9 lines

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.

Quoted 4 lines

else:
properties:
items:

Quoted 11 lines

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

Quoted 4 lines

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

Just 'maxItems' is enough.

Quoted 2 lines

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

Signature · 3 lines
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
#20 lore raw

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

In reply to Herve Codina

Hi Hervé,

Quoted 1 line · Herve Codina

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

Quoted 6 lines

I mean something like:
--- 8< ---
properties:
clocks:
maxItems: 1

Quoted 7 lines

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 -- [email protected]

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

#21 lore raw

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

Hi Geert,

Quoted 8 lines · On Wed, 20 Apr 2022 15:32:27 +0200 Geert Uytterhoeven <[email protected]>…

Hi Hervé,

Quoted 1 line · Herve Codina

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.

Quoted 15 lines
Quoted 7 lines

I mean something like:
--- 8< ---
properties:
clocks:
maxItems: 1

Quoted 7 lines

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

Quoted 1 line

Checking only for the second compatible value should be sufficient.

Ok, changed.

Regards,
Hervé

#22 lore raw

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

In reply to 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.

---
 Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml | 4 ++++
 1 file changed, 4 insertions(+)
Documentation/devicetree/bindings/pci/renesas,pci-usb.yaml +4 -0
index 5b85be751b88..5637f5d7cf05 100644
@@ -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: |
Signature · 1 line
2.35.1
#23 lore raw

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

In reply to 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.

---
 drivers/pci/controller/pci-rcar-gen2.c | 1 +
 1 file changed, 1 insertion(+)
drivers/pci/controller/pci-rcar-gen2.c +1 -0
index 528bc3780e01..c190d1a030e4 100644
@@ -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", },
 	{ },
 };
 
Signature · 1 line
2.35.1
#24 lore raw

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

In reply to Herve Codina

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

---
 arch/arm/boot/dts/r9a06g032.dtsi | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)
arch/arm/boot/dts/r9a06g032.dtsi +28 -0
index 636a6ab31c58..848dc034bb8c 100644
@@ -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 {
Signature · 1 line
2.35.1
#25 lore raw

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

Hello!

Quoted 22 lines · Herve Codina

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

---
 arch/arm/boot/dts/r9a06g032.dtsi | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)
arch/arm/boot/dts/r9a06g032.dtsi +7 -0
index 636a6ab31c58..848dc034bb8c 100644
@@ -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>;

arch/arm/boot/dts/r9a06g032.dtsi gic: interrupt-controller@44101000 { · line 230

Quoted 10 lines
arch/arm/boot/dts/r9a06g032.dtsi +10 -0
+			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...

Quoted 4 lines
arch/arm/boot/dts/r9a06g032.dtsi +4 -0
+			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)

arch/arm/boot/dts/r9a06g032.dtsi gic: interrupt-controller@44101000 { · line 244

Quoted 10 lines
arch/arm/boot/dts/r9a06g032.dtsi +7 -0
+			 */
+			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

#26 lore raw

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

Hi Sergey,

Quoted 30 lines · On Mon, 18 Apr 2022 12:02:52 +0300 Sergey Shtylyov <[email protected]> wrote:

Hello!

Quoted 26 lines · Herve Codina

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

---
 arch/arm/boot/dts/r9a06g032.dtsi | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)
arch/arm/boot/dts/r9a06g032.dtsi +11 -0
index 636a6ab31c58..848dc034bb8c 100644
@@ -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>;
Quoted 8 lines

arch/arm/boot/dts/r9a06g032.dtsi gic: interrupt-controller@44101000 { · line 230

Quoted 6 lines
arch/arm/boot/dts/r9a06g032.dtsi +6 -0
+			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é

Quoted 17 lines
Quoted 5 lines
arch/arm/boot/dts/r9a06g032.dtsi +4 -0
+			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)

arch/arm/boot/dts/r9a06g032.dtsi gic: interrupt-controller@44101000 { · line 244

Quoted 10 lines
arch/arm/boot/dts/r9a06g032.dtsi +7 -0
+			 */
+			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

Signature · 3 lines
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
maintainer
#27 lore raw

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

In reply to Sergey Shtylyov
Quoted 30 lines · Sergey Shtylyov

Hello!

Quoted 26 lines · Herve Codina

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

---
 arch/arm/boot/dts/r9a06g032.dtsi | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)
arch/arm/boot/dts/r9a06g032.dtsi +11 -0
index 636a6ab31c58..848dc034bb8c 100644
@@ -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>;
Quoted 8 lines

arch/arm/boot/dts/r9a06g032.dtsi gic: interrupt-controller@44101000 { · line 230

Quoted 6 lines
arch/arm/boot/dts/r9a06g032.dtsi +6 -0
+			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'.

Quoted 17 lines
Quoted 5 lines
arch/arm/boot/dts/r9a06g032.dtsi +4 -0
+			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)

arch/arm/boot/dts/r9a06g032.dtsi gic: interrupt-controller@44101000 { · line 244

Quoted 10 lines
arch/arm/boot/dts/r9a06g032.dtsi +7 -0
+			 */
+			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

#28 lore raw

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

In reply to Herve Codina

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

---
 arch/arm/boot/dts/r9a06g032.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)
arch/arm/boot/dts/r9a06g032.dtsi +6 -0
index 848dc034bb8c..2f7236e3eff0 100644
@@ -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>;
Signature · 1 line
2.35.1
#29 lore raw

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

In reply to Herve Codina

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

---
 arch/arm/boot/dts/r9a06g032.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)
arch/arm/boot/dts/r9a06g032.dtsi +12 -0
index 2f7236e3eff0..f0007b0447ca 100644
@@ -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";
+			};
 		};
 	};
 
Signature · 1 line
2.35.1