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

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

[Hans de Goede]

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?

Regards,

Hans

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

[Takashi Iwai]

On Wed, 18 Apr 2018 15:18:59 +0200,
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'd love to know the proper fix, too.  It was already concerned from
the beginning I asked about the change, as the 0cf3:3004 is the very
first one that was used for BTUSB_ATH3012 quirk.

In anyway, I myself have no such device but can ask testing if any
further patch is available.


thanks,

Takashi

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

[Hans de Goede]

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 ?

Regards,

Hans

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

[Greg Kroah-Hartman]

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.

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?

thanks,

greg k-h

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

[Hans de Goede]

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?

Regards,

Hans

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

[Greg Kroah-Hartman]

On Wed, Apr 25, 2018 at 02:49:59PM +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.

Even if it does, this issue is still there, so it needs to be handled somehow...

thanks,

greg k-h

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

[Takashi Iwai]

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.

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?


thanks,

Takashi

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

[Hans de Goede]

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.

Regards,

Hans

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

[Takashi Iwai]

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?


thanks,

Takashi

-- 8< --
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -2678,8 +2678,8 @@ static int btusb_setup_qca(struct hci_dev *hdev)
 			info = &qca_devices_table[i];
 	}
 	if (!info) {
-		bt_dev_err(hdev, "don't support firmware rome 0x%x", ver_rom);
-		return -ENODEV;
+		bt_dev_dbg(hdev, "don't support firmware rome 0x%x", ver_rom);
+		return 0;
 	}
 
 	err = btusb_qca_send_vendor_req(hdev, QCA_CHECK_STATUS, &status,

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

[Takashi Iwai]

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

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

[Hans de Goede]

Hi,

On 26-04-18 20:27, Takashi Iwai wrote:
> 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.

Yes that one indeed looks better.

I've started a Fedora kernel (test) build with the revert + the patch
from you below and asked the reporter_s_ of this problem to test, see:
https://bugzilla.redhat.com/show_bug.cgi?id=1568911
(you may want to put yourself in the Cc there).

In the mean time I've also stumbled over this bug, which is another
bug tracking the same issue:
https://bugzilla.kernel.org/show_bug.cgi?id=199271

Regards,

Hans


> -- 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);
> 

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

[Hans de Goede]

Hi,

On 27-04-18 10:57, Hans de Goede wrote:
>> -- 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);
>>


p.s.

This change makes the BTUSB_ATH3012 / BTUSB_QCA_ROME code-paths almost
the same, the only difference is the BTUSB_ATH3012 path also setting the
HCI_QUIRK_STRICT_DUPLICATE_FILTER flag. Does anyone know if it perhaps
would be correct to also set that flag for the QCA_ROME chipset and
then unify the 2 code-paths?

Regards,

Hans

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

[Takashi Iwai]

On Fri, 27 Apr 2018 11:23:49 +0200,
Hans de Goede wrote:
> 
> Hi,
> 
> On 27-04-18 10:57, Hans de Goede wrote:
> >> -- 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);
> >>
> 
> 
> p.s.
> 
> This change makes the BTUSB_ATH3012 / BTUSB_QCA_ROME code-paths almost
> the same, the only difference is the BTUSB_ATH3012 path also setting the
> HCI_QUIRK_STRICT_DUPLICATE_FILTER flag. Does anyone know if it perhaps
> would be correct to also set that flag for the QCA_ROME chipset and
> then unify the 2 code-paths?

I belive it's the way to go, but obviously needs testing :)


Takashi

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

[Hans de Goede]

Hi,

On 27-04-18 10:57, Hans de Goede wrote:
> Hi,
> 
> On 26-04-18 20:27, Takashi Iwai wrote:
>> 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.
> 
> Yes that one indeed looks better.
> 
> I've started a Fedora kernel (test) build with the revert + the patch
> from you below and asked the reporter_s_ of this problem to test, see:
> https://bugzilla.redhat.com/show_bug.cgi?id=1568911
> (you may want to put yourself in the Cc there).
> 
> In the mean time I've also stumbled over this bug, which is another
> bug tracking the same issue:
> https://bugzilla.kernel.org/show_bug.cgi?id=199271

