Subject: netBSD dev/ugen / libusb
To: None <tech-misc@NetBSD.org>
From: Nevil Thatcher <NevilTh@nat.com.au>
List: tech-misc
Date: 03/17/2006 17:38:08
Hi All,

 

I have found an issue with netBSD dev/ugen - libusb and am after a bit of
direction/advice.

 

This has come about whilst trying to get nut (ups monitoring) software
operational under netBSD. Nut uses libusb as the layer to access usb
devices, the device I am using being a MGE Nova 1100 UPS.

 

Software versions are:

 

netBSD 3.0_STABLE (amd64)

libusb 0.1.12

nut 2.0.3

 

Libusb offers a function usb_interrupt_read, the function looks like

 

int usb_interrupt_read(usb_dev_handle *dev, int ep, char *bytes, int size,
int timeout)

 

A call to this should I understand return with *char buffer filled with data
from usb device OR timeout after int timeout seconds.

 

The problem seems to be that the dev/ugen code ugen_do_read does not respect
this timeout and will not return until such time as data is returned from
the usb device.

 

End result is that when libusb usb_interrupt_read is called it won't return
until data is received from the usb device and the buffer filled, so
effectively hangs.

 

To get around this I have added the flag O_NONBLOCK to the call to open the
device. 

If there is no data available from the device then ugen_do_read then returns
with the error EWOULDBLOCK

This is defined as "Resource temporarily unavailable" in errno.h

 

With this patch the program works quite happily, but is this a valid
approach?

 

Should 

a)       netBSD dev/ugen respect the timeout set by = ioctl(fd,
USB_SET_TIMEOUT, &timeout); as implemented in libusb OR

b)       libusb use the O_NONBLOCK flag when opening the device.

 

I guess this needs to be fixed somewhere, but where!

 

Your thoughts?, thanks in advance

 

Nevil

 

Relevant code excerpts below:-

 

 

/*============ netBSD sys/errno.h ================*/

 

#define  EAGAIN                        35                     /* Resource
temporarily unavailable */

#define  EWOULDBLOCK           EAGAIN                        /* Operation
would block */

 

 

/*============= libusb/bsd.c===================================*/

int usb_interrupt_read(usb_dev_handle *dev, int ep, char *bytes, int size,

                       int timeout)

