From: Jonathan Matthew <jonathan@d14n.org>
Subject: uhidev: attach to devices with no input interrupt endpoint
To: tech@openbsd.org
Date: Sun, 7 Sep 2025 21:51:31 +1000


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?

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);