Hi jcbian,
Here are some duplicate comments and additions to what you already
heard from Mark and Dmitry.
> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> index 61834ae..f11c1ab 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -693,4 +693,13 @@ config TOUCHSCREEN_TPS6507X
> To compile this driver as a module, choose M here: the
> module will be called tps6507x_ts.
>
> +config TOUCHSCREEN_PIXCIR
> + tristate "PIXCIR touchscreen panel support"
> + depends on I2C
> + help
> + Say Y here if you have a PIXCIR based touchscreen.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called pixcir_i2c_ts.
> +
> endif
Please keep Kconfig entries in alphabetical order.
> +#define DRIVER_VERSION "v1.0"
> +#define DRIVER_AUTHOR "Bee<jcbian@xxxxxxxxxxxxx>,Reed<dqmeng@xxxxxxxxxxxxx>"
> +#define DRIVER_DESC "Pixcir I2C Touchscreen Driver"
> +#define DRIVER_LICENSE "GPL"
Please insert these directly in the module definition.
> +/* todo:check specs for resolution of touch screen */
> +#define TOUCHSCREEN_MINX 0
> +#define TOUCHSCREEN_MAXX 400
> +#define TOUCHSCREEN_MINY 0
> +#define TOUCHSCREEN_MAXY 600
This should really be settled before mainline inclusion.
> +
> +#define DEBUG 0
Odd one...
> +
> +static struct workqueue_struct *pixcir_wq;
> +
> +struct pixcir_i2c_ts_data {
> + struct i2c_client *client;
> + struct input_dev *input;
> + struct delayed_work work;
> + int irq;
> +};
> +
> +static void pixcir_ts_poscheck(struct work_struct *work)
> +{
> + struct pixcir_i2c_ts_data *tsdata = container_of(work,
> + struct pixcir_i2c_ts_data,
> + work.work);
> +
> + unsigned char touching, oldtouching;
> + int posx1, posy1, posx2, posy2;
> + u_int8_t Rdbuf[10], Wrbuf[1];
Please do not use capitals in variable names.
> + int ret;
> + int z = 50;
> + int w = 15;
Please remove these fake values altogether.
> +
> + memset(Wrbuf, 0, sizeof(Wrbuf));
> + memset(Rdbuf, 0, sizeof(Rdbuf));
> +
> + Wrbuf[0] = 0;
> + ret = i2c_master_send(tsdata->client, Wrbuf, 1);
> + if (ret != 1) {
> + dev_err(&tsdata->client->dev, "Unable to write to i2c touchscreen!\n");
> + goto out;
> + }
> +
> + ret = i2c_master_recv(tsdata->client, Rdbuf, sizeof(Rdbuf));
> + if (ret != sizeof(Rdbuf)) {
> + dev_err(&tsdata->client->dev, "Unable to read i2c page!\n");
> + goto out;
> + }
> +
> + touching = Rdbuf[0];
> + oldtouching = Rdbuf[1];
> + posx1 = ((Rdbuf[3] << 8) | Rdbuf[2]);
> + posy1 = ((Rdbuf[5] << 8) | Rdbuf[4]);
> + posx2 = ((Rdbuf[7] << 8) | Rdbuf[6]);
> + posy2 = ((Rdbuf[9] << 8) | Rdbuf[8]);
> +
> + input_report_key(tsdata->input, BTN_TOUCH, (touching != 0 ? 1 : 0));
> +
> + if (touching == 1) {
> + input_report_abs(tsdata->input, ABS_X, posx1);
> + input_report_abs(tsdata->input, ABS_Y, posy1);
> + }
> +
> + if (!(touching)) {
> + z = 0;
> + w = 0;
> + }
> + if (touching == 2) {
> + input_report_abs(tsdata->input, ABS_MT_TOUCH_MAJOR, z);
> + input_report_abs(tsdata->input, ABS_MT_WIDTH_MAJOR, w);
> + input_report_abs(tsdata->input, ABS_MT_POSITION_X, posx1);
> + input_report_abs(tsdata->input, ABS_MT_POSITION_Y, posy1);
> + input_mt_sync(tsdata->input);
> +
> + input_report_abs(tsdata->input, ABS_MT_TOUCH_MAJOR, z);
> + input_report_abs(tsdata->input, ABS_MT_WIDTH_MAJOR, w);
> + input_report_abs(tsdata->input, ABS_MT_POSITION_X, posx2);
> + input_report_abs(tsdata->input, ABS_MT_POSITION_Y, posy2);
> + input_mt_sync(tsdata->input);
> + }
> + input_sync(tsdata->input);
Please use the slotted type B protocol instead. If the device can
track fingers, it is very simple to do, see for instance
drivers/input/touchscreen/wacom_w8001.c. If the device cannot do
tracking, please do tell and we will add in pending tracking support
to the input core together with this driver.
Thanks,
Henrik
--
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
