Message-Id: <BBBBCF1B-3776-4E52-B7E8-95B5A62ED8CA@canonical.com>
Date:   Tue, 19 Dec 2017 12:28:17 +0800
From:   Kai Heng Feng <kai.heng.feng@...onical.com>
To:     Brian Norris <briannorris@...omium.org>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Matthias Kaehlcke <mka@...omium.org>,
        linux-kernel@...r.kernel.org, stable@...r.kernel.org,
        Leif Liddy <leif.linux@...il.com>,
        Marcel Holtmann <marcel@...tmann.org>,
        Guenter Roeck <groeck@...omium.org>
Subject: Re: [PATCH 4.13 03/28] Bluetooth: btusb: fix QCA Rome suspend/resume

Hi Brian,

> On 19 Dec 2017, at 2:13 AM, Brian Norris <briannorris@...omium.org> wrote:
> 
> Hi Greg,
> 
> On Mon, Dec 18, 2017 at 12:43:48PM +0100, Greg Kroah-Hartman wrote:
>> On Fri, Dec 15, 2017 at 07:05:39PM -0800, Matthias Kaehlcke wrote:
>>> We identified the above patch as the culprit, in combination with USB
>>> autosuspend being enabled for the Bluetooth controller.
>>> 
>>> We found that the following (recent) upstream patch fixes the issue on
>>> most devices (we are still investigating one case on a specific device):
> 
> A key word to highlight here is "most". I have at least one device that
> is consistently having problems even with both patches included; the
> only way things work reliably so far is to simply revert the $subject
> patch in 4.4.x.

The problem we have is that the QCA Rome Bluetooth stops working
after system S3. The reset resume quirk workaround the issue on both
mainline and 4.4.x (at least for me).

> 
> I'll try to investigate further, since this result is a bit confusing
> still. If there's a real problem with the patch (and I suspect there
> might be), it probably shouldn't be in mainline either…

Do you have the same issue on mainline?

> 
>>> commit a0085f2510e8976614ad8f766b209448b385492f
>>> Author: Sukumar Ghorai <sukumar.ghorai@...el.com>
>>> Date:   Wed Aug 16 14:46:55 2017 -0700
>>> 
>>>    Bluetooth: btusb: driver to enable the usb-wakeup feature
> [...]
>>> stable branches are currently broken for BTUSB_QCA_ROME with USB
>>> autosuspend enabled, since the above patch is not included (I only
>>> checked v4.4 and v4.9), so we probably want to integrate it.
>> 
>> Now queued up, thanks for letting me know.
> 
> I think you have almost as much information as I do at this point, and
> I'll try to reply back here if I figure out anything more, so I'll leave
> you to your decisions. But I would personally revert, not backport more
> patches.
> 
> Among the reasons:
> (a) I have at least one device that does not work better with both
>    patches [1]
> (b) So far, I haven't seen any explanation why commit a0085f2510e8
>    should be a dependency for $subject patch; the closest I can imagine
>    would be that commit a0085f2510e8 could effectively neutralize
>    $subject patch -- if the device is marked as a wakeup source, then
>    it will never try to RESET on resume -- but that's still a fuzzy
>    signal; just because it's marked a wakeup source sometimes doesn't
>    mean it always will be. We can disable it in user space too.

Hi Leif,

Can you try if a0085f2510e8 breaks your $subject commit?
If it’s a yes, then what Brian suggests is correct.

Also, can you share the output of "usb-device” for your btusb device?


> (c) Isn't it safer to revert than to backport more? You're delving into
>    feature-land, not bugfix-land…

Sounds reasonable.

We’ll need more information from Leif if we need to do ID-specific quirks.

Kai-Heng

> Brian
> 
> [1] Before you ask: this particular device is not quite fully supported
> on upstream yet, though its sibling devices are. With a bit of effort, I
> might be able to test a clean upstream. At the moment, I'm using our
> Chrom{e,ium}OS 4.4 kernel, where we regularly merge in -stable updates.

Powered by blists - more mailing lists