2022-04-14 13:24:46

by Herve Codina

[permalink] [raw]
Subject: [PATCH v2 0/8] 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

--
2.35.1


2022-04-14 19:01:25

by Herve Codina

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

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

Signed-off-by: Herve Codina <[email protected]>
---
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

2022-04-14 20:09:50

by Herve Codina

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

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

2022-04-14 22:31:39

by Herve Codina

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

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

2022-04-15 11:27:12

by Herve Codina

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

Define that multiple clocks can be present at clocks property.

Signed-off-by: Herve Codina <[email protected]>
---
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

2022-04-15 14:12:08

by Miquel Raynal

[permalink] [raw]
Subject: 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:

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

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

2022-04-16 00:36:17

by Herve Codina

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

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

2022-04-16 01:35:34

by Rob Herring (Arm)

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

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

2022-04-16 01:46:55

by Herve Codina

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

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

Signed-off-by: Herve Codina <[email protected]>
---
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

2022-04-21 06:51:48

by Rob Herring (Arm)

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

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

2022-04-21 23:43:40

by Rob Herring (Arm)

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

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 <[email protected]> wrote:
>
> > Hi Herv?,
> >
> > On Thu, Apr 14, 2022 at 9:40 AM Herve Codina <[email protected]> wrote:
> > > Define that multiple clocks can be present at clocks property.
> > >
> > > Signed-off-by: Herve Codina <[email protected]>
> >
> > 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

2022-04-22 02:48:54

by Herve Codina

[permalink] [raw]
Subject: 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:

...

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

2022-04-22 08:02:59

by Geert Uytterhoeven

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

Hi Hervé,

On Wed, Apr 20, 2022 at 3:08 PM Herve Codina <[email protected]> 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 -- [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

2022-04-22 09:00:47

by Herve Codina

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

Hi Geert,

On Wed, 20 Apr 2022 15:32:27 +0200
Geert Uytterhoeven <[email protected]> wrote:

> Hi Hervé,
>
> On Wed, Apr 20, 2022 at 3:08 PM Herve Codina <[email protected]> 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é

2022-04-22 17:01:25

by Rob Herring (Arm)

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

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

2022-04-22 19:34:50

by Herve Codina

[permalink] [raw]
Subject: 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:

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

2022-04-22 21:06:05

by Herve Codina

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

Hi Rob,

On Thu, 14 Apr 2022 13:15:30 -0500
Rob Herring <[email protected]> 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 <[email protected]>
> > ---
> > .../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 <[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.
> > +
> > + 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