Skip to contentSkip to conversation

USB DWC3 host wake up support from system suspend

PATCH Superseded Sandeep Maheswaram 4 participants 15 messages Sep 29, 2020 – Oct 1, 2020

Subsystems (6)

v3 of 8 Superseded Latest is v8 →

lore
Share & tools
lore
#0 lore raw

v3 cover USB DWC3 host wake up support from system suspend

Avoiding phy powerdown in host mode so that it can be wake up by devices. Set GENPD_FLAG_ACTIVE_WAKEUP flag to keep usb30_prim gdsc active when wakeup capable devices are connected to the host.
Using PDC interrupts instead of GIC interrupst to support wakeup in xo shutdown case.

Changes in v3:
Removed need_phy_for_wakeup flag and by default avoiding phy powerdown. Addressed Matthias comments and added entry for DEV_SUPERSPEED. Added suspend_quirk in dwc3 host and moved the dwc3_set_phy_speed_flags. Added wakeup-source dt entry and reading in dwc-qcom.c glue driver.

Changes in v2:
Dropped the patch in clock to set GENPD_FLAG_ACTIVE_WAKEUP flag and setting in usb dwc3 driver.
Separated the core patch and glue driver patch.
Made need_phy_for_wakeup flag part of dwc structure and hs_phy_flags as unsgined int.
Adrressed the comment on device_init_wakeup call. Corrected offset for reading portsc register.
Added pacth to support wakeup in xo shutdown case.

Sandeep Maheswaram (5):
usb: dwc3: core: Host wake up support from system suspend
usb: dwc3: host: Add suspend_quirk for dwc3 host
usb: dwc3: qcom: Configure wakeup interrupts and set genpd active
wakeup flag
arm64: dts: qcom: sc7180: Use pdc interrupts for USB instead of GIC
interrupts
arm64: dts: qcom: sc7180: Add wakeup-source property for USB
controller node

 arch/arm64/boot/dts/qcom/sc7180-idp.dts      |  1 +
 arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi |  1 +
 arch/arm64/boot/dts/qcom/sc7180.dtsi         |  8 +--
 drivers/usb/dwc3/core.c                      | 14 ++----
 drivers/usb/dwc3/core.h                      |  3 ++
 drivers/usb/dwc3/dwc3-qcom.c                 | 74 ++++++++++++++++++++++------
 drivers/usb/dwc3/host.c                      | 49 ++++++++++++++++++
 7 files changed, 120 insertions(+), 30 deletions(-)
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
#1 lore raw

PATCH v3 1/5 usb: dwc3: core: Host wake up support from system suspend

Avoiding phy powerdown in host mode so that it can be wake up by devices. Added hs_phy_flags and ss_phy_flags to check connection status and set phy mode and configure interrupts.

---
 drivers/usb/dwc3/core.c | 14 +++-----------
 drivers/usb/dwc3/core.h |  3 +++
 2 files changed, 6 insertions(+), 11 deletions(-)
drivers/usb/dwc3/core.c +3 -11
index 385262f..c32ed10 100644
@@ -1663,10 +1663,6 @@ 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;
-		}
 
 		/* Let controller to suspend HSPHY before PHY driver suspends */
 		if (dwc->dis_u2_susphy_quirk ||
@@ -1724,13 +1720,9 @@ static int dwc3_resume_common(struct dwc3 *dwc, pm_message_t msg)
 		spin_unlock_irqrestore(&dwc->lock, flags);
 		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;
-		}
+
+		dwc3_set_prtcap(dwc, DWC3_GCTL_PRTCAP_HOST);
+
 		/* Restore GUSB2PHYCFG bits that were modified in suspend */
 		reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
 		if (dwc->dis_u2_susphy_quirk)
