at24: Support SMBus read/write of 16-bit devices
Subsystems (2)
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH v5 1/1 at24: Support SMBus read/write of 16-bit devices
Previously, the at24 driver would bail out in the case of a 16-bit addressable EEPROM attached to an SMBus controller. This is because SMBus block reads and writes don't map to I2C multi-byte reads and writes when the offset portion is 2 bytes.
Instead of bailing out, this patch settles for functioning with single byte read SMBus cycles. Writes can be block or single-byte, depending on SMBus controller features.
Read access is not without some risk. Multiple SMBus cycles are required to read even one byte. If the SMBus has multiple masters and one accesses this EEPROM between the dummy address write and the subsequent current-address-read cycle(s), this driver will receive data from the wrong address.
Functionality has been tested with the following devices:
AT24CM01 attached to Intel ISCH SMBus
AT24C512 attached to Intel I801 SMBus
Read performance:
3.6 KB/s with 32-byte* access
*limited to 32-bytes by I2C_SMBUS_BLOCK_MAX.
Write performance:
248 B/s with 1-byte page (default)
3.9 KB/s with 128-byte* page (via platform data)
*limited to 31-bytes by I2C_SMBUS_BLOCK_MAX - 1.
---
v2 - Account for changes related to introduction of
i2c_smbus_read_i2c_block_data_or_emulated()
v3 - Consolidate three patches into one
- Expand comments regarding SMBus multi-master read risks.
- Rely on current-address-read for improved read performance (i.e. one dummy address write followed by multiple individual byte reads). This improves performance from 1.4 KiB/s to 3.6 KiB/s.
- Use struct at24_data's writebuf instead of kzalloc-ing
- Only limit write_max by 1-byte when accessing a 16-bit device with block writes instead of attempting to preserve a power-of-two.
- Style fixes (indentation, parentheses, unnecessary masking, etc.)
v4 - Address 16-bit safety in Kconfig
- Set "count" to zero later in at24_smbus_read_block_data()
- Fix over-80-columns issues in at24_eeprom_read()
- Fix write_max off-by-one in at24_probe()
- Check SMBus functionality needed for 16-bit device reads
- Homogenize indentation of SMBus functionality checks for SMBus write
v5 - 16-bit device read needs READ_BYTE not READ_BYTE_DATA
- Clarify write_max limiting with smbus_max
- Add X-ES copyright
drivers/misc/eeprom/Kconfig | 5 +- drivers/misc/eeprom/at24.c | 132 +++++++++++++++++++++++++++++++++++++++----- 2 files changed, 122 insertions(+), 15 deletions(-)
drivers/misc/eeprom/Kconfig +4 -1
@@ -22,7 +22,10 @@ config EEPROM_AT24 If you use this with an SMBus adapter instead of an I2C adapter, full functionality is not available. Only smaller devices are - supported (24c16 and below, max 4 kByte). + supported via block reads (24c16 and below, max 4 kByte). + Larger devices that use 16-bit addresses will only work with + individual byte reads, which is very slow in general and is unsafe + in multi-master SMBus topologies. This driver can also be built as a module. If so, the module will be called at24.
drivers/misc/eeprom/at24.c +118 -14
188 lines not on this page · raw message
Signature · 1 line
1.9.1
Hi Aaron,
drivers/misc/eeprom/at24.c static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id) · line 618
Quoted 7 lines · Aaron Sierra
drivers/misc/eeprom/at24.c +7
-0+if ((chip.flags &AT24_FLAG_ADDR16 ) && +i2c_check_functionality (client->adapter, +I2C_FUNC_SMBUS_READ_BYTE | +I2C_FUNC_SMBUS_WRITE_BYTE_DATA )) { +/* +* We need SMBUS_WRITE_BYTE_DATA and +* SMBUS_READ_BYTE_DATA to implement byte reads for
It's a minor thing really but I'm afraid you forgot to update this comment (SMBUS_READ_BYTE not SMBUS_READ_BYTE_DATA.)
drivers/misc/eeprom/at24.c static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id) · line 627
Quoted 9 lines
drivers/misc/eeprom/at24.c +6
-0+* 16-bit address devices. This will be slow, but +* better than nothing (e.g. read @ 3.6 KiB/s). It is +* also unsafe in a multi-master topology. +*/ + use_smbus =I2C_SMBUS_BYTE_DATA ; + }else if (i2c_check_functionality (client->adapter,I2C_FUNC_SMBUS_READ_I2C_BLOCK )) { use_smbus =I2C_SMBUS_I2C_BLOCK_DATA ; } else if (i2c_check_functionality(client->adapter,
Rest looks alright.
Signature · 2 lines
Jean Delvare SUSE L3 Support