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

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

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

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.

Keep the regulator enabled while disabling its physical VBUS output until
the port role is established. If disabling the hardware output fails,
release the regulator reference to avoid leaving the regulator enabled
after probe fails.

Rename the regulator helper to reflect that it acquires the regulator
without enabling its physical VBUS output.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/phy/renesas/phy-rcar-gen3-usb2.c | 26 +++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
index b5ba751805aa..08db619a43a0 100644
--- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c
+++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
@@ -1114,8 +1114,8 @@ static void rcar_gen3_phy_usb2_vbus_disable_action(void *data)
 	regulator_disable(vbus);
 }
 
-static int rcar_gen3_phy_usb2_vbus_regulator_get_exclusive_enable(struct rcar_gen3_chan *channel,
-								  bool enable)
+static int rcar_gen3_phy_usb2_vbus_regulator_get_exclusive(struct rcar_gen3_chan *channel,
+							   bool acquire)
 {
 	struct device *dev = channel->dev;
 	int ret;
@@ -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);
+			return ret;
+		}
+
 		return devm_add_action_or_reset(dev, rcar_gen3_phy_usb2_vbus_disable_action,
 						channel->vbus);
+	}
 
 	return 0;
 }
@@ -1151,7 +1163,7 @@ static int rcar_gen3_phy_usb2_vbus_regulator_register(struct rcar_gen3_chan *cha
 	struct device *dev = channel->dev;
 	struct regulator_config rcfg = { .dev = dev, };
 	struct regulator_dev *rdev;
-	bool enable = false;
+	bool acquire = false;
 
 	rcfg.of_node = of_get_available_child_by_name(dev->of_node,
 						      "vbus-regulator");
@@ -1165,10 +1177,10 @@ static int rcar_gen3_phy_usb2_vbus_regulator_register(struct rcar_gen3_chan *cha
 					     "Failed to create vbus-regulator\n");
 
 		channel->otg_internal_reg = true;
-		enable = true;
+		acquire = true;
 	}
 
-	return rcar_gen3_phy_usb2_vbus_regulator_get_exclusive_enable(channel, enable);
+	return rcar_gen3_phy_usb2_vbus_regulator_get_exclusive(channel, acquire);
 }
 
 static int rcar_gen3_phy_usb2_probe(struct platform_device *pdev)
-- 
2.55.0


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

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

[sashiko-bot]

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