linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/8] usb: cdns3: add runtime PM support
@ 2020-05-22 10:32 Peter Chen
  2020-05-22 10:32 ` [PATCH 1/8] " Peter Chen
                   ` (7 more replies)
  0 siblings, 8 replies; 15+ messages in thread
From: Peter Chen @ 2020-05-22 10:32 UTC (permalink / raw)
  To: balbi, mathias.nyman
  Cc: linux-usb, linux-imx, pawell, rogerq, gregkh, jun.li, Peter Chen

Hi Felipe & Mathias,

In this series, it adds cdns3 runtime PM support and verified by
NXP i.MX8QM and i.MX8QXP platforms.

Patch 1-2: add main runtime PM support for cdns3 core and imx glue layer
Patch 3-8: add two quirks for xhci-plat.c.
- .suspend_quirk for platform special sequence between xhci_bus_suspend
and xhci_suspend.
- skip_phy_initialization for skip PHY management from HCD core.

Peter Chen (8):
  usb: cdns3: add runtime PM support
  usb: cdns3: imx: add glue layer runtime pm implementation
  usb: host: xhci-plat: add platform data support
  usb: host: xhci-plat: add .suspend_quirk for struct xhci_plat_priv
  usb: host: xhci-plat: delete the unnecessary code
  usb: host: xhci-plat: add priv flag for skip_phy_initialization
  usb: cdns3: host: add .suspend_quirk for xhci-plat.c
  usb: cdns3: host: add xhci_plat_priv's flag skip_phy_initialization

 drivers/usb/cdns3/cdns3-imx.c   | 179 +++++++++++++++++++++++++++++++-
 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-export.h |   6 ++
 drivers/usb/cdns3/host.c        |  48 +++++++++
 drivers/usb/host/xhci-plat.c    |  36 ++++++-
 drivers/usb/host/xhci-plat.h    |   2 +
 9 files changed, 391 insertions(+), 24 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/8] usb: cdns3: add runtime PM support
  2020-05-22 10:32 [PATCH 0/8] usb: cdns3: add runtime PM support Peter Chen
@ 2020-05-22 10:32 ` Peter Chen
  2020-08-31 10:01   ` Felipe Balbi
  2020-05-22 10:32 ` [PATCH 2/8] usb: cdns3: imx: add glue layer runtime pm implementation Peter Chen
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 15+ messages in thread
From: Peter Chen @ 2020-05-22 10:32 UTC (permalink / raw)
  To: balbi, mathias.nyman
  Cc: linux-usb, linux-imx, pawell, rogerq, gregkh, jun.li, Peter Chen

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(-)

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);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/8] usb: cdns3: imx: add glue layer runtime pm implementation
  2020-05-22 10:32 [PATCH 0/8] usb: cdns3: add runtime PM support Peter Chen
  2020-05-22 10:32 ` [PATCH 1/8] " Peter Chen
