| Message ID | 20161007044148.ssgxygdvmiwjsb55@sherka.pkbd.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Christophe, On Fri, Oct 07, 2016 at 12:41:48PM +0800, Christophe Tordeux wrote: > From: Christophe TORDEUX <christophe@tordeux.net> > > With kernel v4.6 and later, the Sentelic touchpad STL3888_C0 and > probably other Sentelic FSP touchpads are detected as a BYD touchpad and > lose multitouch features. > > During the BYD handshake in the byd_detect function, the BYD driver > mistakenly interprets a standard PS/2 protocol status request answer > from the Sentelic touchpad as a successful handshake with a BYD > touchpad. This is clearly a bug of the BYD driver. > > Description of the patch: In byd_detect function, remove positive > detection result based on standard PS/2 protocol status request answer. > Replace it with positive detection based on handshake answers as they > can be inferred from the BYD touchpad datasheets found on BYD website. > > Signed-off-by: Christophe TORDEUX <christophe@tordeux.net> What devices was this tested on? Do you have both BYD and Sentelic devices? I am really concerned about docs on BYD side as they seem to contradict each other... I wonder how accurate they are. Thanks. > > --- > Resubmitting this patch because I got no feedback on my first > submission. > Fixes kernel bug 175421 which is impacting multiple users. > > --- > drivers/input/mouse/byd.c | 76 > ++++++++++++++++++++++++++++++++++++++--------- > 1 file changed, 62 insertions(+), 14 deletions(-) > > diff --git a/drivers/input/mouse/byd.c b/drivers/input/mouse/byd.c > index b27aa63..b5acca0 100644 > --- a/drivers/input/mouse/byd.c > +++ b/drivers/input/mouse/byd.c > @@ -35,6 +35,18 @@ > * BYD pad constants > */ > > +/* Handshake answer of BTP6034 */ > +#define BYD_MODEL_BTP6034 0x00E801 > +/* Handshake answer of BTP6740 */ > +#define BYD_MODEL_BTP6740 0x001155 > +/* Handshake answers of BTP8644, BTP10463 and BTP11484 */ > +#define BYD_MODEL_BTP8644 0x011155 > + > +/* Handshake SETRES byte of BTP6034 and BTP6740 */ > +#define BYD_SHAKE_BYTE_A 0x00 > +/* Handshake SETRES byte of BTP8644, BTP10463 and BTP11484 */ > +#define BYD_SHAKE_BYTE_B 0x03 > + > /* > * True device resolution is unknown, however experiments show the > * resolution is about 111 units/mm. > @@ -434,23 +446,59 @@ static void byd_disconnect(struct psmouse *psmouse) > } > } > > +u32 byd_try_model(u32 model) > +{ > + size_t i; > + > + u32 byd_model[] = { > + BYD_MODEL_BTP6034, > + BYD_MODEL_BTP6740, > + BYD_MODEL_BTP8644 > + }; > + > + for (i=0; i < ARRAY_SIZE(byd_model); i++) { > + if (model == byd_model[i]) > + return model; > + } > + > + return 0; > +} > + > int byd_detect(struct psmouse *psmouse, bool set_properties) > { > struct ps2dev *ps2dev = &psmouse->ps2dev; > - u8 param[4] = {0x03, 0x00, 0x00, 0x00}; > - > - if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES)) > - return -1; > - if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES)) > - return -1; > - if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES)) > - return -1; > - if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES)) > - return -1; > - if (ps2_command(ps2dev, param, PSMOUSE_CMD_GETINFO)) > - return -1; > - > - if (param[1] != 0x03 || param[2] != 0x64) > + size_t i; > + > + u8 byd_shbyte[] = { > + BYD_SHAKE_BYTE_A, > + BYD_SHAKE_BYTE_B > + }; > + > + bool detect = false; > + for (i=0; i < ARRAY_SIZE(byd_shbyte); i++) { > + u32 model; > + u8 param[4] = {byd_shbyte[i], 0x00, 0x00, 0x00}; > + > + if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES)) > + return -1; > + if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES)) > + return -1; > + if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES)) > + return -1; > + if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES)) > + return -1; > + if (ps2_command(ps2dev, param, PSMOUSE_CMD_GETINFO)) > + return -1; > + > + model = param[2]; > + model += param[1] << 8; > + model += param[0] << 16; > + model = byd_try_model(model); > + if (model) > + detect = true; > + } > + > + if (!detect) > return -ENODEV; > > psmouse_dbg(psmouse, "BYD touchpad detected\n");
Hello, On Friday 07 October 2016 at 04:37:26PM, Dmitry Torokhov has written: > Hi Christophe, > > On Fri, Oct 07, 2016 at 12:41:48PM +0800, Christophe Tordeux wrote: > > From: Christophe TORDEUX <christophe@tordeux.net> > > > > With kernel v4.6 and later, the Sentelic touchpad STL3888_C0 and > > probably other Sentelic FSP touchpads are detected as a BYD touchpad and > > lose multitouch features. > > > > During the BYD handshake in the byd_detect function, the BYD driver > > mistakenly interprets a standard PS/2 protocol status request answer > > from the Sentelic touchpad as a successful handshake with a BYD > > touchpad. This is clearly a bug of the BYD driver. > > > > Description of the patch: In byd_detect function, remove positive > > detection result based on standard PS/2 protocol status request answer. > > Replace it with positive detection based on handshake answers as they > > can be inferred from the BYD touchpad datasheets found on BYD website. > > > > Signed-off-by: Christophe TORDEUX <christophe@tordeux.net> > > What devices was this tested on? Do you have both BYD and Sentelic > devices? > I only have a Sentelic STL3888_C0 touchpad, I don't have any BYD touchpad. > I am really concerned about docs on BYD side as they seem to contradict > each other... I wonder how accurate they are. > I agree, there's problems with each of their docs. I tried to infer the handshake answers based on the big picture that seems to emerge when reading all of them. If ever that matters, I found out that my STL3888_C0 touchpad: * answers E8/03/E8/03/E8/03/E8/03/E9 like standard PS/2 protocol * answers E8/00/E8/00/E8/00/E8/00/E9 with 00/88/64 The latter does not look standard PS/2 so may be a kind of handshake from the Sentelic touchpad. And, in the BYD docs, part of the BYD devices also call for 00 during the handshake, others call for 03. Things which can be considered in addition or in parallel to my patch: 1) in psmouse-base.c, move the detection of Sentelic touchpads above the detection of BYD touchpads. However based on the code comments could have adverse effects I don't know about. 2) Exit the byd_detect function without detection based on E8/00/E8/00/E8/00/E8/00/E9 being answered by 00/88/64. However while it would certainly fix my issue with my particular touchpad, it still would leaves the byd_detect function being dirty and greedy otherwise, so I don't really like it. 3) take my patch, keep the 00 and 03 handshakes in sequence, add a detection based on the first byte of data being 01 regardless of the other bytes of data in the answer. It would probably fix the Sentelic detection issue... unless the user is pressing the left button during the BYD handshake. From the BYD docs "it is expected the get the value of 01 in the first byte data (...) and the first byte data is the only data needed to be checked". The issue with the docs is that this statement is sometimes contradicted by the sketch of the handshake... My patch is mostly based on the handshake sketches currently, but I could add a detection based on the first byte being 01, with no other test. Let me know what looks best for you, for me the third choice is probably the most reasonable solution, either this or do it like in my initial patch. If you know someone who has the kernel sources and a BYD touchpad, ask him to get the answer to E8/00/E8/00/E8/00/E8/00/E9 from BYD touchpads. > Thanks. > > > > > --- > > Resubmitting this patch because I got no feedback on my first > > submission. > > Fixes kernel bug 175421 which is impacting multiple users. > > > > --- > > drivers/input/mouse/byd.c | 76 > > ++++++++++++++++++++++++++++++++++++++--------- > > 1 file changed, 62 insertions(+), 14 deletions(-) > > > > diff --git a/drivers/input/mouse/byd.c b/drivers/input/mouse/byd.c > > index b27aa63..b5acca0 100644 > > --- a/drivers/input/mouse/byd.c > > +++ b/drivers/input/mouse/byd.c > > @@ -35,6 +35,18 @@ > > * BYD pad constants > > */ > > > > +/* Handshake answer of BTP6034 */ > > +#define BYD_MODEL_BTP6034 0x00E801 > > +/* Handshake answer of BTP6740 */ > > +#define BYD_MODEL_BTP6740 0x001155 > > +/* Handshake answers of BTP8644, BTP10463 and BTP11484 */ > > +#define BYD_MODEL_BTP8644 0x011155 > > + > > +/* Handshake SETRES byte of BTP6034 and BTP6740 */ > > +#define BYD_SHAKE_BYTE_A 0x00 > > +/* Handshake SETRES byte of BTP8644, BTP10463 and BTP11484 */ > > +#define BYD_SHAKE_BYTE_B 0x03 > > + > > /* > > * True device resolution is unknown, however experiments show the > > * resolution is about 111 units/mm. > > @@ -434,23 +446,59 @@ static void byd_disconnect(struct psmouse *psmouse) > > } > > } > > > > +u32 byd_try_model(u32 model) > > +{ > > + size_t i; > > + > > + u32 byd_model[] = { > > + BYD_MODEL_BTP6034, > > + BYD_MODEL_BTP6740, > > + BYD_MODEL_BTP8644 > > + }; > > + > > + for (i=0; i < ARRAY_SIZE(byd_model); i++) { > > + if (model == byd_model[i]) > > + return model; > > + } > > + > > + return 0; > > +} > > + > > int byd_detect(struct psmouse *psmouse, bool set_properties) > > { > > struct ps2dev *ps2dev = &psmouse->ps2dev; > > - u8 param[4] = {0x03, 0x00, 0x00, 0x00}; > > - > > - if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES)) > > - return -1; > > - if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES)) > > - return -1; > > - if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES)) > > - return -1; > > - if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES)) > > - return -1; > > - if (ps2_command(ps2dev, param, PSMOUSE_CMD_GETINFO)) > > - return -1; > > - > > - if (param[1] != 0x03 || param[2] != 0x64) > > + size_t i; > > + > > + u8 byd_shbyte[] = { > > + BYD_SHAKE_BYTE_A, > > + BYD_SHAKE_BYTE_B > > + }; > > + > > + bool detect = false; > > + for (i=0; i < ARRAY_SIZE(byd_shbyte); i++) { > > + u32 model; > > + u8 param[4] = {byd_shbyte[i], 0x00, 0x00, 0x00}; > > + > > + if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES)) > > + return -1; > > + if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES)) > > + return -1; > > + if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES)) > > + return -1; > > + if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES)) > > + return -1; > > + if (ps2_command(ps2dev, param, PSMOUSE_CMD_GETINFO)) > > + return -1; > > + > > + model = param[2]; > > + model += param[1] << 8; > > + model += param[0] << 16; > > + model = byd_try_model(model); > > + if (model) > > + detect = true; > > + } > > + > > + if (!detect) > > return -ENODEV; > > > > psmouse_dbg(psmouse, "BYD touchpad detected\n"); > > > > -- > Dmitry >
diff --git a/drivers/input/mouse/byd.c b/drivers/input/mouse/byd.c index b27aa63..b5acca0 100644 --- a/drivers/input/mouse/byd.c +++ b/drivers/input/mouse/byd.c @@ -35,6 +35,18 @@ * BYD pad constants */ +/* Handshake answer of BTP6034 */ +#define BYD_MODEL_BTP6034 0x00E801 +/* Handshake answer of BTP6740 */ +#define BYD_MODEL_BTP6740 0x001155 +/* Handshake answers of BTP8644, BTP10463 and BTP11484 */ +#define BYD_MODEL_BTP8644 0x011155 + +/* Handshake SETRES byte of BTP6034 and BTP6740 */ +#define BYD_SHAKE_BYTE_A 0x00 +/* Handshake SETRES byte of BTP8644, BTP10463 and BTP11484 */ +#define BYD_SHAKE_BYTE_B 0x03 + /* * True device resolution is unknown, however experiments show the * resolution is about 111 units/mm. @@ -434,23 +446,59 @@ static void byd_disconnect(struct psmouse *psmouse) } } +u32 byd_try_model(u32 model) +{ + size_t i; + + u32 byd_model[] = { + BYD_MODEL_BTP6034, + BYD_MODEL_BTP6740, + BYD_MODEL_BTP8644 + }; + + for (i=0; i < ARRAY_SIZE(byd_model); i++) { + if (model == byd_model[i]) + return model; + } + + return 0; +} + int byd_detect(struct psmouse *psmouse, bool set_properties) { struct ps2dev *ps2dev = &psmouse->ps2dev; - u8 param[4] = {0x03, 0x00, 0x00, 0x00}; - - if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES)) - return -1; - if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES)) - return -1; - if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES)) - return -1; - if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES)) - return -1; - if (ps2_command(ps2dev, param, PSMOUSE_CMD_GETINFO)) - return -1; - - if (param[1] != 0x03 || param[2] != 0x64) + size_t i; + + u8 byd_shbyte[] = { + BYD_SHAKE_BYTE_A, + BYD_SHAKE_BYTE_B + }; + + bool detect = false; + for (i=0; i < ARRAY_SIZE(byd_shbyte); i++) { + u32 model; + u8 param[4] = {byd_shbyte[i], 0x00, 0x00, 0x00}; + + if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES)) + return -1; + if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES)) + return -1; + if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES)) + return -1; + if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES)) + return -1; + if (ps2_command(ps2dev, param, PSMOUSE_CMD_GETINFO)) + return -1; + + model = param[2]; + model += param[1] << 8; + model += param[0] << 16; + model = byd_try_model(model); + if (model) + detect = true; + } + + if (!detect) return -ENODEV; psmouse_dbg(psmouse, "BYD touchpad detected\n");