Re: [PATCH RFC] Bluetooth: Add generic support for vendor HCI frames
Zijun Hu <[email protected]>
| Newsgroups | org.kernel.vger.linux-bluetooth,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
On 7/20/2026 10:30 PM, Luiz Augusto von Dentz wrote: >> For Qualcomm multi-subsystem BT chips, the transport wire carries both >> BT-HCI and PERI-HCI frames. PERI is a subsystem on the chip. Take the >> upcoming QCC2072 support as an example: >> >> Packet type BT-HCI indicator PERI-HCI indicator >> -------------------------------------------------------------- >> CMD (Host -> Controller) 0x01 0x31 >> ACL Data (bidirectional) 0x02 0x32 >> EVENT (Controller -> Host) 0x04 0x34 > I would say this was fine, but you actuall need to reclassify these to > 0xff and then use an extra byte for the vendor opcode. Btw, you said > you could define H4 headers like this? The spec defines either: > > https://www.bluetooth.com/wp-content/uploads/Files/Specification/HTML/Core_v6.3/out/en/host-controller-interface/three-wire-uart-transport-layer.html#UUID-1cf959bb-57a0-e782-4324-a9bc4ee3f134_table-idm13359018144714 > > Or > > https://www.bluetooth.com/wp-content/uploads/Files/Specification/HTML/Core_v6.3/out/en/host-controller-interface/uart-transport-layer.html#UUID-83881875-5fbb-c366-400f-055ba726f70e_table-idm13359015052542 > Hi Luiz, Let us label this approach — which multiplexes the vendor indicator under 0xff — Solution A: virtual HCI_VENDOR_PKT (0xff) + vendor indicator (0x31/0x32/0x34) + body per indicator I did consider this solution — let us discuss it in the section below. The PERI-HCI indicators (0x31/0x32/0x34) are defined by the Qualcomm Bluetooth Extensions specification for the USB (Bulk Serialization mode), H4 UART, and SPI transports. They have already been deployed in several generations of commercial multi-subsystem BT chips. > We end up defining vendor 0xff because it is at the end of the range > which is probably safer. > I am not sure that fully holds here: First, vendors likely reason the same way — "end of range is safer", Take Qualcomm as an example: it already used 0xfb/0xfc/0xfd/0xfe as internal indicators. Second, current transport drivers already use 0xff in ways that break that expectation, as shown below: 1) bpa10x uses 0xff as the wire indicator for HCI_DIAG_PKT (0xf0): https://elixir.bootlin.com/linux/v7.1/source/drivers/bluetooth/bpa10x.c#L72 2) btmrvl remaps its wire vendor type 0xFE onto 0xff to carry Marvell vendor events: https://elixir.bootlin.com/linux/v7.1/source/drivers/bluetooth/btmrvl_sdio.c#L797 3) hci_vhci uses it for the handshake that creates the virtual hdev: https://elixir.bootlin.com/linux/v7.1/source/drivers/bluetooth/hci_vhci.c#L462 https://elixir.bootlin.com/linux/v7.1/source/drivers/bluetooth/hci_vhci.c#L523 >> To generically support vendor HCI frames: >> >> - Show them in btmon logs as they appear on the wire. >> - Allow userspace to send/receive them via HCI_CHANNEL_USER, with a >> socket option to control RX, defaulting off to eliminate regression >> risk for existing applications. > We shouldn't need to do this, we should just accept 0xff as the vendor > packet type and then use the following byte as the real opcode (0x31, > 0x32, 0x34). Otherwise, if we start accepting vendor packet types > outside 0xff this will get crowded quickly and could potentially clash > with future specs. > The indicator is a full u8, the entire [0x10, 0xef] indicator range is currently unused, and vendor indicators should be deliberately chosen to avoid conflicting with the BT SIG assignments. >> - Add hdev->recv_vendor() to handle vendor frames in hci_rx_work() like >> BT frame handlers. >> - Add hci_send_vendor_frame() API similar to existing __hci_cmd_send(). > Encoding/decoding of the frames should be transparent. I'm fine adding > code to the likes of btmon to decode vendor packets, we already have > something similar for Intel although that uses a vendor event not a > vendor packet (both use 0xff, causing the confusion). The user of the > user channel shall be able to read/write starting with 0xff then > decode/encode the next byte as the actual vendor opcode. > >> Signed-off-by: Zijun Hu <[email protected]> >> --- >> Hi Luiz >> To address the below concerns you raised on previous >> discussion, and inspired from your suggestion to add hdev callback: >> >> - Not use safer skb helper like skb_pull_data() >> - It will skip sending to the monitor, making debugging much harder >> >> Looking forward to your further comments. >> >> Why hdev->is_vendor() instead of multiplexing vendor frames under a >> virtual HCI_VENDOR_PKT + vendor indicator byte ? >> >> 1) The virtual HCI_VENDOR_PKT isn't used by the core itself >> currently, and may be removed later. >> 2) It makes full use of the indicator (hci_skb_pkt_type(skb)) space, >> which is large enough to accommodate vendor frames without an >> extra layer of nesting inside HCI_VENDOR_PKT. >> 3) It lets userspace and btmon logs see the exact same frame as it >> appears on the wire. As noted above, I chose is_vendor() over Solution A. My reasoning follows, for your reference. 1) Userspace sees exactly what is on the wire, in the same form as BT-HCI frames — clear and consistent, as shown below: ┌───────┬──────────┬────────────┬────────────┐ │ Frame │ BT-HCI │ PERI-HCI │ PERI-HCI │ │ │ existing │ is_vendor()│ Solution A │ ├───────┼──────────┼────────────┼────────────┤ │ CMD │ 0x01 … │ 0x31 … │ 0xff 0x31… │ ├───────┼──────────┼────────────┼────────────┤ │ ACL │ 0x02 … │ 0x32 … │ 0xff 0x32… │ ├───────┼──────────┼────────────┼────────────┤ │ EVENT │ 0x04 … │ 0x34 … │ 0xff 0x34… │ └───────┴──────────┴────────────┴────────────┘ 2) is_vendor() puts PERI-HCI on an equal footing with BT-HCI, so userspace and the transport driver can reuse the existing BT-HCI path rather than add a separate encode/decode layer — less effort on both sides. Could you please give me a clear direction Solution A or is_vendor()? I need it before I can move forward with the remaining QCC2072 btusb development work. thank you.