drivers/usb/dwc3/core.h +3 -0
index 83b6c87..cd385a8 100644
@@ -1096,6 +1096,9 @@ struct dwc3 {
 
 	bool			phys_ready;
 
+	unsigned int            hs_phy_flags;
+	unsigned int            ss_phy_flags;
+
 	struct ulpi		*ulpi;
 	bool			ulpi_ready;
 
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
#2 lore raw

Re: [PATCH v3 1/5] usb: dwc3: core: Host wake up support from system suspend

Quoted 1 line · Sandeep Maheswaram

Avoiding phy powerdown in host mode so that it can be wake up by devices.

s/wake/woken/?

Quoted 4 lines

Added hs_phy_flags and ss_phy_flags to check connection status and set phy mode and configure interrupts.

[...]

MBR, Sergei

#3 lore raw

Re: [PATCH v3 1/5] usb: dwc3: core: Host wake up support from system suspend

In reply to Sandeep Maheswaram
Quoted 47 lines · Sandeep Maheswaram

Avoiding phy powerdown in host mode so that it can be wake up by devices. Added hs_phy_flags and ss_phy_flags to check connection status and set phy mode and configure interrupts.

---
 drivers/usb/dwc3/core.c | 14 +++-----------
 drivers/usb/dwc3/core.h |  3 +++
 2 files changed, 6 insertions(+), 11 deletions(-)
drivers/usb/dwc3/core.c +3 -11
index 385262f..c32ed10 100644
@@ -1663,10 +1663,6 @@ 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;
-		}
 
 		/* Let controller to suspend HSPHY before PHY driver suspends */
 		if (dwc->dis_u2_susphy_quirk ||
@@ -1724,13 +1720,9 @@ static int dwc3_resume_common(struct dwc3 *dwc, pm_message_t msg)
 		spin_unlock_irqrestore(&dwc->lock, flags);
 		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;
-		}
+
+		dwc3_set_prtcap(dwc, DWC3_GCTL_PRTCAP_HOST);
+
 		/* Restore GUSB2PHYCFG bits that were modified in suspend */
 		reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
 		if (dwc->dis_u2_susphy_quirk)
drivers/usb/dwc3/core.h +0 -0
index 83b6c87..cd385a8 100644

drivers/usb/dwc3/core.h struct dwc3 { · line 1101

Quoted 6 lines
drivers/usb/dwc3/core.h +3 -0
@@ -1096,6 +1096,9 @@ struct dwc3 {
 
 	bool			phys_ready;
 
+	unsigned int            hs_phy_flags;
+	unsigned int            ss_phy_flags;
+

It doesn't seem ss_phy_flags is really needed, it is only used in dwc3_set_phy_speed_flags(), a local variable could be used instead.

The 'flags' are passed as 'mode' to phy_set_mode(), I think it would be clearer to reflect that in the name, i.e. 'hs_phy_mode' instead of 'hs_phy_flags'.

#4 lore raw

PATCH v3 2/5 usb: dwc3: host: Add suspend_quirk for dwc3 host

In reply to Sandeep Maheswaram

Adding suspend quirk function for dwc3 host which will be called during xhci suspend.
Setting hs_phy_flags, ss_phy_flags and phy mode during host suspend.

---
 drivers/usb/dwc3/host.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)
drivers/usb/dwc3/host.c +49 -0
index e195176..7f316fa 100644
@@ -11,6 +11,13 @@
 #include <linux/platform_device.h>
 
 #include "core.h"
+#include "../host/xhci.h"
+#include "../host/xhci-plat.h"
+int xhci_dwc3_suspend_quirk(struct usb_hcd *hcd);
+
+static const struct xhci_plat_priv xhci_plat_dwc3_xhci = {
+	.suspend_quirk = xhci_dwc3_suspend_quirk,
+};
 
 static int dwc3_host_get_irq(struct dwc3 *dwc)
 {
@@ -115,6 +122,13 @@ int dwc3_host_init(struct dwc3 *dwc)
 		}
 	}
 
+	ret = platform_device_add_data(xhci, &xhci_plat_dwc3_xhci,
+			sizeof(struct xhci_plat_priv));
+	if (ret) {
+		dev_err(dwc->dev, "failed to add data to xHCI\n");
+		goto err;
+	}
+
 	ret = platform_device_add(xhci);
 	if (ret) {
 		dev_err(dwc->dev, "failed to register xHCI device\n");
@@ -127,6 +141,41 @@ int dwc3_host_init(struct dwc3 *dwc)
 	return ret;
 }
 
