Message ID | 20191226155754.25451-1-linux@roeck-us.net (mailing list archive) |
---|---|
State | Mainlined |
Commit | c1ffba305dbcf3fb9ca969c20a97acbddc38f8e9 |
Headers | show |
Series | usb: chipidea: host: Disable port power only if previously enabled | expand |
On Thu, 26 Dec 2019, Guenter Roeck wrote: > On shutdown, ehci_power_off() is called unconditionally to power off > each port, even if it was never called to power on the port. > For chipidea, this results in a call to ehci_ci_portpower() with a request > to power off ports even if the port was never powered on. > This results in the following warning from the regulator code. That's weird -- we should always power-on every port during hub initialization. It looks like there's a bug in hub.c:hub_activate(): The line under HUB_INIT which calls hub_power_on() should call usb_hub_set_port_power() instead. In fact, the comment near the start of hub_power_on() is wrong. It says "Enable power on each port", but in fact it only enables power for ports that had been powered-on previously (i.e., the port's bit in hub->power_bits was set). Apparently this got messed up in commit ad493e5e5805 ("usb: add usb port auto power off mechanism"). Now, the chipidea driver may still need to be updated, because ehci_turn_off_all_ports() will still be called during shutdown and it will try to power-down all ports, even those which are already powered down (for example, because the port is suspended). Alan Stern
On 19-12-26 14:46:15, Alan Stern wrote: > On Thu, 26 Dec 2019, Guenter Roeck wrote: > > > On shutdown, ehci_power_off() is called unconditionally to power off > > each port, even if it was never called to power on the port. > > For chipidea, this results in a call to ehci_ci_portpower() with a request > > to power off ports even if the port was never powered on. > > This results in the following warning from the regulator code. > > That's weird -- we should always power-on every port during hub > initialization. > > It looks like there's a bug in hub.c:hub_activate(): The line under > HUB_INIT which calls hub_power_on() should call > usb_hub_set_port_power() instead. In fact, the comment near the start > of hub_power_on() is wrong. It says "Enable power on each port", but > in fact it only enables power for ports that had been powered-on > previously (i.e., the port's bit in hub->power_bits was set). > Apparently this got messed up in commit ad493e5e5805 ("usb: add usb > port auto power off mechanism"). > > Now, the chipidea driver may still need to be updated, because > ehci_turn_off_all_ports() will still be called during shutdown and it > will try to power-down all ports, even those which are already powered > down (for example, because the port is suspended). > Hi Alan, When the port is suspended, why it was turned off? If it was turned off, how could respond wakeup event?
On 19-12-26 07:57:54, Guenter Roeck wrote: > On shutdown, ehci_power_off() is called unconditionally to power off > each port, even if it was never called to power on the port. > For chipidea, this results in a call to ehci_ci_portpower() with a request > to power off ports even if the port was never powered on. > This results in the following warning from the regulator code. > > WARNING: CPU: 0 PID: 182 at drivers/regulator/core.c:2596 _regulator_disable+0x1a8/0x210 > unbalanced disables for usb_otg2_vbus > Modules linked in: > CPU: 0 PID: 182 Comm: init Not tainted 5.4.6 #1 > Hardware name: Freescale i.MX7 Dual (Device Tree) > [<c0313658>] (unwind_backtrace) from [<c030d698>] (show_stack+0x10/0x14) > [<c030d698>] (show_stack) from [<c1133afc>] (dump_stack+0xe0/0x10c) > [<c1133afc>] (dump_stack) from [<c0349098>] (__warn+0xf4/0x10c) > [<c0349098>] (__warn) from [<c0349128>] (warn_slowpath_fmt+0x78/0xbc) > [<c0349128>] (warn_slowpath_fmt) from [<c09f36ac>] (_regulator_disable+0x1a8/0x210) > [<c09f36ac>] (_regulator_disable) from [<c09f374c>] (regulator_disable+0x38/0xe8) > [<c09f374c>] (regulator_disable) from [<c0df7bac>] (ehci_ci_portpower+0x38/0xdc) > [<c0df7bac>] (ehci_ci_portpower) from [<c0db4fa4>] (ehci_port_power+0x50/0xa4) > [<c0db4fa4>] (ehci_port_power) from [<c0db5420>] (ehci_silence_controller+0x5c/0xc4) > [<c0db5420>] (ehci_silence_controller) from [<c0db7644>] (ehci_stop+0x3c/0xcc) > [<c0db7644>] (ehci_stop) from [<c0d5bdc4>] (usb_remove_hcd+0xe0/0x19c) > [<c0d5bdc4>] (usb_remove_hcd) from [<c0df7638>] (host_stop+0x38/0xa8) > [<c0df7638>] (host_stop) from [<c0df2f34>] (ci_hdrc_remove+0x44/0xe4) > ... > > Keeping track of the power enable state avoids the warning and traceback. > > Fixes: c8679a2fb8dec ("usb: chipidea: host: add portpower override") > Cc: Michael Grzeschik <m.grzeschik@pengutronix.de> > Cc: Peter Chen <peter.chen@freescale.com> > Cc: stable@vger.kernel.org > Signed-off-by: Guenter Roeck <linux@roeck-us.net> > --- > drivers/usb/chipidea/host.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c > index b45ceb91c735..48e4a5ca1835 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) > @@ -37,7 +38,7 @@ static int ehci_ci_portpower(struct usb_hcd *hcd, int portnum, bool enable) > int ret = 0; > int port = HCS_N_PORTS(ehci->hcs_params); > > - if (priv->reg_vbus) { > + if (priv->reg_vbus && enable != priv->enabled) { > if (port > 1) { > dev_warn(dev, > "Not support multi-port regulator control\n"); > @@ -53,6 +54,7 @@ static int ehci_ci_portpower(struct usb_hcd *hcd, int portnum, bool enable) > enable ? "enable" : "disable", ret); > return ret; > } > + priv->enabled = enable; > } > > if (enable && (ci->platdata->phy_mode == USBPHY_INTERFACE_MODE_HSIC)) { > -- Acked-by: Peter Chen <peter.chen@nxp.com>
On Fri, 27 Dec 2019, Peter Chen wrote: > On 19-12-26 14:46:15, Alan Stern wrote: > > On Thu, 26 Dec 2019, Guenter Roeck wrote: > > > > > On shutdown, ehci_power_off() is called unconditionally to power off > > > each port, even if it was never called to power on the port. > > > For chipidea, this results in a call to ehci_ci_portpower() with a request > > > to power off ports even if the port was never powered on. > > > This results in the following warning from the regulator code. > > > > That's weird -- we should always power-on every port during hub > > initialization. > > > > It looks like there's a bug in hub.c:hub_activate(): The line under > > HUB_INIT which calls hub_power_on() should call > > usb_hub_set_port_power() instead. In fact, the comment near the start > > of hub_power_on() is wrong. It says "Enable power on each port", but > > in fact it only enables power for ports that had been powered-on > > previously (i.e., the port's bit in hub->power_bits was set). > > Apparently this got messed up in commit ad493e5e5805 ("usb: add usb > > port auto power off mechanism"). > > > > Now, the chipidea driver may still need to be updated, because > > ehci_turn_off_all_ports() will still be called during shutdown and it > > will try to power-down all ports, even those which are already powered > > down (for example, because the port is suspended). > > > > Hi Alan, > > When the port is suspended, why it was turned off? If it was turned > off, how could respond wakeup event? You're right; if a port is powered off then it can't respond to wakeup events. But if the power/wakeup attribute is set to "disabled" then the port doesn't need to respond to wakeup events. In that situation, usbcore may decide to power-down the port. Alan Stern
On Thu, Dec 26, 2019 at 02:46:15PM -0500, Alan Stern wrote: > On Thu, 26 Dec 2019, Guenter Roeck wrote: > > > On shutdown, ehci_power_off() is called unconditionally to power off > > each port, even if it was never called to power on the port. > > For chipidea, this results in a call to ehci_ci_portpower() with a request > > to power off ports even if the port was never powered on. > > This results in the following warning from the regulator code. > > That's weird -- we should always power-on every port during hub > initialization. > That is what I would have assumed, but test code shows that it doesn't happen. > It looks like there's a bug in hub.c:hub_activate(): The line under > HUB_INIT which calls hub_power_on() should call > usb_hub_set_port_power() instead. In fact, the comment near the start usb_hub_set_port_power() operates on a port of the hub. hub_activate() operates on the hub itself, or at least I think it does. I don't know how to convert the calls. Also, there are more calls to hub_power_on() in the same function. Can you provide more details on what to do, or even better a patch for me to test ? Thanks, Guenter > of hub_power_on() is wrong. It says "Enable power on each port", but > in fact it only enables power for ports that had been powered-on > previously (i.e., the port's bit in hub->power_bits was set). > Apparently this got messed up in commit ad493e5e5805 ("usb: add usb > port auto power off mechanism"). > > Now, the chipidea driver may still need to be updated, because > ehci_turn_off_all_ports() will still be called during shutdown and it > will try to power-down all ports, even those which are already powered > down (for example, because the port is suspended). > > Alan Stern >
On Fri, 27 Dec 2019, Guenter Roeck wrote: > On Thu, Dec 26, 2019 at 02:46:15PM -0500, Alan Stern wrote: > > On Thu, 26 Dec 2019, Guenter Roeck wrote: > > > > > On shutdown, ehci_power_off() is called unconditionally to power off > > > each port, even if it was never called to power on the port. > > > For chipidea, this results in a call to ehci_ci_portpower() with a request > > > to power off ports even if the port was never powered on. > > > This results in the following warning from the regulator code. > > > > That's weird -- we should always power-on every port during hub > > initialization. > > > That is what I would have assumed, but test code shows that it doesn't > happen. > > > It looks like there's a bug in hub.c:hub_activate(): The line under > > HUB_INIT which calls hub_power_on() should call > > usb_hub_set_port_power() instead. In fact, the comment near the start > > usb_hub_set_port_power() operates on a port of the hub. hub_activate() > operates on the hub itself, or at least I think it does. I don't know > how to convert the calls. Also, there are more calls to hub_power_on() > in the same function. Can you provide more details on what to do, > or even better a patch for me to test ? Let's try a slightly different approach. What happens with this patch? Alan Stern Index: usb-devel/drivers/usb/core/hub.c =================================================================== --- usb-devel.orig/drivers/usb/core/hub.c +++ usb-devel/drivers/usb/core/hub.c @@ -1065,6 +1065,7 @@ static void hub_activate(struct usb_hub if (type == HUB_INIT) { delay = hub_power_on_good_delay(hub); + hub->power_bits[0] = ~0UL; /* All ports on */ hub_power_on(hub, false); INIT_DELAYED_WORK(&hub->init_work, hub_init_func2); queue_delayed_work(system_power_efficient_wq,
On Sat, Dec 28, 2019 at 02:33:01PM -0500, Alan Stern wrote: > > Let's try a slightly different approach. What happens with this patch? > > Alan Stern > > > Index: usb-devel/drivers/usb/core/hub.c > =================================================================== > --- usb-devel.orig/drivers/usb/core/hub.c > +++ usb-devel/drivers/usb/core/hub.c > @@ -1065,6 +1065,7 @@ static void hub_activate(struct usb_hub > if (type == HUB_INIT) { > delay = hub_power_on_good_delay(hub); > > + hub->power_bits[0] = ~0UL; /* All ports on */ > hub_power_on(hub, false); > INIT_DELAYED_WORK(&hub->init_work, hub_init_func2); > queue_delayed_work(system_power_efficient_wq, > That doesn't make a difference - the traceback is still seen with this patch applied. Guenter
On Sun, 29 Dec 2019, Guenter Roeck wrote: > On Sat, Dec 28, 2019 at 02:33:01PM -0500, Alan Stern wrote: > > > > Let's try a slightly different approach. What happens with this patch? > > > > Alan Stern > > > > > > Index: usb-devel/drivers/usb/core/hub.c > > =================================================================== > > --- usb-devel.orig/drivers/usb/core/hub.c > > +++ usb-devel/drivers/usb/core/hub.c > > @@ -1065,6 +1065,7 @@ static void hub_activate(struct usb_hub > > if (type == HUB_INIT) { > > delay = hub_power_on_good_delay(hub); > > > > + hub->power_bits[0] = ~0UL; /* All ports on */ > > hub_power_on(hub, false); > > INIT_DELAYED_WORK(&hub->init_work, hub_init_func2); > > queue_delayed_work(system_power_efficient_wq, > > > > That doesn't make a difference - the traceback is still seen with this patch > applied. Can you trace what's going on? Does this code pathway now end up calling ehci_port_power() for each root-hub port, and from there down into the chipidea driver? If not, can you find where it gets sidetracked? Alan Stern
On Sun, Dec 29, 2019 at 11:40:52AM -0500, Alan Stern wrote: > On Sun, 29 Dec 2019, Guenter Roeck wrote: > > > On Sat, Dec 28, 2019 at 02:33:01PM -0500, Alan Stern wrote: > > > > > > Let's try a slightly different approach. What happens with this patch? > > > > > > Alan Stern > > > > > > > > > Index: usb-devel/drivers/usb/core/hub.c > > > =================================================================== > > > --- usb-devel.orig/drivers/usb/core/hub.c > > > +++ usb-devel/drivers/usb/core/hub.c > > > @@ -1065,6 +1065,7 @@ static void hub_activate(struct usb_hub > > > if (type == HUB_INIT) { > > > delay = hub_power_on_good_delay(hub); > > > > > > + hub->power_bits[0] = ~0UL; /* All ports on */ > > > hub_power_on(hub, false); > > > INIT_DELAYED_WORK(&hub->init_work, hub_init_func2); > > > queue_delayed_work(system_power_efficient_wq, > > > > > > > That doesn't make a difference - the traceback is still seen with this patch > > applied. > > Can you trace what's going on? Does this code pathway now end up > calling ehci_port_power() for each root-hub port, and from there down > into the chipidea driver? If not, can you find where it gets > sidetracked? > Sure, I'll do that. It will have to wait for the new year, though - internet connectivity is terrible where I am right now, Guenter
On 12/29/19 8:40 AM, Alan Stern wrote: > On Sun, 29 Dec 2019, Guenter Roeck wrote: > >> On Sat, Dec 28, 2019 at 02:33:01PM -0500, Alan Stern wrote: >>> >>> Let's try a slightly different approach. What happens with this patch? >>> >>> Alan Stern >>> >>> >>> Index: usb-devel/drivers/usb/core/hub.c >>> =================================================================== >>> --- usb-devel.orig/drivers/usb/core/hub.c >>> +++ usb-devel/drivers/usb/core/hub.c >>> @@ -1065,6 +1065,7 @@ static void hub_activate(struct usb_hub >>> if (type == HUB_INIT) { >>> delay = hub_power_on_good_delay(hub); >>> >>> + hub->power_bits[0] = ~0UL; /* All ports on */ >>> hub_power_on(hub, false); >>> INIT_DELAYED_WORK(&hub->init_work, hub_init_func2); >>> queue_delayed_work(system_power_efficient_wq, >>> >> >> That doesn't make a difference - the traceback is still seen with this patch >> applied. > > Can you trace what's going on? Does this code pathway now end up > calling ehci_port_power() for each root-hub port, and from there down > into the chipidea driver? If not, can you find where it gets > sidetracked? > A complete traceback is attached below, so, yes, I think it is safe to say that ehci_port_power() is called unconditionally for each root-hub port on shutdown. The only mystery to me was why ehci_port_power() isn't called to enable port power when the port comes up. As it turns out, HCS_PPC(ehci->hcs_params) is false on my simulated hardware, and thus ehci_port_power(..., true) is not called from ehci_hub_control(). Given that, it may well be that the problem is not seen on "real" hardware, at least not with real mcimx7d-sabre hardware, if the hub on that hardware does support power control. To test this idea, I modified qemu to claim hub power control support by setting the "power control support" capability bit. With that, the traceback is gone. Any suggestion how to proceed ? Thanks, Guenter --- [ 31.916567] ------------[ cut here ]------------ [ 31.917178] WARNING: CPU: 0 PID: 182 at drivers/regulator/core.c:2598 _regulator_disable+0x1a8/0x210 [ 31.917331] unbalanced disables for usb_otg2_vbus [ 31.917468] Modules linked in: [ 31.917877] CPU: 0 PID: 182 Comm: init Not tainted 5.4.8-rc1-00003-gb8e36d27e314 #1 [ 31.918032] Hardware name: Freescale i.MX7 Dual (Device Tree) [ 31.918246] [<c0313658>] (unwind_backtrace) from [<c030d698>] (show_stack+0x10/0x14) [ 31.918397] [<c030d698>] (show_stack) from [<c113439c>] (dump_stack+0xe0/0x10c) [ 31.918522] [<c113439c>] (dump_stack) from [<c0349098>] (__warn+0xf4/0x10c) [ 31.918641] [<c0349098>] (__warn) from [<c0349128>] (warn_slowpath_fmt+0x78/0xbc) [ 31.918768] [<c0349128>] (warn_slowpath_fmt) from [<c09f3c3c>] (_regulator_disable+0x1a8/0x210) [ 31.918900] [<c09f3c3c>] (_regulator_disable) from [<c09f3cdc>] (regulator_disable+0x38/0xe8) [ 31.919032] [<c09f3cdc>] (regulator_disable) from [<c0df837c>] (ehci_ci_portpower+0x38/0xdc) ehci_ci_portpower() calls regulator_enable() or regulator_disable() if priv->reg_vbus is not NULL [ 31.919166] [<c0df837c>] (ehci_ci_portpower) from [<c0db5724>] (ehci_port_power+0x50/0xa4) ehci_port_power() calls hcd->driver->port_power(), which is ehci_ci_portpower() [ 31.919296] [<c0db5724>] (ehci_port_power) from [<c0db5ba0>] (ehci_silence_controller+0x5c/0xc4) ehci_silence_controller() calls ehci_turn_off_all_ports(), which calls ehci_port_power() for each port [ 31.919430] [<c0db5ba0>] (ehci_silence_controller) from [<c0db7dc4>] (ehci_stop+0x3c/0xcc) ehci_silence_controller() called unconditionally from ehci_stop() [ 31.919560] [<c0db7dc4>] (ehci_stop) from [<c0d5c504>] (usb_remove_hcd+0xe0/0x19c) ehci_stop() called unconditionally from usb_remove_hcd() with hcd->driver->stop(hcd); [ 31.919685] [<c0d5c504>] (usb_remove_hcd) from [<c0df7e08>] (host_stop+0x38/0xa8) [ 31.919809] [<c0df7e08>] (host_stop) from [<c0df3704>] (ci_hdrc_remove+0x44/0xe4) [ 31.919932] [<c0df3704>] (ci_hdrc_remove) from [<c0b1a5e4>] (platform_drv_remove+0x20/0x40) [ 31.920062] [<c0b1a5e4>] (platform_drv_remove) from [<c0b18a50>] (device_release_driver_internal+0xe8/0x1b8) [ 31.920210] [<c0b18a50>] (device_release_driver_internal) from [<c0b17464>] (bus_remove_device+0xd0/0xfc) [ 31.920351] [<c0b17464>] (bus_remove_device) from [<c0b1401c>] (device_del+0x140/0x374) [ 31.920477] [<c0b1401c>] (device_del) from [<c0b1aaf8>] (platform_device_del.part.3+0x10/0x74) [ 31.920608] [<c0b1aaf8>] (platform_device_del.part.3) from [<c0b1ab8c>] (platform_device_unregister+0x1c/0x28) [ 31.920758] [<c0b1ab8c>] (platform_device_unregister) from [<c0df22f4>] (ci_hdrc_remove_device+0xc/0x20) [ 31.920898] [<c0df22f4>] (ci_hdrc_remove_device) from [<c0df9ea4>] (ci_hdrc_imx_remove+0x28/0x110) [ 31.921033] [<c0df9ea4>] (ci_hdrc_imx_remove) from [<c0b15ac4>] (device_shutdown+0x180/0x224) [ 31.921166] [<c0b15ac4>] (device_shutdown) from [<c037652c>] (kernel_restart+0xc/0x50) [ 31.921292] [<c037652c>] (kernel_restart) from [<c03767d0>] (__do_sys_reboot+0x15c/0x1ec) [ 31.921444] [<c03767d0>] (__do_sys_reboot) from [<c0301000>] (ret_fast_syscall+0x0/0x28) [ 31.921595] Exception stack(0xc93c1fa8 to 0xc93c1ff0)
On Wed, 1 Jan 2020, Guenter Roeck wrote: > On 12/29/19 8:40 AM, Alan Stern wrote: > > On Sun, 29 Dec 2019, Guenter Roeck wrote: > > > >> On Sat, Dec 28, 2019 at 02:33:01PM -0500, Alan Stern wrote: > >>> > >>> Let's try a slightly different approach. What happens with this patch? > >>> > >>> Alan Stern > >>> > >>> > >>> Index: usb-devel/drivers/usb/core/hub.c > >>> =================================================================== > >>> --- usb-devel.orig/drivers/usb/core/hub.c > >>> +++ usb-devel/drivers/usb/core/hub.c > >>> @@ -1065,6 +1065,7 @@ static void hub_activate(struct usb_hub > >>> if (type == HUB_INIT) { > >>> delay = hub_power_on_good_delay(hub); > >>> > >>> + hub->power_bits[0] = ~0UL; /* All ports on */ > >>> hub_power_on(hub, false); > >>> INIT_DELAYED_WORK(&hub->init_work, hub_init_func2); > >>> queue_delayed_work(system_power_efficient_wq, > >>> > >> > >> That doesn't make a difference - the traceback is still seen with this patch > >> applied. > > > > Can you trace what's going on? Does this code pathway now end up > > calling ehci_port_power() for each root-hub port, and from there down > > into the chipidea driver? If not, can you find where it gets > > sidetracked? > > > > A complete traceback is attached below, so, yes, I think it is safe to say that > ehci_port_power() is called unconditionally for each root-hub port on shutdown. I was really asking about hub activation and powering-up, but you found the answer to that too, so okay. > The only mystery to me was why ehci_port_power() isn't called to enable port power > when the port comes up. As it turns out, HCS_PPC(ehci->hcs_params) is false on my > simulated hardware, and thus ehci_port_power(..., true) is not called from > ehci_hub_control(). > > Given that, it may well be that the problem is not seen on "real" hardware, > at least not with real mcimx7d-sabre hardware, if the hub on that hardware does > support power control. To test this idea, I modified qemu to claim hub power > control support by setting the "power control support" capability bit. With > that, the traceback is gone. > > Any suggestion how to proceed ? Given that HCS_PPC(ehci_hcs_params) is false, I would say that ehci_turn_off_all_ports() shouldn't call ehci_port_power(). You should add that test there. (Although to tell the truth, I'm not really sure we need to test HCS_PPC anywhere...) Did you check what happens without the patch I sent you? I would like to know if that patch really does make a difference. If we don't send the Set-Port-Feature(power) request during hub activation without the patch then it does need to be merged. Alan Stern
diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c index b45ceb91c735..48e4a5ca1835 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) @@ -37,7 +38,7 @@ static int ehci_ci_portpower(struct usb_hcd *hcd, int portnum, bool enable) int ret = 0; int port = HCS_N_PORTS(ehci->hcs_params); - if (priv->reg_vbus) { + if (priv->reg_vbus && enable != priv->enabled) { if (port > 1) { dev_warn(dev, "Not support multi-port regulator control\n"); @@ -53,6 +54,7 @@ static int ehci_ci_portpower(struct usb_hcd *hcd, int portnum, bool enable) enable ? "enable" : "disable", ret); return ret; } + priv->enabled = enable; } if (enable && (ci->platdata->phy_mode == USBPHY_INTERFACE_MODE_HSIC)) {
On shutdown, ehci_power_off() is called unconditionally to power off each port, even if it was never called to power on the port. For chipidea, this results in a call to ehci_ci_portpower() with a request to power off ports even if the port was never powered on. This results in the following warning from the regulator code. WARNING: CPU: 0 PID: 182 at drivers/regulator/core.c:2596 _regulator_disable+0x1a8/0x210 unbalanced disables for usb_otg2_vbus Modules linked in: CPU: 0 PID: 182 Comm: init Not tainted 5.4.6 #1 Hardware name: Freescale i.MX7 Dual (Device Tree) [<c0313658>] (unwind_backtrace) from [<c030d698>] (show_stack+0x10/0x14) [<c030d698>] (show_stack) from [<c1133afc>] (dump_stack+0xe0/0x10c) [<c1133afc>] (dump_stack) from [<c0349098>] (__warn+0xf4/0x10c) [<c0349098>] (__warn) from [<c0349128>] (warn_slowpath_fmt+0x78/0xbc) [<c0349128>] (warn_slowpath_fmt) from [<c09f36ac>] (_regulator_disable+0x1a8/0x210) [<c09f36ac>] (_regulator_disable) from [<c09f374c>] (regulator_disable+0x38/0xe8) [<c09f374c>] (regulator_disable) from [<c0df7bac>] (ehci_ci_portpower+0x38/0xdc) [<c0df7bac>] (ehci_ci_portpower) from [<c0db4fa4>] (ehci_port_power+0x50/0xa4) [<c0db4fa4>] (ehci_port_power) from [<c0db5420>] (ehci_silence_controller+0x5c/0xc4) [<c0db5420>] (ehci_silence_controller) from [<c0db7644>] (ehci_stop+0x3c/0xcc) [<c0db7644>] (ehci_stop) from [<c0d5bdc4>] (usb_remove_hcd+0xe0/0x19c) [<c0d5bdc4>] (usb_remove_hcd) from [<c0df7638>] (host_stop+0x38/0xa8) [<c0df7638>] (host_stop) from [<c0df2f34>] (ci_hdrc_remove+0x44/0xe4) ... Keeping track of the power enable state avoids the warning and traceback. Fixes: c8679a2fb8dec ("usb: chipidea: host: add portpower override") Cc: Michael Grzeschik <m.grzeschik@pengutronix.de> Cc: Peter Chen <peter.chen@freescale.com> Cc: stable@vger.kernel.org Signed-off-by: Guenter Roeck <linux@roeck-us.net> --- drivers/usb/chipidea/host.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)