2022-07-20 14:09:28

by Zijun Hu

[permalink] [raw]
Subject: [PATCH v1 1/3] Bluetooth: hci_sync: Remove HCI_QUIRK_BROKEN_ERR_DATA_REPORTING

Check feature bit "Erroneous Data Reporting" instead of quirk
HCI_QUIRK_BROKEN_ERR_DATA_REPORTING to decide if HCI command
HCI_Read|Write_Default_Erroneous_Data_Reporting work fine.

BLUETOOTH CORE SPECIFICATION Version 5.3 | Vol 2, Part C | page 587
This feature indicates whether the device is able to support the
Packet_Status_Flag and the HCI commands HCI_Write_Default_-
Erroneous_Data_Reporting and HCI_Read_Default_Erroneous_-
Data_Reporting.

ALL BT controller whose device driver set the quirk currently does not
enable the fearture bit as shown by below btmon log:

@ RAW Open: hcitool (privileged) version 2.22
< HCI Command: Read Local Supported Commands (0x04|0x0002) plen 0
> HCI Event: Command Complete (0x0e) plen 68
Read Local Supported Commands (0x04|0x0002) ncmd 1
Status: Success (0x00)
Commands: 288 entries
......
Read Default Erroneous Data Reporting (Octet 18 - Bit 2)
Write Default Erroneous Data Reporting (Octet 18 - Bit 3)
......

< HCI Command: Read Default Erroneous Data Reporting (0x03|0x005a) plen 0
> HCI Event: Command Complete (0x0e) plen 4
Read Default Erroneous Data Reporting (0x03|0x005a) ncmd 1
Status: Unknown HCI Command (0x01)

< HCI Command: Read Local Supported Features (0x04|0x0003) plen 0
> HCI Event: Command Complete (0x0e) plen 12
Read Local Supported Features (0x04|0x0003) ncmd 1
Status: Success (0x00)
Features: 0xff 0xfe 0x0f 0xfe 0xd8 0x3f 0x5b 0x87
3 slot packets
......

