Re: [PATCH v3 2/2] usb: dwc2: add 'mode' which based on Kconfig select or dts setting

2014-08-06 Thread Doug Anderson
Paul,

On Tue, Aug 5, 2014 at 1:08 PM, Paul Zimmerman
 wrote:
>> Maybe dwc2 is refer to the Kconfig in dwc3 for there is a choice from
>> one of "Host only mode", "Gadget only mode" and "Dual Role mode",
>> which means the role is decided by the Kconfig.
>>
>> In my opinion, there maybe more than one controller in a Soc, and for
>> different
>> usage, the mode select should not be done in Kconfig, it's better to do that
>> in dts file.
>> I agree with you that the CONFIG_USB_DWC2_HOST should defined for host
>> functionality, not for role definition.
>>
>> Paul: what do you think?
>
> I'm confused about how your dual-role mode implementation is supposed
> to work. Right now, the host and peripheral modes are compiled as
> separate drivers, and there is no dual-role mode in the Kconfig. So I
> don't see how your code can work. Have you tested it?

Ah, interesting.  I just assumed that since you could compile both in
that dual mode was there, but I clearly was confused.

I don't think Kever had tested dual_role mode.


> Dinh Nguyen is working on a patch series to add dual-role support, I
> think you have seen that, right? That adds the Kconfig options for all
> three modes. Maybe you should build on top of that?

I hadn't personally seen it, but now I have!  I guess this is the
newest .

I haven't done a thorough review, but I think Kever's patch v4
 is still relevant since
I don't see anything parsing the dr_mode in Dinh's patches.

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v3 2/2] usb: dwc2: add 'mode' which based on Kconfig select or dts setting

2014-08-06 Thread Doug Anderson
Paul,

On Tue, Aug 5, 2014 at 1:08 PM, Paul Zimmerman
paul.zimmer...@synopsys.com wrote:
 Maybe dwc2 is refer to the Kconfig in dwc3 for there is a choice from
 one of Host only mode, Gadget only mode and Dual Role mode,
 which means the role is decided by the Kconfig.

 In my opinion, there maybe more than one controller in a Soc, and for
 different
 usage, the mode select should not be done in Kconfig, it's better to do that
 in dts file.
 I agree with you that the CONFIG_USB_DWC2_HOST should defined for host
 functionality, not for role definition.

 Paul: what do you think?

 I'm confused about how your dual-role mode implementation is supposed
 to work. Right now, the host and peripheral modes are compiled as
 separate drivers, and there is no dual-role mode in the Kconfig. So I
 don't see how your code can work. Have you tested it?

Ah, interesting.  I just assumed that since you could compile both in
that dual mode was there, but I clearly was confused.

I don't think Kever had tested dual_role mode.


 Dinh Nguyen is working on a patch series to add dual-role support, I
 think you have seen that, right? That adds the Kconfig options for all
 three modes. Maybe you should build on top of that?

I hadn't personally seen it, but now I have!  I guess this is the
newest https://www.mail-archive.com/linux-usb@vger.kernel.org/msg46103.html.

I haven't done a thorough review, but I think Kever's patch v4
https://patchwork.kernel.org/patch/4682711/ is still relevant since
I don't see anything parsing the dr_mode in Dinh's patches.

-Doug
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH v3 2/2] usb: dwc2: add 'mode' which based on Kconfig select or dts setting

2014-08-05 Thread Paul Zimmerman
> From: Kever Yang [mailto:kever.y...@rock-chips.com]
> Sent: Monday, August 04, 2014 6:46 PM
> 
> On 08/05/2014 12:34 AM, Doug Anderson wrote:
> 
> > Normally I'd say that you should have added Paul's "Acked-by" since
> > you fixed his nit and he told you to include his Acked-by when his nit
> > was fixed, but...
> There are some more changes than Paul's suggestion, so I'm not sure
> if Paul need more review to give me the "Acked-by", I get it now.

Yes, you did the right thing by leaving off my ack (although you should
have mentioned the additional changes in your email).