@ 2020-05-22 10:32 ` Peter Chen
  2020-08-31 10:04   ` Felipe Balbi
  2020-05-22 10:32 ` [PATCH 3/8] usb: host: xhci-plat: add platform data support Peter Chen
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 15+ messages in thread
From: Peter Chen @ 2020-05-22 10:32 UTC (permalink / raw)
  To: balbi, mathias.nyman
  Cc: linux-usb, linux-imx, pawell, rogerq, gregkh, jun.li, Peter Chen

Add imx glue layer runtime pm implementation, and the runtime
pm is default off.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 drivers/usb/cdns3/cdns3-imx.c | 179 +++++++++++++++++++++++++++++++++-
 1 file changed, 177 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/cdns3/cdns3-imx.c b/drivers/usb/cdns3/cdns3-imx.c
index aba988e71958..9d247de8e7eb 100644
--- a/drivers/usb/cdns3/cdns3-imx.c
+++ b/drivers/usb/cdns3/cdns3-imx.c
@@ -15,6 +15,8 @@
 #include <linux/io.h>
 #include <linux/of_platform.h>
 #include <linux/iopoll.h>
+#include <linux/pm_runtime.h>
+#include "core.h"
 
 #define USB3_CORE_CTRL1    0x00
 #define USB3_CORE_CTRL2    0x04
@@ -66,11 +68,30 @@
 #define CLK_VALID_COMPARE_BITS	(0xf << 28)
 #define PHY_REFCLK_REQ		(1 << 0)
 
+/* OTG registers definition */
+#define OTGSTS		0x4
+/* OTGSTS */
+#define OTG_NRDY	(1 << 11)
+
+/* xHCI registers definition  */
+#define XECP_PM_PMCSR		0x8018
+#define XECP_AUX_CTRL_REG1	0x8120
+
+/* Register bits definition */
+/* XECP_AUX_CTRL_REG1 */
+#define CFG_RXDET_P3_EN		(1 << 15)
+
+/* XECP_PM_PMCSR */
+#define PS_MASK			(3 << 0)
+#define PS_D0			0
+#define PS_D1			(1 << 0)
+
 struct cdns_imx {
 	struct device *dev;
 	void __iomem *noncore;
 	struct clk_bulk_data *clks;
 	int num_clks;
+	struct platform_device *cdns3_pdev;
 };
 
 static inline u32 cdns_imx_readl(struct cdns_imx *data, u32 offset)
@@ -126,6 +147,20 @@ static int cdns_imx_noncore_init(struct cdns_imx *data)
 	return ret;
 }
 
+static int cdns_imx_platform_suspend(struct device *dev,
+	bool suspend, bool wakeup);
+static struct cdns3_platform_data cdns_imx_pdata = {
+	.platform_suspend = cdns_imx_platform_suspend,
+};
+
+static struct of_dev_auxdata cdns_imx_auxdata[] = {
+	{
+	.compatible = "cdns,usb3",
+	.platform_data = &cdns_imx_pdata,
+	},
+	{},
+};
+
 static int cdns_imx_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -162,14 +197,18 @@ static int cdns_imx_probe(struct platform_device *pdev)
 	if (ret)
 		goto err;
 
-	ret = of_platform_populate(node, NULL, NULL, dev);
+	ret = of_platform_populate(node, NULL, cdns_imx_auxdata, dev);
 	if (ret) {
 		dev_err(dev, "failed to create children: %d\n", ret);
 		goto err;
 	}
 
-	return ret;
+	device_set_wakeup_capable(dev, true);
+	pm_runtime_set_active(dev);
+	pm_runtime_enable(dev);
+	pm_runtime_forbid(dev);
 
+	return ret;
 err:
 	clk_bulk_disable_unprepare(data->num_clks, data->clks);
 	return ret;
@@ -194,6 +233,141 @@ static int cdns_imx_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_PM
+static void cdns3_set_wakeup(struct cdns_imx *data, bool enable)
+{
+	u32 value;
+
+	value = cdns_imx_readl(data, USB3_INT_REG);
+	if (enable)
+		value |= OTG_WAKEUP_EN | DEVU3_WAEKUP_EN;
+	else
+		value &= ~(OTG_WAKEUP_EN | DEVU3_WAEKUP_EN);
+
+	cdns_imx_writel(data, USB3_INT_REG, value);
+}
+
+static int cdns_imx_platform_suspend(struct device *dev,
+		bool suspend, bool wakeup)
+{
+	struct cdns3 *cdns = dev_get_drvdata(dev);
+	struct device *parent = dev->parent;
+	struct cdns_imx *data = dev_get_drvdata(parent);
+	void __iomem *otg_regs = cdns->otg_regs;
+	void __iomem *xhci_regs = cdns->xhci_regs;
+	u32 value;
+	int ret = 0;
+
+	if (cdns->role != USB_ROLE_HOST)
+		return 0;
+
+	if (suspend) {
+		/* SW request low power when all usb ports allow to it ??? */
+		value = readl(xhci_regs + XECP_PM_PMCSR);
+		value &= ~PS_MASK;
+		value |= PS_D1;
+		writel(value, xhci_regs + XECP_PM_PMCSR);
+
+		/* mdctrl_clk_sel */
+		value = cdns_imx_readl(data, USB3_CORE_CTRL1);
+		value |= MDCTRL_CLK_SEL;
+		cdns_imx_writel(data, USB3_CORE_CTRL1, value);
+
+		/* wait for mdctrl_clk_status */
+		value = cdns_imx_readl(data, USB3_CORE_STATUS);
+		ret = readl_poll_timeout_atomic(data->noncore + USB3_CORE_STATUS, value,
+			(value & MDCTRL_CLK_STATUS) == MDCTRL_CLK_STATUS,
+			10, 100000);
+		if (ret)
+			dev_warn(parent, "wait mdctrl_clk_status timeout\n");
+
+		/* wait lpm_clk_req to be 0 */
+		value = cdns_imx_readl(data, USB3_INT_REG);
+		ret = readl_poll_timeout_atomic(data->noncore + USB3_INT_REG, value,
+			(value & LPM_CLK_REQ) != LPM_CLK_REQ,
+			10, 100000);
+		if (ret)
+			dev_warn(parent, "wait lpm_clk_req timeout\n");
+
+		/* wait phy_refclk_req to be 0 */
+		value = cdns_imx_readl(data, USB3_SSPHY_STATUS);
+		ret = readl_poll_timeout_atomic(data->noncore + USB3_SSPHY_STATUS, value,
+			(value & PHY_REFCLK_REQ) != PHY_REFCLK_REQ,
+			10, 100000);
+		if (ret)
+			dev_warn(parent, "wait phy_refclk_req timeout\n");
+
+		cdns3_set_wakeup(data, wakeup);
+	} else {
+		/* wait CLK_125_REQ to be 1 */
+		value = cdns_imx_readl(data, USB3_INT_REG);
+		ret = readl_poll_timeout_atomic(data->noncore + USB3_INT_REG, value,
+			(value & CLK_125_REQ) != CLK_125_REQ,
+			10, 100000);
+		if (ret)
+			dev_warn(parent, "wait CLK_125_REQ timeout\n");
+
+		cdns3_set_wakeup(data, false);
+
+		/* SW request D0 */
+		value = readl(xhci_regs + XECP_PM_PMCSR);
+		value &= ~PS_MASK;
+		value |= PS_D0;
+		writel(value, xhci_regs + XECP_PM_PMCSR);
+
+		/* clr CFG_RXDET_P3_EN */
+		value = readl(xhci_regs + XECP_AUX_CTRL_REG1);
+		value &= ~CFG_RXDET_P3_EN;
+		writel(value, xhci_regs + XECP_AUX_CTRL_REG1);
+
+		/* clear mdctrl_clk_sel */
+		value = cdns_imx_readl(data, USB3_CORE_CTRL1);
+		value &= ~MDCTRL_CLK_SEL;
+		cdns_imx_writel(data, USB3_CORE_CTRL1, value);
+
+		/* wait for mdctrl_clk_status is cleared */
+		value = cdns_imx_readl(data, USB3_CORE_STATUS);
+		ret = readl_poll_timeout_atomic(data->noncore + USB3_CORE_STATUS, value,
+			(value & MDCTRL_CLK_STATUS) != MDCTRL_CLK_STATUS,
+			10, 100000);
+		if (ret)
+			dev_warn(parent, "wait mdctrl_clk_status cleared timeout\n");
+
+		/* Wait until OTG_NRDY is 0 */
+		value = readl(otg_regs + OTGSTS);
+		ret = readl_poll_timeout_atomic(otg_regs + OTGSTS, value,
+			(value & OTG_NRDY) != OTG_NRDY,
+			10, 100000);
+		if (ret)
+			dev_warn(parent, "wait OTG ready timeout\n");
+	}
+
+	return ret;
+
+}
+
+static int cdns_imx_resume(struct device *dev)
+{
+	struct cdns_imx *data = dev_get_drvdata(dev);
+
+	return clk_bulk_prepare_enable(data->num_clks, data->clks);
+}
+
+static int cdns_imx_suspend(struct device *dev)
+{
+	struct cdns_imx *data = dev_get_drvdata(dev);
+
+	clk_bulk_disable_unprepare(data->num_clks, data->clks);
+
+	return 0;
+}
+
+#endif /* CONFIG_PM */
+
+static const struct dev_pm_ops cdns_imx_pm_ops = {
+	SET_RUNTIME_PM_OPS(cdns_imx_suspend, cdns_imx_resume, NULL)
+};
+
 static const struct of_device_id cdns_imx_of_match[] = {
 	{ .compatible = "fsl,imx8qm-usb3", },
 	{},
@@ -206,6 +380,7 @@ static struct platform_driver cdns_imx_driver = {
 	.driver		= {
 		.name	= "cdns3-imx",
 		.of_match_table	= cdns_imx_of_match,
+		.pm	= &cdns_imx_pm_ops,
 	},
 };
 module_platform_driver(cdns_imx_driver);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 3/8] usb: host: xhci-plat: add platform data support
  2020-05-22 10:32 [PATCH 0/8] usb: cdns3: add runtime PM support Peter Chen
  2020-05-22 10:32 ` [PATCH 1/8] " Peter Chen
  2020-05-22 10:32 ` [PATCH 2/8] usb: cdns3: imx: add glue layer runtime pm implementation Peter Chen
@ 2020-05-22 10:32 ` Peter Chen
  2020-05-22 10:32 ` [PATCH 4/8] usb: host: xhci-plat: add .suspend_quirk for struct xhci_plat_priv Peter Chen
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: Peter Chen @ 2020-05-22 10:32 UTC (permalink / raw)
  To: balbi, mathias.nyman
  Cc: linux-usb, linux-imx, pawell, rogerq, gregkh, jun.li, Peter Chen

Some xhci hosts (eg dwc3 and cdns3) do not use OF to create
platform device, they create xhci-plat platform device runtime.
And these platforms may also have quirks, and the quirks could
be supplied by their parent device through platform data.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 drivers/usb/host/xhci-plat.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 0b4cdc5af130..e00efd84ea1a 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -255,7 +255,11 @@ static int xhci_plat_probe(struct platform_device *pdev)
 	if (ret)
 		goto disable_reg_clk;
 
-	priv_match = of_device_get_match_data(&pdev->dev);
+	if (pdev->dev.of_node)
+		priv_match = of_device_get_match_data(&pdev->dev);
+	else
+		priv_match = dev_get_platdata(&pdev->dev);
+
 	if (priv_match) {
 		struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 4/8] usb: host: xhci-plat: add .suspend_quirk for struct xhci_plat_priv
  2020-05-22 10:32 [PATCH 0/8] usb: cdns3: add runtime PM support Peter Chen
                   ` (2 preceding siblings ...)
  2020-05-22 10:32 ` [PATCH 3/8] usb: host: xhci-plat: add platform data support Peter Chen
@ 2020-05-22 10:32 ` Peter Chen
  2020-08-31 10:06   ` Felipe Balbi
  2020-05-22 10:32 ` [PATCH 5/8] usb: host: xhci-plat: delete the unnecessary code Peter Chen
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 15+ messages in thread
From: Peter Chen @ 2020-05-22 10:32 UTC (permalink / raw)
  To: balbi, mathias.nyman
  Cc: linux-usb, linux-imx, pawell, rogerq, gregkh, jun.li, Peter Chen

