[PATCH v4 0/4] Bluetooth: btusb: MediaTek ISO data transmission

[PATCH v4 0/4] Bluetooth: btusb: MediaTek ISO data transmission

[Chris Lu]

Since SIG has not yet clearly defined the specification for ISO data
transmission over USB, MediaTek has adopted a method of adding an
additional interrupt endpoint for ISO data transmission. This approach
differs from the current method used in the Bluetooth upstream driver,
which utilizes existing bulk endpoints. The interrupt endpoint provides
guaranteed bandwidth, sufficient maximum data length for ISO packets
and error checking.

Certain new functions related to USB transportation, such as urb
allocation and submission, are implemented in btusb.c to leverage
existing functions and data structures defined therein. Meanwhile,
vendor-specific functions unrelated to transportation have been moved
to btmtk.c to streamline btusb.c.

---
Change in v4:
-Fix warning report from bluez.test.bot in btmtk.c.

Change in v3:
-Refactor ISO data transmission flow, keeping the parts related to USB
transmission in btusb.c and moving the rest to btmtk.c

Change in v2:
-Fix warning report from bluez.test.bot in btusb.c, align with other
packet type to parsing dlen when getting iso data.
---

Chris Lu (4):
  Bluetooth: net: add hci_iso_hdr function for iso data
  Bluetooth: btusb: add callback function in btusb suspend/resume
  Bluetooth: btmtk: add macro to get/set/clear MediaTek defined flags
  Bluetooth: btusb: mediatek: add ISO data transmission functions

 drivers/bluetooth/btmtk.c   |  94 ++++++++++++
 drivers/bluetooth/btmtk.h   |  61 ++++++++
 drivers/bluetooth/btusb.c   | 276 ++++++++++++++++++++++++++++++++++++
 include/net/bluetooth/hci.h |   5 +
 4 files changed, 436 insertions(+)

-- 
2.18.0

[PATCH v4 1/4] Bluetooth: net: add hci_iso_hdr function for iso data

[Chris Lu]

Add function hci_iso_hdr to get skb->data when packet type is ISO.

Signed-off-by: Chris Lu <chris.lu@mediatek.com>
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
 include/net/bluetooth/hci.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index e372a88e8c3f..b9f8f91f6c7f 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -2898,6 +2898,11 @@ static inline struct hci_sco_hdr *hci_sco_hdr(const struct sk_buff *skb)
 	return (struct hci_sco_hdr *) skb->data;
 }
 
+static inline struct hci_iso_hdr *hci_iso_hdr(const struct sk_buff *skb)
+{
+	return (struct hci_iso_hdr *) skb->data;
+}
+
 /* Command opcode pack/unpack */
 #define hci_opcode_pack(ogf, ocf)	((__u16) ((ocf & 0x03ff)|(ogf << 10)))
 #define hci_opcode_ogf(op)		(op >> 10)
-- 
2.18.0

RE: Bluetooth: btusb: MediaTek ISO data transmission

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 2577 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=859492

---Test result---

Test Summary:
CheckPatch                    PASS      3.38 seconds
GitLint                       PASS      4.54 seconds
SubjectPrefix                 PASS      0.54 seconds
BuildKernel                   PASS      30.86 seconds
CheckAllWarning               PASS      33.77 seconds
CheckSparse                   PASS      39.37 seconds
CheckSmatch                   FAIL      36.34 seconds
BuildKernel32                 PASS      30.15 seconds
TestRunnerSetup               PASS      530.41 seconds
TestRunner_l2cap-tester       PASS      20.18 seconds
TestRunner_iso-tester         PASS      32.62 seconds
TestRunner_bnep-tester        PASS      4.84 seconds
TestRunner_mgmt-tester        PASS      112.07 seconds
TestRunner_rfcomm-tester      PASS      7.47 seconds
TestRunner_sco-tester         PASS      17.06 seconds
TestRunner_ioctl-tester       PASS      7.86 seconds
TestRunner_mesh-tester        FAIL      6.07 seconds
TestRunner_smp-tester         PASS      6.94 seconds
TestRunner_userchan-tester    PASS      5.02 seconds
IncrementalBuild              PASS      45.59 seconds

Details
##############################
Test: CheckSmatch - FAIL
Desc: Run smatch tool with source
Output:

Segmentation fault (core dumped)
make[4]: *** [scripts/Makefile.build:244: net/bluetooth/hci_core.o] Error 139
make[4]: *** Deleting file 'net/bluetooth/hci_core.o'
make[3]: *** [scripts/Makefile.build:485: net/bluetooth] Error 2
make[2]: *** [scripts/Makefile.build:485: net] Error 2
make[2]: *** Waiting for unfinished jobs....
Segmentation fault (core dumped)
make[4]: *** [scripts/Makefile.build:244: drivers/bluetooth/bcm203x.o] Error 139
make[4]: *** Deleting file 'drivers/bluetooth/bcm203x.o'
make[4]: *** Waiting for unfinished jobs....
make[3]: *** [scripts/Makefile.build:485: drivers/bluetooth] Error 2
make[2]: *** [scripts/Makefile.build:485: drivers] Error 2
make[1]: *** [/github/workspace/src/src/Makefile:1919: .] Error 2
make: *** [Makefile:240: __sub-make] Error 2
##############################
Test: TestRunner_mesh-tester - FAIL
Desc: Run mesh-tester with test-runner
Output:
Total: 10, Passed: 9 (90.0%), Failed: 1, Not Run: 0

Failed Test Cases
Mesh - Send cancel - 2                               Failed       0.095 seconds


---
Regards,
Linux Bluetooth

Re: [PATCH v4 1/4] Bluetooth: net: add hci_iso_hdr function for iso data

[Luiz Augusto von Dentz]

Hi Chris,

On Thu, Jun 6, 2024 at 7:43 AM Chris Lu <chris.lu@mediatek.com> wrote:
>
> Add function hci_iso_hdr to get skb->data when packet type is ISO.
>
> Signed-off-by: Chris Lu <chris.lu@mediatek.com>
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> ---
>  include/net/bluetooth/hci.h | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index e372a88e8c3f..b9f8f91f6c7f 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -2898,6 +2898,11 @@ static inline struct hci_sco_hdr *hci_sco_hdr(const struct sk_buff *skb)
>         return (struct hci_sco_hdr *) skb->data;
>  }
>
> +static inline struct hci_iso_hdr *hci_iso_hdr(const struct sk_buff *skb)
> +{
> +       return (struct hci_iso_hdr *) skb->data;
> +}

We might as well check the skb->len before attempting to cast
skb->data or better yet use skb_pull_data but I guess that is not
possible since you don't want to consume the skb?

>  /* Command opcode pack/unpack */
>  #define hci_opcode_pack(ogf, ocf)      ((__u16) ((ocf & 0x03ff)|(ogf << 10)))
>  #define hci_opcode_ogf(op)             (op >> 10)
> --
> 2.18.0
>


-- 
Luiz Augusto von Dentz

[PATCH v4 2/4] Bluetooth: btusb: add callback function in btusb suspend/resume

[Chris Lu]

Add new function: dev_suspend/dev_resume in btusb_data which are reserved
for vendor specific usage during suspend/resume. dev_suspend will be
added before stop traffic in btusb_suspend and dev_resume will be
added after resubmit urb in btusb_resuem.

