Hi Kiran,
On Tue, Aug 31, 2021 at 4:54 AM Kiran K <kiran.k@xxxxxxxxx> wrote:
>
> From: Chethan T N <chethan.tumkur.narayan@xxxxxxxxx>
>
> Currently usb tranport is not allowed to suspend when SCO over
> HCI tranport is active.
>
> This patch shall enable the usb tranport to suspend when SCO
> link use non-HCI transport
>
> Signed-off-by: Chethan T N <chethan.tumkur.narayan@xxxxxxxxx>
> Signed-off-by: Kiran K <kiran.k@xxxxxxxxx>
> ---
>
> Notes:
> changes in v13:
> - suspend usb in HFP offload use case
>
> drivers/bluetooth/btintel.c | 2 +-
> include/net/bluetooth/bluetooth.h | 4 ++++
> net/bluetooth/hci_event.c | 20 +++++++++++---------
> net/bluetooth/sco.c | 2 +-
> 4 files changed, 17 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> index 6091b691ddc2..2d64e289cf6e 100644
> --- a/drivers/bluetooth/btintel.c
> +++ b/drivers/bluetooth/btintel.c
> @@ -2215,7 +2215,7 @@ static int btintel_get_codec_config_data(struct hci_dev *hdev,
> static int btintel_get_data_path_id(struct hci_dev *hdev, __u8 *data_path_id)
> {
> /* Intel uses 1 as data path id for all the usecases */
> - *data_path_id = 1;
> + *data_path_id = BT_SCO_PCM_PATH;
> return 0;
> }
>
> diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> index c1fa90fb7502..9e2745863b33 100644
> --- a/include/net/bluetooth/bluetooth.h
> +++ b/include/net/bluetooth/bluetooth.h
> @@ -177,6 +177,10 @@ struct bt_codecs {
> #define CODING_FORMAT_TRANSPARENT 0x03
> #define CODING_FORMAT_MSBC 0x05
>
> +/* Audio data transport path used for SCO */
> +#define BT_SCO_HCI_PATH 0x00
> +#define BT_SCO_PCM_PATH 0x01
If this is in fact vendor specific perhaps you should be declared in
btintel.h not here.
> +
> __printf(1, 2)
> void bt_info(const char *fmt, ...);
> __printf(1, 2)
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index b48e24629fa4..7ff11cba89cf 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -4516,15 +4516,17 @@ static void hci_sync_conn_complete_evt(struct hci_dev *hdev,
>
> bt_dev_dbg(hdev, "SCO connected with air mode: %02x", ev->air_mode);
>
> - switch (ev->air_mode) {
> - case 0x02:
> - if (hdev->notify)
> - hdev->notify(hdev, HCI_NOTIFY_ENABLE_SCO_CVSD);
> - break;
> - case 0x03:
> - if (hdev->notify)
> - hdev->notify(hdev, HCI_NOTIFY_ENABLE_SCO_TRANSP);
> - break;
> + if (conn->codec.data_path == BT_SCO_HCI_PATH) {
> + switch (ev->air_mode) {
> + case 0x02:
> + if (hdev->notify)
> + hdev->notify(hdev, HCI_NOTIFY_ENABLE_SCO_CVSD);
> + break;
> + case 0x03:
> + if (hdev->notify)
> + hdev->notify(hdev, HCI_NOTIFY_ENABLE_SCO_TRANSP);
> + break;
> + }
Hmm I think we might need to notify the driver to enable PCM routing
so the likes of btusb can call
usb_disable_endpoint/usb_enable_endpoint for example since in theory
userspace may choose to switch from software to hardware offload and
vice-versa, note without calling usb_disable_endpoint there might not
be much power saving after all since the endpoint will remain active
or do we actually have a good explanation why it shall not be called
when using PCM routing? Note that usb_set_interface will call
usb_enable_interface that will then call usb_enable_endpoint so
perhaps we need to call usb_disable_interface, either way we can't
assume the platform will be restricted to only use one or the other.
> }
>
> hci_connect_cfm(conn, ev->status);
> diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> index 004bce2b5eca..f35c12ca6aa5 100644
> --- a/net/bluetooth/sco.c
> +++ b/net/bluetooth/sco.c
> @@ -506,7 +506,7 @@ static struct sock *sco_sock_alloc(struct net *net, struct socket *sock,
> sco_pi(sk)->codec.id = CODING_FORMAT_CVSD;
> sco_pi(sk)->codec.cid = 0xffff;
> sco_pi(sk)->codec.vid = 0xffff;
> - sco_pi(sk)->codec.data_path = 0x00;
> + sco_pi(sk)->codec.data_path = BT_SCO_HCI_PATH;
>
> bt_sock_link(&sco_sk_list, sk);
> return sk;
> --
> 2.17.1
>
--
Luiz Augusto von Dentz
