Re: [PATCH v3] Bluetooth: add quirk using packet size 60

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Dear Hilda,


Thank you for your patch. Some minor comments.

It’d be great if you addad the chip model to the commit message summary/title.


Am 26.11.24 um 08:05 schrieb Hilda Wu:
The RTL8852BE-VT supports USB alternate setting 6.

Please add a blank line between paragraphs.

However, its descriptor does not report this capability to the host.

Is that a firmware limitation? Is it going to be fixed in the future in other revisions?

Therefore, a quirk is needed to bypass the RTL8852BE-VT's descriptor
and allow it to use USB ALT 6 directly.

Does Linux log the application of this quirk?

The btmon log below shows the case that WBS with the USB alternate
setting 6.

< HCI Command: Enhanced.. (0x01|0x003d) plen 59  #2123 [hci0] 82.701813
         Handle: 1 Address: 78:A7:EB:4C:53:4D (1MORE)
         Transmit bandwidth: 8000
         Receive bandwidth: 8000
         Max latency: 13
         Packet type: 0x0380
           3-EV3 may not be used
           2-EV5 may not be used
           3-EV5 may not be used
         Retransmission effort: Optimize for link quality (0x02)
< ACL Data TX: Handle 1 flags 0x00 dlen 22       #2124 [hci0] 82.701825
       Channel: 65 len 18 [PSM 3 mode Basic (0x00)] {chan 1}
       RFCOMM: Unnumbered Info with Header Check (UIH) (0xef)
          Address: 0x0b cr 1 dlci 0x02
          Control: 0xef poll/final 0
          Length: 14
          FCS: 0x9a
         0d 0a 2b 43 49 45 56 3a 20 32 2c 31 0d 0a 9a     ..+CIEV: 2,1..>
HCI Event: Command Status (0x0f) plen 4        #2125 [hci0] 82.703812
       Enhanced Setup Synchronous Connection (0x01|0x003d) ncmd 2
         Status: Success (0x00)
HCI Event: Number of Complete.. (0x13) plen 5  #2126 [hci0] 82.710834
         Num handles: 1
         Handle: 1 Address: 78:A7:EB:4C:53:4D (1MORE)
         Count: 1
         #2124: len 22 (19 Kb/s)
         Latency: 9 msec (3-56 msec ~13 msec)
         Channel: 65 [PSM 3 mode Basic (0x00)] {chan 1}
         Channel Latency: 9 msec (4-27 msec ~15 msec)
HCI Event: Synchronous Conne.. (0x2c) plen 17  #2127 [hci0] 82.741840
         Status: Success (0x00)
         Handle: 2
         Address: 78:A7:EB:4C:53:4D (1MORE)
         Link type: eSCO (0x02)
         Transmission interval: 0x0c
         Retransmission window: 0x04
         RX packet length: 60
         TX packet length: 60
         Air mode: Transparent (0x03)
@ RAW Open: btmon (privileged) version 2.22          {0x0002} 82.742580
@ RAW Close: btmon                                   {0x0002} 82.742594
SCO Data RX: Handle 2 flags 0x00 dlen 60       #2128 [hci0] 82.764812
< ACL Data TX: Handle 1 flags 0x00 dlen 19       #2129 [hci0] 82.764994
       Channel: 65 len 15 [PSM 3 mode Basic (0x00)] {chan 1}
       RFCOMM: Unnumbered Info with Header Check (UIH) (0xef)
          Address: 0x0b cr 1 dlci 0x02
          Control: 0xef poll/final 0
          Length: 11
          FCS: 0x9a
         0d 0a 2b 56 47 53 3a 20 36 0d 0a 9a              ..+VGS: 6...  >
HCI Event: Max Slots Change (0x1b) plen 3      #2130 [hci0] 82.765814
         Handle: 1 Address: 78:A7:EB:4C:53:4D (1MORE)
         Max slots: 1
< SCO Data TX: Handle 2 flags 0x00 dlen 60       #2131 [hci0] 82.765897
HCI Event: Number of Complete.. (0x13) plen 5  #2132 [hci0] 82.771855
         Num handles: 1
         Handle: 1 Address: 78:A7:EB:4C:53:4D (1MORE)
         Count: 1
         #2129: len 19 (25 Kb/s)
         Latency: 6 msec (3-56 msec ~10 msec)
         Channel: 65 [PSM 3 mode Basic (0x00)] {chan 1}
         Channel Latency: 6 msec (4-27 msec ~11 msec)