Signed-off-by: Chris Lu <chris.lu@mediatek.com>
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
 drivers/bluetooth/btusb.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 79aefdb3324d..83765c0c14b4 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -891,6 +891,9 @@ struct btusb_data {
 
 	int (*setup_on_usb)(struct hci_dev *hdev);
 
+	int (*dev_suspend)(struct hci_dev *hdev);
+	int (*dev_resume)(struct hci_dev *hdev);
+
 	int oob_wake_irq;   /* irq for out-of-band wake-on-bt */
 	unsigned cmd_timeout_cnt;
 
@@ -4715,6 +4718,9 @@ static int btusb_suspend(struct usb_interface *intf, pm_message_t message)
 
 	cancel_work_sync(&data->work);
 
+	if (data->dev_suspend)
+		data->dev_suspend(data->hdev);
+
 	btusb_stop_traffic(data);
 	usb_kill_anchored_urbs(&data->tx_anchor);
 
@@ -4818,6 +4824,9 @@ static int btusb_resume(struct usb_interface *intf)
 			btusb_submit_isoc_urb(hdev, GFP_NOIO);
 	}
 
+	if (data->dev_resume)
+		data->dev_resume(hdev);
+
 	spin_lock_irq(&data->txlock);
 	play_deferred(data);
 	clear_bit(BTUSB_SUSPENDING, &data->flags);
-- 
2.18.0

Re: [PATCH v4 2/4] Bluetooth: btusb: add callback function in btusb suspend/resume

[Luiz Augusto von Dentz]

Hi Chris,

On Thu, Jun 6, 2024 at 7:43 AM Chris Lu <chris.lu@mediatek.com> wrote:
>
> Add new function: dev_suspend/dev_resume in btusb_data which are reserved
> for vendor specific usage during suspend/resume. dev_suspend will be
> added before stop traffic in btusb_suspend and dev_resume will be
> added after resubmit urb in btusb_resuem.
>
> Signed-off-by: Chris Lu <chris.lu@mediatek.com>
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> ---
>  drivers/bluetooth/btusb.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 79aefdb3324d..83765c0c14b4 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -891,6 +891,9 @@ struct btusb_data {
>
>         int (*setup_on_usb)(struct hci_dev *hdev);
>
> +       int (*dev_suspend)(struct hci_dev *hdev);
> +       int (*dev_resume)(struct hci_dev *hdev);

I'd name these suspend, resume respectively.

>         int oob_wake_irq;   /* irq for out-of-band wake-on-bt */
>         unsigned cmd_timeout_cnt;
>
> @@ -4715,6 +4718,9 @@ static int btusb_suspend(struct usb_interface *intf, pm_message_t message)
>
>         cancel_work_sync(&data->work);
>
> +       if (data->dev_suspend)
> +               data->dev_suspend(data->hdev);
> +
>         btusb_stop_traffic(data);
>         usb_kill_anchored_urbs(&data->tx_anchor);
>
> @@ -4818,6 +4824,9 @@ static int btusb_resume(struct usb_interface *intf)
>                         btusb_submit_isoc_urb(hdev, GFP_NOIO);
>         }
>
> +       if (data->dev_resume)
> +               data->dev_resume(hdev);
> +
>         spin_lock_irq(&data->txlock);
>         play_deferred(data);
>         clear_bit(BTUSB_SUSPENDING, &data->flags);
> --
> 2.18.0
>


-- 
Luiz Augusto von Dentz

[PATCH v4 3/4] Bluetooth: btmtk: add macro to get/set/clear MediaTek defined flags

[Chris Lu]

Define a enumeration to store MediaTek specific flags and
macro function to set/test/clear the flags in data structure.

Signed-off-by: Chris Lu <chris.lu@mediatek.com>
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
 drivers/bluetooth/btmtk.h | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/bluetooth/btmtk.h b/drivers/bluetooth/btmtk.h