Signed-off-by: Zijun Hu <[email protected]>
Tested-by: Zijun Hu <[email protected]>
---
include/net/bluetooth/hci.h | 12 +-----------
net/bluetooth/hci_sync.c | 7 ++-----
2 files changed, 3 insertions(+), 16 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 4a45c48eb0d2..927f51b92854 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -228,17 +228,6 @@ enum {
*/
HCI_QUIRK_VALID_LE_STATES,

- /* When this quirk is set, then erroneous data reporting
- * is ignored. This is mainly due to the fact that the HCI
- * Read Default Erroneous Data Reporting command is advertised,
- * but not supported; these controllers often reply with unknown
- * command and tend to lock up randomly. Needing a hard reset.
- *
- * This quirk can be set before hci_register_dev is called or
- * during the hdev->setup vendor callback.
- */
- HCI_QUIRK_BROKEN_ERR_DATA_REPORTING,
-
/*
* When this quirk is set, then the hci_suspend_notifier is not
* registered. This is intended for devices which drop completely
@@ -497,6 +486,7 @@ enum {
#define LMP_EXT_INQ 0x01
#define LMP_SIMUL_LE_BR 0x02
#define LMP_SIMPLE_PAIR 0x08
+#define LMP_ERR_DATA_REPORTING 0x20
#define LMP_NO_FLUSH 0x40

#define LMP_LSTO 0x01
diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index 464a5e2c56fb..0b6987b37c03 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -3221,7 +3221,7 @@ static int hci_read_page_scan_activity_sync(struct hci_dev *hdev)
static int hci_read_def_err_data_reporting_sync(struct hci_dev *hdev)
{
if (!(hdev->commands[18] & 0x04) ||
- test_bit(HCI_QUIRK_BROKEN_ERR_DATA_REPORTING, &hdev->quirks))
+ !(hdev->features[0][6] & LMP_ERR_DATA_REPORTING))
return 0;

return __hci_cmd_sync_status(hdev, HCI_OP_READ_DEF_ERR_DATA_REPORTING,
@@ -3706,7 +3706,7 @@ static int hci_set_err_data_report_sync(struct hci_dev *hdev)
bool enabled = hci_dev_test_flag(hdev, HCI_WIDEBAND_SPEECH_ENABLED);

if (!(hdev->commands[18] & 0x08) ||
- test_bit(HCI_QUIRK_BROKEN_ERR_DATA_REPORTING, &hdev->quirks))
+ !(hdev->features[0][6] & LMP_ERR_DATA_REPORTING))
return 0;

if (enabled == hdev->err_data_reporting)
@@ -3865,9 +3865,6 @@ static const struct {
HCI_QUIRK_BROKEN(STORED_LINK_KEY,
"HCI Delete Stored Link Key command is advertised, "
"but not supported."),
- HCI_QUIRK_BROKEN(ERR_DATA_REPORTING,
- "HCI Read Default Erroneous Data Reporting command is "
- "advertised, but not supported."),
HCI_QUIRK_BROKEN(READ_TRANSMIT_POWER,
"HCI Read Transmit Power Level command is advertised, "
"but not supported."),
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project


2022-07-20 14:30:51

by bluez.test.bot

[permalink] [raw]
Subject: RE: Bluetooth: Remove HCI_QUIRK_BROKEN_ERR_DATA_REPORTING

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=661486

---Test result---

Test Summary:
CheckPatch PASS 4.84 seconds
GitLint FAIL 2.87 seconds
SubjectPrefix PASS 2.63 seconds
BuildKernel PASS 33.91 seconds
BuildKernel32 PASS 30.29 seconds
Incremental Build with patchesFAIL 39.46 seconds
TestRunner: Setup PASS 505.54 seconds
TestRunner: l2cap-tester PASS 17.30 seconds
TestRunner: bnep-tester PASS 6.03 seconds
TestRunner: mgmt-tester PASS 99.74 seconds
TestRunner: rfcomm-tester PASS 9.36 seconds
TestRunner: sco-tester PASS 9.18 seconds
TestRunner: smp-tester PASS 9.26 seconds
TestRunner: userchan-tester PASS 6.18 seconds

Details
##############################
Test: GitLint - FAIL - 2.87 seconds
Run gitlint with rule in .gitlint
[v1,3/3] Bluetooth: btusb: Remove HCI_QUIRK_BROKEN_ERR_DATA_REPORTING for fake CSR
1: T1 Title exceeds max length (82>80): "[v1,3/3] Bluetooth: btusb: Remove HCI_QUIRK_BROKEN_ERR_DATA_REPORTING for fake CSR"


##############################
Test: Incremental Build with patches - FAIL - 39.46 seconds
Incremental build per patch in the series
drivers/bluetooth/btusb.c: In function ‘btusb_setup_csr’:
drivers/bluetooth/btusb.c:2075:11: error: ‘HCI_QUIRK_BROKEN_ERR_DATA_REPORTING’ undeclared (first use in this function); did you mean ‘HCI_OP_READ_DEF_ERR_DATA_REPORTING’?
2075 | set_bit(HCI_QUIRK_BROKEN_ERR_DATA_REPORTING, &hdev->quirks);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| HCI_OP_READ_DEF_ERR_DATA_REPORTING
drivers/bluetooth/btusb.c:2075:11: note: each undeclared identifier is reported only once for each function it appears in
drivers/bluetooth/btusb.c: In function ‘btusb_setup_qca’:
drivers/bluetooth/btusb.c:3358:10: error: ‘HCI_QUIRK_BROKEN_ERR_DATA_REPORTING’ undeclared (first use in this function); did you mean ‘HCI_OP_READ_DEF_ERR_DATA_REPORTING’?
3358 | set_bit(HCI_QUIRK_BROKEN_ERR_DATA_REPORTING, &hdev->quirks);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| HCI_OP_READ_DEF_ERR_DATA_REPORTING
make[2]: *** [scripts/Makefile.build:288: drivers/bluetooth/btusb.o] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [scripts/Makefile.build:550: drivers/bluetooth] Error 2
make: *** [Makefile:1834: drivers] Error 2




---
Regards,
Linux Bluetooth