Hi!
> > > Add suspend and resume methods to usblp, but make it so they
> > > only cancel outstanding I/O, while relying on usbcore to fail
> > > any new I/O requests?
> >
> > It is not *that* bad, actually. In system suspend/resume cases, no new
> > I/O requests can happen, because userspace is frozen. Because of
> > runtime suspend, you should handle I/O errors properly, but you should
> > handle I/O errors properly, anyway, so... looks like a solution to me.
>
> You're right, it's not really all that bad. Note however that in the PPC
> implementation, Ben H. does not freeze userspace before suspend to RAM.
Perhaps he'll change his mind ;-))))).
Yep, for ppc and runtime power management, this is not really
nice. But if someone forces our device into off state, perhaps
returning errors to userspace is acceptable solution?
> > > Unbind usblp in lieu of suspending it.
> >
> > If this can be done in reasonable ammount of not-too-ugly code, why
> > not? I think that even Patrick can be convinced by nice patch.
>
> This isn't very hard either. A sample patch is given below. I haven't
> tested it with vanilla 2.6.15-rc6, but a similar patch works okay on
> Greg's development tree. Apart from the FIXMEs, it's not bad at
> all.
Well, the FIXMEs look quite nasty to my untrained eye...
Pavel
> Index: linux-2.6.15-rc6/drivers/usb/core/usb.c
> ===================================================================
> --- linux-2.6.15-rc6.orig/drivers/usb/core/usb.c
> +++ linux-2.6.15-rc6/drivers/usb/core/usb.c
> @@ -1431,8 +1431,17 @@ static int usb_generic_suspend(struct de
> else
> mark_quiesced(intf);
> } else {
> - // FIXME else if there's no suspend method, disconnect...
> - dev_warn(dev, "no %s?\n", "suspend");
> + dev_warn(dev, "no suspend for %s? unbinding...\n",
> + driver->name);
> + up(&dev->sem);
> +
> + /*
> + * FIXME: You're not supposed to do this without holding
> + * the USB device lock. But we can't just grab the lock
> + * because our caller might already be holding it.
> + */
> + usb_driver_release_interface(driver, intf);
> + down(&dev->sem);
> status = 0;
> }
> return status;
> @@ -1459,9 +1468,22 @@ static int usb_generic_resume(struct dev
> return usb_resume_device (to_usb_device(dev));
> }
>
> - if ((dev->driver == NULL) ||
> - (dev->driver_data == &usb_generic_driver_data))
> + /* try to bind interfaces that have no driver */
> + if (dev->driver == NULL) {
> + up(&dev->sem);
> +
> + /*
> + * FIXME: You're not supposed to do this without holding
> + * the USB device lock. But we can't just grab the lock
> + * because our caller might already be holding it.
> + */
> + device_attach(dev);
> + down(&dev->sem);
> return 0;
> + }
> +
> + if (dev->driver_data == &usb_generic_driver_data)
> + return 0; /* Shouldn't happen */
>
> intf = to_usb_interface(dev);
> driver = to_usb_driver(dev->driver);
> @@ -1481,7 +1503,7 @@ static int usb_generic_resume(struct dev
> mark_quiesced(intf);
> }
> } else
> - dev_warn(dev, "no %s?\n", "resume");
> + dev_warn(dev, "no resume for %s?\n", driver->name);
> return 0;
> }
>
> Index: linux-2.6.15-rc6/drivers/usb/core/hub.c
> ===================================================================
> --- linux-2.6.15-rc6.orig/drivers/usb/core/hub.c
> +++ linux-2.6.15-rc6/drivers/usb/core/hub.c
> @@ -1838,12 +1838,6 @@ int usb_resume_device(struct usb_device
>
> usb_unlock_device(udev);
>
> - /* rebind drivers that had no suspend() */
> - if (status == 0) {
> - usb_lock_all_devices();
> - bus_rescan_devices(&usb_bus_type);
> - usb_unlock_all_devices();
> - }
> return status;
> }
>
>
--
Thanks, Sharp!
