diff mbox series

Bluetooth: btintel: Fix WBS setting for Intel legacy ROM products

Message ID 20220120075004.293700-1-hj.tedd.an@gmail.com (mailing list archive)
State Superseded
Headers show
Series Bluetooth: btintel: Fix WBS setting for Intel legacy ROM products | expand

Checks

Context Check Description
tedd_an/checkpatch success Checkpatch PASS
tedd_an/gitlint success Gitlint PASS
tedd_an/subjectprefix success PASS
tedd_an/buildkernel success Build Kernel PASS
tedd_an/buildkernel32 success Build Kernel32 PASS
tedd_an/incremental_build success Pass
tedd_an/testrunnersetup success Test Runner Setup PASS
tedd_an/testrunnerl2cap-tester success Total: 40, Passed: 40 (100.0%), Failed: 0, Not Run: 0
tedd_an/testrunnerbnep-tester success Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0
tedd_an/testrunnermgmt-tester success Total: 493, Passed: 493 (100.0%), Failed: 0, Not Run: 0
tedd_an/testrunnerrfcomm-tester success Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0
tedd_an/testrunnersco-tester success Total: 12, Passed: 12 (100.0%), Failed: 0, Not Run: 0
tedd_an/testrunnersmp-tester success Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0
tedd_an/testrunneruserchan-tester success Total: 4, Passed: 4 (100.0%), Failed: 0, Not Run: 0

Commit Message

Tedd Ho-Jeong An Jan. 20, 2022, 7:50 a.m. UTC 
From: Tedd Ho-Jeong An <tedd.an@intel.com>

This patch adds the flag to identify the Intel legacy ROM products that
don't support WBS like WP and StP.

Fixes: 3df4dfbec0f29 ("Bluetooth: btintel: Move hci quirks to setup routine")
Signed-off-by: Tedd Ho-Jeong An <tedd.an@intel.com>
---
 drivers/bluetooth/btintel.c | 10 +++++++---
 drivers/bluetooth/btintel.h |  1 +
 drivers/bluetooth/btusb.c   |  6 ++++++
 3 files changed, 14 insertions(+), 3 deletions(-)

Comments

Paul Menzel Jan. 20, 2022, 7:54 a.m. UTC | #1 
Dear Tedd,


Thank you for the patch.

Am 20.01.22 um 08:50 schrieb Tedd Ho-Jeong An:
> From: Tedd Ho-Jeong An <tedd.an@intel.com>

As this seems to be a regression, please describe the failure case, and 
how to reproduce it.

> This patch adds the flag to identify the Intel legacy ROM products that
> don't support WBS like WP and StP.

Please add, why the quirk is only for to 0x07dc and 0x0a2a, and how it 
was tested.

