| Message ID | 20201130052753.111742-1-hui.wang@canonical.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Bluetooth: btusb: Add a parameter to load fw forcibly for Intel BT | expand |
This is automated email and please do not reply to this email! Dear submitter, Thank you for submitting the patches to the linux bluetooth mailing list. This is a CI test results with your patch series: PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=393001 ---Test result--- ############################## Test: CheckPatch - PASS ############################## Test: CheckGitLint - PASS ############################## Test: CheckBuildK - PASS --- Regards, Linux Bluetooth
Hi Hui, > On the machines with Intel new BT module, if users run reboot, the BT > driver will not load the firmware from rootfs during boot since it is > already in operational mode. This will confuse the users if users > update the BT firmware and expect the updated firmware to fix some > problems. > > Let the driver print a message to tell users the firmware is not > loaded and add a module parameter to let driver forcibly load the > firmware from rootfs. After users read this message, they could set > this parameter to load the firmware forcibly. > > Signed-off-by: Hui Wang <hui.wang@canonical.com> > --- > drivers/bluetooth/btusb.c | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c > index 1005b6e8ff74..d4fceedd354b 100644 > --- a/drivers/bluetooth/btusb.c > +++ b/drivers/bluetooth/btusb.c > @@ -32,6 +32,7 @@ static bool force_scofix; > static bool enable_autosuspend = IS_ENABLED(CONFIG_BT_HCIBTUSB_AUTOSUSPEND); > > static bool reset = true; > +static bool force_load_fw; > > static struct usb_driver btusb_driver; > > @@ -2589,8 +2590,15 @@ static int btusb_setup_intel_new(struct hci_dev *hdev) > return err; > > /* controller is already having an operational firmware */ > - if (ver.fw_variant == 0x23) > + if (ver.fw_variant == 0x23) { > + if (force_load_fw) { > + btintel_reset_to_bootloader(hdev); > + force_load_fw = false; > + return -EAGAIN; > + } > + bt_dev_info(hdev, "already in operational mode, not load fw. Set force_load_fw=1 to load fw forcibly"); > goto finish; > + } I don’t like this approach. I rather do this in a more generic way that resets the controller and puts it into boot loader support if support. We can use the experimental mgmt setting for this. Regards Marcel
On 12/1/20 1:05 AM, Marcel Holtmann wrote: > Hi Hui, > >> On the machines with Intel new BT module, if users run reboot, the BT >> driver will not load the firmware from rootfs during boot since it is >> already in operational mode. This will confuse the users if users >> update the BT firmware and expect the updated firmware to fix some >> problems. >> >> Let the driver print a message to tell users the firmware is not >> loaded and add a module parameter to let driver forcibly load the >> firmware from rootfs. After users read this message, they could set >> this parameter to load the firmware forcibly. >> >> Signed-off-by: Hui Wang <hui.wang@canonical.com> >> --- >> drivers/bluetooth/btusb.c | 13 ++++++++++++- >> 1 file changed, 12 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c >> index 1005b6e8ff74..d4fceedd354b 100644 >> --- a/drivers/bluetooth/btusb.c >> +++ b/drivers/bluetooth/btusb.c >> @@ -32,6 +32,7 @@ static bool force_scofix; >> static bool enable_autosuspend = IS_ENABLED(CONFIG_BT_HCIBTUSB_AUTOSUSPEND); >> >> static bool reset = true; >> +static bool force_load_fw; >> >> static struct usb_driver btusb_driver; >> >> @@ -2589,8 +2590,15 @@ static int btusb_setup_intel_new(struct hci_dev *hdev) >> return err; >> >> /* controller is already having an operational firmware */ >> - if (ver.fw_variant == 0x23) >> + if (ver.fw_variant == 0x23) { >> + if (force_load_fw) { >> + btintel_reset_to_bootloader(hdev); >> + force_load_fw = false; >> + return -EAGAIN; >> + } >> + bt_dev_info(hdev, "already in operational mode, not load fw. Set force_load_fw=1 to load fw forcibly"); >> goto finish; >> + } > I don’t like this approach. I rather do this in a more generic way that resets the controller and puts it into boot loader support if support. We can use the experimental mgmt setting for this. OK, got it, will investigate it. Thanks. > > Regards > > Marcel >
>>> >>> /* controller is already having an operational firmware */ >>> - if (ver.fw_variant == 0x23) >>> + if (ver.fw_variant == 0x23) { >>> + if (force_load_fw) { >>> + btintel_reset_to_bootloader(hdev); >>> + force_load_fw = false; >>> + return -EAGAIN; >>> + } >>> + bt_dev_info(hdev, "already in operational mode, not load >>> fw. Set force_load_fw=1 to load fw forcibly"); >>> goto finish; >>> + } >> I don’t like this approach. I rather do this in a more generic way >> that resets the controller and puts it into boot loader support if >> support. We can use the experimental mgmt setting for this. > OK, got it, will investigate it. Thanks. Hi Marcel, Do you mean use experimental features in the mgmt? Recently I read the mgmt exp_features related code, looks like the userspace tool btmgmt only supports 2 commands so far, they are expinfo and exp-debug, it looks like expinfo read some info from kernel, exp-debug set one bool value to kernel, but exp-debug sends the hard-coded uuid to kernel, my understanding is this could only set FEATURE_DEBUG. If i add a new feature like reset_controller_to_bootloader, I can't call it through expinfo or exp-debug. And the kernel commit (cbbdfa6f3319 Bluetooth: Enable controller RPA resolution using Experimental feature) add a new feature, how to set this feature via expinfo and exp-debug or some userspace tool? Thanks, Hui. >> >> Regards >> >> Marcel >>
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c index 1005b6e8ff74..d4fceedd354b 100644 --- a/drivers/bluetooth/btusb.c +++ b/drivers/bluetooth/btusb.c @@ -32,6 +32,7 @@ static bool force_scofix; static bool enable_autosuspend = IS_ENABLED(CONFIG_BT_HCIBTUSB_AUTOSUSPEND); static bool reset = true; +static bool force_load_fw; static struct usb_driver btusb_driver; @@ -2589,8 +2590,15 @@ static int btusb_setup_intel_new(struct hci_dev *hdev) return err; /* controller is already having an operational firmware */ - if (ver.fw_variant == 0x23) + if (ver.fw_variant == 0x23) { + if (force_load_fw) { + btintel_reset_to_bootloader(hdev); + force_load_fw = false; + return -EAGAIN; + } + bt_dev_info(hdev, "already in operational mode, not load fw. Set force_load_fw=1 to load fw forcibly"); goto finish; + } rettime = ktime_get(); delta = ktime_sub(rettime, calltime); @@ -4451,6 +4459,9 @@ MODULE_PARM_DESC(enable_autosuspend, "Enable USB autosuspend by default"); module_param(reset, bool, 0644); MODULE_PARM_DESC(reset, "Send HCI reset command on initialization"); +module_param(force_load_fw, bool, 0644); +MODULE_PARM_DESC(force_load_fw, "Let Intel BT module load the fw unconditionally during boot"); + MODULE_AUTHOR("Marcel Holtmann <marcel@holtmann.org>"); MODULE_DESCRIPTION("Generic Bluetooth USB driver ver " VERSION); MODULE_VERSION(VERSION);
On the machines with Intel new BT module, if users run reboot, the BT driver will not load the firmware from rootfs during boot since it is already in operational mode. This will confuse the users if users update the BT firmware and expect the updated firmware to fix some problems. Let the driver print a message to tell users the firmware is not loaded and add a module parameter to let driver forcibly load the firmware from rootfs. After users read this message, they could set this parameter to load the firmware forcibly. Signed-off-by: Hui Wang <hui.wang@canonical.com> --- drivers/bluetooth/btusb.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)