[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CABBYNZ+QdWQ49ZMxPLj86d=kjr7Sf38LR1PrYrPhU8kZMQuh0A@mail.gmail.com>
Date: Fri, 28 Feb 2025 02:40:37 +0500
From: Luiz Augusto von Dentz <luiz.dentz@...il.com>
To: Hsin-chen Chuang <chharry@...gle.com>
Cc: linux-bluetooth@...r.kernel.org, gregkh@...uxfoundation.org,
pmenzel@...gen.mpg.de, chromeos-bluetooth-upstreaming@...omium.org,
Hsin-chen Chuang <chharry@...omium.org>, Marcel Holtmann <marcel@...tmann.org>,
Ying Hsu <yinghsu@...omium.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] Bluetooth: btusb: Configure altsetting for HCI_USER_CHANNEL
Hi Hsin-chen,
On Thu, Feb 27, 2025 at 12:14 PM Hsin-chen Chuang <chharry@...gle.com> wrote:
>
> From: Hsin-chen Chuang <chharry@...omium.org>
>
> Automatically configure the altsetting for HCI_USER_CHANNEL when a SCO
> is connected.
>
> The motivation is to enable the HCI_USER_CHANNEL user to send out SCO
> data through USB Bluetooth chips, which is mainly used for bidirectional
> audio transfer (voice call). This was not capable because:
>
> - Per Bluetooth Core Spec v5, Vol 4, Part B, 2.1, the corresponding
> alternate setting should be set based on the air mode in order to
> transfer SCO data, but
> - The Linux Bluetooth HCI_USER_CHANNEL exposes the Bluetooth Host
> Controller Interface to the user space, which is something above the
> USB layer. The user space is not able to configure the USB alt while
> keeping the channel open.
>
> This patch intercepts the HCI_EV_SYNC_CONN_COMPLETE packets in btusb,
> extracts the air mode, and configures the alt setting in btusb.
>
> This patch is tested on ChromeOS devices. The USB Bluetooth models
> (CVSD, TRANS alt3 and alt6) could work without a customized kernel.
>
> Fixes: b16b327edb4d ("Bluetooth: btusb: add sysfs attribute to control USB alt setting")
> Signed-off-by: Hsin-chen Chuang <chharry@...omium.org>
> ---
>
> Changes in v3:
> - Remove module parameter
> - Set Kconfig to default y if CHROME_PLATFORMS
>
> Changes in v2:
> - Give up tracking the SCO handles. Only configure the altsetting when
> SCO connected.
> - Put the change behind Kconfig/module parameter
>
> drivers/bluetooth/Kconfig | 11 ++++++++++
> drivers/bluetooth/btusb.c | 43 +++++++++++++++++++++++++++++++++++++++
> 2 files changed, 54 insertions(+)
>
> diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
> index 4ab32abf0f48..cdf7a5caa5c8 100644
> --- a/drivers/bluetooth/Kconfig
> +++ b/drivers/bluetooth/Kconfig
> @@ -56,6 +56,17 @@ config BT_HCIBTUSB_POLL_SYNC
> Say Y here to enable USB poll_sync for Bluetooth USB devices by
> default.
>
> +config BT_HCIBTUSB_AUTO_SET_ISOC_ALT
> + bool "Auto set isoc_altsetting for HCI_USER_CHANNEL when SCO connected"
> + depends on BT_HCIBTUSB
> + default y if CHROME_PLATFORMS
> + help
> + Say Y here to enable auto set isoc_altsetting for HCI_USER_CHANNEL
> + when SCO connected
> +
> + When enabled, btusb intercepts the HCI_EV_SYNC_CONN_COMPLETE packets
> + and configures isoc_altsetting automatically for HCI_USER_CHANNEL.
> +
> config BT_HCIBTUSB_BCM
> bool "Broadcom protocol support"
> depends on BT_HCIBTUSB
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index de3fa725d210..2642d2ca885f 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -34,6 +34,8 @@ static bool force_scofix;
> static bool enable_autosuspend = IS_ENABLED(CONFIG_BT_HCIBTUSB_AUTOSUSPEND);
> static bool enable_poll_sync = IS_ENABLED(CONFIG_BT_HCIBTUSB_POLL_SYNC);
> static bool reset = true;
> +static bool auto_set_isoc_alt =
> + IS_ENABLED(CONFIG_BT_HCIBTUSB_AUTO_SET_ISOC_ALT);
>
> static struct usb_driver btusb_driver;
>
> @@ -1113,6 +1115,42 @@ static inline void btusb_free_frags(struct btusb_data *data)
> spin_unlock_irqrestore(&data->rxlock, flags);
> }
>
> +static void btusb_sco_connected(struct btusb_data *data, struct sk_buff *skb)
> +{
> + struct hci_event_hdr *hdr = (void *) skb->data;
> + struct hci_ev_sync_conn_complete *ev =
> + (void *) skb->data + sizeof(*hdr);
> + struct hci_dev *hdev = data->hdev;
> + unsigned int notify_air_mode;
> +
> + if (hci_skb_pkt_type(skb) != HCI_EVENT_PKT)
> + return;
> +
> + if (skb->len < sizeof(*hdr) || hdr->evt != HCI_EV_SYNC_CONN_COMPLETE)
> + return;
> +
> + 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;
> + }
> +
> + bt_dev_info(hdev, "enabling SCO with air mode %u", ev->air_mode);
> + data->sco_num = 1;
> + data->air_mode = notify_air_mode;
> + schedule_work(&data->work);
> +}
> +
> static int btusb_recv_event(struct btusb_data *data, struct sk_buff *skb)
> {
> if (data->intr_interval) {
> @@ -1120,6 +1158,11 @@ 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 connected */
> + if (auto_set_isoc_alt &&
> + hci_dev_test_flag(data->hdev, HCI_USER_CHANNEL))
> + btusb_sco_connected(data, skb);
> +
> return data->recv_event(data->hdev, skb);
> }
>
> --
> 2.48.1.658.g4767266eb4-goog
This has been merged, I did some minor rewording here and there but
the logic remains the same, now we can problem revert b16b327edb4d
("Bluetooth: btusb: add sysfs attribute to control USB alt setting")?
--
Luiz Augusto von Dentz
Powered by blists - more mailing lists