| Message ID | 20190610072141.29940-2-kai.heng.feng@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | Fix disappearing Qualcomm WiFi/Bluetooth | expand |
Applied on Ubuntu-oem-4.15.0-1041.46
Acked-By: AceLan Kao <acelan.kao@canonical.com>
On Mon, Jun 10, 2019 at 03:21:41PM +0800, Kai-Heng Feng wrote: > BugLink: https://bugs.launchpad.net/bugs/1757218 > > The QCA Rome USB Bluetooth controller has several issues once LPM gets > enabled: > - Fails to get enumerated in coldboot. [1] > - Drains more power (~ 0.2W) when the system is in S5. [2] > - Disappears after a warmboot. [2] > > The issue happens because the device lingers at LPM L1 in S5, so device > can't get enumerated even after a reboot. > > Disable LPM at shutdown to solve the issue. > > [1] https://bugs.launchpad.net/bugs/1757218 > [2] https://patchwork.kernel.org/patch/10607097/ On this upstream thread you leak to some doubts are expressed about this patch, followed by a promise to dig further into the problem but no follow-up. What has transpired since then to verify that this patch is 1) safe and 2) not just papering over some other problem? Thanks, Seth
at 04:26, Seth Forshee <seth.forshee@canonical.com> wrote: > On Mon, Jun 10, 2019 at 03:21:41PM +0800, Kai-Heng Feng wrote: >> BugLink: https://bugs.launchpad.net/bugs/1757218 >> >> The QCA Rome USB Bluetooth controller has several issues once LPM gets >> enabled: >> - Fails to get enumerated in coldboot. [1] >> - Drains more power (~ 0.2W) when the system is in S5. [2] >> - Disappears after a warmboot. [2] >> >> The issue happens because the device lingers at LPM L1 in S5, so device >> can't get enumerated even after a reboot. >> >> Disable LPM at shutdown to solve the issue. >> >> [1] https://bugs.launchpad.net/bugs/1757218 >> [2] https://patchwork.kernel.org/patch/10607097/ > > On this upstream thread you leak to some doubts are expressed about this > patch, followed by a promise to dig further into the problem but no > follow-up. What has transpired since then to verify that this patch is > 1) safe and 2) not just papering over some other problem? Alan Stern, one of the USB core supporter, recently gave a positive feedback [1] and some minor concerns. I’ve answered those concerns and I’ll ask Greg KH again to merge the patch. So 1) it’s safe, 2) the the patch is to solve the root cause, doesn’t paper over some other issues. [1] https://lore.kernel.org/linux-usb/Pine.LNX.4.44L0.1906061013490.1641-100000@iolanthe.rowland.org/ Kai-Heng > > Thanks, > Seth
On Mon, Jun 17, 2019 at 04:53:28PM +0800, Kai-Heng Feng wrote: > at 04:26, Seth Forshee <seth.forshee@canonical.com> wrote: > > > On Mon, Jun 10, 2019 at 03:21:41PM +0800, Kai-Heng Feng wrote: > > > BugLink: https://bugs.launchpad.net/bugs/1757218 > > > > > > The QCA Rome USB Bluetooth controller has several issues once LPM gets > > > enabled: > > > - Fails to get enumerated in coldboot. [1] > > > - Drains more power (~ 0.2W) when the system is in S5. [2] > > > - Disappears after a warmboot. [2] > > > > > > The issue happens because the device lingers at LPM L1 in S5, so device > > > can't get enumerated even after a reboot. > > > > > > Disable LPM at shutdown to solve the issue. > > > > > > [1] https://bugs.launchpad.net/bugs/1757218 > > > [2] https://patchwork.kernel.org/patch/10607097/ > > > > On this upstream thread you leak to some doubts are expressed about this > > patch, followed by a promise to dig further into the problem but no > > follow-up. What has transpired since then to verify that this patch is > > 1) safe and 2) not just papering over some other problem? > > Alan Stern, one of the USB core supporter, recently gave a positive feedback > [1] and some minor concerns. > I’ve answered those concerns and I’ll ask Greg KH again to merge the patch. > > So 1) it’s safe, 2) the the patch is to solve the root cause, doesn’t paper > over some other issues. > > [1] https://lore.kernel.org/linux-usb/Pine.LNX.4.44L0.1906061013490.1641-100000@iolanthe.rowland.org/ Okay, I went ahead and applied this to unstable to get some testing there. I'm not finding your regression potential statement for SRU very convincing though. Simply saying that not many devices support LPM doesn't say anything about the regression potential for those devices which do support it, or give any information about what kind of regression testing you've done. Thanks, Seth
On 10.06.19 09:21, Kai-Heng Feng wrote: > BugLink: https://bugs.launchpad.net/bugs/1757218 > > The QCA Rome USB Bluetooth controller has several issues once LPM gets > enabled: > - Fails to get enumerated in coldboot. [1] > - Drains more power (~ 0.2W) when the system is in S5. [2] > - Disappears after a warmboot. [2] > > The issue happens because the device lingers at LPM L1 in S5, so device > can't get enumerated even after a reboot. > > Disable LPM at shutdown to solve the issue. > > [1] https://bugs.launchpad.net/bugs/1757218 > [2] https://patchwork.kernel.org/patch/10607097/ > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> Acked-by: Stefan Bader <stefan.bader@canonical.com> > --- Still does not seem to be upstream... But hopefully not causing issues. > drivers/usb/core/port.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c > index 1a06a4b5fbb1..bbbb35fa639f 100644 > --- a/drivers/usb/core/port.c > +++ b/drivers/usb/core/port.c > @@ -285,6 +285,14 @@ static int usb_port_runtime_suspend(struct device *dev) > } > #endif > > +static void usb_port_shutdown(struct device *dev) > +{ > + struct usb_port *port_dev = to_usb_port(dev); > + > + if (port_dev->child) > + usb_disable_usb2_hardware_lpm(port_dev->child); > +} > + > static const struct dev_pm_ops usb_port_pm_ops = { > #ifdef CONFIG_PM > .runtime_suspend = usb_port_runtime_suspend, > @@ -301,6 +309,7 @@ struct device_type usb_port_device_type = { > static struct device_driver usb_port_driver = { > .name = "usb", > .owner = THIS_MODULE, > + .shutdown = usb_port_shutdown, > }; > > static int link_peers(struct usb_port *left, struct usb_port *right) >
On 6/10/19 12:21 AM, Kai-Heng Feng wrote: > BugLink: https://bugs.launchpad.net/bugs/1757218 > > The QCA Rome USB Bluetooth controller has several issues once LPM gets > enabled: > - Fails to get enumerated in coldboot. [1] > - Drains more power (~ 0.2W) when the system is in S5. [2] > - Disappears after a warmboot. [2] > > The issue happens because the device lingers at LPM L1 in S5, so device > can't get enumerated even after a reboot. > > Disable LPM at shutdown to solve the issue. > > [1] https://bugs.launchpad.net/bugs/1757218 > [2] https://patchwork.kernel.org/patch/10607097/ > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> Acked-by: Connor Kuehl <connor.kuehl@canonical.com> > --- > drivers/usb/core/port.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c > index 1a06a4b5fbb1..bbbb35fa639f 100644 > --- a/drivers/usb/core/port.c > +++ b/drivers/usb/core/port.c > @@ -285,6 +285,14 @@ static int usb_port_runtime_suspend(struct device *dev) > } > #endif > > +static void usb_port_shutdown(struct device *dev) > +{ > + struct usb_port *port_dev = to_usb_port(dev); > + > + if (port_dev->child) > + usb_disable_usb2_hardware_lpm(port_dev->child); > +} > + > static const struct dev_pm_ops usb_port_pm_ops = { > #ifdef CONFIG_PM > .runtime_suspend = usb_port_runtime_suspend, > @@ -301,6 +309,7 @@ struct device_type usb_port_device_type = { > static struct device_driver usb_port_driver = { > .name = "usb", > .owner = THIS_MODULE, > + .shutdown = usb_port_shutdown, > }; > > static int link_peers(struct usb_port *left, struct usb_port *right) >
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c index 1a06a4b5fbb1..bbbb35fa639f 100644 --- a/drivers/usb/core/port.c +++ b/drivers/usb/core/port.c @@ -285,6 +285,14 @@ static int usb_port_runtime_suspend(struct device *dev) } #endif +static void usb_port_shutdown(struct device *dev) +{ + struct usb_port *port_dev = to_usb_port(dev); + + if (port_dev->child) + usb_disable_usb2_hardware_lpm(port_dev->child); +} + static const struct dev_pm_ops usb_port_pm_ops = { #ifdef CONFIG_PM .runtime_suspend = usb_port_runtime_suspend, @@ -301,6 +309,7 @@ struct device_type usb_port_device_type = { static struct device_driver usb_port_driver = { .name = "usb", .owner = THIS_MODULE, + .shutdown = usb_port_shutdown, }; static int link_peers(struct usb_port *left, struct usb_port *right)
BugLink: https://bugs.launchpad.net/bugs/1757218 The QCA Rome USB Bluetooth controller has several issues once LPM gets enabled: - Fails to get enumerated in coldboot. [1] - Drains more power (~ 0.2W) when the system is in S5. [2] - Disappears after a warmboot. [2] The issue happens because the device lingers at LPM L1 in S5, so device can't get enumerated even after a reboot. Disable LPM at shutdown to solve the issue. [1] https://bugs.launchpad.net/bugs/1757218 [2] https://patchwork.kernel.org/patch/10607097/ Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> --- drivers/usb/core/port.c | 9 +++++++++ 1 file changed, 9 insertions(+)