> >> diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
> >> index 27d2c9b..738bec2 100644
> >> --- a/drivers/usb/dwc2/core.c
> >> +++ b/drivers/usb/dwc2/core.c
> >> @@ -118,6 +118,7 @@ static int dwc2_core_reset(struct dwc2_hsotg *hsotg)
> >>   {
> >>  u32 greset;
> >>  int count = 0;
> >> +   u32 gusbcfg;
> >>
> >>  dev_vdbg(hsotg->dev, "%s()\n", __func__);
> >>
> >> @@ -148,6 +149,23 @@ static int dwc2_core_reset(struct dwc2_hsotg *hsotg)
> >>  }
> >>  } while (greset & GRSTCTL_CSFTRST);
> >>
> >> +   if (hsotg->dr_mode == USB_DR_MODE_HOST) {
> >> +   gusbcfg = readl(hsotg->regs + GUSBCFG);
> >> +   gusbcfg &= ~GUSBCFG_FORCEDEVMODE;
> >> +   gusbcfg |= GUSBCFG_FORCEHOSTMODE;
> >> +   writel(gusbcfg, hsotg->regs + GUSBCFG);
> >> +   } else if (hsotg->dr_mode == USB_DR_MODE_PERIPHERAL) {
> >> +   gusbcfg = readl(hsotg->regs + GUSBCFG);
> >> +   gusbcfg &= ~GUSBCFG_FORCEHOSTMODE;
> >> +   gusbcfg |= GUSBCFG_FORCEDEVMODE;
> >> +   writel(gusbcfg, hsotg->regs + GUSBCFG);
> >> +   } else if (hsotg->dr_mode == USB_DR_MODE_OTG) {
> >> +   gusbcfg = readl(hsotg->regs + GUSBCFG);
> >> +   gusbcfg &= ~GUSBCFG_FORCEHOSTMODE;
> >> +   gusbcfg &= ~GUSBCFG_FORCEDEVMODE;
> >> +   writel(gusbcfg, hsotg->regs + GUSBCFG);
> >> +   }
> >> +
> >>  /*
> >>   * NOTE: This long sleep is _very_ important, otherwise the core 
> >> will
> >>   * not stay in host mode after a connector ID change!
> >> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> >> index 1efd10c..52a4fd2 100644
> >> --- a/drivers/usb/dwc2/core.h
> >> +++ b/drivers/usb/dwc2/core.h
> >> @@ -501,6 +501,10 @@ struct dwc2_hw_params {
> >>*  a_peripheral and b_device=>b_host) this may not 
> >> match
> >>*  the core, but allows the software to determine
> >>*  transitions
> >> + * @dr_mode:Requested mode of operation, one of following:
> >> + *  - USB_DR_MODE_PERIPHERAL
> >> + *  - USB_DR_MODE_HOST
> >> + *  - USB_DR_MODE_OTG
> >>* @queuing_high_bandwidth: True if multiple packets of a high-bandwidth
> >>*  transfer are in process of being queued
> >>* @srp_success:Stores status of SRP request in the case of a FS 
> >> PHY
> >> @@ -592,6 +596,7 @@ struct dwc2_hsotg {
> >>  /** Params to actually use */
> >>  struct dwc2_core_params *core_params;
> >>  enum usb_otg_state op_state;
> >> +   enum usb_dr_mode dr_mode;
> >>
> >>  unsigned int queuing_high_bandwidth:1;
> >>  unsigned int srp_success:1;
> >> diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
> >> index a10e7a3..4d2c738 100644
> >> --- a/drivers/usb/dwc2/platform.c
> >> +++ b/drivers/usb/dwc2/platform.c
> >> @@ -42,6 +42,8 @@
> >>   #include 
> >>   #include 
> >>
> >> +#include 
> >> +
> >>   #include "core.h"
> >>   #include "hcd.h"
> >>
> >> @@ -171,6 +173,16 @@ static int dwc2_driver_probe(struct platform_device 
> >> *dev)
> >>  dev_dbg(>dev, "mapped PA %08lx to VA %p\n",
> >>  (unsigned long)res->start, hsotg->regs);
> >>
> >> +   hsotg->dr_mode = of_usb_get_dr_mode(dev->dev.of_node);
> >> +
> >> +   if (IS_ENABLED(CONFIG_USB_DWC2_HOST))
> >> +   hsotg->dr_mode = USB_DR_MODE_HOST;
> >> +   else if (IS_ENABLED(CONFIG_USB_DWC2_GADGET))
> >> +   hsotg->dr_mode = USB_DR_MODE_PERIPHERAL;
> >> +
> >> +   if (hsotg->dr_mode == USB_DR_MODE_UNKNOWN)
> >> +   hsotg->dr_mode = USB_DR_MODE_OTG;
> >> +
> > Please remove this whole chunk.  Specifically:
> >
> > * CONFIG_USB_DWC2_GADGET is not a config option and is nowhere in any
> > KConfig files.
> This should be USB_DWC2_PERIPHERAL, and I just copy it wrong from dwc3.
> >
> > * CONFIG_USB_DWC2_HOST claims in Kconfig that we'll be in "host only"
> > mode, but I don't think that's right.  The only way you'll get any
> > host functionality is if CONFIG_USB_DWC2_HOST is defined, so we'd need
> > that defined even for OTG mode.
> Yeap, I add the chunk above because I'm confused by "Host only mode" for
> 

RE: [PATCH v3 2/2] usb: dwc2: add 'mode' which based on Kconfig select or dts setting

2014-08-05 Thread Paul Zimmerman
 From: Kever Yang [mailto:kever.y...@rock-chips.com]
 Sent: Monday, August 04, 2014 6:46 PM
 
 On 08/05/2014 12:34 AM, Doug Anderson wrote:
 
  Normally I'd say that you should have added Paul's Acked-by since
  you fixed his nit and he told you to include his Acked-by when his nit
  was fixed, but...
 There are some more changes than Paul's suggestion, so I'm not sure
 if Paul need more review to give me the Acked-by, I get it now.

Yes, you did the right thing by leaving off my ack (although you should
have mentioned the additional changes in your email).

  diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
  index 27d2c9b..738bec2 100644
  --- a/drivers/usb/dwc2/core.c
  +++ b/drivers/usb/dwc2/core.c
  @@ -118,6 +118,7 @@ static int dwc2_core_reset(struct dwc2_hsotg *hsotg)
{
   u32 greset;
   int count = 0;
  +   u32 gusbcfg;
 
   dev_vdbg(hsotg-dev, %s()\n, __func__);
 
  @@ -148,6 +149,23 @@ static int dwc2_core_reset(struct dwc2_hsotg *hsotg)
   }
   } while (greset  GRSTCTL_CSFTRST);
 
  +   if (hsotg-dr_mode == USB_DR_MODE_HOST) {
  +   gusbcfg = readl(hsotg-regs + GUSBCFG);
  +   gusbcfg = ~GUSBCFG_FORCEDEVMODE;
  +   gusbcfg |= GUSBCFG_FORCEHOSTMODE;
  +   writel(gusbcfg, hsotg-regs + GUSBCFG);
  +   } else if (hsotg-dr_mode == USB_DR_MODE_PERIPHERAL) {
  +   gusbcfg = readl(hsotg-regs + GUSBCFG);
  +   gusbcfg = ~GUSBCFG_FORCEHOSTMODE;
  +   gusbcfg |= GUSBCFG_FORCEDEVMODE;
  +   writel(gusbcfg, hsotg-regs + GUSBCFG);
  +   } else if (hsotg-dr_mode == USB_DR_MODE_OTG) {
  +   gusbcfg = readl(hsotg-regs + GUSBCFG);
  +   gusbcfg = ~GUSBCFG_FORCEHOSTMODE;
  +   gusbcfg = ~GUSBCFG_FORCEDEVMODE;
  +   writel(gusbcfg, hsotg-regs + GUSBCFG);
  +   }
  +
   /*
* NOTE: This long sleep is _very_ important, otherwise the core 
  will
* not stay in host mode after a connector ID change!
  diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
  index 1efd10c..52a4fd2 100644
  --- a/drivers/usb/dwc2/core.h
  +++ b/drivers/usb/dwc2/core.h
  @@ -501,6 +501,10 @@ struct dwc2_hw_params {
 *  a_peripheral and b_device=b_host) this may not 
  match
 *  the core, but allows the software to determine
 *  transitions
  + * @dr_mode:Requested mode of operation, one of following:
  + *  - USB_DR_MODE_PERIPHERAL
  + *  - USB_DR_MODE_HOST
  + *  - USB_DR_MODE_OTG
 * @queuing_high_bandwidth: True if multiple packets of a high-bandwidth
 *  transfer are in process of being queued
 * @srp_success:Stores status of SRP request in the case of a FS 
  PHY
  @@ -592,6 +596,7 @@ struct dwc2_hsotg {
   /** Params to actually use */
   struct dwc2_core_params *core_params;
   enum usb_otg_state op_state;
  +   enum usb_dr_mode dr_mode;
 
   unsigned int queuing_high_bandwidth:1;
   unsigned int srp_success:1;
  diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
  index a10e7a3..4d2c738 100644
  --- a/drivers/usb/dwc2/platform.c
  +++ b/drivers/usb/dwc2/platform.c
  @@ -42,6 +42,8 @@
#include linux/of_device.h
#include linux/platform_device.h
 
  +#include linux/usb/of.h
  +
#include core.h
#include hcd.h
 
  @@ -171,6 +173,16 @@ static int dwc2_driver_probe(struct platform_device 
  *dev)
   dev_dbg(dev-dev, mapped PA %08lx to VA %p\n,
   (unsigned long)res-start, hsotg-regs);
 
  +   hsotg-dr_mode = of_usb_get_dr_mode(dev-dev.of_node);
  +
  +   if (IS_ENABLED(CONFIG_USB_DWC2_HOST))
  +   hsotg-dr_mode = USB_DR_MODE_HOST;
  +   else if (IS_ENABLED(CONFIG_USB_DWC2_GADGET))
  +   hsotg-dr_mode = USB_DR_MODE_PERIPHERAL;
  +
  +   if (hsotg-dr_mode == USB_DR_MODE_UNKNOWN)
  +   hsotg-dr_mode = USB_DR_MODE_OTG;
  +
  Please remove this whole chunk.  Specifically:
 
  * CONFIG_USB_DWC2_GADGET is not a config option and is nowhere in any
  KConfig files.
 This should be USB_DWC2_PERIPHERAL, and I just copy it wrong from dwc3.
 
  * CONFIG_USB_DWC2_HOST claims in Kconfig that we'll be in host only
  mode, but I don't think that's right.  The only way you'll get any
  host functionality is if CONFIG_USB_DWC2_HOST is defined, so we'd need
  that defined even for OTG mode.
 Yeap, I add the chunk above because I'm confused by Host only mode for
 CONFIG_USB_DWC2_HOST in Kconfig. If it's the Host only mode, I should
 add the dr_mode for it to make sure the controller works in host mode.
 
 Maybe dwc2 is refer to the Kconfig in dwc3 for there is a choice from
 one of Host only mode, Gadget only mode and Dual Role mode,
 which means the role is 

Re: [PATCH v3 2/2] usb: dwc2: add 'mode' which based on Kconfig select or dts setting

2014-08-04 Thread Doug Anderson
Kever,

On Mon, Aug 4, 2014 at 6:45 PM, Kever Yang  wrote:
> Doug,
>
>
> On 08/05/2014 12:34 AM, Doug Anderson wrote:
>>
>> Kever,
>>
>> On Mon, Aug 4, 2014 at 6:45 AM, Kever Yang 
>> wrote:
>>>
>>> According to the "dr_mode", the otg controller can work as
>>> device role during firmware period, and work as host role in
>>> the kernel, without use of usb_id pin. As the commit "usb: dwc3:
>>> set 'mode' based on selected Kconfig choices".
>>
>> I don't think you need to mention firmware / kernel here.  Just say
>> that on some boards we always want to use host mode and on other
>> boards we want to use gadget mode.  ...and that we don't necessarily
>> have the ID pin hooked up to make this automatic.
>
> OK, I'll change this message again, I just don't know how to describe
> to make everyone understand what I'm doing.
>
>>
>>
>>> Signed-off-by: Kever Yang 
>>
>> Normally I'd say that you should have added Paul's "Acked-by" since
>> you fixed his nit and he told you to include his Acked-by when his nit
>> was fixed, but...
>
> There are some more changes than Paul's suggestion, so I'm not sure
> if Paul need more review to give me the "Acked-by", I get it now.
>
>>
>>
>>> ---
>>>
>>> Changes in v3:
>>> - fix the odd spacing in dwc2_hsotg struct
>>> - From Jingoo's suggestion:
>>>  change the commit message
>>
>> You did more than just this.  You also added some (incorrect) Kconfig
>> things.  See below.
>>
>>
>>> Changes in v2:
>>> - put spaces around '+' operator
>>> - expand the comment for dr_mode
>>> - handle dr_mode is USB_DR_MODE_OTG
>>>
>>>   drivers/usb/dwc2/core.c | 18 ++
>>>   drivers/usb/dwc2/core.h |  5 +
>>>   drivers/usb/dwc2/platform.c | 12 
>>>   3 files changed, 35 insertions(+)
>>>
>>> diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
>>> index 27d2c9b..738bec2 100644
>>> --- a/drivers/usb/dwc2/core.c
>>> +++ b/drivers/usb/dwc2/core.c
>>> @@ -118,6 +118,7 @@ static int dwc2_core_reset(struct dwc2_hsotg *hsotg)
>>>   {
>>>  u32 greset;
>>>  int count = 0;
>>> +   u32 gusbcfg;
>>>
>>>  dev_vdbg(hsotg->dev, "%s()\n", __func__);
>>>
>>> @@ -148,6 +149,23 @@ static int dwc2_core_reset(struct dwc2_hsotg *hsotg)
>>>  }
>>>  } while (greset & GRSTCTL_CSFTRST);
>>>
>>> +   if (hsotg->dr_mode == USB_DR_MODE_HOST) {
>>> +   gusbcfg = readl(hsotg->regs + GUSBCFG);
>>> +   gusbcfg &= ~GUSBCFG_FORCEDEVMODE;
>>> +   gusbcfg |= GUSBCFG_FORCEHOSTMODE;
>>> +   writel(gusbcfg, hsotg->regs + GUSBCFG);
>>> +   } else if (hsotg->dr_mode == USB_DR_MODE_PERIPHERAL) {
>>> +   gusbcfg = readl(hsotg->regs + GUSBCFG);
>>> +   gusbcfg &= ~GUSBCFG_FORCEHOSTMODE;
>>> +   gusbcfg |= GUSBCFG_FORCEDEVMODE;
>>> +   writel(gusbcfg, hsotg->regs + GUSBCFG);
>>> +   } else if (hsotg->dr_mode == USB_DR_MODE_OTG) {
>>> +   gusbcfg = readl(hsotg->regs + GUSBCFG);
>>> +   gusbcfg &= ~GUSBCFG_FORCEHOSTMODE;
>>> +   gusbcfg &= ~GUSBCFG_FORCEDEVMODE;
>>> +   writel(gusbcfg, hsotg->regs + GUSBCFG);
>>> +   }
>>> +
>>>  /*
>>>   * NOTE: This long sleep is _very_ important, otherwise the core
>>> will
>>>   * not stay in host mode after a connector ID change!
>>> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
>>> index 1efd10c..52a4fd2 100644
>>> --- a/drivers/usb/dwc2/core.h
>>> +++ b/drivers/usb/dwc2/core.h
>>> @@ -501,6 +501,10 @@ struct dwc2_hw_params {
>>>*  a_peripheral and b_device=>b_host) this may not
>>> match
>>>*  the core, but allows the software to determine
>>>*  transitions
>>> + * @dr_mode:Requested mode of operation, one of following:
>>> + *  - USB_DR_MODE_PERIPHERAL
>>> + *  - USB_DR_MODE_HOST
>>> + *  - USB_DR_MODE_OTG
>>>* @queuing_high_bandwidth: True if multiple packets of a
>>> high-bandwidth
>>>*  transfer are in process of being queued
>>>* @srp_success:Stores status of SRP request in the case of a
>>> FS PHY
>>> @@ -592,6 +596,7 @@ struct dwc2_hsotg {
>>>  /** Params to actually use */
>>>  struct dwc2_core_params *core_params;
>>>  enum usb_otg_state op_state;
>>> +   enum usb_dr_mode dr_mode;
>>>
>>>  unsigned int queuing_high_bandwidth:1;
>>>  unsigned int srp_success:1;
>>> diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
>>> index a10e7a3..4d2c738 100644
>>> --- a/drivers/usb/dwc2/platform.c
>>> +++ b/drivers/usb/dwc2/platform.c
>>> @@ -42,6 +42,8 @@
>>>   #include 
>>>   #include 
>>>
>>> +#include 
>>> +
>>>   #include "core.h"
>>>   #include "hcd.h"
>>>
>>> @@ -171,6 +173,16 @@ static int dwc2_driver_probe(struct platform_device
>>> *dev)
>>>  

