Linux-USB Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] usb: chipidea: host: Only disable regulator if it was enabled
@ 2019-06-14 17:37 Guenter Roeck
  0 siblings, 0 replies; only message in thread
From: Guenter Roeck @ 2019-06-14 17:37 UTC (permalink / raw)
  To: Peter Chen
  Cc: linux-usb, linux-kernel, Guenter Roeck, Michael Grzeschik,
	Li Jun, Peter Chen

On some systems, the regulator is never enabled if power is controlled by
ehci. This can happen since there is no explicit call to ehci_port_power()
from ehci_run(), but there is one from ehci_stop().

This results in tracebacks on shutdown.

WARNING: CPU: 0 PID: 182 at drivers/regulator/core.c:2597 _regulator_disable+0x190/0x1f8
unbalanced disables for usb_otg2_vbus
Modules linked in:
CPU: 0 PID: 182 Comm: init Not tainted 5.2.0-rc4-00045-gc11fb13a117e-dirty #6
Hardware name: Freescale i.MX7 Dual (Device Tree)
[<c0312f58>] (unwind_backtrace) from [<c030d70c>] (show_stack+0x10/0x14)
[<c030d70c>] (show_stack) from [<c114e4ec>] (dump_stack+0xd4/0x100)
[<c114e4ec>] (dump_stack) from [<c0347918>] (__warn+0xf4/0x10c)
[<c0347918>] (__warn) from [<c034797c>] (warn_slowpath_fmt+0x4c/0x70)
[<c034797c>] (warn_slowpath_fmt) from [<c09d0b10>] (_regulator_disable+0x190/0x1f8)
[<c09d0b10>] (_regulator_disable) from [<c09d0be0>] (regulator_disable+0x68/0xa8)
[<c09d0be0>] (regulator_disable) from [<c0e34f0c>] (ehci_ci_portpower+0x58/0x104)
[<c0e34f0c>] (ehci_ci_portpower) from [<c0df37e0>] (ehci_port_power+0x84/0xc8)
[<c0df37e0>] (ehci_port_power) from [<c0df3998>] (ehci_silence_controller+0x5c/0xc4)
[<c0df3998>] (ehci_silence_controller) from [<c0df5904>] (ehci_stop+0x3c/0xcc)
[<c0df5904>] (ehci_stop) from [<c0d9c4bc>] (usb_remove_hcd+0xf4/0x1b0)
[<c0d9c4bc>] (usb_remove_hcd) from [<c0e34978>] (host_stop+0x38/0xa8)
[<c0e34978>] (host_stop) from [<c0e2fdb8>] (ci_hdrc_remove+0x40/0xe4)
[<c0e2fdb8>] (ci_hdrc_remove) from [<c0b6a388>] (platform_drv_remove+0x20/0x40)
[<c0b6a388>] (platform_drv_remove) from [<c0b68894>] (device_release_driver_internal+0xdc/0x1ac)
[<c0b68894>] (device_release_driver_internal) from [<c0b6739c>] (bus_remove_device+0xd0/0xfc)
[<c0b6739c>] (bus_remove_device) from [<c0b63884>] (device_del+0x148/0x36c)
[<c0b63884>] (device_del) from [<c0b6a8a4>] (platform_device_del.part.3+0x10/0x74)
[<c0b6a8a4>] (platform_device_del.part.3) from [<c0b6a938>] (platform_device_unregister+0x1c/0x28)
[<c0b6a938>] (platform_device_unregister) from [<c0e2f394>] (ci_hdrc_remove_device+0xc/0x20)
[<c0e2f394>] (ci_hdrc_remove_device) from [<c0e36980>] (ci_hdrc_imx_remove+0x20/0xc4)
[<c0e36980>] (ci_hdrc_imx_remove) from [<c0b659d0>] (device_shutdown+0x17c/0x220)
[<c0b659d0>] (device_shutdown) from [<c03730d4>] (kernel_restart+0xc/0x50)
[<c03730d4>] (kernel_restart) from [<c03733c0>] (sys_reboot+0x15c/0x1ec)
[<c03733c0>] (sys_reboot) from [<c0301000>] (ret_fast_syscall+0x0/0x28)

The problem only affects imx7d systems, since all others set
CI_HDRC_TURN_VBUS_EARLY_ON. The regulator on those systems is controlled
by the chipidea driver and not by ehci.

Only disable the regulator if it was previously enabled.

Fixes: c8679a2fb8de ("usb: chipidea: host: add portpower override")
Cc: Michael Grzeschik <m.grzeschik@pengutronix.de>
Cc: Li Jun <jun.li@nxp.com>
Cc: Peter Chen <peter.chen@nxp.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
It might be better to implement a solution in ehci, but I have no idea how
that would look like. Overall it seems odd that imx7d systems work in the
first place, even though the regulator is never enabled. It may also be
that the problem is somehow related to the mcimx7d-sabre emulation in qemu
(which is where I see it), but I have no idea how that would affect
behavior such that ehci_ci_portpower() is never called with enable=true.

Yet another possibility might be to set CI_HDRC_TURN_VBUS_EARLY_ON for
imx7d, but that would effectively just hide the problem.

 drivers/usb/chipidea/host.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
index b45ceb91c735..3d63bf0e7c4b 100644
--- a/drivers/usb/chipidea/host.c
+++ b/drivers/usb/chipidea/host.c
@@ -26,6 +26,7 @@ static int (*orig_bus_suspend)(struct usb_hcd *hcd);
 
 struct ehci_ci_priv {
 	struct regulator *reg_vbus;
+	bool enabled;
 };
 
 static int ehci_ci_portpower(struct usb_hcd *hcd, int portnum, bool enable)
@@ -43,16 +44,20 @@ static int ehci_ci_portpower(struct usb_hcd *hcd, int portnum, bool enable)
 				"Not support multi-port regulator control\n");
 			return 0;
 		}
-		if (enable)
-			ret = regulator_enable(priv->reg_vbus);
-		else
-			ret = regulator_disable(priv->reg_vbus);
+		if (enable) {
+			if (!priv->enabled)
+				ret = regulator_enable(priv->reg_vbus);
+		} else {
+			if (priv->enabled)
+				ret = regulator_disable(priv->reg_vbus);
+		}
 		if (ret) {
 			dev_err(dev,
 				"Failed to %s vbus regulator, ret=%d\n",
 				enable ? "enable" : "disable", ret);
 			return ret;
 		}
+		priv->enabled = enable;
 	}
 
 	if (enable && (ci->platdata->phy_mode == USBPHY_INTERFACE_MODE_HSIC)) {
-- 
2.7.4


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, back to index

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-14 17:37 [PATCH] usb: chipidea: host: Only disable regulator if it was enabled Guenter Roeck

Linux-USB Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-usb/0 linux-usb/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-usb linux-usb/ https://lore.kernel.org/linux-usb \
		linux-usb@vger.kernel.org linux-usb@archiver.kernel.org
	public-inbox-index linux-usb


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-usb


AGPL code for this site: git clone https://public-inbox.org/ public-inbox