+static void dwc3_set_phy_speed_flags(struct usb_hcd *hcd)
+{
+
+	int i, num_ports;
+	u32 reg;
+	struct device *dev = hcd->self.controller;
+	struct dwc3 *dwc = dev_get_drvdata(dev->parent);
+	struct xhci_hcd	*xhci_hcd = hcd_to_xhci(hcd);
+
+	dwc->hs_phy_flags = 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 * 0x04);
+		if (reg & PORT_PE) {
+			if (DEV_HIGHSPEED(reg) || DEV_FULLSPEED(reg))
+				dwc->hs_phy_flags |= PHY_MODE_USB_HOST_HS;
+			else if (DEV_LOWSPEED(reg))
+				dwc->hs_phy_flags |= PHY_MODE_USB_HOST_LS;
+			if (DEV_SUPERSPEED(reg))
+				dwc->ss_phy_flags |= PHY_MODE_USB_HOST_SS;
+		}
+	}
+	phy_set_mode(dwc->usb2_generic_phy, dwc->hs_phy_flags);
+	phy_set_mode(dwc->usb3_generic_phy, dwc->ss_phy_flags);
+}
+
+int xhci_dwc3_suspend_quirk(struct usb_hcd *hcd)
+{
+	dwc3_set_phy_speed_flags(hcd);
+	return 0;
+}
+
 void dwc3_host_exit(struct dwc3 *dwc)
 {
 	platform_device_unregister(dwc->xhci);
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
#5 lore raw

Re: [PATCH v3 2/5] usb: dwc3: host: Add suspend_quirk for dwc3 host

Quoted 42 lines · Sandeep Maheswaram

Adding suspend quirk function for dwc3 host which will be called during xhci suspend.
Setting hs_phy_flags, ss_phy_flags and phy mode during host suspend.

---
 drivers/usb/dwc3/host.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)
drivers/usb/dwc3/host.c +14 -0
index e195176..7f316fa 100644
@@ -11,6 +11,13 @@
 #include <linux/platform_device.h>
 
 #include "core.h"
+#include "../host/xhci.h"
+#include "../host/xhci-plat.h"
+int xhci_dwc3_suspend_quirk(struct usb_hcd *hcd);
+
+static const struct xhci_plat_priv xhci_plat_dwc3_xhci = {
+	.suspend_quirk = xhci_dwc3_suspend_quirk,
+};
 
 static int dwc3_host_get_irq(struct dwc3 *dwc)
 {
@@ -115,6 +122,13 @@ int dwc3_host_init(struct dwc3 *dwc)
 		}
 	}
 
+	ret = platform_device_add_data(xhci, &xhci_plat_dwc3_xhci,
+			sizeof(struct xhci_plat_priv));
+	if (ret) {
+		dev_err(dwc->dev, "failed to add data to xHCI\n");
+		goto err;
+	}
+
 	ret = platform_device_add(xhci);
 	if (ret) {
 		dev_err(dwc->dev, "failed to register xHCI device\n");

drivers/usb/dwc3/host.c int dwc3_host_init(struct dwc3 *dwc) · line 144

Quoted 4 lines
drivers/usb/dwc3/host.c +1 -0
@@ -127,6 +141,41 @@ int dwc3_host_init(struct dwc3 *dwc)
 	return ret;
 }
 
+static void dwc3_set_phy_speed_flags(struct usb_hcd *hcd)

The main thing this function does is setting the PHY mode (see the phy_set_mode() calls), please rename it to dwc3_set_phy_mode() to reflect this.

drivers/usb/dwc3/host.c int dwc3_host_init(struct dwc3 *dwc) · line 153

