[PATCH] cmd: tlv_eeprom: fix signature for populate_serial_number function

[Heinrich Schuchardt]

On 5/16/23 13:49, Josua Mayer wrote:
> Hi Heinrich,
>
> Am 16.05.23 um 14:21 schrieb Heinrich Schuchardt:
>> On 5/16/23 10:27, Josua Mayer wrote:
>>> populate_serial_number is not used internally for the tlv_eeprom
>>> command, but rather provided as a library function for external use..
>>> Remove the devnum that had recently been added by mistake, returning the
>>> function to its original signature.
>>>
>>> Instead place a 0-initialised member variable inside the function to
>>> same purpose, along with a node that it only supports reading from the
>>
>> %s/node/note/
> Good find!
>>
>>> first EEPROM in the system.
>>>
>>> Fixes: dfda0c0 ("cmd: tlv_eeprom: remove use of global variable
>>> current_dev")
>>> Signed-off-by: Josua Mayer <josua at solid-run.com>
>>> Cc: Stefan Roese <sr at denx.de>
>>> Cc: Baruch Siach <baruch at tkos.co.il>
>>> Cc: Heinrich Schuchardt <xypron.glpk at gmx.de>
>>> ---
>>>   cmd/tlv_eeprom.c | 3 ++-
>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/cmd/tlv_eeprom.c b/cmd/tlv_eeprom.c
>>> index 79796394c5c..0ca4d714645 100644
>>> --- a/cmd/tlv_eeprom.c
>>> +++ b/cmd/tlv_eeprom.c
>>> @@ -1100,11 +1100,12 @@ int mac_read_from_eeprom(void)
>>>    *
>>>    *  This function must be called after relocation.
>>>    */
>>> -int populate_serial_number(int devnum)
>>> +int populate_serial_number(void)
>>
>> If populate_serial_number() is to be used as a library function, it
>> should live in lib/ or possibly in drivers/misc/. The definition needs
>> to be provided in an include file. Otherwise the function should be
>> deleted.
>>
>> Where will this library function be used?
> I don't know for sure. GitHub search finds its use in board files on
> some old ONIE u-boot forks.
>
> Main purpose of this patch right now is to undo my accidental change and
> restore the default behaviour:
> The previous instance of devnum variable inside tlv_eeprom.c would have
> had value 0 by default.

The right places to call the function might be
board/solidrun/clearfog/clearfog.c and board/kobol/helios4/helios4.c as
these platforms enable CMD_TLV_EEPROM. But I have no such board.

The function has remained unused in upstream U-Boot since 2020. If we do
not plan to use it, it should be removed.

Stefan and Dennis, what are your thoughts?

Best regards

Heinrich

>
>
>>
>> Shouldn't the EEPROM with the serial number be identified via the
>> device-tree?
> Something like that would be nice.
> However we also want to use identical device-tree between Linux and U-Boot.
> I am not sure if we have such indication.
>>
>> Best regards
>>
>> Heinrich
>>
>>>   {
>>>       char serialstr[257];
>>>       int eeprom_index;
>>>       struct tlvinfo_tlv *eeprom_tlv;
>>> +    int devnum = 0; // TODO: support multiple EEPROMs
>>>
>>>       if (env_get("serial#"))
>>>           return 0;
>>
>
> - Josua Mayer
>