STM32G431 I2C with HAL gets stuck in the interrupt

[qua]

I am trying to use a G431 as an I2C device (slave). Unfortunately it can be made to latch up rather easily, getting stuck with an endlessly triggering I2C interrupt. It appears to be a flaw in the HAL code where it doesn't clear the interrupt flags, possibly due to its internal state machine being out of sync with reality.

There are two situations that can cause this to happen.

1. The bus host (master) starts transactions back to back too quickly. Adding a 1ms delay between transactions fixes the problem.

2. DMA is used for reception, but the host stops sending data early. I.e. HAL_I2C_Slave_Seq_Receive_DMA() is called with a certain buffer size, but the number of bytes sent by the host is less than that size.

I thought that switching to interrupts instead of DMA might help, but it doesn't. It seems like the rate of interrupts triggering can cause the stuck-in-interrupt issue, but I have not deeply debugged it yet.

It seems like a reasonable implementation of the DMA mode would allow for recovery of this situation. I have tried adding code to HAL_I2C_ErrorCallback() that tries to reset both DMA and the I2C peripheral as follows:

HAL_I2C_DeInit(hi2c);
HAL_DMA_DeInit(&hdma_i2c2_rx);
HAL_DMA_DeInit(&hdma_i2c2_tx);
HAL_I2C_Init(hi2c);
HAL_I2C_EnableListen_IT(hi2c);

It does not fix the issue.

Also of note is that HAL_I2C_ErrorCallback() is called at the end of every transaction, even if it sends the expected number of bytes and completes normally. The error code is HAL_I2C_ERROR_AF, which is unclear. The datasheet does not mention it as a possible error condition, and it doesn't seem to align to any of the bits in the status register. It appears to simply trigger at the end of any TX transaction, presumably because the host did not ACK. It is unclear if anything needs to be done to handle it.

How do I make this reliable? My device accepts one byte written which is a register bank select, and then the host either continues to write up to the number of bytes in that bank, or issues a restart condition and reads up to the number of registers in that bank.

It needs to be robust and able to recover from errors. Thus far the only recovery I have found is to reset the MCU.

[TDK]

See here for an example of two boards talking to each other over I2C using DMA:

STM32CubeG4/Projects/NUCLEO-G431RB/Examples/I2C/I2C_TwoBoards_ComDMA/Inc/stm32g4xx_it.h at 5610410611606eae3dfa8d7e5e203312e40d9913 · STMicroelectronics/STM32CubeG4

If the master NACKs something, AF is the expected response. See the linked code for how to handle it.

[qua]

Thanks, I have seen that code before and it doesn't really help. For example, all the error handler does is flash an LED: https://github.com/STMicroelectronics/STM32CubeG4/blob/5610410611606eae3dfa8d7e5e203312e40d9913/Projects/NUCLEO-G431RB/Examples/I2C/I2C_TwoBoards_ComDMA/Src/main.c#L495

I have not tested it, but I would guess that it has the same flaw - if the amount of data transferred is not the expected amount, it gets stuck in the interrupt code. The interrupt code is part of the HAL, so clearly there is a bug because it should always clear the cause of the interrupt and not allow it to re-trigger no matter what.

Edit. Here is the relevant code in my application:

HAL_I2C_EnableListen_IT(&hi2c2)

...

/****************************************************************************************
* Address match callback
*/
void HAL_I2C_AddrCallback(I2C_HandleTypeDef *hi2c, uint8_t TransferDirection, uint16_t AddrMatchCode)
{
	if (hi2c != &hi2c2)	// only care about the device mode peripheral
		return;

	if (TransferDirection == I2C_DIRECTION_TRANSMIT)  // host transmitting data to us
	{
		if (bank_select_next)
		{
			if (HAL_I2C_Slave_Seq_Receive_IT(hi2c, &wo_temp, 1, I2C_NEXT_FRAME) != HAL_OK)
				HAL_I2C_ErrorCallback(hi2c);
		}
		else
		{
			if (HAL_I2C_Slave_Seq_Receive_DMA(hi2c, wo_ptr, wo_max_bytes, I2C_NEXT_FRAME) != HAL_OK)
				HAL_I2C_ErrorCallback(hi2c);
		}
	}
	else  // host reading data from us
	{
		if (HAL_I2C_Slave_Seq_Transmit_DMA(hi2c, ro_ptr, ro_max_bytes, I2C_NEXT_FRAME) != HAL_OK)
			HAL_I2C_ErrorCallback(hi2c);
	}
}


/****************************************************************************************
* RX (host writing to device) callback
*/
void HAL_I2C_SlaveRxCpltCallback(I2C_HandleTypeDef *hi2c)
{
	if (hi2c != &hi2c2)	// only care about the device mode peripheral
		return;

	if (bank_select_next)	// first byte is bank select
	{
		bank_select_next = false;
		ro_ptr = NULL;
		ro_max_bytes = 0;
		ro_bank = 0;
		if (wo_temp < NUM_RO_BANKS)
		{
			ro_bank = wo_temp;
			if (ro_banks[ro_bank].start_read_func != NULL)
				(*ro_banks[ro_bank].start_read_func)(&ro_ptr, &ro_max_bytes);
		}
		ro_read_count = 0;

		wo_ptr = NULL;
		wo_max_bytes = 0;
		wo_bank = 0;
		if (wo_temp < NUM_WO_BANKS)
		{
			wo_bank = wo_temp;
			if (wo_banks[wo_bank].start_write_func != NULL)
				(*wo_banks[wo_bank].start_write_func)(&wo_ptr, &wo_max_bytes);
		}
		wo_write_count = 0;
	}
	else
	{
		if (wo_banks[wo_bank].end_write_func != NULL)
			(*wo_banks[wo_bank].end_write_func)();
	}

	if (wo_ptr != 0)
		HAL_I2C_Slave_Seq_Receive_DMA(hi2c, wo_ptr, wo_max_bytes, I2C_NEXT_FRAME);
}


/****************************************************************************************
* TX (host reading from device) callback
*/
void HAL_I2C_SlaveTxCpltCallback(I2C_HandleTypeDef *hi2c)
{
	if (hi2c != &hi2c2)	// only care about the device mode peripheral
		return;

	if (HAL_I2C_Slave_Seq_Transmit_DMA(hi2c, ro_ptr, ro_max_bytes, I2C_NEXT_FRAME) != HAL_OK)
		HAL_I2C_ErrorCallback(hi2c);
}


void HAL_I2C_ErrorCallback(I2C_HandleTypeDef *hi2c)
{
	if (hi2c != &hi2c2)	// only care about the device mode peripheral
		return;

	uint32_t errorcode = HAL_I2C_GetError(hi2c);

	if (errorcode == HAL_I2C_ERROR_AF)  // AF error
	{
		//DBG_printf_from_isr("ECB end\r\n");
	}
	else
	{
		if (errorcode == HAL_I2C_ERROR_DMA)
		{
			HAL_I2C_DeInit(hi2c);
			HAL_DMA_DeInit(&hdma_i2c2_rx);
			HAL_DMA_DeInit(&hdma_i2c2_tx);
			HAL_I2C_Init(hi2c);
			HAL_I2C_EnableListen_IT(hi2c);
		}

		DBG_printf_from_isr("HAL_I2C_ErrorCallback() error = 0x%lX\r\n", errorcode);
		DBG_printf_from_isr("HAL_I2C_ErrorCallback() ISR = 0x%X\r\n", hi2c->Instance->ISR);
	}

	bank_select_next = true;
}

void HAL_I2C_ListenCpltCallback(I2C_HandleTypeDef *hi2c)
{
	if (hi2c != &hi2c2)	// only care about the device mode peripheral
		return;

	bank_select_next = true;
	HAL_I2C_EnableListen_IT(&hi2c2);
}

void HAL_I2C_AbortCpltCallback(I2C_HandleTypeDef *hi2c)
{
	DBG_printf_from_isr("HAL_I2C_AbortCpltCallback()\r\n");
}

[TDK]

Your code is not at all similar to the linked example. Particularly with code in the transfer complete callbacks.

> For example, all the error handler does is flash an LED:

How is that relevant? AF does not call error_handler and nor should anything else.

I feel like I'm not being too helpful here, but it's hard to get motivated to help when you won't even run the working example code as-is and just claim it also has bugs without even running it.

[qua]

If you don't want to help that's fine, but don't blame me for your lack of effort.

The issue is specifically to do with unexpected situations, which the example code does not handle. To humour you I loaded it up and a Nucleo board and was able to cause the MCU to get stuck in the interrupt by sending it an unexpectedly short buffer.

If you scroll up a bit from my link, you will notice that immediately above it the HAL error callback simply calls that LED flash. It is literally designed to lock up on an error, not recover from it.

As I explained in the very first post, the AF "error" does in fact cause the error handler callback to be called. In fact, your example code tests for it in the error callback: https://github.com/STMicroelectronics/STM32CubeG4/blob/5610410611606eae3dfa8d7e5e203312e40d9913/Projects/NUCLEO-G431RB/Examples/I2C/I2C_TwoBoards_ComDMA/Src/main.c#L483

And avoids locking up in that case, because it's not really an error, it's the normal flow of I2C transactions.