Message ID | 20221213062734.10930-1-n-francis@ti.com |
---|---|
State | Awaiting Upstream |
Delegated to: | Tom Rini |
Headers | show |
Series | [v2] board: ti: common: board_detect: Fix EEPROM read quirk for 2-byte | expand |
On Tue, Dec 13, 2022 at 11:57:34AM +0530, Neha Malcom Francis wrote: > 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 > > Signed-off-by: Neha Malcom Francis <n-francis@ti.com> > Fixes: d2ab2a2bafd5 (board: ti: common: board_detect: Fix EEPROM read quirk for AM6 style data) > Fixes: bf6376642fe8 (board: ti: common: board_detect: Fix EEPROM read quirk) There's not a great choice here between take this now for v2023.01 (when lots of people are going to be busy and not catch a new possible corner case) or put this in -next and have it be in v2023.04 (as places might pick v2023.01 to base on for a while). But, I'm leaning towards putting this in -next since it should be easy to cherry-pick this as needed.
вт, 13 дек. 2022 г. в 09:27, Neha Malcom Francis <n-francis@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 > > Signed-off-by: Neha Malcom Francis <n-francis@ti.com> > Fixes: d2ab2a2bafd5 (board: ti: common: board_detect: Fix EEPROM read quirk for AM6 style data) > Fixes: bf6376642fe8 (board: ti: common: board_detect: Fix EEPROM read quirk) Tested-By: Matwey V. Kornilov <matwey.kornilov@gmail.com> > --- > board/ti/common/board_detect.c | 46 +++++++++++++++++++++++++--------- > 1 file changed, 34 insertions(+), 12 deletions(-) > > Changes since v1: > - modified algorithm for boards that do not have CONFIG_DM_I2C > enabled as well > > diff --git a/board/ti/common/board_detect.c b/board/ti/common/board_detect.c > index c37629fe8a..9b32076ab9 100644 > --- a/board/ti/common/board_detect.c > +++ b/board/ti/common/board_detect.c > @@ -87,6 +87,8 @@ static int __maybe_unused ti_i2c_eeprom_get(int bus_addr, int dev_addr, > u32 header, u32 size, uint8_t *ep) > { > int rc; > + uint8_t offset_test; > + bool one_byte_addressing = true; > > #if CONFIG_IS_ENABLED(DM_I2C) > struct udevice *dev; > @@ -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..) > @@ -151,8 +168,23 @@ static int __maybe_unused ti_i2c_eeprom_get(int bus_addr, int dev_addr, > */ > (void)i2c_read(dev_addr, 0x0, byte, 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 = i2c_read(dev, 0x1, byte, &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 +476,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 >
On Tue, Dec 13, 2022 at 11:57:34AM +0530, Neha Malcom Francis wrote: > 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 > > Signed-off-by: Neha Malcom Francis <n-francis@ti.com> > Fixes: d2ab2a2bafd5 (board: ti: common: board_detect: Fix EEPROM read quirk for AM6 style data) > Fixes: bf6376642fe8 (board: ti: common: board_detect: Fix EEPROM read quirk) > Tested-By: Matwey V. Kornilov <matwey.kornilov@gmail.com> Applied to u-boot/next, thanks!
diff --git a/board/ti/common/board_detect.c b/board/ti/common/board_detect.c index c37629fe8a..9b32076ab9 100644 --- a/board/ti/common/board_detect.c +++ b/board/ti/common/board_detect.c @@ -87,6 +87,8 @@ static int __maybe_unused ti_i2c_eeprom_get(int bus_addr, int dev_addr, u32 header, u32 size, uint8_t *ep) { int rc; + uint8_t offset_test; + bool one_byte_addressing = true; #if CONFIG_IS_ENABLED(DM_I2C) struct udevice *dev; @@ -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..) @@ -151,8 +168,23 @@ static int __maybe_unused ti_i2c_eeprom_get(int bus_addr, int dev_addr, */ (void)i2c_read(dev_addr, 0x0, byte, 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 = i2c_read(dev, 0x1, byte, &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 +476,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;
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 Signed-off-by: Neha Malcom Francis <n-francis@ti.com> Fixes: d2ab2a2bafd5 (board: ti: common: board_detect: Fix EEPROM read quirk for AM6 style data) Fixes: bf6376642fe8 (board: ti: common: board_detect: Fix EEPROM read quirk) --- board/ti/common/board_detect.c | 46 +++++++++++++++++++++++++--------- 1 file changed, 34 insertions(+), 12 deletions(-) Changes since v1: - modified algorithm for boards that do not have CONFIG_DM_I2C enabled as well