* [PATCH v4] Input: add ST1232 touchscreen controller driver. @ 2010-12-16 0:45 chinyeow.sim.xt 2010-12-16 6:19 ` Dmitry Torokhov 2010-12-16 6:50 ` Trilok Soni 0 siblings, 2 replies; 6+ messages in thread From: chinyeow.sim.xt @ 2010-12-16 0:45 UTC (permalink / raw) To: tsoni, dmitry.torokhov, rydberg, linux-input; +Cc: chinyeow.sim.xt From: Tony SIM <chinyeow.sim.xt@renesas.com> This patch set introduces for Sitronix ST1232 touchscreen controller driver. This is an integrated capacitive touchscreen with LCD module which can detect two fingers's touch X/Y coordinate. Signed-off-by: Tony SIM <chinyeow.sim.xt@renesas.com> --- Thanks for review! Changes since v3: - Removed unused variable. - Changed touch counting flag. drivers/input/touchscreen/Kconfig | 11 ++ drivers/input/touchscreen/Makefile | 1 + drivers/input/touchscreen/st1232.c | 273 ++++++++++++++++++++++++++++++++++++ 3 files changed, 285 insertions(+), 0 deletions(-) create mode 100644 drivers/input/touchscreen/st1232.c diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig index 06ea8da..3ca7700 100644 --- a/drivers/input/touchscreen/Kconfig +++ b/drivers/input/touchscreen/Kconfig @@ -681,4 +681,15 @@ config TOUCHSCREEN_STMPE To compile this driver as a module, choose M here: the module will be called stmpe-ts. +config TOUCHSCREEN_ST1232 + tristate "Sitronix ST1232 touchscreen controllers" + depends on I2C + help + Say Y here if you want to support Sitronix ST1232 + touchscreen controller. + + If unsure, say N. + + To compile this driver as a module, choose M here: the + module will be called st1232_ts. endif diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile index 7cc1b4f..718bcc8 100644 --- a/drivers/input/touchscreen/Makefile +++ b/drivers/input/touchscreen/Makefile @@ -39,6 +39,7 @@ obj-$(CONFIG_TOUCHSCREEN_PCAP) += pcap_ts.o obj-$(CONFIG_TOUCHSCREEN_PENMOUNT) += penmount.o obj-$(CONFIG_TOUCHSCREEN_QT602240) += qt602240_ts.o obj-$(CONFIG_TOUCHSCREEN_S3C2410) += s3c2410_ts.o +obj-$(CONFIG_TOUCHSCREEN_ST1232) += st1232.o obj-$(CONFIG_TOUCHSCREEN_STMPE) += stmpe-ts.o obj-$(CONFIG_TOUCHSCREEN_TNETV107X) += tnetv107x-ts.o obj-$(CONFIG_TOUCHSCREEN_TOUCHIT213) += touchit213.o diff --git a/drivers/input/touchscreen/st1232.c b/drivers/input/touchscreen/st1232.c new file mode 100644 index 0000000..f94a718 --- /dev/null +++ b/drivers/input/touchscreen/st1232.c @@ -0,0 +1,273 @@ +/* + * ST1232 Touchscreen Controller Driver + * + * Copyright (C) 2010 Renesas Solutions Corp. + * Tony SIM <chinyeow.sim.xt@renesas.com> + * + * Using code from: + * - android.git.kernel.org: projects/kernel/common.git: synaptics_i2c_rmi.c + * Copyright (C) 2007 Google, Inc. + * + * This software is licensed under the terms of the GNU General Public + * License version 2, as published by the Free Software Foundation, and + * may be copied, distributed, and modified under those terms. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + */ + +#include <linux/delay.h> +#include <linux/i2c.h> +#include <linux/input.h> +#include <linux/interrupt.h> +#include <linux/module.h> +#include <linux/slab.h> + +#define ST1232_TS_NAME "st1232-ts" + +#define MIN_X 0x00 +#define MIN_Y 0x00 +#define MAX_X 0x31f /* (800 - 1) */ +#define MAX_Y 0x1df /* (480 - 1) */ +#define MAX_AREA 0xff +#define MAX_FINGERS 2 + +struct st1232_ts_finger { + uint8_t is_valid; + uint16_t x; + uint16_t y; + uint8_t t; +}; + +struct st1232_ts_data { + struct i2c_client *client; + struct input_dev *input_dev; + struct st1232_ts_finger finger[MAX_FINGERS]; +}; + +static int st1232_ts_read_data(struct st1232_ts_data *ts) +{ + int ret; + struct i2c_msg msg[2]; + uint8_t start_reg; + uint8_t buf[10]; + struct st1232_ts_finger *finger = ts->finger; + + /* read touchscreen data from ST1232 */ + msg[0].addr = ts->client->addr; + msg[0].flags = 0; + msg[0].len = 1; + msg[0].buf = &start_reg; + start_reg = 0x10; + + msg[1].addr = ts->client->addr; + msg[1].flags = I2C_M_RD; + msg[1].len = sizeof(buf); + msg[1].buf = buf; + + ret = i2c_transfer(ts->client->adapter, msg, 2); + if (ret < 0) + return ret; + + /* get valid bit */ + finger[0].is_valid = buf[2] >> 7; + finger[1].is_valid = buf[5] >> 7; + + /* get xy coordinate */ + if (finger[0].is_valid) { + finger[0].x = ((buf[2] & 0x0070) << 4) | buf[3]; + finger[0].y = ((buf[2] & 0x0007) << 8) | buf[4]; + finger[0].t = buf[8]; + } + + if (finger[1].is_valid) { + finger[1].x = ((buf[5] & 0x0070) << 4) | buf[6]; + finger[1].y = ((buf[5] & 0x0007) << 8) | buf[7]; + finger[1].t = buf[9]; + } + + return 0; +} + +static irqreturn_t st1232_ts_irq_handler(int irq, void *dev_id) +{ + int i, ret; + int count = 0; + struct st1232_ts_data *ts = dev_id; + struct st1232_ts_finger *finger = ts->finger; + + ret = st1232_ts_read_data(ts); + if (ret < 0) + goto end; + + /* multi touch protocol */ + for (i = 0; i < MAX_FINGERS; i++) { + if (!finger[i].is_valid) + continue; + + input_report_abs(ts->input_dev, ABS_MT_TOUCH_MAJOR, + finger[i].t); + input_report_abs(ts->input_dev, ABS_MT_POSITION_X, finger[i].x); + input_report_abs(ts->input_dev, ABS_MT_POSITION_Y, finger[i].y); + input_mt_sync(ts->input_dev); + count++; + } + + /* SYN_MT_REPORT only if no contact */ + if (!count) + input_mt_sync(ts->input_dev); + + /* SYN_REPORT */ + input_sync(ts->input_dev); + +end: + return IRQ_HANDLED; +} + +static int __devinit st1232_ts_probe(struct i2c_client *client, + const struct i2c_device_id *id) +{ + struct st1232_ts_data *ts; + int ret; + + if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) { + dev_err(&client->dev, "need I2C_FUNC_I2C\n"); + return -EIO; + } + + ts = kzalloc(sizeof(struct st1232_ts_data), GFP_KERNEL); + if (!ts) { + ret = -ENOMEM; + goto err; + } + + ts->client = client; + i2c_set_clientdata(client, ts); + + ts->input_dev = input_allocate_device(); + if (!ts->input_dev) { + ret = -ENOMEM; + dev_err(&client->dev, "Failed to allocate input device\n"); + goto err_free_mem; + } + ts->input_dev->name = "st1232-touchscreen"; + + __set_bit(EV_SYN, ts->input_dev->evbit); + __set_bit(EV_KEY, ts->input_dev->evbit); + __set_bit(EV_ABS, ts->input_dev->evbit); + + input_set_abs_params(ts->input_dev, ABS_MT_TOUCH_MAJOR, + 0, MAX_AREA, 0, 0); + input_set_abs_params(ts->input_dev, ABS_MT_POSITION_X, + MIN_X, MAX_X, 0, 0); + input_set_abs_params(ts->input_dev, ABS_MT_POSITION_Y, + MIN_Y, MAX_Y, 0, 0); + + ret = input_register_device(ts->input_dev); + if (ret) { + dev_err(&client->dev, "Unable to register %s input device\n", + ts->input_dev->name); + goto err_free_input_device; + } + + ret = request_threaded_irq(client->irq, NULL, st1232_ts_irq_handler, + IRQF_ONESHOT, client->name, ts); + if (ret) { + dev_err(&client->dev, "Failed to register interrupt\n"); + goto err_free_input_device; + } + + device_init_wakeup(&client->dev, 1); + + return 0; + +err_free_input_device: + input_free_device(ts->input_dev); +err_free_mem: + kfree(ts); +err: + return ret; +} + +static int __devexit st1232_ts_remove(struct i2c_client *client) +{ + struct st1232_ts_data *ts = i2c_get_clientdata(client); + + device_init_wakeup(&client->dev, 0); + free_irq(client->irq, ts); + input_unregister_device(ts->input_dev); + kfree(ts); + + return 0; +} + +#ifdef CONFIG_PM +static int st1232_ts_suspend(struct device *dev) +{ + struct st1232_ts_data *ts = dev_get_drvdata(dev); + struct i2c_client *client = ts->client; + + if (device_may_wakeup(&client->dev)) + enable_irq_wake(client->irq); + else + disable_irq(client->irq); + + return 0; +} + +static int st1232_ts_resume(struct device *dev) +{ + struct st1232_ts_data *ts = dev_get_drvdata(dev); + struct i2c_client *client = ts->client; + + if (device_may_wakeup(&client->dev)) + disable_irq_wake(client->irq); + else + enable_irq(client->irq); + + return 0; +} + +static const struct dev_pm_ops st1232_ts_pm_ops = { + .suspend = st1232_ts_suspend, + .resume = st1232_ts_resume, +}; +#endif + +static const struct i2c_device_id st1232_ts_id[] = { + { ST1232_TS_NAME, 0 }, + { } +}; +MODULE_DEVICE_TABLE(i2c, st1232_ts_id); + +static struct i2c_driver st1232_ts_driver = { + .probe = st1232_ts_probe, + .remove = __devexit_p(st1232_ts_remove), + .id_table = st1232_ts_id, + .driver = { + .name = ST1232_TS_NAME, + .owner = THIS_MODULE, +#ifdef CONFIG_PM + .pm = &st1232_ts_pm_ops, +#endif + }, +}; + +static int __init st1232_ts_init(void) +{ + return i2c_add_driver(&st1232_ts_driver); +} +module_init(st1232_ts_init); + +static void __exit st1232_ts_exit(void) +{ + i2c_del_driver(&st1232_ts_driver); +} +module_exit(st1232_ts_exit); + +MODULE_AUTHOR("Tony SIM <chinyeow.sim.xt@renesas.com>"); +MODULE_DESCRIPTION("SITRONIX ST1232 Touchscreen Controller Driver"); +MODULE_LICENSE("GPL"); -- 1.7.2.3 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v4] Input: add ST1232 touchscreen controller driver. 2010-12-16 0:45 [PATCH v4] Input: add ST1232 touchscreen controller driver chinyeow.sim.xt @ 2010-12-16 6:19 ` Dmitry Torokhov 2010-12-16 7:06 ` chinyeow.sim.xt 2010-12-16 7:27 ` Henrik Rydberg 2010-12-16 6:50 ` Trilok Soni 1 sibling, 2 replies; 6+ messages in thread From: Dmitry Torokhov @ 2010-12-16 6:19 UTC (permalink / raw) To: chinyeow.sim.xt; +Cc: tsoni, rydberg, linux-input Hi Tony, On Thu, Dec 16, 2010 at 09:45:25AM +0900, chinyeow.sim.xt@renesas.com wrote: > From: Tony SIM <chinyeow.sim.xt@renesas.com> > > This patch set introduces for Sitronix ST1232 touchscreen controller > driver. This is an integrated capacitive touchscreen with LCD module > which can detect two fingers's touch X/Y coordinate. > > Signed-off-by: Tony SIM <chinyeow.sim.xt@renesas.com> > --- > > Thanks for review! > > Changes since v3: > - Removed unused variable. > - Changed touch counting flag. This looks great now, just a couple more comments: > + > +struct st1232_ts_finger { > + uint8_t is_valid; > + uint16_t x; > + uint16_t y; > + uint8_t t; > +}; If you regroup fields the structure can be packed better. Also we prefer u8, u16 instead of uint8_t, uint16_t in the kernel. > + ts->input_dev->name = "st1232-touchscreen"; Also need to set input->id.bus = BUS_I2C and parent device. > + > + __set_bit(EV_SYN, ts->input_dev->evbit); > + __set_bit(EV_KEY, ts->input_dev->evbit); > + __set_bit(EV_ABS, ts->input_dev->evbit); > + > + input_set_abs_params(ts->input_dev, ABS_MT_TOUCH_MAJOR, > + 0, MAX_AREA, 0, 0); > + input_set_abs_params(ts->input_dev, ABS_MT_POSITION_X, > + MIN_X, MAX_X, 0, 0); > + input_set_abs_params(ts->input_dev, ABS_MT_POSITION_Y, > + MIN_Y, MAX_Y, 0, 0); > + > + ret = input_register_device(ts->input_dev); > + if (ret) { > + dev_err(&client->dev, "Unable to register %s input device\n", > + ts->input_dev->name); > + goto err_free_input_device; > + } > + > + ret = request_threaded_irq(client->irq, NULL, st1232_ts_irq_handler, > + IRQF_ONESHOT, client->name, ts); > + if (ret) { > + dev_err(&client->dev, "Failed to register interrupt\n"); > + goto err_free_input_device; Registered devices should be destroyed by call to input_unregister_device() instead of input_free_device(). Does the following (applied on top of your patch) still work? Thanks! -- Dmitry Input: st1232 - assorted changes Signed-off-by: Dmitry Torokhov <dtor@mail.ru> --- drivers/input/touchscreen/Kconfig | 21 +++--- drivers/input/touchscreen/st1232.c | 133 ++++++++++++++++++------------------ 2 files changed, 78 insertions(+), 76 deletions(-) diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig index 3ca7700..07ac77d 100644 --- a/drivers/input/touchscreen/Kconfig +++ b/drivers/input/touchscreen/Kconfig @@ -659,17 +659,17 @@ config TOUCHSCREEN_PCAP To compile this driver as a module, choose M here: the module will be called pcap_ts. -config TOUCHSCREEN_TPS6507X - tristate "TPS6507x based touchscreens" +config TOUCHSCREEN_ST1232 + tristate "Sitronix ST1232 touchscreen controllers" depends on I2C help - Say Y here if you have a TPS6507x based touchscreen - controller. + Say Y here if you want to support Sitronix ST1232 + touchscreen controller. If unsure, say N. To compile this driver as a module, choose M here: the - module will be called tps6507x_ts. + module will be called st1232_ts. config TOUCHSCREEN_STMPE tristate "STMicroelectronics STMPE touchscreens" @@ -681,15 +681,16 @@ config TOUCHSCREEN_STMPE To compile this driver as a module, choose M here: the module will be called stmpe-ts. -config TOUCHSCREEN_ST1232 - tristate "Sitronix ST1232 touchscreen controllers" +config TOUCHSCREEN_TPS6507X + tristate "TPS6507x based touchscreens" depends on I2C help - Say Y here if you want to support Sitronix ST1232 - touchscreen controller. + Say Y here if you have a TPS6507x based touchscreen + controller. If unsure, say N. To compile this driver as a module, choose M here: the - module will be called st1232_ts. + module will be called tps6507x_ts. + endif diff --git a/drivers/input/touchscreen/st1232.c b/drivers/input/touchscreen/st1232.c index f94a718..4ab3713 100644 --- a/drivers/input/touchscreen/st1232.c +++ b/drivers/input/touchscreen/st1232.c @@ -16,7 +16,6 @@ * but WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * GNU General Public License for more details. - * */ #include <linux/delay.h> @@ -25,21 +24,22 @@ #include <linux/interrupt.h> #include <linux/module.h> #include <linux/slab.h> +#include <linux/types.h> #define ST1232_TS_NAME "st1232-ts" -#define MIN_X 0x00 -#define MIN_Y 0x00 -#define MAX_X 0x31f /* (800 - 1) */ -#define MAX_Y 0x1df /* (480 - 1) */ +#define MIN_X 0x00 +#define MIN_Y 0x00 +#define MAX_X 0x31f /* (800 - 1) */ +#define MAX_Y 0x1df /* (480 - 1) */ #define MAX_AREA 0xff #define MAX_FINGERS 2 struct st1232_ts_finger { - uint8_t is_valid; - uint16_t x; - uint16_t y; - uint8_t t; + u16 x; + u16 y; + u8 t; + bool is_valid; }; struct st1232_ts_data { @@ -50,14 +50,15 @@ struct st1232_ts_data { static int st1232_ts_read_data(struct st1232_ts_data *ts) { - int ret; - struct i2c_msg msg[2]; - uint8_t start_reg; - uint8_t buf[10]; struct st1232_ts_finger *finger = ts->finger; + struct i2c_client *client = ts->client; + struct i2c_msg msg[2]; + int error; + u8 start_reg; + u8 buf[10]; /* read touchscreen data from ST1232 */ - msg[0].addr = ts->client->addr; + msg[0].addr = client->addr; msg[0].flags = 0; msg[0].len = 1; msg[0].buf = &start_reg; @@ -68,11 +69,11 @@ static int st1232_ts_read_data(struct st1232_ts_data *ts) msg[1].len = sizeof(buf); msg[1].buf = buf; - ret = i2c_transfer(ts->client->adapter, msg, 2); - if (ret < 0) - return ret; + error = i2c_transfer(client->adapter, msg, 2); + if (error < 0) + return error; - /* get valid bit */ + /* get "valid" bits */ finger[0].is_valid = buf[2] >> 7; finger[1].is_valid = buf[5] >> 7; @@ -94,10 +95,11 @@ static int st1232_ts_read_data(struct st1232_ts_data *ts) static irqreturn_t st1232_ts_irq_handler(int irq, void *dev_id) { - int i, ret; - int count = 0; struct st1232_ts_data *ts = dev_id; struct st1232_ts_finger *finger = ts->finger; + struct input_dev *input_dev = ts->input_dev; + int count = 0; + int i, ret; ret = st1232_ts_read_data(ts); if (ret < 0) @@ -108,20 +110,19 @@ static irqreturn_t st1232_ts_irq_handler(int irq, void *dev_id) if (!finger[i].is_valid) continue; - input_report_abs(ts->input_dev, ABS_MT_TOUCH_MAJOR, - finger[i].t); - input_report_abs(ts->input_dev, ABS_MT_POSITION_X, finger[i].x); - input_report_abs(ts->input_dev, ABS_MT_POSITION_Y, finger[i].y); - input_mt_sync(ts->input_dev); + input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR, finger[i].t); + input_report_abs(input_dev, ABS_MT_POSITION_X, finger[i].x); + input_report_abs(input_dev, ABS_MT_POSITION_Y, finger[i].y); + input_mt_sync(input_dev); count++; } /* SYN_MT_REPORT only if no contact */ if (!count) - input_mt_sync(ts->input_dev); + input_mt_sync(input_dev); /* SYN_REPORT */ - input_sync(ts->input_dev); + input_sync(input_dev); end: return IRQ_HANDLED; @@ -131,65 +132,67 @@ static int __devinit st1232_ts_probe(struct i2c_client *client, const struct i2c_device_id *id) { struct st1232_ts_data *ts; - int ret; + struct input_dev *input_dev; + int error; if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) { dev_err(&client->dev, "need I2C_FUNC_I2C\n"); return -EIO; } - ts = kzalloc(sizeof(struct st1232_ts_data), GFP_KERNEL); - if (!ts) { - ret = -ENOMEM; - goto err; + if (!client->irq) { + dev_err(&client->dev, "no IRQ?\n"); + return -EINVAL; } - ts->client = client; - i2c_set_clientdata(client, ts); - ts->input_dev = input_allocate_device(); - if (!ts->input_dev) { - ret = -ENOMEM; - dev_err(&client->dev, "Failed to allocate input device\n"); + ts = kzalloc(sizeof(struct st1232_ts_data), GFP_KERNEL); + input_dev = input_allocate_device(); + if (!ts || !input_dev) { + error = -ENOMEM; goto err_free_mem; } - ts->input_dev->name = "st1232-touchscreen"; - __set_bit(EV_SYN, ts->input_dev->evbit); - __set_bit(EV_KEY, ts->input_dev->evbit); - __set_bit(EV_ABS, ts->input_dev->evbit); + ts->client = client; + ts->input_dev = input_dev; - input_set_abs_params(ts->input_dev, ABS_MT_TOUCH_MAJOR, - 0, MAX_AREA, 0, 0); - input_set_abs_params(ts->input_dev, ABS_MT_POSITION_X, - MIN_X, MAX_X, 0, 0); - input_set_abs_params(ts->input_dev, ABS_MT_POSITION_Y, - MIN_Y, MAX_Y, 0, 0); + input_dev->name = "st1232-touchscreen"; + input_dev->id.bustype = BUS_I2C; + input_dev->dev.parent = &client->dev; - ret = input_register_device(ts->input_dev); - if (ret) { - dev_err(&client->dev, "Unable to register %s input device\n", - ts->input_dev->name); - goto err_free_input_device; - } + __set_bit(EV_SYN, input_dev->evbit); + __set_bit(EV_KEY, input_dev->evbit); + __set_bit(EV_ABS, input_dev->evbit); - ret = request_threaded_irq(client->irq, NULL, st1232_ts_irq_handler, - IRQF_ONESHOT, client->name, ts); - if (ret) { + input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR, 0, MAX_AREA, 0, 0); + input_set_abs_params(input_dev, ABS_MT_POSITION_X, MIN_X, MAX_X, 0, 0); + input_set_abs_params(input_dev, ABS_MT_POSITION_Y, MIN_Y, MAX_Y, 0, 0); + + error = request_threaded_irq(client->irq, NULL, st1232_ts_irq_handler, + IRQF_ONESHOT, client->name, ts); + if (error) { dev_err(&client->dev, "Failed to register interrupt\n"); - goto err_free_input_device; + goto err_free_mem; } + error = input_register_device(ts->input_dev); + if (error) { + dev_err(&client->dev, "Unable to register %s input device\n", + input_dev->name); + goto err_free_irq; + } + + i2c_set_clientdata(client, ts); device_init_wakeup(&client->dev, 1); return 0; -err_free_input_device: - input_free_device(ts->input_dev); +err_free_irq: + free_irq(client->irq, ts); err_free_mem: + input_free_device(input_dev); kfree(ts); -err: - return ret; + return error; } static int __devexit st1232_ts_remove(struct i2c_client *client) @@ -207,8 +210,7 @@ static int __devexit st1232_ts_remove(struct i2c_client *client) #ifdef CONFIG_PM static int st1232_ts_suspend(struct device *dev) { - struct st1232_ts_data *ts = dev_get_drvdata(dev); - struct i2c_client *client = ts->client; + struct i2c_client *client = to_i2c_client(dev); if (device_may_wakeup(&client->dev)) enable_irq_wake(client->irq); @@ -220,8 +222,7 @@ static int st1232_ts_suspend(struct device *dev) static int st1232_ts_resume(struct device *dev) { - struct st1232_ts_data *ts = dev_get_drvdata(dev); - struct i2c_client *client = ts->client; + struct i2c_client *client = to_i2c_client(dev); if (device_may_wakeup(&client->dev)) disable_irq_wake(client->irq); ^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: [PATCH v4] Input: add ST1232 touchscreen controller driver. 2010-12-16 6:19 ` Dmitry Torokhov @ 2010-12-16 7:06 ` chinyeow.sim.xt 2010-12-16 7:45 ` Dmitry Torokhov 2010-12-16 7:27 ` Henrik Rydberg 1 sibling, 1 reply; 6+ messages in thread From: chinyeow.sim.xt @ 2010-12-16 7:06 UTC (permalink / raw) To: 'Dmitry Torokhov'; +Cc: tsoni, rydberg, linux-input Hi Dmitry, Thanks for modifing the changes! It works well! / Tony > -----Original Message----- > From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com] > Sent: Thursday, December 16, 2010 3:19 PM > To: chinyeow.sim.xt@renesas.com > Cc: tsoni@codeaurora.org; rydberg@euromail.se; > linux-input@vger.kernel.org > Subject: Re: [PATCH v4] Input: add ST1232 touchscreen > controller driver. > > > Hi Tony, > > On Thu, Dec 16, 2010 at 09:45:25AM +0900, > chinyeow.sim.xt@renesas.com wrote: > > From: Tony SIM <chinyeow.sim.xt@renesas.com> > > > > This patch set introduces for Sitronix ST1232 touchscreen controller > > driver. This is an integrated capacitive touchscreen with LCD module > > which can detect two fingers's touch X/Y coordinate. > > > > Signed-off-by: Tony SIM <chinyeow.sim.xt@renesas.com> > > --- > > > > Thanks for review! > > > > Changes since v3: > > - Removed unused variable. > > - Changed touch counting flag. > > This looks great now, just a couple more comments: > > > + > > +struct st1232_ts_finger { > > + uint8_t is_valid; > > + uint16_t x; > > + uint16_t y; > > + uint8_t t; > > +}; > > If you regroup fields the structure can be packed better. > Also we prefer > u8, u16 instead of uint8_t, uint16_t in the kernel. > > > + ts->input_dev->name = "st1232-touchscreen"; > > Also need to set input->id.bus = BUS_I2C and parent device. > > > + > > + __set_bit(EV_SYN, ts->input_dev->evbit); > > + __set_bit(EV_KEY, ts->input_dev->evbit); > > + __set_bit(EV_ABS, ts->input_dev->evbit); > > + > > + input_set_abs_params(ts->input_dev, ABS_MT_TOUCH_MAJOR, > > + 0, MAX_AREA, 0, 0); > > + input_set_abs_params(ts->input_dev, ABS_MT_POSITION_X, > > + MIN_X, MAX_X, 0, 0); > > + input_set_abs_params(ts->input_dev, ABS_MT_POSITION_Y, > > + MIN_Y, MAX_Y, 0, 0); > > + > > + ret = input_register_device(ts->input_dev); > > + if (ret) { > > + dev_err(&client->dev, "Unable to register %s input device\n", > > + ts->input_dev->name); > > + goto err_free_input_device; > > + } > > + > > + ret = request_threaded_irq(client->irq, NULL, st1232_ts_irq_handler, > > + IRQF_ONESHOT, client->name, ts); > > + if (ret) { > > + dev_err(&client->dev, "Failed to register interrupt\n"); > > + goto err_free_input_device; > > Registered devices should be destroyed by call to > input_unregister_device() instead of input_free_device(). > > Does the following (applied on top of your patch) still work? Thanks! > > -- > Dmitry > > Input: st1232 - assorted changes > > Signed-off-by: Dmitry Torokhov <dtor@mail.ru> > --- > > drivers/input/touchscreen/Kconfig | 21 +++--- > drivers/input/touchscreen/st1232.c | 133 > ++++++++++++++++++------------------ > 2 files changed, 78 insertions(+), 76 deletions(-) > > > diff --git a/drivers/input/touchscreen/Kconfig > b/drivers/input/touchscreen/Kconfig > index 3ca7700..07ac77d 100644 > --- a/drivers/input/touchscreen/Kconfig > +++ b/drivers/input/touchscreen/Kconfig > @@ -659,17 +659,17 @@ config TOUCHSCREEN_PCAP > To compile this driver as a module, choose M here: the > module will be called pcap_ts. > > -config TOUCHSCREEN_TPS6507X > - tristate "TPS6507x based touchscreens" > +config TOUCHSCREEN_ST1232 > + tristate "Sitronix ST1232 touchscreen controllers" > depends on I2C > help > - Say Y here if you have a TPS6507x based touchscreen > - controller. > + Say Y here if you want to support Sitronix ST1232 > + touchscreen controller. > > If unsure, say N. > > To compile this driver as a module, choose M here: the > - module will be called tps6507x_ts. > + module will be called st1232_ts. > > config TOUCHSCREEN_STMPE > tristate "STMicroelectronics STMPE touchscreens" > @@ -681,15 +681,16 @@ config TOUCHSCREEN_STMPE > To compile this driver as a module, choose M here: the > module will be called stmpe-ts. > > -config TOUCHSCREEN_ST1232 > - tristate "Sitronix ST1232 touchscreen controllers" > +config TOUCHSCREEN_TPS6507X > + tristate "TPS6507x based touchscreens" > depends on I2C > help > - Say Y here if you want to support Sitronix ST1232 > - touchscreen controller. > + Say Y here if you have a TPS6507x based touchscreen > + controller. > > If unsure, say N. > > To compile this driver as a module, choose M here: the > - module will be called st1232_ts. > + module will be called tps6507x_ts. > + > endif > diff --git a/drivers/input/touchscreen/st1232.c > b/drivers/input/touchscreen/st1232.c > index f94a718..4ab3713 100644 > --- a/drivers/input/touchscreen/st1232.c > +++ b/drivers/input/touchscreen/st1232.c > @@ -16,7 +16,6 @@ > * but WITHOUT ANY WARRANTY; without even the implied warranty of > * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > * GNU General Public License for more details. > - * > */ > > #include <linux/delay.h> > @@ -25,21 +24,22 @@ > #include <linux/interrupt.h> > #include <linux/module.h> > #include <linux/slab.h> > +#include <linux/types.h> > > #define ST1232_TS_NAME "st1232-ts" > > -#define MIN_X 0x00 > -#define MIN_Y 0x00 > -#define MAX_X 0x31f /* (800 - 1) */ > -#define MAX_Y 0x1df /* (480 - 1) */ > +#define MIN_X 0x00 > +#define MIN_Y 0x00 > +#define MAX_X 0x31f /* (800 - 1) */ > +#define MAX_Y 0x1df /* (480 - 1) */ > #define MAX_AREA 0xff > #define MAX_FINGERS 2 > > struct st1232_ts_finger { > - uint8_t is_valid; > - uint16_t x; > - uint16_t y; > - uint8_t t; > + u16 x; > + u16 y; > + u8 t; > + bool is_valid; > }; > > struct st1232_ts_data { > @@ -50,14 +50,15 @@ struct st1232_ts_data { > > static int st1232_ts_read_data(struct st1232_ts_data *ts) > { > - int ret; > - struct i2c_msg msg[2]; > - uint8_t start_reg; > - uint8_t buf[10]; > struct st1232_ts_finger *finger = ts->finger; > + struct i2c_client *client = ts->client; > + struct i2c_msg msg[2]; > + int error; > + u8 start_reg; > + u8 buf[10]; > > /* read touchscreen data from ST1232 */ > - msg[0].addr = ts->client->addr; > + msg[0].addr = client->addr; > msg[0].flags = 0; > msg[0].len = 1; > msg[0].buf = &start_reg; > @@ -68,11 +69,11 @@ static int st1232_ts_read_data(struct st1232_ts_data *ts) > msg[1].len = sizeof(buf); > msg[1].buf = buf; > > - ret = i2c_transfer(ts->client->adapter, msg, 2); > - if (ret < 0) > - return ret; > + error = i2c_transfer(client->adapter, msg, 2); > + if (error < 0) > + return error; > > - /* get valid bit */ > + /* get "valid" bits */ > finger[0].is_valid = buf[2] >> 7; > finger[1].is_valid = buf[5] >> 7; > > @@ -94,10 +95,11 @@ static int st1232_ts_read_data(struct st1232_ts_data *ts) > > static irqreturn_t st1232_ts_irq_handler(int irq, void *dev_id) > { > - int i, ret; > - int count = 0; > struct st1232_ts_data *ts = dev_id; > struct st1232_ts_finger *finger = ts->finger; > + struct input_dev *input_dev = ts->input_dev; > + int count = 0; > + int i, ret; > > ret = st1232_ts_read_data(ts); > if (ret < 0) > @@ -108,20 +110,19 @@ static irqreturn_t st1232_ts_irq_handler(int irq, void *dev_id) > if (!finger[i].is_valid) > continue; > > - input_report_abs(ts->input_dev, ABS_MT_TOUCH_MAJOR, > - finger[i].t); > - input_report_abs(ts->input_dev, ABS_MT_POSITION_X, finger[i].x); > - input_report_abs(ts->input_dev, ABS_MT_POSITION_Y, finger[i].y); > - input_mt_sync(ts->input_dev); > + input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR, finger[i].t); > + input_report_abs(input_dev, ABS_MT_POSITION_X, finger[i].x); > + input_report_abs(input_dev, ABS_MT_POSITION_Y, finger[i].y); > + input_mt_sync(input_dev); > count++; > } > > /* SYN_MT_REPORT only if no contact */ > if (!count) > - input_mt_sync(ts->input_dev); > + input_mt_sync(input_dev); > > /* SYN_REPORT */ > - input_sync(ts->input_dev); > + input_sync(input_dev); > > end: > return IRQ_HANDLED; > @@ -131,65 +132,67 @@ static int __devinit st1232_ts_probe(struct i2c_client *client, > const struct i2c_device_id *id) > { > struct st1232_ts_data *ts; > - int ret; > + struct input_dev *input_dev; > + int error; > > if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) { > dev_err(&client->dev, "need I2C_FUNC_I2C\n"); > return -EIO; > } > > - ts = kzalloc(sizeof(struct st1232_ts_data), GFP_KERNEL); > - if (!ts) { > - ret = -ENOMEM; > - goto err; > + if (!client->irq) { > + dev_err(&client->dev, "no IRQ?\n"); > + return -EINVAL; > } > > - ts->client = client; > - i2c_set_clientdata(client, ts); > > - ts->input_dev = input_allocate_device(); > - if (!ts->input_dev) { > - ret = -ENOMEM; > - dev_err(&client->dev, "Failed to allocate input device\n"); > + ts = kzalloc(sizeof(struct st1232_ts_data), GFP_KERNEL); > + input_dev = input_allocate_device(); > + if (!ts || !input_dev) { > + error = -ENOMEM; > goto err_free_mem; > } > - ts->input_dev->name = "st1232-touchscreen"; > > - __set_bit(EV_SYN, ts->input_dev->evbit); > - __set_bit(EV_KEY, ts->input_dev->evbit); > - __set_bit(EV_ABS, ts->input_dev->evbit); > + ts->client = client; > + ts->input_dev = input_dev; > > - input_set_abs_params(ts->input_dev, ABS_MT_TOUCH_MAJOR, > - 0, MAX_AREA, 0, 0); > - input_set_abs_params(ts->input_dev, ABS_MT_POSITION_X, > - MIN_X, MAX_X, 0, 0); > - input_set_abs_params(ts->input_dev, ABS_MT_POSITION_Y, > - MIN_Y, MAX_Y, 0, 0); > + input_dev->name = "st1232-touchscreen"; > + input_dev->id.bustype = BUS_I2C; > + input_dev->dev.parent = &client->dev; > > - ret = input_register_device(ts->input_dev); > - if (ret) { > - dev_err(&client->dev, "Unable to register %s input device\n", > - ts->input_dev->name); > - goto err_free_input_device; > - } > + __set_bit(EV_SYN, input_dev->evbit); > + __set_bit(EV_KEY, input_dev->evbit); > + __set_bit(EV_ABS, input_dev->evbit); > > - ret = request_threaded_irq(client->irq, NULL, st1232_ts_irq_handler, > - IRQF_ONESHOT, client->name, ts); > - if (ret) { > + input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR, 0, MAX_AREA, 0, 0); > + input_set_abs_params(input_dev, ABS_MT_POSITION_X, MIN_X, MAX_X, 0, 0); > + input_set_abs_params(input_dev, ABS_MT_POSITION_Y, MIN_Y, MAX_Y, 0, 0); > + > + error = request_threaded_irq(client->irq, NULL, st1232_ts_irq_handler, > + IRQF_ONESHOT, client->name, ts); > + if (error) { > dev_err(&client->dev, "Failed to register interrupt\n"); > - goto err_free_input_device; > + goto err_free_mem; > } > > + error = input_register_device(ts->input_dev); > + if (error) { > + dev_err(&client->dev, "Unable to register %s input device\n", > + input_dev->name); > + goto err_free_irq; > + } > + > + i2c_set_clientdata(client, ts); > device_init_wakeup(&client->dev, 1); > > return 0; > > -err_free_input_device: > - input_free_device(ts->input_dev); > +err_free_irq: > + free_irq(client->irq, ts); > err_free_mem: > + input_free_device(input_dev); > kfree(ts); > -err: > - return ret; > + return error; > } > > static int __devexit st1232_ts_remove(struct i2c_client *client) > @@ -207,8 +210,7 @@ static int __devexit st1232_ts_remove(struct i2c_client *client) > #ifdef CONFIG_PM > static int st1232_ts_suspend(struct device *dev) > { > - struct st1232_ts_data *ts = dev_get_drvdata(dev); > - struct i2c_client *client = ts->client; > + struct i2c_client *client = to_i2c_client(dev); > > if (device_may_wakeup(&client->dev)) > enable_irq_wake(client->irq); > @@ -220,8 +222,7 @@ static int st1232_ts_suspend(struct device *dev) > > static int st1232_ts_resume(struct device *dev) > { > - struct st1232_ts_data *ts = dev_get_drvdata(dev); > - struct i2c_client *client = ts->client; > + struct i2c_client *client = to_i2c_client(dev); > > if (device_may_wakeup(&client->dev)) > disable_irq_wake(client->irq); ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4] Input: add ST1232 touchscreen controller driver. 2010-12-16 7:06 ` chinyeow.sim.xt @ 2010-12-16 7:45 ` Dmitry Torokhov 0 siblings, 0 replies; 6+ messages in thread From: Dmitry Torokhov @ 2010-12-16 7:45 UTC (permalink / raw) To: chinyeow.sim.xt; +Cc: tsoni, rydberg, linux-input On Thu, Dec 16, 2010 at 04:06:10PM +0900, chinyeow.sim.xt@renesas.com wrote: > Hi Dmitry, > > Thanks for modifing the changes! It works well! > > / Tony > Great, I'll queue the driver for .38. Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4] Input: add ST1232 touchscreen controller driver. 2010-12-16 6:19 ` Dmitry Torokhov 2010-12-16 7:06 ` chinyeow.sim.xt @ 2010-12-16 7:27 ` Henrik Rydberg 1 sibling, 0 replies; 6+ messages in thread From: Henrik Rydberg @ 2010-12-16 7:27 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: chinyeow.sim.xt, tsoni, linux-input On 12/16/2010 07:19 AM, Dmitry Torokhov wrote: > Hi Tony, > > On Thu, Dec 16, 2010 at 09:45:25AM +0900, chinyeow.sim.xt@renesas.com wrote: >> From: Tony SIM <chinyeow.sim.xt@renesas.com> >> >> This patch set introduces for Sitronix ST1232 touchscreen controller >> driver. This is an integrated capacitive touchscreen with LCD module >> which can detect two fingers's touch X/Y coordinate. >> >> Signed-off-by: Tony SIM <chinyeow.sim.xt@renesas.com> >> --- Acked-by: Henrik Rydberg <rydberg@euromail.se> Thanks! Henrik ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4] Input: add ST1232 touchscreen controller driver. 2010-12-16 0:45 [PATCH v4] Input: add ST1232 touchscreen controller driver chinyeow.sim.xt 2010-12-16 6:19 ` Dmitry Torokhov @ 2010-12-16 6:50 ` Trilok Soni 1 sibling, 0 replies; 6+ messages in thread From: Trilok Soni @ 2010-12-16 6:50 UTC (permalink / raw) To: chinyeow.sim.xt; +Cc: dmitry.torokhov, rydberg, linux-input Hi Tony, On 12/16/2010 6:15 AM, chinyeow.sim.xt@renesas.com wrote: > From: Tony SIM <chinyeow.sim.xt@renesas.com> > > This patch set introduces for Sitronix ST1232 touchscreen controller > driver. This is an integrated capacitive touchscreen with LCD module > which can detect two fingers's touch X/Y coordinate. > > Signed-off-by: Tony SIM <chinyeow.sim.xt@renesas.com> Thanks. Looks good to me. Reviewed-by: Trilok Soni <tsoni@codeaurora.org> ---Trilok Soni -- Sent by a consultant of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-12-16 7:45 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-12-16 0:45 [PATCH v4] Input: add ST1232 touchscreen controller driver chinyeow.sim.xt 2010-12-16 6:19 ` Dmitry Torokhov 2010-12-16 7:06 ` chinyeow.sim.xt 2010-12-16 7:45 ` Dmitry Torokhov 2010-12-16 7:27 ` Henrik Rydberg 2010-12-16 6:50 ` Trilok Soni
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).