[PATCH 0/4] usb: davinci: Add PM runtime support and improve HOST only mode support

[PATCH 0/4] usb: davinci: Add PM runtime support and improve HOST only mode support

[Bastien Curutchet]

Hi all,

I've worked on adding PM runtime support on the DAVINCI's USB PHY
controller. It revealed some bugs in the MUSB controller that I fixed in
the last three patches.

The platform I work on uses the MUSB controller in HOST-only mode and has
been designed to save as much energy as possible. To do so, the USB's
VBUS supply can be cut off by toggling a GPIO, this triggers the bug
fixed by patch 2.

If an USB stick gets unplugged, then plugged back while the PHY is in
suspend state, a BABBLE error happens. This BABBLE can't be recovered.
This is fixed by last patch.

Bastien Curutchet (4):
  phy: ti: phy-da8xx-usb: Add runtime PM support
  Revert "usb: musb: da8xx: Set phy in OTG mode by default"
  usb: musb: da8xx: Remove try_idle implementation from host-only mode
  usb: musb: da8xx: Implement BABBLE recovery

 drivers/phy/ti/phy-da8xx-usb.c | 49 ++++++++++++++++++++++++++++++++++
 drivers/usb/musb/da8xx.c       | 20 ++++++++------
 2 files changed, 61 insertions(+), 8 deletions(-)

-- 
2.44.0


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH 1/4] phy: ti: phy-da8xx-usb: Add runtime PM support

[Bastien Curutchet]

Runtime PM is not supported while USB PHY can be turned off from
register accesses.

Add runtime PM for the USB2.0 PHY. The PHY is entirely shut down to save
as much power as possible. This means that gadgets will not be discovered
once suspend state is entered, and suspend state can not be left without
an explicit user intervention (through sysfs). That's why runtime PM is
disabled by default.

Signed-off-by: Bastien Curutchet <bastien.curutchet@bootlin.com>
---
 drivers/phy/ti/phy-da8xx-usb.c | 49 ++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/drivers/phy/ti/phy-da8xx-usb.c b/drivers/phy/ti/phy-da8xx-usb.c
index dc614aa09a5f..80bfe37cf846 100644
--- a/drivers/phy/ti/phy-da8xx-usb.c
+++ b/drivers/phy/ti/phy-da8xx-usb.c
@@ -15,11 +15,13 @@
 #include <linux/phy/phy.h>
 #include <linux/platform_data/phy-da8xx-usb.h>
 #include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 
 #define PHY_INIT_BITS	(CFGCHIP2_SESENDEN | CFGCHIP2_VBDTCTEN)
 
 struct da8xx_usb_phy {
+	struct device		*dev;
 	struct phy_provider	*phy_provider;
 	struct phy		*usb11_phy;
 	struct phy		*usb20_phy;
@@ -40,6 +42,12 @@ static int da8xx_usb11_phy_power_on(struct phy *phy)
 	regmap_write_bits(d_phy->regmap, CFGCHIP(2), CFGCHIP2_USB1SUSPENDM,
 			  CFGCHIP2_USB1SUSPENDM);
 
+	/*
+	 * USB1.1 can used USB2.0 output clock as reference clock so this is here to prevent USB2.0
+	 * from shutting PHY's power when USB1.1 might use it
+	 */
+	pm_runtime_get_sync(d_phy->dev);
+
 	return 0;
 }
 
@@ -50,6 +58,7 @@ static int da8xx_usb11_phy_power_off(struct phy *phy)
 	regmap_write_bits(d_phy->regmap, CFGCHIP(2), CFGCHIP2_USB1SUSPENDM, 0);
 
 	clk_disable_unprepare(d_phy->usb11_clk);
+	pm_runtime_put_sync(d_phy->dev);
 
 	return 0;
 }
@@ -119,6 +128,35 @@ static const struct phy_ops da8xx_usb20_phy_ops = {
 	.owner		= THIS_MODULE,
 };
 