Re: [PATCH v3 2/2] usb: dwc2: add 'mode' which based on Kconfig select or dts setting

2014-08-04 Thread Kever Yang

Doug,

On 08/05/2014 12:34 AM, Doug Anderson wrote:

Kever,

On Mon, Aug 4, 2014 at 6:45 AM, Kever Yang  wrote:

According to the "dr_mode", the otg controller can work as
device role during firmware period, and work as host role in
the kernel, without use of usb_id pin. As the commit "usb: dwc3:
set 'mode' based on selected Kconfig choices".

I don't think you need to mention firmware / kernel here.  Just say
that on some boards we always want to use host mode and on other
boards we want to use gadget mode.  ...and that we don't necessarily
have the ID pin hooked up to make this automatic.

OK, I'll change this message again, I just don't know how to describe
to make everyone understand what I'm doing.




Signed-off-by: Kever Yang 

Normally I'd say that you should have added Paul's "Acked-by" since
you fixed his nit and he told you to include his Acked-by when his nit
was fixed, but...

There are some more changes than Paul's suggestion, so I'm not sure
if Paul need more review to give me the "Acked-by", I get it now.




---

Changes in v3:
- fix the odd spacing in dwc2_hsotg struct
- From Jingoo's suggestion:
 change the commit message

You did more than just this.  You also added some (incorrect) Kconfig
things.  See below.



Changes in v2:
- put spaces around '+' operator
- expand the comment for dr_mode
- handle dr_mode is USB_DR_MODE_OTG

  drivers/usb/dwc2/core.c | 18 ++
  drivers/usb/dwc2/core.h |  5 +
  drivers/usb/dwc2/platform.c | 12 
  3 files changed, 35 insertions(+)

diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index 27d2c9b..738bec2 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -118,6 +118,7 @@ static int dwc2_core_reset(struct dwc2_hsotg *hsotg)
  {
 u32 greset;
 int count = 0;
+   u32 gusbcfg;

 dev_vdbg(hsotg->dev, "%s()\n", __func__);

@@ -148,6 +149,23 @@ static int dwc2_core_reset(struct dwc2_hsotg *hsotg)
 }
 } while (greset & GRSTCTL_CSFTRST);

+   if (hsotg->dr_mode == USB_DR_MODE_HOST) {
+   gusbcfg = readl(hsotg->regs + GUSBCFG);
+   gusbcfg &= ~GUSBCFG_FORCEDEVMODE;
+   gusbcfg |= GUSBCFG_FORCEHOSTMODE;
+   writel(gusbcfg, hsotg->regs + GUSBCFG);
+   } else if (hsotg->dr_mode == USB_DR_MODE_PERIPHERAL) {
+   gusbcfg = readl(hsotg->regs + GUSBCFG);
+   gusbcfg &= ~GUSBCFG_FORCEHOSTMODE;
+   gusbcfg |= GUSBCFG_FORCEDEVMODE;
+   writel(gusbcfg, hsotg->regs + GUSBCFG);
+   } else if (hsotg->dr_mode == USB_DR_MODE_OTG) {
+   gusbcfg = readl(hsotg->regs + GUSBCFG);
+   gusbcfg &= ~GUSBCFG_FORCEHOSTMODE;
+   gusbcfg &= ~GUSBCFG_FORCEDEVMODE;
+   writel(gusbcfg, hsotg->regs + GUSBCFG);
+   }
+
 /*
  * NOTE: This long sleep is _very_ important, otherwise the core will
  * not stay in host mode after a connector ID change!
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 1efd10c..52a4fd2 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -501,6 +501,10 @@ struct dwc2_hw_params {
   *  a_peripheral and b_device=>b_host) this may not match
   *  the core, but allows the software to determine
   *  transitions
+ * @dr_mode:Requested mode of operation, one of following:
+ *  - USB_DR_MODE_PERIPHERAL
+ *  - USB_DR_MODE_HOST
+ *  - USB_DR_MODE_OTG
   * @queuing_high_bandwidth: True if multiple packets of a high-bandwidth
   *  transfer are in process of being queued
   * @srp_success:Stores status of SRP request in the case of a FS PHY
@@ -592,6 +596,7 @@ struct dwc2_hsotg {
 /** Params to actually use */
 struct dwc2_core_params *core_params;
 enum usb_otg_state op_state;
