| Message ID | 20090108235304.46ac523b@pedra.chehab.org (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
On Thu, 8 Jan 2009, Mauro Carvalho Chehab wrote: > Hi Mike, > > There were an upstream change usb_lock_device_for_reset() that touched on > pvrusb2 driver. I didn't backport it yet, since I'm not sure if the change is > ok. Could you please check? > > Thanks, > Mauro. Yes, the pvrusb2 part of this change looks fine (just change the treatment of the return code). Acked-By: Mike Isely <isely@pobox.com> I expect this weekend to be working through a backlog of pvrusb2 issues so you might hear more from me soon :-) -Mike > > commit 011b15df465745474e3ec85482633685933ed5a7 > Author: Alan Stern <stern@rowland.harvard.edu> > Date: Tue Nov 4 11:29:27 2008 -0500 > > USB: change interface to usb_lock_device_for_reset() > > This patch (as1161) changes the interface to > usb_lock_device_for_reset(). The existing interface is apparently not > very clear, judging from the fact that several of its callers don't > use it correctly. The new interface always returns 0 for success and > it always requires the caller to unlock the device afterward. > > The new routine will not return immediately if it is called while the > driver's probe method is running. Instead it will wait until the > probe is over and the device has been unlocked. This shouldn't cause > any problems; I don't know of any cases where drivers call > usb_lock_device_for_reset() during probe. > > Signed-off-by: Alan Stern <stern@rowland.harvard.edu> > Cc: Pete Zaitcev <zaitcev@redhat.com> > Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de> > > diff --git a/drivers/block/ub.c b/drivers/block/ub.c > index 048d71d..12fb816 100644 > --- a/drivers/block/ub.c > +++ b/drivers/block/ub.c > @@ -1579,7 +1579,7 @@ static void ub_reset_task(struct work_struct *work) > struct ub_dev *sc = container_of(work, struct ub_dev, reset_work); > unsigned long flags; > struct ub_lun *lun; > - int lkr, rc; > + int rc; > > if (!sc->reset) { > printk(KERN_WARNING "%s: Running reset unrequested\n", > @@ -1597,10 +1597,11 @@ static void ub_reset_task(struct work_struct *work) > } else if (sc->dev->actconfig->desc.bNumInterfaces != 1) { > ; > } else { > - if ((lkr = usb_lock_device_for_reset(sc->dev, sc->intf)) < 0) { > + rc = usb_lock_device_for_reset(sc->dev, sc->intf); > + if (rc < 0) { > printk(KERN_NOTICE > "%s: usb_lock_device_for_reset failed (%d)\n", > - sc->name, lkr); > + sc->name, rc); > } else { > rc = usb_reset_device(sc->dev); > if (rc < 0) { > @@ -1608,9 +1609,7 @@ static void ub_reset_task(struct work_struct *work) > "usb_lock_device_for_reset failed (%d)\n", > sc->name, rc); > } > - > - if (lkr) > - usb_unlock_device(sc->dev); > + usb_unlock_device(sc->dev); > } > } > > diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c > index 03cb494..f0a0f72 100644 > --- a/drivers/hid/usbhid/hid-core.c > +++ b/drivers/hid/usbhid/hid-core.c > @@ -102,7 +102,7 @@ static void hid_reset(struct work_struct *work) > struct usbhid_device *usbhid = > container_of(work, struct usbhid_device, reset_work); > struct hid_device *hid = usbhid->hid; > - int rc_lock, rc = 0; > + int rc = 0; > > if (test_bit(HID_CLEAR_HALT, &usbhid->iofl)) { > dev_dbg(&usbhid->intf->dev, "clear halt\n"); > @@ -113,11 +113,10 @@ static void hid_reset(struct work_struct *work) > > else if (test_bit(HID_RESET_PENDING, &usbhid->iofl)) { > dev_dbg(&usbhid->intf->dev, "resetting device\n"); > - rc = rc_lock = usb_lock_device_for_reset(hid_to_usb_dev(hid), usbhid->intf); > - if (rc_lock >= 0) { > + rc = usb_lock_device_for_reset(hid_to_usb_dev(hid), usbhid->intf); > + if (rc == 0) { > rc = usb_reset_device(hid_to_usb_dev(hid)); > - if (rc_lock) > - usb_unlock_device(hid_to_usb_dev(hid)); > + usb_unlock_device(hid_to_usb_dev(hid)); > } > clear_bit(HID_RESET_PENDING, &usbhid->iofl); > } > diff --git a/drivers/media/video/pvrusb2/pvrusb2-hdw.c b/drivers/media/video/pvrusb2/pvrusb2-hdw.c > index 8fb92ac..fa304e5 100644 > --- a/drivers/media/video/pvrusb2/pvrusb2-hdw.c > +++ b/drivers/media/video/pvrusb2/pvrusb2-hdw.c > @@ -3655,7 +3655,7 @@ void pvr2_hdw_device_reset(struct pvr2_hdw *hdw) > int ret; > pvr2_trace(PVR2_TRACE_INIT,"Performing a device reset..."); > ret = usb_lock_device_for_reset(hdw->usb_dev,NULL); > - if (ret == 1) { > + if (ret == 0) { > ret = usb_reset_device(hdw->usb_dev); > usb_unlock_device(hdw->usb_dev); > } else { > diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c > index 399e15f..400fa4c 100644 > --- a/drivers/usb/core/usb.c > +++ b/drivers/usb/core/usb.c > @@ -513,10 +513,7 @@ EXPORT_SYMBOL_GPL(usb_put_intf); > * disconnect; in some drivers (such as usb-storage) the disconnect() > * or suspend() method will block waiting for a device reset to complete. > * > - * Returns a negative error code for failure, otherwise 1 or 0 to indicate > - * that the device will or will not have to be unlocked. (0 can be > - * returned when an interface is given and is BINDING, because in that > - * case the driver already owns the device lock.) > + * Returns a negative error code for failure, otherwise 0. > */ > int usb_lock_device_for_reset(struct usb_device *udev, > const struct usb_interface *iface) > @@ -527,16 +524,9 @@ int usb_lock_device_for_reset(struct usb_device *udev, > return -ENODEV; > if (udev->state == USB_STATE_SUSPENDED) > return -EHOSTUNREACH; > - if (iface) { > - switch (iface->condition) { > - case USB_INTERFACE_BINDING: > - return 0; > - case USB_INTERFACE_BOUND: > - break; > - default: > - return -EINTR; > - } > - } > + if (iface && (iface->condition == USB_INTERFACE_UNBINDING || > + iface->condition == USB_INTERFACE_UNBOUND)) > + return -EINTR; > > while (usb_trylock_device(udev) != 0) { > > @@ -550,10 +540,11 @@ int usb_lock_device_for_reset(struct usb_device *udev, > return -ENODEV; > if (udev->state == USB_STATE_SUSPENDED) > return -EHOSTUNREACH; > - if (iface && iface->condition != USB_INTERFACE_BOUND) > + if (iface && (iface->condition == USB_INTERFACE_UNBINDING || > + iface->condition == USB_INTERFACE_UNBOUND)) > return -EINTR; > } > - return 1; > + return 0; > } > EXPORT_SYMBOL_GPL(usb_lock_device_for_reset); > > diff --git a/drivers/usb/image/microtek.c b/drivers/usb/image/microtek.c > index 885867a..4541dfc 100644 > --- a/drivers/usb/image/microtek.c > +++ b/drivers/usb/image/microtek.c > @@ -350,17 +350,16 @@ static int mts_scsi_abort(struct scsi_cmnd *srb) > static int mts_scsi_host_reset(struct scsi_cmnd *srb) > { > struct mts_desc* desc = (struct mts_desc*)(srb->device->host->hostdata[0]); > - int result, rc; > + int result; > > MTS_DEBUG_GOT_HERE(); > mts_debug_dump(desc); > > - rc = usb_lock_device_for_reset(desc->usb_dev, desc->usb_intf); > - if (rc < 0) > - return FAILED; > - result = usb_reset_device(desc->usb_dev); > - if (rc) > + result = usb_lock_device_for_reset(desc->usb_dev, desc->usb_intf); > + if (result == 0) { > + result = usb_reset_device(desc->usb_dev); > usb_unlock_device(desc->usb_dev); > + } > return result ? FAILED : SUCCESS; > } > > diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c > index 79108d5..2058db4 100644 > --- a/drivers/usb/storage/transport.c > +++ b/drivers/usb/storage/transport.c > @@ -1173,10 +1173,9 @@ int usb_stor_Bulk_reset(struct us_data *us) > */ > int usb_stor_port_reset(struct us_data *us) > { > - int result, rc_lock; > + int result; > > - result = rc_lock = > - usb_lock_device_for_reset(us->pusb_dev, us->pusb_intf); > + result = usb_lock_device_for_reset(us->pusb_dev, us->pusb_intf); > if (result < 0) > US_DEBUGP("unable to lock device for reset: %d\n", result); > else { > @@ -1189,8 +1188,7 @@ int usb_stor_port_reset(struct us_data *us) > US_DEBUGP("usb_reset_device returns %d\n", > result); > } > - if (rc_lock) > - usb_unlock_device(us->pusb_dev); > + usb_unlock_device(us->pusb_dev); > } > return result; > } >
On Thu, 8 Jan 2009, Mike Isely wrote: > > > On Thu, 8 Jan 2009, Mauro Carvalho Chehab wrote: > > > Hi Mike, > > > > There were an upstream change usb_lock_device_for_reset() that touched on > > pvrusb2 driver. I didn't backport it yet, since I'm not sure if the change is > > ok. Could you please check? > > > > Thanks, > > Mauro. > > Yes, the pvrusb2 part of this change looks fine (just change the > treatment of the return code). Before I cause any confusion, the above sentence has a critical typo. I was just pointing out that the pvrusb2 change in the patch below only adjusts the treatment of the return code, which makes perfect sense given the upstream change. I wasn't asking you to change anything :-) The change is fine. -Mike > > Acked-By: Mike Isely <isely@pobox.com> > > I expect this weekend to be working through a backlog of pvrusb2 issues > so you might hear more from me soon :-) > > -Mike > > > > > > commit 011b15df465745474e3ec85482633685933ed5a7 > > Author: Alan Stern <stern@rowland.harvard.edu> > > Date: Tue Nov 4 11:29:27 2008 -0500 > > > > USB: change interface to usb_lock_device_for_reset() > > > > This patch (as1161) changes the interface to > > usb_lock_device_for_reset(). The existing interface is apparently not > > very clear, judging from the fact that several of its callers don't > > use it correctly. The new interface always returns 0 for success and > > it always requires the caller to unlock the device afterward. > > > > The new routine will not return immediately if it is called while the > > driver's probe method is running. Instead it will wait until the > > probe is over and the device has been unlocked. This shouldn't cause > > any problems; I don't know of any cases where drivers call > > usb_lock_device_for_reset() during probe. > > > > Signed-off-by: Alan Stern <stern@rowland.harvard.edu> > > Cc: Pete Zaitcev <zaitcev@redhat.com> > > Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de> > > > > diff --git a/drivers/block/ub.c b/drivers/block/ub.c > > index 048d71d..12fb816 100644 > > --- a/drivers/block/ub.c > > +++ b/drivers/block/ub.c > > @@ -1579,7 +1579,7 @@ static void ub_reset_task(struct work_struct *work) > > struct ub_dev *sc = container_of(work, struct ub_dev, reset_work); > > unsigned long flags; > > struct ub_lun *lun; > > - int lkr, rc; > > + int rc; > > > > if (!sc->reset) { > > printk(KERN_WARNING "%s: Running reset unrequested\n", > > @@ -1597,10 +1597,11 @@ static void ub_reset_task(struct work_struct *work) > > } else if (sc->dev->actconfig->desc.bNumInterfaces != 1) { > > ; > > } else { > > - if ((lkr = usb_lock_device_for_reset(sc->dev, sc->intf)) < 0) { > > + rc = usb_lock_device_for_reset(sc->dev, sc->intf); > > + if (rc < 0) { > > printk(KERN_NOTICE > > "%s: usb_lock_device_for_reset failed (%d)\n", > > - sc->name, lkr); > > + sc->name, rc); > > } else { > > rc = usb_reset_device(sc->dev); > > if (rc < 0) { > > @@ -1608,9 +1609,7 @@ static void ub_reset_task(struct work_struct *work) > > "usb_lock_device_for_reset failed (%d)\n", > > sc->name, rc); > > } > > - > > - if (lkr) > > - usb_unlock_device(sc->dev); > > + usb_unlock_device(sc->dev); > > } > > } > > > > diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c > > index 03cb494..f0a0f72 100644 > > --- a/drivers/hid/usbhid/hid-core.c > > +++ b/drivers/hid/usbhid/hid-core.c > > @@ -102,7 +102,7 @@ static void hid_reset(struct work_struct *work) > > struct usbhid_device *usbhid = > > container_of(work, struct usbhid_device, reset_work); > > struct hid_device *hid = usbhid->hid; > > - int rc_lock, rc = 0; > > + int rc = 0; > > > > if (test_bit(HID_CLEAR_HALT, &usbhid->iofl)) { > > dev_dbg(&usbhid->intf->dev, "clear halt\n"); > > @@ -113,11 +113,10 @@ static void hid_reset(struct work_struct *work) > > > > else if (test_bit(HID_RESET_PENDING, &usbhid->iofl)) { > > dev_dbg(&usbhid->intf->dev, "resetting device\n"); > > - rc = rc_lock = usb_lock_device_for_reset(hid_to_usb_dev(hid), usbhid->intf); > > - if (rc_lock >= 0) { > > + rc = usb_lock_device_for_reset(hid_to_usb_dev(hid), usbhid->intf); > > + if (rc == 0) { > > rc = usb_reset_device(hid_to_usb_dev(hid)); > > - if (rc_lock) > > - usb_unlock_device(hid_to_usb_dev(hid)); > > + usb_unlock_device(hid_to_usb_dev(hid)); > > } > > clear_bit(HID_RESET_PENDING, &usbhid->iofl); > > } > > diff --git a/drivers/media/video/pvrusb2/pvrusb2-hdw.c b/drivers/media/video/pvrusb2/pvrusb2-hdw.c > > index 8fb92ac..fa304e5 100644 > > --- a/drivers/media/video/pvrusb2/pvrusb2-hdw.c > > +++ b/drivers/media/video/pvrusb2/pvrusb2-hdw.c > > @@ -3655,7 +3655,7 @@ void pvr2_hdw_device_reset(struct pvr2_hdw *hdw) > > int ret; > > pvr2_trace(PVR2_TRACE_INIT,"Performing a device reset..."); > > ret = usb_lock_device_for_reset(hdw->usb_dev,NULL); > > - if (ret == 1) { > > + if (ret == 0) { > > ret = usb_reset_device(hdw->usb_dev); > > usb_unlock_device(hdw->usb_dev); > > } else { > > diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c > > index 399e15f..400fa4c 100644 > > --- a/drivers/usb/core/usb.c > > +++ b/drivers/usb/core/usb.c > > @@ -513,10 +513,7 @@ EXPORT_SYMBOL_GPL(usb_put_intf); > > * disconnect; in some drivers (such as usb-storage) the disconnect() > > * or suspend() method will block waiting for a device reset to complete. > > * > > - * Returns a negative error code for failure, otherwise 1 or 0 to indicate > > - * that the device will or will not have to be unlocked. (0 can be > > - * returned when an interface is given and is BINDING, because in that > > - * case the driver already owns the device lock.) > > + * Returns a negative error code for failure, otherwise 0. > > */ > > int usb_lock_device_for_reset(struct usb_device *udev, > > const struct usb_interface *iface) > > @@ -527,16 +524,9 @@ int usb_lock_device_for_reset(struct usb_device *udev, > > return -ENODEV; > > if (udev->state == USB_STATE_SUSPENDED) > > return -EHOSTUNREACH; > > - if (iface) { > > - switch (iface->condition) { > > - case USB_INTERFACE_BINDING: > > - return 0; > > - case USB_INTERFACE_BOUND: > > - break; > > - default: > > - return -EINTR; > > - } > > - } > > + if (iface && (iface->condition == USB_INTERFACE_UNBINDING || > > + iface->condition == USB_INTERFACE_UNBOUND)) > > + return -EINTR; > > > > while (usb_trylock_device(udev) != 0) { > > > > @@ -550,10 +540,11 @@ int usb_lock_device_for_reset(struct usb_device *udev, > > return -ENODEV; > > if (udev->state == USB_STATE_SUSPENDED) > > return -EHOSTUNREACH; > > - if (iface && iface->condition != USB_INTERFACE_BOUND) > > + if (iface && (iface->condition == USB_INTERFACE_UNBINDING || > > + iface->condition == USB_INTERFACE_UNBOUND)) > > return -EINTR; > > } > > - return 1; > > + return 0; > > } > > EXPORT_SYMBOL_GPL(usb_lock_device_for_reset); > > > > diff --git a/drivers/usb/image/microtek.c b/drivers/usb/image/microtek.c > > index 885867a..4541dfc 100644 > > --- a/drivers/usb/image/microtek.c > > +++ b/drivers/usb/image/microtek.c > > @@ -350,17 +350,16 @@ static int mts_scsi_abort(struct scsi_cmnd *srb) > > static int mts_scsi_host_reset(struct scsi_cmnd *srb) > > { > > struct mts_desc* desc = (struct mts_desc*)(srb->device->host->hostdata[0]); > > - int result, rc; > > + int result; > > > > MTS_DEBUG_GOT_HERE(); > > mts_debug_dump(desc); > > > > - rc = usb_lock_device_for_reset(desc->usb_dev, desc->usb_intf); > > - if (rc < 0) > > - return FAILED; > > - result = usb_reset_device(desc->usb_dev); > > - if (rc) > > + result = usb_lock_device_for_reset(desc->usb_dev, desc->usb_intf); > > + if (result == 0) { > > + result = usb_reset_device(desc->usb_dev); > > usb_unlock_device(desc->usb_dev); > > + } > > return result ? FAILED : SUCCESS; > > } > > > > diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c > > index 79108d5..2058db4 100644 > > --- a/drivers/usb/storage/transport.c > > +++ b/drivers/usb/storage/transport.c > > @@ -1173,10 +1173,9 @@ int usb_stor_Bulk_reset(struct us_data *us) > > */ > > int usb_stor_port_reset(struct us_data *us) > > { > > - int result, rc_lock; > > + int result; > > > > - result = rc_lock = > > - usb_lock_device_for_reset(us->pusb_dev, us->pusb_intf); > > + result = usb_lock_device_for_reset(us->pusb_dev, us->pusb_intf); > > if (result < 0) > > US_DEBUGP("unable to lock device for reset: %d\n", result); > > else { > > @@ -1189,8 +1188,7 @@ int usb_stor_port_reset(struct us_data *us) > > US_DEBUGP("usb_reset_device returns %d\n", > > result); > > } > > - if (rc_lock) > > - usb_unlock_device(us->pusb_dev); > > + usb_unlock_device(us->pusb_dev); > > } > > return result; > > } > > > >
On Thu, 8 Jan 2009 22:28:18 -0600 (CST) Mike Isely <isely@isely.net> wrote: > On Thu, 8 Jan 2009, Mike Isely wrote: > > > > > > > On Thu, 8 Jan 2009, Mauro Carvalho Chehab wrote: > > > > > Hi Mike, > > > > > > There were an upstream change usb_lock_device_for_reset() that touched on > > > pvrusb2 driver. I didn't backport it yet, since I'm not sure if the change is > > > ok. Could you please check? > > > > > > Thanks, > > > Mauro. > > > > Yes, the pvrusb2 part of this change looks fine (just change the > > treatment of the return code). > > Before I cause any confusion, the above sentence has a critical typo. > I was just pointing out that the pvrusb2 change in the patch below only > adjusts the treatment of the return code, which makes perfect sense > given the upstream change. I wasn't asking you to change anything :-) > The change is fine. Ok. The backport of this patch would be something interesting... It will be something like this: #if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,29) if (ret == 0) #else if (ret == 1) #endif Cheers, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 9 Jan 2009, Mauro Carvalho Chehab wrote: > On Thu, 8 Jan 2009 22:28:18 -0600 (CST) > Mike Isely <isely@isely.net> wrote: > > > On Thu, 8 Jan 2009, Mike Isely wrote: > > > > > > > > > > > On Thu, 8 Jan 2009, Mauro Carvalho Chehab wrote: > > > > > > > Hi Mike, > > > > > > > > There were an upstream change usb_lock_device_for_reset() that touched on > > > > pvrusb2 driver. I didn't backport it yet, since I'm not sure if the change is > > > > ok. Could you please check? > > > > > > > > Thanks, > > > > Mauro. > > > > > > Yes, the pvrusb2 part of this change looks fine (just change the > > > treatment of the return code). > > > > Before I cause any confusion, the above sentence has a critical typo. > > I was just pointing out that the pvrusb2 change in the patch below only > > adjusts the treatment of the return code, which makes perfect sense > > given the upstream change. I wasn't asking you to change anything :-) > > The change is fine. > > Ok. The backport of this patch would be something interesting... > > It will be something like this: > > #if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,29) > if (ret == 0) > #else > if (ret == 1) > #endif > Yeah, I know. It's part of the fun of staying in sync with multiple kernels and their ever-changing internal interfaces. I have to support this in the standalone pvrusb2 driver too. One more task this weekend... -Mike
diff --git a/drivers/block/ub.c b/drivers/block/ub.c index 048d71d..12fb816 100644 --- a/drivers/block/ub.c +++ b/drivers/block/ub.c @@ -1579,7 +1579,7 @@ static void ub_reset_task(struct work_struct *work) struct ub_dev *sc = container_of(work, struct ub_dev, reset_work); unsigned long flags; struct ub_lun *lun; - int lkr, rc; + int rc; if (!sc->reset) { printk(KERN_WARNING "%s: Running reset unrequested\n", @@ -1597,10 +1597,11 @@ static void ub_reset_task(struct work_struct *work) } else if (sc->dev->actconfig->desc.bNumInterfaces != 1) { ; } else { - if ((lkr = usb_lock_device_for_reset(sc->dev, sc->intf)) < 0) { + rc = usb_lock_device_for_reset(sc->dev, sc->intf); + if (rc < 0) { printk(KERN_NOTICE "%s: usb_lock_device_for_reset failed (%d)\n", - sc->name, lkr); + sc->name, rc); } else { rc = usb_reset_device(sc->dev); if (rc < 0) { @@ -1608,9 +1609,7 @@ static void ub_reset_task(struct work_struct *work) "usb_lock_device_for_reset failed (%d)\n", sc->name, rc); } - - if (lkr) - usb_unlock_device(sc->dev); + usb_unlock_device(sc->dev); } } diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c index 03cb494..f0a0f72 100644 --- a/drivers/hid/usbhid/hid-core.c +++ b/drivers/hid/usbhid/hid-core.c @@ -102,7 +102,7 @@ static void hid_reset(struct work_struct *work) struct usbhid_device *usbhid = container_of(work, struct usbhid_device, reset_work); struct hid_device *hid = usbhid->hid; - int rc_lock, rc = 0; + int rc = 0; if (test_bit(HID_CLEAR_HALT, &usbhid->iofl)) { dev_dbg(&usbhid->intf->dev, "clear halt\n"); @@ -113,11 +113,10 @@ static void hid_reset(struct work_struct *work) else if (test_bit(HID_RESET_PENDING, &usbhid->iofl)) { dev_dbg(&usbhid->intf->dev, "resetting device\n"); - rc = rc_lock = usb_lock_device_for_reset(hid_to_usb_dev(hid), usbhid->intf); - if (rc_lock >= 0) { + rc = usb_lock_device_for_reset(hid_to_usb_dev(hid), usbhid->intf); + if (rc == 0) { rc = usb_reset_device(hid_to_usb_dev(hid)); - if (rc_lock) - usb_unlock_device(hid_to_usb_dev(hid)); + usb_unlock_device(hid_to_usb_dev(hid)); } clear_bit(HID_RESET_PENDING, &usbhid->iofl); } diff --git a/drivers/media/video/pvrusb2/pvrusb2-hdw.c b/drivers/media/video/pvrusb2/pvrusb2-hdw.c index 8fb92ac..fa304e5 100644 --- a/drivers/media/video/pvrusb2/pvrusb2-hdw.c +++ b/drivers/media/video/pvrusb2/pvrusb2-hdw.c @@ -3655,7 +3655,7 @@ void pvr2_hdw_device_reset(struct pvr2_hdw *hdw) int ret; pvr2_trace(PVR2_TRACE_INIT,"Performing a device reset..."); ret = usb_lock_device_for_reset(hdw->usb_dev,NULL); - if (ret == 1) { + if (ret == 0) { ret = usb_reset_device(hdw->usb_dev); usb_unlock_device(hdw->usb_dev); } else { diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c index 399e15f..400fa4c 100644 --- a/drivers/usb/core/usb.c +++ b/drivers/usb/core/usb.c @@ -513,10 +513,7 @@ EXPORT_SYMBOL_GPL(usb_put_intf); * disconnect; in some drivers (such as usb-storage) the disconnect() * or suspend() method will block waiting for a device reset to complete. * - * Returns a negative error code for failure, otherwise 1 or 0 to indicate - * that the device will or will not have to be unlocked. (0 can be - * returned when an interface is given and is BINDING, because in that - * case the driver already owns the device lock.) + * Returns a negative error code for failure, otherwise 0. */ int usb_lock_device_for_reset(struct usb_device *udev, const struct usb_interface *iface) @@ -527,16 +524,9 @@ int usb_lock_device_for_reset(struct usb_device *udev, return -ENODEV; if (udev->state == USB_STATE_SUSPENDED) return -EHOSTUNREACH; - if (iface) { - switch (iface->condition) { - case USB_INTERFACE_BINDING: - return 0; - case USB_INTERFACE_BOUND: - break; - default: - return -EINTR; - } - } + if (iface && (iface->condition == USB_INTERFACE_UNBINDING || + iface->condition == USB_INTERFACE_UNBOUND)) + return -EINTR; while (usb_trylock_device(udev) != 0) { @@ -550,10 +540,11 @@ int usb_lock_device_for_reset(struct usb_device *udev, return -ENODEV; if (udev->state == USB_STATE_SUSPENDED) return -EHOSTUNREACH; - if (iface && iface->condition != USB_INTERFACE_BOUND) + if (iface && (iface->condition == USB_INTERFACE_UNBINDING || + iface->condition == USB_INTERFACE_UNBOUND)) return -EINTR; } - return 1; + return 0; } EXPORT_SYMBOL_GPL(usb_lock_device_for_reset); diff --git a/drivers/usb/image/microtek.c b/drivers/usb/image/microtek.c index 885867a..4541dfc 100644 --- a/drivers/usb/image/microtek.c +++ b/drivers/usb/image/microtek.c @@ -350,17 +350,16 @@ static int mts_scsi_abort(struct scsi_cmnd *srb) static int mts_scsi_host_reset(struct scsi_cmnd *srb) { struct mts_desc* desc = (struct mts_desc*)(srb->device->host->hostdata[0]); - int result, rc; + int result; MTS_DEBUG_GOT_HERE(); mts_debug_dump(desc); - rc = usb_lock_device_for_reset(desc->usb_dev, desc->usb_intf); - if (rc < 0) - return FAILED; - result = usb_reset_device(desc->usb_dev); - if (rc) + result = usb_lock_device_for_reset(desc->usb_dev, desc->usb_intf); + if (result == 0) { + result = usb_reset_device(desc->usb_dev); usb_unlock_device(desc->usb_dev); + } return result ? FAILED : SUCCESS; } diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c index 79108d5..2058db4 100644 --- a/drivers/usb/storage/transport.c +++ b/drivers/usb/storage/transport.c @@ -1173,10 +1173,9 @@ int usb_stor_Bulk_reset(struct us_data *us) */ int usb_stor_port_reset(struct us_data *us) { - int result, rc_lock; + int result; - result = rc_lock = - usb_lock_device_for_reset(us->pusb_dev, us->pusb_intf); + result = usb_lock_device_for_reset(us->pusb_dev, us->pusb_intf); if (result < 0) US_DEBUGP("unable to lock device for reset: %d\n", result); else { @@ -1189,8 +1188,7 @@ int usb_stor_port_reset(struct us_data *us) US_DEBUGP("usb_reset_device returns %d\n", result); } - if (rc_lock) - usb_unlock_device(us->pusb_dev); + usb_unlock_device(us->pusb_dev); } return result; }
Hi Mike, There were an upstream change usb_lock_device_for_reset() that touched on pvrusb2 driver. I didn't backport it yet, since I'm not sure if the change is ok. Could you please check? Thanks, Mauro. commit 011b15df465745474e3ec85482633685933ed5a7 Author: Alan Stern <stern@rowland.harvard.edu> Date: Tue Nov 4 11:29:27 2008 -0500 USB: change interface to usb_lock_device_for_reset() This patch (as1161) changes the interface to usb_lock_device_for_reset(). The existing interface is apparently not very clear, judging from the fact that several of its callers don't use it correctly. The new interface always returns 0 for success and it always requires the caller to unlock the device afterward. The new routine will not return immediately if it is called while the driver's probe method is running. Instead it will wait until the probe is over and the device has been unlocked. This shouldn't cause any problems; I don't know of any cases where drivers call usb_lock_device_for_reset() during probe. Signed-off-by: Alan Stern <stern@rowland.harvard.edu> Cc: Pete Zaitcev <zaitcev@redhat.com> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de> -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html