> On Oct 3, 2018, at 3:29 PM, Matt Chen <mattsled@xxxxxxxxx> wrote: > > I think Canonical were facing some wifi fw load error from some 8260 > earlier module during the BT still loading the fw. > I believe we had later 8260 sku that fixed this issue. But there are already 8260 that is affected by this bug in the wild. Search "Bluetooth: hci0: Failed to send firmware data (-38)” and there are lots of user are affected. Kai-Heng > > Hi Kai-Heng, > > Can you check with OEM for which SKU they are reporting the issue ? > Kai Heng Feng <kai.heng.feng@xxxxxxxxxxxxx> 於 2018年10月3日 週三 下午3:28寫道: >> >> >> >>> On Oct 3, 2018, at 3:24 PM, Luciano Coelho <luciano.coelho@xxxxxxxxx> wrote: >>> >>> On Wed, 2018-10-03 at 15:15 +0800, Kai-Heng Feng wrote: >>>> To avoid the firmware loading race between Bluetooth and WiFi on Intel >>>> 8260, load firmware exclusively when BT_INTEL is enabled. >>>> >>>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx> >>>> --- >>> >>> Where is this coming from? Can you explain what "the firmware loading >>> race" is? >> >> Looks like the patch is not correctly threaded. I’ll resend the series. >> >>> >>> >>>> .../net/wireless/intel/iwlwifi/pcie/trans.c | 37 ++++++++++++++++++- >>>> 1 file changed, 36 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c >>>> index cc8c53dc0ab6..c30d3989e2a8 100644 >>>> --- a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c >>>> +++ b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c >>>> @@ -71,6 +71,7 @@ >>>> #include <linux/vmalloc.h> >>>> #include <linux/pm_runtime.h> >>>> #include <linux/module.h> >>>> +#include <linux/intel-wifi-bt.h> >>> >>> I don't see this upstream. Is it something that was recently added? >>> Looks odd... >>> >>> Regardless, this should also be protected on CONFIG_BT_INTEL. >> >> Thanks, I’ll update this one. >> >>> >>> >>>> #include "iwl-drv.h" >>>> #include "iwl-trans.h" >>>> @@ -1335,6 +1336,10 @@ static int iwl_trans_pcie_start_fw(struct iwl_trans *trans, >>>> bool hw_rfkill; >>>> int ret; >>>> >>>> +#if IS_ENABLED(CONFIG_BT_INTEL) >>>> + void (*firmware_lock_func)(void); >>>> + void (*firmware_unlock_func)(void); >>>> +#endif >>>> /* This may fail if AMT took ownership of the device */ >>>> if (iwl_pcie_prepare_card_hw(trans)) { >>>> IWL_WARN(trans, "Exit HW not ready\n"); >>>> @@ -1394,6 +1399,7 @@ static int iwl_trans_pcie_start_fw(struct iwl_trans *trans, >>>> * RF-Kill switch is toggled, we will find out after having loaded >>>> * the firmware and return the proper value to the caller. >>>> */ >>>> + >>> >>> Stray empty line. >>> >>>> iwl_enable_fw_load_int(trans); >>>> >>>> /* really make sure rfkill handshake bits are cleared */ >>>> @@ -1401,8 +1407,37 @@ static int iwl_trans_pcie_start_fw(struct iwl_trans *trans, >>>> iwl_write32(trans, CSR_UCODE_DRV_GP1_CLR, CSR_UCODE_SW_BIT_RFKILL); >>>> >>>> /* Load the given image to the HW */ >>>> - if (trans->cfg->device_family >= IWL_DEVICE_FAMILY_8000) >>>> + if (trans->cfg->device_family >= IWL_DEVICE_FAMILY_8000) { >>>> +#if IS_ENABLED(CONFIG_BT_INTEL) >>>> + firmware_lock_func = symbol_request(btintel_firmware_lock); >>>> + firmware_unlock_func = symbol_request(btintel_firmware_unlock); >>>> + if (!firmware_lock_func || !firmware_unlock_func) { >>>> + if (firmware_lock_func) { >>>> + symbol_put(btintel_firmware_lock); >>>> + firmware_lock_func = NULL; >>>> + } >>>> + >>>> + if (firmware_unlock_func) { >>>> + symbol_put(btintel_firmware_unlock); >>>> + firmware_unlock_func = NULL; >>>> + } >>>> + } >>>> + >>>> + if (firmware_lock_func) >>>> + firmware_lock_func(); >>>> +#endif >>>> ret = iwl_pcie_load_given_ucode_8000(trans, fw); >>>> + >>>> +#if IS_ENABLED(CONFIG_BT_INTEL) >>>> + if (firmware_unlock_func) { >>>> + firmware_unlock_func(); >>>> + symbol_put(btintel_firmware_lock); >>>> + firmware_lock_func = NULL; >>>> + symbol_put(btintel_firmware_unlock); >>>> + firmware_unlock_func = NULL; >>>> + } >>>> +#endif >>>> + } >>>> else >>>> ret = iwl_pcie_load_given_ucode(trans, fw); >>>> >>> >>> I'm not sure I like adding this BT-specific stuff here, especially not >>> without a detailed explanation. >>> >>> Did you also send the other patches in this series to linux-wireless? I >>> can't see them… >> >> I’ll resend one soon. >> >> Thanks! >> >> Kai-Heng >> >>> >>> -- >>> Cheers, >>> Luca. >>