Am Donnerstag, 18. Januar 2007 16:17 schrieb Alan Stern:
> On Thu, 18 Jan 2007, Oliver Neukum wrote:
>
> > Hi,
> >
> > this patch introduces a new attribute for USB devices named "autosuspend".
> > It can be used to block autosuspend for each device individually. 0 means
> > disallow suspending; 1 means allow suspending.
> >
> > This is needed for devices which recharge their batteries of the bus.
> > It compiles and is tested. For devices which crash when suspended it
> > is not usefull unfortunately. This has been tested, too :-(
>
> Are you sure the patch passed testing? See below...
Yes, it works.
> > +static ssize_t
> > +show_autosuspend (struct device *dev, struct device_attribute *attr, char
> > *buf)
>
> The current convention is that we will not put a blank between the
> function name and the following open paren in new code. Old code can be
> cleaned up whenever someone feels like doing it (i.e., probably not for a
> long time).
I chose to keep the code uniform. I can change that if you like.
> > +{
> > + int autosusp;
> > + struct usb_device *udev;
> > +
> > + udev = to_usb_device (dev);
> > + usb_lock_device (udev);
> > + autosusp = udev->can_autosuspend;
> > + usb_unlock_device (udev);
> > + return sprintf (buf, "%d\n", autosusp);
> > +}
>
> You don't really need to lock the device here, although it doesn't hurt.
OK.
[..]
> > + device_remove_file(dev, &dev_attr_autosuspend);
> > device_remove_file(dev, &dev_attr_manufacturer);
> > device_remove_file(dev, &dev_attr_product);
> > device_remove_file(dev, &dev_attr_serial);
>
> This isn't necessary. Simply add &dev_attr_autosuspend.attr to the end of
> the dev_attrs array.
Is that clean? Strictly speaking it is not an attribute of the device as such,
but of usbcore's power management.
> > --- a/drivers/usb/core/driver.c 2007-01-18 13:13:22.000000000 +0100
> > +++ b/drivers/usb/core/driver.c 2007-01-18 13:02:46.000000000 +0100
> > @@ -948,7 +948,8 @@
> > int i;
> > struct usb_interface *intf;
> >
> > - /* For autosuspend, fail fast if anything is in use.
> > + /* For autosuspend, fail fast if anything is in use or autosuspend
> > + * is disabled through sysfs.
> > * Also fail if any interfaces require remote wakeup but it
> > * isn't available. */
> > udev->do_remote_wakeup = device_may_wakeup(&udev->dev);
>
> What happened to the code that actually checks the flag?
It got deleted as unnecessary. Indeed they existed :-;
usb_autoresume_device() and usb_autosuspend_device() go to:
status = usb_autopm_do_device(udev, -1);
which goes to:
udev->pm_usage_cnt += inc_usage_cnt;
The variable exists so that the behavior of the attribute behaves as a
boolean.
Regards
Oliver
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
