* [PATCH V2 RESEND 1/3] Input: elantech - Remove redundant field elantech_data::y_max
2023-03-08 13:13 [PATCH V2 RESEND 0/3] Input: elantech - Fix incorrectly halved touchpad range on ELAN v3 touchpads Eirin Nya
@ 2023-03-08 13:13 ` Eirin Nya
2023-03-08 13:13 ` [PATCH V2 RESEND 2/3] Input: elantech - Remove redundant field elantech_data::width Eirin Nya
2023-03-08 13:13 ` [PATCH V2 RESEND 3/3] Input: elantech - Fix incorrectly halved touchpad range on ELAN v3 touchpads Eirin Nya
2 siblings, 0 replies; 4+ messages in thread
From: Eirin Nya @ 2023-03-08 13:13 UTC (permalink / raw)
To: dmitry.torokhov
Cc: mkorpershoek, phoenix, josh.chen, linux-input, linux-kernel,
Eirin Nya
elantech_data::y_max is copied from elantech_device_info::y_max, and
neither is mutated after initialization. So remove the redundant
variable to prevent future bugs when updating y_max.
Signed-off-by: Eirin Nya <nyanpasu256@gmail.com>
Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
---
drivers/input/mouse/elantech.c | 17 ++++++++---------
drivers/input/mouse/elantech.h | 1 -
2 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index ece97f8c6a3e..79e31611fc6e 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -360,7 +360,7 @@ static void elantech_report_absolute_v1(struct psmouse *psmouse)
input_report_abs(dev, ABS_X,
((packet[1] & 0x0c) << 6) | packet[2]);
input_report_abs(dev, ABS_Y,
- etd->y_max - (((packet[1] & 0x03) << 8) | packet[3]));
+ etd->info.y_max - (((packet[1] & 0x03) << 8) | packet[3]));
}
input_report_key(dev, BTN_TOOL_FINGER, fingers == 1);
@@ -435,7 +435,7 @@ static void elantech_report_absolute_v2(struct psmouse *psmouse)
* byte 4: . . . . y11 y10 y9 y8
* byte 5: y7 y6 y5 y4 y3 y2 y1 y0
*/
- y1 = etd->y_max - (((packet[4] & 0x0f) << 8) | packet[5]);
+ y1 = etd->info.y_max - (((packet[4] & 0x0f) << 8) | packet[5]);
pres = (packet[1] & 0xf0) | ((packet[4] & 0xf0) >> 4);
width = ((packet[0] & 0x30) >> 2) | ((packet[3] & 0x30) >> 4);
@@ -450,7 +450,7 @@ static void elantech_report_absolute_v2(struct psmouse *psmouse)
*/
x1 = (((packet[0] & 0x10) << 4) | packet[1]) << 2;
/* byte 2: ay7 ay6 ay5 ay4 ay3 ay2 ay1 ay0 */
- y1 = etd->y_max -
+ y1 = etd->info.y_max -
((((packet[0] & 0x20) << 3) | packet[2]) << 2);
/*
* byte 3: . . by8 bx8 . . . .
@@ -458,7 +458,7 @@ static void elantech_report_absolute_v2(struct psmouse *psmouse)
*/
x2 = (((packet[3] & 0x10) << 4) | packet[4]) << 2;
/* byte 5: by7 by8 by5 by4 by3 by2 by1 by0 */
- y2 = etd->y_max -
+ y2 = etd->info.y_max -
((((packet[3] & 0x20) << 3) | packet[5]) << 2);
/* Unknown so just report sensible values */
@@ -579,7 +579,7 @@ static void elantech_report_absolute_v3(struct psmouse *psmouse,
* byte 4: . . . . y11 y10 y9 y8
* byte 5: y7 y6 y5 y4 y3 y2 y1 y0
*/
- y1 = etd->y_max - (((packet[4] & 0x0f) << 8) | packet[5]);
+ y1 = etd->info.y_max - (((packet[4] & 0x0f) << 8) | packet[5]);
break;
case 2:
@@ -593,7 +593,7 @@ static void elantech_report_absolute_v3(struct psmouse *psmouse,
* byte 4: . . . . ay11 ay10 ay9 ay8
* byte 5: ay7 ay6 ay5 ay4 ay3 ay2 ay1 ay0
*/
- etd->mt[0].y = etd->y_max -
+ etd->mt[0].y = etd->info.y_max -
(((packet[4] & 0x0f) << 8) | packet[5]);
/*
* wait for next packet
@@ -605,7 +605,7 @@ static void elantech_report_absolute_v3(struct psmouse *psmouse,
x1 = etd->mt[0].x;
y1 = etd->mt[0].y;
x2 = ((packet[1] & 0x0f) << 8) | packet[2];
- y2 = etd->y_max - (((packet[4] & 0x0f) << 8) | packet[5]);
+ y2 = etd->info.y_max - (((packet[4] & 0x0f) << 8) | packet[5]);
break;
}
@@ -681,7 +681,7 @@ static void process_packet_head_v4(struct psmouse *psmouse)
return;
etd->mt[id].x = ((packet[1] & 0x0f) << 8) | packet[2];
- etd->mt[id].y = etd->y_max - (((packet[4] & 0x0f) << 8) | packet[5]);
+ etd->mt[id].y = etd->info.y_max - (((packet[4] & 0x0f) << 8) | packet[5]);
pres = (packet[1] & 0xf0) | ((packet[4] & 0xf0) >> 4);
traces = (packet[0] & 0xf0) >> 4;
@@ -1253,7 +1253,6 @@ static int elantech_set_input_params(struct psmouse *psmouse)
input_abs_set_res(dev, ABS_MT_POSITION_Y, info->y_res);
}
- etd->y_max = y_max;
etd->width = width;
return 0;
diff --git a/drivers/input/mouse/elantech.h b/drivers/input/mouse/elantech.h
index 571e6ca11d33..1ec004f72d13 100644
--- a/drivers/input/mouse/elantech.h
+++ b/drivers/input/mouse/elantech.h
@@ -180,7 +180,6 @@ struct elantech_data {
unsigned char reg_25;
unsigned char reg_26;
unsigned int single_finger_reports;
- unsigned int y_max;
unsigned int width;
struct finger_pos mt[ETP_MAX_FINGERS];
unsigned char parity[256];
--
2.39.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH V2 RESEND 2/3] Input: elantech - Remove redundant field elantech_data::width
2023-03-08 13:13 [PATCH V2 RESEND 0/3] Input: elantech - Fix incorrectly halved touchpad range on ELAN v3 touchpads Eirin Nya
2023-03-08 13:13 ` [PATCH V2 RESEND 1/3] Input: elantech - Remove redundant field elantech_data::y_max Eirin Nya
@ 2023-03-08 13:13 ` Eirin Nya
2023-03-08 13:13 ` [PATCH V2 RESEND 3/3] Input: elantech - Fix incorrectly halved touchpad range on ELAN v3 touchpads Eirin Nya
2 siblings, 0 replies; 4+ messages in thread
From: Eirin Nya @ 2023-03-08 13:13 UTC (permalink / raw)
To: dmitry.torokhov
Cc: mkorpershoek, phoenix, josh.chen, linux-input, linux-kernel,
Eirin Nya
elantech_data::width is copied from elantech_device_info::width, and
neither is mutated after initialization. So remove the redundant
variable to prevent future bugs.
Signed-off-by: Eirin Nya <nyanpasu256@gmail.com>
Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
---
drivers/input/mouse/elantech.c | 4 +---
drivers/input/mouse/elantech.h | 1 -
2 files changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index 79e31611fc6e..263779c0313b 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -691,7 +691,7 @@ static void process_packet_head_v4(struct psmouse *psmouse)
input_report_abs(dev, ABS_MT_POSITION_X, etd->mt[id].x);
input_report_abs(dev, ABS_MT_POSITION_Y, etd->mt[id].y);
input_report_abs(dev, ABS_MT_PRESSURE, pres);
- input_report_abs(dev, ABS_MT_TOUCH_MAJOR, traces * etd->width);
+ input_report_abs(dev, ABS_MT_TOUCH_MAJOR, traces * etd->info.width);
/* report this for backwards compatibility */
input_report_abs(dev, ABS_TOOL_WIDTH, traces);
@@ -1253,8 +1253,6 @@ static int elantech_set_input_params(struct psmouse *psmouse)
input_abs_set_res(dev, ABS_MT_POSITION_Y, info->y_res);
}
- etd->width = width;
-
return 0;
}
diff --git a/drivers/input/mouse/elantech.h b/drivers/input/mouse/elantech.h
index 1ec004f72d13..fb093134ead7 100644
--- a/drivers/input/mouse/elantech.h
+++ b/drivers/input/mouse/elantech.h
@@ -180,7 +180,6 @@ struct elantech_data {
unsigned char reg_25;
unsigned char reg_26;
unsigned int single_finger_reports;
- unsigned int width;
struct finger_pos mt[ETP_MAX_FINGERS];
unsigned char parity[256];
struct elantech_device_info info;
--
2.39.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH V2 RESEND 3/3] Input: elantech - Fix incorrectly halved touchpad range on ELAN v3 touchpads
2023-03-08 13:13 [PATCH V2 RESEND 0/3] Input: elantech - Fix incorrectly halved touchpad range on ELAN v3 touchpads Eirin Nya
2023-03-08 13:13 ` [PATCH V2 RESEND 1/3] Input: elantech - Remove redundant field elantech_data::y_max Eirin Nya
2023-03-08 13:13 ` [PATCH V2 RESEND 2/3] Input: elantech - Remove redundant field elantech_data::width Eirin Nya
@ 2023-03-08 13:13 ` Eirin Nya
2 siblings, 0 replies; 4+ messages in thread
From: Eirin Nya @ 2023-03-08 13:13 UTC (permalink / raw)
To: dmitry.torokhov
Cc: mkorpershoek, phoenix, josh.chen, linux-input, linux-kernel,
Eirin Nya
On Linux 6.1.15, on my laptop (Dell Inspiron 15R SE 7520) with an Elan
v3 touchpad (dmesg says "with firmware version 0x450f02"), the reported
size of my touchpad (in userspace by calling mtdev_configure() and
libevdev_get_abs_maximum(), in kernel space
elantech_device_info::x_max/y_max, either way 1470 by 700) is half that
of the actual touch range (2940 by 1400), and the upper half of my
touchpad reports negative values. As a result, with the Synaptics or
libinput X11 driver set to edge scrolling mode, the entire right half of
my touchpad has x-values past evdev's reported maximum size, and acts as
a giant scrollbar!
The problem is that elantech_setup_ps2() -> elantech_set_absolute_mode()
sets up absolute mode and doubles the hardware resolution (doubling the
hardware's maximum reported x/y coordinates and its response to
ETP_FW_ID_QUERY), *after* elantech_query_info() fetches the touchpad
coordinate system size using ETP_FW_ID_QUERY, which gets cached and
reported to userspace through ioctl(fd, EVIOCGABS(ABS_X/Y), ...). So the
touchpad size reported to userspace (and used to subtract vertical
coordinates from) is half the maximum position of actual touches.
This patch splits out a function elantech_query_range_v3() which fetches
*only* ETP_FW_ID_QUERY (touchpad size), and calls it a second time if
elantech_set_absolute_mode() enables double-size mode. This means the
first call is redundant and wasted if a second call occurs, but this
minimizes the need to restructure the driver.
Link: https://lore.kernel.org/linux-input/CAL57YxZNutUVxBtvbVWKMw-V2kqeVB5kTQ5BFdJmN=mdPq8Q8Q@mail.gmail.com/
Link: https://lore.kernel.org/linux-input/20221008093437.72d0f6b0@dell-void.nyanpasu256.gmail.com.beta.tailscale.net/
Fixes: 37548659bb22 ("Input: elantech - query the min/max information beforehand too")
Signed-off-by: Eirin Nya <nyanpasu256@gmail.com>
Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
---
drivers/input/mouse/elantech.c | 30 ++++++++++++++++++++++++++----
1 file changed, 26 insertions(+), 4 deletions(-)
diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index 263779c0313b..a2176f0fd36c 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -1006,6 +1006,9 @@ static void elantech_set_rate_restore_reg_07(struct psmouse *psmouse,
psmouse_err(psmouse, "restoring reg_07 failed\n");
}
+static int elantech_query_range_v3(struct psmouse *psmouse,
+ struct elantech_device_info *info);
+
/*
* Put the touchpad into absolute mode
*/
@@ -1047,6 +1050,14 @@ static int elantech_set_absolute_mode(struct psmouse *psmouse)
if (elantech_write_reg(psmouse, 0x10, etd->reg_10))
rc = -1;
+ /*
+ * If we boost hardware resolution, we have to re-query
+ * info->x_max and y_max.
+ */
+ if (etd->info.set_hw_resolution)
+ if (elantech_query_range_v3(psmouse, &etd->info))
+ rc = -1;
+
break;
case 4:
@@ -1671,6 +1682,20 @@ static int elantech_set_properties(struct elantech_device_info *info)
return 0;
}
+static int elantech_query_range_v3(struct psmouse *psmouse,
+ struct elantech_device_info *info)
+{
+ unsigned char param[3];
+
+ if (info->send_cmd(psmouse, ETP_FW_ID_QUERY, param))
+ return -EINVAL;
+
+ info->x_max = (0x0f & param[0]) << 8 | param[1];
+ info->y_max = (0xf0 & param[0]) << 4 | param[2];
+
+ return 0;
+}
+
static int elantech_query_info(struct psmouse *psmouse,
struct elantech_device_info *info)
{
@@ -1826,11 +1851,8 @@ static int elantech_query_info(struct psmouse *psmouse,
break;
case 3:
- if (info->send_cmd(psmouse, ETP_FW_ID_QUERY, param))
+ if (elantech_query_range_v3(psmouse, info))
return -EINVAL;
-
- info->x_max = (0x0f & param[0]) << 8 | param[1];
- info->y_max = (0xf0 & param[0]) << 4 | param[2];
break;
case 4:
--
2.39.2
^ permalink raw reply related [flat|nested] 4+ messages in thread