Re: [PATCH v6 3/3] Input: cyttsp - add support for Cypress TTSPtouchscreen SPI bus interface

From: Javier Martinez Canillas
Date: Tue Oct 18 2011 - 04:48:33 EST


On Tue, Oct 18, 2011 at 10:27 AM, Shubhrajyoti Datta
<omaplinuxkernel@xxxxxxxxx> wrote:
>
>
> On Sun, Oct 16, 2011 at 12:07 AM, <martinez.javier@xxxxxxxxx> wrote:
>>
>> From: Javier Martinez Canillas <martinez.javier@xxxxxxxxx>
>>
>> The driver is composed of a core driver that process the data sent by
>> the contacts and a set of bus specific interface modules.
>>
>> This patch add supports for the Cypress TTSP SPI bus interface.
>>
>> Signed-off-by: Javier Martinez Canillas <martinez.javier@xxxxxxxxx>
>> ---
>>
>> v2: Fix issues called out by Dmitry Torokhov
>> Â Â - Extract the IRQ from the spi client data and pass to
>> cyttsp_core_init()
>> Â Â - Remove the extra retries and limit the retries to the cyttsp_core.c
>> Â Â Â read/write block functions.
>> Â Â - Cleanup cyttsp_spi_xfer(), check ACK in write operation and fix
>> special
>> Â Â Â EIO case to show its meaning.
>>
>> 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_spi.c | Â295
>> +++++++++++++++++++++++++
>> Â1 files changed, 295 insertions(+), 0 deletions(-)
>> Âcreate mode 100644 drivers/input/touchscreen/cyttsp/cyttsp_spi.c
>>
>> diff --git a/drivers/input/touchscreen/cyttsp/cyttsp_spi.c
>> b/drivers/input/touchscreen/cyttsp/cyttsp_spi.c
>> new file mode 100644
>> index 0000000..66d6dc3
>> --- /dev/null
>> +++ b/drivers/input/touchscreen/cyttsp/cyttsp_spi.c
>> @@ -0,0 +1,295 @@
>> +/*
>> + * Source for:
>> + * Cypress TrueTouch(TM) Standard Product (TTSP) SPI 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/spi/spi.h>
>> +#include <linux/delay.h>
>> +
>> +#define CY_SPI_WR_OP Â Â Â0x00 /* r/~w */
>> +#define CY_SPI_RD_OP Â Â Â0x01
>> +#define CY_SPI_CMD_BYTES Â4
>> +#define CY_SPI_SYNC_BYTE Â2
>> +#define CY_SPI_SYNC_ACK1 Â0x62 /* from protocol v.2 */
>> +#define CY_SPI_SYNC_ACK2 Â0x9D /* from protocol v.2 */
>> +#define CY_SPI_DATA_SIZE Â128
>> +#define CY_SPI_DATA_BUF_SIZE (CY_SPI_CMD_BYTES + CY_SPI_DATA_SIZE)
>> +#define CY_SPI_BITS_PER_WORD 8
>> +
>> +struct cyttsp_spi {
>> + Â Â Â struct cyttsp_bus_ops bus_ops;
>> + Â Â Â struct spi_device *spi_client;
>> + Â Â Â void *ttsp_client;
>> + Â Â Â u8 wr_buf[CY_SPI_DATA_BUF_SIZE];
>> + Â Â Â u8 rd_buf[CY_SPI_DATA_BUF_SIZE];
>> +};
>> +
>> +static int cyttsp_spi_xfer(u8 op, struct cyttsp_spi *ts,
>> + Â Â Â Â Â Â Â Â Â Â Â Â Âu8 reg, u8 *buf, int length)
>> +{
>> + Â Â Â struct spi_message msg;
>> + Â Â Â struct spi_transfer xfer[2];
>> + Â Â Â u8 *wr_buf = ts->wr_buf;
>> + Â Â Â u8 *rd_buf = ts->rd_buf;
>> + Â Â Â int retval;
>> +
>> + Â Â Â if (length > CY_SPI_DATA_SIZE) {
>> + Â Â Â Â Â Â Â dev_dbg(ts->bus_ops.dev,
>> + Â Â Â Â Â Â Â Â Â Â Â "%s: length %d is too big.\n",
>> + Â Â Â Â Â Â Â Â Â Â Â __func__, length);
>> + Â Â Â Â Â Â Â return -EINVAL;
>> + Â Â Â }
>> +
>> + Â Â Â memset(wr_buf, 0, CY_SPI_DATA_BUF_SIZE);
>> + Â Â Â memset(rd_buf, 0, CY_SPI_DATA_BUF_SIZE);
>> +
>> + Â Â Â wr_buf[0] = 0x00; /* header byte 0 */
>> + Â Â Â wr_buf[1] = 0xFF; /* header byte 1 */
>> + Â Â Â wr_buf[2] = reg; Â/* reg index */
>> + Â Â Â wr_buf[3] = op; Â /* r/~w */
>> + Â Â Â if (op == CY_SPI_WR_OP)
>> + Â Â Â Â Â Â Â memcpy(wr_buf + CY_SPI_CMD_BYTES, buf, length);
>> +
>> + Â Â Â memset((void *)xfer, 0, sizeof(xfer));
>> + Â Â Â spi_message_init(&msg);
>> +
>> + Â Â Â /*
>> + Â Â Â Â We set both TX and RX buffers because Cypress TTSP
>> + Â Â Â Â requires full duplex operation.
>> + Â Â Â */
>> + Â Â Â xfer[0].tx_buf = wr_buf;
>> + Â Â Â xfer[0].rx_buf = rd_buf;
>> + Â Â Â if (op == CY_SPI_WR_OP) {
>> + Â Â Â Â Â Â Â xfer[0].len = length + CY_SPI_CMD_BYTES;
>> + Â Â Â Â Â Â Â spi_message_add_tail(&xfer[0], &msg);
>> + Â Â Â } else if (op == CY_SPI_RD_OP) {
>> + Â Â Â Â Â Â Â xfer[0].len = CY_SPI_CMD_BYTES;
>> + Â Â Â Â Â Â Â spi_message_add_tail(&xfer[0], &msg);
>> +
>> + Â Â Â Â Â Â Â xfer[1].rx_buf = buf;
>> + Â Â Â Â Â Â Â xfer[1].len = length;
>> + Â Â Â Â Â Â Â spi_message_add_tail(&xfer[1], &msg);
>> + Â Â Â }
>> +
>> + Â Â Â retval = spi_sync(ts->spi_client, &msg);
>> + Â Â Â if (retval < 0) {
>> + Â Â Â Â Â Â Â dev_dbg(ts->bus_ops.dev,
>> + Â Â Â Â Â Â Â Â Â Â Â "%s: spi_sync() error %d, len=%d, op=%d\n",
>> + Â Â Â Â Â Â Â Â Â Â Â __func__, retval, xfer[1].len, op);
>> +
>> + Â Â Â Â Â Â Â /*
>> + Â Â Â Â Â Â Â Â* do not return here since was a bad ACK sequence
>> + Â Â Â Â Â Â Â Â* let the following ACK check handle any errors and
>> + Â Â Â Â Â Â Â Â* allow silent retries
>> + Â Â Â Â Â Â Â Â*/
>> + Â Â Â }
>> +
>> + Â Â Â if ((rd_buf[CY_SPI_SYNC_BYTE] == CY_SPI_SYNC_ACK1) &&
>> + Â Â Â Â Â (rd_buf[CY_SPI_SYNC_BYTE+1] == CY_SPI_SYNC_ACK2))
>> + Â Â Â Â Â Â Â retval = 0;
>> + Â Â Â else {
>> + Â Â Â Â Â Â Â int i;
>> + Â Â Â Â Â Â Â for (i = 0; i < (CY_SPI_CMD_BYTES); i++)
>> + Â Â Â Â Â Â Â Â Â Â Â dev_dbg(ts->bus_ops.dev,
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â "%s: test rd_buf[%d]:0x%02x\n",
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â __func__, i, rd_buf[i]);
>> + Â Â Â Â Â Â Â for (i = 0; i < (length); i++)
>> + Â Â Â Â Â Â Â Â Â Â Â dev_dbg(ts->bus_ops.dev,
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â "%s: test buf[%d]:0x%02x\n",
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â __func__, i, buf[i]);
>> +
>> + Â Â Â Â Â Â Â /* signal ACK error so silent retry */
>> + Â Â Â Â Â Â Â retval = 1;
>> + Â Â Â }
>> +
>> + Â Â Â return retval;
>> +}
>> +
>> +static s32 ttsp_spi_read_block_data(void *handle, u8 addr,
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â u8 length, void *data)
>> +{
>> + Â Â Â struct cyttsp_spi *ts =
>> + Â Â Â Â Â Â Â container_of(handle, struct cyttsp_spi, bus_ops);
>> + Â Â Â int retval;
>> +
>> + Â Â Â retval = cyttsp_spi_xfer(CY_SPI_RD_OP, ts, addr, data, length);
>> + Â Â Â if (retval < 0)
>> + Â Â Â Â Â Â Â pr_err("%s: ttsp_spi_read_block_data failed\n",
>> + Â Â Â Â Â Â Â Â Â Â Â__func__);
>> +
>> + Â Â Â /*
>> + Â Â Â Â* Do not print the above error if the data sync bytes were not
>> found.
>> + Â Â Â Â* This is a normal condition for the bootloader loader startup
>> and need
>> + Â Â Â Â* to retry until data sync bytes are found.
>> + Â Â Â Â*/
>> + Â Â Â if (retval > 0)
>> + Â Â Â Â Â Â Â retval = -EIO; Â/* now signal fail; so retry can be done
>> */
>> +
>> + Â Â Â return retval;
>> +}
>> +
>> +static s32 ttsp_spi_write_block_data(void *handle, u8 addr,
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âu8 length, const void *data)
>> +{
>> + Â Â Â struct cyttsp_spi *ts =
>> + Â Â Â Â Â Â Â container_of(handle, struct cyttsp_spi, bus_ops);
>> + Â Â Â int retval;
>> +
>> + Â Â Â retval = cyttsp_spi_xfer(CY_SPI_WR_OP, ts, addr, (void *)data,
>> length);
>> + Â Â Â if (retval < 0)
>> + Â Â Â Â Â Â Â pr_err("%s: ttsp_spi_write_block_data failed\n",
>> + Â Â Â Â Â Â Â Â Â Â Â__func__);
>> +
>> + Â Â Â /*
>> + Â Â Â Â* Do not print the above error if the data sync bytes were not
>> found.
>> + Â Â Â Â* This is a normal condition for the bootloader loader startup
>> and need
>> + Â Â Â Â* to retry until data sync bytes are found.
>> + Â Â Â Â*/
>> + Â Â Â if (retval > 0)
>> + Â Â Â Â Â Â Â retval = -EIO; Â/* now signal fail; so retry can be done
>> */
>> +
>> + Â Â Â return retval;
>> +}
>> +
>> +static s32 ttsp_spi_tch_ext(void *handle, void *values)
>> +{
>> + Â Â Â struct cyttsp_spi *ts =
>> + Â Â Â Â Â Â Â container_of(handle, struct cyttsp_spi, bus_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
>> + Â Â Â Â*/
>> +
>
> This is an empty function and could be removed?
>
>

Hi Shubhrajyoti,

Yes, I'll remove it and also fix the other issues called out by you
and resend the patch-set as v7.

Thanks a lot for the review and 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/