From 9082b01572057ba2b6e3cf8e5e03adf2439b3aea Mon Sep 17 00:00:00 2001 From: Eero Nurkkala Date: Tue, 5 Dec 2023 12:53:39 +0200 Subject: [PATCH] risc-v/mpfs: i2c: perform sanity checks Replace risky DEBUGASSERT()s with real sanity checks. Also, do a few more checks as the system might occasionally fire an interrupt if the system has been restarted while in middle of an i2c transaction. Yet, modify i2c_transfer() function so that up_disable_irq() is always called at the end to better prevent ill-timed irqs. Signed-off-by: Eero Nurkkala --- arch/risc-v/src/mpfs/mpfs_i2c.c | 68 ++++++++++++++++++++++++++++----- 1 file changed, 59 insertions(+), 9 deletions(-) diff --git a/arch/risc-v/src/mpfs/mpfs_i2c.c b/arch/risc-v/src/mpfs/mpfs_i2c.c index 35701ae5d491b..d13d55ff8b93e 100644 --- a/arch/risc-v/src/mpfs/mpfs_i2c.c +++ b/arch/risc-v/src/mpfs/mpfs_i2c.c @@ -436,8 +436,6 @@ static int mpfs_i2c_irq(int cpuint, void *context, void *arg) volatile uint32_t status; uint8_t clear_irq = 1u; - DEBUGASSERT(msg != NULL); - status = getreg32(MPFS_I2C_STATUS); switch (status) @@ -478,7 +476,16 @@ static int mpfs_i2c_irq(int cpuint, void *context, void *arg) case MPFS_I2C_ST_TX_DATA_ACK: if (priv->tx_idx < priv->tx_size) { - DEBUGASSERT(priv->tx_buffer != NULL); + if (priv->tx_buffer == NULL) + { + i2cerr("ERROR: tx_buffer is NULL!\n"); + + /* Clear the serial interrupt flag and exit */ + + modifyreg32(MPFS_I2C_CTRL, MPFS_I2C_CTRL_SI_MASK, 0); + return 0; + } + putreg32(priv->tx_buffer[priv->tx_idx], MPFS_I2C_DATA); priv->tx_idx++; } @@ -556,10 +563,18 @@ static int mpfs_i2c_irq(int cpuint, void *context, void *arg) break; case MPFS_I2C_ST_RX_DATA_ACK: + if (priv->rx_buffer == NULL) + { + i2cerr("ERROR: rx_buffer is NULL!\n"); + + /* Clear the serial interrupt flag and exit */ + + modifyreg32(MPFS_I2C_CTRL, MPFS_I2C_CTRL_SI_MASK, 0); + return 0; + } /* Data byte received, ACK returned */ - DEBUGASSERT(priv->rx_buffer != NULL); priv->rx_buffer[priv->rx_idx] = (uint8_t)getreg32(MPFS_I2C_DATA); priv->rx_idx++; @@ -571,10 +586,29 @@ static int mpfs_i2c_irq(int cpuint, void *context, void *arg) case MPFS_I2C_ST_RX_DATA_NACK: + /* Some sanity checks */ + + if (priv->rx_buffer == NULL) + { + i2cerr("ERROR: rx_buffer is NULL!\n"); + + /* Clear the serial interrupt flag and exit */ + + modifyreg32(MPFS_I2C_CTRL, MPFS_I2C_CTRL_SI_MASK, 0); + return 0; + } + else if (priv->rx_idx >= priv->rx_size) + { + i2cerr("ERROR: rx_idx is out of bounds!\n"); + + /* Clear the serial interrupt flag and exit */ + + modifyreg32(MPFS_I2C_CTRL, MPFS_I2C_CTRL_SI_MASK, 0); + return 0; + } + /* Data byte received, NACK returned */ - DEBUGASSERT(priv->rx_buffer != NULL); - DEBUGASSERT(priv->rx_idx < priv->rx_size); priv->rx_buffer[priv->rx_idx] = (uint8_t)getreg32(MPFS_I2C_DATA); priv->rx_idx++; @@ -672,7 +706,11 @@ static int mpfs_i2c_transfer(struct i2c_master_s *dev, int ret = OK; i2cinfo("Starting transfer request of %d message(s):\n", count); - DEBUGASSERT(count > 0); + + if (count <= 0) + { + return -EINVAL; + } ret = nxmutex_lock(&priv->lock); if (ret < 0) @@ -714,9 +752,17 @@ static int mpfs_i2c_transfer(struct i2c_master_s *dev, if (msgs[i].flags & I2C_M_NOSTOP) { - /* Support only write + read combinations */ + /* Support only write + read combinations. No write + write, + * nor read + write without stop condition between supported + * yet. + */ - DEBUGASSERT(!(msgs[i].flags & I2C_M_READ)); + if (msgs[i].flags & I2C_M_READ) + { + i2cerr("No read before write supported!\n"); + nxmutex_unlock(&priv->lock); + return -EINVAL; + } /* Combine write + read transaction into one */ @@ -764,6 +810,10 @@ static int mpfs_i2c_transfer(struct i2c_master_s *dev, i2cinfo("Message %" PRIu8 " transfer complete.\n", priv->msgid); } + /* Irq was enabled at mpfs_i2c_sendstart() */ + + up_disable_irq(priv->plic_irq); + nxmutex_unlock(&priv->lock); return ret; }