Re: [PATCH v2 5/5] arm64: dts: qcom: hamoa: Extend QMPPHY description for USB4

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing cleanup on error paths in qmp_combo_usb4_init() leads to a leaked clock and unbalanced PHY state.
--

--- Patch [5]: [PATCH v2 5/5] arm64: dts: qcom: hamoa: Extend QMPPHY description for USB4 ---
commit c84965f0842bad650afeacff840775c1c5f9c6ae
Author: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

arm64: dts: qcom: hamoa: Extend QMPPHY description for USB4

The USB4 part of the QMPPHY requires that one more GCC clock (P2RR2P -
PHY-to-Router, Router-to-PHY) is enabled for the PHY to initialize
successfully. This patch updates the device tree to describe this clock
for the PHY nodes.

> diff --git a/arch/arm64/boot/dts/qcom/hamoa.dtsi b/arch/arm64/boot/dts/qcom/hamoa.dtsi
> --- a/arch/arm64/boot/dts/qcom/hamoa.dtsi
> +++ b/arch/arm64/boot/dts/qcom/hamoa.dtsi
> @@ -2825,11 +2825,13 @@ usb_1_ss0_qmpphy: phy@fd5000 {
>  			clocks = <&gcc GCC_USB3_PRIM_PHY_AUX_CLK>,
>  				 <&rpmhcc RPMH_CXO_CLK>,
>  				 <&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>,
> -				 <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>;
> +				 <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>,
> +				 <&gcc GCC_USB4_0_PHY_P2RR2P_PIPE_CLK>;
>  			clock-names = "aux",
>  				      "ref",
>  				      "com_aux",
> -				      "usb3_pipe";
> +				      "usb3_pipe",
> +				      "p2rr2p_pipe";

[Severity: Medium]
Does adding this clock lead to a resource leak in the driver on error paths?

In drivers/phy/qualcomm/phy-qcom-qmp-combo.c:qmp_combo_usb4_init(), if an error
occurs after enabling the p2rr2p_pipe clock, the cleanup paths don't seem to
disable it:

	ret = qmp_combo_com_init(qmp, false);
	if (ret)
		return ret;

Here the function returns directly, failing to disable the previously
enabled clock.

Later in the same function:

	ret = qmp_combo_reconfigure_phy(qmp, QMPPHY_MODE_USB4);
	if (ret)
		return ret;

If this fails, it misses both the clock disable and qmp_combo_com_exit().

And finally:

	ret = qmp_combo_usb_power_on(phy);
	if (ret) {
		qmp_combo_com_exit(qmp, false);
		return ret;
	}

This handles the com_exit but still misses disabling the pipe clock.

Could this cause the clock reference to be leaked on hardware initialization
or resumption error paths, preventing the associated clock and power domains
from being disabled?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728-topic-usb4phy-v2-0-5d9dd5149ec7@oss.qualcomm.com?part=5