[PATCH] misc: i2c_eeprom: fix at24c32 offset_len

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.
By having an offset len of 2 bytes, reading from the eeprom at boot time to read the stored MAC address fails on sam9x60ek board which has this eeprom.
When changing the offset len to 1 byte, reading the MAC address from the offset inside the EEPROM works correctly.
Fixes: 821c982e35 ("misc: i2c_eeprom: set offset len and chip addr offset mask") Reported-by: Sergiu Moga sergiu.moga@microchip.com Signed-off-by: Eugen Hristev eugen.hristev@microchip.com Tested-by: Claudiu Beznea claudiu.beznea@microchip.com ---
Hi everyone,
We tested this on sam9x60ek board. I see in DT that various other boards use this eeprom. I have added several people to the patch e-mail, however not comprehensively everyone. I do not wish to affect functionality for other boards, even though this patch appears correct. Some other tests would be welcome, on other boards having this EEPROM.
Thanks! Eugen
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, };
static const struct i2c_eeprom_drv_data atmel24c64_data = {

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.
Rasmus

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
participants (3)
-
Eugen Hristev
-
Eugen.Hristev@microchip.com
-
Rasmus Villemoes