[PATCH V2 0/3] Input: Fix incorrectly halved touchpad range on ELAN v3 touchpads

[PATCH V2 0/3] Input: Fix incorrectly halved touchpad range on ELAN v3 touchpads

[Eirin Nya]

On Linux 5.19.10, on my laptop (Dell Inspiron 15R SE 7520) with an Elan
v3 touchpad, the reported size of my touchpad (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 series 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.

A possible alternative approach is to restructure the driver and set
resolution before querying touchpad size. I did not attempt this myself,
and don't know how the Windows Dell Touchpad ELAN driver handles
double-resolution. See
https://lore.kernel.org/linux-input/20221008093437.72d0f6b0@dell-void.nyanpasu256.gmail.com.beta.tailscale.net/
for discussion.

Changes in v2:
- Fix commit summaries
- Add "Fixes:" tag holding commit that introduced bug

Link: https://lore.kernel.org/linux-input/CAL57YxZNutUVxBtvbVWKMw-V2kqeVB5kTQ5BFdJmN=mdPq8Q8Q@mail.gmail.com/
Link v1: https://lore.kernel.org/linux-input/20220929082119.22112-1-nyanpasu256@gmail.com/

Eirin Nya (3):
  Input: elantech - Remove redundant field elantech_data::y_max
  Input: elantech - Remove redundant field elantech_data::width
  Input: elantech - Fix incorrectly halved touchpad range on ELAN v3
    touchpads

 drivers/input/mouse/elantech.c | 51 +++++++++++++++++++++++-----------
 drivers/input/mouse/elantech.h |  2 --
 2 files changed, 35 insertions(+), 18 deletions(-)


base-commit: d4a596eddb90114f5f5f32a440057a175517b090
-- 
2.38.0

[PATCH V2 1/3] Input: elantech - Remove redundant field elantech_data::y_max

[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>
---
 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 ece97f8c6a..79e31611fc 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 571e6ca11d..1ec004f72d 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.38.0

[PATCH V2 2/3] Input: elantech - Remove redundant field elantech_data::width

[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>
---
 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 79e31611fc..263779c031 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 1ec004f72d..fb093134ea 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.38.0

[PATCH V2 3/3] Input: elantech - Fix incorrectly halved touchpad range on ELAN v3 touchpads

[Eirin Nya]

On Linux 5.19.10, 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>
---

Notes:
    Should we move (elantech_set_absolute_mode ->
    elantech_write_reg(...0x0b or 0x01)) *earlier* into elantech_query_info()
    before "query range information"? See discussion at
    https://lore.kernel.org/linux-input/20221008093437.72d0f6b0@dell-void.nyanpasu256.gmail.com.beta.tailscale.net/

 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 263779c031..a2176f0fd3 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.38.0

Re: [PATCH V2 1/3] Input: elantech - Remove redundant field elantech_data::y_max

[Mattijs Korpershoek]

On Fri, Oct 14, 2022 at 04:15, Eirin Nya <nyanpasu256@gmail.com> wrote:

> 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>

Hi Eirin,

Thank you for v2. I know you're new to kernel development so here is
some more "process feedback".

I've already reviewed this in v1:
https://lore.kernel.org/all/87ilkv7ogc.fsf@mkorpershoek-xps-13-9370.home/

After getting a "Reviewed-by" reply on one of the patches, it is
customary to add that in the commit message footer, if the patch is
unchanged. This encourages reviewers and gives them some credit for
their review :)

This is documented at:
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#reviewer-s-statement-of-oversight

To quote the doc:
> Both Tested-by and Reviewed-by tags, once received on mailing list from
> tester or reviewer, should be added by author to the applicable patches
> when sending next versions.

So please, if you have to send a v3 at some point, please add:

Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>

Note that it's not needed to send a v3 *JUST* to include the trailers.
The maintainer will pick them up if he decides to merge this.

> ---
>  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 ece97f8c6a..79e31611fc 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 571e6ca11d..1ec004f72d 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.38.0

Re: [PATCH V2 2/3] Input: elantech - Remove redundant field elantech_data::width

[Mattijs Korpershoek]

On Fri, Oct 14, 2022 at 04:15, Eirin Nya <nyanpasu256@gmail.com> wrote:

> 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>

Same remark as for patch v2 1/3: I already reviewed v1.

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 79e31611fc..263779c031 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 1ec004f72d..fb093134ea 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.38.0

Re: [PATCH V2 3/3] Input: elantech - Fix incorrectly halved touchpad range on ELAN v3 touchpads

[Mattijs Korpershoek]

On Fri, Oct 14, 2022 at 04:15, Eirin Nya <nyanpasu256@gmail.com> wrote:

> On Linux 5.19.10, 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>
> ---
>
> Notes:
>     Should we move (elantech_set_absolute_mode ->
>     elantech_write_reg(...0x0b or 0x01)) *earlier* into elantech_query_info()
>     before "query range information"? See discussion at
>     https://lore.kernel.org/linux-input/20221008093437.72d0f6b0@dell-void.nyanpasu256.gmail.com.beta.tailscale.net/

I don't think it's a problem to query twice. To me the patch looks fine
as is.

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 263779c031..a2176f0fd3 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.38.0

Re: [PATCH V2 1/3] Input: elantech - Remove redundant field elantech_data::y_max

[Eirin Nya]

Hi,

Is there any progress on incorporating this patch into mainline Linux?
Every time my rolling distro updates its kernel minor/patch version, I
get a broken touchpad until I rebase and reapply my patch, then
rebuild and install the psmouse kernel module. I've had to do this so
many times I wrote a script to automate this process for new kernel
patch versions (and have to fix the script for new minor/major
versions since I didn't figure out string parsing in Bash, or if I
switched Linux distros).

Thanks,
Eirin

On 10/14/22 6:10 AM, Mattijs Korpershoek wrote:
> On Fri, Oct 14, 2022 at 04:15, Eirin Nya <nyanpasu256@gmail.com> wrote:
> 
>> 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>
> 
> Hi Eirin,
> 
> Thank you for v2. I know you're new to kernel development so here is
> some more "process feedback".
> 
> I've already reviewed this in v1:
> https://lore.kernel.org/all/87ilkv7ogc.fsf@mkorpershoek-xps-13-9370.home/
> 
> After getting a "Reviewed-by" reply on one of the patches, it is
> customary to add that in the commit message footer, if the patch is
> unchanged. This encourages reviewers and gives them some credit for
> their review :)
> 
> This is documented at:
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html#reviewer-s-statement-of-oversight
> 
> To quote the doc:
>> Both Tested-by and Reviewed-by tags, once received on mailing list from
>> tester or reviewer, should be added by author to the applicable patches
>> when sending next versions.
> 
> So please, if you have to send a v3 at some point, please add:
> 
> Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
> 
> Note that it's not needed to send a v3 *JUST* to include the trailers.
> The maintainer will pick them up if he decides to merge this.
> 
>> ---
>>  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 ece97f8c6a..79e31611fc 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 571e6ca11d..1ec004f72d 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.38.0

Re: [PATCH V2 1/3] Input: elantech - Remove redundant field elantech_data::y_max

[Mattijs Korpershoek]

Hi Eirin,

On Mon, Nov 28, 2022 at 01:18, Eirin Nya <nyanpasu256@gmail.com> wrote:

> Hi,
>
> Is there any progress on incorporating this patch into mainline Linux?

Dmitry (in To: now) is a very busy maintainer. It's possible that he
missed this series.

If in a week you haven't heard back from him, I suggest you do a RESEND
as documented in [1]

Don't get discouraged, and thank you for your patience!

Mattijs

[1] https://www.kernel.org/doc/html/latest/process/submitting-patches.html?highlight=resend#don-t-get-discouraged-or-impatient

> Every time my rolling distro updates its kernel minor/patch version, I
> get a broken touchpad until I rebase and reapply my patch, then
> rebuild and install the psmouse kernel module. I've had to do this so
> many times I wrote a script to automate this process for new kernel
> patch versions (and have to fix the script for new minor/major
> versions since I didn't figure out string parsing in Bash, or if I
> switched Linux distros).
>
> Thanks,
> Eirin
>
> On 10/14/22 6:10 AM, Mattijs Korpershoek wrote:
>> On Fri, Oct 14, 2022 at 04:15, Eirin Nya <nyanpasu256@gmail.com> wrote:
>> 
>>> 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>
>> 
>> Hi Eirin,
>> 
>> Thank you for v2. I know you're new to kernel development so here is
>> some more "process feedback".
>> 
>> I've already reviewed this in v1:
>> https://lore.kernel.org/all/87ilkv7ogc.fsf@mkorpershoek-xps-13-9370.home/
>> 
>> After getting a "Reviewed-by" reply on one of the patches, it is
>> customary to add that in the commit message footer, if the patch is
>> unchanged. This encourages reviewers and gives them some credit for
>> their review :)
>> 
>> This is documented at:
>> https://www.kernel.org/doc/html/latest/process/submitting-patches.html#reviewer-s-statement-of-oversight
>> 
>> To quote the doc:
>>> Both Tested-by and Reviewed-by tags, once received on mailing list from
>>> tester or reviewer, should be added by author to the applicable patches
>>> when sending next versions.
>> 
>> So please, if you have to send a v3 at some point, please add:
>> 
>> Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
>> 
>> Note that it's not needed to send a v3 *JUST* to include the trailers.
>> The maintainer will pick them up if he decides to merge this.
>> 
>>> ---
>>>  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 ece97f8c6a..79e31611fc 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 571e6ca11d..1ec004f72d 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.38.0

