On Thu, 5 Jan 2012, Oliver Neukum wrote:
> From 7cb1e9d0fe8bfa006d27a3c517307fb3212bebcd Mon Sep 17 00:00:00 2001
> From: Oliver Neukum <oliver@xxxxxxxxxx>
> Date: Thu, 5 Jan 2012 15:37:12 +0100
> Subject: [PATCH] USB: code cleanup in suspend/resume path
>
> Do the cleanup to avoid behaviorial parameters Linus
> requested.
>
> Signed-off-by: Oliver Neukum <oneukum@xxxxxxx>
Very good; you can add my Reviewed-by. However there is more thing
that Linus probably wants to see. You can put it in this patch or do
it separately.
> @@ -1307,15 +1335,20 @@ int usb_resume(struct device *dev, pm_message_t msg)
> struct usb_device *udev = to_usb_device(dev);
> int status;
>
> - /* For PM complete calls, all we do is rebind interfaces */
> + /* For PM complete calls, all we do is rebind interfaces
> + * whose needs_binding flag is set
> + */
> if (msg.event == PM_EVENT_ON) {
> if (udev->state != USB_STATE_NOTATTACHED)
> - do_unbind_rebind(udev, DO_REBIND);
> + do_rebind_interfaces(udev);
> status = 0;
This section of code should go into its own subroutine. It's the sort
of thing Linus hates; passing an argument to tell a function what to do
instead of calling a separate function.
The PM_EVENT_ON event is used in only one place,
usb.c:usb_dev_complete(). You could move this code there (although
then do_rebind_interfaces would have to become non-static) or you could
create a new usb_resume_complete() routine -- the latter feels a little
cleaner to me.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
