Re: [[PATCH v2] 4/4] cmd: tlv_eeprom: enable 'dev' subcommand before 'read'

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

Hi Simon,

I have a private rewrite of the tlv functionality as a seperate library 
outside of the command implementation.
Originally I had hoped to finish upstreaming that about a year ago but 
got sidetracked.

Hopefully within the coming weeks I can submit another version as RFC 
that can address your concerns more easily than the current eeprom_tlv 
implementation.

Sincerely
Josua Mayer

Am 08.05.23 um 17:42 schrieb Simon Glass:
> Hi,
>
> On Fri, 5 May 2023 at 02:22, Josua Mayer <josua@solid-run.com> wrote:
>> Move the handler for "tlv_eeprom dev X" command to the beginning of
>> do_tlv_eeprom, to allow using it before issuing a "read" command for
>> currently selected eeprom.
>>
>> Also remove the check if eeprom exists, since that can only work after
>> the first execution of read_eeprom triggered device lookup.
>> Instead accept values up to the defined array size (MAX_TLV_DEVICES).
>>
>> Signed-off-by: Josua Mayer <josua@solid-run.com>
>> Reviewed-by: Stefan Roese <sr@denx.de>
>> Cc: Stefan Roese <sr@denx.de>
>> Cc: Baruch Siach <baruch@tkos.co.il>
>> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> ---
>>   cmd/tlv_eeprom.c | 26 ++++++++++++++++----------
>>   1 file changed, 16 insertions(+), 10 deletions(-)
> Can someone take a look at fixing up the driver model code?
>
> For example:
> - it maintains its own array of eproms, when it should use driver
> model to find them.
> - the has_been_read flag seems to track only one EEPROM, but the code
> indicates there might be two (should be in the device's private data)
> - current_dev should be a pointer to a device
> - ideally all state should be either in the device or the uclass, so
> it can be used before relocation, etc.
>
> Regards,
> Simon