uhidev(9) improvements

[Taylor R Campbell <campbell+netbsd-tech-kern%mumble.net@localhost>]

The attached patch series partially fixes a bug in the uhidev(9) API
(uhidev_stop didn't do anything at all; now it is merely racy).

But more importantly, the patch series makes struct uhidev_softc
opaque, so the uhidev(9) API can have a more stable ABI.  This will
give us better latitude to fix this race -- and potentially others --
later on and pull them up to netbsd-10 after it branches.

I have tested uhid(4) and ukbd(4).  I haven't tested ucycom(4), which
uses uhidev(9) in a way that nothing else does -- to do async xfers on
the output pipe.  Can anyone review and/or test ucycom(4)?

From 7aa6050cfb88fbe8b42cf339fd42657b94ff5741 Mon Sep 17 00:00:00 2001
From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
Date: Wed, 19 Jan 2022 14:43:11 +0000
Subject: [PATCH 1/4] uhidev(9): Partially fix uhidev_write aborting.

In my previous change, I intended to make uhidev_stop abort any
pending write -- but I forgot to initialize sc->sc_writereportid, so
it never did anything.

This changes the API and ABI of uhidev_write so it takes the struct
uhidev pointer, rather than the struct uhidev_softc pointer; this way
uhidev_write knows what the report id of the client is, so it can
arrange to have uhidev_stop abort only this one.

XXX Except it still doesn't actually work because we do this
unlocked, ugh, so the write might complete before we abort anything.
---
 sys/dev/usb/ucycom.c |  2 +-
 sys/dev/usb/uhid.c   |  3 +--
 sys/dev/usb/uhidev.c | 10 ++++++++--
 sys/dev/usb/uhidev.h |  2 +-
 4 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/sys/dev/usb/ucycom.c b/sys/dev/usb/ucycom.c
index bb6046ae1a12..5191d234a52c 100644
--- a/sys/dev/usb/ucycom.c
+++ b/sys/dev/usb/ucycom.c
@@ -1105,7 +1105,7 @@ ucycom_set_status(struct ucycom_softc *sc)
 	memset(sc->sc_obuf, 0, sc->sc_olen);
 	sc->sc_obuf[0] = sc->sc_mcr;
 
-	err = uhidev_write(sc->sc_hdev.sc_parent, sc->sc_obuf, sc->sc_olen);
+	err = uhidev_write(&sc->sc_hdev, sc->sc_obuf, sc->sc_olen);
 	if (err) {
 		DPRINTF(("ucycom_set_status: err=%d\n", err));
 	}
diff --git a/sys/dev/usb/uhid.c b/sys/dev/usb/uhid.c
index 46769dc0353a..66ab1a74b3c1 100644
--- a/sys/dev/usb/uhid.c
+++ b/sys/dev/usb/uhid.c
@@ -581,8 +581,7 @@ uhid_do_write(struct uhid_softc *sc, struct uio *uio, int flag)
 #endif
 	if (!error) {
 		if (sc->sc_raw)
-			err = uhidev_write(sc->sc_hdev.sc_parent, sc->sc_obuf,
-			    size);
+			err = uhidev_write(&sc->sc_hdev, sc->sc_obuf, size);
 		else
 			err = uhidev_set_report(&sc->sc_hdev,
 			    UHID_OUTPUT_REPORT, sc->sc_obuf, size);
diff --git a/sys/dev/usb/uhidev.c b/sys/dev/usb/uhidev.c
index b0c03092c77a..1a530cd76412 100644
--- a/sys/dev/usb/uhidev.c
+++ b/sys/dev/usb/uhidev.c
@@ -901,7 +901,7 @@ uhidev_stop(struct uhidev *scd)
 		abort = true;
 	mutex_exit(&sc->sc_lock);
 
-	if (abort && sc->sc_opipe)
+	if (abort && sc->sc_opipe) /* XXX sc_opipe might go away */
 		usbd_abort_pipe(sc->sc_opipe);
 }
 
@@ -961,8 +961,9 @@ uhidev_get_report(struct uhidev *scd, int type, void *data, int len)
 }
 
 usbd_status
-uhidev_write(struct uhidev_softc *sc, void *data, int len)
+uhidev_write(struct uhidev *scd, void *data, int len)
 {
+	struct uhidev_softc *sc = scd->sc_parent;
 	usbd_status err;
 
 	DPRINTF(("uhidev_write: data=%p, len=%d\n", data, len));
@@ -985,6 +986,7 @@ uhidev_write(struct uhidev_softc *sc, void *data, int len)
 		}
 	}
 	sc->sc_writelock = curlwp;
+	sc->sc_writereportid = scd->sc_report_id;
 	mutex_exit(&sc->sc_lock);
 
 #ifdef UHIDEV_DEBUG
@@ -1006,6 +1008,10 @@ uhidev_write(struct uhidev_softc *sc, void *data, int len)
 	KASSERT(sc->sc_refcnt);
 	KASSERTMSG(sc->sc_writelock == curlwp, "%s: migrated from %p to %p",
 	    device_xname(sc->sc_dev), curlwp, sc->sc_writelock);
+	KASSERTMSG(sc->sc_writereportid == scd->sc_report_id,
+	    "%s: changed write report ids from %d to %d",
+	    device_xname(sc->sc_dev), scd->sc_report_id, sc->sc_writereportid);
+	sc->sc_writereportid = -1;
 	sc->sc_writelock = NULL;
 	cv_broadcast(&sc->sc_cv);
 out:	mutex_exit(&sc->sc_lock);
diff --git a/sys/dev/usb/uhidev.h b/sys/dev/usb/uhidev.h
index 83c0a357744e..447780987af8 100644
--- a/sys/dev/usb/uhidev.h
+++ b/sys/dev/usb/uhidev.h
@@ -97,7 +97,7 @@ void uhidev_stop(struct uhidev *);
 void uhidev_close(struct uhidev *);
 usbd_status uhidev_set_report(struct uhidev *, int, void *, int);
 usbd_status uhidev_get_report(struct uhidev *, int, void *, int);
-usbd_status uhidev_write(struct uhidev_softc *, void *, int);
+usbd_status uhidev_write(struct uhidev *, void *, int);
 
 #define	UHIDEV_OSIZE	64
 

From 6627dba911522e91e7b7848ff570a5229f0b9176 Mon Sep 17 00:00:00 2001
From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
Date: Tue, 25 Jan 2022 00:37:14 +0000
Subject: [PATCH 2/4] uhidev(9): New uhidev_write_async.

Like uhidev_write but issues the transfer asynchronously with a
callback.

Use it in ucycom(4).
---
 sys/dev/usb/ucycom.c |  7 ++---
 sys/dev/usb/uhidev.c | 67 ++++++++++++++++++++++++++++++++++++++++++++
 sys/dev/usb/uhidev.h |  4 +++
 3 files changed, 73 insertions(+), 5 deletions(-)

