2024-01-23 22:53:44

by Sean Anderson

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

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



2024-01-23 22:53:45

by Sean Anderson

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

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 <[email protected]>
Reviewed-by: Linus Walleij <[email protected]>
---

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


2024-01-23 22:53:53

by Sean Anderson

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

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 <[email protected]>
---

(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


2024-01-29 16:18:48

by Sean Anderson

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

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 <[email protected]>
> ---
>
> (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