So the reporter of this bug has tested Takashi's patch (below) on top
of the revert and his bluetooth is still working, so I think we can
move ahead and get the revert + Takashi's patch merged.

Takashi, can you do a formal submission of your patch please?

Thanks,

Hans


>> -- 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);
>>

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

[Takashi Iwai]

On Fri, 27 Apr 2018 14:11:12 +0200,
Hans de Goede wrote:
> 
> Hi,
> 
> On 27-04-18 10:57, Hans de Goede wrote:
> > Hi,
> >
> > On 26-04-18 20:27, Takashi Iwai wrote:
> >> 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.
> >
> > Yes that one indeed looks better.
> >
> > I've started a Fedora kernel (test) build with the revert + the patch
> > from you below and asked the reporter_s_ of this problem to test, see:
> > https://bugzilla.redhat.com/show_bug.cgi?id=1568911
> > (you may want to put yourself in the Cc there).
> >
> > In the mean time I've also stumbled over this bug, which is another
> > bug tracking the same issue:
> > https://bugzilla.kernel.org/show_bug.cgi?id=199271
> 
> So the reporter of this bug has tested Takashi's patch (below) on top
> of the revert and his bluetooth is still working, so I think we can
> move ahead and get the revert + Takashi's patch merged.
> 
> Takashi, can you do a formal submission of your patch please?

Good to hear that it doesn't break.  But I'm still waiting in my side
for the test result whether it fixes the issue.  So, please be patient
for a while.


thanks,

Takashi

> 
> Thanks,
> 
> Hans
> 
> 
> >> -- 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);
> >>
> 

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

[Takashi Iwai]

On Fri, 27 Apr 2018 14:11:12 +0200,
Hans de Goede wrote:
> 
> Hi,
> 
> On 27-04-18 10:57, Hans de Goede wrote:
> > Hi,
> >
> > On 26-04-18 20:27, Takashi Iwai wrote:
> >> 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.
> >
> > Yes that one indeed looks better.
> >
> > I've started a Fedora kernel (test) build with the revert + the patch
> > from you below and asked the reporter_s_ of this problem to test, see:
> > https://bugzilla.redhat.com/show_bug.cgi?id=1568911
> > (you may want to put yourself in the Cc there).
> >
> > In the mean time I've also stumbled over this bug, which is another
> > bug tracking the same issue:
> > https://bugzilla.kernel.org/show_bug.cgi?id=199271
> 
> So the reporter of this bug has tested Takashi's patch (below) on top
> of the revert and his bluetooth is still working, so I think we can
> move ahead and get the revert + Takashi's patch merged.
> 
> Takashi, can you do a formal submission of your patch please?

Sorry for the delayed response on the thread, as it took long to
figure out what really went wrong (and I had a vacation meanwhile).

Actually there are a few issues behind the scene:
- Some ATH3012 devices are indeed equivalent with Rome, and the same
  patching is needed in btusb, not only what ath3k module does.

- There is no way to distinguish from the device ID, but the ROM
  version can be fetched via Atheros-specific verb, and we can check
  that instead.

- However, ATH3012 quirk has another check of bcdDevice and it bails
  out from the btusb probe function in the early stage.  So the ROM
  version check has to be performed at this point, too.

Below is the result and it seems working on top of the latest tree.

Hans, could you try it and ask reporters whether it does *not* break
things again?  I can submit formally again (if needed) once after
confirmation of no-regression.


thanks,

Takashi

-- 8< --
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] Bluetooth: Apply QCA Rome patches for some ATH3012 models

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_QCA_ROME.  This made
such devices working while it turned out to break other existing chips
with the very same USB ID, hence it was reverted afterwards.

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_qca()
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, the first fix we'll need is to add a check of the ROM version in
the function to skip the setup if the ROM version looks already sane,
so that it can be applied for all ath devices.

However, the world is a bit more complex than that simple solution.
Since BTUSB_ATH3012 quirk checks the bcdDevice and bails out when it's
0x0001 at the beginning of probing, so the device probe always aborts
here.

