[PATCH 1/3] i2c: imx_lpi2c: Fix a typo in bus_i2c_receive

From: Fedor Ross fedor.ross@ifm.com
Fix a typo in a debug message. It should be 'for' not 'fot' .
Signed-off-by: Fedor Ross fedor.ross@ifm.com Cc: Heiko Schocher hs@denx.de Cc: Tom Rini trini@konsulko.com Cc: Marek Vasut marex@denx.de --- drivers/i2c/imx_lpi2c.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/imx_lpi2c.c b/drivers/i2c/imx_lpi2c.c index a1be841b119..3c43f61d819 100644 --- a/drivers/i2c/imx_lpi2c.c +++ b/drivers/i2c/imx_lpi2c.c @@ -130,7 +130,7 @@ static int bus_i2c_receive(struct udevice *bus, u8 *rxbuf, int len)
result = bus_i2c_wait_for_tx_ready(regs); if (result) { - debug("i2c: receive wait fot tx ready: %d\n", result); + debug("i2c: receive wait for tx ready: %d\n", result); return result; }

From: Fedor Ross fedor.ross@ifm.com
Instead of using the hard-coded bus speed value I2C_SPEED_STANDARD_RATE, use the actual configured bus speed. This way the bus speed doesn't change suddenly after calling the imx_lpi2c_probe_chip() function for example.
Signed-off-by: Fedor Ross fedor.ross@ifm.com Cc: Heiko Schocher hs@denx.de Cc: Tom Rini trini@konsulko.com Cc: Marek Vasut marex@denx.de --- drivers/i2c/imx_lpi2c.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/imx_lpi2c.c b/drivers/i2c/imx_lpi2c.c index 3c43f61d819..7b10b2b252d 100644 --- a/drivers/i2c/imx_lpi2c.c +++ b/drivers/i2c/imx_lpi2c.c @@ -163,6 +163,7 @@ static int bus_i2c_receive(struct udevice *bus, u8 *rxbuf, int len) static int bus_i2c_start(struct udevice *bus, u8 addr, u8 dir) { lpi2c_status_t result; + struct dm_i2c_bus *i2c = dev_get_uclass_priv(bus); struct imx_lpi2c_bus *i2c_bus = dev_get_priv(bus); struct imx_lpi2c_reg *regs = (struct imx_lpi2c_reg *)(i2c_bus->base); u32 val; @@ -172,7 +173,7 @@ static int bus_i2c_start(struct udevice *bus, u8 addr, u8 dir) debug("i2c: start check busy bus: 0x%x\n", result);
/* Try to init the lpi2c then check the bus busy again */ - bus_i2c_init(bus, I2C_SPEED_STANDARD_RATE); + bus_i2c_init(bus, i2c->speed_hz); result = imx_lpci2c_check_busy_bus(regs); if (result) { printf("i2c: Error check busy bus: 0x%x\n", result); @@ -383,18 +384,19 @@ static int bus_i2c_init(struct udevice *bus, int speed) static int imx_lpi2c_probe_chip(struct udevice *bus, u32 chip, u32 chip_flags) { + struct dm_i2c_bus *i2c = dev_get_uclass_priv(bus); lpi2c_status_t result;
result = bus_i2c_start(bus, chip, 0); if (result) { bus_i2c_stop(bus); - bus_i2c_init(bus, I2C_SPEED_STANDARD_RATE); + bus_i2c_init(bus, i2c->speed_hz); return result; }
result = bus_i2c_stop(bus); if (result) - bus_i2c_init(bus, I2C_SPEED_STANDARD_RATE); + bus_i2c_init(bus, i2c->speed_hz);
return result; } @@ -439,6 +441,7 @@ __weak int enable_i2c_clk(unsigned char enable, unsigned int i2c_num)
static int imx_lpi2c_probe(struct udevice *bus) { + struct dm_i2c_bus *i2c = dev_get_uclass_priv(bus); struct imx_lpi2c_bus *i2c_bus = dev_get_priv(bus); fdt_addr_t addr; int ret; @@ -489,7 +492,7 @@ static int imx_lpi2c_probe(struct udevice *bus) return ret; }
- ret = bus_i2c_init(bus, I2C_SPEED_STANDARD_RATE); + ret = bus_i2c_init(bus, i2c->speed_hz); if (ret < 0) return ret;

On 7/24/24 11:59 AM, fedorross@gmail.com wrote:
From: Fedor Ross fedor.ross@ifm.com
Instead of using the hard-coded bus speed value I2C_SPEED_STANDARD_RATE, use the actual configured bus speed. This way the bus speed doesn't change suddenly after calling the imx_lpi2c_probe_chip() function for example.
Signed-off-by: Fedor Ross fedor.ross@ifm.com Cc: Heiko Schocher hs@denx.de Cc: Tom Rini trini@konsulko.com Cc: Marek Vasut marex@denx.de
drivers/i2c/imx_lpi2c.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/imx_lpi2c.c b/drivers/i2c/imx_lpi2c.c index 3c43f61d819..7b10b2b252d 100644 --- a/drivers/i2c/imx_lpi2c.c +++ b/drivers/i2c/imx_lpi2c.c @@ -163,6 +163,7 @@ static int bus_i2c_receive(struct udevice *bus, u8 *rxbuf, int len) static int bus_i2c_start(struct udevice *bus, u8 addr, u8 dir) { lpi2c_status_t result;
- struct dm_i2c_bus *i2c = dev_get_uclass_priv(bus); struct imx_lpi2c_bus *i2c_bus = dev_get_priv(bus); struct imx_lpi2c_reg *regs = (struct imx_lpi2c_reg *)(i2c_bus->base); u32 val;
@@ -172,7 +173,7 @@ static int bus_i2c_start(struct udevice *bus, u8 addr, u8 dir) debug("i2c: start check busy bus: 0x%x\n", result);
/* Try to init the lpi2c then check the bus busy again */
bus_i2c_init(bus, I2C_SPEED_STANDARD_RATE);
bus_i2c_init(bus, i2c->speed_hz);
Drop the second parameter of bus_i2c_init(). In bus_i2c_init(), do:
struct dm_i2c_bus *i2c = dev_get_uclass_priv(bus); int speed = i2c->speed_hz;
That should clean the proliferation of "speed" variable.

From: Fedor Ross fedor.ross@ifm.com
The TXFIFO register of LPI2C only has one byte length, and if the length of the data that needs to be read exceeds 256 bytes, it needs to be written to TXFIFO multiple times.
Signed-off-by: Fedor Ross fedor.ross@ifm.com Cc: Heiko Schocher hs@denx.de Cc: Tom Rini trini@konsulko.com Cc: Marek Vasut marex@denx.de --- drivers/i2c/imx_lpi2c.c | 72 +++++++++++++++++++++++++++-------------- 1 file changed, 47 insertions(+), 25 deletions(-)
diff --git a/drivers/i2c/imx_lpi2c.c b/drivers/i2c/imx_lpi2c.c index 7b10b2b252d..dd97228ddc7 100644 --- a/drivers/i2c/imx_lpi2c.c +++ b/drivers/i2c/imx_lpi2c.c @@ -19,6 +19,8 @@ #define LPI2C_NACK_TOUT_MS 1 #define LPI2C_TIMEOUT_MS 100
+#define CHUNK_DATA 256 + static int bus_i2c_init(struct udevice *bus, int speed);
/* Weak linked function for overridden by some SoC power function */ @@ -118,8 +120,10 @@ static int bus_i2c_send(struct udevice *bus, u8 *txbuf, int len)
static int bus_i2c_receive(struct udevice *bus, u8 *rxbuf, int len) { + struct dm_i2c_bus *i2c = dev_get_uclass_priv(bus); struct imx_lpi2c_bus *i2c_bus = dev_get_priv(bus); struct imx_lpi2c_reg *regs = (struct imx_lpi2c_reg *)(i2c_bus->base); + unsigned int chunk_len, rx_remain, timeout; lpi2c_status_t result = LPI2C_SUCESS; u32 val; ulong start_time = get_timer(0); @@ -128,33 +132,51 @@ static int bus_i2c_receive(struct udevice *bus, u8 *rxbuf, int len) if (!len) return result;
- result = bus_i2c_wait_for_tx_ready(regs); - if (result) { - debug("i2c: receive wait for tx ready: %d\n", result); - return result; - } + /* + * Extend the timeout for a bulk read if needed. + * The calculated timeout is the result of multiplying the + * transfer length with 8 bit + ACK + one clock of extra time, + * considering the I2C bus frequency. + */ + timeout = max(len * 10 * 1000 / i2c->speed_hz, LPI2C_TIMEOUT_MS);
- /* clear all status flags */ - writel(0x7f00, ®s->msr); - /* send receive command */ - val = LPI2C_MTDR_CMD(0x1) | LPI2C_MTDR_DATA(len - 1); - writel(val, ®s->mtdr); + rx_remain = len; + while (rx_remain > 0) { + chunk_len = clamp(rx_remain, 1, CHUNK_DATA) - 1;
- while (len--) { - do { - result = imx_lpci2c_check_clear_error(regs); - if (result) { - debug("i2c: receive check clear error: %d\n", - result); - return result; - } - if (get_timer(start_time) > LPI2C_TIMEOUT_MS) { - debug("i2c: receive mrdr: timeout\n"); - return -1; - } - val = readl(®s->mrdr); - } while (val & LPI2C_MRDR_RXEMPTY_MASK); - *rxbuf++ = LPI2C_MRDR_DATA(val); + result = bus_i2c_wait_for_tx_ready(regs); + if (result) { + debug("i2c: receive wait for tx ready: %d\n", result); + return result; + } + + /* clear all status flags */ + writel(0x7f00, ®s->msr); + /* send receive command */ + val = LPI2C_MTDR_CMD(0x1) | LPI2C_MTDR_DATA(chunk_len); + writel(val, ®s->mtdr); + rx_remain = rx_remain - (chunk_len & 0xff) - 1; + + while (len--) { + do { + result = imx_lpci2c_check_clear_error(regs); + if (result) { + debug("i2c: receive check clear error: %d\n", + result); + return result; + } + if (get_timer(start_time) > timeout) { + debug("i2c: receive mrdr: timeout\n"); + return -1; + } + val = readl(®s->mrdr); + } while (val & LPI2C_MRDR_RXEMPTY_MASK); + *rxbuf++ = LPI2C_MRDR_DATA(val); + + /* send next receive command before controller NACKs last byte */ + if ((len - rx_remain) < 2 && rx_remain > 0) + break; + } }
return result;

On 7/24/24 11:59 AM, fedorross@gmail.com wrote:
From: Fedor Ross fedor.ross@ifm.com
The TXFIFO register of LPI2C only has one byte length, and if the length of the data that needs to be read exceeds 256 bytes, it needs to be written to TXFIFO multiple times.
Signed-off-by: Fedor Ross fedor.ross@ifm.com
Add ---
Cc: Heiko Schocher hs@denx.de Cc: Tom Rini trini@konsulko.com Cc: Marek Vasut marex@denx.de
drivers/i2c/imx_lpi2c.c | 72 +++++++++++++++++++++++++++-------------- 1 file changed, 47 insertions(+), 25 deletions(-)
diff --git a/drivers/i2c/imx_lpi2c.c b/drivers/i2c/imx_lpi2c.c index 7b10b2b252d..dd97228ddc7 100644 --- a/drivers/i2c/imx_lpi2c.c +++ b/drivers/i2c/imx_lpi2c.c @@ -19,6 +19,8 @@ #define LPI2C_NACK_TOUT_MS 1 #define LPI2C_TIMEOUT_MS 100
+#define CHUNK_DATA 256
Use LPI2C_CHUNK_DATA to namespace this macro.
static int bus_i2c_init(struct udevice *bus, int speed);
/* Weak linked function for overridden by some SoC power function */ @@ -118,8 +120,10 @@ static int bus_i2c_send(struct udevice *bus, u8 *txbuf, int len)
static int bus_i2c_receive(struct udevice *bus, u8 *rxbuf, int len) {
- struct dm_i2c_bus *i2c = dev_get_uclass_priv(bus); struct imx_lpi2c_bus *i2c_bus = dev_get_priv(bus); struct imx_lpi2c_reg *regs = (struct imx_lpi2c_reg *)(i2c_bus->base);
- unsigned int chunk_len, rx_remain, timeout; lpi2c_status_t result = LPI2C_SUCESS; u32 val; ulong start_time = get_timer(0);
@@ -128,33 +132,51 @@ static int bus_i2c_receive(struct udevice *bus, u8 *rxbuf, int len) if (!len) return result;
- result = bus_i2c_wait_for_tx_ready(regs);
- if (result) {
debug("i2c: receive wait for tx ready: %d\n", result);
return result;
- }
- /*
* Extend the timeout for a bulk read if needed.
* The calculated timeout is the result of multiplying the
* transfer length with 8 bit + ACK + one clock of extra time,
* considering the I2C bus frequency.
*/
- timeout = max(len * 10 * 1000 / i2c->speed_hz, LPI2C_TIMEOUT_MS);
- /* clear all status flags */
- writel(0x7f00, ®s->msr);
- /* send receive command */
- val = LPI2C_MTDR_CMD(0x1) | LPI2C_MTDR_DATA(len - 1);
- writel(val, ®s->mtdr);
- rx_remain = len;
- while (rx_remain > 0) {
chunk_len = clamp(rx_remain, 1, CHUNK_DATA) - 1;
- while (len--) {
do {
result = imx_lpci2c_check_clear_error(regs);
if (result) {
debug("i2c: receive check clear error: %d\n",
result);
return result;
}
if (get_timer(start_time) > LPI2C_TIMEOUT_MS) {
debug("i2c: receive mrdr: timeout\n");
return -1;
}
val = readl(®s->mrdr);
} while (val & LPI2C_MRDR_RXEMPTY_MASK);
*rxbuf++ = LPI2C_MRDR_DATA(val);
result = bus_i2c_wait_for_tx_ready(regs);
if (result) {
debug("i2c: receive wait for tx ready: %d\n", result);
return result;
}
/* clear all status flags */
writel(0x7f00, ®s->msr);
/* send receive command */
val = LPI2C_MTDR_CMD(0x1) | LPI2C_MTDR_DATA(chunk_len);
writel(val, ®s->mtdr);
writel(LPI2C_MTDR_CMD(0x1) | LPI2C_MTDR_DATA(chunk_len);, ®s->mtdr);
and drop the val = assignment here.
rx_remain = rx_remain - (chunk_len & 0xff) - 1;
while (len--) {
do {
result = imx_lpci2c_check_clear_error(regs);
if (result) {
debug("i2c: receive check clear error: %d\n",
result);
return result;
}
if (get_timer(start_time) > timeout) {
debug("i2c: receive mrdr: timeout\n");
return -1;
}
val = readl(®s->mrdr);
} while (val & LPI2C_MRDR_RXEMPTY_MASK);
*rxbuf++ = LPI2C_MRDR_DATA(val);
/* send next receive command before controller NACKs last byte */
if ((len - rx_remain) < 2 && rx_remain > 0)
break;
}}

On Wed, Jul 24, 2024 at 12:57 PM Marek Vasut marex@denx.de wrote:
Hello Marek,
I'll do the changes. Thanks for your input.
Best regards, Fedor
On 7/24/24 11:59 AM, fedorross@gmail.com wrote:
From: Fedor Ross fedor.ross@ifm.com
The TXFIFO register of LPI2C only has one byte length, and if the length of the data that needs to be read exceeds 256 bytes, it needs to be written to TXFIFO multiple times.
Signed-off-by: Fedor Ross fedor.ross@ifm.com
Add ---
Cc: Heiko Schocher hs@denx.de Cc: Tom Rini trini@konsulko.com Cc: Marek Vasut marex@denx.de
drivers/i2c/imx_lpi2c.c | 72 +++++++++++++++++++++++++++-------------- 1 file changed, 47 insertions(+), 25 deletions(-)
diff --git a/drivers/i2c/imx_lpi2c.c b/drivers/i2c/imx_lpi2c.c index 7b10b2b252d..dd97228ddc7 100644 --- a/drivers/i2c/imx_lpi2c.c +++ b/drivers/i2c/imx_lpi2c.c @@ -19,6 +19,8 @@ #define LPI2C_NACK_TOUT_MS 1 #define LPI2C_TIMEOUT_MS 100
+#define CHUNK_DATA 256
Use LPI2C_CHUNK_DATA to namespace this macro.
static int bus_i2c_init(struct udevice *bus, int speed);
/* Weak linked function for overridden by some SoC power function */ @@ -118,8 +120,10 @@ static int bus_i2c_send(struct udevice *bus, u8 *txbuf, int len)
static int bus_i2c_receive(struct udevice *bus, u8 *rxbuf, int len) {
struct dm_i2c_bus *i2c = dev_get_uclass_priv(bus); struct imx_lpi2c_bus *i2c_bus = dev_get_priv(bus); struct imx_lpi2c_reg *regs = (struct imx_lpi2c_reg *)(i2c_bus->base);
unsigned int chunk_len, rx_remain, timeout; lpi2c_status_t result = LPI2C_SUCESS; u32 val; ulong start_time = get_timer(0);
@@ -128,33 +132,51 @@ static int bus_i2c_receive(struct udevice *bus, u8 *rxbuf, int len) if (!len) return result;
result = bus_i2c_wait_for_tx_ready(regs);
if (result) {
debug("i2c: receive wait for tx ready: %d\n", result);
return result;
}
/*
* Extend the timeout for a bulk read if needed.
* The calculated timeout is the result of multiplying the
* transfer length with 8 bit + ACK + one clock of extra time,
* considering the I2C bus frequency.
*/
timeout = max(len * 10 * 1000 / i2c->speed_hz, LPI2C_TIMEOUT_MS);
/* clear all status flags */
writel(0x7f00, ®s->msr);
/* send receive command */
val = LPI2C_MTDR_CMD(0x1) | LPI2C_MTDR_DATA(len - 1);
writel(val, ®s->mtdr);
rx_remain = len;
while (rx_remain > 0) {
chunk_len = clamp(rx_remain, 1, CHUNK_DATA) - 1;
while (len--) {
do {
result = imx_lpci2c_check_clear_error(regs);
if (result) {
debug("i2c: receive check clear error: %d\n",
result);
return result;
}
if (get_timer(start_time) > LPI2C_TIMEOUT_MS) {
debug("i2c: receive mrdr: timeout\n");
return -1;
}
val = readl(®s->mrdr);
} while (val & LPI2C_MRDR_RXEMPTY_MASK);
*rxbuf++ = LPI2C_MRDR_DATA(val);
result = bus_i2c_wait_for_tx_ready(regs);
if (result) {
debug("i2c: receive wait for tx ready: %d\n", result);
return result;
}
/* clear all status flags */
writel(0x7f00, ®s->msr);
/* send receive command */
val = LPI2C_MTDR_CMD(0x1) | LPI2C_MTDR_DATA(chunk_len);
writel(val, ®s->mtdr);
writel(LPI2C_MTDR_CMD(0x1) | LPI2C_MTDR_DATA(chunk_len);, ®s->mtdr);
and drop the val = assignment here.
rx_remain = rx_remain - (chunk_len & 0xff) - 1;
while (len--) {
do {
result = imx_lpci2c_check_clear_error(regs);
if (result) {
debug("i2c: receive check clear error: %d\n",
result);
return result;
}
if (get_timer(start_time) > timeout) {
debug("i2c: receive mrdr: timeout\n");
return -1;
}
val = readl(®s->mrdr);
} while (val & LPI2C_MRDR_RXEMPTY_MASK);
*rxbuf++ = LPI2C_MRDR_DATA(val);
/* send next receive command before controller NACKs last byte */
if ((len - rx_remain) < 2 && rx_remain > 0)
break;
} }

On 7/24/24 11:59 AM, fedorross@gmail.com wrote:
From: Fedor Ross fedor.ross@ifm.com
Fix a typo in a debug message. It should be 'for' not 'fot' .
Signed-off-by: Fedor Ross fedor.ross@ifm.com
Please add '---' here for V2, so the CC list does not get included in commit message when the patch is applied.
Cc: Heiko Schocher hs@denx.de Cc: Tom Rini trini@konsulko.com Cc: Marek Vasut marex@denx.de
drivers/i2c/imx_lpi2c.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/imx_lpi2c.c b/drivers/i2c/imx_lpi2c.c index a1be841b119..3c43f61d819 100644 --- a/drivers/i2c/imx_lpi2c.c +++ b/drivers/i2c/imx_lpi2c.c @@ -130,7 +130,7 @@ static int bus_i2c_receive(struct udevice *bus, u8 *rxbuf, int len)
result = bus_i2c_wait_for_tx_ready(regs); if (result) {
debug("i2c: receive wait fot tx ready: %d\n", result);
debug("i2c: receive wait for tx ready: %d\n", result);
Reviewed-by: Marek Vasut marex@denx.de
participants (3)
-
Fedor Ross
-
fedorross@gmail.com
-
Marek Vasut