Message ID | 20190520110149.27107-1-sliuuxiaonxiao@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | input: alps-fix the issue the special alps trackpoint do not work. | expand |
Tested-by: Hui Wang <hui.wang@canonical.com> On 2019/5/21 上午9:07, Xiaoxiao Liu wrote: > Add Pali Rohár. > > -----邮件原件----- > 发件人: XiaoXiao Liu <sliuuxiaonxiao@gmail.com> > 发送时间: Monday, May 20, 2019 7:02 PM > 收件人: dmitry.torokhov@gmail.com > 抄送: linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; hui.wang@canonical.com; 曹 曉建 Xiaojian Cao <xiaojian.cao@cn.alps.com>; zhangfp1@lenovo.com; 劉 曉曉 Xiaoxiao Liu <xiaoxiao.liu-1@cn.alps.com>; XiaoXiao Liu <sliuuxiaonxiao@gmail.com> > 主题: [PATCH] input: alps-fix the issue the special alps trackpoint do not work. > > when the alps trackpoint is detected and using the alps_v8_protocol_data procotol, the alps driver will not report the input data. > > solution: use standard mouse driver instead of alps driver when the specail trackpoint was detected. > > Signed-off-by: XiaoXiao Liu <sliuuxiaonxiao@gmail.com> > --- > drivers/input/mouse/alps.c | 23 ++++++++++++++++++++++- > 1 file changed, 22 insertions(+), 1 deletion(-) > > diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c index 0a6f7ca883e7..516ae1d0eb17 100644 > --- a/drivers/input/mouse/alps.c > +++ b/drivers/input/mouse/alps.c > @@ -24,7 +24,7 @@ > > #include "psmouse.h" > #include "alps.h" > - > +#include "trackpoint.h" > /* > * Definitions for ALPS version 3 and 4 command mode protocol > */ > @@ -2864,6 +2864,22 @@ static const struct alps_protocol_info *alps_match_table(unsigned char *e7, > return NULL; > } > > +int alps_check_is_trackpoint(struct psmouse *psmouse) { > + u8 param[2] = { 0 }; > + int error; > + > + error = ps2_command(&psmouse->ps2dev, > + param, MAKE_PS2_CMD(0, 2, TP_READ_ID)); > + if (error) > + return error; > + > + if (param[0] == TP_VARIANT_ALPS) > + return 0; > + psmouse_warn(psmouse, "It is not alps trackpoint.\n"); > + return -ENODEV; > +} > + > static int alps_identify(struct psmouse *psmouse, struct alps_data *priv) { > const struct alps_protocol_info *protocol; @@ -2912,6 +2928,11 @@ static int alps_identify(struct psmouse *psmouse, struct alps_data *priv) > protocol = &alps_v3_protocol_data; > } else if (e7[0] == 0x73 && e7[1] == 0x03 && > (e7[2] == 0x14 || e7[2] == 0x28)) { > + if (alps_check_is_trackpoint(psmouse) == 0) { > + psmouse_warn(psmouse, > + "It is alps trackpoint, use the standard mouse driver.\n"); > + return -EINVAL; > + } > protocol = &alps_v8_protocol_data; > } else if (e7[0] == 0x73 && e7[1] == 0x03 && e7[2] == 0xc8) { > protocol = &alps_v9_protocol_data; > -- > 2.20.1 >
Hello! On Tuesday 21 May 2019 10:26:47 Hui Wang wrote: > Tested-by: Hui Wang <hui.wang@canonical.com> > > On 2019/5/21 上午9:07, Xiaoxiao Liu wrote: > > Add Pali Rohár. > > > > -----邮件原件----- > > 发件人: XiaoXiao Liu <sliuuxiaonxiao@gmail.com> > > 发送时间: Monday, May 20, 2019 7:02 PM > > 收件人: dmitry.torokhov@gmail.com > > 抄送: linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; hui.wang@canonical.com; 曹 曉建 Xiaojian Cao <xiaojian.cao@cn.alps.com>; zhangfp1@lenovo.com; 劉 曉曉 Xiaoxiao Liu <xiaoxiao.liu-1@cn.alps.com>; XiaoXiao Liu <sliuuxiaonxiao@gmail.com> > > 主题: [PATCH] input: alps-fix the issue the special alps trackpoint do not work. > > > > when the alps trackpoint is detected and using the alps_v8_protocol_data procotol, > > the alps driver will not report the input data. Why it does not report input data? > > solution: use standard mouse driver instead of alps driver when the specail trackpoint was detected. This looks like an (undocumented) hack or workaround. Not a solution. > > Signed-off-by: XiaoXiao Liu <sliuuxiaonxiao@gmail.com> > > --- > > drivers/input/mouse/alps.c | 23 ++++++++++++++++++++++- > > 1 file changed, 22 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c index 0a6f7ca883e7..516ae1d0eb17 100644 > > --- a/drivers/input/mouse/alps.c > > +++ b/drivers/input/mouse/alps.c > > @@ -24,7 +24,7 @@ > > #include "psmouse.h" > > #include "alps.h" > > - > > +#include "trackpoint.h" > > /* > > * Definitions for ALPS version 3 and 4 command mode protocol > > */ > > @@ -2864,6 +2864,22 @@ static const struct alps_protocol_info *alps_match_table(unsigned char *e7, > > return NULL; > > } > > +int alps_check_is_trackpoint(struct psmouse *psmouse) { > > + u8 param[2] = { 0 }; > > + int error; > > + > > + error = ps2_command(&psmouse->ps2dev, > > + param, MAKE_PS2_CMD(0, 2, TP_READ_ID)); > > + if (error) > > + return error; > > + > > + if (param[0] == TP_VARIANT_ALPS) > > + return 0; > > + psmouse_warn(psmouse, "It is not alps trackpoint.\n"); > > + return -ENODEV; > > +} So, this function returns 0 when detected ALPS trackpoint and -ENODEV when not. > > + > > static int alps_identify(struct psmouse *psmouse, struct alps_data *priv) { > > const struct alps_protocol_info *protocol; @@ -2912,6 +2928,11 @@ static int alps_identify(struct psmouse *psmouse, struct alps_data *priv) > > protocol = &alps_v3_protocol_data; > > } else if (e7[0] == 0x73 && e7[1] == 0x03 && > > (e7[2] == 0x14 || e7[2] == 0x28)) { > > + if (alps_check_is_trackpoint(psmouse) == 0) { > > + psmouse_warn(psmouse, > > + "It is alps trackpoint, use the standard mouse driver.\n"); > > + return -EINVAL; And here I'm lost. If we have *not* detected ALPS trackpoint then if block is not called which means that ALPS driver is used. So why is for non-ALPS trackpoints used ALPS driver? This does not seem like a correct... And when we have detected ALPS trackpoint (return value 0) then standard mouse driver is used and returned -EINVAL. This seems strange too. So either this code is wrong or there are missing more details for explaining this strange logic. But still this looks like a hack not a proper fix/solution. > > + } > > protocol = &alps_v8_protocol_data; > > } else if (e7[0] == 0x73 && e7[1] == 0x03 && e7[2] == 0xc8) { > > protocol = &alps_v9_protocol_data; > > -- > > 2.20.1 > >
Hi Pali, Why it does not report input data? --> The alps devices which detected to use the ALPS_PROTO_V8 contains ALPS touchpad and ALPS track point. But the ALPS_PROTO_V8 do not support the track point device process. When the track point was detected to use ALPS_PROTO_V8 ,the v8 process_packet method alps_process_packet_ss4_v2 will reject to report the data because the v8 device is not set the ALPS_DUALPOINT flag. You can refer below code. /* Report trackstick */ if (alps_get_pkt_id_ss4_v2(packet) == SS4_PACKET_ID_STICK) { if (!(priv->flags & ALPS_DUALPOINT)) { psmouse_warn(psmouse, "Rejected trackstick packet from non DualPoint device"); return; } ... return; } why is for non-ALPS trackpoints used ALPS driver? --> the non-Alps track point is the ALPS touchpad, it is right to use the ALPS driver for ALPS touchpad. The v8 protocol condition may modified as below, it will be more easier to understand. if (e7[0] == 0x73 && e7[1] == 0x03 && (e7[2] == 0x14 || e7[2] == 0x28) && (alps_check_is_trackpoint(psmouse) != 0) ) { protocol = &alps_v8_protocol_data; } Best Regards XiaoXiao Liu sliuuxiaonxiao@gmail.com xiaoxiao.liu-1@cn.alps.com -----邮件原件----- 发件人: Pali Rohár <pali.rohar@gmail.com> 发送时间: Tuesday, May 21, 2019 5:46 PM 收件人: Hui Wang <hui.wang@canonical.com> 抄送: 劉 曉曉 Xiaoxiao Liu <xiaoxiao.liu-1@cn.alps.com>; XiaoXiao Liu <sliuuxiaonxiao@gmail.com>; dmitry.torokhov@gmail.com; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; 曹 曉建 Xiaojian Cao <xiaojian.cao@cn.alps.com>; zhangfp1@lenovo.com 主题: Re: 答复: [PATCH] input: alps-fix the issue the special alps trackpoint do not work. Hello! On Tuesday 21 May 2019 10:26:47 Hui Wang wrote: > Tested-by: Hui Wang <hui.wang@canonical.com> > > On 2019/5/21 上午9:07, Xiaoxiao Liu wrote: > > Add Pali Rohár. > > > > -----邮件原件----- > > 发件人: XiaoXiao Liu <sliuuxiaonxiao@gmail.com> > > 发送时间: Monday, May 20, 2019 7:02 PM > > 收件人: dmitry.torokhov@gmail.com > > 抄送: linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; > > hui.wang@canonical.com; 曹 曉建 Xiaojian Cao > > <xiaojian.cao@cn.alps.com>; zhangfp1@lenovo.com; 劉 曉曉 Xiaoxiao Liu > > <xiaoxiao.liu-1@cn.alps.com>; XiaoXiao Liu > > <sliuuxiaonxiao@gmail.com> > > 主题: [PATCH] input: alps-fix the issue the special alps trackpoint do not work. > > > > when the alps trackpoint is detected and using the > > alps_v8_protocol_data procotol, the alps driver will not report the input data. Why it does not report input data? > > solution: use standard mouse driver instead of alps driver when the specail trackpoint was detected. This looks like an (undocumented) hack or workaround. Not a solution. > > Signed-off-by: XiaoXiao Liu <sliuuxiaonxiao@gmail.com> > > --- > > drivers/input/mouse/alps.c | 23 ++++++++++++++++++++++- > > 1 file changed, 22 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c > > index 0a6f7ca883e7..516ae1d0eb17 100644 > > --- a/drivers/input/mouse/alps.c > > +++ b/drivers/input/mouse/alps.c > > @@ -24,7 +24,7 @@ > > #include "psmouse.h" > > #include "alps.h" > > - > > +#include "trackpoint.h" > > /* > > * Definitions for ALPS version 3 and 4 command mode protocol > > */ > > @@ -2864,6 +2864,22 @@ static const struct alps_protocol_info *alps_match_table(unsigned char *e7, > > return NULL; > > } > > +int alps_check_is_trackpoint(struct psmouse *psmouse) { > > + u8 param[2] = { 0 }; > > + int error; > > + > > + error = ps2_command(&psmouse->ps2dev, > > + param, MAKE_PS2_CMD(0, 2, TP_READ_ID)); > > + if (error) > > + return error; > > + > > + if (param[0] == TP_VARIANT_ALPS) > > + return 0; > > + psmouse_warn(psmouse, "It is not alps trackpoint.\n"); > > + return -ENODEV; > > +} So, this function returns 0 when detected ALPS trackpoint and -ENODEV when not. > > + > > static int alps_identify(struct psmouse *psmouse, struct alps_data *priv) { > > const struct alps_protocol_info *protocol; @@ -2912,6 +2928,11 @@ static int alps_identify(struct psmouse *psmouse, struct alps_data *priv) > > protocol = &alps_v3_protocol_data; > > } else if (e7[0] == 0x73 && e7[1] == 0x03 && > > (e7[2] == 0x14 || e7[2] == 0x28)) { > > + if (alps_check_is_trackpoint(psmouse) == 0) { > > + psmouse_warn(psmouse, > > + "It is alps trackpoint, use the standard mouse driver.\n"); > > + return -EINVAL; And here I'm lost. If we have *not* detected ALPS trackpoint then if block is not called which means that ALPS driver is used. So why is for non-ALPS trackpoints used ALPS driver? This does not seem like a correct... And when we have detected ALPS trackpoint (return value 0) then standard mouse driver is used and returned -EINVAL. This seems strange too. So either this code is wrong or there are missing more details for explaining this strange logic. But still this looks like a hack not a proper fix/solution. > > + } > > protocol = &alps_v8_protocol_data; > > } else if (e7[0] == 0x73 && e7[1] == 0x03 && e7[2] == 0xc8) { > > protocol = &alps_v9_protocol_data; > > -- > > 2.20.1 > > -- Pali Rohár pali.rohar@gmail.com
Hi! On Wednesday 22 May 2019 02:53:18 Xiaoxiao Liu wrote: > Hi Pali, > > Why it does not report input data? > --> The alps devices which detected to use the ALPS_PROTO_V8 contains ALPS touchpad and ALPS track point. > But the ALPS_PROTO_V8 do not support the track point device process. > When the track point was detected to use ALPS_PROTO_V8 ,the v8 process_packet method alps_process_packet_ss4_v2 will reject to report the data because the v8 device is not set the ALPS_DUALPOINT flag. > You can refer below code. Ok, and cannot you set ALPS_DUALPOINT flag based on that alps_check_is_trackpoint() result and then update alps_process_packet_ss4_v3() code to supports also V8 trackpoint packets? > /* Report trackstick */ > if (alps_get_pkt_id_ss4_v2(packet) == SS4_PACKET_ID_STICK) { > if (!(priv->flags & ALPS_DUALPOINT)) { > psmouse_warn(psmouse, > "Rejected trackstick packet from non DualPoint device"); > return; > } > ... > return; > } > > why is for non-ALPS trackpoints used ALPS driver? > --> the non-Alps track point is the ALPS touchpad, it is right to use the ALPS driver for ALPS touchpad. Ok, now I got it. > The v8 protocol condition may modified as below, it will be more easier to understand. > > if (e7[0] == 0x73 && e7[1] == 0x03 && (e7[2] == 0x14 || e7[2] == 0x28) && (alps_check_is_trackpoint(psmouse) != 0) ) { > protocol = &alps_v8_protocol_data; > } Yes, this would be more easier to understand. Anyway, more information should be in commit message. > Best Regards > XiaoXiao Liu > sliuuxiaonxiao@gmail.com > xiaoxiao.liu-1@cn.alps.com > > -----邮件原件----- > 发件人: Pali Rohár <pali.rohar@gmail.com> > 发送时间: Tuesday, May 21, 2019 5:46 PM > 收件人: Hui Wang <hui.wang@canonical.com> > 抄送: 劉 曉曉 Xiaoxiao Liu <xiaoxiao.liu-1@cn.alps.com>; XiaoXiao Liu <sliuuxiaonxiao@gmail.com>; dmitry.torokhov@gmail.com; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; 曹 曉建 Xiaojian Cao <xiaojian.cao@cn.alps.com>; zhangfp1@lenovo.com > 主题: Re: 答复: [PATCH] input: alps-fix the issue the special alps trackpoint do not work. > > Hello! > > On Tuesday 21 May 2019 10:26:47 Hui Wang wrote: > > Tested-by: Hui Wang <hui.wang@canonical.com> > > > > On 2019/5/21 上午9:07, Xiaoxiao Liu wrote: > > > Add Pali Rohár. > > > > > > -----邮件原件----- > > > 发件人: XiaoXiao Liu <sliuuxiaonxiao@gmail.com> > > > 发送时间: Monday, May 20, 2019 7:02 PM > > > 收件人: dmitry.torokhov@gmail.com > > > 抄送: linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; > > > hui.wang@canonical.com; 曹 曉建 Xiaojian Cao > > > <xiaojian.cao@cn.alps.com>; zhangfp1@lenovo.com; 劉 曉曉 Xiaoxiao Liu > > > <xiaoxiao.liu-1@cn.alps.com>; XiaoXiao Liu > > > <sliuuxiaonxiao@gmail.com> > > > 主题: [PATCH] input: alps-fix the issue the special alps trackpoint do not work. > > > > > > when the alps trackpoint is detected and using the > > > alps_v8_protocol_data procotol, the alps driver will not report the input data. > > Why it does not report input data? > > > > solution: use standard mouse driver instead of alps driver when the specail trackpoint was detected. > > This looks like an (undocumented) hack or workaround. Not a solution. > > > > Signed-off-by: XiaoXiao Liu <sliuuxiaonxiao@gmail.com> > > > --- > > > drivers/input/mouse/alps.c | 23 ++++++++++++++++++++++- > > > 1 file changed, 22 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c > > > index 0a6f7ca883e7..516ae1d0eb17 100644 > > > --- a/drivers/input/mouse/alps.c > > > +++ b/drivers/input/mouse/alps.c > > > @@ -24,7 +24,7 @@ > > > #include "psmouse.h" > > > #include "alps.h" > > > - > > > +#include "trackpoint.h" > > > /* > > > * Definitions for ALPS version 3 and 4 command mode protocol > > > */ > > > @@ -2864,6 +2864,22 @@ static const struct alps_protocol_info *alps_match_table(unsigned char *e7, > > > return NULL; > > > } > > > +int alps_check_is_trackpoint(struct psmouse *psmouse) { > > > + u8 param[2] = { 0 }; > > > + int error; > > > + > > > + error = ps2_command(&psmouse->ps2dev, > > > + param, MAKE_PS2_CMD(0, 2, TP_READ_ID)); > > > + if (error) > > > + return error; > > > + > > > + if (param[0] == TP_VARIANT_ALPS) > > > + return 0; > > > + psmouse_warn(psmouse, "It is not alps trackpoint.\n"); > > > + return -ENODEV; > > > +} > > So, this function returns 0 when detected ALPS trackpoint and -ENODEV when not. > > > > + > > > static int alps_identify(struct psmouse *psmouse, struct alps_data *priv) { > > > const struct alps_protocol_info *protocol; @@ -2912,6 +2928,11 @@ static int alps_identify(struct psmouse *psmouse, struct alps_data *priv) > > > protocol = &alps_v3_protocol_data; > > > } else if (e7[0] == 0x73 && e7[1] == 0x03 && > > > (e7[2] == 0x14 || e7[2] == 0x28)) { > > > + if (alps_check_is_trackpoint(psmouse) == 0) { > > > + psmouse_warn(psmouse, > > > + "It is alps trackpoint, use the standard mouse driver.\n"); > > > + return -EINVAL; > > And here I'm lost. If we have *not* detected ALPS trackpoint then if block is not called which means that ALPS driver is used. > > So why is for non-ALPS trackpoints used ALPS driver? This does not seem like a correct... > > And when we have detected ALPS trackpoint (return value 0) then standard mouse driver is used and returned -EINVAL. This seems strange too. > > So either this code is wrong or there are missing more details for explaining this strange logic. But still this looks like a hack not a proper fix/solution. > > > > + } > > > protocol = &alps_v8_protocol_data; > > > } else if (e7[0] == 0x73 && e7[1] == 0x03 && e7[2] == 0xc8) { > > > protocol = &alps_v9_protocol_data; > > > -- > > > 2.20.1 > > > > > -- > Pali Rohár > pali.rohar@gmail.com
Hi Pali, Ok, and cannot you set ALPS_DUALPOINT flag based on that alps_check_is_trackpoint() result and then update alps_process_packet_ss4_v3() code to supports also V8 trackpoint packets? --> Yes, we can do like so, when we use the v8 method to process the trackpoint , the mouse speed is not ideal. Then we choose the standard mouse driver. XiaoXiao Liu sliuuxiaonxiao@gmail.com xiaoxiao.liu-1@cn.alps.com > > -----邮件原件----- 发件人: Pali Rohár <pali.rohar@gmail.com> 发送时间: Wednesday, May 22, 2019 2:36 PM 收件人: 劉 曉曉 Xiaoxiao Liu <xiaoxiao.liu-1@cn.alps.com> 抄送: Hui Wang <hui.wang@canonical.com>; XiaoXiao Liu <sliuuxiaonxiao@gmail.com>; dmitry.torokhov@gmail.com; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; 曹 曉建 Xiaojian Cao <xiaojian.cao@cn.alps.com>; zhangfp1@lenovo.com 主题: Re: 答复: 答复: [PATCH] input: alps-fix the issue the special alps trackpoint do not work. Hi! On Wednesday 22 May 2019 02:53:18 Xiaoxiao Liu wrote: > Hi Pali, > > Why it does not report input data? > --> The alps devices which detected to use the ALPS_PROTO_V8 contains ALPS touchpad and ALPS track point. > But the ALPS_PROTO_V8 do not support the track point device process. > When the track point was detected to use ALPS_PROTO_V8 ,the v8 process_packet method alps_process_packet_ss4_v2 will reject to report the data because the v8 device is not set the ALPS_DUALPOINT flag. > You can refer below code. Ok, and cannot you set ALPS_DUALPOINT flag based on that alps_check_is_trackpoint() result and then update alps_process_packet_ss4_v3() code to supports also V8 trackpoint packets? > /* Report trackstick */ > if (alps_get_pkt_id_ss4_v2(packet) == SS4_PACKET_ID_STICK) { > if (!(priv->flags & ALPS_DUALPOINT)) { > psmouse_warn(psmouse, > "Rejected trackstick packet from non DualPoint device"); > return; > } > ... > return; > } > > why is for non-ALPS trackpoints used ALPS driver? > --> the non-Alps track point is the ALPS touchpad, it is right to use the ALPS driver for ALPS touchpad. Ok, now I got it. > The v8 protocol condition may modified as below, it will be more easier to understand. > > if (e7[0] == 0x73 && e7[1] == 0x03 && (e7[2] == 0x14 || e7[2] == 0x28) && (alps_check_is_trackpoint(psmouse) != 0) ) { > protocol = &alps_v8_protocol_data; > } Yes, this would be more easier to understand. Anyway, more information should be in commit message. > Best Regards > XiaoXiao Liu > sliuuxiaonxiao@gmail.com > xiaoxiao.liu-1@cn.alps.com > > -----邮件原件----- > 发件人: Pali Rohár <pali.rohar@gmail.com> > 发送时间: Tuesday, May 21, 2019 5:46 PM > 收件人: Hui Wang <hui.wang@canonical.com> > 抄送: 劉 曉曉 Xiaoxiao Liu <xiaoxiao.liu-1@cn.alps.com>; XiaoXiao Liu > <sliuuxiaonxiao@gmail.com>; dmitry.torokhov@gmail.com; > linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; 曹 曉建 > Xiaojian Cao <xiaojian.cao@cn.alps.com>; zhangfp1@lenovo.com > 主题: Re: 答复: [PATCH] input: alps-fix the issue the special alps trackpoint do not work. > > Hello! > > On Tuesday 21 May 2019 10:26:47 Hui Wang wrote: > > Tested-by: Hui Wang <hui.wang@canonical.com> > > > > On 2019/5/21 上午9:07, Xiaoxiao Liu wrote: > > > Add Pali Rohár. > > > > > > -----邮件原件----- > > > 发件人: XiaoXiao Liu <sliuuxiaonxiao@gmail.com> > > > 发送时间: Monday, May 20, 2019 7:02 PM > > > 收件人: dmitry.torokhov@gmail.com > > > 抄送: linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; > > > hui.wang@canonical.com; 曹 曉建 Xiaojian Cao > > > <xiaojian.cao@cn.alps.com>; zhangfp1@lenovo.com; 劉 曉曉 Xiaoxiao Liu > > > <xiaoxiao.liu-1@cn.alps.com>; XiaoXiao Liu > > > <sliuuxiaonxiao@gmail.com> > > > 主题: [PATCH] input: alps-fix the issue the special alps trackpoint do not work. > > > > > > when the alps trackpoint is detected and using the > > > alps_v8_protocol_data procotol, the alps driver will not report the input data. > > Why it does not report input data? > > > > solution: use standard mouse driver instead of alps driver when the specail trackpoint was detected. > > This looks like an (undocumented) hack or workaround. Not a solution. > > > > Signed-off-by: XiaoXiao Liu <sliuuxiaonxiao@gmail.com> > > > --- > > > drivers/input/mouse/alps.c | 23 ++++++++++++++++++++++- > > > 1 file changed, 22 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/input/mouse/alps.c > > > b/drivers/input/mouse/alps.c index 0a6f7ca883e7..516ae1d0eb17 > > > 100644 > > > --- a/drivers/input/mouse/alps.c > > > +++ b/drivers/input/mouse/alps.c > > > @@ -24,7 +24,7 @@ > > > #include "psmouse.h" > > > #include "alps.h" > > > - > > > +#include "trackpoint.h" > > > /* > > > * Definitions for ALPS version 3 and 4 command mode protocol > > > */ > > > @@ -2864,6 +2864,22 @@ static const struct alps_protocol_info *alps_match_table(unsigned char *e7, > > > return NULL; > > > } > > > +int alps_check_is_trackpoint(struct psmouse *psmouse) { > > > + u8 param[2] = { 0 }; > > > + int error; > > > + > > > + error = ps2_command(&psmouse->ps2dev, > > > + param, MAKE_PS2_CMD(0, 2, TP_READ_ID)); > > > + if (error) > > > + return error; > > > + > > > + if (param[0] == TP_VARIANT_ALPS) > > > + return 0; > > > + psmouse_warn(psmouse, "It is not alps trackpoint.\n"); > > > + return -ENODEV; > > > +} > > So, this function returns 0 when detected ALPS trackpoint and -ENODEV when not. > > > > + > > > static int alps_identify(struct psmouse *psmouse, struct alps_data *priv) { > > > const struct alps_protocol_info *protocol; @@ -2912,6 +2928,11 @@ static int alps_identify(struct psmouse *psmouse, struct alps_data *priv) > > > protocol = &alps_v3_protocol_data; > > > } else if (e7[0] == 0x73 && e7[1] == 0x03 && > > > (e7[2] == 0x14 || e7[2] == 0x28)) { > > > + if (alps_check_is_trackpoint(psmouse) == 0) { > > > + psmouse_warn(psmouse, > > > + "It is alps trackpoint, use the standard mouse driver.\n"); > > > + return -EINVAL; > > And here I'm lost. If we have *not* detected ALPS trackpoint then if block is not called which means that ALPS driver is used. > > So why is for non-ALPS trackpoints used ALPS driver? This does not seem like a correct... > > And when we have detected ALPS trackpoint (return value 0) then standard mouse driver is used and returned -EINVAL. This seems strange too. > > So either this code is wrong or there are missing more details for explaining this strange logic. But still this looks like a hack not a proper fix/solution. > > > > + } > > > protocol = &alps_v8_protocol_data; > > > } else if (e7[0] == 0x73 && e7[1] == 0x03 && e7[2] == 0xc8) { > > > protocol = &alps_v9_protocol_data; > > > -- > > > 2.20.1 > > > > > -- > Pali Rohár > pali.rohar@gmail.com -- Pali Rohár pali.rohar@gmail.com
On Wednesday 22 May 2019 07:30:43 Xiaoxiao Liu wrote: > Hi Pali, > > Ok, and cannot you set ALPS_DUALPOINT flag based on that > alps_check_is_trackpoint() result and then update > alps_process_packet_ss4_v3() code to supports also > V8 trackpoint packets? > --> Yes, we can do like so, when we use the v8 method to process the trackpoint , the mouse speed is not ideal. > Then we choose the standard mouse driver. Mouse speed is something which is configurable. Have you configured it somehow? Also there is libinput project should handle these settings more properly. Adding Peter Hutterer, maintainer of libinput to loop. I think he could help with this problem. I do not think it is a good idea to force back to generic PS/2 mouse driver for touchpads and trackpoints. Native drivers for touchpads and trackpoints supports multitouch, absolute reporting and lot of other things... Also calculation of mouse speed from absolute positions on touchpads can be more easily fixed as from emulated relative movements. Dmitry, what is your opinion about this problem? What should psmouse.ko do in this situation? Disallow usage of absolute mode and force bare PS/2 relative mode?
On Wed, May 22, 2019 at 09:40:30AM +0200, Pali Rohár wrote: > On Wednesday 22 May 2019 07:30:43 Xiaoxiao Liu wrote: > > Hi Pali, > > > > Ok, and cannot you set ALPS_DUALPOINT flag based on that > > alps_check_is_trackpoint() result and then update > > alps_process_packet_ss4_v3() code to supports also > > V8 trackpoint packets? > > --> Yes, we can do like so, when we use the v8 method to process the trackpoint , the mouse speed is not ideal. > > Then we choose the standard mouse driver. > > Mouse speed is something which is configurable. Have you configured it > somehow? Also there is libinput project should handle these settings > more properly. > > Adding Peter Hutterer, maintainer of libinput to loop. I think he could > help with this problem. libinput has a quirk for a magic multiplier on trackpoints. it was the only solution I found that came close to "working" given that every device seems to provide some other random magic data. Doc for it is here: https://wayland.freedesktop.org/libinput/doc/latest/trackpoint-configuration.html There are also different speeds depending on which xorg driver you'd use (or libinput/Wayland), so a "mouse speed is not ideal" is almost a guarantee, given a large enough variety of setups :) That's why we have the speed toggle, but I'm happy to hear any suggestions on how to make the trackpoint more useful (in libinput anyway). > I do not think it is a good idea to force back to generic PS/2 mouse > driver for touchpads and trackpoints. Native drivers for touchpads and > trackpoints supports multitouch, absolute reporting and lot of other > things... Also calculation of mouse speed from absolute positions on > touchpads can be more easily fixed as from emulated relative movements. Yeah, agree. Using PS/2 mouse drivers means you lose *all* the extra features touchpads have like palm detection, tapping, scrolling, gestures, etc. Cheers, Peter > > Dmitry, what is your opinion about this problem? What should psmouse.ko > do in this situation? Disallow usage of absolute mode and force bare > PS/2 relative mode?
On 2019/5/23 下午2:01, Peter Hutterer wrote: > On Wed, May 22, 2019 at 09:40:30AM +0200, Pali Rohár wrote: >> On Wednesday 22 May 2019 07:30:43 Xiaoxiao Liu wrote: >>> Hi Pali, >>> >>> Ok, and cannot you set ALPS_DUALPOINT flag based on that >>> alps_check_is_trackpoint() result and then update >>> alps_process_packet_ss4_v3() code to supports also >>> V8 trackpoint packets? >>> --> Yes, we can do like so, when we use the v8 method to process the trackpoint , the mouse speed is not ideal. >>> Then we choose the standard mouse driver. >> Mouse speed is something which is configurable. Have you configured it >> somehow? Also there is libinput project should handle these settings >> more properly. >> >> Adding Peter Hutterer, maintainer of libinput to loop. I think he could >> help with this problem. > libinput has a quirk for a magic multiplier on trackpoints. it was the only > solution I found that came close to "working" given that every device seems > to provide some other random magic data. Doc for it is here: > https://wayland.freedesktop.org/libinput/doc/latest/trackpoint-configuration.html Hello Peter Hutterer, To adjust the trackpoint speed from userspace: If the libinput version is lower than 1.9.0, we could set POINTINGSTICK_CONST_ACCEL=0.25 If the libinput version is higher than 1.12.0, we could set AttrTrackpointMultiplier=0.25 But if we use libinput-1.10.0, how could we adjust the speed? Many thanks in advance. Hui. > > There are also different speeds depending on which xorg driver you'd use (or > libinput/Wayland), so a "mouse speed is not ideal" is almost a guarantee, > given a large enough variety of setups :) That's why we have the speed > toggle, but I'm happy to hear any suggestions on how to make the trackpoint > more useful (in libinput anyway). > >> I do not think it is a good idea to force back to generic PS/2 mouse >> driver for touchpads and trackpoints. Native drivers for touchpads and >> trackpoints supports multitouch, absolute reporting and lot of other >> things... Also calculation of mouse speed from absolute positions on >> touchpads can be more easily fixed as from emulated relative movements. > Yeah, agree. Using PS/2 mouse drivers means you lose *all* the extra > features touchpads have like palm detection, tapping, scrolling, gestures, > etc. > > Cheers, > Peter > >> Dmitry, what is your opinion about this problem? What should psmouse.ko >> do in this situation? Disallow usage of absolute mode and force bare >> PS/2 relative mode? >
On Fri, May 24, 2019 at 01:25:52PM +0800, Hui Wang wrote: > > On 2019/5/23 下午2:01, Peter Hutterer wrote: > > On Wed, May 22, 2019 at 09:40:30AM +0200, Pali Rohár wrote: > > > On Wednesday 22 May 2019 07:30:43 Xiaoxiao Liu wrote: > > > > Hi Pali, > > > > > > > > Ok, and cannot you set ALPS_DUALPOINT flag based on that > > > > alps_check_is_trackpoint() result and then update > > > > alps_process_packet_ss4_v3() code to supports also > > > > V8 trackpoint packets? > > > > --> Yes, we can do like so, when we use the v8 method to process the trackpoint , the mouse speed is not ideal. > > > > Then we choose the standard mouse driver. > > > Mouse speed is something which is configurable. Have you configured it > > > somehow? Also there is libinput project should handle these settings > > > more properly. > > > > > > Adding Peter Hutterer, maintainer of libinput to loop. I think he could > > > help with this problem. > > libinput has a quirk for a magic multiplier on trackpoints. it was the only > > solution I found that came close to "working" given that every device seems > > to provide some other random magic data. Doc for it is here: > > https://wayland.freedesktop.org/libinput/doc/latest/trackpoint-configuration.html > > Hello Peter Hutterer, > > To adjust the trackpoint speed from userspace: > > If the libinput version is lower than 1.9.0, we could set > POINTINGSTICK_CONST_ACCEL=0.25 > > If the libinput version is higher than 1.12.0, we could set > AttrTrackpointMultiplier=0.25 > > But if we use libinput-1.10.0, how could we adjust the speed? The LIBINPUT_ATTR_TRACKPOINT_RANGE property, which didn't end up working well (hence why it got replaced again). See the docs here though: https://wayland.freedesktop.org/libinput/doc/1.10.0/trackpoints.html Cheers, Peter > > > > There are also different speeds depending on which xorg driver you'd use (or > > libinput/Wayland), so a "mouse speed is not ideal" is almost a guarantee, > > given a large enough variety of setups :) That's why we have the speed > > toggle, but I'm happy to hear any suggestions on how to make the trackpoint > > more useful (in libinput anyway). > > > > > I do not think it is a good idea to force back to generic PS/2 mouse > > > driver for touchpads and trackpoints. Native drivers for touchpads and > > > trackpoints supports multitouch, absolute reporting and lot of other > > > things... Also calculation of mouse speed from absolute positions on > > > touchpads can be more easily fixed as from emulated relative movements. > > Yeah, agree. Using PS/2 mouse drivers means you lose *all* the extra > > features touchpads have like palm detection, tapping, scrolling, gestures, > > etc. > > > > Cheers, > > Peter > > > > > Dmitry, what is your opinion about this problem? What should psmouse.ko > > > do in this situation? Disallow usage of absolute mode and force bare > > > PS/2 relative mode? > >
On 2019/5/24 下午1:36, Peter Hutterer wrote: > On Fri, May 24, 2019 at 01:25:52PM +0800, Hui Wang wrote: >> On 2019/5/23 下午2:01, Peter Hutterer wrote: >>> On Wed, May 22, 2019 at 09:40:30AM +0200, Pali Rohár wrote: >>>> On Wednesday 22 May 2019 07:30:43 Xiaoxiao Liu wrote: >>>>> Hi Pali, >>>>> >>>>> Ok, and cannot you set ALPS_DUALPOINT flag based on that >>>>> alps_check_is_trackpoint() result and then update >>>>> alps_process_packet_ss4_v3() code to supports also >>>>> V8 trackpoint packets? >>>>> --> Yes, we can do like so, when we use the v8 method to process the trackpoint , the mouse speed is not ideal. >>>>> Then we choose the standard mouse driver. >>>> Mouse speed is something which is configurable. Have you configured it >>>> somehow? Also there is libinput project should handle these settings >>>> more properly. >>>> >>>> Adding Peter Hutterer, maintainer of libinput to loop. I think he could >>>> help with this problem. >>> libinput has a quirk for a magic multiplier on trackpoints. it was the only >>> solution I found that came close to "working" given that every device seems >>> to provide some other random magic data. Doc for it is here: >>> https://wayland.freedesktop.org/libinput/doc/latest/trackpoint-configuration.html >> Hello Peter Hutterer, >> >> To adjust the trackpoint speed from userspace: >> >> If the libinput version is lower than 1.9.0, we could set >> POINTINGSTICK_CONST_ACCEL=0.25 >> >> If the libinput version is higher than 1.12.0, we could set >> AttrTrackpointMultiplier=0.25 >> >> But if we use libinput-1.10.0, how could we adjust the speed? > The LIBINPUT_ATTR_TRACKPOINT_RANGE property, which didn't end up working > well (hence why it got replaced again). See the docs here though: > https://wayland.freedesktop.org/libinput/doc/1.10.0/trackpoints.html > > Cheers, > Peter OK, got it, Thanks. >>> There are also different speeds depending on which xorg driver you'd use (or >>> libinput/Wayland), so a "mouse speed is not ideal" is almost a guarantee, >>> given a large enough variety of setups :) That's why we have the speed >>> toggle, but I'm happy to hear any suggestions on how to make the trackpoint >>> more useful (in libinput anyway). >>> >>>> I do not think it is a good idea to force back to generic PS/2 mouse >>>> driver for touchpads and trackpoints. Native drivers for touchpads and >>>> trackpoints supports multitouch, absolute reporting and lot of other >>>> things... Also calculation of mouse speed from absolute positions on >>>> touchpads can be more easily fixed as from emulated relative movements. >>> Yeah, agree. Using PS/2 mouse drivers means you lose *all* the extra >>> features touchpads have like palm detection, tapping, scrolling, gestures, >>> etc. >>> >>> Cheers, >>> Peter >>> >>>> Dmitry, what is your opinion about this problem? What should psmouse.ko >>>> do in this situation? Disallow usage of absolute mode and force bare >>>> PS/2 relative mode?
On Friday 24 May 2019 13:50:53 Hui Wang wrote: > On 2019/5/24 下午1:36, Peter Hutterer wrote: > > On Fri, May 24, 2019 at 01:25:52PM +0800, Hui Wang wrote: > > > On 2019/5/23 下午2:01, Peter Hutterer wrote: > > > > On Wed, May 22, 2019 at 09:40:30AM +0200, Pali Rohár wrote: > > > > > On Wednesday 22 May 2019 07:30:43 Xiaoxiao Liu wrote: > > > > > > Hi Pali, > > > > > > > > > > > > Ok, and cannot you set ALPS_DUALPOINT flag based on that > > > > > > alps_check_is_trackpoint() result and then update > > > > > > alps_process_packet_ss4_v3() code to supports also > > > > > > V8 trackpoint packets? > > > > > > --> Yes, we can do like so, when we use the v8 method to process the trackpoint , the mouse speed is not ideal. > > > > > > Then we choose the standard mouse driver. > > > > > Mouse speed is something which is configurable. Have you configured it > > > > > somehow? Also there is libinput project should handle these settings > > > > > more properly. > > > > > > > > > > Adding Peter Hutterer, maintainer of libinput to loop. I think he could > > > > > help with this problem. > > > > libinput has a quirk for a magic multiplier on trackpoints. it was the only > > > > solution I found that came close to "working" given that every device seems > > > > to provide some other random magic data. Doc for it is here: > > > > https://wayland.freedesktop.org/libinput/doc/latest/trackpoint-configuration.html > > > Hello Peter Hutterer, > > > > > > To adjust the trackpoint speed from userspace: > > > > > > If the libinput version is lower than 1.9.0, we could set > > > POINTINGSTICK_CONST_ACCEL=0.25 > > > > > > If the libinput version is higher than 1.12.0, we could set > > > AttrTrackpointMultiplier=0.25 > > > > > > But if we use libinput-1.10.0, how could we adjust the speed? > > The LIBINPUT_ATTR_TRACKPOINT_RANGE property, which didn't end up working > > well (hence why it got replaced again). See the docs here though: > > https://wayland.freedesktop.org/libinput/doc/1.10.0/trackpoints.html > > > > Cheers, > > Peter > > OK, got it, Thanks. Is not here some database where for input device name / id is specified that property? So users do not have to invent what is correct value for their hardware?
On 2019/5/24 下午3:26, Pali Rohár wrote: > On Friday 24 May 2019 13:50:53 Hui Wang wrote: >> On 2019/5/24 下午1:36, Peter Hutterer wrote: >>> On Fri, May 24, 2019 at 01:25:52PM +0800, Hui Wang wrote: >>>> On 2019/5/23 下午2:01, Peter Hutterer wrote: >>>>> On Wed, May 22, 2019 at 09:40:30AM +0200, Pali Rohár wrote: >>>>>> On Wednesday 22 May 2019 07:30:43 Xiaoxiao Liu wrote: >>>>>>> Hi Pali, >>>>>>> >>>>>>> Ok, and cannot you set ALPS_DUALPOINT flag based on that >>>>>>> alps_check_is_trackpoint() result and then update >>>>>>> alps_process_packet_ss4_v3() code to supports also >>>>>>> V8 trackpoint packets? >>>>>>> --> Yes, we can do like so, when we use the v8 method to process the trackpoint , the mouse speed is not ideal. >>>>>>> Then we choose the standard mouse driver. >>>>>> Mouse speed is something which is configurable. Have you configured it >>>>>> somehow? Also there is libinput project should handle these settings >>>>>> more properly. >>>>>> >>>>>> Adding Peter Hutterer, maintainer of libinput to loop. I think he could >>>>>> help with this problem. >>>>> libinput has a quirk for a magic multiplier on trackpoints. it was the only >>>>> solution I found that came close to "working" given that every device seems >>>>> to provide some other random magic data. Doc for it is here: >>>>> https://wayland.freedesktop.org/libinput/doc/latest/trackpoint-configuration.html >>>> Hello Peter Hutterer, >>>> >>>> To adjust the trackpoint speed from userspace: >>>> >>>> If the libinput version is lower than 1.9.0, we could set >>>> POINTINGSTICK_CONST_ACCEL=0.25 >>>> >>>> If the libinput version is higher than 1.12.0, we could set >>>> AttrTrackpointMultiplier=0.25 >>>> >>>> But if we use libinput-1.10.0, how could we adjust the speed? >>> The LIBINPUT_ATTR_TRACKPOINT_RANGE property, which didn't end up working >>> well (hence why it got replaced again). See the docs here though: >>> https://wayland.freedesktop.org/libinput/doc/1.10.0/trackpoints.html >>> >>> Cheers, >>> Peter >> OK, got it, Thanks. > Is not here some database where for input device name / id is specified > that property? So users do not have to invent what is correct value for > their hardware? Since the libinput version in the ubuntu 18.04 is 1.10, I tried to set LIBINPUT_ATTR_TRACKPOINT_RANGE with different values (from 25, 20, 10, 5) in the udev hwdb database, I checked it with "udevadm info /dev/input/eventX" to confirm the value is set to LIBINPUT_ATTR_TRACKPOINT_RANGE successfully, but looks like the cursor speed doesn't change at all. So for ubuntu 18.04, looks like we have to adjust the speed in the kernel driver. Thanks, Hui.
On Fri, May 24, 2019 at 09:26:48AM +0200, Pali Rohár wrote: > On Friday 24 May 2019 13:50:53 Hui Wang wrote: > > On 2019/5/24 下午1:36, Peter Hutterer wrote: > > > On Fri, May 24, 2019 at 01:25:52PM +0800, Hui Wang wrote: > > > > On 2019/5/23 下午2:01, Peter Hutterer wrote: > > > > > On Wed, May 22, 2019 at 09:40:30AM +0200, Pali Rohár wrote: > > > > > > On Wednesday 22 May 2019 07:30:43 Xiaoxiao Liu wrote: > > > > > > > Hi Pali, > > > > > > > > > > > > > > Ok, and cannot you set ALPS_DUALPOINT flag based on that > > > > > > > alps_check_is_trackpoint() result and then update > > > > > > > alps_process_packet_ss4_v3() code to supports also > > > > > > > V8 trackpoint packets? > > > > > > > --> Yes, we can do like so, when we use the v8 method to process the trackpoint , the mouse speed is not ideal. > > > > > > > Then we choose the standard mouse driver. > > > > > > Mouse speed is something which is configurable. Have you configured it > > > > > > somehow? Also there is libinput project should handle these settings > > > > > > more properly. > > > > > > > > > > > > Adding Peter Hutterer, maintainer of libinput to loop. I think he could > > > > > > help with this problem. > > > > > libinput has a quirk for a magic multiplier on trackpoints. it was the only > > > > > solution I found that came close to "working" given that every device seems > > > > > to provide some other random magic data. Doc for it is here: > > > > > https://wayland.freedesktop.org/libinput/doc/latest/trackpoint-configuration.html > > > > Hello Peter Hutterer, > > > > > > > > To adjust the trackpoint speed from userspace: > > > > > > > > If the libinput version is lower than 1.9.0, we could set > > > > POINTINGSTICK_CONST_ACCEL=0.25 > > > > > > > > If the libinput version is higher than 1.12.0, we could set > > > > AttrTrackpointMultiplier=0.25 > > > > > > > > But if we use libinput-1.10.0, how could we adjust the speed? > > > The LIBINPUT_ATTR_TRACKPOINT_RANGE property, which didn't end up working > > > well (hence why it got replaced again). See the docs here though: > > > https://wayland.freedesktop.org/libinput/doc/1.10.0/trackpoints.html > > > > > > Cheers, > > > Peter > > > > OK, got it, Thanks. > > Is not here some database where for input device name / id is specified > that property? So users do not have to invent what is correct value for > their hardware? yeah, libinput ships with a quirks database that sets those, but it relies on users to submit the quirks. https://gitlab.freedesktop.org/libinput/libinput/tree/master/quirks There's no "correct" value anyway, if you ask 3 users what the trackpoint should do you'll get 5 answers :) Cheers, Peter
On Fri, May 24, 2019 at 03:37:57PM +0800, Hui Wang wrote: > > On 2019/5/24 下午3:26, Pali Rohár wrote: > > On Friday 24 May 2019 13:50:53 Hui Wang wrote: > > > On 2019/5/24 下午1:36, Peter Hutterer wrote: > > > > On Fri, May 24, 2019 at 01:25:52PM +0800, Hui Wang wrote: > > > > > On 2019/5/23 下午2:01, Peter Hutterer wrote: > > > > > > On Wed, May 22, 2019 at 09:40:30AM +0200, Pali Rohár wrote: > > > > > > > On Wednesday 22 May 2019 07:30:43 Xiaoxiao Liu wrote: > > > > > > > > Hi Pali, > > > > > > > > > > > > > > > > Ok, and cannot you set ALPS_DUALPOINT flag based on that > > > > > > > > alps_check_is_trackpoint() result and then update > > > > > > > > alps_process_packet_ss4_v3() code to supports also > > > > > > > > V8 trackpoint packets? > > > > > > > > --> Yes, we can do like so, when we use the v8 method to process the trackpoint , the mouse speed is not ideal. > > > > > > > > Then we choose the standard mouse driver. > > > > > > > Mouse speed is something which is configurable. Have you configured it > > > > > > > somehow? Also there is libinput project should handle these settings > > > > > > > more properly. > > > > > > > > > > > > > > Adding Peter Hutterer, maintainer of libinput to loop. I think he could > > > > > > > help with this problem. > > > > > > libinput has a quirk for a magic multiplier on trackpoints. it was the only > > > > > > solution I found that came close to "working" given that every device seems > > > > > > to provide some other random magic data. Doc for it is here: > > > > > > https://wayland.freedesktop.org/libinput/doc/latest/trackpoint-configuration.html > > > > > Hello Peter Hutterer, > > > > > > > > > > To adjust the trackpoint speed from userspace: > > > > > > > > > > If the libinput version is lower than 1.9.0, we could set > > > > > POINTINGSTICK_CONST_ACCEL=0.25 > > > > > > > > > > If the libinput version is higher than 1.12.0, we could set > > > > > AttrTrackpointMultiplier=0.25 > > > > > > > > > > But if we use libinput-1.10.0, how could we adjust the speed? > > > > The LIBINPUT_ATTR_TRACKPOINT_RANGE property, which didn't end up working > > > > well (hence why it got replaced again). See the docs here though: > > > > https://wayland.freedesktop.org/libinput/doc/1.10.0/trackpoints.html > > > > > > > > Cheers, > > > > Peter > > > OK, got it, Thanks. > > Is not here some database where for input device name / id is specified > > that property? So users do not have to invent what is correct value for > > their hardware? > > Since the libinput version in the ubuntu 18.04 is 1.10, I tried to set > LIBINPUT_ATTR_TRACKPOINT_RANGE with different values (from 25, 20, 10, 5) in > the udev hwdb database, I checked it with "udevadm info /dev/input/eventX" > to confirm the value is set to LIBINPUT_ATTR_TRACKPOINT_RANGE successfully, > but looks like the cursor speed doesn't change at all. So for ubuntu 18.04, > looks like we have to adjust the speed in the kernel driver. I don't think it's a good idea to make the kernel driver behaviour based on an EOL libinput version just because one version of ubuntu ships with that. 18.10 and 19.04 both ship with libinput 1.12. It'd be better to make sure it works well with a *current* version and then patch libinput on 18.04 where needed. Cheers, Peter
On Friday 24 May 2019 19:32:38 Peter Hutterer wrote: > On Fri, May 24, 2019 at 09:26:48AM +0200, Pali Rohár wrote: > > On Friday 24 May 2019 13:50:53 Hui Wang wrote: > > > On 2019/5/24 下午1:36, Peter Hutterer wrote: > > > > On Fri, May 24, 2019 at 01:25:52PM +0800, Hui Wang wrote: > > > > > On 2019/5/23 下午2:01, Peter Hutterer wrote: > > > > > > On Wed, May 22, 2019 at 09:40:30AM +0200, Pali Rohár wrote: > > > > > > > On Wednesday 22 May 2019 07:30:43 Xiaoxiao Liu wrote: > > > > > > > > Hi Pali, > > > > > > > > > > > > > > > > Ok, and cannot you set ALPS_DUALPOINT flag based on that > > > > > > > > alps_check_is_trackpoint() result and then update > > > > > > > > alps_process_packet_ss4_v3() code to supports also > > > > > > > > V8 trackpoint packets? > > > > > > > > --> Yes, we can do like so, when we use the v8 method to process the trackpoint , the mouse speed is not ideal. > > > > > > > > Then we choose the standard mouse driver. > > > > > > > Mouse speed is something which is configurable. Have you configured it > > > > > > > somehow? Also there is libinput project should handle these settings > > > > > > > more properly. > > > > > > > > > > > > > > Adding Peter Hutterer, maintainer of libinput to loop. I think he could > > > > > > > help with this problem. > > > > > > libinput has a quirk for a magic multiplier on trackpoints. it was the only > > > > > > solution I found that came close to "working" given that every device seems > > > > > > to provide some other random magic data. Doc for it is here: > > > > > > https://wayland.freedesktop.org/libinput/doc/latest/trackpoint-configuration.html > > > > > Hello Peter Hutterer, > > > > > > > > > > To adjust the trackpoint speed from userspace: > > > > > > > > > > If the libinput version is lower than 1.9.0, we could set > > > > > POINTINGSTICK_CONST_ACCEL=0.25 > > > > > > > > > > If the libinput version is higher than 1.12.0, we could set > > > > > AttrTrackpointMultiplier=0.25 > > > > > > > > > > But if we use libinput-1.10.0, how could we adjust the speed? > > > > The LIBINPUT_ATTR_TRACKPOINT_RANGE property, which didn't end up working > > > > well (hence why it got replaced again). See the docs here though: > > > > https://wayland.freedesktop.org/libinput/doc/1.10.0/trackpoints.html > > > > > > > > Cheers, > > > > Peter > > > > > > OK, got it, Thanks. > > > > Is not here some database where for input device name / id is specified > > that property? So users do not have to invent what is correct value for > > their hardware? > > yeah, libinput ships with a quirks database that sets those, but it relies > on users to submit the quirks. > https://gitlab.freedesktop.org/libinput/libinput/tree/master/quirks That is enough! > There's no "correct" value anyway, if you ask 3 users what the trackpoint > should do you'll get 5 answers :) I know :) but reason for such database is to provide some sane defaults. Which seems it is already there.
On Fri, May 24, 2019 at 06:43:58PM +0800, Hui Wang wrote: > > On 2019/5/24 下午5:37, Peter Hutterer wrote: > > On Fri, May 24, 2019 at 03:37:57PM +0800, Hui Wang wrote: > > > On 2019/5/24 下午3:26, Pali Rohár wrote: > > > > On Friday 24 May 2019 13:50:53 Hui Wang wrote: > > > > > On 2019/5/24 下午1:36, Peter Hutterer wrote: > > > > > > On Fri, May 24, 2019 at 01:25:52PM +0800, Hui Wang wrote: > > > > > > > On 2019/5/23 下午2:01, Peter Hutterer wrote: > > > > > > > > On Wed, May 22, 2019 at 09:40:30AM +0200, Pali Rohár wrote: > > > > > > > > > On Wednesday 22 May 2019 07:30:43 Xiaoxiao Liu wrote: > > > > > > > > > > Hi Pali, > > > > > > > > > > > > > > > > > > > > Ok, and cannot you set ALPS_DUALPOINT flag based on that > > > > > > > > > > alps_check_is_trackpoint() result and then update > > > > > > > > > > alps_process_packet_ss4_v3() code to supports also > > > > > > > > > > V8 trackpoint packets? > > > > > > > > > > --> Yes, we can do like so, when we use the v8 method to process the trackpoint , the mouse speed is not ideal. > > > > > > > > > > Then we choose the standard mouse driver. > > > > > > > > > Mouse speed is something which is configurable. Have you configured it > > > > > > > > > somehow? Also there is libinput project should handle these settings > > > > > > > > > more properly. > > > > > > > > > > > > > > > > > > Adding Peter Hutterer, maintainer of libinput to loop. I think he could > > > > > > > > > help with this problem. > > > > > > > > libinput has a quirk for a magic multiplier on trackpoints. it was the only > > > > > > > > solution I found that came close to "working" given that every device seems > > > > > > > > to provide some other random magic data. Doc for it is here: > > > > > > > > https://wayland.freedesktop.org/libinput/doc/latest/trackpoint-configuration.html > > > > > > > Hello Peter Hutterer, > > > > > > > > > > > > > > To adjust the trackpoint speed from userspace: > > > > > > > > > > > > > > If the libinput version is lower than 1.9.0, we could set > > > > > > > POINTINGSTICK_CONST_ACCEL=0.25 > > > > > > > > > > > > > > If the libinput version is higher than 1.12.0, we could set > > > > > > > AttrTrackpointMultiplier=0.25 > > > > > > > > > > > > > > But if we use libinput-1.10.0, how could we adjust the speed? > > > > > > The LIBINPUT_ATTR_TRACKPOINT_RANGE property, which didn't end up working > > > > > > well (hence why it got replaced again). See the docs here though: > > > > > > https://wayland.freedesktop.org/libinput/doc/1.10.0/trackpoints.html > > > > > > > > > > > > Cheers, > > > > > > Peter > > > > > OK, got it, Thanks. > > > > Is not here some database where for input device name / id is specified > > > > that property? So users do not have to invent what is correct value for > > > > their hardware? > > > Since the libinput version in the ubuntu 18.04 is 1.10, I tried to set > > > LIBINPUT_ATTR_TRACKPOINT_RANGE with different values (from 25, 20, 10, 5) in > > > the udev hwdb database, I checked it with "udevadm info /dev/input/eventX" > > > to confirm the value is set to LIBINPUT_ATTR_TRACKPOINT_RANGE successfully, > > > but looks like the cursor speed doesn't change at all. So for ubuntu 18.04, > > > looks like we have to adjust the speed in the kernel driver. > > I don't think it's a good idea to make the kernel driver behaviour based on > > an EOL libinput version just because one version of ubuntu ships with that. > > 18.10 and 19.04 both ship with libinput 1.12. > > > > It'd be better to make sure it works well with a *current* version and then > > patch libinput on 18.04 where needed. > > OK, that is sth we need to do. But anyway it is a bit risky to backport > that much code and the whole folder of quirks to libinput 1.10.4, we need > to do lots of test to make sure there is no regression on other machines. > > Probably we only need to keep the quirks/30-vendor-alps.quirks to 1.10.4 and > drop other quirks, that will be better for our testing effort. might be worth looking at what is in 1.10.7, e.g. a3b3e85c0e looks like it may be of interest. That one suggests the range on some ALPS devices is over 100, so testing with 5-25 may really not have had any effect. Cheers, Peter
On 2019/5/24 下午6:58, Peter Hutterer wrote: > On Fri, May 24, 2019 at 06:43:58PM +0800, Hui Wang wrote: >> On 2019/5/24 下午5:37, Peter Hutterer wrote: >>> On Fri, May 24, 2019 at 03:37:57PM +0800, Hui Wang wrote: >>>> >> OK, that is sth we need to do. But anyway it is a bit risky to backport >> that much code and the whole folder of quirks to libinput 1.10.4, we need >> to do lots of test to make sure there is no regression on other machines. >> >> Probably we only need to keep the quirks/30-vendor-alps.quirks to 1.10.4 and >> drop other quirks, that will be better for our testing effort. > might be worth looking at what is in 1.10.7, e.g. a3b3e85c0e looks like it > may be of interest. That one suggests the range on some ALPS devices is over > 100, so testing with 5-25 may really not have had any effect. Oh, looks exactly the same as our issue, will have a try with it. Thanks, Hui. > > Cheers, > Peter >
diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c index 0a6f7ca883e7..516ae1d0eb17 100644 --- a/drivers/input/mouse/alps.c +++ b/drivers/input/mouse/alps.c @@ -24,7 +24,7 @@ #include "psmouse.h" #include "alps.h" - +#include "trackpoint.h" /* * Definitions for ALPS version 3 and 4 command mode protocol */ @@ -2864,6 +2864,22 @@ static const struct alps_protocol_info *alps_match_table(unsigned char *e7, return NULL; } +int alps_check_is_trackpoint(struct psmouse *psmouse) +{ + u8 param[2] = { 0 }; + int error; + + error = ps2_command(&psmouse->ps2dev, + param, MAKE_PS2_CMD(0, 2, TP_READ_ID)); + if (error) + return error; + + if (param[0] == TP_VARIANT_ALPS) + return 0; + psmouse_warn(psmouse, "It is not alps trackpoint.\n"); + return -ENODEV; +} + static int alps_identify(struct psmouse *psmouse, struct alps_data *priv) { const struct alps_protocol_info *protocol; @@ -2912,6 +2928,11 @@ static int alps_identify(struct psmouse *psmouse, struct alps_data *priv) protocol = &alps_v3_protocol_data; } else if (e7[0] == 0x73 && e7[1] == 0x03 && (e7[2] == 0x14 || e7[2] == 0x28)) { + if (alps_check_is_trackpoint(psmouse) == 0) { + psmouse_warn(psmouse, + "It is alps trackpoint, use the standard mouse driver.\n"); + return -EINVAL; + } protocol = &alps_v8_protocol_data; } else if (e7[0] == 0x73 && e7[1] == 0x03 && e7[2] == 0xc8) { protocol = &alps_v9_protocol_data;
when the alps trackpoint is detected and using the alps_v8_protocol_data procotol, the alps driver will not report the input data. solution: use standard mouse driver instead of alps driver when the specail trackpoint was detected. Signed-off-by: XiaoXiao Liu <sliuuxiaonxiao@gmail.com> --- drivers/input/mouse/alps.c | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-)