Some platforms (eg cdns3) may have special sequences between
xhci_bus_suspend and xhci_suspend, add .suspend_quick for it.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 drivers/usb/host/xhci-plat.c | 19 +++++++++++++++++++
 drivers/usb/host/xhci-plat.h |  1 +
 2 files changed, 20 insertions(+)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index e00efd84ea1a..ca02e7f36238 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -54,6 +54,16 @@ static int xhci_priv_init_quirk(struct usb_hcd *hcd)
 	return priv->init_quirk(hcd);
 }
 
+static int xhci_priv_suspend_quirk(struct usb_hcd *hcd)
+{
+	struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);
+
+	if (!priv->suspend_quirk)
+		return 0;
+
+	return priv->suspend_quirk(hcd);
+}
+
 static int xhci_priv_resume_quirk(struct usb_hcd *hcd)
 {
 	struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);
@@ -396,7 +406,11 @@ static int __maybe_unused xhci_plat_suspend(struct device *dev)
 {
 	struct usb_hcd	*hcd = dev_get_drvdata(dev);
 	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
+	int ret;
 
+	ret = xhci_priv_suspend_quirk(hcd);
+	if (ret)
+		return ret;
 	/*
 	 * xhci_suspend() needs `do_wakeup` to know whether host is allowed
 	 * to do wakeup during suspend. Since xhci_plat_suspend is currently
@@ -425,6 +439,11 @@ static int __maybe_unused xhci_plat_runtime_suspend(struct device *dev)
 {
 	struct usb_hcd  *hcd = dev_get_drvdata(dev);
 	struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+	int ret;
+
+	ret = xhci_priv_suspend_quirk(hcd);
+	if (ret)
+		return ret;
 
 	return xhci_suspend(xhci, true);
 }
diff --git a/drivers/usb/host/xhci-plat.h b/drivers/usb/host/xhci-plat.h
index 5681723fc9cd..9338a4f946d0 100644
--- a/drivers/usb/host/xhci-plat.h
+++ b/drivers/usb/host/xhci-plat.h
@@ -15,6 +15,7 @@ struct xhci_plat_priv {
 	unsigned long long quirks;
 	void (*plat_start)(struct usb_hcd *);
 	int (*init_quirk)(struct usb_hcd *);
+	int (*suspend_quirk)(struct usb_hcd *);
 	int (*resume_quirk)(struct usb_hcd *);
 };
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 5/8] usb: host: xhci-plat: delete the unnecessary code
  2020-05-22 10:32 [PATCH 0/8] usb: cdns3: add runtime PM support Peter Chen
                   ` (3 preceding siblings ...)
  2020-05-22 10:32 ` [PATCH 4/8] usb: host: xhci-plat: add .suspend_quirk for struct xhci_plat_priv Peter Chen
@ 2020-05-22 10:32 ` Peter Chen
  2020-05-22 10:32 ` [PATCH 6/8] usb: host: xhci-plat: add priv flag for skip_phy_initialization Peter Chen
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: Peter Chen @ 2020-05-22 10:32 UTC (permalink / raw)
  To: balbi, mathias.nyman
  Cc: linux-usb, linux-imx, pawell, rogerq, gregkh, jun.li, Peter Chen

The if {} condition is duplicated with outer if {} condition.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 drivers/usb/host/xhci-plat.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index ca02e7f36238..a45c7ec33ed6 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -274,8 +274,7 @@ static int xhci_plat_probe(struct platform_device *pdev)
 		struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);
 
 		/* Just copy data for now */
-		if (priv_match)
-			*priv = *priv_match;
+		*priv = *priv_match;
 	}
 
 	device_wakeup_enable(hcd->self.controller);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 6/8] usb: host: xhci-plat: add priv flag for skip_phy_initialization
  2020-05-22 10:32 [PATCH 0/8] usb: cdns3: add runtime PM support Peter Chen
                   ` (4 preceding siblings ...)
  2020-05-22 10:32 ` [PATCH 5/8] usb: host: xhci-plat: delete the unnecessary code Peter Chen
