[RFC PATCH] board: ti: common: board_detect: Fix EEPROM read quirk for 2-byte

[Neha Malcom Francis]

Hi Matwey

On 29/11/22 17:40, Matwey V. Kornilov wrote:
> вт, 29 нояб. 2022 г. в 09:50, Neha Malcom Francis <n-francis at ti.com>:
>>
>> EEPROM detection logic in ti_i2c_eeprom_get() involves figuring out
>> whether addressing is 1-byte or 2-byte. There are currently different
>> behaviours seen across boards as documented in commit bf6376642fe8
>> ("board: ti: common: board_detect: Fix EEPROM read quirk"). Adding to
>> the list, we see that there are 2-byte EEPROMs that read properly
>> with 1-byte addressing with no offset.
>>
>> For ti_i2c_eeprom_am6_get where eeprom parse operation is dynamic, the
>> earlier commit d2ab2a2bafd5 ("board: ti: common: board_detect: Fix
>> EEPROM read quirk for AM6 style data") tried to resolve this by running
>> ti_i2c_eeprom_get() twice. However this commit along with its former
>> commit fails on J7 platforms where EEPROM successfully return back the
>> header on 1-byte addressing and continues to do so until an offset is
>> introduced. So the second read incorrectly determines the EEPROM as
>> 1-byte addressing.
>>
>> A more generic solution is introduced here to solve
>> this issue: 1-byte read without offset and 1-byte read with offset. If
>> both passes, it follows 1-byte addressing else we proceed with 2-byte
>> addressing check.
>>
>> Tested on J721E, J7200, DRA7xx, AM64x
> 
> I'll try to test this on the AM335x boards I have as soon as possible.

Thanks!
> 
>>
>> Signed-off-by: Neha Malcom Francis <n-francis at ti.com>
>> Fixes: d2ab2a2bafd5 (board: ti: common: board_detect: Fix EEPROM read
>> quirk for AM6 style data) and bf6376642fe8 (board: ti: common: board_detect:
>> Fix EEPROM read quirk)
>> ---
>>   board/ti/common/board_detect.c | 29 ++++++++++++++++++-----------
>>   1 file changed, 18 insertions(+), 11 deletions(-)
>>
>> diff --git a/board/ti/common/board_detect.c b/board/ti/common/board_detect.c
>> index c37629fe8a..b9f2ebf2a0 100644
>> --- a/board/ti/common/board_detect.c
>> +++ b/board/ti/common/board_detect.c
>> @@ -91,6 +91,8 @@ static int __maybe_unused ti_i2c_eeprom_get(int bus_addr, int dev_addr,
>>   #if CONFIG_IS_ENABLED(DM_I2C)
> 
> Should #else branch also be modified according to the new algo?

Yeah you're right; let me try getting into that as well.

> 
>>          struct udevice *dev;
>>          struct udevice *bus;
>> +       uint8_t offset_test;
>> +       bool one_byte_addressing = true;
>>
>>          rc = uclass_get_device_by_seq(UCLASS_I2C, bus_addr, &bus);
>>          if (rc)
>> @@ -114,8 +116,23 @@ static int __maybe_unused ti_i2c_eeprom_get(int bus_addr, int dev_addr,
>>           */
>>          (void)dm_i2c_read(dev, 0, ep, size);
>>
>> +       if (*((u32 *)ep) != header)
>> +               one_byte_addressing = false;
>> +
>> +       /*
>> +        * Handle case of bad 2 byte eeproms that responds to 1 byte addressing
>> +        * but gets stuck in const addressing when read requests are performed
>> +        * on offsets. We perform an offset test to make sure it is not a 2 byte
>> +        * eeprom that works with 1 byte addressing but just without an offset
>> +        */
>> +
>> +       rc = dm_i2c_read(dev, 0x1, &offset_test, sizeof(offset_test));
>> +
>> +       if (*((u32 *)ep) != (header & 0xFF))
>> +               one_byte_addressing = false;
>> +
>>          /* Corrupted data??? */
>> -       if (*((u32 *)ep) != header) {
>> +       if (!one_byte_addressing) {
>>                  /*
>>                   * read the eeprom header using i2c again, but use only a
>>                   * 2 byte address (some newer boards need this..)
>> @@ -444,16 +461,6 @@ int __maybe_unused ti_i2c_eeprom_am6_get(int bus_addr, int dev_addr,
>>          if (rc)
>>                  return rc;
>>
>> -       /*
>> -        * Handle case of bad 2 byte eeproms that responds to 1 byte addressing
>> -        * but gets stuck in const addressing when read requests are performed
>> -        * on offsets. We re-read the board ID to ensure we have sane data back
>> -        */
>> -       rc = ti_i2c_eeprom_get(bus_addr, dev_addr, TI_EEPROM_HEADER_MAGIC,
>> -                              sizeof(board_id), (uint8_t *)&board_id);
>> -       if (rc)
>> -               return rc;
>> -
>>          if (board_id.header.id != TI_AM6_EEPROM_RECORD_BOARD_ID) {
>>                  pr_err("%s: Invalid board ID record!\n", __func__);
>>                  return -EINVAL;
>> --
>> 2.34.1
>>
> 
> 

-- 
Thanking You
Neha Malcom Francis