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

[Christoph Niedermaier <cniedermaier@dh-electronics.com>]

From: Marek Vasut <marex@denx.de>
Sent: Tuesday, October 22, 2024 1:01 AM
> 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.

Sorry, I mean the function dh_get_value_from_eeprom_id_page() here.

>> 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.

Sorry, here I mean also the function dh_get_value_from_eeprom_id_page().

> 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.
> 
> [...]

I don't want to allocate the memory in the function board_init(). I want to handle
size and caching in the function dh_get_value_from_eeprom_id_page(). So I don't
want to deal with size and structure in the function board_init(). For me, the use
of a static variable is OK, but you don't like it. Do you know how I can do this
without assigning a static variable, but not allocate the memory somewhere in a
caller function?


Regards
Christoph