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@xxxxxxxxx>
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@xxxxxxxxxxxxxxxxxxx>
> 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@xxxxxxxxxxxxxxxxxxx>
> Cc: Pete Zaitcev <zaitcev@xxxxxxxxxx>
> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxx>
>
> 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;
> }
>
--
Mike Isely
isely @ pobox (dot) com
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
