On Wed, 13 Jul 2005, Alan Stern wrote:

> On Sun, 10 Jul 2005, Pete Zaitcev wrote:
> 
> > Hi, All:
> > 
> > I've refactored the friends of usb_sg_request to permit it being used
> > by ub. The result is attached FYI. It's probably buggy. I cannot convince
> > myself that all this is correct.
> > 
> > In case anyone is curious, fields in usb_sg_request were renamed because
> > their functions morphed in a way which made some of uses incorrect for
> > each of them. The renaming ensures that programmer examines every use.
> 
> In general this looks pretty good.  I didn't read through your changes to 
> ub, just to the usb_sg library.  Here's what I noticed:

I forgot to mention a couple of other things.  In usb_sg_submit you
commented out the call to usb_sg_cancel when a submission fails.  I think
you should leave it in.  After all, there's no point waiting for the first
few URBs to complete normally if you already know that the transfer as a
whole is doomed.

There's a lot of duplicated code in usb_sg_wait and usb_sg_submit.  When
you're confident that the new code works okay, you could simplify
usb_sg_wait to this:

void usb_sg_wait (struct usb_sg_request *io)
{
        struct completion       done;

        init_completion (&done);
        io->complete = sg_complete_sync;
        io->context = &done;

        if (usb_sg_submit (io) == 0)
                wait_for_completion (&done);
        sg_clean (io);
}

However this ignores the matter of retrying submissions that fail with 
-ENXIO, -EAGAIN, or -ENOMEM.  Maybe you could add an additional argument 
to usb_sg_submit to specify whether such failures should be retried.

Alan Stern



-------------------------------------------------------
This SF.Net email is sponsored by the 'Do More With Dual!' webinar happening
July 14 at 8am PDT/11am EDT. We invite you to explore the latest in dual
core and dual graphics technology at this free one hour event hosted by HP,
AMD, and NVIDIA.  To register visit http://www.hp.com/go/dualwebinar
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to