index e76b8a358be8..6a0697a22b16 100644
--- a/drivers/bluetooth/btmtk.h
+++ b/drivers/bluetooth/btmtk.h
@@ -128,6 +128,10 @@ struct btmtk_hci_wmt_params {
 
 typedef int (*btmtk_reset_sync_func_t)(struct hci_dev *, void *);
 
+enum {
+	__BTMTK_NUM_FLAGS,
+};
+
 struct btmtk_coredump_info {
 	const char *driver_name;
 	u32 fw_version;
@@ -136,11 +140,30 @@ struct btmtk_coredump_info {
 };
 
 struct btmediatek_data {
+	DECLARE_BITMAP(flags, __BTMTK_NUM_FLAGS);
+
 	u32 dev_id;
 	btmtk_reset_sync_func_t reset_sync;
 	struct btmtk_coredump_info cd_info;
 };
 
+#define btmtk_set_flag(hdev, nr)						\
+	do {									\
+		struct btmediatek_data *mediatek = hci_get_priv((hdev));	\
+		set_bit((nr), mediatek->flags);					\
+	} while (0)
+
+#define btmtk_clear_flag(hdev, nr)						\
+	do {									\
+		struct btmediatek_data *mediatek = hci_get_priv((hdev));	\
+		clear_bit((nr), mediatek->flags);				\
+	} while (0)
+
+#define btmtk_get_flag(hdev)							\
+	(((struct btmediatek_data *)hci_get_priv(hdev))->flags)
+
+#define btmtk_test_flag(hdev, nr)	test_bit((nr), btmtk_get_flag(hdev))
+
 typedef int (*wmt_cmd_sync_func_t)(struct hci_dev *,
 				   struct btmtk_hci_wmt_params *);
 
-- 
2.18.0

[PATCH v4 4/4] Bluetooth: btusb: mediatek: add ISO data transmission functions

[Chris Lu]

This patch implements functions for ISO data send and receive in btusb
driver for MediaTek's controller.

MediaTek defines a specific interrupt endpoint for ISO data transmissin
because the characteristics of interrupt endpoint are similar to the
application of ISO data which can support guaranteed transmissin
bandwidth, enough maximum data length and error checking mechanism.

Driver sets up ISO interface and endpoints in btusb_mtk_setup and clears
the setup in btusb_mtk_shutdown. ISO packet anchor stops when driver
suspend and resubmit iso urb in driver resume.

Signed-off-by: Chris Lu <chris.lu@mediatek.com>
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
 drivers/bluetooth/btmtk.c |  94 ++++++++++++++
 drivers/bluetooth/btmtk.h |  38 ++++++
 drivers/bluetooth/btusb.c | 267 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 399 insertions(+)

diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c
index a27c251bf56e..07d27724d915 100644
--- a/drivers/bluetooth/btmtk.c
+++ b/drivers/bluetooth/btmtk.c
@@ -19,6 +19,9 @@
 #define MTK_SEC_MAP_COMMON_SIZE	12
 #define MTK_SEC_MAP_NEED_SEND_SIZE	52
 
+/* It is for mt79xx iso data transmission setting */
+#define MTK_ISO_THRESHOLD	264
+
 struct btmtk_patch_header {
 	u8 datetime[16];
 	u8 platform[4];
@@ -431,6 +434,97 @@ int btmtk_process_coredump(struct hci_dev *hdev, struct sk_buff *skb)
 }
 EXPORT_SYMBOL_GPL(btmtk_process_coredump);
 
+int btmtk_isointf_setup(struct hci_dev *hdev)
+{
+	u8 iso_param[2] = { 0x08, 0x01 };
+	struct sk_buff *skb;
+
+	skb = __hci_cmd_sync(hdev, 0xfd98, sizeof(iso_param), iso_param,
+			     HCI_INIT_TIMEOUT);
+	if (IS_ERR(skb)) {
+		bt_dev_err(hdev, "Failed to apply iso setting (%ld)", PTR_ERR(skb));
+		return PTR_ERR(skb);
+	}
+	kfree_skb(skb);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(btmtk_isointf_setup);
+
+int btmtk_isopkt_pad(struct hci_dev *hdev, struct sk_buff *skb)
+{
+	if (skb->len > MTK_ISO_THRESHOLD)
+		return -EINVAL;
+
+	if (skb_pad(skb, MTK_ISO_THRESHOLD - skb->len))
+		return -ENOMEM;
+
+	__skb_put(skb, MTK_ISO_THRESHOLD - skb->len);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(btmtk_isopkt_pad);
+
+int btmtk_recv_isopkt(struct hci_dev *hdev, void *buffer, int count)
+{
+	struct btmediatek_data *btmtk_data = hci_get_priv(hdev);
+	struct sk_buff *skb;
+	unsigned long flags;
+	int err = 0;
+
+	spin_lock_irqsave(&btmtk_data->isopkt_info.isorxlock, flags);
+	skb = btmtk_data->isopkt_info.isopkt_skb;
+
+	while (count) {
+		int len;
+
+		if (!skb) {
+			skb = bt_skb_alloc(HCI_MAX_ISO_SIZE, GFP_ATOMIC);
+			if (!skb) {
+				err = -ENOMEM;
+				break;
+			}
+
+			hci_skb_pkt_type(skb) = HCI_ISODATA_PKT;
+			hci_skb_expect(skb) = HCI_ISO_HDR_SIZE;
+		}
+
+		len = min_t(uint, hci_skb_expect(skb), count);
+		skb_put_data(skb, buffer, len);
+
+		count -= len;
+		buffer += len;
+		hci_skb_expect(skb) -= len;
+
+		if (skb->len == HCI_ISO_HDR_SIZE) {
+			__le16 dlen = hci_iso_hdr(skb)->dlen;
+
+			/* Complete ISO header */
+			hci_skb_expect(skb) = __le16_to_cpu(dlen);
+
+			if (skb_tailroom(skb) < hci_skb_expect(skb)) {
+				kfree_skb(skb);
+				skb = NULL;
+
+				err = -EILSEQ;
+				break;
+			}
+		}
+
+		if (!hci_skb_expect(skb)) {
+			/* Complete frame */
+			hci_recv_frame(hdev, skb);
+			skb = NULL;
+		}
+	}
+
+	btmtk_data->isopkt_info.isopkt_skb = skb;
+	spin_unlock_irqrestore(&btmtk_data->isopkt_info.isorxlock, flags);
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(btmtk_recv_isopkt);
+
 MODULE_AUTHOR("Sean Wang <sean.wang@mediatek.com>");
 MODULE_AUTHOR("Mark Chen <mark-yw.chen@mediatek.com>");
 MODULE_DESCRIPTION("Bluetooth support for MediaTek devices ver " VERSION);
diff --git a/drivers/bluetooth/btmtk.h b/drivers/bluetooth/btmtk.h
index 6a0697a22b16..cb9dc4ccc266 100644
--- a/drivers/bluetooth/btmtk.h
+++ b/drivers/bluetooth/btmtk.h
@@ -1,6 +1,8 @@
 /* SPDX-License-Identifier: ISC */
 /* Copyright (C) 2021 MediaTek Inc. */
 
+#include <linux/usb.h>
+
 #define FIRMWARE_MT7622		"mediatek/mt7622pr2h.bin"
 #define FIRMWARE_MT7663		"mediatek/mt7663pr2h.bin"
 #define FIRMWARE_MT7668		"mediatek/mt7668pr2h.bin"
@@ -129,6 +131,9 @@ struct btmtk_hci_wmt_params {
 typedef int (*btmtk_reset_sync_func_t)(struct hci_dev *, void *);
 
 enum {
+	BTMTK_ISOPKT_OVER_INTR,
+	BTMTK_ISOPKT_RUNNING,
+
 	__BTMTK_NUM_FLAGS,
 };
 
@@ -139,12 +144,24 @@ struct btmtk_coredump_info {
 	int state;
 };
 
+struct btmtk_isopkt_info {
+	struct usb_endpoint_descriptor *isopkt_tx_ep;
+	struct usb_endpoint_descriptor *isopkt_rx_ep;
+	struct usb_interface *isopkt_intf;
+	struct usb_anchor isopkt_anchor;
+	struct sk_buff *isopkt_skb;
+
+	/* spinlock for ISO data transmission */
+	spinlock_t isorxlock;
+};
+
 struct btmediatek_data {
 	DECLARE_BITMAP(flags, __BTMTK_NUM_FLAGS);
 
 	u32 dev_id;
 	btmtk_reset_sync_func_t reset_sync;
 	struct btmtk_coredump_info cd_info;
+	struct btmtk_isopkt_info isopkt_info;
 };
 
 #define btmtk_set_flag(hdev, nr)						\
@@ -186,6 +203,12 @@ int btmtk_process_coredump(struct hci_dev *hdev, struct sk_buff *skb);
 
 void btmtk_fw_get_filename(char *buf, size_t size, u32 dev_id, u32 fw_ver,
 			   u32 fw_flavor);
+
+int btmtk_isointf_setup(struct hci_dev *hdev);
+
+int btmtk_isopkt_pad(struct hci_dev *hdev, struct sk_buff *skb);
+
+int btmtk_recv_isopkt(struct hci_dev *hdev, void *buffer, int count);
 #else
 
 static inline int btmtk_set_bdaddr(struct hci_dev *hdev,
@@ -225,4 +248,19 @@ static void btmtk_fw_get_filename(char *buf, size_t size, u32 dev_id,
 				  u32 fw_ver, u32 fw_flavor)
 {
 }
+
+static int btmtk_isointf_setup(struct hci_dev *hdev)
+{
+	return -EOPNOTSUPP;
+}
+
+static int btmtk_isopkt_pad(struct hci_dev *hdev, struct sk_buff *skb)
+{
+	return ERR_PTR(-EOPNOTSUPP);
+}
+
+static int btmtk_recv_isopkt(struct hci_dev *hdev, void *buffer, int count)
+{
+	return -EOPNOTSUPP;
+}
 #endif
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 83765c0c14b4..e297dcb6a8e2 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -2653,6 +2653,8 @@ static int btusb_recv_event_realtek(struct hci_dev *hdev, struct sk_buff *skb)
 #define MTK_BT_RESET_REG_CONNV3	0x70028610
 #define MTK_BT_READ_DEV_ID	0x70010200
 
+/* MediaTek ISO Interface */
+#define MTK_ISO_IFNUM		2
 
 static void btusb_mtk_wmt_recv(struct urb *urb)
 {
@@ -3129,6 +3131,239 @@ static int btusb_mtk_reset(struct hci_dev *hdev, void *rst_data)
 	return err;
 }
 
+static void btusb_mtk_intr_complete(struct urb *urb)
+{
+	struct hci_dev *hdev = urb->context;
+	struct btmediatek_data *btmtk_data = hci_get_priv(hdev);
+	struct btusb_data *data = hci_get_drvdata(hdev);
+	int err;
+
+	BT_DBG("%s urb %p status %d count %d", hdev->name, urb, urb->status,
+	       urb->actual_length);
+
+	if (!test_bit(HCI_RUNNING, &hdev->flags))
+		return;
+
+	if (urb->status == 0) {
+		hdev->stat.byte_rx += urb->actual_length;
+
+		if (btmtk_recv_isopkt(hdev, urb->transfer_buffer,
+				      urb->actual_length) < 0) {
+			bt_dev_err(hdev, "corrupted iso packet");
+			hdev->stat.err_rx++;
+		}
+	} else if (urb->status == -ENOENT) {
+		/* Avoid suspend failed when usb_kill_urb */
+		return;
+	}
+
+	if (!btmtk_test_flag(hdev, BTMTK_ISOPKT_RUNNING))
+		return;
+
+	usb_mark_last_busy(data->udev);
+	usb_anchor_urb(urb, &btmtk_data->isopkt_info.isopkt_anchor);
+
+	err = usb_submit_urb(urb, GFP_ATOMIC);
+	if (err < 0) {
+		/* -EPERM: urb is being killed;
+		 * -ENODEV: device got disconnected
+		 */
+		if (err != -EPERM && err != -ENODEV)
+			bt_dev_err(hdev, "urb %p failed to resubmit (%d)",
+				   urb, -err);
+		if (err != -EPERM)
+			hci_cmd_sync_cancel(hdev, -err);
+		usb_unanchor_urb(urb);
+	}
+}
+
+static int btusb_mtk_submit_intr_urb(struct hci_dev *hdev, gfp_t mem_flags)
+{
+	struct btmediatek_data *btmtk_data = hci_get_priv(hdev);
+	struct btusb_data *data = hci_get_drvdata(hdev);
+	unsigned char *buf;
+	unsigned int pipe;
+	struct urb *urb;
+	int err, size;
+
+	BT_DBG("%s", hdev->name);
+
+	if (!btmtk_data->isopkt_info.isopkt_rx_ep)
+		return -ENODEV;
+
+	urb = usb_alloc_urb(0, mem_flags);
+	if (!urb)
+		return -ENOMEM;
+	size = le16_to_cpu(btmtk_data->isopkt_info.isopkt_rx_ep->wMaxPacketSize);
+
+	buf = kmalloc(size, mem_flags);
+	if (!buf) {
+		usb_free_urb(urb);
+		return -ENOMEM;
+	}
+
+	pipe = usb_rcvintpipe(data->udev,
+			      btmtk_data->isopkt_info.isopkt_rx_ep->bEndpointAddress);
+
+	usb_fill_int_urb(urb, data->udev, pipe, buf, size,
+			 btusb_mtk_intr_complete, hdev,
+			 btmtk_data->isopkt_info.isopkt_rx_ep->bInterval);
+
+	urb->transfer_flags |= URB_FREE_BUFFER;
+
+	usb_mark_last_busy(data->udev);
+	usb_anchor_urb(urb, &btmtk_data->isopkt_info.isopkt_anchor);
+
+	err = usb_submit_urb(urb, mem_flags);
+	if (err < 0) {
+		if (err != -EPERM && err != -ENODEV)
+			bt_dev_err(hdev, "urb %p submission failed (%d)",
+				   urb, -err);
+		usb_unanchor_urb(urb);
+	}
+
+	usb_free_urb(urb);
+
+	return err;
+}
+
+static inline int __set_mtk_intr_interface(struct hci_dev *hdev, unsigned int ifnum)
+{
+	struct btusb_data *data = hci_get_drvdata(hdev);
+	struct btmediatek_data *btmtk_data = hci_get_priv(hdev);
+	struct usb_interface *intf = btmtk_data->isopkt_info.isopkt_intf;
+	int i, err;
+
+	if (!btmtk_data->isopkt_info.isopkt_intf)
+		return -ENODEV;
+
+	err = usb_set_interface(data->udev, ifnum, 1);
+	if (err < 0) {
+		bt_dev_err(hdev, "setting interface failed (%d)", -err);
+		return err;
+	}
+
+	btmtk_data->isopkt_info.isopkt_tx_ep = NULL;
+	btmtk_data->isopkt_info.isopkt_rx_ep = NULL;
+
+	for (i = 0; i < intf->cur_altsetting->desc.bNumEndpoints; i++) {
+		struct usb_endpoint_descriptor *ep_desc;
+
+		ep_desc = &intf->cur_altsetting->endpoint[i].desc;
+
+		if (!btmtk_data->isopkt_info.isopkt_tx_ep &&
+		    usb_endpoint_is_int_out(ep_desc)) {
+			btmtk_data->isopkt_info.isopkt_tx_ep = ep_desc;
+			continue;
+		}
+
+		if (!btmtk_data->isopkt_info.isopkt_rx_ep &&
+		    usb_endpoint_is_int_in(ep_desc)) {
+			btmtk_data->isopkt_info.isopkt_rx_ep = ep_desc;
+			continue;
+		}
+	}
+
+	if (!btmtk_data->isopkt_info.isopkt_tx_ep ||
+	    !btmtk_data->isopkt_info.isopkt_rx_ep) {
+		bt_dev_err(hdev, "invalid interrupt descriptors");
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
+static int btusb_mtk_isopkt_stop(struct hci_dev *hdev)
+{
+	struct btmediatek_data *btmtk_data = hci_get_priv(hdev);
+
+	usb_kill_anchored_urbs(&btmtk_data->isopkt_info.isopkt_anchor);
+
+	return 0;
+}
+
+static int btusb_mtk_isopkt_start(struct hci_dev *hdev)
+{
+	if (btmtk_test_flag(hdev, BTMTK_ISOPKT_RUNNING)) {
+		if (btusb_mtk_submit_intr_urb(hdev, GFP_NOIO) < 0)
+			btmtk_clear_flag(hdev, BTMTK_ISOPKT_RUNNING);
+	}
+
+	return 0;
+}
+
+static int btusb_mtk_claim_iso_intf(struct btusb_data *data, struct usb_interface *intf)
+{
+	struct btmediatek_data *btmtk_data = hci_get_priv(data->hdev);
+	int err;
+
+	err = usb_driver_claim_interface(&btusb_driver, intf, data);
+	if (err < 0)
+		return err;
+
+	init_usb_anchor(&btmtk_data->isopkt_info.isopkt_anchor);
+	spin_lock_init(&btmtk_data->isopkt_info.isorxlock);
+
+	__set_mtk_intr_interface(data->hdev, MTK_ISO_IFNUM);
+
+	err = btusb_mtk_submit_intr_urb(data->hdev, GFP_KERNEL);
+	if (err < 0) {
+		btusb_mtk_isopkt_stop(data->hdev);
+		bt_dev_err(data->hdev, "ISO intf not support (%d)", err);
+		return err;
+	}
+
+	btmtk_set_flag(data->hdev, BTMTK_ISOPKT_OVER_INTR);
+
+	return 0;
+}
+
+static struct urb *alloc_mtk_intr_urb(struct hci_dev *hdev, struct sk_buff *skb)
+{
+	struct btusb_data *data = hci_get_drvdata(hdev);
+	struct btmediatek_data *btmtk_data = hci_get_priv(hdev);
+	unsigned int pipe;
+	struct urb *urb;
+
+	if (!btmtk_data->isopkt_info.isopkt_tx_ep)
+		return ERR_PTR(-ENODEV);
+
+	urb = usb_alloc_urb(0, GFP_KERNEL);
+	if (!urb)
+		return ERR_PTR(-ENOMEM);
+
+	if (btmtk_isopkt_pad(hdev, skb))
+		return ERR_PTR(-EINVAL);
+
+	pipe = usb_sndintpipe(data->udev,
+			      btmtk_data->isopkt_info.isopkt_tx_ep->bEndpointAddress);
+
+	usb_fill_int_urb(urb, data->udev, pipe,
+			 skb->data, skb->len, btusb_tx_complete,
+			 skb, btmtk_data->isopkt_info.isopkt_tx_ep->bInterval);
+
+	skb->dev = (void *)hdev;
+
+	return urb;
+}
+
+static int btusb_send_frame_mtk(struct hci_dev *hdev, struct sk_buff *skb)
+{
+	struct urb *urb;
+
+	BT_DBG("%s", hdev->name);
+
+	if (hci_skb_pkt_type(skb) == HCI_ISODATA_PKT) {
+		urb = alloc_mtk_intr_urb(hdev, skb);
+		if (IS_ERR(urb))
+			return PTR_ERR(urb);
+
+		return submit_or_queue_tx_urb(hdev, urb);
+	} else {
+		return btusb_send_frame(hdev, skb);
+	}
+}
+
 static int btusb_mtk_setup(struct hci_dev *hdev)
 {
 	struct btusb_data *data = hci_get_drvdata(hdev);
@@ -3213,6 +3448,12 @@ static int btusb_mtk_setup(struct hci_dev *hdev)
 		/* It's Device EndPoint Reset Option Register */
 		btusb_mtk_uhw_reg_write(data, MTK_EP_RST_OPT, MTK_EP_RST_IN_OUT_OPT);
 
+		/* Claim ISO data interface and endpoint */
+		mediatek->isopkt_info.isopkt_intf = usb_ifnum_to_if(data->udev, MTK_ISO_IFNUM);
+		err = btusb_mtk_claim_iso_intf(data, mediatek->isopkt_info.isopkt_intf);
+		if (err < 0)
+			mediatek->isopkt_info.isopkt_intf = NULL;
+
 		/* Enable Bluetooth protocol */
 		param = 1;
 		wmt_params.op = BTMTK_WMT_FUNC_CTRL;
@@ -3229,6 +3470,13 @@ static int btusb_mtk_setup(struct hci_dev *hdev)
 
 		hci_set_msft_opcode(hdev, 0xFD30);
 		hci_set_aosp_capable(hdev);
+
+		/* Set up ISO interface after protocol enabled */
+		if (btmtk_test_flag(hdev, BTMTK_ISOPKT_OVER_INTR)) {
+			btmtk_isointf_setup(hdev);
+			btmtk_set_flag(data->hdev, BTMTK_ISOPKT_RUNNING);
+		}
+
 		goto done;
 	default:
 		bt_dev_err(hdev, "Unsupported hardware variant (%08x)",
@@ -3321,6 +3569,7 @@ static int btusb_mtk_setup(struct hci_dev *hdev)
 static int btusb_mtk_shutdown(struct hci_dev *hdev)
 {
 	struct btmtk_hci_wmt_params wmt_params;
+	struct btmediatek_data *btmtk_data = hci_get_priv(hdev);
 	u8 param = 0;
 	int err;
 
@@ -3337,6 +3586,21 @@ static int btusb_mtk_shutdown(struct hci_dev *hdev)
 		return err;
 	}
 
+	if (btmtk_test_flag(hdev, BTMTK_ISOPKT_RUNNING)) {
+		btusb_mtk_isopkt_stop(hdev);
+		btmtk_clear_flag(hdev, BTMTK_ISOPKT_RUNNING);
+
+		if (btmtk_data->isopkt_info.isopkt_intf) {
+			usb_set_intfdata(btmtk_data->isopkt_info.isopkt_intf, NULL);
+			usb_driver_release_interface(&btusb_driver,
+						     btmtk_data->isopkt_info.isopkt_intf);
+			dev_kfree_skb_irq(btmtk_data->isopkt_info.isopkt_skb);
+			btmtk_data->isopkt_info.isopkt_skb = NULL;
+
+			btmtk_clear_flag(hdev, BTMTK_ISOPKT_OVER_INTR);
+		}
+	}
+
 	return 0;
 }
 
@@ -4475,9 +4739,12 @@ static int btusb_probe(struct usb_interface *intf,
 		hdev->manufacturer = 70;
 		hdev->cmd_timeout = btmtk_reset_sync;
 		hdev->set_bdaddr = btmtk_set_bdaddr;
+		hdev->send = btusb_send_frame_mtk;
 		set_bit(HCI_QUIRK_BROKEN_ENHANCED_SETUP_SYNC_CONN, &hdev->quirks);
 		set_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks);
 		data->recv_acl = btusb_recv_acl_mtk;
+		data->dev_suspend = btusb_mtk_isopkt_stop;
+		data->dev_resume = btusb_mtk_isopkt_start;
 	}
 
 	if (id->driver_info & BTUSB_SWAVE) {
-- 
2.18.0

Re: [PATCH v4 4/4] Bluetooth: btusb: mediatek: add ISO data transmission functions

[Luiz Augusto von Dentz]

Hi Chris,

On Thu, Jun 6, 2024 at 7:43 AM Chris Lu <chris.lu@mediatek.com> wrote:
>
> This patch implements functions for ISO data send and receive in btusb
> driver for MediaTek's controller.
>
> MediaTek defines a specific interrupt endpoint for ISO data transmissin
> because the characteristics of interrupt endpoint are similar to the
> application of ISO data which can support guaranteed transmissin
> bandwidth, enough maximum data length and error checking mechanism.
>
> Driver sets up ISO interface and endpoints in btusb_mtk_setup and clears
> the setup in btusb_mtk_shutdown. ISO packet anchor stops when driver
> suspend and resubmit iso urb in driver resume.
>
> Signed-off-by: Chris Lu <chris.lu@mediatek.com>
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> ---
>  drivers/bluetooth/btmtk.c |  94 ++++++++++++++
>  drivers/bluetooth/btmtk.h |  38 ++++++
>  drivers/bluetooth/btusb.c | 267 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 399 insertions(+)
>
> diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c
> index a27c251bf56e..07d27724d915 100644
> --- a/drivers/bluetooth/btmtk.c
> +++ b/drivers/bluetooth/btmtk.c
> @@ -19,6 +19,9 @@
>  #define MTK_SEC_MAP_COMMON_SIZE        12
>  #define MTK_SEC_MAP_NEED_SEND_SIZE     52
>
> +/* It is for mt79xx iso data transmission setting */
> +#define MTK_ISO_THRESHOLD      264
> +
>  struct btmtk_patch_header {
>         u8 datetime[16];
>         u8 platform[4];
> @@ -431,6 +434,97 @@ int btmtk_process_coredump(struct hci_dev *hdev, struct sk_buff *skb)
>  }
>  EXPORT_SYMBOL_GPL(btmtk_process_coredump);
>
> +int btmtk_isointf_setup(struct hci_dev *hdev)
> +{
> +       u8 iso_param[2] = { 0x08, 0x01 };
> +       struct sk_buff *skb;
> +
> +       skb = __hci_cmd_sync(hdev, 0xfd98, sizeof(iso_param), iso_param,
> +                            HCI_INIT_TIMEOUT);
> +       if (IS_ERR(skb)) {
> +               bt_dev_err(hdev, "Failed to apply iso setting (%ld)", PTR_ERR(skb));
> +               return PTR_ERR(skb);
> +       }
> +       kfree_skb(skb);
> +
> +       return 0;
> +}
> +EXPORT_SYMBOL_GPL(btmtk_isointf_setup);
> +
> +int btmtk_isopkt_pad(struct hci_dev *hdev, struct sk_buff *skb)
> +{
> +       if (skb->len > MTK_ISO_THRESHOLD)
> +               return -EINVAL;
> +
> +       if (skb_pad(skb, MTK_ISO_THRESHOLD - skb->len))
> +               return -ENOMEM;
> +
> +       __skb_put(skb, MTK_ISO_THRESHOLD - skb->len);
> +
> +       return 0;
> +}
> +EXPORT_SYMBOL_GPL(btmtk_isopkt_pad);
> +
> +int btmtk_recv_isopkt(struct hci_dev *hdev, void *buffer, int count)
> +{
> +       struct btmediatek_data *btmtk_data = hci_get_priv(hdev);
> +       struct sk_buff *skb;
> +       unsigned long flags;
> +       int err = 0;
> +
> +       spin_lock_irqsave(&btmtk_data->isopkt_info.isorxlock, flags);
> +       skb = btmtk_data->isopkt_info.isopkt_skb;
> +
> +       while (count) {
> +               int len;
> +
> +               if (!skb) {
> +                       skb = bt_skb_alloc(HCI_MAX_ISO_SIZE, GFP_ATOMIC);
> +                       if (!skb) {
> +                               err = -ENOMEM;
> +                               break;
> +                       }
> +
> +                       hci_skb_pkt_type(skb) = HCI_ISODATA_PKT;
> +                       hci_skb_expect(skb) = HCI_ISO_HDR_SIZE;
> +               }
> +
> +               len = min_t(uint, hci_skb_expect(skb), count);
> +               skb_put_data(skb, buffer, len);
> +
> +               count -= len;
> +               buffer += len;
> +               hci_skb_expect(skb) -= len;
> +
> +               if (skb->len == HCI_ISO_HDR_SIZE) {
> +                       __le16 dlen = hci_iso_hdr(skb)->dlen;
> +
> +                       /* Complete ISO header */
> +                       hci_skb_expect(skb) = __le16_to_cpu(dlen);
> +
> +                       if (skb_tailroom(skb) < hci_skb_expect(skb)) {
> +                               kfree_skb(skb);
> +                               skb = NULL;
> +
> +                               err = -EILSEQ;
> +                               break;
> +                       }
> +               }
> +
> +               if (!hci_skb_expect(skb)) {
> +                       /* Complete frame */
> +                       hci_recv_frame(hdev, skb);
> +                       skb = NULL;
> +               }
> +       }
> +
> +       btmtk_data->isopkt_info.isopkt_skb = skb;
> +       spin_unlock_irqrestore(&btmtk_data->isopkt_info.isorxlock, flags);
> +
> +       return err;
> +}
> +EXPORT_SYMBOL_GPL(btmtk_recv_isopkt);
> +
>  MODULE_AUTHOR("Sean Wang <sean.wang@mediatek.com>");
>  MODULE_AUTHOR("Mark Chen <mark-yw.chen@mediatek.com>");
>  MODULE_DESCRIPTION("Bluetooth support for MediaTek devices ver " VERSION);
> diff --git a/drivers/bluetooth/btmtk.h b/drivers/bluetooth/btmtk.h
> index 6a0697a22b16..cb9dc4ccc266 100644
> --- a/drivers/bluetooth/btmtk.h
> +++ b/drivers/bluetooth/btmtk.h
> @@ -1,6 +1,8 @@
>  /* SPDX-License-Identifier: ISC */
>  /* Copyright (C) 2021 MediaTek Inc. */
>
> +#include <linux/usb.h>
> +
>  #define FIRMWARE_MT7622                "mediatek/mt7622pr2h.bin"
>  #define FIRMWARE_MT7663                "mediatek/mt7663pr2h.bin"
>  #define FIRMWARE_MT7668                "mediatek/mt7668pr2h.bin"
> @@ -129,6 +131,9 @@ struct btmtk_hci_wmt_params {
>  typedef int (*btmtk_reset_sync_func_t)(struct hci_dev *, void *);
>
>  enum {
> +       BTMTK_ISOPKT_OVER_INTR,
> +       BTMTK_ISOPKT_RUNNING,
> +
>         __BTMTK_NUM_FLAGS,
>  };
>
> @@ -139,12 +144,24 @@ struct btmtk_coredump_info {
>         int state;
>  };
>
> +struct btmtk_isopkt_info {
> +       struct usb_endpoint_descriptor *isopkt_tx_ep;
> +       struct usb_endpoint_descriptor *isopkt_rx_ep;
> +       struct usb_interface *isopkt_intf;
> +       struct usb_anchor isopkt_anchor;
> +       struct sk_buff *isopkt_skb;
> +
> +       /* spinlock for ISO data transmission */
> +       spinlock_t isorxlock;
> +};
> +
>  struct btmediatek_data {
>         DECLARE_BITMAP(flags, __BTMTK_NUM_FLAGS);
>
>         u32 dev_id;
>         btmtk_reset_sync_func_t reset_sync;
>         struct btmtk_coredump_info cd_info;
> +       struct btmtk_isopkt_info isopkt_info;
>  };
>
>  #define btmtk_set_flag(hdev, nr)                                               \
> @@ -186,6 +203,12 @@ int btmtk_process_coredump(struct hci_dev *hdev, struct sk_buff *skb);
>
>  void btmtk_fw_get_filename(char *buf, size_t size, u32 dev_id, u32 fw_ver,
>                            u32 fw_flavor);
> +
> +int btmtk_isointf_setup(struct hci_dev *hdev);
> +
> +int btmtk_isopkt_pad(struct hci_dev *hdev, struct sk_buff *skb);
> +
> +int btmtk_recv_isopkt(struct hci_dev *hdev, void *buffer, int count);
>  #else
>
>  static inline int btmtk_set_bdaddr(struct hci_dev *hdev,
> @@ -225,4 +248,19 @@ static void btmtk_fw_get_filename(char *buf, size_t size, u32 dev_id,
>                                   u32 fw_ver, u32 fw_flavor)
>  {
>  }
> +
> +static int btmtk_isointf_setup(struct hci_dev *hdev)
> +{
> +       return -EOPNOTSUPP;
> +}
> +
> +static int btmtk_isopkt_pad(struct hci_dev *hdev, struct sk_buff *skb)
> +{
> +       return ERR_PTR(-EOPNOTSUPP);
> +}
> +
> +static int btmtk_recv_isopkt(struct hci_dev *hdev, void *buffer, int count)
> +{
> +       return -EOPNOTSUPP;
> +}
>  #endif
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 83765c0c14b4..e297dcb6a8e2 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -2653,6 +2653,8 @@ static int btusb_recv_event_realtek(struct hci_dev *hdev, struct sk_buff *skb)
>  #define MTK_BT_RESET_REG_CONNV3        0x70028610
>  #define MTK_BT_READ_DEV_ID     0x70010200
>
> +/* MediaTek ISO Interface */
> +#define MTK_ISO_IFNUM          2
>
>  static void btusb_mtk_wmt_recv(struct urb *urb)
>  {
> @@ -3129,6 +3131,239 @@ static int btusb_mtk_reset(struct hci_dev *hdev, void *rst_data)
>         return err;
>  }
>
> +static void btusb_mtk_intr_complete(struct urb *urb)
> +{
> +       struct hci_dev *hdev = urb->context;
> +       struct btmediatek_data *btmtk_data = hci_get_priv(hdev);
> +       struct btusb_data *data = hci_get_drvdata(hdev);
> +       int err;
> +
> +       BT_DBG("%s urb %p status %d count %d", hdev->name, urb, urb->status,
> +              urb->actual_length);
> +
> +       if (!test_bit(HCI_RUNNING, &hdev->flags))
> +               return;
> +
> +       if (urb->status == 0) {
> +               hdev->stat.byte_rx += urb->actual_length;
> +
> +               if (btmtk_recv_isopkt(hdev, urb->transfer_buffer,
> +                                     urb->actual_length) < 0) {
> +                       bt_dev_err(hdev, "corrupted iso packet");
> +                       hdev->stat.err_rx++;
> +               }
> +       } else if (urb->status == -ENOENT) {
> +               /* Avoid suspend failed when usb_kill_urb */
> +               return;
> +       }
> +
> +       if (!btmtk_test_flag(hdev, BTMTK_ISOPKT_RUNNING))
> +               return;
> +
> +       usb_mark_last_busy(data->udev);
> +       usb_anchor_urb(urb, &btmtk_data->isopkt_info.isopkt_anchor);
> +
> +       err = usb_submit_urb(urb, GFP_ATOMIC);
> +       if (err < 0) {
> +               /* -EPERM: urb is being killed;
> +                * -ENODEV: device got disconnected
> +                */
> +               if (err != -EPERM && err != -ENODEV)
> +                       bt_dev_err(hdev, "urb %p failed to resubmit (%d)",
> +                                  urb, -err);
> +               if (err != -EPERM)
> +                       hci_cmd_sync_cancel(hdev, -err);
> +               usb_unanchor_urb(urb);
> +       }
> +}
> +
> +static int btusb_mtk_submit_intr_urb(struct hci_dev *hdev, gfp_t mem_flags)
> +{
> +       struct btmediatek_data *btmtk_data = hci_get_priv(hdev);
> +       struct btusb_data *data = hci_get_drvdata(hdev);
> +       unsigned char *buf;
> +       unsigned int pipe;
> +       struct urb *urb;
> +       int err, size;
> +
> +       BT_DBG("%s", hdev->name);
> +
> +       if (!btmtk_data->isopkt_info.isopkt_rx_ep)
> +               return -ENODEV;
> +
> +       urb = usb_alloc_urb(0, mem_flags);
> +       if (!urb)
> +               return -ENOMEM;
> +       size = le16_to_cpu(btmtk_data->isopkt_info.isopkt_rx_ep->wMaxPacketSize);
> +
> +       buf = kmalloc(size, mem_flags);
> +       if (!buf) {
> +               usb_free_urb(urb);
> +               return -ENOMEM;
> +       }
> +
> +       pipe = usb_rcvintpipe(data->udev,
> +                             btmtk_data->isopkt_info.isopkt_rx_ep->bEndpointAddress);
> +
> +       usb_fill_int_urb(urb, data->udev, pipe, buf, size,
> +                        btusb_mtk_intr_complete, hdev,
> +                        btmtk_data->isopkt_info.isopkt_rx_ep->bInterval);
> +
> +       urb->transfer_flags |= URB_FREE_BUFFER;
> +
> +       usb_mark_last_busy(data->udev);
> +       usb_anchor_urb(urb, &btmtk_data->isopkt_info.isopkt_anchor);
> +
> +       err = usb_submit_urb(urb, mem_flags);
> +       if (err < 0) {
> +               if (err != -EPERM && err != -ENODEV)
> +                       bt_dev_err(hdev, "urb %p submission failed (%d)",
> +                                  urb, -err);
> +               usb_unanchor_urb(urb);
> +       }
> +
> +       usb_free_urb(urb);
> +
> +       return err;
> +}
> +
> +static inline int __set_mtk_intr_interface(struct hci_dev *hdev, unsigned int ifnum)
> +{
> +       struct btusb_data *data = hci_get_drvdata(hdev);
> +       struct btmediatek_data *btmtk_data = hci_get_priv(hdev);
> +       struct usb_interface *intf = btmtk_data->isopkt_info.isopkt_intf;
> +       int i, err;
> +
> +       if (!btmtk_data->isopkt_info.isopkt_intf)
> +               return -ENODEV;
> +
> +       err = usb_set_interface(data->udev, ifnum, 1);
> +       if (err < 0) {
> +               bt_dev_err(hdev, "setting interface failed (%d)", -err);
> +               return err;
> +       }
> +
> +       btmtk_data->isopkt_info.isopkt_tx_ep = NULL;
> +       btmtk_data->isopkt_info.isopkt_rx_ep = NULL;
> +
> +       for (i = 0; i < intf->cur_altsetting->desc.bNumEndpoints; i++) {
> +               struct usb_endpoint_descriptor *ep_desc;
> +
> +               ep_desc = &intf->cur_altsetting->endpoint[i].desc;
> +
> +               if (!btmtk_data->isopkt_info.isopkt_tx_ep &&
> +                   usb_endpoint_is_int_out(ep_desc)) {
> +                       btmtk_data->isopkt_info.isopkt_tx_ep = ep_desc;
> +                       continue;
> +               }
> +
> +               if (!btmtk_data->isopkt_info.isopkt_rx_ep &&
> +                   usb_endpoint_is_int_in(ep_desc)) {
> +                       btmtk_data->isopkt_info.isopkt_rx_ep = ep_desc;
> +                       continue;
> +               }
> +       }
> +
> +       if (!btmtk_data->isopkt_info.isopkt_tx_ep ||
> +           !btmtk_data->isopkt_info.isopkt_rx_ep) {
> +               bt_dev_err(hdev, "invalid interrupt descriptors");
> +               return -ENODEV;
> +       }
> +
> +       return 0;
> +}
> +
> +static int btusb_mtk_isopkt_stop(struct hci_dev *hdev)
> +{
> +       struct btmediatek_data *btmtk_data = hci_get_priv(hdev);
> +
> +       usb_kill_anchored_urbs(&btmtk_data->isopkt_info.isopkt_anchor);
> +
> +       return 0;
> +}
> +
> +static int btusb_mtk_isopkt_start(struct hci_dev *hdev)
> +{
> +       if (btmtk_test_flag(hdev, BTMTK_ISOPKT_RUNNING)) {
> +               if (btusb_mtk_submit_intr_urb(hdev, GFP_NOIO) < 0)
> +                       btmtk_clear_flag(hdev, BTMTK_ISOPKT_RUNNING);
> +       }
> +
> +       return 0;
> +}
> +
> +static int btusb_mtk_claim_iso_intf(struct btusb_data *data, struct usb_interface *intf)
> +{
> +       struct btmediatek_data *btmtk_data = hci_get_priv(data->hdev);
> +       int err;
> +
> +       err = usb_driver_claim_interface(&btusb_driver, intf, data);
> +       if (err < 0)
> +               return err;
> +
> +       init_usb_anchor(&btmtk_data->isopkt_info.isopkt_anchor);
> +       spin_lock_init(&btmtk_data->isopkt_info.isorxlock);
> +
> +       __set_mtk_intr_interface(data->hdev, MTK_ISO_IFNUM);
> +
> +       err = btusb_mtk_submit_intr_urb(data->hdev, GFP_KERNEL);
> +       if (err < 0) {
> +               btusb_mtk_isopkt_stop(data->hdev);
> +               bt_dev_err(data->hdev, "ISO intf not support (%d)", err);
> +               return err;
> +       }
> +
> +       btmtk_set_flag(data->hdev, BTMTK_ISOPKT_OVER_INTR);
> +
> +       return 0;
> +}
> +
> +static struct urb *alloc_mtk_intr_urb(struct hci_dev *hdev, struct sk_buff *skb)
> +{
> +       struct btusb_data *data = hci_get_drvdata(hdev);
> +       struct btmediatek_data *btmtk_data = hci_get_priv(hdev);
> +       unsigned int pipe;
> +       struct urb *urb;
> +
> +       if (!btmtk_data->isopkt_info.isopkt_tx_ep)
> +               return ERR_PTR(-ENODEV);
> +
> +       urb = usb_alloc_urb(0, GFP_KERNEL);
> +       if (!urb)
> +               return ERR_PTR(-ENOMEM);
> +
> +       if (btmtk_isopkt_pad(hdev, skb))
> +               return ERR_PTR(-EINVAL);
> +
> +       pipe = usb_sndintpipe(data->udev,
> +                             btmtk_data->isopkt_info.isopkt_tx_ep->bEndpointAddress);
> +
> +       usb_fill_int_urb(urb, data->udev, pipe,
> +                        skb->data, skb->len, btusb_tx_complete,
> +                        skb, btmtk_data->isopkt_info.isopkt_tx_ep->bInterval);
> +
> +       skb->dev = (void *)hdev;
> +
> +       return urb;
> +}
> +
> +static int btusb_send_frame_mtk(struct hci_dev *hdev, struct sk_buff *skb)
> +{
> +       struct urb *urb;
> +
> +       BT_DBG("%s", hdev->name);
> +
> +       if (hci_skb_pkt_type(skb) == HCI_ISODATA_PKT) {
> +               urb = alloc_mtk_intr_urb(hdev, skb);
> +               if (IS_ERR(urb))
> +                       return PTR_ERR(urb);
> +
> +               return submit_or_queue_tx_urb(hdev, urb);
> +       } else {
> +               return btusb_send_frame(hdev, skb);
> +       }
> +}
> +
>  static int btusb_mtk_setup(struct hci_dev *hdev)
>  {
>         struct btusb_data *data = hci_get_drvdata(hdev);
> @@ -3213,6 +3448,12 @@ static int btusb_mtk_setup(struct hci_dev *hdev)
>                 /* It's Device EndPoint Reset Option Register */
>                 btusb_mtk_uhw_reg_write(data, MTK_EP_RST_OPT, MTK_EP_RST_IN_OUT_OPT);
>
> +               /* Claim ISO data interface and endpoint */
> +               mediatek->isopkt_info.isopkt_intf = usb_ifnum_to_if(data->udev, MTK_ISO_IFNUM);
> +               err = btusb_mtk_claim_iso_intf(data, mediatek->isopkt_info.isopkt_intf);
> +               if (err < 0)
> +                       mediatek->isopkt_info.isopkt_intf = NULL;
> +
>                 /* Enable Bluetooth protocol */
>                 param = 1;
>                 wmt_params.op = BTMTK_WMT_FUNC_CTRL;
> @@ -3229,6 +3470,13 @@ static int btusb_mtk_setup(struct hci_dev *hdev)
>
>                 hci_set_msft_opcode(hdev, 0xFD30);
>                 hci_set_aosp_capable(hdev);
> +
> +               /* Set up ISO interface after protocol enabled */
> +               if (btmtk_test_flag(hdev, BTMTK_ISOPKT_OVER_INTR)) {
> +                       btmtk_isointf_setup(hdev);
> +                       btmtk_set_flag(data->hdev, BTMTK_ISOPKT_RUNNING);
> +               }
> +
>                 goto done;
>         default:
>                 bt_dev_err(hdev, "Unsupported hardware variant (%08x)",
> @@ -3321,6 +3569,7 @@ static int btusb_mtk_setup(struct hci_dev *hdev)
>  static int btusb_mtk_shutdown(struct hci_dev *hdev)
>  {
>         struct btmtk_hci_wmt_params wmt_params;
> +       struct btmediatek_data *btmtk_data = hci_get_priv(hdev);
>         u8 param = 0;
>         int err;
>
> @@ -3337,6 +3586,21 @@ static int btusb_mtk_shutdown(struct hci_dev *hdev)
>                 return err;
>         }
>
> +       if (btmtk_test_flag(hdev, BTMTK_ISOPKT_RUNNING)) {
> +               btusb_mtk_isopkt_stop(hdev);
> +               btmtk_clear_flag(hdev, BTMTK_ISOPKT_RUNNING);
> +
> +               if (btmtk_data->isopkt_info.isopkt_intf) {
> +                       usb_set_intfdata(btmtk_data->isopkt_info.isopkt_intf, NULL);
> +                       usb_driver_release_interface(&btusb_driver,
> +                                                    btmtk_data->isopkt_info.isopkt_intf);
> +                       dev_kfree_skb_irq(btmtk_data->isopkt_info.isopkt_skb);
> +                       btmtk_data->isopkt_info.isopkt_skb = NULL;
> +
> +                       btmtk_clear_flag(hdev, BTMTK_ISOPKT_OVER_INTR);
> +               }
> +       }

There is still an awful lot of code into btusb that is actually not
part of the standard interface, for instance the above could also
possible be done in btmtk as well, in fact I'm not really sure why we
need the likes of btusb_mtk_shutdown or btusb_mtk_setup, etc, inside
btusb.c?

>         return 0;
>  }
>
> @@ -4475,9 +4739,12 @@ static int btusb_probe(struct usb_interface *intf,
>                 hdev->manufacturer = 70;
>                 hdev->cmd_timeout = btmtk_reset_sync;
>                 hdev->set_bdaddr = btmtk_set_bdaddr;
> +               hdev->send = btusb_send_frame_mtk;
>                 set_bit(HCI_QUIRK_BROKEN_ENHANCED_SETUP_SYNC_CONN, &hdev->quirks);
>                 set_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks);
>                 data->recv_acl = btusb_recv_acl_mtk;
> +               data->dev_suspend = btusb_mtk_isopkt_stop;
> +               data->dev_resume = btusb_mtk_isopkt_start;
>         }
>
>         if (id->driver_info & BTUSB_SWAVE) {
> --
> 2.18.0
>


-- 
Luiz Augusto von Dentz

Re: [PATCH v4 4/4] Bluetooth: btusb: mediatek: add ISO data transmission functions

[kernel test robot]

Hi Chris,

kernel test robot noticed the following build errors:

[auto build test ERROR on bluetooth-next/master]
[also build test ERROR on next-20240606]
[cannot apply to bluetooth/master linus/master v6.10-rc2]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Chris-Lu/Bluetooth-net-add-hci_iso_hdr-function-for-iso-data/20240606-194619
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
patch link:    https://lore.kernel.org/r/20240606114321.30515-5-chris.lu%40mediatek.com
patch subject: [PATCH v4 4/4] Bluetooth: btusb: mediatek: add ISO data transmission functions
config: x86_64-buildonly-randconfig-003-20240607 (https://download.01.org/0day-ci/archive/20240607/202406070759.EDNrdv4q-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240607/202406070759.EDNrdv4q-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202406070759.EDNrdv4q-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from drivers/bluetooth/btusb.c:28:
>> drivers/bluetooth/btmtk.h:259:9: error: incompatible pointer to integer conversion returning 'void *' from a function with result type 'int' [-Wint-conversion]
     259 |         return ERR_PTR(-EOPNOTSUPP);
         |                ^~~~~~~~~~~~~~~~~~~~
   1 error generated.


vim +259 drivers/bluetooth/btmtk.h

   256	
   257	static int btmtk_isopkt_pad(struct hci_dev *hdev, struct sk_buff *skb)
   258	{
 > 259		return ERR_PTR(-EOPNOTSUPP);
   260	}
   261	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v4 4/4] Bluetooth: btusb: mediatek: add ISO data transmission functions

[kernel test robot]

Hi Chris,

kernel test robot noticed the following build warnings:

[auto build test WARNING on bluetooth-next/master]
[also build test WARNING on next-20240607]
[cannot apply to bluetooth/master linus/master v6.10-rc2]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Chris-Lu/Bluetooth-net-add-hci_iso_hdr-function-for-iso-data/20240606-194619
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
patch link:    https://lore.kernel.org/r/20240606114321.30515-5-chris.lu%40mediatek.com
patch subject: [PATCH v4 4/4] Bluetooth: btusb: mediatek: add ISO data transmission functions
config: arm-omap2plus_defconfig (https://download.01.org/0day-ci/archive/20240608/202406081451.S72dkwTp-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240608/202406081451.S72dkwTp-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202406081451.S72dkwTp-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from drivers/bluetooth/btusb.c:28:
   drivers/bluetooth/btmtk.h: In function 'btmtk_isopkt_pad':
>> drivers/bluetooth/btmtk.h:259:16: warning: returning 'void *' from a function with return type 'int' makes integer from pointer without a cast [-Wint-conversion]
     259 |         return ERR_PTR(-EOPNOTSUPP);
         |                ^~~~~~~~~~~~~~~~~~~~


vim +259 drivers/bluetooth/btmtk.h

   256	
   257	static int btmtk_isopkt_pad(struct hci_dev *hdev, struct sk_buff *skb)
   258	{
 > 259		return ERR_PTR(-EOPNOTSUPP);
   260	}
   261	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki