Re: hidmt: clickpad detection

2022-09-21 Thread Laurence Tratt
On Tue, Sep 13, 2022 at 11:06:33PM +0200, Ulf Brosziewski wrote:

Hello Ulf,

> The diff below improves the way hidmt identifies clickpads, and
> addresses the problems described in
> https://marc.info/?l=openbsd-tech=165296530618617=2
> and
> https://marc.info/?l=openbsd-tech=165980534415586=2
> 
> If devices don't report a HUD_BUTTON_TYPE property, the driver checks
> whether they claim to have an external left button, and if they do,
> hidmt doesn't treat them as clickpads.
> 
> I think this part of the logic should be turned around:  hidmt should
> treat everything as clickpad except if there is no "clickpad button"
> (HUP_BUTTON 1) *and* both an external left and an external right button
> (HUP BUTTON 2 and 3) are being reported.  Touchpads without the internal
> button are still usable with the clickpad configuration, it does less
> harm to err on this side.
> 
> Tests and OKs would be welcome.

Thanks for doing this! This fixes the Framework touchpad and also seems like
a better heuristic than we currently have (certainly it seems better to me
than the Framework-specific quirk I added!).

I think it would be great to try this in snapshots for a little while to
check whether it causes any issues, though I'm optimistic that it will Just
Work!


Laurie



Re: hidmt: clickpad detection

2022-09-18 Thread Lucas Raab
On Tue, Sep 13, 2022 at 11:06:33PM +0200, Ulf Brosziewski wrote:
> The diff below improves the way hidmt identifies clickpads, and
> addresses the problems described in
> https://marc.info/?l=openbsd-tech=165296530618617=2
> and
> https://marc.info/?l=openbsd-tech=165980534415586=2
> 
> If devices don't report a HUD_BUTTON_TYPE property, the driver checks
> whether they claim to have an external left button, and if they do,
> hidmt doesn't treat them as clickpads.
> 
> I think this part of the logic should be turned around:  hidmt should
> treat everything as clickpad except if there is no "clickpad button"
> (HUP_BUTTON 1) *and* both an external left and an external right button
> (HUP BUTTON 2 and 3) are being reported.  Touchpads without the internal
> button are still usable with the clickpad configuration, it does less
> harm to err on this side.
> 
> Tests and OKs would be welcome.

Not qualified to give an OK, but I've been running this for the
past few days on a second gen Framework and double/triple click
works as expected.

Lucas



hidmt: clickpad detection

2022-09-13 Thread Ulf Brosziewski
The diff below improves the way hidmt identifies clickpads, and
addresses the problems described in
https://marc.info/?l=openbsd-tech=165296530618617=2
and
https://marc.info/?l=openbsd-tech=165980534415586=2

If devices don't report a HUD_BUTTON_TYPE property, the driver checks
whether they claim to have an external left button, and if they do,
hidmt doesn't treat them as clickpads.

I think this part of the logic should be turned around:  hidmt should
treat everything as clickpad except if there is no "clickpad button"
(HUP_BUTTON 1) *and* both an external left and an external right button
(HUP BUTTON 2 and 3) are being reported.  Touchpads without the internal
button are still usable with the clickpad configuration, it does less
harm to err on this side.

Tests and OKs would be welcome.


Index: dev/hid/hidmt.c
===
RCS file: /cvs/src/sys/dev/hid/hidmt.c,v
retrieving revision 1.12
diff -u -p -r1.12 hidmt.c
--- dev/hid/hidmt.c 9 Jul 2020 21:01:08 -   1.12
+++ dev/hid/hidmt.c 13 Sep 2022 19:31:38 -
@@ -154,11 +154,13 @@ hidmt_setup(struct device *self, struct
mt->sc_rep_cap, hid_feature, , NULL)) {
d = hid_get_udata(rep, capsize, );
mt->sc_clickpad = (d == 0);
-   } else {
-   /* if there's not a 2nd button, this is probably a clickpad */
-   if (!hid_locate(desc, dlen, HID_USAGE2(HUP_BUTTON, 2),
-   mt->sc_rep_input, hid_input, , NULL))
-   mt->sc_clickpad = 1;
+   } else if (hid_locate(desc, dlen, HID_USAGE2(HUP_BUTTON, 1),
+   mt->sc_rep_input, hid_input, , NULL)
+   || !hid_locate(desc, dlen, HID_USAGE2(HUP_BUTTON, 2),
+   mt->sc_rep_input, hid_input, , NULL)
+   || !hid_locate(desc, dlen, HID_USAGE2(HUP_BUTTON, 3),
+   mt->sc_rep_input, hid_input, , NULL)) {
+   mt->sc_clickpad = 1;
}

/*