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...

> --- a/drivers/usb/core/sysfs.c        2007-01-18 13:13:35.000000000 +0100
> +++ b/drivers/usb/core/sysfs.c        2007-01-18 13:24:50.000000000 +0100
> @@ -148,6 +148,44 @@
>  }
>  static DEVICE_ATTR(maxchild, S_IRUGO, show_maxchild, NULL);
>  
> +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).

> +{
> +     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.

> @@ -234,12 +272,16 @@
>               if (retval)
>                       goto error;
>       }
> +     retval = device_create_file(dev, &dev_attr_autosuspend);
> +     if (retval)
> +             goto error;
>       retval = usb_create_ep_files(dev, &udev->ep0, udev);
>       if (retval)
>               goto error;
>       return 0;
>  error:
>       usb_remove_ep_files(&udev->ep0);
> +     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.

> --- 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?

Alan Stern


-------------------------------------------------------------------------
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
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to