On Wed, 11 Nov 2009, Oliver Neukum wrote:
> > A few lines above this, it would be good to change
> >
> > if (config) {
> >
> > to
> >
> > if (ret != -ENODEV && config) {
> >
> > There's no point sending post_reset notifications if the reset caused
> > the device to go away.
>
> IIRC the reasoning was that many drivers implement pre_reset() as mutex_lock()
> of a mutex also needed in disconnect(), so that pre/post_reset are always
> supposed to be strictly balanced.
You're right. The system is getting too complicated; I can't remember
all the restrictions any more. Okay, forget that idea.
> > > if (!rebind && cintf->dev.driver) {
> > > drv = to_usb_driver(cintf->dev.driver);
> > > - if (drv->post_reset)
> > > - rebind = (drv->post_reset)(cintf);
> > > - else if (cintf->condition ==
> > > - USB_INTERFACE_BOUND)
> > > - rebind = 1;
> > > + rebind = (drv->post_reset)(cintf);
> > > }
> > > - if (ret == 0 && rebind)
> > > + cintf->needs_binding = rebind;
> >
> > You don't need the "rebind" variable here. Directly set
> > cintf->needs_binding to the result of the method call. Or rather, if
> > the method call returns nonzero, set cintf->needs_binding to 1 (since
> > it is a bitflag, not an int).
>
> Roger
>
> >
> > > + }
> > > +
> > > + /* second pass to not call post_reset on new drivers */
> >
> > Rephrase the comment to make it more clear:
> >
> > /* Don't rebind until after all the post_reset calls */
> >
> > > + for (i = config->desc.bNumInterfaces - 1; i >= 0; --i){
> > > + struct usb_interface *cintf = config->interface[i];
> >
> > Missing blank line.
> >
> > > + if (ret == 0 && cintf->needs_binding)
> >
> > If ret is tested above then it doesn't need to be tested here. And in
> > fact the existing test is wrong; it should be against -ENODEV rather
> > than 0. A reset can fail for reasons that don't cause the device to go
> > away.
>
> But in that case the reason for resetting is likely to persist and the device
> won't work. Do we really want to rebind under such conditions?
I guess that's so. This has never come up as far as I know. The other
main reasons for a reset failing are that the device is a root hub (but
the hub driver does implement pre_reset and post_reset methods so it
won't need rebinding) or the device is suspended (and who's going to
reset a suspended device?).
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
