diff mbox

Input: ALPS - fix DualPoint flag for 74 03 28 devices

Message ID 20171115063404.10066-1-aaron.ma@canonical.com (mailing list archive)
State New, archived
Headers show

Commit Message

Aaron Ma Nov. 15, 2017, 6:34 a.m. UTC
There is a regression of commit 4a646580f793 ("Input: ALPS - fix
two-finger scroll breakage"), ALPS device fails with log:

psmouse serio1: alps: Rejected trackstick packet from non DualPoint device

ALPS device with id "74 03 28" report OTP[0] data 0xCE after
commit 4a646580f793, after restore the OTP reading order,
it becomes to 0x10 as before and reports the right flag.

Fixes: 4a646580f793 ("Input: ALPS - fix two-finger scroll breakage")
Cc: <stable@vger.kernel.org>
Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
---
 drivers/input/mouse/alps.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Pali Rohár Nov. 15, 2017, 8:35 a.m. UTC | #1
On Wednesday 15 November 2017 14:34:04 Aaron Ma wrote:
> There is a regression of commit 4a646580f793 ("Input: ALPS - fix
> two-finger scroll breakage"), ALPS device fails with log:
> 
> psmouse serio1: alps: Rejected trackstick packet from non DualPoint device
> 
> ALPS device with id "74 03 28" report OTP[0] data 0xCE after
> commit 4a646580f793, after restore the OTP reading order,
> it becomes to 0x10 as before and reports the right flag.
> 
> Fixes: 4a646580f793 ("Input: ALPS - fix two-finger scroll breakage")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
> ---
>  drivers/input/mouse/alps.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> index 579b899add26..c59b8f7ca2fc 100644
> --- a/drivers/input/mouse/alps.c
> +++ b/drivers/input/mouse/alps.c
> @@ -2562,8 +2562,8 @@ static int alps_set_defaults_ss4_v2(struct psmouse *psmouse,
>  
>  	memset(otp, 0, sizeof(otp));
>  
> -	if (alps_get_otp_values_ss4_v2(psmouse, 1, &otp[1][0]) ||
> -	    alps_get_otp_values_ss4_v2(psmouse, 0, &otp[0][0]))
> +	if (alps_get_otp_values_ss4_v2(psmouse, 0, &otp[0][0]) ||
> +	    alps_get_otp_values_ss4_v2(psmouse, 1, &otp[1][0]))
>  		return -1;
>  
>  	alps_update_device_area_ss4_v2(otp, priv);

Masaki Ota, please look at this patch as it partially revert your commit
4a646580f793 ("Input: ALPS - fix two-finger scroll breakage"). Something
smells here.
Masaki Ota Nov. 16, 2017, 7:27 a.m. UTC | #2
Hi, Pali, Aaron,

Current code is correct device setting, previous code is wrong.
If the trackstick does not work(DUALPOINT flag disable), Device Firmware setting is wrong.

But recently I received the same report from Thinkpad L570 user, and I checked this device and found this device Firmware setting is wrong. Sorry for our mistake.
Is your laptop L570 ?

I will add code that supports the trackstick for this device.

Best Regards,
Masaki Ota
-----Original Message-----
From: Pali Rohár [mailto:pali.rohar@gmail.com] 

Sent: Wednesday, November 15, 2017 5:35 PM
To: 太田 真喜 Masaki Ota <masaki.ota@jp.alps.com>
Cc: linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; dmitry.torokhov@gmail.com; Aaron Ma <aaron.ma@canonical.com>
Subject: Re: [PATCH] Input: ALPS - fix DualPoint flag for 74 03 28 devices

On Wednesday 15 November 2017 14:34:04 Aaron Ma wrote:
> There is a regression of commit 4a646580f793 ("Input: ALPS - fix 

> two-finger scroll breakage"), ALPS device fails with log:

> 

> psmouse serio1: alps: Rejected trackstick packet from non DualPoint 

> device

> 

> ALPS device with id "74 03 28" report OTP[0] data 0xCE after commit 

> 4a646580f793, after restore the OTP reading order, it becomes to 0x10 

> as before and reports the right flag.

> 

> Fixes: 4a646580f793 ("Input: ALPS - fix two-finger scroll breakage")

> Cc: <stable@vger.kernel.org>

> Signed-off-by: Aaron Ma <aaron.ma@canonical.com>

> ---

>  drivers/input/mouse/alps.c | 4 ++--

>  1 file changed, 2 insertions(+), 2 deletions(-)

> 

> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c 

> index 579b899add26..c59b8f7ca2fc 100644

> --- a/drivers/input/mouse/alps.c

> +++ b/drivers/input/mouse/alps.c

> @@ -2562,8 +2562,8 @@ static int alps_set_defaults_ss4_v2(struct 

> psmouse *psmouse,

>  

>  	memset(otp, 0, sizeof(otp));

>  

> -	if (alps_get_otp_values_ss4_v2(psmouse, 1, &otp[1][0]) ||

> -	    alps_get_otp_values_ss4_v2(psmouse, 0, &otp[0][0]))

> +	if (alps_get_otp_values_ss4_v2(psmouse, 0, &otp[0][0]) ||

> +	    alps_get_otp_values_ss4_v2(psmouse, 1, &otp[1][0]))

>  		return -1;

>  

>  	alps_update_device_area_ss4_v2(otp, priv);


Masaki Ota, please look at this patch as it partially revert your commit
4a646580f793 ("Input: ALPS - fix two-finger scroll breakage"). Something smells here.

--
Pali Rohár
pali.rohar@gmail.com
Aaron Ma Nov. 16, 2017, 9:07 a.m. UTC | #3
Hi Masaki Ota:

Yes, the laptop is L570.

If your code is right, the workaround code for L570 must be done.

Please fix it ASAP.

Regards,
Aaron

On 11/16/2017 03:27 PM, Masaki Ota wrote:
> Hi, Pali, Aaron,
> 
> Current code is correct device setting, previous code is wrong.
> If the trackstick does not work(DUALPOINT flag disable), Device Firmware setting is wrong.
> 
> But recently I received the same report from Thinkpad L570 user, and I checked this device and found this device Firmware setting is wrong. Sorry for our mistake.
> Is your laptop L570 ?
> 
> I will add code that supports the trackstick for this device.
> 
> Best Regards,
> Masaki Ota
> -----Original Message-----
> From: Pali Rohár [mailto:pali.rohar@gmail.com] 
> Sent: Wednesday, November 15, 2017 5:35 PM
> To: 太田 真喜 Masaki Ota <masaki.ota@jp.alps.com>
> Cc: linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; dmitry.torokhov@gmail.com; Aaron Ma <aaron.ma@canonical.com>
> Subject: Re: [PATCH] Input: ALPS - fix DualPoint flag for 74 03 28 devices
> 
> On Wednesday 15 November 2017 14:34:04 Aaron Ma wrote:
>> There is a regression of commit 4a646580f793 ("Input: ALPS - fix 
>> two-finger scroll breakage"), ALPS device fails with log:
>>
>> psmouse serio1: alps: Rejected trackstick packet from non DualPoint 
>> device
>>
>> ALPS device with id "74 03 28" report OTP[0] data 0xCE after commit 
>> 4a646580f793, after restore the OTP reading order, it becomes to 0x10 
>> as before and reports the right flag.
>>
>> Fixes: 4a646580f793 ("Input: ALPS - fix two-finger scroll breakage")
>> Cc: <stable@vger.kernel.org>
>> Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
>> ---
>>  drivers/input/mouse/alps.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c 
>> index 579b899add26..c59b8f7ca2fc 100644
>> --- a/drivers/input/mouse/alps.c
>> +++ b/drivers/input/mouse/alps.c
>> @@ -2562,8 +2562,8 @@ static int alps_set_defaults_ss4_v2(struct 
>> psmouse *psmouse,
>>  
>>  	memset(otp, 0, sizeof(otp));
>>  
>> -	if (alps_get_otp_values_ss4_v2(psmouse, 1, &otp[1][0]) ||
>> -	    alps_get_otp_values_ss4_v2(psmouse, 0, &otp[0][0]))
>> +	if (alps_get_otp_values_ss4_v2(psmouse, 0, &otp[0][0]) ||
>> +	    alps_get_otp_values_ss4_v2(psmouse, 1, &otp[1][0]))
>>  		return -1;
>>  
>>  	alps_update_device_area_ss4_v2(otp, priv);
> 
> Masaki Ota, please look at this patch as it partially revert your commit
> 4a646580f793 ("Input: ALPS - fix two-finger scroll breakage"). Something smells here.
> 
> --
> Pali Rohár
> pali.rohar@gmail.com
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dmitry Torokhov Jan. 29, 2018, 10:51 p.m. UTC | #4
Hi,

On Thu, Nov 16, 2017 at 07:27:02AM +0000, Masaki Ota wrote:
> Hi, Pali, Aaron,
> 
> Current code is correct device setting, previous code is wrong.
> If the trackstick does not work(DUALPOINT flag disable), Device Firmware setting is wrong.
> 
> But recently I received the same report from Thinkpad L570 user, and I checked this device and found this device Firmware setting is wrong. Sorry for our mistake.
> Is your laptop L570 ?
> 
> I will add code that supports the trackstick for this device.

Sorry for resurrecting this old thread, I am just trying to understand
what went wrong here. Is the sequence of "f0 f0 e9" and "ea ea e9" is
important in getting the correct OTP data and we originally got this
order wrong? It is not clear from the original patch and discussion that
this change was intentional.

Thanks.

> 
> Best Regards,
> Masaki Ota
> -----Original Message-----
> From: Pali Rohár [mailto:pali.rohar@gmail.com] 
> Sent: Wednesday, November 15, 2017 5:35 PM
> To: 太田 真喜 Masaki Ota <masaki.ota@jp.alps.com>
> Cc: linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; dmitry.torokhov@gmail.com; Aaron Ma <aaron.ma@canonical.com>
> Subject: Re: [PATCH] Input: ALPS - fix DualPoint flag for 74 03 28 devices
> 
> On Wednesday 15 November 2017 14:34:04 Aaron Ma wrote:
> > There is a regression of commit 4a646580f793 ("Input: ALPS - fix 
> > two-finger scroll breakage"), ALPS device fails with log:
> > 
> > psmouse serio1: alps: Rejected trackstick packet from non DualPoint 
> > device
> > 
> > ALPS device with id "74 03 28" report OTP[0] data 0xCE after commit 
> > 4a646580f793, after restore the OTP reading order, it becomes to 0x10 
> > as before and reports the right flag.
> > 
> > Fixes: 4a646580f793 ("Input: ALPS - fix two-finger scroll breakage")
> > Cc: <stable@vger.kernel.org>
> > Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
> > ---
> >  drivers/input/mouse/alps.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c 
> > index 579b899add26..c59b8f7ca2fc 100644
> > --- a/drivers/input/mouse/alps.c
> > +++ b/drivers/input/mouse/alps.c
> > @@ -2562,8 +2562,8 @@ static int alps_set_defaults_ss4_v2(struct 
> > psmouse *psmouse,
> >  
> >  	memset(otp, 0, sizeof(otp));
> >  
> > -	if (alps_get_otp_values_ss4_v2(psmouse, 1, &otp[1][0]) ||
> > -	    alps_get_otp_values_ss4_v2(psmouse, 0, &otp[0][0]))
> > +	if (alps_get_otp_values_ss4_v2(psmouse, 0, &otp[0][0]) ||
> > +	    alps_get_otp_values_ss4_v2(psmouse, 1, &otp[1][0]))
> >  		return -1;
> >  
> >  	alps_update_device_area_ss4_v2(otp, priv);
> 
> Masaki Ota, please look at this patch as it partially revert your commit
> 4a646580f793 ("Input: ALPS - fix two-finger scroll breakage"). Something smells here.
> 
> --
> Pali Rohár
> pali.rohar@gmail.com
Dmitry Torokhov March 20, 2018, 6:47 p.m. UTC | #5
On Mon, Jan 29, 2018 at 2:51 PM, dmitry.torokhov@gmail.com
<dmitry.torokhov@gmail.com> wrote:
> Hi,
>
> On Thu, Nov 16, 2017 at 07:27:02AM +0000, Masaki Ota wrote:
>> Hi, Pali, Aaron,
>>
>> Current code is correct device setting, previous code is wrong.
>> If the trackstick does not work(DUALPOINT flag disable), Device Firmware setting is wrong.
>>
>> But recently I received the same report from Thinkpad L570 user, and I checked this device and found this device Firmware setting is wrong. Sorry for our mistake.
>> Is your laptop L570 ?
>>
>> I will add code that supports the trackstick for this device.
>
> Sorry for resurrecting this old thread, I am just trying to understand
> what went wrong here. Is the sequence of "f0 f0 e9" and "ea ea e9" is
> important in getting the correct OTP data and we originally got this
> order wrong? It is not clear from the original patch and discussion that
> this change was intentional.

Could I please get an answer to my question?

Thanks!

>
> Thanks.
>
>>
>> Best Regards,
>> Masaki Ota
>> -----Original Message-----
>> From: Pali Rohár [mailto:pali.rohar@gmail.com]
>> Sent: Wednesday, November 15, 2017 5:35 PM
>> To: 太田 真喜 Masaki Ota <masaki.ota@jp.alps.com>
>> Cc: linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; dmitry.torokhov@gmail.com; Aaron Ma <aaron.ma@canonical.com>
>> Subject: Re: [PATCH] Input: ALPS - fix DualPoint flag for 74 03 28 devices
>>
>> On Wednesday 15 November 2017 14:34:04 Aaron Ma wrote:
>> > There is a regression of commit 4a646580f793 ("Input: ALPS - fix
>> > two-finger scroll breakage"), ALPS device fails with log:
>> >
>> > psmouse serio1: alps: Rejected trackstick packet from non DualPoint
>> > device
>> >
>> > ALPS device with id "74 03 28" report OTP[0] data 0xCE after commit
>> > 4a646580f793, after restore the OTP reading order, it becomes to 0x10
>> > as before and reports the right flag.
>> >
>> > Fixes: 4a646580f793 ("Input: ALPS - fix two-finger scroll breakage")
>> > Cc: <stable@vger.kernel.org>
>> > Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
>> > ---
>> >  drivers/input/mouse/alps.c | 4 ++--
>> >  1 file changed, 2 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
>> > index 579b899add26..c59b8f7ca2fc 100644
>> > --- a/drivers/input/mouse/alps.c
>> > +++ b/drivers/input/mouse/alps.c
>> > @@ -2562,8 +2562,8 @@ static int alps_set_defaults_ss4_v2(struct
>> > psmouse *psmouse,
>> >
>> >     memset(otp, 0, sizeof(otp));
>> >
>> > -   if (alps_get_otp_values_ss4_v2(psmouse, 1, &otp[1][0]) ||
>> > -       alps_get_otp_values_ss4_v2(psmouse, 0, &otp[0][0]))
>> > +   if (alps_get_otp_values_ss4_v2(psmouse, 0, &otp[0][0]) ||
>> > +       alps_get_otp_values_ss4_v2(psmouse, 1, &otp[1][0]))
>> >             return -1;
>> >
>> >     alps_update_device_area_ss4_v2(otp, priv);
>>
>> Masaki Ota, please look at this patch as it partially revert your commit
>> 4a646580f793 ("Input: ALPS - fix two-finger scroll breakage"). Something smells here.
>>
>> --
>> Pali Rohár
>> pali.rohar@gmail.com
>
> --
> Dmitry
Pali Rohár March 25, 2018, 9:25 p.m. UTC | #6
On Tuesday 20 March 2018 11:47:26 Dmitry Torokhov wrote:
> On Mon, Jan 29, 2018 at 2:51 PM, dmitry.torokhov@gmail.com
> <dmitry.torokhov@gmail.com> wrote:
> > Hi,
> >
> > On Thu, Nov 16, 2017 at 07:27:02AM +0000, Masaki Ota wrote:
> >> Hi, Pali, Aaron,
> >>
> >> Current code is correct device setting, previous code is wrong.
> >> If the trackstick does not work(DUALPOINT flag disable), Device Firmware setting is wrong.
> >>
> >> But recently I received the same report from Thinkpad L570 user, and I checked this device and found this device Firmware setting is wrong. Sorry for our mistake.
> >> Is your laptop L570 ?
> >>
> >> I will add code that supports the trackstick for this device.
> >
> > Sorry for resurrecting this old thread, I am just trying to understand
> > what went wrong here. Is the sequence of "f0 f0 e9" and "ea ea e9" is
> > important in getting the correct OTP data and we originally got this
> > order wrong? It is not clear from the original patch and discussion that
> > this change was intentional.
> 
> Could I please get an answer to my question?
> 
> Thanks!

Masaki, this question is for you ↑↑↑

> >
> > Thanks.
> >
> >>
> >> Best Regards,
> >> Masaki Ota
> >> -----Original Message-----
> >> From: Pali Rohár [mailto:pali.rohar@gmail.com]
> >> Sent: Wednesday, November 15, 2017 5:35 PM
> >> To: 太田 真喜 Masaki Ota <masaki.ota@jp.alps.com>
> >> Cc: linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; dmitry.torokhov@gmail.com; Aaron Ma <aaron.ma@canonical.com>
> >> Subject: Re: [PATCH] Input: ALPS - fix DualPoint flag for 74 03 28 devices
> >>
> >> On Wednesday 15 November 2017 14:34:04 Aaron Ma wrote:
> >> > There is a regression of commit 4a646580f793 ("Input: ALPS - fix
> >> > two-finger scroll breakage"), ALPS device fails with log:
> >> >
> >> > psmouse serio1: alps: Rejected trackstick packet from non DualPoint
> >> > device
> >> >
> >> > ALPS device with id "74 03 28" report OTP[0] data 0xCE after commit
> >> > 4a646580f793, after restore the OTP reading order, it becomes to 0x10
> >> > as before and reports the right flag.
> >> >
> >> > Fixes: 4a646580f793 ("Input: ALPS - fix two-finger scroll breakage")
> >> > Cc: <stable@vger.kernel.org>
> >> > Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
> >> > ---
> >> >  drivers/input/mouse/alps.c | 4 ++--
> >> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >> >
> >> > diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> >> > index 579b899add26..c59b8f7ca2fc 100644
> >> > --- a/drivers/input/mouse/alps.c
> >> > +++ b/drivers/input/mouse/alps.c
> >> > @@ -2562,8 +2562,8 @@ static int alps_set_defaults_ss4_v2(struct
> >> > psmouse *psmouse,
> >> >
> >> >     memset(otp, 0, sizeof(otp));
> >> >
> >> > -   if (alps_get_otp_values_ss4_v2(psmouse, 1, &otp[1][0]) ||
> >> > -       alps_get_otp_values_ss4_v2(psmouse, 0, &otp[0][0]))
> >> > +   if (alps_get_otp_values_ss4_v2(psmouse, 0, &otp[0][0]) ||
> >> > +       alps_get_otp_values_ss4_v2(psmouse, 1, &otp[1][0]))
> >> >             return -1;
> >> >
> >> >     alps_update_device_area_ss4_v2(otp, priv);
> >>
> >> Masaki Ota, please look at this patch as it partially revert your commit
> >> 4a646580f793 ("Input: ALPS - fix two-finger scroll breakage"). Something smells here.
> >>
> >> --
> >> Pali Rohár
> >> pali.rohar@gmail.com
> >
> > --
> > Dmitry
> 
> 
>
Masaki Ota March 27, 2018, 4:37 a.m. UTC | #7
Hi, 

We can get OTP page 0 value by EA EA E9 commands, but we cannot get it by EA EA EA E9.
As far as I remember, Device initialization finish at EA command, then sends EA EA E9 commands.
In this case we cannot get correct OTP page 0 value.
So I changed this order. (We can get OTP page 1 value by both of EA F0 F0 E9 and F0 F0 E9.)

Best Regards,
Masaki Ota
-----Original Message-----
From: Pali Rohár [mailto:pali.rohar@gmail.com] 

Sent: Monday, March 26, 2018 6:26 AM
To: 太田 真喜 Masaki Ota <masaki.ota@jp.alps.com>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; Aaron Ma <aaron.ma@canonical.com>
Subject: Re: [PATCH] Input: ALPS - fix DualPoint flag for 74 03 28 devices

On Tuesday 20 March 2018 11:47:26 Dmitry Torokhov wrote:
> On Mon, Jan 29, 2018 at 2:51 PM, dmitry.torokhov@gmail.com 

> <dmitry.torokhov@gmail.com> wrote:

> > Hi,

> >

> > On Thu, Nov 16, 2017 at 07:27:02AM +0000, Masaki Ota wrote:

> >> Hi, Pali, Aaron,

> >>

> >> Current code is correct device setting, previous code is wrong.

> >> If the trackstick does not work(DUALPOINT flag disable), Device Firmware setting is wrong.

> >>

> >> But recently I received the same report from Thinkpad L570 user, and I checked this device and found this device Firmware setting is wrong. Sorry for our mistake.

> >> Is your laptop L570 ?

> >>

> >> I will add code that supports the trackstick for this device.

> >

> > Sorry for resurrecting this old thread, I am just trying to 

> > understand what went wrong here. Is the sequence of "f0 f0 e9" and 

> > "ea ea e9" is important in getting the correct OTP data and we 

> > originally got this order wrong? It is not clear from the original 

> > patch and discussion that this change was intentional.

> 

> Could I please get an answer to my question?

> 

> Thanks!


Masaki, this question is for you ↑↑↑

> >

> > Thanks.

> >

> >>

> >> Best Regards,

> >> Masaki Ota

> >> -----Original Message-----

> >> From: Pali Rohár [mailto:pali.rohar@gmail.com]

> >> Sent: Wednesday, November 15, 2017 5:35 PM

> >> To: 太田 真喜 Masaki Ota <masaki.ota@jp.alps.com>

> >> Cc: linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; 

> >> dmitry.torokhov@gmail.com; Aaron Ma <aaron.ma@canonical.com>

> >> Subject: Re: [PATCH] Input: ALPS - fix DualPoint flag for 74 03 28 

> >> devices

> >>

> >> On Wednesday 15 November 2017 14:34:04 Aaron Ma wrote:

> >> > There is a regression of commit 4a646580f793 ("Input: ALPS - fix 

> >> > two-finger scroll breakage"), ALPS device fails with log:

> >> >

> >> > psmouse serio1: alps: Rejected trackstick packet from non 

> >> > DualPoint device

> >> >

> >> > ALPS device with id "74 03 28" report OTP[0] data 0xCE after 

> >> > commit 4a646580f793, after restore the OTP reading order, it 

> >> > becomes to 0x10 as before and reports the right flag.

> >> >

> >> > Fixes: 4a646580f793 ("Input: ALPS - fix two-finger scroll 

> >> > breakage")

> >> > Cc: <stable@vger.kernel.org>

> >> > Signed-off-by: Aaron Ma <aaron.ma@canonical.com>

> >> > ---

> >> >  drivers/input/mouse/alps.c | 4 ++--

> >> >  1 file changed, 2 insertions(+), 2 deletions(-)

> >> >

> >> > diff --git a/drivers/input/mouse/alps.c 

> >> > b/drivers/input/mouse/alps.c index 579b899add26..c59b8f7ca2fc 

> >> > 100644

> >> > --- a/drivers/input/mouse/alps.c

> >> > +++ b/drivers/input/mouse/alps.c

> >> > @@ -2562,8 +2562,8 @@ static int alps_set_defaults_ss4_v2(struct 

> >> > psmouse *psmouse,

> >> >

> >> >     memset(otp, 0, sizeof(otp));

> >> >

> >> > -   if (alps_get_otp_values_ss4_v2(psmouse, 1, &otp[1][0]) ||

> >> > -       alps_get_otp_values_ss4_v2(psmouse, 0, &otp[0][0]))

> >> > +   if (alps_get_otp_values_ss4_v2(psmouse, 0, &otp[0][0]) ||

> >> > +       alps_get_otp_values_ss4_v2(psmouse, 1, &otp[1][0]))

> >> >             return -1;

> >> >

> >> >     alps_update_device_area_ss4_v2(otp, priv);

> >>

> >> Masaki Ota, please look at this patch as it partially revert your 

> >> commit

> >> 4a646580f793 ("Input: ALPS - fix two-finger scroll breakage"). Something smells here.

> >>

> >> --

> >> Pali Rohár

> >> pali.rohar@gmail.com

> >

> > --

> > Dmitry

> 

> 

> 


--
Pali Rohár
pali.rohar@gmail.com
Pali Rohár March 28, 2018, 7:38 a.m. UTC | #8
Ideally this information should be put as a comment into the code. It is
really not obvious.

On Tuesday 27 March 2018 04:37:19 Masaki Ota wrote:
> Hi, 
> 
> We can get OTP page 0 value by EA EA E9 commands, but we cannot get it by EA EA EA E9.
> As far as I remember, Device initialization finish at EA command, then sends EA EA E9 commands.
> In this case we cannot get correct OTP page 0 value.
> So I changed this order. (We can get OTP page 1 value by both of EA F0 F0 E9 and F0 F0 E9.)
> 
> Best Regards,
> Masaki Ota
> -----Original Message-----
> From: Pali Rohár [mailto:pali.rohar@gmail.com] 
> Sent: Monday, March 26, 2018 6:26 AM
> To: 太田 真喜 Masaki Ota <masaki.ota@jp.alps.com>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; Aaron Ma <aaron.ma@canonical.com>
> Subject: Re: [PATCH] Input: ALPS - fix DualPoint flag for 74 03 28 devices
> 
> On Tuesday 20 March 2018 11:47:26 Dmitry Torokhov wrote:
> > On Mon, Jan 29, 2018 at 2:51 PM, dmitry.torokhov@gmail.com 
> > <dmitry.torokhov@gmail.com> wrote:
> > > Hi,
> > >
> > > On Thu, Nov 16, 2017 at 07:27:02AM +0000, Masaki Ota wrote:
> > >> Hi, Pali, Aaron,
> > >>
> > >> Current code is correct device setting, previous code is wrong.
> > >> If the trackstick does not work(DUALPOINT flag disable), Device Firmware setting is wrong.
> > >>
> > >> But recently I received the same report from Thinkpad L570 user, and I checked this device and found this device Firmware setting is wrong. Sorry for our mistake.
> > >> Is your laptop L570 ?
> > >>
> > >> I will add code that supports the trackstick for this device.
> > >
> > > Sorry for resurrecting this old thread, I am just trying to 
> > > understand what went wrong here. Is the sequence of "f0 f0 e9" and 
> > > "ea ea e9" is important in getting the correct OTP data and we 
> > > originally got this order wrong? It is not clear from the original 
> > > patch and discussion that this change was intentional.
> > 
> > Could I please get an answer to my question?
> > 
> > Thanks!
> 
> Masaki, this question is for you ↑↑↑
> 
> > >
> > > Thanks.
> > >
> > >>
> > >> Best Regards,
> > >> Masaki Ota
> > >> -----Original Message-----
> > >> From: Pali Rohár [mailto:pali.rohar@gmail.com]
> > >> Sent: Wednesday, November 15, 2017 5:35 PM
> > >> To: 太田 真喜 Masaki Ota <masaki.ota@jp.alps.com>
> > >> Cc: linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; 
> > >> dmitry.torokhov@gmail.com; Aaron Ma <aaron.ma@canonical.com>
> > >> Subject: Re: [PATCH] Input: ALPS - fix DualPoint flag for 74 03 28 
> > >> devices
> > >>
> > >> On Wednesday 15 November 2017 14:34:04 Aaron Ma wrote:
> > >> > There is a regression of commit 4a646580f793 ("Input: ALPS - fix 
> > >> > two-finger scroll breakage"), ALPS device fails with log:
> > >> >
> > >> > psmouse serio1: alps: Rejected trackstick packet from non 
> > >> > DualPoint device
> > >> >
> > >> > ALPS device with id "74 03 28" report OTP[0] data 0xCE after 
> > >> > commit 4a646580f793, after restore the OTP reading order, it 
> > >> > becomes to 0x10 as before and reports the right flag.
> > >> >
> > >> > Fixes: 4a646580f793 ("Input: ALPS - fix two-finger scroll 
> > >> > breakage")
> > >> > Cc: <stable@vger.kernel.org>
> > >> > Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
> > >> > ---
> > >> >  drivers/input/mouse/alps.c | 4 ++--
> > >> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >> >
> > >> > diff --git a/drivers/input/mouse/alps.c 
> > >> > b/drivers/input/mouse/alps.c index 579b899add26..c59b8f7ca2fc 
> > >> > 100644
> > >> > --- a/drivers/input/mouse/alps.c
> > >> > +++ b/drivers/input/mouse/alps.c
> > >> > @@ -2562,8 +2562,8 @@ static int alps_set_defaults_ss4_v2(struct 
> > >> > psmouse *psmouse,
> > >> >
> > >> >     memset(otp, 0, sizeof(otp));
> > >> >
> > >> > -   if (alps_get_otp_values_ss4_v2(psmouse, 1, &otp[1][0]) ||
> > >> > -       alps_get_otp_values_ss4_v2(psmouse, 0, &otp[0][0]))
> > >> > +   if (alps_get_otp_values_ss4_v2(psmouse, 0, &otp[0][0]) ||
> > >> > +       alps_get_otp_values_ss4_v2(psmouse, 1, &otp[1][0]))
> > >> >             return -1;
> > >> >
> > >> >     alps_update_device_area_ss4_v2(otp, priv);
> > >>
> > >> Masaki Ota, please look at this patch as it partially revert your 
> > >> commit
> > >> 4a646580f793 ("Input: ALPS - fix two-finger scroll breakage"). Something smells here.
> > >>
> > >> --
> > >> Pali Rohár
> > >> pali.rohar@gmail.com
> > >
> > > --
> > > Dmitry
> > 
> > 
> > 
> 
> --
> Pali Rohár
> pali.rohar@gmail.com
diff mbox

Patch

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index 579b899add26..c59b8f7ca2fc 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -2562,8 +2562,8 @@  static int alps_set_defaults_ss4_v2(struct psmouse *psmouse,
 
 	memset(otp, 0, sizeof(otp));
 
-	if (alps_get_otp_values_ss4_v2(psmouse, 1, &otp[1][0]) ||
-	    alps_get_otp_values_ss4_v2(psmouse, 0, &otp[0][0]))
+	if (alps_get_otp_values_ss4_v2(psmouse, 0, &otp[0][0]) ||
+	    alps_get_otp_values_ss4_v2(psmouse, 1, &otp[1][0]))
 		return -1;
 
 	alps_update_device_area_ss4_v2(otp, priv);