On Mon, Jul 03, 2023 at 11:22:45AM +0200, Marc Espie wrote: > I hope Vladimir will find the time to complete this answer. > > As far as Vlad's work goes, he did a presentation last week-end: > https://www.lre.epita.fr/news_content/SS_summer_week_pres/Vladimir_Driver_OpenBSD.pptx > > (sorry for the medium, fortunately we have libreoffice) > > In the mean time, here is an updated diff. > > I removed the Gaomon stuff, which if anything should be a different patch. > > And I cleaned up the 20+ minor style violations I could find... > (tabs instead of +4 spaces for continued lines, a few non-style compliant > function declarations and/or code blocks, oh well) > > plus an extra malloc.h that snuck in and is not at all needed. > > And some typos in comments. > And a C++ style comment. Oh well > > I would really for some version of this to get in soonish.
A few nits below. I can't really comment on the HID parser logic although I find it a bit strange to need quirk to attach uwacom. > Index: dev/hid/hidms.c > =================================================================== > RCS file: /cvs/src/sys/dev/hid/hidms.c,v > retrieving revision 1.9 > diff -u -p -r1.9 hidms.c > --- dev/hid/hidms.c 16 Jun 2022 20:52:38 -0000 1.9 > +++ dev/hid/hidms.c 3 Jul 2023 09:04:50 -0000 > @@ -61,6 +61,210 @@ int hidmsdebug = 0; > #define MOUSE_FLAGS_MASK (HIO_CONST | HIO_RELATIVE) > #define NOTMOUSE(f) (((f) & MOUSE_FLAGS_MASK) != HIO_RELATIVE) > > + > +int > +stylus_hid_parse(struct hidms *ms, struct hid_data *d, uint32_t *flags) > +{ > + /* Define stylus reported usages: (maybe macros?) */ > + const uint32_t stylus_usage_tip > + = HID_USAGE2(HUP_WACOM | HUP_DIGITIZERS, HUD_TIP_SWITCH); > + const uint32_t stylus_usage_barrel > + = HID_USAGE2(HUP_WACOM | HUP_DIGITIZERS, HUD_BARREL_SWITCH); > + const uint32_t stylus_usage_sec_barrel = HID_USAGE2( > + HUP_WACOM | HUP_DIGITIZERS, HUD_SECONDARY_BARREL_SWITCH); > + const uint32_t stylus_usage_in_range > + = HID_USAGE2(HUP_WACOM | HUP_DIGITIZERS, HUD_IN_RANGE); > + const uint32_t stylus_usage_quality > + = HID_USAGE2(HUP_WACOM | HUP_DIGITIZERS, HUD_QUALITY); > + const uint32_t stylus_usage_x > + = HID_USAGE2(HUP_WACOM | HUP_DIGITIZERS, HUD_WACOM_X); > + const uint32_t stylus_usage_y > + = HID_USAGE2(HUP_WACOM | HUP_DIGITIZERS, HUD_WACOM_Y); > + const uint32_t stylus_usage_pressure > + = HID_USAGE2(HUP_WACOM | HUP_DIGITIZERS, HUD_TIP_PRESSURE); > + const uint32_t stylus_usage_distance > + = HID_USAGE2(HUP_WACOM | HUP_DIGITIZERS, HUD_WACOM_DISTANCE); > + > + struct hid_item h; > + > + while (hid_get_item(d, &h)) { > + if (h.kind == hid_input && !(h.flags & HIO_CONST)) { > + /* All the possible stylus reported usages go here */ > +#ifdef HIDMS_DEBUG > + printf("stylus usage: 0x%x\n", h.usage); > +#endif > + switch (h.usage) { > + /* Buttons */ > + case stylus_usage_tip: > + DPRINTF("Stylus usage tip set\n"); > + ms->sc_loc_stylus_btn > + [ms->sc_num_stylus_buttons++] = h.loc; > + ms->sc_flags |= HIDMS_TIP; > + break; > + case stylus_usage_barrel: > + DPRINTF("Stylus usage barrel set\n"); > + ms->sc_loc_stylus_btn > + [ms->sc_num_stylus_buttons++] = h.loc; > + ms->sc_flags |= HIDMS_BARREL; > + break; > + case stylus_usage_sec_barrel: > + DPRINTF("Stylus usage secondary barrel set\n"); > + ms->sc_loc_stylus_btn > + [ms->sc_num_stylus_buttons++] = h.loc; > + ms->sc_flags |= HIDMS_SEC_BARREL; > + break; > + case stylus_usage_in_range: > + DPRINTF("Stylus usage in range set\n"); > + ms->sc_loc_stylus_btn > + [ms->sc_num_stylus_buttons++] = h.loc; > + break; > + case stylus_usage_quality: > + DPRINTF("Stylus usage quality set\n"); > + ms->sc_loc_stylus_btn > + [ms->sc_num_stylus_buttons++] = h.loc; > + break; > + /* Axes */ > + case stylus_usage_x: > + DPRINTF("Stylus usage x set\n"); > + ms->sc_loc_x = h.loc; > + ms->sc_tsscale.minx = h.logical_minimum; > + ms->sc_tsscale.maxx = h.logical_maximum; > + ms->sc_flags |= HIDMS_ABSX; > + break; > + case stylus_usage_y: > + DPRINTF("Stylus usage y set\n"); > + ms->sc_loc_y = h.loc; > + ms->sc_tsscale.miny = h.logical_minimum; > + ms->sc_tsscale.maxy = h.logical_maximum; > + ms->sc_flags |= HIDMS_ABSY; > + break; > + case stylus_usage_pressure: > + DPRINTF("Stylus usage pressure set\n"); > + ms->sc_loc_z = h.loc; > + ms->sc_tsscale.minz = h.logical_minimum; > + ms->sc_tsscale.maxz = h.logical_maximum; > + ms->sc_flags |= HIDMS_Z; > + break; > + case stylus_usage_distance: > + DPRINTF("Stylus usage distance set\n"); > + ms->sc_loc_w = h.loc; > + ms->sc_tsscale.minw = h.logical_minimum; > + ms->sc_tsscale.maxw = h.logical_maximum; > + ms->sc_flags |= HIDMS_W; > + break; > + default: > +#ifdef HIDMS_DEBUG > + printf("Unknown stylus usage: 0x%x, please > report to the devs!\n", Not too sure about this messag. I'd drop the "please report to the devs" part. > + h.usage); > +#endif > + break; > + } > + } > + if (h.kind == hid_endcollection) > + break; > + } > + hid_end_parse(d); > + if (flags != NULL) > + *flags = 0; > + return (0); > +} > + > +int > +pad_buttons_hid_parser(struct hidms *ms, struct hid_data *d, uint32_t > *flags) > +{ > + struct hid_item h; > + > + while (hid_get_item(d, &h)) { > + if (h.kind == hid_input && !(h.flags & HIO_CONST) > + && h.usage == HID_USAGE2(HUP_WACOM | HUP_DIGITIZERS, > + HUD_WACOM_PAD_BUTTONS00 | ms->sc_num_pad_buttons)) { > + ms->sc_loc_pad_btn[ms->sc_num_pad_buttons++] = h.loc; > + } > + if (h.kind == hid_endcollection) > + break; > + } > + hid_end_parse(d); > + if (flags != NULL) > + *flags = 0; > + return (0); > +} > + > +int > +hidms_wacom_setup(struct device *self, struct hidms *ms, uint32_t quirks, > + int id, void *desc, int dlen) > +{ > + struct hid_data *global; > + uint32_t flags; > + int i; > + > + quirks = 0; > + ms->sc_device = self; > + ms->sc_rawmode = 1; > + > + ms->sc_flags = quirks; > + > + /* Set x,y,z and w to zero by default */ > + ms->sc_loc_x.size = 0; > + ms->sc_loc_y.size = 0; > + ms->sc_loc_z.size = 0; > + ms->sc_loc_w.size = 0; > + > + if ((global = hid_get_collection_data(desc, dlen, > + HID_USAGE2(HUP_WACOM | HUP_DIGITIZERS, HUD_DIGITIZER), > + HCOLL_APPLICATION))) { > + hid_end_parse(global); > + > + struct hid_data *stylus_col; > + struct hid_data *tablet_keys_col; > + struct hid_data *battery_col; > + > + DPRINTF("found the global collection\n"); > + if ((stylus_col = hid_get_collection_data(desc, dlen, > + HID_USAGE2(HUP_WACOM | HUP_DIGITIZERS, HUD_STYLUS), > + HCOLL_PHYSICAL))) { > + DPRINTF("found stylus collection\n"); > + stylus_hid_parse(ms, stylus_col, &flags); > + } > + if ((tablet_keys_col = hid_get_collection_data(desc, dlen, > + HID_USAGE2(HUP_WACOM | HUP_DIGITIZERS, HUD_TABLET_FKEYS), > + HCOLL_PHYSICAL))) { > + DPRINTF("found tablet keys collection\n"); > + pad_buttons_hid_parser(ms, tablet_keys_col, &flags); > + } > + if ((battery_col = hid_get_collection_data(desc, dlen, > + HID_USAGE2(HUP_WACOM | HUP_DIGITIZERS, HUD_WACOM_BATTERY), > + HCOLL_PHYSICAL))) { > + DPRINTF("found battery collection\n"); > + /* parse and set the battery info */ > + /* not yet used */ > + hid_end_parse(battery_col); > + } > + /* > + * Ignore the device config, it's not really needed > + * Ignore the usage 0x10AC which is the debug collection, and > + * ignore firmware collection and other collections for now > + */ > + } > + > + /* Map the pad and stylus buttons to mouse buttons */ > + for (i = 0; i < ms->sc_num_stylus_buttons; i++) > + memcpy(&(ms->sc_loc_btn[i]), &(ms->sc_loc_stylus_btn[i]), > + sizeof(struct hid_location)); > + for (; i < ms->sc_num_pad_buttons + ms->sc_num_stylus_buttons; i++) > + memcpy(&(ms->sc_loc_btn[i]), &(ms->sc_loc_pad_btn[i]), > + sizeof(struct hid_location)); > + ms->sc_num_buttons = i; > + DPRINTF("Buttons inf\n"); > +#ifdef HIDMS_DEBUG > + for (i = 0; i < ms->sc_num_buttons; i++) > + printf("size: 0x%x, pos: 0x%x, count: 0x%x\n", > + ms->sc_loc_btn[i].size, ms->sc_loc_btn[i].pos, > + ms->sc_loc_btn[i].count); > +#endif > + return 0; > +} > + > + > int > hidms_setup(struct device *self, struct hidms *ms, uint32_t quirks, > int id, void *desc, int dlen) > @@ -74,6 +278,10 @@ hidms_setup(struct device *self, struct > ms->sc_rawmode = 1; > > ms->sc_flags = quirks; > + > + /* We are setting up a WACOM tablet, not a mouse */ > + if (quirks == HIDMS_WACOM_SETUP) > + return hidms_wacom_setup(self, ms, quirks, id, desc, dlen); > > if (!hid_locate(desc, dlen, HID_USAGE2(HUP_GENERIC_DESKTOP, HUG_X), id, > hid_input, &ms->sc_loc_x, &flags)) > Index: dev/usb/usbdevs.h > =================================================================== > RCS file: /cvs/src/sys/dev/usb/usbdevs.h,v > retrieving revision 1.769 > diff -u -p -r1.769 usbdevs.h > --- dev/usb/usbdevs.h 12 Jun 2023 11:26:54 -0000 1.769 > +++ dev/usb/usbdevs.h 3 Jul 2023 09:04:50 -0000 > @@ -1,4 +1,4 @@ > -/* $OpenBSD: usbdevs.h,v 1.769 2023/06/12 11:26:54 jsg Exp $ */ > +/* $OpenBSD$ */ > > /* > * THIS FILE IS AUTOMATICALLY GENERATED. DO NOT EDIT. > @@ -2117,6 +2117,8 @@ > #define USB_PRODUCT_GARMIN_DAKOTA20 0x23c0 /* Dakota 20 */ > #define USB_PRODUCT_GARMIN_GPSMAP62S 0x2459 /* GPSmap 62s */ > > +/* Gaomon */ > + Strange... looks like you edited the file instead of re-generating it. -- Matthieu Herrb