Re: [PATCH] Input: goodix - preliminary support for GT801-2+1

[Priit Laes <plaes@plaes.org>]

On Sun, 2015-04-05 at 18:04 +0200, Bastien Nocera wrote:
> On Sun, 2015-04-05 at 00:08 +0300, Priit Laes wrote:
> > This patch implements GT801x2 touchscreen support.
> > Unfortunately, there is a big difference between GT801 and GT9xx 
> > series
> > chips, therefore some advice is needed on how to proceed.
> > 
> > Differences between GT801x2 and GT9xx series:
> > 
> > 1. I2C registers: 1 byte (GT801x2) vs 2 bytes (GT9xx)
> > 2. Different configuration layout and version info
> > 3. Different touch report protocol
> 
> That doesn't seem like an awful lot of differences. Approximately 80 
> line changes for 500 lines of driver code. You could add an enum for 
> the 8xx and 9xx types near the top, add that as driver data in the 
> match arrays (both the ACPI and OF ones). Then have if statements 
> choose the correct init, read and report functions.

How should I handle the version readout?

Currently the driver has following info in its registers (starting 
from 0xf0):

f0: 47 54 38 30 31 4e 49 5f 33 52 31 35 5f 31 41 56    GT801NI_3R15_1AV


And there's another issue with deactivating interrupts. When I remove 
the goodix module, I get following traceback:

[snip]
WARNING: CPU: 0 PID: 381 at fs/proc/generic.c:552 remove_proc_entry+0x138/0x16c()
remove_proc_entry: removing non-empty directory 'irq/50', leaking at least 'gt801x2'
Modules linked in: goodix(-) rtl8192cu rtl_usb rtl8192c_common rtlwifi
CPU: 0 PID: 381 Comm: rmmod Not tainted 4.0.0-rc1+ #22
Hardware name: Allwinner A1X (Device Tree)
[<c0014544>] (unwind_backtrace) from [<c00116e4>] (show_stack+0x10/0x14)
[<c00116e4>] (show_stack) from [<c042995c>] (dump_stack+0x84/0x94)
[<c042995c>] (dump_stack) from [<c0021140>] (warn_slowpath_common+0x80/0xb0)
[<c0021140>] (warn_slowpath_common) from [<c00211a0>] (warn_slowpath_fmt+0x30/0x40)
[<c00211a0>] (warn_slowpath_fmt) from [<c0124474>] (remove_proc_entry+0x138/0x16c)
[<c0124474>] (remove_proc_entry) from [<c0060358>] (unregister_irq_proc+0xa8/0xb0)
[<c0060358>] (unregister_irq_proc) from [<c00595d0>] (free_desc+0x30/0x60)
[<c00595d0>] (free_desc) from [<c0059648>] (irq_free_descs+0x48/0x80)
[<c0059648>] (irq_free_descs) from [<c02da850>] (i2c_device_remove+0x64/0x80)
[<c02da850>] (i2c_device_remove) from [<c025af5c>] (__device_release_driver+0x70/0xc4)
[<c025af5c>] (__device_release_driver) from [<c025b68c>] (driver_detach+0xcc/0xd0)
[<c025b68c>] (driver_detach) from [<c025acbc>] (bus_remove_driver+0x4c/0xa0)
[<c025acbc>] (bus_remove_driver) from [<c007d170>] (SyS_delete_module+0x16c/0x1b8)
[<c007d170>] (SyS_delete_module) from [<c000e4a0>] (ret_fast_syscall+0x0/0x34)
[/snip]

Devicetree node contains following:

[snip]
&i2c2 {
    pinctrl-names = "default";
    pinctrl-0 = <&i2c2_pins_a>;
    status = "okay";

    /* Touchscreen */
    touchscreen@55 {
        compatible = "goodix,gt801x2";
        reg = <0x55>;

        interrupt-parent = <&pio>;
        interrupts = <21 IRQ_TYPE_EDGE_FALLING>; /* EINT21 (PH21) */
   };
};
[/snip]

Päikest,
Priit :)
--
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