[PATCH v2 0/3] usb: dwc3: Host wake up support from system suspend

[PATCH v2 0/3] usb: dwc3: Host wake up support from system suspend

[Sandeep Maheswaram]

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 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 (3):
  usb: dwc3: core: Host wake up support from system suspend
  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

 arch/arm64/boot/dts/qcom/sc7180.dtsi |  8 ++--
 drivers/usb/dwc3/core.c              | 47 +++++++++++++++++++----
 drivers/usb/dwc3/core.h              |  2 +
 drivers/usb/dwc3/dwc3-qcom.c         | 73 ++++++++++++++++++++++++++++--------
 4 files changed, 103 insertions(+), 27 deletions(-)

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

[PATCH v2 1/3] usb: dwc3: core: Host wake up support from system suspend

[Sandeep Maheswaram]

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

Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
---
 drivers/usb/dwc3/core.c | 47 ++++++++++++++++++++++++++++++++++++++++-------
 drivers/usb/dwc3/core.h |  2 ++
 2 files changed, 42 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 25c686a7..eb7c225 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -31,12 +31,14 @@
 #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"
 #include "io.h"
 
 #include "debug.h"
+#include "../host/xhci.h"
 
 #define DWC3_DEFAULT_AUTOSUSPEND_DELAY	5000 /* ms */
 
@@ -1627,10 +1629,36 @@ static int dwc3_core_init_for_resume(struct dwc3 *dwc)
 	return ret;
 }
 
+static void dwc3_set_phy_speed_flags(struct dwc3 *dwc)
+{
+
+	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_flags &= ~(PHY_MODE_USB_HOST_HS | PHY_MODE_USB_HOST_LS);
+
+	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;
+		}
+	}
+	phy_set_mode(dwc->usb2_generic_phy, dwc->hs_phy_flags);
+}
+
 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:
@@ -1643,9 +1671,12 @@ static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
 		dwc3_core_exit(dwc);
 		break;
 	case DWC3_GCTL_PRTCAP_HOST:
+		dwc3_set_phy_speed_flags(dwc);
 		if (!PMSG_IS_AUTO(msg)) {
-			dwc3_core_exit(dwc);
-			break;
+			if (usb_wakeup_enabled_descendants(hcd->self.root_hub))
+				dwc->need_phy_for_wakeup = true;
+			else
+				dwc->need_phy_for_wakeup = false;
 		}
 
 		/* Let controller to suspend HSPHY before PHY driver suspends */
