Re: [PATCH] phy: renesas: rcar-gen3-usb2: Keep OTG VBUS disabled until role is known

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] phy: renesas: unbalanced regulator disable and probe failure on external VBUS
--

commit 0b0e5d91d1d9b6894fe5132f084e7da147246f55
Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

phy: renesas: rcar-gen3-usb2: Keep OTG VBUS disabled until role is known

Prevent the OTG PHY from driving VBUS during probe, as the port role has
not yet been determined and doing so can back-power an external host.

> diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> index b5ba751805aae..08db619a43a03 100644
> --- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> +++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
[ ... ]
> @@ -1133,15 +1133,27 @@ static int rcar_gen3_phy_usb2_vbus_regulator_get_exclusive_enable(struct rcar_ge
>  		return 0;
>  	}
>  
> -	if (enable) {
> +	if (acquire) {
>  		ret = regulator_enable(channel->vbus);
>  		if (ret)
>  			return ret;
>  	}
>  
> -	if (regulator_is_enabled(channel->vbus))
> +	if (regulator_is_enabled(channel->vbus)) {
> +		/*
> +		 * The internal regulator is enabled above only so that the regulator
> +		 * core holds a reference to it. Do not drive VBUS until the port's
> +		 * role is known; rcar_gen3_enable_vbus_ctrl() switches it from then on.
> +		 */
> +		ret = regulator_hardware_enable(channel->vbus, false);
> +		if (ret) {
> +			regulator_disable(channel->vbus);
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[Severity: High]
Does this error path trigger an unbalanced regulator disable warning?

If rcar_gen3_phy_usb2_vbus_regulator_get_exclusive() is called with
acquire = false when using an external VBUS regulator, regulator_enable() is
bypassed and the consumer's enable_count remains 0.

If the external regulator is already physically enabled (e.g. by the
bootloader), regulator_is_enabled() will return true.

If regulator_hardware_enable(channel->vbus, false) is not supported by the
external regulator (e.g. fixed regulators without GPIO), it returns
-EOPNOTSUPP, taking this error path.

Since the regulator was never enabled by this driver, won't calling
regulator_disable() here trigger an unbalanced disable warning and abort
the probe entirely?

Similarly, if regulator_hardware_enable() succeeds, devm_add_action_or_reset()
registers rcar_gen3_phy_usb2_vbus_disable_action. Since that action calls
regulator_disable(), will it cause the same unbalanced warning when the
driver unbinds?

> +			return ret;
> +		}
> +
>  		return devm_add_action_or_reset(dev, rcar_gen3_phy_usb2_vbus_disable_action,
>  						channel->vbus);
> +	}
>  
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261005154841.168047-1-prabhakar.mahadev-lad.rj@bp.renesas.com?part=1

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