ALPS DualPoint double click bug

ALPS DualPoint double click bug

[Douglas Christman]

Hello all,

Kernel v4.1 introduces a bug for my laptop's (Toshiba Tecra A10)
touchpad.  Clicking twice only registers as a single click (and three
clicks are needed to double-click).

The touchpad shows up as 'AlpsPS/2 ALPS DualPoint' in dmesg.
Bisecting drivers/input/mouse suggests that commit 92bac83d introduced
the bug.

Please let me know if I can provide any other information that would be useful.

Filed as bug #101701 on bugzilla.

Thank you,
Doug

Re: ALPS DualPoint double click bug

[Dmitry Torokhov]

On Sun, Jul 19, 2015 at 09:29:26AM -0400, Douglas Christman wrote:
> Hello all,
> 
> Kernel v4.1 introduces a bug for my laptop's (Toshiba Tecra A10)
> touchpad.  Clicking twice only registers as a single click (and three
> clicks are needed to double-click).
> 
> The touchpad shows up as 'AlpsPS/2 ALPS DualPoint' in dmesg.
> Bisecting drivers/input/mouse suggests that commit 92bac83d introduced
> the bug.
> 
> Please let me know if I can provide any other information that would be useful.
> 
> Filed as bug #101701 on bugzilla.

Let's add a few people...

-- 
Dmitry

Re: ALPS DualPoint double click bug

[Pali Rohár]

On Sunday 19 July 2015 23:54:46 Dmitry Torokhov wrote:
> On Sun, Jul 19, 2015 at 09:29:26AM -0400, Douglas Christman wrote:
> > Hello all,
> > 
> > Kernel v4.1 introduces a bug for my laptop's (Toshiba Tecra A10)
> > touchpad.  Clicking twice only registers as a single click (and three
> > clicks are needed to double-click).
> > 
> > The touchpad shows up as 'AlpsPS/2 ALPS DualPoint' in dmesg.
> > Bisecting drivers/input/mouse suggests that commit 92bac83d introduced
> > the bug.
> > 
> > Please let me know if I can provide any other information that would be useful.
> > 
> > Filed as bug #101701 on bugzilla.
> 
> Let's add a few people...
> 

Hi! Please recompile psmouse.ko with dev_dbg() debug information and
send new dmesg output. We need to know type of ALPS device. Thanks!

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

Re: ALPS DualPoint double click bug

[Douglas Christman]

On Mon, Jul 20, 2015 at 3:25 AM, Pali Rohár <pali.rohar@gmail.com> wrote:
> Hi! Please recompile psmouse.ko with dev_dbg() debug information and
> send new dmesg output. We need to know type of ALPS device. Thanks!

I recompiled with CONFIG_DYNAMIC_DEBUG and echoed 'module psmouse' to
/sys/kernel/debug/dynamic_debug/control as described in
Documentation/dynamic-debug-howto.txt, but I don't see any additional
output in dmesg after reloading the module.  I haven't done this
before, so I'm probably overlooking something...

In the meantime, hopefully this output from hwinfo can be of use:

52: PS/2 00.0: 10500 PS/2 Mouse
  [Created at input.249]
  Unique ID: AH6Q.5+smWHVjPI3
  Hardware Class: mouse
  Model: "AlpsPS/2 ALPS DualPoint Stick"
  Vendor: 0x0002
  Device: 0x0008 "AlpsPS/2 ALPS DualPoint Stick"
  Compatible to: int 0x0210 0x0003
  Device File: /dev/input/mice (/dev/input/mouse0)
  Device Files: /dev/input/mice, /dev/input/mouse0,
/dev/input/event11,
/dev/input/by-path/platform-i8042-serio-1-event-mouse,
/dev/input/by-path/platform-i8042-serio-1-mouse
  Device Number: char 13:63 (char 13:32)
  Driver Info #0:
    Buttons: 3
    Wheels: 0
    XFree86 Protocol: explorerps/2
    GPM Protocol: exps2
  Config Status: cfg=new, avail=yes, need=no, active=unknown

53: PS/2 00.0: 10500 PS/2 Mouse
  [Created at input.249]
  Unique ID: AH6Q.5+smWHVjPI3
  Hardware Class: mouse
  Model: "AlpsPS/2 ALPS DualPoint TouchPad"
  Vendor: 0x0002
  Device: 0x0008 "AlpsPS/2 ALPS DualPoint TouchPad"
  Compatible to: int 0x0210 0x0003
  Device File: /dev/input/mice (/dev/input/mouse1)
  Device Files: /dev/input/mice, /dev/input/mouse1,
/dev/input/event12,
/dev/input/by-path/platform-i8042-serio-1-event-mouse,
/dev/input/by-path/platform-i8042-serio-1-mouse
  Device Number: char 13:63 (char 13:33)
  Driver Info #0:
    Buttons: 3
    Wheels: 0
    XFree86 Protocol: explorerps/2
    GPM Protocol: exps2
  Config Status: cfg=new, avail=yes, need=no, active=unknown
--
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

Re: ALPS DualPoint double click bug

[Pali Rohár]

On Monday 20 July 2015 23:00:27 Douglas Christman wrote:
> On Mon, Jul 20, 2015 at 3:25 AM, Pali Rohár <pali.rohar@gmail.com> wrote:
> > Hi! Please recompile psmouse.ko with dev_dbg() debug information and
> > send new dmesg output. We need to know type of ALPS device. Thanks!
> 
> I recompiled with CONFIG_DYNAMIC_DEBUG and echoed 'module psmouse' to
> /sys/kernel/debug/dynamic_debug/control as described in
> Documentation/dynamic-debug-howto.txt, but I don't see any additional
> output in dmesg after reloading the module.  I haven't done this
> before, so I'm probably overlooking something...
> 
> In the meantime, hopefully this output from hwinfo can be of use:
> 
> 52: PS/2 00.0: 10500 PS/2 Mouse
>   [Created at input.249]
>   Unique ID: AH6Q.5+smWHVjPI3
>   Hardware Class: mouse
>   Model: "AlpsPS/2 ALPS DualPoint Stick"
>   Vendor: 0x0002
>   Device: 0x0008 "AlpsPS/2 ALPS DualPoint Stick"
>   Compatible to: int 0x0210 0x0003
>   Device File: /dev/input/mice (/dev/input/mouse0)
>   Device Files: /dev/input/mice, /dev/input/mouse0,
> /dev/input/event11,
> /dev/input/by-path/platform-i8042-serio-1-event-mouse,
> /dev/input/by-path/platform-i8042-serio-1-mouse
>   Device Number: char 13:63 (char 13:32)
>   Driver Info #0:
>     Buttons: 3
>     Wheels: 0
>     XFree86 Protocol: explorerps/2
>     GPM Protocol: exps2
>   Config Status: cfg=new, avail=yes, need=no, active=unknown
> 
> 53: PS/2 00.0: 10500 PS/2 Mouse
>   [Created at input.249]
>   Unique ID: AH6Q.5+smWHVjPI3
>   Hardware Class: mouse
>   Model: "AlpsPS/2 ALPS DualPoint TouchPad"
>   Vendor: 0x0002
>   Device: 0x0008 "AlpsPS/2 ALPS DualPoint TouchPad"
>   Compatible to: int 0x0210 0x0003
>   Device File: /dev/input/mice (/dev/input/mouse1)
>   Device Files: /dev/input/mice, /dev/input/mouse1,
> /dev/input/event12,
> /dev/input/by-path/platform-i8042-serio-1-event-mouse,
> /dev/input/by-path/platform-i8042-serio-1-mouse
>   Device Number: char 13:63 (char 13:33)
>   Driver Info #0:
>     Buttons: 3
>     Wheels: 0
>     XFree86 Protocol: explorerps/2
>     GPM Protocol: exps2
>   Config Status: cfg=new, avail=yes, need=no, active=unknown

It is not enough. If do not want to recompile full kernel, but just only
psmouse.ko I think that the easiest way is to change this define

 #define psmouse_dbg

in file drivers/input/mouse/psmouse.h to call dev_warn instead dev_dbg.
And then just compile psmouse.ko and reload it.

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

Re: ALPS DualPoint double click bug

[Dmitry Torokhov]

On Mon, Jul 20, 2015 at 11:00:27PM -0400, Douglas Christman wrote:
> On Mon, Jul 20, 2015 at 3:25 AM, Pali Rohár <pali.rohar@gmail.com> wrote:
> > Hi! Please recompile psmouse.ko with dev_dbg() debug information and
> > send new dmesg output. We need to know type of ALPS device. Thanks!
> 
> I recompiled with CONFIG_DYNAMIC_DEBUG and echoed 'module psmouse' to
> /sys/kernel/debug/dynamic_debug/control as described in
> Documentation/dynamic-debug-howto.txt, but I don't see any additional
> output in dmesg after reloading the module.  I haven't done this
> before, so I'm probably overlooking something...

Have you tried doing this as:

rmmod psmouse
modprobe psmouse dyndbg=p

Thanks.

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

Re: ALPS DualPoint double click bug

[Douglas Christman]

On Tue, Jul 21, 2015 at 3:12 AM, Pali Rohár <pali.rohar@gmail.com> wrote:
> It is not enough. If do not want to recompile full kernel, but just only
> psmouse.ko I think that the easiest way is to change this define
>
>  #define psmouse_dbg
>
> in file drivers/input/mouse/psmouse.h to call dev_warn instead dev_dbg.
> And then just compile psmouse.ko and reload it.

Thanks, that seemed to do the trick.  This is what I get from dmesg now:

[    6.281814] psmouse serio1: cypress_ps2: send extension cmd 0x00, [0 0 0 0]
[    6.540028] psmouse serio1: cypress_ps2: Command 0x00 response data
(0x): 00 00 14
[    6.582031] psmouse serio1: alps: E6 report: 00 00 64
[    6.617471] psmouse serio1: alps: E7 report: 22 02 14
[    6.656974] psmouse serio1: alps: EC report: 10 00 64
[    7.104479] psmouse serio1: alps: E6 report: 00 00 64
[    7.139714] psmouse serio1: alps: E7 report: 22 02 14
[    7.176568] psmouse serio1: alps: EC report: 10 00 64
[    7.368849] psmouse serio1: alps: F5 report: 05 01 0a
[    7.541367] input: AlpsPS/2 ALPS DualPoint Stick as
/devices/platform/i8042/serio1/input/input11
[    7.571454] input: AlpsPS/2 ALPS DualPoint TouchPad as
/devices/platform/i8042/serio1/input/input8
--
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

