[U-Boot] [PATCH v4 04/11] usb: host: xhci-rockchip: use fixed regulator to control vbus

[rock-chips(daniel.meng)]

On 2017/6/8 21:17, Marek Vasut wrote:
> On 06/08/2017 09:31 AM, Meng Dongyang wrote:
>> Use fixed regulator to control the voltage of vbus and turn off
>> vbus when usb stop.
>>
>> Signed-off-by: Meng Dongyang <daniel.meng at rock-chips.com>
>> ---
>>
>> Changes in v4:
>> - Splited from patch [Uboot,v3,04/10]
>> - Define set vbus as empty function if the macros aren't set
>>
>> Changes in v3: None
>> Changes in v2:
>> - Use fixed regulator to control vbus instead of gpio
>>
>>   drivers/usb/host/xhci-rockchip.c | 41 +++++++++++++++++++++++++++++++---------
>>   1 file changed, 32 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/usb/host/xhci-rockchip.c b/drivers/usb/host/xhci-rockchip.c
>> index f559830..dc9cd56 100644
>> --- a/drivers/usb/host/xhci-rockchip.c
>> +++ b/drivers/usb/host/xhci-rockchip.c
>> @@ -11,10 +11,10 @@
>>   #include <malloc.h>
>>   #include <usb.h>
>>   #include <watchdog.h>
>> -#include <asm/gpio.h>
>>   #include <linux/errno.h>
>>   #include <linux/compat.h>
>>   #include <linux/usb/dwc3.h>
>> +#include <power/regulator.h>
>>   
>>   #include "xhci.h"
>>   
>> @@ -23,7 +23,7 @@ DECLARE_GLOBAL_DATA_PTR;
>>   struct rockchip_xhci_platdata {
>>   	fdt_addr_t hcd_base;
>>   	fdt_addr_t phy_base;
>> -	struct gpio_desc vbus_gpio;
>> +	struct udevice *vbus_supply;
>>   };
>>   
>>   /*
>> @@ -66,15 +66,37 @@ static int xhci_usb_ofdata_to_platdata(struct udevice *dev)
>>   		return -ENXIO;
>>   	}
>>   
>> -	/* Vbus gpio */
>> -	ret = gpio_request_by_name(dev, "rockchip,vbus-gpio", 0,
>> -				   &plat->vbus_gpio, GPIOD_IS_OUT);
>> +#if defined(CONFIG_DM_USB) && defined(CONFIG_DM_REGULATOR)
>> +	/* Vbus regulator */
>> +	ret = device_get_supply_regulator(dev, "vbus-supply",
>> +					  &plat->vbus_supply);
>>   	if (ret)
>> -		debug("rockchip,vbus-gpio node missing!");
>> +		debug("Can't get vbus supply\n");
> VBUS in caps
>
>> +#endif
>>   
>>   	return 0;
>>   }
>>   
>> +#if defined(CONFIG_DM_USB) && defined(CONFIG_DM_REGULATOR)
>> +static int rockchip_xhci_set_vbus(struct rockchip_xhci_platdata *plat,
>> +				  bool value)
>> +{
>> +	int ret = 0;
> You don't need to init ret, it's always set right below.
>
>> +
>> +	ret = regulator_set_enable(plat->vbus_supply, value);
>> +	if (ret)
>> +		debug("XHCI: Failed to set vbus supply\n");
> That shouldn't be debug, that's a printf() because it's actually a
> failure. Or error() I guess ...
Considering the case vbus is always on, it is right with no vbus regulator.
So maybe it's not an error actually.
>
>> +	return ret;
>> +}
>> +#else
>> +static int rockchip_xhci_set_vbus(struct rockchip_xhci_platdata *plat,
>> +				  bool value)
>> +{
>> +	return 0;
>> +}
>> +#endif
>> +
>>   /*
>>    * rockchip_dwc3_phy_setup() - Configure USB PHY Interface of DWC3 Core
>>    * @dwc: Pointer to our controller context structure
>> @@ -153,9 +175,7 @@ static int xhci_usb_probe(struct udevice *dev)
>>   	hcor = (struct xhci_hcor *)((uint64_t)ctx->hcd +
>>   			HC_LENGTH(xhci_readl(&ctx->hcd->cr_capbase)));
>>   
>> -	/* setup the Vbus gpio here */
>> -	if (dm_gpio_is_valid(&plat->vbus_gpio))
>> -		dm_gpio_set_value(&plat->vbus_gpio, 1);
>> +	rockchip_xhci_set_vbus(plat, true);
> What about handling the return value ?
The return value can be ignored when vbus is always on. So is it enough 
just print message in
the rockchip_xhci_set_vbus function?
>
>>   	ret = rockchip_xhci_core_init(ctx, dev);
>>   	if (ret) {
>> @@ -168,6 +188,7 @@ static int xhci_usb_probe(struct udevice *dev)
>>   
>>   static int xhci_usb_remove(struct udevice *dev)
>>   {
>> +	struct rockchip_xhci_platdata *plat = dev_get_platdata(dev);
>>   	struct rockchip_xhci *ctx = dev_get_priv(dev);
>>   	int ret;
>>   
>> @@ -178,6 +199,8 @@ static int xhci_usb_remove(struct udevice *dev)
>>   	if (ret)
>>   		return ret;
>>   
>> +	rockchip_xhci_set_vbus(plat, false);
> Handle return value
>
>>   	return 0;
>>   }
>>   
>>
>