| Message ID | 1467172878-25483-1-git-send-email-pospeselr@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Richard, Thanks for working on this! Patch looks mostly good, and seems to have worked judging by the comments on the bug report. Not many comments from me as it's just moving stuff around. I think it might be better to keep detect() and init() separate - it seems to me that a detect() function shouldn't really have any side effects (even if we're certain it's a BYD touchpad), so doing kzalloc and setting up the timer in detect() feels a bit wrong. But as mentioned earlier, we shouldn't do the touchpad initialisation twice. So maybe do the magic sequence stuff in detect(), and the actual driver init in init(). > Input: byd - fix issue where generic PS/2 mice are... You may need a shorter commit title. > The secret handshake used was not sufficient to determine whether the connected > device was actually a BYD touchpad. Added some restrictions on what the first > byte returned may be (based off of experiments with BYD touchapd). Moved > subsequent initialization logic from byd_init to tail of byd_detect, and > removed byd_init function. > > Fixes bug 1201781. Tested on laptop with BYD touchpad hardware. > > Applied against commit fcd6eb50eadd83f857eac55f99316f1789707cdb Probably don't need this in the commit message. > > Signed-off-by: Richard Pospesel <pospeselr@gmail.com> > > --- > diff --git a/drivers/input/mouse/byd.c b/drivers/input/mouse/byd.c > index ec73f75..92f5556 100644 > --- a/drivers/input/mouse/byd.c > +++ b/drivers/input/mouse/byd.c > @@ -2,6 +2,10 @@ > * BYD TouchPad PS/2 mouse driver > * > * Copyright (C) 2015 Chris Diamand <chris@diamand.org> > + * Copyright (C) 2015 Richard Pospesel > + * Copyright (C) 2015 Tai Chi Minh Ralph Eastwood > + * Copyright (C) 2015 Martin Wimpress > + * Copyright (C) 2015 Jay Kuri I already updated these in 82aaa086019ce0e6fcd3a44c0a2e4329afe988b6, so no need to include it here. (I think there's a disparity between input-next and mainline). > * > * This program is free software; you can redistribute it and/or modify it > * under the terms of the GNU General Public License version 2 as published by > @@ -355,7 +359,7 @@ static int byd_reset_touchpad(struct psmouse *psmouse) > { PSMOUSE_CMD_ENABLE, 0 }, > /* > * BYD-specific initialization, which enables absolute mode and > - * (if desired), the touchpad's built-in gesture detection. > + * disables the builtin hardware gesture recogniton. > */ > { 0x10E2, 0x00 }, > { 0x10E0, 0x02 }, > @@ -435,6 +439,10 @@ int byd_detect(struct psmouse *psmouse, bool set_properties) > struct ps2dev *ps2dev = &psmouse->ps2dev; > u8 param[4] = {0x03, 0x00, 0x00, 0x00}; > > + if (psmouse_reset(psmouse)) > + return -EIO; > + > + /* 'Secret' handshake */ > if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES)) > return -1; > if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES)) > @@ -446,62 +454,68 @@ int byd_detect(struct psmouse *psmouse, bool set_properties) > if (ps2_command(ps2dev, param, PSMOUSE_CMD_GETINFO)) > return -1; > > - if (param[1] != 0x03 || param[2] != 0x64) > + /* > + * BYD touchpad returns 0x03 for resolution ( 8 count / mm ) and > + * 0x64 ( 100 samples / sec ) for sampling rate Spaces inside brackets? > + * The first byte's value is dependent on the mouse button states: > + * 0 : no button pressed > + * 1 : right button pressed > + * 4 : left button pressed > + * 5 : right and left button pressed > + */ > + if ((param[0] & 0x05) != param[0] || Neat. Took me a while though, would something like this be clearer? param[0] & ~(0x00 | 0x01 | 0x04 | 0x05) Nothing to say on the rest. 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 Mon, Jul 11, 2016 at 09:26:20PM +0100, Chris Diamand wrote: > Hi Richard, > > Thanks for working on this! > > Patch looks mostly good, and seems to have worked judging by the comments on > the bug report. Not many comments from me as it's just moving stuff around. > > I think it might be better to keep detect() and init() separate - it seems to > me that a detect() function shouldn't really have any side effects (even if > we're certain it's a BYD touchpad), so doing kzalloc and setting up the timer > in detect() feels a bit wrong. Right. > > But as mentioned earlier, we shouldn't do the touchpad initialisation twice. > So maybe do the magic sequence stuff in detect(), and the actual driver init > in init(). Since we can't transparently detect the device without changing its state everything should go into byd_init() and we should remove byd_detect() (unless we decide to do some DMI matching). > > > Input: byd - fix issue where generic PS/2 mice are... > > You may need a shorter commit title. > > > The secret handshake used was not sufficient to determine whether the connected > > device was actually a BYD touchpad. Added some restrictions on what the first > > byte returned may be (based off of experiments with BYD touchapd). Moved > > subsequent initialization logic from byd_init to tail of byd_detect, and > > removed byd_init function. > > > > Fixes bug 1201781. Tested on laptop with BYD touchpad hardware. > > > > Applied against commit fcd6eb50eadd83f857eac55f99316f1789707cdb > > Probably don't need this in the commit message. > > > > > Signed-off-by: Richard Pospesel <pospeselr@gmail.com> > > > > --- > > diff --git a/drivers/input/mouse/byd.c b/drivers/input/mouse/byd.c > > index ec73f75..92f5556 100644 > > --- a/drivers/input/mouse/byd.c > > +++ b/drivers/input/mouse/byd.c > > @@ -2,6 +2,10 @@ > > * BYD TouchPad PS/2 mouse driver > > * > > * Copyright (C) 2015 Chris Diamand <chris@diamand.org> > > + * Copyright (C) 2015 Richard Pospesel > > + * Copyright (C) 2015 Tai Chi Minh Ralph Eastwood > > + * Copyright (C) 2015 Martin Wimpress > > + * Copyright (C) 2015 Jay Kuri > > I already updated these in 82aaa086019ce0e6fcd3a44c0a2e4329afe988b6, so no > need to include it here. (I think there's a disparity between input-next and > mainline). I have 2 branches (for-linus and next), and they may have contain different changes (as I re-sync with mainline as needed), not all the time. > > > * > > * This program is free software; you can redistribute it and/or modify it > > * under the terms of the GNU General Public License version 2 as published by > > @@ -355,7 +359,7 @@ static int byd_reset_touchpad(struct psmouse *psmouse) > > { PSMOUSE_CMD_ENABLE, 0 }, > > /* > > * BYD-specific initialization, which enables absolute mode and > > - * (if desired), the touchpad's built-in gesture detection. > > + * disables the builtin hardware gesture recogniton. > > */ > > { 0x10E2, 0x00 }, > > { 0x10E0, 0x02 }, > > @@ -435,6 +439,10 @@ int byd_detect(struct psmouse *psmouse, bool set_properties) > > struct ps2dev *ps2dev = &psmouse->ps2dev; > > u8 param[4] = {0x03, 0x00, 0x00, 0x00}; > > > > + if (psmouse_reset(psmouse)) > > + return -EIO; I would rather we reset the device after the first magic handshake to speed up case when it is not byd device. Hmm... You know, reset is quite slow operation and given how unreliable the first handshake is I really lean towards asking to add DMI matching into the driver. Because we just penalized everyone else with "standard" mice here. > > + > > + /* 'Secret' handshake */ > > if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES)) > > return -1; > > if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES)) > > @@ -446,62 +454,68 @@ int byd_detect(struct psmouse *psmouse, bool set_properties) > > if (ps2_command(ps2dev, param, PSMOUSE_CMD_GETINFO)) > > return -1; > > > > - if (param[1] != 0x03 || param[2] != 0x64) > > + /* > > + * BYD touchpad returns 0x03 for resolution ( 8 count / mm ) and > > + * 0x64 ( 100 samples / sec ) for sampling rate > > Spaces inside brackets? > > > + * The first byte's value is dependent on the mouse button states: > > + * 0 : no button pressed > > + * 1 : right button pressed > > + * 4 : left button pressed > > + * 5 : right and left button pressed > > + */ > > + if ((param[0] & 0x05) != param[0] || > > Neat. Took me a while though, would something like this be clearer? > > param[0] & ~(0x00 | 0x01 | 0x04 | 0x05) Hmm... Either "param[0] & ~0x05" or "param[0] & ~(BIT(0) | BIT(2))" or leave as Richard had it. Thanks.
On Mon, Jul 11, 2016 at 07:57:57PM -0700, Dmitry Torokhov wrote: > On Mon, Jul 11, 2016 at 09:26:20PM +0100, Chris Diamand wrote: > > Hi Richard, > > > > Thanks for working on this! > > > > Patch looks mostly good, and seems to have worked judging by the comments on > > the bug report. Not many comments from me as it's just moving stuff around. > > > > I think it might be better to keep detect() and init() separate - it seems to > > me that a detect() function shouldn't really have any side effects (even if > > we're certain it's a BYD touchpad), so doing kzalloc and setting up the timer > > in detect() feels a bit wrong. > > Right. > > > > > But as mentioned earlier, we shouldn't do the touchpad initialisation twice. > > So maybe do the magic sequence stuff in detect(), and the actual driver init > > in init(). > > Since we can't transparently detect the device without changing its > state everything should go into byd_init() and we should remove > byd_detect() (unless we decide to do some DMI matching). > > > > > > Input: byd - fix issue where generic PS/2 mice are... > > > > You may need a shorter commit title. > > > > > The secret handshake used was not sufficient to determine whether the connected > > > device was actually a BYD touchpad. Added some restrictions on what the first > > > byte returned may be (based off of experiments with BYD touchapd). Moved > > > subsequent initialization logic from byd_init to tail of byd_detect, and > > > removed byd_init function. > > > > > > Fixes bug 1201781. Tested on laptop with BYD touchpad hardware. > > > > > > Applied against commit fcd6eb50eadd83f857eac55f99316f1789707cdb > > > > Probably don't need this in the commit message. > > > > > > > > Signed-off-by: Richard Pospesel <pospeselr@gmail.com> > > > > > > --- > > > diff --git a/drivers/input/mouse/byd.c b/drivers/input/mouse/byd.c > > > index ec73f75..92f5556 100644 > > > --- a/drivers/input/mouse/byd.c > > > +++ b/drivers/input/mouse/byd.c > > > @@ -2,6 +2,10 @@ > > > * BYD TouchPad PS/2 mouse driver > > > * > > > * Copyright (C) 2015 Chris Diamand <chris@diamand.org> > > > + * Copyright (C) 2015 Richard Pospesel > > > + * Copyright (C) 2015 Tai Chi Minh Ralph Eastwood > > > + * Copyright (C) 2015 Martin Wimpress > > > + * Copyright (C) 2015 Jay Kuri > > > > I already updated these in 82aaa086019ce0e6fcd3a44c0a2e4329afe988b6, so no > > need to include it here. (I think there's a disparity between input-next and > > mainline). > > I have 2 branches (for-linus and next), and they may have contain > different changes (as I re-sync with mainline as needed), not all the > time. > > > > > > * > > > * This program is free software; you can redistribute it and/or modify it > > > * under the terms of the GNU General Public License version 2 as published by > > > @@ -355,7 +359,7 @@ static int byd_reset_touchpad(struct psmouse *psmouse) > > > { PSMOUSE_CMD_ENABLE, 0 }, > > > /* > > > * BYD-specific initialization, which enables absolute mode and > > > - * (if desired), the touchpad's built-in gesture detection. > > > + * disables the builtin hardware gesture recogniton. > > > */ > > > { 0x10E2, 0x00 }, > > > { 0x10E0, 0x02 }, > > > @@ -435,6 +439,10 @@ int byd_detect(struct psmouse *psmouse, bool set_properties) > > > struct ps2dev *ps2dev = &psmouse->ps2dev; > > > u8 param[4] = {0x03, 0x00, 0x00, 0x00}; > > > > > > + if (psmouse_reset(psmouse)) > > > + return -EIO; > > I would rather we reset the device after the first magic handshake to > speed up case when it is not byd device. Hmm... You know, reset is quite > slow operation and given how unreliable the first handshake is I really > lean towards asking to add DMI matching into the driver. Because we just > penalized everyone else with "standard" mice here. Chris, Richard, do you guys have the same hardware or different laptops? Could you share DMI data? > > > > + > > > + /* 'Secret' handshake */ > > > if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES)) > > > return -1; > > > if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES)) > > > @@ -446,62 +454,68 @@ int byd_detect(struct psmouse *psmouse, bool set_properties) > > > if (ps2_command(ps2dev, param, PSMOUSE_CMD_GETINFO)) > > > return -1; > > > > > > - if (param[1] != 0x03 || param[2] != 0x64) > > > + /* > > > + * BYD touchpad returns 0x03 for resolution ( 8 count / mm ) and > > > + * 0x64 ( 100 samples / sec ) for sampling rate > > > > Spaces inside brackets? > > > > > + * The first byte's value is dependent on the mouse button states: > > > + * 0 : no button pressed > > > + * 1 : right button pressed > > > + * 4 : left button pressed > > > + * 5 : right and left button pressed > > > + */ > > > + if ((param[0] & 0x05) != param[0] || > > > > Neat. Took me a while though, would something like this be clearer? > > > > param[0] & ~(0x00 | 0x01 | 0x04 | 0x05) > > Hmm... Either "param[0] & ~0x05" or "param[0] & ~(BIT(0) | BIT(2))" or > leave as Richard had it. > > Thanks. > > -- > Dmitry
Hi, Sorry for the delay. >> Chris, Richard, do you guys have the same hardware or different laptops? >> Could you share DMI data? > Not sure, I've a Purism Librem13. Mines's branded as a PC Specialist "Lafite" v1. I'm pretty sure they're from the same OEM though, being sold under different brands, given that they look pretty much identical. Full output of `dmidecode` attached, but I've included some highlights below: Handle 0x0003, DMI type 3, 25 bytes Chassis Information Manufacturer: To Be Filled By O.E.M. Type: Laptop Lock: Not Present Version: To Be Filled By O.E.M. Serial Number: To Be Filled By O.E.M. Asset Tag: To Be Filled By O.E.M. Boot-up State: Safe Power Supply State: Safe Thermal State: Safe Security Status: None OEM Information: 0x00000000 Height: Unspecified Number Of Power Cords: 1 Contained Elements: 1 <OUT OF SPEC> (0) SKU Number: To be filled by O.E.M. ... Handle 0x0004, DMI type 8, 9 bytes Port Connector Information Internal Reference Designator: J1A1 Internal Connector Type: None External Reference Designator: PS2Mouse External Connector Type: PS/2 Port Type: Mouse Port Cheers! Chris # dmidecode 3.0 Getting SMBIOS data from sysfs. SMBIOS 2.8 present. 81 structures occupying 3412 bytes. Table at 0xACEED000. Handle 0x0000, DMI type 0, 24 bytes BIOS Information Vendor: American Megatrends Inc. Version: 5.6.5 Release Date: 06/30/2015 Address: 0xF0000 Runtime Size: 64 kB ROM Size: 6144 kB Characteristics: PCI is supported BIOS is upgradeable BIOS shadowing is allowed Boot from CD is supported Selectable boot is supported BIOS ROM is socketed EDD is supported 5.25"/1.2 MB floppy services are supported (int 13h) 3.5"/720 kB floppy services are supported (int 13h) 3.5"/2.88 MB floppy services are supported (int 13h) Print screen service is supported (int 5h) 8042 keyboard services are supported (int 9h) Serial services are supported (int 14h) Printer services are supported (int 17h) ACPI is supported USB legacy is supported BIOS boot specification is supported Targeted content distribution is supported UEFI is supported BIOS Revision: 5.6 Handle 0x0001, DMI type 1, 27 bytes System Information Manufacturer: Intel Corporation Product Name: SharkBay Platform Version: 0.1 Serial Number: System Serial Number UUID: 03000200-0400-0500-0006-000700080009 Wake-up Type: Power Switch SKU Number: System SKUNumber Family: SharkBay System Handle 0x0002, DMI type 2, 15 bytes Base Board Information Manufacturer: Topstar Product Name: WhiteTip Mountain1 Fab2 Version: Fab2 Serial Number: 1 Asset Tag: Base Board Asset Tag Features: Board is a hosting board Board is replaceable Location In Chassis: Part Component Chassis Handle: 0x0003 Type: Motherboard Contained Object Handles: 0 Handle 0x0003, DMI type 3, 25 bytes Chassis Information Manufacturer: To Be Filled By O.E.M. Type: Laptop Lock: Not Present Version: To Be Filled By O.E.M. Serial Number: To Be Filled By O.E.M. Asset Tag: To Be Filled By O.E.M. Boot-up State: Safe Power Supply State: Safe Thermal State: Safe Security Status: None OEM Information: 0x00000000 Height: Unspecified Number Of Power Cords: 1 Contained Elements: 1 <OUT OF SPEC> (0) SKU Number: To be filled by O.E.M. Handle 0x0004, DMI type 8, 9 bytes Port Connector Information Internal Reference Designator: J1A1 Internal Connector Type: None External Reference Designator: PS2Mouse External Connector Type: PS/2 Port Type: Mouse Port Handle 0x0005, DMI type 8, 9 bytes Port Connector Information Internal Reference Designator: J1A1 Internal Connector Type: None External Reference Designator: Keyboard External Connector Type: PS/2 Port Type: Keyboard Port Handle 0x0006, DMI type 8, 9 bytes Port Connector Information Internal Reference Designator: J2A1 Internal Connector Type: None External Reference Designator: TV Out External Connector Type: Mini Centronics Type-14 Port Type: Other Handle 0x0007, DMI type 8, 9 bytes Port Connector Information Internal Reference Designator: J2A2A Internal Connector Type: None External Reference Designator: COM A External Connector Type: DB-9 male Port Type: Serial Port 16550A Compatible Handle 0x0008, DMI type 8, 9 bytes Port Connector Information Internal Reference Designator: J2A2B Internal Connector Type: None External Reference Designator: Video External Connector Type: DB-15 female Port Type: Video Port Handle 0x0009, DMI type 8, 9 bytes Port Connector Information Internal Reference Designator: J3A1 Internal Connector Type: None External Reference Designator: USB1 External Connector Type: Access Bus (USB) Port Type: USB Handle 0x000A, DMI type 8, 9 bytes Port Connector Information Internal Reference Designator: J3A1 Internal Connector Type: None External Reference Designator: USB2 External Connector Type: Access Bus (USB) Port Type: USB Handle 0x000B, DMI type 8, 9 bytes Port Connector Information Internal Reference Designator: J3A1 Internal Connector Type: None External Reference Designator: USB3 External Connector Type: Access Bus (USB) Port Type: USB Handle 0x000C, DMI type 8, 9 bytes Port Connector Information Internal Reference Designator: J9A1 - TPM HDR Internal Connector Type: Other External Reference Designator: Not Specified External Connector Type: None Port Type: Other Handle 0x000D, DMI type 8, 9 bytes Port Connector Information Internal Reference Designator: J9C1 - PCIE DOCKING CONN Internal Connector Type: Other External Reference Designator: Not Specified External Connector Type: None Port Type: Other Handle 0x000E, DMI type 8, 9 bytes Port Connector Information Internal Reference Designator: J2B3 - CPU FAN Internal Connector Type: Other External Reference Designator: Not Specified External Connector Type: None Port Type: Other Handle 0x000F, DMI type 8, 9 bytes Port Connector Information Internal Reference Designator: J6C2 - EXT HDMI Internal Connector Type: Other External Reference Designator: Not Specified External Connector Type: None Port Type: Other Handle 0x0010, DMI type 8, 9 bytes Port Connector Information Internal Reference Designator: J3C1 - GMCH FAN Internal Connector Type: Other External Reference Designator: Not Specified External Connector Type: None Port Type: Other Handle 0x0011, DMI type 8, 9 bytes Port Connector Information Internal Reference Designator: J1D1 - ITP Internal Connector Type: Other External Reference Designator: Not Specified External Connector Type: None Port Type: Other Handle 0x0012, DMI type 8, 9 bytes Port Connector Information Internal Reference Designator: J9E2 - MDC INTPSR Internal Connector Type: Other External Reference Designator: Not Specified External Connector Type: None Port Type: Other Handle 0x0013, DMI type 8, 9 bytes Port Connector Information Internal Reference Designator: J9E4 - MDC INTPSR Internal Connector Type: Other External Reference Designator: Not Specified External Connector Type: None Port Type: Other Handle 0x0014, DMI type 8, 9 bytes Port Connector Information Internal Reference Designator: J9E3 - LPC HOT DOCKING Internal Connector Type: Other External Reference Designator: Not Specified External Connector Type: None Port Type: Other Handle 0x0015, DMI type 8, 9 bytes Port Connector Information Internal Reference Designator: J9E1 - SCAN MATRIX Internal Connector Type: Other External Reference Designator: Not Specified External Connector Type: None Port Type: Other Handle 0x0016, DMI type 8, 9 bytes Port Connector Information Internal Reference Designator: J9G1 - LPC SIDE BAND Internal Connector Type: Other External Reference Designator: Not Specified External Connector Type: None Port Type: Other Handle 0x0017, DMI type 8, 9 bytes Port Connector Information Internal Reference Designator: J8F1 - UNIFIED Internal Connector Type: Other External Reference Designator: Not Specified External Connector Type: None Port Type: Other Handle 0x0018, DMI type 8, 9 bytes Port Connector Information Internal Reference Designator: J6F1 - LVDS Internal Connector Type: Other External Reference Designator: Not Specified External Connector Type: None Port Type: Other Handle 0x0019, DMI type 8, 9 bytes Port Connector Information Internal Reference Designator: J2F1 - LAI FAN Internal Connector Type: Other External Reference Designator: Not Specified External Connector Type: None Port Type: Other Handle 0x001A, DMI type 8, 9 bytes Port Connector Information Internal Reference Designator: J2G1 - GFX VID Internal Connector Type: Other External Reference Designator: Not Specified External Connector Type: None Port Type: Other Handle 0x001B, DMI type 8, 9 bytes Port Connector Information Internal Reference Designator: J1G6 - AC JACK Internal Connector Type: Other External Reference Designator: Not Specified External Connector Type: None Port Type: Other Handle 0x001C, DMI type 9, 17 bytes System Slot Information Designation: J6B2 Type: x16 PCI Express Current Usage: In Use Length: Long ID: 0 Characteristics: 3.3 V is provided Opening is shared PME signal is supported Bus Address: 0000:00:01.0 Handle 0x001D, DMI type 9, 17 bytes System Slot Information Designation: J6B1 Type: x1 PCI Express Current Usage: In Use Length: Short ID: 1 Characteristics: 3.3 V is provided Opening is shared PME signal is supported Bus Address: 0000:00:1c.3 Handle 0x001E, DMI type 9, 17 bytes System Slot Information Designation: J6D1 Type: x1 PCI Express Current Usage: In Use Length: Short ID: 2 Characteristics: 3.3 V is provided Opening is shared PME signal is supported Bus Address: 0000:00:1c.4 Handle 0x001F, DMI type 9, 17 bytes System Slot Information Designation: J7B1 Type: x1 PCI Express Current Usage: In Use Length: Short ID: 3 Characteristics: 3.3 V is provided Opening is shared PME signal is supported Bus Address: 0000:00:1c.5 Handle 0x0020, DMI type 9, 17 bytes System Slot Information Designation: J8B4 Type: x1 PCI Express Current Usage: In Use Length: Short ID: 4 Characteristics: 3.3 V is provided Opening is shared PME signal is supported Bus Address: 0000:00:1c.6 Handle 0x0021, DMI type 10, 6 bytes On Board Device Information Type: Video Status: Enabled Description: To Be Filled By O.E.M. Handle 0x0022, DMI type 11, 5 bytes OEM Strings String 1: System BIOS Version :109_ [06/30/2015] String 2: Embedded Controller Firmware Version :2.2U_ [05/30/2015] String 3: PCS Handle 0x0023, DMI type 12, 5 bytes System Configuration Options Option 1: None Handle 0x0024, DMI type 32, 20 bytes System Boot Information Status: No errors detected Handle 0x0025, DMI type 34, 11 bytes Management Device Description: LM78-1 Type: LM78 Address: 0x00000000 Address Type: I/O Port Handle 0x0026, DMI type 26, 22 bytes Voltage Probe Description: LM78A Location: <OUT OF SPEC> Status: <OUT OF SPEC> Maximum Value: Unknown Minimum Value: Unknown Resolution: Unknown Tolerance: Unknown Accuracy: Unknown OEM-specific Information: 0x00000000 Nominal Value: Unknown Handle 0x0027, DMI type 36, 16 bytes Management Device Threshold Data Lower Non-critical Threshold: 1 Upper Non-critical Threshold: 2 Lower Critical Threshold: 3 Upper Critical Threshold: 4 Lower Non-recoverable Threshold: 5 Upper Non-recoverable Threshold: 6 Handle 0x0028, DMI type 35, 11 bytes Management Device Component Description: None Management Device Handle: 0x0025 Component Handle: 0x0025 Threshold Handle: 0x0026 Handle 0x0029, DMI type 28, 22 bytes Temperature Probe Description: LM78A Location: <OUT OF SPEC> Status: <OUT OF SPEC> Maximum Value: Unknown Minimum Value: Unknown Resolution: Unknown Tolerance: Unknown Accuracy: Unknown OEM-specific Information: 0x00000000 Nominal Value: Unknown Handle 0x002A, DMI type 36, 16 bytes Management Device Threshold Data Lower Non-critical Threshold: 1 Upper Non-critical Threshold: 2 Lower Critical Threshold: 3 Upper Critical Threshold: 4 Lower Non-recoverable Threshold: 5 Upper Non-recoverable Threshold: 6 Handle 0x002B, DMI type 35, 11 bytes Management Device Component Description: None Management Device Handle: 0x0025 Component Handle: 0x0028 Threshold Handle: 0x0029 Handle 0x002C, DMI type 27, 15 bytes Cooling Device Temperature Probe Handle: 0x0029 Type: <OUT OF SPEC> Status: <OUT OF SPEC> Cooling Unit Group: 1 OEM-specific Information: 0x00000000 Nominal Speed: Unknown Or Non-rotating Description: Cooling Dev 1 Handle 0x002D, DMI type 36, 16 bytes Management Device Threshold Data Lower Non-critical Threshold: 1 Upper Non-critical Threshold: 2 Lower Critical Threshold: 3 Upper Critical Threshold: 4 Lower Non-recoverable Threshold: 5 Upper Non-recoverable Threshold: 6 Handle 0x002E, DMI type 35, 11 bytes Management Device Component Description: None Management Device Handle: 0x0025 Component Handle: 0x002B Threshold Handle: 0x002C Handle 0x002F, DMI type 27, 15 bytes Cooling Device Temperature Probe Handle: 0x0029 Type: <OUT OF SPEC> Status: <OUT OF SPEC> Cooling Unit Group: 1 OEM-specific Information: 0x00000000 Nominal Speed: Unknown Or Non-rotating Description: Not Specified Handle 0x0030, DMI type 36, 16 bytes Management Device Threshold Data Lower Non-critical Threshold: 1 Upper Non-critical Threshold: 2 Lower Critical Threshold: 3 Upper Critical Threshold: 4 Lower Non-recoverable Threshold: 5 Upper Non-recoverable Threshold: 6 Handle 0x0031, DMI type 35, 11 bytes Management Device Component Description: None Management Device Handle: 0x0025 Component Handle: 0x002E Threshold Handle: 0x002F Handle 0x0032, DMI type 29, 22 bytes Electrical Current Probe Description: ABC Location: <OUT OF SPEC> Status: <OUT OF SPEC> Maximum Value: Unknown Minimum Value: Unknown Resolution: Unknown Tolerance: Unknown Accuracy: Unknown OEM-specific Information: 0x00000000 Nominal Value: Unknown Handle 0x0033, DMI type 36, 16 bytes Management Device Threshold Data Handle 0x0034, DMI type 35, 11 bytes Management Device Component Description: None Management Device Handle: 0x0025 Component Handle: 0x0031 Threshold Handle: 0x002F Handle 0x0035, DMI type 26, 22 bytes Voltage Probe Description: LM78A Location: Power Unit Status: OK Maximum Value: Unknown Minimum Value: Unknown Resolution: Unknown Tolerance: Unknown Accuracy: Unknown OEM-specific Information: 0x00000000 Nominal Value: Unknown Handle 0x0036, DMI type 28, 22 bytes Temperature Probe Description: LM78A Location: Power Unit Status: OK Maximum Value: Unknown Minimum Value: Unknown Resolution: Unknown Tolerance: Unknown Accuracy: Unknown OEM-specific Information: 0x00000000 Nominal Value: Unknown Handle 0x0037, DMI type 27, 15 bytes Cooling Device Temperature Probe Handle: 0x0036 Type: Power Supply Fan Status: OK Cooling Unit Group: 1 OEM-specific Information: 0x00000000 Nominal Speed: Unknown Or Non-rotating Description: Cooling Dev 1 Handle 0x0038, DMI type 29, 22 bytes Electrical Current Probe Description: ABC Location: Power Unit Status: OK Maximum Value: Unknown Minimum Value: Unknown Resolution: Unknown Tolerance: Unknown Accuracy: Unknown OEM-specific Information: 0x00000000 Nominal Value: Unknown Handle 0x0039, DMI type 39, 22 bytes System Power Supply Power Unit Group: 1 Location: To Be Filled By O.E.M. Name: To Be Filled By O.E.M. Manufacturer: To Be Filled By O.E.M. Serial Number: To Be Filled By O.E.M. Asset Tag: To Be Filled By O.E.M. Model Part Number: To Be Filled By O.E.M. Revision: To Be Filled By O.E.M. Max Power Capacity: Unknown Status: Present, OK Type: Switching Input Voltage Range Switching: Auto-switch Plugged: Yes Hot Replaceable: No Input Voltage Probe Handle: 0x0035 Cooling Device Handle: 0x0037 Input Current Probe Handle: 0x0038 Handle 0x003A, DMI type 41, 11 bytes Onboard Device Reference Designation: Onboard IGD Type: Video Status: Enabled Type Instance: 1 Bus Address: 0000:00:02.0 Handle 0x003B, DMI type 41, 11 bytes Onboard Device Reference Designation: Onboard LAN Type: Ethernet Status: Enabled Type Instance: 1 Bus Address: 0000:00:19.0 Handle 0x003C, DMI type 41, 11 bytes Onboard Device Reference Designation: Onboard 1394 Type: Other Status: Enabled Type Instance: 1 Bus Address: 0000:03:1c.2 Handle 0x003D, DMI type 7, 19 bytes Cache Information Socket Designation: L1 Cache Configuration: Enabled, Not Socketed, Level 1 Operational Mode: Write Back Location: Internal Installed Size: 32 kB Maximum Size: 32 kB Supported SRAM Types: Synchronous Installed SRAM Type: Synchronous Speed: Unknown Error Correction Type: Parity System Type: Data Associativity: 8-way Set-associative Handle 0x003E, DMI type 7, 19 bytes Cache Information Socket Designation: L1 Cache Configuration: Enabled, Not Socketed, Level 1 Operational Mode: Write Back Location: Internal Installed Size: 32 kB Maximum Size: 32 kB Supported SRAM Types: Synchronous Installed SRAM Type: Synchronous Speed: Unknown Error Correction Type: Parity System Type: Instruction Associativity: 8-way Set-associative Handle 0x003F, DMI type 7, 19 bytes Cache Information Socket Designation: L2 Cache Configuration: Enabled, Not Socketed, Level 2 Operational Mode: Write Back Location: Internal Installed Size: 256 kB Maximum Size: 256 kB Supported SRAM Types: Synchronous Installed SRAM Type: Synchronous Speed: Unknown Error Correction Type: Single-bit ECC System Type: Unified Associativity: 8-way Set-associative Handle 0x0040, DMI type 7, 19 bytes Cache Information Socket Designation: L3 Cache Configuration: Enabled, Not Socketed, Level 3 Operational Mode: Write Back Location: Internal Installed Size: 3072 kB Maximum Size: 3072 kB Supported SRAM Types: Synchronous Installed SRAM Type: Synchronous Speed: Unknown Error Correction Type: Multi-bit ECC System Type: Unified Associativity: 12-way Set-associative Handle 0x0041, DMI type 4, 42 bytes Processor Information Socket Designation: SOCKET 0 Type: Central Processor Family: Core i5 Manufacturer: Intel(R) Corporation ID: D4 06 03 00 FF FB EB BF Signature: Type 0, Family 6, Model 61, Stepping 4 Flags: FPU (Floating-point unit on-chip) VME (Virtual mode extension) DE (Debugging extension) PSE (Page size extension) TSC (Time stamp counter) MSR (Model specific registers) PAE (Physical address extension) MCE (Machine check exception) CX8 (CMPXCHG8 instruction supported) APIC (On-chip APIC hardware supported) SEP (Fast system call) MTRR (Memory type range registers) PGE (Page global enable) MCA (Machine check architecture) CMOV (Conditional move instruction supported) PAT (Page attribute table) PSE-36 (36-bit page size extension) CLFSH (CLFLUSH instruction supported) DS (Debug store) ACPI (ACPI supported) MMX (MMX technology supported) FXSR (FXSAVE and FXSTOR instructions supported) SSE (Streaming SIMD extensions) SSE2 (Streaming SIMD extensions 2) SS (Self-snoop) HTT (Multi-threading) TM (Thermal monitor supported) PBE (Pending break enabled) Version: Intel(R) Core(TM) i5-5200U CPU @ 2.20GHz Voltage: 0.9 V External Clock: 100 MHz Max Speed: 2200 MHz Current Speed: 2500 MHz Status: Populated, Enabled Upgrade: Socket BGA1168 L1 Cache Handle: 0x003E L2 Cache Handle: 0x003F L3 Cache Handle: 0x0040 Serial Number: NULL Asset Tag: To Be Filled By O.E.M Part Number: To Be Filled By O.E.M Core Count: 2 Core Enabled: 2 Thread Count: 4 Characteristics: 64-bit capable Multi-Core Hardware Thread Execute Protection Enhanced Virtualization Power/Performance Control Handle 0x0042, DMI type 221, 12 bytes OEM-specific Type Header and Data: DD 0C 42 00 01 01 00 02 05 00 00 00 Strings: Reference Code - ACPI Handle 0x0043, DMI type 16, 23 bytes Physical Memory Array Location: System Board Or Motherboard Use: System Memory Error Correction Type: None Maximum Capacity: 16 GB Error Information Handle: Not Provided Number Of Devices: 2 Handle 0x0044, DMI type 17, 34 bytes Memory Device Array Handle: 0x0043 Error Information Handle: Not Provided Total Width: 64 bits Data Width: 64 bits Size: 8192 MB Form Factor: SODIMM Set: None Locator: ChannelA-DIMM0 Bank Locator: BANK 0 Type: DDR3 Type Detail: Synchronous Speed: 1600 MHz Manufacturer: Samsung Serial Number: 39A692E8 Asset Tag: 9876543210 Part Number: M471B1G73DM0-YK0 Rank: 2 Configured Clock Speed: 1600 MHz Handle 0x0045, DMI type 17, 34 bytes Memory Device Array Handle: 0x0043 Error Information Handle: Not Provided Total Width: Unknown Data Width: Unknown Size: No Module Installed Form Factor: DIMM Set: None Locator: ChannelB-DIMM0 Bank Locator: BANK 2 Type: Unknown Type Detail: None Speed: Unknown Manufacturer: Not Specified Serial Number: Not Specified Asset Tag: Not Specified Part Number: Not Specified Rank: Unknown Configured Clock Speed: Unknown Handle 0x0046, DMI type 19, 31 bytes Memory Array Mapped Address Starting Address: 0x00000000000 Ending Address: 0x001FFFFFFFF Range Size: 8 GB Physical Array Handle: 0x0043 Partition Width: 2 Handle 0x0047, DMI type 20, 35 bytes Memory Device Mapped Address Starting Address: 0x00000000000 Ending Address: 0x001FFFFFFFF Range Size: 8 GB Physical Device Handle: 0x0045 Memory Array Mapped Address Handle: 0x0046 Partition Row Position: Unknown Interleave Position: Unknown Interleaved Data Depth: Unknown Handle 0x0048, DMI type 221, 54 bytes OEM-specific Type Header and Data: DD 36 48 00 07 01 00 02 05 00 00 00 02 00 02 05 00 03 00 03 00 02 05 00 00 00 04 05 FF FF FF FF FF 06 00 FF FF FF 09 00 07 00 FF FF FF 09 00 08 00 FF FF FF 00 00 Strings: Reference Code - SA - System Agent Reference Code - MRC SA - PCIe Version SA-CRID Status Disabled SA-CRID Original Value SA-CRID New Value OPROM - VBIOS Handle 0x0049, DMI type 221, 26 bytes OEM-specific Type Header and Data: DD 1A 49 00 03 01 00 02 05 00 00 00 02 00 0A 00 00 07 00 03 04 0A 00 20 E8 03 Strings: Reference Code - ME 10.0 MEBx version ME Firmware Version 1.5MB SKU Handle 0x004A, DMI type 221, 68 bytes OEM-specific Type Header and Data: DD 44 4A 00 09 01 00 02 05 00 00 00 02 03 FF FF FF FF FF 04 00 FF FF FF 03 00 05 00 FF FF FF 03 00 06 00 FF FF FF FF FF 07 00 19 00 00 00 00 08 00 2C 00 00 00 00 09 00 03 00 00 00 00 0A 00 01 00 00 00 00 Strings: Reference Code - PCH - Lynxpoint PCH-CRID Status Disabled PCH-CRID Original Value PCH-CRID New Value OPROM - RST - RAID LPTLp Bx Hsio Version LPTH Cx Hsio Version PCH9S A0 Hsio Version WPTLp B0 Hsio Version Handle 0x004B, DMI type 221, 26 bytes OEM-specific Type Header and Data: DD 1A 4B 00 03 01 00 02 05 00 00 00 02 00 00 00 00 18 00 03 00 00 05 00 00 00 Strings: Reference Code - CPU uCode Version TXT ACM version Handle 0x004C, DMI type 136, 6 bytes OEM-specific Type Header and Data: 88 06 4C 00 00 00 Handle 0x004D, DMI type 13, 22 bytes BIOS Language Information Language Description Format: Long Installable Languages: 1 en|US|iso8859-1 Currently Installed Language: en|US|iso8859-1 Handle 0x004E, DMI type 131, 64 bytes OEM-specific Type Header and Data: 83 40 4E 00 31 00 00 00 00 00 00 00 00 00 00 00 F8 00 C3 9C 00 00 00 00 01 60 00 00 00 00 0A 00 E8 03 20 00 00 00 00 00 C8 00 FF FF 00 00 00 00 00 00 00 00 66 00 00 00 76 50 72 6F 00 00 00 00 Handle 0x004F, DMI type 14, 20 bytes Group Associations Name: Firmware Version Info Items: 5 0x0042 (<OUT OF SPEC>) 0x0048 (<OUT OF SPEC>) 0x0049 (<OUT OF SPEC>) 0x004A (<OUT OF SPEC>) 0x004B (<OUT OF SPEC>) Handle 0x0050, DMI type 127, 4 bytes End Of Table
Hi, Raising this thread in everyone's memory. We have people complaining[1,2] regarding these bad detections, and I think given that this is a regression, we should address this ASAP. Apologies for being pushy. Cheers, Benjamin [1] https://bugzilla.redhat.com/show_bug.cgi?id=1352159 [2] https://bugzilla.kernel.org/show_bug.cgi?id=120781 On Tue, Aug 2, 2016 at 1:32 PM, Chris Diamand <chris@diamand.org> wrote: > Hi, > > Sorry for the delay. > >>> Chris, Richard, do you guys have the same hardware or different laptops? >>> Could you share DMI data? > >> Not sure, I've a Purism Librem13. > > Mines's branded as a PC Specialist "Lafite" v1. > > I'm pretty sure they're from the same OEM though, being sold under different > brands, given that they look pretty much identical. > > Full output of `dmidecode` attached, but I've included some highlights below: > > Handle 0x0003, DMI type 3, 25 bytes > Chassis Information > Manufacturer: To Be Filled By O.E.M. > Type: Laptop > Lock: Not Present > Version: To Be Filled By O.E.M. > Serial Number: To Be Filled By O.E.M. > Asset Tag: To Be Filled By O.E.M. > Boot-up State: Safe > Power Supply State: Safe > Thermal State: Safe > Security Status: None > OEM Information: 0x00000000 > Height: Unspecified > Number Of Power Cords: 1 > Contained Elements: 1 > <OUT OF SPEC> (0) > SKU Number: To be filled by O.E.M. > > ... > > Handle 0x0004, DMI type 8, 9 bytes > Port Connector Information > Internal Reference Designator: J1A1 > Internal Connector Type: None > External Reference Designator: PS2Mouse > External Connector Type: PS/2 > Port Type: Mouse Port > > 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
diff --git a/drivers/input/mouse/byd.c b/drivers/input/mouse/byd.c index ec73f75..92f5556 100644 --- a/drivers/input/mouse/byd.c +++ b/drivers/input/mouse/byd.c @@ -2,6 +2,10 @@ * BYD TouchPad PS/2 mouse driver * * Copyright (C) 2015 Chris Diamand <chris@diamand.org> + * Copyright (C) 2015 Richard Pospesel + * Copyright (C) 2015 Tai Chi Minh Ralph Eastwood + * Copyright (C) 2015 Martin Wimpress + * Copyright (C) 2015 Jay Kuri * * This program is free software; you can redistribute it and/or modify it * under the terms of the GNU General Public License version 2 as published by @@ -355,7 +359,7 @@ static int byd_reset_touchpad(struct psmouse *psmouse) { PSMOUSE_CMD_ENABLE, 0 }, /* * BYD-specific initialization, which enables absolute mode and - * (if desired), the touchpad's built-in gesture detection. + * disables the builtin hardware gesture recogniton. */ { 0x10E2, 0x00 }, { 0x10E0, 0x02 }, @@ -435,6 +439,10 @@ int byd_detect(struct psmouse *psmouse, bool set_properties) struct ps2dev *ps2dev = &psmouse->ps2dev; u8 param[4] = {0x03, 0x00, 0x00, 0x00}; + if (psmouse_reset(psmouse)) + return -EIO; + + /* 'Secret' handshake */ if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES)) return -1; if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES)) @@ -446,62 +454,68 @@ int byd_detect(struct psmouse *psmouse, bool set_properties) if (ps2_command(ps2dev, param, PSMOUSE_CMD_GETINFO)) return -1; - if (param[1] != 0x03 || param[2] != 0x64) + /* + * BYD touchpad returns 0x03 for resolution ( 8 count / mm ) and + * 0x64 ( 100 samples / sec ) for sampling rate + * The first byte's value is dependent on the mouse button states: + * 0 : no button pressed + * 1 : right button pressed + * 4 : left button pressed + * 5 : right and left button pressed + */ + if ((param[0] & 0x05) != param[0] || + param[1] != 0x03 || + param[2] != 0x64) + return -ENODEV; + + /* Attempt to set BYD unique settings */ + if (byd_reset_touchpad(psmouse)) return -ENODEV; psmouse_dbg(psmouse, "BYD touchpad detected\n"); if (set_properties) { - psmouse->vendor = "BYD"; - psmouse->name = "TouchPad"; - } + struct input_dev *dev = psmouse->dev; + struct byd_data *priv; - return 0; -} + priv = kzalloc(sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; -int byd_init(struct psmouse *psmouse) -{ - struct input_dev *dev = psmouse->dev; - struct byd_data *priv; - - if (psmouse_reset(psmouse)) - return -EIO; - - if (byd_reset_touchpad(psmouse)) - return -EIO; + setup_timer( + &priv->timer, + byd_clear_touch, + (unsigned long) psmouse); - priv = kzalloc(sizeof(*priv), GFP_KERNEL); - if (!priv) - return -ENOMEM; - - setup_timer(&priv->timer, byd_clear_touch, (unsigned long) psmouse); - - psmouse->private = priv; - psmouse->disconnect = byd_disconnect; - psmouse->reconnect = byd_reconnect; - psmouse->protocol_handler = byd_process_byte; - psmouse->pktsize = 4; - psmouse->resync_time = 0; - - __set_bit(INPUT_PROP_POINTER, dev->propbit); - /* Touchpad */ - __set_bit(BTN_TOUCH, dev->keybit); - __set_bit(BTN_TOOL_FINGER, dev->keybit); - /* Buttons */ - __set_bit(BTN_LEFT, dev->keybit); - __set_bit(BTN_RIGHT, dev->keybit); - __clear_bit(BTN_MIDDLE, dev->keybit); - - /* Absolute position */ - __set_bit(EV_ABS, dev->evbit); - input_set_abs_params(dev, ABS_X, 0, BYD_PAD_WIDTH, 0, 0); - input_set_abs_params(dev, ABS_Y, 0, BYD_PAD_HEIGHT, 0, 0); - input_abs_set_res(dev, ABS_X, BYD_PAD_RESOLUTION); - input_abs_set_res(dev, ABS_Y, BYD_PAD_RESOLUTION); - /* No relative support */ - __clear_bit(EV_REL, dev->evbit); - __clear_bit(REL_X, dev->relbit); - __clear_bit(REL_Y, dev->relbit); + psmouse->private = priv; + psmouse->vendor = "BYD"; + psmouse->name = "TouchPad"; + psmouse->disconnect = byd_disconnect; + psmouse->reconnect = byd_reconnect; + psmouse->protocol_handler = byd_process_byte; + psmouse->pktsize = 4; + psmouse->resync_time = 0; + + __set_bit(INPUT_PROP_POINTER, dev->propbit); + /* Touchpad */ + __set_bit(BTN_TOUCH, dev->keybit); + __set_bit(BTN_TOOL_FINGER, dev->keybit); + /* Buttons */ + __set_bit(BTN_LEFT, dev->keybit); + __set_bit(BTN_RIGHT, dev->keybit); + __clear_bit(BTN_MIDDLE, dev->keybit); + + /* Absolute position */ + __set_bit(EV_ABS, dev->evbit); + input_set_abs_params(dev, ABS_X, 0, BYD_PAD_WIDTH, 0, 0); + input_set_abs_params(dev, ABS_Y, 0, BYD_PAD_HEIGHT, 0, 0); + input_abs_set_res(dev, ABS_X, BYD_PAD_RESOLUTION); + input_abs_set_res(dev, ABS_Y, BYD_PAD_RESOLUTION); + /* No relative support */ + __clear_bit(EV_REL, dev->evbit); + __clear_bit(REL_X, dev->relbit); + __clear_bit(REL_Y, dev->relbit); + } return 0; -} +} \ No newline at end of file diff --git a/drivers/input/mouse/byd.h b/drivers/input/mouse/byd.h index d6c120c..810d19e 100644 --- a/drivers/input/mouse/byd.h +++ b/drivers/input/mouse/byd.h @@ -3,16 +3,11 @@ #ifdef CONFIG_MOUSE_PS2_BYD int byd_detect(struct psmouse *psmouse, bool set_properties); -int byd_init(struct psmouse *psmouse); #else static inline int byd_detect(struct psmouse *psmouse, bool set_properties) { return -ENOSYS; } -static inline int byd_init(struct psmouse *psmouse) -{ - return -ENOSYS; -} #endif /* CONFIG_MOUSE_PS2_BYD */ #endif /* _BYD_H */ diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c index 5784e20..5750a10 100644 --- a/drivers/input/mouse/psmouse-base.c +++ b/drivers/input/mouse/psmouse-base.c @@ -849,7 +849,6 @@ static const struct psmouse_protocol psmouse_protocols[] = { .name = "BYDPS/2", .alias = "byd", .detect = byd_detect, - .init = byd_init, }, #endif {
The secret handshake used was not sufficient to determine whether the connected device was actually a BYD touchpad. Added some restrictions on what the first byte returned may be (based off of experiments with BYD touchapd). Moved subsequent initialization logic from byd_init to tail of byd_detect, and removed byd_init function. Fixes bug 1201781. Tested on laptop with BYD touchpad hardware. Applied against commit fcd6eb50eadd83f857eac55f99316f1789707cdb Signed-off-by: Richard Pospesel <pospeselr@gmail.com> --- -- 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