[PATCH v2 1/4] usb: chipidea: remove the unnecessary delay after clear portsc.phcd

[Peter Chen]

The individual PHY driver should take this responsibility if it
needs to delay between clear portsc.phcd and let the phy leave
low power mode.

Signed-off-by: Peter Chen <peter.chen-***@public.gmane.org>
---
drivers/usb/chipidea/core.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 0d5e7b1..409aba3 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -195,18 +195,12 @@ static void ci_hdrc_enter_lpm(struct ci_hdrc *ci, bool enable)
enum ci_hw_regs reg = ci->hw_bank.lpm ? OP_DEVLC : OP_PORTSC;
bool lpm = !!(hw_read(ci, reg, PORTSC_PHCD(ci->hw_bank.lpm)));

- if (enable && !lpm) {
+ if (enable && !lpm)
hw_write(ci, reg, PORTSC_PHCD(ci->hw_bank.lpm),
PORTSC_PHCD(ci->hw_bank.lpm));
- } else if (!enable && lpm) {
+ else if (!enable && lpm)
hw_write(ci, reg, PORTSC_PHCD(ci->hw_bank.lpm),
0);
- /*
- * the PHY needs some time (less
- * than 1ms) to leave low power mode.
- */
- usleep_range(1000, 1100);
- }
}

static int hw_device_init(struct ci_hdrc *ci, void __iomem *base)

[Peter Chen]

The phy needs some delay to output the stable status from low
power mode. And for OTGSC, the status inputs are debounced
using a 1 ms time constant, so, delay 2ms for controller to get
the stable status(like vbus and id) when the phy leaves low power.

Signed-off-by: Peter Chen <peter.chen-***@public.gmane.org>
---
drivers/usb/chipidea/core.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 409aba3..ac7aa7e 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -189,6 +189,17 @@ u8 hw_port_test_get(struct ci_hdrc *ci)
return hw_read(ci, OP_PORTSC, PORTSC_PTC) >> __ffs(PORTSC_PTC);
}

+static void hw_wait_phy_stable(void)
+{
+ /*
+ * The phy needs some delay to output the stable status from low
+ * power mode. And for OTGSC, the status inputs are debounced
+ * using a 1 ms time constant, so, delay 2ms for controller to get
+ * the stable status, like vbus and id when the phy leaves low power.
+ */
+ usleep_range(2000, 2500);
+}
+
/* The PHY enters/leaves low power mode */
static void ci_hdrc_enter_lpm(struct ci_hdrc *ci, bool enable)
{
@@ -307,7 +318,9 @@ static int ci_usb_phy_init(struct ci_hdrc *ci)
case USBPHY_INTERFACE_MODE_UTMIW:
case USBPHY_INTERFACE_MODE_HSIC:
ret = usb_phy_init(ci->transceiver);
- if (ret)
+ if (!ret)
+ hw_wait_phy_stable();
+ else
return ret;
hw_phymode_configure(ci);
break;
@@ -320,6 +333,8 @@ static int ci_usb_phy_init(struct ci_hdrc *ci)
break;
default:
ret = usb_phy_init(ci->transceiver);
+ if (!ret)
+ hw_wait_phy_stable();
}

return ret;
@@ -620,13 +635,6 @@ static int ci_hdrc_probe(struct platform_device *pdev)
if (ret) {
dev_err(dev, "unable to init phy: %d\n", ret);
return ret;
- } else {
- /*
- * The delay to sync PHY's status, the maximum delay is
- * 2ms since the otgsc uses 1ms timer to debounce the
- * PHY's input
- */
- usleep_range(2000, 2500);
}

ci->hw_bank.phys = res->start;

[David Laight]

From: Peter Chen

