[PATCH v2] board: ti: common: board_detect: Fix EEPROM read quirk for 2-byte
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
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;
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!
participants (3)
-
Matwey V. Kornilov -
Neha Malcom Francis -
Tom Rini