< SCO Data TX: Handle 2 flags 0x00 dlen 60       #2133 [hci0] 82.773344
SCO Data RX: Handle 2 flags 0x00 dlen 60       #2134 [hci0] 82.774836
SCO Data RX: Handle 2 flags 0x00 dlen 60       #2135 [hci0] 82.774839
SCO Data RX: Handle 2 flags 0x00 dlen 60       #2136 [hci0] 82.784840
< SCO Data TX: Handle 2 flags 0x00 dlen 60       #2137 [hci0] 82.787175
< SCO Data TX: Handle 2 flags 0x00 dlen 60       #2138 [hci0] 82.788282
SCO Data RX: Handle 2 flags 0x00 dlen 60       #2139 [hci0] 82.794812
< SCO Data TX: Handle 2 flags 0x00 dlen 60       #2140 [hci0] 82.795797
SCO Data RX: Handle 2 flags 0x00 dlen 60       #2141 [hci0] 82.804838
SCO Data RX: Handle 2 flags 0x00 dlen 60       #2142 [hci0] 82.804840
< SCO Data TX: Handle 2 flags 0x00 dlen 60       #2143 [hci0] 82.808554

Signed-off-by: Alex Lu <alex_lu@xxxxxxxxxxxxxx>
Signed-off-by: Hilda Wu <hildawu@xxxxxxxxxxx>

---
Change:
V3: Fixed SubjectPrefix, use quirk instead of btrealtek_*_flag()
v2: Use btusb_find_altsetting replace duplicating logic, add tested log.
---
---
  drivers/bluetooth/btrtl.c |  3 ++
  drivers/bluetooth/btrtl.h |  1 +
  drivers/bluetooth/btusb.c | 86 ++++++++++++++++++++++++++++++---------
  3 files changed, 70 insertions(+), 20 deletions(-)

diff --git a/drivers/bluetooth/btrtl.c b/drivers/bluetooth/btrtl.c
index 83025f457ca0..7efd733f9e84 100644
--- a/drivers/bluetooth/btrtl.c
+++ b/drivers/bluetooth/btrtl.c
@@ -1312,6 +1312,9 @@ void btrtl_set_quirks(struct hci_dev *hdev, struct btrtl_device_info *btrtl_dev)
  		    btrtl_dev->project_id == CHIP_ID_8852C)
  			set_bit(HCI_QUIRK_USE_MSFT_EXT_ADDRESS_FILTER, &hdev->quirks);
+ if (btrtl_dev->project_id == CHIP_ID_8852BT)
+			btrealtek_set_flag(hdev, REALTEK_ALT6_FORCE);
+
  		hci_set_aosp_capable(hdev);
  		break;
  	default:
