drivers: eeprom: stm32: HAL return value handling #97279
Conversation
zephyrbot
requested review from
FRASTM,
GeorgeCGV,
djiatsaf-st,
erwango,
gautierg-st,
henrikbrixandersen,
marwaiehm-st and
mathieuchopstm
drivers/eeprom/eeprom_stm32.c
Outdated
| if (HAL_FLASHEx_DATAEEPROM_Unlock() != HAL_OK) { | ||
| return -EIO; | ||
| } |
There was a problem hiding this comment.
Nit: there should be consistency about whether driver uses
if (xxx() != HAL_OK) {
...fail
}or
ret = xxx();
if (ret != HAL_OK) {
...fail
}There was a problem hiding this comment.
I think when the return value is needed only to be tested straight, it's not really needed to be stored then tested, as here. Is this function, below, the return value is used not only with an if() instruction.
That said, I'm fine using the latter way (among the 2 you mentioned above).
| if (ret != HAL_OK) { | ||
| LOG_ERR("failed to write to EEPROM (err %d)", ret); | ||
| HAL_FLASHEx_DATAEEPROM_Lock(); | ||
| k_mutex_unlock(&lock); |
There was a problem hiding this comment.
Seomthing's wrong in the 1st commit. EEPROM should be locked back. I'll fix.
drivers/eeprom/eeprom_stm32.c
Outdated
| if (HAL_FLASHEx_DATAEEPROM_Unlock() != HAL_OK) { | ||
| return -EIO; | ||
| } |
There was a problem hiding this comment.
I think when the return value is needed only to be tested straight, it's not really needed to be stored then tested, as here. Is this function, below, the return value is used not only with an if() instruction.
That said, I'm fine using the latter way (among the 2 you mentioned above).
Correct eeprom_stm32_write() to return a valid errno instead of mixing HAL return values and errno return values. Clarify HAL return value is of type HAL_StatusTypeDef and not an int in eeprom_stm32_read(). Signed-off-by: Etienne Carriere <[email protected]>
Add missing test of HAL_FLASHEx_DATAEEPROM_Unlock() return value Signed-off-by: Etienne Carriere <[email protected]>
etienne-lms
force-pushed
the
stm32-hal-ret-eeprom
branch
from
bfde38d to
bc8ef62
Compare
|
4 participants
Correct
eeprom_stm32_write()to return a valid errno instead of mixing HAL return values and errno return values.Clarify HAL return value is of type
HAL_StatusTypeDefand not anintineeprom_stm32_read().