Re: [PATCH] misc: i2c_eeprom: fix at24c32 offset_len

[<Eugen.Hristev@microchip.com>]

On 4/28/22 2:35 PM, Rasmus Villemoes wrote:
> On 28/04/2022 13.11, Eugen Hristev wrote:
>> According to at24c32 datasheet:
>>
>> RANDOM READ: A random read requires a “dummy” byte write sequence to load in
>> the dataword address. Once the device address word and data word address are
>> clocked in and acknowledged by the EEPROM, the microcontroller must generate
>> another start condition.
>>
>> BYTE WRITE: A write operation requires two 8-bit data word addresses following
>> the device address word and acknowledgment. Upon receipt of this address, the
>> EEPROM will again respond with a zero and then clock in the first 8-bit data
>> word.
>>
>>  From this, my understanding is that dataword is 1 byte, and when reading the
>> offset is just 1 byte.
> 
> Yes, you read data byte by byte, but that doesn't mean all those bytes
> can be addressed using a single byte...
> 
>>   drivers/misc/i2c_eeprom.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/misc/i2c_eeprom.c b/drivers/misc/i2c_eeprom.c
>> index 89a450d0f8..c8c67cf028 100644
>> --- a/drivers/misc/i2c_eeprom.c
>> +++ b/drivers/misc/i2c_eeprom.c
>> @@ -230,7 +230,7 @@ static const struct i2c_eeprom_drv_data atmel24c32_data = {
>>        .size = 4096,
>>        .pagesize = 32,
>>        .addr_offset_mask = 0,
>> -     .offset_len = 2,
>> +     .offset_len = 1,
>>   };
> 
> But this can't be correct, because how could one then possibly address
> all 4096 bytes? Note that some eeproms larger than 256 bytes exist but
> still use a 1-byte address; that's because they then respond to multiple
> i2c addresses - that's what the "addr_offset_mask" is about.
> 
> Something else must be going on in your case, I think. Are you sure the
> device tree is correct, i.e. that the eeprom is actually that one and
> not one that does indeed use 1-byte addressing? I got curious and
> downloaded the "SAM9X60-EK Board Design Files" from
> https://www.microchip.com/en-us/development-tool/DT100126#Documentation,
> inside which one finds mention of a "MCHP MEMORY SERIAL EEPROM 2Kb I2C
> 24AA025E48T-I/OT SOT-23-6", and 2Kb == 256 byte.
> 
> If you have the physical board handy, I'd try to locate the eeprom and
> see what's printed on it.


Hi,

You are right. I thought that this EEPROM was handled by the same 
compatible, since this is how it's been in DT and the same in Linux DT. 
Then I looked at the 24c32 datasheet and could not understand what was 
the problem there, since it worked... except offset addressing.
Using a 1byte offset len makes it work great, but as you said, it will 
break boards with the real 24c32 trying to access beyond 1 byte addresses.
I will try to change the compatible to the correct 24aa025e48 and try 
this again, it will likely work because in the driver 24aa02e48 really 
has an offset_len of 1 byte.

24aa02e48 and 24aa025e48 differ in page size, 8 bytes vs 16 bytes, so I 
will add 24aa025e48 with a page size of 16 first and then change the DT 
for the board.

Thanks for helping with this ! (so the patch can be dropped)

Eugen

> 
> Rasmus
>