+   enum usb_dr_mode dr_mode;

 unsigned int queuing_high_bandwidth:1;
 unsigned int srp_success:1;
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index a10e7a3..4d2c738 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -42,6 +42,8 @@
  #include 
  #include 

+#include 
+
  #include "core.h"
  #include "hcd.h"

@@ -171,6 +173,16 @@ static int dwc2_driver_probe(struct platform_device *dev)
 dev_dbg(>dev, "mapped PA %08lx to VA %p\n",
 (unsigned long)res->start, hsotg->regs);

+   hsotg->dr_mode = of_usb_get_dr_mode(dev->dev.of_node);
+
+   if (IS_ENABLED(CONFIG_USB_DWC2_HOST))
+   hsotg->dr_mode = USB_DR_MODE_HOST;
+   else if (IS_ENABLED(CONFIG_USB_DWC2_GADGET))
+   hsotg->dr_mode = USB_DR_MODE_PERIPHERAL;
+
+   if (hsotg->dr_mode == USB_DR_MODE_UNKNOWN)
+   hsotg->dr_mode = USB_DR_MODE_OTG;
+

Please remove this whole chunk.  Specifically:

Re: [PATCH v3 2/2] usb: dwc2: add 'mode' which based on Kconfig select or dts setting

2014-08-04 Thread Doug Anderson
Kever,

On Mon, Aug 4, 2014 at 6:45 AM, Kever Yang  wrote:
> According to the "dr_mode", the otg controller can work as
> device role during firmware period, and work as host role in
> the kernel, without use of usb_id pin. As the commit "usb: dwc3:
> set 'mode' based on selected Kconfig choices".

I don't think you need to mention firmware / kernel here.  Just say
that on some boards we always want to use host mode and on other
boards we want to use gadget mode.  ...and that we don't necessarily
have the ID pin hooked up to make this automatic.


> Signed-off-by: Kever Yang 

Normally I'd say that you should have added Paul's "Acked-by" since
you fixed his nit and he told you to include his Acked-by when his nit
was fixed, but...


> ---
>
> Changes in v3:
> - fix the odd spacing in dwc2_hsotg struct
> - From Jingoo's suggestion:
> change the commit message

You did more than just this.  You also added some (incorrect) Kconfig
things.  See below.


> Changes in v2:
> - put spaces around '+' operator
> - expand the comment for dr_mode
> - handle dr_mode is USB_DR_MODE_OTG
>
>  drivers/usb/dwc2/core.c | 18 ++
>  drivers/usb/dwc2/core.h |  5 +
>  drivers/usb/dwc2/platform.c | 12 
>  3 files changed, 35 insertions(+)
>
> diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
> index 27d2c9b..738bec2 100644
> --- a/drivers/usb/dwc2/core.c
> +++ b/drivers/usb/dwc2/core.c
> @@ -118,6 +118,7 @@ static int dwc2_core_reset(struct dwc2_hsotg *hsotg)
>  {
> u32 greset;
> int count = 0;
> +   u32 gusbcfg;
>
> dev_vdbg(hsotg->dev, "%s()\n", __func__);
>
> @@ -148,6 +149,23 @@ static int dwc2_core_reset(struct dwc2_hsotg *hsotg)
> }
> } while (greset & GRSTCTL_CSFTRST);
>
> +   if (hsotg->dr_mode == USB_DR_MODE_HOST) {
> +   gusbcfg = readl(hsotg->regs + GUSBCFG);
> +   gusbcfg &= ~GUSBCFG_FORCEDEVMODE;
> +   gusbcfg |= GUSBCFG_FORCEHOSTMODE;
> +   writel(gusbcfg, hsotg->regs + GUSBCFG);
> +   } else if (hsotg->dr_mode == USB_DR_MODE_PERIPHERAL) {
> +   gusbcfg = readl(hsotg->regs + GUSBCFG);
> +   gusbcfg &= ~GUSBCFG_FORCEHOSTMODE;
> +   gusbcfg |= GUSBCFG_FORCEDEVMODE;
> +   writel(gusbcfg, hsotg->regs + GUSBCFG);
> +   } else if (hsotg->dr_mode == USB_DR_MODE_OTG) {
> +   gusbcfg = readl(hsotg->regs + GUSBCFG);
> +   gusbcfg &= ~GUSBCFG_FORCEHOSTMODE;
> +   gusbcfg &= ~GUSBCFG_FORCEDEVMODE;
> +   writel(gusbcfg, hsotg->regs + GUSBCFG);
> +   }
> +
> /*
>  * NOTE: This long sleep is _very_ important, otherwise the core will
>  * not stay in host mode after a connector ID change!
> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> index 1efd10c..52a4fd2 100644
> --- a/drivers/usb/dwc2/core.h
> +++ b/drivers/usb/dwc2/core.h
> @@ -501,6 +501,10 @@ struct dwc2_hw_params {
>   *  a_peripheral and b_device=>b_host) this may not match
>   *  the core, but allows the software to determine
>   *  transitions
> + * @dr_mode:Requested mode of operation, one of following:
> + *  - USB_DR_MODE_PERIPHERAL
> + *  - USB_DR_MODE_HOST
> + *  - USB_DR_MODE_OTG
>   * @queuing_high_bandwidth: True if multiple packets of a high-bandwidth
>   *  transfer are in process of being queued
>   * @srp_success:Stores status of SRP request in the case of a FS PHY
> @@ -592,6 +596,7 @@ struct dwc2_hsotg {
> /** Params to actually use */
> struct dwc2_core_params *core_params;
> enum usb_otg_state op_state;
> +   enum usb_dr_mode dr_mode;
>
> unsigned int queuing_high_bandwidth:1;
> unsigned int srp_success:1;
> diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
> index a10e7a3..4d2c738 100644
> --- a/drivers/usb/dwc2/platform.c
> +++ b/drivers/usb/dwc2/platform.c
> @@ -42,6 +42,8 @@
>  #include 
>  #include 
>
> +#include 
> +
>  #include "core.h"
>  #include "hcd.h"
>
> @@ -171,6 +173,16 @@ static int dwc2_driver_probe(struct platform_device *dev)
> dev_dbg(>dev, "mapped PA %08lx to VA %p\n",
> (unsigned long)res->start, hsotg->regs);
>
> +   hsotg->dr_mode = of_usb_get_dr_mode(dev->dev.of_node);
> +
> +   if (IS_ENABLED(CONFIG_USB_DWC2_HOST))
> +   hsotg->dr_mode = USB_DR_MODE_HOST;
> +   else if (IS_ENABLED(CONFIG_USB_DWC2_GADGET))
> +   hsotg->dr_mode = USB_DR_MODE_PERIPHERAL;
> +
> +   if (hsotg->dr_mode == USB_DR_MODE_UNKNOWN)
> +   hsotg->dr_mode = USB_DR_MODE_OTG;
> +

