HI Marcel,
> > For SCO connections, btusb driver sets alternate endpoint
> > setting to 2 by default (17 bytes per USB packet).
> > When connection is completed and airmode transparent has been
> negotiated,
> > alternate setting are not changed to 1 (9 bytes per USB packet).
> > This results in wrong packets being sent and "robotic" audio.
> >
> > Notify the driver that endpoint settings must change.
> >
> > Signed-off-by: Kuba Pawlak <kubax.t.pawlak@xxxxxxxxx>
> > ---
> > drivers/bluetooth/btusb.c | 12 +++++++++++-
> > net/bluetooth/hci_event.c | 5 +++++
> > 2 files changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> > index
> 247b1062cb9affc67d9e7f7a0234c167f90f4b79..65ce054d0755db9078a0f0813828
> dbb278ea8f1c 100644
> > --- a/drivers/bluetooth/btusb.c
> > +++ b/drivers/bluetooth/btusb.c
> > @@ -1191,6 +1191,9 @@ static void btusb_notify(struct hci_dev *hdev,
> unsigned int evt)
> > data->sco_num = hci_conn_num(hdev, SCO_LINK);
> > schedule_work(&data->work);
> > }
> > +
> > + if (evt == HCI_NOTIFY_VOICE_SETTING)
> > + schedule_work(&data->work);
> > }
>
> actually can you try to take the schedule_work out here. Since hdev->notify is
> no longer called from bottom half, it is perfectly fine to sleep in this callback.
> This is a leftover from a long time ago where all the packet processing was done
> in the bottom half.
>
> Once that has been removed, I think you can just switch the alternate setting
> right in btusb_notify.
Pardon my ignorance, but I need to make sure that this would involve removing 'work' workqueue and calling btusb_work (renamed to not contain _work) everywhere, where schedule_work(&data->work); is called?
>
> >
> > static inline int __set_isoc_interface(struct hci_dev *hdev, int altsetting)
> > @@ -1255,7 +1258,11 @@ static void btusb_work(struct work_struct *work)
> > set_bit(BTUSB_DID_ISO_RESUME, &data->flags);
> > }
> >
> > - if (hdev->voice_setting & 0x0020) {
> > + if (hdev->voice_setting & BT_VOICE_TRANSPARENT) {
> > + static const int alts[3] = { 1, 2, 3 };
> > +
> > + new_alts = alts[data->sco_num - 1];
> > + } else if (hdev->voice_setting & 0x0020) {
> > static const int alts[3] = { 2, 4, 5 };
> >
> > new_alts = alts[data->sco_num - 1];
> > @@ -1267,6 +1274,9 @@ static void btusb_work(struct work_struct *work)
> > clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
> > usb_kill_anchored_urbs(&data->isoc_anchor);
> >
> > + if (hdev->voice_setting & BT_VOICE_TRANSPARENT)
> > + hdev->voice_setting &=
> ~BT_VOICE_TRANSPARENT;
> > +
> > /* When isochronous alternate setting needs to be
> > * changed, because SCO connection has been added
> > * or removed, a packet fragment may be left in the
> > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> > index
> 8acec932123ade906192ebbfd2daecee9b51f697..08e77537c6eb69c9727cb41a2ca
> 8429f4d7a4941 100644
> > --- a/net/bluetooth/hci_event.c
> > +++ b/net/bluetooth/hci_event.c
> > @@ -3770,6 +3770,11 @@ static void hci_sync_conn_complete_evt(struct
> hci_dev *hdev,
> > break;
> > }
> >
> > + if (ev->air_mode == SCO_AIRMODE_TRANSP) {
> > + hdev->voice_setting |= BT_VOICE_TRANSPARENT;
> > + hdev->notify(hdev, HCI_NOTIFY_VOICE_SETTING);
> > + }
> > +
>
> This is actually something that is not fully correct. The hdev->voice_setting is
> suppose to represent the value of the setting. It should not be affected by the
> connections. Especially since this gets tricky when you have two eSCO
> connections. One using transparent and another using CVSD.
>
> What we might need to do here is actually track the number of transparent vs
> non-transparent eSCO connections separately.
Is it possible to have transparent and CVSD connections at the same time? Core spec 4.0, Vol 4, part B, 2.1.1 specifies endpoint configuration for multiples of 8bit or 16 bit connections only but not a mix of these. Also, second example provided only shows two connections of the same, 8bit type. I think this example forbids a mix of channels. Two SCO channels need to share USB endpoint, but a rule of having each SCO data packet divided into 3 fragments, each sent every 1ms needs to be preserved. That means that when two connections are carried over USB, packet frequency per channel is halved but packet size doubles. This can only be achieved if both connection have the same packet size. In case of 16 and 8 bit coexistence, 8bit channel would be required to increase its packets to 48 bit, which translates to 3 fragment 17 bytes each every 1ms, but 16bit channel would require 96 byte packets, which means 33 byte fragments every 1ms. One endpoint cannot support it. So when we have a transparent SCO link, we can only add more transparent SCO links and this device is BT_VOICE_TRANSPARENT only. Or am I fundamentally wrong?
I have no way of testing multiple SCO links.
Regards,
Kuba
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Prof. Dr. Hermann Eul
Chairperson of the Supervisory Board: Tiffany Doon Silva
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
��.n��������+%������w��{.n�����{��� �^n�r������&��z�ޗ�zf���h���~����������_��+v���)ߣ�
