Re: [PATCH 1/3] cmd: tlv_eeprom: remove use of global variable has_been_read

[Josua Mayer <josua@solid-run.com>]

Hi Stefan,

Thanks for reviewing. I just rebased on 
https://source.denx.de/u-boot/u-boot.git / master,
and will send new version soon.

- Josua Mayer

Am 03.05.23 um 09:55 schrieb Stefan Roese:
> Hi Josua,
>
> On 4/29/23 11:15, Josua Mayer wrote:
>> has_been_read is only used as an optimization for do_tlv_eeprom.
>> Explicitly use and set inside this function, thus making read_eeprom
>> stateless.
>>
>> Signed-off-by: Josua Mayer <josua@solid-run.com>
>> Cc: Stefan Roese <sr@denx.de>
>> Cc: Baruch Siach <baruch@tkos.co.il>
>> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
>
> This patchset does not apply to master. Could you please check, rebase
> and send again?
>
> Thanks,
> Stefan
>
>> ---
>>   cmd/tlv_eeprom.c | 21 +++++++++++----------
>>   1 file changed, 11 insertions(+), 10 deletions(-)
>>
>> diff --git a/cmd/tlv_eeprom.c b/cmd/tlv_eeprom.c
>> index 8049bf9843c..d36401e9138 100644
>> --- a/cmd/tlv_eeprom.c
>> +++ b/cmd/tlv_eeprom.c
>> @@ -42,8 +42,6 @@ static int set_date(char *buf, const char *string);
>>   static int set_bytes(char *buf, const char *string, int 
>> *converted_accum);
>>   static void show_tlv_devices(int current_dev);
>>   -/* Set to 1 if we've read EEPROM into memory */
>> -static int has_been_read;
>>   /* The EERPOM contents after being read into memory */
>>   static u8 eeprom[TLV_INFO_MAX_LEN];
>>   @@ -130,9 +128,6 @@ static int read_eeprom(int devnum, u8 *eeprom)
>>       struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
>>       struct tlvinfo_tlv *eeprom_tlv = to_entry(&eeprom[HDR_SIZE]);
>>   -    if (has_been_read)
>> -        return 0;
>> -
>>       /* Read the header */
>>       ret = read_tlv_eeprom((void *)eeprom_hdr, 0, HDR_SIZE, devnum);
>>       /* If the header was successfully read, read the TLVs */
>> @@ -149,10 +144,8 @@ static int read_eeprom(int devnum, u8 *eeprom)
>>           update_crc(eeprom);
>>       }
>>   -    has_been_read = 1;
>> -
>>   #ifdef DEBUG
>> -    show_eeprom(eeprom);
>> +    show_eeprom(devnum, eeprom);
>>   #endif
>>         return ret;
>> @@ -432,10 +425,16 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int 
>> flag, int argc, char *const argv[])
>>       char cmd;
>>       struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
>>       static unsigned int current_dev;
>> +    /* Set to 1 if we've read EEPROM into memory */
>> +    static int has_been_read;
>> +    int ret;
>>         // If no arguments, read the EERPOM and display its contents
>>       if (argc == 1) {
>> -        read_eeprom(current_dev, eeprom);
>> +        if (!has_been_read) {
>> +            if (read_eeprom(current_dev, eeprom) == 0)
>> +                has_been_read = 1;
>> +        }
>>           show_eeprom(current_dev, eeprom);
>>           return 0;
>>       }
>> @@ -447,8 +446,10 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int 
>> flag, int argc, char *const argv[])
>>       // Read the EEPROM contents
>>       if (cmd == 'r') {
>>           has_been_read = 0;
>> -        if (!read_eeprom(current_dev, eeprom))
>> +        if (read_eeprom(current_dev, eeprom) == 0) {
>>               printf("EEPROM data loaded from device to memory.\n");
>> +            has_been_read = 1;
>> +        }
>>           return 0;
>>       }
>
> Viele Grüße,
> Stefan Roese
>