@@ -1705,11 +1736,13 @@ 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->need_phy_for_wakeup) {
+				ret = dwc3_core_init_for_resume(dwc);
+				if (ret)
+					return ret;
+				dwc3_set_prtcap(dwc, DWC3_GCTL_PRTCAP_HOST);
+				break;
+			}
 		}
 		/* Restore GUSB2PHYCFG bits that were modified in suspend */
 		reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 013f42a..5367d510e 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -1094,6 +1094,8 @@ struct dwc3 {
 	struct phy		*usb3_generic_phy;
 
 	bool			phys_ready;
+	bool                    need_phy_for_wakeup;
+	unsigned int            hs_phy_flags;
 
 	struct ulpi		*ulpi;
 	bool			ulpi_ready;
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

[PATCH v2 2/3] usb: dwc3: qcom: Configure wakeup interrupts and set genpd active wakeup flag

[Sandeep Maheswaram]

configure interrupts based on hs_phy_flag. Set genpd active wakeup flag
for usb gdsc if wakeup capable devices are connected.

Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
---
 drivers/usb/dwc3/dwc3-qcom.c | 73 ++++++++++++++++++++++++++++++++++----------
 1 file changed, 57 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
index 1dfd024..8902670 100644
--- a/drivers/usb/dwc3/dwc3-qcom.c
+++ b/drivers/usb/dwc3/dwc3-qcom.c
@@ -16,9 +16,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"
 
@@ -192,21 +194,34 @@ static int dwc3_qcom_register_extcon(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 (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->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) {
 		disable_irq_wake(qcom->ss_phy_irq);
 		disable_irq_nosync(qcom->ss_phy_irq);
@@ -215,21 +230,34 @@ 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 (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->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) {
 		enable_irq(qcom->ss_phy_irq);
 		enable_irq_wake(qcom->ss_phy_irq);
@@ -240,6 +268,14 @@ static int dwc3_qcom_suspend(struct dwc3_qcom *qcom)
 {
 	u32 val;
 	int i;
+	struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
+	struct usb_hcd  *hcd = platform_get_drvdata(dwc->xhci);
+	struct generic_pm_domain *genpd;
+
+	genpd = pd_to_genpd(qcom->dev->pm_domain);
+
+	if (genpd && usb_wakeup_enabled_descendants(hcd->self.root_hub))
+		genpd->flags |= GENPD_FLAG_ACTIVE_WAKEUP;
 
 	if (qcom->is_suspended)
 		return 0;
@@ -261,6 +297,11 @@ static int dwc3_qcom_resume(struct dwc3_qcom *qcom)
 {
 	int ret;
 	int i;
+	struct generic_pm_domain *genpd;
+
+	genpd = pd_to_genpd(qcom->dev->pm_domain);
+	if (genpd)
+		genpd->flags &= !GENPD_FLAG_ACTIVE_WAKEUP;
 
 	if (!qcom->is_suspended)
 		return 0;
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

[PATCH v2 3/3] arm64: dts: qcom: sc7180: Use pdc interrupts for USB instead of GIC interrupts

[Sandeep Maheswaram]

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

Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
---
 arch/arm64/boot/dts/qcom/sc7180.dtsi | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
index 2be81a2..2c220367 100644
--- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
@@ -2520,10 +2520,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";
 
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

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

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2939 bytes --]

Hi Sandeep,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on balbi-usb/testing/next]
[also build test WARNING on robh/for-next v5.8-rc4 next-20200708]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use  as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Sandeep-Maheswaram/usb-dwc3-Host-wake-up-support-from-system-suspend/20200709-031939
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git testing/next
config: x86_64-allyesconfig (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 02946de3802d3bc65bc9f2eb9b8d4969b5a7add8)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/usb/dwc3/dwc3-qcom.c:304:20: warning: converting the result of '<<' to a boolean always evaluates to true [-Wtautological-constant-compare]
                   genpd->flags &= !GENPD_FLAG_ACTIVE_WAKEUP;
                                    ^
   include/linux/pm_domain.h:62:38: note: expanded from macro 'GENPD_FLAG_ACTIVE_WAKEUP'
   #define GENPD_FLAG_ACTIVE_WAKEUP (1U << 3)
                                        ^
   1 warning generated.

vim +304 drivers/usb/dwc3/dwc3-qcom.c

   295	
   296	static int dwc3_qcom_resume(struct dwc3_qcom *qcom)
   297	{
   298		int ret;
   299		int i;
   300		struct generic_pm_domain *genpd;
   301	
   302		genpd = pd_to_genpd(qcom->dev->pm_domain);
   303		if (genpd)
 > 304			genpd->flags &= !GENPD_FLAG_ACTIVE_WAKEUP;
   305	
   306		if (!qcom->is_suspended)
   307			return 0;
   308	
   309		dwc3_qcom_disable_interrupts(qcom);
   310	
   311		for (i = 0; i < qcom->num_clocks; i++) {
   312			ret = clk_prepare_enable(qcom->clks[i]);
   313			if (ret < 0) {
   314				while (--i >= 0)
   315					clk_disable_unprepare(qcom->clks[i]);
   316				return ret;
   317			}
   318		}
   319	
   320		/* Clear existing events from PHY related to L2 in/out */
   321		dwc3_qcom_setbits(qcom->qscratch_base, PWR_EVNT_IRQ_STAT_REG,
   322				  PWR_EVNT_LPM_IN_L2_MASK | PWR_EVNT_LPM_OUT_L2_MASK);
   323	
   324		qcom->is_suspended = false;
   325	
   326		return 0;
   327	}
   328	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 73990 bytes --]

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

[Stephen Boyd]

Quoting Sandeep Maheswaram (2020-07-08 12:10:16)
> configure interrupts based on hs_phy_flag. Set genpd active wakeup flag

Please capitalize the start of a sentence. What is 'hs_phy_flag'?

> for usb gdsc if wakeup capable devices are connected.

This tells us what is happening in the code but doesn't tell us the
important part, i.e. _why_ this patch is important. Why do we need to
set the genpd active wakeup flag? Why configure interrupt based on
hs_phy_flag, whatever that is.

> 
> Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
> ---
>  drivers/usb/dwc3/dwc3-qcom.c | 73 ++++++++++++++++++++++++++++++++++----------
>  1 file changed, 57 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> index 1dfd024..8902670 100644
> --- a/drivers/usb/dwc3/dwc3-qcom.c
> +++ b/drivers/usb/dwc3/dwc3-qcom.c
> @@ -192,21 +194,34 @@ static int dwc3_qcom_register_extcon(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 (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->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);
> +               }
>         }
> -

I liked the newline. Please keep it.

>         if (qcom->ss_phy_irq) {
>                 disable_irq_wake(qcom->ss_phy_irq);
>                 disable_irq_nosync(qcom->ss_phy_irq);
> @@ -215,21 +230,34 @@ 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 (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->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) {
>                 enable_irq(qcom->ss_phy_irq);
>                 enable_irq_wake(qcom->ss_phy_irq);

Can we use the wakeup irq support code in the kernel here? That would be
preferred to having the driver enable and disable irq wake at various
times when the irq is enabled and disabled (which is also odd by the
way). Why can't we request the irqs and leave them enabled all the time?
Also it seems like the binding should have 'wakeup-source' in it (see
Documentation/devicetree/bindings/power/wakeup-source.txt for more
info).

> @@ -240,6 +268,14 @@ static int dwc3_qcom_suspend(struct dwc3_qcom *qcom)
>  {
>         u32 val;
>         int i;
> +       struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
> +       struct usb_hcd  *hcd = platform_get_drvdata(dwc->xhci);
> +       struct generic_pm_domain *genpd;
> +
> +       genpd = pd_to_genpd(qcom->dev->pm_domain);
> +
> +       if (genpd && usb_wakeup_enabled_descendants(hcd->self.root_hub))

Feels like a comment would be good to explain why wakeup enabled
descendants matters here.

> +               genpd->flags |= GENPD_FLAG_ACTIVE_WAKEUP;
>  
>         if (qcom->is_suspended)
>                 return 0;
> @@ -261,6 +297,11 @@ static int dwc3_qcom_resume(struct dwc3_qcom *qcom)
>  {
>         int ret;
>         int i;
> +       struct generic_pm_domain *genpd;
> +
> +       genpd = pd_to_genpd(qcom->dev->pm_domain);

This does container_of() so it can't return NULL.

> +       if (genpd)

So this check is wrong?

> +               genpd->flags &= !GENPD_FLAG_ACTIVE_WAKEUP;

Re: [PATCH v2 3/3] arm64: dts: qcom: sc7180: Use pdc interrupts for USB instead of GIC interrupts

[Stephen Boyd]

Quoting Sandeep Maheswaram (2020-07-08 12:10:17)
> Using pdc interrupts for USB instead of GIC interrupts to
> support wake up in case xo shutdown.
> 
> Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
> ---

Reviewed-by: Stephen Boyd <swboyd@chromium.org>

I suppose wakeup-source should be added in the board files.

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

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 1219 bytes --]

Hi Sandeep,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on balbi-usb/testing/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use  as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Sandeep-Maheswaram/usb-dwc3-Host-wake-up-support-from-system-suspend/20200709-031939
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git testing/next
config: i386-randconfig-a012-20200713 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-14) 9.3.0
reproduce (this is a W=1 build):
        # save the attached .config to linux build tree
        make W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   ld: drivers/usb/dwc3/core.o: in function `dwc3_suspend_common':
>> core.c:(.text+0x3f0f): undefined reference to `usb_hcd_is_primary_hcd'
>> ld: core.c:(.text+0x40c7): undefined reference to `usb_wakeup_enabled_descendants'

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 43569 bytes --]

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

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 1250 bytes --]

Hi Sandeep,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on balbi-usb/testing/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use  as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Sandeep-Maheswaram/usb-dwc3-Host-wake-up-support-from-system-suspend/20200709-031939
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git testing/next
config: i386-randconfig-a012-20200713 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-14) 9.3.0
reproduce (this is a W=1 build):
        # save the attached .config to linux build tree
        make W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   ld: drivers/usb/dwc3/core.o: in function `dwc3_suspend_common':
>> core.c:(.text+0x3f0f): undefined reference to `usb_hcd_is_primary_hcd'
>> ld: core.c:(.text+0x40c7): undefined reference to `usb_wakeup_enabled_descendants'

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 43569 bytes --]

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

[Matthias Kaehlcke]

On Mon, Jul 13, 2020 at 11:34:11PM +0800, kernel test robot wrote:
> Hi Sandeep,
> 
> Thank you for the patch! Yet something to improve:
> 
> [auto build test ERROR on balbi-usb/testing/next]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use  as documented in
> https://git-scm.com/docs/git-format-patch]
> 
> url:    https://github.com/0day-ci/linux/commits/Sandeep-Maheswaram/usb-dwc3-Host-wake-up-support-from-system-suspend/20200709-031939
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git testing/next
> config: i386-randconfig-a012-20200713 (attached as .config)
> compiler: gcc-9 (Debian 9.3.0-14) 9.3.0
> reproduce (this is a W=1 build):
>         # save the attached .config to linux build tree
>         make W=1 ARCH=i386 
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All errors (new ones prefixed by >>):
> 
>    ld: drivers/usb/dwc3/core.o: in function `dwc3_suspend_common':
> >> core.c:(.text+0x3f0f): undefined reference to `usb_hcd_is_primary_hcd'
> >> ld: core.c:(.text+0x40c7): undefined reference to `usb_wakeup_enabled_descendants'

The problem here seems to be that:

CONFIG_USB_DWC3=y

and

CONFIG_USB=m

Add a Kconfig condition that disallows usb_dwc3 to be builtin when the
USB core is built as a module?

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

[Matthias Kaehlcke]

[-- Attachment #1: Type: text/plain, Size: 1434 bytes --]

On Mon, Jul 13, 2020 at 11:34:11PM +0800, kernel test robot wrote:
> Hi Sandeep,
> 
> Thank you for the patch! Yet something to improve:
> 
> [auto build test ERROR on balbi-usb/testing/next]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use  as documented in
> https://git-scm.com/docs/git-format-patch]
> 
> url:    https://github.com/0day-ci/linux/commits/Sandeep-Maheswaram/usb-dwc3-Host-wake-up-support-from-system-suspend/20200709-031939
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git testing/next
> config: i386-randconfig-a012-20200713 (attached as .config)
> compiler: gcc-9 (Debian 9.3.0-14) 9.3.0
> reproduce (this is a W=1 build):
>         # save the attached .config to linux build tree
>         make W=1 ARCH=i386 
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All errors (new ones prefixed by >>):
> 
>    ld: drivers/usb/dwc3/core.o: in function `dwc3_suspend_common':
> >> core.c:(.text+0x3f0f): undefined reference to `usb_hcd_is_primary_hcd'
> >> ld: core.c:(.text+0x40c7): undefined reference to `usb_wakeup_enabled_descendants'

The problem here seems to be that:

CONFIG_USB_DWC3=y

and

CONFIG_USB=m

Add a Kconfig condition that disallows usb_dwc3 to be builtin when the
USB core is built as a module?

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

[Matthias Kaehlcke]

Hi Sandeep,

On Thu, Jul 09, 2020 at 12:40:15AM +0530, Sandeep Maheswaram wrote:
> Avoiding phy powerdown in host mode so that it can be wake up by devices.
> Added need_phy_for_wakeup flag to distinugush resume path and hs_phy_flags
> to check connection status and set phy mode and  configure interrupts.
> 
> Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
> ---
>  drivers/usb/dwc3/core.c | 47 ++++++++++++++++++++++++++++++++++++++++-------
>  drivers/usb/dwc3/core.h |  2 ++
>  2 files changed, 42 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 25c686a7..eb7c225 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -31,12 +31,14 @@
>  #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"
>  #include "io.h"
>  
>  #include "debug.h"
> +#include "../host/xhci.h"
>  
>  #define DWC3_DEFAULT_AUTOSUSPEND_DELAY	5000 /* ms */
>  
> @@ -1627,10 +1629,36 @@ static int dwc3_core_init_for_resume(struct dwc3 *dwc)
>  	return ret;
>  }
>  
> +static void dwc3_set_phy_speed_flags(struct dwc3 *dwc)
> +{
> +
> +	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_flags &= ~(PHY_MODE_USB_HOST_HS | PHY_MODE_USB_HOST_LS);

Where is hs_phy_flags initialized? As far as I can tell it isn't, hence when
dwc3_set_phy_speed_flags() is executed the first time it is 0 (from
devm_kzalloc()), and after the '&=' it is still 0. The next time it will have
whatever value it was set to in the below loop, which is then cleared by
the '&='. It seems you could as well just write 'dwc->hs_phy_flags = 0',
which is clearer, unless the field is used in some other way that isn't
obvious to me.

> +
> +	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;

Is another entry for DEV_SUPERSPEED needed?

> +		}
> +	}
> +	phy_set_mode(dwc->usb2_generic_phy, dwc->hs_phy_flags);
> +}
> +
>  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:
> @@ -1643,9 +1671,12 @@ static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
>  		dwc3_core_exit(dwc);
>  		break;
>  	case DWC3_GCTL_PRTCAP_HOST:
> +		dwc3_set_phy_speed_flags(dwc);
>  		if (!PMSG_IS_AUTO(msg)) {
> -			dwc3_core_exit(dwc);
> -			break;
> +			if (usb_wakeup_enabled_descendants(hcd->self.root_hub))
> +				dwc->need_phy_for_wakeup = true;
> +			else
> +				dwc->need_phy_for_wakeup = false;
>  		}
>  
>  		/* Let controller to suspend HSPHY before PHY driver suspends */
> @@ -1705,11 +1736,13 @@ 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->need_phy_for_wakeup) {
> +				ret = dwc3_core_init_for_resume(dwc);

Before this patch we had the combo dwc3_core_exit() / dwc3_core_init_for_resume(),
now it is only dwc3_core_init_for_resume() for !dwc->need_phy_for_wakeup.
Doesn't this cause trouble with enable counts, e.g. with clk_bulk_prepare_enable()
being called in dwc3_core_init_for_resume(), without the corresponding
clk_bulk_disable_unprepare() calls in dwc3_core_exit()?

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

[Matthias Kaehlcke]

Hi Sandeep,

On Thu, Jul 09, 2020 at 12:40:16AM +0530, Sandeep Maheswaram wrote:
> configure interrupts based on hs_phy_flag. Set genpd active wakeup flag
> for usb gdsc if wakeup capable devices are connected.

as Stephen remarked, please describe why this patch is doing what
it is doing.

> Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
> ---
>  drivers/usb/dwc3/dwc3-qcom.c | 73 ++++++++++++++++++++++++++++++++++----------
>  1 file changed, 57 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> index 1dfd024..8902670 100644
> --- a/drivers/usb/dwc3/dwc3-qcom.c
> +++ b/drivers/usb/dwc3/dwc3-qcom.c
> @@ -16,9 +16,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"
>  
> @@ -192,21 +194,34 @@ static int dwc3_qcom_register_extcon(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);
>  	}

