| Message ID | 1408643457-7126-1-git-send-email-addy.ke@rock-chips.com |
|---|---|
| State | Superseded |
| Headers | show |
Hello. On 08/21/2014 09:50 PM, Addy Ke wrote: > In rk3x SOC, the I2C controller can receive/transmit up to 32 bytes data > in one transaction, so the size of data to be write/read to/from > TXDATAx/RXDATAx must be less than or equal 32 bytes at a time. > Test on pinky board, elan receive 158 bytes data. > Signed-off-by: Addy Ke <addy.ke@rock-chips.com> > --- > drivers/i2c/busses/i2c-rk3x.c | 3 +++ > 1 file changed, 3 insertions(+) > diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c > index 69e1185..dc0aa64 100644 > --- a/drivers/i2c/busses/i2c-rk3x.c > +++ b/drivers/i2c/busses/i2c-rk3x.c > @@ -323,6 +323,9 @@ static void rk3x_i2c_handle_read(struct rk3x_i2c *i2c, unsigned int ipd) > /* ack interrupt */ > i2c_writel(i2c, REG_INT_MBRF, REG_IPD); > > + /* Can only handle a maximum of 32 bytes at a time */ > + len = (len > 32) ? 32 : len; Why not min(len, 32)? Or even: if (len > 32) len = 32; WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Aug 21, 2014 at 10:04:31PM +0400, Sergei Shtylyov wrote: > Hello. > > On 08/21/2014 09:50 PM, Addy Ke wrote: > > >In rk3x SOC, the I2C controller can receive/transmit up to 32 bytes data > >in one transaction, so the size of data to be write/read to/from > >TXDATAx/RXDATAx must be less than or equal 32 bytes at a time. > > >Test on pinky board, elan receive 158 bytes data. > > >Signed-off-by: Addy Ke <addy.ke@rock-chips.com> > >--- > > drivers/i2c/busses/i2c-rk3x.c | 3 +++ > > 1 file changed, 3 insertions(+) > > >diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c > >index 69e1185..dc0aa64 100644 > >--- a/drivers/i2c/busses/i2c-rk3x.c > >+++ b/drivers/i2c/busses/i2c-rk3x.c > >@@ -323,6 +323,9 @@ static void rk3x_i2c_handle_read(struct rk3x_i2c *i2c, unsigned int ipd) > > /* ack interrupt */ > > i2c_writel(i2c, REG_INT_MBRF, REG_IPD); > > > >+ /* Can only handle a maximum of 32 bytes at a time */ > >+ len = (len > 32) ? 32 : len; > > Why not min(len, 32)? Or even: > > if (len > 32) > len = 32; No silent trimming, please. The message should be rejected when the transfer is set up. We could assign -EOVERFLOW to that type of failures, so users will know. Sadly, I have seen other controllers having such limits :(
Hi, On Thursday 21 August 2014 at 13:17:38, Wolfram Sang wrote: > On Thu, Aug 21, 2014 at 10:04:31PM +0400, Sergei Shtylyov wrote: > > Hello. > > > > On 08/21/2014 09:50 PM, Addy Ke wrote: > > >In rk3x SOC, the I2C controller can receive/transmit up to 32 bytes data > > >in one transaction, so the size of data to be write/read to/from > > >TXDATAx/RXDATAx must be less than or equal 32 bytes at a time. > > > > > >Test on pinky board, elan receive 158 bytes data. > > > > > >Signed-off-by: Addy Ke <addy.ke@rock-chips.com> > > >--- > > > > > > drivers/i2c/busses/i2c-rk3x.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > >diff --git a/drivers/i2c/busses/i2c-rk3x.c > > >b/drivers/i2c/busses/i2c-rk3x.c > > >index 69e1185..dc0aa64 100644 > > >--- a/drivers/i2c/busses/i2c-rk3x.c > > >+++ b/drivers/i2c/busses/i2c-rk3x.c > > >@@ -323,6 +323,9 @@ static void rk3x_i2c_handle_read(struct rk3x_i2c > > >*i2c, unsigned int ipd)> > > > > /* ack interrupt */ > > > i2c_writel(i2c, REG_INT_MBRF, REG_IPD); > > > > > >+ /* Can only handle a maximum of 32 bytes at a time */ > > >+ len = (len > 32) ? 32 : len; > > > > > Why not min(len, 32)? Or even: > > if (len > 32) > > > > len = 32; > > No silent trimming, please. The message should be rejected when the transfer > is set up. We could assign -EOVERFLOW to that type of failures, so users > will know. Sadly, I have seen other controllers having such limits :( Actually, reads with >32 bytes of data are possible with the controller. The hw returns the data in 32 byte chunks. Our chunk handling code is just buggy. We always read the number of missing bytes from the current chunk, instead of truncating that to 32. The fix is correct. The following lines in the rk3x_i2c_handle_read() function already check if we got everything and trigger an additional chunk read if necessary: /* are we finished? */ if (i2c->processed == i2c->msg->len) rk3x_i2c_stop(i2c, i2c->error); else rk3x_i2c_prepare_read(i2c); I'd like a clearer syntax as suggested by Sergei, though. Apart from that, Acked-By: Max Schwarz <max.schwarz@online.de> Cheers, Max -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> Actually, reads with >32 bytes of data are possible with the controller. The > hw returns the data in 32 byte chunks. Our chunk handling code is just buggy. Understood. Then, I'd prefer 'chunk' in the commit message instead of 'transaction'.
diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c index 69e1185..dc0aa64 100644 --- a/drivers/i2c/busses/i2c-rk3x.c +++ b/drivers/i2c/busses/i2c-rk3x.c @@ -323,6 +323,9 @@ static void rk3x_i2c_handle_read(struct rk3x_i2c *i2c, unsigned int ipd) /* ack interrupt */ i2c_writel(i2c, REG_INT_MBRF, REG_IPD); + /* Can only handle a maximum of 32 bytes at a time */ + len = (len > 32) ? 32 : len; + /* read the data from receive buffer */ for (i = 0; i < len; ++i) { if (i % 4 == 0)
In rk3x SOC, the I2C controller can receive/transmit up to 32 bytes data in one transaction, so the size of data to be write/read to/from TXDATAx/RXDATAx must be less than or equal 32 bytes at a time. Test on pinky board, elan receive 158 bytes data. Signed-off-by: Addy Ke <addy.ke@rock-chips.com> --- drivers/i2c/busses/i2c-rk3x.c | 3 +++ 1 file changed, 3 insertions(+)