stmicroelectronics / stm32g0xx_hal_driver Goto Github PK

Hello,

I found that the Timeout parameter in both HAL_I2C_Master_Receive & HAL_I2C_Mem_Read functions is not used in these two lines:

I'm not sure if this is a bug, but in my case, I need to change it to :
if (I2C_WaitOnFlagUntilTimeout(hi2c, I2C_FLAG_BUSY, SET, Timeout, tickstart) != HAL_OK)

How to reproduce the "bug ": Call the HAL_I2C_Master_Receive or HAL_I2C_Mem_Read function with a timeout parameter of less than 25ms, and don't power up the i2c device to trigger the timeout.

Describe the set-up

• MCU: STM32G0B0RET

Describe the bug
It looks like the I2C HAL driver handles the NACK bit incorrectly, at least in slave mode with Slave Byte Control enabled. Consider e.g. the following line:

Bit 15 NACK: NACK generation (slave mode)
The bit is set by software, cleared by hardware when the NACK is sent, or when a STOP
condition or an Address matched is received, or when PE=0.
0: an ACK is sent after current received byte.
1: a NACK is sent after current received byte.
Note: Writing ‘0’ to this bit has no effect.
This bit is used in slave mode only: in master receiver mode, NACK is automatically
generated after last byte preceding STOP or RESTART condition, whatever the NACK
bit value.
When an overrun occurs in slave receiver NOSTRETCH mode, a NACK is
automatically generated whatever the NACK bit value.
When hardware PEC checking is enabled (PECBYTE=1), the PEC acknowledge value
does not depend on the NACK value.

Source: RM0450, Page 762 (rm0454-stm32g0x0-advanced-armbased-32bit-mcus-stmicroelectronics.pdf)

Note the "Writing 0 to this bit has no effect". This means that once the bit is incorrectly set, it can never be fixed again

How To Reproduce

1. Set up a board with the MCU, enable one of the I2C peripherals & hook up a device capable of being an I2C master
2. Add this code to the main file of a new project:

char buf;

void start_receive(I2C_HandleTypeDef *hi2c)
{
    LL_I2C_SetTransferSize(hi2c->Instance, 0); // set NBYTES to 0 to prevent premature release of clock
    LL_I2C_EnableReloadMode(hi2c->Instance);
    HAL_I2C_Slave_Seq_Receive_IT(hi2c, &buf, 1, I2C_FIRST_AND_NEXT_FRAME);
    LL_I2C_SetTransferSize(hi2c->Instance, 1); // set NBYTES to 1 to release clock & start receiving
}

void HAL_I2C_AddrCallback(I2C_HandleTypeDef *hi2c, uint8_t TransferDirection, uint16_t AddrMatchCode)
{
    LL_I2C_EnableSlaveByteControl(hi2c->Instance);
    start_receive(hi2c);
}

void HAL_I2C_ListenCpltCallback(I2C_HandleTypeDef *hi2c)
{
    HAL_I2C_EnableListen_IT(hi2c);
}

void HAL_I2C_SlaveRxCpltCallback(I2C_HandleTypeDef *hi2c)
{
    start_receive(hi2c);
}

/* USER CODE END 0 */

/**
  * @brief  The application entry point.
  * @retval int
  */
int main(void)
{
  /* USER CODE BEGIN 1 */

  /* USER CODE END 1 */

  /* MCU Configuration--------------------------------------------------------*/

  /* Reset of all peripherals, Initializes the Flash interface and the Systick. */
  HAL_Init();

  /* USER CODE BEGIN Init */

  /* USER CODE END Init */

  /* Configure the system clock */
  SystemClock_Config();

  /* USER CODE BEGIN SysInit */

  /* USER CODE END SysInit */

  /* Initialize all configured peripherals */
  MX_GPIO_Init();
  MX_I2C2_Init();
  /* USER CODE BEGIN 2 */

  HAL_I2C_EnableListen_IT(&hi2c2);
  /* USER CODE END 2 */

  /* Infinite loop */
  /* USER CODE BEGIN WHILE */
  while (1)
  {
    /* USER CODE END WHILE */

    /* USER CODE BEGIN 3 */
  }
  /* USER CODE END 3 */
}

3. This code sets up the I2C slave to continuously receive bytes, e.g.
   
4. Set a breakpoint in the following line:
   

   stm32g0xx_hal_driver/Src/stm32g0xx_hal_i2c.c

   Lines 6291 to 6292 in b363874

   	/* Disable Address Acknowledge */
   	hi2c->Instance->CR2 |= I2C_CR2_NACK;

5. Initiate two more I2C writes. The first will be acknowledged, then the breakpoint will be hit.
6. Continue execution. The second write will not be acknowledged, because the NACK bit was set after the next byte already started being received:
   

Additional context
At least in my use case with Slave Byte Control enabled, this leads to spurious NACKs if you're unlucky with timing (see reproduction steps). Simply removing the lines appears to work in my case, and I don't see what that could break. Am I missing something?

Describe the set-up

• Tested on nucleo-g0b1re
• arm-none-eabi-gcc version 10.3.1 (but compiler does not matter)

Describe the bug
Calling function HAL_SPI_Receive_IT() or HAL_SPI_TransmitReceive_IT() with buffer not aligned to 16 bits crashes MCU

How To Reproduce

1. Call HAL_SPI_Receive_IT() with unaligned buffer and more then 1 byte to send

Additional context
Real problem however is in SPI_2linesTxISR_8BIT() function that tries to optimize send using 16 bits access if there is more data.
For Cortex-M0+ unaligned read results in fault.

static void SPI_2linesTxISR_8BIT(struct __SPI_HandleTypeDef *hspi)
{
  /* Transmit data in packing Bit mode */
  if (hspi->TxXferCount >= 2U &&
    ((uint32_t)(hspi->pTxBuffPtr) & 1) == 0) // <= !!!! Additional check to use 8 bit for unaligned buffer here fixes the problem
  {
    hspi->Instance->DR = *((uint16_t *)hspi->pTxBuffPtr); // <= !!! Crashes here on Cortex-M0+
    hspi->pTxBuffPtr += sizeof(uint16_t);
    hspi->TxXferCount -= 2U;
  }
...

Describe the set-up
Any board.
Any IDE/compiler.
Noticed with MC Library with HAL/LL that uses ADC value left alignment.

Describe the bug
In recent version of HAL/LL for G0, functions LL_ADC_REG_ReadConversionData12, LL_ADC_REG_ReadConversionData10, LL_ADC_REG_ReadConversionData8 and LL_ADC_REG_ReadConversionData6 were modified with data filters, respectively 0x00000FFFUL, 0x000003FFUL, 0x000000FFUL, and 0x0000003FUL.
This prevent them from working when LL_ADC_DATA_ALIGN_LEFT is configured.

How To Reproduce
Any project using MC Library or LL_ADC_DATA_ALIGN_LEFT with G0/HAL_LL.

Additional context
Removing the filters mentioned above solves the issue.
For example, for 12 bit data value, the correct function is:

__STATIC_INLINE uint16_t LL_ADC_REG_ReadConversionData12(const ADC_TypeDef *ADCx)
{
  return (uint16_t)(READ_BIT(ADCx->DR, ADC_DR_DATA));
}

I have a firmware running on the NUCLEO-G0B1RE that receives bulk out transfers over USB and echos them back.
The endpoint is setup for double buffering.
At high bus load some transfers are aborted with a STALL.

The firmware is ported by @marckleinebudde can be found here https://github.com/marckleinebudde/candleLight_fw/tree/stm32g0

I suspect the cause to be in Inc/stm32g0xx_ll_usb.h#L659.
It does two writes to the buffer descriptor table.
One to clear BLSIZE, NUM_BLOCK and COUNTn_RX and one the set BLSIZE and NUM_BLOCK.
This results in a small window in which NUM_BLOCK is 0 while the endpoint is valid.

As stated in RM0444 37.5.2

Reception memory buffer locations are written starting from the address contained in the
ADDRn_RX for a number of bytes corresponding to the received data packet length, or up
to the last allocated memory location, as defined by BLSIZE and NUM_BLOCK, whichever
comes first. In this way, the USB peripheral never writes beyond the end of the allocated
reception memory buffer area. If the length of the data packet payload (actual number of
bytes used by the application) is greater than the allocated buffer, the USB peripheral
detects a buffer overrun condition. In this case, a STALL handshake is sent instead of the
usual ACK to notify the problem to the host, no interrupt is generated and the transaction is
considered failed.

So either make sure COUNTn_RX is cleared without clearing NUM_BLOCK or setting STATRX to NAK while changing the entry.

Please see https://community.st.com/s/question/0D53W00001yhRP0SAM/issue-if-tick-rollover-occurs-while-waiting-for-flash
and fix.