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
