usb: dwc2: Fix and improve power saving modes.
cover usb: dwc2: Fix and improve power saving modes.
This patch set, fixes and improves partial power down and hibernation power saving modes. Also, adds support for entering/exiting hibernation by system issued suspend/resume.
This series contains patches which were submitted to LKML. However, a part of those patches didn't reach to LKML because of local issue related to smtp server.
The patches which reached to LKML are:
- usb: dwc2: Add part. power down exit from dwc2_conn_id_status_change().
- usb: dwc2: Add port conn. sts. checking in _dwc2_hcd_resume() function.
- usb: dwc2: Fix suspend state in host mode for partial power down.
- usb: dwc2: Fix wakeup detected and session request interrupt handlers.
- usb: dwc2: Add descriptive debug messages for Partial Power Down mode.
- usb: dwc2: Fix dwc2_restore_device_registers() function.
The patches which didn't reach to LKML are:
- usb: dwc2: Add enter/exit hibernation from system issued suspend/resume
- usb: dwc2: Clear GINTSTS_RESTOREDONE bit after restore is generated.
- usb: dwc2: Clear fifo_map when resetting core.
- usb: dwc2: Allow exiting hibernation from gpwrdn rst detect
- usb: dwc2: Fix hibernation between host and device modes.
- usb: dwc2: Update dwc2_handle_usb_suspend_intr function.
- usb: dwc2: Add default param to control power optimization.
- usb: dwc2: Reset DEVADDR after exiting gadget hibernation.
Submitting all of the patches together in this version.
Changes from V0:
- Replaced 1 with DWC2_POWER_DOWN_PARAM_PARTIAL in commit "9eed02b9fe96 usb: dwc2: Fix wakeup detected and session request interrupt handlers.
Artur Petrosyan (14):
usb: dwc2: Fix dwc2_restore_device_registers() function.
usb: dwc2: Add descriptive debug messages for Partial Power Down mode.
usb: dwc2: Fix wakeup detected and session request interrupt handlers.
usb: dwc2: Fix suspend state in host mode for partial power down.
usb: dwc2: Add port conn. sts. checking in _dwc2_hcd_resume()
function.
usb: dwc2: Add part. power down exit from
dwc2_conn_id_status_change().
usb: dwc2: Reset DEVADDR after exiting gadget hibernation.
usb: dwc2: Add default param to control power optimization.
usb: dwc2: Update dwc2_handle_usb_suspend_intr function.
usb: dwc2: Fix hibernation between host and device modes.
usb: dwc2: Allow exiting hibernation from gpwrdn rst detect
usb: dwc2: Clear fifo_map when resetting core.
usb: dwc2: Clear GINTSTS_RESTOREDONE bit after restore is generated.
usb: dwc2: Add enter/exit hibernation from system issued
suspend/resume
drivers/usb/dwc2/core.c | 20 +++++ drivers/usb/dwc2/core.h | 3 + drivers/usb/dwc2/core_intr.c | 179 ++++++++++++++++++++++++++----------------- drivers/usb/dwc2/debugfs.c | 2 + drivers/usb/dwc2/gadget.c | 16 +++- drivers/usb/dwc2/hcd.c | 151 +++++++++++++++++++++++++----------- drivers/usb/dwc2/params.c | 19 +++-- 7 files changed, 269 insertions(+), 121 deletions(-)
Signature · 1 line
2.11.0
Quoted 45 lines · Artur Petrosyan
This patch set, fixes and improves partial power down and hibernation power saving modes. Also, adds support for entering/exiting hibernation by system issued suspend/resume.
This series contains patches which were submitted to LKML. However, a part of those patches didn't reach to LKML because of local issue related to smtp server.
The patches which reached to LKML are:
- usb: dwc2: Add part. power down exit from
dwc2_conn_id_status_change().
- usb: dwc2: Add port conn. sts. checking in_dwc2_hcd_resume()function.
- usb: dwc2: Fix suspend state in host mode for partial power down.
- usb: dwc2: Fix wakeup detected and session request interrupt handlers.
- usb: dwc2: Add descriptive debug messages for Partial Power Down mode.
- usb: dwc2: Fixdwc2_restore_device_registers()function.The patches which didn't reach to LKML are:
- usb: dwc2: Add enter/exit hibernation from system issued suspend/resume
- usb: dwc2: ClearGINTSTS_RESTOREDONEbit after restore is generated.
- usb: dwc2: Clearfifo_mapwhen resetting core.
- usb: dwc2: Allow exiting hibernation from gpwrdn rst detect
- usb: dwc2: Fix hibernation between host and device modes.
- usb: dwc2: Updatedwc2_handle_usb_suspend_intrfunction.
- usb: dwc2: Add default param to control power optimization.
- usb: dwc2: Reset DEVADDR after exiting gadget hibernation.Submitting all of the patches together in this version.
Changes from V0:
- Replaced 1 withDWC2_POWER_DOWN_PARAM_PARTIALin commit "9eed02b9fe96usb: dwc2: Fix wakeup detected and session request interrupt handlers.Artur Petrosyan (14):
usb: dwc2: Fixdwc2_restore_device_registers()function.
usb: dwc2: Add descriptive debug messages for Partial Power Down mode.
usb: dwc2: Fix wakeup detected and session request interrupt handlers.
usb: dwc2: Fix suspend state in host mode for partial power down.
usb: dwc2: Add port conn. sts. checking in_dwc2_hcd_resume()
function.
usb: dwc2: Add part. power down exit fromdwc2_conn_id_status_change().
Quoted 9 lines
usb: dwc2: Reset DEVADDR after exiting gadget hibernation.
usb: dwc2: Add default param to control power optimization.
usb: dwc2: Updatedwc2_handle_usb_suspend_intrfunction.
usb: dwc2: Fix hibernation between host and device modes.
usb: dwc2: Allow exiting hibernation from gpwrdn rst detect
usb: dwc2: Clearfifo_mapwhen resetting core.
usb: dwc2: ClearGINTSTS_RESTOREDONEbit after restore is generated.
usb: dwc2: Add enter/exit hibernation from system issued
suspend/resume
patches don't apply.
Signature · 1 line
balbi
Hi,
Quoted 56 lines · Felipe Balbi
Quoted 45 lines · Artur Petrosyan
This patch set, fixes and improves partial power down and hibernation power saving modes. Also, adds support for entering/exiting hibernation by system issued suspend/resume.
This series contains patches which were submitted to LKML. However, a part of those patches didn't reach to LKML because of local issue related to smtp server.
The patches which reached to LKML are:
- usb: dwc2: Add part. power down exit from
dwc2_conn_id_status_change().
- usb: dwc2: Add port conn. sts. checking in_dwc2_hcd_resume()function.
- usb: dwc2: Fix suspend state in host mode for partial power down.
- usb: dwc2: Fix wakeup detected and session request interrupt handlers.
- usb: dwc2: Add descriptive debug messages for Partial Power Down mode.
- usb: dwc2: Fixdwc2_restore_device_registers()function.The patches which didn't reach to LKML are:
- usb: dwc2: Add enter/exit hibernation from system issued suspend/resume
- usb: dwc2: ClearGINTSTS_RESTOREDONEbit after restore is generated.
- usb: dwc2: Clearfifo_mapwhen resetting core.
- usb: dwc2: Allow exiting hibernation from gpwrdn rst detect
- usb: dwc2: Fix hibernation between host and device modes.
- usb: dwc2: Updatedwc2_handle_usb_suspend_intrfunction.
- usb: dwc2: Add default param to control power optimization.
- usb: dwc2: Reset DEVADDR after exiting gadget hibernation.Submitting all of the patches together in this version.
Changes from V0:
- Replaced 1 withDWC2_POWER_DOWN_PARAM_PARTIALin commit "9eed02b9fe96usb: dwc2: Fix wakeup detected and session request interrupt handlers.Artur Petrosyan (14):
usb: dwc2: Fixdwc2_restore_device_registers()function.
usb: dwc2: Add descriptive debug messages for Partial Power Down mode.
usb: dwc2: Fix wakeup detected and session request interrupt handlers.
usb: dwc2: Fix suspend state in host mode for partial power down.
usb: dwc2: Add port conn. sts. checking in_dwc2_hcd_resume()
function.
usb: dwc2: Add part. power down exit fromdwc2_conn_id_status_change().Quoted 9 lines
usb: dwc2: Reset DEVADDR after exiting gadget hibernation.
usb: dwc2: Add default param to control power optimization.
usb: dwc2: Updatedwc2_handle_usb_suspend_intrfunction.
usb: dwc2: Fix hibernation between host and device modes.
usb: dwc2: Allow exiting hibernation from gpwrdn rst detect
usb: dwc2: Clearfifo_mapwhen resetting core.
usb: dwc2: ClearGINTSTS_RESTOREDONEbit after restore is generated.
usb: dwc2: Add enter/exit hibernation from system issued
suspend/resume
Quoted 2 lines
patches don't apply.
Do we need to wait for Minas's acknowledge or there is problem related to the patches?
Signature · 2 lines
Regards, Artur
Hi,
Quoted 60 lines · Artur Petrosyan
Hi,
Quoted 56 lines · Felipe Balbi
Quoted 54 lines · Artur Petrosyan
This patch set, fixes and improves partial power down and hibernation power saving modes. Also, adds support for entering/exiting hibernation by system issued suspend/resume.
This series contains patches which were submitted to LKML. However, a part of those patches didn't reach to LKML because of local issue related to smtp server.
The patches which reached to LKML are:
- usb: dwc2: Add part. power down exit from
dwc2_conn_id_status_change().
- usb: dwc2: Add port conn. sts. checking in_dwc2_hcd_resume()function.
- usb: dwc2: Fix suspend state in host mode for partial power down.
- usb: dwc2: Fix wakeup detected and session request interrupt handlers.
- usb: dwc2: Add descriptive debug messages for Partial Power Down mode.
- usb: dwc2: Fixdwc2_restore_device_registers()function.The patches which didn't reach to LKML are:
- usb: dwc2: Add enter/exit hibernation from system issued suspend/resume
- usb: dwc2: ClearGINTSTS_RESTOREDONEbit after restore is generated.
- usb: dwc2: Clearfifo_mapwhen resetting core.
- usb: dwc2: Allow exiting hibernation from gpwrdn rst detect
- usb: dwc2: Fix hibernation between host and device modes.
- usb: dwc2: Updatedwc2_handle_usb_suspend_intrfunction.
- usb: dwc2: Add default param to control power optimization.
- usb: dwc2: Reset DEVADDR after exiting gadget hibernation.Submitting all of the patches together in this version.
Changes from V0:
- Replaced 1 withDWC2_POWER_DOWN_PARAM_PARTIALin commit "9eed02b9fe96usb: dwc2: Fix wakeup detected and session request interrupt handlers.Artur Petrosyan (14):
usb: dwc2: Fixdwc2_restore_device_registers()function.
usb: dwc2: Add descriptive debug messages for Partial Power Down mode.
usb: dwc2: Fix wakeup detected and session request interrupt handlers.
usb: dwc2: Fix suspend state in host mode for partial power down.
usb: dwc2: Add port conn. sts. checking in_dwc2_hcd_resume()
function.
usb: dwc2: Add part. power down exit fromdwc2_conn_id_status_change().usb: dwc2: Reset DEVADDR after exiting gadget hibernation.
usb: dwc2: Add default param to control power optimization.
usb: dwc2: Updatedwc2_handle_usb_suspend_intrfunction.
usb: dwc2: Fix hibernation between host and device modes.
usb: dwc2: Allow exiting hibernation from gpwrdn rst detect
usb: dwc2: Clearfifo_mapwhen resetting core.
usb: dwc2: ClearGINTSTS_RESTOREDONEbit after restore is generated.
usb: dwc2: Add enter/exit hibernation from system issued
suspend/resume
Quoted 5 lines
Quoted 2 lines
patches don't apply.
Do we need to wait for Minas's acknowledge or there is problem related to the patches?
It looks like the problem is that my patches won the race and Felipe applied them before yours. Thus, presumably, it'll be up to you to rebase your patches atop mine and re-submit. Specifically, you can do:
git remote add linux_usb_balbigit://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git
git fetch linux_usb_balbi
git checkout linux_usb_balbi/testing/next
If you do that and then try to apply your patches you'll find that they no longer apply. AKA try running:
for patch in 10909749 10909737 10909739 10909745 10909533 \
10909531 10909747 10909535 10909523 10909741 10909525 \
10909751 10909527 10909743; do
curl -L https://patchwork.kernel.org/patch/${patch}/mbox | git am
done
You'll see:
Quoted 4 lines
Applying: usb: dwc2: Fix wakeup detected and session request interrupt handlers.
error: patch failed:drivers/usb/dwc2/core_intr.c:435
error:drivers/usb/dwc2/core_intr.c: patch does not apply
Patch failed at 0001 usb: dwc2: Fix wakeup detected and session request interrupt handlers.
NOTE: before reposting it might be a good idea to apply the last 3 patches in my series as per [1] before sending up your series. Since Felipe has already applied patches #1 and #2 in that series presumably he'll also apply #3 - #5.
I know it'a also up to me to try testing our your patches. It's still on my list to give it a shot...
[1] https://lkml.kernel.org/r/CAD=FV=WA07+gUkVvsikN=iDHZLUJQtzjkKtiBHAEDw4gLNWY7w@mail.gmail.com
P.S: It's helpful if you CC LKML on patches and discussions about them. That allows the magic "permalink via message ID" on lkml.kernel.org and also allows your patches to be found on lore.kernel.org/patchwork/
-Doug
Hi Doug,
Quoted 104 lines · Doug Anderson
Hi,
Quoted 60 lines · Artur Petrosyan
Hi,
Quoted 56 lines · Felipe Balbi
Quoted 54 lines · Artur Petrosyan
This patch set, fixes and improves partial power down and hibernation power saving modes. Also, adds support for entering/exiting hibernation by system issued suspend/resume.
This series contains patches which were submitted to LKML. However, a part of those patches didn't reach to LKML because of local issue related to smtp server.
The patches which reached to LKML are:
- usb: dwc2: Add part. power down exit from
dwc2_conn_id_status_change().
- usb: dwc2: Add port conn. sts. checking in_dwc2_hcd_resume()function.
- usb: dwc2: Fix suspend state in host mode for partial power down.
- usb: dwc2: Fix wakeup detected and session request interrupt handlers.
- usb: dwc2: Add descriptive debug messages for Partial Power Down mode.
- usb: dwc2: Fixdwc2_restore_device_registers()function.The patches which didn't reach to LKML are:
- usb: dwc2: Add enter/exit hibernation from system issued suspend/resume
- usb: dwc2: ClearGINTSTS_RESTOREDONEbit after restore is generated.
- usb: dwc2: Clearfifo_mapwhen resetting core.
- usb: dwc2: Allow exiting hibernation from gpwrdn rst detect
- usb: dwc2: Fix hibernation between host and device modes.
- usb: dwc2: Updatedwc2_handle_usb_suspend_intrfunction.
- usb: dwc2: Add default param to control power optimization.
- usb: dwc2: Reset DEVADDR after exiting gadget hibernation.Submitting all of the patches together in this version.
Changes from V0:
- Replaced 1 withDWC2_POWER_DOWN_PARAM_PARTIALin commit "9eed02b9fe96usb: dwc2: Fix wakeup detected and session request interrupt handlers.Artur Petrosyan (14):
usb: dwc2: Fixdwc2_restore_device_registers()function.
usb: dwc2: Add descriptive debug messages for Partial Power Down mode.
usb: dwc2: Fix wakeup detected and session request interrupt handlers.
usb: dwc2: Fix suspend state in host mode for partial power down.
usb: dwc2: Add port conn. sts. checking in_dwc2_hcd_resume()
function.
usb: dwc2: Add part. power down exit from
dwc2_conn_id_status_change().
usb: dwc2: Reset DEVADDR after exiting gadget hibernation.
usb: dwc2: Add default param to control power optimization.
usb: dwc2: Updatedwc2_handle_usb_suspend_intrfunction.
usb: dwc2: Fix hibernation between host and device modes.
usb: dwc2: Allow exiting hibernation from gpwrdn rst detect
usb: dwc2: Clearfifo_mapwhen resetting core.
usb: dwc2: ClearGINTSTS_RESTOREDONEbit after restore is generated.
usb: dwc2: Add enter/exit hibernation from system issued
suspend/resumeQuoted 5 lines
Quoted 2 lines
patches don't apply.
Do we need to wait for Minas's acknowledge or there is problem related to the patches?
It looks like the problem is that my patches won the race and Felipe applied them before yours. Thus, presumably, it'll be up to you to rebase your patches atop mine and re-submit. Specifically, you can do:
git remote add
linux_usb_balbigit://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git
git fetchlinux_usb_balbi
git checkout linux_usb_balbi/testing/nextIf you do that and then try to apply your patches you'll find that they no longer apply. AKA try running:
for patch in 10909749 10909737 10909739 10909745 10909533 \ 10909531 10909747 10909535 10909523 10909741 10909525 \ 10909751 10909527 10909743; doYou'll see:
Quoted 4 lines
Applying: usb: dwc2: Fix wakeup detected and session request interrupt handlers.
error: patch failed:drivers/usb/dwc2/core_intr.c:435
error:drivers/usb/dwc2/core_intr.c: patch does not apply
Patch failed at 0001 usb: dwc2: Fix wakeup detected and session request interrupt handlers.NOTE: before reposting it might be a good idea to apply the last 3 patches in my series as per [1] before sending up your series. Since Felipe has already applied patches #1 and #2 in that series presumably he'll also apply #3 - #5.
I know it'a also up to me to try testing our your patches. It's still on my list to give it a shot...
Quoted 9 lines
P.S: It's helpful if you CC LKML on patches and discussions about them. That allows the magic "permalink via message ID" on lkml.kernel.org and also allows your patches to be found on lore.kernel.org/patchwork/-Doug
Besides the issue that comes from the patch "usb: dwc2: Fix wakeup detected and session request interrupt handlers." there is one more serious conflict with one of your patches.
So the patch "usb: dwc2: bus suspend/resume for hosts with DWC2_POWER_DOWN_PARAM_NONE" have had also been added to the "balbi/testing/next" before my patch series which conflicts with two of my patches.
1. usb: dwc2: Fix suspend state in host mode for partial power down.
2. usb: dwc2: Add enter/exit hibernation from system issued suspend/resume
This patch introduced by you "usb: dwc2: bus suspend/resume for hosts with DWC2_POWER_DOWN_PARAM_NONE" got a little bit of issue. It eliminates entering hibernation through system issued suspend by checking "if (hsotg->params.power_down > DWC2_POWER_DOWN_PARAM_PARTIAL)" . As per the patch you mention that it fixes suspend/resume flow for Altera SOCFPGA and Chrome OS 3.14 kernel tree. I assume that the board has the Partial Power Down enabled core that is why it works out. However, we don't just support the Partial Power Down feature enabled cores. We also support Hibernation feature enabled cores.
The patch set that had been introduced by me which includes "usb: dwc2:
Add enter/exit hibernation from system issued suspend/resume" patch adds support for both hibernation and Partial Power Down feature enabled cores and fixes several of Partial Power Down and hibernation related issues.
This patch set may fix all of the issues related with Altera SOCFPGA or Chrome OS 3.14 kernel tree.
That is why we asked you to test the patch set before we could ACK or have chance to debug your patch deeper to see the help of it and to provide you information related to it.
So now I can rebase my changes to the "balbi/testing/next" but I will have to take the logic of skipping Hibernation out otherwise we will have problems with hibernation enabled cores.
We can ask Balbi to permanently suspend adding of the patch "usb: dwc2:
bus suspend/resume for hosts with DWC2_POWER_DOWN_PARAM_NONE" and my patch series to his "testing/next". After you can have a chance to test my patch series we can see if the patches are acknowledged and ask Balbi
to add them.
Signature · 2 lines
Regards, Artur
Hi,
Quoted 127 lines · Artur Petrosyan
Hi Doug,
Quoted 104 lines · Doug Anderson
Hi,
Quoted 60 lines · Artur Petrosyan
Hi,
Quoted 56 lines · Felipe Balbi
Quoted 54 lines · Artur Petrosyan
This patch set, fixes and improves partial power down and hibernation power saving modes. Also, adds support for entering/exiting hibernation by system issued suspend/resume.
This series contains patches which were submitted to LKML. However, a part of those patches didn't reach to LKML because of local issue related to smtp server.
The patches which reached to LKML are:
- usb: dwc2: Add part. power down exit from
dwc2_conn_id_status_change().
- usb: dwc2: Add port conn. sts. checking in_dwc2_hcd_resume()function.
- usb: dwc2: Fix suspend state in host mode for partial power down.
- usb: dwc2: Fix wakeup detected and session request interrupt handlers.
- usb: dwc2: Add descriptive debug messages for Partial Power Down mode.
- usb: dwc2: Fixdwc2_restore_device_registers()function.The patches which didn't reach to LKML are:
- usb: dwc2: Add enter/exit hibernation from system issued suspend/resume
- usb: dwc2: ClearGINTSTS_RESTOREDONEbit after restore is generated.
- usb: dwc2: Clearfifo_mapwhen resetting core.
- usb: dwc2: Allow exiting hibernation from gpwrdn rst detect
- usb: dwc2: Fix hibernation between host and device modes.
- usb: dwc2: Updatedwc2_handle_usb_suspend_intrfunction.
- usb: dwc2: Add default param to control power optimization.
- usb: dwc2: Reset DEVADDR after exiting gadget hibernation.Submitting all of the patches together in this version.
Changes from V0:
- Replaced 1 withDWC2_POWER_DOWN_PARAM_PARTIALin commit "9eed02b9fe96usb: dwc2: Fix wakeup detected and session request interrupt handlers.Artur Petrosyan (14):
usb: dwc2: Fixdwc2_restore_device_registers()function.
usb: dwc2: Add descriptive debug messages for Partial Power Down mode.
usb: dwc2: Fix wakeup detected and session request interrupt handlers.
usb: dwc2: Fix suspend state in host mode for partial power down.
usb: dwc2: Add port conn. sts. checking in_dwc2_hcd_resume()
function.
usb: dwc2: Add part. power down exit from
dwc2_conn_id_status_change().
usb: dwc2: Reset DEVADDR after exiting gadget hibernation.
usb: dwc2: Add default param to control power optimization.
usb: dwc2: Updatedwc2_handle_usb_suspend_intrfunction.
usb: dwc2: Fix hibernation between host and device modes.
usb: dwc2: Allow exiting hibernation from gpwrdn rst detect
usb: dwc2: Clearfifo_mapwhen resetting core.
usb: dwc2: ClearGINTSTS_RESTOREDONEbit after restore is generated.
usb: dwc2: Add enter/exit hibernation from system issued
suspend/resumeQuoted 5 lines
Quoted 2 lines
patches don't apply.
Do we need to wait for Minas's acknowledge or there is problem related to the patches?
It looks like the problem is that my patches won the race and Felipe applied them before yours. Thus, presumably, it'll be up to you to rebase your patches atop mine and re-submit. Specifically, you can do:
git remote add
linux_usb_balbigit://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git
git fetchlinux_usb_balbi
git checkout linux_usb_balbi/testing/nextIf you do that and then try to apply your patches you'll find that they no longer apply. AKA try running:
for patch in 10909749 10909737 10909739 10909745 10909533 \ 10909531 10909747 10909535 10909523 10909741 10909525 \ 10909751 10909527 10909743; doYou'll see:
Quoted 4 lines
Applying: usb: dwc2: Fix wakeup detected and session request interrupt handlers.
error: patch failed:drivers/usb/dwc2/core_intr.c:435
error:drivers/usb/dwc2/core_intr.c: patch does not apply
Patch failed at 0001 usb: dwc2: Fix wakeup detected and session request interrupt handlers.NOTE: before reposting it might be a good idea to apply the last 3 patches in my series as per [1] before sending up your series. Since Felipe has already applied patches #1 and #2 in that series presumably he'll also apply #3 - #5.
I know it'a also up to me to try testing our your patches. It's still on my list to give it a shot...
Quoted 9 lines
P.S: It's helpful if you CC LKML on patches and discussions about them. That allows the magic "permalink via message ID" on lkml.kernel.org and also allows your patches to be found on lore.kernel.org/patchwork/-Doug
Besides the issue that comes from the patch "usb: dwc2: Fix wakeup detected and session request interrupt handlers." there is one more serious conflict with one of your patches.
So the patch "usb: dwc2: bus suspend/resume for hosts with
DWC2_POWER_DOWN_PARAM_NONE" have had also been added to the "balbi/testing/next" before my patch series which conflicts with two of my patches.
Quoted 7 lines
1. usb: dwc2: Fix suspend state in host mode for partial power down.
2. usb: dwc2: Add enter/exit hibernation from system issued suspend/resumeThis patch introduced by you "usb: dwc2: bus suspend/resume for hosts with
DWC2_POWER_DOWN_PARAM_NONE" got a little bit of issue. It eliminates entering hibernation through system issued suspend by checking "if (hsotg->params.power_down>DWC2_POWER_DOWN_PARAM_PARTIAL)"
To be fair, the patch does not make entering hibernation worse, does it? Specifically, I'll point to this part of the diff:
- if (hsotg->params.power_down != DWC2_POWER_DOWN_PARAM_PARTIAL) {
+ if (hsotg->params.power_down > DWC2_POWER_DOWN_PARAM_PARTIAL) {
As you can see, if power_down == DWC2_POWER_DOWN_PARAM_HIBERNATION the flow for this "if" test is exactly the same before and after my patch.
Quoted 3 lines
. As per the patch you mention that it fixes suspend/resume flow for Altera SOCFPGA and Chrome OS 3.14 kernel tree. I assume that the board has the Partial Power Down enabled core that is why it works out.
I mentioned some of this in my cover letter [1]. To rehash, I said "Turning on partial power down on rk3288 doesn't "just work". I don't get hotplug events. This is despite dwc2 auto-detecting that we are power optimized."
...it is certainly possible that partial power down would work on rk3288 if someone had the time to debug it.
NOTE: I don't have an Altera SOCFPGA, but I'll mention that a previous iteration of my patch (written by Kever Yang at Rockchip) was reverted because it _broke_ Altera SOCFPGAS. Given my requests to test the new version have been met with silence, I'm inclined to land this and hope it's all good. If there are problems then hopefully some actual details can be provided. Last time there were none provided.
Quoted 2 lines
However, we don't just support the Partial Power Down feature enabled cores. We also support Hibernation feature enabled cores.
Sure, but the code that is actually landed upstream (even before my series) almost certainly doesn't function for Hibernation. As pointed out above the entire "_dwc2_hcd_suspend()" function had a great big:
if (hsotg->params.power_down != DWC2_POWER_DOWN_PARAM_PARTIAL)
goto skip_power_saving;
...which, as far as I could tell, meant that Hibernation could not possible work.
Quoted 6 lines
The patch set that had been introduced by me which includes "usb: dwc2:
Add enter/exit hibernation from system issued suspend/resume" patch adds support for both hibernation and Partial Power Down feature enabled cores and fixes several of Partial Power Down and hibernation related issues.
Quoted 6 lines
This patch set may fix all of the issues related with Altera SOCFPGA or Chrome OS 3.14 kernel tree.
That is why we asked you to test the patch set before we could ACK or have chance to debug your patch deeper to see the help of it and to provide you information related to it.
It may well fix my problems and maybe I can use partial power down now. That'd be nice. It was on my list and I would have worked on it last week except that your patches weren't on the mailing list then. ...so I moved on to some other work. To avoid context switching too much I needed to get to a stopping point before testing your patches. I was hoping to have some nice rebased patches from you to test today, but maybe I'll try a hand at rebasing them myself.
NOTE also that though I ported this change from the Chrome OS 3.14 kernel tree, I'm actually currently working on the Chrome OS 4.19 tree. I also made sure to test the changes on mainline Linux.
Quoted 3 lines
So now I can rebase my changes to the "balbi/testing/next" but I will have to take the logic of skipping Hibernation out otherwise we will have problems with hibernation enabled cores.
As per above, please have a careful look at my patches and you'll see that I was not introducing code that skipped hibernation. I was keeping the same flow as the old code that skipped hibernation. So if you are making hibernation work there should be no problems removing that.
Quoted 5 lines
We can ask Balbi to permanently suspend adding of the patch "usb: dwc2:
bus suspend/resume for hosts withDWC2_POWER_DOWN_PARAM_NONE" and my patch series to his "testing/next". After you can have a chance to test my patch series we can see if the patches are acknowledged and ask Balbi
to add them.
Personally I'd prefer if Felipe finished landing my series and then you rebased atop it. As I said I'm convinced I'm not making your hibernation case any worse. If you know of actual things that are made worse by my series then that would be a reason not to land it, but so far I haven't been convinced.
Hi,
Quoted 227 lines · Doug Anderson
Hi,
Quoted 127 lines · Artur Petrosyan
Hi Doug,
Quoted 104 lines · Doug Anderson
Hi,
Quoted 60 lines · Artur Petrosyan
Hi,
Quoted 56 lines · Felipe Balbi
Quoted 54 lines · Artur Petrosyan
This patch set, fixes and improves partial power down and hibernation power saving modes. Also, adds support for entering/exiting hibernation by system issued suspend/resume.
This series contains patches which were submitted to LKML. However, a part of those patches didn't reach to LKML because of local issue related to smtp server.
The patches which reached to LKML are:
- usb: dwc2: Add part. power down exit from
dwc2_conn_id_status_change().
- usb: dwc2: Add port conn. sts. checking in_dwc2_hcd_resume()function.
- usb: dwc2: Fix suspend state in host mode for partial power down.
- usb: dwc2: Fix wakeup detected and session request interrupt handlers.
- usb: dwc2: Add descriptive debug messages for Partial Power Down mode.
- usb: dwc2: Fixdwc2_restore_device_registers()function.The patches which didn't reach to LKML are:
- usb: dwc2: Add enter/exit hibernation from system issued suspend/resume
- usb: dwc2: ClearGINTSTS_RESTOREDONEbit after restore is generated.
- usb: dwc2: Clearfifo_mapwhen resetting core.
- usb: dwc2: Allow exiting hibernation from gpwrdn rst detect
- usb: dwc2: Fix hibernation between host and device modes.
- usb: dwc2: Updatedwc2_handle_usb_suspend_intrfunction.
- usb: dwc2: Add default param to control power optimization.
- usb: dwc2: Reset DEVADDR after exiting gadget hibernation.Submitting all of the patches together in this version.
Changes from V0:
- Replaced 1 withDWC2_POWER_DOWN_PARAM_PARTIALin commit "9eed02b9fe96usb: dwc2: Fix wakeup detected and session request interrupt handlers.Artur Petrosyan (14):
usb: dwc2: Fixdwc2_restore_device_registers()function.
usb: dwc2: Add descriptive debug messages for Partial Power Down mode.
usb: dwc2: Fix wakeup detected and session request interrupt handlers.
usb: dwc2: Fix suspend state in host mode for partial power down.
usb: dwc2: Add port conn. sts. checking in_dwc2_hcd_resume()
function.
usb: dwc2: Add part. power down exit from
dwc2_conn_id_status_change().
usb: dwc2: Reset DEVADDR after exiting gadget hibernation.
usb: dwc2: Add default param to control power optimization.
usb: dwc2: Updatedwc2_handle_usb_suspend_intrfunction.
usb: dwc2: Fix hibernation between host and device modes.
usb: dwc2: Allow exiting hibernation from gpwrdn rst detect
usb: dwc2: Clearfifo_mapwhen resetting core.
usb: dwc2: ClearGINTSTS_RESTOREDONEbit after restore is generated.
usb: dwc2: Add enter/exit hibernation from system issued
suspend/resumeQuoted 5 lines
Quoted 2 lines
patches don't apply.
Do we need to wait for Minas's acknowledge or there is problem related to the patches?
It looks like the problem is that my patches won the race and Felipe applied them before yours. Thus, presumably, it'll be up to you to rebase your patches atop mine and re-submit. Specifically, you can do:
git remote add
linux_usb_balbigit://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git
git fetchlinux_usb_balbi
git checkout linux_usb_balbi/testing/nextIf you do that and then try to apply your patches you'll find that they no longer apply. AKA try running:
for patch in 10909749 10909737 10909739 10909745 10909533 \ 10909531 10909747 10909535 10909523 10909741 10909525 \ 10909751 10909527 10909743; doYou'll see:
Quoted 4 lines
Applying: usb: dwc2: Fix wakeup detected and session request interrupt handlers.
error: patch failed:drivers/usb/dwc2/core_intr.c:435
error:drivers/usb/dwc2/core_intr.c: patch does not apply
Patch failed at 0001 usb: dwc2: Fix wakeup detected and session request interrupt handlers.NOTE: before reposting it might be a good idea to apply the last 3 patches in my series as per [1] before sending up your series. Since Felipe has already applied patches #1 and #2 in that series presumably he'll also apply #3 - #5.
I know it'a also up to me to try testing our your patches. It's still on my list to give it a shot...
Quoted 9 lines
P.S: It's helpful if you CC LKML on patches and discussions about them. That allows the magic "permalink via message ID" on lkml.kernel.org and also allows your patches to be found on lore.kernel.org/patchwork/-Doug
Besides the issue that comes from the patch "usb: dwc2: Fix wakeup detected and session request interrupt handlers." there is one more serious conflict with one of your patches.
So the patch "usb: dwc2: bus suspend/resume for hosts with
DWC2_POWER_DOWN_PARAM_NONE" have had also been added to the "balbi/testing/next" before my patch series which conflicts with two of my patches.Quoted 7 lines
1. usb: dwc2: Fix suspend state in host mode for partial power down.
2. usb: dwc2: Add enter/exit hibernation from system issued suspend/resumeThis patch introduced by you "usb: dwc2: bus suspend/resume for hosts with
DWC2_POWER_DOWN_PARAM_NONE" got a little bit of issue. It eliminates entering hibernation through system issued suspend by checking "if (hsotg->params.power_down>DWC2_POWER_DOWN_PARAM_PARTIAL)"To be fair, the patch does not make entering hibernation worse, does it? Specifically, I'll point to this part of the diff:
- if (
hsotg->params.power_down!=DWC2_POWER_DOWN_PARAM_PARTIAL) {
+ if (hsotg->params.power_down>DWC2_POWER_DOWN_PARAM_PARTIAL) {As you can see, if
power_down==DWC2_POWER_DOWN_PARAM_HIBERNATIONthe flow for this "if" test is exactly the same before and after my patch.Quoted 3 lines
. As per the patch you mention that it fixes suspend/resume flow for Altera SOCFPGA and Chrome OS 3.14 kernel tree. I assume that the board has the Partial Power Down enabled core that is why it works out.
I mentioned some of this in my cover letter [1]. To rehash, I said "Turning on partial power down on rk3288 doesn't "just work". I don't get hotplug events. This is despite dwc2 auto-detecting that we are power optimized."
...it is certainly possible that partial power down would work on rk3288 if someone had the time to debug it.
NOTE: I don't have an Altera SOCFPGA, but I'll mention that a previous iteration of my patch (written by Kever Yang at Rockchip) was reverted because it _broke_ Altera SOCFPGAS. Given my requests to test the new version have been met with silence, I'm inclined to land this and hope it's all good. If there are problems then hopefully some actual details can be provided. Last time there were none provided.
Quoted 2 lines
However, we don't just support the Partial Power Down feature enabled cores. We also support Hibernation feature enabled cores.
Sure, but the code that is actually landed upstream (even before my series) almost certainly doesn't function for Hibernation. As pointed out above the entire "
_dwc2_hcd_suspend()" function had a great big:if (
hsotg->params.power_down!=DWC2_POWER_DOWN_PARAM_PARTIAL)
gotoskip_power_saving;...which, as far as I could tell, meant that Hibernation could not possible work.
Quoted 6 lines
The patch set that had been introduced by me which includes "usb: dwc2:
Add enter/exit hibernation from system issued suspend/resume" patch adds support for both hibernation and Partial Power Down feature enabled cores and fixes several of Partial Power Down and hibernation related issues.Quoted 6 lines
This patch set may fix all of the issues related with Altera SOCFPGA or Chrome OS 3.14 kernel tree.
That is why we asked you to test the patch set before we could ACK or have chance to debug your patch deeper to see the help of it and to provide you information related to it.
It may well fix my problems and maybe I can use partial power down now. That'd be nice. It was on my list and I would have worked on it last week except that your patches weren't on the mailing list then. ...so I moved on to some other work. To avoid context switching too much I needed to get to a stopping point before testing your patches. I was hoping to have some nice rebased patches from you to test today, but maybe I'll try a hand at rebasing them myself.
NOTE also that though I ported this change from the Chrome OS 3.14 kernel tree, I'm actually currently working on the Chrome OS 4.19 tree. I also made sure to test the changes on mainline Linux.
Quoted 3 lines
So now I can rebase my changes to the "balbi/testing/next" but I will have to take the logic of skipping Hibernation out otherwise we will have problems with hibernation enabled cores.
As per above, please have a careful look at my patches and you'll see that I was not introducing code that skipped hibernation. I was keeping the same flow as the old code that skipped hibernation. So if you are making hibernation work there should be no problems removing that.
Quoted 5 lines
We can ask Balbi to permanently suspend adding of the patch "usb: dwc2:
bus suspend/resume for hosts withDWC2_POWER_DOWN_PARAM_NONE" and my patch series to his "testing/next". After you can have a chance to test my patch series we can see if the patches are acknowledged and ask Balbi
to add them.
Quoted 8 lines
Personally I'd prefer if Felipe finished landing my series and then you rebased atop it. As I said I'm convinced I'm not making your hibernation case any worse. If you know of actual things that are made worse by my series then that would be a reason not to land it, but so far I haven't been convinced.
To add a bit of breadcrumbs, I did the rebase atop my patches and also did some basic review of yours. Other than a few nits I think I found at least one bug where you're unlocking a spinlock twice in the partial power down case.
I continue to be convinced that the right thing to do is to finish landing my series and then once you've spun yours atop mine we can look at landing it.
Here's all my picks atop Felipe's tree that show how I resolved
things: https://chromium.googlesource.com/chromiumos/third_party/kernel/+log/refs/sandbox/dianders/190426-dwc2-stuff
-Doug
Hi,
Quoted 237 lines · Doug Anderson
Hi,
Quoted 234 lines · Doug Anderson
Hi,
Quoted 127 lines · Artur Petrosyan
Hi Doug,
Quoted 104 lines · Doug Anderson
Hi,
Quoted 60 lines · Artur Petrosyan
Hi,
Quoted 56 lines · Felipe Balbi
Quoted 54 lines · Artur Petrosyan
This patch set, fixes and improves partial power down and hibernation power saving modes. Also, adds support for entering/exiting hibernation by system issued suspend/resume.
This series contains patches which were submitted to LKML. However, a part of those patches didn't reach to LKML because of local issue related to smtp server.
The patches which reached to LKML are:
- usb: dwc2: Add part. power down exit from
dwc2_conn_id_status_change().
- usb: dwc2: Add port conn. sts. checking in_dwc2_hcd_resume()function.
- usb: dwc2: Fix suspend state in host mode for partial power down.
- usb: dwc2: Fix wakeup detected and session request interrupt handlers.
- usb: dwc2: Add descriptive debug messages for Partial Power Down mode.
- usb: dwc2: Fixdwc2_restore_device_registers()function.The patches which didn't reach to LKML are:
- usb: dwc2: Add enter/exit hibernation from system issued suspend/resume
- usb: dwc2: ClearGINTSTS_RESTOREDONEbit after restore is generated.
- usb: dwc2: Clearfifo_mapwhen resetting core.
- usb: dwc2: Allow exiting hibernation from gpwrdn rst detect
- usb: dwc2: Fix hibernation between host and device modes.
- usb: dwc2: Updatedwc2_handle_usb_suspend_intrfunction.
- usb: dwc2: Add default param to control power optimization.
- usb: dwc2: Reset DEVADDR after exiting gadget hibernation.Submitting all of the patches together in this version.
Changes from V0:
- Replaced 1 withDWC2_POWER_DOWN_PARAM_PARTIALin commit "9eed02b9fe96usb: dwc2: Fix wakeup detected and session request interrupt handlers.Artur Petrosyan (14):
usb: dwc2: Fixdwc2_restore_device_registers()function.
usb: dwc2: Add descriptive debug messages for Partial Power Down mode.
usb: dwc2: Fix wakeup detected and session request interrupt handlers.
usb: dwc2: Fix suspend state in host mode for partial power down.
usb: dwc2: Add port conn. sts. checking in_dwc2_hcd_resume()
function.
usb: dwc2: Add part. power down exit from
dwc2_conn_id_status_change().
usb: dwc2: Reset DEVADDR after exiting gadget hibernation.
usb: dwc2: Add default param to control power optimization.
usb: dwc2: Updatedwc2_handle_usb_suspend_intrfunction.
usb: dwc2: Fix hibernation between host and device modes.
usb: dwc2: Allow exiting hibernation from gpwrdn rst detect
usb: dwc2: Clearfifo_mapwhen resetting core.
usb: dwc2: ClearGINTSTS_RESTOREDONEbit after restore is generated.
usb: dwc2: Add enter/exit hibernation from system issued
suspend/resumeQuoted 5 lines
Quoted 2 lines
patches don't apply.
Do we need to wait for Minas's acknowledge or there is problem related to the patches?
It looks like the problem is that my patches won the race and Felipe applied them before yours. Thus, presumably, it'll be up to you to rebase your patches atop mine and re-submit. Specifically, you can do:
git remote add
linux_usb_balbigit://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git
git fetchlinux_usb_balbi
git checkout linux_usb_balbi/testing/nextIf you do that and then try to apply your patches you'll find that they no longer apply. AKA try running:
for patch in 10909749 10909737 10909739 10909745 10909533 \ 10909531 10909747 10909535 10909523 10909741 10909525 \ 10909751 10909527 10909743; do curl -L https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.kernel.org_patch_-24-257Bpatch-257D_mbox&d=DwIBaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=9hPBFKCJ_nBjJhGVrrlYOeOQjP_HlVzYqrC_D7niMJI&m=gzzEDEbxflLMgk9I-7Re9ytRMvyc_B8iZEmg2xGZN5E&s=aWT1hYtXeIeY8ClQ0sNYxwkmJFKDz4iaa4DchNwx3_w&e= | git amdone
You'll see:
Quoted 4 lines
Applying: usb: dwc2: Fix wakeup detected and session request interrupt handlers.
error: patch failed:drivers/usb/dwc2/core_intr.c:435
error:drivers/usb/dwc2/core_intr.c: patch does not apply
Patch failed at 0001 usb: dwc2: Fix wakeup detected and session request interrupt handlers.NOTE: before reposting it might be a good idea to apply the last 3 patches in my series as per [1] before sending up your series. Since Felipe has already applied patches #1 and #2 in that series presumably he'll also apply #3 - #5.
I know it'a also up to me to try testing our your patches. It's still on my list to give it a shot...
Quoted 9 lines
P.S: It's helpful if you CC LKML on patches and discussions about them. That allows the magic "permalink via message ID" on lkml.kernel.org and also allows your patches to be found on lore.kernel.org/patchwork/-Doug
Besides the issue that comes from the patch "usb: dwc2: Fix wakeup detected and session request interrupt handlers." there is one more serious conflict with one of your patches.
So the patch "usb: dwc2: bus suspend/resume for hosts with
DWC2_POWER_DOWN_PARAM_NONE" have had also been added to the "balbi/testing/next" before my patch series which conflicts with two of my patches.Quoted 7 lines
1. usb: dwc2: Fix suspend state in host mode for partial power down.
2. usb: dwc2: Add enter/exit hibernation from system issued suspend/resumeThis patch introduced by you "usb: dwc2: bus suspend/resume for hosts with
DWC2_POWER_DOWN_PARAM_NONE" got a little bit of issue. It eliminates entering hibernation through system issued suspend by checking "if (hsotg->params.power_down>DWC2_POWER_DOWN_PARAM_PARTIAL)"To be fair, the patch does not make entering hibernation worse, does it? Specifically, I'll point to this part of the diff:
- if (
hsotg->params.power_down!=DWC2_POWER_DOWN_PARAM_PARTIAL) {
+ if (hsotg->params.power_down>DWC2_POWER_DOWN_PARAM_PARTIAL) {As you can see, if
power_down==DWC2_POWER_DOWN_PARAM_HIBERNATIONthe flow for this "if" test is exactly the same before and after my patch.Quoted 3 lines
. As per the patch you mention that it fixes suspend/resume flow for Altera SOCFPGA and Chrome OS 3.14 kernel tree. I assume that the board has the Partial Power Down enabled core that is why it works out.
I mentioned some of this in my cover letter [1]. To rehash, I said "Turning on partial power down on rk3288 doesn't "just work". I don't get hotplug events. This is despite dwc2 auto-detecting that we are power optimized."
...it is certainly possible that partial power down would work on rk3288 if someone had the time to debug it.
NOTE: I don't have an Altera SOCFPGA, but I'll mention that a previous iteration of my patch (written by Kever Yang at Rockchip) was reverted because it _broke_ Altera SOCFPGAS. Given my requests to test the new version have been met with silence, I'm inclined to land this and hope it's all good. If there are problems then hopefully some actual details can be provided. Last time there were none provided.
Quoted 2 lines
However, we don't just support the Partial Power Down feature enabled cores. We also support Hibernation feature enabled cores.
Sure, but the code that is actually landed upstream (even before my series) almost certainly doesn't function for Hibernation. As pointed out above the entire "
_dwc2_hcd_suspend()" function had a great big:if (
hsotg->params.power_down!=DWC2_POWER_DOWN_PARAM_PARTIAL)
gotoskip_power_saving;...which, as far as I could tell, meant that Hibernation could not possible work.
Quoted 6 lines
The patch set that had been introduced by me which includes "usb: dwc2:
Add enter/exit hibernation from system issued suspend/resume" patch adds support for both hibernation and Partial Power Down feature enabled cores and fixes several of Partial Power Down and hibernation related issues.Quoted 6 lines
This patch set may fix all of the issues related with Altera SOCFPGA or Chrome OS 3.14 kernel tree.
That is why we asked you to test the patch set before we could ACK or have chance to debug your patch deeper to see the help of it and to provide you information related to it.
It may well fix my problems and maybe I can use partial power down now. That'd be nice. It was on my list and I would have worked on it last week except that your patches weren't on the mailing list then. ...so I moved on to some other work. To avoid context switching too much I needed to get to a stopping point before testing your patches. I was hoping to have some nice rebased patches from you to test today, but maybe I'll try a hand at rebasing them myself.
NOTE also that though I ported this change from the Chrome OS 3.14 kernel tree, I'm actually currently working on the Chrome OS 4.19 tree. I also made sure to test the changes on mainline Linux.
Quoted 3 lines
So now I can rebase my changes to the "balbi/testing/next" but I will have to take the logic of skipping Hibernation out otherwise we will have problems with hibernation enabled cores.
As per above, please have a careful look at my patches and you'll see that I was not introducing code that skipped hibernation. I was keeping the same flow as the old code that skipped hibernation. So if you are making hibernation work there should be no problems removing that.
Quoted 5 lines
We can ask Balbi to permanently suspend adding of the patch "usb: dwc2:
bus suspend/resume for hosts withDWC2_POWER_DOWN_PARAM_NONE" and my patch series to his "testing/next". After you can have a chance to test my patch series we can see if the patches are acknowledged and ask Balbi
to add them.Personally I'd prefer if Felipe finished landing my series and then you rebased atop it. As I said I'm convinced I'm not making your hibernation case any worse. If you know of actual things that are made worse by my series then that would be a reason not to land it, but so far I haven't been convinced.
Quoted 6 lines
To add a bit of breadcrumbs, I did the rebase atop my patches and also did some basic review of yours. Other than a few nits I think I found at least one bug where you're unlocking a spinlock twice in the partial power down case.
Yeah thank you so much for the review it really helps to make conclusions on the implementations. I have tested those patches on HAPS-DX platform and have not got any problem. Hibernation and partial power down flows are working ok.
Quoted 11 lines
I continue to be convinced that the right thing to do is to finish landing my series and then once you've spun yours atop mine we can look at landing it.
Here's all my picks atop Felipe's tree that show how I resolved
things: https://urldefense.proofpoint.com/v2/url?u=https-3A__chromium.googlesource.com_chromiumos_third-5Fparty_kernel_-2Blog_refs_sandbox_dianders_190426-2Ddwc2-2Dstuff&d=DwIFaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=9hPBFKCJ_nBjJhGVrrlYOeOQjP_HlVzYqrC_D7niMJI&m=mdz-R9O5TUR_zXEeeCZO2341dvcwZro2cedCZzIIans&s=mVfBGtiMQg2XVHXmGCWcd584g0DYRH1JDVnEnJWE6P8&e=-Doug
I will make my changes then will go ahead and rebase.
Signature · 2 lines
Regards, Artur
Hi,
Quoted 9 lines · Artur Petrosyan
Quoted 5 lines
To add a bit of breadcrumbs, I did the rebase atop my patches and also did some basic review of yours. Other than a few nits I think I found at least one bug where you're unlocking a spinlock twice in the partial power down case.
Yeah thank you so much for the review it really helps to make conclusions on the implementations. I have tested those patches on HAPS-DX platform and have not got any problem. Hibernation and partial power down flows are working ok.
Do you happen to have any boards that have "OTG_EN_PWROPT = 0"? I'd love testing of my patch series with that.
Quoted 4 lines
Quoted 4 lines
I continue to be convinced that the right thing to do is to finish landing my series and then once you've spun yours atop mine we can look at landing it.
Quoted 8 lines
Quoted 6 lines
Here's all my picks atop Felipe's tree that show how I resolved
things: https://urldefense.proofpoint.com/v2/url?u=https-3A__chromium.googlesource.com_chromiumos_third-5Fparty_kernel_-2Blog_refs_sandbox_dianders_190426-2Ddwc2-2Dstuff&d=DwIFaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=9hPBFKCJ_nBjJhGVrrlYOeOQjP_HlVzYqrC_D7niMJI&m=mdz-R9O5TUR_zXEeeCZO2341dvcwZro2cedCZzIIans&s=mVfBGtiMQg2XVHXmGCWcd584g0DYRH1JDVnEnJWE6P8&e=-Doug
I will make my changes then will go ahead and rebase.
Thanks!
-Doug
Hi,
Quoted 139 lines · Doug Anderson
Hi,
Quoted 127 lines · Artur Petrosyan
Hi Doug,
Quoted 104 lines · Doug Anderson
Hi,
Quoted 60 lines · Artur Petrosyan
Hi,
Quoted 56 lines · Felipe Balbi
Quoted 54 lines · Artur Petrosyan
This patch set, fixes and improves partial power down and hibernation power saving modes. Also, adds support for entering/exiting hibernation by system issued suspend/resume.
This series contains patches which were submitted to LKML. However, a part of those patches didn't reach to LKML because of local issue related to smtp server.
The patches which reached to LKML are:
- usb: dwc2: Add part. power down exit from
dwc2_conn_id_status_change().
- usb: dwc2: Add port conn. sts. checking in_dwc2_hcd_resume()function.
- usb: dwc2: Fix suspend state in host mode for partial power down.
- usb: dwc2: Fix wakeup detected and session request interrupt handlers.
- usb: dwc2: Add descriptive debug messages for Partial Power Down mode.
- usb: dwc2: Fixdwc2_restore_device_registers()function.The patches which didn't reach to LKML are:
- usb: dwc2: Add enter/exit hibernation from system issued suspend/resume
- usb: dwc2: ClearGINTSTS_RESTOREDONEbit after restore is generated.
- usb: dwc2: Clearfifo_mapwhen resetting core.
- usb: dwc2: Allow exiting hibernation from gpwrdn rst detect
- usb: dwc2: Fix hibernation between host and device modes.
- usb: dwc2: Updatedwc2_handle_usb_suspend_intrfunction.
- usb: dwc2: Add default param to control power optimization.
- usb: dwc2: Reset DEVADDR after exiting gadget hibernation.Submitting all of the patches together in this version.
Changes from V0:
- Replaced 1 withDWC2_POWER_DOWN_PARAM_PARTIALin commit "9eed02b9fe96usb: dwc2: Fix wakeup detected and session request interrupt handlers.Artur Petrosyan (14):
usb: dwc2: Fixdwc2_restore_device_registers()function.
usb: dwc2: Add descriptive debug messages for Partial Power Down mode.
usb: dwc2: Fix wakeup detected and session request interrupt handlers.
usb: dwc2: Fix suspend state in host mode for partial power down.
usb: dwc2: Add port conn. sts. checking in_dwc2_hcd_resume()
function.
usb: dwc2: Add part. power down exit from
dwc2_conn_id_status_change().
usb: dwc2: Reset DEVADDR after exiting gadget hibernation.
usb: dwc2: Add default param to control power optimization.
usb: dwc2: Updatedwc2_handle_usb_suspend_intrfunction.
usb: dwc2: Fix hibernation between host and device modes.
usb: dwc2: Allow exiting hibernation from gpwrdn rst detect
usb: dwc2: Clearfifo_mapwhen resetting core.
usb: dwc2: ClearGINTSTS_RESTOREDONEbit after restore is generated.
usb: dwc2: Add enter/exit hibernation from system issued
suspend/resumeQuoted 5 lines
Quoted 2 lines
patches don't apply.
Do we need to wait for Minas's acknowledge or there is problem related to the patches?
It looks like the problem is that my patches won the race and Felipe applied them before yours. Thus, presumably, it'll be up to you to rebase your patches atop mine and re-submit. Specifically, you can do:
git remote add
linux_usb_balbigit://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git
git fetchlinux_usb_balbi
git checkout linux_usb_balbi/testing/nextIf you do that and then try to apply your patches you'll find that they no longer apply. AKA try running:
for patch in 10909749 10909737 10909739 10909745 10909533 \ 10909531 10909747 10909535 10909523 10909741 10909525 \ 10909751 10909527 10909743; do curl -L https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.kernel.org_patch_-24-257Bpatch-257D_mbox&d=DwIBaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=9hPBFKCJ_nBjJhGVrrlYOeOQjP_HlVzYqrC_D7niMJI&m=gzzEDEbxflLMgk9I-7Re9ytRMvyc_B8iZEmg2xGZN5E&s=aWT1hYtXeIeY8ClQ0sNYxwkmJFKDz4iaa4DchNwx3_w&e= | git amdone
You'll see:
Quoted 4 lines
Applying: usb: dwc2: Fix wakeup detected and session request interrupt handlers.
error: patch failed:drivers/usb/dwc2/core_intr.c:435
error:drivers/usb/dwc2/core_intr.c: patch does not apply
Patch failed at 0001 usb: dwc2: Fix wakeup detected and session request interrupt handlers.NOTE: before reposting it might be a good idea to apply the last 3 patches in my series as per [1] before sending up your series. Since Felipe has already applied patches #1 and #2 in that series presumably he'll also apply #3 - #5.
I know it'a also up to me to try testing our your patches. It's still on my list to give it a shot...
Quoted 9 lines
P.S: It's helpful if you CC LKML on patches and discussions about them. That allows the magic "permalink via message ID" on lkml.kernel.org and also allows your patches to be found on lore.kernel.org/patchwork/-Doug
Besides the issue that comes from the patch "usb: dwc2: Fix wakeup detected and session request interrupt handlers." there is one more serious conflict with one of your patches.
So the patch "usb: dwc2: bus suspend/resume for hosts with
DWC2_POWER_DOWN_PARAM_NONE" have had also been added to the "balbi/testing/next" before my patch series which conflicts with two of my patches.Quoted 7 lines
1. usb: dwc2: Fix suspend state in host mode for partial power down.
2. usb: dwc2: Add enter/exit hibernation from system issued suspend/resumeThis patch introduced by you "usb: dwc2: bus suspend/resume for hosts with
DWC2_POWER_DOWN_PARAM_NONE" got a little bit of issue. It eliminates entering hibernation through system issued suspend by checking "if (hsotg->params.power_down>DWC2_POWER_DOWN_PARAM_PARTIAL)"
Quoted 9 lines
To be fair, the patch does not make entering hibernation worse, does it? Specifically, I'll point to this part of the diff:
Quoted 1 line
- if (
hsotg->params.power_down!=DWC2_POWER_DOWN_PARAM_PARTIAL) {+ if (
hsotg->params.power_down>DWC2_POWER_DOWN_PARAM_PARTIAL) {As you can see, if
power_down==DWC2_POWER_DOWN_PARAM_HIBERNATIONthe flow for this "if" test is exactly the same before and after my patch.
From the point of making hibernation worse no it doesn't.
Quoted 9 lines
Quoted 3 lines
. As per the patch you mention that it fixes suspend/resume flow for Altera SOCFPGA and Chrome OS 3.14 kernel tree. I assume that the board has the Partial Power Down enabled core that is why it works out.
I mentioned some of this in my cover letter [1]. To rehash, I said "Turning on partial power down on rk3288 doesn't "just work". I don't get hotplug events. This is despite dwc2 auto-detecting that we are power optimized."
Have you tested your patch on any board?
Quoted 54 lines
...it is certainly possible that partial power down would work on rk3288 if someone had the time to debug it.
NOTE: I don't have an Altera SOCFPGA, but I'll mention that a previous iteration of my patch (written by Kever Yang at Rockchip) was reverted because it _broke_ Altera SOCFPGAS. Given my requests to test the new version have been met with silence, I'm inclined to land this and hope it's all good. If there are problems then hopefully some actual details can be provided. Last time there were none provided.
Quoted 2 lines
Quoted 1 line
However, we don't just support the Partial Power Down feature enabled
cores. We also support Hibernation feature enabled cores.
Sure, but the code that is actually landed upstream (even before my series) almost certainly doesn't function for Hibernation. As pointed out above the entire "
_dwc2_hcd_suspend()" function had a great big:if (
hsotg->params.power_down!=DWC2_POWER_DOWN_PARAM_PARTIAL)
gotoskip_power_saving;...which, as far as I could tell, meant that Hibernation could not possible work.
Quoted 6 lines
The patch set that had been introduced by me which includes "usb: dwc2:
Add enter/exit hibernation from system issued suspend/resume" patch adds support for both hibernation and Partial Power Down feature enabled cores and fixes several of Partial Power Down and hibernation related issues.Quoted 6 lines
This patch set may fix all of the issues related with Altera SOCFPGA or Chrome OS 3.14 kernel tree.
That is why we asked you to test the patch set before we could ACK or have chance to debug your patch deeper to see the help of it and to provide you information related to it.
It may well fix my problems and maybe I can use partial power down now. That'd be nice. It was on my list and I would have worked on it last week except that your patches weren't on the mailing list then. ...so I moved on to some other work. To avoid context switching too much I needed to get to a stopping point before testing your patches. I was hoping to have some nice rebased patches from you to test today, but maybe I'll try a hand at rebasing them myself.
NOTE also that though I ported this change from the Chrome OS 3.14 kernel tree, I'm actually currently working on the Chrome OS 4.19 tree. I also made sure to test the changes on mainline Linux.
Quoted 3 lines
So now I can rebase my changes to the "balbi/testing/next" but I will have to take the logic of skipping Hibernation out otherwise we will have problems with hibernation enabled cores.
Quoted 7 lines
As per above, please have a careful look at my patches and you'll see that I was not introducing code that skipped hibernation. I was keeping the same flow as the old code that skipped hibernation. So if you are making hibernation work there should be no problems removing that.
Ok so after rebase it may be removed.
Quoted 11 lines
Quoted 5 lines
We can ask Balbi to permanently suspend adding of the patch "usb: dwc2:
bus suspend/resume for hosts withDWC2_POWER_DOWN_PARAM_NONE" and my patch series to his "testing/next". After you can have a chance to test my patch series we can see if the patches are acknowledged and ask Balbi
to add them.Personally I'd prefer if Felipe finished landing my series and then you rebased atop it. As I said I'm convinced I'm not making your hibernation case any worse. If you know of actual things that are made worse by my series then that would be a reason not to land it, but so far I haven't been convinced > [1]
Quoted 1 line
I have had a look on your patch and made some comments.
Also, tested your patch "usb: dwc2: bus suspend/resume for hosts with DWC2_POWER_DOWN_PARAM_NONE" on HAPS-DX. With this patch or without it when I have a device connected core enters to partial power down and doesn't exit from it. The attached device cannot be used.
Signature · 2 lines
Regards, Artur