[U-Boot] [PATCH] 2c: modify i2c_read API to handle multi-bytes writes

[U-Boot] [PATCH] 2c: modify i2c_read API to handle multi-bytes writes

[Shaveta Leekha]

Most of the I2C slaves support accesses in the typical style
viz.read/write series of bytes at particular address offset.
These transactions are currently supportd in the
i2c driver using i2c_read and i2c_write APIs. I2C EEPROMs,
RTC, etc fall in this category.
The transactions look like:"
START:Address:Tx:Offset:RESTART:Address[0..4]:Tx/Rx:data[0..n]:STOP"

However there are certain devices which support accesses in
terms of the transactions as follows:
"START:Address:Tx:Txdata[0..n1]:Clock_stretching:
        RESTART:Address:Rx:data[0..n2]"

The Txdata is typically a command and some associated data,
similarly Rxdata could be command status plus some data received
as a response to the command sent.
i2c_read() function has been modified to handle
both types of transactions:
the one that writes only offset/address before read and other that
writes some bytes(more than 4 bytes) before read

To handle the case:
Negative equivalent of length has been passed and
interpreted accordinglt and txdata is being passed in
rxdata buffer

Signed-off-by: Shaveta Leekha <shaveta@freescale.com>
Signed-off-by: Poonam Aggrwal <poonam.aggrwal@freescale.com>
---
 drivers/i2c/fsl_i2c.c |   40 +++++++++++++++++++++++++++++++---------
 1 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/drivers/i2c/fsl_i2c.c b/drivers/i2c/fsl_i2c.c
index 291ad94..14c66d0 100644
--- a/drivers/i2c/fsl_i2c.c
+++ b/drivers/i2c/fsl_i2c.c
@@ -424,17 +424,39 @@ fsl_i2c_read(struct i2c_adapter *adap, u8 dev, uint addr, int alen, u8 *data,
 	int i = -1; /* signal error */
 	u8 *a = (u8*)&addr;
 
-	if (i2c_wait4bus(adap) < 0)
-		return -1;
+	if (alen < 0) {
+		int len = alen * -1;
+		if (i2c_wait4bus(adap) < 0)
+			return -1;
+
+		/* Generate a START and send the Address and
+		 * the Tx Bytes to the slave.
+		 * "START: Address: Write bytes wdata[wlength]"
+		 * It supports writing any number of bytes in contrast
+		 * to the else part, which supports writing address offset
+		 * of upto 4 bytes only.
+		 */
+		if (i2c_write_addr(adap, dev, I2C_WRITE_BIT, 0) != 0)
+			i = __i2c_write(adap, data, len);
+
+		if (i != len)
+			return -1;
+
+		if (length && i2c_write_addr(adap, dev, I2C_READ_BIT, 1) != 0)
+			i = __i2c_read(adap, data, length);
+	} else {
+		if (i2c_wait4bus(adap) < 0)
+			return -1;
 
-	if ((!length || alen > 0)
-	    && i2c_write_addr(adap, dev, I2C_WRITE_BIT, 0) != 0
-	    && __i2c_write(adap, &a[4 - alen], alen) == alen)
-		i = 0; /* No error so far */
+		if ((!length || alen > 0) &&
+		    i2c_write_addr(adap, dev, I2C_WRITE_BIT, 0) != 0  &&
+		    __i2c_write(adap, &a[4 - alen], alen) == alen)
+			i = 0; /* No error so far */
 
-	if (length &&
-	    i2c_write_addr(adap, dev, I2C_READ_BIT, alen ? 1 : 0) != 0)
-		i = __i2c_read(adap, data, length);
+		if (length &&
+		    i2c_write_addr(adap, dev, I2C_READ_BIT, alen ? 1 : 0) != 0)
+			i = __i2c_read(adap, data, length);
+	}
 
 	writeb(I2C_CR_MEN, &device->cr);
 
-- 
1.7.6.GIT

[U-Boot] [PATCH] 2c: modify i2c_read API to handle multi-bytes writes

[York Sun]

On 04/04/2014 03:08 AM, Shaveta Leekha wrote:
> Most of the I2C slaves support accesses in the typical style
> viz.read/write series of bytes at particular address offset.

viz?


> These transactions are currently supportd in the
> i2c driver using i2c_read and i2c_write APIs. I2C EEPROMs,
> RTC, etc fall in this category.
> The transactions look like:"
> START:Address:Tx:Offset:RESTART:Address[0..4]:Tx/Rx:data[0..n]:STOP"
> 
> However there are certain devices which support accesses in
> terms of the transactions as follows:
> "START:Address:Tx:Txdata[0..n1]:Clock_stretching:
>         RESTART:Address:Rx:data[0..n2]"
> 
> The Txdata is typically a command and some associated data,
> similarly Rxdata could be command status plus some data received
> as a response to the command sent.
> i2c_read() function has been modified to handle
> both types of transactions:
> the one that writes only offset/address before read and other that
> writes some bytes(more than 4 bytes) before read

Please rephrase this sentence. It is not very clear.


> 
> To handle the case:
> Negative equivalent of length has been passed and
> interpreted accordinglt and txdata is being passed in
> rxdata buffer

This belongs to inline comments. You should explain more in detail in the
comment. Commit message helps but it is hard to find in long term.


> 
> Signed-off-by: Shaveta Leekha <shaveta@freescale.com>
> Signed-off-by: Poonam Aggrwal <poonam.aggrwal@freescale.com>
> ---
>  drivers/i2c/fsl_i2c.c |   40 +++++++++++++++++++++++++++++++---------
>  1 files changed, 31 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/i2c/fsl_i2c.c b/drivers/i2c/fsl_i2c.c
> index 291ad94..14c66d0 100644
> --- a/drivers/i2c/fsl_i2c.c
> +++ b/drivers/i2c/fsl_i2c.c
> @@ -424,17 +424,39 @@ fsl_i2c_read(struct i2c_adapter *adap, u8 dev, uint addr, int alen, u8 *data,
>  	int i = -1; /* signal error */
>  	u8 *a = (u8*)&addr;
>  
> -	if (i2c_wait4bus(adap) < 0)
> -		return -1;

You don't need to move these two lines.


> +	if (alen < 0) {

You need to put a lot of comment to explain here.

> +		int len = alen * -1;

You don't really need a new variable here. Even you do, put the variable
declaration to the beginning of the function.

> +		if (i2c_wait4bus(adap) < 0)
> +			return -1;
> +
> +		/* Generate a START and send the Address and
> +		 * the Tx Bytes to the slave.
> +		 * "START: Address: Write bytes wdata[wlength]"
> +		 * It supports writing any number of bytes in contrast
> +		 * to the else part, which supports writing address offset
> +		 * of upto 4 bytes only.
> +		 */
> +		if (i2c_write_addr(adap, dev, I2C_WRITE_BIT, 0) != 0)
> +			i = __i2c_write(adap, data, len);

And explain how you use "data".

York