From: "Sven Peter" <sven@svenpeter.dev>
To: "Luiz Augusto von Dentz" <luiz.dentz@gmail.com>
Cc: "Marcel Holtmann" <marcel@holtmann.org>,
"Johan Hedberg" <johan.hedberg@gmail.com>,
"David S. Miller" <davem@davemloft.net>,
"Eric Dumazet" <edumazet@google.com>,
"Jakub Kicinski" <kuba@kernel.org>,
"Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>,
"Paolo Abeni" <pabeni@redhat.com>,
"Rob Herring" <robh+dt@kernel.org>,
"Hector Martin" <marcan@marcan.st>,
"Alyssa Rosenzweig" <alyssa@rosenzweig.io>,
asahi@lists.linux.dev,
"open list:NETWORKING [GENERAL]" <netdev@vger.kernel.org>,
devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
"linux-bluetooth@vger.kernel.org"
<linux-bluetooth@vger.kernel.org>,
"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 3/5] Bluetooth: hci_event: Add quirk to ignore byte in LE Extended Adv Report
Date: Wed, 07 Sep 2022 21:21:02 +0200 [thread overview]
Message-ID: <220ab728-ed5b-415d-ab15-47a7153e8e5c@www.fastmail.com> (raw)
In-Reply-To: <CABBYNZLWc=2y0aVRc+_k_XzfJeEJkJ_ebaViqUybvaDY49p2_g@mail.gmail.com>
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
next prev parent reply other threads:[~2022-09-07 19:21 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-07 17:09 [PATCH v2 0/5] Broadcom/Apple Bluetooth driver for Apple Silicon Sven Peter
2022-09-07 17:09 ` [PATCH v2 1/5] dt-bindings: net: Add generic Bluetooth controller Sven Peter
2022-09-08 11:16 ` Krzysztof Kozlowski
2022-09-19 15:15 ` Sven Peter
2022-09-07 17:09 ` [PATCH v2 2/5] dt-bindings: net: Add Broadcom BCM4377 family PCIe Bluetooth Sven Peter
2022-09-08 11:19 ` Krzysztof Kozlowski
2022-09-08 11:29 ` Martin Povišer
2022-09-08 11:34 ` Krzysztof Kozlowski
2022-09-08 11:30 ` Hector Martin
2022-09-12 21:12 ` Rob Herring
2022-09-15 13:09 ` Rob Herring
2022-09-19 15:15 ` Sven Peter
2022-09-07 17:09 ` [PATCH v2 3/5] Bluetooth: hci_event: Add quirk to ignore byte in LE Extended Adv Report Sven Peter
2022-09-07 18:49 ` Luiz Augusto von Dentz
2022-09-07 19:21 ` Sven Peter [this message]
2022-09-07 17:09 ` [PATCH v2 4/5] Bluetooth: Add quirk to disable extended scanning Sven Peter
2022-09-07 17:09 ` [PATCH v2 5/5] Bluetooth: hci_bcm4377: Add new driver for BCM4377 PCI boards Sven Peter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=220ab728-ed5b-415d-ab15-47a7153e8e5c@www.fastmail.com \
--to=sven@svenpeter.dev \
--cc=alyssa@rosenzweig.io \
--cc=asahi@lists.linux.dev \
--cc=davem@davemloft.net \
--cc=devicetree@vger.kernel.org \
--cc=edumazet@google.com \
--cc=johan.hedberg@gmail.com \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=kuba@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-bluetooth@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luiz.dentz@gmail.com \
--cc=marcan@marcan.st \
--cc=marcel@holtmann.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=robh+dt@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).