[PATCH v3 0/4] usb: phy: generic: Support enabling/disabling VBUS

[PATCH v3 0/4] usb: phy: generic: Support enabling/disabling VBUS

[Sean Anderson]

The generic USB phy has had VBUS-related code for a long time, but it
has always been broken, since the regulator was never gotten from the
device tree. However, the support itself seems not very useful, since
e.g.  usb_phy_vbus_on/off has no users and usb_phy_set_power is only
used by gadgets to make sure they don't draw too much current. Instead,
use the VBUS regulator to implement otg_set_vbus, which is called from
several drivers. This results in a change in semantics of VBUS, but
since support was always broken I don't think this will have any affect.

This series is pretty unpopular... I only got minor feedback on one
patch last time. I'll try to remember to ping when the merge window
opens up.

Changes in v3:
- Update description as suggested by Linus

Changes in v2:
- Fix dt_binding_check errors

Sean Anderson (4):
  dt-bindings: usb: usb-nop-xceiv: Repurpose vbus-regulator
  usb: phy: generic: Get the vbus supply
  usb: phy: generic: Implement otg->set_vbus
  usb: phy: generic: Disable vbus on removal

 .../bindings/usb/usb-nop-xceiv.yaml           | 11 ++--
 drivers/usb/phy/phy-generic.c                 | 55 +++++++++----------
 2 files changed, 33 insertions(+), 33 deletions(-)

-- 
2.35.1.1320.gc452695387.dirty

[PATCH v3 1/4] dt-bindings: usb: usb-nop-xceiv: Repurpose vbus-regulator

[Sean Anderson]

The vbus-regulator property was never actually read from the device tree.
Introduce a new property vbus-supply to represent the regulator powering
the VBUS when acting as an A-Device. This supply will be enabled and
disabled as necessary. Note that this is different from vbus-regulator,
which represented the available current available to draw from VBUS in
B-Device mode. Because no one was using vbus-regulator, remove it.

Signed-off-by: Sean Anderson <sean.anderson@seco.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---

Changes in v3:
- Update description as suggested by Linus

Changes in v2:
- Fix dt_binding_check errors

 .../devicetree/bindings/usb/usb-nop-xceiv.yaml        | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/usb-nop-xceiv.yaml b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.yaml
index 6734f4d3aa78..9b3ea23654af 100644
--- a/Documentation/devicetree/bindings/usb/usb-nop-xceiv.yaml
+++ b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.yaml
@@ -37,10 +37,11 @@ properties:
     description: Should specify the GPIO detecting a VBus insertion
     maxItems: 1
 
-  vbus-regulator:
-    description: Should specify the regulator supplying current drawn from
-      the VBus line.
-    $ref: /schemas/types.yaml#/definitions/phandle
+  vbus-supply:
+    description: regulator supplying VBUS. It will be enabled and disabled
+                 dynamically in OTG mode. If the regulator is controlled by a
+                 GPIO line, this should be modeled as a regulator-fixed and
+                 referenced by this supply.
 
   wakeup-source:
     description:
@@ -65,7 +66,7 @@ examples:
         vcc-supply = <&hsusb1_vcc_regulator>;
         reset-gpios = <&gpio1 7 GPIO_ACTIVE_LOW>;
         vbus-detect-gpio = <&gpio2 13 GPIO_ACTIVE_HIGH>;
-        vbus-regulator = <&vbus_regulator>;
+        vbus-supply = <&vbus_regulator>;
         #phy-cells = <0>;
     };
 
-- 
2.35.1.1320.gc452695387.dirty

[PATCH v3 2/4] usb: phy: generic: Get the vbus supply

[Sean Anderson]

While support for working with a vbus was added, the regulator was never
actually gotten (despite what was documented). Fix this by actually
getting the supply from the device tree.

Fixes: 7acc9973e3c4 ("usb: phy: generic: add vbus support")
Signed-off-by: Sean Anderson <sean.anderson@seco.com>
---

(no changes since v1)

 drivers/usb/phy/phy-generic.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/usb/phy/phy-generic.c b/drivers/usb/phy/phy-generic.c
