Thread (41 messages) 41 messages, 3 authors, 2014-11-14

Re: [PATCH 1/3] input: alps: Reset mouse before identifying it

From: Pali Rohár <hidden>
Date: 2014-10-19 11:07:48
Also in: lkml

On Wednesday 15 October 2014 20:22:56 Dmitry Torokhov wrote:
On Wed, Oct 15, 2014 at 08:10:39PM +0200, Pali Rohár wrote:
quoted
On Wednesday 15 October 2014 20:00:11 Dmitry Torokhov wrote:
quoted
On Wed, Oct 15, 2014 at 07:57:37PM +0200, Pali Rohár wrote:
quoted
On Wednesday 15 October 2014 19:43:15 Dmitry Torokhov 
wrote:
quoted
quoted
quoted
quoted
On Wed, Oct 15, 2014 at 02:53:11PM +0200, Pali Rohár 
wrote:
quoted
quoted
quoted
quoted
quoted
On Tuesday 14 October 2014 08:08:34 Dmitry Torokhov
wrote:
quoted
quoted
quoted
quoted
quoted
On Fri, Oct 03, 2014 at 11:47:59AM +0200, Hans de
Goede
wrote:
quoted
quoted
quoted
quoted
Hi,

Thanks for working on this!

On 10/03/2014 11:43 AM, Pali Rohár wrote:
quoted
On some systems after starting computer
function alps_identify() does not detect dual
ALPS touchpad+trackstick device correctly and
detect only touchpad.

Resetting ALPS device before identifiying it
fixing this problem and both parts touchpad
and trackstick are detected.

Signed-off-by: Pali Rohár
[off-list ref] Tested-by: Pali Rohár
[off-list ref]
Looks good and seems sensible:

Acked-by: Hans de Goede <redacted>
*sigh* I am not really happy about this, as we
making boot longer and longer for people without
ALPS touchpads. It would be better if we only
reset the mouse when we knew we are dealing with
ALPS, and even better if we only reset it when we
suspected that we missed trackstick. Any chance
of doing this?

Thanks.
Dmitry, problem is that function check which
detecting trackstick does not working when I start
my laptop from power-off state and do not reset
PS/2 device. But detecting ALPS touchpad looks like
working. So if do not like this idea, what about
doing something like this in alps_dectect function?

int alps_detect(...)
{
...
/* detect if device is ALPS */
if (alps_identify(...) < 0)
return -1;
/* now we know that device is ALPS */
if (!(flags & ALPS_DUALPOINT)) {
/* reset it and identify again, maybe there is
trackstick */ psmouse_reset(...);
alps_identify(...);
}
...
}

It will does not affect non ALPS devices (because
first identify call will fail), but will affect
ALPS devices without trackstick (because identify
will be called twice and reset too).
I think this is a step in right direction. Do you know
what exactly fails in alps_identify() on your box if
you do not call psmouse_reset?

Thanks.
Yes, I know. It is failing in
alps_probe_trackstick_v3(). It calls
alps_command_mode_read_reg(...) and it returns 0 which
means trackstick is not there.
OK, so can we try sticking psmouse_reset() there? This
will limit the exposure of the new delay.

Thanks.
Sorry, but I think this is not safe. Function psmouse_reset
will reset device (set it to relative mode, etc...) and
before and after alps_probe_trackstick_v3() are called
other functions. So it could break something else.
We might need to repeat bits of alps_identify() after
resetting the mouse, you are right. It should still be doable
though.
What about checking "E6 report" and if that pass reset device and 
do full alps_identify? With check for "E6 report" we can filter 
probably all PS/2 devices which are not ALPS.

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

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help