| Message ID | 20210930141256.19943-1-redecorating@protonmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | Bluetooth: add quirk disabling query LE tx power | expand |
On 30.09.21 16:13, Orlando Chamberlain wrote: > Querying LE tx power on startup broke Bluetooth on some Broadcom chips > in Apple computers (at least MacBookPro16,1 and iMac20,1). Added a quirk > disabling this query for affected devices, based off their common chip > id 150. Affected devices will not be able to query LE tx power, however > they were not doing this before. > > Fixes: 7c395ea521e6m ("Bluetooth: Query LE tx power on startup") > Signed-off-by: Orlando Chamberlain <redecorating@protonmail.com> FWIW, if you need to respin this for some reason, could you do me a favour and add the following after the "Fixes" line please: Link: https://lore.kernel.org/r/4970a940-211b-25d6-edab-21a815313954@protonmail.com That makes is easier to find related discussions and rationale behind a certain change, as explained here: https://www.kernel.org/doc/html/latest/maintainer/configure-git.html This line is not crucial, but it makes my life easier as well, as I slowly start to track regressions again. And this time I'm doing it with the help of a software I wrote just for this purpose. I used your report as one of the first few to give this "regzbot" a test, hence the issue can now be seen in the webinterface (which is still a bit ugly, but it does the job): https://linux-regtracking.leemhuis.info/regzbot/mainline.html And the thing is: when regzbot sees a patch with above Link:-tag hit mainline it will automatically mark the issue as resolved, saving me some work. Thx! Ciao, Thorsten
Hi Orlando, > Querying LE tx power on startup broke Bluetooth on some Broadcom chips > in Apple computers (at least MacBookPro16,1 and iMac20,1). Added a quirk > disabling this query for affected devices, based off their common chip > id 150. Affected devices will not be able to query LE tx power, however > they were not doing this before. > > Fixes: 7c395ea521e6m ("Bluetooth: Query LE tx power on startup") > Signed-off-by: Orlando Chamberlain <redecorating@protonmail.com> > --- > drivers/bluetooth/btbcm.c | 4 ++++ > include/net/bluetooth/hci.h | 8 ++++++++ > net/bluetooth/hci_core.c | 3 ++- > 3 files changed, 14 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..5e0dd0c39ade 100644 > --- a/include/net/bluetooth/hci.h > +++ b/include/net/bluetooth/hci.h > @@ -246,6 +246,14 @@ enum { > * HCI after resume. > */ > HCI_QUIRK_NO_SUSPEND_NOTIFIER, > + > + /* > + * When this quirk is set, LE tx power is not queried on startup. > + * > + * 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..16e39739c662 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); so I really need the btmon traces from the device init (so unload and reload the module) and we need to see what commands are supported and what commands are failing. Since you say this is on a MacBook, I assume this is an UART based Broadcom chip. Sometimes Broadcom has been really flaky with their actually implemented commands. However in some cases firmware updates do fix this. So any chance you can boot OS X and check that the latest firmware is loaded. Regards Marcel
On 1/10/21 03:58, Marcel Holtmann wrote: > so I really need the btmon traces from the device init (so unload and reload the module) and we need to see what commands are supported and what commands are failing. I'll attach the full file I got with btmon -w MacBookPro16,1.btsnoop, but these seem like the important bits: < HCI Command: Read Local Supported Commands (0x04|0x0002) plen 0 #43 [hci0] 9.217379 > HCI Event: Command Complete (0x0e) plen 68 #44 [hci0] 9.218033 Read Local Supported Commands (0x04|0x0002) ncmd 1 Status: Success (0x00) Commands: 223 entries ## many many lines here ## LE Read Transmit Power (Octet 38 - Bit 7) LE Read RF Path Compensation (Octet 39 - Bit 0) LE Write RF Path Compensation (Octet 39 - Bit 1) LE Set Privacy Mode (Octet 39 - Bit 2) Read Local Simple Pairing Options (Octet 41 - Bit At the end of the trace: < HCI Command: LE Read Transmit Power (0x08|0x004b) plen 0 #69 [hci0] 9.226953 > HCI Event: Command Complete (0x0e) plen 4 #70 [hci0] 9.227515 LE Read Transmit Power (0x08|0x004b) ncmd 1 Status: Unknown HCI Command (0x01) = Close Index: F8:FF:C2:06:46:63 [hci0] 9.227666 I'm guessing that this means it reports that it supports the command but it doesn't, so if this is the case, I'd have to change the description of the quirk to clarify that. > Since you say this is on a MacBook, I assume this is an UART based Broadcom chip. Sometimes Broadcom has been really flaky with their actually implemented commands. However in some cases firmware updates do fix this. So any chance you can boot OS X and check that the latest firmware is loaded. Bluetooth for this device is indeed through uart: # lspci -vvvnnd '8086:a328' 00:1e.0 Communication controller [0780]: Intel Corporation Cannon Lake PCH Serial IO UART Host Controller [8086:a328] (rev 10) Subsystem: Intel Corporation Device [8086:7270] Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx- Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx- Latency: 0, Cache Line Size: 256 bytes Interrupt: pin A routed to IRQ 20 IOMMU group: 8 Region 0: Memory at 4000000000 (64-bit, non-prefetchable) [size=4K] Capabilities: [80] Power Management version 3 Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-) Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME- Capabilities: [90] Vendor Specific Information: Len=14 <?> Kernel driver in use: intel-lpss Kernel modules: intel_lpss_pci $ cat /sys/bus/pci/devices/0000:00:1e.0/dw-apb-uart.0/serial0/serial0-0/modalias acpi:BCM2E7C:APPLE-UART-BLTH: I've just updated macOS to 11.6, which should have updated firmware. The issue is still present.
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..5e0dd0c39ade 100644 --- a/include/net/bluetooth/hci.h +++ b/include/net/bluetooth/hci.h @@ -246,6 +246,14 @@ enum { * HCI after resume. */ HCI_QUIRK_NO_SUSPEND_NOTIFIER, + + /* + * When this quirk is set, LE tx power is not queried on startup. + * + * 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..16e39739c662 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);
Querying LE tx power on startup broke Bluetooth on some Broadcom chips in Apple computers (at least MacBookPro16,1 and iMac20,1). Added a quirk disabling this query for affected devices, based off their common chip id 150. Affected devices will not be able to query LE tx power, however they were not doing this before. Fixes: 7c395ea521e6m ("Bluetooth: Query LE tx power on startup") Signed-off-by: Orlando Chamberlain <redecorating@protonmail.com> --- drivers/bluetooth/btbcm.c | 4 ++++ include/net/bluetooth/hci.h | 8 ++++++++ net/bluetooth/hci_core.c | 3 ++- 3 files changed, 14 insertions(+), 1 deletion(-)