2019-11-11 11:18:20

by Sathish Narasimman

[permalink] [raw]
Subject: [PATCH v2] Bluetooth: btusb: hci_event: handle msbc audio over USB Endpoints

From: Sathish Narasimman <[email protected]>

For msbc encoded audio stream over usb transport, btusb driver
to be set to alternate settings 6 as per BT core spec 5.0. This
done from hci_sync_conn_complete_evt. The type of air mode is known
during this event. For this reason the btusb is to be notifed
about the TRANSPARENT air mode and the ALT setting 6 is selected.
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

(am from https://www.spinics.net/lists/linux-bluetooth/msg76982.html)

Signed-off-by: Sathish Narasimman <[email protected]>
Signed-off-by: Chethan T N <[email protected]>
Signed-off-by: Hsin-Yu Chao <[email protected]>
Signed-off-by: Amit K Bag <[email protected]>
---
drivers/bluetooth/btusb.c | 155 ++++++++++++++++++++++++-------
include/net/bluetooth/hci.h | 7 +-
include/net/bluetooth/hci_core.h | 3 +
net/bluetooth/hci_conn.c | 2 -
net/bluetooth/hci_event.c | 9 ++
5 files changed, 136 insertions(+), 40 deletions(-)

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 04a139e7793f..1615d9dc0f16 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;
@@ -1376,9 +1414,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;
@@ -1474,6 +1516,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);
}
}
@@ -1521,6 +1564,65 @@ 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);
@@ -1540,44 +1642,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);
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 5bc1e30dedde..8183394c2cc0 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -49,9 +49,10 @@
#define HCI_DEV_SETUP 9

/* HCI notify events */
-#define HCI_NOTIFY_CONN_ADD 1
-#define HCI_NOTIFY_CONN_DEL 2
-#define HCI_NOTIFY_VOICE_SETTING 3
+#define HCI_NOTIFY_ENABLE_SCO_CVSD 1
+#define HCI_NOTIFY_ENABLE_SCO_TRANSP 2
+#define HCI_NOTIFY_CONN_DEL 3
+#define HCI_NOTIFY_VOICE_SETTING 4