index 9ab50f26db60..16494030209e 100644
--- a/drivers/usb/phy/phy-generic.c
+++ b/drivers/usb/phy/phy-generic.c
@@ -274,6 +274,13 @@ int usb_phy_gen_create_phy(struct device *dev, struct usb_phy_generic *nop)
 		return dev_err_probe(dev, PTR_ERR(nop->vbus_draw),
 				     "could not get vbus regulator\n");
 
+	nop->vbus_draw = devm_regulator_get_exclusive(dev, "vbus");
+	if (PTR_ERR(nop->vbus_draw) == -ENODEV)
+		nop->vbus_draw = NULL;
+	if (IS_ERR(nop->vbus_draw))
+		return dev_err_probe(dev, PTR_ERR(nop->vbus_draw),
+				     "could not get vbus regulator\n");
+
 	nop->dev		= dev;
 	nop->phy.dev		= nop->dev;
 	nop->phy.label		= "nop-xceiv";
-- 
2.35.1.1320.gc452695387.dirty

Re: [PATCH v3 2/4] usb: phy: generic: Get the vbus supply

[Sean Anderson]

On 1/23/24 17:51, Sean Anderson wrote:
> While support for working with a vbus was added, the regulator was never
> actually gotten (despite what was documented). Fix this by actually
> getting the supply from the device tree.
> 
> Fixes: 7acc9973e3c4 ("usb: phy: generic: add vbus support")
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
> ---
> 
> (no changes since v1)
> 
>  drivers/usb/phy/phy-generic.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/usb/phy/phy-generic.c b/drivers/usb/phy/phy-generic.c
> index 9ab50f26db60..16494030209e 100644
> --- a/drivers/usb/phy/phy-generic.c
> +++ b/drivers/usb/phy/phy-generic.c
> @@ -274,6 +274,13 @@ int usb_phy_gen_create_phy(struct device *dev, struct usb_phy_generic *nop)
>  		return dev_err_probe(dev, PTR_ERR(nop->vbus_draw),
>  				     "could not get vbus regulator\n");
>  
> +	nop->vbus_draw = devm_regulator_get_exclusive(dev, "vbus");
> +	if (PTR_ERR(nop->vbus_draw) == -ENODEV)
> +		nop->vbus_draw = NULL;
> +	if (IS_ERR(nop->vbus_draw))
> +		return dev_err_probe(dev, PTR_ERR(nop->vbus_draw),
> +				     "could not get vbus regulator\n");
> +
>  	nop->dev		= dev;
>  	nop->phy.dev		= nop->dev;
>  	nop->phy.label		= "nop-xceiv";

OK, so as it turns out, this patch (and only this one) got applied as
03e607cbb293 ("usb: phy: generic: Get the vbus supply"). Which of course
messes with the premise of this series that no one was using the vbus
supply (so we can repurpose it), by adding the possibility that someone
was using the vbus supply. However, the only in-tree user is in
arch/arm/boot/dts/armada-388-helios4.dts, which was added before this
patch (so presumably they don't care).

At the very least, this patch should get dropped (and hopefully no one
complains about the next patch).

--Sean

[PATCH v3 3/4] usb: phy: generic: Implement otg->set_vbus

[Sean Anderson]

Some USB controller drivers call otg_set_vbus when entering host or
device mode. Implement this callback so that VBUS can be turned on and
off automatically. This is especially useful when there is no property
for a VBUS supply in the controller's binding.

This results in a change in semantics of the vbus_draw regulator.
Whereas before it represented the VBUS supplied by an A-Device when we
acted as a B-Device, now it represents an internal VBUS source.
Accordingly, we no longer set the current limit or enable/disable the
bus from nop_gpio_vbus_thread. Because this supply was never initialized
before the previous commit, there should be no change in behavior.

Signed-off-by: Sean Anderson <sean.anderson@seco.com>
---

(no changes since v1)

 drivers/usb/phy/phy-generic.c | 45 +++++++++++++----------------------
 1 file changed, 17 insertions(+), 28 deletions(-)

diff --git a/drivers/usb/phy/phy-generic.c b/drivers/usb/phy/phy-generic.c
index 16494030209e..f7db24b5ed5c 100644
--- a/drivers/usb/phy/phy-generic.c
+++ b/drivers/usb/phy/phy-generic.c
@@ -74,33 +74,26 @@ static void nop_reset(struct usb_phy_generic *nop)
 }
 
 /* interface to regulator framework */
