From 8b2053d4b19a19962c766a007f8596d27a8b9de6 Mon Sep 17 00:00:00 2001 From: Eero Nurkkala Date: Thu, 30 Nov 2023 09:38:33 +0200 Subject: [PATCH 1/3] risc-v/mpfs: usb: don't try nonexistent ep int flags Currently the irq handler checks many reserved bits, which is a waste of resources: 1. pending_rx_ep bit 0 is reserved (always 0) 2. pending_rx_ep and pending_tx_ep have only bits 1, 2, 3 and 4 defined, no need to scan MPFS_USB_NENDPOINTS (9) bits as the rest are reserved Fix this by checking only the relevant bits. Signed-off-by: Eero Nurkkala --- arch/risc-v/src/mpfs/mpfs_usb.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/arch/risc-v/src/mpfs/mpfs_usb.c b/arch/risc-v/src/mpfs/mpfs_usb.c index cfb8d6264951d..4a5e87c6def12 100644 --- a/arch/risc-v/src/mpfs/mpfs_usb.c +++ b/arch/risc-v/src/mpfs/mpfs_usb.c @@ -3478,7 +3478,7 @@ static int mpfs_usb_interrupt(int irq, void *context, void *arg) if (pending_tx_ep != 0) { - for (i = 1; i < MPFS_USB_NENDPOINTS; i++) + for (i = 1; i < (MPFS_USB_NENDPOINTS / 2 + 1); i++) { if ((pending_tx_ep & (1 << i)) != 0) { @@ -3489,20 +3489,27 @@ static int mpfs_usb_interrupt(int irq, void *context, void *arg) if (pending_rx_ep != 0) { - for (i = 0; i < MPFS_USB_NENDPOINTS; i++) + for (i = 1; i < (MPFS_USB_NENDPOINTS / 2 + 1); i++) { /* Check if dead connections are back in business */ if (g_linkdead) { - /* This releases all, which is a problem if only some - * endpoints are closed on the remote; whereas some - * are functioning; for example ACM and mass storage; - * now the functioning one likely marks the closed ones - * as no longer dead. + /* This releases all tx counterparts with linkdead flag + * set, which is a problem if only some endpoints are + * closed on the remote; whereas some are functioning; + * for example ACM and mass storage; now the functioning + * one likely marks the closed ones as no longer dead. + * Please note that tx counterparts have MPFS_EPIN_START + * offset on top of the rx eps. + * + * For clarity, the eplist[] is as follows: + * eplist: 0: ep0, + * 1-4: ep1rx, ep2rx, ep3rx, ep4rx, + * 5-8: ep1tx, ep2tx, ep3tx, ep4tx */ - struct mpfs_ep_s *privep = &priv->eplist[i]; + struct mpfs_ep_s *privep = &priv->eplist[i + MPFS_EPIN_START]; privep->linkdead = 0; } From bc63f615023982be509ea85dcd507008d6cb96e3 Mon Sep 17 00:00:00 2001 From: Tero Salminen Date: Mon, 25 Mar 2024 15:59:41 +0200 Subject: [PATCH 2/3] mpfs_i2c: fix semaphore race condition MPFS_I2C_ST_STOP_SENT interrupt might be received right after semaphore timeout, which would cause the semaphore to be in the incorrect signaled state when the next transfer starts. Signed-off-by: Tero Salminen --- arch/risc-v/src/mpfs/mpfs_i2c.c | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/arch/risc-v/src/mpfs/mpfs_i2c.c b/arch/risc-v/src/mpfs/mpfs_i2c.c index 31af4ca7efc6f..b4cef05c99b8a 100644 --- a/arch/risc-v/src/mpfs/mpfs_i2c.c +++ b/arch/risc-v/src/mpfs/mpfs_i2c.c @@ -425,6 +425,8 @@ static int mpfs_i2c_init(struct mpfs_i2c_priv_s *priv) putreg32(MPFS_I2C_CTRL_ENS1_MASK, MPFS_I2C_CTRL); + nxsem_reset(&priv->sem_isr, 0); + priv->initialized = true; } @@ -469,8 +471,27 @@ static void mpfs_i2c_deinit(struct mpfs_i2c_priv_s *priv) static int mpfs_i2c_sem_waitdone(struct mpfs_i2c_priv_s *priv) { + int res = 0; uint32_t timeout = mpfs_i2c_timeout(priv->msgc, priv->msgv); - return nxsem_tickwait_uninterruptible(&priv->sem_isr, USEC2TICK(timeout)); + + res = nxsem_tickwait_uninterruptible(&priv->sem_isr, USEC2TICK(timeout)); + + /* -> race condition <- */ + + priv->inflight = false; + + /* Handle a race condition above in which interrupt MPFS_I2C_ST_STOP_SENT + * was received right after semaphore timeout. In that case semaphore is + * signalled and has the incorrect state when the next transfer starts. + */ + + if (res < 0) + { + res = nxsem_tickwait_uninterruptible(&priv->sem_isr, + USEC2TICK(timeout)); + } + + return res; } /**************************************************************************** @@ -872,7 +893,6 @@ static int mpfs_i2c_transfer(struct i2c_master_s *dev, if (mpfs_i2c_sem_waitdone(priv) < 0) { i2cinfo("Message %" PRIu8 " timed out.\n", priv->msgid); - priv->inflight = false; ret = -ETIMEDOUT; break; } @@ -933,6 +953,8 @@ static int mpfs_i2c_reset(struct i2c_master_s *dev) nxmutex_lock(&priv->lock); + i2cerr("i2c bus %d reset\n", priv->id); + mpfs_i2c_deinit(priv); ret = mpfs_i2c_init(priv); From 9600ab435613f3cd9663e38007c353a7ea42b466 Mon Sep 17 00:00:00 2001 From: Tero Salminen Date: Thu, 28 Mar 2024 09:03:50 +0200 Subject: [PATCH 3/3] mpfs_corespi: fix semaphore race condition SPI TX_DONE interrupt can be received after a semaphore timeout, but before interrupts are disabled. This will leave the semaphore to the signaled state. After a timeout the semaphore is always reset to non-signaled state to fix this race condition. Signed-off-by: Tero Salminen --- arch/risc-v/src/mpfs/mpfs_corespi.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/arch/risc-v/src/mpfs/mpfs_corespi.c b/arch/risc-v/src/mpfs/mpfs_corespi.c index 8476e17298e39..46b4781a7aa4f 100644 --- a/arch/risc-v/src/mpfs/mpfs_corespi.c +++ b/arch/risc-v/src/mpfs/mpfs_corespi.c @@ -928,6 +928,18 @@ static void mpfs_spi_irq_exchange(struct mpfs_spi_priv_s *priv, MPFS_SPI_INTTXDONE, 0); + /* TX_DONE interrupt can be received after a semaphore timeout, but before + * interrupts are disabled. This will leave the semaphore to the signaled + * state. + * After a timeout the semaphore is always reset to non-signaled state + * to fix this race condition. + */ + + if (priv->error == -ETIMEDOUT) + { + nxsem_reset(&priv->sem_isr, 0); + } + putreg32(MPFS_SPI_TXCHUNDRUN | MPFS_SPI_RXCHOVRFLW | MPFS_SPI_DATA_RX |