On Mon, Oct 10, 2011 at 3:55 PM, Shubhrajyoti Datta <omaplinuxkernel@xxxxxxxxx> wrote: > Hello Martinez, > Some small comments. > > Hi Shubhrajyoti, Thanks for the review. > On Sun, Oct 9, 2011 at 12:07 AM, Javier Martinez Canillas > <martinez.javier@xxxxxxxxx> wrote: >> >> The driver is composed of a core driver that process the data sent by >> the contacts and a set of bus specific interface modules. >> >> Signed-off-by: Javier Martinez Canillas <martinez.javier@xxxxxxxxx> >> --- >> >> v2: Fix issues called out by Dmitry Torokhov >> - Extract the IRQ from the i2c client data and pass to >> cyttsp_core_init() >> >> v3: Fix issues called out by Henrik Rydberg and Mohan Pallaka >> - Remove bus type info since it is not used. >> >> drivers/input/touchscreen/cyttsp/cyttsp_i2c.c | 190 >> +++++++++++++++++++++++++ >> 1 files changed, 190 insertions(+), 0 deletions(-) >> create mode 100644 drivers/input/touchscreen/cyttsp/cyttsp_i2c.c >> >> diff --git a/drivers/input/touchscreen/cyttsp/cyttsp_i2c.c >> b/drivers/input/touchscreen/cyttsp/cyttsp_i2c.c >> new file mode 100644 >> index 0000000..146a16d >> --- /dev/null >> +++ b/drivers/input/touchscreen/cyttsp/cyttsp_i2c.c >> @@ -0,0 +1,190 @@ >> +/* >> + * Source for: >> + * Cypress TrueTouch(TM) Standard Product (TTSP) I2C touchscreen driver. >> + * For use with Cypress Txx3xx parts. >> + * Supported parts include: >> + * CY8CTST341 >> + * CY8CTMA340 >> + * >> + * Copyright (C) 2009, 2010, 2011 Cypress Semiconductor, Inc. >> + * Copyright (C) 2011 Javier Martinez Canillas >> <martinez.javier@xxxxxxxxx> >> + * >> + * Multi-touch protocol type B support and cleanups by Javier Martinez >> Canillas >> + * >> + * This program is free software; you can redistribute it and/or >> + * modify it under the terms of the GNU General Public License >> + * version 2, and only version 2, as published by the >> + * Free Software Foundation. >> + * >> + * 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. >> + * >> + * You should have received a copy of the GNU General Public License >> along >> + * with this program; if not, write to the Free Software Foundation, >> Inc., >> + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. >> + * >> + * Contact Cypress Semiconductor at www.cypress.com <kev@xxxxxxxxxxx> >> + * >> + */ >> + >> +#include "cyttsp_core.h" >> + >> +#include <linux/i2c.h> >> +#include <linux/slab.h> >> + >> +#define CY_I2C_DATA_SIZE 128 >> + >> +struct cyttsp_i2c { >> + struct cyttsp_bus_ops ops; >> + struct i2c_client *client; >> + void *ttsp_client; >> + u8 wr_buf[CY_I2C_DATA_SIZE]; >> +}; >> + >> +static s32 ttsp_i2c_read_block_data(void *handle, u8 addr, >> + u8 length, void *values) >> +{ >> + struct cyttsp_i2c *ts = container_of(handle, struct cyttsp_i2c, >> ops); >> + int retval = 0; >> + >> + retval = i2c_master_send(ts->client, &addr, 1); >> + if (retval < 0) >> + return retval; >> + >> + retval = i2c_master_recv(ts->client, values, length); >> + >> + return (retval < 0) ? retval : 0; > > Trivial comment: > Could we check the bytes written ? > Feel free to ignore if you feel so. Yes, you are right. I also think that I should check if retval == length and return an error code otherwise. >> >> +} >> + >> +static s32 ttsp_i2c_write_block_data(void *handle, u8 addr, >> + u8 length, const void *values) >> +{ >> + struct cyttsp_i2c *ts = container_of(handle, struct cyttsp_i2c, >> ops); >> + int retval; >> + >> + ts->wr_buf[0] = addr; >> + memcpy(&ts->wr_buf[1], values, length); >> + >> + retval = i2c_master_send(ts->client, ts->wr_buf, length+1); >> + >> + return (retval < 0) ? retval : 0; >> +} >> + >> +static s32 ttsp_i2c_tch_ext(void *handle, void *values) >> +{ >> + struct cyttsp_i2c *ts = container_of(handle, struct cyttsp_i2c, >> ops); >> + int retval = 0; >> + >> + /* >> + * TODO: Add custom touch extension handling code here >> + * set: retval < 0 for any returned system errors, >> + * retval = 0 if normal touch handling is required, >> + * retval > 0 if normal touch handling is *not* required >> + */ >> + if (!ts || !values) >> + retval = -EINVAL; >> + >> + return retval; >> +} >> + >> +static int __devinit cyttsp_i2c_probe(struct i2c_client *client, >> + const struct i2c_device_id *id) >> +{ >> + struct cyttsp_i2c *ts; >> + >> + if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) >> + return -EIO; >> + >> + /* allocate and clear memory */ >> + ts = kzalloc(sizeof(*ts), GFP_KERNEL); >> + if (!ts) { >> + dev_dbg(&client->dev, "%s: Error, kzalloc.\n", __func__); >> + return -ENOMEM; >> + } >> + >> + /* register driver_data */ >> + ts->client = client; >> + i2c_set_clientdata(client, ts); >> + ts->ops.write = ttsp_i2c_write_block_data; >> + ts->ops.read = ttsp_i2c_read_block_data; >> + ts->ops.ext = ttsp_i2c_tch_ext; >> + ts->ops.dev = &client->dev; >> + >> + ts->ttsp_client = cyttsp_core_init(&ts->ops, &client->dev, >> client->irq); >> + if (IS_ERR(ts->ttsp_client)) { >> + int retval = PTR_ERR(ts->ttsp_client); >> + kfree(ts); >> + return retval; >> + } >> + >> + dev_dbg(ts->ops.dev, "%s: Registration complete\n", __func__); >> + >> + return 0; >> +} >> + >> + >> +/* registered in driver struct */ >> +static int __devexit cyttsp_i2c_remove(struct i2c_client *client) >> +{ >> + struct cyttsp_i2c *ts; >> + >> + ts = i2c_get_clientdata(client); >> + cyttsp_core_release(ts->ttsp_client); >> + kfree(ts); >> + return 0; >> +} >> + >> +#ifdef CONFIG_PM >> +static int cyttsp_i2c_suspend(struct i2c_client *client, pm_message_t >> message) >> +{ >> + struct cyttsp_i2c *ts = i2c_get_clientdata(client); >> + >> + return cyttsp_suspend(ts->ttsp_client); >> +} >> + >> +static int cyttsp_i2c_resume(struct i2c_client *client) >> +{ >> + struct cyttsp_i2c *ts = i2c_get_clientdata(client); >> + >> + return cyttsp_resume(ts->ttsp_client); >> +} >> +#endif >> + >> +static const struct i2c_device_id cyttsp_i2c_id[] = { >> + { CY_I2C_NAME, 0 }, { } >> +}; >> + >> +static struct i2c_driver cyttsp_i2c_driver = { >> + .driver = { >> + .name = CY_I2C_NAME, >> + .owner = THIS_MODULE, >> + }, >> + .probe = cyttsp_i2c_probe, >> + .remove = __devexit_p(cyttsp_i2c_remove), >> + .id_table = cyttsp_i2c_id, >> +#ifdef CONFIG_PM >> + .suspend = cyttsp_i2c_suspend, >> + .resume = cyttsp_i2c_resume, >> +#endif > > How about moving to dev pm ops ? Yes, I can move there. Thank you for your comments, probably Henrik and others find issues too. I will include these two in the next version of the driver. >> >> +}; >> + >> +static int __init cyttsp_i2c_init(void) >> +{ >> + return i2c_add_driver(&cyttsp_i2c_driver); >> +} >> + >> +static void __exit cyttsp_i2c_exit(void) >> +{ >> + return i2c_del_driver(&cyttsp_i2c_driver); >> +} >> + >> +module_init(cyttsp_i2c_init); >> +module_exit(cyttsp_i2c_exit); >> + >> +MODULE_ALIAS("i2c:cyttsp"); >> +MODULE_LICENSE("GPL"); >> +MODULE_DESCRIPTION("Cypress TrueTouch(R) Standard Product (TTSP) I2C >> driver"); >> +MODULE_AUTHOR("Cypress"); >> +MODULE_DEVICE_TABLE(i2c, cyttsp_i2c_id); >> -- >> 1.7.4.1 >> >> -- >> 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 > > Best regards, -- Javier Martínez Canillas (+34) 682 39 81 69 Barcelona, Spain -- 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