@ 2020-05-22 10:32 ` Peter Chen
  2020-05-22 10:32 ` [PATCH 7/8] usb: cdns3: host: add .suspend_quirk for xhci-plat.c Peter Chen
  2020-05-22 10:32 ` [PATCH 8/8] usb: cdns3: host: add xhci_plat_priv's flag skip_phy_initialization Peter Chen
  7 siblings, 0 replies; 15+ messages in thread
From: Peter Chen @ 2020-05-22 10:32 UTC (permalink / raw)
  To: balbi, mathias.nyman
  Cc: linux-usb, linux-imx, pawell, rogerq, gregkh, jun.li, Peter Chen

Some DRD controllers (eg, dwc3 & cdns3) have PHY management at
their own driver to cover both device and host mode, so add one
priv flag for such users to skip PHY management from HCD core.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 drivers/usb/host/xhci-plat.c | 8 ++++++--
 drivers/usb/host/xhci-plat.h | 1 +
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index a45c7ec33ed6..f653b717c098 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -173,6 +173,8 @@ static int xhci_plat_probe(struct platform_device *pdev)
 	struct usb_hcd		*hcd;
 	int			ret;
 	int			irq;
+	struct xhci_plat_priv	*priv = NULL;
+
 
 	if (usb_disabled())
 		return -ENODEV;
@@ -271,8 +273,7 @@ static int xhci_plat_probe(struct platform_device *pdev)
 		priv_match = dev_get_platdata(&pdev->dev);
 
 	if (priv_match) {
-		struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);
-
+		priv = hcd_to_xhci_priv(hcd);
 		/* Just copy data for now */
 		*priv = *priv_match;
 	}
@@ -324,6 +325,9 @@ static int xhci_plat_probe(struct platform_device *pdev)
 
 	hcd->tpl_support = of_usb_host_tpl_support(sysdev->of_node);
 	xhci->shared_hcd->tpl_support = hcd->tpl_support;
+	if (priv && priv->skip_phy_initialization)
+		hcd->skip_phy_initialization = 1;
+
 	ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
 	if (ret)
 		goto disable_usb_phy;
diff --git a/drivers/usb/host/xhci-plat.h b/drivers/usb/host/xhci-plat.h
index 9338a4f946d0..d78e86e14f59 100644
--- a/drivers/usb/host/xhci-plat.h
+++ b/drivers/usb/host/xhci-plat.h
@@ -13,6 +13,7 @@
 struct xhci_plat_priv {
 	const char *firmware_name;
 	unsigned long long quirks;
+	unsigned int skip_phy_initialization:1;
 	void (*plat_start)(struct usb_hcd *);
 	int (*init_quirk)(struct usb_hcd *);
 	int (*suspend_quirk)(struct usb_hcd *);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 7/8] usb: cdns3: host: add .suspend_quirk for xhci-plat.c
  2020-05-22 10:32 [PATCH 0/8] usb: cdns3: add runtime PM support Peter Chen
                   ` (5 preceding siblings ...)
  2020-05-22 10:32 ` [PATCH 6/8] usb: host: xhci-plat: add priv flag for skip_phy_initialization Peter Chen
@ 2020-05-22 10:32 ` Peter Chen
  2020-05-22 10:32 ` [PATCH 8/8] usb: cdns3: host: add xhci_plat_priv's flag skip_phy_initialization Peter Chen
  7 siblings, 0 replies; 15+ messages in thread
From: Peter Chen @ 2020-05-22 10:32 UTC (permalink / raw)
  To: balbi, mathias.nyman
  Cc: linux-usb, linux-imx, pawell, rogerq, gregkh, jun.li, Peter Chen

cdns3 has some special PM sequence between xhci_bus_suspend and
xhci_suspend, add quirk to implement it.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 drivers/usb/cdns3/host-export.h |  6 +++++
 drivers/usb/cdns3/host.c        | 40 +++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+)

diff --git a/drivers/usb/cdns3/host-export.h b/drivers/usb/cdns3/host-export.h
index ae11810f8826..26041718a086 100644
--- a/drivers/usb/cdns3/host-export.h
+++ b/drivers/usb/cdns3/host-export.h
@@ -9,9 +9,11 @@
 #ifndef __LINUX_CDNS3_HOST_EXPORT
 #define __LINUX_CDNS3_HOST_EXPORT
 
+struct usb_hcd;
 #ifdef CONFIG_USB_CDNS3_HOST
 
 int cdns3_host_init(struct cdns3 *cdns);
+int xhci_cdns3_suspend_quirk(struct usb_hcd *hcd);
 
 #else
 
@@ -21,6 +23,10 @@ static inline int cdns3_host_init(struct cdns3 *cdns)
 }
 
 static inline void cdns3_host_exit(struct cdns3 *cdns) { }
+static inline int xhci_cdns3_suspend_quirk(struct usb_hcd *hcd)
+{
+	return 0;
+}
 
 #endif /* CONFIG_USB_CDNS3_HOST */
 
diff --git a/drivers/usb/cdns3/host.c b/drivers/usb/cdns3/host.c
index b579ef15f4e0..030d6421abd3 100644
--- a/drivers/usb/cdns3/host.c
+++ b/drivers/usb/cdns3/host.c
@@ -14,6 +14,18 @@
 #include "drd.h"
 #include "host-export.h"
 #include <linux/usb/hcd.h>
+#include "../host/xhci.h"
+#include "../host/xhci-plat.h"
+
+#define XECP_PORT_CAP_REG	0x8000
+#define XECP_AUX_CTRL_REG1	0x8120
+
+#define CFG_RXDET_P3_EN		(1 << 15)
+#define LPM_2_STB_SWITCH_EN	(1 << 25)
+
+static const struct xhci_plat_priv xhci_plat_cdns3_xhci = {
+	.suspend_quirk = xhci_cdns3_suspend_quirk,
+};
 
 static int __cdns3_host_init(struct cdns3 *cdns)
 {
@@ -39,6 +51,11 @@ static int __cdns3_host_init(struct cdns3 *cdns)
 		goto err1;
 	}
 
+	ret = platform_device_add_data(xhci, &xhci_plat_cdns3_xhci,
+			sizeof(struct xhci_plat_priv));
+	if (ret)
+		goto err1;
+
 	ret = platform_device_add(xhci);
 	if (ret) {
 		dev_err(cdns->dev, "failed to register xHCI device\n");
@@ -56,6 +73,29 @@ static int __cdns3_host_init(struct cdns3 *cdns)
 	return ret;
 }
 
+int xhci_cdns3_suspend_quirk(struct usb_hcd *hcd)
+{
+	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
+	u32 value;
+
+	/* set usbcmd.EU3S */
+	value = readl(&xhci->op_regs->command);
+	value |= CMD_PM_INDEX;
+	writel(value, &xhci->op_regs->command);
+
+	if (hcd->regs) {
+		value = readl(hcd->regs + XECP_AUX_CTRL_REG1);
+		value |= CFG_RXDET_P3_EN;
+		writel(value, hcd->regs + XECP_AUX_CTRL_REG1);
+
+		value = readl(hcd->regs + XECP_PORT_CAP_REG);
+		value |= LPM_2_STB_SWITCH_EN;
+		writel(value, hcd->regs + XECP_PORT_CAP_REG);
+	}
+
+	return 0;
+}
+
 static void cdns3_host_exit(struct cdns3 *cdns)
 {
 	platform_device_unregister(cdns->host_dev);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 8/8] usb: cdns3: host: add xhci_plat_priv's flag skip_phy_initialization
  2020-05-22 10:32 [PATCH 0/8] usb: cdns3: add runtime PM support Peter Chen
                   ` (6 preceding siblings ...)
  2020-05-22 10:32 ` [PATCH 7/8] usb: cdns3: host: add .suspend_quirk for xhci-plat.c Peter Chen
@ 2020-05-22 10:32 ` Peter Chen
  7 siblings, 0 replies; 15+ messages in thread