Re: [PATCH V2 3/3] Input: elantech - Fix incorrectly halved touchpad range on ELAN v3 touchpads

[Dmitry Torokhov]

On Fri, Oct 14, 2022 at 04:15:33AM -0700, Eirin Nya wrote:
> On Linux 5.19.10, 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.

If the setting is indeed double resolution, can we simply multiply x_max
and y_max by 2 instead of re-querying it?

Also let's try adding one of Elan engineers for their take in this.
Phoenix, do you have any suggestions please?

> 
> 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>
> ---
> 
> Notes:
>     Should we move (elantech_set_absolute_mode ->
>     elantech_write_reg(...0x0b or 0x01)) *earlier* into elantech_query_info()
>     before "query range information"? See discussion at
>     https://lore.kernel.org/linux-input/20221008093437.72d0f6b0@dell-void.nyanpasu256.gmail.com.beta.tailscale.net/
> 
>  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 263779c031..a2176f0fd3 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.38.0
> 

Thanks.

-- 
Dmitry

Re: [PATCH V2 3/3] Input: elantech - Fix incorrectly halved touchpad range on ELAN v3 touchpads

[Dmitry Torokhov]

On Mon, Nov 28, 2022 at 09:57:51AM -0800, Dmitry Torokhov wrote:
> On Fri, Oct 14, 2022 at 04:15:33AM -0700, Eirin Nya wrote:
> > On Linux 5.19.10, 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.
> 
> If the setting is indeed double resolution, can we simply multiply x_max
> and y_max by 2 instead of re-querying it?
> 
> Also let's try adding one of Elan engineers for their take in this.
> Phoenix, do you have any suggestions please?

