[PATCH v4 0/4] i2c: stm32: cleanup & stop handling fix
This series corrects the handling of the stop condition and cleanup few bits in the driver stm32f7_i2c.c
v4: additional patch to fix rise/fall timing settings v3: fix stop handling in patch 3/3 v2: update commit message in patch 3/3
Alain Volmat (3): i2c: stm32: fix comment and remove unused AUTOEND bit i2c: stm32: remove unused stop parameter in start & reload handling i2c: stm32: do not set the STOP condition on error
Jorge Ramirez-Ortiz (1): i2c: stm32: fix usage of rise/fall device tree properties
drivers/i2c/stm32f7_i2c.c | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-)
Comment within stm32_i2c_message_start is misleading, indicating that AUTOEND bit is setted while it is actually cleared. Moreover, the bit is actually never setted so there is no need to clear it hence get rid of this bit clear and the bit macro as well.
Signed-off-by: Alain Volmat alain.volmat@foss.st.com Reviewed-by: Patrick Delaunay patrick.delaunay@foss.st.com --- drivers/i2c/stm32f7_i2c.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/stm32f7_i2c.c b/drivers/i2c/stm32f7_i2c.c index bf2a6c9b4b..78d7156492 100644 --- a/drivers/i2c/stm32f7_i2c.c +++ b/drivers/i2c/stm32f7_i2c.c @@ -57,7 +57,6 @@ struct stm32_i2c_regs { #define STM32_I2C_CR1_PE BIT(0)
/* STM32 I2C control 2 */ -#define STM32_I2C_CR2_AUTOEND BIT(25) #define STM32_I2C_CR2_RELOAD BIT(24) #define STM32_I2C_CR2_NBYTES_MASK GENMASK(23, 16) #define STM32_I2C_CR2_NBYTES(n) ((n & 0xff) << 16) @@ -304,9 +303,8 @@ static void stm32_i2c_message_start(struct stm32_i2c_priv *i2c_priv, cr2 |= STM32_I2C_CR2_SADD7(msg->addr); }
- /* Set nb bytes to transfer and reload or autoend bits */ - cr2 &= ~(STM32_I2C_CR2_NBYTES_MASK | STM32_I2C_CR2_RELOAD | - STM32_I2C_CR2_AUTOEND); + /* Set nb bytes to transfer and reload (if needed) */ + cr2 &= ~(STM32_I2C_CR2_NBYTES_MASK | STM32_I2C_CR2_RELOAD); if (msg->len > STM32_I2C_MAX_LEN) { cr2 |= STM32_I2C_CR2_NBYTES(STM32_I2C_MAX_LEN); cr2 |= STM32_I2C_CR2_RELOAD;
Hello Alain,
On 12.09.22 10:41, Alain Volmat wrote:
Comment within stm32_i2c_message_start is misleading, indicating that AUTOEND bit is setted while it is actually cleared. Moreover, the bit is actually never setted so there is no need to clear it hence get rid of this bit clear and the bit macro as well.
Signed-off-by: Alain Volmat alain.volmat@foss.st.com Reviewed-by: Patrick Delaunay patrick.delaunay@foss.st.com
drivers/i2c/stm32f7_i2c.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
Reviewed-by: Heiko Schocher hs@denx.de
bye, Heiko
On 9/12/22 10:41, Alain Volmat wrote:
Comment within stm32_i2c_message_start is misleading, indicating that AUTOEND bit is setted while it is actually cleared. Moreover, the bit is actually never setted so there is no need to clear it hence get rid of this bit clear and the bit macro as well.
Signed-off-by: Alain Volmat alain.volmat@foss.st.com Reviewed-by: Patrick Delaunay patrick.delaunay@foss.st.com
drivers/i2c/stm32f7_i2c.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/stm32f7_i2c.c b/drivers/i2c/stm32f7_i2c.c index bf2a6c9b4b..78d7156492 100644 --- a/drivers/i2c/stm32f7_i2c.c +++ b/drivers/i2c/stm32f7_i2c.c @@ -57,7 +57,6 @@ struct stm32_i2c_regs { #define STM32_I2C_CR1_PE BIT(0)
/* STM32 I2C control 2 */ -#define STM32_I2C_CR2_AUTOEND BIT(25) #define STM32_I2C_CR2_RELOAD BIT(24) #define STM32_I2C_CR2_NBYTES_MASK GENMASK(23, 16) #define STM32_I2C_CR2_NBYTES(n) ((n & 0xff) << 16) @@ -304,9 +303,8 @@ static void stm32_i2c_message_start(struct stm32_i2c_priv *i2c_priv, cr2 |= STM32_I2C_CR2_SADD7(msg->addr); }
- /* Set nb bytes to transfer and reload or autoend bits */
- cr2 &= ~(STM32_I2C_CR2_NBYTES_MASK | STM32_I2C_CR2_RELOAD |
STM32_I2C_CR2_AUTOEND);
- /* Set nb bytes to transfer and reload (if needed) */
- cr2 &= ~(STM32_I2C_CR2_NBYTES_MASK | STM32_I2C_CR2_RELOAD); if (msg->len > STM32_I2C_MAX_LEN) { cr2 |= STM32_I2C_CR2_NBYTES(STM32_I2C_MAX_LEN); cr2 |= STM32_I2C_CR2_RELOAD;
Reviewed-by: Patrice Chotard patrice.chotard@foss.st.com
Thanks Patrice
Hi Alain,
On 9/12/22 10:41, Alain Volmat wrote:
Comment within stm32_i2c_message_start is misleading, indicating that AUTOEND bit is setted while it is actually cleared. Moreover, the bit is actually never setted so there is no need to clear it hence get rid of this bit clear and the bit macro as well.
Signed-off-by: Alain Volmat alain.volmat@foss.st.com Reviewed-by: Patrick Delaunay patrick.delaunay@foss.st.com
drivers/i2c/stm32f7_i2c.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/stm32f7_i2c.c b/drivers/i2c/stm32f7_i2c.c index bf2a6c9b4b..78d7156492 100644 --- a/drivers/i2c/stm32f7_i2c.c +++ b/drivers/i2c/stm32f7_i2c.c @@ -57,7 +57,6 @@ struct stm32_i2c_regs { #define STM32_I2C_CR1_PE BIT(0)
/* STM32 I2C control 2 */ -#define STM32_I2C_CR2_AUTOEND BIT(25) #define STM32_I2C_CR2_RELOAD BIT(24) #define STM32_I2C_CR2_NBYTES_MASK GENMASK(23, 16) #define STM32_I2C_CR2_NBYTES(n) ((n & 0xff) << 16) @@ -304,9 +303,8 @@ static void stm32_i2c_message_start(struct stm32_i2c_priv *i2c_priv, cr2 |= STM32_I2C_CR2_SADD7(msg->addr); }
- /* Set nb bytes to transfer and reload or autoend bits */
- cr2 &= ~(STM32_I2C_CR2_NBYTES_MASK | STM32_I2C_CR2_RELOAD |
STM32_I2C_CR2_AUTOEND);
- /* Set nb bytes to transfer and reload (if needed) */
- cr2 &= ~(STM32_I2C_CR2_NBYTES_MASK | STM32_I2C_CR2_RELOAD); if (msg->len > STM32_I2C_MAX_LEN) { cr2 |= STM32_I2C_CR2_NBYTES(STM32_I2C_MAX_LEN); cr2 |= STM32_I2C_CR2_RELOAD;
Applied to u-boot-stm/master, thanks!
I also add the missing Reviewed-by when I get the patch from patchwork
http://patchwork.ozlabs.org/project/uboot/patch/20220912084201.1826979-2-ala...
+ Reviewed-by: Heiko Schocher hs@denx.de + Reviewed-by: Patrice Chotard patrice.chotard@foss.st.com
Regards Patrick
Functions stm32_i2c_message_start and stm32_i2c_handle_reload both get a stop boolean indicating if the transfer should end with a STOP or not. However no specific handling is needed in those functions hence remove the parameter.
Signed-off-by: Alain Volmat alain.volmat@foss.st.com Reviewed-by: Patrick Delaunay patrick.delaunay@foss.st.com --- drivers/i2c/stm32f7_i2c.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/stm32f7_i2c.c b/drivers/i2c/stm32f7_i2c.c index 78d7156492..0ec67b5c12 100644 --- a/drivers/i2c/stm32f7_i2c.c +++ b/drivers/i2c/stm32f7_i2c.c @@ -282,7 +282,7 @@ static int stm32_i2c_check_device_busy(struct stm32_i2c_priv *i2c_priv) }
static void stm32_i2c_message_start(struct stm32_i2c_priv *i2c_priv, - struct i2c_msg *msg, bool stop) + struct i2c_msg *msg) { struct stm32_i2c_regs *regs = i2c_priv->regs; u32 cr2 = readl(®s->cr2); @@ -325,7 +325,7 @@ static void stm32_i2c_message_start(struct stm32_i2c_priv *i2c_priv, */
static void stm32_i2c_handle_reload(struct stm32_i2c_priv *i2c_priv, - struct i2c_msg *msg, bool stop) + struct i2c_msg *msg) { struct stm32_i2c_regs *regs = i2c_priv->regs; u32 cr2 = readl(®s->cr2); @@ -431,7 +431,7 @@ static int stm32_i2c_message_xfer(struct stm32_i2c_priv *i2c_priv, /* Add errors */ mask |= STM32_I2C_ISR_ERRORS;
- stm32_i2c_message_start(i2c_priv, msg, stop); + stm32_i2c_message_start(i2c_priv, msg);
while (msg->len) { /* @@ -469,7 +469,7 @@ static int stm32_i2c_message_xfer(struct stm32_i2c_priv *i2c_priv, mask = msg->flags & I2C_M_RD ? STM32_I2C_ISR_RXNE : STM32_I2C_ISR_TXIS | STM32_I2C_ISR_NACKF;
- stm32_i2c_handle_reload(i2c_priv, msg, stop); + stm32_i2c_handle_reload(i2c_priv, msg); } else if (!bytes_to_rw) { /* Wait until TC flag is set */ mask = STM32_I2C_ISR_TC;
Hello Alain,
On 12.09.22 10:41, Alain Volmat wrote:
Functions stm32_i2c_message_start and stm32_i2c_handle_reload both get a stop boolean indicating if the transfer should end with a STOP or not. However no specific handling is needed in those functions hence remove the parameter.
Signed-off-by: Alain Volmat alain.volmat@foss.st.com Reviewed-by: Patrick Delaunay patrick.delaunay@foss.st.com
drivers/i2c/stm32f7_i2c.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
Reviewed-by: Heiko Schocher hs@denx.de
bye, Heiko
On 9/12/22 10:41, Alain Volmat wrote:
Functions stm32_i2c_message_start and stm32_i2c_handle_reload both get a stop boolean indicating if the transfer should end with a STOP or not. However no specific handling is needed in those functions hence remove the parameter.
Signed-off-by: Alain Volmat alain.volmat@foss.st.com Reviewed-by: Patrick Delaunay patrick.delaunay@foss.st.com
drivers/i2c/stm32f7_i2c.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/stm32f7_i2c.c b/drivers/i2c/stm32f7_i2c.c index 78d7156492..0ec67b5c12 100644 --- a/drivers/i2c/stm32f7_i2c.c +++ b/drivers/i2c/stm32f7_i2c.c @@ -282,7 +282,7 @@ static int stm32_i2c_check_device_busy(struct stm32_i2c_priv *i2c_priv) }
static void stm32_i2c_message_start(struct stm32_i2c_priv *i2c_priv,
struct i2c_msg *msg, bool stop)
struct i2c_msg *msg){ struct stm32_i2c_regs *regs = i2c_priv->regs; u32 cr2 = readl(®s->cr2); @@ -325,7 +325,7 @@ static void stm32_i2c_message_start(struct stm32_i2c_priv *i2c_priv, */
static void stm32_i2c_handle_reload(struct stm32_i2c_priv *i2c_priv,
struct i2c_msg *msg, bool stop)
struct i2c_msg *msg){ struct stm32_i2c_regs *regs = i2c_priv->regs; u32 cr2 = readl(®s->cr2); @@ -431,7 +431,7 @@ static int stm32_i2c_message_xfer(struct stm32_i2c_priv *i2c_priv, /* Add errors */ mask |= STM32_I2C_ISR_ERRORS;
- stm32_i2c_message_start(i2c_priv, msg, stop);
stm32_i2c_message_start(i2c_priv, msg);
while (msg->len) { /*
@@ -469,7 +469,7 @@ static int stm32_i2c_message_xfer(struct stm32_i2c_priv *i2c_priv, mask = msg->flags & I2C_M_RD ? STM32_I2C_ISR_RXNE : STM32_I2C_ISR_TXIS | STM32_I2C_ISR_NACKF;
stm32_i2c_handle_reload(i2c_priv, msg, stop);
stm32_i2c_handle_reload(i2c_priv, msg);
Reviewed-by: Patrice Chotard patrice.chotard@foss.st.com
Thanks Patrice
Hi Alain,
On 9/12/22 10:41, Alain Volmat wrote:
Functions stm32_i2c_message_start and stm32_i2c_handle_reload both get a stop boolean indicating if the transfer should end with a STOP or not. However no specific handling is needed in those functions hence remove the parameter.
Signed-off-by: Alain Volmat alain.volmat@foss.st.com Reviewed-by: Patrick Delaunay patrick.delaunay@foss.st.com
drivers/i2c/stm32f7_i2c.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/stm32f7_i2c.c b/drivers/i2c/stm32f7_i2c.c index 78d7156492..0ec67b5c12 100644 --- a/drivers/i2c/stm32f7_i2c.c +++ b/drivers/i2c/stm32f7_i2c.c @@ -282,7 +282,7 @@ static int stm32_i2c_check_device_busy(struct stm32_i2c_priv *i2c_priv) }
static void stm32_i2c_message_start(struct stm32_i2c_priv *i2c_priv,
struct i2c_msg *msg, bool stop)
struct i2c_msg *msg)@@ -325,7 +325,7 @@ static void stm32_i2c_message_start(struct stm32_i2c_priv *i2c_priv, */
static void stm32_i2c_handle_reload(struct stm32_i2c_priv *i2c_priv,
struct i2c_msg *msg, bool stop)
struct i2c_msg *msg)@@ -431,7 +431,7 @@ static int stm32_i2c_message_xfer(struct stm32_i2c_priv *i2c_priv, /* Add errors */ mask |= STM32_I2C_ISR_ERRORS;
- stm32_i2c_message_start(i2c_priv, msg, stop);
stm32_i2c_message_start(i2c_priv, msg);
while (msg->len) { /*
@@ -469,7 +469,7 @@ static int stm32_i2c_message_xfer(struct stm32_i2c_priv *i2c_priv, mask = msg->flags & I2C_M_RD ? STM32_I2C_ISR_RXNE : STM32_I2C_ISR_TXIS | STM32_I2C_ISR_NACKF;
stm32_i2c_handle_reload(i2c_priv, msg, stop);
stm32_i2c_handle_reload(i2c_priv, msg);
Applied to u-boot-stm/master, thanks!
I also add the missing Reviewed-by when I get the patch from patchwork
http://patchwork.ozlabs.org/project/uboot/patch/20220912084201.1826979-3-ala...
+ Reviewed-by: Heiko Schocher hs@denx.de + Reviewed-by: Patrice Chotard patrice.chotard@foss.st.com
Regards Patrick
Current function stm32_i2c_message_xfer is sending a STOP whatever the result of the transaction is. This can cause issues such as making the bus busy since the controller itself is already sending automatically a STOP when a NACK is generated.
Thanks to Jorge Ramirez-Ortiz for diagnosing and proposing a first fix for this. [1]
[1] https://lore.kernel.org/u-boot/20220815145211.31342-2-jorge@foundries.io/
Reported-by: Jorge Ramirez-Ortiz, Foundries jorge@foundries.io Signed-off-by: Jorge Ramirez-Ortiz jorge@foundries.io Signed-off-by: Alain Volmat alain.volmat@foss.st.com --- drivers/i2c/stm32f7_i2c.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/stm32f7_i2c.c b/drivers/i2c/stm32f7_i2c.c index 0ec67b5c12..2db7f44d44 100644 --- a/drivers/i2c/stm32f7_i2c.c +++ b/drivers/i2c/stm32f7_i2c.c @@ -483,9 +483,9 @@ static int stm32_i2c_message_xfer(struct stm32_i2c_priv *i2c_priv, } }
- /* End of transfer, send stop condition */ - mask = STM32_I2C_CR2_STOP; - setbits_le32(®s->cr2, mask); + /* End of transfer, send stop condition if appropriate */ + if (!ret && !(status & (STM32_I2C_ISR_NACKF | STM32_I2C_ISR_ERRORS))) + setbits_le32(®s->cr2, STM32_I2C_CR2_STOP);
return stm32_i2c_check_end_of_message(i2c_priv); }
On 9/12/22 10:42, Alain Volmat wrote:
Current function stm32_i2c_message_xfer is sending a STOP whatever the result of the transaction is. This can cause issues such as making the bus busy since the controller itself is already sending automatically a STOP when a NACK is generated.
Thanks to Jorge Ramirez-Ortiz for diagnosing and proposing a first fix for this. [1]
[1] https://lore.kernel.org/u-boot/20220815145211.31342-2-jorge@foundries.io/
Reported-by: Jorge Ramirez-Ortiz, Foundries jorge@foundries.io Signed-off-by: Jorge Ramirez-Ortiz jorge@foundries.io Signed-off-by: Alain Volmat alain.volmat@foss.st.com
drivers/i2c/stm32f7_i2c.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/stm32f7_i2c.c b/drivers/i2c/stm32f7_i2c.c index 0ec67b5c12..2db7f44d44 100644 --- a/drivers/i2c/stm32f7_i2c.c +++ b/drivers/i2c/stm32f7_i2c.c @@ -483,9 +483,9 @@ static int stm32_i2c_message_xfer(struct stm32_i2c_priv *i2c_priv, } }
- /* End of transfer, send stop condition */
- mask = STM32_I2C_CR2_STOP;
- setbits_le32(®s->cr2, mask);
/* End of transfer, send stop condition if appropriate */
if (!ret && !(status & (STM32_I2C_ISR_NACKF | STM32_I2C_ISR_ERRORS)))
setbits_le32(®s->cr2, STM32_I2C_CR2_STOP);return stm32_i2c_check_end_of_message(i2c_priv);
}
Reviewed-by: Patrice Chotard patrice.chotard@foss.st.com
Thanks Patrice
Hello Alain,
On 12.09.22 10:42, Alain Volmat wrote:
Current function stm32_i2c_message_xfer is sending a STOP whatever the result of the transaction is. This can cause issues such as making the bus busy since the controller itself is already sending automatically a STOP when a NACK is generated.
Thanks to Jorge Ramirez-Ortiz for diagnosing and proposing a first fix for this. [1]
[1] https://lore.kernel.org/u-boot/20220815145211.31342-2-jorge@foundries.io/
Reported-by: Jorge Ramirez-Ortiz, Foundries jorge@foundries.io Signed-off-by: Jorge Ramirez-Ortiz jorge@foundries.io Signed-off-by: Alain Volmat alain.volmat@foss.st.com
drivers/i2c/stm32f7_i2c.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
Reviewed-by: Heiko Schocher hs@denx.de
Oh... isn;t here missing the Reviewed-by and Tested-by tag from Patrick?
bye, Heiko
diff --git a/drivers/i2c/stm32f7_i2c.c b/drivers/i2c/stm32f7_i2c.c index 0ec67b5c12..2db7f44d44 100644 --- a/drivers/i2c/stm32f7_i2c.c +++ b/drivers/i2c/stm32f7_i2c.c @@ -483,9 +483,9 @@ static int stm32_i2c_message_xfer(struct stm32_i2c_priv *i2c_priv, } }
- /* End of transfer, send stop condition */
- mask = STM32_I2C_CR2_STOP;
- setbits_le32(®s->cr2, mask);
/* End of transfer, send stop condition if appropriate */
if (!ret && !(status & (STM32_I2C_ISR_NACKF | STM32_I2C_ISR_ERRORS)))
setbits_le32(®s->cr2, STM32_I2C_CR2_STOP);return stm32_i2c_check_end_of_message(i2c_priv);
}
Hi,
On 9/12/22 10:42, Alain Volmat wrote:
Current function stm32_i2c_message_xfer is sending a STOP whatever the result of the transaction is. This can cause issues such as making the bus busy since the controller itself is already sending automatically a STOP when a NACK is generated.
Thanks to Jorge Ramirez-Ortiz for diagnosing and proposing a first fix for this. [1]
[1] https://lore.kernel.org/u-boot/20220815145211.31342-2-jorge@foundries.io/
Reported-by: Jorge Ramirez-Ortiz, Foundries jorge@foundries.io Signed-off-by: Jorge Ramirez-Ortiz jorge@foundries.io Signed-off-by: Alain Volmat alain.volmat@foss.st.com
drivers/i2c/stm32f7_i2c.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/stm32f7_i2c.c b/drivers/i2c/stm32f7_i2c.c index 0ec67b5c12..2db7f44d44 100644 --- a/drivers/i2c/stm32f7_i2c.c +++ b/drivers/i2c/stm32f7_i2c.c @@ -483,9 +483,9 @@ static int stm32_i2c_message_xfer(struct stm32_i2c_priv *i2c_priv, } }
- /* End of transfer, send stop condition */
- mask = STM32_I2C_CR2_STOP;
- setbits_le32(®s->cr2, mask);
/* End of transfer, send stop condition if appropriate */
if (!ret && !(status & (STM32_I2C_ISR_NACKF | STM32_I2C_ISR_ERRORS)))
setbits_le32(®s->cr2, STM32_I2C_CR2_STOP);return stm32_i2c_check_end_of_message(i2c_priv); }
Reviewed-by: Patrick Delaunay patrick.delaunay@foss.st.com Tested-by: Patrick Delaunay patrick.delaunay@foss.st.com [stm32mp157c-dk2]
@Jorge: can you test also for your use-case, thanks
Thanks Patrick
Hi Alain
On 9/12/22 10:42, Alain Volmat wrote:
Current function stm32_i2c_message_xfer is sending a STOP whatever the result of the transaction is. This can cause issues such as making the bus busy since the controller itself is already sending automatically a STOP when a NACK is generated.
Thanks to Jorge Ramirez-Ortiz for diagnosing and proposing a first fix for this. [1]
[1] https://lore.kernel.org/u-boot/20220815145211.31342-2-jorge@foundries.io/
Reported-by: Jorge Ramirez-Ortiz, Foundries jorge@foundries.io Signed-off-by: Jorge Ramirez-Ortiz jorge@foundries.io Signed-off-by: Alain Volmat alain.volmat@foss.st.com
drivers/i2c/stm32f7_i2c.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/stm32f7_i2c.c b/drivers/i2c/stm32f7_i2c.c index 0ec67b5c12..2db7f44d44 100644 --- a/drivers/i2c/stm32f7_i2c.c +++ b/drivers/i2c/stm32f7_i2c.c @@ -483,9 +483,9 @@ static int stm32_i2c_message_xfer(struct stm32_i2c_priv *i2c_priv, } }
- /* End of transfer, send stop condition */
- mask = STM32_I2C_CR2_STOP;
- setbits_le32(®s->cr2, mask);
/* End of transfer, send stop condition if appropriate */
if (!ret && !(status & (STM32_I2C_ISR_NACKF | STM32_I2C_ISR_ERRORS)))
setbits_le32(®s->cr2, STM32_I2C_CR2_STOP);return stm32_i2c_check_end_of_message(i2c_priv); }
Applied to u-boot-stm/master, thanks!
I also add the missing Reviewed-by when I get the patch from patchwork
http://patchwork.ozlabs.org/project/uboot/patch/20220912084201.1826979-4-ala...
+ Reviewed-by: Patrice Chotard patrice.chotard@foss.st.com + Reviewed-by: Heiko Schocher hs@denx.de + Reviewed-by: Patrick Delaunay patrick.delaunay@foss.st.com + Tested-by: Patrick Delaunay patrick.delaunay@foss.st.com
Regards Patrick
Hi Alain,
On 9/12/22 10:42, Alain Volmat wrote:
Current function stm32_i2c_message_xfer is sending a STOP whatever the result of the transaction is. This can cause issues such as making the bus busy since the controller itself is already sending automatically a STOP when a NACK is generated.
Thanks to Jorge Ramirez-Ortiz for diagnosing and proposing a first fix for this. [1]
[1] https://lore.kernel.org/u-boot/20220815145211.31342-2-jorge@foundries.io/
Reported-by: Jorge Ramirez-Ortiz, Foundries jorge@foundries.io Signed-off-by: Jorge Ramirez-Ortiz jorge@foundries.io Signed-off-by: Alain Volmat alain.volmat@foss.st.com
drivers/i2c/stm32f7_i2c.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/stm32f7_i2c.c b/drivers/i2c/stm32f7_i2c.c index 0ec67b5c12..2db7f44d44 100644 --- a/drivers/i2c/stm32f7_i2c.c +++ b/drivers/i2c/stm32f7_i2c.c @@ -483,9 +483,9 @@ static int stm32_i2c_message_xfer(struct stm32_i2c_priv *i2c_priv, } }
- /* End of transfer, send stop condition */
- mask = STM32_I2C_CR2_STOP;
- setbits_le32(®s->cr2, mask);
/* End of transfer, send stop condition if appropriate */
if (!ret && !(status & (STM32_I2C_ISR_NACKF | STM32_I2C_ISR_ERRORS)))
setbits_le32(®s->cr2, STM32_I2C_CR2_STOP);return stm32_i2c_check_end_of_message(i2c_priv); }
Applied to u-boot-stm/master, thanks!
I also add the missing Reviewed-by when I get the patch from patchwork
http://patchwork.ozlabs.org/project/uboot/patch/20220912084201.1826979-5-ala...
+ Reviewed-by: Patrice Chotard patrice.chotard@foss.st.com + Reviewed-by: Heiko Schocher hs@denx.de + Reviewed-by: Patrick Delaunay patrick.delaunay@foss.st.com + Tested-by: Patrick Delaunay patrick.delaunay@foss.st.com
Regards Patrick
From: Jorge Ramirez-Ortiz jorge@foundries.io
These two device tree properties were not being applied.
Fixes: 1fd9eb68d6 ("i2c: stm32f7: move driver data of each instance in a privdata") Signed-off-by: Jorge Ramirez-Ortiz jorge@foundries.io Reviewed-by: Alain Volmat alain.volmat@foss.st.com --- drivers/i2c/stm32f7_i2c.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/i2c/stm32f7_i2c.c b/drivers/i2c/stm32f7_i2c.c index 2db7f44d44..1d2dd8e25d 100644 --- a/drivers/i2c/stm32f7_i2c.c +++ b/drivers/i2c/stm32f7_i2c.c @@ -914,18 +914,19 @@ static int stm32_of_to_plat(struct udevice *dev) { const struct stm32_i2c_data *data; struct stm32_i2c_priv *i2c_priv = dev_get_priv(dev); - u32 rise_time, fall_time; int ret;
data = (const struct stm32_i2c_data *)dev_get_driver_data(dev); if (!data) return -EINVAL;
- rise_time = dev_read_u32_default(dev, "i2c-scl-rising-time-ns", - STM32_I2C_RISE_TIME_DEFAULT); + i2c_priv->setup.rise_time = dev_read_u32_default(dev, + "i2c-scl-rising-time-ns", + STM32_I2C_RISE_TIME_DEFAULT);
- fall_time = dev_read_u32_default(dev, "i2c-scl-falling-time-ns", - STM32_I2C_FALL_TIME_DEFAULT); + i2c_priv->setup.fall_time = dev_read_u32_default(dev, + "i2c-scl-falling-time-ns", + STM32_I2C_FALL_TIME_DEFAULT);
i2c_priv->dnf_dt = dev_read_u32_default(dev, "i2c-digital-filter-width-ns", 0); if (!dev_read_bool(dev, "i2c-digital-filter"))
Hi Alain, Jorge
On 9/12/22 10:42, Alain Volmat wrote:
From: Jorge Ramirez-Ortiz jorge@foundries.io
These two device tree properties were not being applied.
Fixes: 1fd9eb68d6 ("i2c: stm32f7: move driver data of each instance in a privdata") Signed-off-by: Jorge Ramirez-Ortiz jorge@foundries.io Reviewed-by: Alain Volmat alain.volmat@foss.st.com
drivers/i2c/stm32f7_i2c.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/i2c/stm32f7_i2c.c b/drivers/i2c/stm32f7_i2c.c index 2db7f44d44..1d2dd8e25d 100644 --- a/drivers/i2c/stm32f7_i2c.c +++ b/drivers/i2c/stm32f7_i2c.c @@ -914,18 +914,19 @@ static int stm32_of_to_plat(struct udevice *dev) { const struct stm32_i2c_data *data; struct stm32_i2c_priv *i2c_priv = dev_get_priv(dev);
u32 rise_time, fall_time; int ret;
data = (const struct stm32_i2c_data *)dev_get_driver_data(dev); if (!data) return -EINVAL;
rise_time = dev_read_u32_default(dev, "i2c-scl-rising-time-ns",
STM32_I2C_RISE_TIME_DEFAULT);
- i2c_priv->setup.rise_time = dev_read_u32_default(dev,
"i2c-scl-rising-time-ns",STM32_I2C_RISE_TIME_DEFAULT);
- fall_time = dev_read_u32_default(dev, "i2c-scl-falling-time-ns",
STM32_I2C_FALL_TIME_DEFAULT);
i2c_priv->setup.fall_time = dev_read_u32_default(dev,
"i2c-scl-falling-time-ns",STM32_I2C_FALL_TIME_DEFAULT);i2c_priv->dnf_dt = dev_read_u32_default(dev, "i2c-digital-filter-width-ns", 0); if (!dev_read_bool(dev, "i2c-digital-filter"))
Reviewed-by: Patrice Chotard patrice.chotard@foss.st.com
Thanks Patrice
Hello Alain,
On 12.09.22 10:42, Alain Volmat wrote:
From: Jorge Ramirez-Ortiz jorge@foundries.io
These two device tree properties were not being applied.
Fixes: 1fd9eb68d6 ("i2c: stm32f7: move driver data of each instance in a privdata") Signed-off-by: Jorge Ramirez-Ortiz jorge@foundries.io Reviewed-by: Alain Volmat alain.volmat@foss.st.com
drivers/i2c/stm32f7_i2c.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
Reviewed-by: Heiko Schocher hs@denx.de
bye, Heiko
Hi,
On 9/12/22 10:42, Alain Volmat wrote:
From: Jorge Ramirez-Ortiz jorge@foundries.io
These two device tree properties were not being applied.
Fixes: 1fd9eb68d6 ("i2c: stm32f7: move driver data of each instance in a privdata") Signed-off-by: Jorge Ramirez-Ortiz jorge@foundries.io Reviewed-by: Alain Volmat alain.volmat@foss.st.com
drivers/i2c/stm32f7_i2c.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/i2c/stm32f7_i2c.c b/drivers/i2c/stm32f7_i2c.c index 2db7f44d44..1d2dd8e25d 100644 --- a/drivers/i2c/stm32f7_i2c.c +++ b/drivers/i2c/stm32f7_i2c.c @@ -914,18 +914,19 @@ static int stm32_of_to_plat(struct udevice *dev) { const struct stm32_i2c_data *data; struct stm32_i2c_priv *i2c_priv = dev_get_priv(dev);
u32 rise_time, fall_time; int ret;
data = (const struct stm32_i2c_data *)dev_get_driver_data(dev); if (!data) return -EINVAL;
rise_time = dev_read_u32_default(dev, "i2c-scl-rising-time-ns",
STM32_I2C_RISE_TIME_DEFAULT);
- i2c_priv->setup.rise_time = dev_read_u32_default(dev,
"i2c-scl-rising-time-ns",STM32_I2C_RISE_TIME_DEFAULT);
- fall_time = dev_read_u32_default(dev, "i2c-scl-falling-time-ns",
STM32_I2C_FALL_TIME_DEFAULT);
i2c_priv->setup.fall_time = dev_read_u32_default(dev,
"i2c-scl-falling-time-ns",STM32_I2C_FALL_TIME_DEFAULT);i2c_priv->dnf_dt = dev_read_u32_default(dev, "i2c-digital-filter-width-ns", 0); if (!dev_read_bool(dev, "i2c-digital-filter"))
Reviewed-by: Patrick Delaunay patrick.delaunay@foss.st.com
Thanks Patrick
Reviewed-by: Patrick Delaunay patrick.delaunay@foss.st.com Tested-by: Patrick Delaunay patrick.delaunay@foss.st.com [stm32mp157c-dk2]
No regression detection on ST Microelectonics board.
- No error trace on boot
- I2C probe command is OK
STM32MP> i2c probe Valid chip addresses: 28 33
- And other tests done with the 2 I2C devices STPMIC1 & STUSB1600 are ok: regulalor command
pmic status command
USB type C connection/deconnection
@Jorge: can you test also for your use-case, thanks
Thanks Patrick
Hi Alain,
On 9/12/22 10:42, Alain Volmat wrote:
From: Jorge Ramirez-Ortiz jorge@foundries.io
These two device tree properties were not being applied.
Fixes: 1fd9eb68d6 ("i2c: stm32f7: move driver data of each instance in a privdata") Signed-off-by: Jorge Ramirez-Ortiz jorge@foundries.io Reviewed-by: Alain Volmat alain.volmat@foss.st.com
drivers/i2c/stm32f7_i2c.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/i2c/stm32f7_i2c.c b/drivers/i2c/stm32f7_i2c.c index 2db7f44d44..1d2dd8e25d 100644 --- a/drivers/i2c/stm32f7_i2c.c +++ b/drivers/i2c/stm32f7_i2c.c @@ -914,18 +914,19 @@ static int stm32_of_to_plat(struct udevice *dev) { const struct stm32_i2c_data *data; struct stm32_i2c_priv *i2c_priv = dev_get_priv(dev);
u32 rise_time, fall_time; int ret;
data = (const struct stm32_i2c_data *)dev_get_driver_data(dev); if (!data) return -EINVAL;
rise_time = dev_read_u32_default(dev, "i2c-scl-rising-time-ns",
STM32_I2C_RISE_TIME_DEFAULT);
- i2c_priv->setup.rise_time = dev_read_u32_default(dev,
"i2c-scl-rising-time-ns",STM32_I2C_RISE_TIME_DEFAULT);
- fall_time = dev_read_u32_default(dev, "i2c-scl-falling-time-ns",
STM32_I2C_FALL_TIME_DEFAULT);
i2c_priv->setup.fall_time = dev_read_u32_default(dev,
"i2c-scl-falling-time-ns",STM32_I2C_FALL_TIME_DEFAULT);i2c_priv->dnf_dt = dev_read_u32_default(dev, "i2c-digital-filter-width-ns", 0); if (!dev_read_bool(dev, "i2c-digital-filter"))
Applied to u-boot-stm/master, thanks!
I also add the missing Reviewed-by when I get the patch from patchwork
http://patchwork.ozlabs.org/project/uboot/patch/20220912084201.1826979-5-ala...
+ Reviewed-by: Patrice Chotard patrice.chotard@foss.st.com + Reviewed-by: Heiko Schocher hs@denx.de + Reviewed-by: Patrick Delaunay patrick.delaunay@foss.st.com + Tested-by: Patrick Delaunay patrick.delaunay@foss.st.com
Regards Patrick
participants (4)
-
Alain Volmat -
Heiko Schocher -
Patrice CHOTARD -
Patrick DELAUNAY