Re: [PATCH v3 2/3] arm64: dts: nuvoton: ma35d1: add USB controllers and dual-port PHY node

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The DT changes add dual-port USB PHY support but the underlying PHY driver only supports a single port, breaking USB port 1.
--

--- Patch [2]: [PATCH v3 2/3] arm64: dts: nuvoton: ma35d1: add USB controllers and dual-port PHY node ---
commit ae86165665a697c5181c72ed4e3f8f74bcf0beed
Author: Joey Lu <a0987203069@gmail.com>

arm64: dts: nuvoton: ma35d1: add USB controllers and dual-port PHY node

This commit adds USB controller (DWC2, EHCI, OHCI) and dual-port PHY nodes
to the ma35d1 device tree. It also adds pin control configurations for the
HSUSB signals to the ma35d1-iot-512m and ma35d1-som-256m board files.

> diff --git a/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi b/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi
> --- a/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi
> +++ b/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi
[ ... ]
> @@ -88,6 +88,14 @@ sys: system-management@40460000 {
>  			#reset-cells = <1>;
>  		};
>  
> +		usb_phy: usb-phy {
> +			compatible = "nuvoton,ma35d1-usb2-phy";
> +			clocks = <&clk HUSBH0_GATE>;
> +			nuvoton,sys = <&sys>;
> +			#phy-cells = <1>;
> +			status = "disabled";
> +		};
> +
[ ... ]
> +		ehci1: usb@401c0000 {
> +			compatible = "generic-ehci";
> +			reg = <0x0 0x401c0000 0x0 0x1000>;
> +			interrupts = <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&clk HUSBH1_GATE>;
> +			phys = <&usb_phy 1>;
[Severity: High]
Is the ma35d1 USB PHY driver fully capable of supporting dual ports?

Looking at ma35_usb_phy_probe() in drivers/phy/nuvoton/phy-ma35d1-usb2.c,
it registers the provider with of_phy_simple_xlate, which ignores the index
in `<&usb_phy 1>` and unconditionally returns the first PHY instance.

Furthermore, ma35_usb_phy_power_on() hardcodes the power-on routine
to PHY0:

drivers/phy/nuvoton/phy-ma35d1-usb2.c:ma35_usb_phy_power_on() {
    ...
    regmap_update_bits(p_phy->sysreg, MA35_SYS_REG_USBPMISCR, 0x7, (PHY0POR | PHY0SUSPEND));
    ...
}

Will this cause port 1 to fail to initialize since the driver will
incorrectly share PHY0's refcount and leave PHY1 completely uninitialized?

> +			phy-names = "usb";
> +			companion = <&ohci1>;
> +			status = "disabled";
> +		};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708103606.1462960-1-a0987203069@gmail.com?part=2

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy