2015-12-15 22:21 keltezéssel, Dmitry Torokhov írta: > Hi Zoltán, > > On Tue, Dec 15, 2015 at 12:22:07PM +0100, Böszörményi Zoltán wrote: >> From: Böszörményi Zoltán <zboszor@xxxxx> >> >> There are two EETI touchscreen drivers in the kernel (eeti_ts and egalax_ts) >> but both are for I2C-connected panels. This is for a different, serial >> and not multi-touch touchscreen panel. The protocol documentation is at >> http://www.eeti.com.tw/pdf/Software%20Programming%20Guide_v2.0.pdf >> >> Signed-off-by: Böszörményi Zoltán <zboszor@xxxxx> >> > Thank you for your patch, it looks pretty good, just a few comments > below. > >> +config TOUCHSCREEN_EGALAX_SERIO > I'd rather called it TOUCHSCREEN_EGALAX_SERIAL It doesn't matter, I am not attached to names. >> +obj-$(CONFIG_TOUCHSCREEN_EGALAX_SERIO) += egalax.o > I think better name is egalax_ts_serial. Sure, I was thinking about it myself. >> obj-$(CONFIG_TOUCHSCREEN_FT6236) += ft6236.o >> obj-$(CONFIG_TOUCHSCREEN_FUJITSU) += fujitsu_ts.o >> obj-$(CONFIG_TOUCHSCREEN_GOODIX) += goodix.o >> obj-$(CONFIG_TOUCHSCREEN_ILI210X) += ili210x.o >> obj-$(CONFIG_TOUCHSCREEN_IMX6UL_TSC) += imx6ul_tsc.o >> obj-$(CONFIG_TOUCHSCREEN_INEXIO) += inexio.o >> obj-$(CONFIG_TOUCHSCREEN_INTEL_MID) += intel-mid-touch.o >> obj-$(CONFIG_TOUCHSCREEN_IPROC) += bcm_iproc_tsc.o >> obj-$(CONFIG_TOUCHSCREEN_LPC32XX) += lpc32xx_ts.o >> obj-$(CONFIG_TOUCHSCREEN_MAX11801) += max11801_ts.o >> diff --git a/drivers/input/touchscreen/egalax.c b/drivers/input/touchscreen/egalax.c >> new file mode 100644 >> index 0000000..94ac9bd >> --- /dev/null >> +++ b/drivers/input/touchscreen/egalax.c >> @@ -0,0 +1,210 @@ >> +/* >> + * EETI Egalax serial touchscreen driver >> + * >> + * Copyright (c) 2015 Zoltán Böszörményi <zboszor@xxxxx> >> + * >> + * based on the >> + * >> + * Hampshire serial touchscreen driver (Copyright (c) 2010 Adam Bennett) >> + */ >> + >> +/* >> + * 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 >> + * the Free Software Foundation. >> + */ >> + >> +#include <linux/errno.h> >> +#include <linux/kernel.h> >> +#include <linux/module.h> >> +#include <linux/slab.h> >> +#include <linux/input.h> >> +#include <linux/serio.h> >> + >> +#define DRIVER_DESC "EETI Egalax serial touchscreen driver" >> + >> +MODULE_AUTHOR("Zoltán Böszörményi <zboszor@xxxxx>"); >> +MODULE_DESCRIPTION(DRIVER_DESC); >> +MODULE_LICENSE("GPL"); >> + >> +/* >> + * Definitions & global arrays. >> + */ >> + >> +#define EGALAX_FORMAT_MAX_LENGTH 6 >> +#define EGALAX_RESPONSE_BEGIN_BYTE 0x80 >> +#define EGALAX_FORMAT_PRESSURE_BIT 0x40 >> +#define EGALAX_FORMAT_TOUCH_BIT 0x01 > We have BIT() macro that would be very useful here. > >> +#define EGALAX_FORMAT_RESOLUTION 0x06 >> + >> +#define EGALAX_MIN_XC 0 >> +#define EGALAX_MAX_XC 0x4000 >> +#define EGALAX_MIN_YC 0 >> +#define EGALAX_MAX_YC 0x4000 >> + >> +#define EGALAX_GET_XC(data, resbits, shift) ((((data[1] & (resbits)) << 7) | (data[2] & 0x7f)) << shift) >> +#define EGALAX_GET_YC(data, resbits, shift) ((((data[3] & (resbits)) << 7) | (data[4] & 0x7f)) << shift) >> +#define EGALAX_GET_TOUCHED(data) (EGALAX_FORMAT_TOUCH_BIT & data[0]) >> + >> +/* >> + * Per-touchscreen data. >> + */ >> + >> +struct egalax { >> + struct input_dev *dev; >> + struct serio *serio; >> + int idx; >> + int bytes; >> + int resbits; >> + int shift; >> + unsigned char data[EGALAX_FORMAT_MAX_LENGTH]; >> + char phys[32]; >> +}; >> + >> +static void egalax_process_data(struct egalax *pegalax) >> +{ >> + struct input_dev *dev = pegalax->dev; >> + >> + if (++pegalax->idx == pegalax->bytes) { >> + input_report_abs(dev, ABS_X, EGALAX_GET_XC(pegalax->data, pegalax->resbits, pegalax->shift)); >> + input_report_abs(dev, ABS_Y, EGALAX_GET_YC(pegalax->data, pegalax->resbits, pegalax->shift)); >> + input_report_key(dev, BTN_TOUCH, EGALAX_GET_TOUCHED(pegalax->data)); >> + input_sync(dev); >> + >> + pegalax->idx = 0; >> + } >> +} >> + >> +static irqreturn_t egalax_interrupt(struct serio *serio, >> + unsigned char data, unsigned int flags) >> +{ >> + struct egalax *pegalax = serio_get_drvdata(serio); >> + >> + pegalax->data[pegalax->idx] = data; >> + >> + if (EGALAX_RESPONSE_BEGIN_BYTE & pegalax->data[0]) { >> + pegalax->bytes = (EGALAX_FORMAT_PRESSURE_BIT & pegalax->data[0] ? 6 : 5); >> + switch ((EGALAX_FORMAT_RESOLUTION & pegalax->data[0]) >> 1) { >> + case 0: >> + pegalax->resbits = 0x0f; >> + pegalax->shift = 3; >> + break; >> + case 1: >> + pegalax->resbits = 0x1f; >> + pegalax->shift = 2; >> + break; >> + case 2: >> + pegalax->resbits = 0x3f; >> + pegalax->shift = 1; >> + break; >> + default: >> + pegalax->resbits = 0x7f; >> + pegalax->shift = 0; >> + break; >> + } >> + egalax_process_data(pegalax); > There is no reason to recalculate shift and mask on every byte and call > egalax_process_data(), better is to wait till you get full packet and > then do the claculations. Also I think you can avoid conditional > computation (switch) here. See my comment below. >> +static void egalax_disconnect(struct serio *serio) >> +{ >> + struct egalax *pegalax = serio_get_drvdata(serio); >> + >> + input_get_device(pegalax->dev); >> + input_unregister_device(pegalax->dev); >> + serio_close(serio); >> + serio_set_drvdata(serio, NULL); >> + input_put_device(pegalax->dev); >> + kfree(pegalax); > This is needlessly complicated (although I do know we have this code in > other drivers). Since we do not send any data *to* the touch controller > we can close the port first and then unregister the device. OK, whatever you think is better. I am not familiar with the input subsystem internals. > > Could you please try the version of the patch below and let me know if > it works for you? > > Thanks! While the code itself didn't look suspicious at first, testing showed that it doesn't work when compiled in with 4.4-rc5, while my original code did. You got the mask and expanding my macros wrong. I will submit a v3 patch, as yours was v2. Thanks, Zoltán Böszörményi -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
