On 10/17/24 1:55 PM, Christoph Niedermaier wrote:
[...]
+ __func__, ret);
This will be printed on every device, even the ones without ID EEPROM,
correct ? This should not be printed on devices without ID EEPROM. Also,
This is suppressed by the -ENOENT check.
Does i2c_eeprom_read() in dh_get_value_from_eeprom_id_page() return
-ENOENT in case the EEPROM is described in DT, but not populated on the
board ? I suspect it returns some other error code, -ETIMEDOUT or
-EINVAL maybe ?
It could only be possible if the DTO for hardware rev. 100 (which has no
EEPROM ID page) is not loaded. The return value then is -ENODEV.
I will included this in v2.
OK
Also, this shouldn't be repeatedly reading the EEPROM, the EEPROM read
operation is the slow part, the EEPROM is 32 bytes, so the EEPROM should
be read once and cached, and once the cache is populated all read
accesses to the EEPROM should use the cache.
This is already covered in function dh_get_value_from_eeprom_id_page().
It seems that function always calls
ret = i2c_eeprom_read(dev, 0x0, eipa, sizeof(eipa));
?
The 32 bytes are read into a static variable. If the ID (DHE) already exists
in it, the i2c_eeprom_read() function is no longer called.
Hmmm, but it seems all the functions which access this EEPROM WLP are
called from board_init(), are they not ?
If yes, then there is no need for any static variable:
board_init() {
u8 eeprom[32];
dh_read_eeprom_wlp(eeprom); // read the eeprom once
dh_setup_mac_address(eeprom); // extract MAC from EEPROM
dh_add_item_number_and_serial_to_env(eeprom); // extract SN from EEPROM
// once this function exits, the eeprom variable on stack is discarded
// which is OK, since it won't be used anymore anyway
}
[...]
