From: anton <anton@openbsd.org>
Date: Fri, 10 Sep 2021 05:47:38 +0000 (+0000)
Subject: Instead of letting uhidev drivers get the report sizes, do it once in
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=adb06565acb8061f0e1ddb764fb29f86eadeb2f6;p=openbsd

Instead of letting uhidev drivers get the report sizes, do it once in
uhidev and pass the same sizes as part of the attach arguments. Makes
the uhidev drivers a bit less repetitive.

It might look tempting to let uhidev assign the sizes after a driver has
attached, removing the need to repeat this logic in each driver. This
does however not work since the input size must be known while calling
uhidev_open() in order to open the interrupt pipe; and uhidev_open() is
called from several attach routines.

Note that this change only works and applies to when attaching to a
single report ID.

ok jcs@
---

diff --git a/sys/dev/usb/ucc.c b/sys/dev/usb/ucc.c
index 0c87925bc53..a6eabf8686f 100644
--- a/sys/dev/usb/ucc.c
+++ b/sys/dev/usb/ucc.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ucc.c,v 1.24 2021/09/02 15:15:12 anton Exp $	*/
+/*	$OpenBSD: ucc.c,v 1.25 2021/09/10 05:47:38 anton Exp $	*/
 
 /*
  * Copyright (c) 2021 Anton Lindqvist <anton@openbsd.org>
@@ -633,9 +633,9 @@ ucc_match(struct device *parent, void *match, void *aux)
 	void *desc;
 	int size;
 
-	uhidev_get_report_desc(uha->parent, &desc, &size);
-	if (hid_report_size(desc, size, hid_input, uha->reportid) == 0)
+	if (uha->isize == 0)
 		return UMATCH_NONE;
+	uhidev_get_report_desc(uha->parent, &desc, &size);
 	if (!ucc_hid_match(desc, size, uha->reportid))
 		return UMATCH_NONE;
 
@@ -648,7 +648,7 @@ ucc_attach(struct device *parent, struct device *self, void *aux)
 	struct ucc_softc *sc = (struct ucc_softc *)self;
 	struct uhidev_attach_arg *uha = (struct uhidev_attach_arg *)aux;
 	void *desc;
-	int error, repid, size;
+	int error, size;
 
 	sc->sc_mode = WSKBD_TRANSLATED;
 	sc->sc_last_translate = -1;
@@ -659,10 +659,9 @@ ucc_attach(struct device *parent, struct device *self, void *aux)
 	sc->sc_hdev.sc_report_id = uha->reportid;
 
 	uhidev_get_report_desc(uha->parent, &desc, &size);
-	repid = uha->reportid;
-	sc->sc_hdev.sc_isize = hid_report_size(desc, size, hid_input, repid);
-	sc->sc_hdev.sc_osize = hid_report_size(desc, size, hid_output, repid);
-	sc->sc_hdev.sc_fsize = hid_report_size(desc, size, hid_feature, repid);
+	sc->sc_hdev.sc_isize = uha->isize;
+	sc->sc_hdev.sc_osize = uha->osize;
+	sc->sc_hdev.sc_fsize = uha->fsize;
 
 	error = ucc_hid_parse(sc, desc, size);
 	if (error) {
diff --git a/sys/dev/usb/ugold.c b/sys/dev/usb/ugold.c
index 752ecff64d2..c77ce294beb 100644
--- a/sys/dev/usb/ugold.c
+++ b/sys/dev/usb/ugold.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ugold.c,v 1.17 2021/04/05 16:26:06 landry Exp $   */
+/*	$OpenBSD: ugold.c,v 1.18 2021/09/10 05:47:38 anton Exp $   */
 
 /*
  * Copyright (c) 2013 Takayoshi SASANO <uaa@openbsd.org>
@@ -139,7 +139,7 @@ ugold_attach(struct device *parent, struct device *self, void *aux)
 {
 	struct ugold_softc *sc = (struct ugold_softc *)self;
 	struct uhidev_attach_arg *uha = aux;
-	int size, repid;
+	int size;
 	void *desc;
 
 	sc->sc_udev = uha->parent->sc_udev;
@@ -159,10 +159,9 @@ ugold_attach(struct device *parent, struct device *self, void *aux)
 	}
 
 	uhidev_get_report_desc(uha->parent, &desc, &size);
-	repid = uha->reportid;
-	sc->sc_hdev.sc_isize = hid_report_size(desc, size, hid_input, repid);
-	sc->sc_hdev.sc_osize = hid_report_size(desc, size, hid_output, repid);
-	sc->sc_hdev.sc_fsize = hid_report_size(desc, size, hid_feature, repid);
+	sc->sc_hdev.sc_isize = uha->isize;
+	sc->sc_hdev.sc_osize = uha->osize;
+	sc->sc_hdev.sc_fsize = uha->fsize;
 
 	if (uhidev_open(&sc->sc_hdev)) {
 		printf(", unable to open interrupt pipe\n");
diff --git a/sys/dev/usb/uhid.c b/sys/dev/usb/uhid.c
index 085c1523ccf..0d6b8a5c677 100644
--- a/sys/dev/usb/uhid.c
+++ b/sys/dev/usb/uhid.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uhid.c,v 1.84 2021/03/08 14:35:57 jcs Exp $ */
+/*	$OpenBSD: uhid.c,v 1.85 2021/09/10 05:47:38 anton Exp $ */
 /*	$NetBSD: uhid.c,v 1.57 2003/03/11 16:44:00 augustss Exp $	*/
 
 /*
@@ -126,7 +126,7 @@ uhid_attach(struct device *parent, struct device *self, void *aux)
 {
 	struct uhid_softc *sc = (struct uhid_softc *)self;
 	struct uhidev_attach_arg *uha = (struct uhidev_attach_arg *)aux;
-	int size, repid;
+	int size;
 	void *desc;
 
 	sc->sc_hdev.sc_intr = uhid_intr;
@@ -135,10 +135,9 @@ uhid_attach(struct device *parent, struct device *self, void *aux)
 	sc->sc_hdev.sc_report_id = uha->reportid;
 
 	uhidev_get_report_desc(uha->parent, &desc, &size);
-	repid = uha->reportid;
-	sc->sc_hdev.sc_isize = hid_report_size(desc, size, hid_input, repid);
-	sc->sc_hdev.sc_osize = hid_report_size(desc, size, hid_output, repid);
-	sc->sc_hdev.sc_fsize = hid_report_size(desc, size, hid_feature, repid);
+	sc->sc_hdev.sc_isize = uha->isize;
+	sc->sc_hdev.sc_osize = uha->osize;
+	sc->sc_hdev.sc_fsize = uha->fsize;
 
 	printf(": input=%d, output=%d, feature=%d\n",
 	    sc->sc_hdev.sc_isize, sc->sc_hdev.sc_osize, sc->sc_hdev.sc_fsize);
diff --git a/sys/dev/usb/uhidev.c b/sys/dev/usb/uhidev.c
index d67b87ee3fd..2e5837fab3b 100644
--- a/sys/dev/usb/uhidev.c
+++ b/sys/dev/usb/uhidev.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uhidev.c,v 1.93 2021/09/10 05:46:01 anton Exp $	*/
+/*	$OpenBSD: uhidev.c,v 1.94 2021/09/10 05:47:38 anton Exp $	*/
 /*	$NetBSD: uhidev.c,v 1.14 2003/03/11 16:44:00 augustss Exp $	*/
 
 /*
@@ -252,6 +252,7 @@ uhidev_attach(struct device *parent, struct device *self, void *aux)
 	uha.reportid = UHIDEV_CLAIM_MULTIPLE_REPORTID;
 	uha.nreports = nrepid;
 	uha.claimed = malloc(nrepid, M_TEMP, M_WAITOK|M_ZERO);
+	uha.isize = uha.osize = uha.fsize = 0;
 
 	/* Look for a driver claiming multiple report IDs first. */
 	dev = config_found_sm(self, &uha, NULL, uhidevsubmatch);
@@ -272,10 +273,12 @@ uhidev_attach(struct device *parent, struct device *self, void *aux)
 	uha.claimed = NULL;
 
 	for (repid = 0; repid < nrepid; repid++) {
+		int isize, osize, fsize;
+
 		DPRINTF(("%s: try repid=%d\n", __func__, repid));
-		if (hid_report_size(desc, size, hid_input, repid) == 0 &&
-		    hid_report_size(desc, size, hid_output, repid) == 0 &&
-		    hid_report_size(desc, size, hid_feature, repid) == 0)
+		if ((isize = hid_report_size(desc, size, hid_input, repid)) == 0 &&
+		    (osize = hid_report_size(desc, size, hid_output, repid)) == 0 &&
+		    (fsize = hid_report_size(desc, size, hid_feature, repid)) == 0)
 			continue;
 
 		/* Could already be assigned by uhidev_set_report_dev(). */
@@ -283,6 +286,9 @@ uhidev_attach(struct device *parent, struct device *self, void *aux)
 			continue;
 
 		uha.reportid = repid;
+		uha.isize = isize;
+		uha.osize = osize;
+		uha.fsize = fsize;
 		dev = config_found_sm(self, &uha, uhidevprint, uhidevsubmatch);
 		sc->sc_subdevs[repid] = (struct uhidev *)dev;
 	}
