[PATCH v2] misc: i2c_eeprom: implement different probe test eeprom offset

[Heiko Schocher]

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 <eugen.hristev at 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 <hs at 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?

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