From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eugen.Hristev at microchip.com Date: Thu, 28 May 2020 04:25:10 +0000 Subject: [PATCH v2] misc: i2c_eeprom: implement different probe test eeprom offset In-Reply-To: <949801e5-d765-cec0-8484-d94fe206a632@denx.de> References: <20200507085318.559418-1-eugen.hristev@microchip.com> <87a72j7pqo.fsf@tarshish> <12f772a5-0046-f8a4-2a0d-f25a1d47e709@microchip.com> <949801e5-d765-cec0-8484-d94fe206a632@denx.de> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 28.05.2020 06:59, Heiko Schocher wrote: > Hello Eugen, > > Am 07.05.2020 um 17:08 schrieb Eugen.Hristev at 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 >>>>> --- >>>>> >>>>> 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 >>>> >>>> @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------------{= >>> >>> ???? - baruch at 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: hs at denx.de