Re: [PATCH v5 05/11] Bluetooth: btintel: Fix the first HCI command not work with ROM device
From: Marcel Holtmann <marcel@holtmann.org>
Date: 2021-07-30 06:40:04
Hi Tedd,
quoted
quoted
The some legacy ROM controllers have a bug with the first HCI command sent to it returning number of completed commands as zero, which would stall the command processing in the Bluetooth core. As a workaround, send HCI Rest command first which will reset the controller to fix the issue. Signed-off-by: Tedd Ho-Jeong An <redacted> --- drivers/bluetooth/btintel.c | 21 +++++++++++++++++++++ drivers/bluetooth/btintel.h | 1 + drivers/bluetooth/btusb.c | 16 ++++++++++++++-- 3 files changed, 36 insertions(+), 2 deletions(-)diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c index bf0ad05b80fe..65ecf2ae9a10 100644 --- a/drivers/bluetooth/btintel.c +++ b/drivers/bluetooth/btintel.c@@ -1659,6 +1659,7 @@ static int btintel_legacy_rom_setup(struct hci_dev *hdev,int btintel_setup_combined(struct hci_dev *hdev) { + struct btintel_data *intel = hci_get_priv(hdev); const u8 param[1] = { 0xFF }; struct intel_version ver; struct intel_version_tlv ver_tlv;@@ -1667,6 +1668,26 @@ int btintel_setup_combined(struct hci_dev *hdev)BT_DBG("%s", hdev->name); + /* The some controllers have a bug with the first HCI command sent to it + * returning number of completed commands as zero. This would stall the + * command processing in the Bluetooth core. + * + * As a workaround, send HCI Reset command first which will reset the + * number of completed commands and allow normal command processing + * from now on. + */ + if (test_bit(INTEL_BROKEN_READ_VERSION, &intel->flags)) { + skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, + HCI_INIT_TIMEOUT); + if (IS_ERR(skb)) { + bt_dev_err(hdev, + "sending initial HCI reset failed (%ld)", + PTR_ERR(skb)); + return PTR_ERR(skb); + } + kfree_skb(skb); + } + /* Starting from TyP device, the command parameter and response are * changed even though the OCF for HCI_Intel_Read_Version command * remains same. The legacy devices can handle even if thediff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h index df7aa30142b4..29b678364a79 100644 --- a/drivers/bluetooth/btintel.h +++ b/drivers/bluetooth/btintel.h@@ -143,6 +143,7 @@ struct intel_debug_features {#define INTEL_FIRMWARE_LOADED 2 #define INTEL_FIRMWARE_FAILED 3 #define INTEL_BOOTING 4 +#define INTEL_BROKEN_READ_VERSION 5 struct btintel_data { unsigned long flags;diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c index 8c54ab03ee63..a64473c525eb 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_NEWGEN 0x2000000 #define BTUSB_INTEL_COMBINED 0x4000000 +#define BTUSB_INTEL_BROKEN_READ_VERSION 0x8000000 static const struct usb_device_id btusb_table[] = { /* Generic Bluetooth USB device */@@ -376,11 +377,14 @@ static const struct usb_device_id blacklist_table[] = {BTUSB_WIDEBAND_SPEECH | BTUSB_VALID_LE_STATES }, { USB_DEVICE(0x8087, 0x07da), .driver_info = BTUSB_CSR }, - { USB_DEVICE(0x8087, 0x07dc), .driver_info = BTUSB_INTEL_COMBINED }, - { USB_DEVICE(0x8087, 0x0a2a), .driver_info = BTUSB_INTEL_COMBINED }, + { USB_DEVICE(0x8087, 0x07dc), .driver_info = BTUSB_INTEL_COMBINED | + BTUSB_INTEL_BROKEN_READ_VERSION }, + { USB_DEVICE(0x8087, 0x0a2a), .driver_info = BTUSB_INTEL_COMBINED | + BTUSB_INTEL_BROKEN_READ_VERSION }, { USB_DEVICE(0x8087, 0x0a2b), .driver_info = BTUSB_INTEL_NEW | BTUSB_WIDEBAND_SPEECH }, { USB_DEVICE(0x8087, 0x0aa7), .driver_info = BTUSB_INTEL_COMBINED | + BTUSB_INTEL_BROKEN_READ_VERSION | BTUSB_WIDEBAND_SPEECH }, { USB_DEVICE(0x8087, 0x0aaa), .driver_info = BTUSB_INTEL_NEW | BTUSB_WIDEBAND_SPEECH |can you check that all 3 have this problem? Don’t we ever produced a ROM where this is fixed?It looks like the early version of ROM (WP2) causes the problem. StP and SdP don't have the problem. I will update accordingly.
then we should get away with having BTUSB_INTEL_COMBINED (which we can later rename back into BTUSB_INTEL) and one additional BTUSB_INTEL_BROKEN_INITIAL_NCMD. Everything else we can do internally in the Intel vendor specific handling.
WP2 - I only had mini-PCIe form factor card and it is broken. < HCI Command: Intel Read Version (0x3f|0x0005) plen 1 #1 [hci0] 9.212217 Requested Type: All Supported Types(0xff)quoted
HCI Event: Command Complete (0x0e) plen 13 #2 [hci0] 9.213338Intel Read Version (0x3f|0x0005) ncmd 0 Status: Success (0x00) Hardware platform: 0x37 Hardware variant: 0x07 Hardware revision: 1.0 Firmware variant: 0x01 Firmware revision: 8.0 Firmware build: 2-3.2013 Firmware patch: 0 StP - OK. < HCI Command: Intel Read Version (0x3f|0x0005) plen 1 #3 [hci0] 108.053455 Requested Type: All Supported Types(0xff)quoted
HCI Event: Command Complete (0x0e) plen 13 #4 [hci0] 108.054034Intel Read Version (0x3f|0x0005) ncmd 1 Status: Success (0x00) Hardware platform: 0x37 Hardware variant: 0x08 Hardware revision: 1.0 Firmware variant: 0x01 Firmware revision: 1.0 Firmware build: 3-17.2014 Firmware patch: 0 SdP - OK. < HCI Command: Intel Read Version (0x3f|0x0005) plen 1 #400 [hci0] 173.911992 Requested Type: All Supported Types(0xff)quoted
HCI Event: Command Complete (0x0e) plen 13 #401 [hci0] 173.912576Intel Read Version (0x3f|0x0005) ncmd 1 Status: Success (0x00) Hardware platform: 0x37 Hardware variant: 0x08 Hardware revision: 1.0 Firmware variant: 0x22 Firmware revision: 5.0 Firmware build: 25-20.2015 Firmware patch: 0
We should add a document in bluez/doc/intel-variants.txt that documents our own hardware and which variants and firmware combinations we have tested this with. It is good for revision history and if users complain or have slight variations of the SKUs. Regards Marcel