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

--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c

@@ -76,6 +76,10 @@ struct goodix_ts_data {
 #define MAX_CONTACTS_LOC	5
 #define TRIGGER_LOC		6
 
+#define GOODIX_TOUCH_NUM_MASK		0x0f
+#define GOODIX_SOFTBUTTON_MASK		0x10
+#define GOODIX_SOFTBUTTON1		KEY_LEFTMETA
+
 static const unsigned long goodix_irq_flags[] = {
 	IRQ_TYPE_EDGE_RISING,
 	IRQ_TYPE_EDGE_FALLING,

@@ -208,7 +212,7 @@ static int goodix_ts_read_input_report(struct

goodix_ts_data *ts, u8 *data)
 	if (!(data[0] & 0x80))
 		return -EAGAIN;
 
-	touch_num = data[0] & 0x0f;
+	touch_num = data[0] & GOODIX_TOUCH_NUM_MASK;

This should be a separate patch.

quoted

 	if (touch_num > ts->max_touch_num)
 		return -EPROTO;
 

@@ -223,7 +227,7 @@ static int goodix_ts_read_input_report(struct

goodix_ts_data *ts, u8 *data)
 			return error;
 	}
 
-	return touch_num;
+	return data[0];

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.

quoted

 }
 
 static void goodix_ts_report_touch(struct goodix_ts_data *ts, u8
*coor_data)

@@ -260,13 +264,17 @@ static void goodix_ts_report_touch(struct

goodix_ts_data *ts, u8 *coor_data)
 static void goodix_process_events(struct goodix_ts_data *ts)
 {
 	u8  point_data[1 + GOODIX_CONTACT_SIZE *
GOODIX_MAX_CONTACTS];
-	int touch_num;
+	int touch_num, status;
 	int i;
 
-	touch_num = goodix_ts_read_input_report(ts, point_data);
-	if (touch_num < 0)
+	status = goodix_ts_read_input_report(ts, point_data);
+	if (status < 0)
 		return;
 
+	input_report_key(ts->input_dev, GOODIX_SOFTBUTTON1,
+			!!(status & GOODIX_SOFTBUTTON_MASK));
+
+	touch_num = status & GOODIX_TOUCH_NUM_MASK;
 	for (i = 0; i < touch_num; i++)
 		goodix_ts_report_touch(ts,
 				&point_data[1 + GOODIX_CONTACT_SIZE
* i]);

@@ -612,6 +620,8 @@ static int goodix_request_input_dev(struct

goodix_ts_data *ts)
 	ts->input_dev->id.product = ts->id;
 	ts->input_dev->id.version = ts->version;
 
+	input_set_capability(ts->input_dev, EV_KEY,
GOODIX_SOFTBUTTON1);

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,

-- 
Dmitry