From: David Gwynne <loki@animata.net>
Subject: Re: uhidev: attach to devices with no input interrupt endpoint
To: Jonathan Matthew <jonathan@d14n.org>
Cc: OpenBSD Tech <tech@openbsd.org>
Date: Wed, 5 Nov 2025 20:30:00 +1000

Ok by me.

On Wed, 5 Nov 2025, 15:49 Jonathan Matthew, <jonathan@d14n.org> wrote:

> On Sun, Sep 07, 2025 at 09:51:31PM +1000, Jonathan Matthew wrote:
> >
> > While the USB HID spec says an input interrupt endpoint is required,
> > (https://usb.org/sites/default/files/hid1_11.pdf section 4.4) there
> exist
> > devices that don't have one, like this:
> >
> >     Interface Descriptor:
> >       bLength                 9
> >       bDescriptorType         4
> >       bInterfaceNumber        1
> >       bAlternateSetting       0
> >       bNumEndpoints           1
> >       bInterfaceClass         3 Human Interface Device
> >       bInterfaceSubClass      0 No Subclass
> >       bInterfaceProtocol      0 None
> >       iInterface              0
> >         HID Device Descriptor:
> >           [skipped]
> >       Endpoint Descriptor:
> >         bLength                 7
> >         bDescriptorType         5
> >         bEndpointAddress     0x02  EP 2 OUT
> >         bmAttributes            3
> >           Transfer Type            Interrupt
> >           Synch Type               None
> >           Usage Type               Data
> >         wMaxPacketSize     0x0040  1x 64 bytes
> >         bInterval               1
> >
> > This device is a small lcd screen on the side of the AceMagic S1 mini pc.
> > See
> https://github.com/tjaworski/AceMagic-S1-LED-TFT-Linux?tab=readme-ov-file#tft-lcd-display
> > for a bit more about the screen and how it's controlled.
> >
> > The diff below allows uhidev to attach to devices like this, and changes
> > uhidev_open() so it'll open an output pipe if that's the only endpoint
> available.
> > Most of the diff is just moving the code that opens the input pipe
> inside an if
> > block, so it's much less involved than it might initially appear.
> >
> > Attempting to read from a device with no input endpoint just blocks
> forever,
> > because the device can never produce any data to read.
> >
> > ok?  or does anyone feel strongly about following the USB device specs to
> > the letter?
>
> Anyone?
>
> >
> > Index: uhidev.c
> > ===================================================================
> > RCS file: /cvs/src/sys/dev/usb/uhidev.c,v
> > diff -u -p -r1.110 uhidev.c
> > --- uhidev.c  23 May 2024 03:21:09 -0000      1.110
> > +++ uhidev.c  30 Aug 2025 12:42:32 -0000
> > @@ -200,15 +200,6 @@ uhidev_attach(struct device *parent, str
> >               }
> >       }
> >
> > -     /*
> > -      * Check that we found an input interrupt endpoint.
> > -      * The output interrupt endpoint is optional
> > -      */
> > -     if (sc->sc_iep_addr == -1) {
> > -             printf("%s: no input interrupt endpoint\n", DEVNAME(sc));
> > -             return;
> > -     }
> > -
> >  #ifndef SMALL_KERNEL
> >       if (uhidev_use_rdesc(sc, id, uaa->vendor, uaa->product, &desc,
> &size))
> >               return;
> > @@ -575,32 +566,31 @@ uhidev_open(struct uhidev *scd)
> >       if (sc->sc_refcnt++)
> >               return (0);
> >
> > -     if (sc->sc_isize == 0)
> > -             return (0);
> > +     if (sc->sc_isize != 0) {
> > +             sc->sc_ibuf = malloc(sc->sc_isize, M_USBDEV, M_WAITOK);
> >
> > -     sc->sc_ibuf = malloc(sc->sc_isize, M_USBDEV, M_WAITOK);
> > +             /* Set up input interrupt pipe. */
> > +             DPRINTF(("uhidev_open: isize=%d, ep=0x%02x\n",
> sc->sc_isize,
> > +                 sc->sc_iep_addr));
> > +
> > +             err = usbd_open_pipe_intr(sc->sc_iface, sc->sc_iep_addr,
> > +                       USBD_SHORT_XFER_OK, &sc->sc_ipipe, sc,
> sc->sc_ibuf,
> > +                       sc->sc_isize, uhidev_intr,
> USBD_DEFAULT_INTERVAL);
> > +             if (err != USBD_NORMAL_COMPLETION) {
> > +                     DPRINTF(("uhidopen: usbd_open_pipe_intr failed, "
> > +                         "error=%d\n", err));
> > +                     error = EIO;
> > +                     goto out1;
> > +             }
> >
> > -     /* Set up input interrupt pipe. */
> > -     DPRINTF(("uhidev_open: isize=%d, ep=0x%02x\n", sc->sc_isize,
> > -         sc->sc_iep_addr));
> > -
> > -     err = usbd_open_pipe_intr(sc->sc_iface, sc->sc_iep_addr,
> > -               USBD_SHORT_XFER_OK, &sc->sc_ipipe, sc, sc->sc_ibuf,
> > -               sc->sc_isize, uhidev_intr, USBD_DEFAULT_INTERVAL);
> > -     if (err != USBD_NORMAL_COMPLETION) {
> > -             DPRINTF(("uhidopen: usbd_open_pipe_intr failed, "
> > -                 "error=%d\n", err));
> > -             error = EIO;
> > -             goto out1;
> > -     }
> > -
> > -     DPRINTF(("uhidev_open: sc->sc_ipipe=%p\n", sc->sc_ipipe));
> > -
> > -     sc->sc_ixfer = usbd_alloc_xfer(sc->sc_udev);
> > -     if (sc->sc_ixfer == NULL) {
> > -             DPRINTF(("uhidev_open: couldn't allocate an xfer\n"));
> > -             error = ENOMEM;
> > -             goto out1; // xxxx
> > +             DPRINTF(("uhidev_open: sc->sc_ipipe=%p\n", sc->sc_ipipe));
> > +
> > +             sc->sc_ixfer = usbd_alloc_xfer(sc->sc_udev);
> > +             if (sc->sc_ixfer == NULL) {
> > +                     DPRINTF(("uhidev_open: couldn't allocate an
> xfer\n"));
> > +                     error = ENOMEM;
> > +                     goto out1; // xxxx
> > +             }
> >       }
> >
> >       /*
> > @@ -662,7 +652,8 @@ out3:
> >       usbd_close_pipe(sc->sc_opipe);
> >  out2:
> >       /* Abort input pipe */
> > -     usbd_close_pipe(sc->sc_ipipe);
> > +     if (sc->sc_ipipe != NULL)
> > +             usbd_close_pipe(sc->sc_ipipe);
> >  out1:
> >       DPRINTF(("uhidev_open: failed in someway"));
> >       free(sc->sc_ibuf, M_USBDEV, sc->sc_isize);
> >
>
>