On Fri, Mar 05, 2021 at 08:58:44AM -0600, joshua stein wrote:
> uhidev allows a child device to claim all reports by calling *_match
> functions with the report id set to UHIDEV_CLAIM_ALLREPORTID.
>
> umt needs this because it has to access 3 reports which has worked
> okay up until now because devices with umt and a ukbd have usually
> presented them on separate uhidev devices. However, on a new
> Surface Type Cover device, someone reported that these devices are
> on the same uhidev meaning if umt attaches, the ukbd device can't.
>
> To remedy this, probe devices with UHIDEV_CLAIM_MULTIPLE_REPORTID
> instead and include an array in the uhidev_attach_arg the size of
> the available reports. Devices wanting to claim multiple reports
> just set the indexes in the array to 1 that it wants to claim and
> uhidev will reserve those, but still probe other devices with each
> specific unclaimed id.
>
> umt is modified to do its report finding in its match function so it
> can claim just the specific reports it needs.
I ran into the same problem while developing uhidpp(4) and added the
uhidev_{set,unset}_report_dev() API. That driver must be able to talk
the device already in the attach routine, meaning that
`sc->sc_subdevs[repid]' must be assigned. So it cannot make fully use of
the claim multiple reportid feature unfortunately.
However, this looks like a sane approach. Tested on my laptop and
everything still attaches just like before. Just one nit below. Either
way ok anton@
>
>
> Index: dev/usb/uhidev.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/uhidev.c,v
> retrieving revision 1.89
> diff -u -p -u -p -r1.89 uhidev.c
> --- dev/usb/uhidev.c 15 Feb 2021 11:26:00 -0000 1.89
> +++ dev/usb/uhidev.c 5 Mar 2021 14:51:09 -0000
> @@ -250,21 +250,27 @@ uhidev_attach(struct device *parent, str
>
> uha.uaa = uaa;
> uha.parent = sc;
> - uha.reportid = UHIDEV_CLAIM_ALLREPORTID;
> + uha.reportid = UHIDEV_CLAIM_MULTIPLE_REPORTID;
> + uha.nreports = nrepid;
> + uha.claimed = malloc(nrepid, M_TEMP, M_WAITOK|M_ZERO);
>
> - /* Look for a driver claiming all report IDs first. */
> + /* Look for a driver claiming multiple report IDs first. */
> dev = config_found_sm(self, &uha, NULL, uhidevsubmatch);
> if (dev != NULL) {
> for (repid = 0; repid < nrepid; repid++) {
> /*
> * Could already be assigned by uhidev_set_report_dev().
> */
> - if (sc->sc_subdevs[repid] == NULL)
> + if (sc->sc_subdevs[repid] != NULL)
> + continue;
> +
> + if (uha.claimed[repid])
> sc->sc_subdevs[repid] = (struct uhidev *)dev;
> }
> - return;
> }
>
> + free(uha.claimed, M_TEMP, nrepid);
I would put a `uha.claimed = NULL` here.