> Fixes: 3df4dfbec0f29 ("Bluetooth: btintel: Move hci quirks to setup routine")
> Signed-off-by: Tedd Ho-Jeong An <tedd.an@intel.com>
> ---
>   drivers/bluetooth/btintel.c | 10 +++++++---
>   drivers/bluetooth/btintel.h |  1 +
>   drivers/bluetooth/btusb.c   |  6 ++++++
>   3 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> index 1a4f8b227eac..225ed0373e9d 100644
> --- a/drivers/bluetooth/btintel.c
> +++ b/drivers/bluetooth/btintel.c
> @@ -2428,10 +2428,14 @@ static int btintel_setup_combined(struct hci_dev *hdev)
>   
>   			/* Apply the device specific HCI quirks
>   			 *
> -			 * WBS for SdP - SdP and Stp have a same hw_varaint but
> -			 * different fw_variant
> +			 * WBS for SdP - For the Legacy ROM products, only SdP
> +			 * supports the WBS. But the version information is not
> +			 * enough to use here because the StP2 and SdP have same
> +			 * hw_variant and fw_variant. So, this flag is set by
> +			 * the transport driver(btusb) based on the HW info

Please add a space before (.

> +			 * (idProduct)
>   			 */
> -			if (ver.hw_variant == 0x08 && ver.fw_variant == 0x22)
> +			if (!btintel_test_flag(hdev, INTEL_NO_WBS_SUPPORT))
>   				set_bit(HCI_QUIRK_WIDEBAND_SPEECH_SUPPORTED,
>   					&hdev->quirks);
>   
> diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
> index c9b24e9299e2..084a5e8dce39 100644
> --- a/drivers/bluetooth/btintel.h
> +++ b/drivers/bluetooth/btintel.h
> @@ -152,6 +152,7 @@ enum {
>   	INTEL_BROKEN_INITIAL_NCMD,
>   	INTEL_BROKEN_SHUTDOWN_LED,
>   	INTEL_ROM_LEGACY,
> +	INTEL_NO_WBS_SUPPORT,
>   
>   	__INTEL_NUM_FLAGS,
>   };
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index c30d131da784..566501e64c5a 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -62,6 +62,7 @@ static struct usb_driver btusb_driver;
>   #define BTUSB_QCA_WCN6855	0x1000000
>   #define BTUSB_INTEL_BROKEN_SHUTDOWN_LED	0x2000000
>   #define BTUSB_INTEL_BROKEN_INITIAL_NCMD 0x4000000
> +#define BTUSB_INTEL_NO_WBS_SUPPORT	0x8000000
>   
>   static const struct usb_device_id btusb_table[] = {
>   	/* Generic Bluetooth USB device */
> @@ -385,9 +386,11 @@ static const struct usb_device_id blacklist_table[] = {
>   	{ USB_DEVICE(0x8087, 0x0033), .driver_info = BTUSB_INTEL_COMBINED },
>   	{ USB_DEVICE(0x8087, 0x07da), .driver_info = BTUSB_CSR },
>   	{ USB_DEVICE(0x8087, 0x07dc), .driver_info = BTUSB_INTEL_COMBINED |
> +						     BTUSB_INTEL_NO_WBS_SUPPORT |
>   						     BTUSB_INTEL_BROKEN_INITIAL_NCMD |
>   						     BTUSB_INTEL_BROKEN_SHUTDOWN_LED },
>   	{ USB_DEVICE(0x8087, 0x0a2a), .driver_info = BTUSB_INTEL_COMBINED |
> +						     BTUSB_INTEL_NO_WBS_SUPPORT |
>   						     BTUSB_INTEL_BROKEN_SHUTDOWN_LED },
>   	{ USB_DEVICE(0x8087, 0x0a2b), .driver_info = BTUSB_INTEL_COMBINED },
>   	{ USB_DEVICE(0x8087, 0x0aa7), .driver_info = BTUSB_INTEL_COMBINED |
> @@ -3737,6 +3740,9 @@ static int btusb_probe(struct usb_interface *intf,
>   		hdev->send = btusb_send_frame_intel;
>   		hdev->cmd_timeout = btusb_intel_cmd_timeout;
>   
> +		if (id->driver_info & BTUSB_INTEL_NO_WBS_SUPPORT)
> +			btintel_set_flag(hdev, INTEL_NO_WBS_SUPPORT);
> +
>   		if (id->driver_info & BTUSB_INTEL_BROKEN_INITIAL_NCMD)
>   			btintel_set_flag(hdev, INTEL_BROKEN_INITIAL_NCMD);
>   


Kind regards,

Paul
bluez.test.bot@gmail.com Jan. 20, 2022, 9:03 a.m. UTC | #2 
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=606839

---Test result---

Test Summary:
CheckPatch                    PASS      1.07 seconds
GitLint                       PASS      0.35 seconds
SubjectPrefix                 PASS      0.21 seconds
BuildKernel                   PASS      29.94 seconds
BuildKernel32                 PASS      26.51 seconds
Incremental Build with patchesPASS      36.37 seconds
TestRunner: Setup             PASS      478.30 seconds
TestRunner: l2cap-tester      PASS      13.18 seconds
TestRunner: bnep-tester       PASS      5.86 seconds
TestRunner: mgmt-tester       PASS      104.31 seconds
TestRunner: rfcomm-tester     PASS      7.16 seconds
TestRunner: sco-tester        PASS      7.43 seconds
TestRunner: smp-tester        PASS      7.27 seconds
TestRunner: userchan-tester   PASS      6.13 seconds



---
Regards,
Linux Bluetooth
Marcel Holtmann Jan. 20, 2022, 10:13 a.m. UTC | #3 
Hi Tedd,

> This patch adds the flag to identify the Intel legacy ROM products that
> don't support WBS like WP and StP.
> 
> Fixes: 3df4dfbec0f29 ("Bluetooth: btintel: Move hci quirks to setup routine")
> Signed-off-by: Tedd Ho-Jeong An <tedd.an@intel.com>
> ---
> drivers/bluetooth/btintel.c | 10 +++++++---
> drivers/bluetooth/btintel.h |  1 +
> drivers/bluetooth/btusb.c   |  6 ++++++
> 3 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> index 1a4f8b227eac..225ed0373e9d 100644
> --- a/drivers/bluetooth/btintel.c
> +++ b/drivers/bluetooth/btintel.c
> @@ -2428,10 +2428,14 @@ static int btintel_setup_combined(struct hci_dev *hdev)
> 
> 			/* Apply the device specific HCI quirks
> 			 *
> -			 * WBS for SdP - SdP and Stp have a same hw_varaint but
> -			 * different fw_variant
> +			 * WBS for SdP - For the Legacy ROM products, only SdP
> +			 * supports the WBS. But the version information is not
> +			 * enough to use here because the StP2 and SdP have same
> +			 * hw_variant and fw_variant. So, this flag is set by
> +			 * the transport driver(btusb) based on the HW info
> +			 * (idProduct)
> 			 */
> -			if (ver.hw_variant == 0x08 && ver.fw_variant == 0x22)
> +			if (!btintel_test_flag(hdev, INTEL_NO_WBS_SUPPORT))
> 				set_bit(HCI_QUIRK_WIDEBAND_SPEECH_SUPPORTED,
> 					&hdev->quirks);
> 
> diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
> index c9b24e9299e2..084a5e8dce39 100644
> --- a/drivers/bluetooth/btintel.h
> +++ b/drivers/bluetooth/btintel.h
> @@ -152,6 +152,7 @@ enum {
> 	INTEL_BROKEN_INITIAL_NCMD,
> 	INTEL_BROKEN_SHUTDOWN_LED,
> 	INTEL_ROM_LEGACY,
> +	INTEL_NO_WBS_SUPPORT,

please keep it as INTEL_ROM_LEGACY_NO_WBS or INTEL_ROM_LEGACY_NO_WBS_SUPPORT. It is better to make clear that this is only for our ROM products. Especially since above it is in the section for just the ROM products.

Regards

Marcel
diff mbox series

Patch

diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index 1a4f8b227eac..225ed0373e9d 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -2428,10 +2428,14 @@  static int btintel_setup_combined(struct hci_dev *hdev)
 
 			/* Apply the device specific HCI quirks
 			 *
-			 * WBS for SdP - SdP and Stp have a same hw_varaint but
-			 * different fw_variant
+			 * WBS for SdP - For the Legacy ROM products, only SdP
+			 * supports the WBS. But the version information is not
+			 * enough to use here because the StP2 and SdP have same
+			 * hw_variant and fw_variant. So, this flag is set by
+			 * the transport driver(btusb) based on the HW info
+			 * (idProduct)
 			 */
-			if (ver.hw_variant == 0x08 && ver.fw_variant == 0x22)
+			if (!btintel_test_flag(hdev, INTEL_NO_WBS_SUPPORT))
 				set_bit(HCI_QUIRK_WIDEBAND_SPEECH_SUPPORTED,
 					&hdev->quirks);
 
diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
index c9b24e9299e2..084a5e8dce39 100644
--- a/drivers/bluetooth/btintel.h
+++ b/drivers/bluetooth/btintel.h
@@ -152,6 +152,7 @@  enum {
 	INTEL_BROKEN_INITIAL_NCMD,
 	INTEL_BROKEN_SHUTDOWN_LED,
 	INTEL_ROM_LEGACY,
+	INTEL_NO_WBS_SUPPORT,
 
 	__INTEL_NUM_FLAGS,
 };
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index c30d131da784..566501e64c5a 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -62,6 +62,7 @@  static struct usb_driver btusb_driver;
 #define BTUSB_QCA_WCN6855	0x1000000
 #define BTUSB_INTEL_BROKEN_SHUTDOWN_LED	0x2000000
 #define BTUSB_INTEL_BROKEN_INITIAL_NCMD 0x4000000
+#define BTUSB_INTEL_NO_WBS_SUPPORT	0x8000000
 
 static const struct usb_device_id btusb_table[] = {
 	/* Generic Bluetooth USB device */
@@ -385,9 +386,11 @@  static const struct usb_device_id blacklist_table[] = {
 	{ USB_DEVICE(0x8087, 0x0033), .driver_info = BTUSB_INTEL_COMBINED },
 	{ USB_DEVICE(0x8087, 0x07da), .driver_info = BTUSB_CSR },
 	{ USB_DEVICE(0x8087, 0x07dc), .driver_info = BTUSB_INTEL_COMBINED |
+						     BTUSB_INTEL_NO_WBS_SUPPORT |
 						     BTUSB_INTEL_BROKEN_INITIAL_NCMD |
 						     BTUSB_INTEL_BROKEN_SHUTDOWN_LED },
 	{ USB_DEVICE(0x8087, 0x0a2a), .driver_info = BTUSB_INTEL_COMBINED |
+						     BTUSB_INTEL_NO_WBS_SUPPORT |
 						     BTUSB_INTEL_BROKEN_SHUTDOWN_LED },
 	{ USB_DEVICE(0x8087, 0x0a2b), .driver_info = BTUSB_INTEL_COMBINED },
 	{ USB_DEVICE(0x8087, 0x0aa7), .driver_info = BTUSB_INTEL_COMBINED |
@@ -3737,6 +3740,9 @@  static int btusb_probe(struct usb_interface *intf,
 		hdev->send = btusb_send_frame_intel;
 		hdev->cmd_timeout = btusb_intel_cmd_timeout;
 
+		if (id->driver_info & BTUSB_INTEL_NO_WBS_SUPPORT)
+			btintel_set_flag(hdev, INTEL_NO_WBS_SUPPORT);
+
 		if (id->driver_info & BTUSB_INTEL_BROKEN_INITIAL_NCMD)
 			btintel_set_flag(hdev, INTEL_BROKEN_INITIAL_NCMD);