Re: [PATCH v5 7/9] phy: renesas: phy-rcar-gen3-usb2: Fix devm action registration for disabled VBUS regulator

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch replaces logical reference tracking with a hardware state check that evaluates negative error codes as true and skips cleanup on mismatch, causing `enable_count` leaks and kernel warnings.
--

--- Patch [7]: [PATCH v5 7/9] phy: renesas: phy-rcar-gen3-usb2: Fix devm action registration for disabled VBUS regulator ---
commit b4d6d24f697dee807a883f959100e138c3feeef3
Author: Biju Das <biju.das.jz@bp.renesas.com>

phy: renesas: phy-rcar-gen3-usb2: Fix devm action registration for disabled VBUS regulator

This commit modifies the VBUS regulator initialization to always register
the devm disable action if the regulator is currently enabled. It uses
regulator_is_enabled() to determine the hardware state, attempting to
ensure that the initial enabled state from devm_regulator_get_exclusive()
is properly cleaned up.

> diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> index d06fb52ed5f1b..ef38c3b365d42 100644
> --- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> +++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> @@ -905,15 +905,17 @@ static int rcar_gen3_phy_usb2_vbus_regulator_get_exclusive_enable(struct rcar_ge
[ ... ]
> +	if (enable) {
> +		ret = regulator_enable(channel->vbus);
> +		if (ret)
> +			return ret;
> +	}
>  
> -	ret = regulator_enable(channel->vbus);
> -	if (ret)
> -		return ret;
> +	if (regulator_is_enabled(channel->vbus))
> +		return devm_add_action_or_reset(dev, rcar_gen3_phy_usb2_vbus_disable_action,
> +						channel->vbus);

[Severity: Medium]
Can this cause issues if regulator_is_enabled() returns a negative error
code?

If the underlying communication fails, regulator_is_enabled() will return a
negative value which evaluates as true. This would erroneously register the
devm disable action even when the enable_count is 0, triggering a WARN in
regulator_disable() on driver unbind.

Additionally, if the caller explicitly requested enable, regulator_enable()
will unconditionally increment the enable_count. If the subsequent check
returns 0 due to hardware ramp-up delays before the status register reflects
the state, the cleanup action would be skipped.

Would this leak the reference and trigger a WARN_ON() for the enable_count
in regulator_put() upon device removal?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729115022.222340-1-biju.das.jz@bp.renesas.com?part=7

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