| Message ID | 20231030170458.21668-1-p-mantena@ti.com |
|---|---|
| State | Accepted |
| Commit | dd83c1c865f5bd94efae10d4a8e519ad08c8486b |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [v3] board: ti: common: board_detect: Fix EEPROM offset read for 1-byte | expand |
Hi Prasanth, On 30/10/23 22:34, Prasanth Babu Mantena wrote: > EEPROM detection logic in ti_i2c_eeprom_get() involves reading > the total size and the 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 a wrong pointer to > compare. > > The value with one offset is read into offset_test, but the pointer > used to match was still ep, resulting in an invalid comparison of the > values. The intent is to identify bad 2-byte addressing eeproms that > get stuck on the successive reads. > > 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> > --- > v3 <--> v2: > Improved and concise commit description. > > 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??? */ Reviewed-by: Neha Malcom Francis <n-francis@ti.com>
On 15:15-20231107, Neha Malcom Francis wrote: > Hi Prasanth, > > On 30/10/23 22:34, Prasanth Babu Mantena wrote: > > EEPROM detection logic in ti_i2c_eeprom_get() involves reading > > the total size and the 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 a wrong pointer to > > compare. > > > > The value with one offset is read into offset_test, but the pointer > > used to match was still ep, resulting in an invalid comparison of the > > values. The intent is to identify bad 2-byte addressing eeproms that > > get stuck on the successive reads. > > > > 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> > > --- > > v3 <--> v2: > > Improved and concise commit description. > > > > 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??? */ > > Reviewed-by: Neha Malcom Francis <n-francis@ti.com> > Can this patch be considered for merge considering no further comments. Regards, Prasanth > -- > Thanking You > Neha Malcom Francis
On Fri, Dec 15, 2023 at 11:55:20AM +0530, Prasanth Mantena wrote: > On 15:15-20231107, Neha Malcom Francis wrote: > > Hi Prasanth, > > > > On 30/10/23 22:34, Prasanth Babu Mantena wrote: > > > EEPROM detection logic in ti_i2c_eeprom_get() involves reading > > > the total size and the 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 a wrong pointer to > > > compare. > > > > > > The value with one offset is read into offset_test, but the pointer > > > used to match was still ep, resulting in an invalid comparison of the > > > values. The intent is to identify bad 2-byte addressing eeproms that > > > get stuck on the successive reads. > > > > > > 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> > > > --- > > > v3 <--> v2: > > > Improved and concise commit description. > > > > > > 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??? */ > > > > Reviewed-by: Neha Malcom Francis <n-francis@ti.com> > > > > Can this patch be considered for merge considering no further comments. Given the "fun" of the various beagle families I think I was still hoping for a tested-by on some of the corner cases they have.
On Mon, Oct 30, 2023 at 10:34:58PM +0530, Prasanth Babu Mantena wrote: > EEPROM detection logic in ti_i2c_eeprom_get() involves reading > the total size and the 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 a wrong pointer to > compare. > > The value with one offset is read into offset_test, but the pointer > used to match was still ep, resulting in an invalid comparison of the > values. The intent is to identify bad 2-byte addressing eeproms that > get stuck on the successive reads. > > 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> > Reviewed-by: Neha Malcom Francis <n-francis@ti.com> Applied to u-boot/master, thanks!
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??? */
EEPROM detection logic in ti_i2c_eeprom_get() involves reading the total size and the 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 a wrong pointer to compare. The value with one offset is read into offset_test, but the pointer used to match was still ep, resulting in an invalid comparison of the values. The intent is to identify bad 2-byte addressing eeproms that get stuck on the successive reads. 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> --- v3 <--> v2: Improved and concise commit description. 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(-)