Hi Marcel,
Timo Mueller wrote, On 14.10.2013 11:20:
Hi Marcel,
Am 11.10.2013 14:36, schrieb Marcel Holtmann:
Hi Timo,
Since the event handling is done within a workqueue, the notify
callback can now sleep. So no need to trigger a separate workqueue
from within the Bluetooth USB driver.
This should give a little bit better latency with the SCO packet
processing since the ISOC altsetting is correct from the beginning.
However I am not sure if we can actually sleep in the USB reset
handler and what we need to do to restore the correct altsetting
in there. This could potentially fail, so please test ;)
Signed-off-by: Marcel Holtmann <marcel@xxxxxxxxxxxx>
---
drivers/bluetooth/btusb.c | 34 ++++++++++++++--------------------
1 file changed, 14 insertions(+), 20 deletions(-)
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index f3dfc0a..32cae73 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -245,7 +245,6 @@ struct btusb_data {
unsigned long flags;
- struct work_struct work;
struct work_struct waker;
struct usb_anchor tx_anchor;
@@ -685,7 +684,6 @@ static int btusb_close(struct hci_dev *hdev)
if (!test_and_clear_bit(HCI_RUNNING, &hdev->flags))
return 0;
- cancel_work_sync(&data->work);
cancel_work_sync(&data->waker);
clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
@@ -827,18 +825,6 @@ done:
return err;
}
-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 (hdev->conn_hash.sco_num != data->sco_num) {
- data->sco_num = hdev->conn_hash.sco_num;
- schedule_work(&data->work);
- }
-}
-
static inline int __set_isoc_interface(struct hci_dev *hdev, int
altsetting)
{
struct btusb_data *data = hci_get_drvdata(hdev);
@@ -882,9 +868,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_update_isoc_altsetting(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;
@@ -932,6 +917,18 @@ static void btusb_work(struct work_struct *work)
}
}
+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 (hdev->conn_hash.sco_num != data->sco_num) {
+ data->sco_num = hdev->conn_hash.sco_num;
+ btusb_update_isoc_altsetting(data);
+ }
+}
+
static void btusb_waker(struct work_struct *work)
{
struct btusb_data *data = container_of(work, struct
btusb_data, waker);
@@ -1404,7 +1401,6 @@ static int btusb_probe(struct usb_interface
*intf,
spin_lock_init(&data->lock);
- INIT_WORK(&data->work, btusb_work);
INIT_WORK(&data->waker, btusb_waker);
spin_lock_init(&data->txlock);
@@ -1540,8 +1536,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);
@@ -1606,8 +1600,8 @@ 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_update_isoc_altsetting(data);
return 0;
failed:
I have been testing this patch for the last two days at the UPF in
Vienna. It was running fine most of the time, but I experienced two
crashes. Both crashes appeared when there was an active call and the
phone transferred audio to the phone and back. Both times I wasn't
able to reproduce, when I restarted everything and tested again it
worked fine.
Unfortunately the kernel log is not complete but, when it failed the
kernel reported:
[147.344546] Bluetooth: hci0 SCO packet for unknown connection handle 5
[147.354515] Bluetooth: hci0 SCO packet for unknown connection handle 21
[147.354537] Bluetooth: hci0 SCO packet for unknown connection handle 29
[147.354548] Bluetooth: hci0 SCO packet for unknown connection handle
65534
[147.364574] Bluetooth: hci0 SCO packet for unknown connection handle
65532
[147.364581] Bluetooth: hci0 SCO packet for unknown connection handle 27
…
what kind of hardware where you testing with?
It was nothing special, an off-the-shelf CSR USB BT Dongle.
This handle mismatch normally means that our SCO packet frames are out
of sync. I think we are not doing a good job trying to keep them
nicely lined up.
For the crash, do you happen to have a backtrace of the crash.
Personally I was worried about the reset handling and not the actual
alternate setting switching.
Unfortunately I haven't. To be precise the first crash wasn't even a
crash, as everything was continuing to run, but without a working audio
of course. The second time my system completely froze, but due to my
fault I wasn't able to pull the backtrace. I'll see if I can reproduce
this week and send a backtrace if I manage to.
I've been unsuccessfully trying to reproduce the system crash with an
iPhone 3 and a Nexus 4. Routing the Audio to the phone and back
occasionally leads to the observed handle mismatches, but no
crashes/freezes whatsoever. Sometimes the mismatch is even recovered.
Regards
Marcel
Regards,
Timo
Regards,
Timo
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