Re: ALPS DualPoint double click bug

[Pali Rohár]

On Tuesday 21 July 2015 19:51:48 Douglas Christman wrote:
> On Tue, Jul 21, 2015 at 3:12 AM, Pali Rohár <pali.rohar@gmail.com> wrote:
> > It is not enough. If do not want to recompile full kernel, but just only
> > psmouse.ko I think that the easiest way is to change this define
> >
> >  #define psmouse_dbg
> >
> > in file drivers/input/mouse/psmouse.h to call dev_warn instead dev_dbg.
> > And then just compile psmouse.ko and reload it.
> 
> Thanks, that seemed to do the trick.  This is what I get from dmesg now:
> 
> [    6.281814] psmouse serio1: cypress_ps2: send extension cmd 0x00, [0 0 0 0]
> [    6.540028] psmouse serio1: cypress_ps2: Command 0x00 response data
> (0x): 00 00 14
> [    6.582031] psmouse serio1: alps: E6 report: 00 00 64
> [    6.617471] psmouse serio1: alps: E7 report: 22 02 14
> [    6.656974] psmouse serio1: alps: EC report: 10 00 64
> [    7.104479] psmouse serio1: alps: E6 report: 00 00 64
> [    7.139714] psmouse serio1: alps: E7 report: 22 02 14
> [    7.176568] psmouse serio1: alps: EC report: 10 00 64
> [    7.368849] psmouse serio1: alps: F5 report: 05 01 0a
> [    7.541367] input: AlpsPS/2 ALPS DualPoint Stick as
> /devices/platform/i8042/serio1/input/input11
> [    7.571454] input: AlpsPS/2 ALPS DualPoint TouchPad as
> /devices/platform/i8042/serio1/input/input8

Ok, your ALPS touchpad is identified as:

{ { 0x22, 0x02, 0x14 }, 0x00, { ALPS_PROTO_V2, 0xff, 0xff, ALPS_PASS | ALPS_DUALPOINT } },	/* Dell Latitude D600 */

You mentioned that commit 92bac83d introduced your problem:

========================================
commit 92bac83dd79e60e65c475222e41a992a70434beb
Author: Hans de Goede <hdegoede@redhat.com>
Date:   Sun Apr 12 15:42:35 2015 -0700

    Input: alps - non interleaved V2 dualpoint has separate stick button bits
========================================

Hans, can you look at this problem as it is clearly problem with
protocol V2?

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

Re: ALPS DualPoint double click bug

[Benjamin Tissoires]

On Wed, Jul 22, 2015 at 3:21 AM, Pali Rohár <pali.rohar@gmail.com> wrote:
> On Tuesday 21 July 2015 19:51:48 Douglas Christman wrote:
>> On Tue, Jul 21, 2015 at 3:12 AM, Pali Rohár <pali.rohar@gmail.com> wrote:
>> > It is not enough. If do not want to recompile full kernel, but just only
>> > psmouse.ko I think that the easiest way is to change this define
>> >
>> >  #define psmouse_dbg
>> >
>> > in file drivers/input/mouse/psmouse.h to call dev_warn instead dev_dbg.
>> > And then just compile psmouse.ko and reload it.
>>
>> Thanks, that seemed to do the trick.  This is what I get from dmesg now:
>>
>> [    6.281814] psmouse serio1: cypress_ps2: send extension cmd 0x00, [0 0 0 0]
>> [    6.540028] psmouse serio1: cypress_ps2: Command 0x00 response data
>> (0x): 00 00 14
>> [    6.582031] psmouse serio1: alps: E6 report: 00 00 64
>> [    6.617471] psmouse serio1: alps: E7 report: 22 02 14
>> [    6.656974] psmouse serio1: alps: EC report: 10 00 64
>> [    7.104479] psmouse serio1: alps: E6 report: 00 00 64
>> [    7.139714] psmouse serio1: alps: E7 report: 22 02 14
>> [    7.176568] psmouse serio1: alps: EC report: 10 00 64
>> [    7.368849] psmouse serio1: alps: F5 report: 05 01 0a
>> [    7.541367] input: AlpsPS/2 ALPS DualPoint Stick as
>> /devices/platform/i8042/serio1/input/input11
>> [    7.571454] input: AlpsPS/2 ALPS DualPoint TouchPad as
>> /devices/platform/i8042/serio1/input/input8
>
> Ok, your ALPS touchpad is identified as:
>
> { { 0x22, 0x02, 0x14 }, 0x00, { ALPS_PROTO_V2, 0xff, 0xff, ALPS_PASS | ALPS_DUALPOINT } },      /* Dell Latitude D600 */
>
> You mentioned that commit 92bac83d introduced your problem:
>
> ========================================
> commit 92bac83dd79e60e65c475222e41a992a70434beb
> Author: Hans de Goede <hdegoede@redhat.com>
> Date:   Sun Apr 12 15:42:35 2015 -0700
>
>     Input: alps - non interleaved V2 dualpoint has separate stick button bits
> ========================================
>
> Hans, can you look at this problem as it is clearly problem with
> protocol V2?
>

Douglas, can you please make a recording of your touchpad/trackstick
with ps2emu-record?
Download it from https://github.com/Lyude/ps2emu , compile it and run
it as root:
- once the touchpad gets re-initialized:
- click once, wait a few seconds
- click twice, wait a few seconds
- click three times, wait few seconds
- use the touchpad a bit, wait a few secs
- use the trackstick (and its buttons) a bit
- Ctrl-C

And send us the resulting file ps2emu_record.txt that will be created
in the current directory.

Can you also confirm that your laptop has a trackstick as seen on
http://www.newegg.com/Product/Product.aspx?Item=N82E16834114551 ?

Cheers,
Benjamin

PS: I am not sure we tested ps2emu-record on an Alps touchpad before,
apologies if there are some hiccups...
--
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

Re: ALPS DualPoint double click bug

[Stephen Chandler Paul]

Hi! I looked at one of the recordings that was sent, and I just wanted
to clarify that the version you probably should be using to record
everything is ps2emu v0.1.2. The one that's in the master branch right
now is not entirely stable, and it's giving us a couple of problems
replaying the events on our machine. You can find the stable release
here:

	https://github.com/Lyude/ps2emu/releases/tag/v0.1.2

On Wed, 2015-07-22 at 11:08 -0400, Benjamin Tissoires wrote:
> On Wed, Jul 22, 2015 at 3:21 AM, Pali Rohár <pali.rohar@gmail.com> 
> wrote:
> > On Tuesday 21 July 2015 19:51:48 Douglas Christman wrote:
> > > On Tue, Jul 21, 2015 at 3:12 AM, Pali Rohár <pali.rohar@gmail.com
> > > > wrote:
> > > > It is not enough. If do not want to recompile full kernel, but 
> > > > just only
> > > > psmouse.ko I think that the easiest way is to change this 
> > > > define
> > > > 
> > > >  #define psmouse_dbg
> > > > 
> > > > in file drivers/input/mouse/psmouse.h to call dev_warn instead 
> > > > dev_dbg.
> > > > And then just compile psmouse.ko and reload it.
> > > 
> > > Thanks, that seemed to do the trick.  This is what I get from 
> > > dmesg now:
> > > 
> > > [    6.281814] psmouse serio1: cypress_ps2: send extension cmd 
> > > 0x00, [0 0 0 0]
> > > [    6.540028] psmouse serio1: cypress_ps2: Command 0x00 response 
> > > data
> > > (0x): 00 00 14
> > > [    6.582031] psmouse serio1: alps: E6 report: 00 00 64
> > > [    6.617471] psmouse serio1: alps: E7 report: 22 02 14
> > > [    6.656974] psmouse serio1: alps: EC report: 10 00 64
> > > [    7.104479] psmouse serio1: alps: E6 report: 00 00 64
> > > [    7.139714] psmouse serio1: alps: E7 report: 22 02 14
> > > [    7.176568] psmouse serio1: alps: EC report: 10 00 64
> > > [    7.368849] psmouse serio1: alps: F5 report: 05 01 0a
> > > [    7.541367] input: AlpsPS/2 ALPS DualPoint Stick as
> > > /devices/platform/i8042/serio1/input/input11
> > > [    7.571454] input: AlpsPS/2 ALPS DualPoint TouchPad as
> > > /devices/platform/i8042/serio1/input/input8
> > 
> > Ok, your ALPS touchpad is identified as:
> > 
> > { { 0x22, 0x02, 0x14 }, 0x00, { ALPS_PROTO_V2, 0xff, 0xff, 
> > ALPS_PASS | ALPS_DUALPOINT } },      /* Dell Latitude D600 */
> > 
> > You mentioned that commit 92bac83d introduced your problem:
> > 
> > ========================================
> > commit 92bac83dd79e60e65c475222e41a992a70434beb
> > Author: Hans de Goede <hdegoede@redhat.com>
> > Date:   Sun Apr 12 15:42:35 2015 -0700
> > 
> >     Input: alps - non interleaved V2 dualpoint has separate stick 
> > button bits
> > ========================================
> > 
> > Hans, can you look at this problem as it is clearly problem with
> > protocol V2?
> > 
> 
> Douglas, can you please make a recording of your touchpad/trackstick
> with ps2emu-record?
> Download it from https://github.com/Lyude/ps2emu , compile it and run
> it as root:
> - once the touchpad gets re-initialized:
> - click once, wait a few seconds
> - click twice, wait a few seconds
> - click three times, wait few seconds
> - use the touchpad a bit, wait a few secs
> - use the trackstick (and its buttons) a bit
> - Ctrl-C
> 
> And send us the resulting file ps2emu_record.txt that will be created
> in the current directory.
> 
> Can you also confirm that your laptop has a trackstick as seen on
> http://www.newegg.com/Product/Product.aspx?Item=N82E16834114551 ?
> 
> Cheers,
> Benjamin
> 
> PS: I am not sure we tested ps2emu-record on an Alps touchpad before,
> apologies if there are some hiccups...
-- 
Cheers,
	Stephen Chandler Paul