Argh, adding Phoenix for real now.

> 
> > 
> > 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>
> > ---
> > 
> > Notes:
> >     Should we move (elantech_set_absolute_mode ->
> >     elantech_write_reg(...0x0b or 0x01)) *earlier* into elantech_query_info()
> >     before "query range information"? See discussion at
> >     https://lore.kernel.org/linux-input/20221008093437.72d0f6b0@dell-void.nyanpasu256.gmail.com.beta.tailscale.net/
> > 
> >  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 263779c031..a2176f0fd3 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.38.0
> > 
> 
> Thanks.
> 
> -- 
> Dmitry

-- 
Dmitry

Re: [PATCH V2 1/3] Input: elantech - Remove redundant field elantech_data::y_max

[Dmitry Torokhov]

On Mon, Nov 28, 2022 at 02:29:03PM +0100, Mattijs Korpershoek wrote:
> Hi Eirin,
> 
> On Mon, Nov 28, 2022 at 01:18, Eirin Nya <nyanpasu256@gmail.com> wrote:
> 
> > Hi,
> >
> > Is there any progress on incorporating this patch into mainline Linux?
> 
> Dmitry (in To: now) is a very busy maintainer. It's possible that he
> missed this series.
> 
> If in a week you haven't heard back from him, I suggest you do a RESEND
> as documented in [1]
> 
> Don't get discouraged, and thank you for your patience!

