[PATCH] Input: byd - use DMI detection

[PATCH] Input: byd - use DMI detection

[Chris Diamand]

Because the BYD touchpad uses standard PS/2 commands for its
detection sequence, some other models are incorrectly detected as BYD
touchpads. This causes chaos when byd_init() later fails.

To fix this, init() and detect() should be merged. However, this
would slow down detection for other mouse models. Instead, add a DMI
check before attempting touchpad detection.

Signed-off-by: Chris Diamand <chris@diamand.org>
---
Hi all,

This patch should fix the mis-detection of some mouse models as BYD touchpads,
as mentioned in a few bug reports and other threads.

However, I no longer have a machine with a BYD touchpad (although I did record
its DMI data), so this is mostly untested - could anyone *with* a BYD touchpad
please try this patch to check their touchpad still works, and could anyone
with a misdetected non-BYD touchpad please check that this fixes the
misdetection?

Also, the DMI fields really are pretty much all like that ("To Be Filled By
O.E.M.", etc). I've tried to choose the most specific ones, but it's still
fairly arbitrary...

Cheers!
Chris

 drivers/input/mouse/byd.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/input/mouse/byd.c b/drivers/input/mouse/byd.c
index b27aa63..b4c5fd0 100644
--- a/drivers/input/mouse/byd.c
+++ b/drivers/input/mouse/byd.c
@@ -13,6 +13,7 @@
  */
 
 #include <linux/delay.h>
+#include <linux/dmi.h>
 #include <linux/input.h>
 #include <linux/libps2.h>
 #include <linux/serio.h>
@@ -235,6 +236,17 @@ struct byd_data {
 	bool touch;
 };
 
+static const struct dmi_system_id byd_dmi_table[] = {
+	{
+		.matches = {
+			DMI_MATCH(DMI_PRODUCT_NAME, "SharkBay Platform"),
+			DMI_MATCH(DMI_PRODUCT_SERIAL, "System Serial Number"),
+			DMI_MATCH(DMI_BOARD_NAME, "WhiteTip Mountain1 Fab2"),
+			DMI_MATCH(DMI_CHASSIS_VENDOR, "To Be Filled By O.E.M."),
+		},
+	},
+};
+
 static void byd_report_input(struct psmouse *psmouse)
 {
 	struct byd_data *priv = psmouse->private;
@@ -439,6 +451,9 @@ int byd_detect(struct psmouse *psmouse, bool set_properties)
 	struct ps2dev *ps2dev = &psmouse->ps2dev;
 	u8 param[4] = {0x03, 0x00, 0x00, 0x00};
 
+	if (!dmi_check_system(byd_dmi_table))
+		return -1;
+
 	if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES))
 		return -1;
 	if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES))
-- 
2.10.1

Re: [PATCH] Input: byd - use DMI detection

[Pali Rohár]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On So 12. November 2016 00:57 Chris Diamand wrote:
...
> +static const struct dmi_system_id byd_dmi_table[] = {
> +	{
> +		.matches = {
> +			DMI_MATCH(DMI_PRODUCT_NAME, "SharkBay Platform"),
> +			DMI_MATCH(DMI_PRODUCT_SERIAL, "System Serial Number"),
> +			DMI_MATCH(DMI_BOARD_NAME, "WhiteTip Mountain1 Fab2"),
> +			DMI_MATCH(DMI_CHASSIS_VENDOR, "To Be Filled By O.E.M."),
> +		},
> +	},
> +};

Hi! This looks very fragile. We really cannot test presence of some device
by DMI string "To Be Filled By O.E.M." or by "WhiteTip Mountain1 Fab2".
Look at other dmi detection code. We match full device and vendor.

...
> +	if (!dmi_check_system(byd_dmi_table))
> +		return -1;
> +
>  if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES))
>  return -1;
>  if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES))
...

If we really have a problem when byd detect incorrect detect some non-byd
device as byd, we either needs:

1) extend detect code to include also parts of init sequence (this should
   fix problem on wrongly detected non-byd devices, but init code on them
   will take longer)

2) or use another detection technique, which will address above problem (
   your "To Be Filled By O.E.M." is not good; maybe looking at ACPI?)

- -- 
Pali Rohár
pali.rohar@gmail.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)