From: Peter Chen @ 2020-05-22 10:32 UTC (permalink / raw)
  To: balbi, mathias.nyman
  Cc: linux-usb, linux-imx, pawell, rogerq, gregkh, jun.li, Peter Chen

cdns3 manages PHY by own DRD driver, so skip the management by
HCD core.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 drivers/usb/cdns3/host.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/cdns3/host.c b/drivers/usb/cdns3/host.c
index 030d6421abd3..d681c3db25cd 100644
--- a/drivers/usb/cdns3/host.c
+++ b/drivers/usb/cdns3/host.c
@@ -24,6 +24,7 @@
 #define LPM_2_STB_SWITCH_EN	(1 << 25)
 
 static const struct xhci_plat_priv xhci_plat_cdns3_xhci = {
+	.skip_phy_initialization = 1,
 	.suspend_quirk = xhci_cdns3_suspend_quirk,
 };
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/8] usb: cdns3: add runtime PM support
  2020-05-22 10:32 ` [PATCH 1/8] " Peter Chen
@ 2020-08-31 10:01   ` Felipe Balbi
  2020-08-31 10:59     ` Peter Chen
  0 siblings, 1 reply; 15+ messages in thread
From: Felipe Balbi @ 2020-08-31 10:01 UTC (permalink / raw)
  To: Peter Chen, mathias.nyman
  Cc: linux-usb, linux-imx, pawell, rogerq, gregkh, jun.li, Peter Chen

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


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?

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 857 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/8] usb: cdns3: imx: add glue layer runtime pm implementation
  2020-05-22 10:32 ` [PATCH 2/8] usb: cdns3: imx: add glue layer runtime pm implementation Peter Chen
