>-----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/
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
