On Tue, 3 Jan 2012, Oliver Neukum wrote:
> > > +{
> > > + 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?
If the original driver didn't support suspend/resume, it would already
have been unbound by this time and needs_binding would be set.
> > > /* 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()
Okay, but where is the best place to explain this? Probably in your
unbind_no_pm_drivers_interfaces() routine, which checks for suspend and
resume methods but does not check for reset_resume.
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
