On Fri, Aug 19, 2022 at 4:30 AM Nishanth Menon <n...@ti.com> wrote: > > On 11:28-20220818, Matwey V. Kornilov wrote: > > I've played a little and now I believe that the issue is that EEPROM read > > addr > > pointer is somehow corrupted due to 1-byte address write. The EEPROM is > > definitely have two-byte read address accoring the datasheet. > > I've failed to unravel exact rule what is happening when only one address > > byte > > is set, but was able to read random places of EEPROM. > > > > > > However, the following diff makes the board bootable. > > > > > > diff --git a/board/ti/common/board_detect.c b/board/ti/common/board_detect.c > > index ed34991377..26edddccc6 100644 > > --- a/board/ti/common/board_detect.c > > +++ b/board/ti/common/board_detect.c > > @@ -86,7 +86,6 @@ __weak void gpi2c_init(void) > > static int __maybe_unused ti_i2c_eeprom_get(int bus_addr, int dev_addr, > > u32 header, u32 size, uint8_t > > *ep) > > { > > - u32 hdr_read = 0xdeadbeef; > > int rc; > > > > #if CONFIG_IS_ENABLED(DM_I2C) > > @@ -113,10 +112,10 @@ static int __maybe_unused ti_i2c_eeprom_get(int > > bus_addr, int dev_addr, > > * We must allow for fall through to check the data if 2 byte > > * addressing works > > */ > > - (void)dm_i2c_read(dev, 0, (uint8_t *)&hdr_read, 4); > > + rc = dm_i2c_read(dev, 0, ep, size); > > > > /* Corrupted data??? */ > > - if (hdr_read != header) { > > + if (rc || (*((u32*)ep) != header)) { > > /* > > * read the eeprom header using i2c again, but use only a > > * 2 byte address (some newer boards need this..) > > @@ -125,16 +124,13 @@ static int __maybe_unused ti_i2c_eeprom_get(int > > bus_addr, int dev_addr, > > if (rc) > > return rc; > > > > - rc = dm_i2c_read(dev, 0, (uint8_t *)&hdr_read, 4); > > + rc = dm_i2c_read(dev, 0, ep, size); > > if (rc) > > return rc; > > } > > - if (hdr_read != header) > > + if (*((u32*)ep) != header) > > return -1; > > > > - rc = dm_i2c_read(dev, 0, ep, size); > > - if (rc) > > - return rc; > > #else > > u32 byte; > > This does work. I tested a few variations of boards to check this > concept out.. but anyways.. on beaglebone black (element 14 boards): > > NOTE: This will improve detection times for 1 byte eeprom based boot, > since there is no retry.. > > However for boards with 2 byte addressing eeproms: > > master branch: https://pasteboard.co/n3P8yhSq6pem.png > Time from first attempt to read eeprom to actual trigger of final eeprom > read attempt: ~4ms > > With this patch: https://pasteboard.co/IVQzHwMuhc4p.png > Time from first attempt to read eeprom to actual trigger of final eeprom > read attempt: ~18ms > > IMHO, 14ms penalty is'nt a bad deal for dealing with variations of > eeproms we are seeing in the wild. > > You can find the data (analog+digital capture) here: > https://github.com/nmenon/data-captures/tree/main/i2c-eeprom-1byte-captures > Tool used to capture (and view): https://www.saleae.com/downloads/ > > Tom, Robert, folks: what do you folks think?
I'm okay with the delay. if we only had 'one' production run it would be a different story.. But with 10 years of manufacturing, parts will EOL and vary. Let's go with the safe slower route.. Regards, -- Robert Nelson https://rcn-ee.com/