Message ID | 20190619063756.9714-1-hui.wang@canonical.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | [v5] Input: alps - Don't handle ALPS cs19 trackpoint-only device | expand |
On Wednesday 19 June 2019 14:37:56 Hui Wang wrote: > On a latest Lenovo laptop, the trackpoint and 3 buttons below it > don't work at all, when we move the trackpoint or press those 3 > buttons, the kernel will print out: > "Rejected trackstick packet from non DualPoint device" > > This device is identified as an alps touchpad but the packet has > trackpoint format, so the alps.c drops the packet and prints out > the message above. > > According to XiaoXiao's explanation, this device is named cs19 and > is trackpoint-only device, its firmware is only for trackpoint, it > is independent of touchpad and is a device completely different from > DualPoint ones. > > To drive this device with mininal changes to the existing driver, we > just let the alps driver not handle this device, then the trackpoint.c > will be the driver of this device if the trackpoint driver is enabled. > (if not, this device will fallback to a bare PS/2 device) > > With the trackpoint.c, this trackpoint and 3 buttons all work well, > they have all features that the trackpoint should have, like > scrolling-screen, drag-and-drop and frame-selection. > > Signed-off-by: XiaoXiao Liu <sliuuxiaonxiao@gmail.com> > Signed-off-by: Hui Wang <hui.wang@canonical.com> Looks good, you can add my: Reviewed-by: Pali Rohár <pali.rohar@gmail.com> Thanks! > --- > In the v5: > change the commit header to add "fallback to a bare PS/2 device if > trackpont driver is not enabled". > add comment for checking param[0] and param[1] > add psmouse_dbg and psmouse_warn respectively for PS2_TRACKPOINT is > enabled or not enabled. > > drivers/input/mouse/alps.c | 41 ++++++++++++++++++++++++++++++++++++++ > 1 file changed, 41 insertions(+) > > diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c > index 0a6f7ca883e7..536b8e531169 100644 > --- a/drivers/input/mouse/alps.c > +++ b/drivers/input/mouse/alps.c > @@ -24,6 +24,7 @@ > > #include "psmouse.h" > #include "alps.h" > +#include "trackpoint.h" > > /* > * Definitions for ALPS version 3 and 4 command mode protocol > @@ -2864,6 +2865,34 @@ static const struct alps_protocol_info *alps_match_table(unsigned char *e7, > return NULL; > } > > +static bool alps_is_cs19_trackpoint(struct psmouse *psmouse) > +{ > + u8 param[2] = { 0 }; > + > + if (ps2_command(&psmouse->ps2dev, > + param, MAKE_PS2_CMD(0, 2, TP_READ_ID))) > + return false; > + > + /* > + * param[0] contains the trackpoint device variant_id while param[1] > + * contains the firmware_id, so far for all alps trackpoint-only > + * devices, their variant_ids equal TP_VARIANT_ALPS and their > + * firmware_ids are 0x20~0x2f, so here we check param[0] as well as > + * param[1] to detect an ALPS trackpoint-only device. > + */ > + if ((param[0] == TP_VARIANT_ALPS) && (param[1] & 0x20)) { > + if (IS_ENABLED(MOUSE_PS2_TRACKPOINT)) > + psmouse_dbg(psmouse, > + "ALPS CS19 trackpoint-only device detected, not using ALPS touchpad driver\n"); > + else > + psmouse_warn(psmouse, > + "ALPS CS19 trackpoint-only device detected but MOUSE_PS2_TRACKPOINT not enabled, fallback to bare PS/2 mouse\n"); > + return true; > + } > + > + return false; > +} > + > static int alps_identify(struct psmouse *psmouse, struct alps_data *priv) > { > const struct alps_protocol_info *protocol; > @@ -3164,6 +3193,18 @@ int alps_detect(struct psmouse *psmouse, bool set_properties) > if (error) > return error; > > + /* > + * ALPS cs19 is a trackpoint-only device, it is completely independent > + * of touchpad. So it is a device different from DualPoint ones, if it > + * is identified as a cs19 trackpoint device, we return -EINVAL here and > + * let trackpoint.c to drive this device, if the trackpoint driver is > + * not enabled, the device will fallback to a bare PS/2 mouse. > + * If ps2_command() fails here, we depend on the immediate followed > + * psmouse_reset() to reset the device to normal state. > + */ > + if (alps_is_cs19_trackpoint(psmouse)) > + return -EINVAL; > + > /* > * Reset the device to make sure it is fully operational: > * on some laptops, like certain Dell Latitudes, we may
On 2019/6/19 下午3:29, Pali Rohár wrote: > On Wednesday 19 June 2019 14:37:56 Hui Wang wrote: >> On a latest Lenovo laptop, the trackpoint and 3 buttons below it >> don't work at all, when we move the trackpoint or press those 3 >> buttons, the kernel will print out: >> "Rejected trackstick packet from non DualPoint device" >> >> This device is identified as an alps touchpad but the packet has >> trackpoint format, so the alps.c drops the packet and prints out >> the message above. >> >> According to XiaoXiao's explanation, this device is named cs19 and >> is trackpoint-only device, its firmware is only for trackpoint, it >> is independent of touchpad and is a device completely different from >> DualPoint ones. >> >> To drive this device with mininal changes to the existing driver, we >> just let the alps driver not handle this device, then the trackpoint.c >> will be the driver of this device if the trackpoint driver is enabled. >> (if not, this device will fallback to a bare PS/2 device) >> >> With the trackpoint.c, this trackpoint and 3 buttons all work well, >> they have all features that the trackpoint should have, like >> scrolling-screen, drag-and-drop and frame-selection. >> >> Signed-off-by: XiaoXiao Liu <sliuuxiaonxiao@gmail.com> >> Signed-off-by: Hui Wang <hui.wang@canonical.com> > Looks good, you can add my: > > Reviewed-by: Pali Rohár <pali.rohar@gmail.com> > > Thanks! > Thank you Pali. And Dmitry, suppose this patch is ok to merge, do I need to send a new patch to add "Reviewed-by: Pali Rohár <pali.rohar@gmail.com>" or you will add it yourself. Thanks, Hui. >> --- >> In the v5: >> change the commit header to add "fallback to a bare PS/2 device if >
Hi Dmitry, When you have time, could you please take a look at this patch? Thanks, Hui. On 2019/6/19 下午4:11, Hui Wang wrote: > > On 2019/6/19 下午3:29, Pali Rohár wrote: >> On Wednesday 19 June 2019 14:37:56 Hui Wang wrote: >>> On a latest Lenovo laptop, the trackpoint and 3 buttons below it >>> don't work at all, when we move the trackpoint or press those 3 >>> buttons, the kernel will print out: >>> "Rejected trackstick packet from non DualPoint device" >>> >>> This device is identified as an alps touchpad but the packet has >>> trackpoint format, so the alps.c drops the packet and prints out >>> the message above. >>> >>> According to XiaoXiao's explanation, this device is named cs19 and >>> is trackpoint-only device, its firmware is only for trackpoint, it >>> is independent of touchpad and is a device completely different from >>> DualPoint ones. >>> >>> To drive this device with mininal changes to the existing driver, we >>> just let the alps driver not handle this device, then the trackpoint.c >>> will be the driver of this device if the trackpoint driver is enabled. >>> (if not, this device will fallback to a bare PS/2 device) >>> >>> With the trackpoint.c, this trackpoint and 3 buttons all work well, >>> they have all features that the trackpoint should have, like >>> scrolling-screen, drag-and-drop and frame-selection. >>> >>> Signed-off-by: XiaoXiao Liu <sliuuxiaonxiao@gmail.com> >>> Signed-off-by: Hui Wang <hui.wang@canonical.com> >> Looks good, you can add my: >> >> Reviewed-by: Pali Rohár <pali.rohar@gmail.com> >> >> Thanks! >> > Thank you Pali. > > And Dmitry, suppose this patch is ok to merge, do I need to send a new > patch to add "Reviewed-by: Pali Rohár <pali.rohar@gmail.com>" or you > will add it yourself. > > Thanks, > > Hui. > >>> --- >>> In the v5: >>> change the commit header to add "fallback to a bare PS/2 device if >>
diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c index 0a6f7ca883e7..536b8e531169 100644 --- a/drivers/input/mouse/alps.c +++ b/drivers/input/mouse/alps.c @@ -24,6 +24,7 @@ #include "psmouse.h" #include "alps.h" +#include "trackpoint.h" /* * Definitions for ALPS version 3 and 4 command mode protocol @@ -2864,6 +2865,34 @@ static const struct alps_protocol_info *alps_match_table(unsigned char *e7, return NULL; } +static bool alps_is_cs19_trackpoint(struct psmouse *psmouse) +{ + u8 param[2] = { 0 }; + + if (ps2_command(&psmouse->ps2dev, + param, MAKE_PS2_CMD(0, 2, TP_READ_ID))) + return false; + + /* + * param[0] contains the trackpoint device variant_id while param[1] + * contains the firmware_id, so far for all alps trackpoint-only + * devices, their variant_ids equal TP_VARIANT_ALPS and their + * firmware_ids are 0x20~0x2f, so here we check param[0] as well as + * param[1] to detect an ALPS trackpoint-only device. + */ + if ((param[0] == TP_VARIANT_ALPS) && (param[1] & 0x20)) { + if (IS_ENABLED(MOUSE_PS2_TRACKPOINT)) + psmouse_dbg(psmouse, + "ALPS CS19 trackpoint-only device detected, not using ALPS touchpad driver\n"); + else + psmouse_warn(psmouse, + "ALPS CS19 trackpoint-only device detected but MOUSE_PS2_TRACKPOINT not enabled, fallback to bare PS/2 mouse\n"); + return true; + } + + return false; +} + static int alps_identify(struct psmouse *psmouse, struct alps_data *priv) { const struct alps_protocol_info *protocol; @@ -3164,6 +3193,18 @@ int alps_detect(struct psmouse *psmouse, bool set_properties) if (error) return error; + /* + * ALPS cs19 is a trackpoint-only device, it is completely independent + * of touchpad. So it is a device different from DualPoint ones, if it + * is identified as a cs19 trackpoint device, we return -EINVAL here and + * let trackpoint.c to drive this device, if the trackpoint driver is + * not enabled, the device will fallback to a bare PS/2 mouse. + * If ps2_command() fails here, we depend on the immediate followed + * psmouse_reset() to reset the device to normal state. + */ + if (alps_is_cs19_trackpoint(psmouse)) + return -EINVAL; + /* * Reset the device to make sure it is fully operational: * on some laptops, like certain Dell Latitudes, we may