Hi Hsin-chen,
On Sun, Feb 23, 2025 at 9:26 PM Hsin-chen Chuang <chharry@xxxxxxxxxx> wrote:
>
> From: Hsin-chen Chuang <chharry@xxxxxxxxxxxx>
>
> Automatically configure the altsetting for USER_CHANNEL when a SCO is
> connected or disconnected. This adds support for the USER_CHANNEL to
> transfer SCO data over USB transport.
I guess the tracking of handles is about handling disconnections,
right? I wonder if we can get away without doing that, I didn't intend
to add a whole bunch of changes in order to switch to the right mode,
I get that you may want to disable the isochronous endpoint in case
there is no connection, but I guess that only matters if we are
talking about power but the impact is probably small so I don't think
it is worth it. There is an alternative to match the SCO/eSCO handle
via mask, like we do with ISO handles, that is probably a lot cheaper
than attempting to add a whole list for tracking handles, but it has
the downside that it is vendor/model specific.
> Fixes: b16b327edb4d ("Bluetooth: btusb: add sysfs attribute to control USB alt setting")
> Signed-off-by: Hsin-chen Chuang <chharry@xxxxxxxxxxxx>
> ---
>
> drivers/bluetooth/btusb.c | 224 +++++++++++++++++++++++++++++++-------
> 1 file changed, 186 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index de3fa725d210..dfb0918dfe98 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -854,6 +854,11 @@ struct qca_dump_info {
> #define BTUSB_ALT6_CONTINUOUS_TX 16
> #define BTUSB_HW_SSR_ACTIVE 17
>
> +struct sco_handle_list {
> + struct list_head list;
> + u16 handle;
> +};
> +
> struct btusb_data {
> struct hci_dev *hdev;
> struct usb_device *udev;
> @@ -920,6 +925,9 @@ struct btusb_data {
> int oob_wake_irq; /* irq for out-of-band wake-on-bt */
>
> struct qca_dump_info qca_dump;
> +
> + /* Records the exsiting SCO handles for HCI_USER_CHANNEL */
> + struct list_head sco_handle_list;
> };
>
> static void btusb_reset(struct hci_dev *hdev)
> @@ -1113,6 +1121,131 @@ static inline void btusb_free_frags(struct btusb_data *data)
> spin_unlock_irqrestore(&data->rxlock, flags);
> }
>
> +static void btusb_sco_handle_list_clear(struct list_head *list)
> +{
> + struct sco_handle_list *entry, *n;
> +
> + list_for_each_entry_safe(entry, n, list, list) {
> + list_del(&entry->list);
> + kfree(entry);
> + }
> +}
> +
> +static struct sco_handle_list *btusb_sco_handle_list_find(
> + struct list_head *list,
> + u16 handle)
> +{
> + struct sco_handle_list *entry;
> +
> + list_for_each_entry(entry, list, list)
> + if (entry->handle == handle)
> + return entry;
> +
> + return NULL;
> +}
> +
> +static int btusb_sco_handle_list_add(struct list_head *list, u16 handle)
> +{
> + struct sco_handle_list *entry;
> +
> + if (btusb_sco_handle_list_find(list, handle))
> + return -EEXIST;
> +
> + entry = kzalloc(sizeof(*entry), GFP_KERNEL);
> + if (!entry)
> + return -ENOMEM;
> +
> + entry->handle = handle;
> + list_add(&entry->list, list);
> +
> + return 0;
> +}
> +
> +static int btusb_sco_handle_list_del(struct list_head *list, u16 handle)
> +{
> + struct sco_handle_list *entry;
> +
> + entry = btusb_sco_handle_list_find(list, handle);
> + if (!entry)
> + return -ENOENT;
> +
> + list_del(&entry->list);
> + kfree(entry);
> +
> + return 0;
> +}
> +
> +static void btusb_sco_conn_change(struct btusb_data *data, struct sk_buff *skb)
> +{
> + struct hci_event_hdr *hdr = (void *) skb->data;
> + struct hci_dev *hdev = data->hdev;
> +
> + if (hci_skb_pkt_type(skb) != HCI_EVENT_PKT || skb->len < sizeof(*hdr))
> + return;
> +
> + switch (hdr->evt) {
> + case HCI_EV_DISCONN_COMPLETE: {
> + struct hci_ev_disconn_complete *ev =
> + (void *) skb->data + sizeof(*hdr);
> + u16 handle;
> +
> + if (skb->len != sizeof(*hdr) + sizeof(*ev) || ev->status)
> + return;
> +
> + handle = __le16_to_cpu(ev->handle);
> + if (btusb_sco_handle_list_del(&data->sco_handle_list,
> + handle) < 0)
> + return;
> +
> + bt_dev_info(hdev, "disabling SCO");
> + data->sco_num--;
> + data->air_mode = HCI_NOTIFY_DISABLE_SCO;
> + schedule_work(&data->work);
> +
> + break;
> + }
> + case HCI_EV_SYNC_CONN_COMPLETE: {
> + struct hci_ev_sync_conn_complete *ev =
> + (void *) skb->data + sizeof(*hdr);
> + unsigned int notify_air_mode;
> + u16 handle;
> +
> + if (skb->len != sizeof(*hdr) + sizeof(*ev) || ev->status)
> + return;
> +
> + switch (ev->air_mode) {
> + case BT_CODEC_CVSD:
> + notify_air_mode = HCI_NOTIFY_ENABLE_SCO_CVSD;
> + break;
> +
> + case BT_CODEC_TRANSPARENT:
> + notify_air_mode = HCI_NOTIFY_ENABLE_SCO_TRANSP;
> + break;
> +
> + default:
> + return;
> + }
> +
> + handle = __le16_to_cpu(ev->handle);
> + if (btusb_sco_handle_list_add(&data->sco_handle_list,
> + handle) < 0) {
> + bt_dev_err(hdev, "failed to add the new SCO handle");
> + return;
> + }
> +
> + bt_dev_info(hdev, "enabling SCO with air mode %u",
> + ev->air_mode);
> + data->sco_num++;
> + data->air_mode = notify_air_mode;
> + schedule_work(&data->work);
> +
> + break;
> + }
> + default:
> + break;
> + }
> +}
> +
> static int btusb_recv_event(struct btusb_data *data, struct sk_buff *skb)
> {
> if (data->intr_interval) {
> @@ -1120,6 +1253,10 @@ static int btusb_recv_event(struct btusb_data *data, struct sk_buff *skb)
> schedule_delayed_work(&data->rx_work, 0);
> }
>
> + /* Configure altsetting for HCI_USER_CHANNEL on SCO dis/connected */
> + if (hci_dev_test_flag(data->hdev, HCI_USER_CHANNEL))
> + btusb_sco_conn_change(data, skb);
I'd recommend adding a check for Kconfig/module parameter in the if
statement so btusb_sco_conn_change only runs on distros with it
enabled so we don't risk something not working as expected just
because someone decided to open the user channel.
> return data->recv_event(data->hdev, skb);
> }
>
> @@ -1919,44 +2056,6 @@ static void btusb_stop_traffic(struct btusb_data *data)
> usb_kill_anchored_urbs(&data->ctrl_anchor);
> }
>
> -static int btusb_close(struct hci_dev *hdev)
> -{
> - struct btusb_data *data = hci_get_drvdata(hdev);
> - int err;
> -
> - BT_DBG("%s", hdev->name);
> -
> - cancel_delayed_work(&data->rx_work);
> - cancel_work_sync(&data->work);
> - cancel_work_sync(&data->waker);
> -
> - skb_queue_purge(&data->acl_q);
> -
> - clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
> - clear_bit(BTUSB_BULK_RUNNING, &data->flags);
> - clear_bit(BTUSB_INTR_RUNNING, &data->flags);
> - clear_bit(BTUSB_DIAG_RUNNING, &data->flags);
> -
> - btusb_stop_traffic(data);
> - btusb_free_frags(data);
> -
> - err = usb_autopm_get_interface(data->intf);
> - if (err < 0)
> - goto failed;
> -
> - data->intf->needs_remote_wakeup = 0;
> -
> - /* Enable remote wake up for auto-suspend */
> - if (test_bit(BTUSB_WAKEUP_AUTOSUSPEND, &data->flags))
> - data->intf->needs_remote_wakeup = 1;
> -
> - usb_autopm_put_interface(data->intf);
> -
> -failed:
> - usb_scuttle_anchored_urbs(&data->deferred);
> - return 0;
> -}
> -
> static int btusb_flush(struct hci_dev *hdev)
> {
> struct btusb_data *data = hci_get_drvdata(hdev);
> @@ -2327,6 +2426,51 @@ static void btusb_work(struct work_struct *work)
> }
> }
>
> +static int btusb_close(struct hci_dev *hdev)
> +{
> + struct btusb_data *data = hci_get_drvdata(hdev);
> + int err;
> +
> + BT_DBG("%s", hdev->name);
> +
> + if (hci_dev_test_flag(hdev, HCI_USER_CHANNEL)) {
> + btusb_sco_handle_list_clear(&data->sco_handle_list);
> + data->sco_num = 0;
> + if (data->isoc_altsetting != 0)
> + btusb_switch_alt_setting(hdev, 0);
> + }
> +
> + cancel_delayed_work(&data->rx_work);
> + cancel_work_sync(&data->work);
> + cancel_work_sync(&data->waker);
> +
> + skb_queue_purge(&data->acl_q);
> +
> + clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
> + clear_bit(BTUSB_BULK_RUNNING, &data->flags);
> + clear_bit(BTUSB_INTR_RUNNING, &data->flags);
> + clear_bit(BTUSB_DIAG_RUNNING, &data->flags);
> +
> + btusb_stop_traffic(data);
> + btusb_free_frags(data);
> +
> + err = usb_autopm_get_interface(data->intf);
> + if (err < 0)
> + goto failed;
> +
> + data->intf->needs_remote_wakeup = 0;
> +
> + /* Enable remote wake up for auto-suspend */
> + if (test_bit(BTUSB_WAKEUP_AUTOSUSPEND, &data->flags))
> + data->intf->needs_remote_wakeup = 1;
> +
> + usb_autopm_put_interface(data->intf);
> +
> +failed:
> + usb_scuttle_anchored_urbs(&data->deferred);
> + return 0;
> +}
> +
> static void btusb_waker(struct work_struct *work)
> {
> struct btusb_data *data = container_of(work, struct btusb_data, waker);
> @@ -3782,6 +3926,8 @@ static int btusb_probe(struct usb_interface *intf,
> data->udev = interface_to_usbdev(intf);
> data->intf = intf;
>
> + INIT_LIST_HEAD(&data->sco_handle_list);
> +
> INIT_WORK(&data->work, btusb_work);
> INIT_WORK(&data->waker, btusb_waker);
> INIT_DELAYED_WORK(&data->rx_work, btusb_rx_work);
> @@ -4117,6 +4263,8 @@ static void btusb_disconnect(struct usb_interface *intf)
> hdev = data->hdev;
> usb_set_intfdata(data->intf, NULL);
>
> + btusb_sco_handle_list_clear(&data->sco_handle_list);
> +
> if (data->isoc) {
> device_remove_file(&intf->dev, &dev_attr_isoc_alt);
> usb_set_intfdata(data->isoc, NULL);
> --
> 2.48.1.601.g30ceb7b040-goog
>
--
Luiz Augusto von Dentz
