* [PATCH 13/13] Bluetooth: btqca: Fix qca_set_bdaddr() using wrong HCI event type
From: Zijun Hu @ 2026-06-22 14:52 UTC (permalink / raw)
To: Marcel Holtmann, Luiz Augusto von Dentz, Rocky Liao,
Bartosz Golaszewski, Ben Young Tae Kim, Balakrishna Godavarthi,
Matthias Kaehlcke
Cc: Zijun Hu, linux-bluetooth, linux-kernel, Luiz Augusto von Dentz,
linux-arm-msm, Zijun Hu
In-Reply-To: <20260622-bt_bugfix-v1-0-11f936d84e72@oss.qualcomm.com>
EDL_WRITE_BD_ADDR_OPCODE (0xFC14) returns a command complete event,
not a VSE, but qca_set_bdaddr() waits for HCI_EV_VENDOR.
Fix by passing 0 as the event parameter to __hci_cmd_sync_ev() to
wait for the command complete event instead.
Fixes: 5c0a1001c8be ("Bluetooth: hci_qca: Add helper to set device address")
Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
---
drivers/bluetooth/btqca.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
index 875216e15603..f3487de813c2 100644
--- a/drivers/bluetooth/btqca.c
+++ b/drivers/bluetooth/btqca.c
@@ -1011,8 +1011,7 @@ int qca_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr)
baswap(&bdaddr_swapped, bdaddr);
skb = __hci_cmd_sync_ev(hdev, EDL_WRITE_BD_ADDR_OPCODE, 6,
- &bdaddr_swapped, HCI_EV_VENDOR,
- HCI_INIT_TIMEOUT);
+ &bdaddr_swapped, 0, HCI_INIT_TIMEOUT);
if (IS_ERR(skb)) {
err = PTR_ERR(skb);
bt_dev_err(hdev, "QCA Change address cmd failed (%d)", err);
--
2.34.1
^ permalink raw reply related
* [PATCH 12/13] Bluetooth: btqca: Fix undetected error HCI status in qca_send_reset()
From: Zijun Hu @ 2026-06-22 14:52 UTC (permalink / raw)
To: Marcel Holtmann, Luiz Augusto von Dentz, Rocky Liao,
Bartosz Golaszewski, Ben Young Tae Kim, Balakrishna Godavarthi,
Matthias Kaehlcke
Cc: Zijun Hu, linux-bluetooth, linux-kernel, Luiz Augusto von Dentz,
linux-arm-msm, Zijun Hu
In-Reply-To: <20260622-bt_bugfix-v1-0-11f936d84e72@oss.qualcomm.com>
qca_send_reset() uses __hci_cmd_sync() which returns an skb but never
reads the HCI status byte from skb->data[0], so a non-zero error status
returned by the controller is silently ignored.
Fix by replacing qca_send_reset() with __hci_reset_sync() which
properly extracts and converts the HCI status byte to a negative errno.
Fixes: 83e81961ff7e ("Bluetooth: btqca: Introduce generic QCA ROME support")
Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
---
drivers/bluetooth/btqca.c | 22 ++--------------------
1 file changed, 2 insertions(+), 20 deletions(-)
diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
index 04ebe290bc78..875216e15603 100644
--- a/drivers/bluetooth/btqca.c
+++ b/drivers/bluetooth/btqca.c
@@ -190,25 +190,6 @@ static int qca_send_patch_config_cmd(struct hci_dev *hdev)
return err;
}
-static int qca_send_reset(struct hci_dev *hdev)
-{
- struct sk_buff *skb;
- int err;
-
- bt_dev_dbg(hdev, "QCA HCI_RESET");
-
- skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, HCI_INIT_TIMEOUT);
- if (IS_ERR(skb)) {
- err = PTR_ERR(skb);
- bt_dev_err(hdev, "QCA Reset failed (%d)", err);
- return err;
- }
-
- kfree_skb(skb);
-
- return 0;
-}
-
static int qca_read_fw_board_id(struct hci_dev *hdev, u16 *bid)
{
u8 cmd;
@@ -990,7 +971,8 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
}
/* Perform HCI reset */
- err = qca_send_reset(hdev);
+ bt_dev_dbg(hdev, "QCA HCI_RESET");
+ err = __hci_reset_sync(hdev);
if (err < 0) {
bt_dev_err(hdev, "QCA Failed to run HCI_RESET (%d)", err);
return err;
--
2.34.1
^ permalink raw reply related
* [PATCH 11/13] Bluetooth: btusb: Move struct btusb_data and macros into btusb.h
From: Zijun Hu @ 2026-06-22 14:52 UTC (permalink / raw)
To: Marcel Holtmann, Luiz Augusto von Dentz, Rocky Liao,
Bartosz Golaszewski, Ben Young Tae Kim, Balakrishna Godavarthi,
Matthias Kaehlcke
Cc: Zijun Hu, linux-bluetooth, linux-kernel, Luiz Augusto von Dentz,
linux-arm-msm, Zijun Hu
In-Reply-To: <20260622-bt_bugfix-v1-0-11f936d84e72@oss.qualcomm.com>
btusb.c is growing large as vendor-specific code accumulates. Ideally,
btusb.c contains only the default implementation while vendor-specific
code lives in separate files for easier maintenance.
The newly added btusb.h also reduces unnecessary data copies in hooks
like btusb_mtk_setup().
Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
---
drivers/bluetooth/btusb.c | 119 +--------------------------------------
drivers/bluetooth/btusb.h | 139 ++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 140 insertions(+), 118 deletions(-)
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 21e125db1b1f..0d0bd7b559c6 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -23,6 +23,7 @@
#include <net/bluetooth/hci_core.h>
#include <net/bluetooth/hci_drv.h>
+#include "btusb.h"
#include "btintel.h"
#include "btbcm.h"
#include "btrtl.h"
@@ -38,36 +39,6 @@ static bool reset = true;
static struct usb_driver btusb_driver;
-#define BTUSB_IGNORE BIT(0)
-#define BTUSB_DIGIANSWER BIT(1)
-#define BTUSB_CSR BIT(2)
-#define BTUSB_SNIFFER BIT(3)
-#define BTUSB_BCM92035 BIT(4)
-#define BTUSB_BROKEN_ISOC BIT(5)
-#define BTUSB_WRONG_SCO_MTU BIT(6)
-#define BTUSB_ATH3012 BIT(7)
-#define BTUSB_INTEL_COMBINED BIT(8)
-#define BTUSB_INTEL_BOOT BIT(9)
-#define BTUSB_BCM_PATCHRAM BIT(10)
-#define BTUSB_MARVELL BIT(11)
-#define BTUSB_SWAVE BIT(12)
-#define BTUSB_AMP BIT(13)
-#define BTUSB_QCA_ROME BIT(14)
-#define BTUSB_BCM_APPLE BIT(15)
-#define BTUSB_REALTEK BIT(16)
-#define BTUSB_BCM2045 BIT(17)
-#define BTUSB_IFNUM_2 BIT(18)
-#define BTUSB_CW6622 BIT(19)
-#define BTUSB_MEDIATEK BIT(20)
-#define BTUSB_WIDEBAND_SPEECH BIT(21)
-#define BTUSB_INVALID_LE_STATES BIT(22)
-#define BTUSB_QCA_WCN6855 BIT(23)
-#define BTUSB_INTEL_BROKEN_SHUTDOWN_LED BIT(24)
-#define BTUSB_INTEL_BROKEN_INITIAL_NCMD BIT(25)
-#define BTUSB_INTEL_NO_WBS_SUPPORT BIT(26)
-#define BTUSB_ACTIONS_SEMI BIT(27)
-#define BTUSB_BARROT BIT(28)
-
static const struct usb_device_id btusb_table[] = {
/* Generic Bluetooth USB device */
{ USB_DEVICE_INFO(0xe0, 0x01, 0x01) },
@@ -943,94 +914,6 @@ struct btqca_data {
struct qca_dump_info qca_dump;
};
-#define BTUSB_MAX_ISOC_FRAMES 10
-
-#define BTUSB_INTR_RUNNING 0
-#define BTUSB_BULK_RUNNING 1
-#define BTUSB_ISOC_RUNNING 2
-#define BTUSB_SUSPENDING 3
-#define BTUSB_DID_ISO_RESUME 4
-#define BTUSB_BOOTLOADER 5
-#define BTUSB_DOWNLOADING 6
-#define BTUSB_FIRMWARE_LOADED 7
-#define BTUSB_FIRMWARE_FAILED 8
-#define BTUSB_BOOTING 9
-#define BTUSB_DIAG_RUNNING 10
-#define BTUSB_OOB_WAKE_ENABLED 11
-#define BTUSB_HW_RESET_ACTIVE 12
-#define BTUSB_TX_WAIT_VND_EVT 13
-#define BTUSB_WAKEUP_AUTOSUSPEND 14
-#define BTUSB_USE_ALT3_FOR_WBS 15
-#define BTUSB_ALT6_CONTINUOUS_TX 16
-#define BTUSB_HW_SSR_ACTIVE 17
-
-struct btusb_data {
- struct hci_dev *hdev;
- struct usb_device *udev;
- struct usb_interface *intf;
- struct usb_interface *isoc;
- struct usb_interface *diag;
- unsigned isoc_ifnum;
-
- unsigned long flags;
-
- bool poll_sync;
- int intr_interval;
- struct work_struct work;
- struct work_struct waker;
- struct delayed_work rx_work;
-
- struct sk_buff_head acl_q;
-
- struct usb_anchor deferred;
- struct usb_anchor tx_anchor;
- int tx_in_flight;
- spinlock_t txlock;
-
- struct usb_anchor intr_anchor;
- struct usb_anchor bulk_anchor;
- struct usb_anchor isoc_anchor;
- struct usb_anchor diag_anchor;
- struct usb_anchor ctrl_anchor;
- spinlock_t rxlock;
-
- struct sk_buff *evt_skb;
- struct sk_buff *acl_skb;
- struct sk_buff *sco_skb;
-
- struct usb_endpoint_descriptor *intr_ep;
- struct usb_endpoint_descriptor *bulk_tx_ep;
- struct usb_endpoint_descriptor *bulk_rx_ep;
- struct usb_endpoint_descriptor *isoc_tx_ep;
- struct usb_endpoint_descriptor *isoc_rx_ep;
- struct usb_endpoint_descriptor *diag_tx_ep;
- struct usb_endpoint_descriptor *diag_rx_ep;
-
- struct gpio_desc *reset_gpio;
-
- __u8 cmdreq_type;
- __u8 cmdreq;
-
- unsigned int sco_num;
- unsigned int air_mode;
- bool usb_alt6_packet_flow;
- int isoc_altsetting;
- int suspend_count;
- const struct usb_device_id *match_id;
-
- int (*recv_event)(struct hci_dev *hdev, struct sk_buff *skb);
- int (*recv_acl)(struct hci_dev *hdev, struct sk_buff *skb);
- int (*recv_bulk)(struct btusb_data *data, void *buffer, int count);
-
- int (*setup_on_usb)(struct hci_dev *hdev);
-
- int (*suspend)(struct hci_dev *hdev);
- int (*resume)(struct hci_dev *hdev);
- int (*disconnect)(struct hci_dev *hdev);
-
- int oob_wake_irq; /* irq for out-of-band wake-on-bt */
-};
-
static void btusb_reset(struct hci_dev *hdev)
{
struct btusb_data *data;
diff --git a/drivers/bluetooth/btusb.h b/drivers/bluetooth/btusb.h
new file mode 100644
index 000000000000..ad13c7d44836
--- /dev/null
+++ b/drivers/bluetooth/btusb.h
@@ -0,0 +1,139 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ *
+ * Generic Bluetooth USB driver
+ *
+ * Copyright (C) 2005-2008 Marcel Holtmann <marcel@holtmann.org>
+ */
+
+#ifndef __BTUSB_H
+#define __BTUSB_H
+
+#include <linux/usb.h>
+#include <linux/skbuff.h>
+#include <linux/workqueue.h>
+#include <linux/spinlock.h>
+#include <linux/gpio/consumer.h>
+#include <net/bluetooth/hci_core.h>
+
+/* driver_info flags */
+#define BTUSB_IGNORE BIT(0)
+#define BTUSB_DIGIANSWER BIT(1)
+#define BTUSB_CSR BIT(2)
+#define BTUSB_SNIFFER BIT(3)
+#define BTUSB_BCM92035 BIT(4)
+#define BTUSB_BROKEN_ISOC BIT(5)
+#define BTUSB_WRONG_SCO_MTU BIT(6)
+#define BTUSB_ATH3012 BIT(7)
+#define BTUSB_INTEL_COMBINED BIT(8)
+#define BTUSB_INTEL_BOOT BIT(9)
+#define BTUSB_BCM_PATCHRAM BIT(10)
+#define BTUSB_MARVELL BIT(11)
+#define BTUSB_SWAVE BIT(12)
+#define BTUSB_AMP BIT(13)
+#define BTUSB_QCA_ROME BIT(14)
+#define BTUSB_BCM_APPLE BIT(15)
+#define BTUSB_REALTEK BIT(16)
+#define BTUSB_BCM2045 BIT(17)
+#define BTUSB_IFNUM_2 BIT(18)
+#define BTUSB_CW6622 BIT(19)
+#define BTUSB_MEDIATEK BIT(20)
+#define BTUSB_WIDEBAND_SPEECH BIT(21)
+#define BTUSB_INVALID_LE_STATES BIT(22)
+#define BTUSB_QCA_WCN6855 BIT(23)
+#define BTUSB_INTEL_BROKEN_SHUTDOWN_LED BIT(24)
+#define BTUSB_INTEL_BROKEN_INITIAL_NCMD BIT(25)
+#define BTUSB_INTEL_NO_WBS_SUPPORT BIT(26)
+#define BTUSB_ACTIONS_SEMI BIT(27)
+#define BTUSB_BARROT BIT(28)
+
+#define BTUSB_MAX_ISOC_FRAMES 10
+
+/* btusb_data flags */
+#define BTUSB_INTR_RUNNING 0
+#define BTUSB_BULK_RUNNING 1
+#define BTUSB_ISOC_RUNNING 2
+#define BTUSB_SUSPENDING 3
+#define BTUSB_DID_ISO_RESUME 4
+#define BTUSB_BOOTLOADER 5
+#define BTUSB_DOWNLOADING 6
+#define BTUSB_FIRMWARE_LOADED 7
+#define BTUSB_FIRMWARE_FAILED 8
+#define BTUSB_BOOTING 9
+#define BTUSB_DIAG_RUNNING 10
+#define BTUSB_OOB_WAKE_ENABLED 11
+#define BTUSB_HW_RESET_ACTIVE 12
+#define BTUSB_TX_WAIT_VND_EVT 13
+#define BTUSB_WAKEUP_AUTOSUSPEND 14
+#define BTUSB_USE_ALT3_FOR_WBS 15
+#define BTUSB_ALT6_CONTINUOUS_TX 16
+#define BTUSB_HW_SSR_ACTIVE 17
+
+struct btusb_data {
+ struct hci_dev *hdev;
+ struct usb_device *udev;
+ struct usb_interface *intf;
+ struct usb_interface *isoc;
+ struct usb_interface *diag;
+ unsigned int isoc_ifnum;
+
+ unsigned long flags;
+
+ bool poll_sync;
+ int intr_interval;
+ struct work_struct work;
+ struct work_struct waker;
+ struct delayed_work rx_work;
+
+ struct sk_buff_head acl_q;
+
+ struct usb_anchor deferred;
+ struct usb_anchor tx_anchor;
+ int tx_in_flight;
+ spinlock_t txlock;
+
+ struct usb_anchor intr_anchor;
+ struct usb_anchor bulk_anchor;
+ struct usb_anchor isoc_anchor;
+ struct usb_anchor diag_anchor;
+ struct usb_anchor ctrl_anchor;
+ spinlock_t rxlock;
+
+ struct sk_buff *evt_skb;
+ struct sk_buff *acl_skb;
+ struct sk_buff *sco_skb;
+
+ struct usb_endpoint_descriptor *intr_ep;
+ struct usb_endpoint_descriptor *bulk_tx_ep;
+ struct usb_endpoint_descriptor *bulk_rx_ep;
+ struct usb_endpoint_descriptor *isoc_tx_ep;
+ struct usb_endpoint_descriptor *isoc_rx_ep;
+ struct usb_endpoint_descriptor *diag_tx_ep;
+ struct usb_endpoint_descriptor *diag_rx_ep;
+
+ struct gpio_desc *reset_gpio;
+
+ __u8 cmdreq_type;
+ __u8 cmdreq;
+
+ unsigned int sco_num;
+ unsigned int air_mode;
+ bool usb_alt6_packet_flow;
+ int isoc_altsetting;
+ int suspend_count;
+ const struct usb_device_id *match_id;
+
+ int (*recv_event)(struct hci_dev *hdev, struct sk_buff *skb);
+ int (*recv_acl)(struct hci_dev *hdev, struct sk_buff *skb);
+ int (*recv_bulk)(struct btusb_data *data, void *buffer, int count);
+
+ int (*setup_on_usb)(struct hci_dev *hdev);
+
+ int (*suspend)(struct hci_dev *hdev);
+ int (*resume)(struct hci_dev *hdev);
+ int (*disconnect)(struct hci_dev *hdev);
+
+ int oob_wake_irq; /* irq for out-of-band wake-on-bt */
+};
+
+#endif /* __BTUSB_H */
--
2.34.1
^ permalink raw reply related
* [PATCH 10/13] Bluetooth: btusb: Simplify btusb_shutdown_qca() by using __hci_reset_sync()
From: Zijun Hu @ 2026-06-22 14:52 UTC (permalink / raw)
To: Marcel Holtmann, Luiz Augusto von Dentz, Rocky Liao,
Bartosz Golaszewski, Ben Young Tae Kim, Balakrishna Godavarthi,
Matthias Kaehlcke
Cc: Zijun Hu, linux-bluetooth, linux-kernel, Luiz Augusto von Dentz,
linux-arm-msm, Zijun Hu
In-Reply-To: <20260622-bt_bugfix-v1-0-11f936d84e72@oss.qualcomm.com>
Use the new __hci_reset_sync() API instead of open-coding the HCI reset.
Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
---
drivers/bluetooth/btusb.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index e78277e24cd8..21e125db1b1f 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -3900,16 +3900,13 @@ static bool btusb_wakeup(struct hci_dev *hdev)
static int btusb_shutdown_qca(struct hci_dev *hdev)
{
- struct sk_buff *skb;
+ int err;
- skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, HCI_INIT_TIMEOUT);
- if (IS_ERR(skb)) {
+ err = __hci_reset_sync(hdev);
+ if (err)
bt_dev_err(hdev, "HCI reset during shutdown failed");
- return PTR_ERR(skb);
- }
- kfree_skb(skb);
- return 0;
+ return err;
}
static ssize_t force_poll_sync_read(struct file *file, char __user *user_buf,
--
2.34.1
^ permalink raw reply related
* [PATCH 09/13] Bluetooth: hci_sync: Add __hci_reset_sync() for device driver
From: Zijun Hu @ 2026-06-22 14:52 UTC (permalink / raw)
To: Marcel Holtmann, Luiz Augusto von Dentz, Rocky Liao,
Bartosz Golaszewski, Ben Young Tae Kim, Balakrishna Godavarthi,
Matthias Kaehlcke
Cc: Zijun Hu, linux-bluetooth, linux-kernel, Luiz Augusto von Dentz,
linux-arm-msm, Zijun Hu
In-Reply-To: <20260622-bt_bugfix-v1-0-11f936d84e72@oss.qualcomm.com>
Many vendor drivers have requirements to send a raw HCI reset
synchronously with HCI_INIT_TIMEOUT.
Add a dedicated API for them to use.
Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
---
include/net/bluetooth/hci_sync.h | 1 +
net/bluetooth/hci_sync.c | 14 ++++++++++++++
2 files changed, 15 insertions(+)
diff --git a/include/net/bluetooth/hci_sync.h b/include/net/bluetooth/hci_sync.h
index 73e494b2591d..7005fc9f257a 100644
--- a/include/net/bluetooth/hci_sync.h
+++ b/include/net/bluetooth/hci_sync.h
@@ -59,6 +59,7 @@ int __hci_cmd_sync_status(struct hci_dev *hdev, u16 opcode, u32 plen,
int __hci_cmd_sync_status_sk(struct hci_dev *hdev, u16 opcode, u32 plen,
const void *param, u8 event, u32 timeout,
struct sock *sk);
+int __hci_reset_sync(struct hci_dev *hdev);
int hci_cmd_sync_status(struct hci_dev *hdev, u16 opcode, u32 plen,
const void *param, u32 timeout);
diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index 601d44ef975f..40c9725585cb 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -3684,6 +3684,20 @@ int hci_reset_sync(struct hci_dev *hdev)
return -bt_to_errno(err);
}
+/* Send a raw HCI reset for use by vendor drivers */
+int __hci_reset_sync(struct hci_dev *hdev)
+{
+ int err;
+
+ err = __hci_cmd_sync_status(hdev, HCI_OP_RESET, 0, NULL,
+ HCI_INIT_TIMEOUT);
+ if (err < 0)
+ return err;
+
+ return -bt_to_errno(err);
+}
+EXPORT_SYMBOL(__hci_reset_sync);
+
static int hci_init0_sync(struct hci_dev *hdev)
{
int err;
--
2.34.1
^ permalink raw reply related
* [PATCH 07/13] Bluetooth: hci_sync: Simplify hci_reset_sync()
From: Zijun Hu @ 2026-06-22 14:52 UTC (permalink / raw)
To: Marcel Holtmann, Luiz Augusto von Dentz, Rocky Liao,
Bartosz Golaszewski, Ben Young Tae Kim, Balakrishna Godavarthi,
Matthias Kaehlcke
Cc: Zijun Hu, linux-bluetooth, linux-kernel, Luiz Augusto von Dentz,
linux-arm-msm, Zijun Hu
In-Reply-To: <20260622-bt_bugfix-v1-0-11f936d84e72@oss.qualcomm.com>
Return err directly instead of using an if/return pattern.
Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
---
net/bluetooth/hci_sync.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index 3be8c3581c6c..fce9f9526cb5 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -3678,10 +3678,8 @@ int hci_reset_sync(struct hci_dev *hdev)
err = __hci_cmd_sync_status(hdev, HCI_OP_RESET, 0, NULL,
HCI_CMD_TIMEOUT);
- if (err)
- return err;
- return 0;
+ return err;
}
static int hci_init0_sync(struct hci_dev *hdev)
--
2.34.1
^ permalink raw reply related
* [PATCH 08/13] Bluetooth: hci_sync: Fix return value of hci_reset_sync()
From: Zijun Hu @ 2026-06-22 14:52 UTC (permalink / raw)
To: Marcel Holtmann, Luiz Augusto von Dentz, Rocky Liao,
Bartosz Golaszewski, Ben Young Tae Kim, Balakrishna Godavarthi,
Matthias Kaehlcke
Cc: Zijun Hu, linux-bluetooth, linux-kernel, Luiz Augusto von Dentz,
linux-arm-msm, Zijun Hu
In-Reply-To: <20260622-bt_bugfix-v1-0-11f936d84e72@oss.qualcomm.com>
hci_reset_sync() may return positive HCI status byte as-is, but callers
in the chain hci_reset_sync() -> hci_init0_sync() -> hci_unconf_init_sync()
check errors with < 0, so a positive status is silently ignored.
Fix by converting positive HCI status to a negative errno using
bt_to_errno().
Fixes: d0b137062b2d ("Bluetooth: hci_sync: Rework init stages")
Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
---
net/bluetooth/hci_sync.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index fce9f9526cb5..601d44ef975f 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -3678,8 +3678,10 @@ int hci_reset_sync(struct hci_dev *hdev)
err = __hci_cmd_sync_status(hdev, HCI_OP_RESET, 0, NULL,
HCI_CMD_TIMEOUT);
+ if (err < 0)
+ return err;
- return err;
+ return -bt_to_errno(err);
}
static int hci_init0_sync(struct hci_dev *hdev)
--
2.34.1
^ permalink raw reply related
* [PATCH 05/13] Bluetooth: btusb: QCA: move qca_dump out of struct btusb_data
From: Zijun Hu @ 2026-06-22 14:52 UTC (permalink / raw)
To: Marcel Holtmann, Luiz Augusto von Dentz, Rocky Liao,
Bartosz Golaszewski, Ben Young Tae Kim, Balakrishna Godavarthi,
Matthias Kaehlcke
Cc: Zijun Hu, linux-bluetooth, linux-kernel, Luiz Augusto von Dentz,
linux-arm-msm, Zijun Hu
In-Reply-To: <20260622-bt_bugfix-v1-0-11f936d84e72@oss.qualcomm.com>
'struct btusb_data' ideally should not include vendor specific
fields, but it currently includes the QCA devcoredump member
'struct qca_dump_info qca_dump'.
Fix by moving it into hci_dev private area accessed by hci_get_priv().
Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
---
drivers/bluetooth/btusb.c | 56 ++++++++++++++++++++++++++++-------------------
1 file changed, 34 insertions(+), 22 deletions(-)
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 184a87d1234c..6f965c313dff 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -939,6 +939,10 @@ struct qca_dump_info {
u16 ram_dump_seqno;
};
+struct btqca_data {
+ struct qca_dump_info qca_dump;
+};
+
#define BTUSB_MAX_ISOC_FRAMES 10
#define BTUSB_INTR_RUNNING 0
@@ -1025,8 +1029,6 @@ struct btusb_data {
int (*disconnect)(struct hci_dev *hdev);
int oob_wake_irq; /* irq for out-of-band wake-on-bt */
-
- struct qca_dump_info qca_dump;
};
static void btusb_reset(struct hci_dev *hdev)
@@ -3116,14 +3118,15 @@ struct qca_dump_hdr {
static void btusb_dump_hdr_qca(struct hci_dev *hdev, struct sk_buff *skb)
{
char buf[128];
- struct btusb_data *btdata = hci_get_drvdata(hdev);
+ struct btqca_data *btqca_data = hci_get_priv(hdev);
+ struct qca_dump_info *qca_dump_ptr = &btqca_data->qca_dump;
snprintf(buf, sizeof(buf), "Controller Name: 0x%x\n",
- btdata->qca_dump.controller_id);
+ qca_dump_ptr->controller_id);
skb_put_data(skb, buf, strlen(buf));
snprintf(buf, sizeof(buf), "Firmware Version: 0x%x\n",
- btdata->qca_dump.fw_version);
+ qca_dump_ptr->fw_version);
skb_put_data(skb, buf, strlen(buf));
snprintf(buf, sizeof(buf), "Driver: %s\nVendor: qca\n",
@@ -3131,7 +3134,7 @@ static void btusb_dump_hdr_qca(struct hci_dev *hdev, struct sk_buff *skb)
skb_put_data(skb, buf, strlen(buf));
snprintf(buf, sizeof(buf), "VID: 0x%x\nPID:0x%x\n",
- btdata->qca_dump.id_vendor, btdata->qca_dump.id_product);
+ qca_dump_ptr->id_vendor, qca_dump_ptr->id_product);
skb_put_data(skb, buf, strlen(buf));
snprintf(buf, sizeof(buf), "Lmp Subversion: 0x%x\n",
@@ -3160,6 +3163,8 @@ static int handle_dump_pkt_qca(struct hci_dev *hdev, struct sk_buff *skb)
struct qca_dump_hdr *dump_hdr;
struct btusb_data *btdata = hci_get_drvdata(hdev);
+ struct btqca_data *btqca_data = hci_get_priv(hdev);
+ struct qca_dump_info *qca_dump_ptr = &btqca_data->qca_dump;
struct usb_device *udev = btdata->udev;
pkt_type = hci_skb_pkt_type(skb);
@@ -3187,8 +3192,8 @@ static int handle_dump_pkt_qca(struct hci_dev *hdev, struct sk_buff *skb)
goto out;
}
- btdata->qca_dump.ram_dump_size = dump_size;
- btdata->qca_dump.ram_dump_seqno = 0;
+ qca_dump_ptr->ram_dump_size = dump_size;
+ qca_dump_ptr->ram_dump_seqno = 0;
skb_pull(skb, offsetof(struct qca_dump_hdr, data0));
@@ -3200,29 +3205,29 @@ static int handle_dump_pkt_qca(struct hci_dev *hdev, struct sk_buff *skb)
skb_pull(skb, offsetof(struct qca_dump_hdr, data));
}
- if (!btdata->qca_dump.ram_dump_size) {
+ if (!qca_dump_ptr->ram_dump_size) {
ret = -EINVAL;
bt_dev_err(hdev, "memdump is not active");
goto out;
}
- if ((seqno > btdata->qca_dump.ram_dump_seqno + 1) && (seqno != QCA_LAST_SEQUENCE_NUM)) {
- dump_size = QCA_MEMDUMP_PKT_SIZE * (seqno - btdata->qca_dump.ram_dump_seqno - 1);
+ if ((seqno > qca_dump_ptr->ram_dump_seqno + 1) && seqno != QCA_LAST_SEQUENCE_NUM) {
+ dump_size = QCA_MEMDUMP_PKT_SIZE * (seqno - qca_dump_ptr->ram_dump_seqno - 1);
hci_devcd_append_pattern(hdev, 0x0, dump_size);
bt_dev_err(hdev,
"expected memdump seqno(%u) is not received(%u)\n",
- btdata->qca_dump.ram_dump_seqno, seqno);
- btdata->qca_dump.ram_dump_seqno = seqno;
+ qca_dump_ptr->ram_dump_seqno, seqno);
+ qca_dump_ptr->ram_dump_seqno = seqno;
kfree_skb(skb);
return ret;
}
hci_devcd_append(hdev, skb);
- btdata->qca_dump.ram_dump_seqno++;
+ qca_dump_ptr->ram_dump_seqno++;
if (seqno == QCA_LAST_SEQUENCE_NUM) {
bt_dev_info(hdev,
"memdump done: pkts(%u), total(%u)\n",
- btdata->qca_dump.ram_dump_seqno, btdata->qca_dump.ram_dump_size);
+ qca_dump_ptr->ram_dump_seqno, qca_dump_ptr->ram_dump_size);
hci_devcd_complete(hdev);
goto out;
@@ -3230,10 +3235,10 @@ static int handle_dump_pkt_qca(struct hci_dev *hdev, struct sk_buff *skb)
return ret;
out:
- if (btdata->qca_dump.ram_dump_size)
+ if (qca_dump_ptr->ram_dump_size)
usb_enable_autosuspend(udev);
- btdata->qca_dump.ram_dump_size = 0;
- btdata->qca_dump.ram_dump_seqno = 0;
+ qca_dump_ptr->ram_dump_size = 0;
+ qca_dump_ptr->ram_dump_seqno = 0;
clear_bit(BTUSB_HW_SSR_ACTIVE, &btdata->flags);
if (ret < 0)
@@ -3709,8 +3714,10 @@ static int btusb_setup_qca(struct hci_dev *hdev)
return err;
if (btdata->match_id->driver_info & BTUSB_QCA_WCN6855) {
- btdata->qca_dump.fw_version = le32_to_cpu(ver.patch_version);
- btdata->qca_dump.controller_id = le32_to_cpu(ver.rom_version);
+ struct btqca_data *btqca_data = hci_get_priv(hdev);
+
+ btqca_data->qca_dump.fw_version = le32_to_cpu(ver.patch_version);
+ btqca_data->qca_dump.controller_id = le32_to_cpu(ver.rom_version);
}
if (!(status & QCA_SYSCFG_UPDATED)) {
@@ -4174,6 +4181,9 @@ static int btusb_probe(struct usb_interface *intf,
} else if (id->driver_info & BTUSB_MEDIATEK) {
/* Allocate extra space for Mediatek device */
priv_size += sizeof(struct btmtk_data);
+ } else if (id->driver_info & BTUSB_QCA_WCN6855) {
+ /* Allocate extra space for QCA WCN6855 device */
+ priv_size += sizeof(struct btqca_data);
}
data->recv_acl = hci_recv_frame;
@@ -4316,8 +4326,10 @@ static int btusb_probe(struct usb_interface *intf,
}
if (id->driver_info & BTUSB_QCA_WCN6855) {
- data->qca_dump.id_vendor = id->idVendor;
- data->qca_dump.id_product = id->idProduct;
+ struct btqca_data *btqca_data = hci_get_priv(hdev);
+
+ btqca_data->qca_dump.id_vendor = id->idVendor;
+ btqca_data->qca_dump.id_product = id->idProduct;
data->recv_event = btusb_recv_evt_qca;
data->recv_acl = btusb_recv_acl_qca;
hci_devcd_register(hdev, btusb_coredump_qca, btusb_dump_hdr_qca, NULL);
--
2.34.1
^ permalink raw reply related
* [PATCH 06/13] Bluetooth: btusb: Fix BD_ADDR byte order in btusb_set_bdaddr_wcn6855()
From: Zijun Hu @ 2026-06-22 14:52 UTC (permalink / raw)
To: Marcel Holtmann, Luiz Augusto von Dentz, Rocky Liao,
Bartosz Golaszewski, Ben Young Tae Kim, Balakrishna Godavarthi,
Matthias Kaehlcke
Cc: Zijun Hu, linux-bluetooth, linux-kernel, Luiz Augusto von Dentz,
linux-arm-msm, Zijun Hu
In-Reply-To: <20260622-bt_bugfix-v1-0-11f936d84e72@oss.qualcomm.com>
For VSC 0xfc14 to set BD_ADDR, the endianness of the BDA parameter is
opposite to other HCI commands like HCI_Create_Connection, but
btusb_set_bdaddr_wcn6855() sends the address without swapping byte
order, resulting in a wrong BD_ADDR being set.
Fix by swapping the input address before issuing the command.
Fixes: b40f58b97386 ("Bluetooth: btusb: Add Qualcomm Bluetooth SoC WCN6855 support")
Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
---
drivers/bluetooth/btusb.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 6f965c313dff..e78277e24cd8 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -3075,14 +3075,15 @@ static int btusb_set_bdaddr_ath3012(struct hci_dev *hdev,
static int btusb_set_bdaddr_wcn6855(struct hci_dev *hdev,
const bdaddr_t *bdaddr)
{
+ bdaddr_t bdaddr_swapped;
struct sk_buff *skb;
- u8 buf[6];
long ret;
- memcpy(buf, bdaddr, sizeof(bdaddr_t));
+ baswap(&bdaddr_swapped, bdaddr);
- skb = __hci_cmd_sync_ev(hdev, 0xfc14, sizeof(buf), buf,
- HCI_EV_CMD_COMPLETE, HCI_INIT_TIMEOUT);
+ skb = __hci_cmd_sync_ev(hdev, 0xfc14, sizeof(bdaddr_swapped),
+ &bdaddr_swapped, HCI_EV_CMD_COMPLETE,
+ HCI_INIT_TIMEOUT);
if (IS_ERR(skb)) {
ret = PTR_ERR(skb);
bt_dev_err(hdev, "Change address command failed (%ld)", ret);
--
2.34.1
^ permalink raw reply related
* [PATCH 04/13] Bluetooth: btusb: QCA: Do not populate devcoredump fields on ATH3012 or QCA_ROME
From: Zijun Hu @ 2026-06-22 14:52 UTC (permalink / raw)
To: Marcel Holtmann, Luiz Augusto von Dentz, Rocky Liao,
Bartosz Golaszewski, Ben Young Tae Kim, Balakrishna Godavarthi,
Matthias Kaehlcke
Cc: Zijun Hu, linux-bluetooth, linux-kernel, Luiz Augusto von Dentz,
linux-arm-msm, Zijun Hu
In-Reply-To: <20260622-bt_bugfix-v1-0-11f936d84e72@oss.qualcomm.com>
Devcoredump is disabled on ATH3012 or QCA_ROME, but btusb_setup_qca()
used by both unconditionally populates those two devcoredump fields.
Fix by populating devcoredump fields only for BTUSB_QCA_WCN6855 devices,
which are the only ones that enable devcoredump.
Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
---
drivers/bluetooth/btusb.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 6a90f012b226..184a87d1234c 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -3708,8 +3708,10 @@ static int btusb_setup_qca(struct hci_dev *hdev)
if (err < 0)
return err;
- btdata->qca_dump.fw_version = le32_to_cpu(ver.patch_version);
- btdata->qca_dump.controller_id = le32_to_cpu(ver.rom_version);
+ if (btdata->match_id->driver_info & BTUSB_QCA_WCN6855) {
+ btdata->qca_dump.fw_version = le32_to_cpu(ver.patch_version);
+ btdata->qca_dump.controller_id = le32_to_cpu(ver.rom_version);
+ }
if (!(status & QCA_SYSCFG_UPDATED)) {
err = btusb_setup_qca_load_nvm(hdev, &ver, info);
--
2.34.1
^ permalink raw reply related
* [PATCH 03/13] Bluetooth: btusb: Record matched usb_device_id into btusb_data
From: Zijun Hu @ 2026-06-22 14:52 UTC (permalink / raw)
To: Marcel Holtmann, Luiz Augusto von Dentz, Rocky Liao,
Bartosz Golaszewski, Ben Young Tae Kim, Balakrishna Godavarthi,
Matthias Kaehlcke
Cc: Zijun Hu, linux-bluetooth, linux-kernel, Luiz Augusto von Dentz,
linux-arm-msm, Zijun Hu
In-Reply-To: <20260622-bt_bugfix-v1-0-11f936d84e72@oss.qualcomm.com>
Add @match_id to btusb_data to record the matched usb_device_id
which will be used later.
Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
---
drivers/bluetooth/btusb.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index fa6a223d472d..6a90f012b226 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -1012,6 +1012,7 @@ struct btusb_data {
bool usb_alt6_packet_flow;
int isoc_altsetting;
int suspend_count;
+ const struct usb_device_id *match_id;
int (*recv_event)(struct hci_dev *hdev, struct sk_buff *skb);
int (*recv_acl)(struct hci_dev *hdev, struct sk_buff *skb);
@@ -4119,6 +4120,7 @@ static int btusb_probe(struct usb_interface *intf,
if (!data)
return -ENOMEM;
+ data->match_id = id;
err = usb_find_common_endpoints(intf->cur_altsetting, &data->bulk_rx_ep,
&data->bulk_tx_ep, &data->intr_ep, NULL);
if (err)
--
2.34.1
^ permalink raw reply related
* [PATCH 02/13] Bluetooth: btusb: Use & instead of == to test bitflag BTUSB_IGNORE
From: Zijun Hu @ 2026-06-22 14:52 UTC (permalink / raw)
To: Marcel Holtmann, Luiz Augusto von Dentz, Rocky Liao,
Bartosz Golaszewski, Ben Young Tae Kim, Balakrishna Godavarthi,
Matthias Kaehlcke
Cc: Zijun Hu, linux-bluetooth, linux-kernel, Luiz Augusto von Dentz,
linux-arm-msm, Zijun Hu
In-Reply-To: <20260622-bt_bugfix-v1-0-11f936d84e72@oss.qualcomm.com>
The driver_info field is a bitmask, so use & instead of == to test the
BTUSB_IGNORE bitflag against it, which is consistent with how the other
flags are tested.
Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
---
drivers/bluetooth/btusb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 5209e2418493..fa6a223d472d 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -4101,7 +4101,7 @@ static int btusb_probe(struct usb_interface *intf,
id = match;
}
- if (id->driver_info == BTUSB_IGNORE)
+ if (id->driver_info & BTUSB_IGNORE)
return -ENODEV;
if (id->driver_info & BTUSB_ATH3012) {
--
2.34.1
^ permalink raw reply related
* [PATCH 00/13] Bluetooth: btusb/btqca/hci_sync: Clean up btusb and fix several bugs
From: Zijun Hu @ 2026-06-22 14:52 UTC (permalink / raw)
To: Marcel Holtmann, Luiz Augusto von Dentz, Rocky Liao,
Bartosz Golaszewski, Ben Young Tae Kim, Balakrishna Godavarthi,
Matthias Kaehlcke
Cc: Zijun Hu, linux-bluetooth, linux-kernel, Luiz Augusto von Dentz,
linux-arm-msm, Zijun Hu
This series originated as a cleanup of btusb in preparation for adding
support for a new chip, QCC2072: specifically, moving struct btusb_data
and the BTUSB_* macros into a new btusb.h header so that the forthcoming
QCC2072 driver can share them without duplicating definitions.
While doing that cleanup, several pre-existing bugs were found and fixed.
Preparatory cleanups:
- Initialize priv_size to 0 at declaration in btusb_probe() to remove a
redundant assignment a few lines below.
- Move struct btusb_data and the BTUSB_* macros into a new btusb.h header
so that btqca.c (and future drivers) can access them without duplicating
definitions.
- Record the matched usb_device_id pointer in struct btusb_data so that
downstream code (e.g. qca_setup) can inspect driver_info without
needing to re-scan the id table.
- Move qca_dump out of struct btusb_data into a dedicated struct
btqca_data stored in hci priv, decoupling the QCA coredump state from
the generic btusb driver data.
- Simplify the hci_reset_sync() return path (trivial two-liner to one).
- Add __hci_reset_sync() as an exported helper for vendor drivers; it
sends HCI_OP_RESET and returns a proper errno.
- Use __hci_reset_sync() in btusb_shutdown_qca() to remove open-coded
__hci_cmd_sync() boilerplate.
Bug fixes found during the cleanup:
- BTUSB_IGNORE is a bitmask flag and must be tested with '&', not '=='.
The '==' test would miss any device whose driver_info word carries
additional flags alongside BTUSB_IGNORE.
- qca_setup() unconditionally populated devcoredump fw_version and
controller_id fields even on ATH3012 and QCA_ROME devices where those
fields are meaningless. Guard the assignment behind a WCN6855 check.
- btusb_set_bdaddr_wcn6855() passed the raw bdaddr pointer to the HCI
command without byte-swapping it first; use baswap() as other set_bdaddr
implementations do.
- hci_reset_sync() returned the raw HCI status byte instead of a negative
errno on error. Add the bt_to_errno() conversion.
- qca_send_reset() used __hci_cmd_sync() which only checks transport-level
errors and silently ignores a non-zero HCI status in the response. The
function is replaced by a call to the new __hci_reset_sync() which does
the right conversion.
- qca_set_bdaddr() passed HCI_EV_VENDOR as the expected event type; the
correct value is 0 (any), matching how other set_bdaddr callers behave.
Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
---
Zijun Hu (13):
Bluetooth: btusb: Initialize @priv_size at declaration in btusb_probe()
Bluetooth: btusb: Use & instead of == to test bitflag BTUSB_IGNORE
Bluetooth: btusb: Record matched usb_device_id into btusb_data
Bluetooth: btusb: QCA: Do not populate devcoredump fields on ATH3012 or QCA_ROME
Bluetooth: btusb: QCA: move qca_dump out of struct btusb_data
Bluetooth: btusb: Fix BD_ADDR byte order in btusb_set_bdaddr_wcn6855()
Bluetooth: hci_sync: Simplify hci_reset_sync()
Bluetooth: hci_sync: Fix return value of hci_reset_sync()
Bluetooth: hci_sync: Add __hci_reset_sync() for device driver
Bluetooth: btusb: Simplify btusb_shutdown_qca() by using __hci_reset_sync()
Bluetooth: btusb: Move struct btusb_data and macros into btusb.h
Bluetooth: btqca: Fix undetected error HCI status in qca_send_reset()
Bluetooth: btqca: Fix qca_set_bdaddr() using wrong HCI event type
drivers/bluetooth/btqca.c | 25 +----
drivers/bluetooth/btusb.c | 197 +++++++++------------------------------
drivers/bluetooth/btusb.h | 139 +++++++++++++++++++++++++++
include/net/bluetooth/hci_sync.h | 1 +
net/bluetooth/hci_sync.c | 18 +++-
5 files changed, 205 insertions(+), 175 deletions(-)
---
base-commit: cb20f6afc25b2b54c0fec61b45ac0ec9eb875d59
change-id: 20260622-bt_bugfix-fbb216ca1ff9
Best regards,
--
Zijun Hu <zijun.hu@oss.qualcomm.com>
^ permalink raw reply
* [PATCH 01/13] Bluetooth: btusb: Initialize @priv_size at declaration in btusb_probe()
From: Zijun Hu @ 2026-06-22 14:52 UTC (permalink / raw)
To: Marcel Holtmann, Luiz Augusto von Dentz, Rocky Liao,
Bartosz Golaszewski, Ben Young Tae Kim, Balakrishna Godavarthi,
Matthias Kaehlcke
Cc: Zijun Hu, linux-bluetooth, linux-kernel, Luiz Augusto von Dentz,
linux-arm-msm, Zijun Hu
In-Reply-To: <20260622-bt_bugfix-v1-0-11f936d84e72@oss.qualcomm.com>
Initialize @priv_size at declaration to reduce a redundant assignment.
Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
---
drivers/bluetooth/btusb.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 7f14ce96319b..5209e2418493 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -4082,7 +4082,7 @@ static int btusb_probe(struct usb_interface *intf,
struct btusb_data *data;
struct hci_dev *hdev;
unsigned ifnum_base;
- int err, priv_size;
+ int err, priv_size = 0;
BT_DBG("intf %p id %p", intf, id);
@@ -4152,8 +4152,6 @@ static int btusb_probe(struct usb_interface *intf,
init_usb_anchor(&data->ctrl_anchor);
spin_lock_init(&data->rxlock);
- priv_size = 0;
-
data->recv_event = hci_recv_frame;
data->recv_bulk = btusb_recv_bulk;
--
2.34.1
^ permalink raw reply related
* Re: [PATCH BlueZ 2/2] audio: reduce a2dp parser complexity
From: Luiz Augusto von Dentz @ 2026-06-22 13:55 UTC (permalink / raw)
To: Geraldo Netto; +Cc: linux-bluetooth
In-Reply-To: <20260620191735.2675946-3-geraldonetto@gmail.com>
Hi Geraldo,
On Sat, Jun 20, 2026 at 3:18 PM Geraldo Netto <geraldonetto@gmail.com> wrote:
>
> ---
> profiles/audio/a2dp-helpers.c | 55 +++++++++++++++++++++++++++--------
> 1 file changed, 43 insertions(+), 12 deletions(-)
>
> diff --git a/profiles/audio/a2dp-helpers.c b/profiles/audio/a2dp-helpers.c
> index 035236df6..09dc6db91 100644
> --- a/profiles/audio/a2dp-helpers.c
> +++ b/profiles/audio/a2dp-helpers.c
> @@ -74,6 +74,46 @@ static bool parse_caps(const char *value, uint8_t *caps, size_t caps_len,
> return true;
> }
>
> +static bool has_delay_field(const char *value)
> +{
> + return value[0] != '\0' && value[1] != '\0' &&
> + g_ascii_isxdigit(value[0]) &&
> + g_ascii_isxdigit(value[1]) &&
> + value[2] == ':';
> +}
> +
> +static bool parse_endpoint_header(const char **value, uint8_t *type,
> + uint8_t *codec)
> +{
> + if (!parse_hex_byte(value, type) || !parse_colon(value))
> + return false;
> +
> + if (!parse_hex_byte(value, codec) || !parse_colon(value))
> + return false;
> +
> + return true;
> +}
> +
> +static bool parse_delay_field(const char **value, uint8_t *delay)
> +{
> + *delay = 0;
> +
> + if (!has_delay_field(*value))
> + return true;
> +
> + parse_hex_byte(value, delay);
> + parse_colon(value);
> +
> + return *delay <= 1;
> +}
> +
> +static bool valid_endpoint_args(const char *value,
> + const uint8_t *type, const uint8_t *codec,
> + const bool *delay_reporting, const size_t *size)
> +{
> + return value && type && codec && delay_reporting && size;
> +}
> +
> bool a2dp_parse_capabilities_array(DBusMessageIter *value,
> uint8_t **caps, int *size)
> {
> @@ -106,27 +146,18 @@ bool a2dp_parse_persisted_endpoint(const char *value, uint8_t *type,
> const char *pos;
> uint8_t delay = 0;
>
> - if (!value || !type || !codec || !delay_reporting || !size)
> + if (!valid_endpoint_args(value, type, codec, delay_reporting, size))
> return false;
>
> *size = 0;
>
> pos = value;
> - if (!parse_hex_byte(&pos, type) || !parse_colon(&pos))
> + if (!parse_endpoint_header(&pos, type, codec))
> return false;
>
> - if (!parse_hex_byte(&pos, codec) || !parse_colon(&pos))
> + if (!parse_delay_field(&pos, &delay))
> return false;
>
> - if (pos[0] != '\0' && pos[1] != '\0' &&
> - g_ascii_isxdigit(pos[0]) && g_ascii_isxdigit(pos[1]) &&
> - pos[2] == ':') {
> - parse_hex_byte(&pos, &delay);
> - parse_colon(&pos);
> - if (delay > 1)
> - return false;
> - }
I'm not really sure this is any better to be honest. It sounds like
comments explaining the parsing logic are needed, but otherwise, I
can't quite follow the changes you are proposing with this.
> if (!parse_caps(pos, caps, caps_len, size))
> return false;
>
> --
> 2.43.0
>
>
--
Luiz Augusto von Dentz
^ permalink raw reply
* [PATCH BlueZ v1] a2dp: Fix handling of codec capability storage
From: Luiz Augusto von Dentz @ 2026-06-22 13:50 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Codec capability is one byte long (max 255) the storage format is
02hhx which means each byte ends up as 2 characters so the buffer
needs to be doubled in order to handle capabilities of that size.
Reported-by: p0her (_@p0her_) in TeamH4C working with TrendAI Zero Day Initiative
Reported-by: Michael Bommarito <michael.bommarito@gmail.com>
---
profiles/audio/a2dp.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
index a5e002784c02..bf163de0ff03 100644
--- a/profiles/audio/a2dp.c
+++ b/profiles/audio/a2dp.c
@@ -971,7 +971,7 @@ static void store_remote_sep(void *data, void *user_data)
{
struct a2dp_remote_sep *sep = data;
GKeyFile *key_file = user_data;
- char seid[4], value[256];
+ char seid[4], value[9 + 512];
struct avdtp_service_capability *service = avdtp_get_codec(sep->sep);
struct avdtp_media_codec_capability *codec;
unsigned int i;
@@ -2373,7 +2373,7 @@ static void load_remote_sep(struct a2dp_channel *chan, GKeyFile *key_file,
uint8_t codec;
uint8_t delay_reporting;
GSList *l = NULL;
- char caps[256];
+ char caps[513];
uint8_t data[128];
int i, size;
@@ -2386,10 +2386,10 @@ static void load_remote_sep(struct a2dp_channel *chan, GKeyFile *key_file,
continue;
/* Try loading with delay_reporting first */
- if (sscanf(value, "%02hhx:%02hhx:%02hhx:%s", &type, &codec,
+ if (sscanf(value, "%02hhx:%02hhx:%02hhx:%512s", &type, &codec,
&delay_reporting, caps) != 4) {
/* Try old format */
- if (sscanf(value, "%02hhx:%02hhx:%s", &type, &codec,
+ if (sscanf(value, "%02hhx:%02hhx:%512s", &type, &codec,
caps) != 3) {
warn("Unable to load Endpoint: seid %u", rseid);
g_free(value);
@@ -2398,7 +2398,7 @@ static void load_remote_sep(struct a2dp_channel *chan, GKeyFile *key_file,
delay_reporting = false;
}
- for (i = 0, size = strlen(caps); i < size; i += 2) {
+ for (i = 0, size = strlen(caps); i < size || i >= 2; i += 2) {
uint8_t *tmp = data + i / 2;
if (sscanf(caps + i, "%02hhx", tmp) != 1) {
--
2.54.0
^ permalink raw reply related
* Re: [PATCH BlueZ 1/2] shared: harden btsnoop trace parsing
From: Luiz Augusto von Dentz @ 2026-06-22 13:42 UTC (permalink / raw)
To: Geraldo Netto; +Cc: linux-bluetooth
In-Reply-To: <20260620192228.2692610-1-geraldonetto@gmail.com>
Hi Geraldo,
On Sat, Jun 20, 2026 at 3:22 PM Geraldo Netto <geraldonetto@gmail.com> wrote:
>
> ---
> Makefile.am | 6 +
> monitor/analyze.c | 4 +-
> monitor/control.c | 2 +-
> src/shared/btsnoop.c | 303 +++++++++++------
> src/shared/btsnoop.h | 6 +-
> unit/test-btsnoop.c | 794 +++++++++++++++++++++++++++++++++++++++++++
> unit/test-btsnoop.h | 3 +
Looks like this is actually trying to introduce a new test suite for
btsnoop, which is probably fine, but the changes to shared/btsnoop.The
changes to shared/btsnoop.{c,h} should be split into their own commit
with a description of why the changes are necessary. Similarly, the
tests themselves need descriptions explaining what each test does.
> 7 files changed, 1009 insertions(+), 109 deletions(-)
> create mode 100644 unit/test-btsnoop.c
> create mode 100644 unit/test-btsnoop.h
>
> diff --git a/Makefile.am b/Makefile.am
> index 76c4ab5d4..4887934a9 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -587,6 +587,12 @@ unit_tests += unit/test-textfile
> unit_test_textfile_SOURCES = unit/test-textfile.c src/textfile.h src/textfile.c
> unit_test_textfile_LDADD = src/libshared-glib.la $(GLIB_LIBS)
>
> +unit_tests += unit/test-btsnoop
> +
> +unit_test_btsnoop_SOURCES = unit/test-btsnoop.c \
> + src/shared/btsnoop.h src/shared/btsnoop.c
> +unit_test_btsnoop_LDADD = src/libshared-glib.la $(GLIB_LIBS)
> +
> unit_tests += unit/test-crc
>
> unit_test_crc_SOURCES = unit/test-crc.c monitor/crc.h monitor/crc.c
> diff --git a/monitor/analyze.c b/monitor/analyze.c
> index de9c23603..c9400ceb3 100644
> --- a/monitor/analyze.c
> +++ b/monitor/analyze.c
> @@ -1404,8 +1404,8 @@ void analyze_trace(const char *path)
> struct timeval tv;
> uint16_t index, opcode, pktlen;
>
> - if (!btsnoop_read_hci(btsnoop_file, &tv, &index, &opcode,
> - buf, &pktlen))
> + if (!btsnoop_read_hci(btsnoop_file, &tv, &index,
> + &opcode, buf, sizeof(buf), &pktlen))
> break;
>
> switch (opcode) {
> diff --git a/monitor/control.c b/monitor/control.c
> index 83347d5db..975e1d117 100644
> --- a/monitor/control.c
> +++ b/monitor/control.c
> @@ -1564,7 +1564,7 @@ void control_reader(const char *path, bool pager)
> uint16_t index, opcode;
>
> if (!btsnoop_read_hci(btsnoop_file, &tv, &index,
> - &opcode, buf, &pktlen))
> + &opcode, buf, sizeof(buf), &pktlen))
> break;
>
> if (opcode == 0xffff)
> diff --git a/src/shared/btsnoop.c b/src/shared/btsnoop.c
> index 9ce2f2655..39960c4b8 100644
> --- a/src/shared/btsnoop.c
> +++ b/src/shared/btsnoop.c
> @@ -13,6 +13,7 @@
> #endif
>
> #define _GNU_SOURCE
> +#include <errno.h>
> #include <endian.h>
> #include <fcntl.h>
> #include <unistd.h>
> @@ -47,12 +48,16 @@ static const uint8_t btsnoop_id[] = { 0x62, 0x74, 0x73, 0x6e,
>
> static const uint32_t btsnoop_version = 1;
>
> +#define BTSNOOP_EPOCH_OFFSET 0x00E03AB44A676000ull
> +#define BTSNOOP_UNIX_TIME_OFFSET 946684800ll
> +
> struct pklg_pkt {
> uint32_t len;
> uint64_t ts;
> uint8_t type;
> } __attribute__ ((packed));
> #define PKLG_PKT_SIZE (sizeof(struct pklg_pkt))
> +#define PKLG_PAYLOAD_OFFSET (PKLG_PKT_SIZE - sizeof(uint32_t))
>
> struct btsnoop {
> int ref_count;
> @@ -271,13 +276,14 @@ bool btsnoop_write(struct btsnoop *btsnoop, struct timeval *tv,
> if (!btsnoop_rotate(btsnoop))
> return false;
>
> - ts = (tv->tv_sec - 946684800ll) * 1000000ll + tv->tv_usec;
> + ts = (tv->tv_sec - BTSNOOP_UNIX_TIME_OFFSET) * 1000000ll +
> + tv->tv_usec;
>
> pkt.size = htobe32(size);
> pkt.len = htobe32(size);
> pkt.flags = htobe32(flags);
> pkt.drops = htobe32(drops);
> - pkt.ts = htobe64(ts + 0x00E03AB44A676000ll);
> + pkt.ts = htobe64(ts + BTSNOOP_EPOCH_OFFSET);
>
> written = write(btsnoop->fd, &pkt, BTSNOOP_PKT_SIZE);
> if (written < 0)
> @@ -324,6 +330,121 @@ static uint32_t get_flags_from_opcode(uint16_t opcode)
> return 0xff;
> }
>
> +static ssize_t read_exact(int fd, void *data, size_t size)
> +{
> + uint8_t *ptr = data;
> + size_t offset = 0;
> +
> + while (offset < size) {
> + ssize_t len;
> +
> + len = read(fd, ptr + offset, size - offset);
> + if (len < 0) {
> + if (errno == EINTR)
> + continue;
> + return -1;
> + }
> +
> + if (len == 0)
> + break;
> +
> + offset += len;
> + }
> +
> + return offset;
> +}
> +
> +static bool read_packet_data(struct btsnoop *btsnoop, void *data,
> + uint16_t data_size, uint32_t toread,
> + uint16_t *size)
> +{
> + ssize_t len;
> +
> + if (!size || (!data && toread)) {
> + btsnoop->aborted = true;
> + return false;
> + }
> +
> + if (toread > data_size) {
> + btsnoop->aborted = true;
> + return false;
> + }
> +
> + len = read_exact(btsnoop->fd, data, toread);
> + if (len != (ssize_t) toread) {
> + btsnoop->aborted = true;
> + return false;
> + }
> +
> + *size = toread;
> +
> + return true;
> +}
> +
> +static bool decode_btsnoop_timestamp(uint64_t raw_ts, struct timeval *tv)
> +{
> + uint64_t ts;
> +
> + if (raw_ts < BTSNOOP_EPOCH_OFFSET)
> + return false;
> +
> + ts = raw_ts - BTSNOOP_EPOCH_OFFSET;
> + tv->tv_sec = (ts / 1000000ll) + BTSNOOP_UNIX_TIME_OFFSET;
> + tv->tv_usec = ts % 1000000ll;
> +
> + return true;
> +}
> +
> +static void get_pklg_opcode(uint8_t type, uint16_t *index, uint16_t *opcode)
> +{
> + switch (type) {
> + case 0x00:
> + *index = 0x0000;
> + *opcode = BTSNOOP_OPCODE_COMMAND_PKT;
> + break;
> + case 0x01:
> + *index = 0x0000;
> + *opcode = BTSNOOP_OPCODE_EVENT_PKT;
> + break;
> + case 0x02:
> + *index = 0x0000;
> + *opcode = BTSNOOP_OPCODE_ACL_TX_PKT;
> + break;
> + case 0x03:
> + *index = 0x0000;
> + *opcode = BTSNOOP_OPCODE_ACL_RX_PKT;
> + break;
> + case 0x08:
> + *index = 0x0000;
> + *opcode = BTSNOOP_OPCODE_SCO_TX_PKT;
> + break;
> + case 0x09:
> + *index = 0x0000;
> + *opcode = BTSNOOP_OPCODE_SCO_RX_PKT;
> + break;
> + case 0x12:
> + *index = 0x0000;
> + *opcode = BTSNOOP_OPCODE_ISO_TX_PKT;
> + break;
> + case 0x13:
> + *index = 0x0000;
> + *opcode = BTSNOOP_OPCODE_ISO_RX_PKT;
> + break;
> + case 0x0b:
> + *index = 0x0000;
> + *opcode = BTSNOOP_OPCODE_VENDOR_DIAG;
> + break;
> + case 0xfc:
> + *index = 0xffff;
> + *opcode = BTSNOOP_OPCODE_SYSTEM_NOTE;
> + break;
> + default:
> + *index = 0xffff;
> + *opcode = 0xffff;
> + break;
> + }
> +}
> +
> bool btsnoop_write_hci(struct btsnoop *btsnoop, struct timeval *tv,
> uint16_t index, uint16_t opcode, uint32_t drops,
> const void *data, uint16_t size)
> @@ -377,99 +498,53 @@ bool btsnoop_write_phy(struct btsnoop *btsnoop, struct timeval *tv,
> return btsnoop_write(btsnoop, tv, flags, 0, data, size);
> }
>
> -static bool pklg_read_hci(struct btsnoop *btsnoop, struct timeval *tv,
> - uint16_t *index, uint16_t *opcode,
> - void *data, uint16_t *size)
> +static bool pklg_read_hci(struct btsnoop *btsnoop,
> + struct timeval *tv, uint16_t *index, uint16_t *opcode,
> + void *data, uint16_t data_size, uint16_t *size)
> {
> struct pklg_pkt pkt;
> + uint32_t pkt_len;
> uint32_t toread;
> uint64_t ts;
> ssize_t len;
>
> - len = read(btsnoop->fd, &pkt, PKLG_PKT_SIZE);
> + len = read_exact(btsnoop->fd, &pkt, PKLG_PKT_SIZE);
> if (len == 0)
> return false;
>
> - if (len < 0 || len != PKLG_PKT_SIZE) {
> + if (len != PKLG_PKT_SIZE) {
> btsnoop->aborted = true;
> return false;
> }
>
> if (btsnoop->pklg_v2) {
> - toread = le32toh(pkt.len) - (PKLG_PKT_SIZE - 4);
> + pkt_len = le32toh(pkt.len);
>
> ts = le64toh(pkt.ts);
> tv->tv_sec = ts & 0xffffffff;
> tv->tv_usec = ts >> 32;
> } else {
> - toread = be32toh(pkt.len) - (PKLG_PKT_SIZE - 4);
> + pkt_len = be32toh(pkt.len);
>
> ts = be64toh(pkt.ts);
> tv->tv_sec = ts >> 32;
> tv->tv_usec = ts & 0xffffffff;
> }
>
> - if (toread > BTSNOOP_MAX_PACKET_SIZE) {
> - btsnoop->aborted = true;
> - return false;
> - }
> -
> - switch (pkt.type) {
> - case 0x00:
> - *index = 0x0000;
> - *opcode = BTSNOOP_OPCODE_COMMAND_PKT;
> - break;
> - case 0x01:
> - *index = 0x0000;
> - *opcode = BTSNOOP_OPCODE_EVENT_PKT;
> - break;
> - case 0x02:
> - *index = 0x0000;
> - *opcode = BTSNOOP_OPCODE_ACL_TX_PKT;
> - break;
> - case 0x03:
> - *index = 0x0000;
> - *opcode = BTSNOOP_OPCODE_ACL_RX_PKT;
> - break;
> - case 0x08:
> - *index = 0x0000;
> - *opcode = BTSNOOP_OPCODE_SCO_TX_PKT;
> - break;
> - case 0x09:
> - *index = 0x0000;
> - *opcode = BTSNOOP_OPCODE_SCO_RX_PKT;
> - break;
> - case 0x12:
> - *index = 0x0000;
> - *opcode = BTSNOOP_OPCODE_ISO_TX_PKT;
> - break;
> - case 0x13:
> - *index = 0x0000;
> - *opcode = BTSNOOP_OPCODE_ISO_RX_PKT;
> - break;
> - case 0x0b:
> - *index = 0x0000;
> - *opcode = BTSNOOP_OPCODE_VENDOR_DIAG;
> - break;
> - case 0xfc:
> - *index = 0xffff;
> - *opcode = BTSNOOP_OPCODE_SYSTEM_NOTE;
> - break;
> - default:
> - *index = 0xffff;
> - *opcode = 0xffff;
> - break;
> + if (pkt_len < PKLG_PAYLOAD_OFFSET) {
> + btsnoop->aborted = true;
> + return false;
> }
>
> - len = read(btsnoop->fd, data, toread);
> - if (len < 0) {
> + toread = pkt_len - PKLG_PAYLOAD_OFFSET;
> + if (toread > BTSNOOP_MAX_PACKET_SIZE) {
> btsnoop->aborted = true;
> return false;
> }
>
> - *size = toread;
> + get_pklg_opcode(pkt.type, index, opcode);
>
> - return true;
> + return read_packet_data(btsnoop, data, data_size, toread, size);
> }
>
> static uint16_t get_opcode_from_flags(uint8_t type, uint32_t flags)
> @@ -512,84 +587,106 @@ static uint16_t get_opcode_from_flags(uint8_t type, uint32_t flags)
> return 0xffff;
> }
>
> -bool btsnoop_read_hci(struct btsnoop *btsnoop, struct timeval *tv,
> - uint16_t *index, uint16_t *opcode,
> - void *data, uint16_t *size)
> +static bool read_uart_type(struct btsnoop *btsnoop, uint32_t *toread,
> + uint8_t *type)
> {
> - struct btsnoop_pkt pkt;
> - uint32_t toread, flags;
> - uint64_t ts;
> - uint8_t pkt_type;
> ssize_t len;
>
> - if (!btsnoop || btsnoop->aborted)
> - return false;
> -
> - if (btsnoop->pklg_format)
> - return pklg_read_hci(btsnoop, tv, index, opcode, data, size);
> -
> - len = read(btsnoop->fd, &pkt, BTSNOOP_PKT_SIZE);
> - if (len == 0)
> - return false;
> -
> - if (len < 0 || len != BTSNOOP_PKT_SIZE) {
> + if (!*toread) {
> btsnoop->aborted = true;
> return false;
> }
>
> - toread = be32toh(pkt.len);
> - if (toread > BTSNOOP_MAX_PACKET_SIZE) {
> + len = read_exact(btsnoop->fd, type, 1);
> + if (len != 1) {
> btsnoop->aborted = true;
> return false;
> }
>
> - flags = be32toh(pkt.flags);
> + (*toread)--;
>
> - ts = be64toh(pkt.ts) - 0x00E03AB44A676000ll;
> - tv->tv_sec = (ts / 1000000ll) + 946684800ll;
> - tv->tv_usec = ts % 1000000ll;
> + return true;
> +}
> +
> +static bool decode_btsnoop_record(struct btsnoop *btsnoop, uint32_t flags,
> + uint32_t *toread, uint16_t *index,
> + uint16_t *opcode)
> +{
> + uint8_t pkt_type;
>
> switch (btsnoop->format) {
> case BTSNOOP_FORMAT_HCI:
> *index = 0;
> *opcode = get_opcode_from_flags(0xff, flags);
> - break;
> -
> + return true;
> case BTSNOOP_FORMAT_UART:
> - len = read(btsnoop->fd, &pkt_type, 1);
> - if (len < 0) {
> - btsnoop->aborted = true;
> + if (!read_uart_type(btsnoop, toread, &pkt_type))
> return false;
> - }
> - toread--;
>
> *index = 0;
> *opcode = get_opcode_from_flags(pkt_type, flags);
> - break;
> -
> + return true;
> case BTSNOOP_FORMAT_MONITOR:
> *index = flags >> 16;
> *opcode = flags & 0xffff;
> - break;
> -
> + return true;
> default:
> btsnoop->aborted = true;
> return false;
> }
> +}
>
> - len = read(btsnoop->fd, data, toread);
> - if (len < 0) {
> +bool btsnoop_read_hci(struct btsnoop *btsnoop,
> + struct timeval *tv, uint16_t *index, uint16_t *opcode,
> + void *data, uint16_t data_size, uint16_t *size)
> +{
> + struct btsnoop_pkt pkt;
> + uint32_t toread, flags;
> + ssize_t len;
> +
> + if (!btsnoop || !tv || !index || !opcode || !size || btsnoop->aborted)
> + return false;
> +
> + if (btsnoop->pklg_format)
> + return pklg_read_hci(btsnoop, tv, index, opcode,
> + data, data_size, size);
> +
> + len = read_exact(btsnoop->fd, &pkt, BTSNOOP_PKT_SIZE);
> + if (len == 0)
> + return false;
> +
> + if (len != BTSNOOP_PKT_SIZE) {
> btsnoop->aborted = true;
> return false;
> }
>
> - *size = toread;
> + toread = be32toh(pkt.len);
> + if (toread > BTSNOOP_MAX_PACKET_SIZE) {
> + btsnoop->aborted = true;
> + return false;
> + }
>
> - return true;
> + flags = be32toh(pkt.flags);
> +
> + if (!decode_btsnoop_timestamp(be64toh(pkt.ts), tv)) {
> + btsnoop->aborted = true;
> + return false;
> + }
> +
> + if (!decode_btsnoop_record(btsnoop, flags, &toread, index, opcode))
> + return false;
> +
> + return read_packet_data(btsnoop, data, data_size, toread, size);
> }
>
> bool btsnoop_read_phy(struct btsnoop *btsnoop, struct timeval *tv,
> - uint16_t *frequency, void *data, uint16_t *size)
> + uint16_t *frequency, void *data, uint16_t *size)
> {
> + (void) btsnoop;
> + (void) tv;
> + (void) frequency;
> + (void) data;
> + (void) size;
> +
> return false;
> }
> diff --git a/src/shared/btsnoop.h b/src/shared/btsnoop.h
> index c24755d56..796604c58 100644
> --- a/src/shared/btsnoop.h
> +++ b/src/shared/btsnoop.h
> @@ -106,8 +106,8 @@ bool btsnoop_write_hci(struct btsnoop *btsnoop, struct timeval *tv,
> bool btsnoop_write_phy(struct btsnoop *btsnoop, struct timeval *tv,
> uint16_t frequency, const void *data, uint16_t size);
>
> -bool btsnoop_read_hci(struct btsnoop *btsnoop, struct timeval *tv,
> - uint16_t *index, uint16_t *opcode,
> - void *data, uint16_t *size);
> +bool btsnoop_read_hci(struct btsnoop *btsnoop,
> + struct timeval *tv, uint16_t *index, uint16_t *opcode,
> + void *data, uint16_t data_size, uint16_t *size);
> bool btsnoop_read_phy(struct btsnoop *btsnoop, struct timeval *tv,
> uint16_t *frequency, void *data, uint16_t *size);
> diff --git a/unit/test-btsnoop.c b/unit/test-btsnoop.c
> new file mode 100644
> index 000000000..710209097
> --- /dev/null
> +++ b/unit/test-btsnoop.c
> @@ -0,0 +1,794 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#define _GNU_SOURCE
> +#include <endian.h>
> +#include <fcntl.h>
> +#include <stdint.h>
> +#include <string.h>
> +#include <unistd.h>
> +
> +#include <glib.h>
> +
> +#include "src/shared/att-types.h"
> +#include "src/shared/btsnoop.h"
> +#include "unit/test-btsnoop.h"
> +
> +#define BTSNOOP_EPOCH_OFFSET 0x00E03AB44A676000ull
> +#define PKLG_PAYLOAD_OFFSET 9
> +
> +struct test_btsnoop_hdr {
> + uint8_t id[8];
> + uint32_t version;
> + uint32_t type;
> +} __packed;
> +
> +struct test_btsnoop_pkt {
> + uint32_t size;
> + uint32_t len;
> + uint32_t flags;
> + uint32_t drops;
> + uint64_t ts;
> +} __packed;
> +
> +struct test_pklg_pkt {
> + uint32_t len;
> + uint64_t ts;
> + uint8_t type;
> +} __packed;
> +
> +static const uint8_t btsnoop_id[] = {
> + 0x62, 0x74, 0x73, 0x6e, 0x6f, 0x6f, 0x70, 0x00
> +};
> +
> +static void append_bytes(GByteArray *array, const void *data, size_t size)
> +{
> + if (!size)
> + return;
> +
> + g_byte_array_append(array, data, size);
> +}
> +
> +static void append_btsnoop_header(GByteArray *array, uint32_t format)
> +{
> + struct test_btsnoop_hdr hdr;
> +
> + memcpy(hdr.id, btsnoop_id, sizeof(btsnoop_id));
> + hdr.version = htobe32(1);
> + hdr.type = htobe32(format);
> +
> + append_bytes(array, &hdr, sizeof(hdr));
> +}
> +
> +static void append_btsnoop_packet(GByteArray *array, uint32_t len,
> + uint32_t flags, uint64_t ts,
> + const void *data, size_t data_len)
> +{
> + struct test_btsnoop_pkt pkt;
> +
> + pkt.size = htobe32(len);
> + pkt.len = htobe32(len);
> + pkt.flags = htobe32(flags);
> + pkt.drops = 0;
> + pkt.ts = htobe64(ts);
> +
> + append_bytes(array, &pkt, sizeof(pkt));
> + append_bytes(array, data, data_len);
> +}
> +
> +static void append_pklg_packet(GByteArray *array, bool little_endian,
> + uint32_t payload_len, uint64_t ts,
> + uint8_t type, const void *data, size_t data_len)
> +{
> + struct test_pklg_pkt pkt;
> + uint32_t len = PKLG_PAYLOAD_OFFSET + payload_len;
> +
> + pkt.len = little_endian ? htole32(len) : htobe32(len);
> + pkt.ts = little_endian ? htole64(ts) : htobe64(ts);
> + pkt.type = type;
> +
> + append_bytes(array, &pkt, sizeof(pkt));
> + append_bytes(array, data, data_len);
> +}
> +
> +static char *write_tmp_trace(const void *data, size_t size)
> +{
> + char *path = NULL;
> + ssize_t written;
> + int fd;
> +
> + fd = g_file_open_tmp("bluez-btsnoop-XXXXXX", &path, NULL);
> + g_assert(fd >= 0);
> + written = write(fd, data, size);
> + g_assert_cmpint(written, ==, (ssize_t) size);
> + g_assert_cmpint(close(fd), ==, 0);
> +
> + return path;
> +}
> +
> +static char *new_tmp_path(void)
> +{
> + char *path = NULL;
> + int fd;
> +
> + fd = g_file_open_tmp("bluez-btsnoop-XXXXXX", &path, NULL);
> + g_assert(fd >= 0);
> + g_assert_cmpint(close(fd), ==, 0);
> + unlink(path);
> +
> + return path;
> +}
> +
> +static void unlink_rotated(const char *path, unsigned int count)
> +{
> + unsigned int i;
> +
> + for (i = 0; i <= count; i++) {
> + char *name;
> +
> + name = g_strdup_printf("%s.%u", path, i);
> + unlink(name);
> + g_free(name);
> + }
> +}
> +
> +static bool read_tmp_trace(const void *trace, size_t trace_len,
> + unsigned long flags, uint16_t data_size,
> + uint8_t *data, uint16_t *size,
> + uint16_t *index, uint16_t *opcode,
> + struct timeval *tv)
> +{
> + struct btsnoop *btsnoop;
> + char *path;
> + bool result;
> +
> + path = write_tmp_trace(trace, trace_len);
> + btsnoop = btsnoop_open(path, flags);
> + unlink(path);
> + g_free(path);
> +
> + if (!btsnoop)
> + return false;
> +
> + result = btsnoop_read_hci(btsnoop, tv, index, opcode, data,
> + data_size, size);
> + btsnoop_unref(btsnoop);
> +
> + return result;
> +}
> +
> +static bool read_trace_file(const char *path, unsigned long flags,
> + uint8_t *data, uint16_t data_size,
> + uint16_t *size, uint16_t *index,
> + uint16_t *opcode, struct timeval *tv)
> +{
> + struct btsnoop *btsnoop;
> + bool result;
> +
> + btsnoop = btsnoop_open(path, flags);
> + g_assert_nonnull(btsnoop);
> +
> + result = btsnoop_read_hci(btsnoop, tv, index, opcode, data,
> + data_size, size);
> + btsnoop_unref(btsnoop);
> +
> + return result;
> +}
> +
> +static void test_btsnoop_hci_valid(void)
> +{
> + GByteArray *trace = g_byte_array_new();
> + const uint8_t payload[] = { 0x01, 0x02, 0x03 };
> + uint8_t data[sizeof(payload)];
> + struct timeval tv;
> + uint16_t size = 0;
> + uint16_t index = 0xffff;
> + uint16_t opcode = 0xffff;
> +
> + append_btsnoop_header(trace, BTSNOOP_FORMAT_HCI);
> + append_btsnoop_packet(trace, sizeof(payload), 0x02,
> + BTSNOOP_EPOCH_OFFSET + 1234567, payload,
> + sizeof(payload));
> +
> + g_assert_true(read_tmp_trace(trace->data, trace->len, 0, sizeof(data),
> + data, &size, &index, &opcode, &tv));
> + g_assert_cmpint(index, ==, 0);
> + g_assert_cmpint(opcode, ==, BTSNOOP_OPCODE_COMMAND_PKT);
> + g_assert_cmpint(size, ==, sizeof(payload));
> + g_assert_cmpint(memcmp(data, payload, sizeof(payload)), ==, 0);
> + g_assert_cmpint(tv.tv_sec, ==, 946684801);
> + g_assert_cmpint(tv.tv_usec, ==, 234567);
> +
> + g_byte_array_unref(trace);
> +}
> +
> +static void test_btsnoop_create_invalid_args(void)
> +{
> + char *path = new_tmp_path();
> +
> + g_assert_null(btsnoop_create(path, 0, 1, BTSNOOP_FORMAT_HCI));
> + g_assert_null(btsnoop_create("/tmp/bluez/no/such/path", 0, 0,
> + BTSNOOP_FORMAT_HCI));
> + g_assert_null(btsnoop_ref(NULL));
> + btsnoop_unref(NULL);
> + g_assert_cmpint(btsnoop_get_format(NULL), ==, BTSNOOP_FORMAT_INVALID);
> +
> + g_free(path);
> +}
> +
> +static void test_btsnoop_open_invalid_headers(void)
> +{
> + GByteArray *trace = g_byte_array_new();
> + struct test_btsnoop_hdr hdr;
> + char *path;
> +
> + append_btsnoop_header(trace, BTSNOOP_FORMAT_HCI);
> + ((struct test_btsnoop_hdr *) trace->data)->version = htobe32(2);
> + path = write_tmp_trace(trace->data, trace->len);
> + g_assert_null(btsnoop_open(path, 0));
> + unlink(path);
> + g_free(path);
> + g_byte_array_set_size(trace, 0);
> +
> + memset(&hdr, 0x55, sizeof(hdr));
> + append_bytes(trace, &hdr, sizeof(hdr));
> + path = write_tmp_trace(trace->data, trace->len);
> + g_assert_null(btsnoop_open(path, 0));
> + g_assert_null(btsnoop_open(path, BTSNOOP_FLAG_PKLG_SUPPORT));
> + unlink(path);
> + g_free(path);
> + g_byte_array_unref(trace);
> +}
> +
> +static void test_btsnoop_write_hci_roundtrip(void)
> +{
> + const uint8_t command[] = { 0x01, 0x02, 0x03 };
> + const uint8_t event[] = { 0x04, 0x05 };
> + uint8_t data[sizeof(command)];
> + struct btsnoop *btsnoop;
> + struct timeval tv = { .tv_sec = 946684802, .tv_usec = 345678 };
> + uint16_t size = 0;
> + uint16_t index = 0;
> + uint16_t opcode = 0;
> + char *path = new_tmp_path();
> +
> + btsnoop = btsnoop_create(path, 0, 0, BTSNOOP_FORMAT_HCI);
> + g_assert_nonnull(btsnoop);
> + g_assert_cmpint(btsnoop_get_format(btsnoop), ==, BTSNOOP_FORMAT_HCI);
> + g_assert_true(btsnoop_write_hci(btsnoop, &tv, 0,
> + BTSNOOP_OPCODE_COMMAND_PKT, 0,
> + command, sizeof(command)));
> + g_assert_true(btsnoop_write_hci(btsnoop, &tv, 0,
> + BTSNOOP_OPCODE_EVENT_PKT, 0,
> + event, sizeof(event)));
> + g_assert_false(btsnoop_write_hci(btsnoop, &tv, 1,
> + BTSNOOP_OPCODE_COMMAND_PKT, 0,
> + command, sizeof(command)));
> + g_assert_false(btsnoop_write_hci(btsnoop, &tv, 0,
> + BTSNOOP_OPCODE_NEW_INDEX, 0,
> + command, sizeof(command)));
> + btsnoop_unref(btsnoop);
> +
> + g_assert_true(read_trace_file(path, 0, data, sizeof(data), &size,
> + &index, &opcode, &tv));
> + g_assert_cmpint(index, ==, 0);
> + g_assert_cmpint(opcode, ==, BTSNOOP_OPCODE_COMMAND_PKT);
> + g_assert_cmpint(size, ==, sizeof(command));
> + g_assert_cmpint(memcmp(data, command, sizeof(command)), ==, 0);
> +
> + btsnoop = btsnoop_open(path, 0);
> + g_assert_nonnull(btsnoop);
> + g_assert_true(btsnoop_read_hci(btsnoop, &tv, &index, &opcode, data,
> + sizeof(data), &size));
> + g_assert_true(btsnoop_read_hci(btsnoop, &tv, &index, &opcode, data,
> + sizeof(data), &size));
> + g_assert_cmpint(opcode, ==, BTSNOOP_OPCODE_EVENT_PKT);
> + g_assert_cmpint(size, ==, sizeof(event));
> + g_assert_cmpint(memcmp(data, event, sizeof(event)), ==, 0);
> + g_assert_false(btsnoop_read_hci(btsnoop, &tv, &index, &opcode, data,
> + sizeof(data), &size));
> + btsnoop_unref(btsnoop);
> +
> + unlink(path);
> + g_free(path);
> +}
> +
> +static void test_btsnoop_write_monitor_roundtrip(void)
> +{
> + const uint8_t payload[] = { 0xaa, 0xbb };
> + uint8_t data[sizeof(payload)];
> + struct btsnoop *btsnoop;
> + struct timeval tv = { .tv_sec = 946684800, .tv_usec = 0 };
> + uint16_t size = 0;
> + uint16_t index = 0;
> + uint16_t opcode = 0;
> + char *path = new_tmp_path();
> +
> + btsnoop = btsnoop_create(path, 0, 0, BTSNOOP_FORMAT_MONITOR);
> + g_assert_nonnull(btsnoop);
> + g_assert_true(btsnoop_write_hci(btsnoop, &tv, 7, 0x1234, 0,
> + payload, sizeof(payload)));
> + btsnoop_unref(btsnoop);
> +
> + g_assert_true(read_trace_file(path, 0, data, sizeof(data), &size,
> + &index, &opcode, &tv));
> + g_assert_cmpint(index, ==, 7);
> + g_assert_cmpint(opcode, ==, 0x1234);
> + g_assert_cmpint(size, ==, sizeof(payload));
> + g_assert_cmpint(memcmp(data, payload, sizeof(payload)), ==, 0);
> +
> + unlink(path);
> + g_free(path);
> +}
> +
> +static void test_btsnoop_write_phy_and_rotate(void)
> +{
> + const uint8_t payload[] = { 0x01 };
> + struct btsnoop *btsnoop;
> + struct timeval tv = { .tv_sec = 946684800, .tv_usec = 0 };
> + char *path = new_tmp_path();
> +
> + g_assert_false(btsnoop_write(NULL, &tv, 0, 0, payload,
> + sizeof(payload)));
> +
> + btsnoop = btsnoop_create(path, 24, 1, BTSNOOP_FORMAT_SIMULATOR);
> + g_assert_nonnull(btsnoop);
> + g_assert_true(btsnoop_write_phy(btsnoop, &tv, 2402, payload,
> + sizeof(payload)));
> + btsnoop_unref(btsnoop);
> +
> + btsnoop = btsnoop_create(path, 0, 0, BTSNOOP_FORMAT_HCI);
> + g_assert_nonnull(btsnoop);
> + g_assert_false(btsnoop_write_phy(btsnoop, &tv, 2402, payload,
> + sizeof(payload)));
> + btsnoop_unref(btsnoop);
> +
> + unlink(path);
> + unlink_rotated(path, 1);
> + g_free(path);
> +}
> +
> +static void test_btsnoop_monitor_valid(void)
> +{
> + GByteArray *trace = g_byte_array_new();
> + const uint8_t payload[] = { 0xaa, 0xbb };
> + uint8_t data[sizeof(payload)];
> + struct timeval tv;
> + uint16_t size = 0;
> + uint16_t index = 0xffff;
> + uint16_t opcode = 0xffff;
> +
> + append_btsnoop_header(trace, BTSNOOP_FORMAT_MONITOR);
> + append_btsnoop_packet(trace, sizeof(payload), 0x00051234,
> + BTSNOOP_EPOCH_OFFSET, payload, sizeof(payload));
> +
> + g_assert_true(read_tmp_trace(trace->data, trace->len, 0, sizeof(data),
> + data, &size, &index, &opcode, &tv));
> + g_assert_cmpint(index, ==, 5);
> + g_assert_cmpint(opcode, ==, 0x1234);
> + g_assert_cmpint(size, ==, sizeof(payload));
> + g_assert_cmpint(memcmp(data, payload, sizeof(payload)), ==, 0);
> +
> + g_byte_array_unref(trace);
> +}
> +
> +static void test_btsnoop_uart_valid(void)
> +{
> + GByteArray *trace = g_byte_array_new();
> + const uint8_t payload[] = { 0x04, 0x0e, 0x01, 0x00 };
> + uint8_t data[sizeof(payload) - 1];
> + struct timeval tv;
> + uint16_t size = 0;
> + uint16_t index = 0xffff;
> + uint16_t opcode = 0xffff;
> +
> + append_btsnoop_header(trace, BTSNOOP_FORMAT_UART);
> + append_btsnoop_packet(trace, sizeof(payload), 0,
> + BTSNOOP_EPOCH_OFFSET, payload, sizeof(payload));
> +
> + g_assert_true(read_tmp_trace(trace->data, trace->len, 0, sizeof(data),
> + data, &size, &index, &opcode, &tv));
> + g_assert_cmpint(index, ==, 0);
> + g_assert_cmpint(opcode, ==, BTSNOOP_OPCODE_EVENT_PKT);
> + g_assert_cmpint(size, ==, sizeof(payload) - 1);
> + g_assert_cmpint(memcmp(data, payload + 1, sizeof(data)), ==, 0);
> +
> + g_byte_array_unref(trace);
> +}
> +
> +static void test_btsnoop_uart_opcode_map(void)
> +{
> + static const struct {
> + uint8_t type;
> + uint32_t flags;
> + uint16_t opcode;
> + } cases[] = {
> + { 0x01, 0x00, BTSNOOP_OPCODE_COMMAND_PKT },
> + { 0x02, 0x00, BTSNOOP_OPCODE_ACL_TX_PKT },
> + { 0x02, 0x01, BTSNOOP_OPCODE_ACL_RX_PKT },
> + { 0x03, 0x00, BTSNOOP_OPCODE_SCO_TX_PKT },
> + { 0x03, 0x01, BTSNOOP_OPCODE_SCO_RX_PKT },
> + { 0x05, 0x00, BTSNOOP_OPCODE_ISO_TX_PKT },
> + { 0x05, 0x01, BTSNOOP_OPCODE_ISO_RX_PKT },
> + { 0x99, 0x00, 0xffff },
> + };
> + unsigned int i;
> +
> + for (i = 0; i < G_N_ELEMENTS(cases); i++) {
> + GByteArray *trace = g_byte_array_new();
> + const uint8_t payload[] = { cases[i].type, 0x00 };
> + uint8_t data[1];
> + struct timeval tv;
> + uint16_t size = 0;
> + uint16_t index = 0;
> + uint16_t opcode = 0;
> +
> + append_btsnoop_header(trace, BTSNOOP_FORMAT_UART);
> + append_btsnoop_packet(trace, sizeof(payload), cases[i].flags,
> + BTSNOOP_EPOCH_OFFSET, payload, sizeof(payload));
> +
> + g_assert_true(read_tmp_trace(trace->data, trace->len, 0,
> + sizeof(data), data, &size,
> + &index, &opcode, &tv));
> + g_assert_cmpint(opcode, ==, cases[i].opcode);
> + g_byte_array_unref(trace);
> + }
> +}
> +
> +static void test_btsnoop_rejects_small_capacity(void)
> +{
> + GByteArray *trace = g_byte_array_new();
> + const uint8_t payload[] = { 0x01, 0x02, 0x03 };
> + uint8_t data[4] = { 0xa5, 0xa5, 0xa5, 0xa5 };
> + struct timeval tv;
> + uint16_t size = 0;
> + uint16_t index = 0;
> + uint16_t opcode = 0;
> +
> + append_btsnoop_header(trace, BTSNOOP_FORMAT_HCI);
> + append_btsnoop_packet(trace, sizeof(payload), 0x02,
> + BTSNOOP_EPOCH_OFFSET, payload, sizeof(payload));
> +
> + g_assert_false(read_tmp_trace(trace->data, trace->len, 0, 2, data,
> + &size, &index, &opcode, &tv));
> + g_assert_cmpint(data[0], ==, 0xa5);
> + g_assert_cmpint(data[1], ==, 0xa5);
> + g_assert_cmpint(data[2], ==, 0xa5);
> + g_assert_cmpint(data[3], ==, 0xa5);
> +
> + g_byte_array_unref(trace);
> +}
> +
> +static void test_btsnoop_rejects_timestamp_underflow(void)
> +{
> + GByteArray *trace = g_byte_array_new();
> + struct timeval tv;
> + uint16_t size = 0;
> + uint16_t index = 0;
> + uint16_t opcode = 0;
> +
> + append_btsnoop_header(trace, BTSNOOP_FORMAT_HCI);
> + append_btsnoop_packet(trace, 0, 0x02, BTSNOOP_EPOCH_OFFSET - 1,
> + NULL, 0);
> +
> + g_assert_false(read_tmp_trace(trace->data, trace->len, 0, 0, NULL,
> + &size, &index, &opcode, &tv));
> +
> + g_byte_array_unref(trace);
> +}
> +
> +static void test_btsnoop_rejects_uart_zero_length(void)
> +{
> + GByteArray *trace = g_byte_array_new();
> + struct timeval tv;
> + uint16_t size = 0;
> + uint16_t index = 0;
> + uint16_t opcode = 0;
> +
> + append_btsnoop_header(trace, BTSNOOP_FORMAT_UART);
> + append_btsnoop_packet(trace, 0, 0, BTSNOOP_EPOCH_OFFSET, NULL, 0);
> +
> + g_assert_false(read_tmp_trace(trace->data, trace->len, 0, 0, NULL,
> + &size, &index, &opcode, &tv));
> +
> + g_byte_array_unref(trace);
> +}
> +
> +static void test_btsnoop_rejects_uart_short_type(void)
> +{
> + GByteArray *trace = g_byte_array_new();
> + struct timeval tv;
> + uint16_t size = 0;
> + uint16_t index = 0;
> + uint16_t opcode = 0;
> +
> + append_btsnoop_header(trace, BTSNOOP_FORMAT_UART);
> + append_btsnoop_packet(trace, 1, 0, BTSNOOP_EPOCH_OFFSET, NULL, 0);
> +
> + g_assert_false(read_tmp_trace(trace->data, trace->len, 0, 0, NULL,
> + &size, &index, &opcode, &tv));
> +
> + g_byte_array_unref(trace);
> +}
> +
> +static void test_btsnoop_rejects_short_payload(void)
> +{
> + GByteArray *trace = g_byte_array_new();
> + const uint8_t payload[] = { 0x01, 0x02 };
> + uint8_t data[3];
> + struct timeval tv;
> + uint16_t size = 0;
> + uint16_t index = 0;
> + uint16_t opcode = 0;
> +
> + append_btsnoop_header(trace, BTSNOOP_FORMAT_HCI);
> + append_btsnoop_packet(trace, 3, 0x02, BTSNOOP_EPOCH_OFFSET,
> + payload, sizeof(payload));
> +
> + g_assert_false(read_tmp_trace(trace->data, trace->len, 0,
> + sizeof(data), data, &size,
> + &index, &opcode, &tv));
> +
> + g_byte_array_unref(trace);
> +}
> +
> +static void test_pklg_big_endian_valid(void)
> +{
> + GByteArray *trace = g_byte_array_new();
> + const uint8_t payload[] = { 0x0e, 0x01, 0x00 };
> + uint8_t data[sizeof(payload)];
> + struct timeval tv;
> + uint16_t size = 0;
> + uint16_t index = 0xffff;
> + uint16_t opcode = 0xffff;
> +
> + append_pklg_packet(trace, false, sizeof(payload),
> + ((uint64_t) 123 << 32) | 456, 0x01, payload,
> + sizeof(payload));
> +
> + g_assert_true(read_tmp_trace(trace->data, trace->len,
> + BTSNOOP_FLAG_PKLG_SUPPORT, sizeof(data),
> + data, &size, &index, &opcode, &tv));
> + g_assert_cmpint(index, ==, 0);
> + g_assert_cmpint(opcode, ==, BTSNOOP_OPCODE_EVENT_PKT);
> + g_assert_cmpint(size, ==, sizeof(payload));
> + g_assert_cmpint(memcmp(data, payload, sizeof(payload)), ==, 0);
> + g_assert_cmpint(tv.tv_sec, ==, 123);
> + g_assert_cmpint(tv.tv_usec, ==, 456);
> +
> + g_byte_array_unref(trace);
> +}
> +
> +static void test_pklg_little_endian_valid(void)
> +{
> + GByteArray *trace = g_byte_array_new();
> + const uint8_t payload[] = { 0x01, 0x02, 0x03 };
> + uint8_t data[sizeof(payload)];
> + struct timeval tv;
> + uint16_t size = 0;
> + uint16_t index = 0xffff;
> + uint16_t opcode = 0xffff;
> +
> + append_pklg_packet(trace, true, sizeof(payload),
> + ((uint64_t) 456 << 32) | 123, 0x00, payload,
> + sizeof(payload));
> +
> + g_assert_true(read_tmp_trace(trace->data, trace->len,
> + BTSNOOP_FLAG_PKLG_SUPPORT, sizeof(data),
> + data, &size, &index, &opcode, &tv));
> + g_assert_cmpint(index, ==, 0);
> + g_assert_cmpint(opcode, ==, BTSNOOP_OPCODE_COMMAND_PKT);
> + g_assert_cmpint(size, ==, sizeof(payload));
> + g_assert_cmpint(data[0], ==, payload[0]);
> + g_assert_cmpint(tv.tv_sec, ==, 123);
> + g_assert_cmpint(tv.tv_usec, ==, 456);
> +
> + g_byte_array_unref(trace);
> +}
> +
> +static void test_pklg_rejects_short_length(void)
> +{
> + GByteArray *trace = g_byte_array_new();
> + struct test_pklg_pkt pkt;
> + const uint8_t padding[] = { 0x00, 0x00, 0x00 };
> + struct timeval tv;
> + uint16_t size = 0;
> + uint16_t index = 0;
> + uint16_t opcode = 0;
> +
> + pkt.len = htobe32(PKLG_PAYLOAD_OFFSET - 1);
> + pkt.ts = 0;
> + pkt.type = 0x01;
> +
> + append_bytes(trace, &pkt, sizeof(pkt));
> + append_bytes(trace, padding, sizeof(padding));
> +
> + g_assert_false(read_tmp_trace(trace->data, trace->len,
> + BTSNOOP_FLAG_PKLG_SUPPORT, 0, NULL,
> + &size, &index, &opcode, &tv));
> +
> + g_byte_array_unref(trace);
> +}
> +
> +static void test_pklg_rejects_small_capacity(void)
> +{
> + GByteArray *trace = g_byte_array_new();
> + const uint8_t payload[] = { 0x01, 0x02, 0x03 };
> + uint8_t data[4] = { 0xa5, 0xa5, 0xa5, 0xa5 };
> + struct timeval tv;
> + uint16_t size = 0;
> + uint16_t index = 0;
> + uint16_t opcode = 0;
> +
> + append_pklg_packet(trace, false, sizeof(payload), 0, 0x01, payload,
> + sizeof(payload));
> +
> + g_assert_false(read_tmp_trace(trace->data, trace->len,
> + BTSNOOP_FLAG_PKLG_SUPPORT, 2, data,
> + &size, &index, &opcode, &tv));
> + g_assert_cmpint(data[0], ==, 0xa5);
> + g_assert_cmpint(data[1], ==, 0xa5);
> + g_assert_cmpint(data[2], ==, 0xa5);
> + g_assert_cmpint(data[3], ==, 0xa5);
> +
> + g_byte_array_unref(trace);
> +}
> +
> +static void test_pklg_rejects_short_payload(void)
> +{
> + GByteArray *trace = g_byte_array_new();
> + const uint8_t payload[] = { 0x01, 0x02, 0x03 };
> + uint8_t data[4];
> + struct timeval tv;
> + uint16_t size = 0;
> + uint16_t index = 0;
> + uint16_t opcode = 0;
> +
> + append_pklg_packet(trace, false, 4, 0, 0x01, payload,
> + sizeof(payload));
> +
> + g_assert_false(read_tmp_trace(trace->data, trace->len,
> + BTSNOOP_FLAG_PKLG_SUPPORT, sizeof(data),
> + data, &size, &index, &opcode, &tv));
> +
> + g_byte_array_unref(trace);
> +}
> +
> +static void test_pklg_type_map(void)
> +{
> + static const struct {
> + uint8_t type;
> + uint16_t index;
> + uint16_t opcode;
> + } cases[] = {
> + { 0x02, 0x0000, BTSNOOP_OPCODE_ACL_TX_PKT },
> + { 0x03, 0x0000, BTSNOOP_OPCODE_ACL_RX_PKT },
> + { 0x08, 0x0000, BTSNOOP_OPCODE_SCO_TX_PKT },
> + { 0x09, 0x0000, BTSNOOP_OPCODE_SCO_RX_PKT },
> + { 0x12, 0x0000, BTSNOOP_OPCODE_ISO_TX_PKT },
> + { 0x13, 0x0000, BTSNOOP_OPCODE_ISO_RX_PKT },
> + { 0x0b, 0x0000, BTSNOOP_OPCODE_VENDOR_DIAG },
> + { 0xfc, 0xffff, BTSNOOP_OPCODE_SYSTEM_NOTE },
> + { 0xaa, 0xffff, 0xffff },
> + };
> + const uint8_t payload[] = { 0x00, 0x01, 0x02 };
> + unsigned int i;
> +
> + for (i = 0; i < G_N_ELEMENTS(cases); i++) {
> + GByteArray *trace = g_byte_array_new();
> + uint8_t data[sizeof(payload)];
> + struct timeval tv;
> + uint16_t size = 0;
> + uint16_t index = 0;
> + uint16_t opcode = 0;
> +
> + append_pklg_packet(trace, false, sizeof(payload), 0,
> + cases[i].type, payload,
> + sizeof(payload));
> +
> + g_assert_true(read_tmp_trace(trace->data, trace->len,
> + BTSNOOP_FLAG_PKLG_SUPPORT,
> + sizeof(data), data, &size,
> + &index, &opcode, &tv));
> + g_assert_cmpint(index, ==, cases[i].index);
> + g_assert_cmpint(opcode, ==, cases[i].opcode);
> + g_byte_array_unref(trace);
> + }
> +}
> +
> +static void test_btsnoop_truncation_fuzz(void)
> +{
> + GByteArray *trace = g_byte_array_new();
> + const uint8_t payload[] = { 0x01, 0x02, 0x03 };
> + size_t len;
> +
> + append_btsnoop_header(trace, BTSNOOP_FORMAT_HCI);
> + append_btsnoop_packet(trace, sizeof(payload), 0x02,
> + BTSNOOP_EPOCH_OFFSET, payload, sizeof(payload));
> +
> + for (len = 0; len < trace->len; len++) {
> + uint8_t data[sizeof(payload)];
> + struct timeval tv;
> + uint16_t size = 0;
> + uint16_t index = 0;
> + uint16_t opcode = 0;
> +
> + g_assert_false(read_tmp_trace(trace->data, len, 0,
> + sizeof(data), data, &size,
> + &index, &opcode, &tv));
> + }
> +
> + g_byte_array_unref(trace);
> +}
> +
> +static void test_pklg_truncation_fuzz(void)
> +{
> + GByteArray *trace = g_byte_array_new();
> + const uint8_t payload[] = { 0x01, 0x02, 0x03 };
> + size_t len;
> +
> + append_pklg_packet(trace, false, sizeof(payload), 0, 0x01, payload,
> + sizeof(payload));
> +
> + for (len = 0; len < trace->len; len++) {
> + uint8_t data[sizeof(payload)];
> + struct timeval tv;
> + uint16_t size = 0;
> + uint16_t index = 0;
> + uint16_t opcode = 0;
> +
> + g_assert_false(read_tmp_trace(trace->data, len,
> + BTSNOOP_FLAG_PKLG_SUPPORT,
> + sizeof(data), data, &size,
> + &index, &opcode, &tv));
> + }
> +
> + g_byte_array_unref(trace);
> +}
> +
> +int main(int argc, char *argv[])
> +{
> + g_test_init(&argc, &argv, NULL);
> +
> + g_test_add_func("/btsnoop/hci/valid", test_btsnoop_hci_valid);
> + g_test_add_func("/btsnoop/create/invalid",
> + test_btsnoop_create_invalid_args);
> + g_test_add_func("/btsnoop/open/invalid",
> + test_btsnoop_open_invalid_headers);
> + g_test_add_func("/btsnoop/write/hci-roundtrip",
> + test_btsnoop_write_hci_roundtrip);
> + g_test_add_func("/btsnoop/write/monitor-roundtrip",
> + test_btsnoop_write_monitor_roundtrip);
> + g_test_add_func("/btsnoop/write/phy-and-rotate",
> + test_btsnoop_write_phy_and_rotate);
> + g_test_add_func("/btsnoop/monitor/valid", test_btsnoop_monitor_valid);
> + g_test_add_func("/btsnoop/uart/valid", test_btsnoop_uart_valid);
> + g_test_add_func("/btsnoop/uart/opcode-map",
> + test_btsnoop_uart_opcode_map);
> + g_test_add_func("/btsnoop/capacity/reject",
> + test_btsnoop_rejects_small_capacity);
> + g_test_add_func("/btsnoop/timestamp/underflow",
> + test_btsnoop_rejects_timestamp_underflow);
> + g_test_add_func("/btsnoop/uart/zero-length",
> + test_btsnoop_rejects_uart_zero_length);
> + g_test_add_func("/btsnoop/uart/short-type",
> + test_btsnoop_rejects_uart_short_type);
> + g_test_add_func("/btsnoop/payload/short",
> + test_btsnoop_rejects_short_payload);
> + g_test_add_func("/pklg/big-endian/valid", test_pklg_big_endian_valid);
> + g_test_add_func("/pklg/little-endian/valid",
> + test_pklg_little_endian_valid);
> + g_test_add_func("/pklg/length/short", test_pklg_rejects_short_length);
> + g_test_add_func("/pklg/capacity/reject",
> + test_pklg_rejects_small_capacity);
> + g_test_add_func("/pklg/payload/short", test_pklg_rejects_short_payload);
> + g_test_add_func("/pklg/type-map", test_pklg_type_map);
> + g_test_add_func("/btsnoop/fuzz/truncation",
> + test_btsnoop_truncation_fuzz);
> + g_test_add_func("/pklg/fuzz/truncation", test_pklg_truncation_fuzz);
> +
> + return g_test_run();
> +}
> diff --git a/unit/test-btsnoop.h b/unit/test-btsnoop.h
> new file mode 100644
> index 000000000..9a12f3e71
> --- /dev/null
> +++ b/unit/test-btsnoop.h
> @@ -0,0 +1,3 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +void add_pklg_tests(void);
> --
> 2.43.0
>
>
--
Luiz Augusto von Dentz
^ permalink raw reply
* [bluez/bluez] 5792da: monitor: Add decoding of new MSFT extension featur...
From: Marcel Holtmann @ 2026-06-22 12:53 UTC (permalink / raw)
To: linux-bluetooth
Branch: refs/heads/master
Home: https://github.com/bluez/bluez
Commit: 5792da929b5caa454b9143b5cfd6f42950f381c6
https://github.com/bluez/bluez/commit/5792da929b5caa454b9143b5cfd6f42950f381c6
Author: Marcel Holtmann <marcel@holtmann.org>
Date: 2026-06-22 (Mon, 22 Jun 2026)
Changed paths:
M monitor/packet.c
Log Message:
-----------
monitor: Add decoding of new MSFT extension feature bits
To unsubscribe from these emails, change your notification settings at https://github.com/bluez/bluez/settings/notifications
^ permalink raw reply
* RE: [BlueZ,v1] shared: rap: Defer CS Event registration until connection setup
From: bluez.test.bot @ 2026-06-22 7:49 UTC (permalink / raw)
To: linux-bluetooth, naga.akella
In-Reply-To: <20260622062905.3525480-1-naga.akella@oss.qualcomm.com>
[-- Attachment #1: Type: text/plain, Size: 825 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=1114528
---Test result---
Test Summary:
CheckPatch PASS 1.08 seconds
GitLint PASS 0.73 seconds
BuildEll PASS 20.15 seconds
BluezMake PASS 616.98 seconds
CheckSmatch PASS 326.97 seconds
bluezmakeextell PASS 166.59 seconds
IncrementalBuild PASS 619.54 seconds
ScanBuild PASS 932.12 seconds
https://github.com/bluez/bluez/pull/2250
---
Regards,
Linux Bluetooth
^ permalink raw reply
* [bluez/bluez] f46211: shared: rap: Defer CS Event registration until con...
From: Bhavani @ 2026-06-22 7:03 UTC (permalink / raw)
To: linux-bluetooth
Branch: refs/heads/1114528
Home: https://github.com/bluez/bluez
Commit: f462115223abcf275bb6109f19647b902c6fed06
https://github.com/bluez/bluez/commit/f462115223abcf275bb6109f19647b902c6fed06
Author: Naga Bhavani Akella <naga.akella@oss.qualcomm.com>
Date: 2026-06-22 (Mon, 22 Jun 2026)
Changed paths:
M profiles/ranging/rap.c
Log Message:
-----------
shared: rap: Defer CS Event registration until connection setup
Move LE CS event registration from rap_probe()
to rap_accept(). rap_probe() runs during service discovery
while no ACL connection or connection handle exists,
so connection-scoped CS events cannot be delivered or
associated with a device. rap_accept() is called
after the ACL link is established, when a valid
connection handle is available and the HCI layer
can route events correctly.
Registering events at this point ensures
proper handling of CS events.
To unsubscribe from these emails, change your notification settings at https://github.com/bluez/bluez/settings/notifications
^ permalink raw reply
* [PATCH BlueZ v1] shared: rap: Defer CS Event registration until connection setup
From: Naga Bhavani Akella @ 2026-06-22 6:29 UTC (permalink / raw)
To: linux-bluetooth
Cc: luiz.dentz, quic_mohamull, quic_hbandi, quic_anubhavg,
Naga Bhavani Akella
Move LE CS event registration from rap_probe()
to rap_accept(). rap_probe() runs during service discovery
while no ACL connection or connection handle exists,
so connection-scoped CS events cannot be delivered or
associated with a device. rap_accept() is called
after the ACL link is established, when a valid
connection handle is available and the HCI layer
can route events correctly.
Registering events at this point ensures
proper handling of CS events.
---
profiles/ranging/rap.c | 63 +++++++++++++++++++-----------------------
1 file changed, 29 insertions(+), 34 deletions(-)
diff --git a/profiles/ranging/rap.c b/profiles/ranging/rap.c
index dc57eeda6..fa3e27328 100644
--- a/profiles/ranging/rap.c
+++ b/profiles/ranging/rap.c
@@ -322,35 +322,6 @@ static int rap_probe(struct btd_service *service)
return -EINVAL;
}
- /* Get or create shared adapter-level HCI channel */
- data->adapter_data = rap_adapter_data_ref(adapter);
- if (!data->adapter_data) {
- error("Failed to get adapter HCI channel");
- bt_rap_unref(data->rap);
- free(data);
- return -EINVAL;
- }
-
- DBG("Using shared HCI channel for adapter (ref_count=%d)",
- data->adapter_data->ref_count);
-
- /* Create per-device HCI state machine with valid rap instance */
- DBG("Attaching per-device HCI state machine");
- data->hci_sm = bt_rap_attach_hci(data->rap, data->adapter_data->hci,
- btd_opts.defaults.bcs.role,
- btd_opts.defaults.bcs.cs_sync_ant_sel,
- btd_opts.defaults.bcs.max_tx_power);
-
- if (!data->hci_sm) {
- error("Failed to attach HCI state machine for device");
- rap_adapter_data_unref(data->adapter_data);
- bt_rap_unref(data->rap);
- free(data);
- return -EINVAL;
- }
-
- DBG("HCI state machine attached successfully for device");
-
rap_data_add(data);
data->ready_id = bt_rap_ready_register(data->rap, rap_ready, service,
@@ -382,6 +353,7 @@ static void rap_remove(struct btd_service *service)
static int rap_accept(struct btd_service *service)
{
struct btd_device *device = btd_service_get_device(service);
+ struct btd_adapter *adapter = device_get_adapter(device);
struct bt_gatt_client *client = btd_device_get_gatt_client(device);
struct rap_data *data = btd_service_get_user_data(service);
struct bt_att *att;
@@ -398,6 +370,33 @@ static int rap_accept(struct btd_service *service)
return -EINVAL;
}
+ /* init shared adapter HCI channel */
+ if (!data->adapter_data) {
+ data->adapter_data = rap_adapter_data_ref(adapter);
+ if (!data->adapter_data) {
+ error("Failed to get adapter HCI channel");
+ return -EINVAL;
+ }
+ DBG("Using shared HCI channel for adapter (ref_count=%d)",
+ data->adapter_data->ref_count);
+ }
+
+ /* per-device HCI state machine */
+ if (!data->hci_sm) {
+ data->hci_sm = bt_rap_attach_hci(data->rap,
+ data->adapter_data->hci,
+ btd_opts.defaults.bcs.role,
+ btd_opts.defaults.bcs.cs_sync_ant_sel,
+ btd_opts.defaults.bcs.max_tx_power);
+ if (!data->hci_sm) {
+ error("Failed to attach HCI state machine for device");
+ rap_adapter_data_unref(data->adapter_data);
+ data->adapter_data = NULL;
+ return -EINVAL;
+ }
+ DBG("HCI state machine attached successfully for device");
+ }
+
if (!bt_rap_attach(data->rap, client)) {
error("RAP unable to attach");
return -EINVAL;
@@ -408,11 +407,7 @@ static int rap_accept(struct btd_service *service)
bdaddr = device_get_address(device);
bdaddr_type = device_get_le_address_type(device);
- if (att && data->adapter_data && data->adapter_data->hci &&
- data->hci_sm) {
- /* Use bt_hci_get_conn_handle to find the connection handle
- * by bdaddr using HCIGETCONNLIST ioctl
- */
+ if (att && data->adapter_data->hci && data->hci_sm) {
if (bt_hci_get_conn_handle(data->adapter_data->hci,
(const uint8_t *) bdaddr, &handle)) {
DBG("Found conn handle 0x%04X for %s", handle, addr);
--
^ permalink raw reply related
* RE: [PATCH 1/8] PCI: imx6: Add skip_pwrctrl_off flag support
From: Sherry Sun @ 2026-06-22 5:52 UTC (permalink / raw)
To: Frank Li (OSS), Sherry Sun (OSS)
Cc: robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org,
Frank Li, s.hauer@pengutronix.de, kernel@pengutronix.de,
festevam@gmail.com, Amitkumar Karwar, Neeraj Sanjay Kale,
marcel@holtmann.org, luiz.dentz@gmail.com, Hongxing Zhu,
l.stach@pengutronix.de, lpieralisi@kernel.org,
kwilczynski@kernel.org, mani@kernel.org, bhelgaas@google.com,
brgl@kernel.org, imx@lists.linux.dev, linux-pci@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-bluetooth@vger.kernel.org,
linux-pm@vger.kernel.org
In-Reply-To: <ajQ64ZswbmTceIGO@SMW015318>
> On Thu, Jun 18, 2026 at 06:10:40PM +0800, Sherry Sun (OSS) wrote:
> > From: Sherry Sun <sherry.sun@nxp.com>
> >
> > Use dw_pcie::skip_pwrctrl_off to avoid powering off devices during
> > suspend to preserve wakeup capability of the devices and also not to
> > power on the devices in the init path.
> > This allows controller power-off to be skipped when some devices(e.g.
> > M.2 cards key E without auxiliary power) required to support PCIe L2
> > link state and wake-up mechanisms.
> >
> > Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
> > ---
> > drivers/pci/controller/dwc/pci-imx6.c | 36
> > +++++++++++++++++----------
> > 1 file changed, 23 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > b/drivers/pci/controller/dwc/pci-imx6.c
> > index 0fa716d1ed75..ff5a9565dbbf 100644
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > @@ -1382,16 +1382,20 @@ static int imx_pcie_host_init(struct dw_pcie_rp
> *pp)
> > }
> > }
> >
> > - ret = pci_pwrctrl_create_devices(dev);
> > - if (ret) {
> > - dev_err(dev, "failed to create pwrctrl devices\n");
> > - goto err_reg_disable;
> > + if (!pci->suspended) {
> > + ret = pci_pwrctrl_create_devices(dev);
> > + if (ret) {
> > + dev_err(dev, "failed to create pwrctrl devices\n");
> > + goto err_reg_disable;
> > + }
>
> supposed create_devices only do once.
>
> pci_pwrctrl_power_on_devices() controller on and off for difference case.
>
Hi Frank,
Yes, pci_pwrctrl_create_devices() is currently only called once
during imx_pcie_probe.
pci_pwrctrl_power_on_devices() is called during imx_pcie_probe
and during suspend/resume (depending on skip_pwrctrl_off flag).
Best Regards
Sherry
> > }
> >
> > - ret = pci_pwrctrl_power_on_devices(dev);
> > - if (ret) {
> > - dev_err(dev, "failed to power on pwrctrl devices\n");
> > - goto err_pwrctrl_destroy;
> > + if (!pp->skip_pwrctrl_off) {
> > + ret = pci_pwrctrl_power_on_devices(dev);
> > + if (ret) {
> > + dev_err(dev, "failed to power on pwrctrl devices\n");
> > + goto err_pwrctrl_destroy;
> > + }
> > }
> >
> > ret = imx_pcie_clk_enable(imx_pcie); @@ -1460,9 +1464,10 @@
> static
> > int imx_pcie_host_init(struct dw_pcie_rp *pp)
> > err_clk_disable:
> > imx_pcie_clk_disable(imx_pcie);
> > err_pwrctrl_power_off:
> > - pci_pwrctrl_power_off_devices(dev);
> > + if (!pp->skip_pwrctrl_off)
> > + pci_pwrctrl_power_off_devices(dev);
> > err_pwrctrl_destroy:
> > - if (ret != -EPROBE_DEFER)
> > + if (ret != -EPROBE_DEFER && !pci->suspended)
> > pci_pwrctrl_destroy_devices(dev);
> > err_reg_disable:
> > if (imx_pcie->vpcie)
> > @@ -1482,7 +1487,8 @@ static void imx_pcie_host_exit(struct dw_pcie_rp
> *pp)
> > }
> > imx_pcie_clk_disable(imx_pcie);
> >
> > - pci_pwrctrl_power_off_devices(pci->dev);
> > + if (!pci->pp.skip_pwrctrl_off)
> > + pci_pwrctrl_power_off_devices(pci->dev);
> > if (imx_pcie->vpcie)
> > regulator_disable(imx_pcie->vpcie);
> > }
> > @@ -1990,12 +1996,16 @@ static int imx_pcie_probe(struct
> > platform_device *pdev) static void imx_pcie_shutdown(struct
> > platform_device *pdev) {
> > struct imx_pcie *imx_pcie = platform_get_drvdata(pdev);
> > + struct dw_pcie *pci = imx_pcie->pci;
> > + struct dw_pcie_rp *pp = &pci->pp;
> >
> > /* bring down link, so bootloader gets clean state in case of reboot */
> > imx_pcie_assert_core_reset(imx_pcie);
> > imx_pcie_assert_perst(imx_pcie, true);
> > - pci_pwrctrl_power_off_devices(&pdev->dev);
> > - pci_pwrctrl_destroy_devices(&pdev->dev);
> > + if (!pp->skip_pwrctrl_off)
> > + pci_pwrctrl_power_off_devices(&pdev->dev);
> > + if (!pci->suspended)
> > + pci_pwrctrl_destroy_devices(&pdev->dev);
> > }
> >
> > static const struct imx_pcie_drvdata drvdata[] = {
> > --
> > 2.50.1
> >
> >
^ permalink raw reply
* Re: [PATCH v3] Bluetooth: btmtk: Add MT7928 support
From: Chris Lu (陸稚泓) @ 2026-06-22 5:32 UTC (permalink / raw)
To: pmenzel@molgen.mpg.de
Cc: Will-CY Lee (李政穎),
Steve Lee (李視誠), luiz.dentz@gmail.com,
marcel@holtmann.org, SS Wu (巫憲欣),
linux-kernel@vger.kernel.org, johan.hedberg@gmail.com, Sean Wang,
linux-bluetooth@vger.kernel.org,
linux-mediatek@lists.infradead.org
In-Reply-To: <36ecf0bd-8d3f-482d-891d-c07b4c81a145@molgen.mpg.de>
Hi Paul,
On Thu, 2026-06-18 at 07:30 +0200, Paul Menzel wrote:
>
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>
>
> Am 18.06.26 um 01:51 schrieb Chris Lu:
> > Add support for MT7928 (internal device ID is MT7935) which
> > requires additional firmware (CBMCU firmware) loading before
> > Bluetooth firmware.
> >
> > CBMCU is a new component on MT7928 to handle common part shared
> > across the combo chip (Wi-Fi/Bluetooth's subsystem), providing
> > a better user experience through improved coordination between
> > subsystems.
> >
> > Implement two-phase CBMCU firmware download: Phase 1 loads
> > section with type 0x5 containing global descriptor,
> > section maps and signature data; Phase 2 loads remaining
> > firmware sections. Add retry mechanism for concurrent download
> > protection.
> >
> > After CBMCU firmware loads successfully, the driver continues
> > to load corresponding BT firmware based on device ID through
> > fallthrough to case 0x7922/0x7925.
> >
> > The firmware required for MT7928 will be scheduled for upload
> > to linux-firmware at a later stage.
>
> Should you resend please re-flow for 72 characters per line to use
> less
> lines.
>
> Also, please mention the document name, revision and file name, where
> the CBMCU format is described.
I'll reformat the line breaks and add firmware name information for
MT7928 in commit message.
>
> > MT7928 bringup kernel log:
> > [90.209995] usb 1-3: New USB device found, idVendor=0e8d,
> > idProduct=7935, bcdDevice= 1.00
> > [90.210027] usb 1-3: New USB device strings: Mfr=5,
> > Product=6, SerialNumber=7
> > [90.210046] usb 1-3: Product: Wireless_Device
> > [90.210060] usb 1-3: Manufacturer: MediaTek Inc.
> > [90.210075] usb 1-3: SerialNumber: 000000000
> > [90.223089] Bluetooth: hci1: CBMCU Version: 0x00000000,
> > Build Time: 20260601T161751+0800
> > [90.664706] Bluetooth: hci1: CBMCU firmware download completed
> > [90.685424] Bluetooth: hci1: HW/SW Version: 0x00000000,
> > Build Time: 20260527000816
> > [93.771612] Bluetooth: hci1: Device setup in 3467323 usecs
>
> Wow, over three seconds is too long. Can you please check how to
> bring
> this below one second.
Due to current driver design, we can only use control URBs with shorter
data lengths for setup. Our previous IC also took nearly three seconds
to download firmware. MediaTek Bluetooth team's next improvement goal
will focus on reducing setup time.
>
> > [93.771657] Bluetooth: hci1: HCI Enhanced Setup Synchronous
> > Connection command is advertised, but not supported.
> > [93.890840] Bluetooth: hci1: AOSP extensions version v2.00
> > [93.890887] Bluetooth: hci1: AOSP quality report is supported
> > [93.893444] Bluetooth: MGMT ver 1.23
>
> Please do not wrap pasted logs. `scripts/checkpatch.pl` should not
> complain:
Because the previous submission, this section was detected by bluetooth
check robot for exceeding character limit. I will restore its original
form in next submission if wrapping is not required.
>
> ```
> # Check if the commit log is in a possible stack dump
> if ($in_commit_log &&
> !$commit_log_possible_stack_dump &&
> ($line =~ /^\s*(?:WARNING:|BUG:)/ ||
> $line =~ /^\s*\[\s*\d+\.\d{6,6}\s*\]/ ||
> # timestamp
> $line =~ /^\s*\[\<[0-9a-fA-F]{8,}\>\]/) ||
> $line =~ /^(?:\s+\w+:\s+[0-9a-fA-F]+){3,3}/ ||
> $line =~ /^\s*\#\d+\s*\[[0-9a-fA-F]+\]\s*\w+ at
> [0-9a-fA-F]+/) {
> # stack dump address styles
> $commit_log_possible_stack_dump = 1;
> }
> ```
>
> > Signed-off-by: Chris Lu <chris.lu@mediatek.com>
> > ---
> > v1->v2: Update error message; Use macro instead of magic number.
> > v2->v3: Update commit message.
> > ---
> > drivers/bluetooth/btmtk.c | 348
> > +++++++++++++++++++++++++++++++++++++-
> > drivers/bluetooth/btmtk.h | 3 +
> > 2 files changed, 350 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c
> > index 02a96342e964..6bae0b0794dd 100644
> > --- a/drivers/bluetooth/btmtk.c
> > +++ b/drivers/bluetooth/btmtk.c
> > @@ -21,6 +21,8 @@
> > #define MTK_FW_ROM_PATCH_SEC_MAP_SIZE 64
> > #define MTK_SEC_MAP_COMMON_SIZE 12
> > #define MTK_SEC_MAP_NEED_SEND_SIZE 52
> > +#define MTK_SEC_MAP_LENGTH_SIZE 4
> > +#define MTK_SEC_CBMCU_DESC 0x5
> >
> > /* It is for mt79xx iso data transmission setting */
> > #define MTK_ISO_THRESHOLD 264
> > @@ -120,6 +122,10 @@ void btmtk_fw_get_filename(char *buf, size_t
> > size, u32 dev_id, u32 fw_ver,
> > snprintf(buf, size,
> >
> > "mediatek/mt%04x/BT_RAM_CODE_MT%04x_1_%x_hdr.bin",
> > dev_id & 0xffff, dev_id & 0xffff, (fw_ver &
> > 0xff) + 1);
> > + else if (dev_id == 0x7935)
>
> I wonder if the marketing name should be added as a comment.
>
> > + snprintf(buf, size,
> > +
> > "mediatek/mt7928/BT_RAM_CODE_MT%04x_1_1_hdr.bin",
> > + dev_id & 0xffff);
>
> `dev_id` is u32, so the truncatation is unnecessary?
While technically %04x only prints the lower 16 bits, I prefer to keep
"& 0xffff" for consistency with all other cases in
btmtk_fw_get_filename function. The explicit masking makes the intent
clear and maintains the existing code style.
>
> > else if (dev_id == 0x7961 && fw_flavor)
> > snprintf(buf, size,
> > "mediatek/BT_RAM_CODE_MT%04x_1a_%x_hdr.bin",
> > @@ -734,6 +740,7 @@ static int btmtk_usb_hci_wmt_sync(struct
> > hci_dev *hdev,
> > status = BTMTK_WMT_ON_UNDONE;
> > break;
> > case BTMTK_WMT_PATCH_DWNLD:
> > + case BTMTK_WMT_CBMCU_DWNLD:
> > if (wmt_evt->whdr.flag == 2)
> > status = BTMTK_WMT_PATCH_DONE;
> > else if (wmt_evt->whdr.flag == 1)
> > @@ -870,6 +877,334 @@ static u32 btmtk_usb_reset_done(struct
> > hci_dev *hdev)
> > return val & MTK_BT_RST_DONE;
> > }
> >
> > +static int btmtk_cbmcu_patch_status(struct hci_dev *hdev,
> > + wmt_cmd_sync_func_t wmt_cmd_sync,
> > + u8 *patch_status)
> > +{
> > + struct btmtk_hci_wmt_params wmt_params;
> > + int status, err, retry = 20;
> > +
> > + do {
> > + wmt_params.op = BTMTK_WMT_CBMCU_DWNLD;
> > + wmt_params.flag = 0xF0;
> > + wmt_params.dlen = 0;
> > + wmt_params.data = NULL;
> > + wmt_params.status = &status;
> > +
> > + err = wmt_cmd_sync(hdev, &wmt_params);
> > + if (err < 0) {
> > + bt_dev_err(hdev, "Failed to query CBMCU patch
> > status (%d)", err);
> > + return err;
> > + }
> > +
> > + *patch_status = (u8)status;
> > +
> > + if (*patch_status == BTMTK_WMT_PATCH_PROGRESS) {
> > + msleep(100);
> > + retry--;
> > + } else {
> > + break;
> > + }
> > + } while (retry > 0);
> > +
> > + return 0;
> > +}
> > +
> > +static int btmtk_query_cbmcu_section(struct hci_dev *hdev,
> > + wmt_cmd_sync_func_t
> > wmt_cmd_sync,
> > + u8 cbmcu_type,
> > + const u8 *section_map,
> > + u32 cert_len)
> > +{
> > + struct btmtk_hci_wmt_params wmt_params;
> > + u8 cmd[64];
> > + int status, err;
> > +
> > + cmd[0] = 0;
> > + cmd[1] = cbmcu_type;
> > +
> > + if (cbmcu_type == 0)
> > + put_unaligned_le32(cert_len, &cmd[2]);
> > + else
> > + memcpy(&cmd[2], section_map,
> > MTK_SEC_MAP_NEED_SEND_SIZE);
> > +
> > + wmt_params.op = BTMTK_WMT_CBMCU_DWNLD;
> > + wmt_params.flag = 0;
> > + wmt_params.dlen = cbmcu_type ?
> > + MTK_SEC_MAP_NEED_SEND_SIZE + 2 :
> > + MTK_SEC_MAP_LENGTH_SIZE + 2;
> > + wmt_params.data = cmd;
> > + wmt_params.status = &status;
> > +
> > + err = wmt_cmd_sync(hdev, &wmt_params);
> > + if (err < 0) {
> > + bt_dev_err(hdev, "Failed to query CBMCU section
> > (%d)", err);
> > + return err;
> > + }
> > +
> > + /* Query should return UNDONE status for successful section
> > query */
> > + if (status != BTMTK_WMT_PATCH_UNDONE) {
> > + bt_dev_err(hdev, "CBMCU section query status error
> > (%d)", status);
> > + return -EIO;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int btmtk_download_cbmcu_section(struct hci_dev *hdev,
> > + wmt_cmd_sync_func_t
> > wmt_cmd_sync,
> > + const u8 *fw_data,
> > + u32 dl_size)
> > +{
> > + struct btmtk_hci_wmt_params wmt_params;
> > + u32 sent_len, total_size = dl_size;
> > + int err;
> > +
> > + wmt_params.op = BTMTK_WMT_CBMCU_DWNLD;
> > + wmt_params.status = NULL;
> > +
> > + while (dl_size > 0) {
> > + sent_len = min_t(u32, 250, dl_size);
> > +
> > + if (dl_size == total_size)
> > + wmt_params.flag = 1;
> > + else if (dl_size == sent_len)
> > + wmt_params.flag = 3;
> > + else
> > + wmt_params.flag = 2;
>
> This is not easy to read, as it’s not clear right away, what 1, 2 and
> 3
> mean. Maybe use enums?
>
> > +
> > + wmt_params.dlen = sent_len;
> > + wmt_params.data = fw_data;
> > +
> > + err = wmt_cmd_sync(hdev, &wmt_params);
> > + if (err < 0) {
> > + bt_dev_err(hdev, "Failed to send CBMCU
> > section data (%d)", err);
>
> Print the sent parameters? Or will `wmt_cmd_sync()` log something?
>
> > + return err;
> > + }
> > +
> > + dl_size -= sent_len;
> > + fw_data += sent_len;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int btmtk_enable_cbmcu_patch(struct hci_dev *hdev,
> > + wmt_cmd_sync_func_t wmt_cmd_sync)
> > +{
> > + struct btmtk_hci_wmt_params wmt_params;
> > + int err;
> > +
> > + wmt_params.op = BTMTK_WMT_CBMCU_DWNLD;
> > + wmt_params.flag = 0xF1;
>
> Ditto.
>
> > + wmt_params.dlen = 0;
> > + wmt_params.data = NULL;
> > + wmt_params.status = NULL;
> > +
> > + err = wmt_cmd_sync(hdev, &wmt_params);
> > + if (err < 0) {
> > + bt_dev_err(hdev, "Failed to enable CBMCU patch (%d)",
> > err);
> > + return err;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int btmtk_load_cbmcu_firmware(struct hci_dev *hdev,
> > + const char *fwname,
> > + wmt_cmd_sync_func_t
> > wmt_cmd_sync)
> > +{
> > + struct btmtk_patch_header *hdr;
> > + struct btmtk_global_desc *globaldesc;
> > + struct btmtk_section_map *sectionmap;
> > + const struct firmware *fw;
> > + const u8 *fw_ptr;
> > + u8 *cert_buf = NULL;
> > + u32 section_num, section_offset, dl_size, cert_len;
> > + int i, err;
> > +
> > + err = request_firmware(&fw, fwname, &hdev->dev);
> > + if (err < 0) {
> > + bt_dev_err(hdev, "Failed to load CBMCU firmware file
> > %s (%d)",
> > + fwname, err);
> > + return err;
> > + }
> > +
> > + if (fw->size < MTK_FW_ROM_PATCH_HEADER_SIZE +
> > MTK_FW_ROM_PATCH_GD_SIZE) {
> > + bt_dev_err(hdev, "CBMCU firmware too small (%zu
> > bytes)", fw->size);
>
> Please log `MTK_FW_ROM_PATCH_HEADER_SIZE + MTK_FW_ROM_PATCH_GD_SIZE`.
>
> > + err = -EINVAL;
> > + goto err_release_fw;
> > + }
> > +
> > + fw_ptr = fw->data;
> > + hdr = (struct btmtk_patch_header *)fw_ptr;
> > + globaldesc = (struct btmtk_global_desc *)(fw_ptr +
> > MTK_FW_ROM_PATCH_HEADER_SIZE);
> > + section_num = le32_to_cpu(globaldesc->section_num);
> > +
> > + if (fw->size < MTK_FW_ROM_PATCH_HEADER_SIZE +
> > MTK_FW_ROM_PATCH_GD_SIZE +
> > + (size_t)MTK_FW_ROM_PATCH_SEC_MAP_SIZE *
> > section_num) {
> > + bt_dev_err(hdev, "CBMCU firmware truncated: size=%zu,
> > expected=%zu (section_num=%u)",
> > + fw->size,
> > + MTK_FW_ROM_PATCH_HEADER_SIZE +
> > MTK_FW_ROM_PATCH_GD_SIZE +
> > + (size_t)MTK_FW_ROM_PATCH_SEC_MAP_SIZE *
> > section_num,
> > + section_num);
> > + err = -EINVAL;
> > + goto err_release_fw;
> > + }
> > +
> > + bt_dev_info(hdev, "CBMCU Version: 0x%04x%04x, Build Time:
> > %s",
> > + le16_to_cpu(hdr->hwver), le16_to_cpu(hdr->swver),
> > hdr->datetime);
> > +
> > + /* Phase 1: Download section type MTK_SEC_CBMCU_DESC */
> > + for (i = 0; i < section_num; i++) {
> > + sectionmap = (struct btmtk_section_map *)
> > + (fw_ptr + MTK_FW_ROM_PATCH_HEADER_SIZE +
> > + MTK_FW_ROM_PATCH_GD_SIZE +
> > + MTK_FW_ROM_PATCH_SEC_MAP_SIZE * i);
> > +
> > + /* Only process MTK_SEC_CBMCU_DESC section in Phase 1
> > */
> > + if ((le32_to_cpu(sectionmap->sectype) & 0xFFFF) !=
> > MTK_SEC_CBMCU_DESC)
> > + continue;
> > +
> > + section_offset = le32_to_cpu(sectionmap->secoffset);
> > + dl_size = le32_to_cpu(sectionmap->secsize);
> > +
> > + if (dl_size == 0)
> > + continue;
> > +
> > + if (section_offset > fw->size ||
> > + dl_size > fw->size - section_offset) {
> > + bt_dev_err(hdev, "CBMCU Phase 1 section out
> > of bounds");
> > + err = -EINVAL;
> > + goto err_release_fw;
> > + }
> > +
> > + cert_len = MTK_FW_ROM_PATCH_GD_SIZE +
> > + MTK_FW_ROM_PATCH_SEC_MAP_SIZE *
> > section_num +
> > + dl_size;
> > +
> > + /* Query cbmcu section */
> > + err = btmtk_query_cbmcu_section(hdev, wmt_cmd_sync,
> > 0, NULL,
> > + cert_len);
> > + if (err < 0)
> > + goto err_release_fw;
> > +
> > + cert_buf = kmalloc(cert_len, GFP_KERNEL);
> > + if (!cert_buf) {
> > + err = -ENOMEM;
> > + goto err_release_fw;
> > + }
> > +
> > + /* Copy Global Descriptor + All Section Maps */
> > + memcpy(cert_buf,
> > + fw_ptr + MTK_FW_ROM_PATCH_HEADER_SIZE,
> > + MTK_FW_ROM_PATCH_GD_SIZE +
> > MTK_FW_ROM_PATCH_SEC_MAP_SIZE * section_num);
> > +
> > + /* Copy Phase 1 section data */
> > + memcpy(cert_buf + MTK_FW_ROM_PATCH_GD_SIZE +
> > + MTK_FW_ROM_PATCH_SEC_MAP_SIZE * section_num,
> > + fw_ptr + section_offset,
> > + dl_size);
> > +
> > + /* Download Phase 1 section */
> > + err = btmtk_download_cbmcu_section(hdev,
> > wmt_cmd_sync,
> > + cert_buf,
> > cert_len);
> > + kfree(cert_buf);
> > + cert_buf = NULL;
> > +
> > + if (err < 0) {
> > + bt_dev_err(hdev, "Failed to download CBMCU
> > Phase 1 section (%d)", err);
> > + goto err_release_fw;
> > + }
> > +
> > + break;
> > + }
> > +
> > + /* Phase 2: Download other sections (type !=
> > MTK_SEC_CBMCU_DESC) */
> > + for (i = 0; i < section_num; i++) {
> > + sectionmap = (struct btmtk_section_map *)
> > + (fw_ptr + MTK_FW_ROM_PATCH_HEADER_SIZE +
> > + MTK_FW_ROM_PATCH_GD_SIZE +
> > + MTK_FW_ROM_PATCH_SEC_MAP_SIZE * i);
> > +
> > + /* Skip MTK_SEC_CBMCU_DESC section in Phase 2 */
> > + if ((le32_to_cpu(sectionmap->sectype) & 0xFFFF) ==
> > MTK_SEC_CBMCU_DESC)
> > + continue;
> > +
> > + section_offset = le32_to_cpu(sectionmap->secoffset);
> > + dl_size = le32_to_cpu(sectionmap-
> > >bin_info_spec.dlsize);
> > +
> > + if (dl_size == 0)
> > + continue;
> > +
> > + if (section_offset > fw->size ||
> > + dl_size > fw->size - section_offset) {
> > + bt_dev_err(hdev, "CBMCU Phase 2 section %d
> > out of bounds", i);
> > + err = -EINVAL;
> > + goto err_release_fw;
> > + }
> > +
> > + /* Query cbmcu section */
> > + err = btmtk_query_cbmcu_section(hdev, wmt_cmd_sync,
> > 1,
> > + (u8 *)§ionmap-
> > >bin_info_spec,
> > + 0);
> > + if (err < 0)
> > + goto err_release_fw;
> > +
> > + /* Download section data */
> > + err = btmtk_download_cbmcu_section(hdev,
> > wmt_cmd_sync,
> > + fw_ptr +
> > section_offset,
> > + dl_size);
> > + if (err < 0) {
> > + bt_dev_err(hdev, "Failed to download CBMCU
> > section %d (%d)", i, err);
> > + goto err_release_fw;
> > + }
> > + }
> > +
> > + /* Wait for firmware activation */
> > + usleep_range(100000, 120000);
>
> Does the firmware really take this long? (Please report to the
> firmware
> engineers to optimize this. Optimized Linux takes less time. ;-))
> Isn’t
> there a way to poll the readiness?
>
> > +
> > + bt_dev_info(hdev, "CBMCU firmware download completed");
>
> For me, Linux uploads firmware to the device (and the BT device would
> download it). But this is not a BT device message but the OS message?
> Feel free to ignore.
>
MT7928 requires two-phase firmware loading, This message distinguishes
phase boundaries phase boundaries, helpful for debugging initialization
issues.
> > +
> > +err_release_fw:
> > + release_firmware(fw);
> > + return err;
> > +}
> > +
> > +static int btmtk_setup_cbmcu_firmware(struct hci_dev *hdev,
> > + wmt_cmd_sync_func_t
> > wmt_cmd_sync,
> > + u32 dev_id)
> > +{
> > + char cbmcu_fwname[64];
> > + u8 patch_status;
> > + int err;
> > +
> > + err = btmtk_cbmcu_patch_status(hdev, wmt_cmd_sync,
> > &patch_status);
> > + if (err < 0)
> > + return err;
> > +
> > + bt_dev_dbg(hdev, "CBMCU patch status: 0x%02x", patch_status);
> > +
> > + if (patch_status != BTMTK_WMT_PATCH_UNDONE)
> > + return 0;
> > +
> > + snprintf(cbmcu_fwname, sizeof(cbmcu_fwname),
> > + "mediatek/mt7928/CBMCU_CODE_MT%04x_1_1.bin",
> > + dev_id & 0xffff);
> > +
> > + err = btmtk_load_cbmcu_firmware(hdev, cbmcu_fwname,
> > wmt_cmd_sync);
> > + if (err < 0) {
> > + bt_dev_err(hdev, "Failed to download CBMCU firmware
> > (%d)", err);
> > + return err;
> > + }
> > +
> > + err = btmtk_enable_cbmcu_patch(hdev, wmt_cmd_sync);
> > + if (err < 0)
> > + return err;
> > +
> > + return 0;
> > +}
> > +
> > int btmtk_usb_subsys_reset(struct hci_dev *hdev, u32 dev_id)
> > {
> > u32 val;
> > @@ -894,7 +1229,7 @@ int btmtk_usb_subsys_reset(struct hci_dev
> > *hdev, u32 dev_id)
> > if (err < 0)
> > return err;
> > msleep(100);
> > - } else if (dev_id == 0x7925 || dev_id == 0x6639) {
> > + } else if (dev_id == 0x7925 || dev_id == 0x6639 || dev_id ==
> > 0x7935) {
>
> This should be sorted in my opinion. Maybe add a commit before, and
> put
> the new id at the beginning.
>
> > err = btmtk_usb_uhw_reg_read(hdev,
> > MTK_BT_RESET_REG_CONNV3, &val);
> > if (err < 0)
> > return err;
> > @@ -1379,6 +1714,15 @@ int btmtk_usb_setup(struct hci_dev *hdev)
> > case 0x7668:
> > fwname = FIRMWARE_MT7668;
> > break;
> > + case 0x7935:
> > + /* Requires CBMCU firmware before BT firmware */
> > + err = btmtk_setup_cbmcu_firmware(hdev,
> > btmtk_usb_hci_wmt_sync,
> > + dev_id);
> > + if (err < 0) {
> > + bt_dev_err(hdev, "Failed to set up CBMCU
> > firmware (%d)", err);
> > + return err;
> > + }
> > + fallthrough;
> > case 0x7922:
> > case 0x7925:
> > /*
> > @@ -1596,3 +1940,5 @@ MODULE_FIRMWARE(FIRMWARE_MT7922);
> > MODULE_FIRMWARE(FIRMWARE_MT7961);
> > MODULE_FIRMWARE(FIRMWARE_MT7925);
> > MODULE_FIRMWARE(FIRMWARE_MT7927);
> > +MODULE_FIRMWARE(FIRMWARE_MT7928);
> > +MODULE_FIRMWARE(FIRMWARE_MT7928_CBMCU);
> > diff --git a/drivers/bluetooth/btmtk.h b/drivers/bluetooth/btmtk.h
> > index c83c24897c95..6d3bf6b74a1d 100644
> > --- a/drivers/bluetooth/btmtk.h
> > +++ b/drivers/bluetooth/btmtk.h
> > @@ -9,6 +9,8 @@
> > #define FIRMWARE_MT7961
> > "mediatek/BT_RAM_CODE_MT7961_1_2_hdr.bin"
> > #define FIRMWARE_MT7925
> > "mediatek/mt7925/BT_RAM_CODE_MT7925_1_1_hdr.bin"
> > #define FIRMWARE_MT7927
> > "mediatek/mt7927/BT_RAM_CODE_MT6639_2_1_hdr.bin"
> > +#define FIRMWARE_MT7928
> > "mediatek/mt7928/BT_RAM_CODE_MT7935_1_1_hdr.bin"
> > +#define FIRMWARE_MT7928_CBMCU
> > "mediatek/mt7928/CBMCU_CODE_MT7935_1_1.bin"
> >
> > #define HCI_EV_WMT 0xe4
> > #define HCI_WMT_MAX_EVENT_SIZE 64
> > @@ -54,6 +56,7 @@ enum {
> > BTMTK_WMT_RST = 0x7,
> > BTMTK_WMT_REGISTER = 0x8,
> > BTMTK_WMT_SEMAPHORE = 0x17,
> > + BTMTK_WMT_CBMCU_DWNLD = 0x58,
> > };
> >
> > enum {
>
For the other pars that haven't been addressed in this mail, I will
make evaluation and improvement in v4, Thanks for your suggestions.
Chris Lu
^ permalink raw reply
* RE: [PATCH 3/8] Bluetooth: btnxpuart: Add M.2 Bluetooth device support using pwrseq
From: Sherry Sun @ 2026-06-22 3:51 UTC (permalink / raw)
To: Frank Li (OSS), Sherry Sun (OSS)
Cc: robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org,
Frank Li, s.hauer@pengutronix.de, kernel@pengutronix.de,
festevam@gmail.com, Amitkumar Karwar, Neeraj Sanjay Kale,
marcel@holtmann.org, luiz.dentz@gmail.com, Hongxing Zhu,
l.stach@pengutronix.de, lpieralisi@kernel.org,
kwilczynski@kernel.org, mani@kernel.org, bhelgaas@google.com,
brgl@kernel.org, imx@lists.linux.dev, linux-pci@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-bluetooth@vger.kernel.org,
linux-pm@vger.kernel.org
In-Reply-To: <ajQ4oBUNGOrhcPX5@SMW015318>
> On Thu, Jun 18, 2026 at 06:10:42PM +0800, Sherry Sun (OSS) wrote:
> > From: Sherry Sun <sherry.sun@nxp.com>
> >
> > Power supply to the M.2 Bluetooth device attached to the host using
> > M.2 connector is controlled using the 'uart' pwrseq device. So add
> > support for getting the pwrseq device if the OF graph link is present.
> > Once obtained, the existing pwrseq APIs can be used to control the
> > power supplies of the
> > M.2 card.
> >
> > Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
> > ---
> > drivers/bluetooth/btnxpuart.c | 33 ++++++++++++++++++++++++++++++---
> > 1 file changed, 30 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/bluetooth/btnxpuart.c
> > b/drivers/bluetooth/btnxpuart.c index e7036a48ce48..1aa8972f0dab
> > 100644
> > --- a/drivers/bluetooth/btnxpuart.c
> > +++ b/drivers/bluetooth/btnxpuart.c
> > @@ -9,6 +9,8 @@
> >
> > #include <linux/serdev.h>
> > #include <linux/of.h>
> > +#include <linux/of_graph.h>
> > +#include <linux/pwrseq/consumer.h>
> > #include <linux/skbuff.h>
> > #include <linux/unaligned.h>
> > #include <linux/firmware.h>
> > @@ -211,6 +213,7 @@ struct btnxpuart_dev {
> >
> > struct ps_data psdata;
> > struct btnxpuart_data *nxp_data;
> > + struct pwrseq_desc *pwrseq;
> > struct reset_control *pdn;
> > struct hci_uart hu;
> > };
> > @@ -1866,11 +1869,27 @@ static int nxp_serdev_probe(struct
> serdev_device *serdev)
> > return err;
> > }
> >
> > + if (of_graph_is_present(dev_of_node(&serdev->ctrl->dev))) {
> > + struct pwrseq_desc *pwrseq;
> > +
> > + pwrseq = devm_pwrseq_get(&serdev->ctrl->dev, "uart");
> > + if (IS_ERR(pwrseq))
> > + return PTR_ERR(pwrseq);
> > +
> > + nxpdev->pwrseq = pwrseq;
> > + err = pwrseq_power_on(pwrseq);
> > + if (err) {
> > + dev_err(&serdev->dev, "Failed to power on
> pwrseq\n");
> > + return err;
> > + }
>
> Can you provide helper function like devm clk get and enabled?
> like devm_pwrsq_get_on()
>
> So simple below error handle.
Ok, will try.
Best Regards
Sherry
>
> > + }
> > +
> > /* Initialize and register HCI device */
> > hdev = hci_alloc_dev();
> > if (!hdev) {
> > dev_err(&serdev->dev, "Can't allocate HCI device\n");
> > - return -ENOMEM;
> > + err = -ENOMEM;
> > + goto err_pwrseq_power_off;
> > }
> >
> > reset_control_deassert(nxpdev->pdn);
> > @@ -1903,11 +1922,14 @@ static int nxp_serdev_probe(struct
> > serdev_device *serdev)
> >
> > if (hci_register_dev(hdev) < 0) {
> > dev_err(&serdev->dev, "Can't register HCI device\n");
> > + err = -ENODEV;
> > goto probe_fail;
> > }
> >
> > - if (ps_setup(hdev))
> > + if (ps_setup(hdev)) {
> > + err = -ENODEV;
> > goto probe_fail;
> > + }
> >
> > hci_devcd_register(hdev, nxp_coredump, nxp_coredump_hdr,
> > nxp_coredump_notify);
> > @@ -1917,7 +1939,10 @@ static int nxp_serdev_probe(struct
> > serdev_device *serdev)
> > probe_fail:
> > reset_control_assert(nxpdev->pdn);
> > hci_free_dev(hdev);
> > - return -ENODEV;
> > +err_pwrseq_power_off:
> > + if (nxpdev->pwrseq)
> > + pwrseq_power_off(nxpdev->pwrseq);
> > + return err;
> > }
> >
> > static void nxp_serdev_remove(struct serdev_device *serdev) @@
> > -1944,6 +1969,8 @@ static void nxp_serdev_remove(struct serdev_device
> *serdev)
> > ps_cleanup(nxpdev);
> > hci_unregister_dev(hdev);
> > reset_control_assert(nxpdev->pdn);
> > + if (nxpdev->pwrseq)
> > + pwrseq_power_off(nxpdev->pwrseq);
> > hci_free_dev(hdev);
> > }
> >
> > --
> > 2.50.1
> >
> >
^ permalink raw reply
* RE: [PATCH 0/8] Add PCIe M.2 Key E connector support for NXP i.MX boards
From: Sherry Sun @ 2026-06-22 3:18 UTC (permalink / raw)
To: Bartosz Golaszewski, Sherry Sun (OSS)
Cc: imx@lists.linux.dev, linux-pci@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-bluetooth@vger.kernel.org,
linux-pm@vger.kernel.org, robh@kernel.org, krzk+dt@kernel.org,
conor+dt@kernel.org, Frank Li, s.hauer@pengutronix.de,
kernel@pengutronix.de, festevam@gmail.com, Amitkumar Karwar,
Neeraj Sanjay Kale, marcel@holtmann.org, luiz.dentz@gmail.com,
Hongxing Zhu, l.stach@pengutronix.de, lpieralisi@kernel.org,
kwilczynski@kernel.org, mani@kernel.org, bhelgaas@google.com
In-Reply-To: <CAMRc=MfsNa4itdpyGtR16wMb+wMkJwg+9=QJF2-oOoVVfFCF3g@mail.gmail.com>
> On Thu, 18 Jun 2026 12:10:39 +0200, "Sherry Sun (OSS)"
> <sherry.sun@oss.nxp.com> said:
> > From: Sherry Sun <sherry.sun@nxp.com>
> >
> > This series adds support for NXP Wi-Fi/BT combo chips (88W9098, AW693)
> > inserted into PCIe M.2 Key E connectors on several i.MX EVK/MEK boards.
> >
> > For M.2 cards that rely on PCIe L2 link state and wake-up mechanisms,
> > the card must remain powered during suspend. Patch 1 uses the existing
> > dw_pcie_rp::skip_pwrctrl_off flag to skip power-off during suspend and
> > skip power-on during the init path.
> >
> > Alsp the btnxpuart driver is extended to obtain a pwrseq descriptor
> > via the OF graph on the UART controller device in patch 2.
> >
> > Sherry Sun (8):
> > PCI: imx6: Add skip_pwrctrl_off flag support
> > power: sequencing: pcie-m2: Add PCI ID for NXP 88W9098 and AW693
> > Bluetooth
>
> Can this be applied independently without build-time issues?
Hi Bart,
Yes, this patch can be applied independently, I was able to successfully
build it based on the following base-commit:
3ce97bd3c4f18608335e709c24d6a40e7036cab8.
However, please note that it may conflict with the following patch when
applied: https://lore.kernel.org/all/20260617143055.820096-1-wei.deng@oss.qualcomm.com/.
Best Regards
Sherry
>
> > Bluetooth: btnxpuart: Add M.2 Bluetooth device support using pwrseq
> > arm64: dts: imx8mq-evk: Describe the PCIe M.2 Key E connector
> > arm64: dts: imx95-19x19-evk: Describe the PCIe M.2 Key E connector
> > arm64: dts: imx8dxl-evk: Describe the PCIe M.2 Key E connector
> > arm64: dts: imx8qm-mek: Describe the PCIe M.2 Key E connector
> > arm64: dts: imx8qxp-mek: Describe the PCIe M.2 Key E connector
> >
> > arch/arm64/boot/dts/freescale/imx8dxl-evk.dts | 56 +++++++++++++-----
> > arch/arm64/boot/dts/freescale/imx8mq-evk.dts | 44 ++++++++++++--
> > arch/arm64/boot/dts/freescale/imx8qm-mek.dts | 58 ++++++++++++++-----
> > arch/arm64/boot/dts/freescale/imx8qxp-mek.dts | 54 ++++++++++++-----
> > .../boot/dts/freescale/imx95-19x19-evk.dts | 55 +++++++++++++-----
> > drivers/bluetooth/btnxpuart.c | 33 ++++++++++-
> > drivers/pci/controller/dwc/pci-imx6.c | 36 +++++++-----
> > drivers/power/sequencing/pwrseq-pcie-m2.c | 4 ++
> > 8 files changed, 264 insertions(+), 76 deletions(-)
> >
> > --
> > 2.50.1
> >
> >
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox