USB DWC3 host wake up support from system suspend

During suspend read the status of all port and make sure the PHYs are in the correct mode based on current speed.
Phy interrupt masks are set based on this mode. Keep track of the mode of the HS PHY to be able to configure wakeup properly.

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 should be powered down during suspend.

Signed-off-by Sandeep Maheswaram <[email protected]>

---
 drivers/usb/host/xhci-plat.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

drivers/usb/host/xhci-plat.c +1 -0

index c1edcc9..ee87923 100644
@@ -24,6 +24,7 @@
 #include "xhci-plat.h"
 #include "xhci-mvebu.h"
 #include "xhci-rcar.h"
+#include "../dwc3/core.h"
 
 static struct hc_driver __read_mostly xhci_plat_hc_driver;
 

@@ -430,6 +431,39 @@ static int xhci_plat_remove(struct platform_device *dev)
 
 	return 0;

During suspend read the status of all port and make sure the PHYs are in the correct mode based on current speed.
Phy interrupt masks are set based on this mode. Keep track of the mode of the HS PHY to be able to configure wakeup properly.

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 should be powered down during suspend.

drivers/usb/host/xhci-plat.c +22 -0

+{
+	int i, num_ports;
+	u32 reg;
+	unsigned int ss_phy_mode = 0;
+	struct dwc3 *dwc = dev_get_drvdata(hcd->self.controller->parent);
+	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 * 0x04);
+		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;
+
+			if (DEV_SUPERSPEED(reg))
+				ss_phy_mode |= PHY_MODE_USB_HOST_SS;

If wakeup capable devices are connected to the controller (directly or through hubs) at suspend time keep the power domain on in order to support wakeup from these devices.

Signed-off-by Sandeep Maheswaram <[email protected]>

---
Checking phy_power_off flag instead of usb_wakeup_enabled_descendants to keep gdsc active.

 drivers/usb/dwc3/dwc3-qcom.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

drivers/usb/dwc3/dwc3-qcom.c +9 -0

index 82125bc..ba31aa3 100644
@@ -17,6 +17,7 @@
 #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>

@@ -355,9 +356,15 @@ static int dwc3_qcom_suspend(struct dwc3_qcom *qcom)
 	u32 val;
 	int i, ret;
 
+	struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
+	struct generic_pm_domain *genpd = pd_to_genpd(qcom->dev->pm_domain);
+
 	if (qcom->is_suspended)
 		return 0;
 
+	if (!dwc->phy_power_off && dwc->xhci)
+		genpd->flags |= GENPD_FLAG_ACTIVE_WAKEUP;
+
 	val = readl(qcom->qscratch_base + PWR_EVNT_IRQ_STAT_REG);
 	if (!(val & PWR_EVNT_LPM_IN_L2_MASK))
 		dev_err(qcom->dev, "HS-PHY not in L2\n");

@@ -382,9 +389,15 @@ 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);

Quoted 46 lines · Sandeep Maheswaram

If wakeup capable devices are connected to the controller (directly or through hubs) at suspend time keep the power domain on in order to support wakeup from these devices.

Signed-off-by Sandeep Maheswaram <[email protected]>

---
Checking phy_power_off flag instead of usb_wakeup_enabled_descendants to keep gdsc active.

 drivers/usb/dwc3/dwc3-qcom.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

drivers/usb/dwc3/dwc3-qcom.c +9 -0

index 82125bc..ba31aa3 100644
@@ -17,6 +17,7 @@
 #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>

@@ -355,9 +356,15 @@ static int dwc3_qcom_suspend(struct dwc3_qcom *qcom)
 	u32 val;
 	int i, ret;
 
+	struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
+	struct generic_pm_domain *genpd = pd_to_genpd(qcom->dev->pm_domain);
+
 	if (qcom->is_suspended)
 		return 0;
 
+	if (!dwc->phy_power_off && dwc->xhci)
+		genpd->flags |= GENPD_FLAG_ACTIVE_WAKEUP;
+
 	val = readl(qcom->qscratch_base + PWR_EVNT_IRQ_STAT_REG);
 	if (!(val & PWR_EVNT_LPM_IN_L2_MASK))
 		dev_err(qcom->dev, "HS-PHY not in L2\n");

@@ -382,9 +389,15 @@ 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);

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

Quoted 10 lines

drivers/usb/dwc3/dwc3-qcom.c +4 -0

