Re: [PATCH] Bluetooth: btusb: Restore QCA Rome suspend/resume fix with a "rewritten" version

[Hans de Goede]

Hi,

On 23-02-18 04:12, Brian Norris wrote:
> Hi Hans,
>
> Sorry if I'm a little slow to follow up here. This hasn't been my
> top priority...
>
> On Mon, Feb 19, 2018 at 11:17:24AM +0100, Hans de Goede wrote:
> > On 16-02-18 18:59, Brian Norris wrote:
> > > On Fri, Feb 16, 2018 at 01:10:20PM +0100, Hans de Goede wrote:
> > > > Ok, I've asked the reporter of:
> > > >
> > > > https://bugzilla.redhat.com/show_bug.cgi?id=1514836
> > >
> > > Are you even sure that this reporter is seeing the original symptom at
> > > all (BT loses power, and therefore firmware)? Their report shows them
> > > running 4.15, which had this commit:
> > >
> > > fd865802c66b Bluetooth: btusb: fix QCA Rome suspend/resume
> > >
> > > which is admittedly completely broken. It breaks even perfectly working
> > > BT/USB devices, like mine. That's where I first complained, and we got
> > > this into 4.16-rc1:
> > >
> > > 7d06d5895c15 Revert "Bluetooth: btusb: fix QCA Rome suspend/resume"
> > >
> > > Isn't it possible your reporter has no further problem, and none if this
> > > is actually important to them? I'd just caution you to be careful before
> > > assuming you need to add blacklist info for their DMI...
> >
> > Thanks, that is a good question. His problems only started when I
> > enabled usb-autosuspend by default for btusb devices and he got things
> > working by adding "btusb.enable_autosuspend=n" on the kernel commandline,
> > so he was not hitting the firmware loading race introduced by
> > fd865802c66b and runtime suspend/resume is really broken for him.
>
> Hmm? I'm not sure I completely follow here when you say "he was not
> hitting the firmware loading race". If things were functioning fine with
> system suspend (but not with autosuspend), then he's not seeing the
> controller (quoting commit fd865802c66b) "losing power during suspend".

He was running a kernel with the original "fd865802c66b Bluetooth: btusb:
fix QCA Rome suspend/resume" commit, which fixes regular suspend for
devices which are "losing power during suspend", but does nothing for
runtime-suspend.

He ran tests both with and without runtime-pm enabled with that same kernel
and he needed to disable runtime-pm to get working bluetooth.

> So, that would suggest he could only be seeing the race (as I was), and
> that his machine does not deserve a RESET_RESUME quirk?

I hope my above answer helps to clarify why I believe the quirk is
necessary on his machine.

Regards,

Hans


> Or maybe I'm really misunderstanding.
>
> > > As I read it, you need to investigate who are the "numerous reported
> > > instances" that generated commit fd865802c66b in the first place. That's
> > > where this mess started, IIUC. >
> > > But otherwise, yes, option 3 sounds OK. FWIW, my systems are ARM based
> > > and don't have DMI data, so option 2 wouldn't work.
> >
> > Right I think we all agree that the new plan now is to go back to
> > QCA behaving normally wrt (runtime) suspend/resume and then set the
> > USB-core RESET_RESUME quirk (which does not have the firmware
> > loading race) based on a DMI blacklist.
> >
> > I only have the one report for which I will write a patch implementing
> > this new policy soonish. And Kai-Heng Feng has another report which
> > might even be the machine. I certainly would not be surprised if it
> > is another Lenovo machine.
>
> It seems like you folks moved forward on that one. Thanks.
>
> Brian