Re: [PATCH v2] Input: elantech - fix x_max/y_max values

[Bernd Edlinger]

Hi Jingle,

apologies for not responding earlier, but better late than never...

Okay, if I understood your advice correctly, you want me to
elantech_set_absolute_mode from elantech_query_info
for hardware version 2 and 3, before sending the ETP_FW_ID_QUERY
command, while hardware version 4 devices will not need that.

Initially when I wrote this patch I was concerned that there might
be a way how the firmware might be in a different state after doing
this when elantech_setup_smbus is called later.

But meanwhile I convinced myself that this can never happen, since
the elantech_setup_smbus seems to be unreachable for version < 4
devices.

I'll send a new version of the patch later today, if everything works
as expected.


Thanks,
Bernd.


On 4/14/20 8:09 AM, jingle wrote:
> Hi Dmitry, Bernd:
> 
> We have checked the related function internal. 
> It is right to switch the touchpad into absolute mode first to get valid
> results from the ID query call in hardware version 2 and 3.
> 
> Thanks
> jingle
> 
> -----Original Message-----
> From: Dmitry Torokhov [mailto:dmitry.torokhov@xxxxxxxxx] 
> Sent: Friday, April 10, 2020 4:08 AM
> To: Bernd Edlinger
> Cc: Benjamin Tissoires; Dave.Wang; jingle; linux-kernel@xxxxxxxxxxxxxxx;
> linux-input@xxxxxxxxxxxxxxx; Kai-Heng Feng; Enrico Weigelt; Allison Randal;
> Aaron Ma
> Subject: Re: [PATCH v2] Input: elantech - fix x_max/y_max values
> 
> Hi Bernd,
> 
> On Thu, Apr 09, 2020 at 07:38:17PM +0200, Bernd Edlinger wrote:
>> Are you there?
>>
>> Should I re-post the v2 patch, was that dropped somehow?
>>
>> If I don't hear anything I'll assume I just repost, probably as 
>> unchanged v3, right?
> 
> I do not think we ever got confirmation from Dave or Jingle; from my POV I
> really dislike arbitrary mangling of the data.
> 
> I think you are right that the issue is with order of calls, and we need to
> switch the touchpad into absolute mode to get valid results from the ID
> query call. Dave, Jingle, any impot here?
> 
> Benjamin, Kai-Feng, do you know if we try to switch to absolute mode to
> begin with, will it cause issues with SMBus mode?
> 
> Thanks.
> 
>>
>> Thanks,
>> Bernd.
>>
>>
>> On 4/5/20 7:54 AM, Bernd Edlinger wrote:
>>> And, furthermore, there is one thing I find really confusing,
>>>
>>> I do not see the message v2 quoted below which had an updated commit 
>>> message, due to that Benjamin Tissoires request.
>>>
>>> It was from my point of view sent on 2/27/20 11:03 PM, but I cannot 
>>> find it neither on spinics, nor on marc.info.
>>> That is funny.
>>>
>>> Did it reach you guys at all?
>>> Or should I re-send it just in case?
>>>
>>>
>>> Thanks
>>> Bernd.
>>>
>>> On 4/5/20 7:26 AM, Bernd Edlinger wrote:
>>>> Ping...
>>>>
>>>> This patch works fine for me since several weeks, without it I 
>>>> would not be able to use my laptop any more.
>>>>
>>>> Could you please accept this patch?
>>>>
>>>>
>>>> Thanks
>>>> Bernd.
>>>>
>>>>
>>>> On 2/27/20 11:03 PM, Bernd Edlinger wrote:
>>>>> Since commit 37548659bb22 ("Input: elantech - query the min/max 
>>>>> information beforehand too") moved the querying of the x_max/y_max 
>>>>> values from elantech_set_input_params to elantech_query_info, the 
>>>>> returned x_max/y_max values are different than before, at least 
>>>>> for some firmware versions.
>>>>>
>>>>> The reason is likely that this is now done before 
>>>>> elantech_set_absolute_mode does run.  So it may happen that the 
>>>>> returned values are exactly half of what they used to be, which 
>>>>> makes input_report_abs in PS/2 mode report ABS_X values which 
>>>>> exceed the x_max value, which is very annoying since the mouse 
>>>>> stops to move then, and ABS_Y value become negative, which is benign.
>>>>>
>>>>> This was observed with a MSI GX70 laptop:
>>>>>
>>>>> elantech: assuming hardware version 3 (with firmware version 
>>>>> 0x250f01)
>>>>> elantech: Synaptics capabilities query result 0x18, 0x17, 0x0b.
>>>>> elantech: Elan sample query result 05, 0e, 00
>>>>> input: ETPS/2 Elantech Touchpad as /devices/platform/i8042/serio...
>>>>>
>>>>> Correct this by doubling the returned x_max and y_max value for 
>>>>> this specific firmware version.
>>>>>
>>>>> Fixes: 37548659bb22 ("Input: elantech - query the min/max 
>>>>> information beforehand too")
>>>>>
>>>>> Signed-off-by: Bernd Edlinger <bernd.edlinger@xxxxxxxxxx>
>>>>> ---
>>>>>  drivers/input/mouse/elantech.c | 4 ++++
>>>>>  1 file changed, 4 insertions(+)
>>>>>
>>>>> diff --git a/drivers/input/mouse/elantech.c 
>>>>> b/drivers/input/mouse/elantech.c index 2d8434b..3399db8 100644
>>>>> --- a/drivers/input/mouse/elantech.c
>>>>> +++ b/drivers/input/mouse/elantech.c
>>>>> @@ -1720,6 +1720,10 @@ static int elantech_query_info(struct 
>>>>> psmouse *psmouse,
>>>>>  
>>>>>  		info->x_max = (0x0f & param[0]) << 8 | param[1];
>>>>>  		info->y_max = (0xf0 & param[0]) << 4 | param[2];
>>>>> +		if (info->fw_version == 0x250f01) {
>>>>> +			info->x_max <<= 1;
>>>>> +			info->y_max <<= 1;
>>>>> +		}
>>>>>  		break;
>>>>>  
>>>>>  	case 4:
>>>>>
> 
> --
> Dmitry
>