Re: HID parser
On 05/05/14(Mon) 04:37, Daniel Bolgheroni wrote:
On Sun, May 04, 2014 at 06:11:41PM +0200, Martin Pieuchot wrote:
In December 2012 a user reported on misc@ that the Noppoo Mini Choc
84 USB keyboard does not work on OpenBSD [0]. More recently, mcbride@
and yasuoka@ contacted me because they have a mouse that is not properly
recognized. Both issues are related to our HID descriptor parser.
The chinese Rapoo V7 keyboard now works with this patch. At the time, I
asked for something in:
http://marc.info/?l=openbsd-techm=135941037714276w=2
Thanks to all the testers. I'm happy to know that this diff fixes a fair
amount of keyboards and mouses.
Here's a slightly improved version that does not introduce a regression
for big report ID, that are present in some UPS for example.
I plan to commit this version in the following diff, but more tests are
always welcome ;)
Index: hid.c
===
RCS file: /cvs/src/sys/dev/usb/hid.c,v
retrieving revision 1.25
diff -u -p -r1.25 hid.c
--- hid.c 5 Aug 2012 16:07:11 - 1.25
+++ hid.c 10 May 2014 13:44:08 -
@@ -41,36 +41,47 @@
#include dev/usb/hid.h
-#ifdef UHIDEV_DEBUG
-#define DPRINTF(x) do { if (uhidevdebug) printf x; } while (0)
-#define DPRINTFN(n,x) do { if (uhidevdebug(n)) printf x; } while (0)
-extern int uhidevdebug;
+#ifdef USBHID_DEBUG
+#define DPRINTF(x...) do { printf(x); } while (0)
#else
-#define DPRINTF(x)
-#define DPRINTFN(n,x)
+#define DPRINTF(x...)
#endif
-void hid_clear_local(struct hid_item *);
+#defineMAXUSAGE 64
+#defineMAXPUSH 4
+#defineMAXID 16
+
+struct hid_pos_data {
+ int32_t rid;
+ uint32_t pos;
+};
-#define MAXUSAGE 256
struct hid_data {
- u_char *start;
- u_char *end;
- u_char *p;
- struct hid_item cur;
- int32_t usages[MAXUSAGE];
- int nu;
- int minset;
- int multi;
- int multimax;
+ const uint8_t *start;
+ const uint8_t *end;
+ const uint8_t *p;
+ struct hid_item cur[MAXPUSH];
+ struct hid_pos_data last_pos[MAXID];
+ int32_t usages_min[MAXUSAGE];
+ int32_t usages_max[MAXUSAGE];
+ int32_t usage_last; /* last seen usage */
+ uint32_t loc_size; /* last seen size */
+ uint32_t loc_count; /* last seen count */
enum hid_kind kind;
+ uint8_t pushlevel; /* current pushlevel */
+ uint8_t ncount; /* end usage item count */
+ uint8_t icount; /* current usage item count */
+ uint8_t nusage; /* end usages_min/max index */
+ uint8_t iusage; /* current usages_min/max index */
+ uint8_t ousage; /* current usages_min/max offset */
+ uint8_t susage; /* usage set flags */
};
-void
+static void
hid_clear_local(struct hid_item *c)
{
-
- DPRINTFN(5,(hid_clear_local\n));
+ c-loc.count = 0;
+ c-loc.size = 0;
c-usage = 0;
c-usage_minimum = 0;
c-usage_maximum = 0;
@@ -83,15 +94,62 @@ hid_clear_local(struct hid_item *c)
c-set_delimiter = 0;
}
+static void
+hid_switch_rid(struct hid_data *s, struct hid_item *c, int32_t nextid)
+{
+ uint8_t i;
+
+ if (c-report_ID == nextid)
+ return;
+
+ /* save current position for current rID */
+ if (c-report_ID == 0) {
+ i = 0;
+ } else {
+ for (i = 1; i != MAXID; i++) {
+ if (s-last_pos[i].rid == c-report_ID)
+ break;
+ if (s-last_pos[i].rid == 0)
+ break;
+ }
+ }
+ if (i != MAXID) {
+ s-last_pos[i].rid = c-report_ID;
+ s-last_pos[i].pos = c-loc.pos;
+ }
+
+ /* store next report ID */
+ c-report_ID = nextid;
+
+ /* lookup last position for next rID */
+ if (nextid == 0) {
+ i = 0;
+ } else {
+ for (i = 1; i != MAXID; i++) {
+ if (s-last_pos[i].rid == nextid)
+ break;
+ if (s-last_pos[i].rid == 0)
+ break;
+ }
+ }
+ if (i != MAXID) {
+ s-last_pos[i].rid = nextid;
+ c-loc.pos = s-last_pos[i].pos;
+ } else {
+ DPRINTF(Out of RID entries, position is set to zero!\n);
+ c-loc.pos = 0;
+ }
+}
+
struct hid_data *
-hid_start_parse(void *d, int len, enum hid_kind kind)
+hid_start_parse(const void *d, int len, enum hid_kind kind)
{
struct hid_data *s;
- s = malloc(sizeof *s, M_TEMP, M_WAITOK | M_ZERO);
+ s = malloc(sizeof(*s), M_TEMP, M_WAITOK | M_ZERO);
s-start = s-p = d;
- s-end = (char *)d + len;
+ s-end = ((const uint8_t *)d) + len;
s-kind = kind;
return
Re: HID parser
On Sun, May 04, 2014 at 06:11:41PM +0200, Martin Pieuchot wrote: In December 2012 a user reported on misc@ that the Noppoo Mini Choc 84 USB keyboard does not work on OpenBSD [0]. More recently, mcbride@ and yasuoka@ contacted me because they have a mouse that is not properly recognized. Both issues are related to our HID descriptor parser. The chinese Rapoo V7 keyboard now works with this patch. At the time, I asked for something in: http://marc.info/?l=openbsd-techm=135941037714276w=2 The dmesg follows: OpenBSD 5.5-current (GENERIC) #4: Mon May 5 04:08:16 BRT 2014 r...@white.my.domain:/usr/src/sys/arch/amd64/compile/GENERIC real mem = 1687670784 (1609MB) avail mem = 1634091008 (1558MB) mpath0 at root scsibus0 at mpath0: 256 targets mainbus0 at root bios0 at mainbus0: SMBIOS rev. 2.7 @ 0xec710 (45 entries) bios0: vendor American Megatrends Inc. version X401U.313 date 05/14/2013 bios0: ASUSTeK COMPUTER INC. X401U acpi0 at bios0: rev 2 acpi0: sleep states S0 S3 S4 S5 acpi0: tables DSDT FACP APIC FPDT MCFG ECDT HPET SSDT SSDT MSDM acpi0: wakeup devices SBAZ(S4) OHC1(S4) EHC1(S4) OHC2(S4) EHC2(S4) OHC3(S4) EHC3(S4) OHC4(S4) XHC0(S4) XHC1(S4) PE20(S4) PE21(S4) GLAN(S4) PE22(S4) PE23(S4) SLPB(S4) acpitimer0 at acpi0: 3579545 Hz, 32 bits acpimadt0 at acpi0 addr 0xfee0: PC-AT compat cpu0 at mainbus0: apid 0 (boot processor) cpu0: AMD C-70 APU with Radeon(tm) HD Graphics, 998.31 MHz cpu0: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,MWAIT,SSSE3,CX16,POPCNT,NXE,MMXX,FFXSR,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,IBS,SKINIT,ITSC cpu0: 32KB 64b/line 2-way I-cache, 32KB 64b/line 8-way D-cache, 512KB 64b/line 16-way L2 cache cpu0: 8 4MB entries fully associative cpu0: DTLB 40 4KB entries fully associative, 8 4MB entries fully associative cpu0: smt 0, core 0, package 0 mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges cpu0: apic clock running at 199MHz cpu at mainbus0: not configured ioapic0 at mainbus0: apid 3 pa 0xfec0, version 21, 24 pins acpimcfg0 at acpi0 addr 0xe000, bus 0-255 acpiec0 at acpi0 acpihpet0 at acpi0: 14318180 Hz acpiprt0 at acpi0: bus 0 (PCI0) acpiprt1 at acpi0: bus 3 (PE20) acpiprt2 at acpi0: bus 6 (PE21) acpiprt3 at acpi0: bus -1 (PE22) acpiprt4 at acpi0: bus 7 (PE23) acpiprt5 at acpi0: bus -1 (BR15) acpiprt6 at acpi0: bus -1 (PCE7) acpiprt7 at acpi0: bus -1 (PCE8) acpiprt8 at acpi0: bus 1 (BR14) acpicpu0 at acpi0: C2, PSS acpitz0 at acpi0: critical temperature is 95 degC acpiac0 at acpi0: AC unit online acpibat0 at acpi0: BAT0 model X401-44 serial type LIon oem ASUSTek acpibtn0 at acpi0: PWRB acpibtn1 at acpi0: LID_ acpibtn2 at acpi0: SLPB acpivideo0 at acpi0: VGA_ cpu0: 998 MHz: speeds: 1000 800 MHz pci0 at mainbus0 bus 0 pchb0 at pci0 dev 0 function 0 AMD AMD64 14h Host rev 0x00 radeondrm0 at pci0 dev 1 function 0 ATI Radeon HD 7290 rev 0x00 drm0 at radeondrm0 radeondrm0: msi azalia0 at pci0 dev 1 function 1 ATI Radeon HD 6310 HD Audio rev 0x00: msi azalia0: no supported codecs ppb0 at pci0 dev 4 function 0 AMD AMD64 14h PCIE rev 0x00: msi pci1 at ppb0 bus 1 AMD Hudson-2 xHCI rev 0x03 at pci0 dev 16 function 0 not configured ahci0 at pci0 dev 17 function 0 AMD Hudson-2 SATA rev 0x40: msi, AHCI 1.3 scsibus1 at ahci0: 32 targets sd0 at scsibus1 targ 0 lun 0: ATA, Hitachi HTS54505, GG2O SCSI3 0/direct fixed naa.5000cca700c3196d sd0: 476940MB, 512 bytes/sector, 976773168 sectors ohci0 at pci0 dev 18 function 0 AMD Hudson-2 USB rev 0x11: apic 3 int 18, version 1.0, legacy support ehci0 at pci0 dev 18 function 2 AMD Hudson-2 USB2 rev 0x11: apic 3 int 17 usb0 at ehci0: USB revision 2.0 uhub0 at usb0 AMD EHCI root hub rev 2.00/1.00 addr 1 ohci1 at pci0 dev 19 function 0 AMD Hudson-2 USB rev 0x11: apic 3 int 18, version 1.0, legacy support ehci1 at pci0 dev 19 function 2 AMD Hudson-2 USB2 rev 0x11: apic 3 int 17 usb1 at ehci1: USB revision 2.0 uhub1 at usb1 AMD EHCI root hub rev 2.00/1.00 addr 1 piixpm0 at pci0 dev 20 function 0 AMD Hudson-2 SMBus rev 0x14: polling iic0 at piixpm0 piixpm0: exec: op 1, addr 0x18, cmdlen 1, len 0, flags 0x08: timeout, status 0xfBUSY,INTR,DEVERR,BUSERR piixpm0: exec: op 1, addr 0x19, cmdlen 1, len 0, flags 0x08: timeout, status 0xfBUSY,INTR,DEVERR,BUSERR piixpm0: exec: op 1, addr 0x1a, cmdlen 1, len 0, flags 0x08: timeout, status 0xfBUSY,INTR,DEVERR,BUSERR piixpm0: exec: op 1, addr 0x1b, cmdlen 1, len 0, flags 0x08: timeout, status 0xfBUSY,INTR,DEVERR,BUSERR piixpm0: exec: op 1, addr 0x1c, cmdlen 1, len 0, flags 0x08: timeout, status 0xfBUSY,INTR,DEVERR,BUSERR piixpm0: exec: op 1, addr 0x1d, cmdlen 1, len 0, flags 0x08: timeout, status 0xfBUSY,INTR,DEVERR,BUSERR piixpm0: exec: op 1, addr 0x1e, cmdlen 1, len 0, flags 0x08: timeout, status 0xfBUSY,INTR,DEVERR,BUSERR piixpm0: exec: op 1, addr 0x1f, cmdlen 1, len 0, flags 0x08: timeout, status 0xfBUSY,INTR,DEVERR,BUSERR piixpm0: exec: op 1, addr 0x20, cmdlen 1, len 0, flags 0x08: timeout, status
HID parser
In December 2012 a user reported on misc@ that the Noppoo Mini Choc
84 USB keyboard does not work on OpenBSD [0]. More recently, mcbride@
and yasuoka@ contacted me because they have a mouse that is not properly
recognized. Both issues are related to our HID descriptor parser.
The diff below is an updated version of a diff I sent one year ago that
I forgot because of the lack of feedbacks. Here's the original
description of the problem:
The descriptor [of the mouse/keyboard] includes an Item with an Usage
array (Min-Max range) *and* various other single Usage elements.
Unfortunately our HID parser doesn't handle such descriptor, so I
backported FreeBSD's updated version, see below.
I'd appreciate if people could test this diff and tell me if it breaks
something.
I'm also looking for Oks.
Martin
[0] http://marc.info/?l=openbsd-miscm=135287138512465w=2
Index: hid.c
===
RCS file: /cvs/src/sys/dev/usb/hid.c,v
retrieving revision 1.25
diff -u -p -r1.25 hid.c
--- hid.c 5 Aug 2012 16:07:11 - 1.25
+++ hid.c 4 May 2014 15:35:03 -
@@ -41,36 +41,52 @@
#include dev/usb/hid.h
-#ifdef UHIDEV_DEBUG
-#define DPRINTF(x) do { if (uhidevdebug) printf x; } while (0)
-#define DPRINTFN(n,x) do { if (uhidevdebug(n)) printf x; } while (0)
-extern int uhidevdebug;
+#if UHIDEV_DEBUG
+#define DPRINTFN(n,x...) do { if (uhidevdebug(n)) printf(x); } while (0)
+int uhidevdebug = 0;
#else
-#define DPRINTF(x)
-#define DPRINTFN(n,x)
+#define DPRINTFN(n,x...)
#endif
-void hid_clear_local(struct hid_item *);
+void hid_clear_local(struct hid_item *);
+uint8_thid_get_byte(struct hid_data *, const uint16_t);
+void hid_switch_rid(struct hid_data *, struct hid_item *, int32_t);
+
+#defineMAXUSAGE 64
+#defineMAXPUSH 4
+#defineMAXID 16
+
+struct hid_pos_data {
+ int32_t rid;
+ uint32_t pos;
+};
-#define MAXUSAGE 256
struct hid_data {
- u_char *start;
- u_char *end;
- u_char *p;
- struct hid_item cur;
- int32_t usages[MAXUSAGE];
- int nu;
- int minset;
- int multi;
- int multimax;
+ const uint8_t *start;
+ const uint8_t *end;
+ const uint8_t *p;
+ struct hid_item cur[MAXPUSH];
+ struct hid_pos_data last_pos[MAXID];
+ int32_t usages_min[MAXUSAGE];
+ int32_t usages_max[MAXUSAGE];
+ int32_t usage_last; /* last seen usage */
+ uint32_t loc_size; /* last seen size */
+ uint32_t loc_count; /* last seen count */
enum hid_kind kind;
+ uint8_t pushlevel; /* current pushlevel */
+ uint8_t ncount; /* end usage item count */
+ uint8_t icount; /* current usage item count */
+ uint8_t nusage; /* end usages_min/max index */
+ uint8_t iusage; /* current usages_min/max index */
+ uint8_t ousage; /* current usages_min/max offset */
+ uint8_t susage; /* usage set flags */
};
void
hid_clear_local(struct hid_item *c)
{
-
- DPRINTFN(5,(hid_clear_local\n));
+ c-loc.count = 0;
+ c-loc.size = 0;
c-usage = 0;
c-usage_minimum = 0;
c-usage_maximum = 0;
@@ -83,15 +99,62 @@ hid_clear_local(struct hid_item *c)
c-set_delimiter = 0;
}
+void
+hid_switch_rid(struct hid_data *s, struct hid_item *c, int32_t nextid)
+{
+ uint8_t i;
+
+ if (c-report_ID == nextid)
+ return;
+
+ /* save current position for current rID */
+ if (c-report_ID == 0) {
+ i = 0;
+ } else {
+ for (i = 1; i != MAXID; i++) {
+ if (s-last_pos[i].rid == c-report_ID)
+ break;
+ if (s-last_pos[i].rid == 0)
+ break;
+ }
+ }
+ if (i != MAXID) {
+ s-last_pos[i].rid = c-report_ID;
+ s-last_pos[i].pos = c-loc.pos;
+ }
+
+ /* store next report ID */
+ c-report_ID = nextid;
+
+ /* lookup last position for next rID */
+ if (nextid == 0) {
+ i = 0;
+ } else {
+ for (i = 1; i != MAXID; i++) {
+ if (s-last_pos[i].rid == nextid)
+ break;
+ if (s-last_pos[i].rid == 0)
+ break;
+ }
+ }
+ if (i != MAXID) {
+ s-last_pos[i].rid = nextid;
+ c-loc.pos = s-last_pos[i].pos;
+ } else {
+ DPRINTFN(0, Out of RID entries, position is set to zero!\n);
+ c-loc.pos = 0;
+ }
+}
+
struct hid_data *
-hid_start_parse(void *d, int len, enum hid_kind kind)
+hid_start_parse(const void *d, int len, enum hid_kind kind)
{
struct hid_data *s;
s = malloc(sizeof *s, M_TEMP, M_WAITOK | M_ZERO);
s