+static int __maybe_unused da8xx_runtime_suspend(struct device *dev)
+{
+	struct da8xx_usb_phy *d_phy = dev_get_drvdata(dev);
+
+	dev_dbg(dev, "Suspending ...\n");
+
+	regmap_set_bits(d_phy->regmap, CFGCHIP(2), CFGCHIP2_PHYPWRDN | CFGCHIP2_OTGPWRDN);
+
+	return 0;
+}
+
+static int __maybe_unused da8xx_runtime_resume(struct device *dev)
+{
+	u32 mask = CFGCHIP2_RESET | CFGCHIP2_PHYPWRDN | CFGCHIP2_OTGPWRDN | CFGCHIP2_PHY_PLLON;
+	struct da8xx_usb_phy *d_phy = dev_get_drvdata(dev);
+	u32 pll_status;
+
+	regmap_update_bits(d_phy->regmap, CFGCHIP(2), mask, CFGCHIP2_PHY_PLLON);
+
+	dev_dbg(dev, "Resuming ...\n");
+
+	return regmap_read_poll_timeout(d_phy->regmap, CFGCHIP(2), pll_status,
+					pll_status & CFGCHIP2_PHYCLKGD, 1000, 500000);
+}
+
+static const struct dev_pm_ops da8xx_usb_phy_pm_ops = {
+	SET_RUNTIME_PM_OPS(da8xx_runtime_suspend, da8xx_runtime_resume, NULL)
+};
+
 static struct phy *da8xx_usb_phy_of_xlate(struct device *dev,
 					 const struct of_phandle_args *args)
 {
@@ -148,6 +186,8 @@ static int da8xx_usb_phy_probe(struct platform_device *pdev)
 	if (!d_phy)
 		return -ENOMEM;
 
+	d_phy->dev = dev;
+
 	if (pdata)
 		d_phy->regmap = pdata->cfgchip;
 	else
@@ -209,6 +249,14 @@ static int da8xx_usb_phy_probe(struct platform_device *pdev)
 	regmap_write_bits(d_phy->regmap, CFGCHIP(2),
 			  PHY_INIT_BITS, PHY_INIT_BITS);
 
+	pm_runtime_set_active(dev);
+	devm_pm_runtime_enable(dev);
+	/*
+	 * Prevent runtime pm from being ON by default. Users can enable
+	 * it using power/control in sysfs.
+	 */
+	pm_runtime_forbid(dev);
+
 	return 0;
 }
 
@@ -233,6 +281,7 @@ static struct platform_driver da8xx_usb_phy_driver = {
 	.remove_new = da8xx_usb_phy_remove,
 	.driver	= {
 		.name	= "da8xx-usb-phy",
+		.pm	= &da8xx_usb_phy_pm_ops,
 		.of_match_table = da8xx_usb_phy_ids,
 	},
 };
-- 
2.44.0


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH 1/4] phy: ti: phy-da8xx-usb: Add runtime PM support

[Vinod Koul]

On 28-05-24, 12:20, Bastien Curutchet wrote:
> Runtime PM is not supported while USB PHY can be turned off from
> register accesses.
> 
> Add runtime PM for the USB2.0 PHY. The PHY is entirely shut down to save
> as much power as possible. This means that gadgets will not be discovered
> once suspend state is entered, and suspend state can not be left without
> an explicit user intervention (through sysfs). That's why runtime PM is
> disabled by default.

Acked-by: Vinod Koul <vkoul@kernel.org>

-- 
~Vinod

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH 2/4] Revert "usb: musb: da8xx: Set phy in OTG mode by default"

[Bastien Curutchet]

This reverts commit 7ccf62941a38701ec9a42b4a0fa2868af456e96a.

da8xx_musb_set_mode() forces OTG mode regardless the selected mode even
if the property 'dr_mode = "host"' is present in device-tree. This
causes an unrecoverable error when VBUS supply is shut down : plugged
gadgets are no longer detected once VBUS supply is back.

Reverting it allows to have a selected mode coherent with device-tree's
description. IMO, this shouldn't cause regression because OTG mode is
the default selection when 'dr_mode' property is not set.

Signed-off-by: Bastien Curutchet <bastien.curutchet@bootlin.com>
---
 drivers/usb/musb/da8xx.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
index 8abf3a567e30..1681f9fba3da 100644
--- a/drivers/usb/musb/da8xx.c
+++ b/drivers/usb/musb/da8xx.c
@@ -328,13 +328,6 @@ static int da8xx_musb_set_mode(struct musb *musb, u8 musb_mode)
 	struct da8xx_glue *glue = dev_get_drvdata(musb->controller->parent);
 	enum phy_mode phy_mode;
 
