lkml.org 
[lkml]   [2011]   [Aug]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 3/3] Input: cyttsp - add support for Cypress TTSP touchscreen SPI bus interface
Hi Javier,

On Sat, Aug 20, 2011 at 12:01:52AM -0400, Javier Martinez Canillas wrote:
> +
> +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->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);
> + xfer[0].tx_buf = wr_buf;
> + xfer[0].rx_buf = rd_buf;

Since we are setting both TX and RX buffer here does this mean that the
device will not work with controllers in half-duplex mode? What is the
amount of data that will be placed in 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_tmo(ts, &msg);
> + if (retval < 0) {
> + dev_dbg(ts->ops.dev, "%s: spi sync error %d, len=%d, op=%d\n",
> + __func__, retval, xfer[1].len, op);
> + retval = 0;

Why do we ignore the error?

> + }
> +
> + if (op == CY_SPI_RD_OP) {
> + if ((rd_buf[CY_SPI_SYNC_BYTE] == CY_SPI_SYNC_ACK1) &&
> + (rd_buf[CY_SPI_SYNC_BYTE+1] == CY_SPI_SYNC_ACK2))
> + retval = 0;

What about write operation? Does the device send anything in rd_buf for
write command?

> + else {
> + int i;
> + for (i = 0; i < (CY_SPI_CMD_BYTES); i++)
> + dev_dbg(ts->ops.dev,
> + "%s: test rd_buf[%d]:0x%02x\n",
> + __func__, i, rd_buf[i]);
> + for (i = 0; i < (length); i++)
> + dev_dbg(ts->ops.dev,
> + "%s: test buf[%d]:0x%02x\n",
> + __func__, i, buf[i]);
> + retval = 1;
> + }
> + }
> + return retval;
> +}
> +
> +static int cyttsp_spi_xfer(u8 op, struct cyttsp_spi *ts,
> + u8 reg, u8 *buf, int length)
> +{
> + int tries;
> + int retval;
> +
> + if (op == CY_SPI_RD_OP) {
> + for (tries = CY_NUM_RETRY; tries; tries--) {
> + retval = cyttsp_spi_xfer_(op, ts, reg, buf, length);
> + if (retval == 0)
> + break;
> + else
> + msleep(20);

Why do we need retry in SPI case but do not need it when device is
connected via I2C? And not on writes?

> + }
> + } else {
> + retval = cyttsp_spi_xfer_(op, ts, reg, buf, length);
> + }
> +
> + 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, ops);
> + int retval;
> +
> + retval = cyttsp_spi_xfer(CY_SPI_RD_OP, ts, addr, data, length);
> + if (retval < 0)
> + dev_dbg(ts->ops.dev, "%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 = -1; /* 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, ops);
> + int retval;
> +
> + retval = cyttsp_spi_xfer(CY_SPI_WR_OP, ts, addr, (void *)data, length);
> + if (retval < 0)
> + dev_dbg(ts->ops.dev, "%s: ttsp_spi_write_block_data failed\n",
> + __func__);
> +
> + if (retval == -EIO)
> + return 0;

Why is -EIO special?

> + else
> + return retval;
> +}
> +

Thanks.

--
Dmitry


\
 
 \ /
  Last update: 2011-08-22 04:47    [from the cache]
©2003-2011 Jasper Spaans