diff mbox series

[v2] board: ti: common: board_detect: Fix EEPROM offset read for 1-byte

Message ID 20231026122413.4116-1-p-mantena@ti.com
State Superseded
Delegated to: Tom Rini
Headers show
Series [v2] board: ti: common: board_detect: Fix EEPROM offset read for 1-byte | expand

Commit Message

Prasanth Babu Mantena Oct. 26, 2023, 12:24 p.m. UTC
EEPROM detection logic in ti_i2c_eeprom_get() involves reading the total
size followed by reading 1-byte size with an offset 1. This commit fixes
the header matching issue in commit 9f393a2d7af8 ("board: ti: common:
board_detect: Fix EEPROM read quirk for 2-byte").

In the previous commit, the value with one offset is being read into
offset_test, but the pointer used to match was still ep. ep is the
pointer where previously read data is stored, resulting in an invalid
comparision of the values. The intent is to identify bad 2-byte
addressing eerpoms, which respond to the initial 1-byte addressing
request and gets stuck on the succesive reads. After successive read
with an offset 1, the 1-byte data is compared with the second byte of
the header to ensure it as a valid 1byte addressing eeprom.
This is taken care by comparing proper first byte value from, header
with an offset 1 byte, to offset_test having the 1-byte data read from eeprom.

Fixes: 9f393a2d7af8 (board: ti: common: board_detect: Fix EEPROM read quirk for 2-byte)
Signed-off-by: Prasanth Babu Mantena <p-mantena@ti.com>
---
v2 <--> v1:
Fix inplace for the else condition of CONFIG_IS_ENABLED(DM_I2C).
Improved commit message.

 board/ti/common/board_detect.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Matwey V. Kornilov Oct. 26, 2023, 7:49 p.m. UTC | #1
чт, 26 окт. 2023 г. в 15:24, Prasanth Babu Mantena <p-mantena@ti.com>:
>
> EEPROM detection logic in ti_i2c_eeprom_get() involves reading the total
> size followed by reading 1-byte size with an offset 1. This commit fixes
> the header matching issue in commit 9f393a2d7af8 ("board: ti: common:
> board_detect: Fix EEPROM read quirk for 2-byte").
>
> In the previous commit, the value with one offset is being read into
> offset_test, but the pointer used to match was still ep. ep is the
> pointer where previously read data is stored, resulting in an invalid
> comparision of the values. The intent is to identify bad 2-byte
> addressing eerpoms, which respond to the initial 1-byte addressing
> request and gets stuck on the succesive reads. After successive read
> with an offset 1, the 1-byte data is compared with the second byte of
> the header to ensure it as a valid 1byte addressing eeprom.
> This is taken care by comparing proper first byte value from, header
> with an offset 1 byte, to offset_test having the 1-byte data read from eeprom.
>
> Fixes: 9f393a2d7af8 (board: ti: common: board_detect: Fix EEPROM read quirk for 2-byte)
> Signed-off-by: Prasanth Babu Mantena <p-mantena@ti.com>

Tested-by: Matwey V. Kornilov <matwey.kornilov@gmail.com>

At least I see no regressions here.

> ---
> v2 <--> v1:
> Fix inplace for the else condition of CONFIG_IS_ENABLED(DM_I2C).
> Improved commit message.
>
>  board/ti/common/board_detect.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/board/ti/common/board_detect.c b/board/ti/common/board_detect.c
> index 9a53884c98..869f7a47f8 100644
> --- a/board/ti/common/board_detect.c
> +++ b/board/ti/common/board_detect.c
> @@ -128,7 +128,7 @@ static int __maybe_unused ti_i2c_eeprom_get(int bus_addr, int dev_addr,
>
>         rc = dm_i2c_read(dev, 0x1, &offset_test, sizeof(offset_test));
>
> -       if (*((u32 *)ep) != (header & 0xFF))
> +       if (offset_test != ((header >> 8) & 0xFF))
>                 one_byte_addressing = false;
>
>         /* Corrupted data??? */
> @@ -180,7 +180,7 @@ static int __maybe_unused ti_i2c_eeprom_get(int bus_addr, int dev_addr,
>
>         rc = i2c_read(dev_addr, 0x1, byte, &offset_test, sizeof(offset_test));
>
> -       if (*((u32 *)ep) != (header & 0xFF))
> +       if (offset_test != ((header >> 8) & 0xFF))
>                 one_byte_addressing = false;
>
>         /* Corrupted data??? */
> --
> 2.39.0
>
Nishanth Menon Oct. 26, 2023, 9:02 p.m. UTC | #2
On 17:54-20231026, Prasanth Babu Mantena wrote:
> EEPROM detection logic in ti_i2c_eeprom_get() involves reading the total
> size followed by reading 1-byte size with an offset 1. This commit fixes
> the header matching issue in commit 9f393a2d7af8 ("board: ti: common:
> board_detect: Fix EEPROM read quirk for 2-byte").
> 
> In the previous commit, the value with one offset is being read into
> offset_test, but the pointer used to match was still ep. ep is the
> pointer where previously read data is stored, resulting in an invalid
> comparision of the values. The intent is to identify bad 2-byte
> addressing eerpoms, which respond to the initial 1-byte addressing
> request and gets stuck on the succesive reads. After successive read
> with an offset 1, the 1-byte data is compared with the second byte of
> the header to ensure it as a valid 1byte addressing eeprom.
> This is taken care by comparing proper first byte value from, header
> with an offset 1 byte, to offset_test having the 1-byte data read from eeprom.

Rephrase (I at times go to grammarly.com to make sure I use the right
verbage): So, keep it simple (we can read code - no need to give us the
detail readup). Stuff like "in the previous commit" leaves us wondering
which commit.

EEPROM detection logic in ti_i2c_eeprom_get() involves reading the total
size followed by reading 1-byte size with an offset 1.
The commit 9f393a2d7af8 ("board: ti: common: board_detect: Fix EEPROM
read quirk for 2-byte") that attempts to fix this uses the wrong pointer to
to compare.

> 
> Fixes: 9f393a2d7af8 (board: ti: common: board_detect: Fix EEPROM read quirk for 2-byte)
> Signed-off-by: Prasanth Babu Mantena <p-mantena@ti.com>
> ---
> v2 <--> v1:
> Fix inplace for the else condition of CONFIG_IS_ENABLED(DM_I2C).
> Improved commit message.
> 
>  board/ti/common/board_detect.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/board/ti/common/board_detect.c b/board/ti/common/board_detect.c
> index 9a53884c98..869f7a47f8 100644
> --- a/board/ti/common/board_detect.c
> +++ b/board/ti/common/board_detect.c
> @@ -128,7 +128,7 @@ static int __maybe_unused ti_i2c_eeprom_get(int bus_addr, int dev_addr,
>  
>  	rc = dm_i2c_read(dev, 0x1, &offset_test, sizeof(offset_test));
>  
> -	if (*((u32 *)ep) != (header & 0xFF))
> +	if (offset_test != ((header >> 8) & 0xFF))
>  		one_byte_addressing = false;
>  
>  	/* Corrupted data??? */
> @@ -180,7 +180,7 @@ static int __maybe_unused ti_i2c_eeprom_get(int bus_addr, int dev_addr,
>  
>  	rc = i2c_read(dev_addr, 0x1, byte, &offset_test, sizeof(offset_test));
>  
> -	if (*((u32 *)ep) != (header & 0xFF))
> +	if (offset_test != ((header >> 8) & 0xFF))
>  		one_byte_addressing = false;
>  
>  	/* Corrupted data??? */
> -- 
> 2.39.0
>
diff mbox series

Patch

diff --git a/board/ti/common/board_detect.c b/board/ti/common/board_detect.c
index 9a53884c98..869f7a47f8 100644
--- a/board/ti/common/board_detect.c
+++ b/board/ti/common/board_detect.c
@@ -128,7 +128,7 @@  static int __maybe_unused ti_i2c_eeprom_get(int bus_addr, int dev_addr,
 
 	rc = dm_i2c_read(dev, 0x1, &offset_test, sizeof(offset_test));
 
-	if (*((u32 *)ep) != (header & 0xFF))
+	if (offset_test != ((header >> 8) & 0xFF))
 		one_byte_addressing = false;
 
 	/* Corrupted data??? */
@@ -180,7 +180,7 @@  static int __maybe_unused ti_i2c_eeprom_get(int bus_addr, int dev_addr,
 
 	rc = i2c_read(dev_addr, 0x1, byte, &offset_test, sizeof(offset_test));
 
-	if (*((u32 *)ep) != (header & 0xFF))
+	if (offset_test != ((header >> 8) & 0xFF))
 		one_byte_addressing = false;
 
 	/* Corrupted data??? */