Post by Peter Chen
The phy needs some delay to output the stable status from low
power mode. And for OTGSC, the status inputs are debounced
using a 1 ms time constant, so, delay 2ms for controller to get
the stable status(like vbus and id) when the phy leaves low power.
---
drivers/usb/chipidea/core.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 409aba3..ac7aa7e 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -189,6 +189,17 @@ u8 hw_port_test_get(struct ci_hdrc *ci)
return hw_read(ci, OP_PORTSC, PORTSC_PTC) >> __ffs(PORTSC_PTC);
}
+static void hw_wait_phy_stable(void)
+{
+ /*
+ * The phy needs some delay to output the stable status from low
+ * power mode. And for OTGSC, the status inputs are debounced
+ * using a 1 ms time constant, so, delay 2ms for controller to get
+ * the stable status, like vbus and id when the phy leaves low power.
+ */
+ usleep_range(2000, 2500);
+}
+
/* The PHY enters/leaves low power mode */
static void ci_hdrc_enter_lpm(struct ci_hdrc *ci, bool enable)
{
@@ -307,7 +318,9 @@ static int ci_usb_phy_init(struct ci_hdrc *ci)
ret = usb_phy_init(ci->transceiver);
- if (ret)
+ if (!ret)
+ hw_wait_phy_stable();
+ else
return ret;

Why invert the condition.

Post by Peter Chen
hw_phymode_configure(ci);
break;
@@ -320,6 +333,8 @@ static int ci_usb_phy_init(struct ci_hdrc *ci)
break;
ret = usb_phy_init(ci->transceiver);
+ if (!ret)
+ hw_wait_phy_stable();
}
return ret;
@@ -620,13 +635,6 @@ static int ci_hdrc_probe(struct platform_device *pdev)
if (ret) {
dev_err(dev, "unable to init phy: %d\n", ret);
return ret;
- } else {

You don't need else after return.

David

Post by Peter Chen
- /*
- * The delay to sync PHY's status, the maximum delay is
- * 2ms since the otgsc uses 1ms timer to debounce the
- * PHY's input
- */
- usleep_range(2000, 2500);
}
ci->hw_bank.phys = res->start;
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
More majordomo info at http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Peter Chen]

Post by David Laight
From: Peter Chen

Post by Peter Chen
The phy needs some delay to output the stable status from low
power mode. And for OTGSC, the status inputs are debounced
using a 1 ms time constant, so, delay 2ms for controller to get
the stable status(like vbus and id) when the phy leaves low power.
---
drivers/usb/chipidea/core.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 409aba3..ac7aa7e 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -189,6 +189,17 @@ u8 hw_port_test_get(struct ci_hdrc *ci)
return hw_read(ci, OP_PORTSC, PORTSC_PTC) >> __ffs(PORTSC_PTC);
}
+static void hw_wait_phy_stable(void)
+{
+ /*
+ * The phy needs some delay to output the stable status from low
+ * power mode. And for OTGSC, the status inputs are debounced
+ * using a 1 ms time constant, so, delay 2ms for controller to get
+ * the stable status, like vbus and id when the phy leaves low power.
+ */
+ usleep_range(2000, 2500);
+}
+
/* The PHY enters/leaves low power mode */
static void ci_hdrc_enter_lpm(struct ci_hdrc *ci, bool enable)
{
@@ -307,7 +318,9 @@ static int ci_usb_phy_init(struct ci_hdrc *ci)
ret = usb_phy_init(ci->transceiver);
- if (ret)
+ if (!ret)
+ hw_wait_phy_stable();
+ else
return ret;

Why invert the condition.

If usb_phy_init fails, we want to return, and don't need to call
hw_wait_phy_stable.

Post by David Laight

Post by Peter Chen
hw_phymode_configure(ci);
break;
@@ -320,6 +333,8 @@ static int ci_usb_phy_init(struct ci_hdrc *ci)
break;
ret = usb_phy_init(ci->transceiver);
+ if (!ret)
+ hw_wait_phy_stable();
}
return ret;
@@ -620,13 +635,6 @@ static int ci_hdrc_probe(struct platform_device *pdev)
if (ret) {
dev_err(dev, "unable to init phy: %d\n", ret);
return ret;
- } else {

You don't need else after return.

Yes, isn't it?

Post by David Laight
David

Post by Peter Chen
- /*
- * The delay to sync PHY's status, the maximum delay is
- * 2ms since the otgsc uses 1ms timer to debounce the
- * PHY's input
- */
- usleep_range(2000, 2500);
}
ci->hw_bank.phys = res->start;
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Peter Chen]