@ 2020-08-31 10:04   ` Felipe Balbi
  0 siblings, 0 replies; 15+ messages in thread
From: Felipe Balbi @ 2020-08-31 10:04 UTC (permalink / raw)
  To: Peter Chen, mathias.nyman
  Cc: linux-usb, linux-imx, pawell, rogerq, gregkh, jun.li, Peter Chen

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

Peter Chen <peter.chen@nxp.com> writes:

> Add imx glue layer runtime pm implementation, and the runtime
> pm is default off.
>
> Signed-off-by: Peter Chen <peter.chen@nxp.com>
> ---
>  drivers/usb/cdns3/cdns3-imx.c | 179 +++++++++++++++++++++++++++++++++-
>  1 file changed, 177 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/cdns3/cdns3-imx.c b/drivers/usb/cdns3/cdns3-imx.c
> index aba988e71958..9d247de8e7eb 100644
> --- a/drivers/usb/cdns3/cdns3-imx.c
> +++ b/drivers/usb/cdns3/cdns3-imx.c
> @@ -15,6 +15,8 @@
>  #include <linux/io.h>
>  #include <linux/of_platform.h>
>  #include <linux/iopoll.h>
> +#include <linux/pm_runtime.h>
> +#include "core.h"
>  
>  #define USB3_CORE_CTRL1    0x00
>  #define USB3_CORE_CTRL2    0x04
> @@ -66,11 +68,30 @@
>  #define CLK_VALID_COMPARE_BITS	(0xf << 28)
>  #define PHY_REFCLK_REQ		(1 << 0)
>  
> +/* OTG registers definition */
> +#define OTGSTS		0x4

looks like a blank line here would (mildly) aid readability.

> +/* OTGSTS */
> +#define OTG_NRDY	(1 << 11)
> +
> +/* xHCI registers definition  */
> +#define XECP_PM_PMCSR		0x8018
> +#define XECP_AUX_CTRL_REG1	0x8120
> +
> +/* Register bits definition */
> +/* XECP_AUX_CTRL_REG1 */
> +#define CFG_RXDET_P3_EN		(1 << 15)
> +
> +/* XECP_PM_PMCSR */
> +#define PS_MASK			(3 << 0)
> +#define PS_D0			0
> +#define PS_D1			(1 << 0)
> +
>  struct cdns_imx {
>  	struct device *dev;
>  	void __iomem *noncore;
>  	struct clk_bulk_data *clks;
>  	int num_clks;
> +	struct platform_device *cdns3_pdev;

do you really need the entire platform_device? Why don't you use
container_of to get the platform_device from the device pointer? We
already have a generic to_platform_device(), right?

Or are those referring to different devices?

Also, it seems like that pointer isn't used, at least not in $subject

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 857 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 4/8] usb: host: xhci-plat: add .suspend_quirk for struct xhci_plat_priv
  2020-05-22 10:32 ` [PATCH 4/8] usb: host: xhci-plat: add .suspend_quirk for struct xhci_plat_priv Peter Chen
