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

Hello Dmitry,

On Sun, 2011-08-21 at 19:44 -0700, Dmitry Torokhov wrote:

Hi Javier,

quoted

+
+	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?

Both TX and RX buffers are set because Cypress requires full duplex
operation always.

quoted

+	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?

Fixed: In next version a following ACK check code return the status to
the cyttsp core so it can retry after waiting with an msleep() call.

quoted

+	}
+
+	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?

Fixed: Both read and write operations check the ACK bytes now.

quoted

+
+	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?

Fixed: The retries now are made in the cyttsp core so it works both for
SPI and I2C.

quoted

+
+	if (retval == -EIO)
+		return 0;

Why is -EIO special?

Fixed: Now special EIO has been fixed to show meaning.

quoted

+	else
+		return retval;
+}
+

Thanks.

-- 
Dmitry

Best regards,

Javier Martinez Canillas