Change in ...coreboot[master]: sb/intel/common: Rename i2c_block_read() to i2c_eeprom_read()
Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31151
Change subject: sb/intel/common: Rename i2c_block_read() to i2c_eeprom_read() ......................................................................
sb/intel/common: Rename i2c_block_read() to i2c_eeprom_read()
Datasheets describe the used command as 'I2C Read' but adding the word 'eeprom' in between should avoid further confusion with other block commands.
Followups will add a symmetrical pair of commands i2c_block_read() and i2c_block_write() that operate via I2C_EN bit and have a 32 byte size restriction on block transfers. For some hardware revision these block commands are available, while 'I2C Read' was not.
Change-Id: I4494ab2985afc7f737ddacc8d706a5d5395e35cf Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/northbridge/intel/i945/raminit.c M src/northbridge/intel/pineview/raminit.c M src/northbridge/intel/x4x/raminit.c M src/southbridge/intel/common/smbus.c M src/southbridge/intel/common/smbus.h M src/southbridge/intel/i82801gx/early_smbus.c M src/southbridge/intel/i82801gx/i82801gx.h M src/southbridge/intel/i82801jx/early_smbus.c M src/southbridge/intel/i82801jx/i82801jx.h 9 files changed, 25 insertions(+), 14 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/31151/1
diff --git a/src/northbridge/intel/i945/raminit.c b/src/northbridge/intel/i945/raminit.c index f3c3df6..dece9bf 100644 --- a/src/northbridge/intel/i945/raminit.c +++ b/src/northbridge/intel/i945/raminit.c @@ -381,7 +381,7 @@ * only the first 64 bytes contain data needed for raminit. */
- bytes_read = i2c_block_read(device, 0, 64, raw_spd); + bytes_read = i2c_eeprom_read(device, 0, 64, raw_spd); printk(BIOS_DEBUG, "Reading SPD using i2c block operation.\n"); if (IS_ENABLED(CONFIG_DEBUG_RAM_SETUP) && bytes_read > 0) hexdump(raw_spd, bytes_read); diff --git a/src/northbridge/intel/pineview/raminit.c b/src/northbridge/intel/pineview/raminit.c index ee19b61..ed633fd 100644 --- a/src/northbridge/intel/pineview/raminit.c +++ b/src/northbridge/intel/pineview/raminit.c @@ -273,7 +273,7 @@ u8 i, chan; s->dt0mode = 0; FOR_EACH_DIMM(i) { - if (i2c_block_read(s->spd_map[i], 0, 64, s->dimms[i].spd_data) != 64) + if (i2c_eeprom_read(s->spd_map[i], 0, 64, s->dimms[i].spd_data) != 64) s->dimms[i].card_type = 0;
s->dimms[i].card_type = s->dimms[i].spd_data[62] & 0x1f; diff --git a/src/northbridge/intel/x4x/raminit.c b/src/northbridge/intel/x4x/raminit.c index d9fa49d..1fd6004 100644 --- a/src/northbridge/intel/x4x/raminit.c +++ b/src/northbridge/intel/x4x/raminit.c @@ -47,15 +47,15 @@ static u16 ddr2_get_crc(u8 device, u8 len) { u8 raw_spd[128] = {}; - i2c_block_read(device, 64, 9, &raw_spd[64]); - i2c_block_read(device, 93, 6, &raw_spd[93]); + i2c_eeprom_read(device, 64, 9, &raw_spd[64]); + i2c_eeprom_read(device, 93, 6, &raw_spd[93]); return spd_ddr2_calc_unique_crc(raw_spd, len); }
static u16 ddr3_get_crc(u8 device, u8 len) { u8 raw_spd[256] = {}; - i2c_block_read(device, 117, 11, &raw_spd[117]); + i2c_eeprom_read(device, 117, 11, &raw_spd[117]); return spd_ddr3_calc_unique_crc(raw_spd, len); }
@@ -531,7 +531,7 @@ die("Mixing up dimm types is not supported!\n");
printk(BIOS_DEBUG, "Decoding dimm %d\n", i); - if (i2c_block_read(device, 0, 128, raw_spd) != 128) { + if (i2c_eeprom_read(device, 0, 128, raw_spd) != 128) { printk(BIOS_DEBUG, "i2c block operation failed," " trying smbus byte operation.\n"); for (j = 0; j < 128; j++) diff --git a/src/southbridge/intel/common/smbus.c b/src/southbridge/intel/common/smbus.c index fd8b6aa..a842a61 100644 --- a/src/southbridge/intel/common/smbus.c +++ b/src/southbridge/intel/common/smbus.c @@ -363,11 +363,22 @@ }
/* Only since ICH5 */ -int do_i2c_block_read(unsigned int smbus_base, u8 device, +static int has_i2c_read_command(void) +{ + if (IS_ENABLED(CONFIG_SOUTHBRIDGE_INTEL_I82371EB) || + IS_ENABLED(CONFIG_SOUTHBRIDGE_INTEL_I82801DX)) + return 0; + return 1; +} + +int do_i2c_eeprom_read(unsigned int smbus_base, u8 device, unsigned int offset, const unsigned int bytes, u8 *buf) { int ret;
+ if (!has_i2c_read_command()) + return SMBUS_ERROR; + /* Set up for a i2c block data read. * * FIXME: Address parameter changes to XMIT_READ(device) with diff --git a/src/southbridge/intel/common/smbus.h b/src/southbridge/intel/common/smbus.h index be1aa76..ded31d0 100644 --- a/src/southbridge/intel/common/smbus.h +++ b/src/southbridge/intel/common/smbus.h @@ -41,6 +41,6 @@ int do_smbus_block_write(unsigned int smbus_base, u8 device, u8 cmd, unsigned int bytes, const u8 *buf); /* Only since ICH5 */ -int do_i2c_block_read(unsigned int smbus_base, u8 device, +int do_i2c_eeprom_read(unsigned int smbus_base, u8 device, unsigned int offset, unsigned int bytes, u8 *buf); #endif diff --git a/src/southbridge/intel/i82801gx/early_smbus.c b/src/southbridge/intel/i82801gx/early_smbus.c index 698458b..9dddcec 100644 --- a/src/southbridge/intel/i82801gx/early_smbus.c +++ b/src/southbridge/intel/i82801gx/early_smbus.c @@ -54,9 +54,9 @@ return do_smbus_read_byte(SMBUS_IO_BASE, device, address); }
-int i2c_block_read(unsigned int device, unsigned int offset, u32 bytes, u8 *buf) +int i2c_eeprom_read(unsigned int device, unsigned int offset, u32 bytes, u8 *buf) { - return do_i2c_block_read(SMBUS_IO_BASE, device, offset, bytes, buf); + return do_i2c_eeprom_read(SMBUS_IO_BASE, device, offset, bytes, buf); }
int smbus_block_read(unsigned int device, unsigned int cmd, u8 bytes, u8 *buf) diff --git a/src/southbridge/intel/i82801gx/i82801gx.h b/src/southbridge/intel/i82801gx/i82801gx.h index 66d47ae..12935c2 100644 --- a/src/southbridge/intel/i82801gx/i82801gx.h +++ b/src/southbridge/intel/i82801gx/i82801gx.h @@ -45,7 +45,7 @@ #else void enable_smbus(void); int smbus_read_byte(unsigned int device, unsigned int address); -int i2c_block_read(unsigned int device, unsigned int cmd, unsigned int bytes, +int i2c_eeprom_read(unsigned int device, unsigned int cmd, unsigned int bytes, u8 *buf); int smbus_block_read(unsigned int device, unsigned int cmd, u8 bytes, u8 *buf); int smbus_block_write(unsigned int device, unsigned int cmd, u8 bytes, diff --git a/src/southbridge/intel/i82801jx/early_smbus.c b/src/southbridge/intel/i82801jx/early_smbus.c index bd68955..8c87045 100644 --- a/src/southbridge/intel/i82801jx/early_smbus.c +++ b/src/southbridge/intel/i82801jx/early_smbus.c @@ -51,9 +51,9 @@ return do_smbus_read_byte(SMBUS_IO_BASE, device, address); }
-int i2c_block_read(unsigned int device, unsigned int offset, u32 bytes, u8 *buf) +int i2c_eeprom_read(unsigned int device, unsigned int offset, u32 bytes, u8 *buf) { - return do_i2c_block_read(SMBUS_IO_BASE, device, offset, bytes, buf); + return do_i2c_eeprom_read(SMBUS_IO_BASE, device, offset, bytes, buf); }
int smbus_block_read(unsigned int device, unsigned int cmd, u8 bytes, u8 *buf) diff --git a/src/southbridge/intel/i82801jx/i82801jx.h b/src/southbridge/intel/i82801jx/i82801jx.h index ad3b381..0014848 100644 --- a/src/southbridge/intel/i82801jx/i82801jx.h +++ b/src/southbridge/intel/i82801jx/i82801jx.h @@ -233,7 +233,7 @@ #if defined(__PRE_RAM__) void enable_smbus(void); int smbus_read_byte(unsigned device, unsigned address); -int i2c_block_read(unsigned int device, unsigned int cmd, unsigned int bytes, +int i2c_eeprom_read(unsigned int device, unsigned int cmd, unsigned int bytes, u8 *buf); int smbus_block_read(unsigned int device, unsigned int cmd, u8 bytes, u8 *buf); int smbus_block_write(unsigned int device, unsigned int cmd, u8 bytes,
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31151 )
Change subject: sb/intel/common: Rename i2c_block_read() to i2c_eeprom_read() ......................................................................
Patch Set 1:
(3 comments)
https://review.coreboot.org/#/c/31151/1/src/northbridge/intel/pineview/ramin... File src/northbridge/intel/pineview/raminit.c:
https://review.coreboot.org/#/c/31151/1/src/northbridge/intel/pineview/ramin... PS1, Line 276: if (i2c_eeprom_read(s->spd_map[i], 0, 64, s->dimms[i].spd_data) != 64) line over 80 characters
https://review.coreboot.org/#/c/31151/1/src/southbridge/intel/i82801gx/early... File src/southbridge/intel/i82801gx/early_smbus.c:
https://review.coreboot.org/#/c/31151/1/src/southbridge/intel/i82801gx/early... PS1, Line 57: int i2c_eeprom_read(unsigned int device, unsigned int offset, u32 bytes, u8 *buf) line over 80 characters
https://review.coreboot.org/#/c/31151/1/src/southbridge/intel/i82801jx/early... File src/southbridge/intel/i82801jx/early_smbus.c:
https://review.coreboot.org/#/c/31151/1/src/southbridge/intel/i82801jx/early... PS1, Line 54: int i2c_eeprom_read(unsigned int device, unsigned int offset, u32 bytes, u8 *buf) line over 80 characters
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31151 )
Change subject: sb/intel/common: Rename i2c_block_read() to i2c_eeprom_read() ......................................................................
Patch Set 1: Code-Review+1
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31151 )
Change subject: sb/intel/common: Rename i2c_block_read() to i2c_eeprom_read() ......................................................................
Patch Set 1: Code-Review+1
tested on 945G-M4
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31151 )
Change subject: sb/intel/common: Rename i2c_block_read() to i2c_eeprom_read() ......................................................................
Patch Set 1: Code-Review+2
Kyösti Mälkki has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31151 )
Change subject: sb/intel/common: Rename i2c_block_read() to i2c_eeprom_read() ......................................................................
sb/intel/common: Rename i2c_block_read() to i2c_eeprom_read()
Datasheets describe the used command as 'I2C Read' but adding the word 'eeprom' in between should avoid further confusion with other block commands.
Followups will add a symmetrical pair of commands i2c_block_read() and i2c_block_write() that operate via I2C_EN bit and have a 32 byte size restriction on block transfers. For some hardware revision these block commands are available, while 'I2C Read' was not.
Change-Id: I4494ab2985afc7f737ddacc8d706a5d5395e35cf Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-on: https://review.coreboot.org/c/31151 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net Reviewed-by: HAOUAS Elyes ehaouas@noos.fr Reviewed-by: Arthur Heymans arthur@aheymans.xyz --- M src/northbridge/intel/i945/raminit.c M src/northbridge/intel/pineview/raminit.c M src/northbridge/intel/x4x/raminit.c M src/southbridge/intel/common/smbus.c M src/southbridge/intel/common/smbus.h M src/southbridge/intel/i82801gx/early_smbus.c M src/southbridge/intel/i82801gx/i82801gx.h M src/southbridge/intel/i82801jx/early_smbus.c M src/southbridge/intel/i82801jx/i82801jx.h 9 files changed, 25 insertions(+), 14 deletions(-)
Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve HAOUAS Elyes: Looks good to me, but someone else must approve Arthur Heymans: Looks good to me, approved
diff --git a/src/northbridge/intel/i945/raminit.c b/src/northbridge/intel/i945/raminit.c index f3c3df6..dece9bf 100644 --- a/src/northbridge/intel/i945/raminit.c +++ b/src/northbridge/intel/i945/raminit.c @@ -381,7 +381,7 @@ * only the first 64 bytes contain data needed for raminit. */
- bytes_read = i2c_block_read(device, 0, 64, raw_spd); + bytes_read = i2c_eeprom_read(device, 0, 64, raw_spd); printk(BIOS_DEBUG, "Reading SPD using i2c block operation.\n"); if (IS_ENABLED(CONFIG_DEBUG_RAM_SETUP) && bytes_read > 0) hexdump(raw_spd, bytes_read); diff --git a/src/northbridge/intel/pineview/raminit.c b/src/northbridge/intel/pineview/raminit.c index ee19b61..ed633fd 100644 --- a/src/northbridge/intel/pineview/raminit.c +++ b/src/northbridge/intel/pineview/raminit.c @@ -273,7 +273,7 @@ u8 i, chan; s->dt0mode = 0; FOR_EACH_DIMM(i) { - if (i2c_block_read(s->spd_map[i], 0, 64, s->dimms[i].spd_data) != 64) + if (i2c_eeprom_read(s->spd_map[i], 0, 64, s->dimms[i].spd_data) != 64) s->dimms[i].card_type = 0;
s->dimms[i].card_type = s->dimms[i].spd_data[62] & 0x1f; diff --git a/src/northbridge/intel/x4x/raminit.c b/src/northbridge/intel/x4x/raminit.c index d9fa49d..1fd6004 100644 --- a/src/northbridge/intel/x4x/raminit.c +++ b/src/northbridge/intel/x4x/raminit.c @@ -47,15 +47,15 @@ static u16 ddr2_get_crc(u8 device, u8 len) { u8 raw_spd[128] = {}; - i2c_block_read(device, 64, 9, &raw_spd[64]); - i2c_block_read(device, 93, 6, &raw_spd[93]); + i2c_eeprom_read(device, 64, 9, &raw_spd[64]); + i2c_eeprom_read(device, 93, 6, &raw_spd[93]); return spd_ddr2_calc_unique_crc(raw_spd, len); }
static u16 ddr3_get_crc(u8 device, u8 len) { u8 raw_spd[256] = {}; - i2c_block_read(device, 117, 11, &raw_spd[117]); + i2c_eeprom_read(device, 117, 11, &raw_spd[117]); return spd_ddr3_calc_unique_crc(raw_spd, len); }
@@ -531,7 +531,7 @@ die("Mixing up dimm types is not supported!\n");
printk(BIOS_DEBUG, "Decoding dimm %d\n", i); - if (i2c_block_read(device, 0, 128, raw_spd) != 128) { + if (i2c_eeprom_read(device, 0, 128, raw_spd) != 128) { printk(BIOS_DEBUG, "i2c block operation failed," " trying smbus byte operation.\n"); for (j = 0; j < 128; j++) diff --git a/src/southbridge/intel/common/smbus.c b/src/southbridge/intel/common/smbus.c index fd8b6aa..a842a61 100644 --- a/src/southbridge/intel/common/smbus.c +++ b/src/southbridge/intel/common/smbus.c @@ -363,11 +363,22 @@ }
/* Only since ICH5 */ -int do_i2c_block_read(unsigned int smbus_base, u8 device, +static int has_i2c_read_command(void) +{ + if (IS_ENABLED(CONFIG_SOUTHBRIDGE_INTEL_I82371EB) || + IS_ENABLED(CONFIG_SOUTHBRIDGE_INTEL_I82801DX)) + return 0; + return 1; +} + +int do_i2c_eeprom_read(unsigned int smbus_base, u8 device, unsigned int offset, const unsigned int bytes, u8 *buf) { int ret;
+ if (!has_i2c_read_command()) + return SMBUS_ERROR; + /* Set up for a i2c block data read. * * FIXME: Address parameter changes to XMIT_READ(device) with diff --git a/src/southbridge/intel/common/smbus.h b/src/southbridge/intel/common/smbus.h index be1aa76..ded31d0 100644 --- a/src/southbridge/intel/common/smbus.h +++ b/src/southbridge/intel/common/smbus.h @@ -41,6 +41,6 @@ int do_smbus_block_write(unsigned int smbus_base, u8 device, u8 cmd, unsigned int bytes, const u8 *buf); /* Only since ICH5 */ -int do_i2c_block_read(unsigned int smbus_base, u8 device, +int do_i2c_eeprom_read(unsigned int smbus_base, u8 device, unsigned int offset, unsigned int bytes, u8 *buf); #endif diff --git a/src/southbridge/intel/i82801gx/early_smbus.c b/src/southbridge/intel/i82801gx/early_smbus.c index 698458b..9dddcec 100644 --- a/src/southbridge/intel/i82801gx/early_smbus.c +++ b/src/southbridge/intel/i82801gx/early_smbus.c @@ -54,9 +54,9 @@ return do_smbus_read_byte(SMBUS_IO_BASE, device, address); }
-int i2c_block_read(unsigned int device, unsigned int offset, u32 bytes, u8 *buf) +int i2c_eeprom_read(unsigned int device, unsigned int offset, u32 bytes, u8 *buf) { - return do_i2c_block_read(SMBUS_IO_BASE, device, offset, bytes, buf); + return do_i2c_eeprom_read(SMBUS_IO_BASE, device, offset, bytes, buf); }
int smbus_block_read(unsigned int device, unsigned int cmd, u8 bytes, u8 *buf) diff --git a/src/southbridge/intel/i82801gx/i82801gx.h b/src/southbridge/intel/i82801gx/i82801gx.h index 66d47ae..12935c2 100644 --- a/src/southbridge/intel/i82801gx/i82801gx.h +++ b/src/southbridge/intel/i82801gx/i82801gx.h @@ -45,7 +45,7 @@ #else void enable_smbus(void); int smbus_read_byte(unsigned int device, unsigned int address); -int i2c_block_read(unsigned int device, unsigned int cmd, unsigned int bytes, +int i2c_eeprom_read(unsigned int device, unsigned int cmd, unsigned int bytes, u8 *buf); int smbus_block_read(unsigned int device, unsigned int cmd, u8 bytes, u8 *buf); int smbus_block_write(unsigned int device, unsigned int cmd, u8 bytes, diff --git a/src/southbridge/intel/i82801jx/early_smbus.c b/src/southbridge/intel/i82801jx/early_smbus.c index bd68955..8c87045 100644 --- a/src/southbridge/intel/i82801jx/early_smbus.c +++ b/src/southbridge/intel/i82801jx/early_smbus.c @@ -51,9 +51,9 @@ return do_smbus_read_byte(SMBUS_IO_BASE, device, address); }
-int i2c_block_read(unsigned int device, unsigned int offset, u32 bytes, u8 *buf) +int i2c_eeprom_read(unsigned int device, unsigned int offset, u32 bytes, u8 *buf) { - return do_i2c_block_read(SMBUS_IO_BASE, device, offset, bytes, buf); + return do_i2c_eeprom_read(SMBUS_IO_BASE, device, offset, bytes, buf); }
int smbus_block_read(unsigned int device, unsigned int cmd, u8 bytes, u8 *buf) diff --git a/src/southbridge/intel/i82801jx/i82801jx.h b/src/southbridge/intel/i82801jx/i82801jx.h index ad3b381..0014848 100644 --- a/src/southbridge/intel/i82801jx/i82801jx.h +++ b/src/southbridge/intel/i82801jx/i82801jx.h @@ -233,7 +233,7 @@ #if defined(__PRE_RAM__) void enable_smbus(void); int smbus_read_byte(unsigned device, unsigned address); -int i2c_block_read(unsigned int device, unsigned int cmd, unsigned int bytes, +int i2c_eeprom_read(unsigned int device, unsigned int cmd, unsigned int bytes, u8 *buf); int smbus_block_read(unsigned int device, unsigned int cmd, u8 bytes, u8 *buf); int smbus_block_write(unsigned int device, unsigned int cmd, u8 bytes,
-
Arthur Heymans (Code Review) -
build bot (Jenkins) (Code Review)
-
HAOUAS Elyes (Code Review)
-
Kyösti Mälkki (Code Review)
-
Paul Menzel (Code Review)