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/[email protected]/
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
On Fri, Oct 14, 2022 at 04:15, Eirin Nya <[email protected]> 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 <[email protected]>
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/[email protected]/
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 <[email protected]>
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
On Fri, Oct 14, 2022 at 04:15, Eirin Nya <[email protected]> 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 <[email protected]>
Same remark as for patch v2 1/3: I already reviewed v1.
Reviewed-by: Mattijs Korpershoek <[email protected]>
> ---
> 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
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 <[email protected]> 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 <[email protected]>
>
> 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/[email protected]/
>
> 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 <[email protected]>
>
> 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
Hi Eirin,
On Mon, Nov 28, 2022 at 01:18, Eirin Nya <[email protected]> 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 <[email protected]> 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 <[email protected]>
>>
>> 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/[email protected]/
>>
>> 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 <[email protected]>
>>
>> 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
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 <[email protected]> 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