Hi, yeah, sorry, I have most patches either sent to me or I am CCed on
them so I rarely look into the mailing list itself. Please just copy me
next time you send out a patch.

Thanks.

-- 
Dmitry

RE: [PATCH V2 3/3] Input: elantech - Fix incorrectly halved touchpad range on ELAN v3 touchpads

[phoenix]

Loop Josh

-----Original Message-----
From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com] 
Sent: Tuesday, November 29, 2022 1:59 AM
To: Eirin Nya <nyanpasu256@gmail.com>; Phoenix Huang <phoenix@emc.com.tw>
Cc: linux-input@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH V2 3/3] Input: elantech - Fix incorrectly halved
touchpad range on ELAN v3 touchpads

On Mon, Nov 28, 2022 at 09:57:51AM -0800, Dmitry Torokhov wrote:
> On Fri, Oct 14, 2022 at 04:15:33AM -0700, Eirin Nya wrote:
> > On Linux 5.19.10, 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.
> 
> If the setting is indeed double resolution, can we simply multiply 
> x_max and y_max by 2 instead of re-querying it?
> 
> Also let's try adding one of Elan engineers for their take in this.
> Phoenix, do you have any suggestions please?

Argh, adding Phoenix for real now.

> 
> > 
> > Link: 
> > https://lore.kernel.org/linux-input/CAL57YxZNutUVxBtvbVWKMw-V2kqeVB5
> > kTQ5BFdJmN=mdPq8Q8Q@mail.gmail.com/
> > Link: 
> > https://lore.kernel.org/linux-input/20221008093437.72d0f6b0@dell-voi
> > d.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>
> > ---
> > 
> > Notes:
> >     Should we move (elantech_set_absolute_mode ->
> >     elantech_write_reg(...0x0b or 0x01)) *earlier* into
elantech_query_info()
> >     before "query range information"? See discussion at
> >     
> > https://lore.kernel.org/linux-input/20221008093437.72d0f6b0@dell-voi
> > d.nyanpasu256.gmail.com.beta.tailscale.net/
> > 
> >  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 263779c031..a2176f0fd3 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.38.0
> > 
> 
> Thanks.
> 
> --
> Dmitry

--
Dmitry

RE: [PATCH V2 3/3] Input: elantech - Fix incorrectly halved touchpad range on ELAN v3 touchpads

[phoenix]

Consulted with FW team, we suggest re-querying x/y resolution after setting
absolute mode.

-----Original Message-----
From: phoenix [mailto:phoenix@emc.com.tw] 
Sent: Tuesday, November 29, 2022 11:47 AM
To: 'Dmitry Torokhov' <dmitry.torokhov@gmail.com>; 'Eirin Nya'
<nyanpasu256@gmail.com>
Cc: 'linux-input@vger.kernel.org' <linux-input@vger.kernel.org>;
'linux-kernel@vger.kernel.org' <linux-kernel@vger.kernel.org>; 'Josh.Chen'
<josh.chen@emc.com.tw>
Subject: RE: [PATCH V2 3/3] Input: elantech - Fix incorrectly halved
touchpad range on ELAN v3 touchpads

Loop Josh

-----Original Message-----
From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com]
Sent: Tuesday, November 29, 2022 1:59 AM
To: Eirin Nya <nyanpasu256@gmail.com>; Phoenix Huang <phoenix@emc.com.tw>
Cc: linux-input@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH V2 3/3] Input: elantech - Fix incorrectly halved
touchpad range on ELAN v3 touchpads

