diff mbox

[U-Boot,RESEND,v4] usb: dwc2: add support for external vbus supply

Message ID 1488804887-10974-1-git-send-email-kever.yang@rock-chips.com
State Superseded
Delegated to: Marek Vasut
Headers show

Commit Message

Kever Yang March 6, 2017, 12:54 p.m. UTC
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 <kever.yang@rock-chips.com>
Signed-off-by: Simon Glass <sjg@chromium.org>
---

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(-)

Comments

Marek Vasut March 7, 2017, 2:55 a.m. UTC | #1
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 <kever.yang@rock-chips.com>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> 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 <usbroothubdes.h>
>  #include <wait_bit.h>
>  #include <asm/io.h>
> +#include <power/regulator.h>
>
>  #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.

#ifdef (.....)
static foo()
{
  do stuff
}
#else
static foo() { return 0 }
#endif

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, &regs->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(&regs->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)
>
Kever Yang March 7, 2017, 6:50 a.m. UTC | #2
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 <kever.yang@rock-chips.com>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>>
>> 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 <usbroothubdes.h>
>>  #include <wait_bit.h>
>>  #include <asm/io.h>
>> +#include <power/regulator.h>
>>
>>  #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?

Thanks,
- Kever
>
> #ifdef (.....)
> static foo()
> {
>  do stuff
> }
> #else
> static foo() { return 0 }
> #endif
>
> 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, &regs->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(&regs->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)
>>
>
>
>
>
Marek Vasut March 10, 2017, 2:16 a.m. UTC | #3
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 <kever.yang@rock-chips.com>
>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>> ---
>>>
>>> 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 <usbroothubdes.h>
>>>  #include <wait_bit.h>
>>>  #include <asm/io.h>
>>> +#include <power/regulator.h>
>>>
>>>  #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,
> - 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, &regs->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(&regs->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)
>>>
>>
>>
>>
>>
> 
>
Kever Yang March 10, 2017, 4:02 a.m. UTC | #4
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 <kever.yang@rock-chips.com>
>>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>>> ---
>>>>
>>>> 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 <usbroothubdes.h>
>>>>   #include <wait_bit.h>
>>>>   #include <asm/io.h>
>>>> +#include <power/regulator.h>
>>>>
>>>>   #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, &regs->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(&regs->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)
>>>>
>>>
>>>
>>>
>>
>
Marek Vasut March 10, 2017, 4:50 a.m. UTC | #5
On 03/10/2017 05:02 AM, Kever Yang wrote:
> 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 <kever.yang@rock-chips.com>
>>>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>>>> ---
>>>>>
>>>>> 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 <usbroothubdes.h>
>>>>>   #include <wait_bit.h>
>>>>>   #include <asm/io.h>
>>>>> +#include <power/regulator.h>
>>>>>
>>>>>   #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?

It's more readable, you can clearly tell that the whole function is
conditionally compiled in. (and it's consistent with the rest of the
codebase)
diff mbox

Patch

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 <usbroothubdes.h>
 #include <wait_bit.h>
 #include <asm/io.h>
+#include <power/regulator.h>
 
 #include "dwc2.h"
 
@@ -159,6 +160,27 @@  static void dwc_otg_core_reset(struct dwc2_core_regs *regs)
 	mdelay(100);
 }
 
+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, &regs->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(&regs->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)