+
 	if (!qcom->is_suspended)
 		return 0;
 
+	if (dwc->xhci)
+		genpd->flags &= ~GENPD_FLAG_ACTIVE_WAKEUP;
+
 	if (device_may_wakeup(qcom->dev))
 		dwc3_qcom_disable_interrupts(qcom);
 

This is essentially the same as v7, which Felipe NAKed
(https://patchwork.kernel.org/project/linux-arm-msm/patch/[email protected]/)

I think Felipe wants to see the handling of the power domain in the xhci-plat driver. One problem here is that the power domain is owned

Hi,

Quoted 60 lines · Matthias Kaehlcke

Quoted 46 lines · Sandeep Maheswaram

If wakeup capable devices are connected to the controller (directly or through hubs) at suspend time keep the power domain on in order to support wakeup from these devices.

Signed-off-by Sandeep Maheswaram <[email protected]>

---
Checking phy_power_off flag instead of usb_wakeup_enabled_descendants to keep gdsc active.

  drivers/usb/dwc3/dwc3-qcom.c | 13 +++++++++++++
  1 file changed, 13 insertions(+)

drivers/usb/dwc3/dwc3-qcom.c +9 -0

index 82125bc..ba31aa3 100644
@@ -17,6 +17,7 @@
  #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>

@@ -355,9 +356,15 @@ static int dwc3_qcom_suspend(struct dwc3_qcom *qcom)
  	u32 val;
  	int i, ret;
 
+	struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
+	struct generic_pm_domain *genpd = pd_to_genpd(qcom->dev->pm_domain);
+
  	if (qcom->is_suspended)
  		return 0;
 
+	if (!dwc->phy_power_off && dwc->xhci)
+		genpd->flags |= GENPD_FLAG_ACTIVE_WAKEUP;
+
  	val = readl(qcom->qscratch_base + PWR_EVNT_IRQ_STAT_REG);
  	if (!(val & PWR_EVNT_LPM_IN_L2_MASK))
  		dev_err(qcom->dev, "HS-PHY not in L2\n");

@@ -382,9 +389,15 @@ 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);

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

Quoted 10 lines

drivers/usb/dwc3/dwc3-qcom.c +4 -0

+
  	if (!qcom->is_suspended)
  		return 0;
 
+	if (dwc->xhci)
+		genpd->flags &= ~GENPD_FLAG_ACTIVE_WAKEUP;
+
  	if (device_may_wakeup(qcom->dev))
  		dwc3_qcom_disable_interrupts(qcom);
 

This is essentially the same as v7, which Felipe NAKed
(https://patchwork.kernel.org/project/linux-arm-msm/patch/[email protected]/)

Quoted 2 lines

I think Felipe wants to see the handling of the power domain in the xhci-plat driver. One problem here is that the power domain is owned

this is not exactly what I meant to say, though. I want drivers to be self-contained. I.e. dwc3 doesn't modify xhci data and vice-versa. There are a few assummpmtions that we can make, though. The structure is usually like this:

glue {
   dwc3 {
     xhci
   };
};

This means that in order for glue_suspend() to run, dwc3 has to suspend first and xhci has to suspend before dwc3.

For example, in the suspend call above, qcom (the glue) is directly accessing dwc3 core data, which is incorrect. It looks like we want to know if the PHY is not powered off and if it isn't, then we want to change the power domain ACTIVE_WAKEUP flag. Now, phy_power_off is false whenever any of xHCI's children enable USB wakeup.

It seems like we need to way to generically propagate that knowledge up the parent tree. I.e., a parent needs to know if its child is wakeup capable, then dwc3 could, in its suspend routine:

static int dwc3_suspend(struct device *dev)

{
	/* ... */

	if (device_children_wakeup_capable(dev))
         	device_enable_wakeup(dev);

	/* ... */
}

and similarly for qcom glue:

static int dwc3_qcom_suspend(struct device *dev)

{
	/* ... */


	if (device_children_wakeup_capable(dev)) {
         	device_enable_wakeup(dev);
		genpd->flags |= GENPD_FLAG_ACTIVE_WAKEUP;
         }

	/* ... */
}

It also seems plausible that this could be done at driver core and completely hidden away from drivers.

Quoted 17 lines

This means that in order for glue_suspend() to run, dwc3 has to suspend first and xhci has to suspend before dwc3.

For example, in the suspend call above, qcom (the glue) is directly accessing dwc3 core data, which is incorrect. It looks like we want to know if the PHY is not powered off and if it isn't, then we want to change the power domain ACTIVE_WAKEUP flag. Now, phy_power_off is false whenever any of xHCI's children enable USB wakeup.

It seems like we need to way to generically propagate that knowledge up the parent tree. I.e., a parent needs to know if its child is wakeup capable, then dwc3 could, in its suspend routine:

static int dwc3_suspend(struct device *dev)

{
	/* ... */

Quoted 5 lines

	if (device_children_wakeup_capable(dev))
         	device_enable_wakeup(dev);

	/* ... */
}

Can we use like  this device_may_wakeup(&dwc->xhci->dev) to check if children is wakeup capable like below ?

Quoted 15 lines

and similarly for qcom glue:

static int dwc3_qcom_suspend(struct device *dev)

{
	/* ... */


	if (device_children_wakeup_capable(dev)) {
         	device_enable_wakeup(dev);
		genpd->flags |= GENPD_FLAG_ACTIVE_WAKEUP;
         }

	/* ... */
}

Quoted 2 lines

It also seems plausible that this could be done at driver core and completely hidden away from drivers.

And in qcom glue like this

static int dwc3_qcom_suspend(struct dwc3_qcom *qcom) {

/* ... */

    struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);

Quoted 37 lines

Quoted 23 lines

(quoted diff) +8 -0

@@ -355,9 +356,15 @@ static int dwc3_qcom_suspend(struct dwc3_qcom *qcom)
 	u32 val;
 	int i, ret;
 
+	struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
+	struct generic_pm_domain *genpd = pd_to_genpd(qcom->dev->pm_domain);
+
 	if (qcom->is_suspended)
 		return 0;
 
+	if (!dwc->phy_power_off && dwc->xhci)
+		genpd->flags |= GENPD_FLAG_ACTIVE_WAKEUP;
+
 	val = readl(qcom->qscratch_base + PWR_EVNT_IRQ_STAT_REG);
 	if (!(val & PWR_EVNT_LPM_IN_L2_MASK))
 		dev_err(qcom->dev, "HS-PHY not in L2\n");

@@ -382,9 +389,15 @@ 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);

Review static int dwc3_qcom_resume(struct dwc3_qcom *qcom) · line 403

Quoted 10 lines

(quoted diff) +4 -0

+
 	if (!qcom->is_suspended)
 		return 0;
 
+	if (dwc->xhci)
+		genpd->flags &= ~GENPD_FLAG_ACTIVE_WAKEUP;
+
 	if (device_may_wakeup(qcom->dev))
 		dwc3_qcom_disable_interrupts(qcom);
 

This is essentially the same as v7, which Felipe NAKed
(https://patchwork.kernel.org/project/linux-arm-msm/patch/[email protected]/)

Quoted 2 lines

I think Felipe wants to see the handling of the power domain in the xhci-plat driver. One problem here is that the power domain is owned

this is not exactly what I meant to say, though. I want drivers to be self-contained. I.e. dwc3 doesn't modify xhci data and vice-versa. There are a few assummpmtions that we can make, though. The structure is usually like this:

glue {
dwc3 {
xhci
};
};

This means that in order for glue_suspend() to run, dwc3 has to suspend first and xhci has to suspend before dwc3.

For example, in the suspend call above, qcom (the glue) is directly accessing dwc3 core data, which is incorrect. It looks like we want to know if the PHY is not powered off and if it isn't, then we want to change the power domain ACTIVE_WAKEUP flag. Now, phy_power_off is false whenever any of xHCI's children enable USB wakeup.

It seems like we need to way to generically propagate that knowledge up the parent tree. I.e., a parent needs to know if its child is wakeup capable, then dwc3 could, in its suspend routine:

static int dwc3_suspend(struct device *dev)

{
	/* ... */

	if (device_children_wakeup_capable(dev))
        	device_enable_wakeup(dev);

	/* ... */
}

and similarly for qcom glue:

static int dwc3_qcom_suspend(struct device *dev)

{
	/* ... */

	if (device_children_wakeup_capable(dev)) {
        	device_enable_wakeup(dev);
		genpd->flags |= GENPD_FLAG_ACTIVE_WAKEUP;
        }

	/* ... */
}

It also seems plausible that this could be done at driver core and completely hidden away from drivers.