@ 2020-08-31 10:06   ` Felipe Balbi
  2020-08-31 11:00     ` Peter Chen
  0 siblings, 1 reply; 15+ messages in thread
From: Felipe Balbi @ 2020-08-31 10:06 UTC (permalink / raw)
  To: Peter Chen, mathias.nyman
  Cc: linux-usb, linux-imx, pawell, rogerq, gregkh, jun.li, Peter Chen

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

Peter Chen <peter.chen@nxp.com> writes:

> Some platforms (eg cdns3) may have special sequences between
> xhci_bus_suspend and xhci_suspend, add .suspend_quick for it.
                                                  ^^^^^
                                                  quirk?

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 857 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [PATCH 1/8] usb: cdns3: add runtime PM support
  2020-08-31 10:01   ` Felipe Balbi
@ 2020-08-31 10:59     ` Peter Chen
  2020-08-31 11:27       ` Felipe Balbi
  0 siblings, 1 reply; 15+ messages in thread
From: Peter Chen @ 2020-08-31 10:59 UTC (permalink / raw)
  To: Felipe Balbi, mathias.nyman
  Cc: linux-usb, dl-linux-imx, pawell, rogerq, gregkh, Jun Li

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [PATCH 4/8] usb: host: xhci-plat: add .suspend_quirk for struct xhci_plat_priv
  2020-08-31 10:06   ` Felipe Balbi