diff --git a/sys/dev/usb/ucycom.c b/sys/dev/usb/ucycom.c
index 5191d234a52c..242ac817fda5 100644
--- a/sys/dev/usb/ucycom.c
+++ b/sys/dev/usb/ucycom.c
@@ -592,11 +592,9 @@ ucycomstart(struct tty *tp)
 	}
 #endif
 	DPRINTFN(4,("ucycomstart: %d chars\n", len));
-	usbd_setup_xfer(sc->sc_hdev.sc_parent->sc_oxfer, sc, sc->sc_obuf,
-	    sc->sc_olen, 0, USBD_NO_TIMEOUT, ucycomwritecb);
-
 	/* What can we do on error? */
-	err = usbd_transfer(sc->sc_hdev.sc_parent->sc_oxfer);
+	err = uhidev_write_async(&sc->sc_hdev, sc->sc_obuf, sc->sc_olen, 0,
+	    USBD_NO_TIMEOUT, ucycomwritecb, sc);
 
 #ifdef UCYCOM_DEBUG
 	if (err != USBD_IN_PROGRESS)
@@ -621,7 +619,6 @@ ucycomwritecb(struct usbd_xfer *xfer, void *p, usbd_status status)
 
 	if (status) {
 		DPRINTF(("ucycomwritecb: status=%d\n", status));
-		usbd_clear_endpoint_stall(sc->sc_hdev.sc_parent->sc_opipe);
 		/* XXX we should restart after some delay. */
 		goto error;
 	}
diff --git a/sys/dev/usb/uhidev.c b/sys/dev/usb/uhidev.c
index 1a530cd76412..d1a2909c5b59 100644
--- a/sys/dev/usb/uhidev.c
+++ b/sys/dev/usb/uhidev.c
@@ -1017,3 +1017,70 @@ uhidev_write(struct uhidev *scd, void *data, int len)
 out:	mutex_exit(&sc->sc_lock);
 	return err;
 }
