2009/4/3 Hal Murray <hmurray@xxxxxxxxxxxxxxx>:
> It worked in 2.6.26.5. It's broken in 2.6.27.9
>
> I'm slightly surprised it's still broken and/or I don't find more commentary
> via google. It dies every time for me. Maybe my code is doing something
> different. Maybe the garmin driver doesn't get used much. (I sure was happy
> to find it when I wanted it a year or two ago.)
>
> I did find this:
> http://bugs.archlinux.org/task/12079
> Which mostly confirms that there is a problem and suggests switching to
> libusb.
>
> Here is what I've found:
>
> garmin_close does
> mutex_lock(&port->serial->disc_mutex);
>
> then it (maybe) calls:
> process_resetdev_request (in garmin_gps)
>
> that calls usb_reset_device
> I haven't found where that locks the mutex.
>
> It works for me if I move the unlock to before that call. As I understand
> it, that mutex is only to make sure multiple instances of open and close
> don't trip over eachother. I'm not doing that. I don't know if I could
> cause troubles if I tried to write some nasty code.
IMO, the deadlock is caused by mutex_lock(&port->serial->disc_mutex)
(called in garmin_close() and usb_serial_disconnect() ). The garmin usb_driver
doesn't define .pre_reset and .post_reset, so usb_reset_devie() will unbound
garmin usb_driver before doing bus reset, which makes .disconnect() called,
then deadlock happens.
Maybe, attachment patch will fix your deadlock, which is against 2.6.29.
Thanks.
>
>
> I don't know my way around this area. I was expecting all the serial drivers
> to be similar and simple. The garmin driver has a lot of code that I
> expected to be in user space. (I didn't study it carefully.)
>
> The pl2303 driver doesn't lock the mutex or call usb_reset_device.
>
> ftdi_close in ftdi_sio.c locks the mutex so it can disable flow control and
> drop RTS and DTR. I don't see anything that looks tricky in there. It
> doesn't call usb_reset_device.
>
> I can easily test things if somebody who understands this area wants to take
> a look at it and propose a clean/official fix.
>
>
>
> --
> These are my opinions, not necessarily my employer's. I hate spam.
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Lei Ming
diff --git a/drivers/usb/serial/garmin_gps.c b/drivers/usb/serial/garmin_gps.c
index a26a0e2..8c8b716 100644
--- a/drivers/usb/serial/garmin_gps.c
+++ b/drivers/usb/serial/garmin_gps.c
@@ -224,10 +224,22 @@ static struct usb_device_id id_table [] = {
MODULE_DEVICE_TABLE(usb, id_table);
+static int garmin_pre_reset(struct usb_interface *intf)
+{
+ return 0;
+}
+
+static int garmin_post_reset(struct usb_interface *intf)
+{
+ return 0;
+}
+
static struct usb_driver garmin_driver = {
.name = "garmin_gps",
.probe = usb_serial_probe,
.disconnect = usb_serial_disconnect,
+ .pre_reset = garmin_pre_reset,
+ .post_reset = garmin_post_reset,
.id_table = id_table,
.no_dynamic_id = 1,
};
