On 11.08.25 05:16, E Shattow wrote:
On 8/9/25 01:21, Heinrich Schuchardt wrote:
When writing the EEPROM fails, the command usage help text is displayed
after the error message. We should only display the error message instead.
If writing the EEPROM fails, return CMD_RET_FAILURE (1) instead of
CMD_RET_USAGE (-1).
Fixes: aea1bd95b61e ("eeprom: starfive: Enable ID EEPROM configuration")
Signed-off-by: Heinrich Schuchardt <heinrich.schucha...@canonical.com>
---
board/starfive/visionfive2/visionfive2-i2c-eeprom.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
b/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
index 010e386e64d..17a44020bcf 100644
--- a/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
+++ b/board/starfive/visionfive2/visionfive2-i2c-eeprom.c
@@ -275,7 +275,7 @@ static int prog_eeprom(unsigned int size)
if (is_match_magic()) {
printf("MAGIC ERROR, Please check the data@%p.\n", pbuf.buf);
- return -1;
+ return CMD_RET_FAILURE;
}
ret = i2c_get_chip_for_busnum(CONFIG_SYS_EEPROM_BUS_NUM,
@@ -285,7 +285,7 @@ static int prog_eeprom(unsigned int size)
if (ret) {
printf("Get i2c bus:%d addr:%d fail.\n",
CONFIG_SYS_EEPROM_BUS_NUM,
CONFIG_SYS_I2C_EEPROM_ADDR);
- return ret;
+ return CMD_RET_FAILURE;
}
for (i = 0, p = (u8 *)pbuf.buf; i < size; ) {
@@ -314,11 +314,11 @@ static int prog_eeprom(unsigned int size)
if (ret) {
has_been_read = -1;
printf("Programming failed.\n");
- return -1;
+ return CMD_RET_FAILURE;
}
printf("Programming passed.\n");
- return 0;
+ return CMD_RET_SUCCESS;
}
/**
This patch is a good start, but I think maybe more is needed? Quoting
the notes I wrote for myself last year:
"
mac command:
if data did not change then "Programming passed." is printed regardless
of write protect state but no action was taken and no usage help is
printed. 'mac' still outputs in-memory info.
when data was changed the command shows "Programming failed." or
"Programming passed." followed by usage help and 'mac' command shows no
output until 'mac read_eeprom'.
Suggested:
If data did not change at time of 'mac eeprom_write' then it should
still try to write so the message pass/fail will be correct.
The usage info following 'mac write_eeprom' is a bug, should not be printed.
Thank you for reviewing.
The EEPROM driver does the right thing. If data is not changed, do not
write. This avoids wear.
We would have to keep a second copy of the EEPROM contents when reading
if we wanted to observe that no change was done.
This is beyond the scope of the current patch.
prog_eeprom return values inconsistent with command.h
prog_eeprom returns -1 which overlaps with CMD_RET_USAGE
should be 1 for failure
This is covered by the current patch.
show_eeprom()
Print a message "No valid data. Please do mac read_eeprom".
Maybe "Read nnn bytes from EEPROM" and "Displaying local copy of EEPROM
from memory" to be more clear.
show_eeprom() already checks the value of variable has_been_read and
doesn't display the EEPROM content if it is not set. A message could be
added in this case.
But the EEPROM is read before reaching the coammand line if
CONFIG_ID_EEPROM=y:
common/board_r.c:732: INITCALL(mac_read_from_eeprom);
Therefore we typically don't observe has_been_read=0 when executing the
mac command.
Best regards
Heinrich
"
Acked-by: E Shattow <e...@freeshell.de>
