All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] rockchip: i2c: fix >32 byte reads
@ 2017-08-03 11:48 Wadim Egorov
  2017-08-04 22:43 ` [U-Boot] " Philipp Tomsich
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Wadim Egorov @ 2017-08-03 11:48 UTC (permalink / raw)
  To: u-boot

The hw can read up to 32 bytes at a time. If we need
more than one chunk, we have to enter the plain RX mode.

Signed-off-by: Wadim Egorov <w.egorov@phytec.de>
---
 drivers/i2c/rk_i2c.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/rk_i2c.c b/drivers/i2c/rk_i2c.c
index 8bc045a..68e6653 100644
--- a/drivers/i2c/rk_i2c.c
+++ b/drivers/i2c/rk_i2c.c
@@ -164,6 +164,7 @@ static int rk_i2c_read(struct rk_i2c *i2c, uchar chip, uint reg, uint r_len,
 	uint rxdata;
 	uint i, j;
 	int err;
+	bool snd_chunk = false;
 
 	debug("rk_i2c_read: chip = %d, reg = %d, r_len = %d, b_len = %d\n",
 	      chip, reg, r_len, b_len);
@@ -184,15 +185,26 @@ static int rk_i2c_read(struct rk_i2c *i2c, uchar chip, uint reg, uint r_len,
 
 	while (bytes_remain_len) {
 		if (bytes_remain_len > RK_I2C_FIFO_SIZE) {
-			con = I2C_CON_EN | I2C_CON_MOD(I2C_MODE_TRX);
+			con = I2C_CON_EN;
 			bytes_xferred = 32;
 		} else {
-			con = I2C_CON_EN | I2C_CON_MOD(I2C_MODE_TRX) |
-				I2C_CON_LASTACK;
+			/*
+			 * The hw can read up to 32 bytes at a time. If we need
+			 * more than one chunk, send an ACK after the last byte.
+			 */
+			con = I2C_CON_EN | I2C_CON_LASTACK;
 			bytes_xferred = bytes_remain_len;
 		}
 		words_xferred = DIV_ROUND_UP(bytes_xferred, 4);
 
+		/*
+		 * make sure we are in plain RX mode if we read a second chunk
+		 */
+		if (snd_chunk)
+			con |= I2C_CON_MOD(I2C_MODE_RX);
+		else
+			con |= I2C_CON_MOD(I2C_MODE_TRX);
+
 		writel(con, &regs->con);
 		writel(bytes_xferred, &regs->mrxcnt);
 		writel(I2C_MBRFIEN | I2C_NAKRCVIEN, &regs->ien);
@@ -227,6 +239,7 @@ static int rk_i2c_read(struct rk_i2c *i2c, uchar chip, uint reg, uint r_len,
 		}
 
 		bytes_remain_len -= bytes_xferred;
+		snd_chunk = true;
 		debug("I2C Read bytes_remain_len %d\n", bytes_remain_len);
 	}
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [U-Boot] rockchip: i2c: fix >32 byte reads
  2017-08-03 11:48 [U-Boot] [PATCH] rockchip: i2c: fix >32 byte reads Wadim Egorov
@ 2017-08-04 22:43 ` Philipp Tomsich
  2017-08-18 13:06 ` Philipp Tomsich
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Philipp Tomsich @ 2017-08-04 22:43 UTC (permalink / raw)
  To: u-boot

> The hw can read up to 32 bytes at a time. If we need
> more than one chunk, we have to enter the plain RX mode.
> 
> Signed-off-by: Wadim Egorov <w.egorov@phytec.de>
> ---
>  drivers/i2c/rk_i2c.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 

Acked-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [U-Boot] rockchip: i2c: fix >32 byte reads
  2017-08-03 11:48 [U-Boot] [PATCH] rockchip: i2c: fix >32 byte reads Wadim Egorov
  2017-08-04 22:43 ` [U-Boot] " Philipp Tomsich
@ 2017-08-18 13:06 ` Philipp Tomsich
  2017-08-21  8:32   ` Wadim Egorov
  2017-09-05  9:10 ` Philipp Tomsich
  2017-09-05  9:26 ` Philipp Tomsich
  3 siblings, 1 reply; 6+ messages in thread
From: Philipp Tomsich @ 2017-08-18 13:06 UTC (permalink / raw)
  To: u-boot



On Thu, 3 Aug 2017, Wadim Egorov wrote:

> The hw can read up to 32 bytes at a time. If we need
> more than one chunk, we have to enter the plain RX mode.

Why does this need to be in 'plain RX' mode for more than 32 bytes?
What happens if someone wants to write/transmit more than 32 bytes?

