Re: [PATCH v13 2/6] usb: dwc3: core: Host wake up support from system suspend

From: Sandeep Maheswaram (Temp)
Date: Tue Apr 12 2022 - 00:46:59 EST

Hi Matthias,

On 4/12/2022 2:24 AM, Matthias Kaehlcke wrote:
On Tue, Apr 12, 2022 at 12:46:50AM +0530, Sandeep Maheswaram wrote:
During suspend read the status of all port and set hs phy mode
based on current speed. Use this hs phy mode to configure wakeup
interrupts in qcom glue driver.

Check wakep-source property for dwc3 core node to set the
s/wakep/wakeup/
Okay. Will update in next version.

wakeup capability. Drop the device_init_wakeup call from
runtime suspend and resume.

Also check during suspend if any wakeup capable devices are
connected to the controller (directly or through hubs), if there
are none set a flag to indicate that the PHY is powered
down during suspend.

Signed-off-by: Sandeep Maheswaram <quic_c_sanm@xxxxxxxxxxx>
---
A per-patch change log would be really helpful for reviewers, even
if it doesn't include older versions.
Okay. Will update in next version.

drivers/usb/dwc3/core.c | 33 ++++++++++++++++++++-------------
drivers/usb/dwc3/core.h | 4 ++++
drivers/usb/dwc3/host.c | 25 +++++++++++++++++++++++++
3 files changed, 49 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 1170b80..effaa43 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -32,6 +32,7 @@
#include <linux/usb/gadget.h>
#include <linux/usb/of.h>
#include <linux/usb/otg.h>
+#include <linux/usb/hcd.h>
#include "core.h"
#include "gadget.h"
@@ -1723,6 +1724,7 @@ static int dwc3_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, dwc);
dwc3_cache_hwparams(dwc);
+ device_init_wakeup(&pdev->dev, of_property_read_bool(dev->of_node, "wakeup-source"));
spin_lock_init(&dwc->lock);
mutex_init(&dwc->mutex);
@@ -1865,6 +1867,7 @@ static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
{
unsigned long flags;
u32 reg;
+ struct usb_hcd *hcd = platform_get_drvdata(dwc->xhci);
switch (dwc->current_dr_role) {
case DWC3_GCTL_PRTCAP_DEVICE:
@@ -1877,10 +1880,7 @@ static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
dwc3_core_exit(dwc);
break;
case DWC3_GCTL_PRTCAP_HOST:
- if (!PMSG_IS_AUTO(msg)) {
- dwc3_core_exit(dwc);
- break;
- }
+ dwc3_check_phy_speed_mode(dwc);
/* Let controller to suspend HSPHY before PHY driver suspends */
if (dwc->dis_u2_susphy_quirk ||
@@ -1896,6 +1896,16 @@ static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
phy_pm_runtime_put_sync(dwc->usb2_generic_phy);
phy_pm_runtime_put_sync(dwc->usb3_generic_phy);
+
+ if (!PMSG_IS_AUTO(msg)) {
+ if (device_may_wakeup(dwc->dev) &&
+ usb_wakeup_enabled_descendants(hcd->self.root_hub)) {
You did not answer my question on v12, reposting it:

Did you ever try whether you could use device_children_wakeup_capable() from
[1] instead of usb_wakeup_enabled_descendants()?

[1] https://patchwork.kernel.org/project/linux-usb/patch/1635753224-23975-2-git-send-email-quic_c_sanm@xxxxxxxxxxx/#24566065

Sorry ..I have replied in mail yesterday but it is not showing up in patchwork link.

Tried with  device_children_wakeup_capable(dwc->dev) instead of usb_wakeup_enabled_descendants and it always returns true even

when no devices are connected.


+ dwc->phy_power_off = false;
+ } else {
+ dwc->phy_power_off = true;
+ dwc3_core_exit(dwc);
+ }
+ }
break;
case DWC3_GCTL_PRTCAP_OTG:
/* do nothing during runtime_suspend */
@@ -1939,11 +1949,12 @@ static int dwc3_resume_common(struct dwc3 *dwc, pm_message_t msg)
break;
case DWC3_GCTL_PRTCAP_HOST:
if (!PMSG_IS_AUTO(msg)) {
- ret = dwc3_core_init_for_resume(dwc);
- if (ret)
- return ret;
- dwc3_set_prtcap(dwc, DWC3_GCTL_PRTCAP_HOST);
- break;
+ if (dwc->phy_power_off) {
+ ret = dwc3_core_init_for_resume(dwc);
+ if (ret)
+ return ret;
+ dwc3_set_prtcap(dwc, DWC3_GCTL_PRTCAP_HOST);
+ }
}
/* Restore GUSB2PHYCFG bits that were modified in suspend */
reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
@@ -2015,8 +2026,6 @@ static int dwc3_runtime_suspend(struct device *dev)
if (ret)
return ret;
- device_init_wakeup(dev, true);
-
return 0;
}
@@ -2025,8 +2034,6 @@ static int dwc3_runtime_resume(struct device *dev)
struct dwc3 *dwc = dev_get_drvdata(dev);
int ret;
- device_init_wakeup(dev, false);
-
ret = dwc3_resume_common(dwc, PMSG_AUTO_RESUME);
if (ret)
return ret;
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 5c9d467..6a5845f 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -1154,6 +1154,9 @@ struct dwc3 {
bool phys_ready;
+ unsigned int hs_phy_mode;
+ bool phy_power_off;
+
struct ulpi *ulpi;
bool ulpi_ready;
@@ -1537,6 +1540,7 @@ int dwc3_core_soft_reset(struct dwc3 *dwc);
#if IS_ENABLED(CONFIG_USB_DWC3_HOST) || IS_ENABLED(CONFIG_USB_DWC3_DUAL_ROLE)
int dwc3_host_init(struct dwc3 *dwc);
void dwc3_host_exit(struct dwc3 *dwc);
+void dwc3_check_phy_speed_mode(struct dwc3 *dwc);
#else
static inline int dwc3_host_init(struct dwc3 *dwc)
{ return 0; }
diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
index eda8719..7f22675 100644
--- a/drivers/usb/dwc3/host.c
+++ b/drivers/usb/dwc3/host.c
@@ -13,6 +13,7 @@
#include <linux/platform_device.h>
#include "core.h"
+#include "../host/xhci.h"
static void dwc3_host_fill_xhci_irq_res(struct dwc3 *dwc,
int irq, char *name)
@@ -138,3 +139,27 @@ void dwc3_host_exit(struct dwc3 *dwc)
{
platform_device_unregister(dwc->xhci);
}
+
+void dwc3_check_phy_speed_mode(struct dwc3 *dwc)
+{
+
delete empty line
Okay. Will update in next version.

+ int i, num_ports;
+ u32 reg;
+ struct usb_hcd *hcd = platform_get_drvdata(dwc->xhci);
+ struct xhci_hcd *xhci_hcd = hcd_to_xhci(hcd);
+
+ dwc->hs_phy_mode = 0;
+
+ reg = readl(&xhci_hcd->cap_regs->hcs_params1);
+
+ num_ports = HCS_MAX_PORTS(reg);
+ for (i = 0; i < num_ports; i++) {
+ reg = readl(&xhci_hcd->op_regs->port_status_base + i * NUM_PORT_REGS);
+ if (reg & PORT_PE) {
+ if (DEV_HIGHSPEED(reg) || DEV_FULLSPEED(reg))
+ dwc->hs_phy_mode |= PHY_MODE_USB_HOST_HS;
+ else if (DEV_LOWSPEED(reg))
+ dwc->hs_phy_mode |= PHY_MODE_USB_HOST_LS;
+ }
+ }
+}
--
2.7.4