Quoted 9 lines
drivers/usb/dwc3/host.c +9 -0
+{
+
+	int i, num_ports;
+	u32 reg;
+	struct device *dev = hcd->self.controller;
+	struct dwc3 *dwc = dev_get_drvdata(dev->parent);
+	struct xhci_hcd	*xhci_hcd = hcd_to_xhci(hcd);
+
+	dwc->hs_phy_flags = 0;

What about 'dwc->ss_phy_flags'?

I suggested in another patch to use a local variable instead, so you probably have to initialize it anyway or the compiler will be unhappy ;-)

drivers/usb/dwc3/host.c int dwc3_host_init(struct dwc3 *dwc) · line 164

Quoted 11 lines
drivers/usb/dwc3/host.c +11 -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 * 0x04);
+		if (reg & PORT_PE) {
+			if (DEV_HIGHSPEED(reg) || DEV_FULLSPEED(reg))
+				dwc->hs_phy_flags |= PHY_MODE_USB_HOST_HS;
+			else if (DEV_LOWSPEED(reg))
+				dwc->hs_phy_flags |= PHY_MODE_USB_HOST_LS;

nit: add empty line to visually separate HS from SS

drivers/usb/dwc3/host.c int dwc3_host_init(struct dwc3 *dwc) · line 170

Quoted 6 lines
drivers/usb/dwc3/host.c +6 -0
+			if (DEV_SUPERSPEED(reg))
+				dwc->ss_phy_flags |= PHY_MODE_USB_HOST_SS;
+		}
+	}
+	phy_set_mode(dwc->usb2_generic_phy, dwc->hs_phy_flags);
+	phy_set_mode(dwc->usb3_generic_phy, dwc->ss_phy_flags);

Check return values and return any errors?

#6 lore raw

PATCH v3 3/5 usb: dwc3: qcom: Configure wakeup interrupts and set genpd active wakeup flag

In reply to Sandeep Maheswaram

Configure interrupts based on hs_phy_flag to avoid triggering of interrupts during system suspend and suspends successfully. Set genpd active wakeup flag for usb gdsc if wakeup capable devices are connected so that wake up happens without reenumeration.

---
 drivers/usb/dwc3/dwc3-qcom.c | 74 +++++++++++++++++++++++++++++++++++---------
 1 file changed, 59 insertions(+), 15 deletions(-)
drivers/usb/dwc3/dwc3-qcom.c +59 -15
index c703d55..b6f36bd 100644
@@ -17,9 +17,11 @@
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/phy/phy.h>
+#include <linux/pm_domain.h>
 #include <linux/usb/of.h>
 #include <linux/reset.h>
 #include <linux/iopoll.h>
+#include <linux/usb/hcd.h>
 
 #include "core.h"
 