-	/*
-	 * The PHY has some issues when it is forced in device or host mode.
-	 * Unless the user request another mode, configure the PHY in OTG mode.
-	 */
-	if (!musb->is_initialized)
-		return phy_set_mode(glue->phy, PHY_MODE_USB_OTG);
-
 	switch (musb_mode) {
 	case MUSB_HOST:		/* Force VBUS valid, ID = 0 */
 		phy_mode = PHY_MODE_USB_HOST;
-- 
2.44.0


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH 3/4] usb: musb: da8xx: Remove try_idle implementation from host-only mode

[Bastien Curutchet]

The idle_state is not meant for host-only mode. When the device acts as a
host, it fails to exit the idle state once entered. This prevents new
USB gadgets from being detected once plugged in.

Commit 032ec49f5351 ("usb: musb: drop useless board_mode usage") removed
a is_otg_enabled() check in the try_idle() that prevented from entering
idle_state. This was removed because at that time it was not possible to
select host-only mode with CONFIG_USB_MUSB_HOST so dual role was always
set.

Add #ifndef CONFIG_USB_MUSB_HOST around try_idle() callback to prevent
from entering idle_state when host-only mode has been selected.

Signed-off-by: Bastien Curutchet <bastien.curutchet@bootlin.com>
---
 drivers/usb/musb/da8xx.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
index 1681f9fba3da..c5cf733673a2 100644
--- a/drivers/usb/musb/da8xx.c
+++ b/drivers/usb/musb/da8xx.c
@@ -191,7 +191,7 @@ static void otg_timer(struct timer_list *t)
 	spin_unlock_irqrestore(&musb->lock, flags);
 }
 
-static void da8xx_musb_try_idle(struct musb *musb, unsigned long timeout)
+static void __maybe_unused da8xx_musb_try_idle(struct musb *musb, unsigned long timeout)
 {
 	static unsigned long last_timer;
 
@@ -476,7 +476,10 @@ static const struct musb_platform_ops da8xx_ops = {
 	.disable	= da8xx_musb_disable,
 
 	.set_mode	= da8xx_musb_set_mode,
+
+#ifndef CONFIG_USB_MUSB_HOST
 	.try_idle	= da8xx_musb_try_idle,
+#endif
 
 	.set_vbus	= da8xx_musb_set_vbus,
 };
-- 
2.44.0


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH 4/4] usb: musb: da8xx: Implement BABBLE recovery

[Bastien Curutchet]

There is no specific behaviour implemented to recover from a babble
error. When a BABBLE error happens, recovery fails as connected sticks
are no longer detected by the USB controller.

Implement the recover callback of the MUSB operation to reset the USB
controller when a BABBLE happens.

Signed-off-by: Bastien Curutchet <bastien.curutchet@bootlin.com>
---
 drivers/usb/musb/da8xx.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
index c5cf733673a2..fcf06dcf2d61 100644
--- a/drivers/usb/musb/da8xx.c
+++ b/drivers/usb/musb/da8xx.c
@@ -220,6 +220,13 @@ static void __maybe_unused da8xx_musb_try_idle(struct musb *musb, unsigned long
 	mod_timer(&musb->dev_timer, timeout);
 }
 
+static int da8xx_babble_recover(struct musb *musb)
+{
+	dev_dbg(musb->controller, "resetting controller to recover from babble\n");
+	musb_writel(musb->ctrl_base, DA8XX_USB_CTRL_REG, DA8XX_SOFT_RESET_MASK);
+	return 0;
+}
+
 static irqreturn_t da8xx_musb_interrupt(int irq, void *hci)
 {
 	struct musb		*musb = hci;
@@ -480,6 +487,7 @@ static const struct musb_platform_ops da8xx_ops = {
 #ifndef CONFIG_USB_MUSB_HOST
 	.try_idle	= da8xx_musb_try_idle,
 #endif
+	.recover	= da8xx_babble_recover,
 
 	.set_vbus	= da8xx_musb_set_vbus,
 };
-- 
2.44.0


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy