On Wed, 17 Mar 2021 at 09:19:05 -0500, joshua stein wrote:
> On Wed, 17 Mar 2021 at 07:41:53 +0100, Anton Lindqvist wrote:
> > I'm wondering if the umt device is detached more than once since it
> > claims multiple report ids. I ran into the same problem while developing
> > uhidpp:
> >
> > https://github.com/openbsd/src/blob/master/sys/dev/usb/uhidpp.c#L437-L443
> >
> > Also, the output of `show registers` in ddb would be helpful.
> >
>
> Something like this is probably needed (maybe also for
> uhidev_activate):
Here's an updated version with feedback from Anton and a similar fix
for uhidev_activate.
Edd, can you test whether this fixes the panic for you and if
possible, if the machine suspends/resumes properly with the touchpad
attached (if it did before)?
diff --git sys/dev/usb/uhidev.c sys/dev/usb/uhidev.c
index d777cfb6e45..dd43b1bb000 100644
--- sys/dev/usb/uhidev.c
+++ sys/dev/usb/uhidev.c
@@ -382,17 +382,32 @@ int
uhidev_activate(struct device *self, int act)
{
struct uhidev_softc *sc = (struct uhidev_softc *)self;
- int i, rv = 0, r;
+ int i, j, already, rv = 0, r;
switch (act) {
case DVACT_DEACTIVATE:
- for (i = 0; i < sc->sc_nrepid; i++)
- if (sc->sc_subdevs[i] != NULL) {
+ for (i = 0; i < sc->sc_nrepid; i++) {
+ if (sc->sc_subdevs[i] == NULL)
+ continue;
+
+ /*
+ * Only notify devices attached to multiple report ids
+ * once.
+ */
+ for (already = 0, j = 0; j < i; j++) {
+ if (sc->sc_subdevs[i] == sc->sc_subdevs[j]) {
+ already = 1;
+ break;
+ }
+ }
+
+ if (!already) {
r = config_deactivate(
&sc->sc_subdevs[i]->sc_dev);
if (r && r != EOPNOTSUPP)
rv = r;
}
+ }
usbd_deactivate(sc->sc_udev);
break;
}
@@ -403,7 +418,7 @@ int
uhidev_detach(struct device *self, int flags)
{
struct uhidev_softc *sc = (struct uhidev_softc *)self;
- int i, rv = 0;
+ int i, j, rv = 0;
DPRINTF(("uhidev_detach: sc=%p flags=%d\n", sc, flags));
@@ -420,19 +435,20 @@ uhidev_detach(struct device *self, int flags)
if (sc->sc_repdesc != NULL)
free(sc->sc_repdesc, M_USBDEV, sc->sc_repdesc_size);
- /*
- * XXX Check if we have only one children claiming all the Report
- * IDs, this is a hack since we need a dev -> Report ID mapping
- * for uhidev_intr().
- */
- if (sc->sc_nrepid > 1 && sc->sc_subdevs[0] != NULL &&
- sc->sc_subdevs[0] == sc->sc_subdevs[1])
- return (config_detach(&sc->sc_subdevs[0]->sc_dev, flags));
-
for (i = 0; i < sc->sc_nrepid; i++) {
- if (sc->sc_subdevs[i] != NULL) {
- rv |= config_detach(&sc->sc_subdevs[i]->sc_dev, flags);
- sc->sc_subdevs[i] = NULL;
+ if (sc->sc_subdevs[i] == NULL)
+ continue;
+
+ rv |= config_detach(&sc->sc_subdevs[i]->sc_dev, flags);
+ sc->sc_subdevs[i] = NULL;
+
+ /*
+ * Nullify without detaching any other instances of this device
+ * found on other report ids.
+ */
+ for (j = i + 1; j < sc->sc_nrepid; j++) {
+ if (sc->sc_subdevs[i] == sc->sc_subdevs[j])
+ sc->sc_subdevs[j] = NULL;
}
}
