* [PATCH v5 1/2] bluetooth: add support to notify SCO air mode
@ 2019-11-29 12:10 Sathish Narsimman
2019-11-29 12:10 ` [PATCH v5 2/2] bluetooth:btusb: handle msbc audio over USB Endpoints Sathish Narsimman
2020-01-04 10:08 ` [PATCH v5 1/2] bluetooth: add support to notify SCO air mode Marcel Holtmann
0 siblings, 2 replies; 6+ messages in thread
From: Sathish Narsimman @ 2019-11-29 12:10 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Sathish Narsimman
notifying HCI_NOTIFY_CONN_ADD for SCO connection is generic in
case of msbc audio. To differnetiate SCO air mode audio introducing
HCI_NOTIFY_SCO_CVSD and HCI_NOTIFY_SCO_TRASP.
Signed-off-by: Sathish Narsimman <sathish.narasimman@intel.com>
---
include/net/bluetooth/hci.h | 3 +++
include/net/bluetooth/hci_core.h | 3 +++
net/bluetooth/hci_conn.c | 19 +++++++++++++++----
net/bluetooth/hci_event.c | 9 +++++++++
4 files changed, 30 insertions(+), 4 deletions(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 5bc1e30dedde..bfaaef1aa4d6 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -52,6 +52,9 @@
#define HCI_NOTIFY_CONN_ADD 1
#define HCI_NOTIFY_CONN_DEL 2
#define HCI_NOTIFY_VOICE_SETTING 3
+#define HCI_NOTIFY_ENABLE_SCO_CVSD 4
+#define HCI_NOTIFY_ENABLE_SCO_TRANSP 5
+#define HCI_NOTIFY_DISABLE_SCO 6
/* HCI bus types */
#define HCI_VIRTUAL 0
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index b689aceb636b..9258e19f8cc5 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1604,4 +1604,7 @@ void hci_copy_identity_address(struct hci_dev *hdev, bdaddr_t *bdaddr,
#define SCO_AIRMODE_CVSD 0x0000
#define SCO_AIRMODE_TRANSP 0x0003
+#define SCO_CODED_CVSD 0x02
+#define SCO_CODED_TRANSP 0x03
+
#endif /* __HCI_CORE_H */
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 87691404d0c6..097a179418d6 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -122,8 +122,13 @@ static void hci_conn_cleanup(struct hci_conn *conn)
hci_conn_hash_del(hdev, conn);
- if (hdev->notify)
- hdev->notify(hdev, HCI_NOTIFY_CONN_DEL);
+ if (conn->type == SCO_LINK || conn->type == ESCO_LINK) {
+ if (hdev->notify)
+ hdev->notify(hdev, HCI_NOTIFY_DISABLE_SCO);
+ } else {
+ if (hdev->notify)
+ hdev->notify(hdev, HCI_NOTIFY_CONN_DEL);
+ }
hci_conn_del_sysfs(conn);
@@ -561,8 +566,14 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst,
hci_dev_hold(hdev);
hci_conn_hash_add(hdev, conn);
- if (hdev->notify)
- hdev->notify(hdev, HCI_NOTIFY_CONN_ADD);
+
+ /* As HCI_NOTIFY_ENABLE_SCO_CVSD & HCI_NOTIFY_ENABLE_SCO_TRANSP is
+ * used to notify SCO connections during sync_conn_complete event
+ */
+ if (conn->type != SCO_LINK && conn->type != ESCO_LINK) {
+ if (hdev->notify)
+ hdev->notify(hdev, HCI_NOTIFY_CONN_ADD);
+ }
hci_conn_init_sysfs(conn);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index c1d3a303d97f..ecad696db139 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -4231,6 +4231,15 @@ static void hci_sync_conn_complete_evt(struct hci_dev *hdev,
break;
}
+ BT_DBG("sco connected with air mode: %02x", ev->air_mode);
+ if (ev->air_mode == SCO_CODED_CVSD) {
+ if (hdev->notify)
+ hdev->notify(hdev, HCI_NOTIFY_ENABLE_SCO_CVSD);
+ } else if (ev->air_mode == SCO_CODED_TRANSP) {
+ if (hdev->notify)
+ hdev->notify(hdev, HCI_NOTIFY_ENABLE_SCO_TRANSP);
+ }
+
hci_connect_cfm(conn, ev->status);
if (ev->status)
hci_conn_del(conn);
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v5 2/2] bluetooth:btusb: handle msbc audio over USB Endpoints
2019-11-29 12:10 [PATCH v5 1/2] bluetooth: add support to notify SCO air mode Sathish Narsimman
@ 2019-11-29 12:10 ` Sathish Narsimman
2019-11-29 13:38 ` Luiz Augusto von Dentz
2020-01-04 10:08 ` [PATCH v5 1/2] bluetooth: add support to notify SCO air mode Marcel Holtmann
1 sibling, 1 reply; 6+ messages in thread
From: Sathish Narsimman @ 2019-11-29 12:10 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Sathish Narasimman, Chethan T N, Hsin-Yu Chao, Amit K Bag
From: Sathish Narasimman <sathish.narasimman@intel.com>
For msbc encoded audio stream over usb transport, btusb driver
to be set to alternate settings 6 as per BT core spec 5.0. The
type of air mode is used to differenting which alt setting to be
used.
The changes are made considering some discussion over the similar
patch submitted earlier from Kuba Pawlak(link below)
https://www.spinics.net/lists/linux-bluetooth/msg64577.html
Reported-by: kbuild test robot <lkp@intel.com>
Signed-off-by: Sathish Narasimman <sathish.narasimman@intel.com>
Signed-off-by: Chethan T N <chethan.tumkur.narayan@intel.com>
Signed-off-by: Hsin-Yu Chao <hychao@chromium.org>
Signed-off-by: Amit K Bag <amit.k.bag@intel.com>
---
drivers/bluetooth/btusb.c | 157 +++++++++++++++++++++++++++++---------
1 file changed, 121 insertions(+), 36 deletions(-)
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 9b587e662b48..541048d647d4 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -483,6 +483,8 @@ struct btusb_data {
__u8 cmdreq;
unsigned int sco_num;
+ unsigned int air_mode;
+ bool usb_alt6_packet_flow;
int isoc_altsetting;
int suspend_count;
@@ -974,6 +976,42 @@ static void btusb_isoc_complete(struct urb *urb)
}
}
+static inline void __fill_isoc_descriptor_msbc(struct urb *urb, int len,
+ int mtu, bool packet_flow)
+{
+ int i, offset = 0;
+ unsigned int interval;
+
+ /* For msbc ALT 6 setting the host will send the packet at continuous
+ * flow. As per core spec 5, vol 4, part B, table 2.1. For ALT setting
+ * 6 the HCI PACKET INTERVAL should be 7.5ms for every usb packets.
+ * To maintain the rate we send 63bytes of usb packets alternatively for
+ * 7ms and 8ms to maintain the rate as 7.5ms.
+ */
+ if (packet_flow) {
+ interval = 7;
+ packet_flow = false;
+ } else {
+ interval = 6;
+ packet_flow = true;
+ }
+
+ BT_DBG("interval:%d len %d mtu %d", interval, len, mtu);
+
+ for (i = 0; i < interval; i++) {
+ urb->iso_frame_desc[i].offset = offset;
+ urb->iso_frame_desc[i].length = offset;
+ }
+
+ if (len && i < BTUSB_MAX_ISOC_FRAMES) {
+ urb->iso_frame_desc[i].offset = offset;
+ urb->iso_frame_desc[i].length = len;
+ i++;
+ }
+
+ urb->number_of_packets = i;
+}
+
static inline void __fill_isoc_descriptor(struct urb *urb, int len, int mtu)
{
int i, offset = 0;
@@ -1377,9 +1415,13 @@ static struct urb *alloc_isoc_urb(struct hci_dev *hdev, struct sk_buff *skb)
urb->transfer_flags = URB_ISO_ASAP;
- __fill_isoc_descriptor(urb, skb->len,
+ if (data->isoc_altsetting == 6)
+ __fill_isoc_descriptor_msbc(urb, skb->len,
+ le16_to_cpu(data->isoc_tx_ep->wMaxPacketSize),
+ data->usb_alt6_packet_flow);
+ else
+ __fill_isoc_descriptor(urb, skb->len,
le16_to_cpu(data->isoc_tx_ep->wMaxPacketSize));
-
skb->dev = (void *)hdev;
return urb;
@@ -1475,6 +1517,7 @@ static void btusb_notify(struct hci_dev *hdev, unsigned int evt)
if (hci_conn_num(hdev, SCO_LINK) != data->sco_num) {
data->sco_num = hci_conn_num(hdev, SCO_LINK);
+ data->air_mode = evt;
schedule_work(&data->work);
}
}
@@ -1522,11 +1565,70 @@ static inline int __set_isoc_interface(struct hci_dev *hdev, int altsetting)
return 0;
}
+static int bt_switch_alt_setting(struct hci_dev *hdev, int new_alts)
+{
+ struct btusb_data *data = hci_get_drvdata(hdev);
+ int err;
+
+ if (data->isoc_altsetting != new_alts) {
+ unsigned long flags;
+
+ clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
+ usb_kill_anchored_urbs(&data->isoc_anchor);
+
+ /* When isochronous alternate setting needs to be
+ * changed, because SCO connection has been added
+ * or removed, a packet fragment may be left in the
+ * reassembling state. This could lead to wrongly
+ * assembled fragments.
+ *
+ * Clear outstanding fragment when selecting a new
+ * alternate setting.
+ */
+ spin_lock_irqsave(&data->rxlock, flags);
+ kfree_skb(data->sco_skb);
+ data->sco_skb = NULL;
+ spin_unlock_irqrestore(&data->rxlock, flags);
+
+ err = __set_isoc_interface(hdev, new_alts);
+ if (err < 0)
+ return err;
+ }
+ if (!test_and_set_bit(BTUSB_ISOC_RUNNING, &data->flags)) {
+ if (btusb_submit_isoc_urb(hdev, GFP_KERNEL) < 0)
+ clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
+ else
+ btusb_submit_isoc_urb(hdev, GFP_KERNEL);
+ }
+
+ return 0;
+}
+
+static struct usb_host_interface *btusb_find_altsetting(struct btusb_data *data, int alt)
+{
+ struct usb_interface *intf = data->isoc;
+ int i;
+
+ BT_DBG("Looking for Alt no :%d", alt);
+
+ if (intf == NULL) {
+ BT_ERR("INterface NULL");
+ return NULL;
+ }
+
+ for (i = 0; i < intf->num_altsetting; i++) {
+ if (intf->altsetting[i].desc.bAlternateSetting == alt)
+ return &intf->altsetting[i];
+ }
+
+ return NULL;
+}
+
static void btusb_work(struct work_struct *work)
{
struct btusb_data *data = container_of(work, struct btusb_data, work);
struct hci_dev *hdev = data->hdev;
- int new_alts;
+ int new_alts = 0;
int err;
if (data->sco_num > 0) {
@@ -1541,44 +1643,27 @@ static void btusb_work(struct work_struct *work)
set_bit(BTUSB_DID_ISO_RESUME, &data->flags);
}
- if (hdev->voice_setting & 0x0020) {
- static const int alts[3] = { 2, 4, 5 };
-
- new_alts = alts[data->sco_num - 1];
- } else {
- new_alts = data->sco_num;
- }
-
- if (data->isoc_altsetting != new_alts) {
- unsigned long flags;
+ if (data->air_mode == HCI_NOTIFY_ENABLE_SCO_CVSD) {
+ if (hdev->voice_setting & 0x0020) {
+ static const int alts[3] = { 2, 4, 5 };
- clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
- usb_kill_anchored_urbs(&data->isoc_anchor);
-
- /* When isochronous alternate setting needs to be
- * changed, because SCO connection has been added
- * or removed, a packet fragment may be left in the
- * reassembling state. This could lead to wrongly
- * assembled fragments.
- *
- * Clear outstanding fragment when selecting a new
- * alternate setting.
- */
- spin_lock_irqsave(&data->rxlock, flags);
- kfree_skb(data->sco_skb);
- data->sco_skb = NULL;
- spin_unlock_irqrestore(&data->rxlock, flags);
+ new_alts = alts[data->sco_num - 1];
+ } else {
+ new_alts = data->sco_num;
+ }
+ } else if (data->air_mode == HCI_NOTIFY_ENABLE_SCO_TRANSP) {
- if (__set_isoc_interface(hdev, new_alts) < 0)
- return;
- }
+ data->usb_alt6_packet_flow = true;
- if (!test_and_set_bit(BTUSB_ISOC_RUNNING, &data->flags)) {
- if (btusb_submit_isoc_urb(hdev, GFP_KERNEL) < 0)
- clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
+ /* Check if Alt 6 is supported for Transparent audio*/
+ if (btusb_find_altsetting(data, 6))
+ new_alts = 6;
else
- btusb_submit_isoc_urb(hdev, GFP_KERNEL);
+ BT_ERR("%s Device does not support ALT setting 6", hdev->name);
}
+
+ if (bt_switch_alt_setting(hdev, new_alts) < 0)
+ BT_ERR("%s Set USB Alt: %d failed!", hdev->name, new_alts);
} else {
clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
usb_kill_anchored_urbs(&data->isoc_anchor);
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v5 2/2] bluetooth:btusb: handle msbc audio over USB Endpoints
2019-11-29 12:10 ` [PATCH v5 2/2] bluetooth:btusb: handle msbc audio over USB Endpoints Sathish Narsimman
@ 2019-11-29 13:38 ` Luiz Augusto von Dentz
2019-11-29 13:58 ` Sathish Narasimman
0 siblings, 1 reply; 6+ messages in thread
From: Luiz Augusto von Dentz @ 2019-11-29 13:38 UTC (permalink / raw)
To: Sathish Narsimman
Cc: linux-bluetooth, Sathish Narasimman, Chethan T N, Hsin-Yu Chao,
Amit K Bag
Hi Sathish,
On Fri, Nov 29, 2019 at 2:16 PM Sathish Narsimman <nsathish41@gmail.com> wrote:
>
> From: Sathish Narasimman <sathish.narasimman@intel.com>
>
> For msbc encoded audio stream over usb transport, btusb driver
> to be set to alternate settings 6 as per BT core spec 5.0. The
> type of air mode is used to differenting which alt setting to be
> used.
> The changes are made considering some discussion over the similar
> patch submitted earlier from Kuba Pawlak(link below)
> https://www.spinics.net/lists/linux-bluetooth/msg64577.html
>
> Reported-by: kbuild test robot <lkp@intel.com>
> Signed-off-by: Sathish Narasimman <sathish.narasimman@intel.com>
> Signed-off-by: Chethan T N <chethan.tumkur.narayan@intel.com>
> Signed-off-by: Hsin-Yu Chao <hychao@chromium.org>
> Signed-off-by: Amit K Bag <amit.k.bag@intel.com>
> ---
> drivers/bluetooth/btusb.c | 157 +++++++++++++++++++++++++++++---------
> 1 file changed, 121 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 9b587e662b48..541048d647d4 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -483,6 +483,8 @@ struct btusb_data {
> __u8 cmdreq;
>
> unsigned int sco_num;
> + unsigned int air_mode;
> + bool usb_alt6_packet_flow;
> int isoc_altsetting;
> int suspend_count;
>
> @@ -974,6 +976,42 @@ static void btusb_isoc_complete(struct urb *urb)
> }
> }
>
> +static inline void __fill_isoc_descriptor_msbc(struct urb *urb, int len,
> + int mtu, bool packet_flow)
Well I guess is really a role for alt6 since afaik mSBC can be sent in
other settings as as well, besides we should never assume a specific
codec is in use since the userspace can use anything with transparent.
> +{
> + int i, offset = 0;
> + unsigned int interval;
> +
> + /* For msbc ALT 6 setting the host will send the packet at continuous
> + * flow. As per core spec 5, vol 4, part B, table 2.1. For ALT setting
> + * 6 the HCI PACKET INTERVAL should be 7.5ms for every usb packets.
> + * To maintain the rate we send 63bytes of usb packets alternatively for
> + * 7ms and 8ms to maintain the rate as 7.5ms.
> + */
> + if (packet_flow) {
> + interval = 7;
> + packet_flow = false;
> + } else {
> + interval = 6;
> + packet_flow = true;
> + }
> +
> + BT_DBG("interval:%d len %d mtu %d", interval, len, mtu);
> +
> + for (i = 0; i < interval; i++) {
> + urb->iso_frame_desc[i].offset = offset;
> + urb->iso_frame_desc[i].length = offset;
> + }
> +
> + if (len && i < BTUSB_MAX_ISOC_FRAMES) {
> + urb->iso_frame_desc[i].offset = offset;
> + urb->iso_frame_desc[i].length = len;
> + i++;
> + }
> +
> + urb->number_of_packets = i;
> +}
> +
> static inline void __fill_isoc_descriptor(struct urb *urb, int len, int mtu)
> {
> int i, offset = 0;
> @@ -1377,9 +1415,13 @@ static struct urb *alloc_isoc_urb(struct hci_dev *hdev, struct sk_buff *skb)
>
> urb->transfer_flags = URB_ISO_ASAP;
>
> - __fill_isoc_descriptor(urb, skb->len,
> + if (data->isoc_altsetting == 6)
> + __fill_isoc_descriptor_msbc(urb, skb->len,
> + le16_to_cpu(data->isoc_tx_ep->wMaxPacketSize),
> + data->usb_alt6_packet_flow);
> + else
> + __fill_isoc_descriptor(urb, skb->len,
> le16_to_cpu(data->isoc_tx_ep->wMaxPacketSize));
> -
> skb->dev = (void *)hdev;
>
> return urb;
> @@ -1475,6 +1517,7 @@ static void btusb_notify(struct hci_dev *hdev, unsigned int evt)
>
> if (hci_conn_num(hdev, SCO_LINK) != data->sco_num) {
> data->sco_num = hci_conn_num(hdev, SCO_LINK);
> + data->air_mode = evt;
> schedule_work(&data->work);
> }
> }
> @@ -1522,11 +1565,70 @@ static inline int __set_isoc_interface(struct hci_dev *hdev, int altsetting)
> return 0;
> }
>
> +static int bt_switch_alt_setting(struct hci_dev *hdev, int new_alts)
> +{
> + struct btusb_data *data = hci_get_drvdata(hdev);
> + int err;
> +
> + if (data->isoc_altsetting != new_alts) {
> + unsigned long flags;
> +
> + clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
> + usb_kill_anchored_urbs(&data->isoc_anchor);
> +
> + /* When isochronous alternate setting needs to be
> + * changed, because SCO connection has been added
> + * or removed, a packet fragment may be left in the
> + * reassembling state. This could lead to wrongly
> + * assembled fragments.
> + *
> + * Clear outstanding fragment when selecting a new
> + * alternate setting.
> + */
> + spin_lock_irqsave(&data->rxlock, flags);
> + kfree_skb(data->sco_skb);
> + data->sco_skb = NULL;
> + spin_unlock_irqrestore(&data->rxlock, flags);
> +
> + err = __set_isoc_interface(hdev, new_alts);
> + if (err < 0)
> + return err;
> + }
> + if (!test_and_set_bit(BTUSB_ISOC_RUNNING, &data->flags)) {
> + if (btusb_submit_isoc_urb(hdev, GFP_KERNEL) < 0)
> + clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
> + else
> + btusb_submit_isoc_urb(hdev, GFP_KERNEL);
> + }
> +
> + return 0;
> +}
> +
> +static struct usb_host_interface *btusb_find_altsetting(struct btusb_data *data, int alt)
> +{
> + struct usb_interface *intf = data->isoc;
> + int i;
> +
> + BT_DBG("Looking for Alt no :%d", alt);
> +
> + if (intf == NULL) {
> + BT_ERR("INterface NULL");
> + return NULL;
> + }
> +
> + for (i = 0; i < intf->num_altsetting; i++) {
> + if (intf->altsetting[i].desc.bAlternateSetting == alt)
> + return &intf->altsetting[i];
> + }
> +
> + return NULL;
> +}
> +
> static void btusb_work(struct work_struct *work)
> {
> struct btusb_data *data = container_of(work, struct btusb_data, work);
> struct hci_dev *hdev = data->hdev;
> - int new_alts;
> + int new_alts = 0;
> int err;
>
> if (data->sco_num > 0) {
> @@ -1541,44 +1643,27 @@ static void btusb_work(struct work_struct *work)
> set_bit(BTUSB_DID_ISO_RESUME, &data->flags);
> }
>
> - if (hdev->voice_setting & 0x0020) {
> - static const int alts[3] = { 2, 4, 5 };
> -
> - new_alts = alts[data->sco_num - 1];
> - } else {
> - new_alts = data->sco_num;
> - }
> -
> - if (data->isoc_altsetting != new_alts) {
> - unsigned long flags;
> + if (data->air_mode == HCI_NOTIFY_ENABLE_SCO_CVSD) {
> + if (hdev->voice_setting & 0x0020) {
> + static const int alts[3] = { 2, 4, 5 };
>
> - clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
> - usb_kill_anchored_urbs(&data->isoc_anchor);
> -
> - /* When isochronous alternate setting needs to be
> - * changed, because SCO connection has been added
> - * or removed, a packet fragment may be left in the
> - * reassembling state. This could lead to wrongly
> - * assembled fragments.
> - *
> - * Clear outstanding fragment when selecting a new
> - * alternate setting.
> - */
> - spin_lock_irqsave(&data->rxlock, flags);
> - kfree_skb(data->sco_skb);
> - data->sco_skb = NULL;
> - spin_unlock_irqrestore(&data->rxlock, flags);
> + new_alts = alts[data->sco_num - 1];
> + } else {
> + new_alts = data->sco_num;
> + }
> + } else if (data->air_mode == HCI_NOTIFY_ENABLE_SCO_TRANSP) {
>
> - if (__set_isoc_interface(hdev, new_alts) < 0)
> - return;
> - }
> + data->usb_alt6_packet_flow = true;
>
> - if (!test_and_set_bit(BTUSB_ISOC_RUNNING, &data->flags)) {
> - if (btusb_submit_isoc_urb(hdev, GFP_KERNEL) < 0)
> - clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
> + /* Check if Alt 6 is supported for Transparent audio*/
> + if (btusb_find_altsetting(data, 6))
> + new_alts = 6;
> else
> - btusb_submit_isoc_urb(hdev, GFP_KERNEL);
> + BT_ERR("%s Device does not support ALT setting 6", hdev->name);
> }
> +
> + if (bt_switch_alt_setting(hdev, new_alts) < 0)
> + BT_ERR("%s Set USB Alt: %d failed!", hdev->name, new_alts);
> } else {
> clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
> usb_kill_anchored_urbs(&data->isoc_anchor);
> --
> 2.17.1
>
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v5 2/2] bluetooth:btusb: handle msbc audio over USB Endpoints
2019-11-29 13:38 ` Luiz Augusto von Dentz
@ 2019-11-29 13:58 ` Sathish Narasimman
0 siblings, 0 replies; 6+ messages in thread
From: Sathish Narasimman @ 2019-11-29 13:58 UTC (permalink / raw)
To: Luiz Augusto von Dentz
Cc: linux-bluetooth, Sathish Narasimman, Chethan T N, Hsin-Yu Chao,
Amit K Bag
Hi Luiz,
On Fri, Nov 29, 2019 at 7:08 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Sathish,
>
> On Fri, Nov 29, 2019 at 2:16 PM Sathish Narsimman <nsathish41@gmail.com> wrote:
> >
> > From: Sathish Narasimman <sathish.narasimman@intel.com>
> >
> > For msbc encoded audio stream over usb transport, btusb driver
> > to be set to alternate settings 6 as per BT core spec 5.0. The
> > type of air mode is used to differenting which alt setting to be
> > used.
> > The changes are made considering some discussion over the similar
> > patch submitted earlier from Kuba Pawlak(link below)
> > https://www.spinics.net/lists/linux-bluetooth/msg64577.html
> >
> > Reported-by: kbuild test robot <lkp@intel.com>
> > Signed-off-by: Sathish Narasimman <sathish.narasimman@intel.com>
> > Signed-off-by: Chethan T N <chethan.tumkur.narayan@intel.com>
> > Signed-off-by: Hsin-Yu Chao <hychao@chromium.org>
> > Signed-off-by: Amit K Bag <amit.k.bag@intel.com>
> > ---
> > drivers/bluetooth/btusb.c | 157 +++++++++++++++++++++++++++++---------
> > 1 file changed, 121 insertions(+), 36 deletions(-)
> >
> > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> > index 9b587e662b48..541048d647d4 100644
> > --- a/drivers/bluetooth/btusb.c
> > +++ b/drivers/bluetooth/btusb.c
> > @@ -483,6 +483,8 @@ struct btusb_data {
> > __u8 cmdreq;
> >
> > unsigned int sco_num;
> > + unsigned int air_mode;
> > + bool usb_alt6_packet_flow;
> > int isoc_altsetting;
> > int suspend_count;
> >
> > @@ -974,6 +976,42 @@ static void btusb_isoc_complete(struct urb *urb)
> > }
> > }
> >
> > +static inline void __fill_isoc_descriptor_msbc(struct urb *urb, int len,
> > + int mtu, bool packet_flow)
>
> Well I guess is really a role for alt6 since afaik mSBC can be sent in
> other settings as as well, besides we should never assume a specific
> codec is in use since the userspace can use anything with transparent.
I agree the transparent can use any codec.
The patch created considering HFP spec 1.7 which says msbc codec for
WBS(Though we can use any codec)
Also in core spec 5, vol4, part B - Table 2.1(usb primary firmware
interface and endpoint settings)
POinted to this - > one mSBC voice channel ALT setting 6.
yes, we can achieve the same rate(WBS) using ALT setting 1. But the
usage of ALT 6 is decided from the spec.
>
> > +{
> > + int i, offset = 0;
> > + unsigned int interval;
> > +
> > + /* For msbc ALT 6 setting the host will send the packet at continuous
> > + * flow. As per core spec 5, vol 4, part B, table 2.1. For ALT setting
> > + * 6 the HCI PACKET INTERVAL should be 7.5ms for every usb packets.
> > + * To maintain the rate we send 63bytes of usb packets alternatively for
> > + * 7ms and 8ms to maintain the rate as 7.5ms.
> > + */
> > + if (packet_flow) {
> > + interval = 7;
> > + packet_flow = false;
> > + } else {
> > + interval = 6;
> > + packet_flow = true;
> > + }
> > +
> > + BT_DBG("interval:%d len %d mtu %d", interval, len, mtu);
> > +
> > + for (i = 0; i < interval; i++) {
> > + urb->iso_frame_desc[i].offset = offset;
> > + urb->iso_frame_desc[i].length = offset;
> > + }
> > +
> > + if (len && i < BTUSB_MAX_ISOC_FRAMES) {
> > + urb->iso_frame_desc[i].offset = offset;
> > + urb->iso_frame_desc[i].length = len;
> > + i++;
> > + }
> > +
> > + urb->number_of_packets = i;
> > +}
> > +
> > static inline void __fill_isoc_descriptor(struct urb *urb, int len, int mtu)
> > {
> > int i, offset = 0;
> > @@ -1377,9 +1415,13 @@ static struct urb *alloc_isoc_urb(struct hci_dev *hdev, struct sk_buff *skb)
> >
> > urb->transfer_flags = URB_ISO_ASAP;
> >
> > - __fill_isoc_descriptor(urb, skb->len,
> > + if (data->isoc_altsetting == 6)
> > + __fill_isoc_descriptor_msbc(urb, skb->len,
> > + le16_to_cpu(data->isoc_tx_ep->wMaxPacketSize),
> > + data->usb_alt6_packet_flow);
> > + else
> > + __fill_isoc_descriptor(urb, skb->len,
> > le16_to_cpu(data->isoc_tx_ep->wMaxPacketSize));
> > -
> > skb->dev = (void *)hdev;
> >
> > return urb;
> > @@ -1475,6 +1517,7 @@ static void btusb_notify(struct hci_dev *hdev, unsigned int evt)
> >
> > if (hci_conn_num(hdev, SCO_LINK) != data->sco_num) {
> > data->sco_num = hci_conn_num(hdev, SCO_LINK);
> > + data->air_mode = evt;
> > schedule_work(&data->work);
> > }
> > }
> > @@ -1522,11 +1565,70 @@ static inline int __set_isoc_interface(struct hci_dev *hdev, int altsetting)
> > return 0;
> > }
> >
> > +static int bt_switch_alt_setting(struct hci_dev *hdev, int new_alts)
> > +{
> > + struct btusb_data *data = hci_get_drvdata(hdev);
> > + int err;
> > +
> > + if (data->isoc_altsetting != new_alts) {
> > + unsigned long flags;
> > +
> > + clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
> > + usb_kill_anchored_urbs(&data->isoc_anchor);
> > +
> > + /* When isochronous alternate setting needs to be
> > + * changed, because SCO connection has been added
> > + * or removed, a packet fragment may be left in the
> > + * reassembling state. This could lead to wrongly
> > + * assembled fragments.
> > + *
> > + * Clear outstanding fragment when selecting a new
> > + * alternate setting.
> > + */
> > + spin_lock_irqsave(&data->rxlock, flags);
> > + kfree_skb(data->sco_skb);
> > + data->sco_skb = NULL;
> > + spin_unlock_irqrestore(&data->rxlock, flags);
> > +
> > + err = __set_isoc_interface(hdev, new_alts);
> > + if (err < 0)
> > + return err;
> > + }
> > + if (!test_and_set_bit(BTUSB_ISOC_RUNNING, &data->flags)) {
> > + if (btusb_submit_isoc_urb(hdev, GFP_KERNEL) < 0)
> > + clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
> > + else
> > + btusb_submit_isoc_urb(hdev, GFP_KERNEL);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static struct usb_host_interface *btusb_find_altsetting(struct btusb_data *data, int alt)
> > +{
> > + struct usb_interface *intf = data->isoc;
> > + int i;
> > +
> > + BT_DBG("Looking for Alt no :%d", alt);
> > +
> > + if (intf == NULL) {
> > + BT_ERR("INterface NULL");
> > + return NULL;
> > + }
> > +
> > + for (i = 0; i < intf->num_altsetting; i++) {
> > + if (intf->altsetting[i].desc.bAlternateSetting == alt)
> > + return &intf->altsetting[i];
> > + }
> > +
> > + return NULL;
> > +}
> > +
> > static void btusb_work(struct work_struct *work)
> > {
> > struct btusb_data *data = container_of(work, struct btusb_data, work);
> > struct hci_dev *hdev = data->hdev;
> > - int new_alts;
> > + int new_alts = 0;
> > int err;
> >
> > if (data->sco_num > 0) {
> > @@ -1541,44 +1643,27 @@ static void btusb_work(struct work_struct *work)
> > set_bit(BTUSB_DID_ISO_RESUME, &data->flags);
> > }
> >
> > - if (hdev->voice_setting & 0x0020) {
> > - static const int alts[3] = { 2, 4, 5 };
> > -
> > - new_alts = alts[data->sco_num - 1];
> > - } else {
> > - new_alts = data->sco_num;
> > - }
> > -
> > - if (data->isoc_altsetting != new_alts) {
> > - unsigned long flags;
> > + if (data->air_mode == HCI_NOTIFY_ENABLE_SCO_CVSD) {
> > + if (hdev->voice_setting & 0x0020) {
> > + static const int alts[3] = { 2, 4, 5 };
> >
> > - clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
> > - usb_kill_anchored_urbs(&data->isoc_anchor);
> > -
> > - /* When isochronous alternate setting needs to be
> > - * changed, because SCO connection has been added
> > - * or removed, a packet fragment may be left in the
> > - * reassembling state. This could lead to wrongly
> > - * assembled fragments.
> > - *
> > - * Clear outstanding fragment when selecting a new
> > - * alternate setting.
> > - */
> > - spin_lock_irqsave(&data->rxlock, flags);
> > - kfree_skb(data->sco_skb);
> > - data->sco_skb = NULL;
> > - spin_unlock_irqrestore(&data->rxlock, flags);
> > + new_alts = alts[data->sco_num - 1];
> > + } else {
> > + new_alts = data->sco_num;
> > + }
> > + } else if (data->air_mode == HCI_NOTIFY_ENABLE_SCO_TRANSP) {
> >
> > - if (__set_isoc_interface(hdev, new_alts) < 0)
> > - return;
> > - }
> > + data->usb_alt6_packet_flow = true;
> >
> > - if (!test_and_set_bit(BTUSB_ISOC_RUNNING, &data->flags)) {
> > - if (btusb_submit_isoc_urb(hdev, GFP_KERNEL) < 0)
> > - clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
> > + /* Check if Alt 6 is supported for Transparent audio*/
> > + if (btusb_find_altsetting(data, 6))
> > + new_alts = 6;
> > else
> > - btusb_submit_isoc_urb(hdev, GFP_KERNEL);
> > + BT_ERR("%s Device does not support ALT setting 6", hdev->name);
> > }
> > +
> > + if (bt_switch_alt_setting(hdev, new_alts) < 0)
> > + BT_ERR("%s Set USB Alt: %d failed!", hdev->name, new_alts);
> > } else {
> > clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
> > usb_kill_anchored_urbs(&data->isoc_anchor);
> > --
> > 2.17.1
> >
>
>
> --
> Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v5 1/2] bluetooth: add support to notify SCO air mode
2019-11-29 12:10 [PATCH v5 1/2] bluetooth: add support to notify SCO air mode Sathish Narsimman
2019-11-29 12:10 ` [PATCH v5 2/2] bluetooth:btusb: handle msbc audio over USB Endpoints Sathish Narsimman
@ 2020-01-04 10:08 ` Marcel Holtmann
2020-04-02 7:55 ` Sathish Narasimman
1 sibling, 1 reply; 6+ messages in thread
From: Marcel Holtmann @ 2020-01-04 10:08 UTC (permalink / raw)
To: Sathish Narsimman; +Cc: BlueZ devel list, Sathish Narsimman
Hi Satish,
> notifying HCI_NOTIFY_CONN_ADD for SCO connection is generic in
> case of msbc audio. To differnetiate SCO air mode audio introducing
> HCI_NOTIFY_SCO_CVSD and HCI_NOTIFY_SCO_TRASP.
>
> Signed-off-by: Sathish Narsimman <sathish.narasimman@intel.com>
> ---
> include/net/bluetooth/hci.h | 3 +++
> include/net/bluetooth/hci_core.h | 3 +++
> net/bluetooth/hci_conn.c | 19 +++++++++++++++----
> net/bluetooth/hci_event.c | 9 +++++++++
> 4 files changed, 30 insertions(+), 4 deletions(-)
>
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 5bc1e30dedde..bfaaef1aa4d6 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -52,6 +52,9 @@
> #define HCI_NOTIFY_CONN_ADD 1
> #define HCI_NOTIFY_CONN_DEL 2
> #define HCI_NOTIFY_VOICE_SETTING 3
> +#define HCI_NOTIFY_ENABLE_SCO_CVSD 4
> +#define HCI_NOTIFY_ENABLE_SCO_TRANSP 5
> +#define HCI_NOTIFY_DISABLE_SCO 6
>
> /* HCI bus types */
> #define HCI_VIRTUAL 0
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index b689aceb636b..9258e19f8cc5 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -1604,4 +1604,7 @@ void hci_copy_identity_address(struct hci_dev *hdev, bdaddr_t *bdaddr,
> #define SCO_AIRMODE_CVSD 0x0000
> #define SCO_AIRMODE_TRANSP 0x0003
>
> +#define SCO_CODED_CVSD 0x02
> +#define SCO_CODED_TRANSP 0x03
> +
why is this needed. And more important, what is wrong with SCO_AIRMODE_CVSD above?
> #endif /* __HCI_CORE_H */
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 87691404d0c6..097a179418d6 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -122,8 +122,13 @@ static void hci_conn_cleanup(struct hci_conn *conn)
>
> hci_conn_hash_del(hdev, conn);
>
> - if (hdev->notify)
> - hdev->notify(hdev, HCI_NOTIFY_CONN_DEL);
> + if (conn->type == SCO_LINK || conn->type == ESCO_LINK) {
> + if (hdev->notify)
> + hdev->notify(hdev, HCI_NOTIFY_DISABLE_SCO);
> + } else {
> + if (hdev->notify)
> + hdev->notify(hdev, HCI_NOTIFY_CONN_DEL);
> + }
>
> hci_conn_del_sysfs(conn);
>
> @@ -561,8 +566,14 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst,
> hci_dev_hold(hdev);
>
> hci_conn_hash_add(hdev, conn);
> - if (hdev->notify)
> - hdev->notify(hdev, HCI_NOTIFY_CONN_ADD);
> +
> + /* As HCI_NOTIFY_ENABLE_SCO_CVSD & HCI_NOTIFY_ENABLE_SCO_TRANSP is
> + * used to notify SCO connections during sync_conn_complete event
> + */
> + if (conn->type != SCO_LINK && conn->type != ESCO_LINK) {
> + if (hdev->notify)
> + hdev->notify(hdev, HCI_NOTIFY_CONN_ADD);
> + }
>
> hci_conn_init_sysfs(conn);
>
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index c1d3a303d97f..ecad696db139 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -4231,6 +4231,15 @@ static void hci_sync_conn_complete_evt(struct hci_dev *hdev,
> break;
> }
>
> + BT_DBG("sco connected with air mode: %02x", ev->air_mode);
> + if (ev->air_mode == SCO_CODED_CVSD) {
> + if (hdev->notify)
> + hdev->notify(hdev, HCI_NOTIFY_ENABLE_SCO_CVSD);
> + } else if (ev->air_mode == SCO_CODED_TRANSP) {
> + if (hdev->notify)
> + hdev->notify(hdev, HCI_NOTIFY_ENABLE_SCO_TRANSP);
> + }
> +
You are however forgetting the old hci_conn_complete for legacy SCO setup.
Regards
Marcel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v5 1/2] bluetooth: add support to notify SCO air mode
2020-01-04 10:08 ` [PATCH v5 1/2] bluetooth: add support to notify SCO air mode Marcel Holtmann
@ 2020-04-02 7:55 ` Sathish Narasimman
0 siblings, 0 replies; 6+ messages in thread
From: Sathish Narasimman @ 2020-04-02 7:55 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: BlueZ devel list, Sathish Narsimman
Hi Marcel
On Sat, Jan 4, 2020 at 3:38 PM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Satish,
>
> > notifying HCI_NOTIFY_CONN_ADD for SCO connection is generic in
> > case of msbc audio. To differnetiate SCO air mode audio introducing
> > HCI_NOTIFY_SCO_CVSD and HCI_NOTIFY_SCO_TRASP.
> >
> > Signed-off-by: Sathish Narsimman <sathish.narasimman@intel.com>
> > ---
> > include/net/bluetooth/hci.h | 3 +++
> > include/net/bluetooth/hci_core.h | 3 +++
> > net/bluetooth/hci_conn.c | 19 +++++++++++++++----
> > net/bluetooth/hci_event.c | 9 +++++++++
> > 4 files changed, 30 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> > index 5bc1e30dedde..bfaaef1aa4d6 100644
> > --- a/include/net/bluetooth/hci.h
> > +++ b/include/net/bluetooth/hci.h
> > @@ -52,6 +52,9 @@
> > #define HCI_NOTIFY_CONN_ADD 1
> > #define HCI_NOTIFY_CONN_DEL 2
> > #define HCI_NOTIFY_VOICE_SETTING 3
> > +#define HCI_NOTIFY_ENABLE_SCO_CVSD 4
> > +#define HCI_NOTIFY_ENABLE_SCO_TRANSP 5
> > +#define HCI_NOTIFY_DISABLE_SCO 6
> >
> > /* HCI bus types */
> > #define HCI_VIRTUAL 0
> > diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> > index b689aceb636b..9258e19f8cc5 100644
> > --- a/include/net/bluetooth/hci_core.h
> > +++ b/include/net/bluetooth/hci_core.h
> > @@ -1604,4 +1604,7 @@ void hci_copy_identity_address(struct hci_dev *hdev, bdaddr_t *bdaddr,
> > #define SCO_AIRMODE_CVSD 0x0000
> > #define SCO_AIRMODE_TRANSP 0x0003
> >
> > +#define SCO_CODED_CVSD 0x02
> > +#define SCO_CODED_TRANSP 0x03
> > +
>
> why is this needed. And more important, what is wrong with SCO_AIRMODE_CVSD above?
>
AFAIK the above is voice settings param (core spec 5.0, Vol 2, Part E,
6.12) which is used to set the sco pkt type.
This patch is to notify the btusb layer. when we receive
sync_conn_complete_evt based on the AIR_MODE we receive
SCO_CODED_CVSD and SCO_CODED_TRANSP are used to choose alt_setting.
air_mode value we receive are 1 byte
whereas voice setting param is 2 bytes and are different for CVSD
> > #endif /* __HCI_CORE_H */
> > diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> > index 87691404d0c6..097a179418d6 100644
> > --- a/net/bluetooth/hci_conn.c
> > +++ b/net/bluetooth/hci_conn.c
> > @@ -122,8 +122,13 @@ static void hci_conn_cleanup(struct hci_conn *conn)
> >
> > hci_conn_hash_del(hdev, conn);
> >
> > - if (hdev->notify)
> > - hdev->notify(hdev, HCI_NOTIFY_CONN_DEL);
> > + if (conn->type == SCO_LINK || conn->type == ESCO_LINK) {
> > + if (hdev->notify)
> > + hdev->notify(hdev, HCI_NOTIFY_DISABLE_SCO);
> > + } else {
> > + if (hdev->notify)
> > + hdev->notify(hdev, HCI_NOTIFY_CONN_DEL);
> > + }
> >
> > hci_conn_del_sysfs(conn);
> >
> > @@ -561,8 +566,14 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst,
> > hci_dev_hold(hdev);
> >
> > hci_conn_hash_add(hdev, conn);
> > - if (hdev->notify)
> > - hdev->notify(hdev, HCI_NOTIFY_CONN_ADD);
> > +
> > + /* As HCI_NOTIFY_ENABLE_SCO_CVSD & HCI_NOTIFY_ENABLE_SCO_TRANSP is
> > + * used to notify SCO connections during sync_conn_complete event
> > + */
> > + if (conn->type != SCO_LINK && conn->type != ESCO_LINK) {
> > + if (hdev->notify)
> > + hdev->notify(hdev, HCI_NOTIFY_CONN_ADD);
> > + }
> >
> > hci_conn_init_sysfs(conn);
> >
> > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> > index c1d3a303d97f..ecad696db139 100644
> > --- a/net/bluetooth/hci_event.c
> > +++ b/net/bluetooth/hci_event.c
> > @@ -4231,6 +4231,15 @@ static void hci_sync_conn_complete_evt(struct hci_dev *hdev,
> > break;
> > }
> >
> > + BT_DBG("sco connected with air mode: %02x", ev->air_mode);
> > + if (ev->air_mode == SCO_CODED_CVSD) {
> > + if (hdev->notify)
> > + hdev->notify(hdev, HCI_NOTIFY_ENABLE_SCO_CVSD);
> > + } else if (ev->air_mode == SCO_CODED_TRANSP) {
> > + if (hdev->notify)
> > + hdev->notify(hdev, HCI_NOTIFY_ENABLE_SCO_TRANSP);
> > + }
> > +
>
> You are however forgetting the old hci_conn_complete for legacy SCO setup.
Will fix this and update in next version
>
> Regards
>
> Marcel
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-04-02 7:55 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-29 12:10 [PATCH v5 1/2] bluetooth: add support to notify SCO air mode Sathish Narsimman
2019-11-29 12:10 ` [PATCH v5 2/2] bluetooth:btusb: handle msbc audio over USB Endpoints Sathish Narsimman
2019-11-29 13:38 ` Luiz Augusto von Dentz
2019-11-29 13:58 ` Sathish Narasimman
2020-01-04 10:08 ` [PATCH v5 1/2] bluetooth: add support to notify SCO air mode Marcel Holtmann
2020-04-02 7:55 ` Sathish Narasimman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).