--
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

Re: ALPS DualPoint double click bug

[Hans de Goede]

Hi,

On 22-07-15 17:08, Benjamin Tissoires wrote:
> On Wed, Jul 22, 2015 at 3:21 AM, Pali Rohár <pali.rohar@gmail.com> wrote:
>> On Tuesday 21 July 2015 19:51:48 Douglas Christman wrote:
>>> On Tue, Jul 21, 2015 at 3:12 AM, Pali Rohár <pali.rohar@gmail.com> wrote:
>>>> It is not enough. If do not want to recompile full kernel, but just only
>>>> psmouse.ko I think that the easiest way is to change this define
>>>>
>>>>   #define psmouse_dbg
>>>>
>>>> in file drivers/input/mouse/psmouse.h to call dev_warn instead dev_dbg.
>>>> And then just compile psmouse.ko and reload it.
>>>
>>> Thanks, that seemed to do the trick.  This is what I get from dmesg now:
>>>
>>> [    6.281814] psmouse serio1: cypress_ps2: send extension cmd 0x00, [0 0 0 0]
>>> [    6.540028] psmouse serio1: cypress_ps2: Command 0x00 response data
>>> (0x): 00 00 14
>>> [    6.582031] psmouse serio1: alps: E6 report: 00 00 64
>>> [    6.617471] psmouse serio1: alps: E7 report: 22 02 14
>>> [    6.656974] psmouse serio1: alps: EC report: 10 00 64
>>> [    7.104479] psmouse serio1: alps: E6 report: 00 00 64
>>> [    7.139714] psmouse serio1: alps: E7 report: 22 02 14
>>> [    7.176568] psmouse serio1: alps: EC report: 10 00 64
>>> [    7.368849] psmouse serio1: alps: F5 report: 05 01 0a
>>> [    7.541367] input: AlpsPS/2 ALPS DualPoint Stick as
>>> /devices/platform/i8042/serio1/input/input11
>>> [    7.571454] input: AlpsPS/2 ALPS DualPoint TouchPad as
>>> /devices/platform/i8042/serio1/input/input8
>>
>> Ok, your ALPS touchpad is identified as:
>>
>> { { 0x22, 0x02, 0x14 }, 0x00, { ALPS_PROTO_V2, 0xff, 0xff, ALPS_PASS | ALPS_DUALPOINT } },      /* Dell Latitude D600 */
>>
>> You mentioned that commit 92bac83d introduced your problem:
>>
>> ========================================
>> commit 92bac83dd79e60e65c475222e41a992a70434beb
>> Author: Hans de Goede <hdegoede@redhat.com>
>> Date:   Sun Apr 12 15:42:35 2015 -0700
>>
>>      Input: alps - non interleaved V2 dualpoint has separate stick button bits
>> ========================================
>>
>> Hans, can you look at this problem as it is clearly problem with
>> protocol V2?
>>
>
> Douglas, can you please make a recording of your touchpad/trackstick
> with ps2emu-record?
> Download it from https://github.com/Lyude/ps2emu , compile it and run
> it as root:
> - once the touchpad gets re-initialized:
> - click once, wait a few seconds
> - click twice, wait a few seconds
> - click three times, wait few seconds
> - use the touchpad a bit, wait a few secs
> - use the trackstick (and its buttons) a bit
> - Ctrl-C
>
> And send us the resulting file ps2emu_record.txt that will be created
> in the current directory.
>
> Can you also confirm that your laptop has a trackstick as seen on
> http://www.newegg.com/Product/Product.aspx?Item=N82E16834114551 ?

Douglas,

Can you please also make an evemu recording (install evemu, run evemu-record)
of clicking of the 4 buttons? PLease make 1 recording per button click.

Also can you confirm that reverting commit 92bac83dd79e on an otherwise
unmodified 4.1 kernel fixes things for you ?

Regards,

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

Re: ALPS DualPoint double click bug

[Douglas Christman]

On Thu, Jul 23, 2015 at 5:31 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
> On 22-07-15 17:08, Benjamin Tissoires wrote:
>>
>> Douglas, can you please make a recording of your touchpad/trackstick
>> with ps2emu-record?
>> Download it from https://github.com/Lyude/ps2emu , compile it and run
>> it as root:
>> - once the touchpad gets re-initialized:
>> - click once, wait a few seconds
>> - click twice, wait a few seconds
>> - click three times, wait few seconds
>> - use the touchpad a bit, wait a few secs
>> - use the trackstick (and its buttons) a bit
>> - Ctrl-C
>>
>> And send us the resulting file ps2emu_record.txt that will be created
>> in the current directory.
>>
>> Can you also confirm that your laptop has a trackstick as seen on
>> http://www.newegg.com/Product/Product.aspx?Item=N82E16834114551 ?
>
>
> Douglas,
>
> Can you please also make an evemu recording (install evemu, run
> evemu-record)
> of clicking of the 4 buttons? PLease make 1 recording per button click.
>
> Also can you confirm that reverting commit 92bac83dd79e on an otherwise
> unmodified 4.1 kernel fixes things for you ?
>
> Regards,
>
> Hans

I've uploaded all of my recordings to
https://github.com/dobyrch/alps-recordings.  I made recordings on both
kernel versions, as indicated by the "good" or "bad" suffix.

Benjamin, I can confirm that my touchpad looks just like the one in
the picture--four buttons and a trackstick.

