On 2022/09/03 21:37, Marcus Glocker wrote:
> On Sat, Sep 03, 2022 at 05:43:25PM +0200, Caspar Schutijser wrote:
>
> > Hi,
> >
> > On Sat, Sep 03, 2022 at 05:00:00PM +0200, Stefan Hagen wrote:
> > > This is a better version of an earlier attempt to make my wacom tablet
> > > work. I have the tablet here in Bad Liebenzell if you want to give it a
> > > spin (on my or on your machine).
> > >
> > > Comments? OK?
> >
> > I don't feel entirely qualified to give OKs in this area so I won't do
> > that. But I tested it on my machine with sdk@'s tablet and it works well
> > here. Nice!
> >
> > Is there any chance it breaks other supported tablets? Should it be
> > tested there as well?
>
> The driver only attaches to specific products, currently
> Intuos Draw, CTL-490. So there shouldn't be too much to break I guess.
Agreed, I don't see how it can break anything that already works.
> > One whitespace nit below.
+1
also make sure to commit usbdevs first to uodate rcsid, then re-run
"make" to copy the new rcsid to the "generated from" comment before
commiting the usbdevs/usbdevs_data.j files.
OK sthen
> > Caspar
> >
> > >
> > > Best Regards,
> > > Stefan
>
> I have an Wacom One CTL-672, never used it on OpenBSD. Currently
> it attaches to ums(4). Works fine with that. It also works fine
> when attaching to uwacom(4), without and with your diff. It doesn't
> seem to require specific 'tsscale' nor 'loc_tip_press' settings.
> I wonder if it would change something.
>
> Some very few comments inline.
>
> > > Index: share/man/man4/uwacom.4
> > > ===================================================================
> > > RCS file: /cvs/src/share/man/man4/uwacom.4,v
> > > retrieving revision 1.2
> > > diff -u -p -u -p -r1.2 uwacom.4
> > > --- share/man/man4/uwacom.4 12 Sep 2016 10:39:06 -0000 1.2
> > > +++ share/man/man4/uwacom.4 1 Sep 2022 19:57:37 -0000
> > > @@ -42,6 +42,7 @@ driver supports the following Wacom tabl
> > > .Bl -column "Intuos Draw" "Model Number" -offset 6n
> > > .It Em Name Ta Em Model Number
> > > .It Li Intuos Draw Ta CTL-490
> > > +.It Li One Ta CTL-472
>
> Shouldn't that be 'One S'?
>
> > > .El
> > > .Sh SEE ALSO
> > > .Xr uhidev 4 ,
> > > Index: sys/dev/usb/usbdevs
> > > ===================================================================
> > > RCS file: /cvs/src/sys/dev/usb/usbdevs,v
> > > retrieving revision 1.748
> > > diff -u -p -u -p -r1.748 usbdevs
> > > --- sys/dev/usb/usbdevs 23 Aug 2022 08:10:35 -0000 1.748
> > > +++ sys/dev/usb/usbdevs 1 Sep 2022 19:57:38 -0000
> > > @@ -4613,6 +4613,7 @@ product WACOM GRAPHIRE3_4X5 0x0013 Graph
> > > product WACOM GRAPHIRE4_4X5 0x0015 Graphire4 Classic A6
> > > product WACOM INTUOSA5 0x0021 Intuos A5
> > > product WACOM INTUOS_DRAW 0x033b Intuos Draw (CTL-490)
> > > +product WACOM ONE_S 0x037a One S (CTL-472)
> > > product WACOM INTUOS_PRO_S 0x0392 Intuos Pro S
> > >
> > > /* WAGO Kontakttechnik products */
> > > Index: sys/dev/usb/usbdevs.h
> > > ===================================================================
> > > RCS file: /cvs/src/sys/dev/usb/usbdevs.h,v
> > > retrieving revision 1.760
> > > diff -u -p -u -p -r1.760 usbdevs.h
> > > --- sys/dev/usb/usbdevs.h 23 Aug 2022 08:11:01 -0000 1.760
> > > +++ sys/dev/usb/usbdevs.h 1 Sep 2022 19:57:38 -0000
> > > @@ -1,4 +1,4 @@
> > > -/* $OpenBSD: usbdevs.h,v 1.760 2022/08/23 08:11:01 jsg Exp $
> > > */
> > > +/* $OpenBSD$ */
> > >
> > > /*
> > > * THIS FILE IS AUTOMATICALLY GENERATED. DO NOT EDIT.
> > > @@ -4620,6 +4620,7 @@
> > > #define USB_PRODUCT_WACOM_GRAPHIRE4_4X5 0x0015 /* Graphire4
> > > Classic A6 */
> > > #define USB_PRODUCT_WACOM_INTUOSA5 0x0021 /* Intuos A5 */
> > > #define USB_PRODUCT_WACOM_INTUOS_DRAW 0x033b /* Intuos Draw
> > > (CTL-490) */
> > > +#define USB_PRODUCT_WACOM_ONE_S 0x037a /* One S (CTL-472) */
> > > #define USB_PRODUCT_WACOM_INTUOS_PRO_S 0x0392 /* Intuos Pro S
> > > */
> > >
> > > /* WAGO Kontakttechnik products */
> > > Index: sys/dev/usb/usbdevs_data.h
> > > ===================================================================
> > > RCS file: /cvs/src/sys/dev/usb/usbdevs_data.h,v
> > > retrieving revision 1.754
> > > diff -u -p -u -p -r1.754 usbdevs_data.h
> > > --- sys/dev/usb/usbdevs_data.h 23 Aug 2022 08:11:01 -0000 1.754
> > > +++ sys/dev/usb/usbdevs_data.h 1 Sep 2022 19:57:39 -0000
> > > @@ -1,4 +1,4 @@
> > > -/* $OpenBSD: usbdevs_data.h,v 1.754 2022/08/23 08:11:01 jsg Exp $
> > > */
> > > +/* $OpenBSD$ */
> > >
> > > /*
> > > * THIS FILE IS AUTOMATICALLY GENERATED. DO NOT EDIT.
> > > @@ -11824,6 +11824,10 @@ const struct usb_known_product usb_known
> > > {
> > > USB_VENDOR_WACOM, USB_PRODUCT_WACOM_INTUOS_DRAW,
> > > "Intuos Draw (CTL-490)",
> > > + },
> > > + {
> > > + USB_VENDOR_WACOM, USB_PRODUCT_WACOM_ONE_S,
> > > + "One S (CTL-472)",
> > > },
> > > {
> > > USB_VENDOR_WACOM, USB_PRODUCT_WACOM_INTUOS_PRO_S,
> > > Index: sys/dev/usb/uwacom.c
> > > ===================================================================
> > > RCS file: /cvs/src/sys/dev/usb/uwacom.c,v
> > > retrieving revision 1.5
> > > diff -u -p -u -p -r1.5 uwacom.c
> > > --- sys/dev/usb/uwacom.c 22 Nov 2021 11:29:18 -0000 1.5
> > > +++ sys/dev/usb/uwacom.c 1 Sep 2022 19:57:39 -0000
> > > @@ -35,10 +35,14 @@
> > >
> > > #include <dev/hid/hidmsvar.h>
> > >
> > > +#define UWACOM_USE_PRESSURE 0x0001 /* button 0 is flaky, use tip
> > > pressure */
> > > +#define UWACOM_BIG_ENDIAN 0x0002 /* XY reporting byte order */
> > > +
> > > struct uwacom_softc {
> > > struct uhidev sc_hdev;
> > > struct hidms sc_ms;
> > > struct hid_location sc_loc_tip_press;
> > > + int sc_flags;
> > > };
> > >
> > > struct cfdriver uwacom_cd = {
> > > @@ -47,7 +51,8 @@ struct cfdriver uwacom_cd = {
> > >
> > >
> > > const struct usb_devno uwacom_devs[] = {
> > > - { USB_VENDOR_WACOM, USB_PRODUCT_WACOM_INTUOS_DRAW }
> > > + { USB_VENDOR_WACOM, USB_PRODUCT_WACOM_INTUOS_DRAW },
> > > + { USB_VENDOR_WACOM, USB_PRODUCT_WACOM_ONE_S }
> > > };
> > >
> > > int uwacom_match(struct device *, void *, void *);
> > > @@ -110,6 +115,7 @@ uwacom_attach(struct device *parent, str
> > >
> > > uhidev_get_report_desc(uha->parent, &desc, &size);
> > > repid = uha->reportid;
> > > +
> > > sc->sc_hdev.sc_isize = hid_report_size(desc, size, hid_input, repid);
> > > sc->sc_hdev.sc_osize = hid_report_size(desc, size, hid_output, repid);
> > > sc->sc_hdev.sc_fsize = hid_report_size(desc, size, hid_feature, repid);
> > > @@ -118,15 +124,14 @@ uwacom_attach(struct device *parent, str
> > > ms->sc_rawmode = 1;
> > > ms->sc_flags = HIDMS_ABSX | HIDMS_ABSY;
> > > ms->sc_num_buttons = 3;
> > > +
> > > ms->sc_loc_x.pos = 8;
> > > ms->sc_loc_x.size = 16;
> > > ms->sc_loc_y.pos = 24;
> > > ms->sc_loc_y.size = 16;
> > >
> > > ms->sc_tsscale.minx = 0;
> > > - ms->sc_tsscale.maxx = 7600;
> > > ms->sc_tsscale.miny = 0;
> > > - ms->sc_tsscale.maxy = 4750;
> > >
> > > ms->sc_loc_btn[0].pos = 0;
> > > ms->sc_loc_btn[0].size = 1;
> > > @@ -135,8 +140,21 @@ uwacom_attach(struct device *parent, str
> > > ms->sc_loc_btn[2].pos = 2;
> > > ms->sc_loc_btn[2].size = 1;
> > >
> > > - sc->sc_loc_tip_press.pos = 43;
> > > - sc->sc_loc_tip_press.size = 8;
> > > + if (uha->uaa->product == USB_PRODUCT_WACOM_ONE_S) {
> > > + static uByte reportbuf[2] = { 0x02, 0x02 };
> > > + uhidev_set_report(uha->parent, UHID_FEATURE_REPORT, 2,
> > > + &reportbuf, 2);
> >
> > This indentation style is different from style(9). It should be
> > \t\t<4 spaces>&reportbuf, 2);
>
> I agree.
>
> > > + ms->sc_tsscale.maxx = 15200;
> > > + ms->sc_tsscale.maxy = 9500;
> > > + }
> > > +
> > > + if (uha->uaa->product == USB_PRODUCT_WACOM_INTUOS_DRAW) {
> > > + sc->sc_flags = UWACOM_USE_PRESSURE | UWACOM_BIG_ENDIAN;
> > > + sc->sc_loc_tip_press.pos = 43;
> > > + sc->sc_loc_tip_press.size = 8;
> > > + ms->sc_tsscale.maxx = 7600;
> > > + ms->sc_tsscale.maxy = 4750;
> > > + }
> > >
> > > hidms_attach(ms, &uwacom_accessops);
> > > } > > @@ -166,19 +184,25 @@ uwacom_intr(struct uhidev *addr, void *b
> > > if ((data[0] & 0xf0) == 0xc0)
> > > return;
> > >
> > > - x = be16toh(hid_get_data(data, len, &ms->sc_loc_x));
> > > - y = be16toh(hid_get_data(data, len, &ms->sc_loc_y));
> > > - pressure = hid_get_data(data, len, &sc->sc_loc_tip_press);
> > > + x = hid_get_data(data, len, &ms->sc_loc_x);
> > > + y = hid_get_data(data, len, &ms->sc_loc_y);
> > > +
> > > + if (sc->sc_flags & UWACOM_BIG_ENDIAN) {
> > > + x = be16toh(x);
> > > + y = be16toh(y);
> > > + }
> > >
> > > for (i = 0; i < ms->sc_num_buttons; i++)
> > > if (hid_get_data(data, len, &ms->sc_loc_btn[i]))
> > > buttons |= (1 << i);
> > >
> > > - /* button 0 reporting is flaky, use tip pressure for it */
> > > - if (pressure > 10)
> > > - buttons |= 1;
> > > - else
> > > - buttons &= ~1;
> > > + if (sc->sc_flags & UWACOM_USE_PRESSURE) {
> > > + pressure = hid_get_data(data, len, &sc->sc_loc_tip_press);
> > > + if (pressure > 10)
> > > + buttons |= 1;
> > > + else
> > > + buttons &= ~1;
> > > + }
> > >
> > > if (x != 0 || y != 0 || buttons != ms->sc_buttons) {
> > > wsmouse_position(ms->sc_wsmousedev, x, y);
> > >
>