@ 2020-08-31 11:00     ` Peter Chen
  0 siblings, 0 replies; 15+ messages in thread
From: Peter Chen @ 2020-08-31 11:00 UTC (permalink / raw)
  To: Felipe Balbi, mathias.nyman
  Cc: linux-usb, dl-linux-imx, pawell, rogerq, gregkh, Jun Li

 
> Subject: Re: [PATCH 4/8] usb: host: xhci-plat: add .suspend_quirk for struct
> xhci_plat_priv
> 
> Peter Chen <peter.chen@nxp.com> writes:
> 
> > Some platforms (eg cdns3) may have special sequences between
> > xhci_bus_suspend and xhci_suspend, add .suspend_quick for it.
>                                                   ^^^^^
>                                                   quirk?
> 

Thanks, will change.

Peter

^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [PATCH 1/8] usb: cdns3: add runtime PM support
  2020-08-31 10:59     ` Peter Chen
@ 2020-08-31 11:27       ` Felipe Balbi
  0 siblings, 0 replies; 15+ messages in thread
From: Felipe Balbi @ 2020-08-31 11:27 UTC (permalink / raw)
  To: Peter Chen, mathias.nyman
  Cc: linux-usb, dl-linux-imx, pawell, rogerq, gregkh, Jun Li

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


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

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 857 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2020-08-31 11:28 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-22 10:32 [PATCH 0/8] usb: cdns3: add runtime PM support Peter Chen
2020-05-22 10:32 ` [PATCH 1/8] " Peter Chen
2020-08-31 10:01   ` Felipe Balbi
2020-08-31 10:59     ` Peter Chen
2020-08-31 11:27       ` Felipe Balbi
2020-05-22 10:32 ` [PATCH 2/8] usb: cdns3: imx: add glue layer runtime pm implementation Peter Chen
2020-08-31 10:04   ` Felipe Balbi
2020-05-22 10:32 ` [PATCH 3/8] usb: host: xhci-plat: add platform data support Peter Chen
2020-05-22 10:32 ` [PATCH 4/8] usb: host: xhci-plat: add .suspend_quirk for struct xhci_plat_priv Peter Chen
2020-08-31 10:06   ` Felipe Balbi
2020-08-31 11:00     ` Peter Chen
2020-05-22 10:32 ` [PATCH 5/8] usb: host: xhci-plat: delete the unnecessary code Peter Chen
2020-05-22 10:32 ` [PATCH 6/8] usb: host: xhci-plat: add priv flag for skip_phy_initialization Peter Chen
2020-05-22 10:32 ` [PATCH 7/8] usb: cdns3: host: add .suspend_quirk for xhci-plat.c Peter Chen
2020-05-22 10:32 ` [PATCH 8/8] usb: cdns3: host: add xhci_plat_priv's flag skip_phy_initialization Peter Chen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).