From: Jonathan Matthew <jonathan@d14n.org>
Subject: Re: uhidev: attach to devices with no input interrupt endpoint
To: tech@openbsd.org
Date: Wed, 5 Nov 2025 15:47:49 +1000

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