On Wed, Feb 10, 2021 at 04:31:01PM +1300, Peter Kane wrote:
> On Wed, Feb 10, 2021 at 08:10:23AM +1300, Peter Kane wrote:
> > On Tue, Feb 09, 2021 at 08:29:08AM +0100, Anton Lindqvist wrote:
> > >
> > > Sorry about the regression. Could you try the following diff and send me
> > > the complete dmesg. I've also prepared an amd64 kernel with the patch
> > > applied:
> > >
>
> Hi Anton
>
> The patch applied cleanly for me and the mouse is working with the new
> kernel, plus console spam as for yours. My dmesg is below.
Great, the "spamming" is caused by UHIDPP_DEBUG being enabled. The last
diff introduced a bug which causes a panic once uhidpp is detached. The
following diff fixes that.
diff --git sys/dev/usb/uhidev.c sys/dev/usb/uhidev.c
index 9915b660d9d..dfc734aa4ea 100644
--- sys/dev/usb/uhidev.c
+++ sys/dev/usb/uhidev.c
@@ -273,11 +273,13 @@ uhidev_attach(struct device *parent, struct device *self,
void *aux)
hid_report_size(desc, size, hid_feature, repid) == 0)
continue;
+ /* Could already be assigned by uhidev_set_report_dev(). */
+ if (sc->sc_subdevs[repid] != NULL)
+ continue;
+
uha.reportid = repid;
dev = config_found_sm(self, &uha, uhidevprint, uhidevsubmatch);
- /* Could already be assigned by uhidev_set_report_dev(). */
- if (sc->sc_subdevs[repid] == NULL)
- sc->sc_subdevs[repid] = (struct uhidev *)dev;
+ sc->sc_subdevs[repid] = (struct uhidev *)dev;
}
}
@@ -1011,3 +1013,13 @@ uhidev_set_report_dev(struct uhidev_softc *sc, struct
uhidev *dev, int repid)
sc->sc_subdevs[repid] = dev;
return 0;
}
+
+int
+uhidev_unset_report_dev(struct uhidev_softc *sc, int repid)
+{
+ if (repid >= sc->sc_nrepid)
+ return EINVAL;
+
+ sc->sc_subdevs[repid] = NULL;
+ return 0;
+}
diff --git sys/dev/usb/uhidev.h sys/dev/usb/uhidev.h
index 5caba3d8304..85a44bf84a5 100644
--- sys/dev/usb/uhidev.h
+++ sys/dev/usb/uhidev.h
@@ -96,3 +96,4 @@ int uhidev_get_report_async(struct uhidev_softc *, int, int,
void *, int,
void *, void (*)(void *, int, void *, int));
usbd_status uhidev_write(struct uhidev_softc *, void *, int);
int uhidev_set_report_dev(struct uhidev_softc *, struct uhidev *, int);
+int uhidev_unset_report_dev(struct uhidev_softc *, int);
diff --git sys/dev/usb/uhidpp.c sys/dev/usb/uhidpp.c
index b041d86fecd..fe6550c78dd 100644
--- sys/dev/usb/uhidpp.c
+++ sys/dev/usb/uhidpp.c
@@ -281,7 +281,8 @@ uhidpp_match(struct device *parent, void *match, void *aux)
void *desc;
int descsiz, siz;
- if (uha->reportid != UHIDEV_CLAIM_ALLREPORTID)
+ if (uha->reportid != HIDPP_REPORT_ID_SHORT &&
+ uha->reportid != HIDPP_REPORT_ID_LONG)
return UMATCH_NONE;
if (usb_lookup(uhidpp_devs,
@@ -325,7 +326,7 @@ uhidpp_attach(struct device *parent, struct device *self,
void *aux)
error = uhidev_open(&sc->sc_hdev);
if (error) {
- printf(" error %d\n", error);
+ printf(" open error %d\n", error);
return;
}
@@ -338,10 +339,18 @@ uhidpp_attach(struct device *parent, struct device *self,
void *aux)
* in order to receive responses. Necessary as uhidev by default
* performs the wiring after the attach routine has returned.
*/
- uhidev_set_report_dev(sc->sc_hdev.sc_parent, &sc->sc_hdev,
+ error = uhidev_set_report_dev(sc->sc_hdev.sc_parent, &sc->sc_hdev,
HIDPP_REPORT_ID_SHORT);
- uhidev_set_report_dev(sc->sc_hdev.sc_parent, &sc->sc_hdev,
+ if (error) {
+ printf(" short report error %d\n", error);
+ return;
+ }
+ error = uhidev_set_report_dev(sc->sc_hdev.sc_parent, &sc->sc_hdev,
HIDPP_REPORT_ID_LONG);
+ if (error) {
+ printf(" long report error %d\n", error);
+ return;
+ }
/* Probe paired devices. */
for (i = 0; i < UHIDPP_NDEVICES; i++) {
@@ -408,6 +417,14 @@ uhidpp_detach(struct device *self, int flags)
sensor_detach(&sc->sc_sensdev,
&dev->d_battery.b_sens[j]);
}
+ /*
+ * Since this driver has multiple device handlers attached, remove all
+ * of them preventing the uhidev parent from calling this detach routine
+ * more than once.
+ */
+ uhidev_unset_report_dev(sc->sc_hdev.sc_parent, HIDPP_REPORT_ID_SHORT);
+ uhidev_unset_report_dev(sc->sc_hdev.sc_parent, HIDPP_REPORT_ID_LONG);
+
uhidev_close(&sc->sc_hdev);
return 0;
