2021-04-07 20:44:47

by Artur Petrosyan

[permalink] [raw]
Subject: [PATCH 00/14] usb: dwc2: Fix Partial Power down issues.

This patch set fixes and improves the Partial Power Down mode for
dwc2 core.
It adds support for the following cases
1. Entering and exiting partial power down when a port is
suspended, resumed, port reset is asserted.
2. Exiting the partial power down mode before removing driver.
3. Exiting partial power down in wakeup detected interrupt handler.
4. Exiting from partial power down mode when connector ID.
status changes to "connId B

It updates and fixes the implementation of dwc2 entering and
exiting partial power down mode when the system (PC) is suspended.

The patch set also improves the implementation of function handlers
for entering and exiting host or device partial power down.

NOTE: This is the second patch set in the power saving mode fixes
series.
This patch set is part of multiple series and is continuation
of the "usb: dwc2: Fix and improve power saving modes" patch set.
(Patch set link: https://marc.info/?l=linux-usb&m=160379622403975&w=2).
The patches that were included in the "usb: dwc2:
Fix and improve power saving modes" which was submitted
earlier was too large and needed to be split up into
smaller patch sets.


Artur Petrosyan (14):
usb: dwc2: Add device partial power down functions
usb: dwc2: Add host partial power down functions
usb: dwc2: Update enter and exit partial power down functions
usb: dwc2: Add partial power down exit flow in wakeup intr.
usb: dwc2: Update port suspend/resume function definitions.
usb: dwc2: Add enter partial power down when port is suspended
usb: dwc2: Add exit partial power down when port is resumed
usb: dwc2: Add exit partial power down when port reset is asserted
usb: dwc2: Add part. power down exit from
dwc2_conn_id_status_change().
usb: dwc2: Allow exit partial power down in urb enqueue
usb: dwc2: Fix session request interrupt handler
usb: dwc2: Update partial power down entering by system suspend
usb: dwc2: Fix partial power down exiting by system resume
usb: dwc2: Add exit partial power down before removing driver

drivers/usb/dwc2/core.c | 113 ++-------
drivers/usb/dwc2/core.h | 27 ++-
drivers/usb/dwc2/core_intr.c | 46 ++--
drivers/usb/dwc2/gadget.c | 148 ++++++++++-
drivers/usb/dwc2/hcd.c | 458 +++++++++++++++++++++++++----------
drivers/usb/dwc2/hw.h | 1 +
drivers/usb/dwc2/platform.c | 11 +-
7 files changed, 558 insertions(+), 246 deletions(-)


base-commit: e9fcb07704fcef6fa6d0333fd2b3a62442eaf45b
--
2.25.1


2021-04-07 20:47:55

by Artur Petrosyan

[permalink] [raw]
Subject: [PATCH 13/14] usb: dwc2: Fix partial power down exiting by system resume

Fixes the implementation of exiting from partial power down
power saving mode when PC is resumed.

Added port connection status checking which prevents exiting from
Partial Power Down mode from _dwc2_hcd_resume() if not in Partial
Power Down mode.

Rearranged the implementation to get rid of many "if"
statements.

NOTE: Switch case statement is used for hibernation partial
power down and clock gating mode determination. In this patch
only Partial Power Down is implemented the Hibernation and
clock gating implementations are planned to be added.

Cc: <[email protected]>
Fixes: 6f6d70597c15 ("usb: dwc2: bus suspend/resume for hosts with DWC2_POWER_DOWN_PARAM_NONE")
Signed-off-by: Artur Petrosyan <[email protected]>
---
drivers/usb/dwc2/hcd.c | 90 +++++++++++++++++++++---------------------
1 file changed, 46 insertions(+), 44 deletions(-)

diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index 34030bafdff4..f096006df96f 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -4427,7 +4427,7 @@ static int _dwc2_hcd_resume(struct usb_hcd *hcd)
{
struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd);
unsigned long flags;
- u32 pcgctl;
+ u32 hprt0;
int ret = 0;

spin_lock_irqsave(&hsotg->lock, flags);
@@ -4438,11 +4438,40 @@ static int _dwc2_hcd_resume(struct usb_hcd *hcd)
if (hsotg->lx_state != DWC2_L2)
goto unlock;

- if (hsotg->params.power_down > DWC2_POWER_DOWN_PARAM_PARTIAL) {
+ hprt0 = dwc2_read_hprt0(hsotg);
+
+ /*
+ * Added port connection status checking which prevents exiting from
+ * Partial Power Down mode from _dwc2_hcd_resume() if not in Partial
+ * Power Down mode.
+ */
+ if (hprt0 & HPRT0_CONNSTS) {
+ hsotg->lx_state = DWC2_L0;
+ goto unlock;
+ }
+
+ switch (hsotg->params.power_down) {
+ case DWC2_POWER_DOWN_PARAM_PARTIAL:
+ ret = dwc2_exit_partial_power_down(hsotg, 0, true);
+ if (ret)
+ dev_err(hsotg->dev,
+ "exit partial_power_down failed\n");
+ /*
+ * Set HW accessible bit before powering on the controller
+ * since an interrupt may rise.
+ */
+ set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
+ break;
+ case DWC2_POWER_DOWN_PARAM_HIBERNATION:
+ case DWC2_POWER_DOWN_PARAM_NONE:
+ default:
hsotg->lx_state = DWC2_L0;
goto unlock;
}

+ /* Change Root port status, as port status change occurred after resume.*/
+ hsotg->flags.b.port_suspend_change = 1;
+
/*
* Enable power if not already done.
* This must not be spinlocked since duration
@@ -4454,52 +4483,25 @@ static int _dwc2_hcd_resume(struct usb_hcd *hcd)
spin_lock_irqsave(&hsotg->lock, flags);
}

- if (hsotg->params.power_down == DWC2_POWER_DOWN_PARAM_PARTIAL) {
- /*
- * Set HW accessible bit before powering on the controller
- * since an interrupt may rise.
- */
- set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
-
-
- /* Exit partial_power_down */
- ret = dwc2_exit_partial_power_down(hsotg, 0, true);
- if (ret && (ret != -ENOTSUPP))
- dev_err(hsotg->dev, "exit partial_power_down failed\n");
- } else {
- pcgctl = readl(hsotg->regs + PCGCTL);
- pcgctl &= ~PCGCTL_STOPPCLK;
- writel(pcgctl, hsotg->regs + PCGCTL);
- }
-
- hsotg->lx_state = DWC2_L0;
-
+ /* Enable external vbus supply after resuming the port. */
spin_unlock_irqrestore(&hsotg->lock, flags);
+ dwc2_vbus_supply_init(hsotg);

- if (hsotg->bus_suspended) {
- spin_lock_irqsave(&hsotg->lock, flags);
- hsotg->flags.b.port_suspend_change = 1;
- spin_unlock_irqrestore(&hsotg->lock, flags);
- dwc2_port_resume(hsotg);
- } else {
- if (hsotg->params.power_down == DWC2_POWER_DOWN_PARAM_PARTIAL) {
- dwc2_vbus_supply_init(hsotg);
-
- /* Wait for controller to correctly update D+/D- level */
- usleep_range(3000, 5000);
- }
+ /* Wait for controller to correctly update D+/D- level */
+ usleep_range(3000, 5000);
+ spin_lock_irqsave(&hsotg->lock, flags);

- /*
- * Clear Port Enable and Port Status changes.
- * Enable Port Power.
- */
- dwc2_writel(hsotg, HPRT0_PWR | HPRT0_CONNDET |
- HPRT0_ENACHG, HPRT0);
- /* Wait for controller to detect Port Connect */
- usleep_range(5000, 7000);
- }
+ /*
+ * Clear Port Enable and Port Status changes.
+ * Enable Port Power.
+ */
+ dwc2_writel(hsotg, HPRT0_PWR | HPRT0_CONNDET |
+ HPRT0_ENACHG, HPRT0);

- return ret;
+ /* Wait for controller to detect Port Connect */
+ spin_unlock_irqrestore(&hsotg->lock, flags);
+ usleep_range(5000, 7000);
+ spin_lock_irqsave(&hsotg->lock, flags);
unlock:
spin_unlock_irqrestore(&hsotg->lock, flags);

--
2.25.1

2021-04-08 06:00:31

by Artur Petrosyan

[permalink] [raw]
Subject: Re: [PATCH 00/14] usb: dwc2: Fix Partial Power down issues.

Hi Greg,

On 4/7/2021 14:00, Artur Petrosyan wrote:
> This patch set fixes and improves the Partial Power Down mode for
> dwc2 core.
> It adds support for the following cases
> 1. Entering and exiting partial power down when a port is
> suspended, resumed, port reset is asserted.
> 2. Exiting the partial power down mode before removing driver.
> 3. Exiting partial power down in wakeup detected interrupt handler.
> 4. Exiting from partial power down mode when connector ID.
> status changes to "connId B
>
> It updates and fixes the implementation of dwc2 entering and
> exiting partial power down mode when the system (PC) is suspended.
>
> The patch set also improves the implementation of function handlers
> for entering and exiting host or device partial power down.
>
> NOTE: This is the second patch set in the power saving mode fixes
> series.
> This patch set is part of multiple series and is continuation
> of the "usb: dwc2: Fix and improve power saving modes" patch set.
> (Patch set link: https://marc.info/?l=linux-usb&m=160379622403975&w=2).
> The patches that were included in the "usb: dwc2:
> Fix and improve power saving modes" which was submitted
> earlier was too large and needed to be split up into
> smaller patch sets.
>
>
> Artur Petrosyan (14):
> usb: dwc2: Add device partial power down functions
> usb: dwc2: Add host partial power down functions
> usb: dwc2: Update enter and exit partial power down functions
> usb: dwc2: Add partial power down exit flow in wakeup intr.
> usb: dwc2: Update port suspend/resume function definitions.
> usb: dwc2: Add enter partial power down when port is suspended
> usb: dwc2: Add exit partial power down when port is resumed
> usb: dwc2: Add exit partial power down when port reset is asserted
> usb: dwc2: Add part. power down exit from
> dwc2_conn_id_status_change().
> usb: dwc2: Allow exit partial power down in urb enqueue
> usb: dwc2: Fix session request interrupt handler
> usb: dwc2: Update partial power down entering by system suspend
> usb: dwc2: Fix partial power down exiting by system resume
> usb: dwc2: Add exit partial power down before removing driver
>
> drivers/usb/dwc2/core.c | 113 ++-------
> drivers/usb/dwc2/core.h | 27 ++-
> drivers/usb/dwc2/core_intr.c | 46 ++--
> drivers/usb/dwc2/gadget.c | 148 ++++++++++-
> drivers/usb/dwc2/hcd.c | 458 +++++++++++++++++++++++++----------
> drivers/usb/dwc2/hw.h | 1 +
> drivers/usb/dwc2/platform.c | 11 +-
> 7 files changed, 558 insertions(+), 246 deletions(-)
>
>
> base-commit: e9fcb07704fcef6fa6d0333fd2b3a62442eaf45b
>
I have submitted this patch set yesterday. It contains 14 patches. But
only 2 of those patches were received by LKML only the cover letter and
the 13th patch.
(https://lore.kernel.org/linux-usb/[email protected]/T/#t)

I checked here at Synopsys, Minas did receive all the patches as his
email is in To list. Could this be an issue of vger.kernel.org mailing
server?

Because I checked every local possibility that could result to such
behavior. The patch 13 which was received by LKML has the similar
content as the other patches.

The mailing tool that was used is ssmtp, checked all the configurations
everything is fine.

Could you please suggest what should I do in this situation?

Regards,
Artur




2021-04-08 06:16:38

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 00/14] usb: dwc2: Fix Partial Power down issues.

On Thu, Apr 08, 2021 at 05:57:57AM +0000, Artur Petrosyan wrote:
> Hi Greg,
>
> On 4/7/2021 14:00, Artur Petrosyan wrote:
> > This patch set fixes and improves the Partial Power Down mode for
> > dwc2 core.
> > It adds support for the following cases
> > 1. Entering and exiting partial power down when a port is
> > suspended, resumed, port reset is asserted.
> > 2. Exiting the partial power down mode before removing driver.
> > 3. Exiting partial power down in wakeup detected interrupt handler.
> > 4. Exiting from partial power down mode when connector ID.
> > status changes to "connId B
> >
> > It updates and fixes the implementation of dwc2 entering and
> > exiting partial power down mode when the system (PC) is suspended.
> >
> > The patch set also improves the implementation of function handlers
> > for entering and exiting host or device partial power down.
> >
> > NOTE: This is the second patch set in the power saving mode fixes
> > series.
> > This patch set is part of multiple series and is continuation
> > of the "usb: dwc2: Fix and improve power saving modes" patch set.
> > (Patch set link: https://marc.info/?l=linux-usb&m=160379622403975&w=2).
> > The patches that were included in the "usb: dwc2:
> > Fix and improve power saving modes" which was submitted
> > earlier was too large and needed to be split up into
> > smaller patch sets.
> >
> >
> > Artur Petrosyan (14):
> > usb: dwc2: Add device partial power down functions
> > usb: dwc2: Add host partial power down functions
> > usb: dwc2: Update enter and exit partial power down functions
> > usb: dwc2: Add partial power down exit flow in wakeup intr.
> > usb: dwc2: Update port suspend/resume function definitions.
> > usb: dwc2: Add enter partial power down when port is suspended
> > usb: dwc2: Add exit partial power down when port is resumed
> > usb: dwc2: Add exit partial power down when port reset is asserted
> > usb: dwc2: Add part. power down exit from
> > dwc2_conn_id_status_change().
> > usb: dwc2: Allow exit partial power down in urb enqueue
> > usb: dwc2: Fix session request interrupt handler
> > usb: dwc2: Update partial power down entering by system suspend
> > usb: dwc2: Fix partial power down exiting by system resume
> > usb: dwc2: Add exit partial power down before removing driver
> >
> > drivers/usb/dwc2/core.c | 113 ++-------
> > drivers/usb/dwc2/core.h | 27 ++-
> > drivers/usb/dwc2/core_intr.c | 46 ++--
> > drivers/usb/dwc2/gadget.c | 148 ++++++++++-
> > drivers/usb/dwc2/hcd.c | 458 +++++++++++++++++++++++++----------
> > drivers/usb/dwc2/hw.h | 1 +
> > drivers/usb/dwc2/platform.c | 11 +-
> > 7 files changed, 558 insertions(+), 246 deletions(-)
> >
> >
> > base-commit: e9fcb07704fcef6fa6d0333fd2b3a62442eaf45b
> >
> I have submitted this patch set yesterday. It contains 14 patches. But
> only 2 of those patches were received by LKML only the cover letter and
> the 13th patch.
> (https://lore.kernel.org/linux-usb/[email protected]/T/#t)
>
> I checked here at Synopsys, Minas did receive all the patches as his
> email is in To list. Could this be an issue of vger.kernel.org mailing
> server?
>
> Because I checked every local possibility that could result to such
> behavior. The patch 13 which was received by LKML has the similar
> content as the other patches.
>
> The mailing tool that was used is ssmtp, checked all the configurations
> everything is fine.
>
> Could you please suggest what should I do in this situation?

Odd, I got them here, but lore seems to not have them :(

Can you just resend them as a "v2" series so we know which to review and
let's see if that works better...

thanks,

greg k-h

2021-04-08 07:30:20

by Artur Petrosyan

[permalink] [raw]
Subject: [PATCH v2 00/14] usb: dwc2: Fix Partial Power down issues.

This patch set fixes and improves the Partial Power Down mode for
dwc2 core.
It adds support for the following cases
1. Entering and exiting partial power down when a port is
suspended, resumed, port reset is asserted.
2. Exiting the partial power down mode before removing driver.
3. Exiting partial power down in wakeup detected interrupt handler.
4. Exiting from partial power down mode when connector ID.
status changes to "connId B

It updates and fixes the implementation of dwc2 entering and
exiting partial power down mode when the system (PC) is suspended.

The patch set also improves the implementation of function handlers
for entering and exiting host or device partial power down.

NOTE: This is the second patch set in the power saving mode fixes
series.
This patch set is part of multiple series and is continuation
of the "usb: dwc2: Fix and improve power saving modes" patch set.
(Patch set link: https://marc.info/?l=linux-usb&m=160379622403975&w=2).
The patches that were included in the "usb: dwc2:
Fix and improve power saving modes" which was submitted
earlier was too large and needed to be split up into
smaller patch sets.

Changes since V1:
No changes in the patches or the source code.
Sending the second version of the patch set because the first version
was not received by vger.kernel.org.



Artur Petrosyan (14):
usb: dwc2: Add device partial power down functions
usb: dwc2: Add host partial power down functions
usb: dwc2: Update enter and exit partial power down functions
usb: dwc2: Add partial power down exit flow in wakeup intr.
usb: dwc2: Update port suspend/resume function definitions.
usb: dwc2: Add enter partial power down when port is suspended
usb: dwc2: Add exit partial power down when port is resumed
usb: dwc2: Add exit partial power down when port reset is asserted
usb: dwc2: Add part. power down exit from
dwc2_conn_id_status_change().
usb: dwc2: Allow exit partial power down in urb enqueue
usb: dwc2: Fix session request interrupt handler
usb: dwc2: Update partial power down entering by system suspend
usb: dwc2: Fix partial power down exiting by system resume
usb: dwc2: Add exit partial power down before removing driver

drivers/usb/dwc2/core.c | 113 ++-------
drivers/usb/dwc2/core.h | 27 ++-
drivers/usb/dwc2/core_intr.c | 46 ++--
drivers/usb/dwc2/gadget.c | 148 ++++++++++-
drivers/usb/dwc2/hcd.c | 458 +++++++++++++++++++++++++----------
drivers/usb/dwc2/hw.h | 1 +
drivers/usb/dwc2/platform.c | 11 +-
7 files changed, 558 insertions(+), 246 deletions(-)


base-commit: e9fcb07704fcef6fa6d0333fd2b3a62442eaf45b
--
2.25.1

2021-04-08 07:32:30

by Artur Petrosyan

[permalink] [raw]
Subject: [PATCH v2 13/14] usb: dwc2: Fix partial power down exiting by system resume

Fixes the implementation of exiting from partial power down
power saving mode when PC is resumed.

Added port connection status checking which prevents exiting from
Partial Power Down mode from _dwc2_hcd_resume() if not in Partial
Power Down mode.

Rearranged the implementation to get rid of many "if"
statements.

NOTE: Switch case statement is used for hibernation partial
power down and clock gating mode determination. In this patch
only Partial Power Down is implemented the Hibernation and
clock gating implementations are planned to be added.

Cc: <[email protected]>
Fixes: 6f6d70597c15 ("usb: dwc2: bus suspend/resume for hosts with DWC2_POWER_DOWN_PARAM_NONE")
Signed-off-by: Artur Petrosyan <[email protected]>
---
Changes in v2:
- None

drivers/usb/dwc2/hcd.c | 90 +++++++++++++++++++++---------------------
1 file changed, 46 insertions(+), 44 deletions(-)

diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index 34030bafdff4..f096006df96f 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -4427,7 +4427,7 @@ static int _dwc2_hcd_resume(struct usb_hcd *hcd)
{
struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd);
unsigned long flags;
- u32 pcgctl;
+ u32 hprt0;
int ret = 0;

spin_lock_irqsave(&hsotg->lock, flags);
@@ -4438,11 +4438,40 @@ static int _dwc2_hcd_resume(struct usb_hcd *hcd)
if (hsotg->lx_state != DWC2_L2)
goto unlock;

- if (hsotg->params.power_down > DWC2_POWER_DOWN_PARAM_PARTIAL) {
+ hprt0 = dwc2_read_hprt0(hsotg);
+
+ /*
+ * Added port connection status checking which prevents exiting from
+ * Partial Power Down mode from _dwc2_hcd_resume() if not in Partial
+ * Power Down mode.
+ */
+ if (hprt0 & HPRT0_CONNSTS) {
+ hsotg->lx_state = DWC2_L0;
+ goto unlock;
+ }
+
+ switch (hsotg->params.power_down) {
+ case DWC2_POWER_DOWN_PARAM_PARTIAL:
+ ret = dwc2_exit_partial_power_down(hsotg, 0, true);
+ if (ret)
+ dev_err(hsotg->dev,
+ "exit partial_power_down failed\n");
+ /*
+ * Set HW accessible bit before powering on the controller
+ * since an interrupt may rise.
+ */
+ set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
+ break;
+ case DWC2_POWER_DOWN_PARAM_HIBERNATION:
+ case DWC2_POWER_DOWN_PARAM_NONE:
+ default:
hsotg->lx_state = DWC2_L0;
goto unlock;
}

+ /* Change Root port status, as port status change occurred after resume.*/
+ hsotg->flags.b.port_suspend_change = 1;
+
/*
* Enable power if not already done.
* This must not be spinlocked since duration
@@ -4454,52 +4483,25 @@ static int _dwc2_hcd_resume(struct usb_hcd *hcd)
spin_lock_irqsave(&hsotg->lock, flags);
}

- if (hsotg->params.power_down == DWC2_POWER_DOWN_PARAM_PARTIAL) {
- /*
- * Set HW accessible bit before powering on the controller
- * since an interrupt may rise.
- */
- set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
-
-
- /* Exit partial_power_down */
- ret = dwc2_exit_partial_power_down(hsotg, 0, true);
- if (ret && (ret != -ENOTSUPP))
- dev_err(hsotg->dev, "exit partial_power_down failed\n");
- } else {
- pcgctl = readl(hsotg->regs + PCGCTL);
- pcgctl &= ~PCGCTL_STOPPCLK;
- writel(pcgctl, hsotg->regs + PCGCTL);
- }
-
- hsotg->lx_state = DWC2_L0;
-
+ /* Enable external vbus supply after resuming the port. */
spin_unlock_irqrestore(&hsotg->lock, flags);
+ dwc2_vbus_supply_init(hsotg);

- if (hsotg->bus_suspended) {
- spin_lock_irqsave(&hsotg->lock, flags);
- hsotg->flags.b.port_suspend_change = 1;
- spin_unlock_irqrestore(&hsotg->lock, flags);
- dwc2_port_resume(hsotg);
- } else {
- if (hsotg->params.power_down == DWC2_POWER_DOWN_PARAM_PARTIAL) {
- dwc2_vbus_supply_init(hsotg);
-
- /* Wait for controller to correctly update D+/D- level */
- usleep_range(3000, 5000);
- }
+ /* Wait for controller to correctly update D+/D- level */
+ usleep_range(3000, 5000);
+ spin_lock_irqsave(&hsotg->lock, flags);

- /*
- * Clear Port Enable and Port Status changes.
- * Enable Port Power.
- */
- dwc2_writel(hsotg, HPRT0_PWR | HPRT0_CONNDET |
- HPRT0_ENACHG, HPRT0);
- /* Wait for controller to detect Port Connect */
- usleep_range(5000, 7000);
- }
+ /*
+ * Clear Port Enable and Port Status changes.
+ * Enable Port Power.
+ */
+ dwc2_writel(hsotg, HPRT0_PWR | HPRT0_CONNDET |
+ HPRT0_ENACHG, HPRT0);

- return ret;
+ /* Wait for controller to detect Port Connect */
+ spin_unlock_irqrestore(&hsotg->lock, flags);
+ usleep_range(5000, 7000);
+ spin_lock_irqsave(&hsotg->lock, flags);
unlock:
spin_unlock_irqrestore(&hsotg->lock, flags);

--
2.25.1

2021-04-08 09:18:55

by Artur Petrosyan

[permalink] [raw]
Subject: Re: [PATCH v2 00/14] usb: dwc2: Fix Partial Power down issues.

Hi Greg,

On 4/8/2021 11:28, Artur Petrosyan wrote:
> This patch set fixes and improves the Partial Power Down mode for
> dwc2 core.
> It adds support for the following cases
> 1. Entering and exiting partial power down when a port is
> suspended, resumed, port reset is asserted.
> 2. Exiting the partial power down mode before removing driver.
> 3. Exiting partial power down in wakeup detected interrupt handler.
> 4. Exiting from partial power down mode when connector ID.
> status changes to "connId B
>
> It updates and fixes the implementation of dwc2 entering and
> exiting partial power down mode when the system (PC) is suspended.
>
> The patch set also improves the implementation of function handlers
> for entering and exiting host or device partial power down.
>
> NOTE: This is the second patch set in the power saving mode fixes
> series.
> This patch set is part of multiple series and is continuation
> of the "usb: dwc2: Fix and improve power saving modes" patch set.
> (Patch set link: https://marc.info/?l=linux-usb&m=160379622403975&w=2).
> The patches that were included in the "usb: dwc2:
> Fix and improve power saving modes" which was submitted
> earlier was too large and needed to be split up into
> smaller patch sets.
>
> Changes since V1:
> No changes in the patches or the source code.
> Sending the second version of the patch set because the first version
> was not received by vger.kernel.org.
>
>
>
> Artur Petrosyan (14):
> usb: dwc2: Add device partial power down functions
> usb: dwc2: Add host partial power down functions
> usb: dwc2: Update enter and exit partial power down functions
> usb: dwc2: Add partial power down exit flow in wakeup intr.
> usb: dwc2: Update port suspend/resume function definitions.
> usb: dwc2: Add enter partial power down when port is suspended
> usb: dwc2: Add exit partial power down when port is resumed
> usb: dwc2: Add exit partial power down when port reset is asserted
> usb: dwc2: Add part. power down exit from
> dwc2_conn_id_status_change().
> usb: dwc2: Allow exit partial power down in urb enqueue
> usb: dwc2: Fix session request interrupt handler
> usb: dwc2: Update partial power down entering by system suspend
> usb: dwc2: Fix partial power down exiting by system resume
> usb: dwc2: Add exit partial power down before removing driver
>
> drivers/usb/dwc2/core.c | 113 ++-------
> drivers/usb/dwc2/core.h | 27 ++-
> drivers/usb/dwc2/core_intr.c | 46 ++--
> drivers/usb/dwc2/gadget.c | 148 ++++++++++-
> drivers/usb/dwc2/hcd.c | 458 +++++++++++++++++++++++++----------
> drivers/usb/dwc2/hw.h | 1 +
> drivers/usb/dwc2/platform.c | 11 +-
> 7 files changed, 558 insertions(+), 246 deletions(-)
>
>
> base-commit: e9fcb07704fcef6fa6d0333fd2b3a62442eaf45b
>

Re sending as a "v2" did not work :(.
The patches are not in lore again.

Could the issue be with a comma in the end of To: or Cc: list?
Let me remove the comma in the end of those lists and try sending as "v3".

Regards,
Artur

2021-04-08 09:45:52

by Artur Petrosyan

[permalink] [raw]
Subject: [PATCH v3 00/14] usb: dwc2: Fix Partial Power down issues.

This patch set fixes and improves the Partial Power Down mode for
dwc2 core.
It adds support for the following cases
1. Entering and exiting partial power down when a port is
suspended, resumed, port reset is asserted.
2. Exiting the partial power down mode before removing driver.
3. Exiting partial power down in wakeup detected interrupt handler.
4. Exiting from partial power down mode when connector ID.
status changes to "connId B

It updates and fixes the implementation of dwc2 entering and
exiting partial power down mode when the system (PC) is suspended.

The patch set also improves the implementation of function handlers
for entering and exiting host or device partial power down.

NOTE: This is the second patch set in the power saving mode fixes
series.
This patch set is part of multiple series and is continuation
of the "usb: dwc2: Fix and improve power saving modes" patch set.
(Patch set link: https://marc.info/?l=linux-usb&m=160379622403975&w=2).
The patches that were included in the "usb: dwc2:
Fix and improve power saving modes" which was submitted
earlier was too large and needed to be split up into
smaller patch sets.

Changes since V2:
No changes in the patches or the source code.
Assuming that the issue due to which the patches are not reaching to
vger.kernel.org is a comma in the end of To: or Cc: lists removed
commas in the end of those lists in each email of patches.


Artur Petrosyan (14):
usb: dwc2: Add device partial power down functions
usb: dwc2: Add host partial power down functions
usb: dwc2: Update enter and exit partial power down functions
usb: dwc2: Add partial power down exit flow in wakeup intr.
usb: dwc2: Update port suspend/resume function definitions.
usb: dwc2: Add enter partial power down when port is suspended
usb: dwc2: Add exit partial power down when port is resumed
usb: dwc2: Add exit partial power down when port reset is asserted
usb: dwc2: Add part. power down exit from
dwc2_conn_id_status_change().
usb: dwc2: Allow exit partial power down in urb enqueue
usb: dwc2: Fix session request interrupt handler
usb: dwc2: Update partial power down entering by system suspend
usb: dwc2: Fix partial power down exiting by system resume
usb: dwc2: Add exit partial power down before removing driver

drivers/usb/dwc2/core.c | 113 ++-------
drivers/usb/dwc2/core.h | 27 ++-
drivers/usb/dwc2/core_intr.c | 46 ++--
drivers/usb/dwc2/gadget.c | 148 ++++++++++-
drivers/usb/dwc2/hcd.c | 458 +++++++++++++++++++++++++----------
drivers/usb/dwc2/hw.h | 1 +
drivers/usb/dwc2/platform.c | 11 +-
7 files changed, 558 insertions(+), 246 deletions(-)


base-commit: e9fcb07704fcef6fa6d0333fd2b3a62442eaf45b
--
2.25.1

2021-04-08 09:46:00

by Artur Petrosyan

[permalink] [raw]
Subject: [PATCH v3 01/14] usb: dwc2: Add device partial power down functions

For device mode Partial Power Down entering and exiting
separate functions are needed to implement the logic.
Earlier the logic was implemented in one function. Which was
confusing the readability. Also both host and device implementations
were in the same function.

- Added device partial power down functions which must be called
by dwc2_enter_partial_power_down()/dwc2_exit_partial_power_down()
functions.

- Added "in_ppd" flag in "dwc2_hsotg" struct to indicate the
core state after entering into partial power down mode.

Added function names:
dwc2_gadget_enter_partial_power_down()
dwc2_gadget_exit_partial_power_down()

NOTE: There is a checkpatch "CHECK" warning on "udelay(100)".
The delay is needed to properly exit gadget Partial Power Down
A delay less than 100 doesn't work.

Signed-off-by: Artur Petrosyan <[email protected]>
Acked-by: Minas Harutyunyan <[email protected]>
---
drivers/usb/dwc2/core.h | 10 +++
drivers/usb/dwc2/gadget.c | 128 ++++++++++++++++++++++++++++++++++++++
2 files changed, 138 insertions(+)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index d0ebe721fb98..ed54d834138d 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -865,6 +865,7 @@ struct dwc2_hregs_backup {
* @gadget_enabled: Peripheral mode sub-driver initialization indicator.
* @ll_hw_enabled: Status of low-level hardware resources.
* @hibernated: True if core is hibernated
+ * @in_ppd: True if core is partial power down mode.
* @reset_phy_on_wake: Quirk saying that we should assert PHY reset on a
* remote wakeup.
* @phy_off_for_suspend: Status of whether we turned the PHY off at suspend.
@@ -1060,6 +1061,7 @@ struct dwc2_hsotg {
unsigned int gadget_enabled:1;
unsigned int ll_hw_enabled:1;
unsigned int hibernated:1;
+ unsigned int in_ppd:1;
unsigned int reset_phy_on_wake:1;
unsigned int need_phy_for_wake:1;
unsigned int phy_off_for_suspend:1;
@@ -1409,6 +1411,9 @@ int dwc2_restore_device_registers(struct dwc2_hsotg *hsotg, int remote_wakeup);
int dwc2_gadget_enter_hibernation(struct dwc2_hsotg *hsotg);
int dwc2_gadget_exit_hibernation(struct dwc2_hsotg *hsotg,
int rem_wakeup, int reset);
+int dwc2_gadget_enter_partial_power_down(struct dwc2_hsotg *hsotg);
+int dwc2_gadget_exit_partial_power_down(struct dwc2_hsotg *hsotg,
+ bool restore);
int dwc2_hsotg_tx_fifo_count(struct dwc2_hsotg *hsotg);
int dwc2_hsotg_tx_fifo_total_depth(struct dwc2_hsotg *hsotg);
int dwc2_hsotg_tx_fifo_average_depth(struct dwc2_hsotg *hsotg);
@@ -1442,6 +1447,11 @@ static inline int dwc2_gadget_enter_hibernation(struct dwc2_hsotg *hsotg)
static inline int dwc2_gadget_exit_hibernation(struct dwc2_hsotg *hsotg,
int rem_wakeup, int reset)
{ return 0; }
+static inline int dwc2_gadget_enter_partial_power_down(struct dwc2_hsotg *hsotg)
+{ return 0; }
+static inline int dwc2_gadget_exit_partial_power_down(struct dwc2_hsotg *hsotg,
+ bool restore)
+{ return 0; }
static inline int dwc2_hsotg_tx_fifo_count(struct dwc2_hsotg *hsotg)
{ return 0; }
static inline int dwc2_hsotg_tx_fifo_total_depth(struct dwc2_hsotg *hsotg)
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index ad4c94366dad..98a2a63c67ae 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -5351,3 +5351,131 @@ int dwc2_gadget_exit_hibernation(struct dwc2_hsotg *hsotg,

return ret;
}
+
+/**
+ * dwc2_gadget_enter_partial_power_down() - Put controller in partial
+ * power down.
+ *
+ * @hsotg: Programming view of the DWC_otg controller
+ *
+ * Return: non-zero if failed to enter device partial power down.
+ *
+ * This function is for entering device mode partial power down.
+ */
+int dwc2_gadget_enter_partial_power_down(struct dwc2_hsotg *hsotg)
+{
+ u32 pcgcctl;
+ int ret = 0;
+
+ dev_dbg(hsotg->dev, "Entering device partial power down started.\n");
+
+ /* Backup all registers */
+ ret = dwc2_backup_global_registers(hsotg);
+ if (ret) {
+ dev_err(hsotg->dev, "%s: failed to backup global registers\n",
+ __func__);
+ return ret;
+ }
+
+ ret = dwc2_backup_device_registers(hsotg);
+ if (ret) {
+ dev_err(hsotg->dev, "%s: failed to backup device registers\n",
+ __func__);
+ return ret;
+ }
+
+ /*
+ * Clear any pending interrupts since dwc2 will not be able to
+ * clear them after entering partial_power_down.
+ */
+ dwc2_writel(hsotg, 0xffffffff, GINTSTS);
+
+ /* Put the controller in low power state */
+ pcgcctl = dwc2_readl(hsotg, PCGCTL);
+
+ pcgcctl |= PCGCTL_PWRCLMP;
+ dwc2_writel(hsotg, pcgcctl, PCGCTL);
+ udelay(5);
+
+ pcgcctl |= PCGCTL_RSTPDWNMODULE;
+ dwc2_writel(hsotg, pcgcctl, PCGCTL);
+ udelay(5);
+
+ pcgcctl |= PCGCTL_STOPPCLK;
+ dwc2_writel(hsotg, pcgcctl, PCGCTL);
+
+ /* Set in_ppd flag to 1 as here core enters suspend. */
+ hsotg->in_ppd = 1;
+ hsotg->lx_state = DWC2_L2;
+
+ dev_dbg(hsotg->dev, "Entering device partial power down completed.\n");
+
+ return ret;
+}
+
+/*
+ * dwc2_gadget_exit_partial_power_down() - Exit controller from device partial
+ * power down.
+ *
+ * @hsotg: Programming view of the DWC_otg controller
+ * @restore: indicates whether need to restore the registers or not.
+ *
+ * Return: non-zero if failed to exit device partial power down.
+ *
+ * This function is for exiting from device mode partial power down.
+ */
+int dwc2_gadget_exit_partial_power_down(struct dwc2_hsotg *hsotg,
+ bool restore)
+{
+ u32 pcgcctl;
+ u32 dctl;
+ struct dwc2_dregs_backup *dr;
+ int ret = 0;
+
+ dr = &hsotg->dr_backup;
+
+ dev_dbg(hsotg->dev, "Exiting device partial Power Down started.\n");
+
+ pcgcctl = dwc2_readl(hsotg, PCGCTL);
+ pcgcctl &= ~PCGCTL_STOPPCLK;
+ dwc2_writel(hsotg, pcgcctl, PCGCTL);
+
+ pcgcctl = dwc2_readl(hsotg, PCGCTL);
+ pcgcctl &= ~PCGCTL_PWRCLMP;
+ dwc2_writel(hsotg, pcgcctl, PCGCTL);
+
+ pcgcctl = dwc2_readl(hsotg, PCGCTL);
+ pcgcctl &= ~PCGCTL_RSTPDWNMODULE;
+ dwc2_writel(hsotg, pcgcctl, PCGCTL);
+
+ udelay(100);
+ if (restore) {
+ ret = dwc2_restore_global_registers(hsotg);
+ if (ret) {
+ dev_err(hsotg->dev, "%s: failed to restore registers\n",
+ __func__);
+ return ret;
+ }
+ /* Restore DCFG */
+ dwc2_writel(hsotg, dr->dcfg, DCFG);
+
+ ret = dwc2_restore_device_registers(hsotg, 0);
+ if (ret) {
+ dev_err(hsotg->dev, "%s: failed to restore device registers\n",
+ __func__);
+ return ret;
+ }
+ }
+
+ /* Set the Power-On Programming done bit */
+ dctl = dwc2_readl(hsotg, DCTL);
+ dctl |= DCTL_PWRONPRGDONE;
+ dwc2_writel(hsotg, dctl, DCTL);
+
+ /* Set in_ppd flag to 0 as here core exits from suspend. */
+ hsotg->in_ppd = 0;
+ hsotg->lx_state = DWC2_L0;
+
+ dev_dbg(hsotg->dev, "Exiting device partial Power Down completed.\n");
+ return ret;
+}
--
2.25.1

2021-04-08 09:46:25

by Artur Petrosyan

[permalink] [raw]
Subject: [PATCH v3 03/14] usb: dwc2: Update enter and exit partial power down functions

These are wrapper functions which are calling device or host
enter/exit partial power down functions.

This change is done because we need to separate device and
host partial power down functions as the programming flow
has a lot of difference between host and device. With this
update during partial power down exit driver relies on
backup value of "GOTGCTL_CURMODE_HOST" to determine the
mode of core before entering to PPD.

Signed-off-by: Artur Petrosyan <[email protected]>
Acked-by: Minas Harutyunyan <[email protected]>
---
drivers/usb/dwc2/core.c | 113 ++++++-----------------------------
drivers/usb/dwc2/core.h | 3 +-
drivers/usb/dwc2/core_intr.c | 21 ++++---
drivers/usb/dwc2/gadget.c | 20 ++++---
drivers/usb/dwc2/hcd.c | 2 +-
drivers/usb/dwc2/hw.h | 1 +
6 files changed, 45 insertions(+), 115 deletions(-)

diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index fec17a2d2447..cb65f7f60573 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -131,54 +131,26 @@ int dwc2_restore_global_registers(struct dwc2_hsotg *hsotg)
* dwc2_exit_partial_power_down() - Exit controller from Partial Power Down.
*
* @hsotg: Programming view of the DWC_otg controller
+ * @rem_wakeup: indicates whether resume is initiated by Reset.
* @restore: Controller registers need to be restored
*/
-int dwc2_exit_partial_power_down(struct dwc2_hsotg *hsotg, bool restore)
+int dwc2_exit_partial_power_down(struct dwc2_hsotg *hsotg, int rem_wakeup,
+ bool restore)
{
- u32 pcgcctl;
- int ret = 0;
-
- if (hsotg->params.power_down != DWC2_POWER_DOWN_PARAM_PARTIAL)
- return -ENOTSUPP;
-
- pcgcctl = dwc2_readl(hsotg, PCGCTL);
- pcgcctl &= ~PCGCTL_STOPPCLK;
- dwc2_writel(hsotg, pcgcctl, PCGCTL);
-
- pcgcctl = dwc2_readl(hsotg, PCGCTL);
- pcgcctl &= ~PCGCTL_PWRCLMP;
- dwc2_writel(hsotg, pcgcctl, PCGCTL);
-
- pcgcctl = dwc2_readl(hsotg, PCGCTL);
- pcgcctl &= ~PCGCTL_RSTPDWNMODULE;
- dwc2_writel(hsotg, pcgcctl, PCGCTL);
+ struct dwc2_gregs_backup *gr;

- udelay(100);
- if (restore) {
- ret = dwc2_restore_global_registers(hsotg);
- if (ret) {
- dev_err(hsotg->dev, "%s: failed to restore registers\n",
- __func__);
- return ret;
- }
- if (dwc2_is_host_mode(hsotg)) {
- ret = dwc2_restore_host_registers(hsotg);
- if (ret) {
- dev_err(hsotg->dev, "%s: failed to restore host registers\n",
- __func__);
- return ret;
- }
- } else {
- ret = dwc2_restore_device_registers(hsotg, 0);
- if (ret) {
- dev_err(hsotg->dev, "%s: failed to restore device registers\n",
- __func__);
- return ret;
- }
- }
- }
+ gr = &hsotg->gr_backup;

- return ret;
+ /*
+ * Restore host or device regisers with the same mode core enterted
+ * to partial power down by checking "GOTGCTL_CURMODE_HOST" backup
+ * value of the "gotgctl" register.
+ */
+ if (gr->gotgctl & GOTGCTL_CURMODE_HOST)
+ return dwc2_host_exit_partial_power_down(hsotg, rem_wakeup,
+ restore);
+ else
+ return dwc2_gadget_exit_partial_power_down(hsotg, restore);
}

/**
@@ -188,57 +160,10 @@ int dwc2_exit_partial_power_down(struct dwc2_hsotg *hsotg, bool restore)
*/
int dwc2_enter_partial_power_down(struct dwc2_hsotg *hsotg)
{
- u32 pcgcctl;
- int ret = 0;
-
- if (!hsotg->params.power_down)
- return -ENOTSUPP;
-
- /* Backup all registers */
- ret = dwc2_backup_global_registers(hsotg);
- if (ret) {
- dev_err(hsotg->dev, "%s: failed to backup global registers\n",
- __func__);
- return ret;
- }
-
- if (dwc2_is_host_mode(hsotg)) {
- ret = dwc2_backup_host_registers(hsotg);
- if (ret) {
- dev_err(hsotg->dev, "%s: failed to backup host registers\n",
- __func__);
- return ret;
- }
- } else {
- ret = dwc2_backup_device_registers(hsotg);
- if (ret) {
- dev_err(hsotg->dev, "%s: failed to backup device registers\n",
- __func__);
- return ret;
- }
- }
-
- /*
- * Clear any pending interrupts since dwc2 will not be able to
- * clear them after entering partial_power_down.
- */
- dwc2_writel(hsotg, 0xffffffff, GINTSTS);
-
- /* Put the controller in low power state */
- pcgcctl = dwc2_readl(hsotg, PCGCTL);
-
- pcgcctl |= PCGCTL_PWRCLMP;
- dwc2_writel(hsotg, pcgcctl, PCGCTL);
- ndelay(20);
-
- pcgcctl |= PCGCTL_RSTPDWNMODULE;
- dwc2_writel(hsotg, pcgcctl, PCGCTL);
- ndelay(20);
-
- pcgcctl |= PCGCTL_STOPPCLK;
- dwc2_writel(hsotg, pcgcctl, PCGCTL);
-
- return ret;
+ if (dwc2_is_host_mode(hsotg))
+ return dwc2_host_enter_partial_power_down(hsotg);
+ else
+ return dwc2_gadget_enter_partial_power_down(hsotg);
}

/**
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 1a97df8bf5cb..39037709a2ad 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -1303,7 +1303,8 @@ static inline bool dwc2_is_hs_iot(struct dwc2_hsotg *hsotg)
*/
int dwc2_core_reset(struct dwc2_hsotg *hsotg, bool skip_wait);
int dwc2_enter_partial_power_down(struct dwc2_hsotg *hsotg);
-int dwc2_exit_partial_power_down(struct dwc2_hsotg *hsotg, bool restore);
+int dwc2_exit_partial_power_down(struct dwc2_hsotg *hsotg, int rem_wakeup,
+ bool restore);
int dwc2_enter_hibernation(struct dwc2_hsotg *hsotg, int is_host);
int dwc2_exit_hibernation(struct dwc2_hsotg *hsotg, int rem_wakeup,
int reset, int is_host);
diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c
index 55f1d14fc414..1fb957ce6c25 100644
--- a/drivers/usb/dwc2/core_intr.c
+++ b/drivers/usb/dwc2/core_intr.c
@@ -315,9 +315,10 @@ static void dwc2_handle_session_req_intr(struct dwc2_hsotg *hsotg)
hsotg->lx_state);

if (dwc2_is_device_mode(hsotg)) {
- if (hsotg->lx_state == DWC2_L2) {
- ret = dwc2_exit_partial_power_down(hsotg, true);
- if (ret && (ret != -ENOTSUPP))
+ if (hsotg->lx_state == DWC2_L2 && hsotg->in_ppd) {
+ ret = dwc2_exit_partial_power_down(hsotg, 0,
+ true);
+ if (ret)
dev_err(hsotg->dev,
"exit power_down failed\n");
}
@@ -406,18 +407,16 @@ static void dwc2_handle_wakeup_detected_intr(struct dwc2_hsotg *hsotg)
if (dwc2_is_device_mode(hsotg)) {
dev_dbg(hsotg->dev, "DSTS=0x%0x\n",
dwc2_readl(hsotg, DSTS));
- if (hsotg->lx_state == DWC2_L2) {
+ if (hsotg->lx_state == DWC2_L2 && hsotg->in_ppd) {
u32 dctl = dwc2_readl(hsotg, DCTL);
-
/* Clear Remote Wakeup Signaling */
dctl &= ~DCTL_RMTWKUPSIG;
dwc2_writel(hsotg, dctl, DCTL);
- ret = dwc2_exit_partial_power_down(hsotg, true);
- if (ret && (ret != -ENOTSUPP))
- dev_err(hsotg->dev, "exit power_down failed\n");
-
- /* Change to L0 state */
- hsotg->lx_state = DWC2_L0;
+ ret = dwc2_exit_partial_power_down(hsotg, 1,
+ true);
+ if (ret)
+ dev_err(hsotg->dev,
+ "exit partial_power_down failed\n");
call_gadget(hsotg, resume);
} else {
/* Change to L0 state */
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 98a2a63c67ae..e08baee4987b 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -3689,10 +3689,10 @@ static irqreturn_t dwc2_hsotg_irq(int irq, void *pw)
dwc2_writel(hsotg, GINTSTS_RESETDET, GINTSTS);

/* This event must be used only if controller is suspended */
- if (hsotg->lx_state == DWC2_L2) {
- dwc2_exit_partial_power_down(hsotg, true);
- hsotg->lx_state = DWC2_L0;
- }
+ if (hsotg->in_ppd && hsotg->lx_state == DWC2_L2)
+ dwc2_exit_partial_power_down(hsotg, 0, true);
+
+ hsotg->lx_state = DWC2_L0;
}

if (gintsts & (GINTSTS_USBRST | GINTSTS_RESETDET)) {
@@ -4615,11 +4615,15 @@ static int dwc2_hsotg_vbus_session(struct usb_gadget *gadget, int is_active)
spin_lock_irqsave(&hsotg->lock, flags);

/*
- * If controller is hibernated, it must exit from power_down
- * before being initialized / de-initialized
+ * If controller is in partial power down state, it must exit from
+ * that state before being initialized / de-initialized
*/
- if (hsotg->lx_state == DWC2_L2)
- dwc2_exit_partial_power_down(hsotg, false);
+ if (hsotg->lx_state == DWC2_L2 && hsotg->in_ppd)
+ /*
+ * No need to check the return value as
+ * registers are not being restored.
+ */
+ dwc2_exit_partial_power_down(hsotg, 0, false);

if (is_active) {
hsotg->op_state = OTG_STATE_B_PERIPHERAL;
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index 35e617be4bc3..dd0362e07444 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -4418,7 +4418,7 @@ static int _dwc2_hcd_resume(struct usb_hcd *hcd)


/* Exit partial_power_down */
- ret = dwc2_exit_partial_power_down(hsotg, true);
+ ret = dwc2_exit_partial_power_down(hsotg, 0, true);
if (ret && (ret != -ENOTSUPP))
dev_err(hsotg->dev, "exit partial_power_down failed\n");
} else {
diff --git a/drivers/usb/dwc2/hw.h b/drivers/usb/dwc2/hw.h
index c3d6dde2aca4..6b16fbf98bc6 100644
--- a/drivers/usb/dwc2/hw.h
+++ b/drivers/usb/dwc2/hw.h
@@ -44,6 +44,7 @@
#define GOTGCTL_CHIRPEN BIT(27)
#define GOTGCTL_MULT_VALID_BC_MASK (0x1f << 22)
#define GOTGCTL_MULT_VALID_BC_SHIFT 22
+#define GOTGCTL_CURMODE_HOST BIT(21)
#define GOTGCTL_OTGVER BIT(20)
#define GOTGCTL_BSESVLD BIT(19)
#define GOTGCTL_ASESVLD BIT(18)
--
2.25.1

2021-04-08 09:46:26

by Artur Petrosyan

[permalink] [raw]
Subject: [PATCH v3 02/14] usb: dwc2: Add host partial power down functions

For host mode Partial Power Down entering and exiting
separate functions are needed to implement the logic.
Earlier the logic was implemented in one function. Which was
confusing the readability. Also both host and device implementations
were in the same function.

- Added host partial power down functions which must be called
by dwc2_enter_partial_power_down()/dwc2_exit_partial_power_down()
functions.

Added function names:
dwc2_host_enter_partial_power_down()
dwc2_host_exit_partial_power_down()

NOTE: There is a checkpatch "CHECK" warning on "udelay(100)".
The delay is needed to properly exit gadget Partial Power Down
A delay less than 100 doesn't work.

Signed-off-by: Artur Petrosyan <[email protected]>
Acked-by: Minas Harutyunyan <[email protected]>
---
drivers/usb/dwc2/core.h | 8 ++
drivers/usb/dwc2/hcd.c | 160 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 168 insertions(+)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index ed54d834138d..1a97df8bf5cb 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -1474,6 +1474,9 @@ int dwc2_restore_host_registers(struct dwc2_hsotg *hsotg);
int dwc2_host_enter_hibernation(struct dwc2_hsotg *hsotg);
int dwc2_host_exit_hibernation(struct dwc2_hsotg *hsotg,
int rem_wakeup, int reset);
+int dwc2_host_enter_partial_power_down(struct dwc2_hsotg *hsotg);
+int dwc2_host_exit_partial_power_down(struct dwc2_hsotg *hsotg,
+ int rem_wakeup, bool restore);
bool dwc2_host_can_poweroff_phy(struct dwc2_hsotg *dwc2);
static inline void dwc2_host_schedule_phy_reset(struct dwc2_hsotg *hsotg)
{ schedule_work(&hsotg->phy_reset_work); }
@@ -1500,6 +1503,11 @@ static inline int dwc2_host_enter_hibernation(struct dwc2_hsotg *hsotg)
static inline int dwc2_host_exit_hibernation(struct dwc2_hsotg *hsotg,
int rem_wakeup, int reset)
{ return 0; }
+static inline int dwc2_host_enter_partial_power_down(struct dwc2_hsotg *hsotg)
+{ return 0; }
+static inline int dwc2_host_exit_partial_power_down(struct dwc2_hsotg *hsotg,
+ int rem_wakeup, bool restore)
+{ return 0; }
static inline bool dwc2_host_can_poweroff_phy(struct dwc2_hsotg *dwc2)
{ return false; }
static inline void dwc2_host_schedule_phy_reset(struct dwc2_hsotg *hsotg) {}
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index 1a9789ec5847..35e617be4bc3 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -5607,3 +5607,163 @@ bool dwc2_host_can_poweroff_phy(struct dwc2_hsotg *dwc2)
/* No reason to keep the PHY powered, so allow poweroff */
return true;
}
+
+/**
+ * dwc2_host_enter_partial_power_down() - Put controller in partial
+ * power down.
+ *
+ * @hsotg: Programming view of the DWC_otg controller
+ *
+ * Return: non-zero if failed to enter host partial power down.
+ *
+ * This function is for entering Host mode partial power down.
+ */
+int dwc2_host_enter_partial_power_down(struct dwc2_hsotg *hsotg)
+{
+ u32 pcgcctl;
+ u32 hprt0;
+ int ret = 0;
+
+ dev_dbg(hsotg->dev, "Entering host partial power down started.\n");
+
+ /* Put this port in suspend mode. */
+ hprt0 = dwc2_read_hprt0(hsotg);
+ hprt0 |= HPRT0_SUSP;
+ dwc2_writel(hsotg, hprt0, HPRT0);
+ udelay(5);
+
+ /* Wait for the HPRT0.PrtSusp register field to be set */
+ if (dwc2_hsotg_wait_bit_set(hsotg, HPRT0, HPRT0_SUSP, 3000))
+ dev_warn(hsotg->dev, "Suspend wasn't generated\n");
+
+ /* Backup all registers */
+ ret = dwc2_backup_global_registers(hsotg);
+ if (ret) {
+ dev_err(hsotg->dev, "%s: failed to backup global registers\n",
+ __func__);
+ return ret;
+ }
+
+ ret = dwc2_backup_host_registers(hsotg);
+ if (ret) {
+ dev_err(hsotg->dev, "%s: failed to backup host registers\n",
+ __func__);
+ return ret;
+ }
+
+ /*
+ * Clear any pending interrupts since dwc2 will not be able to
+ * clear them after entering partial_power_down.
+ */
+ dwc2_writel(hsotg, 0xffffffff, GINTSTS);
+
+ /* Put the controller in low power state */
+ pcgcctl = dwc2_readl(hsotg, PCGCTL);
+
+ pcgcctl |= PCGCTL_PWRCLMP;
+ dwc2_writel(hsotg, pcgcctl, PCGCTL);
+ udelay(5);
+
+ pcgcctl |= PCGCTL_RSTPDWNMODULE;
+ dwc2_writel(hsotg, pcgcctl, PCGCTL);
+ udelay(5);
+
+ pcgcctl |= PCGCTL_STOPPCLK;
+ dwc2_writel(hsotg, pcgcctl, PCGCTL);
+
+ /* Set in_ppd flag to 1 as here core enters suspend. */
+ hsotg->in_ppd = 1;
+ hsotg->lx_state = DWC2_L2;
+ hsotg->bus_suspended = true;
+
+ dev_dbg(hsotg->dev, "Entering host partial power down completed.\n");
+
+ return ret;
+}
+
+/*
+ * dwc2_host_exit_partial_power_down() - Exit controller from host partial
+ * power down.
+ *
+ * @hsotg: Programming view of the DWC_otg controller
+ * @rem_wakeup: indicates whether resume is initiated by Reset.
+ * @restore: indicates whether need to restore the registers or not.
+ *
+ * Return: non-zero if failed to exit host partial power down.
+ *
+ * This function is for exiting from Host mode partial power down.
+ */
+int dwc2_host_exit_partial_power_down(struct dwc2_hsotg *hsotg,
+ int rem_wakeup, bool restore)
+{
+ u32 pcgcctl;
+ int ret = 0;
+ u32 hprt0;
+
+ dev_dbg(hsotg->dev, "Exiting host partial power down started.\n");
+
+ pcgcctl = dwc2_readl(hsotg, PCGCTL);
+ pcgcctl &= ~PCGCTL_STOPPCLK;
+ dwc2_writel(hsotg, pcgcctl, PCGCTL);
+ udelay(5);
+
+ pcgcctl = dwc2_readl(hsotg, PCGCTL);
+ pcgcctl &= ~PCGCTL_PWRCLMP;
+ dwc2_writel(hsotg, pcgcctl, PCGCTL);
+ udelay(5);
+
+ pcgcctl = dwc2_readl(hsotg, PCGCTL);
+ pcgcctl &= ~PCGCTL_RSTPDWNMODULE;
+ dwc2_writel(hsotg, pcgcctl, PCGCTL);
+
+ udelay(100);
+ if (restore) {
+ ret = dwc2_restore_global_registers(hsotg);
+ if (ret) {
+ dev_err(hsotg->dev, "%s: failed to restore registers\n",
+ __func__);
+ return ret;
+ }
+
+ ret = dwc2_restore_host_registers(hsotg);
+ if (ret) {
+ dev_err(hsotg->dev, "%s: failed to restore host registers\n",
+ __func__);
+ return ret;
+ }
+ }
+
+ /* Drive resume signaling and exit suspend mode on the port. */
+ hprt0 = dwc2_read_hprt0(hsotg);
+ hprt0 |= HPRT0_RES;
+ hprt0 &= ~HPRT0_SUSP;
+ dwc2_writel(hsotg, hprt0, HPRT0);
+ udelay(5);
+
+ if (!rem_wakeup) {
+ /* Stop driveing resume signaling on the port. */
+ hprt0 = dwc2_read_hprt0(hsotg);
+ hprt0 &= ~HPRT0_RES;
+ dwc2_writel(hsotg, hprt0, HPRT0);
+
+ hsotg->bus_suspended = false;
+ } else {
+ /* Turn on the port power bit. */
+ hprt0 = dwc2_read_hprt0(hsotg);
+ hprt0 |= HPRT0_PWR;
+ dwc2_writel(hsotg, hprt0, HPRT0);
+
+ /* Connect hcd. */
+ dwc2_hcd_connect(hsotg);
+
+ mod_timer(&hsotg->wkp_timer,
+ jiffies + msecs_to_jiffies(71));
+ }
+
+ /* Set lx_state to and in_ppd to 0 as here core exits from suspend. */
+ hsotg->in_ppd = 0;
+ hsotg->lx_state = DWC2_L0;
+
+ dev_dbg(hsotg->dev, "Exiting host partial power down completed.\n");
+ return ret;
+}
--
2.25.1

2021-04-08 09:46:44

by Artur Petrosyan

[permalink] [raw]
Subject: [PATCH v3 04/14] usb: dwc2: Add partial power down exit flow in wakeup intr.

According to programming guide added host partial power
down exit flow in wakeup detected interrupt handler.

Signed-off-by: Artur Petrosyan <[email protected]>
Acked-by: Minas Harutyunyan <[email protected]>
---
drivers/usb/dwc2/core_intr.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c
index 1fb957ce6c25..0a7f9330907f 100644
--- a/drivers/usb/dwc2/core_intr.c
+++ b/drivers/usb/dwc2/core_intr.c
@@ -423,15 +423,14 @@ static void dwc2_handle_wakeup_detected_intr(struct dwc2_hsotg *hsotg)
hsotg->lx_state = DWC2_L0;
}
} else {
- if (hsotg->params.power_down)
- return;
-
- if (hsotg->lx_state != DWC2_L1) {
- u32 pcgcctl = dwc2_readl(hsotg, PCGCTL);
-
- /* Restart the Phy Clock */
- pcgcctl &= ~PCGCTL_STOPPCLK;
- dwc2_writel(hsotg, pcgcctl, PCGCTL);
+ if (hsotg->lx_state == DWC2_L2) {
+ if (hsotg->in_ppd) {
+ ret = dwc2_exit_partial_power_down(hsotg, 1,
+ true);
+ if (ret)
+ dev_err(hsotg->dev,
+ "exit partial_power_down failed\n");
+ }

/*
* If we've got this quirk then the PHY is stuck upon
--
2.25.1

2021-04-08 09:46:58

by Artur Petrosyan

[permalink] [raw]
Subject: [PATCH v3 05/14] usb: dwc2: Update port suspend/resume function definitions.

Earlier "dwc2_port_suspend()" and "dwc2_port_resume()" functions
were implemented without proper description and host or device mode
difference.

- Added "dwc2_port_suspend" and "dwc2_port_resume" functions to
"core.h" header file.

- Updated function description in documentation.

Signed-off-by: Artur Petrosyan <[email protected]>
---
Changes in v3:
- None
Changes in v2:
- None

drivers/usb/dwc2/core.h | 4 ++++
drivers/usb/dwc2/hcd.c | 25 +++++++++++++++++++------
2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 39037709a2ad..b7d99cf9e84c 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -1470,6 +1470,8 @@ void dwc2_hcd_connect(struct dwc2_hsotg *hsotg);
void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg, bool force);
void dwc2_hcd_start(struct dwc2_hsotg *hsotg);
int dwc2_core_init(struct dwc2_hsotg *hsotg, bool initial_setup);
+void dwc2_port_suspend(struct dwc2_hsotg *hsotg, u16 windex);
+void dwc2_port_resume(struct dwc2_hsotg *hsotg);
int dwc2_backup_host_registers(struct dwc2_hsotg *hsotg);
int dwc2_restore_host_registers(struct dwc2_hsotg *hsotg);
int dwc2_host_enter_hibernation(struct dwc2_hsotg *hsotg);
@@ -1493,6 +1495,8 @@ static inline void dwc2_hcd_start(struct dwc2_hsotg *hsotg) {}
static inline void dwc2_hcd_remove(struct dwc2_hsotg *hsotg) {}
static inline int dwc2_core_init(struct dwc2_hsotg *hsotg, bool initial_setup)
{ return 0; }
+static inline void dwc2_port_suspend(struct dwc2_hsotg *hsotg, u16 windex) {}
+static inline void dwc2_port_resume(struct dwc2_hsotg *hsotg) {}
static inline int dwc2_hcd_init(struct dwc2_hsotg *hsotg)
{ return 0; }
static inline int dwc2_backup_host_registers(struct dwc2_hsotg *hsotg)
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index dd0362e07444..f4247a66c2b2 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -56,8 +56,6 @@
#include "core.h"
#include "hcd.h"

-static void dwc2_port_resume(struct dwc2_hsotg *hsotg);
-
/*
* =========================================================================
* Host Core Layer Functions
@@ -3277,8 +3275,16 @@ static int dwc2_host_is_b_hnp_enabled(struct dwc2_hsotg *hsotg)
return hcd->self.b_hnp_enable;
}

-/* Must NOT be called with interrupt disabled or spinlock held */
-static void dwc2_port_suspend(struct dwc2_hsotg *hsotg, u16 windex)
+/**
+ * dwc2_port_suspend() - Put controller in suspend mode for host.
+ *
+ * @hsotg: Programming view of the DWC_otg controller
+ * @windex: The control request wIndex field
+ *
+ * This function is for entering Host mode suspend.
+ * Must NOT be called with interrupt disabled or spinlock held.
+ */
+void dwc2_port_suspend(struct dwc2_hsotg *hsotg, u16 windex)
{
unsigned long flags;
u32 hprt0;
@@ -3328,8 +3334,15 @@ static void dwc2_port_suspend(struct dwc2_hsotg *hsotg, u16 windex)
}
}

-/* Must NOT be called with interrupt disabled or spinlock held */
-static void dwc2_port_resume(struct dwc2_hsotg *hsotg)
+/**
+ * dwc2_port_resume() - Exit controller from suspend mode for host.
+ *
+ * @hsotg: Programming view of the DWC_otg controller
+ *
+ * This function is for exiting Host mode suspend.
+ * Must NOT be called with interrupt disabled or spinlock held.
+ */
+void dwc2_port_resume(struct dwc2_hsotg *hsotg)
{
unsigned long flags;
u32 hprt0;
--
2.25.1

2021-04-08 09:47:23

by Artur Petrosyan

[permalink] [raw]
Subject: [PATCH v3 07/14] usb: dwc2: Add exit partial power down when port is resumed

Added flow of exiting Partial Power Down in
"dwc2_port_resume()" function when core receives resume.

NOTE: Switch case statement is used for hibernation partial
power down and clock gating mode determination. In this patch
only Partial Power Down is implemented the Hibernation and
clock gating implementations are planned to be added.

Signed-off-by: Artur Petrosyan <[email protected]>
---
Changes in v3:
- None
Changes in v2:
- None

drivers/usb/dwc2/core.h | 5 ++--
drivers/usb/dwc2/hcd.c | 61 ++++++++++++++++++++++++++---------------
2 files changed, 42 insertions(+), 24 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 76807abd753b..5a7850482e57 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -1471,7 +1471,7 @@ void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg, bool force);
void dwc2_hcd_start(struct dwc2_hsotg *hsotg);
int dwc2_core_init(struct dwc2_hsotg *hsotg, bool initial_setup);
int dwc2_port_suspend(struct dwc2_hsotg *hsotg, u16 windex);
-void dwc2_port_resume(struct dwc2_hsotg *hsotg);
+int dwc2_port_resume(struct dwc2_hsotg *hsotg);
int dwc2_backup_host_registers(struct dwc2_hsotg *hsotg);
int dwc2_restore_host_registers(struct dwc2_hsotg *hsotg);
int dwc2_host_enter_hibernation(struct dwc2_hsotg *hsotg);
@@ -1497,7 +1497,8 @@ static inline int dwc2_core_init(struct dwc2_hsotg *hsotg, bool initial_setup)
{ return 0; }
static inline int dwc2_port_suspend(struct dwc2_hsotg *hsotg, u16 windex)
{ return 0; }
-static inline void dwc2_port_resume(struct dwc2_hsotg *hsotg) {}
+static inline int dwc2_port_resume(struct dwc2_hsotg *hsotg)
+{ return 0; }
static inline int dwc2_hcd_init(struct dwc2_hsotg *hsotg)
{ return 0; }
static inline int dwc2_backup_host_registers(struct dwc2_hsotg *hsotg)
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index e7fb0d5940bc..720354df014b 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -3353,44 +3353,61 @@ int dwc2_port_suspend(struct dwc2_hsotg *hsotg, u16 windex)
*
* @hsotg: Programming view of the DWC_otg controller
*
+ * Return: non-zero if failed to exit suspend mode for host.
+ *
* This function is for exiting Host mode suspend.
* Must NOT be called with interrupt disabled or spinlock held.
*/
-void dwc2_port_resume(struct dwc2_hsotg *hsotg)
+int dwc2_port_resume(struct dwc2_hsotg *hsotg)
{
unsigned long flags;
u32 hprt0;
u32 pcgctl;
+ int ret = 0;

spin_lock_irqsave(&hsotg->lock, flags);

- /*
- * If power_down is supported, Phy clock is already resumed
- * after registers restore.
- */
- if (!hsotg->params.power_down) {
- pcgctl = dwc2_readl(hsotg, PCGCTL);
- pcgctl &= ~PCGCTL_STOPPCLK;
- dwc2_writel(hsotg, pcgctl, PCGCTL);
+ switch (hsotg->params.power_down) {
+ case DWC2_POWER_DOWN_PARAM_PARTIAL:
+ ret = dwc2_exit_partial_power_down(hsotg, 0, true);
+ if (ret)
+ dev_err(hsotg->dev,
+ "exit partial_power_down failed.\n");
+ break;
+ case DWC2_POWER_DOWN_PARAM_HIBERNATION:
+ case DWC2_POWER_DOWN_PARAM_NONE:
+ default:
+ /*
+ * If power_down is supported, Phy clock is already resumed
+ * after registers restore.
+ */
+ if (!hsotg->params.power_down) {
+ pcgctl = dwc2_readl(hsotg, PCGCTL);
+ pcgctl &= ~PCGCTL_STOPPCLK;
+ dwc2_writel(hsotg, pcgctl, PCGCTL);
+ spin_unlock_irqrestore(&hsotg->lock, flags);
+ msleep(20);
+ spin_lock_irqsave(&hsotg->lock, flags);
+ }
+
+ hprt0 = dwc2_read_hprt0(hsotg);
+ hprt0 |= HPRT0_RES;
+ hprt0 &= ~HPRT0_SUSP;
+ dwc2_writel(hsotg, hprt0, HPRT0);
spin_unlock_irqrestore(&hsotg->lock, flags);
- msleep(20);
+
+ msleep(USB_RESUME_TIMEOUT);
+
spin_lock_irqsave(&hsotg->lock, flags);
+ hprt0 = dwc2_read_hprt0(hsotg);
+ hprt0 &= ~(HPRT0_RES | HPRT0_SUSP);
+ dwc2_writel(hsotg, hprt0, HPRT0);
+ hsotg->bus_suspended = false;
}

- hprt0 = dwc2_read_hprt0(hsotg);
- hprt0 |= HPRT0_RES;
- hprt0 &= ~HPRT0_SUSP;
- dwc2_writel(hsotg, hprt0, HPRT0);
spin_unlock_irqrestore(&hsotg->lock, flags);

- msleep(USB_RESUME_TIMEOUT);
-
- spin_lock_irqsave(&hsotg->lock, flags);
- hprt0 = dwc2_read_hprt0(hsotg);
- hprt0 &= ~(HPRT0_RES | HPRT0_SUSP);
- dwc2_writel(hsotg, hprt0, HPRT0);
- hsotg->bus_suspended = false;
- spin_unlock_irqrestore(&hsotg->lock, flags);
+ return ret;
}

/* Handles hub class-specific requests */
--
2.25.1

2021-04-08 09:47:26

by Artur Petrosyan

[permalink] [raw]
Subject: [PATCH v3 08/14] usb: dwc2: Add exit partial power down when port reset is asserted

Adds Partial Power Down exiting flow when set port feature
reset is received in suspended state.

Signed-off-by: Artur Petrosyan <[email protected]>
---
Changes in v3:
- None
Changes in v2:
- None

drivers/usb/dwc2/hcd.c | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index 720354df014b..7c7496719152 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -3694,6 +3694,15 @@ static int dwc2_hcd_hub_control(struct dwc2_hsotg *hsotg, u16 typereq,
if (hsotg->params.power_down == DWC2_POWER_DOWN_PARAM_HIBERNATION &&
hsotg->hibernated)
dwc2_exit_hibernation(hsotg, 0, 1, 1);
+
+ if (hsotg->in_ppd) {
+ retval = dwc2_exit_partial_power_down(hsotg, 1,
+ true);
+ if (retval)
+ dev_err(hsotg->dev,
+ "exit partial_power_down failed\n");
+ }
+
hprt0 = dwc2_read_hprt0(hsotg);
dev_dbg(hsotg->dev,
"SetPortFeature - USB_PORT_FEAT_RESET\n");
--
2.25.1

2021-04-08 09:47:58

by Artur Petrosyan

[permalink] [raw]
Subject: [PATCH v3 11/14] usb: dwc2: Fix session request interrupt handler

According to programming guide in host mode, port
power must be turned on in session request
interrupt handlers.

Cc: <[email protected]>
Fixes: 21795c826a45 ("usb: dwc2: exit hibernation on session request")
Signed-off-by: Artur Petrosyan <[email protected]>
Acked-by: Minas Harutyunyan <[email protected]>
---
drivers/usb/dwc2/core_intr.c | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c
index 0a7f9330907f..8c0152b514be 100644
--- a/drivers/usb/dwc2/core_intr.c
+++ b/drivers/usb/dwc2/core_intr.c
@@ -307,6 +307,7 @@ static void dwc2_handle_conn_id_status_change_intr(struct dwc2_hsotg *hsotg)
static void dwc2_handle_session_req_intr(struct dwc2_hsotg *hsotg)
{
int ret;
+ u32 hprt0;

/* Clear interrupt */
dwc2_writel(hsotg, GINTSTS_SESSREQINT, GINTSTS);
@@ -328,6 +329,13 @@ static void dwc2_handle_session_req_intr(struct dwc2_hsotg *hsotg)
* established
*/
dwc2_hsotg_disconnect(hsotg);
+ } else {
+ /* Turn on the port power bit. */
+ hprt0 = dwc2_read_hprt0(hsotg);
+ hprt0 |= HPRT0_PWR;
+ dwc2_writel(hsotg, hprt0, HPRT0);
+ /* Connect hcd after port power is set. */
+ dwc2_hcd_connect(hsotg);
}
}

--
2.25.1

2021-04-08 09:48:15

by Artur Petrosyan

[permalink] [raw]
Subject: [PATCH v3 12/14] usb: dwc2: Update partial power down entering by system suspend

With current implementation the port power is being disabled,
which is not required by the programming guide. Also, if there
is a system which works only in "DWC2_POWER_DOWN_PARAM_NONE"
(clock gating) mode the current implementation does not set
Gate hclk bit in pcgctl register.

Rearranges and updates the implementation of entering to partial
power down power saving mode when PC is suspended to get
rid of many "if" statements and removes disabling of port power.

NOTE: Switch case statement is used for hibernation partial
power down and clock gating mode determination. In this patch
only Partial Power Down is implemented the Hibernation and
clock gating implementations are planned to be added.

Signed-off-by: Artur Petrosyan <[email protected]>
---
Changes in v3:
- None
Changes in v2:
- None

drivers/usb/dwc2/hcd.c | 53 ++++++++++++++----------------------------
1 file changed, 18 insertions(+), 35 deletions(-)

diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index cb52bc41bfb8..34030bafdff4 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -4367,8 +4367,6 @@ static int _dwc2_hcd_suspend(struct usb_hcd *hcd)
struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd);
unsigned long flags;
int ret = 0;
- u32 hprt0;
- u32 pcgctl;

spin_lock_irqsave(&hsotg->lock, flags);

@@ -4384,47 +4382,32 @@ static int _dwc2_hcd_suspend(struct usb_hcd *hcd)
if (hsotg->op_state == OTG_STATE_B_PERIPHERAL)
goto unlock;

- if (hsotg->params.power_down != DWC2_POWER_DOWN_PARAM_PARTIAL ||
- hsotg->flags.b.port_connect_status == 0)
+ if (hsotg->bus_suspended)
goto skip_power_saving;

- /*
- * Drive USB suspend and disable port Power
- * if usb bus is not suspended.
- */
- if (!hsotg->bus_suspended) {
- hprt0 = dwc2_read_hprt0(hsotg);
- if (hprt0 & HPRT0_CONNSTS) {
- hprt0 |= HPRT0_SUSP;
- if (hsotg->params.power_down == DWC2_POWER_DOWN_PARAM_PARTIAL)
- hprt0 &= ~HPRT0_PWR;
- dwc2_writel(hsotg, hprt0, HPRT0);
- }
- if (hsotg->params.power_down == DWC2_POWER_DOWN_PARAM_PARTIAL) {
- spin_unlock_irqrestore(&hsotg->lock, flags);
- dwc2_vbus_supply_exit(hsotg);
- spin_lock_irqsave(&hsotg->lock, flags);
- } else {
- pcgctl = readl(hsotg->regs + PCGCTL);
- pcgctl |= PCGCTL_STOPPCLK;
- writel(pcgctl, hsotg->regs + PCGCTL);
- }
- }
+ if (hsotg->flags.b.port_connect_status == 0)
+ goto skip_power_saving;

- if (hsotg->params.power_down == DWC2_POWER_DOWN_PARAM_PARTIAL) {
+ switch (hsotg->params.power_down) {
+ case DWC2_POWER_DOWN_PARAM_PARTIAL:
/* Enter partial_power_down */
ret = dwc2_enter_partial_power_down(hsotg);
- if (ret) {
- if (ret != -ENOTSUPP)
- dev_err(hsotg->dev,
- "enter partial_power_down failed\n");
- goto skip_power_saving;
- }
-
- /* After entering partial_power_down, hardware is no more accessible */
+ if (ret)
+ dev_err(hsotg->dev,
+ "enter partial_power_down failed\n");
+ /* After entering suspend, hardware is not accessible */
clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
+ break;
+ case DWC2_POWER_DOWN_PARAM_HIBERNATION:
+ case DWC2_POWER_DOWN_PARAM_NONE:
+ default:
+ goto skip_power_saving;
}

+ spin_unlock_irqrestore(&hsotg->lock, flags);
+ dwc2_vbus_supply_exit(hsotg);
+ spin_lock_irqsave(&hsotg->lock, flags);
+
/* Ask phy to be suspended */
if (!IS_ERR_OR_NULL(hsotg->uphy)) {
spin_unlock_irqrestore(&hsotg->lock, flags);
--
2.25.1

2021-04-08 09:48:41

by Artur Petrosyan

[permalink] [raw]
Subject: [PATCH v3 14/14] usb: dwc2: Add exit partial power down before removing driver

When dwc2 core is in partial power down mode
loading driver again causes driver fail. Because in
that mode registers are not accessible.

Added a flow of exiting the partial power down mode
to avoid the driver reload failure.

Signed-off-by: Artur Petrosyan <[email protected]>
---
Changes in v3:
- None
Changes in v2:
- None

drivers/usb/dwc2/platform.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index 5f18acac7406..b28b8cd45799 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -316,6 +316,15 @@ static int dwc2_lowlevel_hw_init(struct dwc2_hsotg *hsotg)
static int dwc2_driver_remove(struct platform_device *dev)
{
struct dwc2_hsotg *hsotg = platform_get_drvdata(dev);
+ int ret = 0;
+
+ /* Exit Partial Power Down when driver is removed. */
+ if (hsotg->in_ppd) {
+ ret = dwc2_exit_partial_power_down(hsotg, 0, true);
+ if (ret)
+ dev_err(hsotg->dev,
+ "exit partial_power_down failed\n");
+ }

dwc2_debugfs_exit(hsotg);
if (hsotg->hcd_enabled)
@@ -334,7 +343,7 @@ static int dwc2_driver_remove(struct platform_device *dev)
reset_control_assert(hsotg->reset);
reset_control_assert(hsotg->reset_ecc);

- return 0;
+ return ret;
}

/**
--
2.25.1

2021-04-08 09:49:07

by Artur Petrosyan

[permalink] [raw]
Subject: [PATCH v3 06/14] usb: dwc2: Add enter partial power down when port is suspended

Adds flow of entering Partial Power Down in
"dwc2_port_suspend()" function when core receives suspend.

NOTE: Switch case statement is used for hibernation partial
power down and clock gating mode determination. In this patch
only Partial Power Down is implemented the Hibernation and
clock gating implementations are planned to be added.

Signed-off-by: Artur Petrosyan <[email protected]>
---
Changes in v3:
- None
Changes in v2:
- None

drivers/usb/dwc2/core.h | 5 +++--
drivers/usb/dwc2/hcd.c | 48 ++++++++++++++++++++++++++---------------
2 files changed, 34 insertions(+), 19 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index b7d99cf9e84c..76807abd753b 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -1470,7 +1470,7 @@ void dwc2_hcd_connect(struct dwc2_hsotg *hsotg);
void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg, bool force);
void dwc2_hcd_start(struct dwc2_hsotg *hsotg);
int dwc2_core_init(struct dwc2_hsotg *hsotg, bool initial_setup);
-void dwc2_port_suspend(struct dwc2_hsotg *hsotg, u16 windex);
+int dwc2_port_suspend(struct dwc2_hsotg *hsotg, u16 windex);
void dwc2_port_resume(struct dwc2_hsotg *hsotg);
int dwc2_backup_host_registers(struct dwc2_hsotg *hsotg);
int dwc2_restore_host_registers(struct dwc2_hsotg *hsotg);
@@ -1495,7 +1495,8 @@ static inline void dwc2_hcd_start(struct dwc2_hsotg *hsotg) {}
static inline void dwc2_hcd_remove(struct dwc2_hsotg *hsotg) {}
static inline int dwc2_core_init(struct dwc2_hsotg *hsotg, bool initial_setup)
{ return 0; }
-static inline void dwc2_port_suspend(struct dwc2_hsotg *hsotg, u16 windex) {}
+static inline int dwc2_port_suspend(struct dwc2_hsotg *hsotg, u16 windex)
+{ return 0; }
static inline void dwc2_port_resume(struct dwc2_hsotg *hsotg) {}
static inline int dwc2_hcd_init(struct dwc2_hsotg *hsotg)
{ return 0; }
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index f4247a66c2b2..e7fb0d5940bc 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -3281,15 +3281,18 @@ static int dwc2_host_is_b_hnp_enabled(struct dwc2_hsotg *hsotg)
* @hsotg: Programming view of the DWC_otg controller
* @windex: The control request wIndex field
*
+ * Return: non-zero if failed to enter suspend mode for host.
+ *
* This function is for entering Host mode suspend.
* Must NOT be called with interrupt disabled or spinlock held.
*/
-void dwc2_port_suspend(struct dwc2_hsotg *hsotg, u16 windex)
+int dwc2_port_suspend(struct dwc2_hsotg *hsotg, u16 windex)
{
unsigned long flags;
u32 hprt0;
u32 pcgctl;
u32 gotgctl;
+ int ret = 0;

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

@@ -3302,22 +3305,31 @@ void dwc2_port_suspend(struct dwc2_hsotg *hsotg, u16 windex)
hsotg->op_state = OTG_STATE_A_SUSPEND;
}

- hprt0 = dwc2_read_hprt0(hsotg);
- hprt0 |= HPRT0_SUSP;
- dwc2_writel(hsotg, hprt0, HPRT0);
-
- hsotg->bus_suspended = true;
-
- /*
- * If power_down is supported, Phy clock will be suspended
- * after registers are backuped.
- */
- if (!hsotg->params.power_down) {
- /* Suspend the Phy Clock */
- pcgctl = dwc2_readl(hsotg, PCGCTL);
- pcgctl |= PCGCTL_STOPPCLK;
- dwc2_writel(hsotg, pcgctl, PCGCTL);
- udelay(10);
+ switch (hsotg->params.power_down) {
+ case DWC2_POWER_DOWN_PARAM_PARTIAL:
+ ret = dwc2_enter_partial_power_down(hsotg);
+ if (ret)
+ dev_err(hsotg->dev,
+ "enter partial_power_down failed.\n");
+ break;
+ case DWC2_POWER_DOWN_PARAM_HIBERNATION:
+ case DWC2_POWER_DOWN_PARAM_NONE:
+ default:
+ hprt0 = dwc2_read_hprt0(hsotg);
+ hprt0 |= HPRT0_SUSP;
+ dwc2_writel(hsotg, hprt0, HPRT0);
+ hsotg->bus_suspended = true;
+ /*
+ * If power_down is supported, Phy clock will be suspended
+ * after registers are backuped.
+ */
+ if (!hsotg->params.power_down) {
+ /* Suspend the Phy Clock */
+ pcgctl = dwc2_readl(hsotg, PCGCTL);
+ pcgctl |= PCGCTL_STOPPCLK;
+ dwc2_writel(hsotg, pcgctl, PCGCTL);
+ udelay(10);
+ }
}

/* For HNP the bus must be suspended for at least 200ms */
@@ -3332,6 +3344,8 @@ void dwc2_port_suspend(struct dwc2_hsotg *hsotg, u16 windex)
} else {
spin_unlock_irqrestore(&hsotg->lock, flags);
}
+
+ return ret;
}

/**
--
2.25.1

2021-04-08 09:49:31

by Artur Petrosyan

[permalink] [raw]
Subject: [PATCH v3 09/14] usb: dwc2: Add part. power down exit from dwc2_conn_id_status_change().

Before changing to connector B exiting from Partial
Power Down is required.

- Added exiting from Partial Power Down mode when
connector ID status changes to "connId B".
Because if connector ID status changed to B connector
while core was in partial power down mode, HANG would
accrue from a soft reset.

Signed-off-by: Artur Petrosyan <[email protected]>
Acked-by: Minas Harutyunyan <[email protected]>
---
drivers/usb/dwc2/hcd.c | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index 7c7496719152..9529e9839961 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -3206,6 +3206,15 @@ static void dwc2_conn_id_status_change(struct work_struct *work)
if (count > 250)
dev_err(hsotg->dev,
"Connection id status change timed out\n");
+
+ /*
+ * Exit Partial Power Down without restoring registers.
+ * No need to check the return value as registers
+ * are not being restored.
+ */
+ if (hsotg->in_ppd && hsotg->lx_state == DWC2_L2)
+ dwc2_exit_partial_power_down(hsotg, 0, false);
+
hsotg->op_state = OTG_STATE_B_PERIPHERAL;
dwc2_core_init(hsotg, false);
dwc2_enable_global_interrupts(hsotg);
--
2.25.1

2021-04-08 09:49:45

by Artur Petrosyan

[permalink] [raw]
Subject: [PATCH v3 10/14] usb: dwc2: Allow exit partial power down in urb enqueue

When core is in partial power down state and an external
hub is connected, upper layer sends URB enqueue request,
which results in port reset issue.

Added exit from partial power down state to avoid port
reset issue and process upper layer request correctly.

Signed-off-by: Artur Petrosyan <[email protected]>
---
Changes in v3:
- None
Changes in v2:
- None

drivers/usb/dwc2/hcd.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index 9529e9839961..cb52bc41bfb8 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -4633,6 +4633,13 @@ static int _dwc2_hcd_urb_enqueue(struct usb_hcd *hcd, struct urb *urb,
dwc2_dump_urb_info(hcd, urb, "urb_enqueue");
}

+ if (hsotg->in_ppd) {
+ retval = dwc2_exit_partial_power_down(hsotg, 0, true);
+ if (retval)
+ dev_err(hsotg->dev,
+ "exit partial_power_down failed\n");
+ }
+
if (!ep)
return -EINVAL;

--
2.25.1

2021-04-08 09:50:19

by Artur Petrosyan

[permalink] [raw]
Subject: [PATCH v3 13/14] usb: dwc2: Fix partial power down exiting by system resume

Fixes the implementation of exiting from partial power down
power saving mode when PC is resumed.

Added port connection status checking which prevents exiting from
Partial Power Down mode from _dwc2_hcd_resume() if not in Partial
Power Down mode.

Rearranged the implementation to get rid of many "if"
statements.

NOTE: Switch case statement is used for hibernation partial
power down and clock gating mode determination. In this patch
only Partial Power Down is implemented the Hibernation and
clock gating implementations are planned to be added.

Cc: <[email protected]>
Fixes: 6f6d70597c15 ("usb: dwc2: bus suspend/resume for hosts with DWC2_POWER_DOWN_PARAM_NONE")
Signed-off-by: Artur Petrosyan <[email protected]>
---
Changes in v3:
- None
Changes in v2:
- None

drivers/usb/dwc2/hcd.c | 90 +++++++++++++++++++++---------------------
1 file changed, 46 insertions(+), 44 deletions(-)

diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index 34030bafdff4..f096006df96f 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -4427,7 +4427,7 @@ static int _dwc2_hcd_resume(struct usb_hcd *hcd)
{
struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd);
unsigned long flags;
- u32 pcgctl;
+ u32 hprt0;
int ret = 0;

spin_lock_irqsave(&hsotg->lock, flags);
@@ -4438,11 +4438,40 @@ static int _dwc2_hcd_resume(struct usb_hcd *hcd)
if (hsotg->lx_state != DWC2_L2)
goto unlock;

- if (hsotg->params.power_down > DWC2_POWER_DOWN_PARAM_PARTIAL) {
+ hprt0 = dwc2_read_hprt0(hsotg);
+
+ /*
+ * Added port connection status checking which prevents exiting from
+ * Partial Power Down mode from _dwc2_hcd_resume() if not in Partial
+ * Power Down mode.
+ */
+ if (hprt0 & HPRT0_CONNSTS) {
+ hsotg->lx_state = DWC2_L0;
+ goto unlock;
+ }
+
+ switch (hsotg->params.power_down) {
+ case DWC2_POWER_DOWN_PARAM_PARTIAL:
+ ret = dwc2_exit_partial_power_down(hsotg, 0, true);
+ if (ret)
+ dev_err(hsotg->dev,
+ "exit partial_power_down failed\n");
+ /*
+ * Set HW accessible bit before powering on the controller
+ * since an interrupt may rise.
+ */
+ set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
+ break;
+ case DWC2_POWER_DOWN_PARAM_HIBERNATION:
+ case DWC2_POWER_DOWN_PARAM_NONE:
+ default:
hsotg->lx_state = DWC2_L0;
goto unlock;
}

+ /* Change Root port status, as port status change occurred after resume.*/
+ hsotg->flags.b.port_suspend_change = 1;
+
/*
* Enable power if not already done.
* This must not be spinlocked since duration
@@ -4454,52 +4483,25 @@ static int _dwc2_hcd_resume(struct usb_hcd *hcd)
spin_lock_irqsave(&hsotg->lock, flags);
}

- if (hsotg->params.power_down == DWC2_POWER_DOWN_PARAM_PARTIAL) {
- /*
- * Set HW accessible bit before powering on the controller
- * since an interrupt may rise.
- */
- set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
-
-
- /* Exit partial_power_down */
- ret = dwc2_exit_partial_power_down(hsotg, 0, true);
- if (ret && (ret != -ENOTSUPP))
- dev_err(hsotg->dev, "exit partial_power_down failed\n");
- } else {
- pcgctl = readl(hsotg->regs + PCGCTL);
- pcgctl &= ~PCGCTL_STOPPCLK;
- writel(pcgctl, hsotg->regs + PCGCTL);
- }
-
- hsotg->lx_state = DWC2_L0;
-
+ /* Enable external vbus supply after resuming the port. */
spin_unlock_irqrestore(&hsotg->lock, flags);
+ dwc2_vbus_supply_init(hsotg);

- if (hsotg->bus_suspended) {
- spin_lock_irqsave(&hsotg->lock, flags);
- hsotg->flags.b.port_suspend_change = 1;
- spin_unlock_irqrestore(&hsotg->lock, flags);
- dwc2_port_resume(hsotg);
- } else {
- if (hsotg->params.power_down == DWC2_POWER_DOWN_PARAM_PARTIAL) {
- dwc2_vbus_supply_init(hsotg);
-
- /* Wait for controller to correctly update D+/D- level */
- usleep_range(3000, 5000);
- }
+ /* Wait for controller to correctly update D+/D- level */
+ usleep_range(3000, 5000);
+ spin_lock_irqsave(&hsotg->lock, flags);

- /*
- * Clear Port Enable and Port Status changes.
- * Enable Port Power.
- */
- dwc2_writel(hsotg, HPRT0_PWR | HPRT0_CONNDET |
- HPRT0_ENACHG, HPRT0);
- /* Wait for controller to detect Port Connect */
- usleep_range(5000, 7000);
- }
+ /*
+ * Clear Port Enable and Port Status changes.
+ * Enable Port Power.
+ */
+ dwc2_writel(hsotg, HPRT0_PWR | HPRT0_CONNDET |
+ HPRT0_ENACHG, HPRT0);

- return ret;
+ /* Wait for controller to detect Port Connect */
+ spin_unlock_irqrestore(&hsotg->lock, flags);
+ usleep_range(5000, 7000);
+ spin_lock_irqsave(&hsotg->lock, flags);
unlock:
spin_unlock_irqrestore(&hsotg->lock, flags);

--
2.25.1

2021-04-08 10:10:24

by Artur Petrosyan

[permalink] [raw]
Subject: Re: [PATCH v2 00/14] usb: dwc2: Fix Partial Power down issues.

Hi Greg,

On 4/8/2021 13:17, Artur Petrosyan wrote:
> Hi Greg,
>
> On 4/8/2021 11:28, Artur Petrosyan wrote:
>> This patch set fixes and improves the Partial Power Down mode for
>> dwc2 core.
>> It adds support for the following cases
>> 1. Entering and exiting partial power down when a port is
>> suspended, resumed, port reset is asserted.
>> 2. Exiting the partial power down mode before removing driver.
>> 3. Exiting partial power down in wakeup detected interrupt handler.
>> 4. Exiting from partial power down mode when connector ID.
>> status changes to "connId B
>>
>> It updates and fixes the implementation of dwc2 entering and
>> exiting partial power down mode when the system (PC) is suspended.
>>
>> The patch set also improves the implementation of function handlers
>> for entering and exiting host or device partial power down.
>>
>> NOTE: This is the second patch set in the power saving mode fixes
>> series.
>> This patch set is part of multiple series and is continuation
>> of the "usb: dwc2: Fix and improve power saving modes" patch set.
>> (Patch set link: https://urldefense.com/v3/__https://marc.info/?l=linux-usb&m=160379622403975&w=2__;!!A4F2R9G_pg!IJ-Xl1ZwQU2kmqHB3ITyWyno9BgpWUsC647AqK7GIlgzJu9BzT6VN7jt--__fGdMtgWF69M$ ).
>> The patches that were included in the "usb: dwc2:
>> Fix and improve power saving modes" which was submitted
>> earlier was too large and needed to be split up into
>> smaller patch sets.
>>
>> Changes since V1:
>> No changes in the patches or the source code.
>> Sending the second version of the patch set because the first version
>> was not received by vger.kernel.org.
>>
>>
>>
>> Artur Petrosyan (14):
>> usb: dwc2: Add device partial power down functions
>> usb: dwc2: Add host partial power down functions
>> usb: dwc2: Update enter and exit partial power down functions
>> usb: dwc2: Add partial power down exit flow in wakeup intr.
>> usb: dwc2: Update port suspend/resume function definitions.
>> usb: dwc2: Add enter partial power down when port is suspended
>> usb: dwc2: Add exit partial power down when port is resumed
>> usb: dwc2: Add exit partial power down when port reset is asserted
>> usb: dwc2: Add part. power down exit from
>> dwc2_conn_id_status_change().
>> usb: dwc2: Allow exit partial power down in urb enqueue
>> usb: dwc2: Fix session request interrupt handler
>> usb: dwc2: Update partial power down entering by system suspend
>> usb: dwc2: Fix partial power down exiting by system resume
>> usb: dwc2: Add exit partial power down before removing driver
>>
>> drivers/usb/dwc2/core.c | 113 ++-------
>> drivers/usb/dwc2/core.h | 27 ++-
>> drivers/usb/dwc2/core_intr.c | 46 ++--
>> drivers/usb/dwc2/gadget.c | 148 ++++++++++-
>> drivers/usb/dwc2/hcd.c | 458 +++++++++++++++++++++++++----------
>> drivers/usb/dwc2/hw.h | 1 +
>> drivers/usb/dwc2/platform.c | 11 +-
>> 7 files changed, 558 insertions(+), 246 deletions(-)
>>
>>
>> base-commit: e9fcb07704fcef6fa6d0333fd2b3a62442eaf45b
>>
>
> Re sending as a "v2" did not work :(.
> The patches are not in lore again.
>
> Could the issue be with a comma in the end of To: or Cc: list?
> Let me remove the comma in the end of those lists and try sending as "v3".
>
> Regards,
> Artur
>

I just removed the comma in the end of those lists and resent the patch
set as a "v3" and they are already seen in lore.
There is one strange thing though on lore. Some patch titles are not
fully visible.

For sure the issue was comma in the end of To: or Cc: lists.
Not working example.
To: Greg Kroah-Hartman <[email protected]>,
[email protected], [email protected],

Working example.
To: Greg Kroah-Hartman <[email protected]>,
[email protected], [email protected]

If the comma is at least in the end of one of those lists (To: or Cc:)
vger.kernel.org mailing server will not accept them.

Regards,
Artur

2021-04-08 11:01:50

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v2 00/14] usb: dwc2: Fix Partial Power down issues.

On Thu, Apr 08, 2021 at 10:09:20AM +0000, Artur Petrosyan wrote:
> Hi Greg,
>
> On 4/8/2021 13:17, Artur Petrosyan wrote:
> > Hi Greg,
> >
> > On 4/8/2021 11:28, Artur Petrosyan wrote:
> >> This patch set fixes and improves the Partial Power Down mode for
> >> dwc2 core.
> >> It adds support for the following cases
> >> 1. Entering and exiting partial power down when a port is
> >> suspended, resumed, port reset is asserted.
> >> 2. Exiting the partial power down mode before removing driver.
> >> 3. Exiting partial power down in wakeup detected interrupt handler.
> >> 4. Exiting from partial power down mode when connector ID.
> >> status changes to "connId B
> >>
> >> It updates and fixes the implementation of dwc2 entering and
> >> exiting partial power down mode when the system (PC) is suspended.
> >>
> >> The patch set also improves the implementation of function handlers
> >> for entering and exiting host or device partial power down.
> >>
> >> NOTE: This is the second patch set in the power saving mode fixes
> >> series.
> >> This patch set is part of multiple series and is continuation
> >> of the "usb: dwc2: Fix and improve power saving modes" patch set.
> >> (Patch set link: https://urldefense.com/v3/__https://marc.info/?l=linux-usb&m=160379622403975&w=2__;!!A4F2R9G_pg!IJ-Xl1ZwQU2kmqHB3ITyWyno9BgpWUsC647AqK7GIlgzJu9BzT6VN7jt--__fGdMtgWF69M$ ).
> >> The patches that were included in the "usb: dwc2:
> >> Fix and improve power saving modes" which was submitted
> >> earlier was too large and needed to be split up into
> >> smaller patch sets.
> >>
> >> Changes since V1:
> >> No changes in the patches or the source code.
> >> Sending the second version of the patch set because the first version
> >> was not received by vger.kernel.org.
> >>
> >>
> >>
> >> Artur Petrosyan (14):
> >> usb: dwc2: Add device partial power down functions
> >> usb: dwc2: Add host partial power down functions
> >> usb: dwc2: Update enter and exit partial power down functions
> >> usb: dwc2: Add partial power down exit flow in wakeup intr.
> >> usb: dwc2: Update port suspend/resume function definitions.
> >> usb: dwc2: Add enter partial power down when port is suspended
> >> usb: dwc2: Add exit partial power down when port is resumed
> >> usb: dwc2: Add exit partial power down when port reset is asserted
> >> usb: dwc2: Add part. power down exit from
> >> dwc2_conn_id_status_change().
> >> usb: dwc2: Allow exit partial power down in urb enqueue
> >> usb: dwc2: Fix session request interrupt handler
> >> usb: dwc2: Update partial power down entering by system suspend
> >> usb: dwc2: Fix partial power down exiting by system resume
> >> usb: dwc2: Add exit partial power down before removing driver
> >>
> >> drivers/usb/dwc2/core.c | 113 ++-------
> >> drivers/usb/dwc2/core.h | 27 ++-
> >> drivers/usb/dwc2/core_intr.c | 46 ++--
> >> drivers/usb/dwc2/gadget.c | 148 ++++++++++-
> >> drivers/usb/dwc2/hcd.c | 458 +++++++++++++++++++++++++----------
> >> drivers/usb/dwc2/hw.h | 1 +
> >> drivers/usb/dwc2/platform.c | 11 +-
> >> 7 files changed, 558 insertions(+), 246 deletions(-)
> >>
> >>
> >> base-commit: e9fcb07704fcef6fa6d0333fd2b3a62442eaf45b
> >>
> >
> > Re sending as a "v2" did not work :(.
> > The patches are not in lore again.
> >
> > Could the issue be with a comma in the end of To: or Cc: list?
> > Let me remove the comma in the end of those lists and try sending as "v3".
> >
> > Regards,
> > Artur
> >
>
> I just removed the comma in the end of those lists and resent the patch
> set as a "v3" and they are already seen in lore.
> There is one strange thing though on lore. Some patch titles are not
> fully visible.
>
> For sure the issue was comma in the end of To: or Cc: lists.
> Not working example.
> To: Greg Kroah-Hartman <[email protected]>,
> [email protected], [email protected],

That's an invalid To: line for email.

> Working example.
> To: Greg Kroah-Hartman <[email protected]>,
> [email protected], [email protected]

That's a correct line.

> If the comma is at least in the end of one of those lists (To: or Cc:)
> vger.kernel.org mailing server will not accept them.

I recommend using 'git send-email' with the --to="[email protected]" type
options so that you don't have to hand-edit the lines to try to get
stuff like this correct, as it is easy to get wrong.

thanks,

greg k-h

2021-04-08 13:40:51

by Minas Harutyunyan

[permalink] [raw]
Subject: Re: [PATCH v3 07/14] usb: dwc2: Add exit partial power down when port is resumed

On 4/8/2021 1:45 PM, Artur Petrosyan wrote:
> Added flow of exiting Partial Power Down in
> "dwc2_port_resume()" function when core receives resume.
>
> NOTE: Switch case statement is used for hibernation partial
> power down and clock gating mode determination. In this patch
> only Partial Power Down is implemented the Hibernation and
> clock gating implementations are planned to be added.
>
> Signed-off-by: Artur Petrosyan <[email protected]>

Acked-by: Minas Harutyunyan <[email protected]>

> ---
> Changes in v3:
> - None
> Changes in v2:
> - None
>
> drivers/usb/dwc2/core.h | 5 ++--
> drivers/usb/dwc2/hcd.c | 61 ++++++++++++++++++++++++++---------------
> 2 files changed, 42 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> index 76807abd753b..5a7850482e57 100644
> --- a/drivers/usb/dwc2/core.h
> +++ b/drivers/usb/dwc2/core.h
> @@ -1471,7 +1471,7 @@ void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg, bool force);
> void dwc2_hcd_start(struct dwc2_hsotg *hsotg);
> int dwc2_core_init(struct dwc2_hsotg *hsotg, bool initial_setup);
> int dwc2_port_suspend(struct dwc2_hsotg *hsotg, u16 windex);
> -void dwc2_port_resume(struct dwc2_hsotg *hsotg);
> +int dwc2_port_resume(struct dwc2_hsotg *hsotg);
> int dwc2_backup_host_registers(struct dwc2_hsotg *hsotg);
> int dwc2_restore_host_registers(struct dwc2_hsotg *hsotg);
> int dwc2_host_enter_hibernation(struct dwc2_hsotg *hsotg);
> @@ -1497,7 +1497,8 @@ static inline int dwc2_core_init(struct dwc2_hsotg *hsotg, bool initial_setup)
> { return 0; }
> static inline int dwc2_port_suspend(struct dwc2_hsotg *hsotg, u16 windex)
> { return 0; }
> -static inline void dwc2_port_resume(struct dwc2_hsotg *hsotg) {}
> +static inline int dwc2_port_resume(struct dwc2_hsotg *hsotg)
> +{ return 0; }
> static inline int dwc2_hcd_init(struct dwc2_hsotg *hsotg)
> { return 0; }
> static inline int dwc2_backup_host_registers(struct dwc2_hsotg *hsotg)
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index e7fb0d5940bc..720354df014b 100644
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -3353,44 +3353,61 @@ int dwc2_port_suspend(struct dwc2_hsotg *hsotg, u16 windex)
> *
> * @hsotg: Programming view of the DWC_otg controller
> *
> + * Return: non-zero if failed to exit suspend mode for host.
> + *
> * This function is for exiting Host mode suspend.
> * Must NOT be called with interrupt disabled or spinlock held.
> */
> -void dwc2_port_resume(struct dwc2_hsotg *hsotg)
> +int dwc2_port_resume(struct dwc2_hsotg *hsotg)
> {
> unsigned long flags;
> u32 hprt0;
> u32 pcgctl;
> + int ret = 0;
>
> spin_lock_irqsave(&hsotg->lock, flags);
>
> - /*
> - * If power_down is supported, Phy clock is already resumed
> - * after registers restore.
> - */
> - if (!hsotg->params.power_down) {
> - pcgctl = dwc2_readl(hsotg, PCGCTL);
> - pcgctl &= ~PCGCTL_STOPPCLK;
> - dwc2_writel(hsotg, pcgctl, PCGCTL);
> + switch (hsotg->params.power_down) {
> + case DWC2_POWER_DOWN_PARAM_PARTIAL:
> + ret = dwc2_exit_partial_power_down(hsotg, 0, true);
> + if (ret)
> + dev_err(hsotg->dev,
> + "exit partial_power_down failed.\n");
> + break;
> + case DWC2_POWER_DOWN_PARAM_HIBERNATION:
> + case DWC2_POWER_DOWN_PARAM_NONE:
> + default:
> + /*
> + * If power_down is supported, Phy clock is already resumed
> + * after registers restore.
> + */
> + if (!hsotg->params.power_down) {
> + pcgctl = dwc2_readl(hsotg, PCGCTL);
> + pcgctl &= ~PCGCTL_STOPPCLK;
> + dwc2_writel(hsotg, pcgctl, PCGCTL);
> + spin_unlock_irqrestore(&hsotg->lock, flags);
> + msleep(20);
> + spin_lock_irqsave(&hsotg->lock, flags);
> + }
> +
> + hprt0 = dwc2_read_hprt0(hsotg);
> + hprt0 |= HPRT0_RES;
> + hprt0 &= ~HPRT0_SUSP;
> + dwc2_writel(hsotg, hprt0, HPRT0);
> spin_unlock_irqrestore(&hsotg->lock, flags);
> - msleep(20);
> +
> + msleep(USB_RESUME_TIMEOUT);
> +
> spin_lock_irqsave(&hsotg->lock, flags);
> + hprt0 = dwc2_read_hprt0(hsotg);
> + hprt0 &= ~(HPRT0_RES | HPRT0_SUSP);
> + dwc2_writel(hsotg, hprt0, HPRT0);
> + hsotg->bus_suspended = false;
> }
>
> - hprt0 = dwc2_read_hprt0(hsotg);
> - hprt0 |= HPRT0_RES;
> - hprt0 &= ~HPRT0_SUSP;
> - dwc2_writel(hsotg, hprt0, HPRT0);
> spin_unlock_irqrestore(&hsotg->lock, flags);
>
> - msleep(USB_RESUME_TIMEOUT);
> -
> - spin_lock_irqsave(&hsotg->lock, flags);
> - hprt0 = dwc2_read_hprt0(hsotg);
> - hprt0 &= ~(HPRT0_RES | HPRT0_SUSP);
> - dwc2_writel(hsotg, hprt0, HPRT0);
> - hsotg->bus_suspended = false;
> - spin_unlock_irqrestore(&hsotg->lock, flags);
> + return ret;
> }
>
> /* Handles hub class-specific requests */
>

2021-04-08 13:41:07

by Minas Harutyunyan

[permalink] [raw]
Subject: Re: [PATCH v3 06/14] usb: dwc2: Add enter partial power down when port is suspended

On 4/8/2021 1:45 PM, Artur Petrosyan wrote:
> Adds flow of entering Partial Power Down in
> "dwc2_port_suspend()" function when core receives suspend.
>
> NOTE: Switch case statement is used for hibernation partial
> power down and clock gating mode determination. In this patch
> only Partial Power Down is implemented the Hibernation and
> clock gating implementations are planned to be added.
>
> Signed-off-by: Artur Petrosyan <[email protected]>

Acked-by: Minas Harutyunyan <[email protected]>

> ---
> Changes in v3:
> - None
> Changes in v2:
> - None
>
> drivers/usb/dwc2/core.h | 5 +++--
> drivers/usb/dwc2/hcd.c | 48 ++++++++++++++++++++++++++---------------
> 2 files changed, 34 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> index b7d99cf9e84c..76807abd753b 100644
> --- a/drivers/usb/dwc2/core.h
> +++ b/drivers/usb/dwc2/core.h
> @@ -1470,7 +1470,7 @@ void dwc2_hcd_connect(struct dwc2_hsotg *hsotg);
> void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg, bool force);
> void dwc2_hcd_start(struct dwc2_hsotg *hsotg);
> int dwc2_core_init(struct dwc2_hsotg *hsotg, bool initial_setup);
> -void dwc2_port_suspend(struct dwc2_hsotg *hsotg, u16 windex);
> +int dwc2_port_suspend(struct dwc2_hsotg *hsotg, u16 windex);
> void dwc2_port_resume(struct dwc2_hsotg *hsotg);
> int dwc2_backup_host_registers(struct dwc2_hsotg *hsotg);
> int dwc2_restore_host_registers(struct dwc2_hsotg *hsotg);
> @@ -1495,7 +1495,8 @@ static inline void dwc2_hcd_start(struct dwc2_hsotg *hsotg) {}
> static inline void dwc2_hcd_remove(struct dwc2_hsotg *hsotg) {}
> static inline int dwc2_core_init(struct dwc2_hsotg *hsotg, bool initial_setup)
> { return 0; }
> -static inline void dwc2_port_suspend(struct dwc2_hsotg *hsotg, u16 windex) {}
> +static inline int dwc2_port_suspend(struct dwc2_hsotg *hsotg, u16 windex)
> +{ return 0; }
> static inline void dwc2_port_resume(struct dwc2_hsotg *hsotg) {}
> static inline int dwc2_hcd_init(struct dwc2_hsotg *hsotg)
> { return 0; }
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index f4247a66c2b2..e7fb0d5940bc 100644
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -3281,15 +3281,18 @@ static int dwc2_host_is_b_hnp_enabled(struct dwc2_hsotg *hsotg)
> * @hsotg: Programming view of the DWC_otg controller
> * @windex: The control request wIndex field
> *
> + * Return: non-zero if failed to enter suspend mode for host.
> + *
> * This function is for entering Host mode suspend.
> * Must NOT be called with interrupt disabled or spinlock held.
> */
> -void dwc2_port_suspend(struct dwc2_hsotg *hsotg, u16 windex)
> +int dwc2_port_suspend(struct dwc2_hsotg *hsotg, u16 windex)
> {
> unsigned long flags;
> u32 hprt0;
> u32 pcgctl;
> u32 gotgctl;
> + int ret = 0;
>
> dev_dbg(hsotg->dev, "%s()\n", __func__);
>
> @@ -3302,22 +3305,31 @@ void dwc2_port_suspend(struct dwc2_hsotg *hsotg, u16 windex)
> hsotg->op_state = OTG_STATE_A_SUSPEND;
> }
>
> - hprt0 = dwc2_read_hprt0(hsotg);
> - hprt0 |= HPRT0_SUSP;
> - dwc2_writel(hsotg, hprt0, HPRT0);
> -
> - hsotg->bus_suspended = true;
> -
> - /*
> - * If power_down is supported, Phy clock will be suspended
> - * after registers are backuped.
> - */
> - if (!hsotg->params.power_down) {
> - /* Suspend the Phy Clock */
> - pcgctl = dwc2_readl(hsotg, PCGCTL);
> - pcgctl |= PCGCTL_STOPPCLK;
> - dwc2_writel(hsotg, pcgctl, PCGCTL);
> - udelay(10);
> + switch (hsotg->params.power_down) {
> + case DWC2_POWER_DOWN_PARAM_PARTIAL:
> + ret = dwc2_enter_partial_power_down(hsotg);
> + if (ret)
> + dev_err(hsotg->dev,
> + "enter partial_power_down failed.\n");
> + break;
> + case DWC2_POWER_DOWN_PARAM_HIBERNATION:
> + case DWC2_POWER_DOWN_PARAM_NONE:
> + default:
> + hprt0 = dwc2_read_hprt0(hsotg);
> + hprt0 |= HPRT0_SUSP;
> + dwc2_writel(hsotg, hprt0, HPRT0);
> + hsotg->bus_suspended = true;
> + /*
> + * If power_down is supported, Phy clock will be suspended
> + * after registers are backuped.
> + */
> + if (!hsotg->params.power_down) {
> + /* Suspend the Phy Clock */
> + pcgctl = dwc2_readl(hsotg, PCGCTL);
> + pcgctl |= PCGCTL_STOPPCLK;
> + dwc2_writel(hsotg, pcgctl, PCGCTL);
> + udelay(10);
> + }
> }
>
> /* For HNP the bus must be suspended for at least 200ms */
> @@ -3332,6 +3344,8 @@ void dwc2_port_suspend(struct dwc2_hsotg *hsotg, u16 windex)
> } else {
> spin_unlock_irqrestore(&hsotg->lock, flags);
> }
> +
> + return ret;
> }
>
> /**
>

2021-04-08 13:41:16

by Minas Harutyunyan

[permalink] [raw]
Subject: Re: [PATCH v3 08/14] usb: dwc2: Add exit partial power down when port reset is asserted

On 4/8/2021 1:45 PM, Artur Petrosyan wrote:
> Adds Partial Power Down exiting flow when set port feature
> reset is received in suspended state.
>
> Signed-off-by: Artur Petrosyan <[email protected]>

Acked-by: Minas Harutyunyan <[email protected]>

> ---
> Changes in v3:
> - None
> Changes in v2:
> - None
>
> drivers/usb/dwc2/hcd.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index 720354df014b..7c7496719152 100644
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -3694,6 +3694,15 @@ static int dwc2_hcd_hub_control(struct dwc2_hsotg *hsotg, u16 typereq,
> if (hsotg->params.power_down == DWC2_POWER_DOWN_PARAM_HIBERNATION &&
> hsotg->hibernated)
> dwc2_exit_hibernation(hsotg, 0, 1, 1);
> +
> + if (hsotg->in_ppd) {
> + retval = dwc2_exit_partial_power_down(hsotg, 1,
> + true);
> + if (retval)
> + dev_err(hsotg->dev,
> + "exit partial_power_down failed\n");
> + }
> +
> hprt0 = dwc2_read_hprt0(hsotg);
> dev_dbg(hsotg->dev,
> "SetPortFeature - USB_PORT_FEAT_RESET\n");
>

2021-04-08 13:41:53

by Minas Harutyunyan

[permalink] [raw]
Subject: Re: [PATCH v3 10/14] usb: dwc2: Allow exit partial power down in urb enqueue

On 4/8/2021 1:45 PM, Artur Petrosyan wrote:
> When core is in partial power down state and an external
> hub is connected, upper layer sends URB enqueue request,
> which results in port reset issue.
>
> Added exit from partial power down state to avoid port
> reset issue and process upper layer request correctly.
>
> Signed-off-by: Artur Petrosyan <[email protected]>

Acked-by: Minas Harutyunyan <[email protected]>

> ---
> Changes in v3:
> - None
> Changes in v2:
> - None
>
> drivers/usb/dwc2/hcd.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index 9529e9839961..cb52bc41bfb8 100644
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -4633,6 +4633,13 @@ static int _dwc2_hcd_urb_enqueue(struct usb_hcd *hcd, struct urb *urb,
> dwc2_dump_urb_info(hcd, urb, "urb_enqueue");
> }
>
> + if (hsotg->in_ppd) {
> + retval = dwc2_exit_partial_power_down(hsotg, 0, true);
> + if (retval)
> + dev_err(hsotg->dev,
> + "exit partial_power_down failed\n");
> + }
> +
> if (!ep)
> return -EINVAL;
>
>

2021-04-08 13:41:59

by Minas Harutyunyan

[permalink] [raw]
Subject: Re: [PATCH v3 12/14] usb: dwc2: Update partial power down entering by system suspend

On 4/8/2021 1:45 PM, Artur Petrosyan wrote:
> With current implementation the port power is being disabled,
> which is not required by the programming guide. Also, if there
> is a system which works only in "DWC2_POWER_DOWN_PARAM_NONE"
> (clock gating) mode the current implementation does not set
> Gate hclk bit in pcgctl register.
>
> Rearranges and updates the implementation of entering to partial
> power down power saving mode when PC is suspended to get
> rid of many "if" statements and removes disabling of port power.
>
> NOTE: Switch case statement is used for hibernation partial
> power down and clock gating mode determination. In this patch
> only Partial Power Down is implemented the Hibernation and
> clock gating implementations are planned to be added.
>
> Signed-off-by: Artur Petrosyan <[email protected]>

Acked-by: Minas Harutyunyan <[email protected]>

> ---
> Changes in v3:
> - None
> Changes in v2:
> - None
>
> drivers/usb/dwc2/hcd.c | 53 ++++++++++++++----------------------------
> 1 file changed, 18 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index cb52bc41bfb8..34030bafdff4 100644
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -4367,8 +4367,6 @@ static int _dwc2_hcd_suspend(struct usb_hcd *hcd)
> struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd);
> unsigned long flags;
> int ret = 0;
> - u32 hprt0;
> - u32 pcgctl;
>
> spin_lock_irqsave(&hsotg->lock, flags);
>
> @@ -4384,47 +4382,32 @@ static int _dwc2_hcd_suspend(struct usb_hcd *hcd)
> if (hsotg->op_state == OTG_STATE_B_PERIPHERAL)
> goto unlock;
>
> - if (hsotg->params.power_down != DWC2_POWER_DOWN_PARAM_PARTIAL ||
> - hsotg->flags.b.port_connect_status == 0)
> + if (hsotg->bus_suspended)
> goto skip_power_saving;
>
> - /*
> - * Drive USB suspend and disable port Power
> - * if usb bus is not suspended.
> - */
> - if (!hsotg->bus_suspended) {
> - hprt0 = dwc2_read_hprt0(hsotg);
> - if (hprt0 & HPRT0_CONNSTS) {
> - hprt0 |= HPRT0_SUSP;
> - if (hsotg->params.power_down == DWC2_POWER_DOWN_PARAM_PARTIAL)
> - hprt0 &= ~HPRT0_PWR;
> - dwc2_writel(hsotg, hprt0, HPRT0);
> - }
> - if (hsotg->params.power_down == DWC2_POWER_DOWN_PARAM_PARTIAL) {
> - spin_unlock_irqrestore(&hsotg->lock, flags);
> - dwc2_vbus_supply_exit(hsotg);
> - spin_lock_irqsave(&hsotg->lock, flags);
> - } else {
> - pcgctl = readl(hsotg->regs + PCGCTL);
> - pcgctl |= PCGCTL_STOPPCLK;
> - writel(pcgctl, hsotg->regs + PCGCTL);
> - }
> - }
> + if (hsotg->flags.b.port_connect_status == 0)
> + goto skip_power_saving;
>
> - if (hsotg->params.power_down == DWC2_POWER_DOWN_PARAM_PARTIAL) {
> + switch (hsotg->params.power_down) {
> + case DWC2_POWER_DOWN_PARAM_PARTIAL:
> /* Enter partial_power_down */
> ret = dwc2_enter_partial_power_down(hsotg);
> - if (ret) {
> - if (ret != -ENOTSUPP)
> - dev_err(hsotg->dev,
> - "enter partial_power_down failed\n");
> - goto skip_power_saving;
> - }
> -
> - /* After entering partial_power_down, hardware is no more accessible */
> + if (ret)
> + dev_err(hsotg->dev,
> + "enter partial_power_down failed\n");
> + /* After entering suspend, hardware is not accessible */
> clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
> + break;
> + case DWC2_POWER_DOWN_PARAM_HIBERNATION:
> + case DWC2_POWER_DOWN_PARAM_NONE:
> + default:
> + goto skip_power_saving;
> }
>
> + spin_unlock_irqrestore(&hsotg->lock, flags);
> + dwc2_vbus_supply_exit(hsotg);
> + spin_lock_irqsave(&hsotg->lock, flags);
> +
> /* Ask phy to be suspended */
> if (!IS_ERR_OR_NULL(hsotg->uphy)) {
> spin_unlock_irqrestore(&hsotg->lock, flags);
>

2021-04-08 13:42:19

by Minas Harutyunyan

[permalink] [raw]
Subject: Re: [PATCH v3 13/14] usb: dwc2: Fix partial power down exiting by system resume

On 4/8/2021 1:46 PM, Artur Petrosyan wrote:
> Fixes the implementation of exiting from partial power down
> power saving mode when PC is resumed.
>
> Added port connection status checking which prevents exiting from
> Partial Power Down mode from _dwc2_hcd_resume() if not in Partial
> Power Down mode.
>
> Rearranged the implementation to get rid of many "if"
> statements.
>
> NOTE: Switch case statement is used for hibernation partial
> power down and clock gating mode determination. In this patch
> only Partial Power Down is implemented the Hibernation and
> clock gating implementations are planned to be added.
>
> Cc: <[email protected]>
> Fixes: 6f6d70597c15 ("usb: dwc2: bus suspend/resume for hosts with DWC2_POWER_DOWN_PARAM_NONE")
> Signed-off-by: Artur Petrosyan <[email protected]>

Acked-by: Minas Harutyunyan <[email protected]>

> ---
> Changes in v3:
> - None
> Changes in v2:
> - None
>
> drivers/usb/dwc2/hcd.c | 90 +++++++++++++++++++++---------------------
> 1 file changed, 46 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index 34030bafdff4..f096006df96f 100644
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -4427,7 +4427,7 @@ static int _dwc2_hcd_resume(struct usb_hcd *hcd)
> {
> struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd);
> unsigned long flags;
> - u32 pcgctl;
> + u32 hprt0;
> int ret = 0;
>
> spin_lock_irqsave(&hsotg->lock, flags);
> @@ -4438,11 +4438,40 @@ static int _dwc2_hcd_resume(struct usb_hcd *hcd)
> if (hsotg->lx_state != DWC2_L2)
> goto unlock;
>
> - if (hsotg->params.power_down > DWC2_POWER_DOWN_PARAM_PARTIAL) {
> + hprt0 = dwc2_read_hprt0(hsotg);
> +
> + /*
> + * Added port connection status checking which prevents exiting from
> + * Partial Power Down mode from _dwc2_hcd_resume() if not in Partial
> + * Power Down mode.
> + */
> + if (hprt0 & HPRT0_CONNSTS) {
> + hsotg->lx_state = DWC2_L0;
> + goto unlock;
> + }
> +
> + switch (hsotg->params.power_down) {
> + case DWC2_POWER_DOWN_PARAM_PARTIAL:
> + ret = dwc2_exit_partial_power_down(hsotg, 0, true);
> + if (ret)
> + dev_err(hsotg->dev,
> + "exit partial_power_down failed\n");
> + /*
> + * Set HW accessible bit before powering on the controller
> + * since an interrupt may rise.
> + */
> + set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
> + break;
> + case DWC2_POWER_DOWN_PARAM_HIBERNATION:
> + case DWC2_POWER_DOWN_PARAM_NONE:
> + default:
> hsotg->lx_state = DWC2_L0;
> goto unlock;
> }
>
> + /* Change Root port status, as port status change occurred after resume.*/
> + hsotg->flags.b.port_suspend_change = 1;
> +
> /*
> * Enable power if not already done.
> * This must not be spinlocked since duration
> @@ -4454,52 +4483,25 @@ static int _dwc2_hcd_resume(struct usb_hcd *hcd)
> spin_lock_irqsave(&hsotg->lock, flags);
> }
>
> - if (hsotg->params.power_down == DWC2_POWER_DOWN_PARAM_PARTIAL) {
> - /*
> - * Set HW accessible bit before powering on the controller
> - * since an interrupt may rise.
> - */
> - set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
> -
> -
> - /* Exit partial_power_down */
> - ret = dwc2_exit_partial_power_down(hsotg, 0, true);
> - if (ret && (ret != -ENOTSUPP))
> - dev_err(hsotg->dev, "exit partial_power_down failed\n");
> - } else {
> - pcgctl = readl(hsotg->regs + PCGCTL);
> - pcgctl &= ~PCGCTL_STOPPCLK;
> - writel(pcgctl, hsotg->regs + PCGCTL);
> - }
> -
> - hsotg->lx_state = DWC2_L0;
> -
> + /* Enable external vbus supply after resuming the port. */
> spin_unlock_irqrestore(&hsotg->lock, flags);
> + dwc2_vbus_supply_init(hsotg);
>
> - if (hsotg->bus_suspended) {
> - spin_lock_irqsave(&hsotg->lock, flags);
> - hsotg->flags.b.port_suspend_change = 1;
> - spin_unlock_irqrestore(&hsotg->lock, flags);
> - dwc2_port_resume(hsotg);
> - } else {
> - if (hsotg->params.power_down == DWC2_POWER_DOWN_PARAM_PARTIAL) {
> - dwc2_vbus_supply_init(hsotg);
> -
> - /* Wait for controller to correctly update D+/D- level */
> - usleep_range(3000, 5000);
> - }
> + /* Wait for controller to correctly update D+/D- level */
> + usleep_range(3000, 5000);
> + spin_lock_irqsave(&hsotg->lock, flags);
>
> - /*
> - * Clear Port Enable and Port Status changes.
> - * Enable Port Power.
> - */
> - dwc2_writel(hsotg, HPRT0_PWR | HPRT0_CONNDET |
> - HPRT0_ENACHG, HPRT0);
> - /* Wait for controller to detect Port Connect */
> - usleep_range(5000, 7000);
> - }
> + /*
> + * Clear Port Enable and Port Status changes.
> + * Enable Port Power.
> + */
> + dwc2_writel(hsotg, HPRT0_PWR | HPRT0_CONNDET |
> + HPRT0_ENACHG, HPRT0);
>
> - return ret;
> + /* Wait for controller to detect Port Connect */
> + spin_unlock_irqrestore(&hsotg->lock, flags);
> + usleep_range(5000, 7000);
> + spin_lock_irqsave(&hsotg->lock, flags);
> unlock:
> spin_unlock_irqrestore(&hsotg->lock, flags);
>
>

2021-04-08 13:42:49

by Minas Harutyunyan

[permalink] [raw]
Subject: Re: [PATCH v3 05/14] usb: dwc2: Update port suspend/resume function definitions.

On 4/8/2021 1:45 PM, Artur Petrosyan wrote:
> Earlier "dwc2_port_suspend()" and "dwc2_port_resume()" functions
> were implemented without proper description and host or device mode
> difference.
>
> - Added "dwc2_port_suspend" and "dwc2_port_resume" functions to
> "core.h" header file.
>
> - Updated function description in documentation.
>
> Signed-off-by: Artur Petrosyan <[email protected]>

Acked-by: Minas Harutyunyan <[email protected]>

> ---
> Changes in v3:
> - None
> Changes in v2:
> - None
>
> drivers/usb/dwc2/core.h | 4 ++++
> drivers/usb/dwc2/hcd.c | 25 +++++++++++++++++++------
> 2 files changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> index 39037709a2ad..b7d99cf9e84c 100644
> --- a/drivers/usb/dwc2/core.h
> +++ b/drivers/usb/dwc2/core.h
> @@ -1470,6 +1470,8 @@ void dwc2_hcd_connect(struct dwc2_hsotg *hsotg);
> void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg, bool force);
> void dwc2_hcd_start(struct dwc2_hsotg *hsotg);
> int dwc2_core_init(struct dwc2_hsotg *hsotg, bool initial_setup);
> +void dwc2_port_suspend(struct dwc2_hsotg *hsotg, u16 windex);
> +void dwc2_port_resume(struct dwc2_hsotg *hsotg);
> int dwc2_backup_host_registers(struct dwc2_hsotg *hsotg);
> int dwc2_restore_host_registers(struct dwc2_hsotg *hsotg);
> int dwc2_host_enter_hibernation(struct dwc2_hsotg *hsotg);
> @@ -1493,6 +1495,8 @@ static inline void dwc2_hcd_start(struct dwc2_hsotg *hsotg) {}
> static inline void dwc2_hcd_remove(struct dwc2_hsotg *hsotg) {}
> static inline int dwc2_core_init(struct dwc2_hsotg *hsotg, bool initial_setup)
> { return 0; }
> +static inline void dwc2_port_suspend(struct dwc2_hsotg *hsotg, u16 windex) {}
> +static inline void dwc2_port_resume(struct dwc2_hsotg *hsotg) {}
> static inline int dwc2_hcd_init(struct dwc2_hsotg *hsotg)
> { return 0; }
> static inline int dwc2_backup_host_registers(struct dwc2_hsotg *hsotg)
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index dd0362e07444..f4247a66c2b2 100644
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -56,8 +56,6 @@
> #include "core.h"
> #include "hcd.h"
>
> -static void dwc2_port_resume(struct dwc2_hsotg *hsotg);
> -
> /*
> * =========================================================================
> * Host Core Layer Functions
> @@ -3277,8 +3275,16 @@ static int dwc2_host_is_b_hnp_enabled(struct dwc2_hsotg *hsotg)
> return hcd->self.b_hnp_enable;
> }
>
> -/* Must NOT be called with interrupt disabled or spinlock held */
> -static void dwc2_port_suspend(struct dwc2_hsotg *hsotg, u16 windex)
> +/**
> + * dwc2_port_suspend() - Put controller in suspend mode for host.
> + *
> + * @hsotg: Programming view of the DWC_otg controller
> + * @windex: The control request wIndex field
> + *
> + * This function is for entering Host mode suspend.
> + * Must NOT be called with interrupt disabled or spinlock held.
> + */
> +void dwc2_port_suspend(struct dwc2_hsotg *hsotg, u16 windex)
> {
> unsigned long flags;
> u32 hprt0;
> @@ -3328,8 +3334,15 @@ static void dwc2_port_suspend(struct dwc2_hsotg *hsotg, u16 windex)
> }
> }
>
> -/* Must NOT be called with interrupt disabled or spinlock held */
> -static void dwc2_port_resume(struct dwc2_hsotg *hsotg)
> +/**
> + * dwc2_port_resume() - Exit controller from suspend mode for host.
> + *
> + * @hsotg: Programming view of the DWC_otg controller
> + *
> + * This function is for exiting Host mode suspend.
> + * Must NOT be called with interrupt disabled or spinlock held.
> + */
> +void dwc2_port_resume(struct dwc2_hsotg *hsotg)
> {
> unsigned long flags;
> u32 hprt0;
>

2021-04-08 13:45:16

by Minas Harutyunyan

[permalink] [raw]
Subject: Re: [PATCH v3 14/14] usb: dwc2: Add exit partial power down before removing driver

On 4/8/2021 1:46 PM, Artur Petrosyan wrote:
> When dwc2 core is in partial power down mode
> loading driver again causes driver fail. Because in
> that mode registers are not accessible.
>
> Added a flow of exiting the partial power down mode
> to avoid the driver reload failure.
>
> Signed-off-by: Artur Petrosyan <[email protected]>

Acked-by: Minas Harutyunyan <[email protected]>

> ---
> Changes in v3:
> - None
> Changes in v2:
> - None
>
> drivers/usb/dwc2/platform.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
> index 5f18acac7406..b28b8cd45799 100644
> --- a/drivers/usb/dwc2/platform.c
> +++ b/drivers/usb/dwc2/platform.c
> @@ -316,6 +316,15 @@ static int dwc2_lowlevel_hw_init(struct dwc2_hsotg *hsotg)
> static int dwc2_driver_remove(struct platform_device *dev)
> {
> struct dwc2_hsotg *hsotg = platform_get_drvdata(dev);
> + int ret = 0;
> +
> + /* Exit Partial Power Down when driver is removed. */
> + if (hsotg->in_ppd) {
> + ret = dwc2_exit_partial_power_down(hsotg, 0, true);
> + if (ret)
> + dev_err(hsotg->dev,
> + "exit partial_power_down failed\n");
> + }
>
> dwc2_debugfs_exit(hsotg);
> if (hsotg->hcd_enabled)
> @@ -334,7 +343,7 @@ static int dwc2_driver_remove(struct platform_device *dev)
> reset_control_assert(hsotg->reset);
> reset_control_assert(hsotg->reset_ecc);
>
> - return 0;
> + return ret;
> }
>
> /**
>