| Message ID | 20250304162955.149884-1-luiz.dentz@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v4] Bluetooth: hci_core: Enable buffer flow control for SCO/eSCO | expand |
Hi Pauli, On Tue, Mar 4, 2025 at 11:29 AM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > > 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 > > Fixes: 7fedd3bb6b77 ("Bluetooth: Prioritize SCO traffic") > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > --- > include/net/bluetooth/hci.h | 6 ++++++ > include/net/bluetooth/hci_core.h | 1 + > net/bluetooth/hci_core.c | 28 ++++++++++++++++++++++++++++ > net/bluetooth/hci_sync.c | 23 +++++++++++++++++++++++ > 4 files changed, 58 insertions(+) > > diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h > index b99818df8ee7..70169533c940 100644 > --- a/include/net/bluetooth/hci.h > +++ b/include/net/bluetooth/hci.h > @@ -448,6 +448,7 @@ enum { > HCI_WIDEBAND_SPEECH_ENABLED, > HCI_EVENT_FILTER_CONFIGURED, > HCI_PA_SYNC, > + HCI_SCO_FLOWCTL, > > HCI_DUT_MODE, > HCI_VENDOR_DIAG, > @@ -1544,6 +1545,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 2f320eeddfec..cf18cf65fe5e 100644 > --- a/include/net/bluetooth/hci_core.h > +++ b/include/net/bluetooth/hci_core.h > @@ -1857,6 +1857,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 e7ec12437c8b..63eec8b80ff1 100644 > --- a/net/bluetooth/hci_core.c > +++ b/net/bluetooth/hci_core.c > @@ -3564,11 +3564,25 @@ static void hci_sched_sco(struct hci_dev *hdev) > BT_DBG("skb %p len %d", skb, skb->len); > hci_send_frame(hdev, skb); > > + hdev->sco_cnt--; > conn->sent++; > if (conn->sent == ~0) > conn->sent = 0; > } > } > + > + /* Restore sco_cnt if flow control has not been enabled as > + * HCI_EV_NUM_COMP_PKTS won't be generated. > + */ > + if (!hci_dev_test_flag(hdev, HCI_SCO_FLOWCTL)) { > + hdev->sco_cnt = hdev->sco_pkts; > + > + /* As flow control is disabled force tx_work to run if there are > + * still packets left in the queue. > + */ > + if (conn && !skb_queue_empty(&conn->data_q)) > + queue_work(hdev->workqueue, &hdev->tx_work); > + } > } > > static void hci_sched_esco(struct hci_dev *hdev) > @@ -3588,11 +3602,25 @@ static void hci_sched_esco(struct hci_dev *hdev) > BT_DBG("skb %p len %d", skb, skb->len); > hci_send_frame(hdev, skb); > > + hdev->sco_cnt--; > conn->sent++; > if (conn->sent == ~0) > conn->sent = 0; > } > } > + > + /* Restore sco_cnt if flow control has not been enabled as > + * HCI_EV_NUM_COMP_PKTS won't be generated. > + */ > + if (!hci_dev_test_flag(hdev, HCI_SCO_FLOWCTL)) { > + hdev->sco_cnt = hdev->sco_pkts; > + > + /* As flow control is disabled force tx_work to run if there are > + * still packets left in the queue. > + */ > + if (!skb_queue_empty(&conn->data_q)) > + queue_work(hdev->workqueue, &hdev->tx_work); > + } > } > > static void hci_sched_acl_pkt(struct hci_dev *hdev) > diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c > index c4c2cf51b219..aaa6164fc3e3 100644 > --- a/net/bluetooth/hci_sync.c > +++ b/net/bluetooth/hci_sync.c > @@ -3769,6 +3769,27 @@ 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))) > + 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 +3808,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), > {} > }; > > -- > 2.48.1 Looks like this is working when internally, so I wonder what is going on if you don't receive NOCP on Intel controllers, that said perhaps we need some way to detect if NOCP is not being generated, perhaps via timer, then disable HCI_SCO_FLOWCTL, the issue is this perhaps could cause hiccups at the start of the stream so perhaps we could do the opposite and always start without setting HCI_SCO_FLOWCTL and only if NOCP is generated then set HCI_SCO_FLOWCTL which can then be persisted, thoughts?
Hi Luiz, ti, 2025-03-04 kello 11:38 -0500, Luiz Augusto von Dentz kirjoitti: [clip] > > Looks like this is working when internally, so I wonder what is going > on if you don't receive NOCP on Intel controllers, that said perhaps > we need some way to detect if NOCP is not being generated, perhaps via > timer, then disable HCI_SCO_FLOWCTL, the issue is this perhaps could > cause hiccups at the start of the stream so perhaps we could do the > opposite and always start without setting HCI_SCO_FLOWCTL and only if > NOCP is generated then set HCI_SCO_FLOWCTL which can then be > persisted, thoughts? That's weird. The full btmon dumps are here: https://gitlab.freedesktop.org/pvir/repros/-/raw/main/2025-03-sco-flowctl/sco-flowctl-intel-ax210.txt https://gitlab.freedesktop.org/pvir/repros/-/raw/main/2025-03-sco-flowctl/sco-flowctl-rtl8761bu.txt with Intel firmware [ 5.749140] Bluetooth: hci0: Firmware timestamp 2024.33 buildtype 1 build 81755 [ 5.749146] Bluetooth: hci0: Firmware SHA1: 0xd028ffe4 [ 5.792830] Bluetooth: hci0: Found device firmware: intel/ibt-0041-0041.sfi [ 5.792868] Bluetooth: hci0: Boot Address: 0x100800 [ 5.792870] Bluetooth: hci0: Firmware Version: 91-33.24 [ 5.792872] Bluetooth: hci0: Firmware already loaded [ 5.800638] Bluetooth: hci0: Fseq status: Success (0x00) [ 5.800644] Bluetooth: hci0: Fseq executed: 00.00.02.41 [ 5.800647] Bluetooth: hci0: Fseq BT Top: 00.00.02.41 I checked now again with v4 of the patch, and result was the same: no NOCP, so it writes SCO max packet packets and then stops. This was in KVM virtual machine on Linux with USB redirection, but I don't think that should matter as everything else works in this setup. Enabling flow control only after observing NOCP for SCO handle sounds safer. This probably needs timer, since if you write more than max SCO buffers initially, I guess it's not guaranteed you get flow control messages for each of them.
Hi, ti, 2025-03-04 kello 11:29 -0500, 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 > > Fixes: 7fedd3bb6b77 ("Bluetooth: Prioritize SCO traffic") > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > --- > include/net/bluetooth/hci.h | 6 ++++++ > include/net/bluetooth/hci_core.h | 1 + > net/bluetooth/hci_core.c | 28 ++++++++++++++++++++++++++++ > net/bluetooth/hci_sync.c | 23 +++++++++++++++++++++++ > 4 files changed, 58 insertions(+) > > diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h > index b99818df8ee7..70169533c940 100644 > --- a/include/net/bluetooth/hci.h > +++ b/include/net/bluetooth/hci.h > @@ -448,6 +448,7 @@ enum { > HCI_WIDEBAND_SPEECH_ENABLED, > HCI_EVENT_FILTER_CONFIGURED, > HCI_PA_SYNC, > + HCI_SCO_FLOWCTL, > > HCI_DUT_MODE, > HCI_VENDOR_DIAG, > @@ -1544,6 +1545,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 2f320eeddfec..cf18cf65fe5e 100644 > --- a/include/net/bluetooth/hci_core.h > +++ b/include/net/bluetooth/hci_core.h > @@ -1857,6 +1857,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 e7ec12437c8b..63eec8b80ff1 100644 > --- a/net/bluetooth/hci_core.c > +++ b/net/bluetooth/hci_core.c > @@ -3564,11 +3564,25 @@ static void hci_sched_sco(struct hci_dev *hdev) > BT_DBG("skb %p len %d", skb, skb->len); > hci_send_frame(hdev, skb); > > + hdev->sco_cnt--; > conn->sent++; > if (conn->sent == ~0) > conn->sent = 0; > } > } > + > + /* Restore sco_cnt if flow control has not been enabled as > + * HCI_EV_NUM_COMP_PKTS won't be generated. > + */ > + if (!hci_dev_test_flag(hdev, HCI_SCO_FLOWCTL)) { > + hdev->sco_cnt = hdev->sco_pkts; > + > + /* As flow control is disabled force tx_work to run if there are > + * still packets left in the queue. > + */ > + if (conn && !skb_queue_empty(&conn->data_q)) conn may be uninitialized pointer here (if sco_cnt == 0 when entering function). I think this should be doing something like if (hci_low_sent(hdev, SCO_LINK, "e)) ... otherwise some other conn than that last seen in the loop may still have data to send. > + queue_work(hdev->workqueue, &hdev->tx_work); > > + } > } > > static void hci_sched_esco(struct hci_dev *hdev) > @@ -3588,11 +3602,25 @@ static void hci_sched_esco(struct hci_dev *hdev) > BT_DBG("skb %p len %d", skb, skb->len); > hci_send_frame(hdev, skb); > > + hdev->sco_cnt--; > conn->sent++; > if (conn->sent == ~0) > conn->sent = 0; > } > } > + > + /* Restore sco_cnt if flow control has not been enabled as > + * HCI_EV_NUM_COMP_PKTS won't be generated. > + */ > + if (!hci_dev_test_flag(hdev, HCI_SCO_FLOWCTL)) { > + hdev->sco_cnt = hdev->sco_pkts; > + > + /* As flow control is disabled force tx_work to run if there are > + * still packets left in the queue. > + */ > + if (!skb_queue_empty(&conn->data_q)) Same here. > + queue_work(hdev->workqueue, &hdev->tx_work); > + } > } > > static void hci_sched_acl_pkt(struct hci_dev *hdev) > diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c > index c4c2cf51b219..aaa6164fc3e3 100644 > --- a/net/bluetooth/hci_sync.c > +++ b/net/bluetooth/hci_sync.c > @@ -3769,6 +3769,27 @@ 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))) > + 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 +3808,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 4, 2025 at 1:30 PM Pauli Virtanen <pav@iki.fi> wrote: > > Hi, > > ti, 2025-03-04 kello 11:29 -0500, 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 > > > > Fixes: 7fedd3bb6b77 ("Bluetooth: Prioritize SCO traffic") > > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > --- > > include/net/bluetooth/hci.h | 6 ++++++ > > include/net/bluetooth/hci_core.h | 1 + > > net/bluetooth/hci_core.c | 28 ++++++++++++++++++++++++++++ > > net/bluetooth/hci_sync.c | 23 +++++++++++++++++++++++ > > 4 files changed, 58 insertions(+) > > > > diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h > > index b99818df8ee7..70169533c940 100644 > > --- a/include/net/bluetooth/hci.h > > +++ b/include/net/bluetooth/hci.h > > @@ -448,6 +448,7 @@ enum { > > HCI_WIDEBAND_SPEECH_ENABLED, > > HCI_EVENT_FILTER_CONFIGURED, > > HCI_PA_SYNC, > > + HCI_SCO_FLOWCTL, > > > > HCI_DUT_MODE, > > HCI_VENDOR_DIAG, > > @@ -1544,6 +1545,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 2f320eeddfec..cf18cf65fe5e 100644 > > --- a/include/net/bluetooth/hci_core.h > > +++ b/include/net/bluetooth/hci_core.h > > @@ -1857,6 +1857,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 e7ec12437c8b..63eec8b80ff1 100644 > > --- a/net/bluetooth/hci_core.c > > +++ b/net/bluetooth/hci_core.c > > @@ -3564,11 +3564,25 @@ static void hci_sched_sco(struct hci_dev *hdev) > > BT_DBG("skb %p len %d", skb, skb->len); > > hci_send_frame(hdev, skb); > > > > + hdev->sco_cnt--; > > conn->sent++; > > if (conn->sent == ~0) > > conn->sent = 0; > > } > > } > > + > > + /* Restore sco_cnt if flow control has not been enabled as > > + * HCI_EV_NUM_COMP_PKTS won't be generated. > > + */ > > + if (!hci_dev_test_flag(hdev, HCI_SCO_FLOWCTL)) { > > + hdev->sco_cnt = hdev->sco_pkts; > > + > > + /* As flow control is disabled force tx_work to run if there are > > + * still packets left in the queue. > > + */ > > + if (conn && !skb_queue_empty(&conn->data_q)) > > conn may be uninitialized pointer here (if sco_cnt == 0 when entering > function). > > I think this should be doing something like > > if (hci_low_sent(hdev, SCO_LINK, "e)) > ... > > otherwise some other conn than that last seen in the loop may still > have data to send. There is a v5 which reworks most of the code that you had just commented since it implements the idea of only enabling HCI_SCO_FLOWCTL if we observe NOCP is generated: https://patchwork.kernel.org/project/bluetooth/patch/20250304181019.214207-1-luiz.dentz@gmail.com/ > > + queue_work(hdev->workqueue, &hdev->tx_work); > > > > + } > > } > > > > static void hci_sched_esco(struct hci_dev *hdev) > > @@ -3588,11 +3602,25 @@ static void hci_sched_esco(struct hci_dev *hdev) > > BT_DBG("skb %p len %d", skb, skb->len); > > hci_send_frame(hdev, skb); > > > > + hdev->sco_cnt--; > > conn->sent++; > > if (conn->sent == ~0) > > conn->sent = 0; > > } > > } > > + > > + /* Restore sco_cnt if flow control has not been enabled as > > + * HCI_EV_NUM_COMP_PKTS won't be generated. > > + */ > > + if (!hci_dev_test_flag(hdev, HCI_SCO_FLOWCTL)) { > > + hdev->sco_cnt = hdev->sco_pkts; > > + > > + /* As flow control is disabled force tx_work to run if there are > > + * still packets left in the queue. > > + */ > > + if (!skb_queue_empty(&conn->data_q)) > > Same here. > > > + queue_work(hdev->workqueue, &hdev->tx_work); > > + } > > } > > > > static void hci_sched_acl_pkt(struct hci_dev *hdev) > > diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c > > index c4c2cf51b219..aaa6164fc3e3 100644 > > --- a/net/bluetooth/hci_sync.c > > +++ b/net/bluetooth/hci_sync.c > > @@ -3769,6 +3769,27 @@ 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))) > > + 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 +3808,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), > > {} > > }; > > > > -- > Pauli Virtanen
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h index b99818df8ee7..70169533c940 100644 --- a/include/net/bluetooth/hci.h +++ b/include/net/bluetooth/hci.h @@ -448,6 +448,7 @@ enum { HCI_WIDEBAND_SPEECH_ENABLED, HCI_EVENT_FILTER_CONFIGURED, HCI_PA_SYNC, + HCI_SCO_FLOWCTL, HCI_DUT_MODE, HCI_VENDOR_DIAG, @@ -1544,6 +1545,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 2f320eeddfec..cf18cf65fe5e 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -1857,6 +1857,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 e7ec12437c8b..63eec8b80ff1 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -3564,11 +3564,25 @@ static void hci_sched_sco(struct hci_dev *hdev) BT_DBG("skb %p len %d", skb, skb->len); hci_send_frame(hdev, skb); + hdev->sco_cnt--; conn->sent++; if (conn->sent == ~0) conn->sent = 0; } } + + /* Restore sco_cnt if flow control has not been enabled as + * HCI_EV_NUM_COMP_PKTS won't be generated. + */ + if (!hci_dev_test_flag(hdev, HCI_SCO_FLOWCTL)) { + hdev->sco_cnt = hdev->sco_pkts; + + /* As flow control is disabled force tx_work to run if there are + * still packets left in the queue. + */ + if (conn && !skb_queue_empty(&conn->data_q)) + queue_work(hdev->workqueue, &hdev->tx_work); + } } static void hci_sched_esco(struct hci_dev *hdev) @@ -3588,11 +3602,25 @@ static void hci_sched_esco(struct hci_dev *hdev) BT_DBG("skb %p len %d", skb, skb->len); hci_send_frame(hdev, skb); + hdev->sco_cnt--; conn->sent++; if (conn->sent == ~0) conn->sent = 0; } } + + /* Restore sco_cnt if flow control has not been enabled as + * HCI_EV_NUM_COMP_PKTS won't be generated. + */ + if (!hci_dev_test_flag(hdev, HCI_SCO_FLOWCTL)) { + hdev->sco_cnt = hdev->sco_pkts; + + /* As flow control is disabled force tx_work to run if there are + * still packets left in the queue. + */ + if (!skb_queue_empty(&conn->data_q)) + queue_work(hdev->workqueue, &hdev->tx_work); + } } static void hci_sched_acl_pkt(struct hci_dev *hdev) diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c index c4c2cf51b219..aaa6164fc3e3 100644 --- a/net/bluetooth/hci_sync.c +++ b/net/bluetooth/hci_sync.c @@ -3769,6 +3769,27 @@ 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))) + 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 +3808,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), {} };