Re: [PATCH] USB Core hub.c

On Tue, 22 May 2007, Pandita, Vikram wrote:

> >From: Alan Stern [mailto:[email protected]]
> >On Mon, 21 May 2007, David Brownell wrote:
> >> On Monday 21 May 2007, Vikram Pandita wrote:
> >> > ---
> >> > diff -purN -X ./dontdiff a/drivers/usb/core/hub.c
> >> > b/drivers/usb/core/hub.c
> >> > --- a/drivers/usb/core/hub.c    2007-05-13 14:24:43.000000000 +0530
> >> > +++ b/drivers/usb/core/hub.c    2007-05-21 13:19:07.000000000 +0530
> >> > @@ -2571,7 +2571,7 @@ loop:
> >> >                 ep0_reinit(udev);
> >> >                 release_address(udev);
> >> >                 usb_put_dev(udev);
> >> > -               if (status == -ENOTCONN)
> >> > +               if ((status == -ENOTCONN) || (status == -ENODEV))
> >> >                         break;
> >> >         }
> >
> >This is wrong.  -ENOTCONN means what it says, that nothing is connected
> >to the port.  So when it occurs we really do want to break out of the
> >loop.
> >
> >On the other hand, -ENODEV means that there is a device but it doesn't
> >match our preconceptions (maybe its speed has somehow changed or some
> >other aspect is different).  Since there still is a device connected to
> >the port, we don't want to break out of the loop.
> >
> >Did you notice any particular example where the port was unconnected
> >and the code still tried to enumerate three times?
> 
> Yes. This is to fix the OTG HNP case. 
> 
> In OTG enabled environment when the b-device attached is not supported
> by host (is_targeted() returns Zero), the host suspends to give chance
> to device to become b-host. If this fix is not in place, then the host
> tries to enumerate the attached device SET_CONFIG_TRIES times which is
> not required.
> 
> Following is the snippet of the code already in place that does the HNP
> and returns the error code to stop re-enumerations and the proposed
> patch checks for this error code:
> 
>         if (!is_targeted(udev)) {
> 
>                 /* Maybe it can talk to us, though we can't talk to it.
>                  * (Includes HNP test device.)
>                  */
>                 if (udev->bus->b_hnp_enable || udev->bus->is_b_host) {
>                         err = __usb_port_suspend(udev,
> udev->bus->otg_port);
>                         if (err < 0)
>                                 dev_dbg(&udev->dev, "HNP fail, %d\n",
> err);
>                 }
>                 err = -ENODEV;
>                 goto fail;
>         }

I understand.

The problem is that hub_port_init() also returns -ENODEV under some
circumstances where we don't want to break out of the loop.  So either
you should change the code shown above to return something else (for
example -EBADE) or else you should change hub_port_init().

Alan Stern


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel