Date: Fri, 11 Jun 2004 17:03:05 -0400 From: Anish Mistry <mistry.7@osu.edu> To: freebsd-hardware@freebsd.org Cc: Markus Wild <mwild@vianetworks.ch> Subject: Re: Fix for Logitech DiNovo cordless mouse [PATCH] Message-ID: <200406111703.14772.mistry.7@osu.edu> In-Reply-To: <200406101318.24419.jhb@FreeBSD.org> References: <200406091838.i59Icugc063061@smsgw.vianetworks.ch> <200406101215.24347.mistry.7@osu.edu> <200406101318.24419.jhb@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--Boundary-00=_J4hyAtqhgAiHEVq
Content-Type: Text/Plain;
charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: inline
=2D----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On Thursday 10 June 2004 01:18 pm, John Baldwin wrote:
> On Thursday 10 June 2004 12:15 pm, Anish Mistry wrote:
> > On Thursday 10 June 2004 11:22 am, John Baldwin wrote:
> > > On Wednesday 09 June 2004 08:16 pm, Anish Mistry wrote:
> > > > On Wednesday 09 June 2004 02:38 pm, Markus Wild wrote:
> > > > > Since yesterday I'm a happy owner of a Logitech dinovo
> > > > > cordless USB keyboard/mouse. The keyboard works fine, however
> > > > > the mouse didn't move a bit. I saw that other people had similar
> > > > > "luck", so I enabled a bit of debugging. This is with
> > > > > FreeBSD-current, btw.
> > > > >
> > > > > The result of the quest was: the hid.c:hid_report_size() function
> > > > > returns a bogus iid value:
> > > > >
> > > > > Jun 9 19:37:06 mothra kernel: ums0: Logitech USB Receiver, rev
> > > > > 1.10/24.04, addr 3, iclass 3/1
> > > > > Jun 9 19:37:06 mothra kernel: ums_attach: bLength=3D7
> > > > > bDescriptorType=3D5 bEndpoint Address=3D2-in bmAttributes=3D3
> > > > > wMaxPacketSize=3D8 bInterval=3D10 Jun 9 19:37:06 mothra kernel: =
ums0:
> > > > > 7 buttons and Z dir.
> > > > > Jun 9 19:37:06 mothra kernel: ums_attach: sc=3D0xc23a1800
> > > > > Jun 9 19:37:06 mothra kernel: ums_attach: X 8/12
> > > > > Jun 9 19:37:06 mothra kernel: ums_attach: Y 20/12
> > > > > Jun 9 19:37:06 mothra kernel: ums_attach: Z 32/8
> > > > > Jun 9 19:37:06 mothra kernel: ums_attach: B1 0/1
> > > > > Jun 9 19:37:06 mothra kernel: ums_attach: B2 1/1
> > > > > Jun 9 19:37:06 mothra kernel: ums_attach: B3 2/1
> > > > > Jun 9 19:37:06 mothra kernel: ums_attach: B4 3/1
> > > > > Jun 9 19:37:06 mothra kernel: ums_attach: B5 4/1
> > > > > Jun 9 19:37:06 mothra kernel: ums_attach: B6 5/1
> > > > > Jun 9 19:37:06 mothra kernel: ums_attach: B7 6/1
> > > > > Jun 9 19:37:06 mothra kernel: ums_attach: size=3D36, id=3D17
> > > > >
> > > > > Since actual interrupt reports are issed with id 2:
> > > > > Jun 9 18:42:10 mothra kernel: ums_intr: sc=3D0xc23a1800 status=
=3D0
> > > > > Jun 9 18:42:10 mothra kernel: ums_intr: data =3D 02 00 fa
> > > > >
> > > > > So I added a bit of debugging to the id setting for-loop. It
> > > > > looks like the ID cycles thru the following values at attach()
> > > > > time: Jun 9 19:40:57 mothra kernel: hid_report_size: 00 -> 02
> > > > > Jun 9 19:40:57 mothra kernel: hid_report_size: 02 -> 03
> > > > > Jun 9 19:40:57 mothra kernel: hid_report_size: 03 -> 04
> > > > > Jun 9 19:40:57 mothra kernel: hid_report_size: 04 -> 10
> > > > > Jun 9 19:40:57 mothra kernel: hid_report_size: 10 -> 11
> > > > > (numbers are hex here)
> > > > >
> > > > > With this, my current fix is simple: only set id if it's not
> > > > > set already:
> > > > > diff -u -r1.23 hid.c
> > > > > --- hid.c 24 Aug 2003 17:55:54 -0000 1.23
> > > > > +++ hid.c 9 Jun 2004 18:34:23 -0000
> > > > > @@ -374,9 +374,10 @@
> > > > > int size, id;
> > > > >
> > > > > id =3D 0;
> > > > > + bzero (&h, sizeof (h));
> > > > > for (d =3D hid_start_parse(buf, len, 1<<k); hid_get_item(=
d,
> > > > > &h); ) - if (h.report_ID !=3D 0)
> > > > > - id =3D h.report_ID;
> > > > > + if (h.report_ID !=3D 0 && !id)
> > > > > + id =3D h.report_ID;
> > > > > hid_end_parse(d);
> > > > > size =3D h.loc.pos;
> > > > > if (id !=3D 0) {
> > > >
> > > > I've attached at big patch that should fix the problem as well as a
> > > > bunch of updates from the NetBSD sources. This is a patch against
> > > > -CURRENT, so you may have to massage it a bit if you are on -STABLE=
=2E=20
> > > > I won't have an offending device to test for at least a week so let
> > > > me know of any problems.
> > >
> > > - M_ZERO is preferred to malloc() + memset(), so please don't make th=
at
> > > change.
> > > - Lots of the changes add style bugs by adding spaces to the end of
> > > lines. Please remove any trailing whitespace from your files.
> > >
> > > Other than that the patch looks cool.
> >
> > Ok, I think the style changes are done and I've attached the updated
> > patch. I've got a question though, in style it says the enums should be
> > in all caps, but the previous code wasn't and the NetBSD import code
> > wasn't. Should I change it to what style says, or is it better to stay =
in
> > sync with NetBSD?
>
> Just keep it the way NetBSD has it. I also missed another minor nit
> (sorry!) in that you want to leave the earlier sys/cdefs.h and FBSDID()
> lines as that is a FreeBSD-specific change. Other than that it looks goo=
d.
It should be fixed now. I'm going to file a PR if you say it is fine.
=2D --=20
Anish Mistry
=2D----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.4 (FreeBSD)
iD8DBQFAyh4RxqA5ziudZT0RAhUvAJ9gtKiiKF2j8mc8t1gmI7dKpTcA1QCgjxA5
=46MyZgqeQkxBLCudtxRCZdS8=3D
=3DD+G/
=2D----END PGP SIGNATURE-----
--Boundary-00=_J4hyAtqhgAiHEVq
Content-Type: text/x-diff;
charset="iso-8859-1";
name="hid-updates3.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename="hid-updates3.patch"
diff -ruN sys/dev/usb.orig/hid.c sys/dev/usb/hid.c
--- sys/dev/usb.orig/hid.c Wed Jun 9 20:10:55 2004
+++ sys/dev/usb/hid.c Thu Jun 10 16:03:03 2004
@@ -1,5 +1,5 @@
-/* $NetBSD: hid.c,v 1.17 2001/11/13 06:24:53 lukem Exp $ */
-
+/* $NetBSD: hid.c,v 1.21 2002/01/02 11:10:50 augustss Exp $ */
+/* $FreeBSD: src/sys/dev/usb/usbhid.h,v 1.13 2002/01/02 20:16:53 joe Exp $ */
#include <sys/cdefs.h>
__FBSDID("$FreeBSD: src/sys/dev/usb/hid.c,v 1.23 2003/08/24 17:55:54 obrien Exp $");
@@ -52,10 +52,10 @@
#include <dev/usb/hid.h>
-#ifdef USB_DEBUG
-#define DPRINTF(x) if (usbdebug) logprintf x
-#define DPRINTFN(n,x) if (usbdebug>(n)) logprintf x
-extern int usbdebug;
+#ifdef UHIDEV_DEBUG
+#define DPRINTF(x) if (uhidevdebug) logprintf x
+#define DPRINTFN(n,x) if (uhidevdebug>(n)) logprintf x
+extern int uhidevdebug;
#else
#define DPRINTF(x)
#define DPRINTFN(n,x)
@@ -63,7 +63,7 @@
Static void hid_clear_local(struct hid_item *);
-#define MAXUSAGE 100
+#define MAXUSAGE 256
struct hid_data {
u_char *start;
u_char *end;
@@ -74,13 +74,14 @@
int minset;
int multi;
int multimax;
- int kindset;
+ enum hid_kind kind;
};
Static void
hid_clear_local(struct hid_item *c)
{
+ DPRINTFN(5,("hid_clear_local\n"));
c->usage = 0;
c->usage_minimum = 0;
c->usage_maximum = 0;
@@ -94,14 +95,14 @@
}
struct hid_data *
-hid_start_parse(void *d, int len, int kindset)
+hid_start_parse(void *d, int len, enum hid_kind kind)
{
struct hid_data *s;
s = malloc(sizeof *s, M_TEMP, M_WAITOK|M_ZERO);
s->start = s->p = d;
s->end = (char *)d + len;
- s->kindset = kindset;
+ s->kind = kind;
return (s);
}
@@ -128,15 +129,19 @@
u_char *p;
struct hid_item *hi;
int i;
+ enum hid_kind retkind;
top:
+ DPRINTFN(5,("hid_get_item: multi=%d multimax=%d\n",
+ s->multi, s->multimax));
if (s->multimax != 0) {
if (s->multi < s->multimax) {
c->usage = s->usages[min(s->multi, s->nu-1)];
s->multi++;
*h = *c;
c->loc.pos += c->loc.size;
- h->next = 0;
+ h->next = NULL;
+ DPRINTFN(5,("return multi\n"));
return (1);
} else {
c->loc.count = s->multimax;
@@ -174,12 +179,12 @@
dval = 0;
break;
case 1:
- dval = (int8_t)*data++;
+ dval = /*(int8_t)*/ *data++;
break;
case 2:
dval = *data++;
dval |= *data++ << 8;
- dval = (int16_t)dval;
+ dval = /*(int16_t)*/ dval;
break;
case 4:
dval = *data++;
@@ -191,16 +196,23 @@
printf("BAD LENGTH %d\n", bSize);
continue;
}
-
+
+ DPRINTFN(5,("hid_get_item: bType=%d bTag=%d dval=%d\n",
+ bType, bTag, dval));
switch (bType) {
case 0: /* Main */
switch (bTag) {
case 8: /* Input */
- if (!(s->kindset & (1 << hid_input)))
+ retkind = hid_input;
+ ret:
+ if (s->kind != retkind) {
+ s->minset = 0;
+ s->nu = 0;
+ hid_clear_local(c);
continue;
- c->kind = hid_input;
+ }
+ c->kind = retkind;
c->flags = dval;
- ret:
if (c->flags & HIO_VARIABLE) {
s->multimax = c->loc.count;
s->multi = 0;
@@ -217,19 +229,18 @@
}
goto top;
} else {
+ c->usage = c->_usage_page; /* XXX */
*h = *c;
- h->next = 0;
+ h->next = NULL;
c->loc.pos +=
- c->loc.size * c->loc.count;
- hid_clear_local(c);
+ c->loc.size * c->loc.count;
s->minset = 0;
+ s->nu = 0;
+ hid_clear_local(c);
return (1);
}
case 9: /* Output */
- if (!(s->kindset & (1 << hid_output)))
- continue;
- c->kind = hid_output;
- c->flags = dval;
+ retkind = hid_output;
goto ret;
case 10: /* Collection */
c->kind = hid_collection;
@@ -240,16 +251,12 @@
s->nu = 0;
return (1);
case 11: /* Feature */
- if (!(s->kindset & (1 << hid_feature)))
- continue;
- c->kind = hid_feature;
- c->flags = dval;
+ retkind = hid_feature;
goto ret;
case 12: /* End collection */
c->kind = hid_endcollection;
c->collevel--;
*h = *c;
- hid_clear_local(c);
s->nu = 0;
return (1);
default:
@@ -285,6 +292,7 @@
break;
case 8:
c->report_ID = dval;
+ c->loc.pos = 0;
break;
case 9:
c->loc.count = dval;
@@ -367,35 +375,51 @@
}
int
-hid_report_size(void *buf, int len, enum hid_kind k, u_int8_t *idp)
+hid_report_size(void *buf, int len, enum hid_kind k, u_int8_t id)
{
struct hid_data *d;
struct hid_item h;
- int size, id;
+ int lo, hi;
- id = 0;
- for (d = hid_start_parse(buf, len, 1<<k); hid_get_item(d, &h); )
- if (h.report_ID != 0)
- id = h.report_ID;
+ h.report_ID = 0;
+ lo = hi = -1;
+ DPRINTFN(2,("hid_report_size: kind=%d id=%d\n", k, id));
+ for (d = hid_start_parse(buf, len, k); hid_get_item(d, &h); ) {
+ DPRINTFN(2,("hid_report_size: item kind=%d id=%d pos=%d "
+ "size=%d count=%d\n",
+ h.kind, h.report_ID, h.loc.pos, h.loc.size,
+ h.loc.count));
+ if (h.report_ID == id && h.kind == k) {
+ if (lo < 0) {
+ lo = h.loc.pos;
+#ifdef DIAGNOSTIC
+ if (lo != 0) {
+ printf("hid_report_size: lo != 0\n");
+ }
+#endif
+ }
+ hi = h.loc.pos + h.loc.size * h.loc.count;
+ DPRINTFN(2,("hid_report_size: lo=%d hi=%d\n", lo, hi));
+ }
+ }
hid_end_parse(d);
- size = h.loc.pos;
- if (id != 0) {
- size += 8;
- *idp = id; /* XXX wrong */
- } else
- *idp = 0;
- return ((size + 7) / 8);
+ return ((hi - lo + 7) / 8);
}
int
-hid_locate(void *desc, int size, u_int32_t u, enum hid_kind k,
+hid_locate(void *desc, int size, u_int32_t u, u_int8_t id, enum hid_kind k,
struct hid_location *loc, u_int32_t *flags)
{
struct hid_data *d;
struct hid_item h;
- for (d = hid_start_parse(desc, size, 1<<k); hid_get_item(d, &h); ) {
- if (h.kind == k && !(h.flags & HIO_CONST) && h.usage == u) {
+ h.report_ID = 0;
+ DPRINTFN(5,("hid_locate: enter usage=0x%x kind=%d id=%d\n", u, k, id));
+ for (d = hid_start_parse(desc, size, k); hid_get_item(d, &h); ) {
+ DPRINTFN(5,("hid_locate: usage=0x%x kind=%d id=%d flags=0x%x\n",
+ h.usage, h.kind, h.report_ID, h.flags));
+ if (h.kind == k && !(h.flags & HIO_CONST) &&
+ h.usage == u && h.report_ID == id) {
if (loc != NULL)
*loc = h.loc;
if (flags != NULL)
@@ -437,19 +461,33 @@
}
int
-hid_is_collection(void *desc, int size, u_int32_t usage)
+hid_is_collection(void *desc, int size, u_int8_t id, u_int32_t usage)
{
struct hid_data *hd;
struct hid_item hi;
- int err;
+ u_int32_t coll_usage = ~0;
- hd = hid_start_parse(desc, size, hid_input);
+ hd = hid_start_parse(desc, size, hid_none);
if (hd == NULL)
return (0);
- err = hid_get_item(hd, &hi) &&
- hi.kind == hid_collection &&
- hi.usage == usage;
+ DPRINTFN(2,("hid_is_collection: id=%d usage=0x%x\n", id, usage));
+ while (hid_get_item(hd, &hi)) {
+ DPRINTFN(2,("hid_is_collection: kind=%d id=%d usage=0x%x"
+ "(0x%x)\n",
+ hi.kind, hi.report_ID, hi.usage, coll_usage));
+ if (hi.kind == hid_collection &&
+ hi.collection == HCOLL_APPLICATION)
+ coll_usage = hi.usage;
+ if (hi.kind == hid_endcollection &&
+ coll_usage == usage &&
+ hi.report_ID == id) {
+ DPRINTFN(2,("hid_is_collection: found\n"));
+ hid_end_parse(hd);
+ return (1);
+ }
+ }
+ DPRINTFN(2,("hid_is_collection: not found\n"));
hid_end_parse(hd);
- return (err);
+ return (0);
}
diff -ruN sys/dev/usb.orig/hid.h sys/dev/usb/hid.h
--- sys/dev/usb.orig/hid.h Wed Jun 9 20:10:55 2004
+++ sys/dev/usb/hid.h Thu Jun 10 11:56:34 2004
@@ -1,4 +1,4 @@
-/* $NetBSD: hid.h,v 1.6 2000/06/01 14:28:57 augustss Exp $ */
+/* $NetBSD: hid.h,v 1.7 2001/12/28 17:32:36 augustss Exp $ */
/* $FreeBSD: src/sys/dev/usb/hid.h,v 1.12 2003/07/04 01:50:38 jmg Exp $ */
/*
@@ -39,7 +39,12 @@
*/
enum hid_kind {
- hid_input, hid_output, hid_feature, hid_collection, hid_endcollection
+ hid_input,
+ hid_output,
+ hid_feature,
+ hid_collection,
+ hid_endcollection,
+ hid_none
};
struct hid_location {
@@ -80,12 +85,11 @@
struct hid_item *next;
};
-struct hid_data *hid_start_parse(void *d, int len, int kindset);
+struct hid_data *hid_start_parse(void *d, int len, enum hid_kind kind);
void hid_end_parse(struct hid_data *s);
int hid_get_item(struct hid_data *s, struct hid_item *h);
-int hid_report_size(void *buf, int len, enum hid_kind k, u_int8_t *id);
-int hid_locate(void *desc, int size, u_int32_t usage,
- enum hid_kind kind, struct hid_location *loc,
- u_int32_t *flags);
+int hid_report_size(void *buf, int len, enum hid_kind k, u_int8_t id);
+int hid_locate(void *desc, int size, u_int32_t usage, u_int8_t id,
+ enum hid_kind kind, struct hid_location *loc, u_int32_t *flags);
u_long hid_get_data(u_char *buf, struct hid_location *loc);
-int hid_is_collection(void *desc, int size, u_int32_t usage);
+int hid_is_collection(void *desc, int size, u_int8_t id, u_int32_t usage);
diff -ruN sys/dev/usb.orig/uhid.c sys/dev/usb/uhid.c
--- sys/dev/usb.orig/uhid.c Wed Jun 9 20:10:55 2004
+++ sys/dev/usb/uhid.c Wed Jun 9 20:35:14 2004
@@ -266,9 +266,9 @@
(void)usbd_set_idle(iface, 0, 0);
- sc->sc_isize = hid_report_size(desc, size, hid_input, &sc->sc_iid);
- sc->sc_osize = hid_report_size(desc, size, hid_output, &sc->sc_oid);
- sc->sc_fsize = hid_report_size(desc, size, hid_feature, &sc->sc_fid);
+ sc->sc_isize = hid_report_size(desc, size, hid_input, sc->sc_iid);
+ sc->sc_osize = hid_report_size(desc, size, hid_output, sc->sc_oid);
+ sc->sc_fsize = hid_report_size(desc, size, hid_feature, sc->sc_fid);
sc->sc_repdesc = desc;
sc->sc_repdesc_size = size;
diff -ruN sys/dev/usb.orig/usbhid.h sys/dev/usb/usbhid.h
--- sys/dev/usb.orig/usbhid.h Wed Jun 9 20:10:53 2004
+++ sys/dev/usb/usbhid.h Wed Jun 9 20:07:13 2004
@@ -1,4 +1,4 @@
-/* $NetBSD: usbhid.h,v 1.9 2000/09/03 19:09:14 augustss Exp $ */
+/* $NetBSD: usbhid.h,v 1.9.4.1 2002/01/10 19:59:11 thorpej Exp $ */
/* $FreeBSD: src/sys/dev/usb/usbhid.h,v 1.13 2002/01/02 20:16:53 joe Exp $ */
/*
@@ -164,11 +164,24 @@
#define HUD_ERASER 0x0045
#define HUD_TABLET_PICK 0x0046
-#define HID_USAGE2(p,u) (((p) << 16) | u)
+/* Usages LEDs */
+#define HUD_LED_NUM_LOCK 0x0001
+#define HUD_LED_CAPS_LOCK 0x0002
+#define HUD_LED_SCROLL_LOCK 0x0003
+#define HUD_LED_COMPOSE 0x0004
+#define HUD_LED_KANA 0x0005
+
+#define HID_USAGE2(p, u) (((p) << 16) | u)
+#define HID_GET_USAGE(u) ((u) & 0xffff)
+#define HID_GET_USAGE_PAGE(u) (((u) >> 16) & 0xffff)
#define UHID_INPUT_REPORT 0x01
#define UHID_OUTPUT_REPORT 0x02
#define UHID_FEATURE_REPORT 0x03
+
+#define HCOLL_PHYSICAL 0
+#define HCOLL_APPLICATION 1
+#define HCOLL_LOGICAL 2
/* Bits in the input/output/feature items */
#define HIO_CONST 0x001
--Boundary-00=_J4hyAtqhgAiHEVq--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200406111703.14772.mistry.7>