diff mbox series

[1/8] usb: cdns3: add runtime PM support

Message ID 20200522103256.16322-2-peter.chen@nxp.com (mailing list archive)
State New, archived
Headers show
Series usb: cdns3: add runtime PM support | expand

Commit Message

Peter Chen May 22, 2020, 10:32 a.m. UTC
Introduce runtime PM and wakeup interrupt handler for cdns3,
the runtime PM is default off since other cdns3 has not implemented
glue layer support for runtime PM.

When the controller is in low power mode, the lpm flag will be set.
The interrupt triggered later than lpm flag is set considers as
wakeup interrupt and handled at cdns_drd_irq. Once the wakeup
occurs, it first disables interrupt to avoid later interrupt
occurrence since the controller is in low power mode at that
time, and access registers may be invalid at that time. At wakeup
handler, it will call pm_runtime_get to wakeup controller, and
at runtime resume handler, it will enable interrupt again.

The API platform_suspend is introduced for glue layer to implement
platform specific PM sequence.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 drivers/usb/cdns3/core.c   | 119 +++++++++++++++++++++++++++++++------
 drivers/usb/cdns3/core.h   |  14 +++++
 drivers/usb/cdns3/drd.c    |   7 +++
 drivers/usb/cdns3/gadget.c |   4 ++
 drivers/usb/cdns3/host.c   |   7 +++
 5 files changed, 134 insertions(+), 17 deletions(-)

Comments

Felipe Balbi Aug. 31, 2020, 10:01 a.m. UTC | #1
Hi,

Peter Chen <peter.chen@nxp.com> writes:
> Introduce runtime PM and wakeup interrupt handler for cdns3,
> the runtime PM is default off since other cdns3 has not implemented
> glue layer support for runtime PM.

it should be left disabled by default even after every glue implements
it. Enabling runtime PM is a policy decision better left to userspace,
IMO.

> When the controller is in low power mode, the lpm flag will be set.
> The interrupt triggered later than lpm flag is set considers as
> wakeup interrupt and handled at cdns_drd_irq. Once the wakeup

This sentence doesn't parse very well. Do you want to rephrase it?

> occurs, it first disables interrupt to avoid later interrupt
> occurrence since the controller is in low power mode at that
> time, and access registers may be invalid at that time. At wakeup
> handler, it will call pm_runtime_get to wakeup controller, and
> at runtime resume handler, it will enable interrupt again.

Then what? Are you going to, somehow, process pending interrupts? At
what point?

> The API platform_suspend is introduced for glue layer to implement
> platform specific PM sequence.

I really think that's unnecessary. Why can't we rely on parent->child
relationship for this?
Peter Chen Aug. 31, 2020, 10:59 a.m. UTC | #2
> 
> Peter Chen <peter.chen@nxp.com> writes:
> > Introduce runtime PM and wakeup interrupt handler for cdns3, the
> > runtime PM is default off since other cdns3 has not implemented glue
> > layer support for runtime PM.
> 
> it should be left disabled by default even after every glue implements it.
> Enabling runtime PM is a policy decision better left to userspace, IMO.
> 
> > When the controller is in low power mode, the lpm flag will be set.
> > The interrupt triggered later than lpm flag is set considers as wakeup
> > interrupt and handled at cdns_drd_irq. Once the wakeup
> 
> This sentence doesn't parse very well. Do you want to rephrase it?
> 
> > occurs, it first disables interrupt to avoid later interrupt
> > occurrence since the controller is in low power mode at that time, and
> > access registers may be invalid at that time. At wakeup handler, it
> > will call pm_runtime_get to wakeup controller, and at runtime resume
> > handler, it will enable interrupt again.
> 
> Then what? Are you going to, somehow, process pending interrupts? At what
> point?
> 
> > The API platform_suspend is introduced for glue layer to implement
> > platform specific PM sequence.
> 
> I really think that's unnecessary. Why can't we rely on parent->child
> relationship for this?
> 
 
Hi Felipe,

This is the old version patch; I replied your questions at your reply for new
version at last Friday, thanks.

