[PATCH 2/2] arm64: imx8mp: Read item and serial number from EEPROM ID page on DH i.MX8MP DHCOM

[Marek Vasut]

On 10/21/24 5:38 PM, Christoph Niedermaier wrote:

[...]

>> 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
>> }
> 
> The idea is that function dh_add_item_number_and_serial_to_env() encapsulates
> the reading.

The function which interacts with the EEPROM on start up, once, can have 
this name, sure.

> I don't want to have to worry about the structure and number of
> bytes of the EEPROM ID page and want to be independent of it. It is planned to
> extend the structure and increase the number of bytes for the STM32MP2. The
> changes to the size will then depend on the version of the data in the header
> within the structure.

It is OK to allocate 32 or 64 bytes on stack, since that chunk of memory 
is free'd on return from the function. The lifespan of that allocated 
memory is exactly as long as it should be, and it does not waste U-Boot 
memory unnecessarily.

So far, all known EEPROMs have WLP size of 32 or 64 Byes.

> However, these changes should then only have to be made
> within the function dh_add_item_number_and_serial_to_env() for reading the
> EEPROM ID page. I accept the static variable in order to better isolate the
> function.

This can very well be:

dh_add_item_number_and_serial_to_env() {
    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
}

board_init() {
   ..
   dh_add_item_number_and_serial_to_env();
   ...
}

> Since the memory is freed up again when the operating system boots,
> this consumption of 32 bytes in U-Boot is not a problem, because it is only
> temporary.
This is not good, please do not waste memory in U-Boot if it can be 
easily avoided by automatically allocating it on stack and freeing it 
when not needed.

[...]