nit: add empty line

> +	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 {
>

delete empty line

> -	if (qcom->dp_hs_phy_irq) {
> -		disable_irq_wake(qcom->dp_hs_phy_irq);
> -		disable_irq_nosync(qcom->dp_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 (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) {
>  		disable_irq_wake(qcom->ss_phy_irq);
>  		disable_irq_nosync(qcom->ss_phy_irq);
> @@ -215,21 +230,34 @@ 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);
>  	}

nit: add empty line

> +	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 {
>

delete empty line

> -	if (qcom->dp_hs_phy_irq) {
> -		enable_irq(qcom->dp_hs_phy_irq);
> -		enable_irq_wake(qcom->dp_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 (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) {
>  		enable_irq(qcom->ss_phy_irq);
>  		enable_irq_wake(qcom->ss_phy_irq);
> @@ -240,6 +268,14 @@ static int dwc3_qcom_suspend(struct dwc3_qcom *qcom)
>  {
>  	u32 val;
>  	int i;
> +	struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
> +	struct usb_hcd  *hcd = platform_get_drvdata(dwc->xhci);
> +	struct generic_pm_domain *genpd; 
> +
> +	genpd = pd_to_genpd(qcom->dev->pm_domain);

nit: assign in declaration?

> +
> +	if (genpd && usb_wakeup_enabled_descendants(hcd->self.root_hub))
> +		genpd->flags |= GENPD_FLAG_ACTIVE_WAKEUP;
>  
>  	if (qcom->is_suspended)
>  		return 0;
> @@ -261,6 +297,11 @@ static int dwc3_qcom_resume(struct dwc3_qcom *qcom)
>  {
>  	int ret;
>  	int i;
> +	struct generic_pm_domain *genpd;
> +
> +	genpd = pd_to_genpd(qcom->dev->pm_domain);
> +	if (genpd)
> +		genpd->flags &= !GENPD_FLAG_ACTIVE_WAKEUP;

  			     	~GENPD_FLAG_ACTIVE_WAKEUP; ?

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

[Matthias Kaehlcke]

On Thu, Jul 09, 2020 at 12:40:15AM +0530, Sandeep Maheswaram wrote:
> Avoiding phy powerdown in host mode so that it can be wake up by devices.
> Added need_phy_for_wakeup flag to distinugush resume path and hs_phy_flags
> to check connection status and set phy mode and  configure interrupts.
> 
> Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
> ---
>  drivers/usb/dwc3/core.c | 47 ++++++++++++++++++++++++++++++++++++++++-------
>  drivers/usb/dwc3/core.h |  2 ++
>  2 files changed, 42 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 25c686a7..eb7c225 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -31,12 +31,14 @@
>  #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"
>  #include "io.h"
>  
>  #include "debug.h"
> +#include "../host/xhci.h"
>  
>  #define DWC3_DEFAULT_AUTOSUSPEND_DELAY	5000 /* ms */
>  
> @@ -1627,10 +1629,36 @@ static int dwc3_core_init_for_resume(struct dwc3 *dwc)
>  	return ret;
>  }
>  
> +static void dwc3_set_phy_speed_flags(struct dwc3 *dwc)
> +{
> +
> +	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_flags &= ~(PHY_MODE_USB_HOST_HS | PHY_MODE_USB_HOST_LS);
> +
> +	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;
> +		}
> +	}
> +	phy_set_mode(dwc->usb2_generic_phy, dwc->hs_phy_flags);
> +}
> +
>  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:
> @@ -1643,9 +1671,12 @@ static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
>  		dwc3_core_exit(dwc);
>  		break;
>  	case DWC3_GCTL_PRTCAP_HOST:
> +		dwc3_set_phy_speed_flags(dwc);
>  		if (!PMSG_IS_AUTO(msg)) {
> -			dwc3_core_exit(dwc);
> -			break;
> +			if (usb_wakeup_enabled_descendants(hcd->self.root_hub))
> +				dwc->need_phy_for_wakeup = true;
> +			else
> +				dwc->need_phy_for_wakeup = false;
>  		}
>  
>  		/* Let controller to suspend HSPHY before PHY driver suspends */
> @@ -1705,11 +1736,13 @@ 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->need_phy_for_wakeup) {
> +				ret = dwc3_core_init_for_resume(dwc);
> +				if (ret)
> +					return ret;
> +				dwc3_set_prtcap(dwc, DWC3_GCTL_PRTCAP_HOST);
> +				break;
> +			}
>  		}
>  		/* Restore GUSB2PHYCFG bits that were modified in suspend */
>  		reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 013f42a..5367d510e 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -1094,6 +1094,8 @@ struct dwc3 {
>  	struct phy		*usb3_generic_phy;
>  
>  	bool			phys_ready;
> +	bool                    need_phy_for_wakeup;
> +	unsigned int            hs_phy_flags;
>  
>  	struct ulpi		*ulpi;
>  	bool			ulpi_ready;

Should this include a check for the 'wakeup-source' DT attribute as in
xhci-mtk.c, to make wakeup support optional?