* [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@vger.kernel.org, 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
[parent not found: <CAOVXEJ+CLxgr-eZ-98U_U=JFCCu6eiVnuv=R_VwewiOE=PXsmg@mail.gmail.com>]
* 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@vger.kernel.org, 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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).