/* 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..c4a2c3efb4b7 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 7ff92dd4c53c..3a9a4b1c2bb2 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -561,8 +561,6 @@ 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);

hci_conn_init_sysfs(conn);

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index c1d3a303d97f..7cf0e5135cd8 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("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


2019-11-12 21:26:28

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v2] Bluetooth: btusb: hci_event: handle msbc audio over USB Endpoints

Hi Sathish,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on bluetooth-next/master]
[also build test WARNING on v5.4-rc7 next-20191112]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url: https://github.com/0day-ci/linux/commits/Sathish-Narsimman/Bluetooth-btusb-hci_event-handle-msbc-audio-over-USB-Endpoints/20191113-022414
base: https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 7.4.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.4.0 make.cross ARCH=ia64

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <[email protected]>

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

drivers//bluetooth/btusb.c: In function 'btusb_work':
>> drivers//bluetooth/btusb.c:1534:6: warning: 'new_alts' may be used uninitialized in this function [-Wmaybe-uninitialized]
err = usb_set_interface(data->udev, data->isoc_ifnum, altsetting);
~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers//bluetooth/btusb.c:1630:6: note: 'new_alts' was declared here
int new_alts;
^~~~~~~~

vim +/new_alts +1534 drivers//bluetooth/btusb.c

5e23b923da03de Marcel Holtmann 2007-10-20 1523
42b16b3fbb5ee4 Jesper Juhl 2011-01-17 1524 static inline int __set_isoc_interface(struct hci_dev *hdev, int altsetting)
9bfa35fe422c74 Marcel Holtmann 2008-08-18 1525 {
155961e8001719 David Herrmann 2012-02-09 1526 struct btusb_data *data = hci_get_drvdata(hdev);
9bfa35fe422c74 Marcel Holtmann 2008-08-18 1527 struct usb_interface *intf = data->isoc;
9bfa35fe422c74 Marcel Holtmann 2008-08-18 1528 struct usb_endpoint_descriptor *ep_desc;
9bfa35fe422c74 Marcel Holtmann 2008-08-18 1529 int i, err;
9bfa35fe422c74 Marcel Holtmann 2008-08-18 1530
9bfa35fe422c74 Marcel Holtmann 2008-08-18 1531 if (!data->isoc)
9bfa35fe422c74 Marcel Holtmann 2008-08-18 1532 return -ENODEV;
9bfa35fe422c74 Marcel Holtmann 2008-08-18 1533
459232fc0e2505 Marcel Holtmann 2017-10-24 @1534 err = usb_set_interface(data->udev, data->isoc_ifnum, altsetting);
9bfa35fe422c74 Marcel Holtmann 2008-08-18 1535 if (err < 0) {
2064ee332e4c1b Marcel Holtmann 2017-10-30 1536 bt_dev_err(hdev, "setting interface failed (%d)", -err);
9bfa35fe422c74 Marcel Holtmann 2008-08-18 1537 return err;
9bfa35fe422c74 Marcel Holtmann 2008-08-18 1538 }
9bfa35fe422c74 Marcel Holtmann 2008-08-18 1539
9bfa35fe422c74 Marcel Holtmann 2008-08-18 1540 data->isoc_altsetting = altsetting;
9bfa35fe422c74 Marcel Holtmann 2008-08-18 1541
9bfa35fe422c74 Marcel Holtmann 2008-08-18 1542 data->isoc_tx_ep = NULL;
9bfa35fe422c74 Marcel Holtmann 2008-08-18 1543 data->isoc_rx_ep = NULL;
9bfa35fe422c74 Marcel Holtmann 2008-08-18 1544
9bfa35fe422c74 Marcel Holtmann 2008-08-18 1545 for (i = 0; i < intf->cur_altsetting->desc.bNumEndpoints; i++) {
9bfa35fe422c74 Marcel Holtmann 2008-08-18 1546 ep_desc = &intf->cur_altsetting->endpoint[i].desc;
9bfa35fe422c74 Marcel Holtmann 2008-08-18 1547
9bfa35fe422c74 Marcel Holtmann 2008-08-18 1548 if (!data->isoc_tx_ep && usb_endpoint_is_isoc_out(ep_desc)) {
9bfa35fe422c74 Marcel Holtmann 2008-08-18 1549 data->isoc_tx_ep = ep_desc;
9bfa35fe422c74 Marcel Holtmann 2008-08-18 1550 continue;
9bfa35fe422c74 Marcel Holtmann 2008-08-18 1551 }
9bfa35fe422c74 Marcel Holtmann 2008-08-18 1552
9bfa35fe422c74 Marcel Holtmann 2008-08-18 1553 if (!data->isoc_rx_ep && usb_endpoint_is_isoc_in(ep_desc)) {
9bfa35fe422c74 Marcel Holtmann 2008-08-18 1554 data->isoc_rx_ep = ep_desc;
9bfa35fe422c74 Marcel Holtmann 2008-08-18 1555 continue;
9bfa35fe422c74 Marcel Holtmann 2008-08-18 1556 }
9bfa35fe422c74 Marcel Holtmann 2008-08-18 1557 }
9bfa35fe422c74 Marcel Holtmann 2008-08-18 1558
9bfa35fe422c74 Marcel Holtmann 2008-08-18 1559 if (!data->isoc_tx_ep || !data->isoc_rx_ep) {
2064ee332e4c1b Marcel Holtmann 2017-10-30 1560 bt_dev_err(hdev, "invalid SCO descriptors");
9bfa35fe422c74 Marcel Holtmann 2008-08-18 1561 return -ENODEV;
9bfa35fe422c74 Marcel Holtmann 2008-08-18 1562 }
9bfa35fe422c74 Marcel Holtmann 2008-08-18 1563
9bfa35fe422c74 Marcel Holtmann 2008-08-18 1564 return 0;
9bfa35fe422c74 Marcel Holtmann 2008-08-18 1565 }
9bfa35fe422c74 Marcel Holtmann 2008-08-18 1566

:::::: The code at line 1534 was first introduced by commit
:::::: 459232fc0e2505d489e2dc3befc1ad01dcdccb47 Bluetooth: btusb: Fix isochronous interface assignments

:::::: TO: Marcel Holtmann <[email protected]>
:::::: CC: Johan Hedberg <[email protected]>

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/[email protected] Intel Corporation


Attachments:
(No filename) (5.29 kB)
.config.gz (53.62 kB)
Download all attachments