* [Patch v2 01/10] usb: chipidea: Add power management support
2013-10-22 6:23 [Patch v2 00/10] Add power management support for chipidea Peter Chen
@ 2013-10-22 6:23 ` Peter Chen
2013-10-22 6:23 ` [Patch v2 02/10] usb: chipidea: imx: add " Peter Chen
` (8 subsequent siblings)
9 siblings, 0 replies; 21+ messages in thread
From: Peter Chen @ 2013-10-22 6:23 UTC (permalink / raw)
To: linux-arm-kernel
This commit adds runtime and system power management support for
chipidea core. The runtime pm support is controlled by glue
layer, it can be enabled by flag CI_HDRC_SUPPORTS_RUNTIME_PM.
Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
drivers/usb/chipidea/ci.h | 2 +
drivers/usb/chipidea/core.c | 119 ++++++++++++++++++++++++++++++++++++++++++
include/linux/usb/chipidea.h | 1 +
3 files changed, 122 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
index 1c94fc5..6ea1892 100644
--- a/drivers/usb/chipidea/ci.h
+++ b/drivers/usb/chipidea/ci.h
@@ -173,6 +173,8 @@ struct ci_hdrc {
struct dentry *debugfs;
bool id_event;
bool b_sess_valid_event;
+ bool supports_runtime_pm;
+ bool in_lpm;
};
static inline struct ci_role_driver *ci_role(struct ci_hdrc *ci)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 87546a0..a5b8f43 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -578,6 +578,10 @@ static int ci_hdrc_probe(struct platform_device *pdev)
hw_phymode_configure(ci);
dr_mode = ci->platdata->dr_mode;
+
+ ci->supports_runtime_pm = !!(ci->platdata->flags &
+ CI_HDRC_SUPPORTS_RUNTIME_PM);
+
/* initialize role(s) before the interrupt is requested */
if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_HOST) {
ret = ci_hdrc_host_init(ci);
@@ -651,6 +655,13 @@ static int ci_hdrc_probe(struct platform_device *pdev)
if (ret)
goto stop;
+ device_set_wakeup_capable(&pdev->dev, true);
+
+ if (ci->supports_runtime_pm) {
+ pm_runtime_set_active(&pdev->dev);
+ pm_runtime_enable(&pdev->dev);
+ }
+
ret = dbg_create_files(ci);
if (!ret)
return 0;
@@ -668,6 +679,11 @@ static int ci_hdrc_remove(struct platform_device *pdev)
{
struct ci_hdrc *ci = platform_get_drvdata(pdev);
+ if (ci->supports_runtime_pm) {
+ pm_runtime_get_sync(&pdev->dev);
+ pm_runtime_disable(&pdev->dev);
+ pm_runtime_put_noidle(&pdev->dev);
+ }
dbg_remove_files(ci);
free_irq(ci->irq, ci);
ci_role_destroy(ci);
@@ -678,11 +694,114 @@ static int ci_hdrc_remove(struct platform_device *pdev)
return 0;
}
+#ifdef CONFIG_PM
+static int ci_controller_suspend(struct device *dev)
+{
+ struct ci_hdrc *ci = dev_get_drvdata(dev);
+
+ dev_dbg(dev, "at %s\n", __func__);
+
+ if (ci->in_lpm)
+ return 0;
+
+ disable_irq(ci->irq);
+
+ if (ci->transceiver)
+ usb_phy_set_wakeup(ci->transceiver, true);
+
+ ci_hdrc_enter_lpm(ci, true);
+
+ if (ci->transceiver)
+ usb_phy_set_suspend(ci->transceiver, 1);
+
+ ci->in_lpm = true;
+
+ enable_irq(ci->irq);
+
+ return 0;
+}
+
+static int ci_controller_resume(struct device *dev)
+{
+ struct ci_hdrc *ci = dev_get_drvdata(dev);
+
+ dev_dbg(dev, "at %s\n", __func__);
+
+ if (!ci->in_lpm)
+ return 0;
+
+ ci_hdrc_enter_lpm(ci, false);
+
+ if (ci->transceiver) {
+ usb_phy_set_suspend(ci->transceiver, 0);
+ usb_phy_set_wakeup(ci->transceiver, false);
+ }
+
+ ci->in_lpm = false;
+
+ return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int ci_suspend(struct device *dev)
+{
+ struct ci_hdrc *ci = dev_get_drvdata(dev);
+ int ret;
+
+ ret = ci_controller_suspend(dev);
+ if (ret)
+ return ret;
+
+ if (device_may_wakeup(dev))
+ enable_irq_wake(ci->irq);
+
+ return ret;
+}
+
+static int ci_resume(struct device *dev)
+{
+ struct ci_hdrc *ci = dev_get_drvdata(dev);
+ int ret;
+
+ if (device_may_wakeup(dev))
+ disable_irq_wake(ci->irq);
+
+ ret = ci_controller_resume(dev);
+ if (!ret && ci->supports_runtime_pm) {
+ pm_runtime_disable(dev);
+ pm_runtime_set_active(dev);
+ pm_runtime_enable(dev);
+ }
+
+ return ret;
+}
+#endif /* CONFIG_PM_SLEEP */
+
+#ifdef CONFIG_PM_RUNTIME
+static int ci_runtime_suspend(struct device *dev)
+{
+ return ci_controller_suspend(dev);
+}
+
+static int ci_runtime_resume(struct device *dev)
+{
+ return ci_controller_resume(dev);
+}
+#endif /* CONFIG_PM_RUNTIME */
+
+#endif /* CONFIG_PM */
+static const struct dev_pm_ops ci_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(ci_suspend, ci_resume)
+ SET_RUNTIME_PM_OPS(ci_runtime_suspend,
+ ci_runtime_resume, NULL)
+};
+
static struct platform_driver ci_hdrc_driver = {
.probe = ci_hdrc_probe,
.remove = ci_hdrc_remove,
.driver = {
.name = "ci_hdrc",
+ .pm = &ci_pm_ops,
},
};
diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h
index 7d39967..667b46c 100644
--- a/include/linux/usb/chipidea.h
+++ b/include/linux/usb/chipidea.h
@@ -18,6 +18,7 @@ struct ci_hdrc_platform_data {
unsigned long flags;
#define CI_HDRC_REGS_SHARED BIT(0)
#define CI_HDRC_REQUIRE_TRANSCEIVER BIT(1)
+#define CI_HDRC_SUPPORTS_RUNTIME_PM BIT(2)
#define CI_HDRC_DISABLE_STREAMING BIT(3)
/*
* Only set it when DCCPARAMS.DC==1 and DCCPARAMS.HC==1,
--
1.7.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Patch v2 02/10] usb: chipidea: imx: add power management support
2013-10-22 6:23 [Patch v2 00/10] Add power management support for chipidea Peter Chen
2013-10-22 6:23 ` [Patch v2 01/10] usb: chipidea: Add power management support Peter Chen
@ 2013-10-22 6:23 ` Peter Chen
2013-10-22 6:23 ` [Patch v2 03/10] usb: chipidea: add wakeup interrupt handler Peter Chen
` (7 subsequent siblings)
9 siblings, 0 replies; 21+ messages in thread
From: Peter Chen @ 2013-10-22 6:23 UTC (permalink / raw)
To: linux-arm-kernel
Add system and runtime power management support for imx gluy layer.
Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
drivers/usb/chipidea/ci_hdrc_imx.c | 94 ++++++++++++++++++++++++++++++++++-
1 files changed, 91 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
index 023d3cb..a1e5634 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -28,6 +28,8 @@ struct ci_hdrc_imx_data {
struct platform_device *ci_pdev;
struct clk *clk;
struct imx_usbmisc_data *usbmisc_data;
+ bool supports_runtime_pm;
+ bool in_lpm;
};
/* Common functions shared by usbmisc drivers */
@@ -82,6 +84,7 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
CI_HDRC_DISABLE_STREAMING,
};
int ret;
+ struct device_node *np = pdev->dev.of_node;
data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
if (!data) {
@@ -151,8 +154,12 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, data);
- pm_runtime_no_callbacks(&pdev->dev);
- pm_runtime_enable(&pdev->dev);
+ device_set_wakeup_capable(&pdev->dev, true);
+
+ if (data->supports_runtime_pm) {
+ pm_runtime_set_active(&pdev->dev);
+ pm_runtime_enable(&pdev->dev);
+ }
return 0;
@@ -167,13 +174,93 @@ static int ci_hdrc_imx_remove(struct platform_device *pdev)
{
struct ci_hdrc_imx_data *data = platform_get_drvdata(pdev);
- pm_runtime_disable(&pdev->dev);
ci_hdrc_remove_device(data->ci_pdev);
+ if (data->supports_runtime_pm) {
+ pm_runtime_get_sync(&pdev->dev);
+ pm_runtime_disable(&pdev->dev);
+ pm_runtime_put_noidle(&pdev->dev);
+ }
clk_disable_unprepare(data->clk);
return 0;
}
+#ifdef CONFIG_PM
+static int imx_controller_suspend(struct device *dev)
+{
+ struct ci_hdrc_imx_data *data = dev_get_drvdata(dev);
+
+ dev_dbg(dev, "at %s\n", __func__);
+
+ if (data->in_lpm)
+ return 0;
+
+ clk_disable_unprepare(data->clk);
+
+ data->in_lpm = true;
+
+ return 0;
+}
+
+static int imx_controller_resume(struct device *dev)
+{
+ struct ci_hdrc_imx_data *data = dev_get_drvdata(dev);
+ int ret = 0;
+
+ dev_dbg(dev, "at %s\n", __func__);
+
+ if (!data->in_lpm)
+ return 0;
+
+ ret = clk_prepare_enable(data->clk);
+ if (ret)
+ return ret;
+
+ data->in_lpm = false;
+
+ return ret;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int ci_hdrc_imx_suspend(struct device *dev)
+{
+ return imx_controller_suspend(dev);
+}
+
+static int ci_hdrc_imx_resume(struct device *dev)
+{
+ struct ci_hdrc_imx_data *data = dev_get_drvdata(dev);
+ int ret;
+
+ ret = imx_controller_resume(dev);
+ if (!ret && data->supports_runtime_pm) {
+ pm_runtime_disable(dev);
+ pm_runtime_set_active(dev);
+ pm_runtime_enable(dev);
+ }
+
+ return ret;
+}
+#endif /* CONFIG_PM_SLEEP */
+
+#ifdef CONFIG_PM_RUNTIME
+static int ci_hdrc_imx_runtime_suspend(struct device *dev)
+{
+ return imx_controller_suspend(dev);
+}
+
+static int ci_hdrc_imx_runtime_resume(struct device *dev)
+{
+ return imx_controller_resume(dev);
+}
+#endif /* CONFIG_PM_RUNTIME */
+
+#endif /* CONFIG_PM */
+static const struct dev_pm_ops ci_hdrc_imx_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(ci_hdrc_imx_suspend, ci_hdrc_imx_resume)
+ SET_RUNTIME_PM_OPS(ci_hdrc_imx_runtime_suspend,
+ ci_hdrc_imx_runtime_resume, NULL)
+};
static const struct of_device_id ci_hdrc_imx_dt_ids[] = {
{ .compatible = "fsl,imx27-usb", },
{ /* sentinel */ }
@@ -187,6 +274,7 @@ static struct platform_driver ci_hdrc_imx_driver = {
.name = "imx_usb",
.owner = THIS_MODULE,
.of_match_table = ci_hdrc_imx_dt_ids,
+ .pm = &ci_hdrc_imx_pm_ops,
},
};
--
1.7.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Patch v2 03/10] usb: chipidea: add wakeup interrupt handler
2013-10-22 6:23 [Patch v2 00/10] Add power management support for chipidea Peter Chen
2013-10-22 6:23 ` [Patch v2 01/10] usb: chipidea: Add power management support Peter Chen
2013-10-22 6:23 ` [Patch v2 02/10] usb: chipidea: imx: add " Peter Chen
@ 2013-10-22 6:23 ` Peter Chen
2013-10-22 6:23 ` [Patch v2 04/10] usb: chipidea: usbmisc_imx: remove the controller's clock information Peter Chen
` (6 subsequent siblings)
9 siblings, 0 replies; 21+ messages in thread
From: Peter Chen @ 2013-10-22 6:23 UTC (permalink / raw)
To: linux-arm-kernel
When the controller is at suspend mode, it can be waken up by
external events (like vbus, dp/dm or id change). Once we receive
the wakeup interrupt, we need to resume the controller first, eg
open clocks, disable some wakeup settings, etc. After that, the
controller can receive the normal USB interrupts.
Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
drivers/usb/chipidea/ci.h | 1 +
drivers/usb/chipidea/core.c | 20 ++++++++++++++++++++
drivers/usb/chipidea/otg.c | 5 +++++
3 files changed, 26 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
index 6ea1892..38598b3 100644
--- a/drivers/usb/chipidea/ci.h
+++ b/drivers/usb/chipidea/ci.h
@@ -175,6 +175,7 @@ struct ci_hdrc {
bool b_sess_valid_event;
bool supports_runtime_pm;
bool in_lpm;
+ bool wakeup_int;
};
static inline struct ci_role_driver *ci_role(struct ci_hdrc *ci)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index a5b8f43..a9497eb 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -190,6 +190,13 @@ static void ci_hdrc_enter_lpm(struct ci_hdrc *ci, bool enable)
* than 1ms) to leave low power mode.
*/
usleep_range(1500, 2000);
+ } else if (!enable) {
+ /*
+ * At wakeup interrupt, the phcd will be cleared by hardware
+ * automatically, but the controller needs at least 1ms
+ * to reflect PHY's status.
+ */
+ usleep_range(1200, 1800);
}
}
@@ -355,6 +362,13 @@ static irqreturn_t ci_irq(int irq, void *data)
irqreturn_t ret = IRQ_NONE;
u32 otgsc = 0;
+ if (ci->in_lpm) {
+ disable_irq_nosync(irq);
+ ci->wakeup_int = true;
+ pm_runtime_get(ci->dev);
+ return IRQ_HANDLED;
+ }
+
if (ci->is_otg)
otgsc = hw_read(ci, OP_OTGSC, ~0);
@@ -739,6 +753,12 @@ static int ci_controller_resume(struct device *dev)
ci->in_lpm = false;
+ if (ci->wakeup_int) {
+ ci->wakeup_int = false;
+ enable_irq(ci->irq);
+ pm_runtime_put(ci->dev);
+ }
+
return 0;
}
diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
index 39bd7ec..54bc7c0 100644
--- a/drivers/usb/chipidea/otg.c
+++ b/drivers/usb/chipidea/otg.c
@@ -78,10 +78,15 @@ static void ci_otg_work(struct work_struct *work)
if (ci->id_event) {
ci->id_event = false;
+ /* Keep controller active during id switch */
+ pm_runtime_get_sync(ci->dev);
ci_handle_id_switch(ci);
+ pm_runtime_put_sync(ci->dev);
} else if (ci->b_sess_valid_event) {
ci->b_sess_valid_event = false;
+ pm_runtime_get_sync(ci->dev);
ci_handle_vbus_change(ci);
+ pm_runtime_put_sync(ci->dev);
} else
dev_err(ci->dev, "unexpected event occurs at %s\n", __func__);
--
1.7.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Patch v2 04/10] usb: chipidea: usbmisc_imx: remove the controller's clock information
2013-10-22 6:23 [Patch v2 00/10] Add power management support for chipidea Peter Chen
` (2 preceding siblings ...)
2013-10-22 6:23 ` [Patch v2 03/10] usb: chipidea: add wakeup interrupt handler Peter Chen
@ 2013-10-22 6:23 ` Peter Chen
2013-10-22 6:23 ` [Patch v2 05/10] usb: chipidea: usbmisc_imx: add set_wakup API Peter Chen
` (5 subsequent siblings)
9 siblings, 0 replies; 21+ messages in thread
From: Peter Chen @ 2013-10-22 6:23 UTC (permalink / raw)
To: linux-arm-kernel
Currently, the usbmisc is just an API supplier for controller
driver, the controller calls related APIs to handle different
things among the SoCs, before calling it, the clock must
be on. So the clock operation is useless for usbmisc, it also
increases the difficulties to manage the clock, especially at
runtime power management situation.
Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
drivers/usb/chipidea/usbmisc_imx.c | 18 ------------------
1 files changed, 0 insertions(+), 18 deletions(-)
diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
index 8a1094b..1fd9a12 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -11,7 +11,6 @@
#include <linux/module.h>
#include <linux/of_platform.h>
-#include <linux/clk.h>
#include <linux/err.h>
#include <linux/io.h>
#include <linux/delay.h>
@@ -40,7 +39,6 @@ struct usbmisc_ops {
struct imx_usbmisc {
void __iomem *base;
spinlock_t lock;
- struct clk *clk;
const struct usbmisc_ops *ops;
};
@@ -177,7 +175,6 @@ static int usbmisc_imx_probe(struct platform_device *pdev)
{
struct resource *res;
struct imx_usbmisc *data;
- int ret;
struct of_device_id *tmp_dev;
if (usbmisc)
@@ -194,20 +191,6 @@ static int usbmisc_imx_probe(struct platform_device *pdev)
if (IS_ERR(data->base))
return PTR_ERR(data->base);
- data->clk = devm_clk_get(&pdev->dev, NULL);
- if (IS_ERR(data->clk)) {
- dev_err(&pdev->dev,
- "failed to get clock, err=%ld\n", PTR_ERR(data->clk));
- return PTR_ERR(data->clk);
- }
-
- ret = clk_prepare_enable(data->clk);
- if (ret) {
- dev_err(&pdev->dev,
- "clk_prepare_enable failed, err=%d\n", ret);
- return ret;
- }
-
tmp_dev = (struct of_device_id *)
of_match_device(usbmisc_imx_dt_ids, &pdev->dev);
data->ops = (const struct usbmisc_ops *)tmp_dev->data;
@@ -218,7 +201,6 @@ static int usbmisc_imx_probe(struct platform_device *pdev)
static int usbmisc_imx_remove(struct platform_device *pdev)
{
- clk_disable_unprepare(usbmisc->clk);
usbmisc = NULL;
return 0;
}
--
1.7.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Patch v2 05/10] usb: chipidea: usbmisc_imx: add set_wakup API
2013-10-22 6:23 [Patch v2 00/10] Add power management support for chipidea Peter Chen
` (3 preceding siblings ...)
2013-10-22 6:23 ` [Patch v2 04/10] usb: chipidea: usbmisc_imx: remove the controller's clock information Peter Chen
@ 2013-10-22 6:23 ` Peter Chen
2013-10-22 6:23 ` [Patch v2 06/10] usb: chipidea: imx: call set_wakeup when necessary Peter Chen
` (4 subsequent siblings)
9 siblings, 0 replies; 21+ messages in thread
From: Peter Chen @ 2013-10-22 6:23 UTC (permalink / raw)
To: linux-arm-kernel
It is used to enable USB wakeup, currently only imx6 SoC serial
usb's wakeup is enabled.
Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
drivers/usb/chipidea/ci_hdrc_imx.h | 1 +
drivers/usb/chipidea/usbmisc_imx.c | 41 ++++++++++++++++++++++++++++++++++++
2 files changed, 42 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/chipidea/ci_hdrc_imx.h b/drivers/usb/chipidea/ci_hdrc_imx.h
index c727159..92f4c30 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.h
+++ b/drivers/usb/chipidea/ci_hdrc_imx.h
@@ -18,3 +18,4 @@ struct imx_usbmisc_data {
int imx_usbmisc_init(struct imx_usbmisc_data *);
int imx_usbmisc_init_post(struct imx_usbmisc_data *);
+int imx_usbmisc_set_wakeup(struct imx_usbmisc_data *, bool);
diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
index 1fd9a12..55b59e0 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -28,12 +28,18 @@
#define MX53_BM_OVER_CUR_DIS_UHx BIT(30)
#define MX6_BM_OVER_CUR_DIS BIT(7)
+#define MX6_BM_WAKEUP_ENABLE BIT(10)
+#define MX6_BM_ID_WAKEUP BIT(16)
+#define MX6_BM_VBUS_WAKEUP BIT(17)
+#define MX6_BM_WAKEUP_INTR BIT(31)
struct usbmisc_ops {
/* It's called once when probe a usb device */
int (*init)(struct imx_usbmisc_data *data);
/* It's called once after adding a usb device */
int (*post)(struct imx_usbmisc_data *data);
+ /* It's called when we need to enable usb wakeup */
+ int (*set_wakeup)(struct imx_usbmisc_data *data, bool enabled);
};
struct imx_usbmisc {
@@ -122,6 +128,30 @@ static int usbmisc_imx6q_init(struct imx_usbmisc_data *data)
return 0;
}
+static int usbmisc_imx6q_set_wakeup
+ (struct imx_usbmisc_data *data, bool enabled)
+{
+ unsigned long flags;
+ u32 reg, val = (MX6_BM_WAKEUP_ENABLE | MX6_BM_VBUS_WAKEUP
+ | MX6_BM_ID_WAKEUP);
+
+ if (data->index > 3)
+ return -EINVAL;
+
+ spin_lock_irqsave(&usbmisc->lock, flags);
+ reg = readl(usbmisc->base + data->index * 4);
+ if (enabled) {
+ writel(reg | val, usbmisc->base + data->index * 4);
+ } else {
+ if (reg & MX6_BM_WAKEUP_INTR)
+ pr_debug("wakeup int at ci_hdrc.%d\n", data->index);
+ writel(reg & ~val, usbmisc->base + data->index * 4);
+ }
+ spin_unlock_irqrestore(&usbmisc->lock, flags);
+
+ return 0;
+}
+
static const struct usbmisc_ops imx25_usbmisc_ops = {
.post = usbmisc_imx25_post,
};
@@ -132,6 +162,7 @@ static const struct usbmisc_ops imx53_usbmisc_ops = {
static const struct usbmisc_ops imx6q_usbmisc_ops = {
.init = usbmisc_imx6q_init,
+ .set_wakeup = usbmisc_imx6q_set_wakeup,
};
int imx_usbmisc_init(struct imx_usbmisc_data *data)
@@ -154,6 +185,16 @@ int imx_usbmisc_init_post(struct imx_usbmisc_data *data)
}
EXPORT_SYMBOL_GPL(imx_usbmisc_init_post);
+int imx_usbmisc_set_wakeup(struct imx_usbmisc_data *data, bool enabled)
+{
+ if (!usbmisc)
+ return -ENODEV;
+ if (!usbmisc->ops->set_wakeup)
+ return 0;
+ return usbmisc->ops->set_wakeup(data, enabled);
+}
+EXPORT_SYMBOL_GPL(imx_usbmisc_set_wakeup);
+
static const struct of_device_id usbmisc_imx_dt_ids[] = {
{
.compatible = "fsl,imx25-usbmisc",
--
1.7.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Patch v2 06/10] usb: chipidea: imx: call set_wakeup when necessary
2013-10-22 6:23 [Patch v2 00/10] Add power management support for chipidea Peter Chen
` (4 preceding siblings ...)
2013-10-22 6:23 ` [Patch v2 05/10] usb: chipidea: usbmisc_imx: add set_wakup API Peter Chen
@ 2013-10-22 6:23 ` Peter Chen
2013-10-22 6:23 ` [Patch v2 07/10] usb: chipidea: imx: Enable runtime pm support for imx6 SoC serial Peter Chen
` (3 subsequent siblings)
9 siblings, 0 replies; 21+ messages in thread
From: Peter Chen @ 2013-10-22 6:23 UTC (permalink / raw)
To: linux-arm-kernel
- Disable wakeup after probe
- Enable wakeup during the suspend
- Disable wakeup after controller is active
Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
drivers/usb/chipidea/ci_hdrc_imx.c | 36 ++++++++++++++++++++++++++++++++++++
1 files changed, 36 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
index a1e5634..2a14152 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -152,6 +152,15 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
}
}
+ if (data->usbmisc_data) {
+ ret = imx_usbmisc_set_wakeup(data->usbmisc_data, false);
+ if (ret) {
+ dev_err(&pdev->dev, "usbmisc set_wakeup failed, ret=%d\n",
+ ret);
+ goto disable_device;
+ }
+ }
+
platform_set_drvdata(pdev, data);
device_set_wakeup_capable(&pdev->dev, true);
@@ -189,12 +198,23 @@ static int ci_hdrc_imx_remove(struct platform_device *pdev)
static int imx_controller_suspend(struct device *dev)
{
struct ci_hdrc_imx_data *data = dev_get_drvdata(dev);
+ int ret;
dev_dbg(dev, "at %s\n", __func__);
if (data->in_lpm)
return 0;
+ if (data->usbmisc_data) {
+ ret = imx_usbmisc_set_wakeup(data->usbmisc_data, true);
+ if (ret) {
+ dev_err(dev,
+ "usbmisc set_wakeup failed, ret=%d\n",
+ ret);
+ return ret;
+ }
+ }
+
clk_disable_unprepare(data->clk);
data->in_lpm = true;
@@ -218,6 +238,22 @@ static int imx_controller_resume(struct device *dev)
data->in_lpm = false;
+ if (data->usbmisc_data) {
+ ret = imx_usbmisc_set_wakeup(data->usbmisc_data, false);
+ if (ret) {
+ dev_err(dev,
+ "usbmisc set_wakeup failed, ret=%d\n",
+ ret);
+ ret = -EINVAL;
+ goto clk_disable;
+ }
+ }
+
+ return 0;
+
+clk_disable:
+ clk_disable_unprepare(data->clk);
+
return ret;
}
--
1.7.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Patch v2 07/10] usb: chipidea: imx: Enable runtime pm support for imx6 SoC serial
2013-10-22 6:23 [Patch v2 00/10] Add power management support for chipidea Peter Chen
` (5 preceding siblings ...)
2013-10-22 6:23 ` [Patch v2 06/10] usb: chipidea: imx: call set_wakeup when necessary Peter Chen
@ 2013-10-22 6:23 ` Peter Chen
2013-10-22 6:23 ` [Patch v2 08/10] usb: chipidea: host: add quirk for ehci operation Peter Chen
` (2 subsequent siblings)
9 siblings, 0 replies; 21+ messages in thread
From: Peter Chen @ 2013-10-22 6:23 UTC (permalink / raw)
To: linux-arm-kernel
Currently, only imx6 SoC serial adds wakeup logic, so only
enable runtime pm for imx6.
Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
drivers/usb/chipidea/ci_hdrc_imx.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
index 2a14152..d4775b3 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -118,6 +118,11 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
pdata.phy = data->phy;
+ if (of_device_is_compatible(np, "fsl,imx6q-usb")) {
+ pdata.flags |= CI_HDRC_SUPPORTS_RUNTIME_PM;
+ data->supports_runtime_pm = true;
+ }
+
if (!pdev->dev.dma_mask)
pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
if (!pdev->dev.coherent_dma_mask)
--
1.7.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Patch v2 08/10] usb: chipidea: host: add quirk for ehci operation
2013-10-22 6:23 [Patch v2 00/10] Add power management support for chipidea Peter Chen
` (6 preceding siblings ...)
2013-10-22 6:23 ` [Patch v2 07/10] usb: chipidea: imx: Enable runtime pm support for imx6 SoC serial Peter Chen
@ 2013-10-22 6:23 ` Peter Chen
2013-10-23 14:42 ` Alan Stern
2013-10-24 5:51 ` Lothar Waßmann
2013-10-22 6:23 ` [Patch v2 09/10] usb: chipidea: host: add ehci quirk for imx controller Peter Chen
2013-10-22 6:23 ` [Patch v2 10/10] usb: chipidea: imx: Enable CI_HDRC_IMX_EHCI_QUIRK if the phy has notify APIs Peter Chen
9 siblings, 2 replies; 21+ messages in thread
From: Peter Chen @ 2013-10-22 6:23 UTC (permalink / raw)
To: linux-arm-kernel
For chipidea controller, it does not follow ehci spec strictly.
Taking resume signal as an example, it will stop resume signal about
20-21ms later automatically, but standard ehci spec says, the resume
signal is controlled by software (clear portsc.PORT_RESUME).
This operation causes some remote wakeup problems for high speed
devices due to host controller does not send SOF in time since
software can't guarantee set run/stop bit in time (run/stop bit
was cleared at the ehci suspend routine).
When software sets run/stop bit, it needs 1 SoF time to make it effect.
If we close the PHY clock just after setting run/stop bit, it does
not be set in practice, so a software delay is needed.
Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
drivers/usb/chipidea/host.c | 51 +++++++++++++++++++++++++++++++++++++++++++
1 files changed, 51 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
index 59e6020..283b385 100644
--- a/drivers/usb/chipidea/host.c
+++ b/drivers/usb/chipidea/host.c
@@ -33,6 +33,53 @@
#include "host.h"
static struct hc_driver __read_mostly ci_ehci_hc_driver;
+static int (*orig_bus_suspend)(struct usb_hcd *hcd);
+
+static int ci_ehci_bus_suspend(struct usb_hcd *hcd)
+{
+ struct ehci_hcd *ehci = hcd_to_ehci(hcd);
+ int port;
+ u32 tmp;
+
+ int ret = orig_bus_suspend(hcd);
+
+ if (ret)
+ return ret;
+
+ port = HCS_N_PORTS(ehci->hcs_params);
+ while (port--) {
+ u32 __iomem *reg = &ehci->regs->port_status[port];
+ u32 portsc = ehci_readl(ehci, reg);
+
+ if (portsc & PORT_CONNECT) {
+ /*
+ * For chipidea, the resume signal will be ended
+ * automatically, so for remote wakeup case, the
+ * usbcmd.rs may not be set before the resume has
+ * ended if other resume path consumes too much
+ * time (~23ms-24ms), in that case, the SOF will not
+ * send out within 3ms after resume ends, then the
+ * device will enter suspend again.
+ */
+ if (hcd->self.root_hub->do_remote_wakeup) {
+ ehci_dbg(ehci,
+ "Remote wakeup is enabled, "
+ "and device is on the port\n");
+
+ tmp = ehci_readl(ehci, &ehci->regs->command);
+ tmp |= CMD_RUN;
+ ehci_writel(ehci, tmp, &ehci->regs->command);
+ /*
+ * It needs a short delay between set RUNSTOP
+ * and set PHCD.
+ */
+ udelay(125);
+ }
+ }
+ }
+
+ return 0;
+}
static irqreturn_t host_irq(struct ci_hdrc *ci)
{
@@ -134,5 +181,9 @@ int ci_hdrc_host_init(struct ci_hdrc *ci)
ehci_init_driver(&ci_ehci_hc_driver, NULL);
+ orig_bus_suspend = ci_ehci_hc_driver.bus_suspend;
+
+ ci_ehci_hc_driver.bus_suspend = ci_ehci_bus_suspend;
+
return 0;
}
--
1.7.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Patch v2 08/10] usb: chipidea: host: add quirk for ehci operation
2013-10-22 6:23 ` [Patch v2 08/10] usb: chipidea: host: add quirk for ehci operation Peter Chen
@ 2013-10-23 14:42 ` Alan Stern
2013-10-24 0:47 ` Peter Chen
2013-10-24 5:51 ` Lothar Waßmann
1 sibling, 1 reply; 21+ messages in thread
From: Alan Stern @ 2013-10-23 14:42 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, 22 Oct 2013, Peter Chen wrote:
> For chipidea controller, it does not follow ehci spec strictly.
> Taking resume signal as an example, it will stop resume signal about
> 20-21ms later automatically, but standard ehci spec says, the resume
> signal is controlled by software (clear portsc.PORT_RESUME).
>
> This operation causes some remote wakeup problems for high speed
> devices due to host controller does not send SOF in time since
> software can't guarantee set run/stop bit in time (run/stop bit
> was cleared at the ehci suspend routine).
>
> When software sets run/stop bit, it needs 1 SoF time to make it effect.
> If we close the PHY clock just after setting run/stop bit, it does
> not be set in practice, so a software delay is needed.
>
> Signed-off-by: Peter Chen <peter.chen@freescale.com>
> ---
> drivers/usb/chipidea/host.c | 51 +++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 51 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
> index 59e6020..283b385 100644
> --- a/drivers/usb/chipidea/host.c
> +++ b/drivers/usb/chipidea/host.c
> @@ -33,6 +33,53 @@
> #include "host.h"
>
> static struct hc_driver __read_mostly ci_ehci_hc_driver;
> +static int (*orig_bus_suspend)(struct usb_hcd *hcd);
> +
> +static int ci_ehci_bus_suspend(struct usb_hcd *hcd)
> +{
> + struct ehci_hcd *ehci = hcd_to_ehci(hcd);
> + int port;
> + u32 tmp;
> +
> + int ret = orig_bus_suspend(hcd);
> +
> + if (ret)
> + return ret;
> +
> + port = HCS_N_PORTS(ehci->hcs_params);
> + while (port--) {
> + u32 __iomem *reg = &ehci->regs->port_status[port];
> + u32 portsc = ehci_readl(ehci, reg);
> +
> + if (portsc & PORT_CONNECT) {
> + /*
> + * For chipidea, the resume signal will be ended
> + * automatically, so for remote wakeup case, the
> + * usbcmd.rs may not be set before the resume has
> + * ended if other resume path consumes too much
> + * time (~23ms-24ms), in that case, the SOF will not
> + * send out within 3ms after resume ends, then the
> + * device will enter suspend again.
> + */
> + if (hcd->self.root_hub->do_remote_wakeup) {
> + ehci_dbg(ehci,
> + "Remote wakeup is enabled, "
> + "and device is on the port\n");
> +
> + tmp = ehci_readl(ehci, &ehci->regs->command);
> + tmp |= CMD_RUN;
> + ehci_writel(ehci, tmp, &ehci->regs->command);
> + /*
> + * It needs a short delay between set RUNSTOP
> + * and set PHCD.
> + */
> + udelay(125);
> + }
> + }
> + }
This code is a little strange. If there is only one port then you
don't need the loop. But if there is more than one port then you don't
want to put the udelay inside the loop. (If two devices are attached,
you don't want to delay the suspend by 250 us.)
Perhaps you should add a "break;" statement following the udelay.
Alan Stern
^ permalink raw reply [flat|nested] 21+ messages in thread* [Patch v2 08/10] usb: chipidea: host: add quirk for ehci operation
2013-10-23 14:42 ` Alan Stern
@ 2013-10-24 0:47 ` Peter Chen
0 siblings, 0 replies; 21+ messages in thread
From: Peter Chen @ 2013-10-24 0:47 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Oct 23, 2013 at 10:42:58AM -0400, Alan Stern wrote:
> On Tue, 22 Oct 2013, Peter Chen wrote:
>
> > For chipidea controller, it does not follow ehci spec strictly.
> > Taking resume signal as an example, it will stop resume signal about
> > 20-21ms later automatically, but standard ehci spec says, the resume
> > signal is controlled by software (clear portsc.PORT_RESUME).
> >
> > This operation causes some remote wakeup problems for high speed
> > devices due to host controller does not send SOF in time since
> > software can't guarantee set run/stop bit in time (run/stop bit
> > was cleared at the ehci suspend routine).
> >
> > When software sets run/stop bit, it needs 1 SoF time to make it effect.
> > If we close the PHY clock just after setting run/stop bit, it does
> > not be set in practice, so a software delay is needed.
> >
> > Signed-off-by: Peter Chen <peter.chen@freescale.com>
> > ---
> > drivers/usb/chipidea/host.c | 51 +++++++++++++++++++++++++++++++++++++++++++
> > 1 files changed, 51 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
> > index 59e6020..283b385 100644
> > --- a/drivers/usb/chipidea/host.c
> > +++ b/drivers/usb/chipidea/host.c
> > @@ -33,6 +33,53 @@
> > #include "host.h"
> >
> > static struct hc_driver __read_mostly ci_ehci_hc_driver;
> > +static int (*orig_bus_suspend)(struct usb_hcd *hcd);
> > +
> > +static int ci_ehci_bus_suspend(struct usb_hcd *hcd)
> > +{
> > + struct ehci_hcd *ehci = hcd_to_ehci(hcd);
> > + int port;
> > + u32 tmp;
> > +
> > + int ret = orig_bus_suspend(hcd);
> > +
> > + if (ret)
> > + return ret;
> > +
> > + port = HCS_N_PORTS(ehci->hcs_params);
> > + while (port--) {
> > + u32 __iomem *reg = &ehci->regs->port_status[port];
> > + u32 portsc = ehci_readl(ehci, reg);
> > +
> > + if (portsc & PORT_CONNECT) {
> > + /*
> > + * For chipidea, the resume signal will be ended
> > + * automatically, so for remote wakeup case, the
> > + * usbcmd.rs may not be set before the resume has
> > + * ended if other resume path consumes too much
> > + * time (~23ms-24ms), in that case, the SOF will not
> > + * send out within 3ms after resume ends, then the
> > + * device will enter suspend again.
> > + */
> > + if (hcd->self.root_hub->do_remote_wakeup) {
> > + ehci_dbg(ehci,
> > + "Remote wakeup is enabled, "
> > + "and device is on the port\n");
> > +
> > + tmp = ehci_readl(ehci, &ehci->regs->command);
> > + tmp |= CMD_RUN;
> > + ehci_writel(ehci, tmp, &ehci->regs->command);
> > + /*
> > + * It needs a short delay between set RUNSTOP
> > + * and set PHCD.
> > + */
> > + udelay(125);
> > + }
> > + }
> > + }
>
> This code is a little strange. If there is only one port then you
> don't need the loop. But if there is more than one port then you don't
> want to put the udelay inside the loop. (If two devices are attached,
> you don't want to delay the suspend by 250 us.)
>
Yes, you are right. I will add break, that is once there is port connected,
delay 125us, then quit while.
--
Best Regards,
Peter Chen
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Patch v2 08/10] usb: chipidea: host: add quirk for ehci operation
2013-10-22 6:23 ` [Patch v2 08/10] usb: chipidea: host: add quirk for ehci operation Peter Chen
2013-10-23 14:42 ` Alan Stern
@ 2013-10-24 5:51 ` Lothar Waßmann
2013-10-24 5:50 ` Peter Chen
1 sibling, 1 reply; 21+ messages in thread
From: Lothar Waßmann @ 2013-10-24 5:51 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
Peter Chen wrote:
> For chipidea controller, it does not follow ehci spec strictly.
> Taking resume signal as an example, it will stop resume signal about
> 20-21ms later automatically, but standard ehci spec says, the resume
> signal is controlled by software (clear portsc.PORT_RESUME).
>
> This operation causes some remote wakeup problems for high speed
> devices due to host controller does not send SOF in time since
> software can't guarantee set run/stop bit in time (run/stop bit
> was cleared at the ehci suspend routine).
>
> When software sets run/stop bit, it needs 1 SoF time to make it effect.
> If we close the PHY clock just after setting run/stop bit, it does
> not be set in practice, so a software delay is needed.
>
> Signed-off-by: Peter Chen <peter.chen@freescale.com>
> ---
> drivers/usb/chipidea/host.c | 51 +++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 51 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
> index 59e6020..283b385 100644
> --- a/drivers/usb/chipidea/host.c
> +++ b/drivers/usb/chipidea/host.c
> @@ -33,6 +33,53 @@
> #include "host.h"
>
> static struct hc_driver __read_mostly ci_ehci_hc_driver;
> +static int (*orig_bus_suspend)(struct usb_hcd *hcd);
> +
> +static int ci_ehci_bus_suspend(struct usb_hcd *hcd)
> +{
> + struct ehci_hcd *ehci = hcd_to_ehci(hcd);
> + int port;
> + u32 tmp;
> +
> + int ret = orig_bus_suspend(hcd);
> +
> + if (ret)
> + return ret;
> +
> + port = HCS_N_PORTS(ehci->hcs_params);
> + while (port--) {
> + u32 __iomem *reg = &ehci->regs->port_status[port];
> + u32 portsc = ehci_readl(ehci, reg);
> +
> + if (portsc & PORT_CONNECT) {
> + /*
> + * For chipidea, the resume signal will be ended
> + * automatically, so for remote wakeup case, the
> + * usbcmd.rs may not be set before the resume has
> + * ended if other resume path consumes too much
> + * time (~23ms-24ms), in that case, the SOF will not
> + * send out within 3ms after resume ends, then the
> + * device will enter suspend again.
> + */
> + if (hcd->self.root_hub->do_remote_wakeup) {
> + ehci_dbg(ehci,
> + "Remote wakeup is enabled, "
> + "and device is on the port\n");
>
Please don't line wrap message texts, since it makes it harder to grep
the kernel source for messages found in a logfile.
Lothar Wa?mann
--
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Gesch?ftsf?hrer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
www.karo-electronics.de | info at karo-electronics.de
___________________________________________________________
^ permalink raw reply [flat|nested] 21+ messages in thread* [Patch v2 08/10] usb: chipidea: host: add quirk for ehci operation
2013-10-24 5:51 ` Lothar Waßmann
@ 2013-10-24 5:50 ` Peter Chen
0 siblings, 0 replies; 21+ messages in thread
From: Peter Chen @ 2013-10-24 5:50 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Oct 24, 2013 at 07:51:03AM +0200, Lothar Wa?mann wrote:
> Hi,
>
> Peter Chen wrote:
> > For chipidea controller, it does not follow ehci spec strictly.
> > Taking resume signal as an example, it will stop resume signal about
> > 20-21ms later automatically, but standard ehci spec says, the resume
> > signal is controlled by software (clear portsc.PORT_RESUME).
> >
> > This operation causes some remote wakeup problems for high speed
> > devices due to host controller does not send SOF in time since
> > software can't guarantee set run/stop bit in time (run/stop bit
> > was cleared at the ehci suspend routine).
> >
> > When software sets run/stop bit, it needs 1 SoF time to make it effect.
> > If we close the PHY clock just after setting run/stop bit, it does
> > not be set in practice, so a software delay is needed.
> >
> > Signed-off-by: Peter Chen <peter.chen@freescale.com>
> > ---
> > drivers/usb/chipidea/host.c | 51 +++++++++++++++++++++++++++++++++++++++++++
> > 1 files changed, 51 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
> > index 59e6020..283b385 100644
> > --- a/drivers/usb/chipidea/host.c
> > +++ b/drivers/usb/chipidea/host.c
> > @@ -33,6 +33,53 @@
> > #include "host.h"
> >
> > static struct hc_driver __read_mostly ci_ehci_hc_driver;
> > +static int (*orig_bus_suspend)(struct usb_hcd *hcd);
> > +
> > +static int ci_ehci_bus_suspend(struct usb_hcd *hcd)
> > +{
> > + struct ehci_hcd *ehci = hcd_to_ehci(hcd);
> > + int port;
> > + u32 tmp;
> > +
> > + int ret = orig_bus_suspend(hcd);
> > +
> > + if (ret)
> > + return ret;
> > +
> > + port = HCS_N_PORTS(ehci->hcs_params);
> > + while (port--) {
> > + u32 __iomem *reg = &ehci->regs->port_status[port];
> > + u32 portsc = ehci_readl(ehci, reg);
> > +
> > + if (portsc & PORT_CONNECT) {
> > + /*
> > + * For chipidea, the resume signal will be ended
> > + * automatically, so for remote wakeup case, the
> > + * usbcmd.rs may not be set before the resume has
> > + * ended if other resume path consumes too much
> > + * time (~23ms-24ms), in that case, the SOF will not
> > + * send out within 3ms after resume ends, then the
> > + * device will enter suspend again.
> > + */
> > + if (hcd->self.root_hub->do_remote_wakeup) {
> > + ehci_dbg(ehci,
> > + "Remote wakeup is enabled, "
> > + "and device is on the port\n");
> >
> Please don't line wrap message texts, since it makes it harder to grep
> the kernel source for messages found in a logfile.
>
>
Thanks, will change.
--
Best Regards,
Peter Chen
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Patch v2 09/10] usb: chipidea: host: add ehci quirk for imx controller
2013-10-22 6:23 [Patch v2 00/10] Add power management support for chipidea Peter Chen
` (7 preceding siblings ...)
2013-10-22 6:23 ` [Patch v2 08/10] usb: chipidea: host: add quirk for ehci operation Peter Chen
@ 2013-10-22 6:23 ` Peter Chen
2013-10-23 14:46 ` Alan Stern
2013-10-22 6:23 ` [Patch v2 10/10] usb: chipidea: imx: Enable CI_HDRC_IMX_EHCI_QUIRK if the phy has notify APIs Peter Chen
9 siblings, 1 reply; 21+ messages in thread
From: Peter Chen @ 2013-10-22 6:23 UTC (permalink / raw)
To: linux-arm-kernel
When the port goes to suspend or finishes resme, it needs to
notify PHY, it is not a standard EHCI operation, so we add a
quirk for it.
Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
drivers/usb/chipidea/host.c | 129 ++++++++++++++++++++++++++++++++++++++++++
include/linux/usb/chipidea.h | 1 +
2 files changed, 130 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
index 283b385..c1d05c4 100644
--- a/drivers/usb/chipidea/host.c
+++ b/drivers/usb/chipidea/host.c
@@ -34,6 +34,10 @@
static struct hc_driver __read_mostly ci_ehci_hc_driver;
static int (*orig_bus_suspend)(struct usb_hcd *hcd);
+static int (*orig_bus_resume)(struct usb_hcd *hcd);
+static int (*orig_hub_control)(struct usb_hcd *hcd,
+ u16 typeReq, u16 wValue, u16 wIndex,
+ char *buf, u16 wLength);
static int ci_ehci_bus_suspend(struct usb_hcd *hcd)
{
@@ -75,12 +79,131 @@ static int ci_ehci_bus_suspend(struct usb_hcd *hcd)
*/
udelay(125);
}
+ if (hcd->phy && test_bit(port, &ehci->bus_suspended)
+ && (ehci_port_speed(ehci, portsc) ==
+ USB_PORT_STAT_HIGH_SPEED))
+ /*
+ * notify the USB PHY, it is for global
+ * suspend case.
+ */
+ usb_phy_notify_suspend(hcd->phy,
+ USB_SPEED_HIGH);
}
}
return 0;
}
+static int ci_imx_ehci_bus_resume(struct usb_hcd *hcd)
+{
+ struct ehci_hcd *ehci = hcd_to_ehci(hcd);
+ int port;
+
+ int ret = orig_bus_resume(hcd);
+
+ if (ret)
+ return ret;
+
+ port = HCS_N_PORTS(ehci->hcs_params);
+ while (port--) {
+ u32 __iomem *reg = &ehci->regs->port_status[port];
+ u32 portsc = ehci_readl(ehci, reg);
+ /*
+ * Notify PHY after resume signal has finished, it is
+ * for global suspend case.
+ */
+ if (hcd->phy
+ && test_bit(port, &ehci->bus_suspended)
+ && (portsc & PORT_CONNECT)
+ && (ehci_port_speed(ehci, portsc) ==
+ USB_PORT_STAT_HIGH_SPEED))
+ /* notify the USB PHY */
+ usb_phy_notify_resume(hcd->phy, USB_SPEED_HIGH);
+ }
+
+ return 0;
+}
+
+/* The below code is based on tegra ehci driver */
+static int ci_imx_ehci_hub_control(
+ struct usb_hcd *hcd,
+ u16 typeReq,
+ u16 wValue,
+ u16 wIndex,
+ char *buf,
+ u16 wLength
+)
+{
+ struct ehci_hcd *ehci = hcd_to_ehci(hcd);
+ u32 __iomem *status_reg;
+ u32 temp;
+ unsigned long flags;
+ int retval = 0;
+
+ status_reg = &ehci->regs->port_status[(wIndex & 0xff) - 1];
+
+ spin_lock_irqsave(&ehci->lock, flags);
+
+ if (typeReq == SetPortFeature && wValue == USB_PORT_FEAT_SUSPEND) {
+ temp = ehci_readl(ehci, status_reg);
+ if ((temp & PORT_PE) == 0 || (temp & PORT_RESET) != 0) {
+ retval = -EPIPE;
+ goto done;
+ }
+
+ temp &= ~(PORT_RWC_BITS | PORT_WKCONN_E);
+ temp |= PORT_WKDISC_E | PORT_WKOC_E;
+ ehci_writel(ehci, temp | PORT_SUSPEND, status_reg);
+
+ /*
+ * If a transaction is in progress, there may be a delay in
+ * suspending the port. Poll until the port is suspended.
+ */
+ if (ehci_handshake(ehci, status_reg, PORT_SUSPEND,
+ PORT_SUSPEND, 5000))
+ ehci_err(ehci, "timeout waiting for SUSPEND\n");
+
+ spin_unlock_irqrestore(&ehci->lock, flags);
+ if (ehci_port_speed(ehci, temp) ==
+ USB_PORT_STAT_HIGH_SPEED && hcd->phy) {
+ /* notify the USB PHY */
+ usb_phy_notify_suspend(hcd->phy, USB_SPEED_HIGH);
+ }
+ spin_lock_irqsave(&ehci->lock, flags);
+
+ set_bit((wIndex & 0xff) - 1, &ehci->suspended_ports);
+ goto done;
+ }
+
+ /*
+ * After resume has finished, it needs do some post resume
+ * operation for some SoCs.
+ */
+ else if (typeReq == ClearPortFeature &&
+ wValue == USB_PORT_FEAT_C_SUSPEND) {
+
+ /* Make sure the resume has finished, it should be finished */
+ if (ehci_handshake(ehci, status_reg, PORT_RESUME, 0, 25000))
+ ehci_err(ehci, "timeout waiting for resume\n");
+
+ temp = ehci_readl(ehci, status_reg);
+
+ if (ehci_port_speed(ehci, temp) ==
+ USB_PORT_STAT_HIGH_SPEED && hcd->phy) {
+ /* notify the USB PHY */
+ usb_phy_notify_resume(hcd->phy, USB_SPEED_HIGH);
+ }
+ }
+
+ spin_unlock_irqrestore(&ehci->lock, flags);
+
+ /* Handle the hub control events here */
+ return orig_hub_control(hcd, typeReq, wValue, wIndex, buf, wLength);
+done:
+ spin_unlock_irqrestore(&ehci->lock, flags);
+ return retval;
+}
+
static irqreturn_t host_irq(struct ci_hdrc *ci)
{
return usb_hcd_irq(ci->irq, ci->hcd);
@@ -182,8 +305,14 @@ int ci_hdrc_host_init(struct ci_hdrc *ci)
ehci_init_driver(&ci_ehci_hc_driver, NULL);
orig_bus_suspend = ci_ehci_hc_driver.bus_suspend;
+ orig_bus_resume = ci_ehci_hc_driver.bus_resume;
+ orig_hub_control = ci_ehci_hc_driver.hub_control;
ci_ehci_hc_driver.bus_suspend = ci_ehci_bus_suspend;
+ if (ci->platdata->flags & CI_HDRC_IMX_EHCI_QUIRK) {
+ ci_ehci_hc_driver.bus_resume = ci_imx_ehci_bus_resume;
+ ci_ehci_hc_driver.hub_control = ci_imx_ehci_hub_control;
+ }
return 0;
}
diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h
index 667b46c..8ce3daf 100644
--- a/include/linux/usb/chipidea.h
+++ b/include/linux/usb/chipidea.h
@@ -25,6 +25,7 @@ struct ci_hdrc_platform_data {
* but otg is not supported (no register otgsc).
*/
#define CI_HDRC_DUAL_ROLE_NOT_OTG BIT(4)
+#define CI_HDRC_IMX_EHCI_QUIRK BIT(5)
enum usb_dr_mode dr_mode;
#define CI_HDRC_CONTROLLER_RESET_EVENT 0
#define CI_HDRC_CONTROLLER_STOPPED_EVENT 1
--
1.7.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Patch v2 09/10] usb: chipidea: host: add ehci quirk for imx controller
2013-10-22 6:23 ` [Patch v2 09/10] usb: chipidea: host: add ehci quirk for imx controller Peter Chen
@ 2013-10-23 14:46 ` Alan Stern
2013-10-23 18:06 ` Felipe Balbi
0 siblings, 1 reply; 21+ messages in thread
From: Alan Stern @ 2013-10-23 14:46 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, 22 Oct 2013, Peter Chen wrote:
> When the port goes to suspend or finishes resme, it needs to
> notify PHY, it is not a standard EHCI operation, so we add a
> quirk for it.
Actually, this _should_ be a standard EHCI operation. But we have to
figure out a way to do it that will work on all platforms.
Felipe, any ideas?
Alan Stern
>
> Signed-off-by: Peter Chen <peter.chen@freescale.com>
> ---
> drivers/usb/chipidea/host.c | 129 ++++++++++++++++++++++++++++++++++++++++++
> include/linux/usb/chipidea.h | 1 +
> 2 files changed, 130 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
> index 283b385..c1d05c4 100644
> --- a/drivers/usb/chipidea/host.c
> +++ b/drivers/usb/chipidea/host.c
> @@ -34,6 +34,10 @@
>
> static struct hc_driver __read_mostly ci_ehci_hc_driver;
> static int (*orig_bus_suspend)(struct usb_hcd *hcd);
> +static int (*orig_bus_resume)(struct usb_hcd *hcd);
> +static int (*orig_hub_control)(struct usb_hcd *hcd,
> + u16 typeReq, u16 wValue, u16 wIndex,
> + char *buf, u16 wLength);
>
> static int ci_ehci_bus_suspend(struct usb_hcd *hcd)
> {
> @@ -75,12 +79,131 @@ static int ci_ehci_bus_suspend(struct usb_hcd *hcd)
> */
> udelay(125);
> }
> + if (hcd->phy && test_bit(port, &ehci->bus_suspended)
> + && (ehci_port_speed(ehci, portsc) ==
> + USB_PORT_STAT_HIGH_SPEED))
> + /*
> + * notify the USB PHY, it is for global
> + * suspend case.
> + */
> + usb_phy_notify_suspend(hcd->phy,
> + USB_SPEED_HIGH);
> }
> }
>
> return 0;
> }
>
> +static int ci_imx_ehci_bus_resume(struct usb_hcd *hcd)
> +{
> + struct ehci_hcd *ehci = hcd_to_ehci(hcd);
> + int port;
> +
> + int ret = orig_bus_resume(hcd);
> +
> + if (ret)
> + return ret;
> +
> + port = HCS_N_PORTS(ehci->hcs_params);
> + while (port--) {
> + u32 __iomem *reg = &ehci->regs->port_status[port];
> + u32 portsc = ehci_readl(ehci, reg);
> + /*
> + * Notify PHY after resume signal has finished, it is
> + * for global suspend case.
> + */
> + if (hcd->phy
> + && test_bit(port, &ehci->bus_suspended)
> + && (portsc & PORT_CONNECT)
> + && (ehci_port_speed(ehci, portsc) ==
> + USB_PORT_STAT_HIGH_SPEED))
> + /* notify the USB PHY */
> + usb_phy_notify_resume(hcd->phy, USB_SPEED_HIGH);
> + }
> +
> + return 0;
> +}
^ permalink raw reply [flat|nested] 21+ messages in thread* [Patch v2 09/10] usb: chipidea: host: add ehci quirk for imx controller
2013-10-23 14:46 ` Alan Stern
@ 2013-10-23 18:06 ` Felipe Balbi
2013-10-23 18:45 ` Alan Stern
0 siblings, 1 reply; 21+ messages in thread
From: Felipe Balbi @ 2013-10-23 18:06 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On Wed, Oct 23, 2013 at 10:46:09AM -0400, Alan Stern wrote:
> On Tue, 22 Oct 2013, Peter Chen wrote:
>
> > When the port goes to suspend or finishes resme, it needs to
> > notify PHY, it is not a standard EHCI operation, so we add a
> > quirk for it.
>
> Actually, this _should_ be a standard EHCI operation. But we have to
> figure out a way to do it that will work on all platforms.
>
> Felipe, any ideas?
isn't it enough to call usb_phy_set_suspend() at apropriate places ?
This should work on all platforms if the PHY driver is implemented
correctly.
--
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131023/0d6262c5/attachment.sig>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Patch v2 09/10] usb: chipidea: host: add ehci quirk for imx controller
2013-10-23 18:06 ` Felipe Balbi
@ 2013-10-23 18:45 ` Alan Stern
2013-10-24 1:33 ` Peter Chen
0 siblings, 1 reply; 21+ messages in thread
From: Alan Stern @ 2013-10-23 18:45 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, 23 Oct 2013, Felipe Balbi wrote:
> Hi,
>
> On Wed, Oct 23, 2013 at 10:46:09AM -0400, Alan Stern wrote:
> > On Tue, 22 Oct 2013, Peter Chen wrote:
> >
> > > When the port goes to suspend or finishes resme, it needs to
> > > notify PHY, it is not a standard EHCI operation, so we add a
> > > quirk for it.
> >
> > Actually, this _should_ be a standard EHCI operation. But we have to
> > figure out a way to do it that will work on all platforms.
> >
> > Felipe, any ideas?
>
> isn't it enough to call usb_phy_set_suspend() at apropriate places ?
>
> This should work on all platforms if the PHY driver is implemented
> correctly.
On Tue, 22 Oct 2013, Peter Chen wrote:
> + /*
> + * notify the USB PHY, it is for global
> + * suspend case.
> + */
> + usb_phy_notify_suspend(hcd->phy,
> + USB_SPEED_HIGH);
Hmmm. This calls usb_phy_notify_suspend(), and later on,
usb_phy_notify_resume(). AFAICT, those routines don't exist.
Instead we have usb_phy_set_suspend(), as Felipe mentioned. It has a
different interface, because the second argument specifies whether we
are entering or leaving suspend, not the connection speed.
Peter, you need to straighten this out.
Also, there's the question of where are the appropriate places to make
the calls? After the root hub goes into suspend, I suppose. But when
is the right time during resume?
And what if there is more than one port on the root hub?
Alan Stern
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Patch v2 09/10] usb: chipidea: host: add ehci quirk for imx controller
2013-10-23 18:45 ` Alan Stern
@ 2013-10-24 1:33 ` Peter Chen
2013-10-24 12:04 ` Felipe Balbi
0 siblings, 1 reply; 21+ messages in thread
From: Peter Chen @ 2013-10-24 1:33 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Oct 23, 2013 at 02:45:39PM -0400, Alan Stern wrote:
> On Wed, 23 Oct 2013, Felipe Balbi wrote:
>
> > Hi,
> >
> > On Wed, Oct 23, 2013 at 10:46:09AM -0400, Alan Stern wrote:
> > > On Tue, 22 Oct 2013, Peter Chen wrote:
> > >
> > > > When the port goes to suspend or finishes resme, it needs to
> > > > notify PHY, it is not a standard EHCI operation, so we add a
> > > > quirk for it.
> > >
> > > Actually, this _should_ be a standard EHCI operation. But we have to
> > > figure out a way to do it that will work on all platforms.
> > >
> > > Felipe, any ideas?
> >
> > isn't it enough to call usb_phy_set_suspend() at apropriate places ?
> >
> > This should work on all platforms if the PHY driver is implemented
> > correctly.
>
> On Tue, 22 Oct 2013, Peter Chen wrote:
>
> > + /*
> > + * notify the USB PHY, it is for global
> > + * suspend case.
> > + */
> > + usb_phy_notify_suspend(hcd->phy,
> > + USB_SPEED_HIGH);
>
> Hmmm. This calls usb_phy_notify_suspend(), and later on,
> usb_phy_notify_resume(). AFAICT, those routines don't exist.
>
Hi Alan and Felipe,
It is at my another patchset: "Add power management support for MXS PHY"
http://marc.info/?l=linux-usb&m=138242248913823&w=2
Since they are two things, one is from PHY, the other is for controller.
I split them to two patchset.
For these two notifications, please see:
[Patch v2 07/14] usb: phy: add notify suspend and resume callback
http://marc.info/?l=linux-usb&m=138242252713848&w=2
[Patch v2 08/14] usb: phy-mxs: Add implementation of
nofity_suspend and notify_resume
http://marc.info/?l=linux-usb&m=138242253313856&w=2
> Instead we have usb_phy_set_suspend(), as Felipe mentioned. It has a
> different interface, because the second argument specifies whether we
> are entering or leaving suspend, not the connection speed.
>
> Peter, you need to straighten this out.
.set_suspend is different with .notify_suspend.
.set_suspend is called when the PHY enters suspend.
.notify_suspend is called when port enters suspends (setting portsc.suspendM,
and stop sending SOF).
After calling .notify_suspend, the PHY may still need to access,
eg, set PHY wakeup setting, etc.
The .notify_suspend is in ehci operation, .set_suspend can be at
controller driver, since when to put the PHY enters suspend may
platform specific.
>
> Also, there's the question of where are the appropriate places to make
> the calls? After the root hub goes into suspend, I suppose. But when
> is the right time during resume?
>
If we can take it as standard EHCI operation, we need to put it at below
places:
For .notify_suspend, we need to put it after set portsc.suspendM.
- For selective suspend, at ehci_hub_control
- For global suspend, at ehci_bus_suspend
For .notify_resume, we need to put it after clear portsc.fpr,
it is similar for .notify_suspend
- For selective suspend, at ehci_hub_control
- For global suspend, at ehci_bus_resume
--
Best Regards,
Peter Chen
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Patch v2 09/10] usb: chipidea: host: add ehci quirk for imx controller
2013-10-24 1:33 ` Peter Chen
@ 2013-10-24 12:04 ` Felipe Balbi
2013-10-24 12:11 ` Peter Chen
0 siblings, 1 reply; 21+ messages in thread
From: Felipe Balbi @ 2013-10-24 12:04 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On Thu, Oct 24, 2013 at 09:33:26AM +0800, Peter Chen wrote:
> On Wed, Oct 23, 2013 at 02:45:39PM -0400, Alan Stern wrote:
> > On Wed, 23 Oct 2013, Felipe Balbi wrote:
> >
> > > Hi,
> > >
> > > On Wed, Oct 23, 2013 at 10:46:09AM -0400, Alan Stern wrote:
> > > > On Tue, 22 Oct 2013, Peter Chen wrote:
> > > >
> > > > > When the port goes to suspend or finishes resme, it needs to
> > > > > notify PHY, it is not a standard EHCI operation, so we add a
> > > > > quirk for it.
> > > >
> > > > Actually, this _should_ be a standard EHCI operation. But we have to
> > > > figure out a way to do it that will work on all platforms.
> > > >
> > > > Felipe, any ideas?
> > >
> > > isn't it enough to call usb_phy_set_suspend() at apropriate places ?
> > >
> > > This should work on all platforms if the PHY driver is implemented
> > > correctly.
> >
> > On Tue, 22 Oct 2013, Peter Chen wrote:
> >
> > > + /*
> > > + * notify the USB PHY, it is for global
> > > + * suspend case.
> > > + */
> > > + usb_phy_notify_suspend(hcd->phy,
> > > + USB_SPEED_HIGH);
> >
> > Hmmm. This calls usb_phy_notify_suspend(), and later on,
> > usb_phy_notify_resume(). AFAICT, those routines don't exist.
> >
>
> Hi Alan and Felipe,
>
> It is at my another patchset: "Add power management support for MXS PHY"
> http://marc.info/?l=linux-usb&m=138242248913823&w=2
> Since they are two things, one is from PHY, the other is for controller.
> I split them to two patchset.
>
> For these two notifications, please see:
> [Patch v2 07/14] usb: phy: add notify suspend and resume callback
> http://marc.info/?l=linux-usb&m=138242252713848&w=2
>
> [Patch v2 08/14] usb: phy-mxs: Add implementation of
> nofity_suspend and notify_resume
> http://marc.info/?l=linux-usb&m=138242253313856&w=2
>
> > Instead we have usb_phy_set_suspend(), as Felipe mentioned. It has a
> > different interface, because the second argument specifies whether we
> > are entering or leaving suspend, not the connection speed.
> >
> > Peter, you need to straighten this out.
>
> .set_suspend is different with .notify_suspend.
>
> .set_suspend is called when the PHY enters suspend.
> .notify_suspend is called when port enters suspends (setting portsc.suspendM,
> and stop sending SOF).
>
> After calling .notify_suspend, the PHY may still need to access,
> eg, set PHY wakeup setting, etc.
right, this just means we need usage counters for the PHY layer. So that
usb_phy_set_suspend() only decreaments a counter and will only suspend
the PHY when said counter reaches zero.
Also, why can't you *always* setup wakeup events for the PHY ? Everytime
you're about to suspend the PHY, enable its wakeup events.
> The .notify_suspend is in ehci operation, .set_suspend can be at
> controller driver, since when to put the PHY enters suspend may
> platform specific.
not really, what's platform specific is *HOW* to suspend the PHY.
*WHERE* to tell it to suspend is really more of a policy decision.
> > Also, there's the question of where are the appropriate places to make
> > the calls? After the root hub goes into suspend, I suppose. But when
> > is the right time during resume?
> >
>
> If we can take it as standard EHCI operation, we need to put it at below
> places:
>
> For .notify_suspend, we need to put it after set portsc.suspendM.
why do you need this notification for suspendM ? PHY should
automatically enter a lower power state automatically when suspendM
signal is asserted.
--
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131024/182e5a77/attachment-0001.sig>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Patch v2 09/10] usb: chipidea: host: add ehci quirk for imx controller
2013-10-24 12:04 ` Felipe Balbi
@ 2013-10-24 12:11 ` Peter Chen
0 siblings, 0 replies; 21+ messages in thread
From: Peter Chen @ 2013-10-24 12:11 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Oct 24, 2013 at 07:04:11AM -0500, Felipe Balbi wrote:
> Hi,
>
> On Thu, Oct 24, 2013 at 09:33:26AM +0800, Peter Chen wrote:
> > On Wed, Oct 23, 2013 at 02:45:39PM -0400, Alan Stern wrote:
> > > On Wed, 23 Oct 2013, Felipe Balbi wrote:
> > >
> > > > Hi,
> > > >
> > > > On Wed, Oct 23, 2013 at 10:46:09AM -0400, Alan Stern wrote:
> > > > > On Tue, 22 Oct 2013, Peter Chen wrote:
> > > > >
> > > > > > When the port goes to suspend or finishes resme, it needs to
> > > > > > notify PHY, it is not a standard EHCI operation, so we add a
> > > > > > quirk for it.
> > > > >
> > > > > Actually, this _should_ be a standard EHCI operation. But we have to
> > > > > figure out a way to do it that will work on all platforms.
> > > > >
> > > > > Felipe, any ideas?
> > > >
> > > > isn't it enough to call usb_phy_set_suspend() at apropriate places ?
> > > >
> > > > This should work on all platforms if the PHY driver is implemented
> > > > correctly.
> > >
> > > On Tue, 22 Oct 2013, Peter Chen wrote:
> > >
> > > > + /*
> > > > + * notify the USB PHY, it is for global
> > > > + * suspend case.
> > > > + */
> > > > + usb_phy_notify_suspend(hcd->phy,
> > > > + USB_SPEED_HIGH);
> > >
> > > Hmmm. This calls usb_phy_notify_suspend(), and later on,
> > > usb_phy_notify_resume(). AFAICT, those routines don't exist.
> > >
> >
> > Hi Alan and Felipe,
> >
> > It is at my another patchset: "Add power management support for MXS PHY"
> > http://marc.info/?l=linux-usb&m=138242248913823&w=2
> > Since they are two things, one is from PHY, the other is for controller.
> > I split them to two patchset.
> >
> > For these two notifications, please see:
> > [Patch v2 07/14] usb: phy: add notify suspend and resume callback
> > http://marc.info/?l=linux-usb&m=138242252713848&w=2
> >
> > [Patch v2 08/14] usb: phy-mxs: Add implementation of
> > nofity_suspend and notify_resume
> > http://marc.info/?l=linux-usb&m=138242253313856&w=2
> >
> > > Instead we have usb_phy_set_suspend(), as Felipe mentioned. It has a
> > > different interface, because the second argument specifies whether we
> > > are entering or leaving suspend, not the connection speed.
> > >
> > > Peter, you need to straighten this out.
> >
> > .set_suspend is different with .notify_suspend.
> >
> > .set_suspend is called when the PHY enters suspend.
> > .notify_suspend is called when port enters suspends (setting portsc.suspendM,
> > and stop sending SOF).
> >
> > After calling .notify_suspend, the PHY may still need to access,
> > eg, set PHY wakeup setting, etc.
>
> right, this just means we need usage counters for the PHY layer. So that
> usb_phy_set_suspend() only decreaments a counter and will only suspend
> the PHY when said counter reaches zero.
>
> Also, why can't you *always* setup wakeup events for the PHY ? Everytime
> you're about to suspend the PHY, enable its wakeup events.
Most of cases, the user doesn't want USB wakeup event. At these cases,
if the PHY's wakeup is set, when external signal changes (id/vbus/dpdm),
the PHY will be woken up and more power consumption will be.
>
> > The .notify_suspend is in ehci operation, .set_suspend can be at
> > controller driver, since when to put the PHY enters suspend may
> > platform specific.
>
> not really, what's platform specific is *HOW* to suspend the PHY.
> *WHERE* to tell it to suspend is really more of a policy decision.
>
How to suspend the PHY is indeed platform specific, but sometimes,
the whole suspend PHY operation may not only include PHY register, but also
needs controller register, Eg, for chipidea controller, if we have not used
chipidea PHY, we need to control both controller register (portsc.phcd)
and PHY's register (Different register mapping), so it is hard
to include two operations at usb_phy_set_suspend, we have to put them
at controller's suspend.
> > > Also, there's the question of where are the appropriate places to make
> > > the calls? After the root hub goes into suspend, I suppose. But when
> > > is the right time during resume?
> > >
> >
> > If we can take it as standard EHCI operation, we need to put it at below
> > places:
> >
> > For .notify_suspend, we need to put it after set portsc.suspendM.
>
> why do you need this notification for suspendM ? PHY should
> automatically enter a lower power state automatically when suspendM
> signal is asserted.
>
For some PHYs, it may correct. For chipidea controller, it needs to set
portsc.phcd to let the PHY enter suspend. The software needs to control it.
--
Best Regards,
Peter Chen
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Patch v2 10/10] usb: chipidea: imx: Enable CI_HDRC_IMX_EHCI_QUIRK if the phy has notify APIs
2013-10-22 6:23 [Patch v2 00/10] Add power management support for chipidea Peter Chen
` (8 preceding siblings ...)
2013-10-22 6:23 ` [Patch v2 09/10] usb: chipidea: host: add ehci quirk for imx controller Peter Chen
@ 2013-10-22 6:23 ` Peter Chen
9 siblings, 0 replies; 21+ messages in thread
From: Peter Chen @ 2013-10-22 6:23 UTC (permalink / raw)
To: linux-arm-kernel
If the PHY has .notify_suspend and .notify_resume, it means this
imx usb controller needs to add quirks for standard ehci routine.
Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
drivers/usb/chipidea/ci_hdrc_imx.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
index d4775b3..4045c76 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -118,6 +118,11 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
pdata.phy = data->phy;
+ /* notify APIs are used for adding quirks at standard EHCI routine */
+ if (pdata.phy && pdata.phy->notify_suspend
+ && pdata.phy->notify_resume)
+ pdata.flags |= CI_HDRC_IMX_EHCI_QUIRK;
+
if (of_device_is_compatible(np, "fsl,imx6q-usb")) {
pdata.flags |= CI_HDRC_SUPPORTS_RUNTIME_PM;
data->supports_runtime_pm = true;
--
1.7.1
^ permalink raw reply related [flat|nested] 21+ messages in thread