On 07.05.2020 18:02, Baruch Siach wrote: > Hi Heiko, > > On Thu, May 07 2020, Heiko Schocher wrote: >> Am 07.05.2020 um 10:53 schrieb Eugen Hristev: >>> Because of this commit : >>> 5ae84860b0 ("misc: i2c_eeprom: verify that the chip is functional at >>> probe()") >>> at probe time, each eeprom is tested for read at offset 0. >>> >>> The Atmel AT24MAC402 eeprom has different mapping. One i2c slave address is >>> used for the lower 0x80 bytes and another i2c slave address is used for the >>> upper 0x80 bytes. Because of this basically the i2c master sees 2 different >>> slaves. We need the upper bytes because we read the unique MAC address from >>> this EEPROM area. >>> >>> However this implies that our slave address will return error on reads >>> from address 0x0 to 0x80. >>> >>> To solve this, implemented an offset field inside platform data that is by >>> default 0 (as it is used now), but can be changed in the compatible table. >>> >>> The probe function will now read at this offset and use it, instead of >>> blindly >>> checking offset 0. >>> >>> This will fix the regression noticed on these EEPROMs since the commit >>> abovementioned that introduces the probe failed issue. >>> >>> Signed-off-by: Eugen Hristev <eugen.hris...@microchip.com> >>> --- >>> >>> Please disregard patch >>> [PATCH] misc: i2c_eeprom: implement different probe test eeprom offset >>> >>> as that patch was mistakenly done on an older u-boot version. >>> This v2 patch applies correctly on u-boot/master >>> >>> Changes in v2: >>> - adapt to latest changes in driver. v1 was done on u-boot 2020.01 >>> accidentaly. >>> >>> drivers/misc/i2c_eeprom.c | 8 +++++++- >>> 1 file changed, 7 insertions(+), 1 deletion(-) >> >> Thanks for rebasing! >> >> I prefer to fix the issue instead reverting commit: >> >> 5ae84860b0 ("misc: i2c_eeprom: verify that the chip is functional at >> probe()") >> >> Reviewed-by: Heiko Schocher <h...@denx.de> >> >> @Baruch, is this Ok for you? > > According to the Linux driver there are more devices that need read > offset. 24cs32 and 24cs64 are affected. This patch does not fix the > regression for those devices. > > Eugen, would it be possible for you to extend the fix to 24cs32/64 and > test on real hardware?
Hi Baruch, This patch fixes my issue at hand and the eeprom which I have at my disposal. I will look to see if I have those, but, most likely not. I can extend the fix, but I cannot test, so, I rather not go blind with it. Eugen > > baruch > >>> diff --git a/drivers/misc/i2c_eeprom.c b/drivers/misc/i2c_eeprom.c >>> index ef5f103c98..32a1b20856 100644 >>> --- a/drivers/misc/i2c_eeprom.c >>> +++ b/drivers/misc/i2c_eeprom.c >>> @@ -17,6 +17,7 @@ struct i2c_eeprom_drv_data { >>> u32 pagesize; /* page size in bytes */ >>> u32 addr_offset_mask; /* bits in addr used for offset overflow */ >>> u32 offset_len; /* size in bytes of offset */ >>> + u32 start_offset; /* valid start offset inside memory, by default 0 */ >>> }; >>> int i2c_eeprom_read(struct udevice *dev, int offset, uint8_t *buf, int >>> size) >>> @@ -147,7 +148,11 @@ static int i2c_eeprom_std_probe(struct udevice *dev) >>> i2c_set_chip_addr_offset_mask(dev, data->addr_offset_mask); >>> /* Verify that the chip is functional */ >>> - ret = i2c_eeprom_read(dev, 0, &test_byte, 1); >>> + /* >>> + * Not all eeproms start from offset 0. Valid offset is available >>> + * in the platform data struct. >>> + */ >>> + ret = i2c_eeprom_read(dev, data->start_offset, &test_byte, 1); >>> if (ret) >>> return -ENODEV; >>> @@ -215,6 +220,7 @@ static const struct i2c_eeprom_drv_data >>> atmel24mac402_data = { >>> .pagesize = 16, >>> .addr_offset_mask = 0, >>> .offset_len = 1, >>> + .start_offset = 0x80, >>> }; >>> static const struct i2c_eeprom_drv_data atmel24c32_data = { >>> > > > -- > ~. .~ Tk Open Systems > =}------------------------------------------------ooO--U--Ooo------------{= > - bar...@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il - >