[PATCH v1 0/2] i2c: microchip: misc minor fixes
Hey all, Pair of fixes here for some of the logic in the microchip i2c driver. Both were reported against the Linux driver, which formed the basis for the U-Boot driver & the issues are present here too. Thanks, Conor.
Conor Dooley (2): i2c: microchip: fix ack sending logic i2c: microchip: fix erroneous late ack send
drivers/i2c/i2c-microchip.c | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-)
"Master receive mode" was not correctly sending ACKs/NACKs in the interrupt handler. Bring the handling of M_SLAR_ACK, M_RX_DATA_ACKED & M_RX_DATA_NACKED in line with the Linux driver.
Fixes: 0dc0d1e094 ("i2c: Add Microchip PolarFire SoC I2C driver") Reported-by: Shravan Chippa shravan.chippa@microchip.com Signed-off-by: Conor Dooley conor.dooley@microchip.com --- drivers/i2c/i2c-microchip.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/drivers/i2c/i2c-microchip.c b/drivers/i2c/i2c-microchip.c index 12f65d0af7..3a27459386 100644 --- a/drivers/i2c/i2c-microchip.c +++ b/drivers/i2c/i2c-microchip.c @@ -2,8 +2,9 @@ /* * Microchip I2C controller driver * - * Copyright (C) 2021 Microchip Technology Inc. + * Copyright (C) 2021-2022 Microchip Technology Inc. * Padmarao Begari padmarao.begari@microchip.com + * Conor Dooley conor.dooley@microchip.com */ #include <common.h> #include <clk.h> @@ -265,16 +266,27 @@ static int mpfs_i2c_service_handler(struct mpfs_i2c_bus *bus) } break; case STATUS_M_SLAR_ACK: - ctrl = readl(bus->base + MPFS_I2C_CTRL); - ctrl |= CTRL_AA; - writel(ctrl, bus->base + MPFS_I2C_CTRL); - if (bus->msg_len == 0) { + if (bus->msg_len > 1u) { + ctrl = readl(bus->base + MPFS_I2C_CTRL); + ctrl |= CTRL_AA; + writel(ctrl, bus->base + MPFS_I2C_CTRL); + } else if (bus->msg_len == 1u) { + ctrl = readl(bus->base + MPFS_I2C_CTRL); + ctrl &= ~CTRL_AA; + writel(ctrl, bus->base + MPFS_I2C_CTRL); + } else { + ctrl = readl(bus->base + MPFS_I2C_CTRL); + ctrl |= CTRL_AA; + writel(ctrl, bus->base + MPFS_I2C_CTRL); /* On the last byte to be transmitted, send STOP */ mpfs_i2c_stop(bus); finish = true; } break; case STATUS_M_RX_DATA_ACKED: + mpfs_i2c_empty_rx(bus); + break; + case STATUS_M_RX_DATA_NACKED: mpfs_i2c_empty_rx(bus); if (bus->msg_len == 0) { /* On the last byte to be transmitted, send STOP */ @@ -283,7 +295,6 @@ static int mpfs_i2c_service_handler(struct mpfs_i2c_bus *bus) } break; case STATUS_M_TX_DATA_NACK: - case STATUS_M_RX_DATA_NACKED: case STATUS_M_SLAR_NACK: case STATUS_M_SLAW_NACK: bus->msg_err = -ENXIO;
On Wed, 2022-10-26 at 08:49 +0100, Conor Dooley wrote: "Master receive mode" was not correctly sending ACKs/NACKs in the interrupt handler. Bring the handling of M_SLAR_ACK, M_RX_DATA_ACKED & M_RX_DATA_NACKED in line with the Linux driver.
Fixes: 0dc0d1e094 ("i2c: Add Microchip PolarFire SoC I2C driver") Reported-by: Shravan Chippa shravan.chippa@microchip.com Signed-off-by: Conor Dooley conor.dooley@microchip.com
drivers/i2c/i2c-microchip.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/drivers/i2c/i2c-microchip.c b/drivers/i2c/i2c- microchip.c index 12f65d0af7..3a27459386 100644 --- a/drivers/i2c/i2c-microchip.c +++ b/drivers/i2c/i2c-microchip.c @@ -2,8 +2,9 @@ /*
- Microchip I2C controller driver
- Copyright (C) 2021 Microchip Technology Inc.
- Copyright (C) 2021-2022 Microchip Technology Inc.
- Padmarao Begari padmarao.begari@microchip.com
*/
- Conor Dooley conor.dooley@microchip.com
#include <common.h> #include <clk.h> @@ -265,16 +266,27 @@ static int mpfs_i2c_service_handler(struct mpfs_i2c_bus *bus) } break; case STATUS_M_SLAR_ACK:
ctrl = readl(bus->base + MPFS_I2C_CTRL);ctrl |= CTRL_AA;writel(ctrl, bus->base + MPFS_I2C_CTRL);if (bus->msg_len == 0) {
if (bus->msg_len > 1u) {ctrl = readl(bus->base + MPFS_I2C_CTRL);ctrl |= CTRL_AA;writel(ctrl, bus->base + MPFS_I2C_CTRL);} else if (bus->msg_len == 1u) {ctrl = readl(bus->base + MPFS_I2C_CTRL);ctrl &= ~CTRL_AA;writel(ctrl, bus->base + MPFS_I2C_CTRL);} else {ctrl = readl(bus->base + MPFS_I2C_CTRL);ctrl |= CTRL_AA;writel(ctrl, bus->base + MPFS_I2C_CTRL); /* On the last byte to be transmitted, sendSTOP */ mpfs_i2c_stop(bus); finish = true; } break; case STATUS_M_RX_DATA_ACKED:
mpfs_i2c_empty_rx(bus);break;- case STATUS_M_RX_DATA_NACKED: mpfs_i2c_empty_rx(bus); if (bus->msg_len == 0) { /* On the last byte to be transmitted, send
STOP */ @@ -283,7 +295,6 @@ static int mpfs_i2c_service_handler(struct mpfs_i2c_bus *bus) } break; case STATUS_M_TX_DATA_NACK:
- case STATUS_M_RX_DATA_NACKED: case STATUS_M_SLAR_NACK: case STATUS_M_SLAW_NACK: bus->msg_err = -ENXIO;
Reviewed-by: Padmarao Begari padmarao.begari@microchip.com
Hello Conor,
On 26.10.22 09:49, Conor Dooley wrote:
"Master receive mode" was not correctly sending ACKs/NACKs in the interrupt handler. Bring the handling of M_SLAR_ACK, M_RX_DATA_ACKED & M_RX_DATA_NACKED in line with the Linux driver.
Fixes: 0dc0d1e094 ("i2c: Add Microchip PolarFire SoC I2C driver") Reported-by: Shravan Chippa shravan.chippa@microchip.com Signed-off-by: Conor Dooley conor.dooley@microchip.com
drivers/i2c/i2c-microchip.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-)
Thanks!
Reviewed-by: Heiko Schocher hs@denx.de
bye, Heiko
Hello Conor,
On 26.10.22 09:49, Conor Dooley wrote:
"Master receive mode" was not correctly sending ACKs/NACKs in the interrupt handler. Bring the handling of M_SLAR_ACK, M_RX_DATA_ACKED & M_RX_DATA_NACKED in line with the Linux driver.
Fixes: 0dc0d1e094 ("i2c: Add Microchip PolarFire SoC I2C driver") Reported-by: Shravan Chippa shravan.chippa@microchip.com Signed-off-by: Conor Dooley conor.dooley@microchip.com
drivers/i2c/i2c-microchip.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-)
Applied to u-boot-i2c.git master
Thanks!
bye, Heiko
A late ack is currently being sent at the end of a transfer due to incorrect logic in mchp_corei2c_empty_rx(). Currently the Assert Ack bit is being written to the controller's control reg after the last byte has been received, causing it to sent another byte with the ack. Instead, the AA flag should be written to the control register when the penultimate byte is read so it is sent out for the last byte.
Reported-by: Andreas Buerkler andreas.buerkler@enclustra.com Fixes: 0dc0d1e094 ("i2c: Add Microchip PolarFire SoC I2C driver") Fixes: 0190d48488 ("i2c: microchip: fix ack sending logic") Signed-off-by: Conor Dooley conor.dooley@microchip.com --- drivers/i2c/i2c-microchip.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/i2c-microchip.c b/drivers/i2c/i2c-microchip.c index 3a27459386..d82b80f535 100644 --- a/drivers/i2c/i2c-microchip.c +++ b/drivers/i2c/i2c-microchip.c @@ -224,7 +224,7 @@ static void mpfs_i2c_empty_rx(struct mpfs_i2c_bus *bus) bus->msg_len--; }
- if (bus->msg_len == 0) { + if (bus->msg_len <= 1) { ctrl = readl(bus->base + MPFS_I2C_CTRL); ctrl &= ~CTRL_AA; writel(ctrl, bus->base + MPFS_I2C_CTRL);
On 26/10/2022 08:49, Conor Dooley wrote:
A late ack is currently being sent at the end of a transfer due to incorrect logic in mchp_corei2c_empty_rx(). Currently the Assert Ack bit is being written to the controller's control reg after the last byte has been received, causing it to sent another byte with the ack. Instead, the AA flag should be written to the control register when the penultimate byte is read so it is sent out for the last byte.
Reported-by: Andreas Buerkler andreas.buerkler@enclustra.com Fixes: 0dc0d1e094 ("i2c: Add Microchip PolarFire SoC I2C driver") Fixes: 0190d48488 ("i2c: microchip: fix ack sending logic")
I had removed this fixes tag but I must have aborted the rebase in which I did. If nothing else needs changing, please drop it, otherwise I'll remove it if/when I send a v2.
Thanks, Conor.
Signed-off-by: Conor Dooley conor.dooley@microchip.com
drivers/i2c/i2c-microchip.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/i2c-microchip.c b/drivers/i2c/i2c-microchip.c index 3a27459386..d82b80f535 100644 --- a/drivers/i2c/i2c-microchip.c +++ b/drivers/i2c/i2c-microchip.c @@ -224,7 +224,7 @@ static void mpfs_i2c_empty_rx(struct mpfs_i2c_bus *bus) bus->msg_len--; }
- if (bus->msg_len == 0) {
- if (bus->msg_len <= 1) { ctrl = readl(bus->base + MPFS_I2C_CTRL); ctrl &= ~CTRL_AA; writel(ctrl, bus->base + MPFS_I2C_CTRL);
Hi Conor,
On Wed, 2022-10-26 at 07:54 +0000, Conor Dooley - M52691 wrote: On 26/10/2022 08:49, Conor Dooley wrote:
A late ack is currently being sent at the end of a transfer due to incorrect logic in mchp_corei2c_empty_rx(). Currently the Assert Ack bit is being written to the controller's control reg after the last byte has been received, causing it to sent another byte with the ack. Instead, the AA flag should be written to the control register when the penultimate byte is read so it is sent out for the last byte.
Reported-by: Andreas Buerkler andreas.buerkler@enclustra.com Fixes: 0dc0d1e094 ("i2c: Add Microchip PolarFire SoC I2C driver") Fixes: 0190d48488 ("i2c: microchip: fix ack sending logic")
I had removed this fixes tag but I must have aborted the rebase in which I did. If nothing else needs changing, please drop it, otherwise I'll remove it if/when I send a v2.
Yes you can remove it because the patch check is showing warning for this fixes tag
Regards Padmarao
Thanks, Conor.
Signed-off-by: Conor Dooley conor.dooley@microchip.com
drivers/i2c/i2c-microchip.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/i2c-microchip.c b/drivers/i2c/i2c- microchip.c index 3a27459386..d82b80f535 100644 --- a/drivers/i2c/i2c-microchip.c +++ b/drivers/i2c/i2c-microchip.c @@ -224,7 +224,7 @@ static void mpfs_i2c_empty_rx(struct mpfs_i2c_bus *bus) bus->msg_len--; }
- if (bus->msg_len == 0) {
- if (bus->msg_len <= 1) { ctrl = readl(bus->base + MPFS_I2C_CTRL); ctrl &= ~CTRL_AA; writel(ctrl, bus->base + MPFS_I2C_CTRL);
On Wed, 2022-10-26 at 08:49 +0100, Conor Dooley wrote: A late ack is currently being sent at the end of a transfer due to incorrect logic in mchp_corei2c_empty_rx(). Currently the Assert Ack bit is being written to the controller's control reg after the last byte has been received, causing it to sent another byte with the ack. Instead, the AA flag should be written to the control register when the penultimate byte is read so it is sent out for the last byte.
Reported-by: Andreas Buerkler andreas.buerkler@enclustra.com Fixes: 0dc0d1e094 ("i2c: Add Microchip PolarFire SoC I2C driver") Fixes: 0190d48488 ("i2c: microchip: fix ack sending logic") Signed-off-by: Conor Dooley conor.dooley@microchip.com
drivers/i2c/i2c-microchip.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/i2c-microchip.c b/drivers/i2c/i2c- microchip.c index 3a27459386..d82b80f535 100644 --- a/drivers/i2c/i2c-microchip.c +++ b/drivers/i2c/i2c-microchip.c @@ -224,7 +224,7 @@ static void mpfs_i2c_empty_rx(struct mpfs_i2c_bus *bus) bus->msg_len--; }
- if (bus->msg_len == 0) {
- if (bus->msg_len <= 1) { ctrl = readl(bus->base + MPFS_I2C_CTRL); ctrl &= ~CTRL_AA; writel(ctrl, bus->base + MPFS_I2C_CTRL);
Reviewed-by: Padmarao Begari padmarao.begari@microchip.com
Hello Conor,
On 26.10.22 09:49, Conor Dooley wrote:
A late ack is currently being sent at the end of a transfer due to incorrect logic in mchp_corei2c_empty_rx(). Currently the Assert Ack bit is being written to the controller's control reg after the last byte has been received, causing it to sent another byte with the ack. Instead, the AA flag should be written to the control register when the penultimate byte is read so it is sent out for the last byte.
Reported-by: Andreas Buerkler andreas.buerkler@enclustra.com Fixes: 0dc0d1e094 ("i2c: Add Microchip PolarFire SoC I2C driver") Fixes: 0190d48488 ("i2c: microchip: fix ack sending logic") Signed-off-by: Conor Dooley conor.dooley@microchip.com
drivers/i2c/i2c-microchip.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Thanks!
Reviewed-by: Heiko Schocher hs@denx.de
bye, Heiko
Hello Conor,
On 26.10.22 09:49, Conor Dooley wrote:
A late ack is currently being sent at the end of a transfer due to incorrect logic in mchp_corei2c_empty_rx(). Currently the Assert Ack bit is being written to the controller's control reg after the last byte has been received, causing it to sent another byte with the ack. Instead, the AA flag should be written to the control register when the penultimate byte is read so it is sent out for the last byte.
Reported-by: Andreas Buerkler andreas.buerkler@enclustra.com Fixes: 0dc0d1e094 ("i2c: Add Microchip PolarFire SoC I2C driver") Fixes: 0190d48488 ("i2c: microchip: fix ack sending logic") Signed-off-by: Conor Dooley conor.dooley@microchip.com
drivers/i2c/i2c-microchip.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Applied after removing wrong Fixes tag to u-boot-i2c.git master
Thanks!
bye, Heiko
participants (4)
-
Conor Dooley -
Conor.Dooley＠microchip.com
-
Heiko Schocher -
Padmarao.Begari＠microchip.com