Re: [PATCH] usb/phy-generic: Add support for OTG VBUS supply control

[Mike Looijmans <mike.looijmans@topic.nl>]

Met vriendelijke groet / kind regards,

Mike Looijmans
System Expert


TOPIC Embedded Products B.V.
Materiaalweg 4, 5681 RJ Best
The Netherlands

T: +31 (0) 499 33 69 69
E: mike.looijmans@topicproducts.com
W: www.topicproducts.com

Please consider the environment before printing this e-mail
On 28-05-2020 13:20, Peter Chen wrote:
> On 20-05-26 16:50:51, Mike Looijmans wrote:
>> This enables support for VBUS on boards where the power is supplied
>> by a regulator. The regulator is enabled when the USB port enters
>> HOST mode.
>>
> Why you don't do this at your host controller driver?

That was my first thought too, but it made more sense to do it here. 
It's about how things are connected on the board, and not about the 
controller. Also makes for a consistent devicetree when using different 
SOCs on the same carrier board.

I already needed this driver to properly reset the USB phy, which the 
controller driver also did not do. So it made sense to add the vbus 
power control to this driver too.

If you have a strong preference for the controller driver, I can move 
this to the DWC3 driver which happens to be the controller for the 
latest batch of SOMs.


>
> Peter
>> Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
>> ---
>>   .../devicetree/bindings/usb/usb-nop-xceiv.txt |  3 ++
>>   drivers/usb/phy/phy-generic.c                 | 44 ++++++++++++++++++-
>>   drivers/usb/phy/phy-generic.h                 |  2 +
>>   3 files changed, 48 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
>> index 4dc6a8ee3071..775a19fdb613 100644
>> --- a/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
>> +++ b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
>> @@ -16,6 +16,9 @@ Optional properties:
>>   
>>   - vcc-supply: phandle to the regulator that provides power to the PHY.
>>   
>> +- vbus-supply: phandle to the regulator that provides the VBUS power for when
>> +  the device is in HOST mode.
>> +
>>   - reset-gpios: Should specify the GPIO for reset.
>>   
>>   - vbus-detect-gpio: should specify the GPIO detecting a VBus insertion
>> diff --git a/drivers/usb/phy/phy-generic.c b/drivers/usb/phy/phy-generic.c
>> index 661a229c105d..ebfb90764511 100644
>> --- a/drivers/usb/phy/phy-generic.c
>> +++ b/drivers/usb/phy/phy-generic.c
>> @@ -203,13 +203,43 @@ static int nop_set_host(struct usb_otg *otg, struct usb_bus *host)
>>   	return 0;
>>   }
>>   
>> +static int nop_set_vbus(struct usb_otg *otg, bool enabled)
>> +{
>> +	struct usb_phy_generic *nop;
>> +	int ret;
>> +
>> +	if (!otg)
>> +		return -ENODEV;
>> +
>> +	nop = container_of(otg->usb_phy, struct usb_phy_generic, phy);
>> +
>> +	if (!nop->vbus_reg)
>> +		return 0;
>> +
>> +	if (enabled) {
>> +		if (nop->vbus_reg_enabled)
>> +			return 0;
>> +		ret = regulator_enable(nop->vbus_reg);
>> +		if (ret < 0)
>> +			return ret;
>> +		nop->vbus_reg_enabled = true;
>> +	} else {
>> +		if (!nop->vbus_reg_enabled)
>> +			return 0;
>> +		ret = regulator_disable(nop->vbus_reg);
>> +		if (ret < 0)
>> +			return ret;
>> +		nop->vbus_reg_enabled = false;
>> +	}
There's a "return 0" missing here, will fix that in v2
>> +}
>> +
>>   int usb_phy_gen_create_phy(struct device *dev, struct usb_phy_generic *nop)
>>   {
>>   	enum usb_phy_type type = USB_PHY_TYPE_USB2;
>>   	int err = 0;
>>   
>>   	u32 clk_rate = 0;
>> -	bool needs_vcc = false, needs_clk = false;
>> +	bool needs_vcc = false, needs_clk = false, needs_vbus = false;
>>   
>>   	if (dev->of_node) {
>>   		struct device_node *node = dev->of_node;
>> @@ -219,6 +249,7 @@ int usb_phy_gen_create_phy(struct device *dev, struct usb_phy_generic *nop)
>>   
>>   		needs_vcc = of_property_read_bool(node, "vcc-supply");
>>   		needs_clk = of_property_read_bool(node, "clocks");
>> +		needs_vbus = of_property_read_bool(node, "vbus-supply");
>>   	}
>>   	nop->gpiod_reset = devm_gpiod_get_optional(dev, "reset",
>>   						   GPIOD_ASIS);
>> @@ -268,6 +299,16 @@ int usb_phy_gen_create_phy(struct device *dev, struct usb_phy_generic *nop)
>>   			return -EPROBE_DEFER;
>>   	}
>>   
>> +	nop->vbus_reg = devm_regulator_get(dev, "vbus");
>> +	if (IS_ERR(nop->vbus_reg)) {
>> +		dev_dbg(dev, "Error getting vbus regulator: %ld\n",
>> +					PTR_ERR(nop->vbus_reg));
>> +		if (needs_vbus)
>> +			return -EPROBE_DEFER;
>> +
>> +		nop->vbus_reg = NULL;
>> +	}
>> +
>>   	nop->dev		= dev;
>>   	nop->phy.dev		= nop->dev;
>>   	nop->phy.label		= "nop-xceiv";
>> @@ -278,6 +319,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;
>>   }
>> diff --git a/drivers/usb/phy/phy-generic.h b/drivers/usb/phy/phy-generic.h
>> index 7ee80211a688..a3663639ea1e 100644
>> --- a/drivers/usb/phy/phy-generic.h
>> +++ b/drivers/usb/phy/phy-generic.h
>> @@ -14,7 +14,9 @@ struct usb_phy_generic {
>>   	struct gpio_desc *gpiod_reset;
>>   	struct gpio_desc *gpiod_vbus;
>>   	struct regulator *vbus_draw;
>> +	struct regulator *vbus_reg;
>>   	bool vbus_draw_enabled;
>> +	bool vbus_reg_enabled;
>>   	unsigned long mA;
>>   	unsigned int vbus;
>>   };
>> -- 
>> 2.17.1
>>

-- 
Mike Looijmans