In this patch, we add another check of ROM version again, and if the
device needs patching, the probe continues.  For that, a slight
refactoring of btusb_qca_send_vendor_req() was required so that the
probe function can pass usb_device pointer directly before allocating
hci_dev stuff.

Fixes: commit f44cb4b19ed4 ("Bluetooth: btusb: Fix quirk for Atheros 1525/QCA6174")
Bugzilla: http://bugzilla.opensuse.org/show_bug.cgi?id=1082504
Tested-by: Ivan Levshin <ivan.levshin@microfocus.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>

---
 drivers/bluetooth/btusb.c | 32 +++++++++++++++++++++++++-------
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index b937cc1e2c07..9a91f44ee33b 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -2497,11 +2497,9 @@ static const struct qca_device_info qca_devices_table[] = {
 	{ 0x00000302, 28, 4, 18 }, /* Rome 3.2 */
 };
 
-static int btusb_qca_send_vendor_req(struct hci_dev *hdev, u8 request,
+static int btusb_qca_send_vendor_req(struct usb_device *udev, u8 request,
 				     void *data, u16 size)
 {
-	struct btusb_data *btdata = hci_get_drvdata(hdev);
-	struct usb_device *udev = btdata->udev;
 	int pipe, err;
 	u8 *buf;
 
@@ -2516,7 +2514,7 @@ static int btusb_qca_send_vendor_req(struct hci_dev *hdev, u8 request,
 	err = usb_control_msg(udev, pipe, request, USB_TYPE_VENDOR | USB_DIR_IN,
 			      0, 0, buf, size, USB_CTRL_SET_TIMEOUT);
 	if (err < 0) {
-		bt_dev_err(hdev, "Failed to access otp area (%d)", err);
+		dev_err(&udev->dev, "Failed to access otp area (%d)", err);
 		goto done;
 	}
 
@@ -2666,20 +2664,38 @@ static int btusb_setup_qca_load_nvm(struct hci_dev *hdev,
 	return err;
 }
 
+/* identify the ROM version and check whether patches are needed */
+static bool btusb_qca_need_patch(struct usb_device *udev)
+{
+	struct qca_version ver;
+
+	if (btusb_qca_send_vendor_req(udev, QCA_GET_TARGET_VERSION, &ver,
+				      sizeof(ver)) < 0)
+		return false;
+	/* only low ROM versions need patches */
+	return !(le32_to_cpu(ver.rom_version) & ~0xffffU);
+}
+
 static int btusb_setup_qca(struct hci_dev *hdev)
 {
+	struct btusb_data *btdata = hci_get_drvdata(hdev);
+	struct usb_device *udev = btdata->udev;
 	const struct qca_device_info *info = NULL;
 	struct qca_version ver;
 	u32 ver_rom;
 	u8 status;
 	int i, err;
 
-	err = btusb_qca_send_vendor_req(hdev, QCA_GET_TARGET_VERSION, &ver,
+	err = btusb_qca_send_vendor_req(udev, QCA_GET_TARGET_VERSION, &ver,
 					sizeof(ver));
 	if (err < 0)
 		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];
@@ -2689,7 +2705,7 @@ static int btusb_setup_qca(struct hci_dev *hdev)
 		return -ENODEV;
 	}
 
-	err = btusb_qca_send_vendor_req(hdev, QCA_CHECK_STATUS, &status,
+	err = btusb_qca_send_vendor_req(udev, QCA_CHECK_STATUS, &status,
 					sizeof(status));
 	if (err < 0)
 		return err;
@@ -2903,7 +2919,8 @@ static int btusb_probe(struct usb_interface *intf,
 		/* Old firmware would otherwise let ath3k driver load
 		 * patch and sysconfig files
 		 */
-		if (le16_to_cpu(udev->descriptor.bcdDevice) <= 0x0001)
+		if (le16_to_cpu(udev->descriptor.bcdDevice) <= 0x0001 &&
+		    !btusb_qca_need_patch(udev))
 			return -ENODEV;
 	}
 
@@ -3065,6 +3082,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