On Fri, Aug 27, 2021 at 07:05:06AM +0200, Anton Lindqvist wrote:
> Hi,
> An interrupt report contains the state of all items (Input, Output and
> Feature) from the corresponding descriptor report for a given report ID.
> The ordering of the items is identical in both the descriptor and
> interrupt report. As the interrupt report can cover more than Consumer
> Control related key presses, ucc must be more careful while examining
> the interrupt report in order to not confuse other items as key presses.
>
> While parsing the descriptor report, take note of the bits that
> represents Consumer Control key presses and use it to slice the
> interrupt report.
>
> Testing would be much appreciated. Note that UCC_DEBUG is intentionally
> enabled in the diff below. Please send me your dmesg after pressing at
> least one recognized key.
Updated diff fixing handling constant padding input.
Index: dev/usb/ucc.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/ucc.c,v
retrieving revision 1.13
diff -u -p -r1.13 ucc.c
--- dev/usb/ucc.c 26 Aug 2021 10:32:35 -0000 1.13
+++ dev/usb/ucc.c 27 Aug 2021 05:34:34 -0000
@@ -31,7 +31,7 @@
#include <dev/wscons/wsksymdef.h>
#include <dev/wscons/wsksymvar.h>
-/* #define UCC_DEBUG */
+#define UCC_DEBUG
#ifdef UCC_DEBUG
#define DPRINTF(x...) do { if (ucc_debug) printf(x); } while (0)
void ucc_dump(const char *, u_char *, u_int);
@@ -58,6 +58,17 @@ struct ucc_softc {
int sc_isarray;
int sc_mode;
+ /*
+ * Slice of the interrupt buffer which represents a pressed key.
+ * See section 8 (Report Protocol) of the HID specification v1.11.
+ */
+ struct {
+ uint8_t *i_buf;
+ uint32_t i_bufsiz;
+ uint32_t i_off; /* offset in bits */
+ uint32_t i_len; /* length in bits */
+ } sc_input;
+
/* Last pressed key. */
union {
int sc_last_translate;
@@ -92,6 +103,7 @@ int ucc_add_key(struct ucc_softc *, int3
int ucc_bit_to_sym(struct ucc_softc *, u_int, const struct ucc_keysym **);
int ucc_usage_to_sym(int32_t, const struct ucc_keysym **);
int ucc_bits_to_usage(u_char *, u_int, int32_t *);
+int ucc_intr_slice(struct ucc_softc *, u_char *, u_char *, int *);
void ucc_input(struct ucc_softc *, u_int, int);
void ucc_rawinput(struct ucc_softc *, u_char, int);
int ucc_setbits(u_char *, int, u_int *);
@@ -194,6 +206,7 @@ ucc_detach(struct device *self, int flag
if (sc->sc_wskbddev != NULL)
error = config_detach(sc->sc_wskbddev, flags);
uhidev_close(&sc->sc_hdev);
+ free(sc->sc_input.i_buf, M_USBDEV, sc->sc_input.i_bufsiz);
free(sc->sc_map, M_USBDEV, sc->sc_mapsiz * sizeof(*sc->sc_map));
free(sc->sc_raw, M_USBDEV, sc->sc_rawsiz * sizeof(*sc->sc_raw));
return error;
@@ -204,13 +217,30 @@ ucc_intr(struct uhidev *addr, void *data
{
struct ucc_softc *sc = (struct ucc_softc *)addr;
const struct ucc_keysym *us;
+ u_char *buf = sc->sc_input.i_buf;
int raw = sc->sc_mode == WSKBD_RAW;
+ int error;
u_int bit = 0;
u_char c = 0;
ucc_dump(__func__, data, len);
- if (ucc_setbits(data, len, &bit)) {
+ if (len > sc->sc_input.i_bufsiz) {
+ DPRINTF("%s: too much data: len %d, bufsiz %d\n", __func__,
+ len, sc->sc_input.i_bufsiz);
+ return;
+ }
+
+ error = ucc_intr_slice(sc, data, buf, &len);
+ if (error) {
+ DPRINTF("%s: slice failure: error %d\n", __func__, error);
+ return;
+ }
+
+ /* Dump the buffer again after slicing. */
+ ucc_dump(__func__, buf, len);
+
+ if (ucc_setbits(buf, len, &bit)) {
/* All zeroes, assume key up event. */
if (raw) {
if (sc->sc_last_raw != 0) {
@@ -227,7 +257,7 @@ ucc_intr(struct uhidev *addr, void *data
} else if (sc->sc_isarray) {
int32_t usage;
- if (ucc_bits_to_usage(data, len, &usage) ||
+ if (ucc_bits_to_usage(buf, len, &usage) ||
ucc_usage_to_sym(usage, &us))
goto unknown;
bit = us->us_usage;
@@ -338,10 +368,12 @@ ucc_ioctl(void *v, u_long cmd, caddr_t d
int
ucc_hid_parse(struct ucc_softc *sc, void *desc, int descsiz)
{
+ enum { OFFSET, LENGTH } istate = OFFSET;
struct hid_item hi;
struct hid_data *hd;
- int nsyms = nitems(ucc_keysyms);
int error = 0;
+ int nsyms = nitems(ucc_keysyms);
+ int repid = sc->sc_hdev.sc_report_id;
int isize;
/*
@@ -353,6 +385,10 @@ ucc_hid_parse(struct ucc_softc *sc, void
if (isize == 0)
return ENXIO;
+ /* Allocate buffer used to slice interrupt data. */
+ sc->sc_input.i_bufsiz = sc->sc_hdev.sc_isize;
+ sc->sc_input.i_buf = malloc(sc->sc_input.i_bufsiz, M_USBDEV, M_WAITOK);
+
/*
* Create mapping between each input bit and the corresponding usage,
* used in translating mode. Two entries are needed per bit in order
@@ -375,9 +411,28 @@ ucc_hid_parse(struct ucc_softc *sc, void
while (hid_get_item(hd, &hi)) {
u_int bit;
- if (hi.kind != hid_input ||
- HID_GET_USAGE_PAGE(hi.usage) != HUP_CONSUMER)
+ if (hi.report_ID != repid || hi.kind != hid_input)
+ continue;
+
+ if (HID_GET_USAGE_PAGE(hi.usage) != HUP_CONSUMER) {
+ uint32_t len = hi.loc.size * hi.loc.count;
+
+ switch (istate) {
+ case OFFSET:
+ sc->sc_input.i_off = hi.loc.pos + len;
+ break;
+ case LENGTH:
+ /* Constant padding. */
+ if (hi.flags & HIO_CONST)
+ sc->sc_input.i_len += len;
+ break;
+ }
continue;
+ }
+
+ /* Signal that the input offset is reached. */
+ istate = LENGTH;
+ sc->sc_input.i_len += hi.loc.size * hi.loc.count;
/*
* The usages could be expressed as an array instead of
@@ -395,6 +450,9 @@ ucc_hid_parse(struct ucc_softc *sc, void
}
hid_end_parse(hd);
+ DPRINTF("%s: input: off %d, len %d\n", __func__,
+ sc->sc_input.i_off, sc->sc_input.i_len);
+
return error;
}
@@ -482,25 +540,49 @@ int
ucc_bits_to_usage(u_char *buf, u_int buflen, int32_t *usage)
{
int32_t x = 0;
- int maxlen = sizeof(*usage);
int i;
- if (buflen == 0)
+ if (buflen == 0 || buflen > sizeof(*usage))
return 1;
- /*
- * XXX should only look at the bits given by the report size and report
- * count associated with the Consumer usage page.
- */
- if (buflen > maxlen)
- buflen = maxlen;
-
for (i = buflen - 1; i >= 0; i--) {
x |= buf[i];
if (i > 0)
x <<= 8;
}
*usage = x;
+ return 0;
+}
+
+int
+ucc_intr_slice(struct ucc_softc *sc, u_char *src, u_char *dst, int *len)
+{
+ int ilen = sc->sc_input.i_len;
+ int ioff = sc->sc_input.i_off;
+ int maxlen = *len;
+ int di, si;
+
+ if (maxlen == 0)
+ return 1;
+
+ memset(dst, 0, maxlen);
+ si = ioff;
+ di = 0;
+ for (; ilen > 0; ilen--) {
+ int db, sb;
+
+ sb = si / 8;
+ db = di / 8;
+ if (sb >= maxlen || db >= maxlen)
+ return 1;
+
+ if (src[sb] & (1 << (si % 8)))
+ dst[db] |= 1 << (di % 8);
+ si++;
+ di++;
+ }
+
+ *len = (sc->sc_input.i_len + 7) / 8;
return 0;
}
