Re: [PATCH v2 1/3] Bluetooth: btintel: Add firmware lock function

[Kai-Heng Feng]

at 21:36, Marcel Holtmann <marcel@xxxxxxxxxxxx> wrote:

> Hi Kai-Heng,
>
> > When Intel 8260 starts to load Bluetooth firmware and WiFi firmware, by
> > calling btintel_download_firmware() and iwl_pcie_load_given_ucode_8000()
> > respectively, the Bluetooth btintel_download_firmware() aborts half way:
> > [   11.950216] Bluetooth: hci0: Failed to send firmware data (-38)
> >
> > Let btusb and iwlwifi load firmwares exclusively can avoid the issue, so
> > introduce a lock to use in btusb and iwlwifi.
> >
> > This issue still occurs with latest WiFi and Bluetooth firmwares.
> >
> > BugLink: https://bugs.launchpad.net/bugs/1832988
> >
> > Signed-off-by: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx>
> > ---
> > v2:
> > - Add bug report link.
> > - Rebase on latest wireless-next.
> >
> > drivers/bluetooth/btintel.c   | 14 ++++++++++++++
> > drivers/bluetooth/btintel.h   | 10 ++++++++++
> > include/linux/intel-wifi-bt.h |  8 ++++++++
> > 3 files changed, 32 insertions(+)
> > create mode 100644 include/linux/intel-wifi-bt.h
> >
> > diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> > index bb99c8653aab..93ab18d6ddad 100644
> > --- a/drivers/bluetooth/btintel.c
> > +++ b/drivers/bluetooth/btintel.c
> > @@ -20,6 +20,8 @@
> >
> > #define BDADDR_INTEL (&(bdaddr_t) {{0x00, 0x8b, 0x9e, 0x19, 0x03, 0x00}})
> >
> > +static DEFINE_MUTEX(firmware_lock);
> > +
> > int btintel_check_bdaddr(struct hci_dev *hdev)
> > {
> > 	struct hci_rp_read_bd_addr *bda;
> > @@ -709,6 +711,18 @@ int btintel_download_firmware(struct hci_dev *hdev,  
> > const struct firmware *fw,
> > }
> > EXPORT_SYMBOL_GPL(btintel_download_firmware);
> >
> > +void btintel_firmware_lock(void)
> > +{
> > +	mutex_lock(&firmware_lock);
> > +}
> > +EXPORT_SYMBOL_GPL(btintel_firmware_lock);
> > +
> > +void btintel_firmware_unlock(void)
> > +{
> > +	mutex_unlock(&firmware_lock);
> > +}
> > +EXPORT_SYMBOL_GPL(btintel_firmware_unlock);
> > +
>
> so I am not in favor of this solution. The hardware guys should start  
> looking into fixing the firmware loading and provide proper firmware that  
> can be loaded at the same time.

Of course itâs much better to fix from hardware side.

> I am also not for sure penalizing all Intel Bluetooth/WiFi combos only  
> because one of them has a bug during simultaneous loading of WiFi and  
> Bluetooth firmware.

Yes, itâs not ideal.

> Frankly it would be better to detect a failed load and try a second time  
> instead of trying to lock each other out. The cross-contamination of WiFi  
> and Bluetooth drivers is just not clean.

Ok. Where do you think is better to handle it, Bluetooth core or USB core?

Kai-Heng

> Regards
>
> Marcel