* [PATCH] Bluetooth: btusb: Handling MSBC encoded Audio stream
@ 2018-08-20 13:31 Sathish Narasimman
2018-08-20 13:32 ` [PATCH] Bluetooth: btusb: hci_event: handle msbc audio over USB Endpoints Sathish Narasimman
0 siblings, 1 reply; 10+ messages in thread
From: Sathish Narasimman @ 2018-08-20 13:31 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Sathish Narasimman
Msbc encoded audio stream over USB transport should be directed
through alternate setting 6 as per Bluetooth core spec 5.0.
The patch is made keeping some part of discussion from the below
link.
https://www.spinics.net/lists/linux-bluetooth/msg64577.html
Sathish Narasimman (1):
Bluetooth: btusb: hci_event: handle msbc audio over USB Endpoints
drivers/bluetooth/btusb.c | 95 +++++++++++++++++++++++++--------------------
include/net/bluetooth/hci.h | 1 +
net/bluetooth/hci_event.c | 5 +++
3 files changed, 59 insertions(+), 42 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] Bluetooth: btusb: hci_event: handle msbc audio over USB Endpoints
2018-08-20 13:31 [PATCH] Bluetooth: btusb: Handling MSBC encoded Audio stream Sathish Narasimman
@ 2018-08-20 13:32 ` Sathish Narasimman
2018-08-20 16:05 ` Luiz Augusto von Dentz
0 siblings, 1 reply; 10+ messages in thread
From: Sathish Narasimman @ 2018-08-20 13:32 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Sathish Narasimman
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
Signed-off-by: Sathish Narasimman <sathish.narasimman@intel.com>
---
drivers/bluetooth/btusb.c | 95 +++++++++++++++++++++++++--------------------
include/net/bluetooth/hci.h | 1 +
net/bluetooth/hci_event.c | 5 +++
3 files changed, 59 insertions(+), 42 deletions(-)
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index cd2e5cf..ae924b6 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -1390,18 +1390,6 @@ static int btusb_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
return -EILSEQ;
}
-static void btusb_notify(struct hci_dev *hdev, unsigned int evt)
-{
- struct btusb_data *data = hci_get_drvdata(hdev);
-
- BT_DBG("%s evt %d", hdev->name, evt);
-
- if (hci_conn_num(hdev, SCO_LINK) != data->sco_num) {
- data->sco_num = hci_conn_num(hdev, SCO_LINK);
- schedule_work(&data->work);
- }
-}
-
static inline int __set_isoc_interface(struct hci_dev *hdev, int altsetting)
{
struct btusb_data *data = hci_get_drvdata(hdev);
@@ -1445,6 +1433,58 @@ static inline int __set_isoc_interface(struct hci_dev *hdev, int altsetting)
return 0;
}
+static void bt_switch_alt_setting(struct hci_dev *hdev, int new_alts)
+{
+ struct btusb_data *data = hci_get_drvdata(hdev);
+
+ 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);
+
+ if (__set_isoc_interface(hdev, new_alts) < 0)
+ return;
+ }
+ 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);
+ }
+}
+
+static void btusb_notify(struct hci_dev *hdev, unsigned int evt)
+{
+ struct btusb_data *data = hci_get_drvdata(hdev);
+
+ BT_DBG("%s evt %d", hdev->name, evt);
+
+ if (hci_conn_num(hdev, SCO_LINK) != data->sco_num) {
+ data->sco_num = hci_conn_num(hdev, SCO_LINK);
+ schedule_work(&data->work);
+ }
+
+ if (evt == HCI_NOTIFY_AIR_MODE_TRANSP) {
+ /* Alt setting 6 is for msbc encoded audio channel */
+ bt_switch_alt_setting(hdev, 6);
+ }
+}
+
static void btusb_work(struct work_struct *work)
{
struct btusb_data *data = container_of(work, struct btusb_data, work);
@@ -1472,36 +1512,7 @@ static void btusb_work(struct work_struct *work)
new_alts = data->sco_num;
}
- 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);
-
- if (__set_isoc_interface(hdev, new_alts) < 0)
- return;
- }
-
- 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);
- }
+ bt_switch_alt_setting(hdev, 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 cdd9f1f..3498c6b 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -52,6 +52,7 @@
#define HCI_NOTIFY_CONN_ADD 1
#define HCI_NOTIFY_CONN_DEL 2
#define HCI_NOTIFY_VOICE_SETTING 3
+#define HCI_NOTIFY_AIR_MODE_TRANSP 4
/* HCI bus types */
#define HCI_VIRTUAL 0
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index f12555f..8ef5220 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -4100,6 +4100,11 @@ static void hci_sync_conn_complete_evt(struct hci_dev *hdev,
break;
}
+ if (ev->air_mode == SCO_AIRMODE_TRANSP) {
+ if (hdev->notify)
+ hdev->notify(hdev, HCI_NOTIFY_AIR_MODE_TRANSP);
+ }
+
hci_connect_cfm(conn, ev->status);
if (ev->status)
hci_conn_del(conn);
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] Bluetooth: btusb: hci_event: handle msbc audio over USB Endpoints
2018-08-20 13:32 ` [PATCH] Bluetooth: btusb: hci_event: handle msbc audio over USB Endpoints Sathish Narasimman
@ 2018-08-20 16:05 ` Luiz Augusto von Dentz
[not found] ` <CAOVXEJ+CLxgr-eZ-98U_U=JFCCu6eiVnuv=R_VwewiOE=PXsmg@mail.gmail.com>
0 siblings, 1 reply; 10+ messages in thread
From: Luiz Augusto von Dentz @ 2018-08-20 16:05 UTC (permalink / raw)
To: Sathish Narasimman; +Cc: linux-bluetooth, Sathish Narasimman
Hi Sathish,
On Mon, Aug 20, 2018 at 4:32 PM, Sathish Narasimman
<nsathish41@gmail.com> wrote:
> 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
>
> Signed-off-by: Sathish Narasimman <sathish.narasimman@intel.com>
> ---
> drivers/bluetooth/btusb.c | 95 +++++++++++++++++++++++++--------------------
> include/net/bluetooth/hci.h | 1 +
> net/bluetooth/hci_event.c | 5 +++
> 3 files changed, 59 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index cd2e5cf..ae924b6 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -1390,18 +1390,6 @@ static int btusb_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
> return -EILSEQ;
> }
>
> -static void btusb_notify(struct hci_dev *hdev, unsigned int evt)
> -{
> - struct btusb_data *data = hci_get_drvdata(hdev);
> -
> - BT_DBG("%s evt %d", hdev->name, evt);
> -
> - if (hci_conn_num(hdev, SCO_LINK) != data->sco_num) {
> - data->sco_num = hci_conn_num(hdev, SCO_LINK);
> - schedule_work(&data->work);
> - }
> -}
> -
> static inline int __set_isoc_interface(struct hci_dev *hdev, int altsetting)
> {
> struct btusb_data *data = hci_get_drvdata(hdev);
> @@ -1445,6 +1433,58 @@ static inline int __set_isoc_interface(struct hci_dev *hdev, int altsetting)
> return 0;
> }
>
> +static void bt_switch_alt_setting(struct hci_dev *hdev, int new_alts)
> +{
> + struct btusb_data *data = hci_get_drvdata(hdev);
> +
> + 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);
> +
> + if (__set_isoc_interface(hdev, new_alts) < 0)
> + return;
> + }
> + 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);
> + }
> +}
> +
> +static void btusb_notify(struct hci_dev *hdev, unsigned int evt)
> +{
> + struct btusb_data *data = hci_get_drvdata(hdev);
> +
> + BT_DBG("%s evt %d", hdev->name, evt);
> +
> + if (hci_conn_num(hdev, SCO_LINK) != data->sco_num) {
> + data->sco_num = hci_conn_num(hdev, SCO_LINK);
> + schedule_work(&data->work);
> + }
> +
> + if (evt == HCI_NOTIFY_AIR_MODE_TRANSP) {
> + /* Alt setting 6 is for msbc encoded audio channel */
> + bt_switch_alt_setting(hdev, 6);
Have you checked that this works on older controllers? I suppose those
don't have alt_set 6.
> + }
> +}
> +
> static void btusb_work(struct work_struct *work)
> {
> struct btusb_data *data = container_of(work, struct btusb_data, work);
> @@ -1472,36 +1512,7 @@ static void btusb_work(struct work_struct *work)
> new_alts = data->sco_num;
> }
>
> - 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);
> -
> - if (__set_isoc_interface(hdev, new_alts) < 0)
> - return;
> - }
> -
> - 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);
> - }
> + bt_switch_alt_setting(hdev, 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 cdd9f1f..3498c6b 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -52,6 +52,7 @@
> #define HCI_NOTIFY_CONN_ADD 1
> #define HCI_NOTIFY_CONN_DEL 2
> #define HCI_NOTIFY_VOICE_SETTING 3
> +#define HCI_NOTIFY_AIR_MODE_TRANSP 4
>
> /* HCI bus types */
> #define HCI_VIRTUAL 0
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index f12555f..8ef5220 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -4100,6 +4100,11 @@ static void hci_sync_conn_complete_evt(struct hci_dev *hdev,
> break;
> }
>
> + if (ev->air_mode == SCO_AIRMODE_TRANSP) {
> + if (hdev->notify)
> + hdev->notify(hdev, HCI_NOTIFY_AIR_MODE_TRANSP);
> + }
> +
> hci_connect_cfm(conn, ev->status);
> if (ev->status)
> hci_conn_del(conn);
> --
> 2.7.4
>
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Bluetooth: btusb: hci_event: handle msbc audio over USB Endpoints
[not found] ` <CAOVXEJ+CLxgr-eZ-98U_U=JFCCu6eiVnuv=R_VwewiOE=PXsmg@mail.gmail.com>
@ 2018-08-21 8:12 ` Luiz Augusto von Dentz
2018-08-21 9:49 ` Sathish Narasimman
0 siblings, 1 reply; 10+ messages in thread
From: Luiz Augusto von Dentz @ 2018-08-21 8:12 UTC (permalink / raw)
To: Sathish Narasimman; +Cc: linux-bluetooth, Sathish Narasimman
Hi Sathish,
On Tue, Aug 21, 2018 at 8:58 AM, Sathish Narasimman
<nsathish41@gmail.com> wrote:
> Hi Luiz,
>
> On Mon, Aug 20, 2018 at 9:35 PM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
>>
>> Hi Sathish,
>>
>> On Mon, Aug 20, 2018 at 4:32 PM, Sathish Narasimman
>> <nsathish41@gmail.com> wrote:
>> > 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
>> >
>> > Signed-off-by: Sathish Narasimman <sathish.narasimman@intel.com>
>> > ---
>> > drivers/bluetooth/btusb.c | 95
>> > +++++++++++++++++++++++++--------------------
>> > include/net/bluetooth/hci.h | 1 +
>> > net/bluetooth/hci_event.c | 5 +++
>> > 3 files changed, 59 insertions(+), 42 deletions(-)
>> >
>> > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
>> > index cd2e5cf..ae924b6 100644
>> > --- a/drivers/bluetooth/btusb.c
>> > +++ b/drivers/bluetooth/btusb.c
>> > @@ -1390,18 +1390,6 @@ static int btusb_send_frame(struct hci_dev *hdev,
>> > struct sk_buff *skb)
>> > return -EILSEQ;
>> > }
>> >
>> > -static void btusb_notify(struct hci_dev *hdev, unsigned int evt)
>> > -{
>> > - struct btusb_data *data = hci_get_drvdata(hdev);
>> > -
>> > - BT_DBG("%s evt %d", hdev->name, evt);
>> > -
>> > - if (hci_conn_num(hdev, SCO_LINK) != data->sco_num) {
>> > - data->sco_num = hci_conn_num(hdev, SCO_LINK);
>> > - schedule_work(&data->work);
>> > - }
>> > -}
>> > -
>> > static inline int __set_isoc_interface(struct hci_dev *hdev, int
>> > altsetting)
>> > {
>> > struct btusb_data *data = hci_get_drvdata(hdev);
>> > @@ -1445,6 +1433,58 @@ static inline int __set_isoc_interface(struct
>> > hci_dev *hdev, int altsetting)
>> > return 0;
>> > }
>> >
>> > +static void bt_switch_alt_setting(struct hci_dev *hdev, int new_alts)
>> > +{
>> > + struct btusb_data *data = hci_get_drvdata(hdev);
>> > +
>> > + 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);
>> > +
>> > + if (__set_isoc_interface(hdev, new_alts) < 0)
>> > + return;
>> > + }
>> > + 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);
>> > + }
>> > +}
>> > +
>> > +static void btusb_notify(struct hci_dev *hdev, unsigned int evt)
>> > +{
>> > + struct btusb_data *data = hci_get_drvdata(hdev);
>> > +
>> > + BT_DBG("%s evt %d", hdev->name, evt);
>> > +
>> > + if (hci_conn_num(hdev, SCO_LINK) != data->sco_num) {
>> > + data->sco_num = hci_conn_num(hdev, SCO_LINK);
>> > + schedule_work(&data->work);
>> > + }
>> > +
>> > + if (evt == HCI_NOTIFY_AIR_MODE_TRANSP) {
>> > + /* Alt setting 6 is for msbc encoded audio channel */
>> > + bt_switch_alt_setting(hdev, 6);
>>
>> Have you checked that this works on older controllers? I suppose those
>> don't have alt_set 6.
>
> I took the reference from the core spec 5, Vol 4, Part B, Table 2.1
> For USB transport the ALT setting 6 should be used for one msbc voice
> channel.
> Yes, for the controllers that does not support ALT setting 6. this patch
> will not work.
It _must_ keep working with controller that don't support such
setting, there is no option on that, so there should be a way to just
read what supported alt settings that are supported and fallback if 6
is not supported.
> Also, I failed to submit a small part of the patch to maintain 7.5ms HCI
> packet intervel in ALT 6 setting i will submitt the next set with that
> patch.
> Table 2.1: USB Primary firmware interface and endpoint settings
>>
>> > + }
>> > +}
>> > +
>> > static void btusb_work(struct work_struct *work)
>> > {
>> > struct btusb_data *data = container_of(work, struct btusb_data,
>> > work);
>> > @@ -1472,36 +1512,7 @@ static void btusb_work(struct work_struct *work)
>> > new_alts = data->sco_num;
>> > }
>> >
>> > - 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);
>> > -
>> > - if (__set_isoc_interface(hdev, new_alts) < 0)
>> > - return;
>> > - }
>> > -
>> > - 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);
>> > - }
>> > + bt_switch_alt_setting(hdev, 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 cdd9f1f..3498c6b 100644
>> > --- a/include/net/bluetooth/hci.h
>> > +++ b/include/net/bluetooth/hci.h
>> > @@ -52,6 +52,7 @@
>> > #define HCI_NOTIFY_CONN_ADD 1
>> > #define HCI_NOTIFY_CONN_DEL 2
>> > #define HCI_NOTIFY_VOICE_SETTING 3
>> > +#define HCI_NOTIFY_AIR_MODE_TRANSP 4
>> >
>> > /* HCI bus types */
>> > #define HCI_VIRTUAL 0
>> > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
>> > index f12555f..8ef5220 100644
>> > --- a/net/bluetooth/hci_event.c
>> > +++ b/net/bluetooth/hci_event.c
>> > @@ -4100,6 +4100,11 @@ static void hci_sync_conn_complete_evt(struct
>> > hci_dev *hdev,
>> > break;
>> > }
>> >
>> > + if (ev->air_mode == SCO_AIRMODE_TRANSP) {
>> > + if (hdev->notify)
>> > + hdev->notify(hdev, HCI_NOTIFY_AIR_MODE_TRANSP);
>> > + }
>> > +
>> > hci_connect_cfm(conn, ev->status);
>> > if (ev->status)
>> > hci_conn_del(conn);
>> > --
>> > 2.7.4
>> >
>>
>>
>>
>> --
>> Luiz Augusto von Dentz
>
>
> Thanks,
> Sathish N
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Bluetooth: btusb: hci_event: handle msbc audio over USB Endpoints
2018-08-21 8:12 ` Luiz Augusto von Dentz
@ 2018-08-21 9:49 ` Sathish Narasimman
2018-08-21 13:58 ` Marcel Holtmann
0 siblings, 1 reply; 10+ messages in thread
From: Sathish Narasimman @ 2018-08-21 9:49 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth, Sathish Narasimman
Hi Luiz,
On Tue, Aug 21, 2018 at 1:42 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Sathish,
>
> On Tue, Aug 21, 2018 at 8:58 AM, Sathish Narasimman
> <nsathish41@gmail.com> wrote:
> > Hi Luiz,
> >
> > On Mon, Aug 20, 2018 at 9:35 PM Luiz Augusto von Dentz
> > <luiz.dentz@gmail.com> wrote:
> >>
> >> Hi Sathish,
> >>
> >> On Mon, Aug 20, 2018 at 4:32 PM, Sathish Narasimman
> >> <nsathish41@gmail.com> wrote:
> >> > 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
> >> >
> >> > Signed-off-by: Sathish Narasimman <sathish.narasimman@intel.com>
> >> > ---
> >> > drivers/bluetooth/btusb.c | 95
> >> > +++++++++++++++++++++++++--------------------
> >> > include/net/bluetooth/hci.h | 1 +
> >> > net/bluetooth/hci_event.c | 5 +++
> >> > 3 files changed, 59 insertions(+), 42 deletions(-)
> >> >
> >> > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> >> > index cd2e5cf..ae924b6 100644
> >> > --- a/drivers/bluetooth/btusb.c
> >> > +++ b/drivers/bluetooth/btusb.c
> >> > @@ -1390,18 +1390,6 @@ static int btusb_send_frame(struct hci_dev *hdev,
> >> > struct sk_buff *skb)
> >> > return -EILSEQ;
> >> > }
> >> >
> >> > -static void btusb_notify(struct hci_dev *hdev, unsigned int evt)
> >> > -{
> >> > - struct btusb_data *data = hci_get_drvdata(hdev);
> >> > -
> >> > - BT_DBG("%s evt %d", hdev->name, evt);
> >> > -
> >> > - if (hci_conn_num(hdev, SCO_LINK) != data->sco_num) {
> >> > - data->sco_num = hci_conn_num(hdev, SCO_LINK);
> >> > - schedule_work(&data->work);
> >> > - }
> >> > -}
> >> > -
> >> > static inline int __set_isoc_interface(struct hci_dev *hdev, int
> >> > altsetting)
> >> > {
> >> > struct btusb_data *data = hci_get_drvdata(hdev);
> >> > @@ -1445,6 +1433,58 @@ static inline int __set_isoc_interface(struct
> >> > hci_dev *hdev, int altsetting)
> >> > return 0;
> >> > }
> >> >
> >> > +static void bt_switch_alt_setting(struct hci_dev *hdev, int new_alts)
> >> > +{
> >> > + struct btusb_data *data = hci_get_drvdata(hdev);
> >> > +
> >> > + 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);
> >> > +
> >> > + if (__set_isoc_interface(hdev, new_alts) < 0)
> >> > + return;
If controller does not support ALT 6. above function will return negative.
will take care of falling back to ALT 2.
> >> > + }
> >> > + 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);
> >> > + }
> >> > +}
> >> > +
> >> > +static void btusb_notify(struct hci_dev *hdev, unsigned int evt)
> >> > +{
> >> > + struct btusb_data *data = hci_get_drvdata(hdev);
> >> > +
> >> > + BT_DBG("%s evt %d", hdev->name, evt);
> >> > +
> >> > + if (hci_conn_num(hdev, SCO_LINK) != data->sco_num) {
> >> > + data->sco_num = hci_conn_num(hdev, SCO_LINK);
> >> > + schedule_work(&data->work);
> >> > + }
> >> > +
> >> > + if (evt == HCI_NOTIFY_AIR_MODE_TRANSP) {
> >> > + /* Alt setting 6 is for msbc encoded audio channel */
> >> > + bt_switch_alt_setting(hdev, 6);
> >>
> >> Have you checked that this works on older controllers? I suppose those
> >> don't have alt_set 6.
> >
> > I took the reference from the core spec 5, Vol 4, Part B, Table 2.1
> > For USB transport the ALT setting 6 should be used for one msbc voice
> > channel.
> > Yes, for the controllers that does not support ALT setting 6. this patch
> > will not work.
>
> It _must_ keep working with controller that don't support such
> setting, there is no option on that, so there should be a way to just
> read what supported alt settings that are supported and fallback if 6
> is not supported.
>
If we trying to set to ALT SET 6 and controller does not support. in
btusb.c __set_isoc_interface function
will through error and returns negative if fails to set. So the error
check was available already.
I need to take care of returning to ALT Setting 2 if controller fails
to set ALT 6.
This can be done. will update the patch. please take a look into patch
v2 where the 7.5ms time interval is maintained.
> > Also, I failed to submit a small part of the patch to maintain 7.5ms HCI
> > packet intervel in ALT 6 setting i will submitt the next set with that
> > patch.
> > Table 2.1: USB Primary firmware interface and endpoint settings
> >>
> >> > + }
> >> > +}
> >> > +
> >> > static void btusb_work(struct work_struct *work)
> >> > {
> >> > struct btusb_data *data = container_of(work, struct btusb_data,
> >> > work);
> >> > @@ -1472,36 +1512,7 @@ static void btusb_work(struct work_struct *work)
> >> > new_alts = data->sco_num;
> >> > }
> >> >
> >> > - 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);
> >> > -
> >> > - if (__set_isoc_interface(hdev, new_alts) < 0)
> >> > - return;
> >> > - }
> >> > -
> >> > - 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);
> >> > - }
> >> > + bt_switch_alt_setting(hdev, 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 cdd9f1f..3498c6b 100644
> >> > --- a/include/net/bluetooth/hci.h
> >> > +++ b/include/net/bluetooth/hci.h
> >> > @@ -52,6 +52,7 @@
> >> > #define HCI_NOTIFY_CONN_ADD 1
> >> > #define HCI_NOTIFY_CONN_DEL 2
> >> > #define HCI_NOTIFY_VOICE_SETTING 3
> >> > +#define HCI_NOTIFY_AIR_MODE_TRANSP 4
> >> >
> >> > /* HCI bus types */
> >> > #define HCI_VIRTUAL 0
> >> > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> >> > index f12555f..8ef5220 100644
> >> > --- a/net/bluetooth/hci_event.c
> >> > +++ b/net/bluetooth/hci_event.c
> >> > @@ -4100,6 +4100,11 @@ static void hci_sync_conn_complete_evt(struct
> >> > hci_dev *hdev,
> >> > break;
> >> > }
> >> >
> >> > + if (ev->air_mode == SCO_AIRMODE_TRANSP) {
> >> > + if (hdev->notify)
> >> > + hdev->notify(hdev, HCI_NOTIFY_AIR_MODE_TRANSP);
> >> > + }
> >> > +
> >> > hci_connect_cfm(conn, ev->status);
> >> > if (ev->status)
> >> > hci_conn_del(conn);
> >> > --
> >> > 2.7.4
> >> >
> >>
> >>
> >>
> >> --
> >> Luiz Augusto von Dentz
> >
> >
> > Thanks,
> > Sathish N
>
>
>
> --
> Luiz Augusto von Dentz
Thanks,
Sathish N
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Bluetooth: btusb: hci_event: handle msbc audio over USB Endpoints
2018-08-21 9:49 ` Sathish Narasimman
@ 2018-08-21 13:58 ` Marcel Holtmann
0 siblings, 0 replies; 10+ messages in thread
From: Marcel Holtmann @ 2018-08-21 13:58 UTC (permalink / raw)
To: Sathish Narasimman
Cc: Luiz Augusto von Dentz, Bluez mailing list, Sathish Narasimman
Hi Sathish,
>>>>> 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
>>>>>
>>>>> Signed-off-by: Sathish Narasimman <sathish.narasimman@intel.com>
>>>>> ---
>>>>> drivers/bluetooth/btusb.c | 95
>>>>> +++++++++++++++++++++++++--------------------
>>>>> include/net/bluetooth/hci.h | 1 +
>>>>> net/bluetooth/hci_event.c | 5 +++
>>>>> 3 files changed, 59 insertions(+), 42 deletions(-)
>>>>>
>>>>> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
>>>>> index cd2e5cf..ae924b6 100644
>>>>> --- a/drivers/bluetooth/btusb.c
>>>>> +++ b/drivers/bluetooth/btusb.c
>>>>> @@ -1390,18 +1390,6 @@ static int btusb_send_frame(struct hci_dev *hdev,
>>>>> struct sk_buff *skb)
>>>>> return -EILSEQ;
>>>>> }
>>>>>
>>>>> -static void btusb_notify(struct hci_dev *hdev, unsigned int evt)
>>>>> -{
>>>>> - struct btusb_data *data = hci_get_drvdata(hdev);
>>>>> -
>>>>> - BT_DBG("%s evt %d", hdev->name, evt);
>>>>> -
>>>>> - if (hci_conn_num(hdev, SCO_LINK) != data->sco_num) {
>>>>> - data->sco_num = hci_conn_num(hdev, SCO_LINK);
>>>>> - schedule_work(&data->work);
>>>>> - }
>>>>> -}
>>>>> -
>>>>> static inline int __set_isoc_interface(struct hci_dev *hdev, int
>>>>> altsetting)
>>>>> {
>>>>> struct btusb_data *data = hci_get_drvdata(hdev);
>>>>> @@ -1445,6 +1433,58 @@ static inline int __set_isoc_interface(struct
>>>>> hci_dev *hdev, int altsetting)
>>>>> return 0;
>>>>> }
>>>>>
>>>>> +static void bt_switch_alt_setting(struct hci_dev *hdev, int new_alts)
>>>>> +{
>>>>> + struct btusb_data *data = hci_get_drvdata(hdev);
>>>>> +
>>>>> + 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);
>>>>> +
>>>>> + if (__set_isoc_interface(hdev, new_alts) < 0)
>>>>> + return;
> If controller does not support ALT 6. above function will return negative.
> will take care of falling back to ALT 2.
>>>>> + }
>>>>> + 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);
>>>>> + }
>>>>> +}
>>>>> +
>>>>> +static void btusb_notify(struct hci_dev *hdev, unsigned int evt)
>>>>> +{
>>>>> + struct btusb_data *data = hci_get_drvdata(hdev);
>>>>> +
>>>>> + BT_DBG("%s evt %d", hdev->name, evt);
>>>>> +
>>>>> + if (hci_conn_num(hdev, SCO_LINK) != data->sco_num) {
>>>>> + data->sco_num = hci_conn_num(hdev, SCO_LINK);
>>>>> + schedule_work(&data->work);
>>>>> + }
>>>>> +
>>>>> + if (evt == HCI_NOTIFY_AIR_MODE_TRANSP) {
>>>>> + /* Alt setting 6 is for msbc encoded audio channel */
>>>>> + bt_switch_alt_setting(hdev, 6);
>>>>
>>>> Have you checked that this works on older controllers? I suppose those
>>>> don't have alt_set 6.
>>>
>>> I took the reference from the core spec 5, Vol 4, Part B, Table 2.1
>>> For USB transport the ALT setting 6 should be used for one msbc voice
>>> channel.
>>> Yes, for the controllers that does not support ALT setting 6. this patch
>>> will not work.
>>
>> It _must_ keep working with controller that don't support such
>> setting, there is no option on that, so there should be a way to just
>> read what supported alt settings that are supported and fallback if 6
>> is not supported.
>>
> If we trying to set to ALT SET 6 and controller does not support. in
> btusb.c __set_isoc_interface function
> will through error and returns negative if fails to set. So the error
> check was available already.
> I need to take care of returning to ALT Setting 2 if controller fails
> to set ALT 6.
> This can be done. will update the patch. please take a look into patch
> v2 where the 7.5ms time interval is maintained.
we are not doing try-and-error here. In probe() do a proper enumeration of the alternate settings and check if setting 6 is available.
Regards
Marcel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Bluetooth: btusb: hci_event: handle msbc audio over USB Endpoints
2019-09-26 6:24 ` Marcel Holtmann
@ 2019-11-11 11:04 ` Sathish Narasimman
0 siblings, 0 replies; 10+ messages in thread
From: Sathish Narasimman @ 2019-11-11 11:04 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Amit K Bag, Bluez mailing list, ravishankar.srivatsa,
chethan.tumkur.narayan, Sathish Narasimman, Raghuram Hegde,
Hsin-Yu Chao
Hi Marcel,
I will submit the next version of the patch and tried to fix all your comments.
Please check the same.
On Thu, Sep 26, 2019 at 3:52 PM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Amit,
>
> > 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: Chethan T N <chethan.tumkur.narayan@intel.com>
> > Signed-off-by: Sathish Narasimman <sathish.narasimman@intel.com>
> > Signed-off-by: Raghuram Hegde <raghuram.hegde@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 | 145 ++++++++++++++++++++++++++++++--------------
> > include/net/bluetooth/hci.h | 1 +
> > net/bluetooth/hci_event.c | 5 ++
> > 3 files changed, 106 insertions(+), 45 deletions(-)
> >
> > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> > index a9c35ebb30f8..368cc93cb17e 100644
> > --- a/drivers/bluetooth/btusb.c
> > +++ b/drivers/bluetooth/btusb.c
> > @@ -57,6 +57,9 @@ static struct usb_driver btusb_driver;
> > #define BTUSB_IFNUM_2 0x80000
> > #define BTUSB_CW6622 0x100000
> > #define BTUSB_MEDIATEK 0x200000
> > +#define BTUSB_ALT6_FLOW_CNTRL 6
> > +
> > +static int set_hci_packet_interval_flow = BTUSB_ALT6_FLOW_CNTRL;
>
> this can never be a global variable. We allow multiple devices be attached to the same host. You need to move this into btusb_data struct.
>
> >
> > static const struct usb_device_id btusb_table[] = {
> > /* Generic Bluetooth USB device */
> > @@ -974,6 +977,38 @@ static void btusb_isoc_complete(struct urb *urb)
> > }
> > }
> >
> > +static inline void __fill_isoc_descriptor_msbc(struct urb *urb, int len,
> > + int mtu)
> > +{
> > + int i, offset = 0;
> > +
> > + /* 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 (set_hci_packet_interval_flow == 6)
> > + set_hci_packet_interval_flow = 7;
> > + else if (set_hci_packet_interval_flow == 7)
> > + set_hci_packet_interval_flow = 6;
>
> can’t this just be a bool that you just toggle.
Done.
>
> > +
> > + BT_DBG("len %d mtu %d", len, mtu);
> > +
> > + for (i = 0; i < set_hci_packet_interval_flow; 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 +1411,12 @@ 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));
> > + else
> > + __fill_isoc_descriptor(urb, skb->len,
> > le16_to_cpu(data->isoc_tx_ep->wMaxPacketSize));
> > -
> > skb->dev = (void *)hdev;
> >
> > return urb;
> > @@ -1466,18 +1504,6 @@ static int btusb_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
> > return -EILSEQ;
> > }
> >
> > -static void btusb_notify(struct hci_dev *hdev, unsigned int evt)
> > -{
> > - struct btusb_data *data = hci_get_drvdata(hdev);
> > -
> > - BT_DBG("%s evt %d", hdev->name, evt);
> > -
> > - if (hci_conn_num(hdev, SCO_LINK) != data->sco_num) {
> > - data->sco_num = hci_conn_num(hdev, SCO_LINK);
> > - schedule_work(&data->work);
> > - }
> > -}
> > -
> > static inline int __set_isoc_interface(struct hci_dev *hdev, int altsetting)
> > {
> > struct btusb_data *data = hci_get_drvdata(hdev);
> > @@ -1521,6 +1547,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 void btusb_notify(struct hci_dev *hdev, unsigned int evt)
> > +{
> > + struct btusb_data *data = hci_get_drvdata(hdev);
> > +
> > + BT_DBG("%s evt %d", hdev->name, evt);
> > +
> > + if (hci_conn_num(hdev, SCO_LINK) != data->sco_num) {
> > + data->sco_num = hci_conn_num(hdev, SCO_LINK);
> > + schedule_work(&data->work);
> > + }
>
> Since the internal processing of packets moved from a tasklet to a workqueue, I think the extra workqueue inside btusb can be actually removed. So I would get rid of this also for the non-mSBC case.
Have left as it is. Expecting you will change this.
>
> > +
> > + if (evt == HCI_NOTIFY_AIR_MODE_TRANSP) {
> > + /* Alt setting 6 is used for msbc encoded
> > + * audio channel
> > + */
> > + if (bt_switch_alt_setting(hdev, 6) < 0)
> > + BT_ERR("%s Set USB Alt6 failed", hdev->name);
> > + }
> > +}
> > +
>
> I think this is really a one or the other. If the controller supports alternate setting 6 (which you actually have to check first) and it is air mode transparent then switch to alternate setting 6. Otherwise do what we have done before.
Done.
>
> > static void btusb_work(struct work_struct *work)
> > {
> > struct btusb_data *data = container_of(work, struct btusb_data, work);
> > @@ -1547,37 +1632,7 @@ static void btusb_work(struct work_struct *work)
> > } else {
> > new_alts = data->sco_num;
> > }
> > -
> > - 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);
> > -
> > - if (__set_isoc_interface(hdev, new_alts) < 0)
> > - return;
> > - }
> > -
> > - 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);
> > - }
> > + bt_switch_alt_setting(hdev, 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..89ac29f1dffa 100644
> > --- a/include/net/bluetooth/hci.h
> > +++ b/include/net/bluetooth/hci.h
> > @@ -52,6 +52,7 @@
> > #define HCI_NOTIFY_CONN_ADD 1
> > #define HCI_NOTIFY_CONN_DEL 2
> > #define HCI_NOTIFY_VOICE_SETTING 3
> > +#define HCI_NOTIFY_AIR_MODE_TRANSP 4
> >
> > /* HCI bus types */
> > #define HCI_VIRTUAL 0
> > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> > index c1d3a303d97f..1c268932422c 100644
> > --- a/net/bluetooth/hci_event.c
> > +++ b/net/bluetooth/hci_event.c
> > @@ -4231,6 +4231,11 @@ static void hci_sync_conn_complete_evt(struct hci_dev *hdev,
> > break;
> > }
> >
> > + if (ev->air_mode == SCO_AIRMODE_TRANSP) {
> > + if (hdev->notify)
> > + hdev->notify(hdev, HCI_NOTIFY_AIR_MODE_TRANSP);
> > + }
> > +
>
> So this might work, but causes the USB subsystem to change the alternate setting twice. Once for CONN_ADD and once for SCO_AIRMODE_TRANSP. I prefer we just change the alternate setting once.
> Maybe it is better to change CONN_ADD and CONN_DEL to an explicit ENABLE_SCO_CVSD and ENABLE_SCO_TRANSP and DISABLE_SCO.
Done.
>
> Regards
>
> Marcel
>
Regards
Sathish N
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Bluetooth: btusb: hci_event: handle msbc audio over USB Endpoints
2019-09-26 5:35 Amit K Bag
@ 2019-09-26 6:24 ` Marcel Holtmann
2019-11-11 11:04 ` Sathish Narasimman
0 siblings, 1 reply; 10+ messages in thread
From: Marcel Holtmann @ 2019-09-26 6:24 UTC (permalink / raw)
To: Amit K Bag
Cc: Bluez mailing list, ravishankar.srivatsa, chethan.tumkur.narayan,
Sathish Narasimman, Raghuram Hegde, Hsin-Yu Chao
Hi Amit,
> 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: Chethan T N <chethan.tumkur.narayan@intel.com>
> Signed-off-by: Sathish Narasimman <sathish.narasimman@intel.com>
> Signed-off-by: Raghuram Hegde <raghuram.hegde@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 | 145 ++++++++++++++++++++++++++++++--------------
> include/net/bluetooth/hci.h | 1 +
> net/bluetooth/hci_event.c | 5 ++
> 3 files changed, 106 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index a9c35ebb30f8..368cc93cb17e 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -57,6 +57,9 @@ static struct usb_driver btusb_driver;
> #define BTUSB_IFNUM_2 0x80000
> #define BTUSB_CW6622 0x100000
> #define BTUSB_MEDIATEK 0x200000
> +#define BTUSB_ALT6_FLOW_CNTRL 6
> +
> +static int set_hci_packet_interval_flow = BTUSB_ALT6_FLOW_CNTRL;
this can never be a global variable. We allow multiple devices be attached to the same host. You need to move this into btusb_data struct.
>
> static const struct usb_device_id btusb_table[] = {
> /* Generic Bluetooth USB device */
> @@ -974,6 +977,38 @@ static void btusb_isoc_complete(struct urb *urb)
> }
> }
>
> +static inline void __fill_isoc_descriptor_msbc(struct urb *urb, int len,
> + int mtu)
> +{
> + int i, offset = 0;
> +
> + /* 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 (set_hci_packet_interval_flow == 6)
> + set_hci_packet_interval_flow = 7;
> + else if (set_hci_packet_interval_flow == 7)
> + set_hci_packet_interval_flow = 6;
can’t this just be a bool that you just toggle.
> +
> + BT_DBG("len %d mtu %d", len, mtu);
> +
> + for (i = 0; i < set_hci_packet_interval_flow; 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 +1411,12 @@ 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));
> + else
> + __fill_isoc_descriptor(urb, skb->len,
> le16_to_cpu(data->isoc_tx_ep->wMaxPacketSize));
> -
> skb->dev = (void *)hdev;
>
> return urb;
> @@ -1466,18 +1504,6 @@ static int btusb_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
> return -EILSEQ;
> }
>
> -static void btusb_notify(struct hci_dev *hdev, unsigned int evt)
> -{
> - struct btusb_data *data = hci_get_drvdata(hdev);
> -
> - BT_DBG("%s evt %d", hdev->name, evt);
> -
> - if (hci_conn_num(hdev, SCO_LINK) != data->sco_num) {
> - data->sco_num = hci_conn_num(hdev, SCO_LINK);
> - schedule_work(&data->work);
> - }
> -}
> -
> static inline int __set_isoc_interface(struct hci_dev *hdev, int altsetting)
> {
> struct btusb_data *data = hci_get_drvdata(hdev);
> @@ -1521,6 +1547,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 void btusb_notify(struct hci_dev *hdev, unsigned int evt)
> +{
> + struct btusb_data *data = hci_get_drvdata(hdev);
> +
> + BT_DBG("%s evt %d", hdev->name, evt);
> +
> + if (hci_conn_num(hdev, SCO_LINK) != data->sco_num) {
> + data->sco_num = hci_conn_num(hdev, SCO_LINK);
> + schedule_work(&data->work);
> + }
Since the internal processing of packets moved from a tasklet to a workqueue, I think the extra workqueue inside btusb can be actually removed. So I would get rid of this also for the non-mSBC case.
> +
> + if (evt == HCI_NOTIFY_AIR_MODE_TRANSP) {
> + /* Alt setting 6 is used for msbc encoded
> + * audio channel
> + */
> + if (bt_switch_alt_setting(hdev, 6) < 0)
> + BT_ERR("%s Set USB Alt6 failed", hdev->name);
> + }
> +}
> +
I think this is really a one or the other. If the controller supports alternate setting 6 (which you actually have to check first) and it is air mode transparent then switch to alternate setting 6. Otherwise do what we have done before.
> static void btusb_work(struct work_struct *work)
> {
> struct btusb_data *data = container_of(work, struct btusb_data, work);
> @@ -1547,37 +1632,7 @@ static void btusb_work(struct work_struct *work)
> } else {
> new_alts = data->sco_num;
> }
> -
> - 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);
> -
> - if (__set_isoc_interface(hdev, new_alts) < 0)
> - return;
> - }
> -
> - 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);
> - }
> + bt_switch_alt_setting(hdev, 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..89ac29f1dffa 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -52,6 +52,7 @@
> #define HCI_NOTIFY_CONN_ADD 1
> #define HCI_NOTIFY_CONN_DEL 2
> #define HCI_NOTIFY_VOICE_SETTING 3
> +#define HCI_NOTIFY_AIR_MODE_TRANSP 4
>
> /* HCI bus types */
> #define HCI_VIRTUAL 0
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index c1d3a303d97f..1c268932422c 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -4231,6 +4231,11 @@ static void hci_sync_conn_complete_evt(struct hci_dev *hdev,
> break;
> }
>
> + if (ev->air_mode == SCO_AIRMODE_TRANSP) {
> + if (hdev->notify)
> + hdev->notify(hdev, HCI_NOTIFY_AIR_MODE_TRANSP);
> + }
> +
So this might work, but causes the USB subsystem to change the alternate setting twice. Once for CONN_ADD and once for SCO_AIRMODE_TRANSP. I prefer we just change the alternate setting once.
Maybe it is better to change CONN_ADD and CONN_DEL to an explicit ENABLE_SCO_CVSD and ENABLE_SCO_TRANSP and DISABLE_SCO.
Regards
Marcel
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] Bluetooth: btusb: hci_event: handle msbc audio over USB Endpoints
@ 2019-09-26 5:35 Amit K Bag
2019-09-26 6:24 ` Marcel Holtmann
0 siblings, 1 reply; 10+ messages in thread
From: Amit K Bag @ 2019-09-26 5:35 UTC (permalink / raw)
To: linux-bluetooth
Cc: ravishankar.srivatsa, chethan.tumkur.narayan, Amit K Bag,
Sathish Narasimman, Raghuram Hegde, Hsin-Yu Chao
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: Chethan T N <chethan.tumkur.narayan@intel.com>
Signed-off-by: Sathish Narasimman <sathish.narasimman@intel.com>
Signed-off-by: Raghuram Hegde <raghuram.hegde@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 | 145 ++++++++++++++++++++++++++++++--------------
include/net/bluetooth/hci.h | 1 +
net/bluetooth/hci_event.c | 5 ++
3 files changed, 106 insertions(+), 45 deletions(-)
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index a9c35ebb30f8..368cc93cb17e 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -57,6 +57,9 @@ static struct usb_driver btusb_driver;
#define BTUSB_IFNUM_2 0x80000
#define BTUSB_CW6622 0x100000
#define BTUSB_MEDIATEK 0x200000
+#define BTUSB_ALT6_FLOW_CNTRL 6
+
+static int set_hci_packet_interval_flow = BTUSB_ALT6_FLOW_CNTRL;
static const struct usb_device_id btusb_table[] = {
/* Generic Bluetooth USB device */
@@ -974,6 +977,38 @@ static void btusb_isoc_complete(struct urb *urb)
}
}
+static inline void __fill_isoc_descriptor_msbc(struct urb *urb, int len,
+ int mtu)
+{
+ int i, offset = 0;
+
+ /* 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 (set_hci_packet_interval_flow == 6)
+ set_hci_packet_interval_flow = 7;
+ else if (set_hci_packet_interval_flow == 7)
+ set_hci_packet_interval_flow = 6;
+
+ BT_DBG("len %d mtu %d", len, mtu);
+
+ for (i = 0; i < set_hci_packet_interval_flow; 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 +1411,12 @@ 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));
+ else
+ __fill_isoc_descriptor(urb, skb->len,
le16_to_cpu(data->isoc_tx_ep->wMaxPacketSize));
-
skb->dev = (void *)hdev;
return urb;
@@ -1466,18 +1504,6 @@ static int btusb_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
return -EILSEQ;
}
-static void btusb_notify(struct hci_dev *hdev, unsigned int evt)
-{
- struct btusb_data *data = hci_get_drvdata(hdev);
-
- BT_DBG("%s evt %d", hdev->name, evt);
-
- if (hci_conn_num(hdev, SCO_LINK) != data->sco_num) {
- data->sco_num = hci_conn_num(hdev, SCO_LINK);
- schedule_work(&data->work);
- }
-}
-
static inline int __set_isoc_interface(struct hci_dev *hdev, int altsetting)
{
struct btusb_data *data = hci_get_drvdata(hdev);
@@ -1521,6 +1547,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 void btusb_notify(struct hci_dev *hdev, unsigned int evt)
+{
+ struct btusb_data *data = hci_get_drvdata(hdev);
+
+ BT_DBG("%s evt %d", hdev->name, evt);
+
+ if (hci_conn_num(hdev, SCO_LINK) != data->sco_num) {
+ data->sco_num = hci_conn_num(hdev, SCO_LINK);
+ schedule_work(&data->work);
+ }
+
+ if (evt == HCI_NOTIFY_AIR_MODE_TRANSP) {
+ /* Alt setting 6 is used for msbc encoded
+ * audio channel
+ */
+ if (bt_switch_alt_setting(hdev, 6) < 0)
+ BT_ERR("%s Set USB Alt6 failed", hdev->name);
+ }
+}
+
static void btusb_work(struct work_struct *work)
{
struct btusb_data *data = container_of(work, struct btusb_data, work);
@@ -1547,37 +1632,7 @@ static void btusb_work(struct work_struct *work)
} else {
new_alts = data->sco_num;
}
-
- 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);
-
- if (__set_isoc_interface(hdev, new_alts) < 0)
- return;
- }
-
- 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);
- }
+ bt_switch_alt_setting(hdev, 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..89ac29f1dffa 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -52,6 +52,7 @@
#define HCI_NOTIFY_CONN_ADD 1
#define HCI_NOTIFY_CONN_DEL 2
#define HCI_NOTIFY_VOICE_SETTING 3
+#define HCI_NOTIFY_AIR_MODE_TRANSP 4
/* HCI bus types */
#define HCI_VIRTUAL 0
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index c1d3a303d97f..1c268932422c 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -4231,6 +4231,11 @@ static void hci_sync_conn_complete_evt(struct hci_dev *hdev,
break;
}
+ if (ev->air_mode == SCO_AIRMODE_TRANSP) {
+ if (hdev->notify)
+ hdev->notify(hdev, HCI_NOTIFY_AIR_MODE_TRANSP);
+ }
+
hci_connect_cfm(conn, ev->status);
if (ev->status)
hci_conn_del(conn);
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH] Bluetooth: btusb: hci_event: handle msbc audio over USB Endpoints
2018-08-20 12:19 [PATCH] Bluetooth: btusb: Handling MSBC encoded Audio stream Sathish Narasimman
@ 2018-08-20 12:19 ` Sathish Narasimman
0 siblings, 0 replies; 10+ messages in thread
From: Sathish Narasimman @ 2018-08-20 12:19 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Sathish Narasimman
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
Signed-off-by: Sathish Narasimman <sathish.narasimman@intel.com>
---
drivers/bluetooth/btusb.c | 95 +++++++++++++++++++++++++--------------------
include/net/bluetooth/hci.h | 1 +
net/bluetooth/hci_event.c | 5 +++
3 files changed, 59 insertions(+), 42 deletions(-)
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index cd2e5cf..ae924b6 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -1390,18 +1390,6 @@ static int btusb_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
return -EILSEQ;
}
-static void btusb_notify(struct hci_dev *hdev, unsigned int evt)
-{
- struct btusb_data *data = hci_get_drvdata(hdev);
-
- BT_DBG("%s evt %d", hdev->name, evt);
-
- if (hci_conn_num(hdev, SCO_LINK) != data->sco_num) {
- data->sco_num = hci_conn_num(hdev, SCO_LINK);
- schedule_work(&data->work);
- }
-}
-
static inline int __set_isoc_interface(struct hci_dev *hdev, int altsetting)
{
struct btusb_data *data = hci_get_drvdata(hdev);
@@ -1445,6 +1433,58 @@ static inline int __set_isoc_interface(struct hci_dev *hdev, int altsetting)
return 0;
}
+static void bt_switch_alt_setting(struct hci_dev *hdev, int new_alts)
+{
+ struct btusb_data *data = hci_get_drvdata(hdev);
+
+ 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);
+
+ if (__set_isoc_interface(hdev, new_alts) < 0)
+ return;
+ }
+ 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);
+ }
+}
+
+static void btusb_notify(struct hci_dev *hdev, unsigned int evt)
+{
+ struct btusb_data *data = hci_get_drvdata(hdev);
+
+ BT_DBG("%s evt %d", hdev->name, evt);
+
+ if (hci_conn_num(hdev, SCO_LINK) != data->sco_num) {
+ data->sco_num = hci_conn_num(hdev, SCO_LINK);
+ schedule_work(&data->work);
+ }
+
+ if (evt == HCI_NOTIFY_AIR_MODE_TRANSP) {
+ /* Alt setting 6 is for msbc encoded audio channel */
+ bt_switch_alt_setting(hdev, 6);
+ }
+}
+
static void btusb_work(struct work_struct *work)
{
struct btusb_data *data = container_of(work, struct btusb_data, work);
@@ -1472,36 +1512,7 @@ static void btusb_work(struct work_struct *work)
new_alts = data->sco_num;
}
- 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);
-
- if (__set_isoc_interface(hdev, new_alts) < 0)
- return;
- }
-
- 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);
- }
+ bt_switch_alt_setting(hdev, 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 cdd9f1f..3498c6b 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -52,6 +52,7 @@
#define HCI_NOTIFY_CONN_ADD 1
#define HCI_NOTIFY_CONN_DEL 2
#define HCI_NOTIFY_VOICE_SETTING 3
+#define HCI_NOTIFY_AIR_MODE_TRANSP 4
/* HCI bus types */
#define HCI_VIRTUAL 0
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index f12555f..8ef5220 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -4100,6 +4100,11 @@ static void hci_sync_conn_complete_evt(struct hci_dev *hdev,
break;
}
+ if (ev->air_mode == SCO_AIRMODE_TRANSP) {
+ if (hdev->notify)
+ hdev->notify(hdev, HCI_NOTIFY_AIR_MODE_TRANSP);
+ }
+
hci_connect_cfm(conn, ev->status);
if (ev->status)
hci_conn_del(conn);
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-11-11 11:04 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-20 13:31 [PATCH] Bluetooth: btusb: Handling MSBC encoded Audio stream Sathish Narasimman
2018-08-20 13:32 ` [PATCH] Bluetooth: btusb: hci_event: handle msbc audio over USB Endpoints Sathish Narasimman
2018-08-20 16:05 ` Luiz Augusto von Dentz
[not found] ` <CAOVXEJ+CLxgr-eZ-98U_U=JFCCu6eiVnuv=R_VwewiOE=PXsmg@mail.gmail.com>
2018-08-21 8:12 ` Luiz Augusto von Dentz
2018-08-21 9:49 ` Sathish Narasimman
2018-08-21 13:58 ` Marcel Holtmann
-- strict thread matches above, loose matches on Subject: below --
2019-09-26 5:35 Amit K Bag
2019-09-26 6:24 ` Marcel Holtmann
2019-11-11 11:04 ` Sathish Narasimman
2018-08-20 12:19 [PATCH] Bluetooth: btusb: Handling MSBC encoded Audio stream Sathish Narasimman
2018-08-20 12:19 ` [PATCH] Bluetooth: btusb: hci_event: handle msbc audio over USB Endpoints Sathish Narasimman
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.