| Message ID | 1421276154-8689-6-git-send-email-dmitry.torokhov@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wednesday 14 January 2015 23:55:53 Dmitry Torokhov wrote: > + /* > + * Reset the device to make sure it is fully operational: > + * on some laptops, like certain Dell Latitudes, we may > + * fail to properly detect presence of trackstick if device > + * has not been reset. > + */ > + psmouse_reset(psmouse); > + > + priv = kzalloc(sizeof(struct alps_data), GFP_KERNEL); > + if (priv) if (!priv) > + return -ENOMEM; > +
On Thu, Jan 15, 2015 at 09:21:34PM +0100, Pali Rohár wrote: > On Wednesday 14 January 2015 23:55:53 Dmitry Torokhov wrote: > > + /* > > + * Reset the device to make sure it is fully operational: > > + * on some laptops, like certain Dell Latitudes, we may > > + * fail to properly detect presence of trackstick if device > > + * has not been reset. > > + */ > > + psmouse_reset(psmouse); > > + > > + priv = kzalloc(sizeof(struct alps_data), GFP_KERNEL); > > + if (priv) > > if (!priv) Yep, thanks.
On Wednesday 14 January 2015 23:55:53 Dmitry Torokhov wrote: > On some Dell Latitudes we fail to identify presence of > trackstick unless we reset the device. The issue is quite > benign as we do perform reset in alps_init(), so the > trackstick ends up working, but mouse name reported to > userspace is not accurate. > > In order to fix the issue while avoiding the additional > lengthy reset we move the resrt to alps_detect() and keep the > discovered state to be used later in alps_init(). > > Reported-by: Pali Rohár <pali.rohar@gmail.com> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> This patch is not enough. ALPS_DUALPOINT flag can be removed also in function alps_hw_init_rushmore_v3() which is called from alps_init() but not from alps_detect(). So this patch does not have to set correct name in alps_detect() based on ALPS_DUALPOINT flag. My original patch set name in alps_init() after hw_init() which handled also this problem...
On Sat, Jan 17, 2015 at 11:26:50AM +0100, Pali Rohár wrote: > On Wednesday 14 January 2015 23:55:53 Dmitry Torokhov wrote: > > On some Dell Latitudes we fail to identify presence of > > trackstick unless we reset the device. The issue is quite > > benign as we do perform reset in alps_init(), so the > > trackstick ends up working, but mouse name reported to > > userspace is not accurate. > > > > In order to fix the issue while avoiding the additional > > lengthy reset we move the resrt to alps_detect() and keep the > > discovered state to be used later in alps_init(). > > > > Reported-by: Pali Rohár <pali.rohar@gmail.com> > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> > > This patch is not enough. ALPS_DUALPOINT flag can be removed also > in function alps_hw_init_rushmore_v3() which is called from > alps_init() but not from alps_detect(). So this patch does not > have to set correct name in alps_detect() based on ALPS_DUALPOINT > flag. My original patch set name in alps_init() after hw_init() > which handled also this problem... Hmm, I think if we are still seeing these after somewhat recent addition of full reset in detect procedure we need to fix our detection instead of tweaking capabilities after initialization phase fails. So I will just remove that bit from alps_hw_init_rushmore_v3(). FWIW I did a quick search on Google and do not seem to find dmesgs with message "trackstick E7 report failed". Thanks.
On Monday 02 February 2015 06:34:08 Dmitry Torokhov wrote: > On Sat, Jan 17, 2015 at 11:26:50AM +0100, Pali Rohár wrote: > > On Wednesday 14 January 2015 23:55:53 Dmitry Torokhov wrote: > > > On some Dell Latitudes we fail to identify presence of > > > trackstick unless we reset the device. The issue is quite > > > benign as we do perform reset in alps_init(), so the > > > trackstick ends up working, but mouse name reported to > > > userspace is not accurate. > > > > > > In order to fix the issue while avoiding the additional > > > lengthy reset we move the resrt to alps_detect() and keep > > > the discovered state to be used later in alps_init(). > > > > > > Reported-by: Pali Rohár <pali.rohar@gmail.com> > > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> > > > > This patch is not enough. ALPS_DUALPOINT flag can be removed > > also in function alps_hw_init_rushmore_v3() which is called > > from alps_init() but not from alps_detect(). So this patch > > does not have to set correct name in alps_detect() based on > > ALPS_DUALPOINT flag. My original patch set name in > > alps_init() after hw_init() which handled also this > > problem... > > Hmm, I think if we are still seeing these after somewhat > recent addition of full reset in detect procedure we need to > fix our detection instead of tweaking capabilities after > initialization phase fails. So I will just remove that bit > from alps_hw_init_rushmore_v3(). FWIW I did a quick search on > Google and do not seem to find dmesgs with message > "trackstick E7 report failed". > > Thanks. Ok, in this case kernel just register redundant input device which does not send any events to userspace.
diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c index cabb267..fd3303d 100644 --- a/drivers/input/mouse/alps.c +++ b/drivers/input/mouse/alps.c @@ -2304,6 +2304,7 @@ static int alps_identify(struct psmouse *psmouse, struct alps_data *priv) { const struct alps_protocol_info *protocol; unsigned char e6[4], e7[4], ec[4]; + int error; /* * First try "E6 report". @@ -2349,10 +2350,15 @@ static int alps_identify(struct psmouse *psmouse, struct alps_data *priv) } } - /* Save the Firmware version */ - memcpy(priv->fw_ver, ec, 3); + if (priv) { + /* Save the Firmware version */ + memcpy(priv->fw_ver, ec, 3); + error = alps_set_protocol(psmouse, priv, protocol); + if (error) + return error; + } - return alps_set_protocol(psmouse, priv, protocol); + return 0; } static int alps_reconnect(struct psmouse *psmouse) @@ -2406,22 +2412,20 @@ static void alps_set_abs_params_mt(struct alps_data *priv, int alps_init(struct psmouse *psmouse) { - struct alps_data *priv; + struct alps_data *priv = psmouse->private; struct input_dev *dev1 = psmouse->dev, *dev2; + int error; - priv = kzalloc(sizeof(struct alps_data), GFP_KERNEL); dev2 = input_allocate_device(); - if (!priv || !dev2) + if (!dev2) { + error = -ENOMEM; goto init_fail; + } priv->dev2 = dev2; - psmouse_reset(psmouse); - - if (alps_identify(psmouse, priv) < 0) - goto init_fail; - - if (priv->hw_init(psmouse)) + error = priv->hw_init(psmouse); + if (error) goto init_fail; /* @@ -2519,24 +2523,56 @@ int alps_init(struct psmouse *psmouse) init_fail: psmouse_reset(psmouse); input_free_device(dev2); - kfree(priv); + /* + * Even though we did not allocate psmouse->private we do free + * it here. + */ + kfree(psmouse->private); psmouse->private = NULL; - return -1; + return error; } int alps_detect(struct psmouse *psmouse, bool set_properties) { - struct alps_data dummy; + struct alps_data *priv; + int error; - if (alps_identify(psmouse, &dummy) < 0) - return -1; + error = alps_identify(psmouse, NULL); + if (error) + return error; + + /* + * Reset the device to make sure it is fully operational: + * on some laptops, like certain Dell Latitudes, we may + * fail to properly detect presence of trackstick if device + * has not been reset. + */ + psmouse_reset(psmouse); + + priv = kzalloc(sizeof(struct alps_data), GFP_KERNEL); + if (priv) + return -ENOMEM; + + error = alps_identify(psmouse, priv); + if (error) + return error; if (set_properties) { psmouse->vendor = "ALPS"; - psmouse->name = dummy.flags & ALPS_DUALPOINT ? + psmouse->name = priv->flags & ALPS_DUALPOINT ? "DualPoint TouchPad" : "GlidePoint"; - psmouse->model = dummy.proto_version; + psmouse->model = priv->proto_version; + } else { + /* + * Destroy alps_data structure we allocated earlier since + * this was just a "trial run". Otherwise we'll keep it + * to be used by alps_init() which has to be called if + * we succeed and set_properties is true. + */ + kfree(priv); + psmouse->private = NULL; } + return 0; }
On some Dell Latitudes we fail to identify presence of trackstick unless we reset the device. The issue is quite benign as we do perform reset in alps_init(), so the trackstick ends up working, but mouse name reported to userspace is not accurate. In order to fix the issue while avoiding the additional lengthy reset we move the resrt to alps_detect() and keep the discovered state to be used later in alps_init(). Reported-by: Pali Rohár <pali.rohar@gmail.com> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> --- drivers/input/mouse/alps.c | 74 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 55 insertions(+), 19 deletions(-)