Message ID | 20241126070542.3828431-1-hildawu@realtek.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | [v3] Bluetooth: add quirk using packet size 60 | expand |
Context | Check | Description |
---|---|---|
tedd_an/pre-ci_am | success | Success |
tedd_an/SubjectPrefix | success | Gitlint PASS |
tedd_an/BuildKernel | success | BuildKernel PASS |
tedd_an/CheckAllWarning | success | CheckAllWarning PASS |
tedd_an/CheckSparse | success | CheckSparse PASS |
tedd_an/BuildKernel32 | success | BuildKernel32 PASS |
tedd_an/TestRunnerSetup | success | TestRunnerSetup PASS |
tedd_an/TestRunner_l2cap-tester | success | TestRunner PASS |
tedd_an/TestRunner_iso-tester | fail | TestRunner_iso-tester: WARNING: possible circular locking dependency detected |
tedd_an/TestRunner_bnep-tester | success | TestRunner PASS |
tedd_an/TestRunner_mgmt-tester | fail | TestRunner_mgmt-tester: Total: 492, Passed: 487 (99.0%), Failed: 1, Not Run: 4 |
tedd_an/TestRunner_rfcomm-tester | success | TestRunner PASS |
tedd_an/TestRunner_sco-tester | success | TestRunner PASS |
tedd_an/TestRunner_ioctl-tester | success | TestRunner PASS |
tedd_an/TestRunner_mesh-tester | fail | TestRunner_mesh-tester: WARNING: CPU: 0 PID: 33 at kernel/workqueue.c:2257 __queue_work+0x687/0xb40 |
tedd_an/TestRunner_smp-tester | success | TestRunner PASS |
tedd_an/TestRunner_userchan-tester | success | TestRunner PASS |
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@realsil.com.cn> > Signed-off-by: Hilda Wu <hildawu@realtek.com> > > --- > 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
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=912422 ---Test result--- Test Summary: CheckPatch PENDING 0.33 seconds GitLint PENDING 0.25 seconds SubjectPrefix PASS 0.05 seconds BuildKernel PASS 24.43 seconds CheckAllWarning PASS 27.33 seconds CheckSparse PASS 30.13 seconds BuildKernel32 PASS 24.54 seconds TestRunnerSetup PASS 434.24 seconds TestRunner_l2cap-tester PASS 20.32 seconds TestRunner_iso-tester FAIL 30.07 seconds TestRunner_bnep-tester PASS 4.75 seconds TestRunner_mgmt-tester FAIL 119.70 seconds TestRunner_rfcomm-tester PASS 7.48 seconds TestRunner_sco-tester PASS 11.33 seconds TestRunner_ioctl-tester PASS 8.05 seconds TestRunner_mesh-tester FAIL 6.17 seconds TestRunner_smp-tester PASS 7.00 seconds TestRunner_userchan-tester PASS 4.96 seconds IncrementalBuild PENDING 0.63 seconds Details ############################## Test: CheckPatch - PENDING Desc: Run checkpatch.pl script Output: ############################## Test: GitLint - PENDING Desc: Run gitlint Output: ############################## Test: TestRunner_iso-tester - FAIL Desc: Run iso-tester with test-runner Output: WARNING: possible circular locking dependency detected Total: 124, Passed: 120 (96.8%), Failed: 0, Not Run: 4 ############################## Test: TestRunner_mgmt-tester - FAIL Desc: Run mgmt-tester with test-runner Output: Total: 492, Passed: 487 (99.0%), Failed: 1, Not Run: 4 Failed Test Cases LL Privacy - Start Discovery 2 (Disable RL) Failed 0.190 seconds ############################## Test: TestRunner_mesh-tester - FAIL Desc: Run mesh-tester with test-runner Output: BUG: KASAN: slab-use-after-free in run_timer_softirq+0x76c/0x7d0 WARNING: CPU: 0 PID: 33 at kernel/workqueue.c:2257 __queue_work+0x687/0xb40 Total: 10, Passed: 9 (90.0%), Failed: 1, Not Run: 0 Failed Test Cases Mesh - Send cancel - 1 Failed 0.118 seconds ############################## Test: IncrementalBuild - PENDING Desc: Incremental build with the patches in the series Output: --- Regards, Linux Bluetooth
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 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; 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. */ + 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; }