Re: [PATCH v3 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] A resource leak is present in `qmp_combo_usb4_init()` (introduced by a previous patch in this series) and is exposed when this DT patch provides the missing `p2rr2p_pipe` clock. If `clk_prepare_enable(qmp->p2rr2p_pipe_clk)` succeeds, the function may later fail during `qmp_combo_com_init()`, `qmp_combo_reconfigure_phy()`, or `qmp_combo_usb_power_on()`. On these error paths, the function returns without calling `clk_disable_unprepare(qmp->p2rr2p_pipe_clk)`, leaking the clock reference.
--

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

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

This commit extends the QMPPHY description for USB4 by adding the missing 
GCC_USB4_X_PHY_P2RR2P_PIPE_CLK clock. This additional clock is required 
for the PHY to initialize successfully.

> diff --git a/arch/arm64/boot/dts/qcom/hamoa.dtsi b/arch/arm64/boot/dts/qcom/hamoa.dtsi
> index 09527dcf95767..2effc0319a01a 100644
> --- 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]
This isn't a bug introduced by this device tree patch itself, but providing
this missing p2rr2p_pipe clock exposes a resource leak in the driver's
error path.

If the call to clk_prepare_enable(qmp->p2rr2p_pipe_clk) succeeds in
qmp_combo_usb4_init():

drivers/phy/qualcomm/phy-qcom-qmp-combo.c:qmp_combo_usb4_init() {
    ...
	ret = clk_prepare_enable(qmp->p2rr2p_pipe_clk);
	if (ret) {
		dev_err(qmp->dev, "p2rr2p_pipe enable failed: %d\n", ret);
		return ret;
	}

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

Does this code leak the p2rr2p_pipe_clk reference if qmp_combo_com_init(),
qmp_combo_reconfigure_phy(), or qmp_combo_usb_power_on() fails? 

Should it call clk_disable_unprepare(qmp->p2rr2p_pipe_clk) on these error
paths before returning?

[ ... ]

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