Please remove this whole chunk.  Specifically:

* CONFIG_USB_DWC2_GADGET is not a config option and is nowhere in any
KConfig files.

* CONFIG_USB_DWC2_HOST claims in Kconfig that 

Re: [PATCH v3 2/2] usb: dwc2: add 'mode' which based on Kconfig select or dts setting

2014-08-04 Thread Doug Anderson
Kever,

On Mon, Aug 4, 2014 at 6:45 AM, Kever Yang kever.y...@rock-chips.com wrote:
 According to the dr_mode, the otg controller can work as
 device role during firmware period, and work as host role in
 the kernel, without use of usb_id pin. As the commit usb: dwc3:
 set 'mode' based on selected Kconfig choices.

I don't think you need to mention firmware / kernel here.  Just say
that on some boards we always want to use host mode and on other
boards we want to use gadget mode.  ...and that we don't necessarily
have the ID pin hooked up to make this automatic.


 Signed-off-by: Kever Yang kever.y...@rock-chips.com

Normally I'd say that you should have added Paul's Acked-by since
you fixed his nit and he told you to include his Acked-by when his nit
was fixed, but...


 ---

 Changes in v3:
 - fix the odd spacing in dwc2_hsotg struct
 - From Jingoo's suggestion:
 change the commit message

You did more than just this.  You also added some (incorrect) Kconfig
things.  See below.


 Changes in v2:
 - put spaces around '+' operator
 - expand the comment for dr_mode
 - handle dr_mode is USB_DR_MODE_OTG

  drivers/usb/dwc2/core.c | 18 ++
  drivers/usb/dwc2/core.h |  5 +
  drivers/usb/dwc2/platform.c | 12 
  3 files changed, 35 insertions(+)

 diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
 index 27d2c9b..738bec2 100644
 --- a/drivers/usb/dwc2/core.c
 +++ b/drivers/usb/dwc2/core.c
 @@ -118,6 +118,7 @@ static int dwc2_core_reset(struct dwc2_hsotg *hsotg)
  {
 u32 greset;
 int count = 0;
 +   u32 gusbcfg;

 dev_vdbg(hsotg-dev, %s()\n, __func__);

 @@ -148,6 +149,23 @@ static int dwc2_core_reset(struct dwc2_hsotg *hsotg)
 }
 } while (greset  GRSTCTL_CSFTRST);

 +   if (hsotg-dr_mode == USB_DR_MODE_HOST) {
 +   gusbcfg = readl(hsotg-regs + GUSBCFG);
 +   gusbcfg = ~GUSBCFG_FORCEDEVMODE;
 +   gusbcfg |= GUSBCFG_FORCEHOSTMODE;
 +   writel(gusbcfg, hsotg-regs + GUSBCFG);
 +   } else if (hsotg-dr_mode == USB_DR_MODE_PERIPHERAL) {
 +   gusbcfg = readl(hsotg-regs + GUSBCFG);
 +   gusbcfg = ~GUSBCFG_FORCEHOSTMODE;
 +   gusbcfg |= GUSBCFG_FORCEDEVMODE;
 +   writel(gusbcfg, hsotg-regs + GUSBCFG);
 +   } else if (hsotg-dr_mode == USB_DR_MODE_OTG) {
 +   gusbcfg = readl(hsotg-regs + GUSBCFG);
 +   gusbcfg = ~GUSBCFG_FORCEHOSTMODE;
 +   gusbcfg = ~GUSBCFG_FORCEDEVMODE;
 +   writel(gusbcfg, hsotg-regs + GUSBCFG);
 +   }
 +
 /*
  * NOTE: This long sleep is _very_ important, otherwise the core will
  * not stay in host mode after a connector ID change!
 diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
 index 1efd10c..52a4fd2 100644
 --- a/drivers/usb/dwc2/core.h
 +++ b/drivers/usb/dwc2/core.h
 @@ -501,6 +501,10 @@ struct dwc2_hw_params {
   *  a_peripheral and b_device=b_host) this may not match
   *  the core, but allows the software to determine
   *  transitions
 + * @dr_mode:Requested mode of operation, one of following:
 + *  - USB_DR_MODE_PERIPHERAL
 + *  - USB_DR_MODE_HOST
 + *  - USB_DR_MODE_OTG
   * @queuing_high_bandwidth: True if multiple packets of a high-bandwidth
   *  transfer are in process of being queued
   * @srp_success:Stores status of SRP request in the case of a FS PHY
 @@ -592,6 +596,7 @@ struct dwc2_hsotg {
 /** Params to actually use */
 struct dwc2_core_params *core_params;
 enum usb_otg_state op_state;
 +   enum usb_dr_mode dr_mode;

 unsigned int queuing_high_bandwidth:1;
 unsigned int srp_success:1;
 diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
 index a10e7a3..4d2c738 100644
 --- a/drivers/usb/dwc2/platform.c
 +++ b/drivers/usb/dwc2/platform.c
 @@ -42,6 +42,8 @@
  #include linux/of_device.h
  #include linux/platform_device.h

 +#include linux/usb/of.h
 +
  #include core.h
  #include hcd.h

 @@ -171,6 +173,16 @@ static int dwc2_driver_probe(struct platform_device *dev)
 dev_dbg(dev-dev, mapped PA %08lx to VA %p\n,
 (unsigned long)res-start, hsotg-regs);

 +   hsotg-dr_mode = of_usb_get_dr_mode(dev-dev.of_node);
 +
 +   if (IS_ENABLED(CONFIG_USB_DWC2_HOST))
 +   hsotg-dr_mode = USB_DR_MODE_HOST;
 +   else if (IS_ENABLED(CONFIG_USB_DWC2_GADGET))
 +   hsotg-dr_mode = USB_DR_MODE_PERIPHERAL;
 +
 +   if (hsotg-dr_mode == USB_DR_MODE_UNKNOWN)
 +   hsotg-dr_mode = USB_DR_MODE_OTG;
 +

