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@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx>
>> ---
>> 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
