> On 06-Nov-2021, at 3:17 AM, Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx> wrote: > > Hi Orlando, > > On Fri, Oct 1, 2021 at 1:56 AM Orlando Chamberlain > <redecorating@xxxxxxxxxxxxxx> wrote: >> >> The LE Read Transmit Power command is Advertised on some Broadcom >> controlers, but not supported. Using this command breaks Bluetooth >> on the MacBookPro16,1 and iMac20,1. Added a quirk disabling LE Read >> Transmit Power for these devices, based off their common chip id 150. >> >> Link: https://lore.kernel.org/r/4970a940-211b-25d6-edab-21a815313954@xxxxxxxxxxxxxx >> Signed-off-by: Orlando Chamberlain <redecorating@xxxxxxxxxxxxxx> >> --- >> v1->v2: Clarified quirk description >> >> drivers/bluetooth/btbcm.c | 4 ++++ >> include/net/bluetooth/hci.h | 11 +++++++++++ >> net/bluetooth/hci_core.c | 3 ++- >> 3 files changed, 17 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c >> index e4182acee488..4ecc50d93107 100644 >> --- a/drivers/bluetooth/btbcm.c >> +++ b/drivers/bluetooth/btbcm.c >> @@ -353,6 +353,10 @@ static int btbcm_read_info(struct hci_dev *hdev) >> return PTR_ERR(skb); >> >> bt_dev_info(hdev, "BCM: chip id %u", skb->data[1]); >> + >> + if (skb->data[1] == 150) >> + set_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks); >> + >> kfree_skb(skb); >> >> /* Read Controller Features */ >> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h >> index b80415011dcd..6da9bd6b7259 100644 >> --- a/include/net/bluetooth/hci.h >> +++ b/include/net/bluetooth/hci.h >> @@ -246,6 +246,17 @@ enum { >> * HCI after resume. >> */ >> HCI_QUIRK_NO_SUSPEND_NOTIFIER, >> + >> + /* When this quirk is set, LE Read Transmit Power is disabled. >> + * This is mainly due to the fact that the HCI LE Read Transmit >> + * Power command is advertised, but not supported; these >> + * controllers often reply with unknown command and need a hard >> + * reset. >> + * >> + * This quirk can be set before hci_register_dev is called or >> + * during the hdev->setup vendor callback. >> + */ >> + HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, >> }; >> >> /* HCI device flags */ >> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c >> index 8a47a3017d61..9a23fe7c8d67 100644 >> --- a/net/bluetooth/hci_core.c >> +++ b/net/bluetooth/hci_core.c >> @@ -742,7 +742,8 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt) >> hci_req_add(req, HCI_OP_LE_READ_ADV_TX_POWER, 0, NULL); >> } >> >> - if (hdev->commands[38] & 0x80) { >> + if (hdev->commands[38] & 0x80 && >> + !test_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks)) { >> /* Read LE Min/Max Tx Power*/ >> hci_req_add(req, HCI_OP_LE_READ_TRANSMIT_POWER, >> 0, NULL); >> -- >> 2.33.0 > > Nowadays it is possible to treat errors such like this on a per > command basis (assuming it is not essential for the init sequence): > > diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h > index 979da5179ff4..f244f42cc609 100644 > --- a/include/net/bluetooth/hci.h > +++ b/include/net/bluetooth/hci.h > @@ -551,6 +551,7 @@ enum { > #define HCI_LK_AUTH_COMBINATION_P256 0x08 > > /* ---- HCI Error Codes ---- */ > +#define HCI_ERROR_UNKNOWN_CMD 0x01 > #define HCI_ERROR_UNKNOWN_CONN_ID 0x02 > #define HCI_ERROR_AUTH_FAILURE 0x05 > #define HCI_ERROR_PIN_OR_KEY_MISSING 0x06 > diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c This diff cannot be applied to stable 5.15. Could you provide a patch capable of being applied to stable. > index bb88d31d2212..9c697e058974 100644 > --- a/net/bluetooth/hci_sync.c > +++ b/net/bluetooth/hci_sync.c > @@ -3325,11 +3325,18 @@ static int > hci_le_read_adv_tx_power_sync(struct hci_dev *hdev) > /* Read LE Min/Max Tx Power*/ > static int hci_le_read_tx_power_sync(struct hci_dev *hdev) > { > + int status; > + > if (!(hdev->commands[38] & 0x80)) > return 0; > > - return __hci_cmd_sync_status(hdev, HCI_OP_LE_READ_TRANSMIT_POWER, > - 0, NULL, HCI_CMD_TIMEOUT); > + status = __hci_cmd_sync_status(hdev, HCI_OP_LE_READ_TRANSMIT_POWER, > + 0, NULL, HCI_CMD_TIMEOUT); > + /* Ignore if command is not really supported */ > + if (status == HCI_ERROR_UNKNOWN_CMD) > + return 0; > + > + return status; > } > > /* Read LE Accept List Size */ > > Anyway, it would probably be worth pointing out to the vendor they > have a broken firmware if they do mark the command as supported but > return such error. > > -- > Luiz Augusto von Dentz >
