usbd_bulk_transfer returns 1 (USBD_IN_PROGRESS) ?!

[Hans Petter Selasky]

On Thursday 26 May 2005 14:21, Seb wrote:
> Hello,
>
> On Thursday 26 May 2005 13:46, Hans Petter Selasky wrote:
> > Have you got a link to the tarball of your driver?
>
> Sure.
> http://lekernel.host.lya-network.biz/download/prism54u-bsd-0.1.tar.bz2
>
> > I downloaded one of the files I found and had a look.
> >
> > When you detach, you have to stop the pipes/transfers you are using. Then
> > usbd_bulk_transfer will return USBD_CANCELLED and must not be called
> > again!
>
> Ok, I'll add this, thanks for the info. But this won't solve the
> USBD_IN_PROGRESS problem...
>
> > Also you must make sure that transfers (xfer) are not started two times
> > in success, before the first call has returned transfer status.
> >
> > I'm not too familiar with swi_sched, but it might be that it runs the
> > interrupt handler in parallell.
>
> According to the ithread and swi_add manual pages, since I don't specify
> the INTR_MPSAFE flag the Giant mutex is acquired before the interrupt
> thread runs my code and is released right after my function returns. So
> there is no possibility for parallel USB transfers AFAIK.

That is what people expect. The problem is that when "usbd_bulk_transfer()" is 
called, it will call tsleep() which will exit the Giant lock silently and 
then sleep. If you used a custom lock you would have gotten an error. So I 
recommend using custom locks. The problem is that the existing USB API is not 
compatible with that. You have to lock Giant before calling any "usbxxxx" 
function, which usually means that you have to unlock your custom mutex 
first, to avoid locking order reversal, but that leads to loss of  
synchronization in the code! For example:

lock(custom);
unlock(custom);

lock(Giant);
usbd_bulk_transfer(); /* will sleep and exit Giant */
unlock(Giant);

lock(custom);
unlock(custom);

Here "usbd_bulk_transfer" can be called multiple times in parallel! The bad 
solution is to implement an SX-lock to solve this problem (which is very 
inefficient):

lock(custom);

while(starting)
{
  waiting = 1;
  msleep(&waiting,...custom);
}

starting = 1;

unlock(custom);

lock(Giant);
usbd_bulk_transfer();
unlock(Giant);

lock(custom);

if(waiting)
{
  waiting = 0;
  wakeup(&waiting);
}

starting = 0;

unlock(custom);

----


Here I give an example on how you can solve things asynchronously, using my 
USB-API, which supports custom mutexes:

See "/sys/dev/usb2/_ugen.c" for a complete reference
(after that you have installed my USB driver).



at attach:
==========

sc->mtx = should be pointer to a statically allocated mutex.

setup.callback = &your_usb_callback;

if(usbd_transfer_setup(... &setup ... &sc->output_xfer, sc->mtx))
 return ENXIO;



at detach:
==========

mtx_lock(sc->mtx);

usbd_transfer_stop(sc->output_xfer);

/* make sure that "usbd_transfer_start" is not 
 * called again from if_start! 
 */

mtx_unlock(sc->mtx);

usbd_transfer_unsetup(&sc->output_xfer, 1); /* will free xfer */



if_start:
=========

mtx_lock(sc->mtx);

queue mbuf into queue

usbd_transfer_start(sc->output_xfer);

/* will only start transfer once
 * even if called multiple times !
 */
mtx_unlock(sc->mtx);


static void
your_usb_callback(struct usbd_xfer *xfer)
=========================================
{
  /* caller will apply the custom lock !!! */

  USBD_CHECK_STATUS(xfer);

  tr_transferred:
  tr_setup:
    m = DEQUEUE mbuf;
    if(m)
    {
       xfer->length = m->length;
       bcopy(m->data, xfer->buffer, xfer->length);
       free(m);
       usbd_start_hardware(xfer);
    }
    return;

  tr_error:
    if(xfer->error != USBD_CANCELLED)
    printf("%s: transfer returned error(%s) - "
           "please replug your device to continue operation\n",
           __FUNCTION__, usbd_errstr(xfer->error));

    /* or just call "usbd_start_hardware(xfer);" again
     * or wait for next "usbd_transfer_start(xfer);"
     */
    return;
}

--HPS