Please remove this whole chunk.  Specifically:

* CONFIG_USB_DWC2_GADGET is not a config option and is nowhere in any
KConfig files.

* CONFIG_USB_DWC2_HOST claims in Kconfig that we'll be in host only
mode, but I don't 

Re: [PATCH v3 2/2] usb: dwc2: add 'mode' which based on Kconfig select or dts setting

2014-08-04 Thread Kever Yang

Doug,

On 08/05/2014 12:34 AM, Doug Anderson wrote:

Kever,

On Mon, Aug 4, 2014 at 6:45 AM, Kever Yang kever.y...@rock-chips.com wrote:

According to the dr_mode, the otg controller can work as
device role during firmware period, and work as host role in
the kernel, without use of usb_id pin. As the commit usb: dwc3:
set 'mode' based on selected Kconfig choices.

I don't think you need to mention firmware / kernel here.  Just say
that on some boards we always want to use host mode and on other
boards we want to use gadget mode.  ...and that we don't necessarily
have the ID pin hooked up to make this automatic.

OK, I'll change this message again, I just don't know how to describe
to make everyone understand what I'm doing.




Signed-off-by: Kever Yang kever.y...@rock-chips.com

Normally I'd say that you should have added Paul's Acked-by since
you fixed his nit and he told you to include his Acked-by when his nit
was fixed, but...

There are some more changes than Paul's suggestion, so I'm not sure
if Paul need more review to give me the Acked-by, I get it now.




---

Changes in v3:
- fix the odd spacing in dwc2_hsotg struct
- From Jingoo's suggestion:
 change the commit message

You did more than just this.  You also added some (incorrect) Kconfig
things.  See below.



Changes in v2:
- put spaces around '+' operator
- expand the comment for dr_mode
- handle dr_mode is USB_DR_MODE_OTG

  drivers/usb/dwc2/core.c | 18 ++
  drivers/usb/dwc2/core.h |  5 +
  drivers/usb/dwc2/platform.c | 12 
  3 files changed, 35 insertions(+)

diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index 27d2c9b..738bec2 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -118,6 +118,7 @@ static int dwc2_core_reset(struct dwc2_hsotg *hsotg)
  {
 u32 greset;
 int count = 0;
+   u32 gusbcfg;

 dev_vdbg(hsotg-dev, %s()\n, __func__);

@@ -148,6 +149,23 @@ static int dwc2_core_reset(struct dwc2_hsotg *hsotg)
 }
 } while (greset  GRSTCTL_CSFTRST);

+   if (hsotg-dr_mode == USB_DR_MODE_HOST) {
+   gusbcfg = readl(hsotg-regs + GUSBCFG);
+   gusbcfg = ~GUSBCFG_FORCEDEVMODE;
+   gusbcfg |= GUSBCFG_FORCEHOSTMODE;
+   writel(gusbcfg, hsotg-regs + GUSBCFG);
+   } else if (hsotg-dr_mode == USB_DR_MODE_PERIPHERAL) {
+   gusbcfg = readl(hsotg-regs + GUSBCFG);
+   gusbcfg = ~GUSBCFG_FORCEHOSTMODE;
+   gusbcfg |= GUSBCFG_FORCEDEVMODE;
+   writel(gusbcfg, hsotg-regs + GUSBCFG);
+   } else if (hsotg-dr_mode == USB_DR_MODE_OTG) {
+   gusbcfg = readl(hsotg-regs + GUSBCFG);
+   gusbcfg = ~GUSBCFG_FORCEHOSTMODE;
+   gusbcfg = ~GUSBCFG_FORCEDEVMODE;
+   writel(gusbcfg, hsotg-regs + GUSBCFG);
+   }
+
 /*
  * NOTE: This long sleep is _very_ important, otherwise the core will
  * not stay in host mode after a connector ID change!
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 1efd10c..52a4fd2 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -501,6 +501,10 @@ struct dwc2_hw_params {
   *  a_peripheral and b_device=b_host) this may not match
   *  the core, but allows the software to determine
   *  transitions
+ * @dr_mode:Requested mode of operation, one of following:
+ *  - USB_DR_MODE_PERIPHERAL
+ *  - USB_DR_MODE_HOST
+ *  - USB_DR_MODE_OTG
   * @queuing_high_bandwidth: True if multiple packets of a high-bandwidth
   *  transfer are in process of being queued
   * @srp_success:Stores status of SRP request in the case of a FS PHY
@@ -592,6 +596,7 @@ struct dwc2_hsotg {
 /** Params to actually use */
 struct dwc2_core_params *core_params;
 enum usb_otg_state op_state;
+   enum usb_dr_mode dr_mode;

 unsigned int queuing_high_bandwidth:1;
 unsigned int srp_success:1;
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index a10e7a3..4d2c738 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -42,6 +42,8 @@
  #include linux/of_device.h
  #include linux/platform_device.h

+#include linux/usb/of.h
+
  #include core.h
  #include hcd.h

@@ -171,6 +173,16 @@ static int dwc2_driver_probe(struct platform_device *dev)
 dev_dbg(dev-dev, mapped PA %08lx to VA %p\n,
 (unsigned long)res-start, hsotg-regs);

