Re: [4.15 stable regression] "Bluetooth: btusb: Fix quirk for Atheros 1525/QCA6174" breaks bluetooth on some devices

[Takashi Iwai <tiwai@suse.de>]

On Thu, 26 Apr 2018 18:33:38 +0200,
Takashi Iwai wrote:
> 
> On Thu, 26 Apr 2018 14:18:23 +0200,
> Hans de Goede wrote:
> > 
> > Hi,
> > 
> > On 25-04-18 15:10, Takashi Iwai wrote:
> > > On Wed, 25 Apr 2018 14:49:59 +0200,
> > > Hans de Goede wrote:
> > >>
> > >> Hi,
> > >>
> > >> On 25-04-18 14:47, Greg Kroah-Hartman wrote:
> > >>> On Wed, Apr 25, 2018 at 02:40:37PM +0200, Hans de Goede wrote:
> > >>>> Hi,
> > >>>>
> > >>>> On 18-04-18 15:18, Hans de Goede wrote:
> > >>>>> Hi Takashi, Marcel,
> > >>>>>
> > >>>>> It seems that this commit:
> > >>>>> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/commit/?h=linux-4.15.y&id=7ec32f585fefd7c154453aa29ccf8fa2a11cc865
> > >>>>>
> > >>>>> Is breaking bluetooth on some devices, see:
> > >>>>>
> > >>>>> https://bugzilla.redhat.com/show_bug.cgi?id=1568911
> > >>>>>
> > >>>>> The problem is the following error now being thrown:
> > >>>>>
> > >>>>> [   28.466248] Bluetooth: hci0: don't support firmware rome 0x1020200
> > >>>>>
> > >>>>> Looking at the code I wonder if maybe we need to mask the ver_rom
> > >>>>> with & 0xfff when comparing it to the qca_devices_table[i].rom_version
> > >>>>> filed ?
> > >>>>>
> > >>>>> Or maybe the commit is actually wrong, or maybe devices with the
> > >>>>> 0cf3:3004 USB id need either the BTUSB_QCA_ROM or BTUSB_ATH3012
> > >>>>> quirk depending on the device and we need to probe this somehow?
> > >>>>
> > >>>> I've been receiving more complaints from users about this on
> > >>>> various devices, so I think that the 7ec32f585fefd7c154453aa29ccf8fa2a11cc865
> > >>>> commit should be reverted from 4.15.x while we figure this out.
> > >>>>
> > >>>> Does anyone have any idea how we cam distinguish between the 2
> > >>>> different versions which seem to be hiding between the same USB-id ?
> > >>>
> > >>> 4.15.y is end-of-life, so there is no more releases being made for it,
> > >>> sorry.
> > >>
> > >> Ah, right, no problem, Fedora should be moving to 4.16.x soon then
> > >> anyways.
> > >>
> > >>> But, this commit is in 4.4.y, 4.9.y, 4.14.y, and 4.16.  Can you revert
> > >>> it in Linus's tree and I can revert it everywhere else as well?
> > >>
> > >> Takashi, do you agree that reverting this for now is best? And if so
> > >> can you please send a revert?
> > >
> > > The best would be to fix it properly :)
> > >
> > > But I agree that it needs a quick resolution, and the revert is
> > > appropriate in this case.  Since you've confirmed that the revert
> > > worked, feel free to submit the revert patch from your side.
> > 
> > Done.
> > 
> > > Back to the original issue: now I'm wondering what made such
> > > inconsistent behavior.  My current suspect is the racy driver loading
> > > between btusb and ath3k.  Both have the same USB ID, and the driver
> > > loading order may interfere the behavior with each other?
> > >
> > > Or might it be a WiFi/BT combo chip that may have a racy firmware
> > > initialization?
> > 
> > I've no idea I'm afraid.
> 
> I checked with the original reporter about this possibility, and all
> failed.  So, it's not about the interaction between them.
> 
> And, now after looking at the log in your bugzilla, I guess the fix
> would be something like below.
> 
> Basically the only significant difference between these two BT quirks
> is the RAM-patching.  Without RAM patching, the wrong ROM versions are
> read by ath3k as is on the device of our reporter.
> 
> OTOH, there are devices with the correct ROM versions, and at
> btusb_setup_qca(), we give -ENODEV at open just because the function
> expects only the buggy version numbers.  Instead of -ENODEV, it should
> just return 0.
> 
> Hans, could you check whether it works for them?

... or better with the one below.


Takashi

-- 8< --
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] Bluetooth: btusb: Apply QCQ_ROME setup for BTUSB_ATH3012
 quirk, too

In commit f44cb4b19ed4 ("Bluetooth: btusb: Fix quirk for Atheros
1525/QCA6174") we tried to address the non-working Atheros BT devices
by changing the quirk from BTUSB_ATH3012 to BTUSB_QCQ_ROME.  This made
such devices working while it turned out to break other existing chips
with the very same USB ID.

This is another attempt to tackle the issue.  The essential point to
use BTUSB_QCA_ROME is to apply the btusb_setup_qca() and do RAM-
patching.  And the previous attempt failed because btusb_setup_qcq()
returns -ENODEV if the ROM version doesn't match with the expected
ones.  For some devices that have already the "correct" ROM versions,
we may just skip the setup procedure and continue the rest.

So, this patch applies btusb_setup_qca() also in BTUSB_ATH3012 quirk,
and adds a check of the ROM version in the function to skip the setup
if the ROM version looks already sane.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/bluetooth/btusb.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index c8c8b0b8d333..720356320ace 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -2673,6 +2673,10 @@ static int btusb_setup_qca(struct hci_dev *hdev)
 		return err;
 
 	ver_rom = le32_to_cpu(ver.rom_version);
+	/* Don't care about high ROM versions */
+	if (ver_rom & ~0xffffU)
+		return 0;
+
 	for (i = 0; i < ARRAY_SIZE(qca_devices_table); i++) {
 		if (ver_rom == qca_devices_table[i].rom_version)
 			info = &qca_devices_table[i];
@@ -3055,6 +3059,7 @@ static int btusb_probe(struct usb_interface *intf,
 	}
 
 	if (id->driver_info & BTUSB_ATH3012) {
+		data->setup_on_usb = btusb_setup_qca;
 		hdev->set_bdaddr = btusb_set_bdaddr_ath3012;
 		set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
 		set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);
-- 
2.16.3