| Message ID | 20220907170935.11757-1-sven@svenpeter.dev |
|---|---|
| Headers | show |
| Series | Broadcom/Apple Bluetooth driver for Apple Silicon | expand |
Hi Sven, On Wed, Sep 7, 2022 at 10:10 AM Sven Peter <sven@svenpeter.dev> wrote: > > Broadcom controllers present on Apple Silicon devices use the upper > 8 bits of the event type in the LE Extended Advertising Report for > the channel on which the frame has been received. > Add a quirk to drop the upper byte to ensure that the advertising > results are parsed correctly. > > The following excerpt from a btmon trace shows a report received on > channel 37 by these controllers: > > > HCI Event: LE Meta Event (0x3e) plen 55 > LE Extended Advertising Report (0x0d) > Num reports: 1 > Entry 0 > Event type: 0x2513 > Props: 0x0013 > Connectable > Scannable > Use legacy advertising PDUs > Data status: Complete > Reserved (0x2500) > Legacy PDU Type: Reserved (0x2513) > Address type: Public (0x00) > Address: XX:XX:XX:XX:XX:XX (Shenzhen Jingxun Software [...]) > Primary PHY: LE 1M > Secondary PHY: No packets > SID: no ADI field (0xff) > TX power: 127 dBm > RSSI: -76 dBm (0xb4) > Periodic advertising interval: 0.00 msec (0x0000) > Direct address type: Public (0x00) > Direct address: 00:00:00:00:00:00 (OUI 00-00-00) > Data length: 0x1d > [...] > Flags: 0x18 > Simultaneous LE and BR/EDR (Controller) > Simultaneous LE and BR/EDR (Host) > Company: Harman International Industries, Inc. (87) > Data: [...] > Service Data (UUID 0xfddf): > Name (complete): JBL Flip 5 > > Signed-off-by: Sven Peter <sven@svenpeter.dev> > --- > changes from v1: > - adjusted the commit message a bit to make checkpatch happy > > include/net/bluetooth/hci.h | 11 +++++++++++ > net/bluetooth/hci_event.c | 4 ++++ > 2 files changed, 15 insertions(+) > > diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h > index cf29511b25a8..62539c1a6bf2 100644 > --- a/include/net/bluetooth/hci.h > +++ b/include/net/bluetooth/hci.h > @@ -263,6 +263,17 @@ enum { > * during the hdev->setup vendor callback. > */ > HCI_QUIRK_BROKEN_ENHANCED_SETUP_SYNC_CONN, > + > + /* > + * When this quirk is set, the upper 8 bits of the evt_type field of > + * the LE Extended Advertising Report events are discarded. > + * Some Broadcom controllers found in Apple machines put the channel > + * the report was received on into these reserved bits. > + * > + * This quirk can be set before hci_register_dev is called or > + * during the hdev->setup vendor callback. > + */ > + HCI_QUIRK_FIXUP_LE_EXT_ADV_REPORT_EVT_TYPE, > }; > > /* HCI device flags */ > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c > index 485c814cf44a..b50d05211f0d 100644 > --- a/net/bluetooth/hci_event.c > +++ b/net/bluetooth/hci_event.c > @@ -6471,6 +6471,10 @@ static void hci_le_ext_adv_report_evt(struct hci_dev *hdev, void *data, > break; > > evt_type = __le16_to_cpu(info->type); > + if (test_bit(HCI_QUIRK_FIXUP_LE_EXT_ADV_REPORT_EVT_TYPE, > + &hdev->quirks)) > + evt_type &= 0xff; > + Don't think we really need to quirk in order to mask the reserved bits, according to the 5.3 spec only bits 0-6 are actually valid, that said the usage of the upper byte is sort of non-standard so I don't know what is broadcom/apple thinking that they could use like this, instead this should probably be placed in a vendor command or even add as part of the data itself with a vendor type. > legacy_evt_type = ext_evt_type_to_legacy(hdev, evt_type); > if (legacy_evt_type != LE_ADV_INVALID) { > process_adv_report(hdev, legacy_evt_type, &info->bdaddr, > -- > 2.25.1 >
Hi Luiz, On Wed, Sep 7, 2022, at 20:49, Luiz Augusto von Dentz wrote: > Hi Sven, > > On Wed, Sep 7, 2022 at 10:10 AM Sven Peter <sven@svenpeter.dev> wrote: >> >> Broadcom controllers present on Apple Silicon devices use the upper >> 8 bits of the event type in the LE Extended Advertising Report for >> the channel on which the frame has been received. >> Add a quirk to drop the upper byte to ensure that the advertising >> results are parsed correctly. >> >> The following excerpt from a btmon trace shows a report received on >> channel 37 by these controllers: >> >> > HCI Event: LE Meta Event (0x3e) plen 55 >> LE Extended Advertising Report (0x0d) >> Num reports: 1 >> Entry 0 >> Event type: 0x2513 >> Props: 0x0013 >> Connectable >> Scannable >> Use legacy advertising PDUs >> Data status: Complete >> Reserved (0x2500) >> Legacy PDU Type: Reserved (0x2513) >> Address type: Public (0x00) >> Address: XX:XX:XX:XX:XX:XX (Shenzhen Jingxun Software [...]) >> Primary PHY: LE 1M >> Secondary PHY: No packets >> SID: no ADI field (0xff) >> TX power: 127 dBm >> RSSI: -76 dBm (0xb4) >> Periodic advertising interval: 0.00 msec (0x0000) >> Direct address type: Public (0x00) >> Direct address: 00:00:00:00:00:00 (OUI 00-00-00) >> Data length: 0x1d >> [...] >> Flags: 0x18 >> Simultaneous LE and BR/EDR (Controller) >> Simultaneous LE and BR/EDR (Host) >> Company: Harman International Industries, Inc. (87) >> Data: [...] >> Service Data (UUID 0xfddf): >> Name (complete): JBL Flip 5 >> >> Signed-off-by: Sven Peter <sven@svenpeter.dev> >> --- >> changes from v1: >> - adjusted the commit message a bit to make checkpatch happy >> >> include/net/bluetooth/hci.h | 11 +++++++++++ >> net/bluetooth/hci_event.c | 4 ++++ >> 2 files changed, 15 insertions(+) >> >> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h >> index cf29511b25a8..62539c1a6bf2 100644 >> --- a/include/net/bluetooth/hci.h >> +++ b/include/net/bluetooth/hci.h >> @@ -263,6 +263,17 @@ enum { >> * during the hdev->setup vendor callback. >> */ >> HCI_QUIRK_BROKEN_ENHANCED_SETUP_SYNC_CONN, >> + >> + /* >> + * When this quirk is set, the upper 8 bits of the evt_type field of >> + * the LE Extended Advertising Report events are discarded. >> + * Some Broadcom controllers found in Apple machines put the channel >> + * the report was received on into these reserved bits. >> + * >> + * This quirk can be set before hci_register_dev is called or >> + * during the hdev->setup vendor callback. >> + */ >> + HCI_QUIRK_FIXUP_LE_EXT_ADV_REPORT_EVT_TYPE, >> }; >> >> /* HCI device flags */ >> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c >> index 485c814cf44a..b50d05211f0d 100644 >> --- a/net/bluetooth/hci_event.c >> +++ b/net/bluetooth/hci_event.c >> @@ -6471,6 +6471,10 @@ static void hci_le_ext_adv_report_evt(struct hci_dev *hdev, void *data, >> break; >> >> evt_type = __le16_to_cpu(info->type); >> + if (test_bit(HCI_QUIRK_FIXUP_LE_EXT_ADV_REPORT_EVT_TYPE, >> + &hdev->quirks)) >> + evt_type &= 0xff; >> + > > Don't think we really need to quirk in order to mask the reserved > bits, according to the 5.3 spec only bits 0-6 are actually valid, that > said the usage of the upper byte is sort of non-standard so I don't > know what is broadcom/apple thinking that they could use like this, > instead this should probably be placed in a vendor command or even add > as part of the data itself with a vendor type. Sure, I'll just mask them unconditionally then for v3. I originally thought it was a strange bug in their firmware but then I found their btmon-like tool called "PacketLogger" which actually decodes that byte and shows it in the UI as "Channel". It seems to be intentional :/ Best, Sven
Hi, v1: https://lore.kernel.org/asahi/20220801103633.27772-1-sven@svenpeter.dev/ Here's v2 of the Apple/Broadcom Bluetooth series. Most changes from v1 are only related to the device tree bindings. Other than that I fixed a few minor bugs and adjusted some commit messages to make checkpatch happier. Best, Sven Sven Peter (5): dt-bindings: net: Add generic Bluetooth controller dt-bindings: net: Add Broadcom BCM4377 family PCIe Bluetooth Bluetooth: hci_event: Add quirk to ignore byte in LE Extended Adv Report Bluetooth: Add quirk to disable extended scanning Bluetooth: hci_bcm4377: Add new driver for BCM4377 PCI boards .../bindings/net/bluetooth-controller.yaml | 30 + .../devicetree/bindings/net/bluetooth.txt | 5 - .../bindings/net/brcm,bcm4377-bluetooth.yaml | 78 + .../bindings/net/qualcomm-bluetooth.yaml | 4 +- .../bindings/soc/qcom/qcom,wcnss.yaml | 8 +- MAINTAINERS | 2 + drivers/bluetooth/Kconfig | 12 + drivers/bluetooth/Makefile | 1 + drivers/bluetooth/hci_bcm4377.c | 2515 +++++++++++++++++ include/net/bluetooth/hci.h | 21 + include/net/bluetooth/hci_core.h | 4 +- net/bluetooth/hci_event.c | 4 + 12 files changed, 2671 insertions(+), 13 deletions(-) create mode 100644 Documentation/devicetree/bindings/net/bluetooth-controller.yaml delete mode 100644 Documentation/devicetree/bindings/net/bluetooth.txt create mode 100644 Documentation/devicetree/bindings/net/brcm,bcm4377-bluetooth.yaml create mode 100644 drivers/bluetooth/hci_bcm4377.c