| Message ID | 1412329392-5580-2-git-send-email-pali.rohar@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, Thanks for working on this! On 10/03/2014 11:43 AM, Pali Rohár wrote: > 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 <pali.rohar@gmail.com> > Tested-by: Pali Rohár <pali.rohar@gmail.com> Looks good and seems sensible: Acked-by: Hans de Goede <hdegoede@redhat.com> Regards, Hans > --- > drivers/input/mouse/alps.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c > index 35a49bf..1bd5aa1 100644 > --- a/drivers/input/mouse/alps.c > +++ b/drivers/input/mouse/alps.c > @@ -2403,6 +2403,8 @@ int alps_detect(struct psmouse *psmouse, bool set_properties) > { > struct alps_data dummy; > > + psmouse_reset(psmouse); > + > if (alps_identify(psmouse, &dummy) < 0) > return -1; > > -- 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
On Fri, Oct 03, 2014 at 11:47:59AM +0200, Hans de Goede wrote: > Hi, > > Thanks for working on this! > > On 10/03/2014 11:43 AM, Pali Rohár wrote: > > 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 <pali.rohar@gmail.com> > > Tested-by: Pali Rohár <pali.rohar@gmail.com> > > Looks good and seems sensible: > > Acked-by: Hans de Goede <hdegoede@redhat.com> *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.
On Tuesday 14 October 2014 08:08:34 Dmitry Torokhov wrote: > On Fri, Oct 03, 2014 at 11:47:59AM +0200, Hans de Goede wrote: > > Hi, > > > > Thanks for working on this! > > > > On 10/03/2014 11:43 AM, Pali Rohár wrote: > > > 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 <pali.rohar@gmail.com> > > > Tested-by: Pali Rohár <pali.rohar@gmail.com> > > > > Looks good and seems sensible: > > > > Acked-by: Hans de Goede <hdegoede@redhat.com> > > *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).
On Wed, Oct 15, 2014 at 02:53:11PM +0200, Pali Rohár wrote: > On Tuesday 14 October 2014 08:08:34 Dmitry Torokhov wrote: > > On Fri, Oct 03, 2014 at 11:47:59AM +0200, Hans de Goede wrote: > > > Hi, > > > > > > Thanks for working on this! > > > > > > On 10/03/2014 11:43 AM, Pali Rohár wrote: > > > > 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 <pali.rohar@gmail.com> > > > > Tested-by: Pali Rohár <pali.rohar@gmail.com> > > > > > > Looks good and seems sensible: > > > > > > Acked-by: Hans de Goede <hdegoede@redhat.com> > > > > *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.
On Wednesday 15 October 2014 19:43:15 Dmitry Torokhov wrote: > On Wed, Oct 15, 2014 at 02:53:11PM +0200, Pali Rohár wrote: > > On Tuesday 14 October 2014 08:08:34 Dmitry Torokhov wrote: > > > On Fri, Oct 03, 2014 at 11:47:59AM +0200, Hans de Goede wrote: > > > > Hi, > > > > > > > > Thanks for working on this! > > > > > > > > On 10/03/2014 11:43 AM, Pali Rohár wrote: > > > > > 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 <pali.rohar@gmail.com> > > > > > Tested-by: Pali Rohár <pali.rohar@gmail.com> > > > > > > > > Looks good and seems sensible: > > > > > > > > Acked-by: Hans de Goede <hdegoede@redhat.com> > > > > > > *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.
On Wed, Oct 15, 2014 at 07:57:37PM +0200, Pali Rohár wrote: > On Wednesday 15 October 2014 19:43:15 Dmitry Torokhov wrote: > > On Wed, Oct 15, 2014 at 02:53:11PM +0200, Pali Rohár wrote: > > > On Tuesday 14 October 2014 08:08:34 Dmitry Torokhov wrote: > > > > On Fri, Oct 03, 2014 at 11:47:59AM +0200, Hans de Goede > wrote: > > > > > Hi, > > > > > > > > > > Thanks for working on this! > > > > > > > > > > On 10/03/2014 11:43 AM, Pali Rohár wrote: > > > > > > 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 <pali.rohar@gmail.com> > > > > > > Tested-by: Pali Rohár <pali.rohar@gmail.com> > > > > > > > > > > Looks good and seems sensible: > > > > > > > > > > Acked-by: Hans de Goede <hdegoede@redhat.com> > > > > > > > > *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.
On Wednesday 15 October 2014 20:00:11 Dmitry Torokhov wrote: > On Wed, Oct 15, 2014 at 07:57:37PM +0200, Pali Rohár wrote: > > On Wednesday 15 October 2014 19:43:15 Dmitry Torokhov wrote: > > > On Wed, Oct 15, 2014 at 02:53:11PM +0200, Pali Rohár wrote: > > > > On Tuesday 14 October 2014 08:08:34 Dmitry Torokhov wrote: > > > > > On Fri, Oct 03, 2014 at 11:47:59AM +0200, Hans de > > > > > Goede > > > > wrote: > > > > > > Hi, > > > > > > > > > > > > Thanks for working on this! > > > > > > > > > > > > On 10/03/2014 11:43 AM, Pali Rohár wrote: > > > > > > > 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 <pali.rohar@gmail.com> > > > > > > > Tested-by: Pali Rohár <pali.rohar@gmail.com> > > > > > > > > > > > > Looks good and seems sensible: > > > > > > > > > > > > Acked-by: Hans de Goede <hdegoede@redhat.com> > > > > > > > > > > *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. Tommy (added To header), what do you think? How could be this problem solved? When or where to call psmouse_reset() so that it will not affect non ALPS devices and also it call will be safe?
On Wed, Oct 15, 2014 at 08:10:39PM +0200, Pali Rohár wrote: > On Wednesday 15 October 2014 20:00:11 Dmitry Torokhov wrote: > > On Wed, Oct 15, 2014 at 07:57:37PM +0200, Pali Rohár wrote: > > > On Wednesday 15 October 2014 19:43:15 Dmitry Torokhov wrote: > > > > On Wed, Oct 15, 2014 at 02:53:11PM +0200, Pali Rohár wrote: > > > > > On Tuesday 14 October 2014 08:08:34 Dmitry Torokhov > wrote: > > > > > > On Fri, Oct 03, 2014 at 11:47:59AM +0200, Hans de > > > > > > Goede > > > > > > wrote: > > > > > > > Hi, > > > > > > > > > > > > > > Thanks for working on this! > > > > > > > > > > > > > > On 10/03/2014 11:43 AM, Pali Rohár wrote: > > > > > > > > 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 <pali.rohar@gmail.com> > > > > > > > > Tested-by: Pali Rohár <pali.rohar@gmail.com> > > > > > > > > > > > > > > Looks good and seems sensible: > > > > > > > > > > > > > > Acked-by: Hans de Goede <hdegoede@redhat.com> > > > > > > > > > > > > *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.
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: > > On Wednesday 15 October 2014 20:00:11 Dmitry Torokhov wrote: > > > On Wed, Oct 15, 2014 at 07:57:37PM +0200, Pali Rohár wrote: > > > > On Wednesday 15 October 2014 19:43:15 Dmitry Torokhov wrote: > > > > > On Wed, Oct 15, 2014 at 02:53:11PM +0200, Pali Rohár wrote: > > > > > > On Tuesday 14 October 2014 08:08:34 Dmitry Torokhov > > > > wrote: > > > > > > > On Fri, Oct 03, 2014 at 11:47:59AM +0200, Hans de > > > > > > > Goede > > > > > > > > wrote: > > > > > > > > Hi, > > > > > > > > > > > > > > > > Thanks for working on this! > > > > > > > > > > > > > > > > On 10/03/2014 11:43 AM, Pali Rohár wrote: > > > > > > > > > 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 > > > > > > > > > <pali.rohar@gmail.com> Tested-by: Pali Rohár > > > > > > > > > <pali.rohar@gmail.com> > > > > > > > > > > > > > > > > Looks good and seems sensible: > > > > > > > > > > > > > > > > Acked-by: Hans de Goede <hdegoede@redhat.com> > > > > > > > > > > > > > > *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.
On Sun, Oct 19, 2014 at 01:07:41PM +0200, Pali Rohár wrote: > 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: > > > On Wednesday 15 October 2014 20:00:11 Dmitry Torokhov wrote: > > > > On Wed, Oct 15, 2014 at 07:57:37PM +0200, Pali Rohár wrote: > > > > > On Wednesday 15 October 2014 19:43:15 Dmitry Torokhov > wrote: > > > > > > On Wed, Oct 15, 2014 at 02:53:11PM +0200, Pali Rohár > wrote: > > > > > > > On Tuesday 14 October 2014 08:08:34 Dmitry Torokhov > > > > > > wrote: > > > > > > > > On Fri, Oct 03, 2014 at 11:47:59AM +0200, Hans de > > > > > > > > Goede > > > > > > > > > > wrote: > > > > > > > > > Hi, > > > > > > > > > > > > > > > > > > Thanks for working on this! > > > > > > > > > > > > > > > > > > On 10/03/2014 11:43 AM, Pali Rohár wrote: > > > > > > > > > > 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 > > > > > > > > > > <pali.rohar@gmail.com> Tested-by: Pali Rohár > > > > > > > > > > <pali.rohar@gmail.com> > > > > > > > > > > > > > > > > > > Looks good and seems sensible: > > > > > > > > > > > > > > > > > > Acked-by: Hans de Goede <hdegoede@redhat.com> > > > > > > > > > > > > > > > > *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. > Why don't you pull alps_probe_trackstick_v3() from alps_identify(), rename it into __alps_identify() and then have real alps_identify be: static int alps_identfy(struct psmouse *psmouse, struct alps_data *priv) { int error; error = __alps_identify(psmouse, priv); if (error) return error; if (priv->proto_version == ALPS_PROTO_V3 && (priv->flags & ALPS_IS_RUSHMORE)) { /* * Some Dell Lattitudes may not always recognize * tracksticks without resetting the device. */ psmouse_reset(psmouse); error = __alps_identify(psmouse, priv); if (error) return error; if (alps_probe_trackstick_v3(psmouse, ALPS_REG_BASE_RUSHMORE)) priv->flags &= ~ALPS_DUALPOINT; } return 0; } This way you minimize number of devices exposed to extra reset. Thanks.
On Thursday 23 October 2014 17:44:04 Dmitry Torokhov wrote: > On Sun, Oct 19, 2014 at 01:07:41PM +0200, Pali Rohár wrote: > > 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: > > > > On Wednesday 15 October 2014 20:00:11 Dmitry Torokhov wrote: > > > > > On Wed, Oct 15, 2014 at 07:57:37PM +0200, Pali Rohár wrote: > > > > > > On Wednesday 15 October 2014 19:43:15 Dmitry > > > > > > Torokhov > > > > wrote: > > > > > > > On Wed, Oct 15, 2014 at 02:53:11PM +0200, Pali > > > > > > > Rohár > > > > wrote: > > > > > > > > On Tuesday 14 October 2014 08:08:34 Dmitry > > > > > > > > Torokhov > > > > > > > > wrote: > > > > > > > > > On Fri, Oct 03, 2014 at 11:47:59AM +0200, Hans > > > > > > > > > de Goede > > > > > > > > > > > > wrote: > > > > > > > > > > Hi, > > > > > > > > > > > > > > > > > > > > Thanks for working on this! > > > > > > > > > > > > > > > > > > > > On 10/03/2014 11:43 AM, Pali Rohár wrote: > > > > > > > > > > > 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 > > > > > > > > > > > <pali.rohar@gmail.com> Tested-by: Pali > > > > > > > > > > > Rohár <pali.rohar@gmail.com> > > > > > > > > > > > > > > > > > > > > Looks good and seems sensible: > > > > > > > > > > > > > > > > > > > > Acked-by: Hans de Goede > > > > > > > > > > <hdegoede@redhat.com> > > > > > > > > > > > > > > > > > > *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. > > Why don't you pull alps_probe_trackstick_v3() from > alps_identify(), rename it into __alps_identify() and then > have real alps_identify be: > > static int alps_identfy(struct psmouse *psmouse, struct > alps_data *priv) { > int error; > > error = __alps_identify(psmouse, priv); > if (error) > return error; > > if (priv->proto_version == ALPS_PROTO_V3 && > (priv->flags & ALPS_IS_RUSHMORE)) { > /* > * Some Dell Lattitudes may not always recognize > * tracksticks without resetting the device. > */ > psmouse_reset(psmouse); > > error = __alps_identify(psmouse, priv); > if (error) > return error; > > if (alps_probe_trackstick_v3(psmouse, > ALPS_REG_BASE_RUSHMORE)) > priv->flags &= ~ALPS_DUALPOINT; > } > > return 0; > } > > This way you minimize number of devices exposed to extra > reset. > > Thanks. Hello Dmitry, I sent new patch series v3 with another solution for fixing this problem. It just effectively move more parts of alps code and trackstick detection should work. It does not introduce new reset call (like in this version) so it is better.
diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c index 35a49bf..1bd5aa1 100644 --- a/drivers/input/mouse/alps.c +++ b/drivers/input/mouse/alps.c @@ -2403,6 +2403,8 @@ int alps_detect(struct psmouse *psmouse, bool set_properties) { struct alps_data dummy; + psmouse_reset(psmouse); + if (alps_identify(psmouse, &dummy) < 0) return -1;