{

  int fd, ret, retrieved = 0, one = 1;

 

  /* Ensure the endpoint address is correct */

  ep |= USB_ENDPOINT_IN;

 

/* open in non-blocking mode so that call will return if no data */
<======= fix

  fd = ensure_ep_open(dev, ep, O_RDONLY | O_NONBLOCK);   <======= to ensure
call

/*  fd = ensure_ep_open(dev, ep, O_RDONLY );  */  if (fd < 0) {     <=======
returns 

    if (usb_debug >= 2) {

#ifdef __FreeBSD_kernel__

      fprintf (stderr, "usb_interrupt_read: got negative open file
descriptor for endpoint %d\n", UE_GET_ADDR(ep));

#else

      fprintf (stderr, "usb_interrupt_read: got negative open file
descriptor for endpoint %02d\n", UE_GET_ADDR(ep));

#endif

    }

    return fd;

  }

 

  ret = ioctl(fd, USB_SET_TIMEOUT, &timeout);

  if (ret < 0)

    USB_ERROR_STR(-errno, "error setting timeout: %s", strerror(errno));

 

  ret = ioctl(fd, USB_SET_SHORT_XFER, &one);

  if (ret < 0)

    USB_ERROR_STR(-errno, "error setting short xfer: %s", strerror(errno));

 

  do {

    ret = read(fd, bytes+retrieved, size-retrieved);

    if (ret < 0)

#ifdef __FreeBSD_kernel__

      USB_ERROR_STR(-errno, "error reading from interrupt endpoint %s.%d:
%s",

                    dev->device->filename, UE_GET_ADDR(ep),
strerror(errno));

#else

      USB_ERROR_STR(-errno, "error reading from interrupt endpoint %s.%02d:
%s",

                  dev->device->filename, UE_GET_ADDR(ep), strerror(errno));

#endif

    retrieved += ret;

  } while (ret > 0 && retrieved < size);

 

  return retrieved;

}

/*===================================================================*/

 

static int ensure_ep_open(usb_dev_handle *dev, int ep, int mode)

{

  struct bsd_usb_dev_handle_info *info = dev->impl_info;

  int fd;

  char buf[20];

 

  /* Get the address for this endpoint; we could check

   * the mode against the direction; but we've done that

   * already in the usb_bulk_read/write.

   */

  ep = UE_GET_ADDR(ep);

 

  if (info->ep_fd[ep] < 0) {

#ifdef __FreeBSD_kernel__

    snprintf(buf, sizeof(buf) - 1, "%s.%d", dev->device->filename, ep);

#else

    snprintf(buf, sizeof(buf) - 1, "%s.%02d", dev->device->filename, ep);

#endif

    /* Try to open it O_RDWR first for those devices which have in and out

     * endpoints with the same address (eg 0x02 and 0x82)

     */

    fd = open(buf, O_RDWR);

    if (fd < 0 && errno == ENXIO)

      fd = open(buf, mode);

    if (fd < 0)

      USB_ERROR_STR(-errno, "can't open %s for bulk read: %s",

                    buf, strerror(errno));

    info->ep_fd[ep] = fd;

  }

 

/*====================netBSD dev/usb/ugen.c ===============*/

Static int

ugen_do_read(struct ugen_softc *sc, int endpt, struct uio *uio, int flag)

{

            struct ugen_endpoint *sce = &sc->sc_endpoints[endpt][IN];

            u_int32_t n, tn;

            char buf[UGEN_BBSIZE];

            usbd_xfer_handle xfer;

            usbd_status err;

            int s;

            int error = 0;

            u_char buffer[UGEN_CHUNK];

 

            DPRINTFN(5, ("%s: ugenread: %d\n", USBDEVNAME(sc->sc_dev),
endpt));

 

            if (sc->sc_dying)

                        return (EIO);

 

            if (endpt == USB_CONTROL_ENDPOINT)

                        return (ENODEV);

 

#ifdef DIAGNOSTIC

            if (sce->edesc == NULL) {

                        printf("ugenread: no edesc\n");

                        return (EIO);

            }

            if (sce->pipeh == NULL) {

                        printf("ugenread: no pipe\n");

                        return (EIO);

            }

#endif

 

            switch (sce->edesc->bmAttributes & UE_XFERTYPE) {

            case UE_INTERRUPT:

                        /* Block until activity occurred. */

                        s = splusb();

                        while (sce->q.c_cc == 0) {

                                    if (flag & IO_NDELAY) {

                                                splx(s);

                                                return (EWOULDBLOCK);

                                    }

                                    sce->state |= UGEN_ASLP;

                                    DPRINTFN(5, ("ugenread: sleep on %p\n",
sce));

                                    error = tsleep(sce, PZERO | PCATCH,
"ugenri", 0);

                                    DPRINTFN(5, ("ugenread: woke,
error=%d\n", error));

                                    if (sc->sc_dying)

                                                error = EIO;

                                    if (error) {

                                                sce->state &= ~UGEN_ASLP;

                                                break;

                                    }

                        }

                        splx(s);

 

                        /* Transfer as many chunks as possible. */

                        while (sce->q.c_cc > 0 && uio->uio_resid > 0 &&
!error) {

                                    n = min(sce->q.c_cc, uio->uio_resid);

                                    if (n > sizeof(buffer))

                                                n = sizeof(buffer);

 

                                    /* Remove a small chunk from the input
queue. */

                                    q_to_b(&sce->q, buffer, n);

                                    DPRINTFN(5, ("ugenread: got %d chars\n",
n));

 

                                    /* Copy the data to the user process. */

                                    error = uiomove(buffer, n, uio);

                                    if (error)

                                                break;

                        }

                        break;

            case UE_BULK:

                        xfer = usbd_alloc_xfer(sc->sc_udev);

                        if (xfer == 0)

                                    return (ENOMEM);

                        while ((n = min(UGEN_BBSIZE, uio->uio_resid)) != 0)
{

                                    DPRINTFN(1, ("ugenread: start transfer
%d bytes\n",n));

                                    tn = n;

                                    err = usbd_bulk_transfer(

                                                  xfer, sce->pipeh,

                                                  sce->state & UGEN_SHORT_OK
?

                                                      USBD_SHORT_XFER_OK :
0,

                                                  sce->timeout, buf, &tn,
"ugenrb");

                                    if (err) {

                                                if (err == USBD_INTERRUPTED)

                                                            error = EINTR;

                                                else if (err ==
USBD_TIMEOUT)

                                                            error =
ETIMEDOUT;

                                                else

                                                            error = EIO;

                                                break;

                                    }

                                    DPRINTFN(1, ("ugenread: got %d bytes\n",
tn));

                                    error = uiomove(buf, tn, uio);

                                    if (error || tn < n)

                                                break;

                        }

                        usbd_free_xfer(xfer);

                        break;

            case UE_ISOCHRONOUS:

                        s = splusb();

                        while (sce->cur == sce->fill) {

                                    if (flag & IO_NDELAY) {

                                                splx(s);

                                                return (EWOULDBLOCK);

                                    }

                                    sce->state |= UGEN_ASLP;

                                    DPRINTFN(5, ("ugenread: sleep on %p\n",
sce));

                                    error = tsleep(sce, PZERO | PCATCH,
"ugenri", 0);

                                    DPRINTFN(5, ("ugenread: woke,
error=%d\n", error));

                                    if (sc->sc_dying)

                                                error = EIO;

                                    if (error) {

                                                sce->state &= ~UGEN_ASLP;

                                                break;

                                    }

                        }

 

                        while (sce->cur != sce->fill && uio->uio_resid > 0
&& !error) {

                                    if(sce->fill > sce->cur)

                                                n = min(sce->fill -
sce->cur, uio->uio_resid);

                                    else

                                                n = min(sce->limit -
sce->cur, uio->uio_resid);

 

                                    DPRINTFN(5, ("ugenread: isoc got %d
chars\n", n));

 

                                    /* Copy the data to the user process. */

                                    error = uiomove(sce->cur, n, uio);

                                    if (error)

                                                break;

                                    sce->cur += n;

                                    if(sce->cur >= sce->limit)

                                                sce->cur = sce->ibuf;

                        }

                        splx(s);

                        break;

 

 

            default:

                        return (ENXIO);

            }

            return (error);

}