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

[Christopher Heiny]

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.

We tried this change once before a couple of years ago, but the 
conversion was unsuccessful on some older platforms.  I've tested it on 
some more current platforms, though, and it works there.  The old 
platforms are running 2.6.xx series kernels, and don't look likely ever 
to be updated, So....

Acked-by: Christopher Heiny <cheiny@xxxxxxxxxxxxx>

> 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))
> +		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-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/