Peter
Felipe Balbi Aug. 31, 2020, 11:27 a.m. UTC | #3
Hi,

Peter Chen <peter.chen@nxp.com> writes:
>> Peter Chen <peter.chen@nxp.com> writes:
>> > Introduce runtime PM and wakeup interrupt handler for cdns3, the
>> > runtime PM is default off since other cdns3 has not implemented glue
>> > layer support for runtime PM.
>> 
>> it should be left disabled by default even after every glue implements it.
>> Enabling runtime PM is a policy decision better left to userspace, IMO.
>> 
>> > When the controller is in low power mode, the lpm flag will be set.
>> > The interrupt triggered later than lpm flag is set considers as wakeup
>> > interrupt and handled at cdns_drd_irq. Once the wakeup
>> 
>> This sentence doesn't parse very well. Do you want to rephrase it?
>> 
>> > occurs, it first disables interrupt to avoid later interrupt
>> > occurrence since the controller is in low power mode at that time, and
>> > access registers may be invalid at that time. At wakeup handler, it
>> > will call pm_runtime_get to wakeup controller, and at runtime resume
>> > handler, it will enable interrupt again.
>> 
>> Then what? Are you going to, somehow, process pending interrupts? At what
>> point?
>> 
>> > The API platform_suspend is introduced for glue layer to implement
>> > platform specific PM sequence.
>> 
>> I really think that's unnecessary. Why can't we rely on parent->child
>> relationship for this?
>> 
>  
> Hi Felipe,
>
> This is the old version patch; I replied your questions at your reply for new
> version at last Friday, thanks.

yeah sorry, I'm getting caught up on my inbox from oldest to
newest. This slip through the cracks ;-)
diff mbox series

Patch

diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
index bfd09aa98c12..5221c1306e64 100644
--- a/drivers/usb/cdns3/core.c
+++ b/drivers/usb/cdns3/core.c
@@ -432,6 +432,7 @@  static int cdns3_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	cdns->dev = dev;
+	cdns->pdata = dev_get_platdata(dev);
 
 	platform_set_drvdata(pdev, cdns);
 
@@ -525,9 +526,11 @@  static int cdns3_probe(struct platform_device *pdev)
 	if (ret)
 		goto err4;
 
+	spin_lock_init(&cdns->lock);
 	device_set_wakeup_capable(dev, true);
 	pm_runtime_set_active(dev);
 	pm_runtime_enable(dev);
