Re: add support for crc_enabled Elantech v3 touchpads

2015-04-02 Thread Fasse
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

2015-04-02 Thread Theo de Raadt
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

2015-04-02 Thread Mark Kettenis
 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

2015-04-02 Thread Ulf Brosziewski

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

2015-04-02 Thread Martin Pieuchot
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 :)