iEYEARECAAYFAlgnBy0ACgkQi/DJPQPkQ1JWmwCffbqKKjwjxVc+y2Gghb9bCcSB
Ae8AoJ8Hn+xP4h3tFqEi2Yna9hpYGYql
=Dvey
-----END PGP SIGNATURE-----

Re: [PATCH] Input: byd - use DMI detection

[Chris Diamand]

Hi, and thanks for your reply.

> Hi! This looks very fragile. We really cannot test presence of some device
> by DMI string "To Be Filled By O.E.M." or by "WhiteTip Mountain1 Fab2".
> Look at other dmi detection code. We match full device and vendor.

Do you think it's likely to match too many or too few machines? I can match
the device and vendor instead, but I think that would have more false
positives (DMI_SYS_VENDOR and DMI_PRODUCT_NAME are "Intel Corporation" and
"Sharkbay Platform").

Here are all the DMI fields we can use, with their values on my (former) BYD
machine. As you can see, none of them are particularly helpful. However, I
*hope* that most other manufacturers fill them in, so I'd be surprised if
there are many false positives.

DMI_BIOS_VENDOR: American Megatrends Inc.
DMI_BIOS_VERSION: 5.6.5
DMI_BIOS_DATE: 06/30/2015
DMI_SYS_VENDOR: Intel Corporation
DMI_PRODUCT_NAME: Sharkbay Platform
DMI_PRODUCT_VERSION: 0.1
DMI_PRODUCT_SERIAL: System Serial Number
DMI_PRODUCT_UUID: 03000200-0400-0500-0006-000700080009
DMI_BOARD_VENDOR: Topstar
DMI_BOARD_NAME: WhiteTip Mountain1 Fab2
DMI_BOARD_VERSION: Fab2
DMI_BOARD_SERIAL: 1
DMI_BOARD_ASSET_TAG: Base Board Asset Tag
DMI_CHASSIS_VENDOR: To Be Filled By O.E.M.
DMI_CHASSIS_TYPE: Laptop
DMI_CHASSIS_VERSION: To Be Filled By O.E.M.
DMI_CHASSIS_SERIAL: To Be Filled By O.E.M.
DMI_CHASSIS_ASSET_TAG: To Be Filled By O.E.M.

> If we really have a problem when byd detect incorrect detect some non-byd
> device as byd, we either needs:
>
> 1) extend detect code to include also parts of init sequence (this should
>    fix problem on wrongly detected non-byd devices, but init code on them
>    will take longer)

I agree. What I am trying to achieve with this patch is to fix 99% of cases
where we wrongly detect a non-BYD device. There may still be a few false
positives, but for those we can apply Richard's patch, which moves the
whole init sequence to byd_detect().
http://www.spinics.net/lists/linux-input/msg45539.html

This will slow down detection of normal mice, but if my DMI patch is also
applied, it will only affect a tiny fraction of users.

Also, Richard's patch would fix the even rarer case of someone trying to use
a mis-detected mouse on an actual BYD device.

> 2) or use another detection technique, which will address above problem (
>    your "To Be Filled By O.E.M." is not good; maybe looking at ACPI?)

That could also work, but I wouldn't be able to write such a patch as I no
longer have access to the device.

Anyway, in summary; the ideal solution seems to be DMI matching + extended
detection sequence - this patch is the first part.

Cheers!
Chris

Re: [PATCH] Input: byd - use DMI detection

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 4107 bytes --]

