From: Sergei A. Trusov <hidden> Date: 2017-06-17 09:49:02
On some x86 tablets with a goodix touchscreen the windows logo on the
front is a capacitive home button. Touching this button results in a touch
with bit 4 of the 0th byte set, while normally only the lower 4 bits
are used to indicate the number of touches.
Detect this and report a KEY_LEFTMETA press when this happens. Note the
hardware might support more than one button, the number of a button is
reported by the 'id' byte of coor_data. Now we ignore button id.
Signed-off-by: Sergei A. Trusov <redacted>
---
drivers/input/touchscreen/goodix.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
From: Hans de Goede <hidden> Date: 2017-06-17 19:34:55
Hi,
On 17-06-17 11:42, Sergei A. Trusov wrote:
On some x86 tablets with a goodix touchscreen the windows logo on the
front is a capacitive home button. Touching this button results in a touch
with bit 4 of the 0th byte set, while normally only the lower 4 bits
are used to indicate the number of touches.
Detect this and report a KEY_LEFTMETA press when this happens. Note the
hardware might support more than one button, the number of a button is
reported by the 'id' byte of coor_data. Now we ignore button id.
Signed-off-by: Sergei A. Trusov <redacted>
The changes look good to me:
Acked-by: Hans de Goede <redacted>
Also this makes the capacitive home button on my HP stream 7 work, so:
Tested-by: Hans de Goede <redacted>
Regards,
Hans
On Sat, 2017-06-17 at 19:42 +1000, Sergei A. Trusov wrote:
quoted hunk
On some x86 tablets with a goodix touchscreen the windows logo on the
front is a capacitive home button. Touching this button results in a
touch
with bit 4 of the 0th byte set, while normally only the lower 4 bits
are used to indicate the number of touches.
Detect this and report a KEY_LEFTMETA press when this happens. Note
the
hardware might support more than one button, the number of a button
is
reported by the 'id' byte of coor_data. Now we ignore button id.
Signed-off-by: Sergei A. Trusov <redacted>
---
drivers/input/touchscreen/goodix.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/drivers/input/touchscreen/goodix.c
b/drivers/input/touchscreen/goodix.c
index 240b16f3ee97..a4c17c461792 100644
I really don't like it when we change the meaning of a function's
return value without changing the function's name, or explaining that
we're doing that. This should be a separate patch please.
I don't like this define though, please use KEY_LEFTMETA directly. We
can change it to a constant when we have more than one softbutton.
General question though, should we not only export and advertise the
button if it exists on the device?
+
error = input_register_device(ts->input_dev);
if (error) {
dev_err(&ts->client->dev,
On Tue, Jun 20, 2017 at 12:32:03AM +0200, Bastien Nocera wrote:
On Sat, 2017-06-17 at 19:42 +1000, Sergei A. Trusov wrote:
quoted
On some x86 tablets with a goodix touchscreen the windows logo on the
front is a capacitive home button. Touching this button results in a
touch
with bit 4 of the 0th byte set, while normally only the lower 4 bits
are used to indicate the number of touches.
Detect this and report a KEY_LEFTMETA press when this happens. Note
the
hardware might support more than one button, the number of a button
is
reported by the 'id' byte of coor_data. Now we ignore button id.
Signed-off-by: Sergei A. Trusov <redacted>
---
drivers/input/touchscreen/goodix.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/drivers/input/touchscreen/goodix.c
b/drivers/input/touchscreen/goodix.c
index 240b16f3ee97..a4c17c461792 100644
I really don't like it when we change the meaning of a function's
return value without changing the function's name, or explaining that
we're doing that. This should be a separate patch please.
I don't like this define though, please use KEY_LEFTMETA directly. We
can change it to a constant when we have more than one softbutton.
General question though, should we not only export and advertise the
button if it exists on the device?
Yes, this is preferred behavior.
quoted
+
error = input_register_device(ts->input_dev);
if (error) {
dev_err(&ts->client->dev,
From: Hans de Goede <hidden> Date: 2017-06-20 09:02:48
Hi,
On 20-06-17 10:36, Bastien Nocera wrote:
On Mon, 2017-06-19 at 19:51 -0700, Dmitry Torokhov wrote:
quoted
quoted
General question though, should we not only export and advertise
the
button if it exists on the device?
Yes, this is preferred behavior.
So we'll want a dmi match table for the devices that need this feature,
and only export the button for those.
Note Dmitry said that this is preferred behavior, not something which
we MUST do. And TBH I don't think that using a DMI table is a good idea,
the capacitive home-button is not only found on the HP stream 7 but also
on various cheap tablets, of which there tend to be a lot, so the DMI
table is going to be use. And the cheap tablets often have useful
model strings such as e.g. "Default String" making DMI matching a
nasty problem.
So IMHO it is better to just always report the presence of the
home button, in practice I don't think userspace is going to have
any problems with this.
Regards,
Hans
From: Sergei A. Trusov <hidden> Date: 2017-06-20 09:32:56
On Tuesday, 20 June 2017 08:32:03 +10 Bastien Nocera wrote:
On Sat, 2017-06-17 at 19:42 +1000, Sergei A. Trusov wrote:
quoted
On some x86 tablets with a goodix touchscreen the windows logo on the
front is a capacitive home button. Touching this button results in a
touch
with bit 4 of the 0th byte set, while normally only the lower 4 bits
are used to indicate the number of touches.
Detect this and report a KEY_LEFTMETA press when this happens. Note
the
hardware might support more than one button, the number of a button
is
reported by the 'id' byte of coor_data. Now we ignore button id.
Signed-off-by: Sergei A. Trusov <redacted>
---
drivers/input/touchscreen/goodix.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/drivers/input/touchscreen/goodix.c
b/drivers/input/touchscreen/goodix.c
index 240b16f3ee97..a4c17c461792 100644
I really don't like it when we change the meaning of a function's
return value without changing the function's name, or explaining that
we're doing that. This should be a separate patch please.
Thank you for this point. Now I see all the changes above are redundant:
we can use a value returned in the *data buffer. It is enough to add
only two lines of code to achieve the behaviour of this entire patch.
I don't like this define though, please use KEY_LEFTMETA directly. We
can change it to a constant when we have more than one softbutton.
Ok.
General question though, should we not only export and advertise the
button if it exists on the device?
quoted
+
error = input_register_device(ts->input_dev);
if (error) {
dev_err(&ts->client->dev,
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2017-06-20 at 19:26 +1000, Sergei A. Trusov wrote:
<snip>
Thank you for this point. Now I see all the changes above are
redundant:
we can use a value returned in the *data buffer. It is enough to add
only two lines of code to achieve the behaviour of this entire patch.
A bit more with a comment explaining the reason behind those 2 lines,
but shorter patches get my vote ;)
From: Sergei A. Trusov <hidden> Date: 2017-06-20 10:10:57
On Tuesday, 20 June 2017 19:29:01 +10 Bastien Nocera wrote:
On Tue, 2017-06-20 at 19:26 +1000, Sergei A. Trusov wrote:
quoted
<snip>
quoted
Thank you for this point. Now I see all the changes above are
redundant:
we can use a value returned in the *data buffer. It is enough to add
only two lines of code to achieve the behaviour of this entire patch.
A bit more with a comment explaining the reason behind those 2 lines,
but shorter patches get my vote ;)
Current goodix_ts_read_input_report() function puts data from hardware
into point_data buffer. Then in masks high niblle of the first byte and
returns it as touch_num. We need this masked nibble to check, so I was
trying to remove mask operation. After your comment I just realized that
proposed patch should be much shorter. I have included more context lines
to make cleaner my attempts to explain (as I am affraid my English is not
so good):
---
On Tue, 2017-06-20 at 20:11 +1000, Sergei A. Trusov wrote:
On Tuesday, 20 June 2017 19:29:01 +10 Bastien Nocera wrote:
quoted
On Tue, 2017-06-20 at 19:26 +1000, Sergei A. Trusov wrote:
quoted
<snip>
quoted
Thank you for this point. Now I see all the changes above are
redundant:
we can use a value returned in the *data buffer. It is enough to add
only two lines of code to achieve the behaviour of this entire patch.
A bit more with a comment explaining the reason behind those 2 lines,
but shorter patches get my vote ;)
Current goodix_ts_read_input_report() function puts data from hardware
into point_data buffer. Then in masks high niblle of the first byte and
returns it as touch_num. We need this masked nibble to check, so I was
trying to remove mask operation. After your comment I just realized that
proposed patch should be much shorter. I have included more context lines
to make cleaner my attempts to explain (as I am affraid my English is not
so good):
Do you want to re-send with the comments added as below?
From: Sergei A. Trusov <hidden> Date: 2017-06-20 10:52:13
On Tuesday, 20 June 2017 20:20:03 +10 Bastien Nocera wrote:
On Tue, 2017-06-20 at 20:11 +1000, Sergei A. Trusov wrote:
quoted
On Tuesday, 20 June 2017 19:29:01 +10 Bastien Nocera wrote:
quoted
On Tue, 2017-06-20 at 19:26 +1000, Sergei A. Trusov wrote:
quoted
<snip>
quoted
Thank you for this point. Now I see all the changes above are
redundant:
we can use a value returned in the *data buffer. It is enough to add
only two lines of code to achieve the behaviour of this entire patch.
A bit more with a comment explaining the reason behind those 2 lines,
but shorter patches get my vote ;)
Current goodix_ts_read_input_report() function puts data from hardware
into point_data buffer. Then in masks high niblle of the first byte and
returns it as touch_num. We need this masked nibble to check, so I was
trying to remove mask operation. After your comment I just realized that
proposed patch should be much shorter. I have included more context lines
to make cleaner my attempts to explain (as I am affraid my English is not
so good):
Do you want to re-send with the comments added as below?
Add a comment above:
Capacitive Windows/Home button on some devices
quoted
error = input_register_device(ts->input_dev);
if (error) {
dev_err(&ts->client->dev,
"Failed to register input device: %d", error);
return error;
}
return 0;
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html