+   hsotg-dr_mode = of_usb_get_dr_mode(dev-dev.of_node);
+
+   if (IS_ENABLED(CONFIG_USB_DWC2_HOST))
+   hsotg-dr_mode = USB_DR_MODE_HOST;
+   else if (IS_ENABLED(CONFIG_USB_DWC2_GADGET))
+   hsotg-dr_mode = USB_DR_MODE_PERIPHERAL;
+
+   if (hsotg-dr_mode == USB_DR_MODE_UNKNOWN)
+   hsotg-dr_mode = 

Re: [PATCH v3 2/2] usb: dwc2: add 'mode' which based on Kconfig select or dts setting

2014-08-04 Thread Doug Anderson
Kever,

On Mon, Aug 4, 2014 at 6:45 PM, Kever Yang kever.y...@rock-chips.com wrote:
 Doug,


 On 08/05/2014 12:34 AM, Doug Anderson wrote:

 Kever,

 On Mon, Aug 4, 2014 at 6:45 AM, Kever Yang kever.y...@rock-chips.com
 wrote:

 According to the dr_mode, the otg controller can work as
 device role during firmware period, and work as host role in
 the kernel, without use of usb_id pin. As the commit usb: dwc3:
 set 'mode' based on selected Kconfig choices.

 I don't think you need to mention firmware / kernel here.  Just say
 that on some boards we always want to use host mode and on other
 boards we want to use gadget mode.  ...and that we don't necessarily
 have the ID pin hooked up to make this automatic.

 OK, I'll change this message again, I just don't know how to describe
 to make everyone understand what I'm doing.



 Signed-off-by: Kever Yang kever.y...@rock-chips.com

 Normally I'd say that you should have added Paul's Acked-by since
 you fixed his nit and he told you to include his Acked-by when his nit
 was fixed, but...

 There are some more changes than Paul's suggestion, so I'm not sure
 if Paul need more review to give me the Acked-by, I get it now.



 ---

 Changes in v3:
 - fix the odd spacing in dwc2_hsotg struct
 - From Jingoo's suggestion:
  change the commit message

 You did more than just this.  You also added some (incorrect) Kconfig
 things.  See below.


 Changes in v2:
 - put spaces around '+' operator
 - expand the comment for dr_mode
 - handle dr_mode is USB_DR_MODE_OTG

   drivers/usb/dwc2/core.c | 18 ++
   drivers/usb/dwc2/core.h |  5 +
   drivers/usb/dwc2/platform.c | 12 
   3 files changed, 35 insertions(+)

 diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
 index 27d2c9b..738bec2 100644
 --- a/drivers/usb/dwc2/core.c
 +++ b/drivers/usb/dwc2/core.c
 @@ -118,6 +118,7 @@ static int dwc2_core_reset(struct dwc2_hsotg *hsotg)
   {
  u32 greset;
  int count = 0;
 +   u32 gusbcfg;

  dev_vdbg(hsotg-dev, %s()\n, __func__);

 @@ -148,6 +149,23 @@ static int dwc2_core_reset(struct dwc2_hsotg *hsotg)
  }
  } while (greset  GRSTCTL_CSFTRST);

 +   if (hsotg-dr_mode == USB_DR_MODE_HOST) {
 +   gusbcfg = readl(hsotg-regs + GUSBCFG);
 +   gusbcfg = ~GUSBCFG_FORCEDEVMODE;
 +   gusbcfg |= GUSBCFG_FORCEHOSTMODE;
 +   writel(gusbcfg, hsotg-regs + GUSBCFG);
 +   } else if (hsotg-dr_mode == USB_DR_MODE_PERIPHERAL) {
 +   gusbcfg = readl(hsotg-regs + GUSBCFG);
 +   gusbcfg = ~GUSBCFG_FORCEHOSTMODE;
 +   gusbcfg |= GUSBCFG_FORCEDEVMODE;
 +   writel(gusbcfg, hsotg-regs + GUSBCFG);
 +   } else if (hsotg-dr_mode == USB_DR_MODE_OTG) {
 +   gusbcfg = readl(hsotg-regs + GUSBCFG);
 +   gusbcfg = ~GUSBCFG_FORCEHOSTMODE;
 +   gusbcfg = ~GUSBCFG_FORCEDEVMODE;
 +   writel(gusbcfg, hsotg-regs + GUSBCFG);
 +   }
 +
  /*
   * NOTE: This long sleep is _very_ important, otherwise the core
 will
   * not stay in host mode after a connector ID change!
 diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
 index 1efd10c..52a4fd2 100644
 --- a/drivers/usb/dwc2/core.h
 +++ b/drivers/usb/dwc2/core.h
 @@ -501,6 +501,10 @@ struct dwc2_hw_params {
*  a_peripheral and b_device=b_host) this may not
 match
*  the core, but allows the software to determine
*  transitions
 + * @dr_mode:Requested mode of operation, one of following:
 + *  - USB_DR_MODE_PERIPHERAL
 + *  - USB_DR_MODE_HOST
 + *  - USB_DR_MODE_OTG
* @queuing_high_bandwidth: True if multiple packets of a
 high-bandwidth
*  transfer are in process of being queued
* @srp_success:Stores status of SRP request in the case of a
 FS PHY
 @@ -592,6 +596,7 @@ struct dwc2_hsotg {
  /** Params to actually use */
  struct dwc2_core_params *core_params;
  enum usb_otg_state op_state;
 +   enum usb_dr_mode dr_mode;

  unsigned int queuing_high_bandwidth:1;
  unsigned int srp_success:1;
 diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
 index a10e7a3..4d2c738 100644
 --- a/drivers/usb/dwc2/platform.c
 +++ b/drivers/usb/dwc2/platform.c
 @@ -42,6 +42,8 @@
   #include linux/of_device.h
   #include linux/platform_device.h

 +#include linux/usb/of.h
 +
   #include core.h
   #include hcd.h

 @@ -171,6 +173,16 @@ static int dwc2_driver_probe(struct platform_device
 *dev)
  dev_dbg(dev-dev, mapped PA %08lx to VA %p\n,
  (unsigned long)res-start, hsotg-regs);

 +   hsotg-dr_mode = of_usb_get_dr_mode(dev-dev.of_node);
 +
 +   if (IS_ENABLED(CONFIG_USB_DWC2_HOST))
 +   hsotg-dr_mode =