On Saturday 12 November 2016 15:48:14 Chris Diamand wrote:
> Hi, and thanks for your reply.
> 
> > Hi! This looks very fragile. We really cannot test presence of some
> > device by DMI string "To Be Filled By O.E.M." or by "WhiteTip
> > Mountain1 Fab2". Look at other dmi detection code. We match full
> > device and vendor.
> 
> Do you think it's likely to match too many or too few machines? I can
> match the device and vendor instead, but I think that would have
> more false positives (DMI_SYS_VENDOR and DMI_PRODUCT_NAME are "Intel
> Corporation" and "Sharkbay Platform").
> 
> Here are all the DMI fields we can use, with their values on my
> (former) BYD machine. As you can see, none of them are particularly
> helpful. However, I *hope* that most other manufacturers fill them
> in, so I'd be surprised if there are many false positives.
> 
> DMI_BIOS_VENDOR: American Megatrends Inc.
> DMI_BIOS_VERSION: 5.6.5
> DMI_BIOS_DATE: 06/30/2015
> DMI_SYS_VENDOR: Intel Corporation
> DMI_PRODUCT_NAME: Sharkbay Platform
> DMI_PRODUCT_VERSION: 0.1
> DMI_PRODUCT_SERIAL: System Serial Number
> DMI_PRODUCT_UUID: 03000200-0400-0500-0006-000700080009
> DMI_BOARD_VENDOR: Topstar
> DMI_BOARD_NAME: WhiteTip Mountain1 Fab2
> DMI_BOARD_VERSION: Fab2
> DMI_BOARD_SERIAL: 1
> DMI_BOARD_ASSET_TAG: Base Board Asset Tag
> DMI_CHASSIS_VENDOR: To Be Filled By O.E.M.
> DMI_CHASSIS_TYPE: Laptop
> DMI_CHASSIS_VERSION: To Be Filled By O.E.M.
> DMI_CHASSIS_SERIAL: To Be Filled By O.E.M.
> DMI_CHASSIS_ASSET_TAG: To Be Filled By O.E.M.

Sorry, but this DMI information does not tell anything about presense of 
BYD touchpad. Moreover such generic matches like "Sharkbay Platform" or 
"To Be Filled By O.E.M." can be found on any other non-BYD touchpad 
devices and it just cause lot of other problems... Also what about 
devices which do not have "To Be Filled By O.E.M." and have BYD 
touchpad? You basically break support for such devices. I'm sure such 
logic just cause problems in future...

How is windows detecting presence of BYD touchpads? Should we use 
something similar? Or are not there some (public) documentation about 
it?

> > If we really have a problem when byd detect incorrect detect some
> > non-byd device as byd, we either needs:
> > 
> > 1) extend detect code to include also parts of init sequence (this
> > should
> > 
> >    fix problem on wrongly detected non-byd devices, but init code
> >    on them will take longer)
> 
> I agree. What I am trying to achieve with this patch is to fix 99% of
> cases where we wrongly detect a non-BYD device.

Still I think this is just ugly and non-working hack, not a proper 
solution. But I'm not in position to decide, so wait what will Dmitry 
(as maintainer) wrote about it...

> There may still be a
> few false positives, but for those we can apply Richard's patch,
> which moves the whole init sequence to byd_detect().
> http://www.spinics.net/lists/linux-input/msg45539.html
> 
> This will slow down detection of normal mice, but if my DMI patch is
> also applied, it will only affect a tiny fraction of users.

Maybe you should check if there is device or port DMI information? 
dmidecode will print it, but I sceptic about it...

> Also, Richard's patch would fix the even rarer case of someone trying
> to use a mis-detected mouse on an actual BYD device.

I would prefer slower, but correct detection. Not just some nonsense 
heuristic based on "To Be Filled By O.E.M.".

> > 2) or use another detection technique, which will address above
> > problem (
> > 
> >    your "To Be Filled By O.E.M." is not good; maybe looking at
> >    ACPI?)
> 
> That could also work, but I wouldn't be able to write such a patch as
> I no longer have access to the device.

Then we need acpi DSDT dumps from those machines... And check if acpi 
based enumeration is more useful or not...

> Anyway, in summary; the ideal solution seems to be DMI matching +
> extended detection sequence - this patch is the first part.

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] Input: byd - use DMI detection

[Richard Pospesel]

> How is windows detecting presence of BYD touchpads? Should we use
> something similar? Or are not there some (public) documentation about
> it?

Windows doesn't detect it correctly without BYD's kernel driver installed.

The only publicly available info on these can be found here:

http://bydit.com/doce/products/microelectronics/2474.html

Iv'e already tried getting mroe info from BYD, all they could find was a PDF
which enumerated the gesture types and the packet id used for each (byte 4).
No info there regarding protocol, etc.

best,
-Richard

Re: [PATCH] Input: byd - use DMI detection

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 1272 bytes --]

On Saturday 12 November 2016 18:33:57 Richard Pospesel wrote:
> > How is windows detecting presence of BYD touchpads? Should we use
> > something similar? Or are not there some (public) documentation
> > about it?
> 
> Windows doesn't detect it correctly without BYD's kernel driver
> installed.

