Bluetooth: btrtl: Ask ic_info to drop firmware

Commit Message

Hilda Wu Sept. 30, 2021, 10:36 a.m. UTC

From: Hilda Wu <[e-mail]>

Some un-support wakeup platforms keep USB power and suspend signal
is coming late, this makes Realtek some chip keep its firmware,
and make it never load new firmware.

So use vendor specific HCI command to ask them drop its firmware after
system shutdown or resume.

Signed-off-by: Hilda Wu <[e-mail]>
---
 drivers/bluetooth/btrtl.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

Comments

Marcel Holtmann Oct. 1, 2021, 9:42 a.m. UTC | #1

Hi Hilda,

> Some un-support wakeup platforms keep USB power and suspend signal
> is coming late, this makes Realtek some chip keep its firmware,
> and make it never load new firmware.
> 
> So use vendor specific HCI command to ask them drop its firmware after
> system shutdown or resume.
> 
> Signed-off-by: Hilda Wu <[e-mail]>
> ---
> drivers/bluetooth/btrtl.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)

patch has been applied to bluetooth-next tree.

Btw. is there a simple way (via vendor HCI commands or similar) to tell which RTL device supports the MSFT or AOSP extensions. I rather have this done once and not keep hacking it over and over again.

Regards

Marcel

Hilda Wu Oct. 5, 2021, 5:26 a.m. UTC | #2

Hi Marcel,

Thank you for your review and suggestions.

The MSFT extension has a HCI_VS_MSFT_Read_Supported_Features command. The AOSP extension has a read capability cmd too.
https://docs.microsoft.com/en-us/windows-hardware/drivers/bluetooth/microsoft-defined-bluetooth-hci-commands-and-events#hci_vs_msft_read_supported_features
https://source.android.com/devices/bluetooth/hci_requirements#vendor-specific-capabilities
If commands did not support, the controller should feedback event status as Unknown HCI Command (0x01).
We can go on this way.

Regards,
Hilda

-----Original Message-----
From: Marcel Holtmann <[e-mail]> 
Sent: Friday, October 1, 2021 5:42 PM
To: Hilda Wu <[e-mail]>
Cc: Johan Hedberg <[e-mail]>; Luiz Augusto von Dentz <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; [e-mail]; Max Chou <[e-mail]>; [e-mail]; KidmanLee <[e-mail]>
Subject: Re: [PATCH] Bluetooth: btrtl: Ask ic_info to drop firmware

Hi Hilda,

> Some un-support wakeup platforms keep USB power and suspend signal is 
> coming late, this makes Realtek some chip keep its firmware, and make 
> it never load new firmware.
> 
> So use vendor specific HCI command to ask them drop its firmware after 
> system shutdown or resume.
> 
> Signed-off-by: Hilda Wu <[e-mail]>
> ---
> drivers/bluetooth/btrtl.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)

patch has been applied to bluetooth-next tree.

Btw. is there a simple way (via vendor HCI commands or similar) to tell which RTL device supports the MSFT or AOSP extensions. I rather have this done once and not keep hacking it over and over again.

Regards

Marcel

------Please consider the environment before printing this e-mail.

Marcel Holtmann Oct. 7, 2021, 3:56 p.m. UTC | #3

Hi Hilda,

> The MSFT extension has a HCI_VS_MSFT_Read_Supported_Features command. The AOSP extension has a read capability cmd too.
> https://docs.microsoft.com/en-us/windows-hardware/drivers/bluetooth/microsoft-defined-bluetooth-hci-commands-and-events#hci_vs_msft_read_supported_features
> https://source.android.com/devices/bluetooth/hci_requirements#vendor-specific-capabilities
> If commands did not support, the controller should feedback event status as Unknown HCI Command (0x01).
> We can go on this way.

I am not doing trial-and-error programming here. I rather better disable any extensions for Realtek devices altogether.

Regards

Marcel

Hilda Wu Oct. 8, 2021, 10:08 a.m. UTC | #4

Hi Marcel,