diff --git a/drivers/bluetooth/btrtl.h b/drivers/bluetooth/btrtl.h
index a2d9d34f9fb0..ffec2fca88ec 100644
--- a/drivers/bluetooth/btrtl.h
+++ b/drivers/bluetooth/btrtl.h
@@ -105,6 +105,7 @@ struct rtl_vendor_cmd {
enum {
  	REALTEK_ALT6_CONTINUOUS_TX_CHIP,
+	REALTEK_ALT6_FORCE,
__REALTEK_NUM_FLAGS,
  };
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 279fe6c115fa..6acdff772ca5 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -814,6 +814,8 @@ struct qca_dump_info {
  #define BTUSB_USE_ALT3_FOR_WBS	15
  #define BTUSB_ALT6_CONTINUOUS_TX	16
  #define BTUSB_HW_SSR_ACTIVE	17
+#define BTUSB_ALT6_QUIRK	18
+#define BTUSB_ISOC_ALT_CHANGED	19

What is BTUSB_ISOC_ALT_CHANGED needed for? For me it’d be great, if you mentioned it in the commit message.

  struct btusb_data {
  	struct hci_dev       *hdev;
@@ -866,6 +868,7 @@ struct btusb_data {
  	unsigned int air_mode;
  	bool usb_alt6_packet_flow;
  	int isoc_altsetting;
+	u16 isoc_mps;

What is mps? Add a comment? Why not unsigned int?

  	int suspend_count;
int (*recv_event)(struct hci_dev *hdev, struct sk_buff *skb);
@@ -2140,16 +2143,58 @@ static void btusb_notify(struct hci_dev *hdev, unsigned int evt)
  	}
  }
+static struct usb_host_interface *btusb_find_altsetting(struct btusb_data *data,
+							int alt)
+{
+	struct usb_interface *intf = data->isoc;
+	int i;
+
+	BT_DBG("Looking for Alt no :%d", alt);
+
+	if (!intf)
+		return NULL;
+
+	for (i = 0; i < intf->num_altsetting; i++) {
+		if (intf->altsetting[i].desc.bAlternateSetting == alt)
+			return &intf->altsetting[i];
+	}
+
+	return NULL;
+}
+
  static inline int __set_isoc_interface(struct hci_dev *hdev, int altsetting)
  {
  	struct btusb_data *data = hci_get_drvdata(hdev);
  	struct usb_interface *intf = data->isoc;
  	struct usb_endpoint_descriptor *ep_desc;
+	struct usb_host_interface *alt;
  	int i, err;
if (!data->isoc)
  		return -ENODEV;
+ /* For some Realtek chips, they actually have the altsetting 6, but its
+	 * altsetting descriptor is not exposed. We can activate altsetting 6 by
+	 * replacing the altsetting 5.
+	 */
+	if (altsetting == 6 && !btusb_find_altsetting(data, 6) &&
+	    test_bit(BTUSB_ALT6_QUIRK, &data->flags)) {
+		alt = btusb_find_altsetting(data, 5);
+		if (alt) {
+			data->isoc_mps = 49;
+			for (i = 0; i < alt->desc.bNumEndpoints; i++) {
+				ep_desc = &alt->endpoint[i].desc;
+				if (!usb_endpoint_xfer_isoc(ep_desc))
+					continue;
+				data->isoc_mps =
+					le16_to_cpu(ep_desc->wMaxPacketSize);
+				ep_desc->wMaxPacketSize = cpu_to_le16(63);
+			}
+			alt->desc.bAlternateSetting = 6;
+			set_bit(BTUSB_ISOC_ALT_CHANGED, &data->flags);
+		}
+	}
+
  	err = usb_set_interface(data->udev, data->isoc_ifnum, altsetting);
  	if (err < 0) {
  		bt_dev_err(hdev, "setting interface failed (%d)", -err);
@@ -2161,6 +2206,22 @@ static inline int __set_isoc_interface(struct hci_dev *hdev, int altsetting)
  	data->isoc_tx_ep = NULL;
  	data->isoc_rx_ep = NULL;
+ /* Recover alt 5 desc if alt 0 is set. */

Why?

+	if (!altsetting && test_bit(BTUSB_ISOC_ALT_CHANGED, &data->flags)) {
+		alt = btusb_find_altsetting(data, 6);
+		if (alt) {
+			for (i = 0; i < alt->desc.bNumEndpoints; i++) {
+				ep_desc = &alt->endpoint[i].desc;
+				if (!usb_endpoint_xfer_isoc(ep_desc))
+					continue;
+				ep_desc->wMaxPacketSize =
+					cpu_to_le16(data->isoc_mps);
+			}
+			alt->desc.bAlternateSetting = 5;
+			clear_bit(BTUSB_ISOC_ALT_CHANGED, &data->flags);
+		}
+	}
+
  	for (i = 0; i < intf->cur_altsetting->desc.bNumEndpoints; i++) {
  		ep_desc = &intf->cur_altsetting->endpoint[i].desc;
@@ -2223,25 +2284,6 @@ static int btusb_switch_alt_setting(struct hci_dev *hdev, int new_alts)
  	return 0;
  }
-static struct usb_host_interface *btusb_find_altsetting(struct btusb_data *data,
-							int alt)
-{
-	struct usb_interface *intf = data->isoc;
-	int i;
-
-	BT_DBG("Looking for Alt no :%d", alt);
-
-	if (!intf)
-		return NULL;
-
-	for (i = 0; i < intf->num_altsetting; i++) {
-		if (intf->altsetting[i].desc.bAlternateSetting == alt)
-			return &intf->altsetting[i];
-	}
-
-	return NULL;
-}
-
  static void btusb_work(struct work_struct *work)
  {
  	struct btusb_data *data = container_of(work, struct btusb_data, work);
@@ -2279,7 +2321,8 @@ static void btusb_work(struct work_struct *work)
  			 * MTU >= 3 (packets) * 25 (size) - 3 (headers) = 72
  			 * see also Core spec 5, vol 4, B 2.1.1 & Table 2.1.
  			 */
-			if (btusb_find_altsetting(data, 6))
+			if (btusb_find_altsetting(data, 6) ||
+			    test_bit(BTUSB_ALT6_QUIRK, &data->flags))
  				new_alts = 6;
  			else if (btusb_find_altsetting(data, 3) &&
  				 hdev->sco_mtu >= 72 &&
@@ -2611,6 +2654,9 @@ static int btusb_setup_realtek(struct hci_dev *hdev)
  	if (btrealtek_test_flag(data->hdev, REALTEK_ALT6_CONTINUOUS_TX_CHIP))
  		set_bit(BTUSB_ALT6_CONTINUOUS_TX, &data->flags);
+ if (btrealtek_test_flag(data->hdev, REALTEK_ALT6_FORCE))
+		set_bit(BTUSB_ALT6_QUIRK, &data->flags);
+
  	return ret;
  }


Kind regards,

Paul




[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux