[PATCH v3 1/4] common: Add generic function for reading serial number

[Artur Rojek]

>Hi Artur,
>
>On Mon, 2 Oct 2023 at 06:42, Artur Rojek <artur at conclusive.pl> wrote:
>>
>> Provide a generic way for boards to read their serial number from EEPROM
>> at init.
>>
>> If CONFIG_ID_EEPROM is set, the new serial_read_from_eeprom() function
>> will now be called during the post-relocation part of the board init.
>>
>> Provided is the tlv eeprom implementation of the above function, making
>> use of the existing, yet never utilized, populate_serial_number().
>> Boards which use custom logic for interaction with their EEPROMs need to
>> supply their own implementation.
>>
>> Signed-off-by: Artur Rojek <artur at conclusive.pl>
>> ---
>>
>> v3: - restore original function name and make it static
>>     - provide a generic function for reading EEPROM serial number and
>>       wrap it around the existing tlv logic
>>     - move the env var check out of populate_serial_number() and into
>>       the new serial_read_from_eeprom() in order to stay consistent with
>>       the documentation
>>
>> v2: - rename the function
>>     - move function documentation from .c file to the prototype location
>>
>>  cmd/tlv_eeprom.c | 25 +++++++++----------------
>>  common/board_r.c |  8 ++++++++
>>  include/init.h   | 14 ++++++++++++++
>>  3 files changed, 31 insertions(+), 16 deletions(-)
>
>Can you please use events for this? Something like EVT_SETTINGS_R ?
>
>See the one recently added for how to do this:
>
>INITCALL_EVENT(EVT_LAST_STAGE_INIT),

I like this approach, but just to be clear with your intention - you
want me to move both serial_read_from_eeprom AND mac_read_from_eeprom
into a separate function, defined for each affected board? To do this
for mac_read_from_eeprom becomes slightly cumbersome, because there are
this many of them, depending on the current scheme:

$ grep -r "ID_EEPROM" ./configs/ | wc -l
55

If each of them needs to contain something like this:

>static int settings_r(void)
>{
>#if defined(CONFIG_ID_EEPROM)
>       mac_read_from_eeprom();
>#endif
>       return 0;
>}
>EVENT_SPY_SIMPLE(EVT_SETTINGS_R, settings_r);

then this strays very far from the original intention of this series,
which is to add support for a single board :)
Unless you only care about serial_read_from_eeprom, then I don't need to
modify any of the existing boards.

Cheers,
Artur

>
>Regards,
>Simon