I'm a little confused about this.
Did you mean that if use existing MSFT/AOSP extensions vendor cmd/event to check device has this feature.
This way is not meeting your conception, a simple way to tell which RTL device supports the MSFT or AOSP extensions?

Thanks.

Regards,
Hilda

-----Original Message-----
From: Marcel Holtmann <[e-mail]> 
Sent: Thursday, October 7, 2021 11:57 PM
To: Hilda Wu <[e-mail]>
Cc: Johan Hedberg <[e-mail]>; Luiz Augusto von Dentz <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; [e-mail]; Max Chou <[e-mail]>; [e-mail]; KidmanLee <[e-mail]>
Subject: Re: [PATCH] Bluetooth: btrtl: Ask ic_info to drop firmware

Hi Hilda,

> The MSFT extension has a HCI_VS_MSFT_Read_Supported_Features command. The AOSP extension has a read capability cmd too.
> https://docs.microsoft.com/en-us/windows-hardware/drivers/bluetooth/mi
> crosoft-defined-bluetooth-hci-commands-and-events#hci_vs_msft_read_sup
> ported_features 
> https://source.android.com/devices/bluetooth/hci_requirements#vendor-s
> pecific-capabilities If commands did not support, the controller 
> should feedback event status as Unknown HCI Command (0x01).
> We can go on this way.

I am not doing trial-and-error programming here. I rather better disable any extensions for Realtek devices altogether.

Regards

Marcel

------Please consider the environment before printing this e-mail.

Marcel Holtmann Oct. 12, 2021, 3:41 p.m. UTC | #5

Hi Hilda,

> I'm a little confused about this.
> Did you mean that if use existing MSFT/AOSP extensions vendor cmd/event to check device has this feature.
> This way is not meeting your conception, a simple way to tell which RTL device supports the MSFT or AOSP extensions?

issuing a HCI command and checking HCI command not supported error is not a good design. The Bluetooth Core spec doesn’t use that kind of design. It has supported features and supported commands. And so should any vendor extension.

Regards

Marcel

Patch

diff --git a/drivers/bluetooth/btrtl.c b/drivers/bluetooth/btrtl.c
index 1f8afa0244d8..60ddba8962ff 100644
--- a/drivers/bluetooth/btrtl.c
+++ b/drivers/bluetooth/btrtl.c
@@ -594,8 +594,10 @@  struct btrtl_device_info *btrtl_initialize(struct hci_dev *hdev,
 	hci_rev = le16_to_cpu(resp->hci_rev);
 	lmp_subver = le16_to_cpu(resp->lmp_subver);
 
-	if (resp->hci_ver == 0x8 && le16_to_cpu(resp->hci_rev) == 0x826c &&
-	    resp->lmp_ver == 0x8 && le16_to_cpu(resp->lmp_subver) == 0xa99e)
+	btrtl_dev->ic_info = btrtl_match_ic(lmp_subver, hci_rev, hci_ver,
+					    hdev->bus);
+
+	if (!btrtl_dev->ic_info)
 		btrtl_dev->drop_fw = true;
 
 	if (btrtl_dev->drop_fw) {
@@ -634,13 +636,13 @@  struct btrtl_device_info *btrtl_initialize(struct hci_dev *hdev,
 		hci_ver = resp->hci_ver;
 		hci_rev = le16_to_cpu(resp->hci_rev);
 		lmp_subver = le16_to_cpu(resp->lmp_subver);
+
+		btrtl_dev->ic_info = btrtl_match_ic(lmp_subver, hci_rev, hci_ver,
+						    hdev->bus);
 	}
 out_free:
 	kfree_skb(skb);
 
-	btrtl_dev->ic_info = btrtl_match_ic(lmp_subver, hci_rev, hci_ver,
-					    hdev->bus);
-
 	if (!btrtl_dev->ic_info) {
 		rtl_dev_info(hdev, "unknown IC info, lmp subver %04x, hci rev %04x, hci ver %04x",
 			    lmp_subver, hci_rev, hci_ver);