+
+static void
+uhidev_write_callback(struct usbd_xfer *xfer, void *cookie, usbd_status err)
+{
+	struct uhidev_softc *sc = cookie;
+	usbd_callback writecallback;
+	void *writecookie;
+
+	if (err)
+		usbd_clear_endpoint_stall(sc->sc_opipe);
+
+	mutex_enter(&sc->sc_lock);
+	KASSERT(sc->sc_writelock == (void *)1);
+	writecallback = sc->sc_writecallback;
+	writecookie = sc->sc_writecookie;
+	sc->sc_writereportid = -1;
+	sc->sc_writelock = NULL;
+	sc->sc_writecallback = NULL;
+	sc->sc_writecookie = NULL;
+	cv_broadcast(&sc->sc_cv);
+	mutex_exit(&sc->sc_lock);
+
+	(*writecallback)(xfer, writecookie, err);
+}
+
+usbd_status
+uhidev_write_async(struct uhidev *scd, void *data, int len, int flags,
+    int timo, usbd_callback writecallback, void *writecookie)
+{
+	struct uhidev_softc *sc = scd->sc_parent;
+	usbd_status err;
+
+	DPRINTF(("%s: data=%p, len=%d\n", __func__, data, len));
+
+	if (sc->sc_opipe == NULL)
+		return USBD_INVAL;
+
+	mutex_enter(&sc->sc_lock);
+	KASSERT(sc->sc_refcnt);
+	if (sc->sc_dying) {
+		err = USBD_IOERROR;
+		goto out;
+	}
+	if (sc->sc_writelock != NULL) {
+		err = USBD_IN_USE;
+		goto out;
+	}
+	sc->sc_writelock = (void *)1; /* XXX no lwp to attribute async xfer */
+	sc->sc_writereportid = scd->sc_report_id;
+	sc->sc_writecallback = writecallback;
+	sc->sc_writecookie = writecookie;
+	usbd_setup_xfer(sc->sc_oxfer, sc, data, len, flags, timo,
+	    uhidev_write_callback);
+	err = usbd_transfer(sc->sc_oxfer);
+	switch (err) {
+	case USBD_IN_PROGRESS:
+		break;
+	case USBD_NORMAL_COMPLETION:
+		panic("unexpected normal completion of async xfer under lock");
+	default:		/* error */
+		sc->sc_writelock = NULL;
+		sc->sc_writereportid = -1;
+		cv_broadcast(&sc->sc_cv);
+	}
+out:	mutex_exit(&sc->sc_lock);
+	return err;
+}
diff --git a/sys/dev/usb/uhidev.h b/sys/dev/usb/uhidev.h
index 447780987af8..0b19e9015fcd 100644
--- a/sys/dev/usb/uhidev.h
+++ b/sys/dev/usb/uhidev.h
@@ -68,6 +68,8 @@ struct uhidev_softc {
 	struct usbd_pipe *sc_ipipe;	/* input interrupt pipe */
 	struct usbd_pipe *sc_opipe;	/* output interrupt pipe */
 	struct usbd_xfer *sc_oxfer;	/* write request */
+	usbd_callback sc_writecallback;	/* async write request callback */
+	void *sc_writecookie;
 
 	u_int sc_flags;
 #define UHIDEV_F_XB1	0x0001	/* Xbox 1 controller */
@@ -98,6 +100,8 @@ void uhidev_close(struct uhidev *);
 usbd_status uhidev_set_report(struct uhidev *, int, void *, int);
 usbd_status uhidev_get_report(struct uhidev *, int, void *, int);
 usbd_status uhidev_write(struct uhidev *, void *, int);
+usbd_status uhidev_write_async(struct uhidev *, void *, int, int, int,
+    usbd_callback, void *);
 
 #define	UHIDEV_OSIZE	64
 

From 2f5f3af6a339aace77590048f159cfb93fca70a8 Mon Sep 17 00:00:00 2001
From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
Date: Wed, 19 Jan 2022 17:18:13 +0000
Subject: [PATCH 3/4] uhidev(9): Get the device and interface through attach
 args.

This way uhidev drivers don't need access to uhidev_softc itself for
it.
---
 sys/arch/macppc/dev/pbms.c |  7 +++++--
 sys/dev/usb/uatp.c         | 14 ++++++++------
 sys/dev/usb/uhid.c         | 11 +++++++----
 sys/dev/usb/ukbd.c         | 15 ++++++++++-----
 sys/dev/usb/ums.c          |  4 +++-
 sys/dev/usb/uthum.c        |  2 +-
 6 files changed, 34 insertions(+), 19 deletions(-)

diff --git a/sys/arch/macppc/dev/pbms.c b/sys/arch/macppc/dev/pbms.c
index a1bfb88516cb..edc2213c619c 100644
--- a/sys/arch/macppc/dev/pbms.c
+++ b/sys/arch/macppc/dev/pbms.c
@@ -307,7 +307,8 @@ pbms_match(device_t parent, cfdata_t match, void *aux)
 	 * we expect. 
 	 */
 	if (uha->uiaa->uiaa_proto == UIPROTO_MOUSE &&
-	    (udd = usbd_get_device_descriptor(uha->parent->sc_udev)) != NULL) {
+	    ((udd = usbd_get_device_descriptor(uha->uiaa->uiaa_device))
+		!= NULL)) {
 		vendor = UGETW(udd->idVendor);
 		product = UGETW(udd->idProduct);
 		for (i = 0; i < PBMS_NUM_DEVICES; i++) {
@@ -329,6 +330,7 @@ pbms_attach(device_t parent, device_t self, void *aux)
 	struct uhidev_attach_arg *uha = aux;
 	struct pbms_dev *pd;
 	struct pbms_softc *sc = device_private(self);
+	struct usbd_device *udev;
 	usb_device_descriptor_t *udd;
 	int i;
 	uint16_t vendor, product;
@@ -341,7 +343,8 @@ pbms_attach(device_t parent, device_t self, void *aux)
 	sc->sc_datalen = PBMS_DATA_LEN;
 
 	/* Fill in device-specific parameters. */
-	if ((udd = usbd_get_device_descriptor(uha->parent->sc_udev)) != NULL) {
+	udev = uha->uiaa->uiaa_udevice;
+	if ((udd = usbd_get_device_descriptor(udev)) != NULL) {
 		product = UGETW(udd->idProduct);
 		vendor = UGETW(udd->idVendor);
 		for (i = 0; i < PBMS_NUM_DEVICES; i++) {
diff --git a/sys/dev/usb/uatp.c b/sys/dev/usb/uatp.c
index a94e755f1ed2..d0a564eca616 100644
--- a/sys/dev/usb/uatp.c
+++ b/sys/dev/usb/uatp.c
@@ -486,7 +486,8 @@ static const struct uatp_knobs default_knobs = {
 };
 
 struct uatp_softc {
-	struct uhidev sc_hdev;		/* USB parent.  */
+	struct uhidev sc_hdev;		/* uhidev(9) parent.  */
+	struct usbd_device *sc_udev;	/* USB device.  */
 	device_t sc_wsmousedev;		/* Attached wsmouse device.  */
 	const struct uatp_parameters *sc_parameters;
 	struct uatp_knobs sc_knobs;
@@ -936,6 +937,8 @@ uatp_attach(device_t parent, device_t self, void *aux)
 	sc->sc_hdev.sc_parent = uha->parent;
 	sc->sc_hdev.sc_report_id = uha->reportid;
 
+	sc->sc_udev = uha->uiaa->uiaa_device;
+
 	/* Identify ourselves to dmesg.  */
 	uatp_descriptor = find_uatp_descriptor(uha);
 	KASSERT(uatp_descriptor != NULL);
@@ -1296,7 +1299,7 @@ uatp_ioctl(void *v, unsigned long cmd, void *data, int flag, struct lwp *p)
 static void
 geyser34_enable_raw_mode(struct uatp_softc *sc)
 {
-	struct usbd_device *udev = sc->sc_hdev.sc_parent->sc_udev;
+	struct usbd_device *udev = sc->sc_udev;
 	usb_device_request_t req;
 	usbd_status status;
 	uint8_t report[GEYSER34_MODE_PACKET_SIZE];
@@ -1368,8 +1371,8 @@ geyser34_finalize(struct uatp_softc *sc)
 {
 
 	DPRINTF(sc, UATP_DEBUG_MISC, ("finalizing\n"));
-	usb_rem_task_wait(sc->sc_hdev.sc_parent->sc_udev, &sc->sc_reset_task,
-	    USB_TASKQ_DRIVER, NULL);
+	usb_rem_task_wait(sc->sc_udev, &sc->sc_reset_task, USB_TASKQ_DRIVER,
+	    NULL);
 
 	return 0;
 }
@@ -1379,8 +1382,7 @@ geyser34_deferred_reset(struct uatp_softc *sc)
 {
 
 	DPRINTF(sc, UATP_DEBUG_RESET, ("deferring reset\n"));
-	usb_add_task(sc->sc_hdev.sc_parent->sc_udev, &sc->sc_reset_task,
-	    USB_TASKQ_DRIVER);
+	usb_add_task(sc->sc_udev, &sc->sc_reset_task, USB_TASKQ_DRIVER);
 }
 
 static void
diff --git a/sys/dev/usb/uhid.c b/sys/dev/usb/uhid.c
index 66ab1a74b3c1..3d2841a1867d 100644
--- a/sys/dev/usb/uhid.c
+++ b/sys/dev/usb/uhid.c
@@ -86,6 +86,7 @@ int	uhiddebug = 0;
 
 struct uhid_softc {
 	struct uhidev sc_hdev;
+	struct usbd_device *sc_udev;
 
 	kmutex_t sc_lock;
 	kcondvar_t sc_cv;
@@ -182,6 +183,8 @@ uhid_attach(device_t parent, device_t self, void *aux)
 	sc->sc_hdev.sc_parent = uha->parent;
 	sc->sc_hdev.sc_report_id = uha->reportid;
 
+	sc->sc_udev = uha->uiaa->uiaa_device;
+
 	uhidev_get_report_desc(uha->parent, &desc, &size);
 	repid = uha->reportid;
 	sc->sc_isize = hid_report_size(desc, size, hid_input,   repid);
@@ -755,16 +758,16 @@ uhid_do_ioctl(struct uhid_softc *sc, u_long cmd, void *addr,
 
 	case USB_GET_DEVICE_DESC:
 		*(usb_device_descriptor_t *)addr =
-			*usbd_get_device_descriptor(sc->sc_hdev.sc_parent->sc_udev);
+			*usbd_get_device_descriptor(sc->sc_udev);
 		break;
 
 	case USB_GET_DEVICEINFO:
-		usbd_fill_deviceinfo(sc->sc_hdev.sc_parent->sc_udev,
+		usbd_fill_deviceinfo(sc->sc_udev,
 				     (struct usb_device_info *)addr, 0);
 		break;
 	case USB_GET_DEVICEINFO_OLD:
 		MODULE_HOOK_CALL(usb_subr_fill_30_hook,
-                    (sc->sc_hdev.sc_parent->sc_udev,
+                    (sc->sc_udev,
 		      (struct usb_device_info_old *)addr, 0,
                       usbd_devinfo_vp, usbd_printBCD),
                     enosys(), err);
@@ -774,7 +777,7 @@ uhid_do_ioctl(struct uhid_softc *sc, u_long cmd, void *addr,
 	case USB_GET_STRING_DESC:
 	    {
 		struct usb_string_desc *si = (struct usb_string_desc *)addr;
-		err = usbd_get_string_desc(sc->sc_hdev.sc_parent->sc_udev,
+		err = usbd_get_string_desc(sc->sc_udev,
 			si->usd_string_index,
 			si->usd_language_id, &si->usd_desc, &size);
 		if (err)
diff --git a/sys/dev/usb/ukbd.c b/sys/dev/usb/ukbd.c
index 14df4d53730e..4b96e831c8b5 100644
--- a/sys/dev/usb/ukbd.c
+++ b/sys/dev/usb/ukbd.c
@@ -236,6 +236,8 @@ Static const uint8_t ukbd_trtab[256] = {
 
 struct ukbd_softc {
 	struct uhidev sc_hdev;
+	struct usbd_device *sc_udev;
+	struct usbd_interface *sc_iface;
 
 	struct ukbd_data sc_ndata;
 	struct ukbd_data sc_odata;
@@ -410,6 +412,8 @@ ukbd_attach(device_t parent, device_t self, void *aux)
 	sc->sc_hdev.sc_intr = ukbd_intr;
 	sc->sc_hdev.sc_parent = uha->parent;
 	sc->sc_hdev.sc_report_id = uha->reportid;
+	sc->sc_udev = uha->uiaa->uiaa_device;
+	sc->sc_iface = uha->uiaa->uiaa_iface;
 	sc->sc_flags = 0;
 
 	aprint_naive("\n");
@@ -427,7 +431,7 @@ ukbd_attach(device_t parent, device_t self, void *aux)
 	}
 
 	/* Quirks */
-	qflags = usbd_get_quirks(uha->parent->sc_udev)->uq_flags;
+	qflags = usbd_get_quirks(sc->sc_udev)->uq_flags;
 	if (qflags & UQ_SPUR_BUT_UP)
 		sc->sc_flags |= FLAG_DEBOUNCE;
 	if (qflags & UQ_APPLE_ISO)
@@ -579,7 +583,7 @@ ukbd_detach(device_t self, int flags)
 
 	callout_halt(&sc->sc_delay, NULL);
 	callout_halt(&sc->sc_ledreset, NULL);
-	usb_rem_task_wait(sc->sc_hdev.sc_parent->sc_udev, &sc->sc_ledtask,
+	usb_rem_task_wait(sc->sc_udev, &sc->sc_ledtask,
 	    USB_TASKQ_DRIVER, NULL);
 
 	/* The console keyboard does not get a disable call, so check pipe. */
@@ -885,7 +889,7 @@ void
 ukbd_set_leds(void *v, int leds)
 {
 	struct ukbd_softc *sc = v;
-	struct usbd_device *udev = sc->sc_hdev.sc_parent->sc_udev;
+	struct usbd_device *udev = sc->sc_udev;
 
 	DPRINTF(("%s: sc=%p leds=%d, sc_leds=%d\n", __func__,
 		 sc, leds, sc->sc_leds));
@@ -995,7 +999,7 @@ ukbd_cngetc(void *v, u_int *type, int *data)
 	DPRINTFN(0,("%s: enter\n", __func__));
 	sc->sc_flags |= FLAG_POLLING;
 	if (sc->sc_npollchar <= 0)
-		usbd_dopoll(sc->sc_hdev.sc_parent->sc_iface);
+		usbd_dopoll(sc->sc_iface);
 	sc->sc_flags &= ~FLAG_POLLING;
 	if (sc->sc_npollchar > 0) {
 		c = sc->sc_pollchars[0];
@@ -1021,7 +1025,8 @@ ukbd_cnpollc(void *v, int on)
 
 	DPRINTFN(2,("%s: sc=%p on=%d\n", __func__, v, on));
 
-	usbd_interface2device_handle(sc->sc_hdev.sc_parent->sc_iface, &dev);
+	/* XXX Can this just use sc->sc_udev, or am I mistaken?  */
+	usbd_interface2device_handle(sc->sc_iface, &dev);
 	if (on) {
 		sc->sc_spl = splusb();
 		pollenter++;
diff --git a/sys/dev/usb/ums.c b/sys/dev/usb/ums.c
index 0f49020b7e9e..a445cc9cfc3c 100644
--- a/sys/dev/usb/ums.c
+++ b/sys/dev/usb/ums.c
@@ -76,6 +76,7 @@ int	umsdebug = 0;
 
 struct ums_softc {
 	struct uhidev sc_hdev;
+	struct usbd_device *sc_udev;
 	struct hidms sc_ms;
 
 	bool	sc_alwayson;
@@ -149,8 +150,9 @@ ums_attach(device_t parent, device_t self, void *aux)
 	sc->sc_hdev.sc_intr = ums_intr;
 	sc->sc_hdev.sc_parent = uha->parent;
 	sc->sc_hdev.sc_report_id = uha->reportid;
+	sc->sc_udev = uha->uiaa->uiaa_device;
 
-	quirks = usbd_get_quirks(uha->parent->sc_udev)->uq_flags;
+	quirks = usbd_get_quirks(sc->sc_udev)->uq_flags;
 	if (quirks & UQ_MS_REVZ)
 		sc->sc_ms.flags |= HIDMS_REVZ;
 	if (quirks & UQ_SPUR_BUT_UP)
diff --git a/sys/dev/usb/uthum.c b/sys/dev/usb/uthum.c
index 924a6586ad21..e2fbd70584e0 100644
--- a/sys/dev/usb/uthum.c
+++ b/sys/dev/usb/uthum.c
@@ -130,7 +130,7 @@ uthum_attach(device_t parent, device_t self, void *aux)
 {
 	struct uthum_softc *sc = device_private(self);
 	struct uhidev_attach_arg *uha = aux;
-	struct usbd_device *dev = uha->parent->sc_udev;
+	struct usbd_device *dev = uha->uiaa->uiaa_device;
 	int size, repid;
 	void *desc;
 

From 7536a756168f47c5afdb8ec41ac73cc291297823 Mon Sep 17 00:00:00 2001
From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
Date: Tue, 25 Jan 2022 00:43:58 +0000
Subject: [PATCH 4/4] uhidev(9): Move struct uhidev_softc into uhidev.c.

No longer part of any ABI for uhidev modules.
---
 sys/dev/usb/uhidev.c | 40 ++++++++++++++++++++++++++++++++++++++++
 sys/dev/usb/uhidev.h | 40 ----------------------------------------
 2 files changed, 40 insertions(+), 40 deletions(-)

diff --git a/sys/dev/usb/uhidev.c b/sys/dev/usb/uhidev.c
index d1a2909c5b59..cb7130c1dfde 100644
--- a/sys/dev/usb/uhidev.c
+++ b/sys/dev/usb/uhidev.c
@@ -74,6 +74,46 @@ __KERNEL_RCSID(0, "$NetBSD: uhidev.c,v 1.81 2021/08/07 16:19:17 thorpej Exp $");
 
 #include "locators.h"
 
+struct uhidev_softc {
+	device_t sc_dev;		/* base device */
+	struct usbd_device *sc_udev;
+	struct usbd_interface *sc_iface;	/* interface */
+	int sc_iep_addr;
+	int sc_oep_addr;
+	u_int sc_isize;
+
+	int sc_repdesc_size;
+	void *sc_repdesc;
+
+	u_int sc_nrepid;
+	device_t *sc_subdevs;
+
+	kmutex_t sc_lock;
+	kcondvar_t sc_cv;
+
+	/* Read/written under sc_lock.  */
+	struct lwp *sc_writelock;
+	struct lwp *sc_configlock;
+	int sc_refcnt;
+	int sc_writereportid;
+	u_char sc_dying;
+
+	/*
+	 * - Read under sc_lock, provided sc_refcnt > 0.
+	 * - Written under sc_configlock only when transitioning to and
+	 *   from sc_refcnt = 0.
+	 */
+	u_char *sc_ibuf;
+	struct usbd_pipe *sc_ipipe;	/* input interrupt pipe */
+	struct usbd_pipe *sc_opipe;	/* output interrupt pipe */
+	struct usbd_xfer *sc_oxfer;	/* write request */
+	usbd_callback sc_writecallback;	/* async write request callback */
+	void *sc_writecookie;
+
+	u_int sc_flags;
+#define UHIDEV_F_XB1	0x0001	/* Xbox 1 controller */
+};
+
 #ifdef UHIDEV_DEBUG
 #define DPRINTF(x)	if (uhidevdebug) printf x
 #define DPRINTFN(n,x)	if (uhidevdebug>(n)) printf x
diff --git a/sys/dev/usb/uhidev.h b/sys/dev/usb/uhidev.h
index 0b19e9015fcd..b75f6f6f74c1 100644
--- a/sys/dev/usb/uhidev.h
+++ b/sys/dev/usb/uhidev.h
@@ -35,46 +35,6 @@
 
 #include <sys/rndsource.h>
 
-struct uhidev_softc {
-	device_t sc_dev;		/* base device */
-	struct usbd_device *sc_udev;
-	struct usbd_interface *sc_iface;	/* interface */
-	int sc_iep_addr;
-	int sc_oep_addr;
-	u_int sc_isize;
-
-	int sc_repdesc_size;
-	void *sc_repdesc;
-
-	u_int sc_nrepid;
-	device_t *sc_subdevs;
-
-	kmutex_t sc_lock;
-	kcondvar_t sc_cv;
-
-	/* Read/written under sc_lock.  */
-	struct lwp *sc_writelock;
-	struct lwp *sc_configlock;
-	int sc_refcnt;
-	int sc_writereportid;
-	u_char sc_dying;
-
-	/*
-	 * - Read under sc_lock, provided sc_refcnt > 0.
-	 * - Written under sc_configlock only when transitioning to and
-	 *   from sc_refcnt = 0.
-	 */
-	u_char *sc_ibuf;
-	struct usbd_pipe *sc_ipipe;	/* input interrupt pipe */
-	struct usbd_pipe *sc_opipe;	/* output interrupt pipe */
-	struct usbd_xfer *sc_oxfer;	/* write request */
-	usbd_callback sc_writecallback;	/* async write request callback */
-	void *sc_writecookie;
-
-	u_int sc_flags;
-#define UHIDEV_F_XB1	0x0001	/* Xbox 1 controller */
-};
-
 struct uhidev {
 	device_t sc_dev;		/* base device */
 	struct uhidev_softc *sc_parent;

diff --git a/sys/arch/macppc/dev/pbms.c b/sys/arch/macppc/dev/pbms.c
index a1bfb88516cb..edc2213c619c 100644
--- a/sys/arch/macppc/dev/pbms.c
+++ b/sys/arch/macppc/dev/pbms.c
@@ -307,7 +307,8 @@ pbms_match(device_t parent, cfdata_t match, void *aux)
 	 * we expect. 
 	 */
 	if (uha->uiaa->uiaa_proto == UIPROTO_MOUSE &&
-	    (udd = usbd_get_device_descriptor(uha->parent->sc_udev)) != NULL) {
+	    ((udd = usbd_get_device_descriptor(uha->uiaa->uiaa_device))
+		!= NULL)) {
 		vendor = UGETW(udd->idVendor);
 		product = UGETW(udd->idProduct);
 		for (i = 0; i < PBMS_NUM_DEVICES; i++) {
@@ -329,6 +330,7 @@ pbms_attach(device_t parent, device_t self, void *aux)
 	struct uhidev_attach_arg *uha = aux;
 	struct pbms_dev *pd;
 	struct pbms_softc *sc = device_private(self);
+	struct usbd_device *udev;
 	usb_device_descriptor_t *udd;
 	int i;
 	uint16_t vendor, product;
@@ -341,7 +343,8 @@ pbms_attach(device_t parent, device_t self, void *aux)
 	sc->sc_datalen = PBMS_DATA_LEN;
 
 	/* Fill in device-specific parameters. */
-	if ((udd = usbd_get_device_descriptor(uha->parent->sc_udev)) != NULL) {
+	udev = uha->uiaa->uiaa_udevice;
+	if ((udd = usbd_get_device_descriptor(udev)) != NULL) {
 		product = UGETW(udd->idProduct);
 		vendor = UGETW(udd->idVendor);
 		for (i = 0; i < PBMS_NUM_DEVICES; i++) {
diff --git a/sys/dev/usb/uatp.c b/sys/dev/usb/uatp.c
index a94e755f1ed2..d0a564eca616 100644
--- a/sys/dev/usb/uatp.c
+++ b/sys/dev/usb/uatp.c
@@ -486,7 +486,8 @@ static const struct uatp_knobs default_knobs = {
 };
 
 struct uatp_softc {
-	struct uhidev sc_hdev;		/* USB parent.  */
+	struct uhidev sc_hdev;		/* uhidev(9) parent.  */
+	struct usbd_device *sc_udev;	/* USB device.  */
 	device_t sc_wsmousedev;		/* Attached wsmouse device.  */
 	const struct uatp_parameters *sc_parameters;
 	struct uatp_knobs sc_knobs;
@@ -936,6 +937,8 @@ uatp_attach(device_t parent, device_t self, void *aux)
 	sc->sc_hdev.sc_parent = uha->parent;
 	sc->sc_hdev.sc_report_id = uha->reportid;
 
+	sc->sc_udev = uha->uiaa->uiaa_device;
+
 	/* Identify ourselves to dmesg.  */
 	uatp_descriptor = find_uatp_descriptor(uha);
 	KASSERT(uatp_descriptor != NULL);
@@ -1296,7 +1299,7 @@ uatp_ioctl(void *v, unsigned long cmd, void *data, int flag, struct lwp *p)
 static void
 geyser34_enable_raw_mode(struct uatp_softc *sc)
 {
-	struct usbd_device *udev = sc->sc_hdev.sc_parent->sc_udev;
+	struct usbd_device *udev = sc->sc_udev;
 	usb_device_request_t req;
 	usbd_status status;
 	uint8_t report[GEYSER34_MODE_PACKET_SIZE];
@@ -1368,8 +1371,8 @@ geyser34_finalize(struct uatp_softc *sc)
 {
 
 	DPRINTF(sc, UATP_DEBUG_MISC, ("finalizing\n"));
-	usb_rem_task_wait(sc->sc_hdev.sc_parent->sc_udev, &sc->sc_reset_task,
-	    USB_TASKQ_DRIVER, NULL);
+	usb_rem_task_wait(sc->sc_udev, &sc->sc_reset_task, USB_TASKQ_DRIVER,
+	    NULL);
 
 	return 0;
 }
@@ -1379,8 +1382,7 @@ geyser34_deferred_reset(struct uatp_softc *sc)
 {
 
 	DPRINTF(sc, UATP_DEBUG_RESET, ("deferring reset\n"));
-	usb_add_task(sc->sc_hdev.sc_parent->sc_udev, &sc->sc_reset_task,
-	    USB_TASKQ_DRIVER);
+	usb_add_task(sc->sc_udev, &sc->sc_reset_task, USB_TASKQ_DRIVER);
 }
 
 static void
diff --git a/sys/dev/usb/ucycom.c b/sys/dev/usb/ucycom.c
index bb6046ae1a12..242ac817fda5 100644
--- a/sys/dev/usb/ucycom.c
+++ b/sys/dev/usb/ucycom.c
@@ -592,11 +592,9 @@ ucycomstart(struct tty *tp)
 	}
 #endif
 	DPRINTFN(4,("ucycomstart: %d chars\n", len));
-	usbd_setup_xfer(sc->sc_hdev.sc_parent->sc_oxfer, sc, sc->sc_obuf,
-	    sc->sc_olen, 0, USBD_NO_TIMEOUT, ucycomwritecb);
-
 	/* What can we do on error? */
-	err = usbd_transfer(sc->sc_hdev.sc_parent->sc_oxfer);
+	err = uhidev_write_async(&sc->sc_hdev, sc->sc_obuf, sc->sc_olen, 0,
+	    USBD_NO_TIMEOUT, ucycomwritecb, sc);
 
 #ifdef UCYCOM_DEBUG
 	if (err != USBD_IN_PROGRESS)
@@ -621,7 +619,6 @@ ucycomwritecb(struct usbd_xfer *xfer, void *p, usbd_status status)
 
 	if (status) {
 		DPRINTF(("ucycomwritecb: status=%d\n", status));
-		usbd_clear_endpoint_stall(sc->sc_hdev.sc_parent->sc_opipe);
 		/* XXX we should restart after some delay. */
 		goto error;
 	}
@@ -1105,7 +1102,7 @@ ucycom_set_status(struct ucycom_softc *sc)
 	memset(sc->sc_obuf, 0, sc->sc_olen);
 	sc->sc_obuf[0] = sc->sc_mcr;
 
-	err = uhidev_write(sc->sc_hdev.sc_parent, sc->sc_obuf, sc->sc_olen);
+	err = uhidev_write(&sc->sc_hdev, sc->sc_obuf, sc->sc_olen);
 	if (err) {
 		DPRINTF(("ucycom_set_status: err=%d\n", err));
 	}
diff --git a/sys/dev/usb/uhid.c b/sys/dev/usb/uhid.c
index 46769dc0353a..3d2841a1867d 100644
--- a/sys/dev/usb/uhid.c
+++ b/sys/dev/usb/uhid.c
@@ -86,6 +86,7 @@ int	uhiddebug = 0;
 
 struct uhid_softc {
 	struct uhidev sc_hdev;
+	struct usbd_device *sc_udev;
 
 	kmutex_t sc_lock;
 	kcondvar_t sc_cv;
@@ -182,6 +183,8 @@ uhid_attach(device_t parent, device_t self, void *aux)
 	sc->sc_hdev.sc_parent = uha->parent;
 	sc->sc_hdev.sc_report_id = uha->reportid;
 
+	sc->sc_udev = uha->uiaa->uiaa_device;
+
 	uhidev_get_report_desc(uha->parent, &desc, &size);
 	repid = uha->reportid;
 	sc->sc_isize = hid_report_size(desc, size, hid_input,   repid);
@@ -581,8 +584,7 @@ uhid_do_write(struct uhid_softc *sc, struct uio *uio, int flag)
 #endif
 	if (!error) {
 		if (sc->sc_raw)
-			err = uhidev_write(sc->sc_hdev.sc_parent, sc->sc_obuf,
-			    size);
+			err = uhidev_write(&sc->sc_hdev, sc->sc_obuf, size);
 		else
 			err = uhidev_set_report(&sc->sc_hdev,
 			    UHID_OUTPUT_REPORT, sc->sc_obuf, size);
@@ -756,16 +758,16 @@ uhid_do_ioctl(struct uhid_softc *sc, u_long cmd, void *addr,
 
 	case USB_GET_DEVICE_DESC:
 		*(usb_device_descriptor_t *)addr =
-			*usbd_get_device_descriptor(sc->sc_hdev.sc_parent->sc_udev);
+			*usbd_get_device_descriptor(sc->sc_udev);
 		break;
 
 	case USB_GET_DEVICEINFO:
-		usbd_fill_deviceinfo(sc->sc_hdev.sc_parent->sc_udev,
+		usbd_fill_deviceinfo(sc->sc_udev,
 				     (struct usb_device_info *)addr, 0);
 		break;
 	case USB_GET_DEVICEINFO_OLD:
 		MODULE_HOOK_CALL(usb_subr_fill_30_hook,
-                    (sc->sc_hdev.sc_parent->sc_udev,
+                    (sc->sc_udev,
 		      (struct usb_device_info_old *)addr, 0,
                       usbd_devinfo_vp, usbd_printBCD),
                     enosys(), err);
@@ -775,7 +777,7 @@ uhid_do_ioctl(struct uhid_softc *sc, u_long cmd, void *addr,
 	case USB_GET_STRING_DESC:
 	    {
 		struct usb_string_desc *si = (struct usb_string_desc *)addr;
-		err = usbd_get_string_desc(sc->sc_hdev.sc_parent->sc_udev,
+		err = usbd_get_string_desc(sc->sc_udev,
 			si->usd_string_index,
 			si->usd_language_id, &si->usd_desc, &size);
 		if (err)
diff --git a/sys/dev/usb/uhidev.c b/sys/dev/usb/uhidev.c
index b0c03092c77a..cb7130c1dfde 100644
--- a/sys/dev/usb/uhidev.c
+++ b/sys/dev/usb/uhidev.c
@@ -74,6 +74,46 @@ __KERNEL_RCSID(0, "$NetBSD: uhidev.c,v 1.81 2021/08/07 16:19:17 thorpej Exp $");
 
 #include "locators.h"
 
+struct uhidev_softc {
+	device_t sc_dev;		/* base device */
+	struct usbd_device *sc_udev;
+	struct usbd_interface *sc_iface;	/* interface */
+	int sc_iep_addr;
+	int sc_oep_addr;
+	u_int sc_isize;
+
+	int sc_repdesc_size;
+	void *sc_repdesc;
+
+	u_int sc_nrepid;
+	device_t *sc_subdevs;
+
+	kmutex_t sc_lock;
+	kcondvar_t sc_cv;
+
+	/* Read/written under sc_lock.  */
+	struct lwp *sc_writelock;
+	struct lwp *sc_configlock;
+	int sc_refcnt;
+	int sc_writereportid;
+	u_char sc_dying;
+
+	/*
+	 * - Read under sc_lock, provided sc_refcnt > 0.
+	 * - Written under sc_configlock only when transitioning to and
+	 *   from sc_refcnt = 0.
+	 */
+	u_char *sc_ibuf;
+	struct usbd_pipe *sc_ipipe;	/* input interrupt pipe */
+	struct usbd_pipe *sc_opipe;	/* output interrupt pipe */
+	struct usbd_xfer *sc_oxfer;	/* write request */
+	usbd_callback sc_writecallback;	/* async write request callback */
+	void *sc_writecookie;
+
+	u_int sc_flags;
+#define UHIDEV_F_XB1	0x0001	/* Xbox 1 controller */
+};
+
 #ifdef UHIDEV_DEBUG
 #define DPRINTF(x)	if (uhidevdebug) printf x
 #define DPRINTFN(n,x)	if (uhidevdebug>(n)) printf x
@@ -901,7 +941,7 @@ uhidev_stop(struct uhidev *scd)
 		abort = true;
 	mutex_exit(&sc->sc_lock);
 
-	if (abort && sc->sc_opipe)
+	if (abort && sc->sc_opipe) /* XXX sc_opipe might go away */
 		usbd_abort_pipe(sc->sc_opipe);
 }
 
@@ -961,8 +1001,9 @@ uhidev_get_report(struct uhidev *scd, int type, void *data, int len)
 }
 
 usbd_status
-uhidev_write(struct uhidev_softc *sc, void *data, int len)
+uhidev_write(struct uhidev *scd, void *data, int len)
 {
+	struct uhidev_softc *sc = scd->sc_parent;
 	usbd_status err;
 
 	DPRINTF(("uhidev_write: data=%p, len=%d\n", data, len));
@@ -985,6 +1026,7 @@ uhidev_write(struct uhidev_softc *sc, void *data, int len)
 		}
 	}
 	sc->sc_writelock = curlwp;
+	sc->sc_writereportid = scd->sc_report_id;
 	mutex_exit(&sc->sc_lock);
 
 #ifdef UHIDEV_DEBUG
@@ -1006,8 +1048,79 @@ uhidev_write(struct uhidev_softc *sc, void *data, int len)
 	KASSERT(sc->sc_refcnt);
 	KASSERTMSG(sc->sc_writelock == curlwp, "%s: migrated from %p to %p",
 	    device_xname(sc->sc_dev), curlwp, sc->sc_writelock);
+	KASSERTMSG(sc->sc_writereportid == scd->sc_report_id,
+	    "%s: changed write report ids from %d to %d",
+	    device_xname(sc->sc_dev), scd->sc_report_id, sc->sc_writereportid);
+	sc->sc_writereportid = -1;
 	sc->sc_writelock = NULL;
 	cv_broadcast(&sc->sc_cv);
 out:	mutex_exit(&sc->sc_lock);
 	return err;
 }
+
+static void
+uhidev_write_callback(struct usbd_xfer *xfer, void *cookie, usbd_status err)
+{
+	struct uhidev_softc *sc = cookie;
+	usbd_callback writecallback;
+	void *writecookie;
+
+	if (err)
+		usbd_clear_endpoint_stall(sc->sc_opipe);
+
+	mutex_enter(&sc->sc_lock);
+	KASSERT(sc->sc_writelock == (void *)1);
+	writecallback = sc->sc_writecallback;
+	writecookie = sc->sc_writecookie;
+	sc->sc_writereportid = -1;
+	sc->sc_writelock = NULL;
+	sc->sc_writecallback = NULL;
+	sc->sc_writecookie = NULL;
+	cv_broadcast(&sc->sc_cv);
+	mutex_exit(&sc->sc_lock);
+
+	(*writecallback)(xfer, writecookie, err);
+}
+
+usbd_status
+uhidev_write_async(struct uhidev *scd, void *data, int len, int flags,
+    int timo, usbd_callback writecallback, void *writecookie)
+{
+	struct uhidev_softc *sc = scd->sc_parent;
+	usbd_status err;
+
+	DPRINTF(("%s: data=%p, len=%d\n", __func__, data, len));
+
+	if (sc->sc_opipe == NULL)
+		return USBD_INVAL;
+
+	mutex_enter(&sc->sc_lock);
+	KASSERT(sc->sc_refcnt);
+	if (sc->sc_dying) {
+		err = USBD_IOERROR;
+		goto out;
+	}
+	if (sc->sc_writelock != NULL) {
+		err = USBD_IN_USE;
+		goto out;
+	}
+	sc->sc_writelock = (void *)1; /* XXX no lwp to attribute async xfer */
+	sc->sc_writereportid = scd->sc_report_id;
+	sc->sc_writecallback = writecallback;
+	sc->sc_writecookie = writecookie;
+	usbd_setup_xfer(sc->sc_oxfer, sc, data, len, flags, timo,
+	    uhidev_write_callback);
+	err = usbd_transfer(sc->sc_oxfer);
+	switch (err) {
+	case USBD_IN_PROGRESS:
+		break;
+	case USBD_NORMAL_COMPLETION:
+		panic("unexpected normal completion of async xfer under lock");
+	default:		/* error */
+		sc->sc_writelock = NULL;
+		sc->sc_writereportid = -1;
+		cv_broadcast(&sc->sc_cv);
+	}
+out:	mutex_exit(&sc->sc_lock);
+	return err;
+}
diff --git a/sys/dev/usb/uhidev.h b/sys/dev/usb/uhidev.h
index 83c0a357744e..b75f6f6f74c1 100644
--- a/sys/dev/usb/uhidev.h
+++ b/sys/dev/usb/uhidev.h
@@ -35,44 +35,6 @@
 
 #include <sys/rndsource.h>
 
-struct uhidev_softc {
-	device_t sc_dev;		/* base device */
-	struct usbd_device *sc_udev;
-	struct usbd_interface *sc_iface;	/* interface */
-	int sc_iep_addr;
-	int sc_oep_addr;
-	u_int sc_isize;
-
-	int sc_repdesc_size;
-	void *sc_repdesc;
-
-	u_int sc_nrepid;
-	device_t *sc_subdevs;
-
-	kmutex_t sc_lock;
-	kcondvar_t sc_cv;
-
-	/* Read/written under sc_lock.  */
-	struct lwp *sc_writelock;
-	struct lwp *sc_configlock;
-	int sc_refcnt;
-	int sc_writereportid;
-	u_char sc_dying;
-
-	/*
-	 * - Read under sc_lock, provided sc_refcnt > 0.
-	 * - Written under sc_configlock only when transitioning to and
-	 *   from sc_refcnt = 0.
-	 */
-	u_char *sc_ibuf;
-	struct usbd_pipe *sc_ipipe;	/* input interrupt pipe */
-	struct usbd_pipe *sc_opipe;	/* output interrupt pipe */
-	struct usbd_xfer *sc_oxfer;	/* write request */
-
-	u_int sc_flags;
-#define UHIDEV_F_XB1	0x0001	/* Xbox 1 controller */
-};
-
 struct uhidev {
 	device_t sc_dev;		/* base device */
 	struct uhidev_softc *sc_parent;
@@ -97,7 +59,9 @@ void uhidev_stop(struct uhidev *);
 void uhidev_close(struct uhidev *);
 usbd_status uhidev_set_report(struct uhidev *, int, void *, int);
 usbd_status uhidev_get_report(struct uhidev *, int, void *, int);
-usbd_status uhidev_write(struct uhidev_softc *, void *, int);
+usbd_status uhidev_write(struct uhidev *, void *, int);
+usbd_status uhidev_write_async(struct uhidev *, void *, int, int, int,
+    usbd_callback, void *);
 
 #define	UHIDEV_OSIZE	64
 
diff --git a/sys/dev/usb/ukbd.c b/sys/dev/usb/ukbd.c
index 14df4d53730e..4b96e831c8b5 100644
--- a/sys/dev/usb/ukbd.c
+++ b/sys/dev/usb/ukbd.c
@@ -236,6 +236,8 @@ Static const uint8_t ukbd_trtab[256] = {
 
 struct ukbd_softc {
 	struct uhidev sc_hdev;
+	struct usbd_device *sc_udev;
+	struct usbd_interface *sc_iface;
 
 	struct ukbd_data sc_ndata;
 	struct ukbd_data sc_odata;
@@ -410,6 +412,8 @@ ukbd_attach(device_t parent, device_t self, void *aux)
 	sc->sc_hdev.sc_intr = ukbd_intr;
 	sc->sc_hdev.sc_parent = uha->parent;
 	sc->sc_hdev.sc_report_id = uha->reportid;
+	sc->sc_udev = uha->uiaa->uiaa_device;
+	sc->sc_iface = uha->uiaa->uiaa_iface;
 	sc->sc_flags = 0;
 
 	aprint_naive("\n");
@@ -427,7 +431,7 @@ ukbd_attach(device_t parent, device_t self, void *aux)
 	}
 
 	/* Quirks */
-	qflags = usbd_get_quirks(uha->parent->sc_udev)->uq_flags;
+	qflags = usbd_get_quirks(sc->sc_udev)->uq_flags;
 	if (qflags & UQ_SPUR_BUT_UP)
 		sc->sc_flags |= FLAG_DEBOUNCE;
 	if (qflags & UQ_APPLE_ISO)
@@ -579,7 +583,7 @@ ukbd_detach(device_t self, int flags)
 
 	callout_halt(&sc->sc_delay, NULL);
 	callout_halt(&sc->sc_ledreset, NULL);
-	usb_rem_task_wait(sc->sc_hdev.sc_parent->sc_udev, &sc->sc_ledtask,
+	usb_rem_task_wait(sc->sc_udev, &sc->sc_ledtask,
 	    USB_TASKQ_DRIVER, NULL);
 
 	/* The console keyboard does not get a disable call, so check pipe. */
@@ -885,7 +889,7 @@ void
 ukbd_set_leds(void *v, int leds)
 {
 	struct ukbd_softc *sc = v;
-	struct usbd_device *udev = sc->sc_hdev.sc_parent->sc_udev;
+	struct usbd_device *udev = sc->sc_udev;
 
 	DPRINTF(("%s: sc=%p leds=%d, sc_leds=%d\n", __func__,
 		 sc, leds, sc->sc_leds));
@@ -995,7 +999,7 @@ ukbd_cngetc(void *v, u_int *type, int *data)
 	DPRINTFN(0,("%s: enter\n", __func__));
 	sc->sc_flags |= FLAG_POLLING;
 	if (sc->sc_npollchar <= 0)
-		usbd_dopoll(sc->sc_hdev.sc_parent->sc_iface);
+		usbd_dopoll(sc->sc_iface);
 	sc->sc_flags &= ~FLAG_POLLING;
 	if (sc->sc_npollchar > 0) {
 		c = sc->sc_pollchars[0];
@@ -1021,7 +1025,8 @@ ukbd_cnpollc(void *v, int on)
 
 	DPRINTFN(2,("%s: sc=%p on=%d\n", __func__, v, on));
 
-	usbd_interface2device_handle(sc->sc_hdev.sc_parent->sc_iface, &dev);
+	/* XXX Can this just use sc->sc_udev, or am I mistaken?  */
+	usbd_interface2device_handle(sc->sc_iface, &dev);
 	if (on) {
 		sc->sc_spl = splusb();
 		pollenter++;
diff --git a/sys/dev/usb/ums.c b/sys/dev/usb/ums.c
index 0f49020b7e9e..a445cc9cfc3c 100644
--- a/sys/dev/usb/ums.c
+++ b/sys/dev/usb/ums.c
@@ -76,6 +76,7 @@ int	umsdebug = 0;
 
 struct ums_softc {
 	struct uhidev sc_hdev;
+	struct usbd_device *sc_udev;
 	struct hidms sc_ms;
 
 	bool	sc_alwayson;
@@ -149,8 +150,9 @@ ums_attach(device_t parent, device_t self, void *aux)
 	sc->sc_hdev.sc_intr = ums_intr;
 	sc->sc_hdev.sc_parent = uha->parent;
 	sc->sc_hdev.sc_report_id = uha->reportid;
+	sc->sc_udev = uha->uiaa->uiaa_device;
 
-	quirks = usbd_get_quirks(uha->parent->sc_udev)->uq_flags;
+	quirks = usbd_get_quirks(sc->sc_udev)->uq_flags;
 	if (quirks & UQ_MS_REVZ)
 		sc->sc_ms.flags |= HIDMS_REVZ;
 	if (quirks & UQ_SPUR_BUT_UP)
diff --git a/sys/dev/usb/uthum.c b/sys/dev/usb/uthum.c
index 924a6586ad21..e2fbd70584e0 100644
--- a/sys/dev/usb/uthum.c
+++ b/sys/dev/usb/uthum.c
@@ -130,7 +130,7 @@ uthum_attach(device_t parent, device_t self, void *aux)
 {
 	struct uthum_softc *sc = device_private(self);
 	struct uhidev_attach_arg *uha = aux;
-	struct usbd_device *dev = uha->parent->sc_udev;
+	struct usbd_device *dev = uha->uiaa->uiaa_device;
 	int size, repid;
 	void *desc;