| Message ID | 20220120075004.293700-1-hj.tedd.an@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Bluetooth: btintel: Fix WBS setting for Intel legacy ROM products | expand |
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
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 --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);