Hi Pauli,
On Thu, Feb 20, 2025 at 4:57 PM Luiz Augusto von Dentz
<luiz.dentz@xxxxxxxxx> wrote:
>
> From: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx>
>
> 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@xxxxxxxxx>
> ---
> include/net/bluetooth/hci.h | 6 ++++++
> include/net/bluetooth/hci_core.h | 1 +
> net/bluetooth/hci_core.c | 15 +++++++++++++++
> net/bluetooth/hci_sync.c | 23 +++++++++++++++++++++++
> 4 files changed, 45 insertions(+)
>
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 0d51970d809f..a6a375cf97f9 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -432,6 +432,7 @@ enum {
> HCI_WIDEBAND_SPEECH_ENABLED,
> HCI_EVENT_FILTER_CONFIGURED,
> HCI_PA_SYNC,
> + HCI_SCO_FLOWCTL,
>
> HCI_DUT_MODE,
> HCI_VENDOR_DIAG,
> @@ -1528,6 +1529,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 f756fac95488..6abe1115fa92 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..a0a14fb8a496 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -3564,11 +3564,19 @@ 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;
> }
>
> static void hci_sched_esco(struct hci_dev *hdev)
> @@ -3588,11 +3596,18 @@ 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;
> }
>
> 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 dd770ef5ec36..42ee9c5e11ad 100644
> --- a/net/bluetooth/hci_sync.c
> +++ b/net/bluetooth/hci_sync.c
> @@ -3766,6 +3766,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 */
> @@ -3784,6 +3805,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
>
Let me know if this addresses your concerns, I figure this was why we
could not track packet completion of SCO/eSCO so I guess it is a plus
if we can finally enable it across all sockets.
--
Luiz Augusto von Dentz
