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@xxxxxxxxx
xiaoxiao.liu-1@xxxxxxxxxxx
>
> -----邮件原件-----
发件人: Pali Rohár <pali.rohar@xxxxxxxxx>
发送时间: Wednesday, May 22, 2019 2:36 PM
收件人: 劉 曉曉 Xiaoxiao Liu <xiaoxiao.liu-1@xxxxxxxxxxx>
抄送: Hui Wang <hui.wang@xxxxxxxxxxxxx>; XiaoXiao Liu <sliuuxiaonxiao@xxxxxxxxx>; dmitry.torokhov@xxxxxxxxx; linux-input@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; 曹 曉建 Xiaojian Cao <xiaojian.cao@xxxxxxxxxxx>; zhangfp1@xxxxxxxxxx
主题: 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@xxxxxxxxx
> xiaoxiao.liu-1@xxxxxxxxxxx
>
> -----邮件原件-----
> 发件人: Pali Rohár <pali.rohar@xxxxxxxxx>
> 发送时间: Tuesday, May 21, 2019 5:46 PM
> 收件人: Hui Wang <hui.wang@xxxxxxxxxxxxx>
> 抄送: 劉 曉曉 Xiaoxiao Liu <xiaoxiao.liu-1@xxxxxxxxxxx>; XiaoXiao Liu
> <sliuuxiaonxiao@xxxxxxxxx>; dmitry.torokhov@xxxxxxxxx;
> linux-input@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; 曹 曉建
> Xiaojian Cao <xiaojian.cao@xxxxxxxxxxx>; zhangfp1@xxxxxxxxxx
> 主题: 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@xxxxxxxxxxxxx>
> >
> > On 2019/5/21 上午9:07, Xiaoxiao Liu wrote:
> > > Add Pali Rohár.
> > >
> > > -----邮件原件-----
> > > 发件人: XiaoXiao Liu <sliuuxiaonxiao@xxxxxxxxx>
> > > 发送时间: Monday, May 20, 2019 7:02 PM
> > > 收件人: dmitry.torokhov@xxxxxxxxx
> > > 抄送: linux-input@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> > > hui.wang@xxxxxxxxxxxxx; 曹 曉建 Xiaojian Cao
> > > <xiaojian.cao@xxxxxxxxxxx>; zhangfp1@xxxxxxxxxx; 劉 曉曉 Xiaoxiao Liu
> > > <xiaoxiao.liu-1@xxxxxxxxxxx>; XiaoXiao Liu
> > > <sliuuxiaonxiao@xxxxxxxxx>
> > > 主题: [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@xxxxxxxxx>
> > > ---
> > > 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@xxxxxxxxx
--
Pali Rohár
pali.rohar@xxxxxxxxx
