lkml.org 
[lkml]   [2011]   [Oct]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH v5 2/3] Input: cyttsp - add support for Cypress TTSP touchscreen I2C bus interface
    On Mon, Oct 10, 2011 at 3:55 PM, Shubhrajyoti Datta
    <omaplinuxkernel@gmail.com> 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@gmail.com> 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@gmail.com>
    >> ---
    >>
    >> 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@gmail.com>
    >> + *
    >> + * 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@cypress.com>
    >> + *
    >> + */
    >> +
    >> +#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@vger.kernel.org
    >> 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@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2011-10-10 20:53    [W:0.048 / U:60.692 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site