| Message ID | 20161111235759.11988-1-chris@diamand.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
-----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----- -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
> 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. -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 >
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))
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(+)