| Message ID | b4129291df7b2d061e93c03862c081b6a35b2e7f.1555075927.git.arturp@synopsys.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: dwc2: Fix and improve power saving modes. | expand |
Artur Petrosyan <arthur.petrosyan@synopsys.com> writes: > - Added backup of DCFG register. > - Added Set the Power-On Programming done bit. > > Signed-off-by: Artur Petrosyan <arturp@synopsys.com> won't apply.
Hi, On Fri, Apr 12, 2019 at 6:38 AM Artur Petrosyan <arthur.petrosyan@synopsys.com> wrote: > > - Added backup of DCFG register. > - Added Set the Power-On Programming done bit. > > Signed-off-by: Artur Petrosyan <arturp@synopsys.com> > --- > drivers/usb/dwc2/gadget.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c > index 6812a8a3a98b..dcb0fbb8bc42 100644 > --- a/drivers/usb/dwc2/gadget.c > +++ b/drivers/usb/dwc2/gadget.c > @@ -5004,6 +5004,7 @@ int dwc2_restore_device_registers(struct dwc2_hsotg *hsotg, int remote_wakeup) > { > struct dwc2_dregs_backup *dr; > int i; > + u32 dctl; > > dev_dbg(hsotg->dev, "%s\n", __func__); > > @@ -5019,6 +5020,15 @@ int dwc2_restore_device_registers(struct dwc2_hsotg *hsotg, int remote_wakeup) > if (!remote_wakeup) > dwc2_writel(hsotg, dr->dctl, DCTL); > > + if (hsotg->params.power_down == DWC2_POWER_DOWN_PARAM_PARTIAL) { > + dwc2_writel(hsotg, dr->dcfg, DCFG); > + > + /* Set the Power-On Programming done bit */ > + dctl = dwc2_readl(hsotg, DCTL); > + dctl |= DCTL_PWRONPRGDONE; > + dwc2_writel(hsotg, dctl, DCTL); > + } I can't vouch one way or the other for the correctness of this change, but I will say that it's frustrating how asymmetric hibernate and partial power down are. It makes things really hard to reason about. Is there any way you could restructure this so it happens in the same way between hibernate and partial power down? Specifically looking at the similar sequence in dwc2_gadget_exit_hibernation() (which calls this function), I see: 1. There are some extra delays. Are those needed for partial power down? 2. For exiting hibernation the setting of "DCTL_PWRONPRGDONE" only happens if "not remote wakeup". Is it truly on purpose that you don't do that? 3. I see that dctl gets "DCTL_PWRONPRGDONE" set as part of the sequence in the "not remote wakeup" case before calling this function. ...but then part of the function (that you didn't change) clobbers it, I think. I have no idea if any of that is a problem but the fact that the hibernate and partial power down code runs through such different paths makes it really hard to know / reason about. Many of those differences exist before your patch, but you're adding a new difference rather than trying to unify and that worries me. -Doug
On 4/27/2019 00:43, Doug Anderson wrote: > Hi, > > On Fri, Apr 12, 2019 at 6:38 AM Artur Petrosyan > <arthur.petrosyan@synopsys.com> wrote: >> >> - Added backup of DCFG register. >> - Added Set the Power-On Programming done bit. >> >> Signed-off-by: Artur Petrosyan <arturp@synopsys.com> >> --- >> drivers/usb/dwc2/gadget.c | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c >> index 6812a8a3a98b..dcb0fbb8bc42 100644 >> --- a/drivers/usb/dwc2/gadget.c >> +++ b/drivers/usb/dwc2/gadget.c >> @@ -5004,6 +5004,7 @@ int dwc2_restore_device_registers(struct dwc2_hsotg *hsotg, int remote_wakeup) >> { >> struct dwc2_dregs_backup *dr; >> int i; >> + u32 dctl; >> >> dev_dbg(hsotg->dev, "%s\n", __func__); >> >> @@ -5019,6 +5020,15 @@ int dwc2_restore_device_registers(struct dwc2_hsotg *hsotg, int remote_wakeup) >> if (!remote_wakeup) >> dwc2_writel(hsotg, dr->dctl, DCTL); >> >> + if (hsotg->params.power_down == DWC2_POWER_DOWN_PARAM_PARTIAL) { >> + dwc2_writel(hsotg, dr->dcfg, DCFG); >> + >> + /* Set the Power-On Programming done bit */ >> + dctl = dwc2_readl(hsotg, DCTL); >> + dctl |= DCTL_PWRONPRGDONE; >> + dwc2_writel(hsotg, dctl, DCTL); >> + } > > I can't vouch one way or the other for the correctness of this change, > but I will say that it's frustrating how asymmetric hibernate and > partial power down are. It makes things really hard to reason about. > Is there any way you could restructure this so it happens in the same > way between hibernate and partial power down? > > Specifically looking at the similar sequence in > dwc2_gadget_exit_hibernation() (which calls this function), I see: > > 1. There are some extra delays. Are those needed for partial power down? Do you mean delays in dwc2_gadget_exit_hibernation() ? If yes they are needed for hibernation flow. What relates to delays in hibernation needed for partial power down. Anything that is implemented in the hibernation delays or other things are part of hibernation and are not used in partial power down because they are two different flows of power saving. > > 2. For exiting hibernation the setting of "DCTL_PWRONPRGDONE" only > happens if "not remote wakeup". Is it truly on purpose that you don't > do that? Currently partial power down doesn't support remote wakeup flow. > > 3. I see that dctl gets "DCTL_PWRONPRGDONE" set as part of the > sequence in the "not remote wakeup" case before calling this function. > ...but then part of the function (that you didn't change) clobbers it, > I think. > Setting device programming done bit in dwc2_gadget_exit_hibernation() comes from older code and from debugging I noticed that if it is not done at that point then the flow brakes. So in partial power down flow we need to set that bit wile restoring device registers. That is why the implementation is such. > > I have no idea if any of that is a problem but the fact that the > hibernate and partial power down code runs through such different > paths makes it really hard to know / reason about. Many of those > differences exist before your patch, but you're adding a new > difference rather than trying to unify and that worries me. > > So to make it easy to reason about I think we should debug it. Please point out where it fails. Have you tested this flow and did you see any wrong behavior of hibernation or partial power down? if yes please provide the debug logs so that they can be investigated. All of these patches have been tested on HAPS-DX and and Linaro HiKey 960 board. These patches fix Hibernation and Partial Power down issues. > > -Doug >
Hi, On Mon, Apr 29, 2019 at 3:51 AM Artur Petrosyan <Arthur.Petrosyan@synopsys.com> wrote: > > On 4/27/2019 00:43, Doug Anderson wrote: > > Hi, > > > > On Fri, Apr 12, 2019 at 6:38 AM Artur Petrosyan > > <arthur.petrosyan@synopsys.com> wrote: > >> > >> - Added backup of DCFG register. > >> - Added Set the Power-On Programming done bit. > >> > >> Signed-off-by: Artur Petrosyan <arturp@synopsys.com> > >> --- > >> drivers/usb/dwc2/gadget.c | 10 ++++++++++ > >> 1 file changed, 10 insertions(+) > >> > >> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c > >> index 6812a8a3a98b..dcb0fbb8bc42 100644 > >> --- a/drivers/usb/dwc2/gadget.c > >> +++ b/drivers/usb/dwc2/gadget.c > >> @@ -5004,6 +5004,7 @@ int dwc2_restore_device_registers(struct dwc2_hsotg *hsotg, int remote_wakeup) > >> { > >> struct dwc2_dregs_backup *dr; > >> int i; > >> + u32 dctl; > >> > >> dev_dbg(hsotg->dev, "%s\n", __func__); > >> > >> @@ -5019,6 +5020,15 @@ int dwc2_restore_device_registers(struct dwc2_hsotg *hsotg, int remote_wakeup) > >> if (!remote_wakeup) > >> dwc2_writel(hsotg, dr->dctl, DCTL); > >> > >> + if (hsotg->params.power_down == DWC2_POWER_DOWN_PARAM_PARTIAL) { > >> + dwc2_writel(hsotg, dr->dcfg, DCFG); > >> + > >> + /* Set the Power-On Programming done bit */ > >> + dctl = dwc2_readl(hsotg, DCTL); > >> + dctl |= DCTL_PWRONPRGDONE; > >> + dwc2_writel(hsotg, dctl, DCTL); > >> + } > > > > I can't vouch one way or the other for the correctness of this change, > > but I will say that it's frustrating how asymmetric hibernate and > > partial power down are. It makes things really hard to reason about. > > Is there any way you could restructure this so it happens in the same > > way between hibernate and partial power down? > > > > > Specifically looking at the similar sequence in > > dwc2_gadget_exit_hibernation() (which calls this function), I see: > > > > 1. There are some extra delays. Are those needed for partial power down? > Do you mean delays in dwc2_gadget_exit_hibernation() ? If yes they are > needed for hibernation flow. What relates to delays in hibernation > needed for partial power down. Anything that is implemented in the > hibernation delays or other things are part of hibernation and are not > used in partial power down because they are two different flows of power > saving. OK, if they aren't needed for partial power down then that's fine. When I see two functions doing nearly the same sets of writes but one has delays and the other doesn't it makes me wonder if that was on purpose or not. > > 2. For exiting hibernation the setting of "DCTL_PWRONPRGDONE" only > > happens if "not remote wakeup". Is it truly on purpose that you don't > > do that? > Currently partial power down doesn't support remote wakeup flow. Oh. What happens if you have partial power down enabled and try to enable remote wakeup? Does it give an error? > > 3. I see that dctl gets "DCTL_PWRONPRGDONE" set as part of the > > sequence in the "not remote wakeup" case before calling this function. > > ...but then part of the function (that you didn't change) clobbers it, > > I think. > > > Setting device programming done bit in dwc2_gadget_exit_hibernation() > comes from older code and from debugging I noticed that if it is not > done at that point then the flow brakes. > > So in partial power down flow we need to set that bit wile restoring > device registers. That is why the implementation is such. > > > > > I have no idea if any of that is a problem but the fact that the > > hibernate and partial power down code runs through such different > > paths makes it really hard to know / reason about. Many of those > > differences exist before your patch, but you're adding a new > > difference rather than trying to unify and that worries me. > > > > > > So to make it easy to reason about I think we should debug it. Please > point out where it fails. Have you tested this flow and did you see any > wrong behavior of hibernation or partial power down? if yes please > provide the debug logs so that they can be investigated. > > All of these patches have been tested on HAPS-DX and and Linaro HiKey > 960 board. These patches fix Hibernation and Partial Power down issues. I have no real way to test this code. I'm only setup to use dwc2 as a USB host since my target device is a laptop with type A ports on it. Although one of the ports could be made a gadget and I could force the mode and use an A-to-A cable, I don't have any use cases here nor do I really have any experience using dwc2 as a gadget. ...so if you and others are happy with the code I won't stand in the way--I was just reviewing the rest of the series so I figured I'd do a cursory pass on this patch too. -Doug
On 4/29/2019 21:34, Doug Anderson wrote: > Hi, > > On Mon, Apr 29, 2019 at 3:51 AM Artur Petrosyan > <Arthur.Petrosyan@synopsys.com> wrote: >> >> On 4/27/2019 00:43, Doug Anderson wrote: >>> Hi, >>> >>> On Fri, Apr 12, 2019 at 6:38 AM Artur Petrosyan >>> <arthur.petrosyan@synopsys.com> wrote: >>>> >>>> - Added backup of DCFG register. >>>> - Added Set the Power-On Programming done bit. >>>> >>>> Signed-off-by: Artur Petrosyan <arturp@synopsys.com> >>>> --- >>>> drivers/usb/dwc2/gadget.c | 10 ++++++++++ >>>> 1 file changed, 10 insertions(+) >>>> >>>> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c >>>> index 6812a8a3a98b..dcb0fbb8bc42 100644 >>>> --- a/drivers/usb/dwc2/gadget.c >>>> +++ b/drivers/usb/dwc2/gadget.c >>>> @@ -5004,6 +5004,7 @@ int dwc2_restore_device_registers(struct dwc2_hsotg *hsotg, int remote_wakeup) >>>> { >>>> struct dwc2_dregs_backup *dr; >>>> int i; >>>> + u32 dctl; >>>> >>>> dev_dbg(hsotg->dev, "%s\n", __func__); >>>> >>>> @@ -5019,6 +5020,15 @@ int dwc2_restore_device_registers(struct dwc2_hsotg *hsotg, int remote_wakeup) >>>> if (!remote_wakeup) >>>> dwc2_writel(hsotg, dr->dctl, DCTL); >>>> >>>> + if (hsotg->params.power_down == DWC2_POWER_DOWN_PARAM_PARTIAL) { >>>> + dwc2_writel(hsotg, dr->dcfg, DCFG); >>>> + >>>> + /* Set the Power-On Programming done bit */ >>>> + dctl = dwc2_readl(hsotg, DCTL); >>>> + dctl |= DCTL_PWRONPRGDONE; >>>> + dwc2_writel(hsotg, dctl, DCTL); >>>> + } >>> >>> I can't vouch one way or the other for the correctness of this change, >>> but I will say that it's frustrating how asymmetric hibernate and >>> partial power down are. It makes things really hard to reason about. >>> Is there any way you could restructure this so it happens in the same >>> way between hibernate and partial power down? >>> >> >>> Specifically looking at the similar sequence in >>> dwc2_gadget_exit_hibernation() (which calls this function), I see: >>> >>> 1. There are some extra delays. Are those needed for partial power down? >> Do you mean delays in dwc2_gadget_exit_hibernation() ? If yes they are >> needed for hibernation flow. What relates to delays in hibernation >> needed for partial power down. Anything that is implemented in the >> hibernation delays or other things are part of hibernation and are not >> used in partial power down because they are two different flows of power >> saving. > > OK, if they aren't needed for partial power down then that's fine. > When I see two functions doing nearly the same sets of writes but one > has delays and the other doesn't it makes me wonder if that was on > purpose or not. > > >>> 2. For exiting hibernation the setting of "DCTL_PWRONPRGDONE" only >>> happens if "not remote wakeup". Is it truly on purpose that you don't >>> do that? >> Currently partial power down doesn't support remote wakeup flow. > > Oh. What happens if you have partial power down enabled and try to > enable remote wakeup? Does it give an error? As far as I have been debugging I have not seen error in that case. Do you mean like it should print a message saying that current partial power down code doesn't support remote wakeup? > > >>> 3. I see that dctl gets "DCTL_PWRONPRGDONE" set as part of the >>> sequence in the "not remote wakeup" case before calling this function. >>> ...but then part of the function (that you didn't change) clobbers it, >>> I think. >>> >> Setting device programming done bit in dwc2_gadget_exit_hibernation() >> comes from older code and from debugging I noticed that if it is not >> done at that point then the flow brakes. >> >> So in partial power down flow we need to set that bit wile restoring >> device registers. That is why the implementation is such. >> >>> >>> I have no idea if any of that is a problem but the fact that the >>> hibernate and partial power down code runs through such different >>> paths makes it really hard to know / reason about. Many of those >>> differences exist before your patch, but you're adding a new >>> difference rather than trying to unify and that worries me. >>> >>> >> >> So to make it easy to reason about I think we should debug it. Please >> point out where it fails. Have you tested this flow and did you see any >> wrong behavior of hibernation or partial power down? if yes please >> provide the debug logs so that they can be investigated. >> >> All of these patches have been tested on HAPS-DX and and Linaro HiKey >> 960 board. These patches fix Hibernation and Partial Power down issues. > > I have no real way to test this code. I'm only setup to use dwc2 as a > USB host since my target device is a laptop with type A ports on it. > Although one of the ports could be made a gadget and I could force the > mode and use an A-to-A cable, I don't have any use cases here nor do I > really have any experience using dwc2 as a gadget. > > ...so if you and others are happy with the code I won't stand in the > way--I was just reviewing the rest of the series so I figured I'd do a > cursory pass on this patch too. > > > -Doug >
Hi, On Mon, Apr 29, 2019 at 11:59 PM Artur Petrosyan <Arthur.Petrosyan@synopsys.com> wrote: > > On 4/29/2019 21:34, Doug Anderson wrote: > > Hi, > > > > On Mon, Apr 29, 2019 at 3:51 AM Artur Petrosyan > > <Arthur.Petrosyan@synopsys.com> wrote: > >> > >> On 4/27/2019 00:43, Doug Anderson wrote: > >>> Hi, > >>> > >>> On Fri, Apr 12, 2019 at 6:38 AM Artur Petrosyan > >>> <arthur.petrosyan@synopsys.com> wrote: > >>>> > >>>> - Added backup of DCFG register. > >>>> - Added Set the Power-On Programming done bit. > >>>> > >>>> Signed-off-by: Artur Petrosyan <arturp@synopsys.com> > >>>> --- > >>>> drivers/usb/dwc2/gadget.c | 10 ++++++++++ > >>>> 1 file changed, 10 insertions(+) > >>>> > >>>> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c > >>>> index 6812a8a3a98b..dcb0fbb8bc42 100644 > >>>> --- a/drivers/usb/dwc2/gadget.c > >>>> +++ b/drivers/usb/dwc2/gadget.c > >>>> @@ -5004,6 +5004,7 @@ int dwc2_restore_device_registers(struct dwc2_hsotg *hsotg, int remote_wakeup) > >>>> { > >>>> struct dwc2_dregs_backup *dr; > >>>> int i; > >>>> + u32 dctl; > >>>> > >>>> dev_dbg(hsotg->dev, "%s\n", __func__); > >>>> > >>>> @@ -5019,6 +5020,15 @@ int dwc2_restore_device_registers(struct dwc2_hsotg *hsotg, int remote_wakeup) > >>>> if (!remote_wakeup) > >>>> dwc2_writel(hsotg, dr->dctl, DCTL); > >>>> > >>>> + if (hsotg->params.power_down == DWC2_POWER_DOWN_PARAM_PARTIAL) { > >>>> + dwc2_writel(hsotg, dr->dcfg, DCFG); > >>>> + > >>>> + /* Set the Power-On Programming done bit */ > >>>> + dctl = dwc2_readl(hsotg, DCTL); > >>>> + dctl |= DCTL_PWRONPRGDONE; > >>>> + dwc2_writel(hsotg, dctl, DCTL); > >>>> + } > >>> > >>> I can't vouch one way or the other for the correctness of this change, > >>> but I will say that it's frustrating how asymmetric hibernate and > >>> partial power down are. It makes things really hard to reason about. > >>> Is there any way you could restructure this so it happens in the same > >>> way between hibernate and partial power down? > >>> > >> > >>> Specifically looking at the similar sequence in > >>> dwc2_gadget_exit_hibernation() (which calls this function), I see: > >>> > >>> 1. There are some extra delays. Are those needed for partial power down? > >> Do you mean delays in dwc2_gadget_exit_hibernation() ? If yes they are > >> needed for hibernation flow. What relates to delays in hibernation > >> needed for partial power down. Anything that is implemented in the > >> hibernation delays or other things are part of hibernation and are not > >> used in partial power down because they are two different flows of power > >> saving. > > > > OK, if they aren't needed for partial power down then that's fine. > > When I see two functions doing nearly the same sets of writes but one > > has delays and the other doesn't it makes me wonder if that was on > > purpose or not. > > > > > >>> 2. For exiting hibernation the setting of "DCTL_PWRONPRGDONE" only > >>> happens if "not remote wakeup". Is it truly on purpose that you don't > >>> do that? > >> Currently partial power down doesn't support remote wakeup flow. > > > > Oh. What happens if you have partial power down enabled and try to > > enable remote wakeup? Does it give an error? > As far as I have been debugging I have not seen error in that case. > > Do you mean like it should print a message saying that current partial > power down code doesn't support remote wakeup? Not sure. ...why don't we just forget about this question? I don't have enough gadget knowledge nor any way to test. -Doug
On 4/30/2019 19:29, Doug Anderson wrote: > Hi, > > On Mon, Apr 29, 2019 at 11:59 PM Artur Petrosyan > <Arthur.Petrosyan@synopsys.com> wrote: >> >> On 4/29/2019 21:34, Doug Anderson wrote: >>> Hi, >>> >>> On Mon, Apr 29, 2019 at 3:51 AM Artur Petrosyan >>> <Arthur.Petrosyan@synopsys.com> wrote: >>>> >>>> On 4/27/2019 00:43, Doug Anderson wrote: >>>>> Hi, >>>>> >>>>> On Fri, Apr 12, 2019 at 6:38 AM Artur Petrosyan >>>>> <arthur.petrosyan@synopsys.com> wrote: >>>>>> >>>>>> - Added backup of DCFG register. >>>>>> - Added Set the Power-On Programming done bit. >>>>>> >>>>>> Signed-off-by: Artur Petrosyan <arturp@synopsys.com> >>>>>> --- >>>>>> drivers/usb/dwc2/gadget.c | 10 ++++++++++ >>>>>> 1 file changed, 10 insertions(+) >>>>>> >>>>>> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c >>>>>> index 6812a8a3a98b..dcb0fbb8bc42 100644 >>>>>> --- a/drivers/usb/dwc2/gadget.c >>>>>> +++ b/drivers/usb/dwc2/gadget.c >>>>>> @@ -5004,6 +5004,7 @@ int dwc2_restore_device_registers(struct dwc2_hsotg *hsotg, int remote_wakeup) >>>>>> { >>>>>> struct dwc2_dregs_backup *dr; >>>>>> int i; >>>>>> + u32 dctl; >>>>>> >>>>>> dev_dbg(hsotg->dev, "%s\n", __func__); >>>>>> >>>>>> @@ -5019,6 +5020,15 @@ int dwc2_restore_device_registers(struct dwc2_hsotg *hsotg, int remote_wakeup) >>>>>> if (!remote_wakeup) >>>>>> dwc2_writel(hsotg, dr->dctl, DCTL); >>>>>> >>>>>> + if (hsotg->params.power_down == DWC2_POWER_DOWN_PARAM_PARTIAL) { >>>>>> + dwc2_writel(hsotg, dr->dcfg, DCFG); >>>>>> + >>>>>> + /* Set the Power-On Programming done bit */ >>>>>> + dctl = dwc2_readl(hsotg, DCTL); >>>>>> + dctl |= DCTL_PWRONPRGDONE; >>>>>> + dwc2_writel(hsotg, dctl, DCTL); >>>>>> + } >>>>> >>>>> I can't vouch one way or the other for the correctness of this change, >>>>> but I will say that it's frustrating how asymmetric hibernate and >>>>> partial power down are. It makes things really hard to reason about. >>>>> Is there any way you could restructure this so it happens in the same >>>>> way between hibernate and partial power down? >>>>> >>>> >>>>> Specifically looking at the similar sequence in >>>>> dwc2_gadget_exit_hibernation() (which calls this function), I see: >>>>> >>>>> 1. There are some extra delays. Are those needed for partial power down? >>>> Do you mean delays in dwc2_gadget_exit_hibernation() ? If yes they are >>>> needed for hibernation flow. What relates to delays in hibernation >>>> needed for partial power down. Anything that is implemented in the >>>> hibernation delays or other things are part of hibernation and are not >>>> used in partial power down because they are two different flows of power >>>> saving. >>> >>> OK, if they aren't needed for partial power down then that's fine. >>> When I see two functions doing nearly the same sets of writes but one >>> has delays and the other doesn't it makes me wonder if that was on >>> purpose or not. >>> >>> >>>>> 2. For exiting hibernation the setting of "DCTL_PWRONPRGDONE" only >>>>> happens if "not remote wakeup". Is it truly on purpose that you don't >>>>> do that? >>>> Currently partial power down doesn't support remote wakeup flow. >>> >>> Oh. What happens if you have partial power down enabled and try to >>> enable remote wakeup? Does it give an error? >> As far as I have been debugging I have not seen error in that case. >> >> Do you mean like it should print a message saying that current partial >> power down code doesn't support remote wakeup? > > Not sure. ...why don't we just forget about this question? I don't > have enough gadget knowledge nor any way to test. Ok let's forget about that. > > -Doug >
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c index 6812a8a3a98b..dcb0fbb8bc42 100644 --- a/drivers/usb/dwc2/gadget.c +++ b/drivers/usb/dwc2/gadget.c @@ -5004,6 +5004,7 @@ int dwc2_restore_device_registers(struct dwc2_hsotg *hsotg, int remote_wakeup) { struct dwc2_dregs_backup *dr; int i; + u32 dctl; dev_dbg(hsotg->dev, "%s\n", __func__); @@ -5019,6 +5020,15 @@ int dwc2_restore_device_registers(struct dwc2_hsotg *hsotg, int remote_wakeup) if (!remote_wakeup) dwc2_writel(hsotg, dr->dctl, DCTL); + if (hsotg->params.power_down == DWC2_POWER_DOWN_PARAM_PARTIAL) { + dwc2_writel(hsotg, dr->dcfg, DCFG); + + /* Set the Power-On Programming done bit */ + dctl = dwc2_readl(hsotg, DCTL); + dctl |= DCTL_PWRONPRGDONE; + dwc2_writel(hsotg, dctl, DCTL); + } + dwc2_writel(hsotg, dr->daintmsk, DAINTMSK); dwc2_writel(hsotg, dr->diepmsk, DIEPMSK); dwc2_writel(hsotg, dr->doepmsk, DOEPMSK);
- Added backup of DCFG register. - Added Set the Power-On Programming done bit. Signed-off-by: Artur Petrosyan <arturp@synopsys.com> --- drivers/usb/dwc2/gadget.c | 10 ++++++++++ 1 file changed, 10 insertions(+)