On Thu, 17 Dec 2009, Dan Streetman wrote:
> Currently a USB device's wakeup settings are initialized when the device
> is set to a configured state using device_init_wakeup(), but this is not
> correct as wakeup is split into "capable" (can_wakeup) and "enabled"
> (should_wakeup). The settings should be initialized instead in the device
> initialization (usb_new_device) with the "capable" setting disabled and the
> "enabled" setting enabled. The "capable" setting should be set based on the
> device being configured or unconfigured.
>
> This patch retains the sysfs power/wakeup setting of a USB device over a
> USB device re-configuration, which can happen (for example) after a
> suspend/resume cycle.
> - /* Increment the parent's count of unsuspended children */
> - if (udev->parent)
> + if (udev->parent) {
Why don't you do this for root hubs too? They can be unconfigured,
just like anything else. Do you want them not to be wakeup-enabled for
some reason?
> + /* Increment the parent's count of unsuspended children */
> usb_autoresume_device(udev->parent);
>
> + /* Initialize wakeup and disable wakeup capable;
> + * wakeup capability is set by configured state,
> + * wakeup enabled/disabled is set by sysfs control
> + */
> + device_init_wakeup(&udev->dev, 1);
> + device_set_wakeup_capable(&udev->dev, 0);
This code sequence looks strange. You're saying that the device can
issue remote wakeups and then that it can't. It would make more sense
to do:
device_init_wakeup(&udev->dev, 0);
device_set_wakeup_enable(&udev->dev, 1);
With a corresponding change to the comment, of course.
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
