Re: [PATCH 4/4] Input: synaptics-rmi4 - switch to using i2c_transfer()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 01/09/2014 11:44 PM, Dmitry Torokhov wrote:
Instead of using 2 separate transactions when reading from the device let's
use i2c_transfer. Because we now have single point of failure I had to
change how we collect statistics. I elected to drop control data from the
stats and only track number of bytes read/written for the device data.

Also, since we are not prepared to deal with short reads and writes change
read_block_data and write_block_data to indicate error if we detect short
transfers.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
---
  drivers/input/rmi4/rmi_i2c.c | 71 ++++++++++++++++++++++++--------------------
  1 file changed, 39 insertions(+), 32 deletions(-)

diff --git a/drivers/input/rmi4/rmi_i2c.c b/drivers/input/rmi4/rmi_i2c.c
index c176218..51f5bc8 100644
--- a/drivers/input/rmi4/rmi_i2c.c
+++ b/drivers/input/rmi4/rmi_i2c.c
@@ -57,22 +57,17 @@ struct rmi_i2c_xport {
   */
  static int rmi_set_page(struct rmi_i2c_xport *rmi_i2c, u8 page)
  {
-	struct rmi_transport_dev *xport = &rmi_i2c->xport;
  	struct i2c_client *client = rmi_i2c->client;
  	u8 txbuf[2] = {RMI_PAGE_SELECT_REGISTER, page};
  	int retval;

-	dev_dbg(&client->dev, "writes 3 bytes: %02x %02x\n",
-			txbuf[0], txbuf[1]);
-	xport->stats.tx_count++;
-	xport->stats.tx_bytes += sizeof(txbuf);
  	retval = i2c_master_send(client, txbuf, sizeof(txbuf));
  	if (retval != sizeof(txbuf)) {
-		xport->stats.tx_errs++;
  		dev_err(&client->dev,
  			"%s: set page failed: %d.", __func__, retval);
  		return (retval < 0) ? retval : -EIO;
  	}
+
  	rmi_i2c->page = page;
  	return 0;
  }
@@ -107,22 +102,27 @@ static int rmi_i2c_write_block(struct rmi_transport_dev *xport, u16 addr,

  	if (RMI_I2C_PAGE(addr) != rmi_i2c->page) {
  		retval = rmi_set_page(rmi_i2c, RMI_I2C_PAGE(addr));
-		if (retval < 0)
+		if (retval)
  			goto exit;
  	}

+	retval = i2c_master_send(client, rmi_i2c->tx_buf, tx_size);
+	if (retval == tx_size)
+		retval = 0;
+	else if (retval >= 0)
+		retval = -EIO;
+
+exit:
  	dev_dbg(&client->dev,
-		"writes %zd bytes at %#06x: %*ph\n", len, addr, (int)len, buf);
+		"write %zd bytes at %#06x: %d (%*ph)\n",
+		len, addr, retval, (int)len, buf);

  	xport->stats.tx_count++;
-	xport->stats.tx_bytes += tx_size;
-	retval = i2c_master_send(client, rmi_i2c->tx_buf, tx_size);
-	if (retval < 0)
+	if (retval)
  		xport->stats.tx_errs++;
  	else
-		retval--; /* don't count the address byte */
+		xport->stats.tx_bytes += len;

-exit:
  	mutex_unlock(&rmi_i2c->page_mutex);
  	return retval;
  }
@@ -133,40 +133,47 @@ static int rmi_i2c_read_block(struct rmi_transport_dev *xport, u16 addr,
  	struct rmi_i2c_xport *rmi_i2c =
  		container_of(xport, struct rmi_i2c_xport, xport);
  	struct i2c_client *client = rmi_i2c->client;
-	u8 txbuf[1] = {addr & 0xff};
+	u8 addr_offset = addr & 0xff;
  	int retval;
+	struct i2c_msg msgs[] = {
+		{
+			.addr	= client->addr,
+			.len	= sizeof(addr_offset),
+			.buf	= &addr_offset,
+		},
+		{
+			.addr	= client->addr,
+			.flags	= I2C_M_RD,
+			.len	= len,
+			.buf	= buf,
+		},
+	};

  	mutex_lock(&rmi_i2c->page_mutex);

  	if (RMI_I2C_PAGE(addr) != rmi_i2c->page) {
  		retval = rmi_set_page(rmi_i2c, RMI_I2C_PAGE(addr));
-		if (retval < 0)
+		if (retval)
  			goto exit;
  	}

-	dev_dbg(&client->dev, "writes 1 bytes: %02x\n", txbuf[0]);
+	retval = i2c_transfer(client->adapter, msgs, sizeof(msgs));
+	if (retval == sizeof(msgs))

I think this should be:
	retval = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
	if (retval == ARRAY_SIZE(msgs))
At least, that change resolved some random misbehaviors, including kernel panics.


+		retval = 0; /* success */
+	else if (retval >= 0)
+		retval = -EIO;

-	xport->stats.tx_count++;
-	xport->stats.tx_bytes += sizeof(txbuf);
-	retval = i2c_master_send(client, txbuf, sizeof(txbuf));
-	if (retval != sizeof(txbuf)) {
-		xport->stats.tx_errs++;
-		retval = (retval < 0) ? retval : -EIO;
-		goto exit;
-	}
+exit:
+	dev_dbg(&client->dev,
+		"read %zd bytes at %#06x: %d (%*ph)\n",
+		len, addr, retval, (int)len, buf);

  	xport->stats.rx_count++;
-	xport->stats.rx_bytes += len;
-
-	retval = i2c_master_recv(client, buf, len);
-	if (retval < 0)
+	if (retval)
  		xport->stats.rx_errs++;
  	else
-		dev_dbg(&client->dev,
-			"read %zd bytes at %#06x: %*ph\n",
-			len, addr, (int)len, buf);
+		xport->stats.rx_bytes += len;

-exit:
  	mutex_unlock(&rmi_i2c->page_mutex);
  	return retval;
  }



--

Christopher Heiny
Senior Staff Firmware Engineer
Synaptics Incorporated
--
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




[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux