Am Dienstag, 3. Januar 2012, 19:35:28 schrieb Alan Stern:
> On Tue, 3 Jan 2012, Oliver Neukum wrote:
>
> > What about this version?
>
> Much better. Just one little problem and a couple of suggestions...
>
> > +static void unbind_no_reset_resume_drivers_interfaces(struct usb_device *udev)
>
> Add a comment before this routine:
>
> /* Unbind drivers for @udev's interfaces that failed to support reset-resume.
> * These interfaces have the needs_binding flag set by usb_resume_interface().
> *
> * The caller must hold @udev's device lock.
> */
Roger
> > +{
> > + struct usb_host_config *config;
> > + int i;
> > + struct usb_interface *intf;
> > +
> > + config = udev->actconfig;
> > + if (config) {
> > + for (i = 0; i < config->desc.bNumInterfaces; ++i) {
> > + intf = config->interface[i];
> > +
> > + if (intf->needs_binding)
>
> This test should be: if (intf->dev.driver && intf->needs_binding)
> (And you don't need to add the blank line.)
How can we not have a driver at this point?
> > /* For all other calls, take the device back to full power and
> > * tell the PM core in case it was autosuspended previously.
> > - * Unbind the interfaces that will need rebinding later.
> > + * Unbind the interfaces that will need rebinding later,
> > + * because they fail to support reset_resume if it was needed
> > */
>
> It might be better to say:
>
> * Also, unbind the interfaces whose drivers failed to support
> * reset-resume. (This can't be done in usb_resume_interface()
> * above because it doesn't own the right set of locks.)
Well, the next question the average reader would ask is:
So why don't they unbind on suspend if the driver doesn't do
reset_resume?
The comment needs to explain that we cannot know when
we'll need reset_resume()
Regards
Oliver
--
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
