Re: [PATCH v2 12/14] arm64: dts: qcom: ipq5332: Enable USB Super-Speed PHY

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Incorrect Device Tree Compatible String
- [Low] Missing USB Controller PHY Reference / Device Tree Build Failure
--

--- Patch [12]: [PATCH v2 12/14] arm64: dts: qcom: ipq5332: Enable USB Super-Speed PHY ---
commit 3876ed72a1a7faf7ac5d0f75eed308d2c1085ec6
Author: Praveenkumar I <[email protected]>

arm64: dts: qcom: ipq5332: Enable USB Super-Speed PHY

This commit enables USB Super-Speed PHY support on the IPQ5332-RDP468
board. It overrides the PCIe PHY compatible string so the combo PHY is
handled by the USB3 PHY driver, and sets the required clocks and resets.

> diff --git a/arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts b/arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts
> index 25a70cd884c03..164f9a9a6c757 100644
> --- a/arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts
> +++ b/arch/arm64/boot/dts/qcom/ipq5332-rdp468.dts
[ ... ]
> @@ -100,3 +100,22 @@ &usbphy0 {
>  
>  	status = "okay";
>  };
> +
> +/* this combo PHY supports PCIe and USB3 */
> +&pcie0_phy {
> +	compatible = "qcom,ipq5018-uniphy-usb-ss-phy";

[Severity: High]
Does this compatible string use the wrong SoC version for this board?

Because this is an IPQ5332 board, using the IPQ5018 compatible string will
cause the phy-qcom-uniphy-usb-ss-22ull driver to write the IPQ5018
initialization sequence (targeting offsets like 0xa8) instead of the correct
IPQ5332 sequence (targeting 0x7C4 and 0x7E8). 

This will cause initialization to fail and may corrupt unrelated registers.

> +
> +	clocks = <&gcc GCC_PCIE3X1_PHY_AHB_CLK>,
> +		 <&gcc GCC_USB0_PHY_CFG_AHB_CLK>,
> +		 <&gcc GCC_USB0_PIPE_CLK>;
> +
> +	resets =  <&gcc GCC_USB0_PHY_BCR>;
> +
> +	qcom,phy-usb-mux-sel = <&tcsr 0x10540>;
> +
> +	vdd-supply = <&regulator_fixed_5p0>;
> +
> +	/delete-property/ num-lanes;
> +
> +	status = "okay";
> +};

[Severity: Low]
Is an override for the USB controller node missing from this file?

An earlier patch in this series adds phys = <&usbphy0>, <&usbphy1>; to
ipq5332.dtsi, but usbphy1 is never defined. This will cause a device tree
build failure due to the unresolved phandle.

Should this board file override the phys property in the &usb node to link
this newly configured &pcie0_phy combo PHY, replacing the non-existent
<&usbphy1> reference? 

Without this link, the USB controller will not use the combo PHY and will
remain degraded to High-Speed 2.0.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=12

-- 
linux-phy mailing list
[email protected]
https://lists.infradead.org/mailman/listinfo/linux-phy