-static void nop_set_vbus_draw(struct usb_phy_generic *nop, unsigned mA)
+static int nop_set_vbus(struct usb_otg *otg, bool enable)
 {
-	struct regulator *vbus_draw = nop->vbus_draw;
-	int enabled;
-	int ret;
+	int ret = 0;
+	struct usb_phy_generic *nop = dev_get_drvdata(otg->usb_phy->dev);
 
-	if (!vbus_draw)
-		return;
+	if (!nop->vbus_draw)
+		return 0;
 
-	enabled = nop->vbus_draw_enabled;
-	if (mA) {
-		regulator_set_current_limit(vbus_draw, 0, 1000 * mA);
-		if (!enabled) {
-			ret = regulator_enable(vbus_draw);
-			if (ret < 0)
-				return;
-			nop->vbus_draw_enabled = 1;
-		}
-	} else {
-		if (enabled) {
-			ret = regulator_disable(vbus_draw);
-			if (ret < 0)
-				return;
-			nop->vbus_draw_enabled = 0;
-		}
+	if (enable && !nop->vbus_draw_enabled) {
+		ret = regulator_enable(nop->vbus_draw);
+		if (ret)
+			nop->vbus_draw_enabled = false;
+		else
+			nop->vbus_draw_enabled = true;
+
+	} else if (!enable && nop->vbus_draw_enabled) {
+		ret = regulator_disable(nop->vbus_draw);
+		nop->vbus_draw_enabled = false;
 	}
-	nop->mA = mA;
+	return ret;
 }
 
 
@@ -120,14 +113,9 @@ static irqreturn_t nop_gpio_vbus_thread(int irq, void *data)
 		otg->state = OTG_STATE_B_PERIPHERAL;
 		nop->phy.last_event = status;
 
-		/* drawing a "unit load" is *always* OK, except for OTG */
-		nop_set_vbus_draw(nop, 100);
-
 		atomic_notifier_call_chain(&nop->phy.notifier, status,
 					   otg->gadget);
 	} else {
-		nop_set_vbus_draw(nop, 0);
-
 		status = USB_EVENT_NONE;
 		otg->state = OTG_STATE_B_IDLE;
 		nop->phy.last_event = status;
@@ -291,6 +279,7 @@ int usb_phy_gen_create_phy(struct device *dev, struct usb_phy_generic *nop)
 	nop->phy.otg->usb_phy		= &nop->phy;
 	nop->phy.otg->set_host		= nop_set_host;
 	nop->phy.otg->set_peripheral	= nop_set_peripheral;
+	nop->phy.otg->set_vbus          = nop_set_vbus;
 
 	return 0;
 }
-- 
2.35.1.1320.gc452695387.dirty

[PATCH v3 4/4] usb: phy: generic: Disable vbus on removal

[Sean Anderson]

If we enabled vbus, we need to balance that with a disable.

Signed-off-by: Sean Anderson <sean.anderson@seco.com>
---

(no changes since v1)

 drivers/usb/phy/phy-generic.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/phy/phy-generic.c b/drivers/usb/phy/phy-generic.c
index f7db24b5ed5c..8f735a86cd19 100644
--- a/drivers/usb/phy/phy-generic.c
+++ b/drivers/usb/phy/phy-generic.c
@@ -337,6 +337,9 @@ static void usb_phy_generic_remove(struct platform_device *pdev)
 	struct usb_phy_generic *nop = platform_get_drvdata(pdev);
 
 	usb_remove_phy(&nop->phy);
+
+	if (nop->vbus_draw && nop->vbus_draw_enabled)
+		regulator_disable(nop->vbus_draw);
 }
 
 static const struct of_device_id nop_xceiv_dt_ids[] = {
-- 
2.35.1.1320.gc452695387.dirty