On Sun, 6 Sep 2015, Peter Chen wrote:

> On Wed, Sep 02, 2015 at 10:23:39AM -0400, Alan Stern wrote:
> > On Wed, 2 Sep 2015, Peter Chen wrote:
> > 
> > > The bulk queue tests are used to show 'best performance' for bulk
> > > transfer, we are often asked this question by users.
> > > 
> > > It's result should be very close to IC simulation, in order
> > > to get that, the device side should also need to prepare enough
> > > queue.
> > > 
> > > We have got the 'best performance' (IN: ~41MB, OUT: ~39MB) at i.mx 
> > > platform
> > > (USB2, ARM Cortex A9) with below command:
> > > 
> > > Host side:
> > > modprobe usbtest
> > > ./testusb -a -t 27 -g 64 -s 16384
> > > ./testusb -a -t 28 -g 64 -s 16384
> > > Gadget side:
> > > modprobe g_zero loopdefault=1 qlen=64 buflen=16384
> > > 
> > > Signed-off-by: Peter Chen <[email protected]>
> > > ---
> > > 
> > > I am not sure if it is good to reuse iso structure, so I take
> > > it as RFC. If reuse is accepted, I will rename the iso
> > > structures to common one.
> > > 
> > >  drivers/usb/misc/usbtest.c | 88 
> > > ++++++++++++++++++++++++++++++++++++----------
> > >  1 file changed, 69 insertions(+), 19 deletions(-)
> > > 
> > > diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
> > > index ad6dd4a..f1faeb9 100644
> > > --- a/drivers/usb/misc/usbtest.c
> > > +++ b/drivers/usb/misc/usbtest.c
> > > @@ -17,6 +17,7 @@
> > >  static int override_alt = -1;
> > >  module_param_named(alt, override_alt, int, 0644);
> > >  MODULE_PARM_DESC(alt, ">= 0 to override altsetting selection");
> > > +static void iso_callback(struct urb *urb);
> > >  
> > >  
> > > /*-------------------------------------------------------------------------*/
> > >  
> > > @@ -239,7 +240,8 @@ static struct urb *usbtest_alloc_urb(
> > >   unsigned long           bytes,
> > >   unsigned                transfer_flags,
> > >   unsigned                offset,
> > > - u8                      bInterval)
> > > + u8                      bInterval,
> > > + bool                    queue_callback)
> > 
> > Should this be named "is_iso" instead?  It would be be more clear.
> > 
> 
> ISO transfer uses different function iso_alloc_urb to allocate urb,
> the reason we need one more parameter at usbtest_alloc_urb is we
> use different callback for simple polling bulk transfer and
> interrupt-triggered bulk transfer.

Sorry, I'm a little slow today.  I can understand that you want to use
different callback routines for the different tests, but I don't
understand why you want to use iso_callback for a bulk test.  Do you
plan to rename the callback as well as the structure?

Would it make more sense for the extra argument here to be a pointer to 
the callback function, instead of a flag?

> So, do you agree I reuse the iso structures to implement it? 

I don't see anything bad about using a common structure for two 
different purposes, so long as the comments explain what is happening.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to