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);
> > > 
> 

Reply via email to