Yes, that is expected. What is needed to to find out how that BYD's 
kernel driver detect if device is normal PS/2 mouse connected to 
ordinary PS/2 port, or if it is PS/2 BYD touched.

Microsoft has only preinstalled genetic PS/2 driver, so touchpad is 
probably working as ordinary mouse (if that BYD's kernel driver is not 
installed).

> The only publicly available info on these can be found here:
> 
> http://bydit.com/doce/products/microelectronics/2474.html

Webserver is really slow... HTML page is still loading...

> Iv'e already tried getting mroe info from BYD, all they could find
> was a PDF which enumerated the gesture types and the packet id used
> for each (byte 4). No info there regarding protocol, etc.

I understand... it is hard to get some information...

One option is to trying to ask BYD again, another is to try reverse 
engineer or sniff data exchange from windows driver.

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] Input: byd - use DMI detection

[Richard Pospesel]

> I understand... it is hard to get some information...
>
> One option is to trying to ask BYD again, another is to try reverse
> engineer or sniff data exchange from windows driver.

That's how we've gotten this far with the linux driver, by
intercepting and reverse engineering the protocol using the
Windows driver.

Re: [PATCH] Input: byd - use DMI detection

[Dmitry Torokhov]

On Fri, Nov 11, 2016 at 11:57:59PM +0000, Chris Diamand wrote:
> Because the BYD touchpad uses standard PS/2 commands for its
> detection sequence, some other models are incorrectly detected as BYD
> touchpads. This causes chaos when byd_init() later fails.
> 
> To fix this, init() and detect() should be merged. However, this
> would slow down detection for other mouse models. Instead, add a DMI
> check before attempting touchpad detection.
> 
> Signed-off-by: Chris Diamand <chris@diamand.org>
> ---
> Hi all,
> 
> This patch should fix the mis-detection of some mouse models as BYD touchpads,
> as mentioned in a few bug reports and other threads.
> 
> However, I no longer have a machine with a BYD touchpad (although I did record
> its DMI data), so this is mostly untested - could anyone *with* a BYD touchpad
> please try this patch to check their touchpad still works, and could anyone
> with a misdetected non-BYD touchpad please check that this fixes the
> misdetection?
> 
> Also, the DMI fields really are pretty much all like that ("To Be Filled By
> O.E.M.", etc). I've tried to choose the most specific ones, but it's still
> fairly arbitrary...

No, unfortunately this will trigger on many more whitebox devices.

> 
> Cheers!
> Chris
> 
>  drivers/input/mouse/byd.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/input/mouse/byd.c b/drivers/input/mouse/byd.c
> index b27aa63..b4c5fd0 100644
> --- a/drivers/input/mouse/byd.c
> +++ b/drivers/input/mouse/byd.c
> @@ -13,6 +13,7 @@
>   */
>  
>  #include <linux/delay.h>
> +#include <linux/dmi.h>
>  #include <linux/input.h>
>  #include <linux/libps2.h>
>  #include <linux/serio.h>
> @@ -235,6 +236,17 @@ struct byd_data {
>  	bool touch;
>  };
>  
> +static const struct dmi_system_id byd_dmi_table[] = {
> +	{
> +		.matches = {
> +			DMI_MATCH(DMI_PRODUCT_NAME, "SharkBay Platform"),
> +			DMI_MATCH(DMI_PRODUCT_SERIAL, "System Serial Number"),
> +			DMI_MATCH(DMI_BOARD_NAME, "WhiteTip Mountain1 Fab2"),
> +			DMI_MATCH(DMI_CHASSIS_VENDOR, "To Be Filled By O.E.M."),
> +		},
> +	},
> +};
> +
>  static void byd_report_input(struct psmouse *psmouse)
>  {
>  	struct byd_data *priv = psmouse->private;
> @@ -439,6 +451,9 @@ int byd_detect(struct psmouse *psmouse, bool set_properties)
>  	struct ps2dev *ps2dev = &psmouse->ps2dev;
>  	u8 param[4] = {0x03, 0x00, 0x00, 0x00};
>  
> +	if (!dmi_check_system(byd_dmi_table))
> +		return -1;
> +
>  	if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES))
>  		return -1;
>  	if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES))
> -- 
> 2.10.1
> 

-- 
Dmitry