* [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; 7+ 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] 7+ 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; 7+ 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] 7+ 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; 7+ 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] 7+ 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; 7+ 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] 7+ 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; 7+ 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] 7+ 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; 7+ 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] 7+ messages in thread
* [PATCH] Bluetooth: btusb: Handling MSBC encoded Audio stream
@ 2018-08-20 12:19 Sathish Narasimman
0 siblings, 0 replies; 7+ messages in thread
From: Sathish Narasimman @ 2018-08-20 12:19 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Sathish Narasimman
Msbc encodec 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] 7+ messages in thread
end of thread, other threads:[~2018-08-21 13:58 UTC | newest]
Thread overview: 7+ 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 --
2018-08-20 12:19 [PATCH] Bluetooth: btusb: Handling MSBC encoded Audio stream 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.