[PATCH v13 04/35] phy: rockchip: usbdp: Fix devm_clk_bulk_get_all check

[sashiko-bot at kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] If the PHY was pre-enabled by the bootloader, the driver leaves the clocks enabled and resets deasserted. If probe fails subsequently or the driver is unbound, these resources are permanently leaked.
--

--- Patch [4]: [PATCH v13 04/35] phy: rockchip: usbdp: Fix devm_clk_bulk_get_all check ---
commit 553b68092e2ed3747377c52ed53225389dff5096
Author: Sebastian Reichel <sebastian.reichel at collabora.com>

phy: rockchip: usbdp: Fix devm_clk_bulk_get_all check

This commit fixes the return value check for devm_clk_bulk_get_all() in
the rockchip usbdp combo phy driver. It prevents an issue where returning
-ENODEV would permanently fail the driver probe instead of allowing it
to defer.

Link: https://lore.kernel.org/linux-phy/20260612164107.C7DB21F000E9@smtp.kernel.org/

> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 12dfdd470a782..2845a012eafcc 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -427,8 +427,8 @@ static int rk_udphy_clk_init(struct rk_udphy *udphy, struct device *dev)
>  	int i;
>  
>  	udphy->num_clks = devm_clk_bulk_get_all(dev, &udphy->clks);
> -	if (udphy->num_clks < 1)
> -		return -ENODEV;
> +	if (udphy->num_clks < 0)
> +		return udphy->num_clks;
>  
>  	/* used for configure phy reference clock frequency */
>  	for (i = 0; i < udphy->num_clks; i++) {

[Severity: High]
This is a pre-existing issue, but does rk_udphy_probe() permanently leak
clocks and resets if the driver probe fails or is unbound?

If the bootloader has already enabled the hardware,
rk_udphy_get_initial_status() leaves the clocks enabled and resets
deasserted by intentionally skipping rk_udphy_disable().

However, if a subsequent step in rk_udphy_probe() fails:

drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_probe() {
	...
	ret = rk_udphy_get_initial_status(udphy);
	if (ret)
		return ret;
	...
	if (device_property_present(dev, "orientation-switch")) {
		ret = rk_udphy_setup_orien_switch(udphy);
		if (ret)
			return ret;
	}
	...
}

The function returns an error without disabling the clocks or asserting
resets.

Since devm_clk_bulk_get_all() only registers a cleanup action to clk_put()
the clocks (not disable/unprepare them), the clock enable counts are leaked,
preventing the clock subsystem from disabling them later.

Does this also cause a similar resource leak if the driver binds successfully
but is later unbound via sysfs, since the driver lacks a .remove callback to
tear down the pre-enabled hardware state?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714-rockchip-usbdp-cleanup-v13-0-6cb3e769d4c5@collabora.com?part=4