[PATCH] Bluetooth: btusb: Add a parameter to load fw forcibly for Intel BT

[PATCH] Bluetooth: btusb: Add a parameter to load fw forcibly for Intel BT

[Hui Wang]

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;
+	}
 
 	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);
-- 
2.25.1

RE: Bluetooth: btusb: Add a parameter to load fw forcibly for Intel BT

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 503 bytes --]

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

Re: [PATCH] Bluetooth: btusb: Add a parameter to load fw forcibly for Intel BT

[Marcel Holtmann]

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

Re: [PATCH] Bluetooth: btusb: Add a parameter to load fw forcibly for Intel BT

[Hui Wang]

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
>

Re: [PATCH] Bluetooth: btusb: Add a parameter to load fw forcibly for Intel BT

[Hui Wang]

>>>
>>>     /* 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
>>