Tested the current input-wacom master (9d7af79697) *without* this
patch on my CentOS 6.9 VM using three devices (all with touch
switches): 1st-gen Intuos, 2nd-gen Intuos Pro, and Cintiq Pro 32. In
all cases, everything seemed to work as expected:
* Single finger touch worked correctly
* Touch switch enabled / disabled touch input
* Touch switch state is correctly reported by xsetwacom
* Multi-finger touch works correctly for the two Pro devices (it is
disabled for the 1st-gen Intuos by the HEAD commit due to its style of
multitouch events [DOUBLE,TRIPLE,QUADTAP] confusing the X driver).
There must be something different between our test setups...
Jason
---
Now instead of four in the eights place /
you’ve got three, ‘Cause you added one /
(That is to say, eight) to the two, /
But you can’t take seven from three, /
So you look at the sixty-fours....
On Tue, Apr 24, 2018 at 9:42 PM, Jason Gerecke <killert...@gmail.com> wrote:
> On Tue, Apr 24, 2018 at 12:19 AM, Ping Cheng <pingli...@gmail.com> wrote:
>> On Mon, Apr 23, 2018 at 4:37 PM Peter Hutterer <peter.hutte...@who-t.net>
>> wrote:
>>>
>>> On Mon, Apr 23, 2018 at 12:44:51PM -0700, Aaron Armstrong Skomra wrote:
>>> > Setting the bit for SW_MUTE_DEVICE on kernels which do not have
>>> > SW_MUTE_DEVICE declared in input.h (prior to 3.13), causes these
>>> > devices' touch interfaces not to work.
>>>
>>> Why is that? is that switch always enabled or something? because if it's
>>> always on 0, no-one in userspace should care. This seems like a separate
>>> issue somewhere.
>>
>>
>> The switch can be enabled/disabled by some devices. So, if it is set, its
>> value could be 1.
>>
>
> Agreed with Peter... This sounds like a symptom of something else. In
> my experience the kernel doesn't care about "unknown" SW/ABS/KEY
> events, so any problem would have to be in our kernel or X driver. The
> fact that "only" removing the declaration SW_MUTE_DEVICE is enough to
> fix the issue is a head-scratcher.
>
>>> > Fixes: 7d13c26dd972 ("Backport support for the touch on/off switch").
>>> > Signed-off-by: Aaron Armstrong Skomra<sko...@gmail.com>
>>> > ---
>>> > 1. Does INTUOSHT have a touch switch?
>>
>>
>> Yes, INTUOSHT has a hard touch switch.
>>
>>> Maybe the check for
>>> > BTN_TOOL_DOUBLETAP excludes INTUOSHT right after we catch its case.
>>
>>
>> Have to look into the details...
>>
>>> >
>>> > 2. Does TABLETPC somehow need this commit to disable touch? I don't
>>> > think
>>> > it could be using this non-existant constant, but I don't know which
>>> > devices TABLETPC applies to etc.
>>
>>
>> No TPC devices have touch switch.
>>
>>> On the Intuos Pro 2 and Intuos (CTH-680)
>>> > this switch is purely the reporting of the switch (the hardware disables
>>> > the touch) so removing our attempt at reporting the switch is of minor
>>> > consequence.
>>>
>>> with my "i don't follow input-wacom that closely" hat on:
>>> TABLETPC are the devices built into laptops. Google Bill Gates presenting
>>> a
>>> tablet pc, that's how far they go back. They have styli that only activate
>>> on press, with no proximity, etc. But they don't have the MUTE switch,
>>> that
>>> one was added for the 24QHD (or so?) to represent the physical hw switch.
>>
>>
>> Touch switch was first added to INTUOSHT.
>>
>>
>>> > 3. Why was SW_MUTE_DEVICE orignally backported? My guess was that it was
>>> > to remove differences between each of our kernel directories. But it
>>> > could
>>
>>
>> I feel it is just to sync with upstream code. Is that right Jason?
>>
>
> Git blame says it's been in the 2.6.38 tree since December 2013
> (commit 955bc05d68). I added it to the 2.6.30 tree in December 2017
> about a month after doing my Cintiq Pro backports. I *probably* tested
> that code the touch switch (and touch) worked for the Cintiq Pro, but
> I can absolutely guarantee that touch has been tested on 2.6.38 at
> multiple points in the past four years.
>
>>> > be for some other reason that I don't know of, in that case I will be
>>> > missing something here.
>>> >
>>> > 4. My guess is that SW_MUTE_DEVICE is supported in RHEL 7 (this bug was
>>> > found on RHEL 6) and that this code was tested against it, which is why
>>> > we
>>> > didn't catch this. ie RHEL probably added SW_MUTE_DEVICE to their
>>> > kernel.
>>>
>>> yep, looks like it was backported to the RHEL7.x kernel at some point but
>>> I
>>
>>
>> SW_MUTE_DEVICE is in RHEL 7 doesn’t mean other kernels older than 3.13
>> backported it. We have to support those vanilla kernels. Looks like we may
>> have to test more kernels and probably treat RHEL7 differently.
>> .
>
> I doubt RHEL having SW_MUTE_DEVICE "support" matters at all.
> SW_MUTE_DEVICE is only declared in a header and defined in our driver.
> Nothing else in the kernel should care if a random (known or unknown)
> SW integer is declared.
>
>>>
>>> don't have the exact version on-hand. We tend to backport the professional
>>> tablets so it was likely part of the 24QHD or whichever device needed that
>>> one.
>>
>>
>> Intuos Pro 2 is the first professional tablet that has a hardware touch
>> switch.
>>
>> Cheers,
>> Ping
>>
>>> > It could also be that later input subsystems fail more gracefully when
>>> > they recieve a "__set_bit" that they are unfamiliar with, I'm not sure
>>> > if
>>> > we want to find supported vanilla versions of these older kernels to
>>> > test
>>> > against. Also, I have no idea how to check RHEL 7 source or if that's
>>> > possible. If necessary we can either revise this commit to check for
>>> > RHEL
>>> > 7 or come back and do that at a later date for this event which again is
>>> > of minor importance.
>>>
>
> I'd verify that something's not wrong with our test setup before
> moving forward with this patch. It seems too odd that touch is broken
> on anything merely declaring SW_MUTE_DEVICE, especially when that code
> has been working (at the very least for 2.6.38, and likely even for
> 2.6.30) in the past.
>
> Jason
> ---
> Now instead of four in the eights place /
> you’ve got three, ‘Cause you added one /
> (That is to say, eight) to the two, /
> But you can’t take seven from three, /
> So you look at the sixty-fours....
>
>>> The official way if you don't have a RHEL install
>>> is https://git.centos.org/project/rpms
>>> yes, that's a CentOS link, for more backstory see:
>>> https://lwn.net/Articles/603865/
>>>
>>> The kernel is here:
>>> https://git.centos.org/summary/?r=rpms/kernel.git
>>>
>>> And to 'unzip' the RPM file, I have a script that wraps this:
>>> rpm2cpio $rpmfile | cpio -idmv --no-absolute-filenames
>>>
>>>
>>> Alternatively, there's also the Red Hat Developers program which gives you
>>> free access to RHEL.
>>> https://developers.redhat.com/
>>>
>>> Cheers,
>>> Peter
>>>
>>>
>>> >
>>> > 2.6.32/wacom_wac.c | 3 ---
>>> > 2.6.38/wacom_wac.c | 3 ---
>>> > 3.7/wacom_wac.c | 3 ---
>>> > 3 files changed, 9 deletions(-)
>>> >
>>> > diff --git a/2.6.32/wacom_wac.c b/2.6.32/wacom_wac.c
>>> > index 9f9899917a41..98589bea8f17 100644
>>> > --- a/2.6.32/wacom_wac.c
>>> > +++ b/2.6.32/wacom_wac.c
>>> > @@ -2276,7 +2276,6 @@ void wacom_setup_input_capabilities(struct
>>> > input_dev *input_dev,
>>> > __set_bit(BTN_TOOL_DOUBLETAP, input_dev->keybit);
>>> >
>>> > input_dev->evbit[0] |= BIT_MASK(EV_SW);
>>> > - __set_bit(SW_MUTE_DEVICE, input_dev->swbit);
>>> > wacom_wac->shared->has_mute_touch_switch = true;
>>> > } else {
>>> > __set_bit(BTN_STYLUS3, input_dev->keybit);
>>> > @@ -2351,7 +2350,6 @@ void wacom_setup_input_capabilities(struct
>>> > input_dev *input_dev,
>>> >
>>> > if ((input_dev->id.product >= 0x353 &&
>>> > input_dev->id.product <= 0x356)) {
>>> > input_dev->evbit[0] |= BIT_MASK(EV_SW);
>>> > - __set_bit(SW_MUTE_DEVICE,
>>> > input_dev->swbit);
>>> > wacom_wac->shared->has_mute_touch_switch =
>>> > true;
>>> > wacom_wac->shared->is_touch_on = true;
>>> > }
>>> > @@ -2394,7 +2392,6 @@ void wacom_setup_input_capabilities(struct
>>> > input_dev *input_dev,
>>> > if (features->touch_max &&
>>> > features->device_type == BTN_TOOL_DOUBLETAP) {
>>> > input_dev->evbit[0] |= BIT_MASK(EV_SW);
>>> > - __set_bit(SW_MUTE_DEVICE, input_dev->swbit);
>>> > wacom_wac->shared->has_mute_touch_switch = true;
>>> > }
>>> > /* fall through */
>>> > diff --git a/2.6.38/wacom_wac.c b/2.6.38/wacom_wac.c
>>> > index e0b5af1fc850..858602daf993 100644
>>> > --- a/2.6.38/wacom_wac.c
>>> > +++ b/2.6.38/wacom_wac.c
>>> > @@ -2539,7 +2539,6 @@ int wacom_setup_input_capabilities(struct
>>> > input_dev *input_dev,
>>> > }
>>> > else {
>>> > input_dev->evbit[0] |= BIT_MASK(EV_SW);
>>> > - __set_bit(SW_MUTE_DEVICE, input_dev->swbit);
>>> > wacom_wac->shared->has_mute_touch_switch = true;
>>> > }
>>> > err = wacom_create_slots(wacom_wac);
>>> > @@ -2647,7 +2646,6 @@ int wacom_setup_input_capabilities(struct
>>> > input_dev *input_dev,
>>> > if ((features->device_type == BTN_TOOL_FINGER) &&
>>> > (input_dev->id.product >= 0x353 &&
>>> > input_dev->id.product <= 0x356)) {
>>> > input_dev->evbit[0] |= BIT_MASK(EV_SW);
>>> > - __set_bit(SW_MUTE_DEVICE, input_dev->swbit);
>>> > wacom_wac->shared->has_mute_touch_switch = true;
>>> > wacom_wac->shared->is_touch_on = true;
>>> > }
>>> > @@ -2693,7 +2691,6 @@ int wacom_setup_input_capabilities(struct
>>> > input_dev *input_dev,
>>> > if (features->touch_max &&
>>> > features->device_type == BTN_TOOL_FINGER) {
>>> > input_dev->evbit[0] |= BIT_MASK(EV_SW);
>>> > - __set_bit(SW_MUTE_DEVICE, input_dev->swbit);
>>> > wacom_wac->shared->has_mute_touch_switch = true;
>>> > }
>>> > /* fall through */
>>> > diff --git a/3.7/wacom_wac.c b/3.7/wacom_wac.c
>>> > index c91093e13de3..42b2050eb1e4 100644
>>> > --- a/3.7/wacom_wac.c
>>> > +++ b/3.7/wacom_wac.c
>>> > @@ -2511,7 +2511,6 @@ int wacom_setup_input_capabilities(struct
>>> > input_dev *input_dev,
>>> > }
>>> > else {
>>> > input_dev->evbit[0] |= BIT_MASK(EV_SW);
>>> > - __set_bit(SW_MUTE_DEVICE, input_dev->swbit);
>>> > wacom_wac->shared->has_mute_touch_switch = true;
>>> > }
>>> > /* fall through */
>>> > @@ -2592,7 +2591,6 @@ int wacom_setup_input_capabilities(struct
>>> > input_dev *input_dev,
>>> > if ((features->device_type == BTN_TOOL_FINGER) &&
>>> > (input_dev->id.product >= 0x353 &&
>>> > input_dev->id.product <= 0x356)) {
>>> > input_dev->evbit[0] |= BIT_MASK(EV_SW);
>>> > - __set_bit(SW_MUTE_DEVICE, input_dev->swbit);
>>> > wacom_wac->shared->has_mute_touch_switch = true;
>>> > wacom_wac->shared->is_touch_on = true;
>>> > }
>>> > @@ -2638,7 +2636,6 @@ int wacom_setup_input_capabilities(struct
>>> > input_dev *input_dev,
>>> > if (features->touch_max &&
>>> > features->device_type == BTN_TOOL_FINGER) {
>>> > input_dev->evbit[0] |= BIT_MASK(EV_SW);
>>> > - __set_bit(SW_MUTE_DEVICE, input_dev->swbit);
>>> > wacom_wac->shared->has_mute_touch_switch = true;
>>> > }
>>> > /* fall through */
>>> > --
>>> > 2.7.4
>>> >
>>> >
>>> >
>>> > ------------------------------------------------------------------------------
>>> > Check out the vibrant tech community on one of the world's most
>>> > engaging tech sites, Slashdot.org! http://sdm.link/slashdot
>>> > _______________________________________________
>>> > Linuxwacom-devel mailing list
>>> > Linuxwacom-devel@lists.sourceforge.net
>>> > https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
>>> >
>>>
>>>
>>> ------------------------------------------------------------------------------
>>> Check out the vibrant tech community on one of the world's most
>>> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
>>> _______________________________________________
>>> Linuxwacom-devel mailing list
>>> Linuxwacom-devel@lists.sourceforge.net
>>> https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