@@ -293,19 +295,33 @@ static void dwc3_qcom_interconnect_exit(struct dwc3_qcom *qcom)
 
 static void dwc3_qcom_disable_interrupts(struct dwc3_qcom *qcom)
 {
+	struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
+
 	if (qcom->hs_phy_irq) {
 		disable_irq_wake(qcom->hs_phy_irq);
 		disable_irq_nosync(qcom->hs_phy_irq);
 	}
 
-	if (qcom->dp_hs_phy_irq) {
-		disable_irq_wake(qcom->dp_hs_phy_irq);
-		disable_irq_nosync(qcom->dp_hs_phy_irq);
-	}
+	if (dwc->hs_phy_flags & PHY_MODE_USB_HOST_LS) {
+		if (qcom->dp_hs_phy_irq) {
+			disable_irq_wake(qcom->dp_hs_phy_irq);
+			disable_irq_nosync(qcom->dp_hs_phy_irq);
+		}
+	} else if (dwc->hs_phy_flags & PHY_MODE_USB_HOST_HS) {
+		if (qcom->dm_hs_phy_irq) {
+			disable_irq_wake(qcom->dm_hs_phy_irq);
+			disable_irq_nosync(qcom->dm_hs_phy_irq);
+		}
+	} else {
+		if (qcom->dp_hs_phy_irq) {
+			disable_irq_wake(qcom->dp_hs_phy_irq);
+			disable_irq_nosync(qcom->dp_hs_phy_irq);
+		}
 
-	if (qcom->dm_hs_phy_irq) {
-		disable_irq_wake(qcom->dm_hs_phy_irq);
-		disable_irq_nosync(qcom->dm_hs_phy_irq);
+		if (qcom->dm_hs_phy_irq) {
+			disable_irq_wake(qcom->dm_hs_phy_irq);
+			disable_irq_nosync(qcom->dm_hs_phy_irq);
+		}
 	}
 
 	if (qcom->ss_phy_irq) {
@@ -316,19 +332,33 @@ static void dwc3_qcom_disable_interrupts(struct dwc3_qcom *qcom)
 
 static void dwc3_qcom_enable_interrupts(struct dwc3_qcom *qcom)
 {
+	struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
+
 	if (qcom->hs_phy_irq) {
 		enable_irq(qcom->hs_phy_irq);
 		enable_irq_wake(qcom->hs_phy_irq);
 	}
 
-	if (qcom->dp_hs_phy_irq) {
-		enable_irq(qcom->dp_hs_phy_irq);
-		enable_irq_wake(qcom->dp_hs_phy_irq);
-	}
+	if (dwc->hs_phy_flags & PHY_MODE_USB_HOST_LS) {
+		if (qcom->dp_hs_phy_irq) {
+			enable_irq(qcom->dp_hs_phy_irq);
+			enable_irq_wake(qcom->dp_hs_phy_irq);
+		}
+	} else if (dwc->hs_phy_flags & PHY_MODE_USB_HOST_HS) {
+		if (qcom->dm_hs_phy_irq) {
+			enable_irq(qcom->dm_hs_phy_irq);
+			enable_irq_wake(qcom->dm_hs_phy_irq);
+		}
+	} else {
+		if (qcom->dp_hs_phy_irq) {
+			enable_irq(qcom->dp_hs_phy_irq);
+			enable_irq_wake(qcom->dp_hs_phy_irq);
+		}
 
-	if (qcom->dm_hs_phy_irq) {
-		enable_irq(qcom->dm_hs_phy_irq);
-		enable_irq_wake(qcom->dm_hs_phy_irq);
+		if (qcom->dm_hs_phy_irq) {
+			enable_irq(qcom->dm_hs_phy_irq);
+			enable_irq_wake(qcom->dm_hs_phy_irq);
+		}
 	}
 
 	if (qcom->ss_phy_irq) {
@@ -341,6 +371,15 @@ static int dwc3_qcom_suspend(struct dwc3_qcom *qcom)
 {
 	u32 val;
 	int i, ret;
+	struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
+	struct usb_hcd  *hcd;
+	struct generic_pm_domain *genpd = pd_to_genpd(qcom->dev->pm_domain);
+
+	if (dwc->xhci) {
+		hcd = platform_get_drvdata(dwc->xhci);
+		if (usb_wakeup_enabled_descendants(hcd->self.root_hub))
+			genpd->flags |= GENPD_FLAG_ACTIVE_WAKEUP;
+	}
 
 	if (qcom->is_suspended)
 		return 0;
@@ -366,6 +405,11 @@ static int dwc3_qcom_resume(struct dwc3_qcom *qcom)
 {
 	int ret;
 	int i;
+	struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
+	struct generic_pm_domain *genpd = pd_to_genpd(qcom->dev->pm_domain);
+
+	if (dwc->xhci)
+		genpd->flags &= ~GENPD_FLAG_ACTIVE_WAKEUP;
 
 	if (!qcom->is_suspended)
 		return 0;
@@ -764,7 +808,7 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
 	if (ret)
 		goto interconnect_exit;
 
-	device_init_wakeup(&pdev->dev, 1);
+	device_init_wakeup(&pdev->dev, of_property_read_bool(np, "wakeup-source"));
 	qcom->is_suspended = false;
 	pm_runtime_set_active(dev);
 	pm_runtime_enable(dev);
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
#7 lore raw

Re: [PATCH v3 3/5] usb: dwc3: qcom: Configure wakeup interrupts and set genpd active wakeup flag

Quoted 54 lines · Sandeep Maheswaram

Configure interrupts based on hs_phy_flag to avoid triggering of interrupts during system suspend and suspends successfully. Set genpd active wakeup flag for usb gdsc if wakeup capable devices are connected so that wake up happens without reenumeration.

---
 drivers/usb/dwc3/dwc3-qcom.c | 74 +++++++++++++++++++++++++++++++++++---------
 1 file changed, 59 insertions(+), 15 deletions(-)
drivers/usb/dwc3/dwc3-qcom.c +17 -4
index c703d55..b6f36bd 100644
@@ -17,9 +17,11 @@
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/phy/phy.h>
+#include <linux/pm_domain.h>
 #include <linux/usb/of.h>
 #include <linux/reset.h>
 #include <linux/iopoll.h>
+#include <linux/usb/hcd.h>
 
 #include "core.h"
 
@@ -293,19 +295,33 @@ static void dwc3_qcom_interconnect_exit(struct dwc3_qcom *qcom)
 
 static void dwc3_qcom_disable_interrupts(struct dwc3_qcom *qcom)
 {
+	struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
+
 	if (qcom->hs_phy_irq) {
 		disable_irq_wake(qcom->hs_phy_irq);
 		disable_irq_nosync(qcom->hs_phy_irq);
 	}
 
-	if (qcom->dp_hs_phy_irq) {
-		disable_irq_wake(qcom->dp_hs_phy_irq);
-		disable_irq_nosync(qcom->dp_hs_phy_irq);
-	}
+	if (dwc->hs_phy_flags & PHY_MODE_USB_HOST_LS) {
+		if (qcom->dp_hs_phy_irq) {
+			disable_irq_wake(qcom->dp_hs_phy_irq);
+			disable_irq_nosync(qcom->dp_hs_phy_irq);
+		}
+	} else if (dwc->hs_phy_flags & PHY_MODE_USB_HOST_HS) {
+		if (qcom->dm_hs_phy_irq) {
+			disable_irq_wake(qcom->dm_hs_phy_irq);
+			disable_irq_nosync(qcom->dm_hs_phy_irq);
+		}
+	} else {
+		if (qcom->dp_hs_phy_irq) {
+			disable_irq_wake(qcom->dp_hs_phy_irq);

drivers/usb/dwc3/dwc3-qcom.c static void dwc3_qcom_interconnect_exit(struct dwc3_qcom *qcom) · line 324

Quoted 10 lines
drivers/usb/dwc3/dwc3-qcom.c +6 -3
+			disable_irq_nosync(qcom->dp_hs_phy_irq);
+		}
 
-	if (qcom->dm_hs_phy_irq) {
-		disable_irq_wake(qcom->dm_hs_phy_irq);
-		disable_irq_nosync(qcom->dm_hs_phy_irq);
+		if (qcom->dm_hs_phy_irq) {
+			disable_irq_wake(qcom->dm_hs_phy_irq);
+			disable_irq_nosync(qcom->dm_hs_phy_irq);
+		}

This function would benefit from a helper like this:

static void dwc3_qcon_enable_wakeup_irq(int wake_irq)

{
	if (wake_irq) {
		disable_irq_wake(wake_irq);
		disable_irq_nosync(wake_irq);
	}
}
Quoted 32 lines
drivers/usb/dwc3/dwc3-qcom.c +16 -4
 	}
 
 	if (qcom->ss_phy_irq) {
@@ -316,19 +332,33 @@ static void dwc3_qcom_disable_interrupts(struct dwc3_qcom *qcom)
 
 static void dwc3_qcom_enable_interrupts(struct dwc3_qcom *qcom)
 {
+	struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
+
 	if (qcom->hs_phy_irq) {
 		enable_irq(qcom->hs_phy_irq);
 		enable_irq_wake(qcom->hs_phy_irq);
 	}
 
-	if (qcom->dp_hs_phy_irq) {
-		enable_irq(qcom->dp_hs_phy_irq);
-		enable_irq_wake(qcom->dp_hs_phy_irq);
-	}
+	if (dwc->hs_phy_flags & PHY_MODE_USB_HOST_LS) {
+		if (qcom->dp_hs_phy_irq) {
+			enable_irq(qcom->dp_hs_phy_irq);
+			enable_irq_wake(qcom->dp_hs_phy_irq);
+		}
+	} else if (dwc->hs_phy_flags & PHY_MODE_USB_HOST_HS) {
+		if (qcom->dm_hs_phy_irq) {
+			enable_irq(qcom->dm_hs_phy_irq);
+			enable_irq_wake(qcom->dm_hs_phy_irq);
+		}
+	} else {
+		if (qcom->dp_hs_phy_irq) {
+			enable_irq(qcom->dp_hs_phy_irq);
+			enable_irq_wake(qcom->dp_hs_phy_irq);

drivers/usb/dwc3/dwc3-qcom.c static void dwc3_qcom_disable_interrupts(struct dwc3_qcom *qcom) · line 362

Quoted 10 lines
drivers/usb/dwc3/dwc3-qcom.c +5 -3
+		}
 
-	if (qcom->dm_hs_phy_irq) {
-		enable_irq(qcom->dm_hs_phy_irq);
-		enable_irq_wake(qcom->dm_hs_phy_irq);
+		if (qcom->dm_hs_phy_irq) {
+			enable_irq(qcom->dm_hs_phy_irq);
+			enable_irq_wake(qcom->dm_hs_phy_irq);
+		}
 	}

Same as for _disable(), a helper would make this function more digestable.

drivers/usb/dwc3/dwc3-qcom.c static int dwc3_qcom_suspend(struct dwc3_qcom *qcom) · line 375

Quoted 7 lines
drivers/usb/dwc3/dwc3-qcom.c +2 -0
 	if (qcom->ss_phy_irq) {
@@ -341,6 +371,15 @@ static int dwc3_qcom_suspend(struct dwc3_qcom *qcom)
 {
 	u32 val;
 	int i, ret;
+	struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
+	struct usb_hcd  *hcd;

nit: remove extra blank

drivers/usb/dwc3/dwc3-qcom.c static int dwc3_qcom_suspend(struct dwc3_qcom *qcom) · line 382

Quoted 7 lines
drivers/usb/dwc3/dwc3-qcom.c +7 -0
+	struct generic_pm_domain *genpd = pd_to_genpd(qcom->dev->pm_domain);
+
+	if (dwc->xhci) {
+		hcd = platform_get_drvdata(dwc->xhci);
+		if (usb_wakeup_enabled_descendants(hcd->self.root_hub))
+			genpd->flags |= GENPD_FLAG_ACTIVE_WAKEUP;
+	}

Do this after the check for 'qcom->is_suspended' below

drivers/usb/dwc3/dwc3-qcom.c static int dwc3_qcom_resume(struct dwc3_qcom *qcom) · line 412

Quoted 12 lines
drivers/usb/dwc3/dwc3-qcom.c +5 -0
 	if (qcom->is_suspended)
 		return 0;
@@ -366,6 +405,11 @@ static int dwc3_qcom_resume(struct dwc3_qcom *qcom)
 {
 	int ret;
 	int i;
+	struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
+	struct generic_pm_domain *genpd = pd_to_genpd(qcom->dev->pm_domain);
+
+	if (dwc->xhci)
+		genpd->flags &= ~GENPD_FLAG_ACTIVE_WAKEUP;

ditto

Quoted 13 lines
drivers/usb/dwc3/dwc3-qcom.c +1 -1
 	if (!qcom->is_suspended)
 		return 0;
@@ -764,7 +808,7 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
 	if (ret)
 		goto interconnect_exit;
 
-	device_init_wakeup(&pdev->dev, 1);
+	device_init_wakeup(&pdev->dev, of_property_read_bool(np, "wakeup-source"));
 	qcom->is_suspended = false;
 	pm_runtime_set_active(dev);
 	pm_runtime_enable(dev);
#8 lore raw

PATCH v3 4/5 arm64: dts: qcom: sc7180: Use pdc interrupts for USB instead of GIC interrupts

In reply to Sandeep Maheswaram

Using pdc interrupts for USB instead of GIC interrupts to support wake up in case xo shutdown.

---
 arch/arm64/boot/dts/qcom/sc7180.dtsi | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
arch/arm64/boot/dts/qcom/sc7180.dtsi +4 -4
index 6678f1e..b196764 100644
@@ -2687,10 +2687,10 @@
 					  <&gcc GCC_USB30_PRIM_MASTER_CLK>;
 			assigned-clock-rates = <19200000>, <150000000>;
 
-			interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>,
-				     <GIC_SPI 486 IRQ_TYPE_LEVEL_HIGH>,
-				     <GIC_SPI 488 IRQ_TYPE_LEVEL_HIGH>,
-				     <GIC_SPI 489 IRQ_TYPE_LEVEL_HIGH>;
+			interrupts-extended = <&intc GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>,
+					      <&pdc 6 IRQ_TYPE_LEVEL_HIGH>,
+					      <&pdc 8 IRQ_TYPE_LEVEL_HIGH>,
+					      <&pdc 9 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "hs_phy_irq", "ss_phy_irq",
 					  "dm_hs_phy_irq", "dp_hs_phy_irq";
 
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
#10 lore raw

Re: [PATCH v3 4/5] arm64: dts: qcom: sc7180: Use pdc interrupts for USB instead of GIC interrupts

Quoting Sergei Shtylyov (2020-09-29 04:41:19)

Quoted 8 lines

Hello!

Quoted 2 lines · Sandeep Maheswaram

Using pdc interrupts for USB instead of GIC interrupts to support wake up in case xo shutdown.

    s/xo/of/?

No it is xo. If anything it could be capitalized because it's the pin name and usually stands for "crystal oscillator".

Quoted 3 lines
Quoted 3 lines
#11 lore raw

Re: [PATCH v3 4/5] arm64: dts: qcom: sc7180: Use pdc interrupts for USB instead of GIC interrupts

On 01.10.2020 4:05, Stephen Boyd wrote:

[...]

Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

Using pdc interrupts for USB instead of GIC interrupts to support wake up in case xo shutdown.

     s/xo/of/?

No it is xo. If anything it could be capitalized because it's the pin name and usually stands for "crystal oscillator".

In this case, "of" is still needed. :-)

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

MBR, Sergei

#12 lore raw

PATCH v3 5/5 arm64: dts: qcom: sc7180: Add wakeup-source property for USB controller node

In reply to Sandeep Maheswaram

Adding wakeup-source property for USB controller for SC7180 boards.

---
 arch/arm64/boot/dts/qcom/sc7180-idp.dts      | 1 +
 arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 1 +
 2 files changed, 2 insertions(+)
arch/arm64/boot/dts/qcom/sc7180-idp.dts +1 -0
index e77a792..fe1366a 100644
@@ -369,6 +369,7 @@
 
 &usb_1 {
 	status = "okay";
+	wakeup-source;
 };
 
 &usb_1_dwc3 {
arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi +1 -0
index bf87558..c320c09 100644
@@ -818,6 +818,7 @@ ap_spi_fp: &spi10 {
 
 &usb_1 {
 	status = "okay";
+	wakeup-source;
 };
 
 &usb_1_dwc3 {
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
Reviewed-by on v3 5/5
#13 lore raw

Re: [PATCH v3 5/5] arm64: dts: qcom: sc7180: Add wakeup-source property for USB controller node

Quoted 3 lines · Sandeep Maheswaram

Adding wakeup-source property for USB controller for SC7180 boards.

nit: it would be better to have two separate patches or at least mention in the subject that this change is for the IDP and trogdor, not SC7180 in general.

Besides that: