Bug in stm32f4xx_hal_smbus.c as generated by STM32CubeMX

[LGjes.1]

STM32CubeMX v6.2.1 emits the following line of code in function HAL_SMBUS_Init() in file stm32f4xx_hal_smbus.c:

MODIFY_REG(hsmbus->Instance->CR1, (I2C_CR1_NOSTRETCH | I2C_CR1_ENGC | I2C_CR1_PEC | I2C_CR1_ENARP | I2C_CR1_SMBTYPE | I2C_CR1_SMBUS), (hsmbus->Init.NoStretchMode | hsmbus->Init.GeneralCallMode | hsmbus->Init.PacketErrorCheckMode | hsmbus->Init.PeripheralMode));

Isn't is a bug to use the constant I2C_CR1_PEC in this place? I would think is should be I2C_CR1_ENPEC instead.

[Bubbles]

Hello,

I believe I2C_CR1_PEC is correct name. It matches the bit name in RM and the header file.

Also, check out this expansion package:

https://www.st.com/en/embedded-software/x-cube-smbus.html/

It should help a lot with SMBUS development, providing middleware and basic self-test templates.

Rgds,

Jarda

[LGjes.1]

Well, I still believe it is an error:

Consider a scenario where you first configure SMBus with PEC enabled and then reconfigure SMBus to disable PEC.

This will not work, because the ENPEC bit of CR1 (bit position 5) will not be modified from 1 to 0 by the line of code I showed above.

What will happen is that the PEC bit (position 12) will be set to 0, but that is not the intention.

[Bubbles]

Hello @LGjes.1​ ,

sorry I did not realize this is the legacy I2C interface used in F4. It works differently in most (newer) products.

You are right. Thank you for reporting the bug.