While making these recordings, I discovered another strange behavior.
The two lower mouse buttons do not trigger any mouse events when
clicked on their own.  However, If I click one of the top mouse
buttons followed by a lower mouse button, it triggers a press event
(but no release) for all three mouse buttons--left, right, and middle
(even though I don't have a middle mouse button).

Hans, I'll have to double-check that reverting 92bac on v4.1 fixes
things.  When I have a chance to recompile I'll let you know the
outcome.

Thanks everyone,
Doug

Re: ALPS DualPoint double click bug

[Hans de Goede]

Hi,

On 25-07-15 16:07, Douglas Christman wrote:
> On Thu, Jul 23, 2015 at 5:31 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
>>
>> On 22-07-15 17:08, Benjamin Tissoires wrote:
>>>
>>> Douglas, can you please make a recording of your touchpad/trackstick
>>> with ps2emu-record?
>>> Download it from https://github.com/Lyude/ps2emu , compile it and run
>>> it as root:
>>> - once the touchpad gets re-initialized:
>>> - click once, wait a few seconds
>>> - click twice, wait a few seconds
>>> - click three times, wait few seconds
>>> - use the touchpad a bit, wait a few secs
>>> - use the trackstick (and its buttons) a bit
>>> - Ctrl-C
>>>
>>> And send us the resulting file ps2emu_record.txt that will be created
>>> in the current directory.
>>>
>>> Can you also confirm that your laptop has a trackstick as seen on
>>> http://www.newegg.com/Product/Product.aspx?Item=N82E16834114551 ?
>>
>>
>> Douglas,
>>
>> Can you please also make an evemu recording (install evemu, run
>> evemu-record)
>> of clicking of the 4 buttons? PLease make 1 recording per button click.
>>
>> Also can you confirm that reverting commit 92bac83dd79e on an otherwise
>> unmodified 4.1 kernel fixes things for you ?
>>
>> Regards,
>>
>> Hans
>
> I've uploaded all of my recordings to
> https://github.com/dobyrch/alps-recordings.  I made recordings on both
> kernel versions, as indicated by the "good" or "bad" suffix.

Thanks.

> Benjamin, I can confirm that my touchpad looks just like the one in
> the picture--four buttons and a trackstick.
>
> While making these recordings, I discovered another strange behavior.
> The two lower mouse buttons do not trigger any mouse events when
> clicked on their own.  However, If I click one of the top mouse
> buttons followed by a lower mouse button, it triggers a press event
> (but no release) for all three mouse buttons--left, right, and middle
> (even though I don't have a middle mouse button).
>
> Hans, I'll have to double-check that reverting 92bac on v4.1 fixes
> things.  When I have a chance to recompile I'll let you know the
> outcome.

OK, please let me know soon, I would like to get to the bottom
of this, and knowing the exact commit causing the problem will
help a lot.

Regards,

hans

Re: ALPS DualPoint double click bug

[Douglas Christman]

On Mon, Jul 27, 2015 at 12:40 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>
> OK, please let me know soon, I would like to get to the bottom
> of this, and knowing the exact commit causing the problem will
> help a lot.
>
> Regards,
>
> hans

I've verified that reverting 92bac83d on a clean v4.1 kernel
(b953c0d2) resolves the issue.

Doug

[PATCH 0/1] Alps button reporting bugfix

[cpaul]

From: Stephen Chandler Paul <cpaul@redhat.com>

Hi! This was a perfect test case for ps2emu. I managed to reproduce the issue
you were having perfectly, and I'm pretty sure I've debugged the issue and
written a patch to fix it. Would you mind applying the next patch I'll be
sending to a clean build of the latest kernel, and telling me if it fixes your
issue or not?

Cheers,
	Stephen Chandler Paul

Stephen Chandler Paul (1):
  Input - alps: Fix button reporting on the V2 Alps protocol

 drivers/input/mouse/alps.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

-- 
2.4.3

[PATCH 1/1] Input - alps: Fix button reporting on the V2 Alps protocol

[cpaul]

From: Stephen Chandler Paul <cpaul@redhat.com>

The data concerning which buttons on the touchpad are held down or not
are in the fourth packet we receive from the mouse, not the first.

Signed-off-by: Stephen Chandler Paul <cpaul@redhat.com>
---
 drivers/input/mouse/alps.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index 113d6f1..e2f9b25 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -254,9 +254,9 @@ static void alps_process_packet_v1_v2(struct psmouse *psmouse)
 	/* Non interleaved V2 dualpoint has separate stick button bits */
 	if (priv->proto_version == ALPS_PROTO_V2 &&
 	    priv->flags == (ALPS_PASS | ALPS_DUALPOINT)) {
-		left |= packet[0] & 1;
-		right |= packet[0] & 2;
-		middle |= packet[0] & 4;
+		left |= packet[3] & 1;
+		right |= packet[3] & 2;
+		middle |= packet[3] & 4;
 	}
 
 	alps_report_buttons(dev, dev2, left, right, middle);
-- 
2.4.3

Re: [PATCH 1/1] Input - alps: Fix button reporting on the V2 Alps protocol

[Dmitry Torokhov]

On Wed, Jul 29, 2015 at 04:45:26PM -0400, cpaul@redhat.com wrote:
> From: Stephen Chandler Paul <cpaul@redhat.com>
> 
> The data concerning which buttons on the touchpad are held down or not
> are in the fourth packet we receive from the mouse, not the first.
> 
> Signed-off-by: Stephen Chandler Paul <cpaul@redhat.com>

Let's also make sure it hoes not break Hans' (de Bruin) touchpad.

> ---
>  drivers/input/mouse/alps.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> index 113d6f1..e2f9b25 100644
> --- a/drivers/input/mouse/alps.c
> +++ b/drivers/input/mouse/alps.c
> @@ -254,9 +254,9 @@ static void alps_process_packet_v1_v2(struct psmouse *psmouse)
>  	/* Non interleaved V2 dualpoint has separate stick button bits */
>  	if (priv->proto_version == ALPS_PROTO_V2 &&
>  	    priv->flags == (ALPS_PASS | ALPS_DUALPOINT)) {
> -		left |= packet[0] & 1;
> -		right |= packet[0] & 2;
> -		middle |= packet[0] & 4;
> +		left |= packet[3] & 1;
> +		right |= packet[3] & 2;
> +		middle |= packet[3] & 4;
>  	}
>  
>  	alps_report_buttons(dev, dev2, left, right, middle);
> -- 
> 2.4.3
> 

-- 
Dmitry

Re: [PATCH 1/1] Input - alps: Fix button reporting on the V2 Alps protocol

[Pali Rohár]

On Wednesday 29 July 2015 16:45:26 cpaul@redhat.com wrote:
> From: Stephen Chandler Paul <cpaul@redhat.com>
> 
> The data concerning which buttons on the touchpad are held down or not
> are in the fourth packet we receive from the mouse, not the first.
> 
> Signed-off-by: Stephen Chandler Paul <cpaul@redhat.com>
> ---
>  drivers/input/mouse/alps.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> index 113d6f1..e2f9b25 100644
> --- a/drivers/input/mouse/alps.c
> +++ b/drivers/input/mouse/alps.c
> @@ -254,9 +254,9 @@ static void alps_process_packet_v1_v2(struct psmouse *psmouse)
>  	/* Non interleaved V2 dualpoint has separate stick button bits */
>  	if (priv->proto_version == ALPS_PROTO_V2 &&
>  	    priv->flags == (ALPS_PASS | ALPS_DUALPOINT)) {
> -		left |= packet[0] & 1;
> -		right |= packet[0] & 2;
> -		middle |= packet[0] & 4;
> +		left |= packet[3] & 1;
> +		right |= packet[3] & 2;
> +		middle |= packet[3] & 4;
>  	}
>  
>  	alps_report_buttons(dev, dev2, left, right, middle);

This patch will break other ALPS devices...
So we cannot accept it :-(

Looks like some proto v2 devices report button clicks in packet[0] and
some in packet[3].

Any idea how to fix it correctly? Do you have other laptops with proto
V2 ALPS devices, to do more tests and dumps and distinguish buttons?

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

Re: [PATCH 1/1] Input - alps: Fix button reporting on the V2 Alps protocol

[Hans de Goede]

Hi Chandler,

On 29-07-15 22:45, cpaul@redhat.com wrote:
> From: Stephen Chandler Paul <cpaul@redhat.com>
>
> The data concerning which buttons on the touchpad are held down or not
> are in the fourth packet we receive from the mouse, not the first.
>
> Signed-off-by: Stephen Chandler Paul <cpaul@redhat.com>
> ---
>   drivers/input/mouse/alps.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> index 113d6f1..e2f9b25 100644
> --- a/drivers/input/mouse/alps.c
> +++ b/drivers/input/mouse/alps.c
> @@ -254,9 +254,9 @@ static void alps_process_packet_v1_v2(struct psmouse *psmouse)
>   	/* Non interleaved V2 dualpoint has separate stick button bits */
>   	if (priv->proto_version == ALPS_PROTO_V2 &&
>   	    priv->flags == (ALPS_PASS | ALPS_DUALPOINT)) {
> -		left |= packet[0] & 1;
> -		right |= packet[0] & 2;
> -		middle |= packet[0] & 4;
> +		left |= packet[3] & 1;
> +		right |= packet[3] & 2;
> +		middle |= packet[3] & 4;
>   	}
>
>   	alps_report_buttons(dev, dev2, left, right, middle);

Thanks for taking a look at the recordings, but the above patch is wrong,
if you look slightly higher in the lps_process_packet_v1_v2() function there
is this:

if (priv->proto_version == ALPS_PROTO_V1) {
...
} else {
	left = packet[3] & 1;
	right = packet[3] & 2;
	middle = packet[3] & 4;
}

So with your patch for the devices in question the entire code flow
becomes:

	left = packet[3] & 1;
	right = packet[3] & 2;
	middle = packet[3] & 4;
	left |= packet[3] & 1;
	right |= packet[3] & 2;
	middle |= packet[3] & 4;

Which is not really helpful for the devices for which I added
commit 92bac83dd:

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/input/mouse/alps.c?id=92bac83dd79e60e65c475222e41a992a70434beb

and will cause these devices to regress.

Since Hans de Bruin's laptop is a Dell Latitude D430 and I saw
the same problem and tested my patch on a Dell Latitude D630,
it seems the use of the low bits of packet[0] to report the
trackpoint buttons separately when the touchpad is active is
a Dell specific thing, so I believe that a patch to only
activate this code block on Dell's is the right solution for
the regression Douglas is seeing.

I'll write such a patch and post it shortly.

Regards,

Hans

Re: [PATCH 1/1] Input - alps: Fix button reporting on the V2 Alps protocol

[Pali Rohár]

On Thursday 30 July 2015 15:51:25 Hans de Goede wrote:
> Hi Chandler,
> 
> On 29-07-15 22:45, cpaul@redhat.com wrote:
> >From: Stephen Chandler Paul <cpaul@redhat.com>
> >
> >The data concerning which buttons on the touchpad are held down or not
> >are in the fourth packet we receive from the mouse, not the first.
> >
> >Signed-off-by: Stephen Chandler Paul <cpaul@redhat.com>
> >---
> >  drivers/input/mouse/alps.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> >diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> >index 113d6f1..e2f9b25 100644
> >--- a/drivers/input/mouse/alps.c
> >+++ b/drivers/input/mouse/alps.c
> >@@ -254,9 +254,9 @@ static void alps_process_packet_v1_v2(struct psmouse *psmouse)
> >  	/* Non interleaved V2 dualpoint has separate stick button bits */
> >  	if (priv->proto_version == ALPS_PROTO_V2 &&
> >  	    priv->flags == (ALPS_PASS | ALPS_DUALPOINT)) {
> >-		left |= packet[0] & 1;
> >-		right |= packet[0] & 2;
> >-		middle |= packet[0] & 4;
> >+		left |= packet[3] & 1;
> >+		right |= packet[3] & 2;
> >+		middle |= packet[3] & 4;
> >  	}
> >
> >  	alps_report_buttons(dev, dev2, left, right, middle);
> 
> Thanks for taking a look at the recordings, but the above patch is wrong,
> if you look slightly higher in the lps_process_packet_v1_v2() function there
> is this:
> 
> if (priv->proto_version == ALPS_PROTO_V1) {
> ...
> } else {
> 	left = packet[3] & 1;
> 	right = packet[3] & 2;
> 	middle = packet[3] & 4;
> }
> 
> So with your patch for the devices in question the entire code flow
> becomes:
> 
> 	left = packet[3] & 1;
> 	right = packet[3] & 2;
> 	middle = packet[3] & 4;
> 	left |= packet[3] & 1;
> 	right |= packet[3] & 2;
> 	middle |= packet[3] & 4;
> 
> Which is not really helpful for the devices for which I added
> commit 92bac83dd:
> 
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/input/mouse/alps.c?id=92bac83dd79e60e65c475222e41a992a70434beb
> 
> and will cause these devices to regress.
> 
> Since Hans de Bruin's laptop is a Dell Latitude D430 and I saw
> the same problem and tested my patch on a Dell Latitude D630,
> it seems the use of the low bits of packet[0] to report the
> trackpoint buttons separately when the touchpad is active is
> a Dell specific thing, so I believe that a patch to only
> activate this code block on Dell's is the right solution for
> the regression Douglas is seeing.
> 
> I'll write such a patch and post it shortly.
> 
> Regards,
> 
> Hans
> 
> 
> 

Hans, can you check ec and e7 registers if are same or if they differs?

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

Re: ALPS DualPoint double click bug

[Hans de Goede]

[-- Attachment #1: Type: text/plain, Size: 527 bytes --]

Hi,

On 28-07-15 01:38, Douglas Christman wrote:
> On Mon, Jul 27, 2015 at 12:40 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> OK, please let me know soon, I would like to get to the bottom
>> of this, and knowing the exact commit causing the problem will
>> help a lot.
>>
>> Regards,
>>
>> hans
>
> I've verified that reverting 92bac83d on a clean v4.1 kernel
> (b953c0d2) resolves the issue.

Thanks,

Can you please apply the attached patch on a clean v4.1 kernel,
and confirm that that fixes this ?

Regards,

Hans

[-- Attachment #2: 0001-alps-Only-Dell-laptops-have-separate-button-bits-for.patch --]
[-- Type: text/x-patch, Size: 1765 bytes --]

>From ee3d5d5a298b178ae5284b9766ca849665a37670 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Thu, 30 Jul 2015 15:49:16 +0200
Subject: [PATCH] alps: Only Dell laptops have separate button bits for v2
 dualpoint sticks

It turns out that only Dell laptops have the separate button bits for
v2 dualpoint sticks and that commit 92bac83dd79e ("Input: alps - non
interleaved V2 dualpoint has separate stick button bits") causes
regressions on Toshiba laptops.

This commit adds a check for Dell laptops to the code for handling these
extra button bits, fixing this regression.

This patch has been tested on a Dell Latitude D620 to make sure that it
does not reintroduce the original problem.

Reported-by: Douglas Christman <douglaschristman@gmail.com>
Cc: stable@vger.kernel.org
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/input/mouse/alps.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index 113d6f1..889aec1 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -20,6 +20,7 @@
 #include <linux/input/mt.h>
 #include <linux/serio.h>
 #include <linux/libps2.h>
+#include <linux/dmi.h>
 
 #include "psmouse.h"
 #include "alps.h"
@@ -251,8 +252,9 @@ static void alps_process_packet_v1_v2(struct psmouse *psmouse)
 		return;
 	}
 
-	/* Non interleaved V2 dualpoint has separate stick button bits */
-	if (priv->proto_version == ALPS_PROTO_V2 &&
+	/* Dell non interleaved V2 dualpoint has separate stick button bits */
+	if (dmi_name_in_vendors("Dell") &&
+	    priv->proto_version == ALPS_PROTO_V2 &&
 	    priv->flags == (ALPS_PASS | ALPS_DUALPOINT)) {
 		left |= packet[0] & 1;
 		right |= packet[0] & 2;
-- 
2.4.3

Re: [PATCH 1/1] Input - alps: Fix button reporting on the V2 Alps protocol

[Hans de Goede]

Hi,

On 30-07-15 16:11, Pali Rohár wrote:
> On Thursday 30 July 2015 15:51:25 Hans de Goede wrote:
>> Hi Chandler,
>>
>> On 29-07-15 22:45, cpaul@redhat.com wrote:
>>> From: Stephen Chandler Paul <cpaul@redhat.com>
>>>
>>> The data concerning which buttons on the touchpad are held down or not
>>> are in the fourth packet we receive from the mouse, not the first.
>>>
>>> Signed-off-by: Stephen Chandler Paul <cpaul@redhat.com>
>>> ---
>>>   drivers/input/mouse/alps.c | 6 +++---
>>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
>>> index 113d6f1..e2f9b25 100644
>>> --- a/drivers/input/mouse/alps.c
>>> +++ b/drivers/input/mouse/alps.c
>>> @@ -254,9 +254,9 @@ static void alps_process_packet_v1_v2(struct psmouse *psmouse)
>>>   	/* Non interleaved V2 dualpoint has separate stick button bits */
>>>   	if (priv->proto_version == ALPS_PROTO_V2 &&
>>>   	    priv->flags == (ALPS_PASS | ALPS_DUALPOINT)) {
>>> -		left |= packet[0] & 1;
>>> -		right |= packet[0] & 2;
>>> -		middle |= packet[0] & 4;
>>> +		left |= packet[3] & 1;
>>> +		right |= packet[3] & 2;
>>> +		middle |= packet[3] & 4;
>>>   	}
>>>
>>>   	alps_report_buttons(dev, dev2, left, right, middle);
>>
>> Thanks for taking a look at the recordings, but the above patch is wrong,
>> if you look slightly higher in the lps_process_packet_v1_v2() function there
>> is this:
>>
>> if (priv->proto_version == ALPS_PROTO_V1) {
>> ...
>> } else {
>> 	left = packet[3] & 1;
>> 	right = packet[3] & 2;
>> 	middle = packet[3] & 4;
>> }
>>
>> So with your patch for the devices in question the entire code flow
>> becomes:
>>
>> 	left = packet[3] & 1;
>> 	right = packet[3] & 2;
>> 	middle = packet[3] & 4;
>> 	left |= packet[3] & 1;
>> 	right |= packet[3] & 2;
>> 	middle |= packet[3] & 4;
>>
>> Which is not really helpful for the devices for which I added
>> commit 92bac83dd:
>>
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/input/mouse/alps.c?id=92bac83dd79e60e65c475222e41a992a70434beb
>>
>> and will cause these devices to regress.
>>
>> Since Hans de Bruin's laptop is a Dell Latitude D430 and I saw
>> the same problem and tested my patch on a Dell Latitude D630,
>> it seems the use of the low bits of packet[0] to report the
>> trackpoint buttons separately when the touchpad is active is
>> a Dell specific thing, so I believe that a patch to only
>> activate this code block on Dell's is the right solution for
>> the regression Douglas is seeing.
>>
>> I'll write such a patch and post it shortly.
>>
>> Regards,
>>
>> Hans
>>
>>
>>
>
> Hans, can you check ec and e7 registers if are same or if they differs?

As Benjamin already pointed out Douglas' touchpad matches this
line in alps.c :

{ { 0x22, 0x02, 0x14 }, 0x00, { ALPS_PROTO_V2, 0xff, 0xff, ALPS_PASS | ALPS_DUALPOINT } },      /* Dell Latitude D600 */

This is a full match, and note the "Dell" in the comment after the line,
so it seems that going by the registers is not good enough here, whereas
doing a vendor check is actually quite easy, so the patch I just
send.

Regards,

Hans


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

Re: [PATCH 1/1] Input - alps: Fix button reporting on the V2 Alps protocol

[Pali Rohár]

On Thursday 30 July 2015 16:18:47 Hans de Goede wrote:
> Hi,
> 
> On 30-07-15 16:11, Pali Rohár wrote:
> >On Thursday 30 July 2015 15:51:25 Hans de Goede wrote:
> >>Hi Chandler,
> >>
> >>On 29-07-15 22:45, cpaul@redhat.com wrote:
> >>>From: Stephen Chandler Paul <cpaul@redhat.com>
> >>>
> >>>The data concerning which buttons on the touchpad are held down or not
> >>>are in the fourth packet we receive from the mouse, not the first.
> >>>
> >>>Signed-off-by: Stephen Chandler Paul <cpaul@redhat.com>
> >>>---
> >>>  drivers/input/mouse/alps.c | 6 +++---
> >>>  1 file changed, 3 insertions(+), 3 deletions(-)
> >>>
> >>>diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> >>>index 113d6f1..e2f9b25 100644
> >>>--- a/drivers/input/mouse/alps.c
> >>>+++ b/drivers/input/mouse/alps.c
> >>>@@ -254,9 +254,9 @@ static void alps_process_packet_v1_v2(struct psmouse *psmouse)
> >>>  	/* Non interleaved V2 dualpoint has separate stick button bits */
> >>>  	if (priv->proto_version == ALPS_PROTO_V2 &&
> >>>  	    priv->flags == (ALPS_PASS | ALPS_DUALPOINT)) {
> >>>-		left |= packet[0] & 1;
> >>>-		right |= packet[0] & 2;
> >>>-		middle |= packet[0] & 4;
> >>>+		left |= packet[3] & 1;
> >>>+		right |= packet[3] & 2;
> >>>+		middle |= packet[3] & 4;
> >>>  	}
> >>>
> >>>  	alps_report_buttons(dev, dev2, left, right, middle);
> >>
> >>Thanks for taking a look at the recordings, but the above patch is wrong,
> >>if you look slightly higher in the lps_process_packet_v1_v2() function there
> >>is this:
> >>
> >>if (priv->proto_version == ALPS_PROTO_V1) {
> >>...
> >>} else {
> >>	left = packet[3] & 1;
> >>	right = packet[3] & 2;
> >>	middle = packet[3] & 4;
> >>}
> >>
> >>So with your patch for the devices in question the entire code flow
> >>becomes:
> >>
> >>	left = packet[3] & 1;
> >>	right = packet[3] & 2;
> >>	middle = packet[3] & 4;
> >>	left |= packet[3] & 1;
> >>	right |= packet[3] & 2;
> >>	middle |= packet[3] & 4;
> >>
> >>Which is not really helpful for the devices for which I added
> >>commit 92bac83dd:
> >>
> >>https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/input/mouse/alps.c?id=92bac83dd79e60e65c475222e41a992a70434beb
> >>
> >>and will cause these devices to regress.
> >>
> >>Since Hans de Bruin's laptop is a Dell Latitude D430 and I saw
> >>the same problem and tested my patch on a Dell Latitude D630,
> >>it seems the use of the low bits of packet[0] to report the
> >>trackpoint buttons separately when the touchpad is active is
> >>a Dell specific thing, so I believe that a patch to only
> >>activate this code block on Dell's is the right solution for
> >>the regression Douglas is seeing.
> >>
> >>I'll write such a patch and post it shortly.
> >>
> >>Regards,
> >>
> >>Hans
> >>
> >>
> >>
> >
> >Hans, can you check ec and e7 registers if are same or if they differs?
> 
> As Benjamin already pointed out Douglas' touchpad matches this
> line in alps.c :
> 
> { { 0x22, 0x02, 0x14 }, 0x00, { ALPS_PROTO_V2, 0xff, 0xff, ALPS_PASS | ALPS_DUALPOINT } },      /* Dell Latitude D600 */
> 
> This is a full match

No, this is not full match. It just check e7, not ec. This is reason why
I asked for both ec and e7 registers of all affected machines.

> and note the "Dell" in the comment after the line,
> so it seems that going by the registers is not good enough here, whereas
> doing a vendor check is actually quite easy, so the patch I just
> send.
> 

If registers are different, then I think it is good idea to check for
them. I do not like vendor checks in such device driver unless we know
that there is some special vendor code/bug in firmware which cause it...

> Regards,
> 
> Hans
> 
> 

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

Re: [PATCH 1/1] Input - alps: Fix button reporting on the V2 Alps protocol

[Pali Rohár]

On Thursday 30 July 2015 16:28:39 Pali Rohár wrote:
> On Thursday 30 July 2015 16:18:47 Hans de Goede wrote:
> > Hi,
> > 
> > On 30-07-15 16:11, Pali Rohár wrote:
> > >On Thursday 30 July 2015 15:51:25 Hans de Goede wrote:
> > >>Hi Chandler,
> > >>
> > >>On 29-07-15 22:45, cpaul@redhat.com wrote:
> > >>>From: Stephen Chandler Paul <cpaul@redhat.com>
> > >>>
> > >>>The data concerning which buttons on the touchpad are held down or not
> > >>>are in the fourth packet we receive from the mouse, not the first.
> > >>>
> > >>>Signed-off-by: Stephen Chandler Paul <cpaul@redhat.com>
> > >>>---
> > >>>  drivers/input/mouse/alps.c | 6 +++---
> > >>>  1 file changed, 3 insertions(+), 3 deletions(-)
> > >>>
> > >>>diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> > >>>index 113d6f1..e2f9b25 100644
> > >>>--- a/drivers/input/mouse/alps.c
> > >>>+++ b/drivers/input/mouse/alps.c
> > >>>@@ -254,9 +254,9 @@ static void alps_process_packet_v1_v2(struct psmouse *psmouse)
> > >>>  	/* Non interleaved V2 dualpoint has separate stick button bits */
> > >>>  	if (priv->proto_version == ALPS_PROTO_V2 &&
> > >>>  	    priv->flags == (ALPS_PASS | ALPS_DUALPOINT)) {
> > >>>-		left |= packet[0] & 1;
> > >>>-		right |= packet[0] & 2;
> > >>>-		middle |= packet[0] & 4;
> > >>>+		left |= packet[3] & 1;
> > >>>+		right |= packet[3] & 2;
> > >>>+		middle |= packet[3] & 4;
> > >>>  	}
> > >>>
> > >>>  	alps_report_buttons(dev, dev2, left, right, middle);
> > >>
> > >>Thanks for taking a look at the recordings, but the above patch is wrong,
> > >>if you look slightly higher in the lps_process_packet_v1_v2() function there
> > >>is this:
> > >>
> > >>if (priv->proto_version == ALPS_PROTO_V1) {
> > >>...
> > >>} else {
> > >>	left = packet[3] & 1;
> > >>	right = packet[3] & 2;
> > >>	middle = packet[3] & 4;
> > >>}
> > >>
> > >>So with your patch for the devices in question the entire code flow
> > >>becomes:
> > >>
> > >>	left = packet[3] & 1;
> > >>	right = packet[3] & 2;
> > >>	middle = packet[3] & 4;
> > >>	left |= packet[3] & 1;
> > >>	right |= packet[3] & 2;
> > >>	middle |= packet[3] & 4;
> > >>
> > >>Which is not really helpful for the devices for which I added
> > >>commit 92bac83dd:
> > >>
> > >>https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/input/mouse/alps.c?id=92bac83dd79e60e65c475222e41a992a70434beb
> > >>
> > >>and will cause these devices to regress.
> > >>
> > >>Since Hans de Bruin's laptop is a Dell Latitude D430 and I saw
> > >>the same problem and tested my patch on a Dell Latitude D630,
> > >>it seems the use of the low bits of packet[0] to report the
> > >>trackpoint buttons separately when the touchpad is active is
> > >>a Dell specific thing, so I believe that a patch to only
> > >>activate this code block on Dell's is the right solution for
> > >>the regression Douglas is seeing.
> > >>
> > >>I'll write such a patch and post it shortly.
> > >>
> > >>Regards,
> > >>
> > >>Hans
> > >>
> > >>
> > >>
> > >
> > >Hans, can you check ec and e7 registers if are same or if they differs?
> > 
> > As Benjamin already pointed out Douglas' touchpad matches this
> > line in alps.c :
> > 
> > { { 0x22, 0x02, 0x14 }, 0x00, { ALPS_PROTO_V2, 0xff, 0xff, ALPS_PASS | ALPS_DUALPOINT } },      /* Dell Latitude D600 */
> > 
> > This is a full match
> 
> No, this is not full match. It just check e7, not ec. This is reason why
> I asked for both ec and e7 registers of all affected machines.
> 

Douglas already sent e7 and ec regs:
[    6.617471] psmouse serio1: alps: E7 report: 22 02 14
[    6.656974] psmouse serio1: alps: EC report: 10 00 64

Can you post ec regs from your tested Dell machine?

> > and note the "Dell" in the comment after the line,
> > so it seems that going by the registers is not good enough here, whereas
> > doing a vendor check is actually quite easy, so the patch I just
> > send.
> > 
> 
> If registers are different, then I think it is good idea to check for
> them. I do not like vendor checks in such device driver unless we know
> that there is some special vendor code/bug in firmware which cause it...
> 
> > Regards,
> > 
> > Hans
> > 
> > 
> 

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

Re: [PATCH 1/1] Input - alps: Fix button reporting on the V2 Alps protocol

[Hans de Goede]

Hi,

On 30-07-15 16:32, Pali Rohár wrote:
> On Thursday 30 July 2015 16:28:39 Pali Rohár wrote:
>> On Thursday 30 July 2015 16:18:47 Hans de Goede wrote:
>>> Hi,
>>>
>>> On 30-07-15 16:11, Pali Rohár wrote:
>>>> On Thursday 30 July 2015 15:51:25 Hans de Goede wrote:
>>>>> Hi Chandler,
>>>>>
>>>>> On 29-07-15 22:45, cpaul@redhat.com wrote:
>>>>>> From: Stephen Chandler Paul <cpaul@redhat.com>
>>>>>>
>>>>>> The data concerning which buttons on the touchpad are held down or not
>>>>>> are in the fourth packet we receive from the mouse, not the first.
>>>>>>
>>>>>> Signed-off-by: Stephen Chandler Paul <cpaul@redhat.com>
>>>>>> ---
>>>>>>   drivers/input/mouse/alps.c | 6 +++---
>>>>>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
>>>>>> index 113d6f1..e2f9b25 100644
>>>>>> --- a/drivers/input/mouse/alps.c
>>>>>> +++ b/drivers/input/mouse/alps.c
>>>>>> @@ -254,9 +254,9 @@ static void alps_process_packet_v1_v2(struct psmouse *psmouse)
>>>>>>   	/* Non interleaved V2 dualpoint has separate stick button bits */
>>>>>>   	if (priv->proto_version == ALPS_PROTO_V2 &&
>>>>>>   	    priv->flags == (ALPS_PASS | ALPS_DUALPOINT)) {
>>>>>> -		left |= packet[0] & 1;
>>>>>> -		right |= packet[0] & 2;
>>>>>> -		middle |= packet[0] & 4;
>>>>>> +		left |= packet[3] & 1;
>>>>>> +		right |= packet[3] & 2;
>>>>>> +		middle |= packet[3] & 4;
>>>>>>   	}
>>>>>>
>>>>>>   	alps_report_buttons(dev, dev2, left, right, middle);
>>>>>
>>>>> Thanks for taking a look at the recordings, but the above patch is wrong,
>>>>> if you look slightly higher in the lps_process_packet_v1_v2() function there
>>>>> is this:
>>>>>
>>>>> if (priv->proto_version == ALPS_PROTO_V1) {
>>>>> ...
>>>>> } else {
>>>>> 	left = packet[3] & 1;
>>>>> 	right = packet[3] & 2;
>>>>> 	middle = packet[3] & 4;
>>>>> }
>>>>>
>>>>> So with your patch for the devices in question the entire code flow
>>>>> becomes:
>>>>>
>>>>> 	left = packet[3] & 1;
>>>>> 	right = packet[3] & 2;
>>>>> 	middle = packet[3] & 4;
>>>>> 	left |= packet[3] & 1;
>>>>> 	right |= packet[3] & 2;
>>>>> 	middle |= packet[3] & 4;
>>>>>
>>>>> Which is not really helpful for the devices for which I added
>>>>> commit 92bac83dd:
>>>>>
>>>>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/input/mouse/alps.c?id=92bac83dd79e60e65c475222e41a992a70434beb
>>>>>
>>>>> and will cause these devices to regress.
>>>>>
>>>>> Since Hans de Bruin's laptop is a Dell Latitude D430 and I saw
>>>>> the same problem and tested my patch on a Dell Latitude D630,
>>>>> it seems the use of the low bits of packet[0] to report the
>>>>> trackpoint buttons separately when the touchpad is active is
>>>>> a Dell specific thing, so I believe that a patch to only
>>>>> activate this code block on Dell's is the right solution for
>>>>> the regression Douglas is seeing.
>>>>>
>>>>> I'll write such a patch and post it shortly.
>>>>>
>>>>> Regards,
>>>>>
>>>>> Hans
>>>>>
>>>>>
>>>>>
>>>>
>>>> Hans, can you check ec and e7 registers if are same or if they differs?
>>>
>>> As Benjamin already pointed out Douglas' touchpad matches this
>>> line in alps.c :
>>>
>>> { { 0x22, 0x02, 0x14 }, 0x00, { ALPS_PROTO_V2, 0xff, 0xff, ALPS_PASS | ALPS_DUALPOINT } },      /* Dell Latitude D600 */
>>>
>>> This is a full match
>>
>> No, this is not full match. It just check e7, not ec. This is reason why
>> I asked for both ec and e7 registers of all affected machines.
>>
>
> Douglas already sent e7 and ec regs:
> [    6.617471] psmouse serio1: alps: E7 report: 22 02 14
> [    6.656974] psmouse serio1: alps: EC report: 10 00 64
>
> Can you post ec regs from your tested Dell machine?

[    1.906031] psmouse serio1: alps: EC report: 10 00 64

Regards,

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

Re: [PATCH 1/1] Input - alps: Fix button reporting on the V2 Alps protocol

[Pali Rohár]

On Thursday 30 July 2015 16:38:47 Hans de Goede wrote:
> Hi,
> 
> On 30-07-15 16:32, Pali Rohár wrote:
> >On Thursday 30 July 2015 16:28:39 Pali Rohár wrote:
> >>On Thursday 30 July 2015 16:18:47 Hans de Goede wrote:
> >>>Hi,
> >>>
> >>>On 30-07-15 16:11, Pali Rohár wrote:
> >>>>On Thursday 30 July 2015 15:51:25 Hans de Goede wrote:
> >>>>>Hi Chandler,
> >>>>>
> >>>>>On 29-07-15 22:45, cpaul@redhat.com wrote:
> >>>>>>From: Stephen Chandler Paul <cpaul@redhat.com>
> >>>>>>
> >>>>>>The data concerning which buttons on the touchpad are held down or not
> >>>>>>are in the fourth packet we receive from the mouse, not the first.
> >>>>>>
> >>>>>>Signed-off-by: Stephen Chandler Paul <cpaul@redhat.com>
> >>>>>>---
> >>>>>>  drivers/input/mouse/alps.c | 6 +++---
> >>>>>>  1 file changed, 3 insertions(+), 3 deletions(-)
> >>>>>>
> >>>>>>diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> >>>>>>index 113d6f1..e2f9b25 100644
> >>>>>>--- a/drivers/input/mouse/alps.c
> >>>>>>+++ b/drivers/input/mouse/alps.c
> >>>>>>@@ -254,9 +254,9 @@ static void alps_process_packet_v1_v2(struct psmouse *psmouse)
> >>>>>>  	/* Non interleaved V2 dualpoint has separate stick button bits */
> >>>>>>  	if (priv->proto_version == ALPS_PROTO_V2 &&
> >>>>>>  	    priv->flags == (ALPS_PASS | ALPS_DUALPOINT)) {
> >>>>>>-		left |= packet[0] & 1;
> >>>>>>-		right |= packet[0] & 2;
> >>>>>>-		middle |= packet[0] & 4;
> >>>>>>+		left |= packet[3] & 1;
> >>>>>>+		right |= packet[3] & 2;
> >>>>>>+		middle |= packet[3] & 4;
> >>>>>>  	}
> >>>>>>
> >>>>>>  	alps_report_buttons(dev, dev2, left, right, middle);
> >>>>>
> >>>>>Thanks for taking a look at the recordings, but the above patch is wrong,
> >>>>>if you look slightly higher in the lps_process_packet_v1_v2() function there
> >>>>>is this:
> >>>>>
> >>>>>if (priv->proto_version == ALPS_PROTO_V1) {
> >>>>>...
> >>>>>} else {
> >>>>>	left = packet[3] & 1;
> >>>>>	right = packet[3] & 2;
> >>>>>	middle = packet[3] & 4;
> >>>>>}
> >>>>>
> >>>>>So with your patch for the devices in question the entire code flow
> >>>>>becomes:
> >>>>>
> >>>>>	left = packet[3] & 1;
> >>>>>	right = packet[3] & 2;
> >>>>>	middle = packet[3] & 4;
> >>>>>	left |= packet[3] & 1;
> >>>>>	right |= packet[3] & 2;
> >>>>>	middle |= packet[3] & 4;
> >>>>>
> >>>>>Which is not really helpful for the devices for which I added
> >>>>>commit 92bac83dd:
> >>>>>
> >>>>>https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/input/mouse/alps.c?id=92bac83dd79e60e65c475222e41a992a70434beb
> >>>>>
> >>>>>and will cause these devices to regress.
> >>>>>
> >>>>>Since Hans de Bruin's laptop is a Dell Latitude D430 and I saw
> >>>>>the same problem and tested my patch on a Dell Latitude D630,
> >>>>>it seems the use of the low bits of packet[0] to report the
> >>>>>trackpoint buttons separately when the touchpad is active is
> >>>>>a Dell specific thing, so I believe that a patch to only
> >>>>>activate this code block on Dell's is the right solution for
> >>>>>the regression Douglas is seeing.
> >>>>>
> >>>>>I'll write such a patch and post it shortly.
> >>>>>
> >>>>>Regards,
> >>>>>
> >>>>>Hans
> >>>>>
> >>>>>
> >>>>>
> >>>>
> >>>>Hans, can you check ec and e7 registers if are same or if they differs?
> >>>
> >>>As Benjamin already pointed out Douglas' touchpad matches this
> >>>line in alps.c :
> >>>
> >>>{ { 0x22, 0x02, 0x14 }, 0x00, { ALPS_PROTO_V2, 0xff, 0xff, ALPS_PASS | ALPS_DUALPOINT } },      /* Dell Latitude D600 */
> >>>
> >>>This is a full match
> >>
> >>No, this is not full match. It just check e7, not ec. This is reason why
> >>I asked for both ec and e7 registers of all affected machines.
> >>
> >
> >Douglas already sent e7 and ec regs:
> >[    6.617471] psmouse serio1: alps: E7 report: 22 02 14
> >[    6.656974] psmouse serio1: alps: EC report: 10 00 64
> >
> >Can you post ec regs from your tested Dell machine?
> 
> [    1.906031] psmouse serio1: alps: EC report: 10 00 64
> 
> Regards,
> 
> Hans

Ok, in this case DMI based check is maybe only possible solution.

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

Re: ALPS DualPoint double click bug

[Pali Rohár]

On Thursday 30 July 2015 16:17:23 Hans de Goede wrote:
> Hi,
> 
> On 28-07-15 01:38, Douglas Christman wrote:
> >On Mon, Jul 27, 2015 at 12:40 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> >>
> >>OK, please let me know soon, I would like to get to the bottom
> >>of this, and knowing the exact commit causing the problem will
> >>help a lot.
> >>
> >>Regards,
> >>
> >>hans
> >
> >I've verified that reverting 92bac83d on a clean v4.1 kernel
> >(b953c0d2) resolves the issue.
> 
> Thanks,
> 
> Can you please apply the attached patch on a clean v4.1 kernel,
> and confirm that that fixes this ?
> 
> Regards,
> 
> Hans

> From ee3d5d5a298b178ae5284b9766ca849665a37670 Mon Sep 17 00:00:00 2001
> From: Hans de Goede <hdegoede@redhat.com>
> Date: Thu, 30 Jul 2015 15:49:16 +0200
> Subject: [PATCH] alps: Only Dell laptops have separate button bits for v2
>  dualpoint sticks
> 
> It turns out that only Dell laptops have the separate button bits for
> v2 dualpoint sticks and that commit 92bac83dd79e ("Input: alps - non
> interleaved V2 dualpoint has separate stick button bits") causes
> regressions on Toshiba laptops.
> 
> This commit adds a check for Dell laptops to the code for handling these
> extra button bits, fixing this regression.
> 
> This patch has been tested on a Dell Latitude D620 to make sure that it
> does not reintroduce the original problem.
> 
> Reported-by: Douglas Christman <douglaschristman@gmail.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/input/mouse/alps.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> index 113d6f1..889aec1 100644
> --- a/drivers/input/mouse/alps.c
> +++ b/drivers/input/mouse/alps.c
> @@ -20,6 +20,7 @@
>  #include <linux/input/mt.h>
>  #include <linux/serio.h>
>  #include <linux/libps2.h>
> +#include <linux/dmi.h>
>  
>  #include "psmouse.h"
>  #include "alps.h"
> @@ -251,8 +252,9 @@ static void alps_process_packet_v1_v2(struct psmouse *psmouse)
>  		return;
>  	}
>  
> -	/* Non interleaved V2 dualpoint has separate stick button bits */
> -	if (priv->proto_version == ALPS_PROTO_V2 &&
> +	/* Dell non interleaved V2 dualpoint has separate stick button bits */
> +	if (dmi_name_in_vendors("Dell") &&
> +	    priv->proto_version == ALPS_PROTO_V2 &&
>  	    priv->flags == (ALPS_PASS | ALPS_DUALPOINT)) {
>  		left |= packet[0] & 1;
>  		right |= packet[0] & 2;

What about introducing new flag ALPS_<something> instead calling
dmi_name_in_vendors() function every time when we need to process
packet?

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

Re: ALPS DualPoint double click bug

[Hans de Goede]

[-- Attachment #1: Type: text/plain, Size: 2859 bytes --]

Hi,

On 30-07-15 16:46, Pali Rohár wrote:
> On Thursday 30 July 2015 16:17:23 Hans de Goede wrote:
>> Hi,
>>
>> On 28-07-15 01:38, Douglas Christman wrote:
>>> On Mon, Jul 27, 2015 at 12:40 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>
>>>> OK, please let me know soon, I would like to get to the bottom
>>>> of this, and knowing the exact commit causing the problem will
>>>> help a lot.
>>>>
>>>> Regards,
>>>>
>>>> hans
>>>
>>> I've verified that reverting 92bac83d on a clean v4.1 kernel
>>> (b953c0d2) resolves the issue.
>>
>> Thanks,
>>
>> Can you please apply the attached patch on a clean v4.1 kernel,
>> and confirm that that fixes this ?
>>
>> Regards,
>>
>> Hans
>
>>  From ee3d5d5a298b178ae5284b9766ca849665a37670 Mon Sep 17 00:00:00 2001
>> From: Hans de Goede <hdegoede@redhat.com>
>> Date: Thu, 30 Jul 2015 15:49:16 +0200
>> Subject: [PATCH] alps: Only Dell laptops have separate button bits for v2
>>   dualpoint sticks
>>
>> It turns out that only Dell laptops have the separate button bits for
>> v2 dualpoint sticks and that commit 92bac83dd79e ("Input: alps - non
>> interleaved V2 dualpoint has separate stick button bits") causes
>> regressions on Toshiba laptops.
>>
>> This commit adds a check for Dell laptops to the code for handling these
>> extra button bits, fixing this regression.
>>
>> This patch has been tested on a Dell Latitude D620 to make sure that it
>> does not reintroduce the original problem.
>>
>> Reported-by: Douglas Christman <douglaschristman@gmail.com>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   drivers/input/mouse/alps.c | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
>> index 113d6f1..889aec1 100644
>> --- a/drivers/input/mouse/alps.c
>> +++ b/drivers/input/mouse/alps.c
>> @@ -20,6 +20,7 @@
>>   #include <linux/input/mt.h>
>>   #include <linux/serio.h>
>>   #include <linux/libps2.h>
>> +#include <linux/dmi.h>
>>
>>   #include "psmouse.h"
>>   #include "alps.h"
>> @@ -251,8 +252,9 @@ static void alps_process_packet_v1_v2(struct psmouse *psmouse)
>>   		return;
>>   	}
>>
>> -	/* Non interleaved V2 dualpoint has separate stick button bits */
>> -	if (priv->proto_version == ALPS_PROTO_V2 &&
>> +	/* Dell non interleaved V2 dualpoint has separate stick button bits */
>> +	if (dmi_name_in_vendors("Dell") &&
>> +	    priv->proto_version == ALPS_PROTO_V2 &&
>>   	    priv->flags == (ALPS_PASS | ALPS_DUALPOINT)) {
>>   		left |= packet[0] & 1;
>>   		right |= packet[0] & 2;
>
> What about introducing new flag ALPS_<something> instead calling
> dmi_name_in_vendors() function every time when we need to process
> packet?

That is a good idea. Douglas can you test the attached version
instead of the previous one please ?

Thanks & Regards,

Hans

[-- Attachment #2: 0001-alps-Only-Dell-laptops-have-separate-button-bits-for.patch --]
[-- Type: text/x-patch, Size: 2497 bytes --]

>From 93a1852f32ab37b2cbcdb7e797f1ad59eb6bbef4 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Thu, 30 Jul 2015 15:49:16 +0200
Subject: [PATCH] alps: Only Dell laptops have separate button bits for v2
 dualpoint sticks

It turns out that only Dell laptops have the separate button bits for
v2 dualpoint sticks and that commit 92bac83dd79e ("Input: alps - non
interleaved V2 dualpoint has separate stick button bits") causes
regressions on Toshiba laptops.

This commit adds a check for Dell laptops to the code for handling these
extra button bits, fixing this regression.

This patch has been tested on a Dell Latitude D620 to make sure that it
does not reintroduce the original problem.

Reported-by: Douglas Christman <douglaschristman@gmail.com>
Cc: stable@vger.kernel.org
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/input/mouse/alps.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index 113d6f1..4d24686 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -20,6 +20,7 @@
 #include <linux/input/mt.h>
 #include <linux/serio.h>
 #include <linux/libps2.h>
+#include <linux/dmi.h>
 
 #include "psmouse.h"
 #include "alps.h"
@@ -99,6 +100,7 @@ static const struct alps_nibble_commands alps_v6_nibble_commands[] = {
 #define ALPS_FOUR_BUTTONS	0x40	/* 4 direction button present */
 #define ALPS_PS2_INTERLEAVED	0x80	/* 3-byte PS/2 packet interleaved with
 					   6-byte ALPS packet */
+#define ALPS_DELL		0x100	/* device is a Dell laptop */
 #define ALPS_BUTTONPAD		0x200	/* device is a clickpad */
 
 static const struct alps_model_info alps_model_data[] = {
@@ -251,9 +253,9 @@ static void alps_process_packet_v1_v2(struct psmouse *psmouse)
 		return;
 	}
 
-	/* Non interleaved V2 dualpoint has separate stick button bits */
+	/* Dell non interleaved V2 dualpoint has separate stick button bits */
 	if (priv->proto_version == ALPS_PROTO_V2 &&
-	    priv->flags == (ALPS_PASS | ALPS_DUALPOINT)) {
+	    priv->flags == (ALPS_DELL | ALPS_PASS | ALPS_DUALPOINT)) {
 		left |= packet[0] & 1;
 		right |= packet[0] & 2;
 		middle |= packet[0] & 4;
@@ -2550,6 +2552,8 @@ static int alps_set_protocol(struct psmouse *psmouse,
 	priv->byte0 = protocol->byte0;
 	priv->mask0 = protocol->mask0;
 	priv->flags = protocol->flags;
+	if (dmi_name_in_vendors("Dell"))
+		priv->flags |= ALPS_DELL;
 
 	priv->x_max = 2000;
 	priv->y_max = 1400;
-- 
2.4.3

Re: ALPS DualPoint double click bug

[Pali Rohár]

On Thursday 30 July 2015 17:00:56 Hans de Goede wrote:
> Hi,
> 
> On 30-07-15 16:46, Pali Rohár wrote:
> >What about introducing new flag ALPS_<something> instead calling
> >dmi_name_in_vendors() function every time when we need to process
> >packet?
> 
> That is a good idea. Douglas can you test the attached version
> instead of the previous one please ?
> 
> Thanks & Regards,
> 
> Hans

> @@ -251,9 +253,9 @@ static void alps_process_packet_v1_v2(struct psmouse *psmouse)
>  		return;
>  	}
>  
> -	/* Non interleaved V2 dualpoint has separate stick button bits */
> +	/* Dell non interleaved V2 dualpoint has separate stick button bits */
>  	if (priv->proto_version == ALPS_PROTO_V2 &&
> -	    priv->flags == (ALPS_PASS | ALPS_DUALPOINT)) {
> +	    priv->flags == (ALPS_DELL | ALPS_PASS | ALPS_DUALPOINT)) {

Hi again. Now I'm trying to understand what this condition means and you
probably wanted to write... priv->flags is field and so == comparator is
hard to decode and understood. Now it means that priv->flags must have
set ALPS_DELL, ALPS_PASS and ALPS_DUALPOINT and must not set ALPS_WHEEL,
ALPS_FW_BK_1, ALPS_FW_BK_2, ALPS_FOUR_BUTTONS, ALPS_PS2_INTERLEAVED,
ALPS_BUTTONPAD and all other future flags! With future flags this code
is fragile and can be easy broken in future (by introducing new flags).
Because of "Non interleaved" in description you probably wanted
something like this?

 if (priv->proto_version == ALPS_PROTO_V2 &&
     (priv->flags & (ALPS_DELL | ALPS_PASS | ALPS_DUALPOINT)) &&
     !(priv->flags & ALPS_PS2_INTERLEAVED))

(flags must contains ALPS_DELL, ALPS_PASS, ALPS_DUALPOINT and must not
ALPS_PS2_INTERLEAVED)

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

Re: ALPS DualPoint double click bug

[Hans de Goede]

Hi,

On 30-07-15 17:49, Pali Rohár wrote:
> On Thursday 30 July 2015 17:00:56 Hans de Goede wrote:
>> Hi,
>>
>> On 30-07-15 16:46, Pali Rohár wrote:
>>> What about introducing new flag ALPS_<something> instead calling
>>> dmi_name_in_vendors() function every time when we need to process
>>> packet?
>>
>> That is a good idea. Douglas can you test the attached version
>> instead of the previous one please ?
>>
>> Thanks & Regards,
>>
>> Hans
>
>> @@ -251,9 +253,9 @@ static void alps_process_packet_v1_v2(struct psmouse *psmouse)
>>   		return;
>>   	}
>>
>> -	/* Non interleaved V2 dualpoint has separate stick button bits */
>> +	/* Dell non interleaved V2 dualpoint has separate stick button bits */
>>   	if (priv->proto_version == ALPS_PROTO_V2 &&
>> -	    priv->flags == (ALPS_PASS | ALPS_DUALPOINT)) {
>> +	    priv->flags == (ALPS_DELL | ALPS_PASS | ALPS_DUALPOINT)) {
>
> Hi again. Now I'm trying to understand what this condition means and you
> probably wanted to write... priv->flags is field and so == comparator is
> hard to decode and understood.

The == operator was already being used before this patch, and it does the job
just fine IMHO.

Regards,

Hans


  Now it means that priv->flags must have
> set ALPS_DELL, ALPS_PASS and ALPS_DUALPOINT and must not set ALPS_WHEEL,
> ALPS_FW_BK_1, ALPS_FW_BK_2, ALPS_FOUR_BUTTONS, ALPS_PS2_INTERLEAVED,
> ALPS_BUTTONPAD and all other future flags! With future flags this code
> is fragile and can be easy broken in future (by introducing new flags).
> Because of "Non interleaved" in description you probably wanted
> something like this?
>
>   if (priv->proto_version == ALPS_PROTO_V2 &&
>       (priv->flags & (ALPS_DELL | ALPS_PASS | ALPS_DUALPOINT)) &&
>       !(priv->flags & ALPS_PS2_INTERLEAVED))
>
> (flags must contains ALPS_DELL, ALPS_PASS, ALPS_DUALPOINT and must not
> ALPS_PS2_INTERLEAVED)
>
--
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

Re: ALPS DualPoint double click bug

[Douglas Christman]

On Thu, Jul 30, 2015 at 11:00 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
> On 30-07-15 16:46, Pali Rohár wrote:
>> What about introducing new flag ALPS_<something> instead calling
>> dmi_name_in_vendors() function every time when we need to process
>> packet?
>
> That is a good idea. Douglas can you test the attached version
> instead of the previous one please ?
>
> Thanks & Regards,
>
> Hans

This patch works for me.

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

Re: ALPS DualPoint double click bug

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 744 bytes --]

On Friday 31 July 2015 23:12:56 Douglas Christman wrote:
> On Thu, Jul 30, 2015 at 11:00 AM, Hans de Goede <hdegoede@redhat.com>
> wrote:
> > Hi,
> > 
> > On 30-07-15 16:46, Pali Rohár wrote:
> >> What about introducing new flag ALPS_<something> instead calling
> >> dmi_name_in_vendors() function every time when we need to process
> >> packet?
> > 
> > That is a good idea. Douglas can you test the attached version
> > instead of the previous one please ?
> > 
> > Thanks & Regards,
> > 
> > Hans
> 
> This patch works for me.
> 
> Doug

Douglas: Thanks for testing!

Hans: Can you update also documentation in Documentation/input/alps.txt 
to match with this your change?

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: ALPS DualPoint double click bug

[Hans de Goede]

Hi,

On 31-07-15 23:17, Pali Rohár wrote:
> On Friday 31 July 2015 23:12:56 Douglas Christman wrote:
>> On Thu, Jul 30, 2015 at 11:00 AM, Hans de Goede <hdegoede@redhat.com>
>> wrote:
>>> Hi,
>>>
>>> On 30-07-15 16:46, Pali Rohár wrote:
>>>> What about introducing new flag ALPS_<something> instead calling
>>>> dmi_name_in_vendors() function every time when we need to process
>>>> packet?
>>>
>>> That is a good idea. Douglas can you test the attached version
>>> instead of the previous one please ?
>>>
>>> Thanks & Regards,
>>>
>>> Hans
>>
>> This patch works for me.
>>
>> Doug
>
> Douglas: Thanks for testing!

Ack, thanks for testing :)

> Hans: Can you update also documentation in Documentation/input/alps.txt
> to match with this your change?

Good point, I've fixed this up and submitted the patch fixing
this upstream.

Regards,

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