From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kever Yang Date: Fri, 10 Mar 2017 12:02:41 +0800 Subject: [U-Boot] [RESEND PATCH v4] usb: dwc2: add support for external vbus supply In-Reply-To: <28d2b121-c562-8766-6bf7-445069728980@denx.de> References: <1488804887-10974-1-git-send-email-kever.yang@rock-chips.com> <58BE583C.30900@rock-chips.com> <28d2b121-c562-8766-6bf7-445069728980@denx.de> Message-ID: <58C22561.6050308@rock-chips.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Marek, On 03/10/2017 10:16 AM, Marek Vasut wrote: > On 03/07/2017 07:50 AM, Kever Yang wrote: >> Hi Marek, >> >> On 03/07/2017 10:55 AM, Marek Vasut wrote: >>> On 03/06/2017 01:54 PM, Kever Yang wrote: >>>> Some board do not use the dwc2 internal VBUS_DRV signal, but >>>> use a gpio pin to enable the 5.0V VBUS power, add interface to >>>> enable the power in dwc2 driver. >>>> >>>> Signed-off-by: Kever Yang >>>> Signed-off-by: Simon Glass >>>> --- >>>> >>>> Changes in v4: >>>> - Drop no use code comment by Marek. >>>> >>>> Changes in v3: >>>> - Drop use of static variable >>>> >>>> drivers/usb/host/dwc2.c | 37 ++++++++++++++++++++++++++++++++----- >>>> 1 file changed, 32 insertions(+), 5 deletions(-) >>>> >>>> diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c >>>> index d253b94..5ac602e 100644 >>>> --- a/drivers/usb/host/dwc2.c >>>> +++ b/drivers/usb/host/dwc2.c >>>> @@ -15,6 +15,7 @@ >>>> #include >>>> #include >>>> #include >>>> +#include >>>> >>>> #include "dwc2.h" >>>> >>>> @@ -159,6 +160,27 @@ static void dwc_otg_core_reset(struct >>>> dwc2_core_regs *regs) >>>> mdelay(100); >>>> } >>> Nit: >>> >>> add the ifdef around the function, ie. >> I don't understand what to do here, we already have >> "#if defined(CONFIG_DM_USB) && defined(CONFIG_DM_REGULATOR)" >> in the function, do you mean move this out of the function or >> do you need add any other MACRO? > The example of what you should do is right below Thanks for your example, I just don't understand why do you think add the ifdef around the function is better than inside the function? Does this a rule in U-Boot? Thanks, - Kever > >> Thanks, >> - Kever >>> #ifdef (.....) >>> static foo() >>> { >>> do stuff >>> } >>> #else >>> static foo() { return 0 } >>> #endif > here ^ > >>> The rest is great, thanks! >>> >>>> +static int dwc_vbus_supply_init(struct udevice *dev) >>>> +{ >>>> +#if defined(CONFIG_DM_USB) && defined(CONFIG_DM_REGULATOR) >>>> + struct udevice *vbus_supply; >>>> + int ret; >>>> + >>>> + ret = device_get_supply_regulator(dev, "vbus-supply", >>>> &vbus_supply); >>>> + if (ret) { >>>> + debug("%s: No vbus supply\n", dev->name); >>>> + return 0; >>>> + } >>>> + >>>> + ret = regulator_set_enable(vbus_supply, true); >>>> + if (ret) { >>>> + error("Error enabling vbus supply\n"); >>>> + return ret; >>>> + } >>>> +#endif >>>> + return 0; >>>> +} >>>> + >>>> /* >>>> * This function initializes the DWC_otg controller registers for >>>> * host mode. >>>> @@ -167,10 +189,12 @@ static void dwc_otg_core_reset(struct >>>> dwc2_core_regs *regs) >>>> * request queues. Host channels are reset to ensure that they are >>>> ready for >>>> * performing transfers. >>>> * >>>> + * @param dev USB Device (NULL if driver model is not being used) >>>> * @param regs Programming view of DWC_otg controller >>>> * >>>> */ >>>> -static void dwc_otg_core_host_init(struct dwc2_core_regs *regs) >>>> +static void dwc_otg_core_host_init(struct udevice *dev, >>>> + struct dwc2_core_regs *regs) >>>> { >>>> uint32_t nptxfifosize = 0; >>>> uint32_t ptxfifosize = 0; >>>> @@ -248,6 +272,9 @@ static void dwc_otg_core_host_init(struct >>>> dwc2_core_regs *regs) >>>> writel(hprt0, ®s->hprt0); >>>> } >>>> } >>>> + >>>> + if (dev) >>>> + dwc_vbus_supply_init(dev); >>>> } >>>> >>>> /* >>>> @@ -1048,7 +1075,7 @@ int _submit_int_msg(struct dwc2_priv *priv, >>>> struct usb_device *dev, >>>> } >>>> } >>>> >>>> -static int dwc2_init_common(struct dwc2_priv *priv) >>>> +static int dwc2_init_common(struct udevice *dev, struct dwc2_priv >>>> *priv) >>>> { >>>> struct dwc2_core_regs *regs = priv->regs; >>>> uint32_t snpsid; >>>> @@ -1070,7 +1097,7 @@ static int dwc2_init_common(struct dwc2_priv >>>> *priv) >>>> #endif >>>> >>>> dwc_otg_core_init(priv); >>>> - dwc_otg_core_host_init(regs); >>>> + dwc_otg_core_host_init(dev, regs); >>>> >>>> clrsetbits_le32(®s->hprt0, DWC2_HPRT0_PRTENA | >>>> DWC2_HPRT0_PRTCONNDET | DWC2_HPRT0_PRTENCHNG | >>>> @@ -1143,7 +1170,7 @@ int usb_lowlevel_init(int index, enum >>>> usb_init_type init, void **controller) >>>> if (board_usb_init(index, USB_INIT_HOST)) >>>> return -1; >>>> >>>> - return dwc2_init_common(priv); >>>> + return dwc2_init_common(NULL, priv); >>>> } >>>> >>>> int usb_lowlevel_stop(int index) >>>> @@ -1214,7 +1241,7 @@ static int dwc2_usb_probe(struct udevice *dev) >>>> >>>> bus_priv->desc_before_addr = true; >>>> >>>> - return dwc2_init_common(priv); >>>> + return dwc2_init_common(dev, priv); >>>> } >>>> >>>> static int dwc2_usb_remove(struct udevice *dev) >>>> >>> >>> >>> >> >