On Mon, Nov 28, 2022 at 09:57:51AM -0800, Dmitry Torokhov wrote:
> On Fri, Oct 14, 2022 at 04:15:33AM -0700, Eirin Nya wrote:
> > On Linux 5.19.10, 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.
> 
> If the setting is indeed double resolution, can we simply multiply 
> x_max and y_max by 2 instead of re-querying it?
> 
> Also let's try adding one of Elan engineers for their take in this.
> Phoenix, do you have any suggestions please?

Argh, adding Phoenix for real now.

> 
> > 
> > Link: 
> > https://lore.kernel.org/linux-input/CAL57YxZNutUVxBtvbVWKMw-V2kqeVB5
> > kTQ5BFdJmN=mdPq8Q8Q@mail.gmail.com/
> > Link: 
> > https://lore.kernel.org/linux-input/20221008093437.72d0f6b0@dell-voi
> > d.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>
> > ---
> > 
> > Notes:
> >     Should we move (elantech_set_absolute_mode ->
> >     elantech_write_reg(...0x0b or 0x01)) *earlier* into
elantech_query_info()
> >     before "query range information"? See discussion at
> >     
> > https://lore.kernel.org/linux-input/20221008093437.72d0f6b0@dell-voi
> > d.nyanpasu256.gmail.com.beta.tailscale.net/
> > 
> >  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 263779c031..a2176f0fd3 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.38.0
> > 
> 
> Thanks.
> 
> --
> Dmitry

--
Dmitry

Re: [PATCH V2 3/3] Input: elantech - Fix incorrectly halved touchpad range on ELAN v3 touchpads

[Eirin Nya]

> If the setting is indeed double resolution, can we simply multiply 
> x_max and y_max by 2 instead of re-querying it?

Perhaps in some laptops, x_max or y_max is odd when resolution is
doubled, and rounded up or down in half-resolution mode, so multiplying
the half-resolution size by 2 results in a slightly incorrect full
size. I don't know if that's the case in other touchpads, but my
laptop's touchpad has even size in full-resolution (doubled) mode,
meaning it doesn't round its size in half-resolution mode (so
multiplying both values by 2 would work on my machine).

On Wed, 30 Nov 2022 19:22:25 +0800
"phoenix" <phoenix@emc.com.tw> wrote:

> Consulted with FW team, we suggest re-querying x/y resolution after
> setting absolute mode.

My current patch re-queries x/y resolution after setting absolute mode.
If this is the process that Elan's FW team recommends, should the patch
be kept as-is?

Re: [PATCH V2 3/3] Input: elantech - Fix incorrectly halved touchpad range on ELAN v3 touchpads

[Mattijs Korpershoek]

Hi Eirin,

On Fri, Dec 02, 2022 at 23:42, Eirin Nya <nyanpasu256@gmail.com> wrote:

>> If the setting is indeed double resolution, can we simply multiply 
>> x_max and y_max by 2 instead of re-querying it?
>
> Perhaps in some laptops, x_max or y_max is odd when resolution is
> doubled, and rounded up or down in half-resolution mode, so multiplying
> the half-resolution size by 2 results in a slightly incorrect full
> size. I don't know if that's the case in other touchpads, but my
> laptop's touchpad has even size in full-resolution (doubled) mode,
> meaning it doesn't round its size in half-resolution mode (so
> multiplying both values by 2 would work on my machine).
>
> On Wed, 30 Nov 2022 19:22:25 +0800
> "phoenix" <phoenix@emc.com.tw> wrote:
>
>> Consulted with FW team, we suggest re-querying x/y resolution after
>> setting absolute mode.
>
> My current patch re-queries x/y resolution after setting absolute mode.
> If this is the process that Elan's FW team recommends, should the patch
> be kept as-is?

Yes, I think you can keep it the way it is. Dmitry will either pick it
up or suggest additional changes.