Hi Marcel,
> >>>> Workqueue handling btusb_work is no longer needed as btusb_notify
> >>>> can sleep.
> >>>>
> >>>> Signed-off-by: Kuba Pawlak <kubax.t.pawlak@xxxxxxxxx>
> >>>> ---
> >>>> drivers/bluetooth/btusb.c | 14 +++++---------
> >>>> 1 file changed, 5 insertions(+), 9 deletions(-)
> >>>>
> >>>> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> >>>> index
> >>>
> 247b1062cb9affc67d9e7f7a0234c167f90f4b79..98c616ddd7bac53b2d5d5f8b3dcc
> >>> 2dca2b201e63 100644
> >>>> --- a/drivers/bluetooth/btusb.c
> >>>> +++ b/drivers/bluetooth/btusb.c
> >>>> @@ -350,7 +350,6 @@ struct btusb_data {
> >>>>
> >>>> unsigned long flags;
> >>>>
> >>>> - struct work_struct work;
> >>>> struct work_struct waker;
> >>>>
> >>>> struct usb_anchor deferred;
> >>>> @@ -980,7 +979,6 @@ static int btusb_close(struct hci_dev *hdev)
> >>>>
> >>>> BT_DBG("%s", hdev->name);
> >>>>
> >>>> - cancel_work_sync(&data->work);
> >>>> cancel_work_sync(&data->waker);
> >>>>
> >>>> clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
> >>>> @@ -1181,6 +1179,8 @@ static int btusb_send_frame(struct hci_dev
> *hdev,
> >>> struct sk_buff *skb)
> >>>> return -EILSEQ;
> >>>> }
> >>>>
> >>>> +static void btusb_sco_change(struct btusb_data *data);
> >>>> +
> >>>
> >>> do we need this forward declaration or can we just move code around to
> avoid
> >>> it?
> >>>
> >>>> static void btusb_notify(struct hci_dev *hdev, unsigned int evt)
> >>>> {
> >>>> struct btusb_data *data = hci_get_drvdata(hdev);
> >>>> @@ -1189,7 +1189,7 @@ static void btusb_notify(struct hci_dev *hdev,
> >>> unsigned int evt)
> >>>>
> >>>> if (hci_conn_num(hdev, SCO_LINK) != data->sco_num) {
> >>>> data->sco_num = hci_conn_num(hdev, SCO_LINK);
> >>>> - schedule_work(&data->work);
> >>>> + btusb_sco_change(data);
> >>>> }
> >>>> }
> >>>>
> >>>> @@ -1236,9 +1236,8 @@ static inline int __set_isoc_interface(struct
> hci_dev
> >>> *hdev, int altsetting)
> >>>> return 0;
> >>>> }
> >>>>
> >>>> -static void btusb_work(struct work_struct *work)
> >>>> +static void btusb_sco_change(struct btusb_data *data)
> >>>> {
> >>>> - struct btusb_data *data = container_of(work, struct btusb_data,
> >>> work);
> >>>> struct hci_dev *hdev = data->hdev;
> >>>> int new_alts;
> >>>> int err;
> >>>> @@ -2618,7 +2617,6 @@ static int btusb_probe(struct usb_interface
> *intf,
> >>>> data->udev = interface_to_usbdev(intf);
> >>>> data->intf = intf;
> >>>>
> >>>> - INIT_WORK(&data->work, btusb_work);
> >>>> INIT_WORK(&data->waker, btusb_waker);
> >>>> init_usb_anchor(&data->deferred);
> >>>> init_usb_anchor(&data->tx_anchor);
> >>>> @@ -2848,8 +2846,6 @@ static int btusb_suspend(struct usb_interface
> *intf,
> >>> pm_message_t message)
> >>>> return -EBUSY;
> >>>> }
> >>>>
> >>>> - cancel_work_sync(&data->work);
> >>>> -
> >>>> btusb_stop_traffic(data);
> >>>> usb_kill_anchored_urbs(&data->tx_anchor);
> >>>>
> >>>> @@ -2922,7 +2918,7 @@ static int btusb_resume(struct usb_interface
> *intf)
> >>>> play_deferred(data);
> >>>> clear_bit(BTUSB_SUSPENDING, &data->flags);
> >>>> spin_unlock_irq(&data->txlock);
> >>>> - schedule_work(&data->work);
> >>>> + btusb_sco_change(data);
> >>>
> >>> Can we actually sleep in the resume callbacks? I think so, but I have not
> testing
> >>> runtime power management in a while.
> >>
> >> Apparently we can't, so workqueue needs to stay and this patch is invalid.
> >
> > I think that removing the usage of the workqueue in btusb_notify is still a
> good thing since it reduces the latency when establishing SCO connections.
>
> actually I wonder if the call to usb_set_interface is needed on resume in the
> first place. Maybe this is actually not needed and we just need to restart the
> URBs.
>
> Do you have the chance to test and see if the alternate setting actually changed
> over resume?
Our platform does not support suspend/resume. I have an Ellisys analyser with external WP module connected through it over USB, so I can trace what altsettings are set and when. I will look into a possibility of testing this using a laptop, but I don’t know how to easily make phone open SCO channel, so I don't need to port our PulseAudio.
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���)ߣ�
