Re: add support for crc_enabled Elantech v3 touchpads
On Thu, 2 Apr 2015 18:51:38 +0200 Martin Pieuchot m...@openbsd.org wrote: Even if that's true, nothing prevent us to commit this diff first, as long as it does not introduce regression and then work on the possible refactoring needed to support debounce packets :) That is great to hear. Because the way pms_sync_elantech_v3 now handles crc mode pms_proc_elantech_v3 can't possibly receive a debounce packet. Therefore I moved the debounce packet check in pms_proc_elantech_v3 into the part that handles non-crc packets. Below is the full diff for anyone that wants to try out these changes. Index: sys/dev/pckbc/pms.c === RCS file: /cvs/src/sys/dev/pckbc/pms.c,v retrieving revision 1.58 diff -u -p -r1.58 pms.c --- sys/dev/pckbc/pms.c 26 Mar 2015 01:30:22 - 1.58 +++ sys/dev/pckbc/pms.c 2 Apr 2015 20:43:23 - @@ -137,6 +137,7 @@ struct elantech_softc { #define ELANTECH_F_HAS_ROCKER 0x02 #define ELANTECH_F_2FINGER_PACKET 0x04 #define ELANTECH_F_HW_V1_OLD 0x08 +#define ELANTECH_F_CRC_ENABLED 0x10 int fw_version; int min_x, min_y; @@ -1812,6 +1813,9 @@ elantech_get_hwinfo_v3(struct pms_softc elantech-fw_version = fw_version; elantech-flags |= ELANTECH_F_REPORTS_PRESSURE; + if ((fw_version 0x4000) == 0x4000) + elantech-flags |= ELANTECH_F_CRC_ENABLED; + if (elantech_set_absolute_mode_v3(sc)) return (-1); @@ -2164,14 +2168,23 @@ pms_sync_elantech_v2(struct pms_softc *s int pms_sync_elantech_v3(struct pms_softc *sc, int data) { + struct elantech_softc *elantech = sc-elantech; + switch (sc-inputstate) { case 0: + if (elantech-flags ELANTECH_F_CRC_ENABLED) + break; if ((data 0x0c) != 0x04 (data 0x0c) != 0x0c) return (-1); break; case 3: - if ((data 0xcf) != 0x02 (data 0xce) != 0x0c) - return (-1); + if (elantech-flags ELANTECH_F_CRC_ENABLED) { + if ((data 0x09) != 0x08 (data 0x09) != 0x09) + return (-1); + } else { + if ((data 0xcf) != 0x02 (data 0xce) != 0x0c) + return (-1); + } break; } @@ -2256,11 +2269,6 @@ pms_proc_elantech_v3(struct pms_softc *s struct elantech_softc *elantech = sc-elantech; int x, y, w, z; - /* The hardware sends this packet when in debounce state. -* The packet should be ignored. */ - if (!memcmp(sc-packet, debounce_pkt, sizeof(debounce_pkt))) - return; - x = ((sc-packet[1] 0x0f) 8 | sc-packet[2]); y = ((sc-packet[4] 0x0f) 8 | sc-packet[5]); z = 0; @@ -2271,10 +2279,19 @@ pms_proc_elantech_v3(struct pms_softc *s * and a tail packet. We report a single event and ignore * the tail packet. */ - if ((sc-packet[0] 0x0c) != 0x04 - (sc-packet[3] 0xcf) != 0x02) { - /* not the head packet -- ignore */ - return; + if (elantech-flags ELANTECH_F_CRC_ENABLED) { + if ((sc-packet[3] 0x09) != 0x08) + return; + } else { + /* The hardware sends this packet when in debounce state. +* The packet should be ignored. */ + if (!memcmp(sc-packet, debounce_pkt, sizeof(debounce_pkt))) + return; + if ((sc-packet[0] 0x0c) != 0x04 + (sc-packet[3] 0xcf) != 0x02) { + /* not the head packet -- ignore */ + return; + } } }
Re: add support for crc_enabled Elantech v3 touchpads
This pms driver is becoming a nightmare, full of messy code that only a few people understand all the secrets behind. Can someone out there put a serious effort into refactoring this so that there is a layer inside it, with sub-drivers for specific models? I say this, because I senes this problem will get worse soon.
Re: bounds checks in aml_rwgas
Date: Sat, 28 Mar 2015 23:27:30 +1000 From: Jonathan Matthew jonat...@d14n.org The diff below fixes a uvm fault I'm seeing when booting an MP kernel on a hp bc2500 blade, somewhere during acpi attach. SP kernels don't crash, but I think that's down to luck. It looks like this: ioapic0 at mainbus0: apid 2 pa 0xfec0, version 21, 24 pins acpimcfg0 at acpi0 addr 0xe000, bus 0-64 acpihpet0 at acpi0: 14318180 Hz uvm_fault(0x818c5860, 0x80063000, 0, 1) - e kernel: page fault trap, code=0 Stopped at memcpy+0xa: repe movsq (%rsi),%es:(%rdi) memcpy() at memcpy+0xa aml_rwfield() at aml_rwfield+0x205 aml_store() at aml_store+0x1eb aml_parse() at aml_parse+0xf4c aml_eval() at aml_eval+0x1c8 aml_parse() at aml_parse+0x183d aml_eval() at aml_eval+0x1c8 aml_evalnode() at aml_evalnode+0x74 acpi_inidev() at acpi_inidev+0x57 aml_find_node() at aml_find_node+0x92 end trace frame: 0x81a1eab0, count: 0 This turns out to be because aml_rwgas doesn't do bounds checking on source buffers. _SB.PCI0._INI (evaluated in acpi_inidev) tries to figure out what OS is running. The method it calls to do this creates a temporary buffer: Name (STR0, Buffer (0x50) {}) that it copies some lies into, like Microsoft Windows Vista, then copies that somewhere else: WMIB = STR0 /* \OSFG.STR0 */ where WMIB is: OperationRegion (HABS, SystemMemory, HBIO, HBSZ) Field (HABS, AnyAcc, NoLock, Preserve) { WMIB, 33280, which is a fair bit bigger than the STR0 buffer. aml_rwgas tries to read all 33280 bits from STR0 anyway, which obviously leads to crashes. I've tested the fix on several machines running amd64 and i386 and nothing breaks as far as I can tell. oks, more tests, etc.? ok kettenis@ Index: dsdt.c === RCS file: /cvs/src/sys/dev/acpi/dsdt.c,v retrieving revision 1.216 diff -u -p -u -p -r1.216 dsdt.c --- dsdt.c16 Mar 2015 20:31:46 - 1.216 +++ dsdt.c28 Mar 2015 11:57:07 - @@ -2286,6 +2286,9 @@ aml_rwgas(struct aml_value *rgn, int bpo } else { /* Write to a large field.. create or convert buffer */ val = aml_convert(val, AML_OBJTYPE_BUFFER, -1); + + if (blen (val-length 3)) + blen = val-length 3; } vbit = val-v_buffer; } else {
Re: add support for crc_enabled Elantech v3 touchpads
On 04/02/2015 03:39 AM, Fasse wrote: On Wed, 01 Apr 2015 21:23:15 +0200 Ulf Brosziewskiulf.brosziew...@t-online.de wrote: Yes, without some refactoring there won't be an elegant way. pms_sync_elantech_v2 encodes some sync state in the 'flags' field (ELANTECH_F_2FINGER_PACKET), but doing the same in the v3/CRC case might be ugly. Admittedly I am biased because I don't want to refactor ~2400 LOC to get my touchpad working but I don't think that crc enabled v3 touchpads use the debounce packet. I just installed Ubuntu and compiled the 3.19.3 linux kernel with added printk statements in the elantech_packet_check_v3 function on my laptop. In the linux kernel documentation [0] for elantech touchpads it says about the debounce packet: Note on debounce: In case the box has unstable power supply or other electricity issues, or when number of finger changes, F/W would send debounce packet to inform driver that the hardware is in debounce status. I could not reproduce the unstable power supply but when switching the number of fingers on the touchpad no debounce packet is issued. Instead just the head and tail packets are registered and processed (unlike the OpenBSD driver which ignores the tail packet). This leads me to belief that v3/crc does not use debounce packets. Do you think this is possible/likely? (...) Why not? You seem to have shown that it is possible, at least for your hardware and multiple touches. It might well be that the author(s) of the Linux driver just wanted to be on the safe side. (...) There doesn't appear to be any official documentation in regard to elantech touchpads, hence I can't be certain. Nonetheless a refactor would obviously be beneficial but in the interim the patch might do a sufficient job. [0] https://www.kernel.org/doc/Documentation/input/elantech.txt
Re: add support for crc_enabled Elantech v3 touchpads
On 02/04/15(Thu) 18:43, Ulf Brosziewski wrote: On 04/02/2015 03:39 AM, Fasse wrote: On Wed, 01 Apr 2015 21:23:15 +0200 Ulf Brosziewskiulf.brosziew...@t-online.de wrote: Yes, without some refactoring there won't be an elegant way. pms_sync_elantech_v2 encodes some sync state in the 'flags' field (ELANTECH_F_2FINGER_PACKET), but doing the same in the v3/CRC case might be ugly. Admittedly I am biased because I don't want to refactor ~2400 LOC to get my touchpad working but I don't think that crc enabled v3 touchpads use the debounce packet. I just installed Ubuntu and compiled the 3.19.3 linux kernel with added printk statements in the elantech_packet_check_v3 function on my laptop. In the linux kernel documentation [0] for elantech touchpads it says about the debounce packet: Note on debounce: In case the box has unstable power supply or other electricity issues, or when number of finger changes, F/W would send debounce packet to inform driver that the hardware is in debounce status. I could not reproduce the unstable power supply but when switching the number of fingers on the touchpad no debounce packet is issued. Instead just the head and tail packets are registered and processed (unlike the OpenBSD driver which ignores the tail packet). This leads me to belief that v3/crc does not use debounce packets. Do you think this is possible/likely? (...) Why not? You seem to have shown that it is possible, at least for your hardware and multiple touches. It might well be that the author(s) of the Linux driver just wanted to be on the safe side. Even if that's true, nothing prevent us to commit this diff first, as long as it does not introduce regression and then work on the possible refactoring needed to support debounce packets :)