Re: [PATCH v3 1/4] arm64: dts: renesas: r9a08g046: Add USB2.0 host/phy device nodes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The device tree node `phyrst: usbphy-ctrl@11e00000` specifies `compatible = "renesas,r9a08g046-usbphy-ctrl"` without any fallback compatible string, and this specific string is not supported by any reset driver in the tree.
- [High] The VBUS regulators are defined inside a generic `regulators` wrapper node, which violates the device tree schema and causes a probe failure in the regulator driver.
- [Low] The newly introduced compatible strings `renesas,r9a08g046-usbphy-ctrl` and `renesas,usb2-phy-r9a08g046` are not documented in the YAML device tree bindings.
--

commit 2f4cbd3dd8ab4b32e48fa0090fc979178999df6f
Author: Biju Das <biju.das.jz@bp.renesas.com>

arm64: dts: renesas: r9a08g046: Add USB2.0 host/phy device nodes

This commit adds USB2.0 device nodes to the RZ/G3L (r9a08g046) SoC DTSI.
It covers the USB PHY controller, OHCI/EHCI host controllers, and USB2 PHYs
for both ports.

> diff --git a/arch/arm64/boot/dts/renesas/r9a08g046.dtsi b/arch/arm64/boot/dts/renesas/r9a08g046.dtsi
> index 608ba1f9e2f8c..10123281c53c4 100644
> --- a/arch/arm64/boot/dts/renesas/r9a08g046.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r9a08g046.dtsi
> @@ -1127,6 +1127,110 @@ queue3 {
>  			};
>  		};
>  
> +		phyrst: usbphy-ctrl@11e00000 {
> +			compatible = "renesas,r9a08g046-usbphy-ctrl";

[Severity: Low]
Is this compatible string documented? It appears to be missing from the schema
definition in Documentation/devicetree/bindings/reset/renesas,rzg2l-usbphy-ctrl.yaml.

[Severity: High]
Does this node need a fallback compatible string?

Since "renesas,r9a08g046-usbphy-ctrl" is not supported by any reset driver in
the tree, the reset-rzg2l-usbphy-ctrl driver will not probe for this device node.

This will cause dependent USB nodes that request resets from this node to fail
with -EPROBE_DEFER indefinitely.

> +			reg = <0 0x11e00000 0 0x10000>;
> +			clocks = <&cpg CPG_MOD R9A08G046_USB_PCLK>;
> +			resets = <&cpg R9A08G046_USB_PRESETN>;
> +			power-domains = <&cpg>;
> +			#reset-cells = <1>;
> +			status = "disabled";
> +
> +			regulators {
> +				usb0_vbus_otg: vbus0 {
> +					regulator-name = "usb0_vbus";
> +				};
> +
> +				usb1_vbus_otg: vbus1 {
> +					regulator-name = "usb1_vbus";
> +				};
> +			};

[Severity: High]
Will the renesas-usb-vbus-regulator driver be able to probe this structure?

The driver expects exactly one child node named "regulator-vbus":

    of_get_child_by_name(dev->parent->of_node, "regulator-vbus")

Because the child node is named "regulators", the driver will return -ENODEV.

The driver is also hardcoded to register only a single regulator instance via
enable_mask = BIT(0), so it cannot support the two separate regulators declared
here.

Additionally, does this violate the renesas,rzg2l-usbphy-ctrl.yaml device tree
schema which forbids the "regulators" wrapper?

> +		};
> +
> +		ohci0: usb@11e10000 {
> +			compatible = "generic-ohci";
> +			reg = <0 0x11e10000 0 0x100>;
> +			interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&cpg CPG_MOD R9A08G046_USB_PCLK>,
> +				 <&cpg CPG_MOD R9A08G046_USB_U2H0_HCLK>;
> +			resets = <&phyrst 0>,

[Severity: High]
Since this controller strictly depends on the "phyrst" reset controller, will it
fail to probe due to the missing fallback compatible string in the "phyrst"
node mentioned above?

[ ... ]

> +		usb2_phy0: usb-phy@11e10200 {
> +			compatible = "renesas,usb2-phy-r9a08g046";

[Severity: Low]
Is this compatible string documented? It appears to be missing from the schema
definition in Documentation/devicetree/bindings/phy/renesas,usb2-phy.yaml.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1788943229.git.tommaso.merciai.xr@bp.renesas.com?part=1