Add system power management support

Signed-off-by: Peter Chen <peter.chen-***@public.gmane.org>
---
drivers/usb/chipidea/core.c | 48 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 48 insertions(+)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index ac7aa7e..466ddf4 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -745,11 +745,59 @@ static int ci_hdrc_remove(struct platform_device *pdev)
return 0;
}

+#ifdef CONFIG_PM_SLEEP
+static void ci_controller_suspend(struct ci_hdrc *ci)
+{
+ ci_hdrc_enter_lpm(ci, true);
+
+ if (ci->transceiver)
+ usb_phy_set_suspend(ci->transceiver, 1);
+}
+
+static int ci_controller_resume(struct device *dev)
+{
+ struct ci_hdrc *ci = dev_get_drvdata(dev);
+
+ dev_dbg(dev, "at %s\n", __func__);
+
+ ci_hdrc_enter_lpm(ci, false);
+
+ if (ci->transceiver) {
+ usb_phy_set_suspend(ci->transceiver, 0);
+ usb_phy_set_wakeup(ci->transceiver, false);
+ hw_wait_phy_stable();
+ }
+
+ return 0;
+}
+
+static int ci_suspend(struct device *dev)
+{
+ struct ci_hdrc *ci = dev_get_drvdata(dev);
+
+ if (ci->wq)
+ flush_workqueue(ci->wq);
+
+ ci_controller_suspend(ci);
+
+ return 0;
+}
+
+static int ci_resume(struct device *dev)
+{
+ return ci_controller_resume(dev);
+}
+#endif /* CONFIG_PM_SLEEP */
+
+static const struct dev_pm_ops ci_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(ci_suspend, ci_resume)
+};
static struct platform_driver ci_hdrc_driver = {
.probe = ci_hdrc_probe,
.remove = ci_hdrc_remove,
.driver = {
.name = "ci_hdrc",
+ .pm = &ci_pm_ops,
.owner = THIS_MODULE,
},
};

[Fabio Estevam]

Post by Peter Chen
+static const struct dev_pm_ops ci_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(ci_suspend, ci_resume)
+};

You could replace this with a single line:

static SIMPLE_DEV_PM_OPS(ci_pm_ops, ci_suspend, ci_resume);
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Peter Chen]

Post by Fabio Estevam

Post by Peter Chen
+static const struct dev_pm_ops ci_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(ci_suspend, ci_resume)
+};

static SIMPLE_DEV_PM_OPS(ci_pm_ops, ci_suspend, ci_resume);

Since I plan to add runtime pm soon, don't want to change again at that time.

[Peter Chen]

Add basic system power management support

Signed-off-by: Peter Chen <peter.chen-***@public.gmane.org>
---
drivers/usb/chipidea/ci_hdrc_imx.c | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)

diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
index 74b5b09..aa66199 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -208,6 +208,41 @@ static int ci_hdrc_imx_remove(struct platform_device *pdev)
return 0;
}

+#ifdef CONFIG_PM_SLEEP
+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__);
+
+ clk_disable_unprepare(data->clk);
+
+ return 0;
+}
+
+static int imx_controller_resume(struct device *dev)
+{
+ struct ci_hdrc_imx_data *data = dev_get_drvdata(dev);
+
+ dev_dbg(dev, "at %s\n", __func__);
+
+ return clk_prepare_enable(data->clk);
+}
+
+static int ci_hdrc_imx_suspend(struct device *dev)
+{
+ return imx_controller_suspend(dev);
+}
+
+static int ci_hdrc_imx_resume(struct device *dev)
+{
+ return imx_controller_resume(dev);
+}
+#endif /* CONFIG_PM_SLEEP */
+
+static const struct dev_pm_ops ci_hdrc_imx_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(ci_hdrc_imx_suspend, ci_hdrc_imx_resume)
+};
static struct platform_driver ci_hdrc_imx_driver = {
.probe = ci_hdrc_imx_probe,
.remove = ci_hdrc_imx_remove,
@@ -215,6 +250,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,
},
};