+	pm_runtime_forbid(dev);
 
 	/*
 	 * The controller needs less time between bus and controller suspend,
@@ -574,52 +577,134 @@  static int cdns3_remove(struct platform_device *pdev)
 	return 0;
 }
 
-#ifdef CONFIG_PM_SLEEP
+#ifdef CONFIG_PM
 
-static int cdns3_suspend(struct device *dev)
+static int cdns3_set_platform_suspend(struct device *dev,
+		bool suspend, bool wakeup)
+{
+	struct cdns3 *cdns = dev_get_drvdata(dev);
+	int ret = 0;
+	unsigned long flags;
+
+	spin_lock_irqsave(&cdns->lock, flags);
+	if (cdns->pdata && cdns->pdata->platform_suspend)
+		ret = cdns->pdata->platform_suspend(dev, suspend, wakeup);
+
+	spin_unlock_irqrestore(&cdns->lock, flags);
+
+	return ret;
+}
+
+static int cdns3_controller_suspend(struct device *dev, pm_message_t msg)
 {
 	struct cdns3 *cdns = dev_get_drvdata(dev);
+	bool wakeup;
 	unsigned long flags;
 
-	if (cdns->role == USB_ROLE_HOST)
+	if (cdns->in_lpm)
 		return 0;
 
-	if (pm_runtime_status_suspended(dev))
-		pm_runtime_resume(dev);
+	if (PMSG_IS_AUTO(msg))
+		wakeup = true;
+	else
+		wakeup = false;
 
-	if (cdns->roles[cdns->role]->suspend) {
-		spin_lock_irqsave(&cdns->gadget_dev->lock, flags);
-		cdns->roles[cdns->role]->suspend(cdns, false);
-		spin_unlock_irqrestore(&cdns->gadget_dev->lock, flags);
-	}
+	cdns3_set_platform_suspend(cdns->dev, true, wakeup);
+	cdns3_set_phy_power(cdns, false);
+	spin_lock_irqsave(&cdns->lock, flags);
+	cdns->in_lpm = true;
+	spin_unlock_irqrestore(&cdns->lock, flags);
 
 	return 0;
 }
 
-static int cdns3_resume(struct device *dev)
+static int cdns3_controller_resume(struct device *dev, pm_message_t msg)
 {
 	struct cdns3 *cdns = dev_get_drvdata(dev);
+	int ret;
 	unsigned long flags;
 
-	if (cdns->role == USB_ROLE_HOST)
+	if (!cdns->in_lpm)
 		return 0;
 
-	if (cdns->roles[cdns->role]->resume) {
-		spin_lock_irqsave(&cdns->gadget_dev->lock, flags);
+	ret = cdns3_set_phy_power(cdns, true);
+	if (ret)
+		return ret;
+
+	cdns3_set_platform_suspend(cdns->dev, false, false);
+
+	spin_lock_irqsave(&cdns->lock, flags);
+	if (cdns->roles[cdns->role]->resume && !PMSG_IS_AUTO(msg))
 		cdns->roles[cdns->role]->resume(cdns, false);
-		spin_unlock_irqrestore(&cdns->gadget_dev->lock, flags);
+
+	cdns->in_lpm = false;
+	spin_unlock_irqrestore(&cdns->lock, flags);
+	if (cdns->wakeup_int) {
+		cdns->wakeup_int = false;
+		if (cdns->role == USB_ROLE_HOST) {
+			/* Trigger xhci-plat.c runtime runtime */
+			pm_runtime_get(&cdns->host_dev->dev);
+			pm_runtime_mark_last_busy(&cdns->host_dev->dev);
+			pm_runtime_put_autosuspend(&cdns->host_dev->dev);
+			/* balence the pm_runtime_get at cdns3_drd_irq */
+			pm_runtime_mark_last_busy(cdns->dev);
+			pm_runtime_put_autosuspend(cdns->dev);
+		}
+
+		enable_irq(cdns->otg_irq);
+	}
+
+	return ret;
+}
+
+static int cdns3_runtime_suspend(struct device *dev)
+{
+	return cdns3_controller_suspend(dev, PMSG_AUTO_SUSPEND);
+}
+
+static int cdns3_runtime_resume(struct device *dev)
+{
+	return cdns3_controller_resume(dev, PMSG_AUTO_RESUME);
+}
+#ifdef CONFIG_PM_SLEEP
+
+static int cdns3_suspend(struct device *dev)
+{
+	struct cdns3 *cdns = dev_get_drvdata(dev);
+	unsigned long flags;
+
+	if (pm_runtime_status_suspended(dev))
+		pm_runtime_resume(dev);
+
+	if (cdns->roles[cdns->role]->suspend) {
+		spin_lock_irqsave(&cdns->lock, flags);
+		cdns->roles[cdns->role]->suspend(cdns, false);
+		spin_unlock_irqrestore(&cdns->lock, flags);
 	}
 
+	return cdns3_controller_suspend(dev, PMSG_SUSPEND);
+}
+
+static int cdns3_resume(struct device *dev)
+{
+	int ret;
+
+	ret = cdns3_controller_resume(dev, PMSG_RESUME);
+	if (ret)
+		return ret;
+
 	pm_runtime_disable(dev);
 	pm_runtime_set_active(dev);
 	pm_runtime_enable(dev);
 
-	return 0;
+	return ret;
 }
-#endif
+#endif /* CONFIG_PM_SLEEP */
+#endif /* CONFIG_PM */
 
 static const struct dev_pm_ops cdns3_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(cdns3_suspend, cdns3_resume)
+	SET_RUNTIME_PM_OPS(cdns3_runtime_suspend, cdns3_runtime_resume, NULL)
 };
 
 #ifdef CONFIG_OF
