| Message ID | 20250311195528.41993-1-luiz.dentz@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v1,1/2] Bluetooth: hci_core: Enable buffer flow control for SCO/eSCO | 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 | warning | CheckSparse WARNING net/bluetooth/hci_event.c: note: in included file (through include/net/bluetooth/hci_core.h): |
| 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 | success | TestRunner PASS |
| tedd_an/TestRunner_bnep-tester | success | TestRunner PASS |
| tedd_an/TestRunner_mgmt-tester | fail | TestRunner_mgmt-tester: Total: 490, Passed: 485 (99.0%), Failed: 1, Not Run: 4 |
| tedd_an/TestRunner_rfcomm-tester | success | TestRunner PASS |
| tedd_an/TestRunner_sco-tester | fail | TestRunner_sco-tester: Total: 19, Passed: 17 (89.5%), Failed: 1, Not Run: 1 |
| tedd_an/TestRunner_ioctl-tester | success | TestRunner PASS |
| tedd_an/TestRunner_mesh-tester | success | TestRunner PASS |
| tedd_an/TestRunner_smp-tester | success | TestRunner PASS |
| tedd_an/TestRunner_userchan-tester | success | TestRunner PASS |
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=942852 ---Test result--- Test Summary: CheckPatch PENDING 0.32 seconds GitLint PENDING 0.19 seconds SubjectPrefix PASS 0.19 seconds BuildKernel PASS 24.38 seconds CheckAllWarning PASS 26.74 seconds CheckSparse WARNING 32.92 seconds BuildKernel32 PASS 24.00 seconds TestRunnerSetup PASS 437.90 seconds TestRunner_l2cap-tester PASS 21.71 seconds TestRunner_iso-tester PASS 37.67 seconds TestRunner_bnep-tester PASS 4.93 seconds TestRunner_mgmt-tester FAIL 123.93 seconds TestRunner_rfcomm-tester PASS 8.14 seconds TestRunner_sco-tester FAIL 19.11 seconds TestRunner_ioctl-tester PASS 8.70 seconds TestRunner_mesh-tester PASS 6.17 seconds TestRunner_smp-tester PASS 7.45 seconds TestRunner_userchan-tester PASS 5.09 seconds IncrementalBuild PENDING 0.72 seconds Details ############################## Test: CheckPatch - PENDING Desc: Run checkpatch.pl script Output: ############################## Test: GitLint - PENDING Desc: Run gitlint Output: ############################## Test: CheckSparse - WARNING Desc: Run sparse tool with linux kernel Output: net/bluetooth/hci_event.c: note: in included file (through include/net/bluetooth/hci_core.h): ############################## Test: TestRunner_mgmt-tester - FAIL Desc: Run mgmt-tester with test-runner Output: Total: 490, Passed: 485 (99.0%), Failed: 1, Not Run: 4 Failed Test Cases LL Privacy - Start Discovery 2 (Disable RL) Failed 0.199 seconds ############################## Test: TestRunner_sco-tester - FAIL Desc: Run sco-tester with test-runner Output: Total: 19, Passed: 17 (89.5%), Failed: 1, Not Run: 1 Failed Test Cases SCO CVSD Send - Success Timed out 2.547 seconds ############################## Test: IncrementalBuild - PENDING Desc: Incremental build with the patches in the series Output: --- Regards, Linux Bluetooth
Hi, ti, 2025-03-11 kello 15:55 -0400, Luiz Augusto von Dentz kirjoitti: > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > This enables buffer flow control for SCO/eSCO > (see: Bluetooth Core 6.0 spec: 6.22. Synchronous Flow Control Enable), > recently this has caused the following problem and is actually a nice > addition for the likes of Socket TX complete: > > < HCI Command: Read Buffer Size (0x04|0x0005) plen 0 > > HCI Event: Command Complete (0x0e) plen 11 > Read Buffer Size (0x04|0x0005) ncmd 1 > Status: Success (0x00) > ACL MTU: 1021 ACL max packet: 5 > SCO MTU: 240 SCO max packet: 8 > ... > < SCO Data TX: Handle 257 flags 0x00 dlen 120 > < SCO Data TX: Handle 257 flags 0x00 dlen 120 > < SCO Data TX: Handle 257 flags 0x00 dlen 120 > < SCO Data TX: Handle 257 flags 0x00 dlen 120 > < SCO Data TX: Handle 257 flags 0x00 dlen 120 > < SCO Data TX: Handle 257 flags 0x00 dlen 120 > < SCO Data TX: Handle 257 flags 0x00 dlen 120 > < SCO Data TX: Handle 257 flags 0x00 dlen 120 > < SCO Data TX: Handle 257 flags 0x00 dlen 120 > > HCI Event: Hardware Error (0x10) plen 1 > Code: 0x0a > > To fix the code will now attempt to enable buffer flow control: > > < HCI Command: Write Sync Fl.. (0x03|0x002f) plen 1 > Flow control: Enabled (0x01) > > HCI Event: Command Complete (0x0e) plen 4 > Write Sync Flow Control Enable (0x03|0x002f) ncmd 1 > Status: Success (0x00) > > And then test it by sending an empty packet to confirm the controller > will generate HCI_EV_NUM_COMP_PKTS events: It's not doing this any more in this version. > > sco-tester[40]: < SCO Data TX:.. flags 0x00 dlen 9 > > HCI Event: Number of Completed P.. (0x13) plen 5 > Num handles: 1 > Handle: 42 Address: 00:AA:01:01:00:00 (Intel Corporation) > Count: 1 > #103: len 10 (40 Kb/s) > Latency: 2 msec (2-2 msec ~2 msec) > > Fixes: 7fedd3bb6b77 ("Bluetooth: Prioritize SCO traffic") > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > --- > include/net/bluetooth/hci.h | 13 +++++++ > include/net/bluetooth/hci_core.h | 1 + > net/bluetooth/hci_core.c | 67 ++++++++++++++++---------------- > net/bluetooth/hci_event.c | 2 + > net/bluetooth/hci_sync.c | 24 ++++++++++++ > 5 files changed, 74 insertions(+), 33 deletions(-) > > diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h > index b99818df8ee7..3c8f95174fcf 100644 > --- a/include/net/bluetooth/hci.h > +++ b/include/net/bluetooth/hci.h > @@ -208,6 +208,13 @@ enum { > */ > HCI_QUIRK_WIDEBAND_SPEECH_SUPPORTED, > > + /* When this quirk is set consider Sync Flow Control as supported by > + * the driver. > + * > + * This quirk must be set before hci_register_dev is called. > + */ > + HCI_QUIRK_SYNC_FLOWCTL_SUPPORTED, > + > /* When this quirk is set, the LE states reported through the > * HCI_LE_READ_SUPPORTED_STATES are invalid/broken. > * > @@ -448,6 +455,7 @@ enum { > HCI_WIDEBAND_SPEECH_ENABLED, > HCI_EVENT_FILTER_CONFIGURED, > HCI_PA_SYNC, > + HCI_SCO_FLOWCTL, > > HCI_DUT_MODE, > HCI_VENDOR_DIAG, > @@ -1544,6 +1552,11 @@ struct hci_rp_read_tx_power { > __s8 tx_power; > } __packed; > > +#define HCI_OP_WRITE_SYNC_FLOWCTL 0x0c2f > +struct hci_cp_write_sync_flowctl { > + __u8 enable; > +} __packed; > + > #define HCI_OP_READ_PAGE_SCAN_TYPE 0x0c46 > struct hci_rp_read_page_scan_type { > __u8 status; > diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h > index 7966db4038cc..f78e4298e39a 100644 > --- a/include/net/bluetooth/hci_core.h > +++ b/include/net/bluetooth/hci_core.h > @@ -1858,6 +1858,7 @@ void hci_conn_del_sysfs(struct hci_conn *conn); > #define lmp_hold_capable(dev) ((dev)->features[0][0] & LMP_HOLD) > #define lmp_sniff_capable(dev) ((dev)->features[0][0] & LMP_SNIFF) > #define lmp_park_capable(dev) ((dev)->features[0][1] & LMP_PARK) > +#define lmp_sco_capable(dev) ((dev)->features[0][1] & LMP_SCO) > #define lmp_inq_rssi_capable(dev) ((dev)->features[0][3] & LMP_RSSI_INQ) > #define lmp_esco_capable(dev) ((dev)->features[0][3] & LMP_ESCO) > #define lmp_bredr_capable(dev) (!((dev)->features[0][4] & LMP_NO_BREDR)) > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c > index 012fc107901a..efba2b539902 100644 > --- a/net/bluetooth/hci_core.c > +++ b/net/bluetooth/hci_core.c > @@ -3552,51 +3552,52 @@ static void __check_timeout(struct hci_dev *hdev, unsigned int cnt, u8 type) > } > > /* Schedule SCO */ > -static void hci_sched_sco(struct hci_dev *hdev) > +static void hci_sched_sco_type(struct hci_dev *hdev, __u8 type) > { > struct hci_conn *conn; > struct sk_buff *skb; > int quote; > + unsigned int pkts; > > - BT_DBG("%s", hdev->name); > + bt_dev_dbg(hdev, "type %u", type); > > - if (!hci_conn_num(hdev, SCO_LINK)) > + if (!hci_conn_num(hdev, type)) > return; > > - while (hdev->sco_cnt && (conn = hci_low_sent(hdev, SCO_LINK, "e))) { > + /* Use sco_pkts if flow control has not been enabled yet which will > + * limit the amount of buffer sent in a row. > + */ > + if (!hci_dev_test_flag(hdev, HCI_SCO_FLOWCTL)) > + pkts = hdev->sco_pkts; > + else > + pkts = hdev->sco_cnt; > + > + if (!pkts) > + return; > + > + while (pkts && (conn = hci_low_sent(hdev, type, "e))) { > while (quote-- && (skb = skb_dequeue(&conn->data_q))) { hci_low_sent -> hci_quote_sent uses the current value of hdev->sco_cnt to determine `quote`, which can then be larger than pkts, possibly causing it to underflow here. I guess this is why the other sched() functions are doing the (*cnt)-- thing. > BT_DBG("skb %p len %d", skb, skb->len); > hci_send_frame(hdev, skb); > > + pkts--; > + > + if (hdev->sco_cnt > 0) > + hdev->sco_cnt--; > + > conn->sent++; > if (conn->sent == ~0) > conn->sent = 0; > } > } > -} > > -static void hci_sched_esco(struct hci_dev *hdev) > -{ > - struct hci_conn *conn; > - struct sk_buff *skb; > - int quote; > - > - BT_DBG("%s", hdev->name); > - > - if (!hci_conn_num(hdev, ESCO_LINK)) > - return; > - > - while (hdev->sco_cnt && (conn = hci_low_sent(hdev, ESCO_LINK, > - "e))) { > - while (quote-- && (skb = skb_dequeue(&conn->data_q))) { > - BT_DBG("skb %p len %d", skb, skb->len); > - hci_send_frame(hdev, skb); > - > - conn->sent++; > - if (conn->sent == ~0) > - conn->sent = 0; > - } > - } > + /* Rescheduled if all packets were sent and flow control is not enabled > + * as there could be more packets queued that could not be sent and > + * since no HCI_EV_NUM_COMP_PKTS event will be generated the reschedule > + * needs to be forced. > + */ > + if (!pkts && !hci_dev_test_flag(hdev, HCI_SCO_FLOWCTL)) > + queue_work(hdev->workqueue, &hdev->tx_work); > } > > static void hci_sched_acl_pkt(struct hci_dev *hdev) > @@ -3632,8 +3633,8 @@ static void hci_sched_acl_pkt(struct hci_dev *hdev) > chan->conn->sent++; > > /* Send pending SCO packets right away */ > - hci_sched_sco(hdev); > - hci_sched_esco(hdev); > + hci_sched_sco_type(hdev, SCO_LINK); > + hci_sched_sco_type(hdev, ESCO_LINK); > } > } > > @@ -3688,8 +3689,8 @@ static void hci_sched_le(struct hci_dev *hdev) > chan->conn->sent++; > > /* Send pending SCO packets right away */ > - hci_sched_sco(hdev); > - hci_sched_esco(hdev); > + hci_sched_sco_type(hdev, SCO_LINK); > + hci_sched_sco_type(hdev, ESCO_LINK); > } > } > > @@ -3734,8 +3735,8 @@ static void hci_tx_work(struct work_struct *work) > > if (!hci_dev_test_flag(hdev, HCI_USER_CHANNEL)) { > /* Schedule queues and send stuff to HCI driver */ > - hci_sched_sco(hdev); > - hci_sched_esco(hdev); > + hci_sched_sco_type(hdev, SCO_LINK); > + hci_sched_sco_type(hdev, ESCO_LINK); > hci_sched_iso(hdev); > hci_sched_acl(hdev); > hci_sched_le(hdev); > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c > index 19e19c9f5e68..6d0138b778aa 100644 > --- a/net/bluetooth/hci_event.c > +++ b/net/bluetooth/hci_event.c > @@ -4445,9 +4445,11 @@ static void hci_num_comp_pkts_evt(struct hci_dev *hdev, void *data, > break; > > case SCO_LINK: > + case ESCO_LINK: > hdev->sco_cnt += count; > if (hdev->sco_cnt > hdev->sco_pkts) > hdev->sco_cnt = hdev->sco_pkts; > + > break; > > case ISO_LINK: > diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c > index c4c2cf51b219..609b035e5c90 100644 > --- a/net/bluetooth/hci_sync.c > +++ b/net/bluetooth/hci_sync.c > @@ -3769,6 +3769,28 @@ static int hci_write_ca_timeout_sync(struct hci_dev *hdev) > sizeof(param), ¶m, HCI_CMD_TIMEOUT); > } > > +/* Enable SCO flow control if supported */ > +static int hci_write_sync_flowctl_sync(struct hci_dev *hdev) > +{ > + struct hci_cp_write_sync_flowctl cp; > + int err; > + > + /* Check if the controller supports SCO and HCI_OP_WRITE_SYNC_FLOWCTL */ > + if (!lmp_sco_capable(hdev) || !(hdev->commands[10] & BIT(4)) || > + !test_bit(HCI_QUIRK_SYNC_FLOWCTL_SUPPORTED, &hdev->quirks)) > + return 0; > + > + memset(&cp, 0, sizeof(cp)); > + cp.enable = 0x01; > + > + err = __hci_cmd_sync_status(hdev, HCI_OP_WRITE_SYNC_FLOWCTL, > + sizeof(cp), &cp, HCI_CMD_TIMEOUT); > + if (!err) > + hci_dev_set_flag(hdev, HCI_SCO_FLOWCTL); > + > + return err; > +} > + > /* BR Controller init stage 2 command sequence */ > static const struct hci_init_stage br_init2[] = { > /* HCI_OP_READ_BUFFER_SIZE */ > @@ -3787,6 +3809,8 @@ static const struct hci_init_stage br_init2[] = { > HCI_INIT(hci_clear_event_filter_sync), > /* HCI_OP_WRITE_CA_TIMEOUT */ > HCI_INIT(hci_write_ca_timeout_sync), > + /* HCI_OP_WRITE_SYNC_FLOWCTL */ > + HCI_INIT(hci_write_sync_flowctl_sync), > {} > }; >
Hi Pauli, On Tue, Mar 11, 2025 at 6:21 PM Pauli Virtanen <pav@iki.fi> wrote: > > Hi, > > ti, 2025-03-11 kello 15:55 -0400, Luiz Augusto von Dentz kirjoitti: > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > > > This enables buffer flow control for SCO/eSCO > > (see: Bluetooth Core 6.0 spec: 6.22. Synchronous Flow Control Enable), > > recently this has caused the following problem and is actually a nice > > addition for the likes of Socket TX complete: > > > > < HCI Command: Read Buffer Size (0x04|0x0005) plen 0 > > > HCI Event: Command Complete (0x0e) plen 11 > > Read Buffer Size (0x04|0x0005) ncmd 1 > > Status: Success (0x00) > > ACL MTU: 1021 ACL max packet: 5 > > SCO MTU: 240 SCO max packet: 8 > > ... > > < SCO Data TX: Handle 257 flags 0x00 dlen 120 > > < SCO Data TX: Handle 257 flags 0x00 dlen 120 > > < SCO Data TX: Handle 257 flags 0x00 dlen 120 > > < SCO Data TX: Handle 257 flags 0x00 dlen 120 > > < SCO Data TX: Handle 257 flags 0x00 dlen 120 > > < SCO Data TX: Handle 257 flags 0x00 dlen 120 > > < SCO Data TX: Handle 257 flags 0x00 dlen 120 > > < SCO Data TX: Handle 257 flags 0x00 dlen 120 > > < SCO Data TX: Handle 257 flags 0x00 dlen 120 > > > HCI Event: Hardware Error (0x10) plen 1 > > Code: 0x0a > > > > To fix the code will now attempt to enable buffer flow control: > > > > < HCI Command: Write Sync Fl.. (0x03|0x002f) plen 1 > > Flow control: Enabled (0x01) > > > HCI Event: Command Complete (0x0e) plen 4 > > Write Sync Flow Control Enable (0x03|0x002f) ncmd 1 > > Status: Success (0x00) > > > > And then test it by sending an empty packet to confirm the controller > > will generate HCI_EV_NUM_COMP_PKTS events: > > It's not doing this any more in this version. Oh yeah, forgot to remove it from the patch description. > > > > sco-tester[40]: < SCO Data TX:.. flags 0x00 dlen 9 > > > HCI Event: Number of Completed P.. (0x13) plen 5 > > Num handles: 1 > > Handle: 42 Address: 00:AA:01:01:00:00 (Intel Corporation) > > Count: 1 > > #103: len 10 (40 Kb/s) > > Latency: 2 msec (2-2 msec ~2 msec) > > > > Fixes: 7fedd3bb6b77 ("Bluetooth: Prioritize SCO traffic") > > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > --- > > include/net/bluetooth/hci.h | 13 +++++++ > > include/net/bluetooth/hci_core.h | 1 + > > net/bluetooth/hci_core.c | 67 ++++++++++++++++---------------- > > net/bluetooth/hci_event.c | 2 + > > net/bluetooth/hci_sync.c | 24 ++++++++++++ > > 5 files changed, 74 insertions(+), 33 deletions(-) > > > > diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h > > index b99818df8ee7..3c8f95174fcf 100644 > > --- a/include/net/bluetooth/hci.h > > +++ b/include/net/bluetooth/hci.h > > @@ -208,6 +208,13 @@ enum { > > */ > > HCI_QUIRK_WIDEBAND_SPEECH_SUPPORTED, > > > > + /* When this quirk is set consider Sync Flow Control as supported by > > + * the driver. > > + * > > + * This quirk must be set before hci_register_dev is called. > > + */ > > + HCI_QUIRK_SYNC_FLOWCTL_SUPPORTED, > > + > > /* When this quirk is set, the LE states reported through the > > * HCI_LE_READ_SUPPORTED_STATES are invalid/broken. > > * > > @@ -448,6 +455,7 @@ enum { > > HCI_WIDEBAND_SPEECH_ENABLED, > > HCI_EVENT_FILTER_CONFIGURED, > > HCI_PA_SYNC, > > + HCI_SCO_FLOWCTL, > > > > HCI_DUT_MODE, > > HCI_VENDOR_DIAG, > > @@ -1544,6 +1552,11 @@ struct hci_rp_read_tx_power { > > __s8 tx_power; > > } __packed; > > > > +#define HCI_OP_WRITE_SYNC_FLOWCTL 0x0c2f > > +struct hci_cp_write_sync_flowctl { > > + __u8 enable; > > +} __packed; > > + > > #define HCI_OP_READ_PAGE_SCAN_TYPE 0x0c46 > > struct hci_rp_read_page_scan_type { > > __u8 status; > > diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h > > index 7966db4038cc..f78e4298e39a 100644 > > --- a/include/net/bluetooth/hci_core.h > > +++ b/include/net/bluetooth/hci_core.h > > @@ -1858,6 +1858,7 @@ void hci_conn_del_sysfs(struct hci_conn *conn); > > #define lmp_hold_capable(dev) ((dev)->features[0][0] & LMP_HOLD) > > #define lmp_sniff_capable(dev) ((dev)->features[0][0] & LMP_SNIFF) > > #define lmp_park_capable(dev) ((dev)->features[0][1] & LMP_PARK) > > +#define lmp_sco_capable(dev) ((dev)->features[0][1] & LMP_SCO) > > #define lmp_inq_rssi_capable(dev) ((dev)->features[0][3] & LMP_RSSI_INQ) > > #define lmp_esco_capable(dev) ((dev)->features[0][3] & LMP_ESCO) > > #define lmp_bredr_capable(dev) (!((dev)->features[0][4] & LMP_NO_BREDR)) > > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c > > index 012fc107901a..efba2b539902 100644 > > --- a/net/bluetooth/hci_core.c > > +++ b/net/bluetooth/hci_core.c > > @@ -3552,51 +3552,52 @@ static void __check_timeout(struct hci_dev *hdev, unsigned int cnt, u8 type) > > } > > > > /* Schedule SCO */ > > -static void hci_sched_sco(struct hci_dev *hdev) > > +static void hci_sched_sco_type(struct hci_dev *hdev, __u8 type) > > { > > struct hci_conn *conn; > > struct sk_buff *skb; > > int quote; > > + unsigned int pkts; > > > > - BT_DBG("%s", hdev->name); > > + bt_dev_dbg(hdev, "type %u", type); > > > > - if (!hci_conn_num(hdev, SCO_LINK)) > > + if (!hci_conn_num(hdev, type)) > > return; > > > > - while (hdev->sco_cnt && (conn = hci_low_sent(hdev, SCO_LINK, "e))) { > > + /* Use sco_pkts if flow control has not been enabled yet which will > > + * limit the amount of buffer sent in a row. > > + */ > > + if (!hci_dev_test_flag(hdev, HCI_SCO_FLOWCTL)) > > + pkts = hdev->sco_pkts; > > + else > > + pkts = hdev->sco_cnt; > > + > > + if (!pkts) > > + return; > > + > > + while (pkts && (conn = hci_low_sent(hdev, type, "e))) { > > while (quote-- && (skb = skb_dequeue(&conn->data_q))) { > > hci_low_sent -> hci_quote_sent uses the current value of hdev->sco_cnt > to determine `quote`, which can then be larger than pkts, possibly > causing it to underflow here. Well there is a check for pkts not underflow (< 0), or do you mean something else? > I guess this is why the other sched() functions are doing the (*cnt)-- > thing. Right, we might want to switch to do that here as well, that said perhaps hci_low_sent shouldn't be using sco_cnt if HCI_SCO_FLOWCTL is not enabled. > > > BT_DBG("skb %p len %d", skb, skb->len); > > hci_send_frame(hdev, skb); > > > > + pkts--; > > + > > + if (hdev->sco_cnt > 0) > > + hdev->sco_cnt--; > > + > > conn->sent++; > > if (conn->sent == ~0) > > conn->sent = 0; > > } > > } > > -} > > > > -static void hci_sched_esco(struct hci_dev *hdev) > > -{ > > - struct hci_conn *conn; > > - struct sk_buff *skb; > > - int quote; > > - > > - BT_DBG("%s", hdev->name); > > - > > - if (!hci_conn_num(hdev, ESCO_LINK)) > > - return; > > - > > - while (hdev->sco_cnt && (conn = hci_low_sent(hdev, ESCO_LINK, > > - "e))) { > > - while (quote-- && (skb = skb_dequeue(&conn->data_q))) { > > - BT_DBG("skb %p len %d", skb, skb->len); > > - hci_send_frame(hdev, skb); > > - > > - conn->sent++; > > - if (conn->sent == ~0) > > - conn->sent = 0; > > - } > > - } > > + /* Rescheduled if all packets were sent and flow control is not enabled > > + * as there could be more packets queued that could not be sent and > > + * since no HCI_EV_NUM_COMP_PKTS event will be generated the reschedule > > + * needs to be forced. > > + */ > > + if (!pkts && !hci_dev_test_flag(hdev, HCI_SCO_FLOWCTL)) > > + queue_work(hdev->workqueue, &hdev->tx_work); > > } > > > > static void hci_sched_acl_pkt(struct hci_dev *hdev) > > @@ -3632,8 +3633,8 @@ static void hci_sched_acl_pkt(struct hci_dev *hdev) > > chan->conn->sent++; > > > > /* Send pending SCO packets right away */ > > - hci_sched_sco(hdev); > > - hci_sched_esco(hdev); > > + hci_sched_sco_type(hdev, SCO_LINK); > > + hci_sched_sco_type(hdev, ESCO_LINK); > > } > > } > > > > @@ -3688,8 +3689,8 @@ static void hci_sched_le(struct hci_dev *hdev) > > chan->conn->sent++; > > > > /* Send pending SCO packets right away */ > > - hci_sched_sco(hdev); > > - hci_sched_esco(hdev); > > + hci_sched_sco_type(hdev, SCO_LINK); > > + hci_sched_sco_type(hdev, ESCO_LINK); > > } > > } > > > > @@ -3734,8 +3735,8 @@ static void hci_tx_work(struct work_struct *work) > > > > if (!hci_dev_test_flag(hdev, HCI_USER_CHANNEL)) { > > /* Schedule queues and send stuff to HCI driver */ > > - hci_sched_sco(hdev); > > - hci_sched_esco(hdev); > > + hci_sched_sco_type(hdev, SCO_LINK); > > + hci_sched_sco_type(hdev, ESCO_LINK); > > hci_sched_iso(hdev); > > hci_sched_acl(hdev); > > hci_sched_le(hdev); > > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c > > index 19e19c9f5e68..6d0138b778aa 100644 > > --- a/net/bluetooth/hci_event.c > > +++ b/net/bluetooth/hci_event.c > > @@ -4445,9 +4445,11 @@ static void hci_num_comp_pkts_evt(struct hci_dev *hdev, void *data, > > break; > > > > case SCO_LINK: > > + case ESCO_LINK: > > hdev->sco_cnt += count; > > if (hdev->sco_cnt > hdev->sco_pkts) > > hdev->sco_cnt = hdev->sco_pkts; > > + > > break; > > > > case ISO_LINK: > > diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c > > index c4c2cf51b219..609b035e5c90 100644 > > --- a/net/bluetooth/hci_sync.c > > +++ b/net/bluetooth/hci_sync.c > > @@ -3769,6 +3769,28 @@ static int hci_write_ca_timeout_sync(struct hci_dev *hdev) > > sizeof(param), ¶m, HCI_CMD_TIMEOUT); > > } > > > > +/* Enable SCO flow control if supported */ > > +static int hci_write_sync_flowctl_sync(struct hci_dev *hdev) > > +{ > > + struct hci_cp_write_sync_flowctl cp; > > + int err; > > + > > + /* Check if the controller supports SCO and HCI_OP_WRITE_SYNC_FLOWCTL */ > > + if (!lmp_sco_capable(hdev) || !(hdev->commands[10] & BIT(4)) || > > + !test_bit(HCI_QUIRK_SYNC_FLOWCTL_SUPPORTED, &hdev->quirks)) > > + return 0; > > + > > + memset(&cp, 0, sizeof(cp)); > > + cp.enable = 0x01; > > + > > + err = __hci_cmd_sync_status(hdev, HCI_OP_WRITE_SYNC_FLOWCTL, > > + sizeof(cp), &cp, HCI_CMD_TIMEOUT); > > + if (!err) > > + hci_dev_set_flag(hdev, HCI_SCO_FLOWCTL); > > + > > + return err; > > +} > > + > > /* BR Controller init stage 2 command sequence */ > > static const struct hci_init_stage br_init2[] = { > > /* HCI_OP_READ_BUFFER_SIZE */ > > @@ -3787,6 +3809,8 @@ static const struct hci_init_stage br_init2[] = { > > HCI_INIT(hci_clear_event_filter_sync), > > /* HCI_OP_WRITE_CA_TIMEOUT */ > > HCI_INIT(hci_write_ca_timeout_sync), > > + /* HCI_OP_WRITE_SYNC_FLOWCTL */ > > + HCI_INIT(hci_write_sync_flowctl_sync), > > {} > > }; > > >
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h index b99818df8ee7..3c8f95174fcf 100644 --- a/include/net/bluetooth/hci.h +++ b/include/net/bluetooth/hci.h @@ -208,6 +208,13 @@ enum { */ HCI_QUIRK_WIDEBAND_SPEECH_SUPPORTED, + /* When this quirk is set consider Sync Flow Control as supported by + * the driver. + * + * This quirk must be set before hci_register_dev is called. + */ + HCI_QUIRK_SYNC_FLOWCTL_SUPPORTED, + /* When this quirk is set, the LE states reported through the * HCI_LE_READ_SUPPORTED_STATES are invalid/broken. * @@ -448,6 +455,7 @@ enum { HCI_WIDEBAND_SPEECH_ENABLED, HCI_EVENT_FILTER_CONFIGURED, HCI_PA_SYNC, + HCI_SCO_FLOWCTL, HCI_DUT_MODE, HCI_VENDOR_DIAG, @@ -1544,6 +1552,11 @@ struct hci_rp_read_tx_power { __s8 tx_power; } __packed; +#define HCI_OP_WRITE_SYNC_FLOWCTL 0x0c2f +struct hci_cp_write_sync_flowctl { + __u8 enable; +} __packed; + #define HCI_OP_READ_PAGE_SCAN_TYPE 0x0c46 struct hci_rp_read_page_scan_type { __u8 status; diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index 7966db4038cc..f78e4298e39a 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -1858,6 +1858,7 @@ void hci_conn_del_sysfs(struct hci_conn *conn); #define lmp_hold_capable(dev) ((dev)->features[0][0] & LMP_HOLD) #define lmp_sniff_capable(dev) ((dev)->features[0][0] & LMP_SNIFF) #define lmp_park_capable(dev) ((dev)->features[0][1] & LMP_PARK) +#define lmp_sco_capable(dev) ((dev)->features[0][1] & LMP_SCO) #define lmp_inq_rssi_capable(dev) ((dev)->features[0][3] & LMP_RSSI_INQ) #define lmp_esco_capable(dev) ((dev)->features[0][3] & LMP_ESCO) #define lmp_bredr_capable(dev) (!((dev)->features[0][4] & LMP_NO_BREDR)) diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index 012fc107901a..efba2b539902 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -3552,51 +3552,52 @@ static void __check_timeout(struct hci_dev *hdev, unsigned int cnt, u8 type) } /* Schedule SCO */ -static void hci_sched_sco(struct hci_dev *hdev) +static void hci_sched_sco_type(struct hci_dev *hdev, __u8 type) { struct hci_conn *conn; struct sk_buff *skb; int quote; + unsigned int pkts; - BT_DBG("%s", hdev->name); + bt_dev_dbg(hdev, "type %u", type); - if (!hci_conn_num(hdev, SCO_LINK)) + if (!hci_conn_num(hdev, type)) return; - while (hdev->sco_cnt && (conn = hci_low_sent(hdev, SCO_LINK, "e))) { + /* Use sco_pkts if flow control has not been enabled yet which will + * limit the amount of buffer sent in a row. + */ + if (!hci_dev_test_flag(hdev, HCI_SCO_FLOWCTL)) + pkts = hdev->sco_pkts; + else + pkts = hdev->sco_cnt; + + if (!pkts) + return; + + while (pkts && (conn = hci_low_sent(hdev, type, "e))) { while (quote-- && (skb = skb_dequeue(&conn->data_q))) { BT_DBG("skb %p len %d", skb, skb->len); hci_send_frame(hdev, skb); + pkts--; + + if (hdev->sco_cnt > 0) + hdev->sco_cnt--; + conn->sent++; if (conn->sent == ~0) conn->sent = 0; } } -} -static void hci_sched_esco(struct hci_dev *hdev) -{ - struct hci_conn *conn; - struct sk_buff *skb; - int quote; - - BT_DBG("%s", hdev->name); - - if (!hci_conn_num(hdev, ESCO_LINK)) - return; - - while (hdev->sco_cnt && (conn = hci_low_sent(hdev, ESCO_LINK, - "e))) { - while (quote-- && (skb = skb_dequeue(&conn->data_q))) { - BT_DBG("skb %p len %d", skb, skb->len); - hci_send_frame(hdev, skb); - - conn->sent++; - if (conn->sent == ~0) - conn->sent = 0; - } - } + /* Rescheduled if all packets were sent and flow control is not enabled + * as there could be more packets queued that could not be sent and + * since no HCI_EV_NUM_COMP_PKTS event will be generated the reschedule + * needs to be forced. + */ + if (!pkts && !hci_dev_test_flag(hdev, HCI_SCO_FLOWCTL)) + queue_work(hdev->workqueue, &hdev->tx_work); } static void hci_sched_acl_pkt(struct hci_dev *hdev) @@ -3632,8 +3633,8 @@ static void hci_sched_acl_pkt(struct hci_dev *hdev) chan->conn->sent++; /* Send pending SCO packets right away */ - hci_sched_sco(hdev); - hci_sched_esco(hdev); + hci_sched_sco_type(hdev, SCO_LINK); + hci_sched_sco_type(hdev, ESCO_LINK); } } @@ -3688,8 +3689,8 @@ static void hci_sched_le(struct hci_dev *hdev) chan->conn->sent++; /* Send pending SCO packets right away */ - hci_sched_sco(hdev); - hci_sched_esco(hdev); + hci_sched_sco_type(hdev, SCO_LINK); + hci_sched_sco_type(hdev, ESCO_LINK); } } @@ -3734,8 +3735,8 @@ static void hci_tx_work(struct work_struct *work) if (!hci_dev_test_flag(hdev, HCI_USER_CHANNEL)) { /* Schedule queues and send stuff to HCI driver */ - hci_sched_sco(hdev); - hci_sched_esco(hdev); + hci_sched_sco_type(hdev, SCO_LINK); + hci_sched_sco_type(hdev, ESCO_LINK); hci_sched_iso(hdev); hci_sched_acl(hdev); hci_sched_le(hdev); diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index 19e19c9f5e68..6d0138b778aa 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -4445,9 +4445,11 @@ static void hci_num_comp_pkts_evt(struct hci_dev *hdev, void *data, break; case SCO_LINK: + case ESCO_LINK: hdev->sco_cnt += count; if (hdev->sco_cnt > hdev->sco_pkts) hdev->sco_cnt = hdev->sco_pkts; + break; case ISO_LINK: diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c index c4c2cf51b219..609b035e5c90 100644 --- a/net/bluetooth/hci_sync.c +++ b/net/bluetooth/hci_sync.c @@ -3769,6 +3769,28 @@ static int hci_write_ca_timeout_sync(struct hci_dev *hdev) sizeof(param), ¶m, HCI_CMD_TIMEOUT); } +/* Enable SCO flow control if supported */ +static int hci_write_sync_flowctl_sync(struct hci_dev *hdev) +{ + struct hci_cp_write_sync_flowctl cp; + int err; + + /* Check if the controller supports SCO and HCI_OP_WRITE_SYNC_FLOWCTL */ + if (!lmp_sco_capable(hdev) || !(hdev->commands[10] & BIT(4)) || + !test_bit(HCI_QUIRK_SYNC_FLOWCTL_SUPPORTED, &hdev->quirks)) + return 0; + + memset(&cp, 0, sizeof(cp)); + cp.enable = 0x01; + + err = __hci_cmd_sync_status(hdev, HCI_OP_WRITE_SYNC_FLOWCTL, + sizeof(cp), &cp, HCI_CMD_TIMEOUT); + if (!err) + hci_dev_set_flag(hdev, HCI_SCO_FLOWCTL); + + return err; +} + /* BR Controller init stage 2 command sequence */ static const struct hci_init_stage br_init2[] = { /* HCI_OP_READ_BUFFER_SIZE */ @@ -3787,6 +3809,8 @@ static const struct hci_init_stage br_init2[] = { HCI_INIT(hci_clear_event_filter_sync), /* HCI_OP_WRITE_CA_TIMEOUT */ HCI_INIT(hci_write_ca_timeout_sync), + /* HCI_OP_WRITE_SYNC_FLOWCTL */ + HCI_INIT(hci_write_sync_flowctl_sync), {} };