Re: uhub: retry attaching device

2013-04-19 Thread Martin Pieuchot
On 21/01/13(Mon) 03:53, Alexander Polakov wrote:
> So I have got this device:
>  addr 2: File-backed Storage Gadget, Linux 2.6.24.2-Boeye with s3c-udc
> 
> The problem is that when I connect it to PC, it asks for confirmation
> and I have to press button on the device. I guess uhub exploration code
> timeouts while I'm doing that and gives me "uhub0: device problem,
> disabling port 5". With diff attached it works reliably:
> 
> umass0 at uhub0 port 5 configuration 1 interface 0 "Linux 2.6.24.2-Boeye with 
> s3c-udc File-backed Storage Gadget" rev 2.00/3.22 addr 2
> umass0: using SCSI over Bulk-Only
> scsibus4 at umass0: 2 targets, initiator 0
> sd2 at scsibus4 targ 1 lun 0:  SCSI2 0/direct 
> removable serial.00850600372041756775
> sd2: 1885MB, 512 bytes/sector, 3862393 sectors

Could you analyse a bit further why usbd_new_device() is returning an
error? It's certainly when trying to fetch the device descriptor and
as you can see the timing and logic in there is already fishy :)

It would be nice to know what is failing in this function with your
device and properly document it. There's already a ton of hacks and
non standard wait times in it, so I'd prefer no to such a big
workaround.


> Index: uhub.c
> ===
> RCS file: /cvs/src/sys/dev/usb/uhub.c,v
> retrieving revision 1.59
> diff -u -p -u -r1.59 uhub.c
> --- uhub.c29 Sep 2011 11:18:01 -  1.59
> +++ uhub.c20 Jan 2013 23:33:57 -
> @@ -349,6 +349,7 @@ uhub_explore(usbd_device_handle dev)
>   int speed;
>   int port;
>   int change, status, reconnect;
> + int i;
>  
>   DPRINTFN(10, ("uhub_explore dev=%p addr=%d\n", dev, dev->address));
>  
> @@ -486,10 +487,14 @@ uhub_explore(usbd_device_handle dev)
>   speed = USB_SPEED_LOW;
>   else
>   speed = USB_SPEED_FULL;
> - /* Get device info and set its address. */
> - err = usbd_new_device(&sc->sc_dev, dev->bus,
> -   dev->depth + 1, speed, port, up);
> - /* XXX retry a few times? */
> + for (i = 0; i < 3; i++) {
> + /* Get device info and set its address. */
> + err = usbd_new_device(&sc->sc_dev, dev->bus,
> +   dev->depth + 1, speed, port, up);
> + usbd_delay_ms(dev, 100);
> + if (!err)
> + break;
> + }
>   if (err) {
>   DPRINTFN(-1,("uhub_explore: usbd_new_device failed, "
>"error=%s\n", usbd_errstr(err)));
> 
> -- 
> Alexander Polakov | plhk.ru
> 



uhub: retry attaching device

2013-01-20 Thread Alexander Polakov
So I have got this device:
 addr 2: File-backed Storage Gadget, Linux 2.6.24.2-Boeye with s3c-udc

The problem is that when I connect it to PC, it asks for confirmation
and I have to press button on the device. I guess uhub exploration code
timeouts while I'm doing that and gives me "uhub0: device problem,
disabling port 5". With diff attached it works reliably:

umass0 at uhub0 port 5 configuration 1 interface 0 "Linux 2.6.24.2-Boeye with 
s3c-udc File-backed Storage Gadget" rev 2.00/3.22 addr 2
umass0: using SCSI over Bulk-Only
scsibus4 at umass0: 2 targets, initiator 0
sd2 at scsibus4 targ 1 lun 0:  SCSI2 0/direct 
removable serial.00850600372041756775
sd2: 1885MB, 512 bytes/sector, 3862393 sectors

Index: uhub.c
===
RCS file: /cvs/src/sys/dev/usb/uhub.c,v
retrieving revision 1.59
diff -u -p -u -r1.59 uhub.c
--- uhub.c  29 Sep 2011 11:18:01 -  1.59
+++ uhub.c  20 Jan 2013 23:33:57 -
@@ -349,6 +349,7 @@ uhub_explore(usbd_device_handle dev)
int speed;
int port;
int change, status, reconnect;
+   int i;
 
DPRINTFN(10, ("uhub_explore dev=%p addr=%d\n", dev, dev->address));
 
@@ -486,10 +487,14 @@ uhub_explore(usbd_device_handle dev)
speed = USB_SPEED_LOW;
else
speed = USB_SPEED_FULL;
-   /* Get device info and set its address. */
-   err = usbd_new_device(&sc->sc_dev, dev->bus,
- dev->depth + 1, speed, port, up);
-   /* XXX retry a few times? */
+   for (i = 0; i < 3; i++) {
+   /* Get device info and set its address. */
+   err = usbd_new_device(&sc->sc_dev, dev->bus,
+ dev->depth + 1, speed, port, up);
+   usbd_delay_ms(dev, 100);
+   if (!err)
+   break;
+   }
if (err) {
DPRINTFN(-1,("uhub_explore: usbd_new_device failed, "
 "error=%s\n", usbd_errstr(err)));

-- 
Alexander Polakov | plhk.ru