diff --git a/drivers/usb/cdns3/core.h b/drivers/usb/cdns3/core.h
index 1ad1f1fe61e9..9eb8aeea2d20 100644
--- a/drivers/usb/cdns3/core.h
+++ b/drivers/usb/cdns3/core.h
@@ -38,6 +38,12 @@  struct cdns3_role_driver {
 };
 
 #define CDNS3_XHCI_RESOURCES_NUM	2
+
+struct cdns3_platform_data {
+	int (*platform_suspend)(struct device *dev,
+			bool suspend, bool wakeup);
+};
+
 /**
  * struct cdns3 - Representation of Cadence USB3 DRD controller.
  * @dev: pointer to Cadence device struct
@@ -62,6 +68,10 @@  struct cdns3_role_driver {
  *           This field based on firmware setting, kernel configuration
  *           and hardware configuration.
  * @role_sw: pointer to role switch object.
+ * @in_lpm: indicate the controller is in low power mode
+ * @wakeup_int: wakeup interrupt occurs
+ * @pdata: platform data from glue layer
+ * @lock: spinlock structure
  */
 struct cdns3 {
 	struct device			*dev;
@@ -89,6 +99,10 @@  struct cdns3 {
 	struct mutex			mutex;
 	enum usb_dr_mode		dr_mode;
 	struct usb_role_switch		*role_sw;
+	bool				in_lpm;
+	bool				wakeup_int;
+	struct cdns3_platform_data	*pdata;
+	spinlock_t			lock;
 };
 
 int cdns3_hw_role_switch(struct cdns3 *cdns);
diff --git a/drivers/usb/cdns3/drd.c b/drivers/usb/cdns3/drd.c
index 58089841ed52..292ea248c0ec 100644
--- a/drivers/usb/cdns3/drd.c
+++ b/drivers/usb/cdns3/drd.c
@@ -278,6 +278,13 @@  static irqreturn_t cdns3_drd_irq(int irq, void *data)
 	struct cdns3 *cdns = data;
 	u32 reg;
 
+	if (cdns->in_lpm) {
+		disable_irq_nosync(irq);
+		cdns->wakeup_int = true;
+		pm_runtime_get(cdns->dev);
+		return IRQ_HANDLED;
+	}
+
 	if (cdns->dr_mode != USB_DR_MODE_OTG)
 		return ret;
 
diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c
index a6723f46d509..2cee12195978 100644
--- a/drivers/usb/cdns3/gadget.c
+++ b/drivers/usb/cdns3/gadget.c
@@ -1766,9 +1766,13 @@  static void cdns3_check_usb_interrupt_proceed(struct cdns3_device *priv_dev,
 static irqreturn_t cdns3_device_irq_handler(int irq, void *data)
 {
 	struct cdns3_device *priv_dev = data;
+	struct cdns3 *cdns = dev_get_drvdata(priv_dev->dev);
 	irqreturn_t ret = IRQ_NONE;
 	u32 reg;
 
+	if (cdns->in_lpm)
+		return ret;
+
 	/* check USB device interrupt */
 	reg = readl(&priv_dev->regs->usb_ists);
 	if (reg) {
diff --git a/drivers/usb/cdns3/host.c b/drivers/usb/cdns3/host.c
index ad788bf3fe4f..b579ef15f4e0 100644
--- a/drivers/usb/cdns3/host.c
+++ b/drivers/usb/cdns3/host.c
@@ -13,11 +13,13 @@ 
 #include "core.h"
 #include "drd.h"
 #include "host-export.h"
+#include <linux/usb/hcd.h>
 
 static int __cdns3_host_init(struct cdns3 *cdns)
 {
 	struct platform_device *xhci;
 	int ret;
+	struct usb_hcd *hcd;
 
 	cdns3_drd_switch_host(cdns, 1);
 
@@ -43,6 +45,11 @@  static int __cdns3_host_init(struct cdns3 *cdns)
 		goto err1;
 	}
 
+	/* Glue needs to access xHCI region register for Power management */
+	hcd = platform_get_drvdata(xhci);
+	if (hcd)
+		cdns->xhci_regs = hcd->regs;
+
 	return 0;
 err1:
 	platform_device_put(xhci);