-
Notifications
You must be signed in to change notification settings - Fork 211
at24: SMBus fallback read/write for 16-bit addressed EEPROMs on PIIX4 #550
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
nissampa
wants to merge
1
commit into
sonic-net:master
Choose a base branch
from
nissampa:at24_smbus
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+322
−0
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
322 changes: 322 additions & 0 deletions
322
patches-sonic/driver-at24-smbus-fallback-for-addr16-on-piix4.patch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,322 @@ | ||
| From 07e943ccf6b98e941a39cd1f74db250e862151f0 Mon Sep 17 00:00:00 2001 | ||
| From: Nishanth Sampath Kumar <nissampa@cisco.com> | ||
| Date: Fri, 27 Mar 2026 14:48:36 -0700 | ||
| Subject: [PATCH] at24: SMBus fallback for both 16-bit and 8-bit EEPROMs on PIIX4 | ||
|
|
||
| AMD PIIX4 SMBus adapters (found on AMD SP5/EPYC platforms including | ||
| Cisco 8000 series) support BYTE_DATA and WORD_DATA but lack I2C_FUNC_I2C | ||
| and I2C_FUNC_SMBUS_I2C_BLOCK. This causes two distinct failures: | ||
|
|
||
| 1. AT24_FLAG_ADDR16 EEPROMs (e.g. 24c64 syseeprom on SMB1-MUX-0): | ||
| regmap_get_i2c_bus() returns -ENOTSUPP for reg_bits=16. | ||
|
|
||
| 2. 8-bit addressed EEPROMs (e.g. 24c02 fan EEPROMs on PIIX4 mux children): | ||
| regmap picks regmap_i2c_smbus_i2c_block which fails at runtime. | ||
|
|
||
| Add two fallback paths selected after chip flags are fully resolved | ||
| (i.e. after flags = cdata->flags and DT overrides): | ||
|
|
||
| smbus_addr16_fallback (AT24_FLAG_ADDR16): | ||
| READ: write_byte_data(addr_hi, addr_lo) sets 16-bit pointer, | ||
| repeated read_byte() fetches bytes (EEPROM auto-increments). | ||
| WRITE: write_word_data encodes address+data in one WORD. | ||
|
|
||
| smbus_addr8_fallback (no AT24_FLAG_ADDR16): | ||
| READ: read_byte_data(client, (u8)offset) per byte. | ||
| WRITE: write_byte_data(client, (u8)offset, data) per byte. | ||
|
|
||
| Also add a regmap init retry with reg_bits=8/val_bits=8 so probe | ||
| succeeds on adapters that reject regmap init entirely. | ||
|
|
||
| Key bug fixed vs v1: detection was done before flags = cdata->flags, | ||
| so AT24_FLAG_ADDR16 was never seen and all EEPROMs fell into the 8-bit | ||
| path, corrupting 24c64 reads. | ||
|
|
||
| Signed-off-by: Nishanth Sampath Kumar <nissampa@cisco.com> | ||
| --- | ||
| --- a/drivers/misc/eeprom/at24.c | ||
| +++ b/drivers/misc/eeprom/at24.c | ||
| @@ -88,6 +88,11 @@ | ||
| struct regulator *vcc_reg; | ||
| void (*read_post)(unsigned int off, char *buf, size_t count); | ||
|
|
||
| + /* SMBus-only fallback for EEPROMs on PIIX4 adapters */ | ||
| + bool smbus_addr16_fallback; | ||
| + bool smbus_addr8_fallback; | ||
| + struct i2c_client *smbus_client; | ||
| + | ||
| /* | ||
| * Some chips tie up multiple I2C addresses; dummy devices reserve | ||
| * them for us. | ||
| @@ -343,6 +348,139 @@ | ||
| return count; | ||
| } | ||
|
|
||
| +/* forward declaration needed by at24_smbus_write below */ | ||
| +static size_t at24_adjust_write_count(struct at24_data *at24, | ||
| + unsigned int offset, size_t count); | ||
| + | ||
| +/* | ||
| + * SMBus primitive fallback for 16-bit addressed EEPROMs. | ||
| + * | ||
| + * PIIX4 SMBus supports BYTE_DATA and WORD_DATA but NOT I2C_FUNC_I2C or | ||
| + * I2C_FUNC_SMBUS_I2C_BLOCK, so regmap_get_i2c_bus() returns -ENOTSUPP | ||
| + * for reg_bits=16. We bypass regmap entirely for the I/O path: | ||
| + * | ||
| + * READ : write_byte_data(addr_hi, addr_lo) sets the 16-bit EEPROM | ||
| + * address pointer, then read_byte() fetches bytes one-by-one | ||
| + * (EEPROM auto-increments the pointer after each read). | ||
| + * | ||
| + * WRITE: write_word_data(addr_hi, cpu_to_le16((data<<8)|addr_lo)) | ||
| + * encodes the address and data byte in one SMBus word write. | ||
| + */ | ||
| +static ssize_t at24_smbus_read(struct at24_data *at24, char *buf, | ||
| + unsigned int offset, size_t count) | ||
| +{ | ||
| + struct i2c_client *client = at24->smbus_client; | ||
| + unsigned long timeout, read_time; | ||
| + u8 addr_hi = (offset >> 8) & 0xff; | ||
| + u8 addr_lo = offset & 0xff; | ||
| + ssize_t ret; | ||
| + size_t i; | ||
| + | ||
| + count = at24_adjust_read_count(at24, offset, count); | ||
| + | ||
| + timeout = jiffies + msecs_to_jiffies(at24_write_timeout); | ||
| + do { | ||
| + read_time = jiffies; | ||
| + | ||
| + /* Set 16-bit address pointer: command=addr_hi, value=addr_lo */ | ||
| + ret = i2c_smbus_write_byte_data(client, addr_hi, addr_lo); | ||
| + if (ret < 0) | ||
| + goto retry; | ||
| + | ||
| + /* Read bytes sequentially -- EEPROM auto-increments pointer */ | ||
| + for (i = 0; i < count; i++) { | ||
| + ret = i2c_smbus_read_byte(client); | ||
| + if (ret < 0) | ||
| + goto retry; | ||
| + buf[i] = (u8)ret; | ||
| + } | ||
| + dev_dbg(&client->dev, "smbus read %zu@%u --> ok\n", count, offset); | ||
| + return count; | ||
| +retry: | ||
| + usleep_range(1000, 1500); | ||
| + } while (time_before(read_time, timeout)); | ||
| + | ||
| + return -ETIMEDOUT; | ||
| +} | ||
| + | ||
| +static ssize_t at24_smbus_write(struct at24_data *at24, const char *buf, | ||
| + unsigned int offset, size_t count) | ||
| +{ | ||
| + struct i2c_client *client = at24->smbus_client; | ||
| + unsigned long timeout, write_time; | ||
| + size_t i; | ||
| + int ret; | ||
| + | ||
| + /* Write one byte at a time (page writes need raw I2C transfers) */ | ||
| + count = at24_adjust_write_count(at24, offset, count); | ||
| + | ||
| + for (i = 0; i < count; i++, offset++) { | ||
| + timeout = jiffies + msecs_to_jiffies(at24_write_timeout); | ||
| + do { | ||
| + write_time = jiffies; | ||
| + /* | ||
| + * command = addr[15:8] | ||
| + * word LSB = addr[7:0] | ||
| + * word MSB = data byte | ||
| + * EEPROM: set 16-bit address then write one data byte. | ||
| + */ | ||
| + ret = i2c_smbus_write_word_data(client, | ||
| + (offset >> 8) & 0xff, | ||
| + cpu_to_le16(((u16)(u8)buf[i] << 8) | | ||
| + (offset & 0xff))); | ||
| + if (!ret) { | ||
| + dev_dbg(&client->dev, "smbus write 1@%u --> ok\n", | ||
| + offset); | ||
| + break; | ||
| + } | ||
| + usleep_range(1000, 1500); | ||
| + } while (time_before(write_time, timeout)); | ||
| + | ||
| + if (ret) | ||
| + return (i > 0) ? (ssize_t)i : -ETIMEDOUT; | ||
| + } | ||
| + | ||
| + return count; | ||
| +} | ||
| + | ||
| +static ssize_t at24_smbus8_read(struct at24_data *at24, char *buf, | ||
| + unsigned int offset, size_t count) | ||
| +{ | ||
| + struct i2c_client *client = at24->smbus_client; | ||
| + size_t i; | ||
| + int ret; | ||
| + | ||
| + count = at24_adjust_read_count(at24, offset, count); | ||
| + | ||
| + for (i = 0; i < count; i++) { | ||
| + ret = i2c_smbus_read_byte_data(client, (u8)(offset + i)); | ||
| + if (ret < 0) | ||
| + return ret; | ||
| + buf[i] = (u8)ret; | ||
| + } | ||
| + dev_dbg(&client->dev, "smbus8 read %zu@%u --> ok\n", count, offset); | ||
| + return count; | ||
| +} | ||
| + | ||
| +static ssize_t at24_smbus8_write(struct at24_data *at24, const char *buf, | ||
| + unsigned int offset, size_t count) | ||
| +{ | ||
| + struct i2c_client *client = at24->smbus_client; | ||
| + size_t i; | ||
| + int ret; | ||
| + | ||
| + count = at24_adjust_write_count(at24, offset, count); | ||
| + | ||
| + for (i = 0; i < count; i++) { | ||
| + ret = i2c_smbus_write_byte_data(client, (u8)(offset + i), | ||
| + (u8)buf[i]); | ||
| + if (ret < 0) | ||
| + return (i > 0) ? (ssize_t)i : ret; | ||
| + } | ||
| + dev_dbg(&client->dev, "smbus8 write %zu@%u --> ok\n", count, offset); | ||
| + return count; | ||
| +} | ||
| + | ||
| static ssize_t at24_regmap_read(struct at24_data *at24, char *buf, | ||
| unsigned int offset, size_t count) | ||
| { | ||
| @@ -353,6 +491,12 @@ | ||
| regmap = at24_translate_offset(at24, &offset); | ||
| count = at24_adjust_read_count(at24, offset, count); | ||
|
|
||
| + if (at24->smbus_addr16_fallback) | ||
| + return at24_smbus_read(at24, buf, offset, count); | ||
| + | ||
| + if (at24->smbus_addr8_fallback) | ||
| + return at24_smbus8_read(at24, buf, offset, count); | ||
| + | ||
| /* adjust offset for mac and serial read ops */ | ||
| offset += at24->offset_adj; | ||
|
|
||
| @@ -411,6 +555,13 @@ | ||
|
|
||
| regmap = at24_translate_offset(at24, &offset); | ||
| count = at24_adjust_write_count(at24, offset, count); | ||
| + | ||
| + if (at24->smbus_addr16_fallback) | ||
| + return at24_smbus_write(at24, buf, offset, count); | ||
| + | ||
| + if (at24->smbus_addr8_fallback) | ||
| + return at24_smbus8_write(at24, buf, offset, count); | ||
| + | ||
| timeout = jiffies + msecs_to_jiffies(at24_write_timeout); | ||
|
|
||
| do { | ||
| @@ -601,6 +752,8 @@ | ||
| bool i2c_fn_i2c, i2c_fn_block; | ||
| unsigned int i, num_addresses; | ||
| struct at24_data *at24; | ||
| + bool smbus_addr16_fallback; | ||
| + bool smbus_addr8_fallback; | ||
| bool full_power; | ||
| struct regmap *regmap; | ||
| bool writable; | ||
| @@ -611,6 +764,18 @@ | ||
| i2c_fn_block = i2c_check_functionality(client->adapter, | ||
| I2C_FUNC_SMBUS_WRITE_I2C_BLOCK); | ||
|
|
||
| + /* | ||
| + * Detect SMBus-only adapters (e.g. AMD PIIX4) that lack true I2C | ||
| + * transfer capability. Two cases: | ||
| + * | ||
| + * 1. No I2C_FUNC_I2C and no I2C_FUNC_SMBUS_I2C_BLOCK: | ||
| + * For 16-bit addressed EEPROMs (AT24_FLAG_ADDR16), bypass regmap | ||
| + * and use SMBus BYTE_DATA/WORD_DATA helpers. | ||
| + * | ||
| + * 2. No I2C_FUNC_I2C but I2C_FUNC_SMBUS_READ_I2C_BLOCK advertised | ||
| + * (PIIX4 mux children): regmap picks the block-read path which | ||
| + * fails at runtime. For 8-bit addressed EEPROMs use BYTE_DATA. | ||
| + */ | ||
| cdata = i2c_get_match_data(client); | ||
| if (!cdata) | ||
| return -ENODEV; | ||
| @@ -648,6 +813,33 @@ | ||
| } | ||
| } | ||
|
|
||
| + /* | ||
| + * Detect SMBus-only adapters (e.g. AMD PIIX4) that lack true I2C | ||
| + * capability. Detection is done here, after flags has been resolved | ||
| + * from cdata and any device-tree overrides. | ||
| + */ | ||
| + smbus_addr16_fallback = false; | ||
| + smbus_addr8_fallback = false; | ||
| + if (!i2c_fn_i2c) { | ||
| + bool has_byte = i2c_check_functionality(client->adapter, | ||
| + I2C_FUNC_SMBUS_BYTE_DATA); | ||
| + bool has_word = i2c_check_functionality(client->adapter, | ||
| + I2C_FUNC_SMBUS_WORD_DATA); | ||
| + if (has_byte && has_word) { | ||
| + /* | ||
| + * SMBus-only adapter (e.g. AMD PIIX4): no true I2C transfers. | ||
| + * Choose fallback based on EEPROM address width. | ||
| + */ | ||
| + if (flags & AT24_FLAG_ADDR16) { | ||
| + smbus_addr16_fallback = true; | ||
| + dev_dbg(dev, "SMBus-only adapter: enabling 16-bit addr SMBus fallback\n"); | ||
| + } else { | ||
| + smbus_addr8_fallback = true; | ||
| + dev_dbg(dev, "SMBus-only adapter: enabling 8-bit addr SMBus fallback\n"); | ||
| + } | ||
| + } | ||
| + } | ||
| + | ||
| err = device_property_read_u32(dev, "size", &byte_len); | ||
| if (err) | ||
| byte_len = cdata->byte_len; | ||
| @@ -681,10 +873,30 @@ | ||
| regmap_config.val_bits = 8; | ||
| regmap_config.reg_bits = (flags & AT24_FLAG_ADDR16) ? 16 : 8; | ||
| regmap_config.disable_locking = true; | ||
| + /* | ||
| + * Force reg_bits=8 when the SMBus fallback path is active so that | ||
| + * regmap_get_i2c_bus() can pick regmap_smbus_byte and succeed on PIIX4. | ||
| + * The actual 16-bit address I/O is done entirely by our helpers. | ||
| + */ | ||
| + if (smbus_addr16_fallback && (flags & AT24_FLAG_ADDR16)) | ||
| + regmap_config.reg_bits = 8; | ||
|
|
||
| regmap = devm_regmap_init_i2c(client, ®map_config); | ||
| - if (IS_ERR(regmap)) | ||
| - return PTR_ERR(regmap); | ||
| + if (IS_ERR(regmap)) { | ||
| + /* | ||
| + * For SMBus fallback paths the regmap is never used for I/O. | ||
| + * If regmap init fails (e.g. adapter lacks I2C_FUNC_SMBUS_I2C_BLOCK | ||
| + * at probe time), retry with a minimal byte config so probe succeeds. | ||
| + */ | ||
| + if (smbus_addr16_fallback || smbus_addr8_fallback) { | ||
| + struct regmap_config smbus_config = regmap_config; | ||
| + smbus_config.reg_bits = 8; | ||
| + smbus_config.val_bits = 8; | ||
| + regmap = devm_regmap_init_i2c(client, &smbus_config); | ||
| + } | ||
| + if (IS_ERR(regmap)) | ||
| + return PTR_ERR(regmap); | ||
| + } | ||
|
|
||
| at24 = devm_kzalloc(dev, struct_size(at24, client_regmaps, num_addresses), | ||
| GFP_KERNEL); | ||
| @@ -700,6 +912,9 @@ | ||
| at24->num_addresses = num_addresses; | ||
| at24->offset_adj = at24_get_offset_adj(flags, byte_len); | ||
| at24->client_regmaps[0] = regmap; | ||
| + at24->smbus_addr16_fallback = smbus_addr16_fallback; | ||
| + at24->smbus_addr8_fallback = smbus_addr8_fallback; | ||
| + at24->smbus_client = client; | ||
|
|
||
| at24->vcc_reg = devm_regulator_get(dev, "vcc"); | ||
| if (IS_ERR(at24->vcc_reg)) | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.