Re: [PATCH v5 2/3] Input: cyttsp - add support for Cypress TTSPtouchscreen I2C bus interface

[Javier Martinez Canillas]

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-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/