From c6f7e98a9acf2d1dcc71c822d407e0bf6f20bad0 Mon Sep 17 00:00:00 2001 From: Jukka Laitinen Date: Wed, 10 Jan 2024 08:45:25 +0200 Subject: [PATCH 1/5] arch/risc-v/src/mpfs/mpfs_i2c.c: Clear I2C_CTRL bits when initializing/deinitializing bus Ensure that there are no pending state or interrupts in the i2c controller. This removes errors caused by deinitialize/initialize sequences in error cases. Signed-off-by: Jukka Laitinen --- arch/risc-v/src/mpfs/mpfs_i2c.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/arch/risc-v/src/mpfs/mpfs_i2c.c b/arch/risc-v/src/mpfs/mpfs_i2c.c index d13d55ff8b93e..77034747196a6 100644 --- a/arch/risc-v/src/mpfs/mpfs_i2c.c +++ b/arch/risc-v/src/mpfs/mpfs_i2c.c @@ -357,10 +357,9 @@ static int mpfs_i2c_init(struct mpfs_i2c_priv_s *priv) putreg32(priv->ser_address, MPFS_I2C_ADDR); - /* Enable i2c bus */ + /* Enable i2c bus, clear all other bits */ - modifyreg32(MPFS_I2C_CTRL, MPFS_I2C_CTRL_ENS1_MASK, - MPFS_I2C_CTRL_ENS1_MASK); + putreg32(MPFS_I2C_CTRL_ENS1_MASK, MPFS_I2C_CTRL); priv->initialized = true; } @@ -384,8 +383,7 @@ static void mpfs_i2c_deinit(struct mpfs_i2c_priv_s *priv) up_disable_irq(priv->plic_irq); irq_detach(priv->plic_irq); - modifyreg32(MPFS_I2C_CTRL, MPFS_I2C_CTRL_ENS1_MASK, - ~MPFS_I2C_CTRL_ENS1_MASK); + putreg32(0, MPFS_I2C_CTRL); priv->initialized = false; } From b3d6bb7b0f1e484c0b9f11641999cdfdfc9c6533 Mon Sep 17 00:00:00 2001 From: Jukka Laitinen Date: Wed, 10 Jan 2024 08:45:25 +0200 Subject: [PATCH 2/5] arch/risc-v/src/mpfs/mpfs_i2c.c: Add more error status codes Add more error status codes to help debugging in the future. Signed-off-by: Jukka Laitinen --- arch/risc-v/src/mpfs/mpfs_i2c.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/arch/risc-v/src/mpfs/mpfs_i2c.c b/arch/risc-v/src/mpfs/mpfs_i2c.c index 77034747196a6..cc0ce76461b94 100644 --- a/arch/risc-v/src/mpfs/mpfs_i2c.c +++ b/arch/risc-v/src/mpfs/mpfs_i2c.c @@ -82,7 +82,10 @@ typedef enum mpfs_i2c_status MPFS_I2C_SUCCESS = 0u, MPFS_I2C_IN_PROGRESS, MPFS_I2C_FAILED, - MPFS_I2C_TIMED_OUT + MPFS_I2C_FAILED_SLAW_NACK, + MPFS_I2C_FAILED_SLAR_NACK, + MPFS_I2C_FAILED_TX_DATA_NACK, + MPFS_I2C_FAILED_BUS_ERROR, } mpfs_i2c_status_t; typedef enum mpfs_i2c_clock_divider @@ -467,7 +470,7 @@ static int mpfs_i2c_irq(int cpuint, void *context, void *arg) case MPFS_I2C_ST_SLAW_NACK: modifyreg32(MPFS_I2C_CTRL, 0, MPFS_I2C_CTRL_STO_MASK); - priv->status = MPFS_I2C_FAILED; + priv->status = MPFS_I2C_FAILED_SLAW_NACK; break; case MPFS_I2C_ST_SLAW_ACK: @@ -535,7 +538,7 @@ static int mpfs_i2c_irq(int cpuint, void *context, void *arg) case MPFS_I2C_ST_TX_DATA_NACK: modifyreg32(MPFS_I2C_CTRL, 0, MPFS_I2C_CTRL_STO_MASK); - priv->status = MPFS_I2C_FAILED; + priv->status = MPFS_I2C_FAILED_TX_DATA_NACK; break; case MPFS_I2C_ST_SLAR_ACK: /* SLA+R tx'ed. */ @@ -557,7 +560,7 @@ static int mpfs_i2c_irq(int cpuint, void *context, void *arg) case MPFS_I2C_ST_SLAR_NACK: /* SLA+R tx'ed; send a stop condition */ modifyreg32(MPFS_I2C_CTRL, 0, MPFS_I2C_CTRL_STO_MASK); - priv->status = MPFS_I2C_FAILED; + priv->status = MPFS_I2C_FAILED_SLAR_NACK; break; case MPFS_I2C_ST_RX_DATA_ACK: @@ -609,7 +612,6 @@ static int mpfs_i2c_irq(int cpuint, void *context, void *arg) priv->rx_buffer[priv->rx_idx] = (uint8_t)getreg32(MPFS_I2C_DATA); priv->rx_idx++; - priv->status = MPFS_I2C_SUCCESS; modifyreg32(MPFS_I2C_CTRL, 0, MPFS_I2C_CTRL_STO_MASK); break; @@ -650,7 +652,7 @@ static int mpfs_i2c_irq(int cpuint, void *context, void *arg) if (priv->status == MPFS_I2C_IN_PROGRESS) { - priv->status = MPFS_I2C_FAILED; + priv->status = MPFS_I2C_FAILED_BUS_ERROR; } break; From d200fbf43074427b3f6e11dfbb91dd93dbcfd21f Mon Sep 17 00:00:00 2001 From: Jukka Laitinen Date: Thu, 11 Jan 2024 12:13:35 +0200 Subject: [PATCH 3/5] arch/risc-v/src/mpfs/mpfs_i2c.c: Correct i2c reset / error recovery - Use mpfs_i2c_deinit+mpfs_i2c_init sequence to re-initialize i2c block - Use the i2c mutex to protect the reset; in case there are several devices on the same bus, and one of them resets the bus, reset must not occur in the middle of another device's transfer. - Move irq attach to the i2c_init as the irq detach is in i2c_deinit Signed-off-by: Jukka Laitinen --- arch/risc-v/src/mpfs/mpfs_i2c.c | 45 ++++++++++++++++----------------- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/arch/risc-v/src/mpfs/mpfs_i2c.c b/arch/risc-v/src/mpfs/mpfs_i2c.c index cc0ce76461b94..4dd24665effd4 100644 --- a/arch/risc-v/src/mpfs/mpfs_i2c.c +++ b/arch/risc-v/src/mpfs/mpfs_i2c.c @@ -125,6 +125,8 @@ static const uint32_t mpfs_i2c_freqs_fpga[MPFS_I2C_NUMBER_OF_DIVIDERS] = MPFS_FPGA_BCLK / 8 }; +static int mpfs_i2c_irq(int cpuint, void *context, void *arg); + static int mpfs_i2c_transfer(struct i2c_master_s *dev, struct i2c_msg_s *msgs, int count); @@ -300,8 +302,22 @@ static uint32_t mpfs_i2c_timeout(int msgc, struct i2c_msg_s *msgv); static int mpfs_i2c_init(struct mpfs_i2c_priv_s *priv) { + int ret = OK; + if (!priv->initialized) { + /* Clear any pending serial interrupt flag */ + + modifyreg32(MPFS_I2C_CTRL, MPFS_I2C_CTRL_SI_MASK, 0); + + /* Attach interrupt */ + + ret = irq_attach(priv->plic_irq, mpfs_i2c_irq, priv); + if (ret != OK) + { + return ret; + } + if (priv->fpga) { /* FIC3 is used by many, don't reset it here, or many @@ -837,26 +853,15 @@ static int mpfs_i2c_reset(struct i2c_master_s *dev) { struct mpfs_i2c_priv_s *priv = (struct mpfs_i2c_priv_s *)dev; int ret; - irqstate_t flags; - - DEBUGASSERT(priv != NULL); - flags = enter_critical_section(); - - /* Disabling I2C interrupts. - * NOTE: up_enable_irq() will be called at mpfs_i2c_sendstart() - */ - - up_disable_irq(priv->plic_irq); + nxmutex_lock(&priv->lock); - priv->inflight = false; - priv->status = MPFS_I2C_SUCCESS; - priv->initialized = false; + mpfs_i2c_deinit(priv); ret = mpfs_i2c_init(priv); if (ret != OK) { - leave_critical_section(flags); + nxmutex_unlock(&priv->lock); return ret; } @@ -864,8 +869,10 @@ static int mpfs_i2c_reset(struct i2c_master_s *dev) priv->tx_idx = 0; priv->rx_size = 0; priv->rx_idx = 0; + priv->inflight = false; + priv->status = MPFS_I2C_SUCCESS; - leave_critical_section(flags); + nxmutex_unlock(&priv->lock); return OK; } @@ -1059,14 +1066,6 @@ struct i2c_master_s *mpfs_i2cbus_initialize(int port) priv->fpga = true; #endif - ret = irq_attach(priv->plic_irq, mpfs_i2c_irq, priv); - if (ret != OK) - { - priv->refs--; - nxmutex_unlock(&priv->lock); - return NULL; - } - ret = mpfs_i2c_init(priv); if (ret != OK) { From 5b460bf4691677d821b5f4d594d79e1d93f463df Mon Sep 17 00:00:00 2001 From: Jukka Laitinen Date: Wed, 10 Jan 2024 08:45:25 +0200 Subject: [PATCH 4/5] arch/risc-v/src/mpfs/mpfs_i2c.c: Add more i2cerr traces Add sanity checks for debugging possible errors in the driver. Signed-off-by: Jukka Laitinen --- arch/risc-v/src/mpfs/mpfs_i2c.c | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/arch/risc-v/src/mpfs/mpfs_i2c.c b/arch/risc-v/src/mpfs/mpfs_i2c.c index 4dd24665effd4..15bdb3189ff7d 100644 --- a/arch/risc-v/src/mpfs/mpfs_i2c.c +++ b/arch/risc-v/src/mpfs/mpfs_i2c.c @@ -720,6 +720,10 @@ static int mpfs_i2c_transfer(struct i2c_master_s *dev, { struct mpfs_i2c_priv_s *priv = (struct mpfs_i2c_priv_s *)dev; int ret = OK; +#ifdef CONFIG_DEBUG_I2C_ERROR + int sval; + uint32_t status; +#endif i2cinfo("Starting transfer request of %d message(s):\n", count); @@ -734,6 +738,24 @@ static int mpfs_i2c_transfer(struct i2c_master_s *dev, return ret; } +#ifdef CONFIG_DEBUG_I2C_ERROR + /* We should never start at transfer with semaphore already signalled */ + + sem_getvalue(&priv->sem_isr, &sval); + if (sval != 0) + { + i2cerr("Already signalled at start? %d\n", sval); + } + + /* We should always be idle before transfer */ + + status = getreg32(MPFS_I2C_STATUS); + if (status != MPFS_I2C_ST_IDLE) + { + i2cerr("I2C bus not idle before transfer! Status: 0x%x\n", status); + } +#endif + priv->msgv = msgs; priv->msgc = count; @@ -826,6 +848,16 @@ static int mpfs_i2c_transfer(struct i2c_master_s *dev, i2cinfo("Message %" PRIu8 " transfer complete.\n", priv->msgid); } +#ifdef CONFIG_DEBUG_I2C_ERROR + /* We should always be idle after the transfers */ + + status = getreg32(MPFS_I2C_STATUS); + if (status != MPFS_I2C_ST_IDLE) + { + i2cerr("I2C bus not idle after transfer! Status: 0x%x\n", status); + } +#endif + /* Irq was enabled at mpfs_i2c_sendstart() */ up_disable_irq(priv->plic_irq); From 145cba8b858ab40460a4b3db84558d5d3718f0c9 Mon Sep 17 00:00:00 2001 From: Jukka Laitinen Date: Fri, 12 Jan 2024 15:49:27 +0200 Subject: [PATCH 5/5] arch/risc-v/src/mpfs/mpfs_i2c.c: Recover i2c from pending transactions in warm boot Signed-off-by: Jukka Laitinen --- arch/risc-v/src/mpfs/mpfs_i2c.c | 49 +++++++++++++++++++++++++++++++-- 1 file changed, 47 insertions(+), 2 deletions(-) diff --git a/arch/risc-v/src/mpfs/mpfs_i2c.c b/arch/risc-v/src/mpfs/mpfs_i2c.c index 15bdb3189ff7d..31af4ca7efc6f 100644 --- a/arch/risc-v/src/mpfs/mpfs_i2c.c +++ b/arch/risc-v/src/mpfs/mpfs_i2c.c @@ -303,12 +303,57 @@ static uint32_t mpfs_i2c_timeout(int msgc, struct i2c_msg_s *msgv); static int mpfs_i2c_init(struct mpfs_i2c_priv_s *priv) { int ret = OK; + uint32_t ctrl; + uint32_t status; if (!priv->initialized) { - /* Clear any pending serial interrupt flag */ + /* In case of warm boot, or after reset, check that the IP block is + * not already active and try to recover from any pending data + * transfer if it is. + */ - modifyreg32(MPFS_I2C_CTRL, MPFS_I2C_CTRL_SI_MASK, 0); + ctrl = getreg32(MPFS_I2C_CTRL); + if (ctrl != 0) + { + /* Check if the IP is enabled */ + + status = getreg32(MPFS_I2C_STATUS); + if (ctrl & MPFS_I2C_CTRL_ENS1_MASK) + { + if (status == MPFS_I2C_ST_RX_DATA_ACK) + { + /* In case the machine was in the middle of data RX, try to + * receive one byte and nack it + */ + + modifyreg32(MPFS_I2C_CTRL, MPFS_I2C_CTRL_AA_MASK, 0); + modifyreg32(MPFS_I2C_CTRL, MPFS_I2C_CTRL_SI_MASK, 0); + usleep(100); + status = getreg32(MPFS_I2C_STATUS); + } + + if (status != MPFS_I2C_ST_IDLE) + { + /* If the bus is not idle, send STOP */ + + modifyreg32(MPFS_I2C_CTRL, MPFS_I2C_CTRL_SI_MASK, 0); + modifyreg32(MPFS_I2C_CTRL, 0, MPFS_I2C_CTRL_STO_MASK); + usleep(100); + modifyreg32(MPFS_I2C_CTRL, MPFS_I2C_CTRL_SI_MASK, 0); + status = getreg32(MPFS_I2C_STATUS); + } + } + + if (status != MPFS_I2C_ST_IDLE) + { + i2cerr("Bus not idle before init\n"); + } + + /* Disable IP and continue initialization */ + + putreg32(0, MPFS_I2C_CTRL); + } /* Attach interrupt */