| Message ID | 1396606108-8486-1-git-send-email-shaveta@freescale.com |
|---|---|
| State | Rejected |
| Delegated to: | Heiko Schocher |
| Headers | show |
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
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);
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(-)