Some time back I wrote some code to control an I2C peripheral. It is working fine, but it is not so robust against noise on the lines. I am now handling all error conditions and then manually reset the I2C bus via software before continuing. This helped a lot, but one issue remains.
When I do bus fault injection tests that short SDA to ground I can make the CPU lock up. What happens is that the BERR flag is set in SR1. This flag is not handled, so the interrupt keeps firing. Everything hangs and finally the watchdog resets the system.
I fixed this as follows:
Code: Select all
if (event & I2C_SR1_BERR) {
dp->SR1 &= ~I2C_SR1_BERR;
i2cp->errors = I2C_BUS_ERROR;
_i2c_wakeup_error_isr(i2cp);
return;
}
This fixes most of the problems, but still after some seconds of testing the CPU crashes. There are two possible things that seem to happen:
The first is that the BERR flag simply stays set. I cannot clear it, of course, the handler keeps getting called. I tried to clear it via gdb but this also didn't work. I then added the following code:
Code: Select all
dp->CR2 &=~ I2C_CR2_ITEVTEN | I2C_CR2_ITERREN | I2C_CR2_ITBUFEN;
This disables all interrupts, so even if the BERR flag stays set, the CPU does not crash. I then stop and start the peripheral from userspace, which is able to clear the BERR condition (due to the call to rccResetI2Cn() in the lld_start).
The second thing that can happen is more harmful. The handler keeps getting called again, but now no special flags are set. As such, it is not really possible to detect this case. As a workaround, I added a simple counter that will terminate the transaction if more interrupts are handled than that would be required to complete the current transfer.
With these three fixes I am able to inject errors (SDA hard low/high, SCL hard low/high, SDA open, SCL open, SDA short to SCL) indefinitely without causing any crashes and I am able to recover from it.
I am happy that this works, but I am wondering what would be the clean way to implement the fix in the ChibiOS HAL so I can contribute it back. Normally you should be able to do a new transfer after the MSG_ERR return value, but with this workaround you also need to restart the peripheral (since interrupts are disabled, and it is in a broken state anyway). Should I set a flag that the peripheral needs to be reinitialized before starting a new transfer, and then reinit if needed in the transfer request function?
Sincerely,
Dodo