diff --git a/sys/dev/usb/uhidev.h b/sys/dev/usb/uhidev.h
index ad66873ea75..e8c966cf0c1 100644
--- a/sys/dev/usb/uhidev.h
+++ b/sys/dev/usb/uhidev.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uhidev.h,v 1.29 2021/03/18 09:21:53 anton Exp $	*/
+/*	$OpenBSD: uhidev.h,v 1.30 2021/09/10 05:47:38 anton Exp $	*/
 /*	$NetBSD: uhidev.h,v 1.3 2002/10/08 09:56:17 dan Exp $	*/
 
 /*
@@ -84,6 +84,9 @@ struct uhidev_attach_arg {
 #define	UHIDEV_CLAIM_MULTIPLE_REPORTID	255
 	uint8_t			 nreports;
 	uint8_t			 *claimed;
+	int			 isize;
+	int			 osize;
+	int			 fsize;
 };
 
 int uhidev_report_type_conv(int);
diff --git a/sys/dev/usb/ukbd.c b/sys/dev/usb/ukbd.c
index d6eaf64f2ef..b2901e99e67 100644
--- a/sys/dev/usb/ukbd.c
+++ b/sys/dev/usb/ukbd.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ukbd.c,v 1.82 2021/01/29 16:59:41 sthen Exp $	*/
+/*	$OpenBSD: ukbd.c,v 1.83 2021/09/10 05:47:38 anton Exp $	*/
 /*      $NetBSD: ukbd.c,v 1.85 2003/03/11 16:44:00 augustss Exp $        */
 
 /*
@@ -231,9 +231,9 @@ ukbd_attach(struct device *parent, struct device *self, void *aux)
 
 	uhidev_get_report_desc(uha->parent, &desc, &dlen);
 	repid = uha->reportid;
-	sc->sc_hdev.sc_isize = hid_report_size(desc, dlen, hid_input, repid);
-	sc->sc_hdev.sc_osize = hid_report_size(desc, dlen, hid_output, repid);
-	sc->sc_hdev.sc_fsize = hid_report_size(desc, dlen, hid_feature, repid);
+	sc->sc_hdev.sc_isize = uha->isize;
+	sc->sc_hdev.sc_osize = uha->osize;
+	sc->sc_hdev.sc_fsize = uha->fsize;
 
 	 /*
 	  * Since the HID-Proxy is always detected before any
diff --git a/sys/dev/usb/ums.c b/sys/dev/usb/ums.c
index bd34bcea1f1..8421c278f70 100644
--- a/sys/dev/usb/ums.c
+++ b/sys/dev/usb/ums.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ums.c,v 1.48 2021/03/24 02:49:57 jcs Exp $ */
+/*	$OpenBSD: ums.c,v 1.49 2021/09/10 05:47:38 anton Exp $ */
 /*	$NetBSD: ums.c,v 1.60 2003/03/11 16:44:00 augustss Exp $	*/
 
 /*
@@ -121,7 +121,7 @@ ums_attach(struct device *parent, struct device *self, void *aux)
 	struct hidms *ms = &sc->sc_ms;
 	struct uhidev_attach_arg *uha = (struct uhidev_attach_arg *)aux;
 	struct usb_attach_arg *uaa = uha->uaa;
-	int size, repid;
+	int size;
 	void *desc;
 	u_int32_t qflags = 0;
 
@@ -138,10 +138,9 @@ ums_attach(struct device *parent, struct device *self, void *aux)
 	if (uaa->vendor == USB_VENDOR_ELECOM)
 		ums_fix_elecom_descriptor(sc, desc, size, uaa->product);
 
-	repid = uha->reportid;
-	sc->sc_hdev.sc_isize = hid_report_size(desc, size, hid_input, repid);
-	sc->sc_hdev.sc_osize = hid_report_size(desc, size, hid_output, repid);
-	sc->sc_hdev.sc_fsize = hid_report_size(desc, size, hid_feature, repid);
+	sc->sc_hdev.sc_isize = uha->isize;
+	sc->sc_hdev.sc_osize = uha->osize;
+	sc->sc_hdev.sc_fsize = uha->fsize;
 
 	if (sc->sc_quirks & UQ_MS_REVZ)
 		qflags |= HIDMS_REVZ;
diff --git a/sys/dev/usb/umstc.c b/sys/dev/usb/umstc.c
index 0f232671f95..14e8d308fca 100644
--- a/sys/dev/usb/umstc.c
+++ b/sys/dev/usb/umstc.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: umstc.c,v 1.4 2021/03/08 14:39:34 jcs Exp $ */
+/*	$OpenBSD: umstc.c,v 1.5 2021/09/10 05:47:38 anton Exp $ */
 
 /*
  * Copyright (c) 2020 joshua stein <jcs@jcs.org>
@@ -98,7 +98,7 @@ umstc_attach(struct device *parent, struct device *self, void *aux)
 	struct umstc_softc *sc = (struct umstc_softc *)self;
 	struct uhidev_attach_arg *uha = (struct uhidev_attach_arg *)aux;
 	struct usb_attach_arg *uaa = uha->uaa;
-	int size, repid;
+	int size;
 	void *desc;
 
 	sc->sc_hdev.sc_intr = umstc_intr;
@@ -109,10 +109,9 @@ umstc_attach(struct device *parent, struct device *self, void *aux)
 	usbd_set_idle(uha->parent->sc_udev, uha->parent->sc_ifaceno, 0, 0);
 
 	uhidev_get_report_desc(uha->parent, &desc, &size);
-	repid = uha->reportid;
-	sc->sc_hdev.sc_isize = hid_report_size(desc, size, hid_input, repid);
-	sc->sc_hdev.sc_osize = hid_report_size(desc, size, hid_output, repid);
-	sc->sc_hdev.sc_fsize = hid_report_size(desc, size, hid_feature, repid);
+	sc->sc_hdev.sc_isize = uha->isize;
+	sc->sc_hdev.sc_osize = uha->osize;
+	sc->sc_hdev.sc_fsize = uha->fsize;
 
 	uhidev_open(&sc->sc_hdev);
 
diff --git a/sys/dev/usb/uwacom.c b/sys/dev/usb/uwacom.c
index 30e087ae1eb..eeabe94b33d 100644
--- a/sys/dev/usb/uwacom.c
+++ b/sys/dev/usb/uwacom.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uwacom.c,v 1.2 2020/08/23 11:08:02 mglocker Exp $	*/
+/*	$OpenBSD: uwacom.c,v 1.3 2021/09/10 05:47:38 anton Exp $	*/
 
 /*
  * Copyright (c) 2016 Frank Groeneveld <frank@frankgroeneveld.nl>
@@ -95,7 +95,7 @@ uwacom_attach(struct device *parent, struct device *self, void *aux)
 	struct hidms *ms = &sc->sc_ms;
 	struct uhidev_attach_arg *uha = (struct uhidev_attach_arg *)aux;
 	struct usb_attach_arg *uaa = uha->uaa;
-	int size, repid;
+	int size;
 	void *desc;
 
 	sc->sc_hdev.sc_intr = uwacom_intr;
@@ -106,10 +106,9 @@ uwacom_attach(struct device *parent, struct device *self, void *aux)
 	usbd_set_idle(uha->parent->sc_udev, uha->parent->sc_ifaceno, 0, 0);
 
 	uhidev_get_report_desc(uha->parent, &desc, &size);
-	repid = uha->reportid;
-	sc->sc_hdev.sc_isize = hid_report_size(desc, size, hid_input, repid);
-	sc->sc_hdev.sc_osize = hid_report_size(desc, size, hid_output, repid);
-	sc->sc_hdev.sc_fsize = hid_report_size(desc, size, hid_feature, repid);
+	sc->sc_hdev.sc_isize = uha->isize;
+	sc->sc_hdev.sc_osize = uha->osize;
+	sc->sc_hdev.sc_fsize = uha->fsize;
 
 	ms->sc_device = self;
 	ms->sc_rawmode = 1;