Skip to content

Commit

Permalink
RP2040: Fix I2C lockups and improve error handling
Browse files Browse the repository at this point in the history 
Problem:

* in edge-case scenarios the tx empty interrupt was still enabled after a
  transmission was finished. This would lead to endless interrupt tail
  chaining that completely starved the system.

Solution:

* all irqs are now disabled at the end of a transaction by default

Error handling:

* all error condition irqs like over and underruns of tx and rx fifos
  are now enabled and handled with an wake-up of the sleeping thread
  plus disabling of all irqs

Optimizations:

* irq status register is only read once for evalutation in the irq
  handler
* better Utilization of the rx fifos during reception, which are now read
  as long as there is data left for the transaction
  • Loading branch information
@KarlK90
KarlK90 committed Jul 21, 2022
1 parent 2c374c7 commit 9f5d970
Showing 1 changed file with 79 additions and 38 deletions.
117 changes: 79 additions & 38 deletions os/hal/ports/RP/LLD/I2Cv1/hal_i2c_lld.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,43 +113,60 @@ static void setup_frequency(I2CDriver *i2cp) {
*/
static void serve_interrupt(I2CDriver *i2cp) {
I2C_TypeDef *dp = i2cp->i2c;
uint32_t intr = dp->INTRSTAT;
uint32_t data;

if (i2cp->state == I2C_ACTIVE_TX) {
/* Transmission phase. */
if (dp->INTRSTAT & I2C_IC_INTR_STAT_R_TX_EMPTY_Msk) {
if (i2cp->txbytes) {
while (i2cp->txbytes && (dp->TXFLR & I2C_IC_TXFLR_TXFLR_Msk) < 16) {
dp->DATACMD = (uint32_t)i2cp->txbuf[i2cp->txindex] |
(i2cp->txbytes == 1 ? I2C_IC_DATA_CMD_STOP : 0);
i2cp->txindex++;
i2cp->txbytes--;
}
/* Transmission phase. */
if (intr & I2C_IC_INTR_STAT_R_TX_EMPTY && i2cp->state == I2C_ACTIVE_TX) {
while (i2cp->txbytes > 0U && dp->STATUS & I2C_IC_STATUS_TFNF) {
data = i2cp->txbuf[i2cp->txindex];

if (i2cp->txbytes == 0U) {
// Disable TX_EMPTY irq
dp->CLR.CON = I2C_IC_CON_TX_EMPTY_CTRL;
dp->CLR.INTRMASK = I2C_IC_INTR_MASK_M_TX_EMPTY;
}
/* Send STOP after last byte */
if (i2cp->txbytes == 1U) {
data |= I2C_IC_DATA_CMD_STOP;
}
dp->DATACMD = data;
i2cp->txindex++;
i2cp->txbytes--;

/* Error during transmission */
if(dp->INTRSTAT & I2C_IC_INTR_STAT_R_TX_ABRT) {
return;
}
}
} else {
/* Receive phase. */
if (dp->INTRSTAT & I2C_IC_INTR_STAT_R_RX_FULL_Msk) {

/* Everything is send, therefore disable TX FIFO empty IRQ. */
if (i2cp->txbytes == 0U) {
dp->CLR.INTRMASK = I2C_IC_INTR_MASK_M_TX_EMPTY;
}

return;
}

/* Receive phase. */
if (intr & I2C_IC_INTR_STAT_R_RX_FULL && i2cp->state == I2C_ACTIVE_RX) {
while(i2cp->rxbytes > 0U && dp->STATUS & I2C_IC_STATUS_RFNE) {
/* Read out received data. */
i2cp->rxbuf[i2cp->rxindex] = (uint8_t)dp->DATACMD;
i2cp->rxindex++;
i2cp->rxbytes--;
if (i2cp->rxbytes > 0) {
// Set to master read
dp->DATACMD = I2C_IC_DATA_CMD_CMD |
(i2cp->rxbytes == 1 ? I2C_IC_DATA_CMD_STOP : 0);
return;

if (i2cp->rxbytes > 0U) {
/* Set to master read */
data = I2C_IC_DATA_CMD_CMD;

/* Send STOP after last byte. */
if (i2cp->rxbytes == 1U) {
data |= I2C_IC_DATA_CMD_STOP;
}

dp->DATACMD = data;
}
}
return;
}

if (dp->INTRSTAT & I2C_IC_INTR_STAT_R_STOP_DET) {
if (intr & I2C_IC_INTR_STAT_R_STOP_DET) {
/* Clear irq flag. */
(void)dp->CLRSTOPDET;
if (i2cp->state == I2C_ACTIVE_TX) {
Expand All @@ -158,13 +175,21 @@ static void serve_interrupt(I2CDriver *i2cp) {
/* Setup RX. */
/* Set interrupt mask. */
dp->INTRMASK = I2C_IC_INTR_MASK_M_STOP_DET |
I2C_IC_INTR_MASK_M_RX_FULL |
I2C_IC_INTR_MASK_M_TX_ABRT |
I2C_IC_INTR_MASK_M_RX_FULL;
I2C_IC_INTR_MASK_M_TX_OVER |
I2C_IC_INTR_MASK_M_RX_OVER |
I2C_IC_INTR_MASK_M_RX_UNDER;

data = I2C_IC_DATA_CMD_CMD | I2C_IC_CON_IC_RESTART_EN;

/* Send STOP after last byte. */
if (i2cp->rxbytes == 1U) {
data |= I2C_IC_DATA_CMD_STOP;
}

/* Set read flag. */
dp->DATACMD = I2C_IC_DATA_CMD_CMD |
I2C_IC_CON_IC_RESTART_EN |
(i2cp->rxbytes == 1 ? I2C_IC_DATA_CMD_STOP : 0);
dp->DATACMD = data;

/* State change to RX. */
i2cp->state = I2C_ACTIVE_RX;
Expand All @@ -176,17 +201,21 @@ static void serve_interrupt(I2CDriver *i2cp) {
}
}

if (dp->INTRSTAT & I2C_IC_INTR_STAT_R_TX_ABRT_Msk) {
// Reason, dp->TXABRTSOURCE
/* Transmission error detected. */
if (intr & (I2C_IC_INTR_STAT_R_TX_ABRT |
I2C_IC_INTR_STAT_R_TX_OVER |
I2C_IC_INTR_STAT_R_RX_OVER |
I2C_IC_INTR_STAT_R_RX_UNDER)) {
i2cp->errors = dp->TXABRTSOURCE;

_i2c_wakeup_error_isr(i2cp);
} else {
/* Transmission complete */
_i2c_wakeup_isr(i2cp);
}

/* Clear interrupt flags. */
/* Disable and clear interrupts. */
dp->INTRMASK = 0U;
(void)dp->CLRINTR;

_i2c_wakeup_isr(i2cp);
}

/*===========================================================================*/
Expand Down Expand Up @@ -349,6 +378,7 @@ msg_t i2c_lld_master_receive_timeout(I2CDriver *i2cp, i2caddr_t addr,
msg_t msg;
systime_t start, end;
I2C_TypeDef *dp = i2cp->i2c;
uint32_t data;

/* Releases the lock from high level driver.*/
osalSysUnlock();
Expand Down Expand Up @@ -387,14 +417,22 @@ msg_t i2c_lld_master_receive_timeout(I2CDriver *i2cp, i2caddr_t addr,

/* Set interrupt mask, active low. */
dp->INTRMASK = I2C_IC_INTR_MASK_M_STOP_DET |
I2C_IC_INTR_MASK_M_RX_FULL |
I2C_IC_INTR_MASK_M_TX_ABRT |
I2C_IC_INTR_MASK_M_RX_FULL;
I2C_IC_INTR_MASK_M_TX_OVER |
I2C_IC_INTR_MASK_M_RX_OVER |
I2C_IC_INTR_MASK_M_RX_UNDER;

dp->ENABLE = 1;

/* Read flag. */
dp->DATACMD = I2C_IC_DATA_CMD_CMD |
(i2cp->rxbytes == 1 ? I2C_IC_DATA_CMD_STOP : 0);
data = I2C_IC_DATA_CMD_CMD;
/* Send STOP after last byte. */
if (i2cp->rxbytes == 1){
data |= I2C_IC_DATA_CMD_STOP;
}

dp->DATACMD = data;

/* Waits for the operation completion or a timeout.*/
msg = osalThreadSuspendTimeoutS(&i2cp->thread, timeout);
Expand Down Expand Up @@ -483,8 +521,11 @@ msg_t i2c_lld_master_transmit_timeout(I2CDriver *i2cp, i2caddr_t addr,

/* Set interrupt mask. */
dp->INTRMASK = I2C_IC_INTR_MASK_M_STOP_DET |
I2C_IC_INTR_MASK_M_TX_EMPTY |
I2C_IC_INTR_MASK_M_TX_ABRT |
I2C_IC_INTR_MASK_M_TX_EMPTY;
I2C_IC_INTR_MASK_M_TX_OVER |
I2C_IC_INTR_MASK_M_RX_OVER |
I2C_IC_INTR_MASK_M_RX_UNDER;

dp->ENABLE = 1;

Expand Down

0 comments on commit 9f5d970

Please sign in to comment.