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
sliuuxiaonx...@gmail.com
xiaoxiao.li...@cn.alps.com
> 
> -----邮件原件-----
发件人: Pali Rohár <pali.ro...@gmail.com> 
发送时间: Wednesday, May 22, 2019 2:36 PM
收件人: 劉 曉曉 Xiaoxiao Liu <xiaoxiao.li...@cn.alps.com>
抄送: Hui Wang <hui.w...@canonical.com>; XiaoXiao Liu <sliuuxiaonx...@gmail.com>; 
dmitry.torok...@gmail.com; linux-in...@vger.kernel.org; 
linux-kernel@vger.kernel.org; 曹 曉建 Xiaojian Cao <xiaojian....@cn.alps.com>; 
zhang...@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
> sliuuxiaonx...@gmail.com
> xiaoxiao.li...@cn.alps.com
> 
> -----邮件原件-----
> 发件人: Pali Rohár <pali.ro...@gmail.com>
> 发送时间: Tuesday, May 21, 2019 5:46 PM
> 收件人: Hui Wang <hui.w...@canonical.com>
> 抄送: 劉 曉曉 Xiaoxiao Liu <xiaoxiao.li...@cn.alps.com>; XiaoXiao Liu 
> <sliuuxiaonx...@gmail.com>; dmitry.torok...@gmail.com; 
> linux-in...@vger.kernel.org; linux-kernel@vger.kernel.org; 曹 曉建 
> Xiaojian Cao <xiaojian....@cn.alps.com>; zhang...@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.w...@canonical.com>
> > 
> > On 2019/5/21 上午9:07, Xiaoxiao Liu wrote:
> > > Add Pali Rohár.
> > > 
> > > -----邮件原件-----
> > > 发件人: XiaoXiao Liu <sliuuxiaonx...@gmail.com>
> > > 发送时间: Monday, May 20, 2019 7:02 PM
> > > 收件人: dmitry.torok...@gmail.com
> > > 抄送: linux-in...@vger.kernel.org; linux-kernel@vger.kernel.org; 
> > > hui.w...@canonical.com; 曹 曉建 Xiaojian Cao 
> > > <xiaojian....@cn.alps.com>; zhang...@lenovo.com; 劉 曉曉 Xiaoxiao Liu 
> > > <xiaoxiao.li...@cn.alps.com>; XiaoXiao Liu 
> > > <sliuuxiaonx...@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 <sliuuxiaonx...@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.ro...@gmail.com

--
Pali Rohár
pali.ro...@gmail.com

Reply via email to