>-----Original Message----- >From: Alan Stern [mailto:[EMAIL PROTECTED] >Sent: Tuesday, May 22, 2007 7:45 PM >To: Pandita, Vikram >Cc: David Brownell; USB development list >Subject: RE: [linux-usb-devel] [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().
New proposed patch based on your inputs: Patch is to prevent the OTG host of doing 3 times enumeration of device when the Host suspends for HNP. The error code used in this case is ENOTSUPP. Signed-off-by: Vikram Pandita <[EMAIL PROTECTED]> --- diff -purN -X ./dontdiff a/drivers/usb/core/hub.c c/drivers/usb/core/hub.c --- a/drivers/usb/core/hub.c 2007-05-13 14:24:43.000000000 +0530 +++ c/drivers/usb/core/hub.c 2007-05-23 11:58:40.000000000 +0530 @@ -1361,7 +1361,7 @@ int usb_new_device(struct usb_device *ud if (err < 0) dev_dbg(&udev->dev, "HNP fail, %d\n", err); } - err = -ENODEV; + err = -ENOTSUPP; goto fail; } #endif @@ -2571,7 +2571,7 @@ loop: ep0_reinit(udev); release_address(udev); usb_put_dev(udev); - if (status == -ENOTCONN) + if ((status == -ENOTCONN) || (status == -ENOTSUPP)) break; } --- > >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/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel