* [PATCH 1/4] Bluetooth: Introduce HCI Driver protocol
@ 2025-04-11 13:33 Hsin-chen Chuang
2025-04-11 13:33 ` [PATCH 2/4] Bluetooth: btusb: Add HCI Drv commands for configuring altsetting Hsin-chen Chuang
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: Hsin-chen Chuang @ 2025-04-11 13:33 UTC (permalink / raw)
To: luiz.dentz
Cc: Hsin-chen Chuang, chromeos-bluetooth-upstreaming, David S. Miller,
Eric Dumazet, Jakub Kicinski, Johan Hedberg, Marcel Holtmann,
Paolo Abeni, Simon Horman, Ying Hsu, linux-bluetooth,
linux-kernel, netdev
From: Hsin-chen Chuang <chharry@chromium.org>
Although commit 75ddcd5ad40e ("Bluetooth: btusb: Configure altsetting
for HCI_USER_CHANNEL") has enabled the HCI_USER_CHANNEL user to send out
SCO data through USB Bluetooth chips, it's observed that with the patch
HFP is flaky on most of the existing USB Bluetooth controllers: Intel
chips sometimes send out no packet for Transparent codec; MTK chips may
generate SCO data with a wrong handle for CVSD codec; RTK could split
the data with a wrong packet size for Transparent codec; ... etc.
To address the issue above one needs to reset the altsetting back to
zero when there is no active SCO connection, which is the same as the
BlueZ behavior, and another benefit is the bus doesn't need to reserve
bandwidth when no SCO connection.
This patch adds the infrastructure that allow the user space program to
talk to Bluetooth drivers directly:
- Define the new packet type HCI_DRV_PKT which is specifically used for
communication between the user space program and the Bluetooth drviers
- hci_send_frame intercepts the packets and invokes drivers' HCI Drv
callbacks (so far only defined for btusb)
- 2 kinds of events to user space: Command Status and Command Complete,
the former simply returns the status while the later may contain
additional response data.
Cc: chromeos-bluetooth-upstreaming@chromium.org
Fixes: b16b327edb4d ("Bluetooth: btusb: add sysfs attribute to control USB alt setting")
Signed-off-by: Hsin-chen Chuang <chharry@chromium.org>
---
drivers/bluetooth/btusb.c | 65 ++++++++++++++++++--
include/net/bluetooth/hci.h | 1 +
include/net/bluetooth/hci_core.h | 3 +
include/net/bluetooth/hci_drv.h | 74 ++++++++++++++++++++++
include/net/bluetooth/hci_mon.h | 2 +
net/bluetooth/Makefile | 3 +-
net/bluetooth/hci_core.c | 10 +++
net/bluetooth/hci_drv.c | 102 +++++++++++++++++++++++++++++++
net/bluetooth/hci_sock.c | 12 +++-
9 files changed, 263 insertions(+), 9 deletions(-)
create mode 100644 include/net/bluetooth/hci_drv.h
create mode 100644 net/bluetooth/hci_drv.c
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 7a9b89bcea22..a33c6b9f8433 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -21,6 +21,7 @@
#include <net/bluetooth/bluetooth.h>
#include <net/bluetooth/hci_core.h>
+#include <net/bluetooth/hci_drv.h>
#include "btintel.h"
#include "btbcm.h"
@@ -3754,6 +3755,57 @@ static ssize_t isoc_alt_store(struct device *dev,
static DEVICE_ATTR_RW(isoc_alt);
+static const struct {
+ u16 opcode;
+ const char *desc;
+} btusb_hci_drv_supported_commands[] = {
+ /* Common commands */
+ { HCI_DRV_OP_READ_INFO, "Read Info" },
+};
+static int btusb_hci_drv_read_info(struct hci_dev *hdev, void *data,
+ u16 data_len)
+{
+ struct hci_drv_rp_read_info *rp;
+ size_t rp_size;
+ int err, i;
+ u16 num_supported_commands = ARRAY_SIZE(btusb_hci_drv_supported_commands);
+
+ rp_size = sizeof(*rp) + num_supported_commands * 2;
+
+ rp = kmalloc(rp_size, GFP_KERNEL);
+ if (!rp)
+ return -ENOMEM;
+
+ strscpy_pad(rp->driver_name, btusb_driver.name);
+
+ rp->num_supported_commands = cpu_to_le16(num_supported_commands);
+ for (i = 0; i < num_supported_commands; i++) {
+ bt_dev_info(hdev, "Supported HCI Driver command: %s",
+ btusb_hci_drv_supported_commands[i].desc);
+ rp->supported_commands[i] =
+ cpu_to_le16(btusb_hci_drv_supported_commands[i].opcode);
+ }
+
+ err = hci_drv_cmd_complete(hdev, HCI_DRV_OP_READ_INFO,
+ HCI_DRV_STATUS_SUCCESS, rp, rp_size);
+
+ kfree(rp);
+ return err;
+}
+
+static const struct hci_drv_handler btusb_hci_drv_common_handlers[] = {
+ { btusb_hci_drv_read_info, HCI_DRV_READ_INFO_SIZE },
+};
+
+static const struct hci_drv_handler btusb_hci_drv_specific_handlers[] = {};
+
+static struct hci_drv btusb_hci_drv = {
+ .common_handler_count = ARRAY_SIZE(btusb_hci_drv_common_handlers),
+ .common_handlers = btusb_hci_drv_common_handlers,
+ .specific_handler_count = ARRAY_SIZE(btusb_hci_drv_specific_handlers),
+ .specific_handlers = btusb_hci_drv_specific_handlers,
+};
+
static int btusb_probe(struct usb_interface *intf,
const struct usb_device_id *id)
{
@@ -3893,12 +3945,13 @@ static int btusb_probe(struct usb_interface *intf,
data->reset_gpio = reset_gpio;
}
- hdev->open = btusb_open;
- hdev->close = btusb_close;
- hdev->flush = btusb_flush;
- hdev->send = btusb_send_frame;
- hdev->notify = btusb_notify;
- hdev->wakeup = btusb_wakeup;
+ hdev->open = btusb_open;
+ hdev->close = btusb_close;
+ hdev->flush = btusb_flush;
+ hdev->send = btusb_send_frame;
+ hdev->notify = btusb_notify;
+ hdev->wakeup = btusb_wakeup;
+ hdev->hci_drv = &btusb_hci_drv;
#ifdef CONFIG_PM
err = btusb_config_oob_wake(hdev);
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index a8586c3058c7..e297b312d2b7 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -494,6 +494,7 @@ enum {
#define HCI_EVENT_PKT 0x04
#define HCI_ISODATA_PKT 0x05
#define HCI_DIAG_PKT 0xf0
+#define HCI_DRV_PKT 0xf1
#define HCI_VENDOR_PKT 0xff
/* HCI packet types */
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 5115da34f881..dd80f1a398be 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -31,6 +31,7 @@
#include <linux/rculist.h>
#include <net/bluetooth/hci.h>
+#include <net/bluetooth/hci_drv.h>
#include <net/bluetooth/hci_sync.h>
#include <net/bluetooth/hci_sock.h>
#include <net/bluetooth/coredump.h>
@@ -613,6 +614,8 @@ struct hci_dev {
struct list_head monitored_devices;
bool advmon_pend_notify;
+ struct hci_drv *hci_drv;
+
#if IS_ENABLED(CONFIG_BT_LEDS)
struct led_trigger *power_led;
#endif
diff --git a/include/net/bluetooth/hci_drv.h b/include/net/bluetooth/hci_drv.h
new file mode 100644
index 000000000000..a05227b6e2df
--- /dev/null
+++ b/include/net/bluetooth/hci_drv.h
@@ -0,0 +1,74 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2025 Google Corporation
+ */
+
+#ifndef __HCI_DRV_H
+#define __HCI_DRV_H
+
+#include <linux/types.h>
+
+#include <net/bluetooth/bluetooth.h>
+#include <net/bluetooth/hci.h>
+
+struct hci_drv_cmd_hdr {
+ __le16 opcode;
+ __le16 len;
+} __packed;
+
+struct hci_drv_ev_hdr {
+ __le16 opcode;
+ __le16 len;
+} __packed;
+
+#define HCI_DRV_EV_CMD_STATUS 0x0000
+struct hci_drv_ev_cmd_status {
+ __le16 opcode;
+ __u8 status;
+} __packed;
+
+#define HCI_DRV_EV_CMD_COMPLETE 0x0001
+struct hci_drv_ev_cmd_complete {
+ __le16 opcode;
+ __u8 status;
+ __u8 data[];
+} __packed;
+
+#define HCI_DRV_STATUS_SUCCESS 0x00
+#define HCI_DRV_STATUS_UNSPECIFIED_ERROR 0x01
+#define HCI_DRV_STATUS_UNKNOWN_COMMAND 0x02
+#define HCI_DRV_STATUS_INVALID_PARAMETERS 0x03
+
+#define HCI_DRV_MAX_DRIVER_NAME_LENGTH 32
+
+/* Common commands that make sense on all drivers start from 0x0000 */
+#define HCI_DRV_OP_READ_INFO 0x0000
+#define HCI_DRV_READ_INFO_SIZE 0
+struct hci_drv_rp_read_info {
+ __u8 driver_name[HCI_DRV_MAX_DRIVER_NAME_LENGTH];
+ __le16 num_supported_commands;
+ __le16 supported_commands[];
+} __packed;
+
+/* Driver specific commands start from 0x1135 */
+#define HCI_DRV_OP_DRIVER_SPECIFIC_BASE 0x1135
+
+int hci_drv_cmd_status(struct hci_dev *hdev, u16 cmd, u8 status);
+int hci_drv_cmd_complete(struct hci_dev *hdev, u16 cmd, u8 status, void *rp,
+ size_t rp_len);
+int hci_drv_process_cmd(struct hci_dev *hdev, struct sk_buff *cmd_skb);
+
+struct hci_drv_handler {
+ int (*func)(struct hci_dev *hdev, void *data, u16 data_len);
+ size_t data_len;
+};
+
+struct hci_drv {
+ size_t common_handler_count;
+ const struct hci_drv_handler *common_handlers;
+
+ size_t specific_handler_count;
+ const struct hci_drv_handler *specific_handlers;
+};
+
+#endif /* __HCI_DRV_H */
diff --git a/include/net/bluetooth/hci_mon.h b/include/net/bluetooth/hci_mon.h
index 082f89531b88..bbd752494ef9 100644
--- a/include/net/bluetooth/hci_mon.h
+++ b/include/net/bluetooth/hci_mon.h
@@ -51,6 +51,8 @@ struct hci_mon_hdr {
#define HCI_MON_CTRL_EVENT 17
#define HCI_MON_ISO_TX_PKT 18
#define HCI_MON_ISO_RX_PKT 19
+#define HCI_MON_DRV_TX_PKT 20
+#define HCI_MON_DRV_RX_PKT 21
struct hci_mon_new_index {
__u8 type;
diff --git a/net/bluetooth/Makefile b/net/bluetooth/Makefile
index 5a3835b7dfcd..a7eede7616d8 100644
--- a/net/bluetooth/Makefile
+++ b/net/bluetooth/Makefile
@@ -14,7 +14,8 @@ bluetooth_6lowpan-y := 6lowpan.o
bluetooth-y := af_bluetooth.o hci_core.o hci_conn.o hci_event.o mgmt.o \
hci_sock.o hci_sysfs.o l2cap_core.o l2cap_sock.o smp.o lib.o \
- ecdh_helper.o mgmt_util.o mgmt_config.o hci_codec.o eir.o hci_sync.o
+ ecdh_helper.o mgmt_util.o mgmt_config.o hci_codec.o eir.o hci_sync.o \
+ hci_drv.o
bluetooth-$(CONFIG_DEV_COREDUMP) += coredump.o
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 5eb0600bbd03..2815b2d7d28d 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2911,6 +2911,8 @@ int hci_recv_frame(struct hci_dev *hdev, struct sk_buff *skb)
break;
case HCI_ISODATA_PKT:
break;
+ case HCI_DRV_PKT:
+ break;
default:
kfree_skb(skb);
return -EINVAL;
@@ -3019,6 +3021,14 @@ static int hci_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
return -EINVAL;
}
+ if (hci_skb_pkt_type(skb) == HCI_DRV_PKT) {
+ // Intercept HCI Drv packet here and don't go with hdev->send
+ // callabck.
+ err = hci_drv_process_cmd(hdev, skb);
+ kfree_skb(skb);
+ return err;
+ }
+
err = hdev->send(hdev, skb);
if (err < 0) {
bt_dev_err(hdev, "sending frame failed (%d)", err);
diff --git a/net/bluetooth/hci_drv.c b/net/bluetooth/hci_drv.c
new file mode 100644
index 000000000000..7b7a5b05740c
--- /dev/null
+++ b/net/bluetooth/hci_drv.c
@@ -0,0 +1,102 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2025 Google Corporation
+ */
+
+#include <linux/skbuff.h>
+#include <linux/types.h>
+
+#include <net/bluetooth/bluetooth.h>
+#include <net/bluetooth/hci_core.h>
+#include <net/bluetooth/hci_drv.h>
+
+int hci_drv_cmd_status(struct hci_dev *hdev, u16 cmd, u8 status)
+{
+ struct hci_drv_ev_hdr *hdr;
+ struct hci_drv_ev_cmd_status *ev;
+ struct sk_buff *skb;
+
+ skb = bt_skb_alloc(sizeof(*hdr) + sizeof(*ev), GFP_KERNEL);
+ if (!skb)
+ return -ENOMEM;
+
+ hdr = skb_put(skb, sizeof(*hdr));
+ hdr->opcode = __cpu_to_le16(HCI_DRV_EV_CMD_STATUS);
+ hdr->len = __cpu_to_le16(sizeof(*ev));
+
+ ev = skb_put(skb, sizeof(*ev));
+ ev->opcode = __cpu_to_le16(cmd);
+ ev->status = status;
+
+ hci_skb_pkt_type(skb) = HCI_DRV_PKT;
+
+ return hci_recv_frame(hdev, skb);
+}
+EXPORT_SYMBOL(hci_drv_cmd_status);
+
+int hci_drv_cmd_complete(struct hci_dev *hdev, u16 cmd, u8 status, void *rp,
+ size_t rp_len)
+{
+ struct hci_drv_ev_hdr *hdr;
+ struct hci_drv_ev_cmd_complete *ev;
+ struct sk_buff *skb;
+
+ skb = bt_skb_alloc(sizeof(*hdr) + sizeof(*ev) + rp_len, GFP_KERNEL);
+ if (!skb)
+ return -ENOMEM;
+
+ hdr = skb_put(skb, sizeof(*hdr));
+ hdr->opcode = __cpu_to_le16(HCI_DRV_EV_CMD_COMPLETE);
+ hdr->len = __cpu_to_le16(sizeof(*ev) + rp_len);
+
+ ev = skb_put(skb, sizeof(*ev));
+ ev->opcode = __cpu_to_le16(cmd);
+ ev->status = status;
+
+ skb_put_data(skb, rp, rp_len);
+
+ hci_skb_pkt_type(skb) = HCI_DRV_PKT;
+
+ return hci_recv_frame(hdev, skb);
+}
+EXPORT_SYMBOL(hci_drv_cmd_complete);
+
+int hci_drv_process_cmd(struct hci_dev *hdev, struct sk_buff *skb)
+{
+ struct hci_drv_cmd_hdr *hdr;
+ const struct hci_drv_handler *handler = NULL;
+ u16 opcode, len, offset;
+
+ hdr = skb_pull_data(skb, sizeof(*hdr));
+ if (!hdr)
+ return -EILSEQ;
+
+ opcode = __le16_to_cpu(hdr->opcode);
+ len = __le16_to_cpu(hdr->len);
+ if (len != skb->len)
+ return -EILSEQ;
+
+ if (!hdev->hci_drv)
+ return hci_drv_cmd_status(hdev, opcode,
+ HCI_DRV_STATUS_UNKNOWN_COMMAND);
+
+ if (opcode < HCI_DRV_OP_DRIVER_SPECIFIC_BASE) {
+ if (opcode < hdev->hci_drv->common_handler_count)
+ handler = &hdev->hci_drv->common_handlers[opcode];
+ } else {
+ offset = opcode - HCI_DRV_OP_DRIVER_SPECIFIC_BASE;
+ if (offset < hdev->hci_drv->specific_handler_count)
+ handler = &hdev->hci_drv->specific_handlers[offset];
+ }
+
+ if (!handler || !handler->func)
+ return hci_drv_cmd_status(hdev, opcode,
+ HCI_DRV_STATUS_UNKNOWN_COMMAND);
+
+ if (len != handler->data_len)
+ return hci_drv_cmd_status(hdev, opcode,
+ HCI_DRV_STATUS_INVALID_PARAMETERS);
+
+ return handler->func(hdev, skb->data, len);
+}
+EXPORT_SYMBOL(hci_drv_process_cmd);
diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
index 022b86797acd..428ee5c7de7e 100644
--- a/net/bluetooth/hci_sock.c
+++ b/net/bluetooth/hci_sock.c
@@ -234,7 +234,8 @@ void hci_send_to_sock(struct hci_dev *hdev, struct sk_buff *skb)
if (hci_skb_pkt_type(skb) != HCI_EVENT_PKT &&
hci_skb_pkt_type(skb) != HCI_ACLDATA_PKT &&
hci_skb_pkt_type(skb) != HCI_SCODATA_PKT &&
- hci_skb_pkt_type(skb) != HCI_ISODATA_PKT)
+ hci_skb_pkt_type(skb) != HCI_ISODATA_PKT &&
+ hci_skb_pkt_type(skb) != HCI_DRV_PKT)
continue;
} else {
/* Don't send frame to other channel types */
@@ -391,6 +392,12 @@ void hci_send_to_monitor(struct hci_dev *hdev, struct sk_buff *skb)
else
opcode = cpu_to_le16(HCI_MON_ISO_TX_PKT);
break;
+ case HCI_DRV_PKT:
+ if (bt_cb(skb)->incoming)
+ opcode = cpu_to_le16(HCI_MON_DRV_RX_PKT);
+ else
+ opcode = cpu_to_le16(HCI_MON_DRV_TX_PKT);
+ break;
case HCI_DIAG_PKT:
opcode = cpu_to_le16(HCI_MON_VENDOR_DIAG);
break;
@@ -1860,7 +1867,8 @@ static int hci_sock_sendmsg(struct socket *sock, struct msghdr *msg,
if (hci_skb_pkt_type(skb) != HCI_COMMAND_PKT &&
hci_skb_pkt_type(skb) != HCI_ACLDATA_PKT &&
hci_skb_pkt_type(skb) != HCI_SCODATA_PKT &&
- hci_skb_pkt_type(skb) != HCI_ISODATA_PKT) {
+ hci_skb_pkt_type(skb) != HCI_ISODATA_PKT &&
+ hci_skb_pkt_type(skb) != HCI_DRV_PKT) {
err = -EINVAL;
goto drop;
}
--
2.49.0.604.gff1f9ca942-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/4] Bluetooth: btusb: Add HCI Drv commands for configuring altsetting
2025-04-11 13:33 [PATCH 1/4] Bluetooth: Introduce HCI Driver protocol Hsin-chen Chuang
@ 2025-04-11 13:33 ` Hsin-chen Chuang
2025-04-11 13:33 ` [PATCH 3/4] Revert "Bluetooth: btusb: Configure altsetting for HCI_USER_CHANNEL" Hsin-chen Chuang
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Hsin-chen Chuang @ 2025-04-11 13:33 UTC (permalink / raw)
To: luiz.dentz
Cc: Hsin-chen Chuang, chromeos-bluetooth-upstreaming, Marcel Holtmann,
Ying Hsu, linux-bluetooth, linux-kernel
From: Hsin-chen Chuang <chharry@chromium.org>
Although commit 75ddcd5ad40e ("Bluetooth: btusb: Configure altsetting
for HCI_USER_CHANNEL") has enabled the HCI_USER_CHANNEL user to send out
SCO data through USB Bluetooth chips, it's observed that with the patch
HFP is flaky on most of the existing USB Bluetooth controllers: Intel
chips sometimes send out no packet for Transparent codec; MTK chips may
generate SCO data with a wrong handle for CVSD codec; RTK could split
the data with a wrong packet size for Transparent codec; ... etc.
To address the issue above one needs to reset the altsetting back to
zero when there is no active SCO connection, which is the same as the
BlueZ behavior, and another benefit is the bus doesn't need to reserve
bandwidth when no SCO connection.
This patch adds "Supported Altsettings" and "Switch Altsetting" commands
that allow the user space program to configure the altsetting freely.
This patch is tested on ChromeOS devices. The USB Bluetooth models
(CVSD, TRANS alt3, and TRANS alt6) could pass the stress HFP test narrow
band speech and wide band speech.
Cc: chromeos-bluetooth-upstreaming@chromium.org
Fixes: b16b327edb4d ("Bluetooth: btusb: add sysfs attribute to control USB alt setting")
Signed-off-by: Hsin-chen Chuang <chharry@chromium.org>
---
drivers/bluetooth/btusb.c | 70 ++++++++++++++++++++++++++++++++++++++-
1 file changed, 69 insertions(+), 1 deletion(-)
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index a33c6b9f8433..fcaee5cd728b 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -3755,12 +3755,29 @@ static ssize_t isoc_alt_store(struct device *dev,
static DEVICE_ATTR_RW(isoc_alt);
+#define BTUSB_HCI_DRV_OP_SUPPORTED_ALTSETTINGS HCI_DRV_OP_DRIVER_SPECIFIC_BASE
+#define BTUSB_HCI_DRV_SUPPORTED_ALTSETTINGS_SIZE 0
+struct btusb_hci_drv_rp_supported_altsettings {
+ __u8 num_supported_altsettings;
+ __u8 supported_altsettings[];
+} __packed;
+
+#define BTUSB_HCI_DRV_OP_SWITCH_ALTSETTING (HCI_DRV_OP_DRIVER_SPECIFIC_BASE+1)
+#define BTUSB_HCI_DRV_SWITCH_ALTSETTING_SIZE 1
+struct btusb_hci_drv_cmd_switch_altsetting {
+ __u8 new_altsetting;
+} __packed;
+
static const struct {
u16 opcode;
const char *desc;
} btusb_hci_drv_supported_commands[] = {
/* Common commands */
{ HCI_DRV_OP_READ_INFO, "Read Info" },
+
+ /* Driver specific commands */
+ { BTUSB_HCI_DRV_OP_SUPPORTED_ALTSETTINGS, "Supported Altsettings" },
+ { BTUSB_HCI_DRV_OP_SWITCH_ALTSETTING, "Switch Altsetting" },
};
static int btusb_hci_drv_read_info(struct hci_dev *hdev, void *data,
u16 data_len)
@@ -3793,11 +3810,62 @@ static int btusb_hci_drv_read_info(struct hci_dev *hdev, void *data,
return err;
}
+static int btusb_hci_drv_supported_altsettings(struct hci_dev *hdev, void *data,
+ u16 data_len)
+{
+ struct btusb_data *drvdata = hci_get_drvdata(hdev);
+ struct btusb_hci_drv_rp_supported_altsettings *rp;
+ size_t rp_size;
+ int err;
+ u8 i;
+
+ /* There are at most 7 alt (0 - 6) */
+ rp = kmalloc(sizeof(*rp) + 7, GFP_KERNEL);
+
+ rp->num_supported_altsettings = 0;
+ if (drvdata->isoc)
+ for (i = 0; i <= 6; i++)
+ if (btusb_find_altsetting(drvdata, i))
+ rp->supported_altsettings[
+ rp->num_supported_altsettings++] = i;
+
+ rp_size = sizeof(*rp) + rp->num_supported_altsettings;
+
+ err = hci_drv_cmd_complete(hdev, BTUSB_HCI_DRV_OP_SUPPORTED_ALTSETTINGS,
+ HCI_DRV_STATUS_SUCCESS, rp, rp_size);
+ kfree(rp);
+ return err;
+}
+
+static int btusb_hci_drv_switch_altsetting(struct hci_dev *hdev, void *data,
+ u16 data_len)
+{
+ struct btusb_hci_drv_cmd_switch_altsetting *cmd = data;
+ u8 status;
+
+ if (cmd->new_altsetting > 6) {
+ status = HCI_DRV_STATUS_INVALID_PARAMETERS;
+ } else {
+ if (btusb_switch_alt_setting(hdev, cmd->new_altsetting))
+ status = HCI_DRV_STATUS_UNSPECIFIED_ERROR;
+ else
+ status = HCI_DRV_STATUS_SUCCESS;
+ }
+
+ return hci_drv_cmd_status(hdev, BTUSB_HCI_DRV_OP_SWITCH_ALTSETTING,
+ status);
+}
+
static const struct hci_drv_handler btusb_hci_drv_common_handlers[] = {
{ btusb_hci_drv_read_info, HCI_DRV_READ_INFO_SIZE },
};
-static const struct hci_drv_handler btusb_hci_drv_specific_handlers[] = {};
+static const struct hci_drv_handler btusb_hci_drv_specific_handlers[] = {
+ { btusb_hci_drv_supported_altsettings,
+ BTUSB_HCI_DRV_SUPPORTED_ALTSETTINGS_SIZE },
+ { btusb_hci_drv_switch_altsetting,
+ BTUSB_HCI_DRV_SWITCH_ALTSETTING_SIZE },
+};
static struct hci_drv btusb_hci_drv = {
.common_handler_count = ARRAY_SIZE(btusb_hci_drv_common_handlers),
--
2.49.0.604.gff1f9ca942-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/4] Revert "Bluetooth: btusb: Configure altsetting for HCI_USER_CHANNEL"
2025-04-11 13:33 [PATCH 1/4] Bluetooth: Introduce HCI Driver protocol Hsin-chen Chuang
2025-04-11 13:33 ` [PATCH 2/4] Bluetooth: btusb: Add HCI Drv commands for configuring altsetting Hsin-chen Chuang
@ 2025-04-11 13:33 ` Hsin-chen Chuang
2025-04-11 13:33 ` [PATCH 4/4] Revert "Bluetooth: btusb: add sysfs attribute to control USB alt setting" Hsin-chen Chuang
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Hsin-chen Chuang @ 2025-04-11 13:33 UTC (permalink / raw)
To: luiz.dentz
Cc: Hsin-chen Chuang, chromeos-bluetooth-upstreaming, Marcel Holtmann,
linux-bluetooth, linux-kernel
From: Hsin-chen Chuang <chharry@chromium.org>
This reverts commit 75ddcd5ad40ecd9fbc9f5a7a2ed0e1e74921db3c.
This patch doesn't work quite well - It's observed that with this patch
HFP is flaky on most of the existing USB Bluetooth controllers: Intel
chips sometimes send out no packet for Transparent codec; MTK chips may
generate SCO data with a wrong handle for CVSD codec; RTK could split
the data with a wrong packet size for Transparent codec; ... etc.
Cc: chromeos-bluetooth-upstreaming@chromium.org
Signed-off-by: Hsin-chen Chuang <chharry@chromium.org>
---
drivers/bluetooth/Kconfig | 12 ------------
drivers/bluetooth/btusb.c | 41 ---------------------------------------
2 files changed, 53 deletions(-)
diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
index 7771edf54fb3..4ab32abf0f48 100644
--- a/drivers/bluetooth/Kconfig
+++ b/drivers/bluetooth/Kconfig
@@ -56,18 +56,6 @@ config BT_HCIBTUSB_POLL_SYNC
Say Y here to enable USB poll_sync for Bluetooth USB devices by
default.
-config BT_HCIBTUSB_AUTO_ISOC_ALT
- bool "Automatically adjust alternate setting for Isoc endpoints"
- depends on BT_HCIBTUSB
- default y if CHROME_PLATFORMS
- help
- Say Y here to automatically adjusting the alternate setting for
- HCI_USER_CHANNEL whenever a SCO link is established.
-
- When enabled, btusb intercepts the HCI_EV_SYNC_CONN_COMPLETE packets
- and configures isoc endpoint alternate setting automatically when
- HCI_USER_CHANNEL is in use.
-
config BT_HCIBTUSB_BCM
bool "Broadcom protocol support"
depends on BT_HCIBTUSB
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index fcaee5cd728b..b7040747b890 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -35,7 +35,6 @@ static bool force_scofix;
static bool enable_autosuspend = IS_ENABLED(CONFIG_BT_HCIBTUSB_AUTOSUSPEND);
static bool enable_poll_sync = IS_ENABLED(CONFIG_BT_HCIBTUSB_POLL_SYNC);
static bool reset = true;
-static bool auto_isoc_alt = IS_ENABLED(CONFIG_BT_HCIBTUSB_AUTO_ISOC_ALT);
static struct usb_driver btusb_driver;
@@ -1121,42 +1120,6 @@ static inline void btusb_free_frags(struct btusb_data *data)
spin_unlock_irqrestore(&data->rxlock, flags);
}
-static void btusb_sco_connected(struct btusb_data *data, struct sk_buff *skb)
-{
- struct hci_event_hdr *hdr = (void *) skb->data;
- struct hci_ev_sync_conn_complete *ev =
- (void *) skb->data + sizeof(*hdr);
- struct hci_dev *hdev = data->hdev;
- unsigned int notify_air_mode;
-
- if (hci_skb_pkt_type(skb) != HCI_EVENT_PKT)
- return;
-
- if (skb->len < sizeof(*hdr) || hdr->evt != HCI_EV_SYNC_CONN_COMPLETE)
- return;
-
- if (skb->len != sizeof(*hdr) + sizeof(*ev) || ev->status)
- return;
-
- switch (ev->air_mode) {
- case BT_CODEC_CVSD:
- notify_air_mode = HCI_NOTIFY_ENABLE_SCO_CVSD;
- break;
-
- case BT_CODEC_TRANSPARENT:
- notify_air_mode = HCI_NOTIFY_ENABLE_SCO_TRANSP;
- break;
-
- default:
- return;
- }
-
- bt_dev_info(hdev, "enabling SCO with air mode %u", ev->air_mode);
- data->sco_num = 1;
- data->air_mode = notify_air_mode;
- schedule_work(&data->work);
-}
-
static int btusb_recv_event(struct btusb_data *data, struct sk_buff *skb)
{
if (data->intr_interval) {
@@ -1164,10 +1127,6 @@ static int btusb_recv_event(struct btusb_data *data, struct sk_buff *skb)
schedule_delayed_work(&data->rx_work, 0);
}
- /* Configure altsetting for HCI_USER_CHANNEL on SCO connected */
- if (auto_isoc_alt && hci_dev_test_flag(data->hdev, HCI_USER_CHANNEL))
- btusb_sco_connected(data, skb);
-
return data->recv_event(data->hdev, skb);
}
--
2.49.0.604.gff1f9ca942-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 4/4] Revert "Bluetooth: btusb: add sysfs attribute to control USB alt setting"
2025-04-11 13:33 [PATCH 1/4] Bluetooth: Introduce HCI Driver protocol Hsin-chen Chuang
2025-04-11 13:33 ` [PATCH 2/4] Bluetooth: btusb: Add HCI Drv commands for configuring altsetting Hsin-chen Chuang
2025-04-11 13:33 ` [PATCH 3/4] Revert "Bluetooth: btusb: Configure altsetting for HCI_USER_CHANNEL" Hsin-chen Chuang
@ 2025-04-11 13:33 ` Hsin-chen Chuang
2025-04-11 14:03 ` [1/4] Bluetooth: Introduce HCI Driver protocol bluez.test.bot
2025-04-15 19:46 ` [PATCH 1/4] " Luiz Augusto von Dentz
4 siblings, 0 replies; 6+ messages in thread
From: Hsin-chen Chuang @ 2025-04-11 13:33 UTC (permalink / raw)
To: luiz.dentz
Cc: Hsin-chen Chuang, chromeos-bluetooth-upstreaming, Marcel Holtmann,
linux-bluetooth, linux-kernel
From: Hsin-chen Chuang <chharry@chromium.org>
This reverts commit b16b327edb4d030fb4c8fe38c7d299074d47ee3f.
The sysfs node introduced by this patch could potentially race with user
space. The original motivation - Support configuring altsetting from the
user space will be added by another series.
Cc: chromeos-bluetooth-upstreaming@chromium.org
Signed-off-by: Hsin-chen Chuang <chharry@chromium.org>
---
drivers/bluetooth/btusb.c | 34 +---------------------------------
1 file changed, 1 insertion(+), 33 deletions(-)
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index b7040747b890..304ec6f830f1 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -3688,32 +3688,6 @@ static const struct file_operations force_poll_sync_fops = {
.llseek = default_llseek,
};
-static ssize_t isoc_alt_show(struct device *dev,
- struct device_attribute *attr,
- char *buf)
-{
- struct btusb_data *data = dev_get_drvdata(dev);
-
- return sysfs_emit(buf, "%d\n", data->isoc_altsetting);
-}
-
-static ssize_t isoc_alt_store(struct device *dev,
- struct device_attribute *attr,
- const char *buf, size_t count)
-{
- struct btusb_data *data = dev_get_drvdata(dev);
- int alt;
- int ret;
-
- if (kstrtoint(buf, 10, &alt))
- return -EINVAL;
-
- ret = btusb_switch_alt_setting(data->hdev, alt);
- return ret < 0 ? ret : count;
-}
-
-static DEVICE_ATTR_RW(isoc_alt);
-
#define BTUSB_HCI_DRV_OP_SUPPORTED_ALTSETTINGS HCI_DRV_OP_DRIVER_SPECIFIC_BASE
#define BTUSB_HCI_DRV_SUPPORTED_ALTSETTINGS_SIZE 0
struct btusb_hci_drv_rp_supported_altsettings {
@@ -4197,10 +4171,6 @@ static int btusb_probe(struct usb_interface *intf,
data->isoc, data);
if (err < 0)
goto out_free_dev;
-
- err = device_create_file(&intf->dev, &dev_attr_isoc_alt);
- if (err)
- goto out_free_dev;
}
if (IS_ENABLED(CONFIG_BT_HCIBTUSB_BCM) && data->diag) {
@@ -4247,10 +4217,8 @@ static void btusb_disconnect(struct usb_interface *intf)
hdev = data->hdev;
usb_set_intfdata(data->intf, NULL);
- if (data->isoc) {
- device_remove_file(&intf->dev, &dev_attr_isoc_alt);
+ if (data->isoc)
usb_set_intfdata(data->isoc, NULL);
- }
if (data->diag)
usb_set_intfdata(data->diag, NULL);
--
2.49.0.604.gff1f9ca942-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: [1/4] Bluetooth: Introduce HCI Driver protocol
2025-04-11 13:33 [PATCH 1/4] Bluetooth: Introduce HCI Driver protocol Hsin-chen Chuang
` (2 preceding siblings ...)
2025-04-11 13:33 ` [PATCH 4/4] Revert "Bluetooth: btusb: add sysfs attribute to control USB alt setting" Hsin-chen Chuang
@ 2025-04-11 14:03 ` bluez.test.bot
2025-04-15 19:46 ` [PATCH 1/4] " Luiz Augusto von Dentz
4 siblings, 0 replies; 6+ messages in thread
From: bluez.test.bot @ 2025-04-11 14:03 UTC (permalink / raw)
To: linux-bluetooth, chharry
[-- Attachment #1: Type: text/plain, Size: 2214 bytes --]
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=952530
---Test result---
Test Summary:
CheckPatch PENDING 0.30 seconds
GitLint PENDING 0.20 seconds
SubjectPrefix PASS 0.46 seconds
BuildKernel PASS 24.45 seconds
CheckAllWarning PASS 26.95 seconds
CheckSparse PASS 30.86 seconds
BuildKernel32 PASS 24.07 seconds
TestRunnerSetup PASS 460.92 seconds
TestRunner_l2cap-tester PASS 21.13 seconds
TestRunner_iso-tester PASS 36.61 seconds
TestRunner_bnep-tester PASS 7.18 seconds
TestRunner_mgmt-tester FAIL 119.99 seconds
TestRunner_rfcomm-tester PASS 7.91 seconds
TestRunner_sco-tester PASS 12.57 seconds
TestRunner_ioctl-tester PASS 8.32 seconds
TestRunner_mesh-tester FAIL 6.40 seconds
TestRunner_smp-tester PASS 7.24 seconds
TestRunner_userchan-tester PASS 5.04 seconds
IncrementalBuild PENDING 0.87 seconds
Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:
##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Total: 490, Passed: 485 (99.0%), Failed: 1, Not Run: 4
Failed Test Cases
LL Privacy - Add Device 3 (AL is full) Failed 0.193 seconds
##############################
Test: TestRunner_mesh-tester - FAIL
Desc: Run mesh-tester with test-runner
Output:
Total: 10, Passed: 9 (90.0%), Failed: 1, Not Run: 0
Failed Test Cases
Mesh - Send cancel - 2 Failed 0.120 seconds
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:
---
Regards,
Linux Bluetooth
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/4] Bluetooth: Introduce HCI Driver protocol
2025-04-11 13:33 [PATCH 1/4] Bluetooth: Introduce HCI Driver protocol Hsin-chen Chuang
` (3 preceding siblings ...)
2025-04-11 14:03 ` [1/4] Bluetooth: Introduce HCI Driver protocol bluez.test.bot
@ 2025-04-15 19:46 ` Luiz Augusto von Dentz
4 siblings, 0 replies; 6+ messages in thread
From: Luiz Augusto von Dentz @ 2025-04-15 19:46 UTC (permalink / raw)
To: Hsin-chen Chuang
Cc: Hsin-chen Chuang, chromeos-bluetooth-upstreaming, David S. Miller,
Eric Dumazet, Jakub Kicinski, Johan Hedberg, Marcel Holtmann,
Paolo Abeni, Simon Horman, Ying Hsu, linux-bluetooth,
linux-kernel, netdev
Hi Hsin-chen,
On Fri, Apr 11, 2025 at 9:33 AM Hsin-chen Chuang <chharry@google.com> wrote:
>
> From: Hsin-chen Chuang <chharry@chromium.org>
>
> Although commit 75ddcd5ad40e ("Bluetooth: btusb: Configure altsetting
> for HCI_USER_CHANNEL") has enabled the HCI_USER_CHANNEL user to send out
> SCO data through USB Bluetooth chips, it's observed that with the patch
> HFP is flaky on most of the existing USB Bluetooth controllers: Intel
> chips sometimes send out no packet for Transparent codec; MTK chips may
> generate SCO data with a wrong handle for CVSD codec; RTK could split
> the data with a wrong packet size for Transparent codec; ... etc.
>
> To address the issue above one needs to reset the altsetting back to
> zero when there is no active SCO connection, which is the same as the
> BlueZ behavior, and another benefit is the bus doesn't need to reserve
> bandwidth when no SCO connection.
>
> This patch adds the infrastructure that allow the user space program to
> talk to Bluetooth drivers directly:
> - Define the new packet type HCI_DRV_PKT which is specifically used for
> communication between the user space program and the Bluetooth drviers
> - hci_send_frame intercepts the packets and invokes drivers' HCI Drv
> callbacks (so far only defined for btusb)
> - 2 kinds of events to user space: Command Status and Command Complete,
> the former simply returns the status while the later may contain
> additional response data.
>
> Cc: chromeos-bluetooth-upstreaming@chromium.org
> Fixes: b16b327edb4d ("Bluetooth: btusb: add sysfs attribute to control USB alt setting")
> Signed-off-by: Hsin-chen Chuang <chharry@chromium.org>
> ---
>
> drivers/bluetooth/btusb.c | 65 ++++++++++++++++++--
> include/net/bluetooth/hci.h | 1 +
> include/net/bluetooth/hci_core.h | 3 +
> include/net/bluetooth/hci_drv.h | 74 ++++++++++++++++++++++
> include/net/bluetooth/hci_mon.h | 2 +
> net/bluetooth/Makefile | 3 +-
> net/bluetooth/hci_core.c | 10 +++
> net/bluetooth/hci_drv.c | 102 +++++++++++++++++++++++++++++++
> net/bluetooth/hci_sock.c | 12 +++-
> 9 files changed, 263 insertions(+), 9 deletions(-)
> create mode 100644 include/net/bluetooth/hci_drv.h
> create mode 100644 net/bluetooth/hci_drv.c
>
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 7a9b89bcea22..a33c6b9f8433 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -21,6 +21,7 @@
>
> #include <net/bluetooth/bluetooth.h>
> #include <net/bluetooth/hci_core.h>
> +#include <net/bluetooth/hci_drv.h>
>
> #include "btintel.h"
> #include "btbcm.h"
> @@ -3754,6 +3755,57 @@ static ssize_t isoc_alt_store(struct device *dev,
>
> static DEVICE_ATTR_RW(isoc_alt);
>
> +static const struct {
> + u16 opcode;
> + const char *desc;
> +} btusb_hci_drv_supported_commands[] = {
> + /* Common commands */
> + { HCI_DRV_OP_READ_INFO, "Read Info" },
> +};
> +static int btusb_hci_drv_read_info(struct hci_dev *hdev, void *data,
> + u16 data_len)
> +{
> + struct hci_drv_rp_read_info *rp;
> + size_t rp_size;
> + int err, i;
> + u16 num_supported_commands = ARRAY_SIZE(btusb_hci_drv_supported_commands);
> +
> + rp_size = sizeof(*rp) + num_supported_commands * 2;
> +
> + rp = kmalloc(rp_size, GFP_KERNEL);
> + if (!rp)
> + return -ENOMEM;
> +
> + strscpy_pad(rp->driver_name, btusb_driver.name);
> +
> + rp->num_supported_commands = cpu_to_le16(num_supported_commands);
> + for (i = 0; i < num_supported_commands; i++) {
> + bt_dev_info(hdev, "Supported HCI Driver command: %s",
> + btusb_hci_drv_supported_commands[i].desc);
> + rp->supported_commands[i] =
> + cpu_to_le16(btusb_hci_drv_supported_commands[i].opcode);
> + }
> +
> + err = hci_drv_cmd_complete(hdev, HCI_DRV_OP_READ_INFO,
> + HCI_DRV_STATUS_SUCCESS, rp, rp_size);
> +
> + kfree(rp);
> + return err;
> +}
> +
> +static const struct hci_drv_handler btusb_hci_drv_common_handlers[] = {
> + { btusb_hci_drv_read_info, HCI_DRV_READ_INFO_SIZE },
> +};
> +
> +static const struct hci_drv_handler btusb_hci_drv_specific_handlers[] = {};
> +
> +static struct hci_drv btusb_hci_drv = {
> + .common_handler_count = ARRAY_SIZE(btusb_hci_drv_common_handlers),
> + .common_handlers = btusb_hci_drv_common_handlers,
> + .specific_handler_count = ARRAY_SIZE(btusb_hci_drv_specific_handlers),
> + .specific_handlers = btusb_hci_drv_specific_handlers,
> +};
> +
> static int btusb_probe(struct usb_interface *intf,
> const struct usb_device_id *id)
> {
> @@ -3893,12 +3945,13 @@ static int btusb_probe(struct usb_interface *intf,
> data->reset_gpio = reset_gpio;
> }
>
> - hdev->open = btusb_open;
> - hdev->close = btusb_close;
> - hdev->flush = btusb_flush;
> - hdev->send = btusb_send_frame;
> - hdev->notify = btusb_notify;
> - hdev->wakeup = btusb_wakeup;
> + hdev->open = btusb_open;
> + hdev->close = btusb_close;
> + hdev->flush = btusb_flush;
> + hdev->send = btusb_send_frame;
> + hdev->notify = btusb_notify;
> + hdev->wakeup = btusb_wakeup;
> + hdev->hci_drv = &btusb_hci_drv;
>
> #ifdef CONFIG_PM
> err = btusb_config_oob_wake(hdev);
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index a8586c3058c7..e297b312d2b7 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -494,6 +494,7 @@ enum {
> #define HCI_EVENT_PKT 0x04
> #define HCI_ISODATA_PKT 0x05
> #define HCI_DIAG_PKT 0xf0
> +#define HCI_DRV_PKT 0xf1
> #define HCI_VENDOR_PKT 0xff
>
> /* HCI packet types */
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 5115da34f881..dd80f1a398be 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -31,6 +31,7 @@
> #include <linux/rculist.h>
>
> #include <net/bluetooth/hci.h>
> +#include <net/bluetooth/hci_drv.h>
> #include <net/bluetooth/hci_sync.h>
> #include <net/bluetooth/hci_sock.h>
> #include <net/bluetooth/coredump.h>
> @@ -613,6 +614,8 @@ struct hci_dev {
> struct list_head monitored_devices;
> bool advmon_pend_notify;
>
> + struct hci_drv *hci_drv;
> +
> #if IS_ENABLED(CONFIG_BT_LEDS)
> struct led_trigger *power_led;
> #endif
> diff --git a/include/net/bluetooth/hci_drv.h b/include/net/bluetooth/hci_drv.h
> new file mode 100644
> index 000000000000..a05227b6e2df
> --- /dev/null
> +++ b/include/net/bluetooth/hci_drv.h
> @@ -0,0 +1,74 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2025 Google Corporation
> + */
> +
> +#ifndef __HCI_DRV_H
> +#define __HCI_DRV_H
> +
> +#include <linux/types.h>
> +
> +#include <net/bluetooth/bluetooth.h>
> +#include <net/bluetooth/hci.h>
> +
> +struct hci_drv_cmd_hdr {
> + __le16 opcode;
> + __le16 len;
> +} __packed;
> +
> +struct hci_drv_ev_hdr {
> + __le16 opcode;
> + __le16 len;
> +} __packed;
> +
> +#define HCI_DRV_EV_CMD_STATUS 0x0000
> +struct hci_drv_ev_cmd_status {
> + __le16 opcode;
> + __u8 status;
> +} __packed;
> +
> +#define HCI_DRV_EV_CMD_COMPLETE 0x0001
> +struct hci_drv_ev_cmd_complete {
> + __le16 opcode;
> + __u8 status;
> + __u8 data[];
> +} __packed;
> +
> +#define HCI_DRV_STATUS_SUCCESS 0x00
> +#define HCI_DRV_STATUS_UNSPECIFIED_ERROR 0x01
> +#define HCI_DRV_STATUS_UNKNOWN_COMMAND 0x02
> +#define HCI_DRV_STATUS_INVALID_PARAMETERS 0x03
> +
> +#define HCI_DRV_MAX_DRIVER_NAME_LENGTH 32
> +
> +/* Common commands that make sense on all drivers start from 0x0000 */
> +#define HCI_DRV_OP_READ_INFO 0x0000
> +#define HCI_DRV_READ_INFO_SIZE 0
> +struct hci_drv_rp_read_info {
> + __u8 driver_name[HCI_DRV_MAX_DRIVER_NAME_LENGTH];
> + __le16 num_supported_commands;
> + __le16 supported_commands[];
> +} __packed;
> +
> +/* Driver specific commands start from 0x1135 */
> +#define HCI_DRV_OP_DRIVER_SPECIFIC_BASE 0x1135
Or perhaps we just use the hci_opcode_ogf/hci_opcode_ocf so we have 10
bits for driver specific commands, since we are reusing the command
status/complete logic this should be really straightforward.
> +int hci_drv_cmd_status(struct hci_dev *hdev, u16 cmd, u8 status);
> +int hci_drv_cmd_complete(struct hci_dev *hdev, u16 cmd, u8 status, void *rp,
> + size_t rp_len);
> +int hci_drv_process_cmd(struct hci_dev *hdev, struct sk_buff *cmd_skb);
> +
> +struct hci_drv_handler {
> + int (*func)(struct hci_dev *hdev, void *data, u16 data_len);
> + size_t data_len;
> +};
> +
> +struct hci_drv {
> + size_t common_handler_count;
> + const struct hci_drv_handler *common_handlers;
> +
> + size_t specific_handler_count;
> + const struct hci_drv_handler *specific_handlers;
> +};
> +
> +#endif /* __HCI_DRV_H */
> diff --git a/include/net/bluetooth/hci_mon.h b/include/net/bluetooth/hci_mon.h
> index 082f89531b88..bbd752494ef9 100644
> --- a/include/net/bluetooth/hci_mon.h
> +++ b/include/net/bluetooth/hci_mon.h
> @@ -51,6 +51,8 @@ struct hci_mon_hdr {
> #define HCI_MON_CTRL_EVENT 17
> #define HCI_MON_ISO_TX_PKT 18
> #define HCI_MON_ISO_RX_PKT 19
> +#define HCI_MON_DRV_TX_PKT 20
> +#define HCI_MON_DRV_RX_PKT 21
>
> struct hci_mon_new_index {
> __u8 type;
> diff --git a/net/bluetooth/Makefile b/net/bluetooth/Makefile
> index 5a3835b7dfcd..a7eede7616d8 100644
> --- a/net/bluetooth/Makefile
> +++ b/net/bluetooth/Makefile
> @@ -14,7 +14,8 @@ bluetooth_6lowpan-y := 6lowpan.o
>
> bluetooth-y := af_bluetooth.o hci_core.o hci_conn.o hci_event.o mgmt.o \
> hci_sock.o hci_sysfs.o l2cap_core.o l2cap_sock.o smp.o lib.o \
> - ecdh_helper.o mgmt_util.o mgmt_config.o hci_codec.o eir.o hci_sync.o
> + ecdh_helper.o mgmt_util.o mgmt_config.o hci_codec.o eir.o hci_sync.o \
> + hci_drv.o
>
> bluetooth-$(CONFIG_DEV_COREDUMP) += coredump.o
>
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 5eb0600bbd03..2815b2d7d28d 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -2911,6 +2911,8 @@ int hci_recv_frame(struct hci_dev *hdev, struct sk_buff *skb)
> break;
> case HCI_ISODATA_PKT:
> break;
> + case HCI_DRV_PKT:
> + break;
> default:
> kfree_skb(skb);
> return -EINVAL;
> @@ -3019,6 +3021,14 @@ static int hci_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
> return -EINVAL;
> }
>
> + if (hci_skb_pkt_type(skb) == HCI_DRV_PKT) {
> + // Intercept HCI Drv packet here and don't go with hdev->send
> + // callabck.
> + err = hci_drv_process_cmd(hdev, skb);
> + kfree_skb(skb);
> + return err;
> + }
> +
> err = hdev->send(hdev, skb);
> if (err < 0) {
> bt_dev_err(hdev, "sending frame failed (%d)", err);
> diff --git a/net/bluetooth/hci_drv.c b/net/bluetooth/hci_drv.c
> new file mode 100644
> index 000000000000..7b7a5b05740c
> --- /dev/null
> +++ b/net/bluetooth/hci_drv.c
> @@ -0,0 +1,102 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2025 Google Corporation
> + */
> +
> +#include <linux/skbuff.h>
> +#include <linux/types.h>
> +
> +#include <net/bluetooth/bluetooth.h>
> +#include <net/bluetooth/hci_core.h>
> +#include <net/bluetooth/hci_drv.h>
> +
> +int hci_drv_cmd_status(struct hci_dev *hdev, u16 cmd, u8 status)
> +{
> + struct hci_drv_ev_hdr *hdr;
> + struct hci_drv_ev_cmd_status *ev;
> + struct sk_buff *skb;
> +
> + skb = bt_skb_alloc(sizeof(*hdr) + sizeof(*ev), GFP_KERNEL);
> + if (!skb)
> + return -ENOMEM;
> +
> + hdr = skb_put(skb, sizeof(*hdr));
> + hdr->opcode = __cpu_to_le16(HCI_DRV_EV_CMD_STATUS);
> + hdr->len = __cpu_to_le16(sizeof(*ev));
> +
> + ev = skb_put(skb, sizeof(*ev));
> + ev->opcode = __cpu_to_le16(cmd);
> + ev->status = status;
> +
> + hci_skb_pkt_type(skb) = HCI_DRV_PKT;
> +
> + return hci_recv_frame(hdev, skb);
> +}
> +EXPORT_SYMBOL(hci_drv_cmd_status);
> +
> +int hci_drv_cmd_complete(struct hci_dev *hdev, u16 cmd, u8 status, void *rp,
> + size_t rp_len)
> +{
> + struct hci_drv_ev_hdr *hdr;
> + struct hci_drv_ev_cmd_complete *ev;
> + struct sk_buff *skb;
> +
> + skb = bt_skb_alloc(sizeof(*hdr) + sizeof(*ev) + rp_len, GFP_KERNEL);
> + if (!skb)
> + return -ENOMEM;
> +
> + hdr = skb_put(skb, sizeof(*hdr));
> + hdr->opcode = __cpu_to_le16(HCI_DRV_EV_CMD_COMPLETE);
> + hdr->len = __cpu_to_le16(sizeof(*ev) + rp_len);
> +
> + ev = skb_put(skb, sizeof(*ev));
> + ev->opcode = __cpu_to_le16(cmd);
> + ev->status = status;
> +
> + skb_put_data(skb, rp, rp_len);
> +
> + hci_skb_pkt_type(skb) = HCI_DRV_PKT;
> +
> + return hci_recv_frame(hdev, skb);
> +}
> +EXPORT_SYMBOL(hci_drv_cmd_complete);
> +
> +int hci_drv_process_cmd(struct hci_dev *hdev, struct sk_buff *skb)
> +{
> + struct hci_drv_cmd_hdr *hdr;
> + const struct hci_drv_handler *handler = NULL;
> + u16 opcode, len, offset;
> +
> + hdr = skb_pull_data(skb, sizeof(*hdr));
> + if (!hdr)
> + return -EILSEQ;
> +
> + opcode = __le16_to_cpu(hdr->opcode);
> + len = __le16_to_cpu(hdr->len);
> + if (len != skb->len)
> + return -EILSEQ;
> +
> + if (!hdev->hci_drv)
> + return hci_drv_cmd_status(hdev, opcode,
> + HCI_DRV_STATUS_UNKNOWN_COMMAND);
> +
> + if (opcode < HCI_DRV_OP_DRIVER_SPECIFIC_BASE) {
> + if (opcode < hdev->hci_drv->common_handler_count)
> + handler = &hdev->hci_drv->common_handlers[opcode];
> + } else {
> + offset = opcode - HCI_DRV_OP_DRIVER_SPECIFIC_BASE;
> + if (offset < hdev->hci_drv->specific_handler_count)
> + handler = &hdev->hci_drv->specific_handlers[offset];
> + }
> +
> + if (!handler || !handler->func)
> + return hci_drv_cmd_status(hdev, opcode,
> + HCI_DRV_STATUS_UNKNOWN_COMMAND);
> +
> + if (len != handler->data_len)
> + return hci_drv_cmd_status(hdev, opcode,
> + HCI_DRV_STATUS_INVALID_PARAMETERS);
> +
> + return handler->func(hdev, skb->data, len);
> +}
> +EXPORT_SYMBOL(hci_drv_process_cmd);
> diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
> index 022b86797acd..428ee5c7de7e 100644
> --- a/net/bluetooth/hci_sock.c
> +++ b/net/bluetooth/hci_sock.c
> @@ -234,7 +234,8 @@ void hci_send_to_sock(struct hci_dev *hdev, struct sk_buff *skb)
> if (hci_skb_pkt_type(skb) != HCI_EVENT_PKT &&
> hci_skb_pkt_type(skb) != HCI_ACLDATA_PKT &&
> hci_skb_pkt_type(skb) != HCI_SCODATA_PKT &&
> - hci_skb_pkt_type(skb) != HCI_ISODATA_PKT)
> + hci_skb_pkt_type(skb) != HCI_ISODATA_PKT &&
> + hci_skb_pkt_type(skb) != HCI_DRV_PKT)
> continue;
> } else {
> /* Don't send frame to other channel types */
> @@ -391,6 +392,12 @@ void hci_send_to_monitor(struct hci_dev *hdev, struct sk_buff *skb)
> else
> opcode = cpu_to_le16(HCI_MON_ISO_TX_PKT);
> break;
> + case HCI_DRV_PKT:
> + if (bt_cb(skb)->incoming)
> + opcode = cpu_to_le16(HCI_MON_DRV_RX_PKT);
> + else
> + opcode = cpu_to_le16(HCI_MON_DRV_TX_PKT);
> + break;
> case HCI_DIAG_PKT:
> opcode = cpu_to_le16(HCI_MON_VENDOR_DIAG);
> break;
> @@ -1860,7 +1867,8 @@ static int hci_sock_sendmsg(struct socket *sock, struct msghdr *msg,
> if (hci_skb_pkt_type(skb) != HCI_COMMAND_PKT &&
> hci_skb_pkt_type(skb) != HCI_ACLDATA_PKT &&
> hci_skb_pkt_type(skb) != HCI_SCODATA_PKT &&
> - hci_skb_pkt_type(skb) != HCI_ISODATA_PKT) {
> + hci_skb_pkt_type(skb) != HCI_ISODATA_PKT &&
> + hci_skb_pkt_type(skb) != HCI_DRV_PKT) {
> err = -EINVAL;
> goto drop;
> }
> --
> 2.49.0.604.gff1f9ca942-goog
>
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-04-15 19:46 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-11 13:33 [PATCH 1/4] Bluetooth: Introduce HCI Driver protocol Hsin-chen Chuang
2025-04-11 13:33 ` [PATCH 2/4] Bluetooth: btusb: Add HCI Drv commands for configuring altsetting Hsin-chen Chuang
2025-04-11 13:33 ` [PATCH 3/4] Revert "Bluetooth: btusb: Configure altsetting for HCI_USER_CHANNEL" Hsin-chen Chuang
2025-04-11 13:33 ` [PATCH 4/4] Revert "Bluetooth: btusb: add sysfs attribute to control USB alt setting" Hsin-chen Chuang
2025-04-11 14:03 ` [1/4] Bluetooth: Introduce HCI Driver protocol bluez.test.bot
2025-04-15 19:46 ` [PATCH 1/4] " Luiz Augusto von Dentz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox