On 28.05.2020 06:59, Heiko Schocher wrote: > Hello Eugen, > > Am 07.05.2020 um 17:08 schrieb eugen.hris...@microchip.com: >> 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. > > Any news here?
Hello Heiko, I do not have any other memory in the compatible list at my disposal right now. I assumed that you would take this patch as-is. If I hit the issue on another memory, I will send another patch. Right now with the lockdown this is the only memory I have. Eugen > > Thanks! > > bye, > HEiko >> >> 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 - >>> >> > > -- > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: h...@denx.de