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