>
> Signed-off-by: Wadim Egorov <w.egorov@phytec.de>
> Acked-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
> drivers/i2c/rk_i2c.c | 19 ++++++++++++++++---
> 1 file changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/i2c/rk_i2c.c b/drivers/i2c/rk_i2c.c
> index 8bc045a..68e6653 100644
> --- a/drivers/i2c/rk_i2c.c
> +++ b/drivers/i2c/rk_i2c.c
> @@ -164,6 +164,7 @@ static int rk_i2c_read(struct rk_i2c *i2c, uchar chip, uint reg, uint r_len,
> 	uint rxdata;
> 	uint i, j;
> 	int err;
> +	bool snd_chunk = false;
>
> 	debug("rk_i2c_read: chip = %d, reg = %d, r_len = %d, b_len = %d\n",
> 	      chip, reg, r_len, b_len);
> @@ -184,15 +185,26 @@ static int rk_i2c_read(struct rk_i2c *i2c, uchar chip, uint reg, uint r_len,
>
> 	while (bytes_remain_len) {
> 		if (bytes_remain_len > RK_I2C_FIFO_SIZE) {
> -			con = I2C_CON_EN | I2C_CON_MOD(I2C_MODE_TRX);
> +			con = I2C_CON_EN;
> 			bytes_xferred = 32;
> 		} else {
> -			con = I2C_CON_EN | I2C_CON_MOD(I2C_MODE_TRX) |
> -				I2C_CON_LASTACK;
> +			/*
> +			 * The hw can read up to 32 bytes at a time. If we need
> +			 * more than one chunk, send an ACK after the last byte.
> +			 */
> +			con = I2C_CON_EN | I2C_CON_LASTACK;
> 			bytes_xferred = bytes_remain_len;
> 		}
> 		words_xferred = DIV_ROUND_UP(bytes_xferred, 4);
>
> +		/*
> +		 * make sure we are in plain RX mode if we read a second chunk
> +		 */
> +		if (snd_chunk)
> +			con |= I2C_CON_MOD(I2C_MODE_RX);
> +		else
> +			con |= I2C_CON_MOD(I2C_MODE_TRX);
> +
> 		writel(con, &regs->con);
> 		writel(bytes_xferred, &regs->mrxcnt);
> 		writel(I2C_MBRFIEN | I2C_NAKRCVIEN, &regs->ien);
> @@ -227,6 +239,7 @@ static int rk_i2c_read(struct rk_i2c *i2c, uchar chip, uint reg, uint r_len,
> 		}
>
> 		bytes_remain_len -= bytes_xferred;
> +		snd_chunk = true;
> 		debug("I2C Read bytes_remain_len %d\n", bytes_remain_len);
> 	}
>
>

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [U-Boot] rockchip: i2c: fix >32 byte reads
  2017-08-18 13:06 ` Philipp Tomsich
@ 2017-08-21  8:32   ` Wadim Egorov
  0 siblings, 0 replies; 6+ messages in thread
From: Wadim Egorov @ 2017-08-21  8:32 UTC (permalink / raw)
  To: u-boot



Am 18.08.2017 um 15:06 schrieb Philipp Tomsich:
>
>
> On Thu, 3 Aug 2017, Wadim Egorov wrote:
>
>> The hw can read up to 32 bytes at a time. If we need
>> more than one chunk, we have to enter the plain RX mode.
>
> Why does this need to be in 'plain RX' mode for more than 32 bytes?

I am not sure why this need to be in receive only mode. Probably that's
how the I2C controller operates for more than 32 byte reads?
Anyway, if you try to read more than 32 bytes from an EEPROM, you will
get RXDATA registers filled with 0xFF in the following chunks without
this patch.
Same thing is done in the linux i2c driver:
 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/i2c/busses/i2c-rk3x.c?h=v4.13-rc5#n322


> What happens if someone wants to write/transmit more than 32 bytes?

Should not affect other operations as the I2C_CON_MODE will be set
properly in rk_i2c_read/write() calls before every I2C transmit.

Regards,
Wadim

>
>>
>> Signed-off-by: Wadim Egorov <w.egorov@phytec.de>
>> Acked-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
>> ---
>> drivers/i2c/rk_i2c.c | 19 ++++++++++++++++---
>> 1 file changed, 16 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/i2c/rk_i2c.c b/drivers/i2c/rk_i2c.c
>> index 8bc045a..68e6653 100644
>> --- a/drivers/i2c/rk_i2c.c
>> +++ b/drivers/i2c/rk_i2c.c
>> @@ -164,6 +164,7 @@ static int rk_i2c_read(struct rk_i2c *i2c, uchar
>> chip, uint reg, uint r_len,
>>     uint rxdata;
>>     uint i, j;
>>     int err;
>> +    bool snd_chunk = false;
>>
>>     debug("rk_i2c_read: chip = %d, reg = %d, r_len = %d, b_len = %d\n",
>>           chip, reg, r_len, b_len);
>> @@ -184,15 +185,26 @@ static int rk_i2c_read(struct rk_i2c *i2c,
>> uchar chip, uint reg, uint r_len,
>>
>>     while (bytes_remain_len) {
>>         if (bytes_remain_len > RK_I2C_FIFO_SIZE) {
>> -            con = I2C_CON_EN | I2C_CON_MOD(I2C_MODE_TRX);
>> +            con = I2C_CON_EN;
>>             bytes_xferred = 32;
>>         } else {
>> -            con = I2C_CON_EN | I2C_CON_MOD(I2C_MODE_TRX) |
>> -                I2C_CON_LASTACK;
>> +            /*
>> +             * The hw can read up to 32 bytes at a time. If we need
>> +             * more than one chunk, send an ACK after the last byte.
>> +             */
>> +            con = I2C_CON_EN | I2C_CON_LASTACK;
>>             bytes_xferred = bytes_remain_len;
>>         }
>>         words_xferred = DIV_ROUND_UP(bytes_xferred, 4);
>>
>> +        /*
>> +         * make sure we are in plain RX mode if we read a second chunk
>> +         */
>> +        if (snd_chunk)
>> +            con |= I2C_CON_MOD(I2C_MODE_RX);
>> +        else
>> +            con |= I2C_CON_MOD(I2C_MODE_TRX);
>> +
>>         writel(con, &regs->con);
>>         writel(bytes_xferred, &regs->mrxcnt);
>>         writel(I2C_MBRFIEN | I2C_NAKRCVIEN, &regs->ien);
>> @@ -227,6 +239,7 @@ static int rk_i2c_read(struct rk_i2c *i2c, uchar
>> chip, uint reg, uint r_len,
>>         }
>>
>>         bytes_remain_len -= bytes_xferred;
>> +        snd_chunk = true;
>>         debug("I2C Read bytes_remain_len %d\n", bytes_remain_len);
>>     }
>>
>>

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [U-Boot] rockchip: i2c: fix >32 byte reads
  2017-08-03 11:48 [U-Boot] [PATCH] rockchip: i2c: fix >32 byte reads Wadim Egorov
  2017-08-04 22:43 ` [U-Boot] " Philipp Tomsich
  2017-08-18 13:06 ` Philipp Tomsich
@ 2017-09-05  9:10 ` Philipp Tomsich
  2017-09-05  9:26 ` Philipp Tomsich
  3 siblings, 0 replies; 6+ messages in thread
From: Philipp Tomsich @ 2017-09-05  9:10 UTC (permalink / raw)
  To: u-boot

> The hw can read up to 32 bytes at a time. If we need
> more than one chunk, we have to enter the plain RX mode.
> 
> Signed-off-by: Wadim Egorov <w.egorov@phytec.de>
> Acked-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
>  drivers/i2c/rk_i2c.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 

Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [U-Boot] rockchip: i2c: fix >32 byte reads
  2017-08-03 11:48 [U-Boot] [PATCH] rockchip: i2c: fix >32 byte reads Wadim Egorov
                   ` (2 preceding siblings ...)
  2017-09-05  9:10 ` Philipp Tomsich
@ 2017-09-05  9:26 ` Philipp Tomsich
  3 siblings, 0 replies; 6+ messages in thread
From: Philipp Tomsich @ 2017-09-05  9:26 UTC (permalink / raw)
  To: u-boot

> The hw can read up to 32 bytes at a time. If we need
> more than one chunk, we have to enter the plain RX mode.
> 
> Signed-off-by: Wadim Egorov <w.egorov@phytec.de>
> Acked-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
>  drivers/i2c/rk_i2c.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 

Applied to u-boot-rockchip, thanks!

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-09-05  9:26 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-03 11:48 [U-Boot] [PATCH] rockchip: i2c: fix >32 byte reads Wadim Egorov
2017-08-04 22:43 ` [U-Boot] " Philipp Tomsich
2017-08-18 13:06 ` Philipp Tomsich
2017-08-21  8:32   ` Wadim Egorov
2017-09-05  9:10 ` Philipp Tomsich
2017-09-05  9:26 ` Philipp Tomsich

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.