Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38268 )
Change subject: intel/x4x,i82801{ix|jx}: Move enable_smbus() call ......................................................................
intel/x4x,i82801{ix|jx}: Move enable_smbus() call
Change-Id: Idc7631abb550b31af722ccf3b69afdc01fdb616e Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/northbridge/intel/x4x/romstage.c M src/southbridge/intel/i82801ix/early_init.c M src/southbridge/intel/i82801jx/early_init.c 3 files changed, 6 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/38268/1
diff --git a/src/northbridge/intel/x4x/romstage.c b/src/northbridge/intel/x4x/romstage.c index eae87f3..26d336b 100644 --- a/src/northbridge/intel/x4x/romstage.c +++ b/src/northbridge/intel/x4x/romstage.c @@ -34,8 +34,6 @@ u8 boot_path = 0; u8 s3_resume;
- enable_smbus(); - #if CONFIG(SOUTHBRIDGE_INTEL_I82801JX) i82801jx_early_init(); #elif CONFIG(SOUTHBRIDGE_INTEL_I82801GX) diff --git a/src/southbridge/intel/i82801ix/early_init.c b/src/southbridge/intel/i82801ix/early_init.c index 92db7d8..4ce4fbe 100644 --- a/src/southbridge/intel/i82801ix/early_init.c +++ b/src/southbridge/intel/i82801ix/early_init.c @@ -23,6 +23,9 @@ { const pci_devfn_t d31f0 = PCI_DEV(0, 0x1f, 0);
+ if (ENV_ROMSTAGE) + enable_smbus(); + /* Set up RCBA. */ pci_write_config32(d31f0, RCBA, (uintptr_t)DEFAULT_RCBA | 1);
diff --git a/src/southbridge/intel/i82801jx/early_init.c b/src/southbridge/intel/i82801jx/early_init.c index 1afc6b3..87831bb 100644 --- a/src/southbridge/intel/i82801jx/early_init.c +++ b/src/southbridge/intel/i82801jx/early_init.c @@ -81,6 +81,9 @@ { const pci_devfn_t d31f0 = PCI_DEV(0, 0x1f, 0);
+ if (ENV_ROMSTAGE) + enable_smbus(); + printk(BIOS_DEBUG, "Setting up static southbridge registers..."); i82801jx_setup_bars(); printk(BIOS_DEBUG, " done.\n");
Hello Patrick Rudolph, build bot (Jenkins), Damien Zammit,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38268
to look at the new patch set (#2).
Change subject: intel/x4x,i82801{ix|jx}: Move enable_smbus() call ......................................................................
intel/x4x,i82801{ix|jx}: Move enable_smbus() call
Change-Id: Idc7631abb550b31af722ccf3b69afdc01fdb616e Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/northbridge/intel/x4x/romstage.c M src/southbridge/intel/i82801ix/early_init.c M src/southbridge/intel/i82801jx/early_init.c M src/southbridge/intel/i82801jx/i82801jx.h 4 files changed, 7 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/38268/2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38268 )
Change subject: intel/x4x,i82801{ix|jx}: Move enable_smbus() call ......................................................................
Patch Set 2: Code-Review+2
Hello Patrick Rudolph, Angel Pons, build bot (Jenkins), Damien Zammit,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38268
to look at the new patch set (#3).
Change subject: intel/{gm45,x4x},i82801{ix|jx}: Move enable_smbus() call ......................................................................
intel/{gm45,x4x},i82801{ix|jx}: Move enable_smbus() call
Change-Id: Idc7631abb550b31af722ccf3b69afdc01fdb616e Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/northbridge/intel/gm45/raminit.c M src/northbridge/intel/x4x/romstage.c M src/southbridge/intel/i82801ix/early_init.c M src/southbridge/intel/i82801jx/early_init.c M src/southbridge/intel/i82801jx/i82801jx.h 5 files changed, 7 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/38268/3
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38268 )
Change subject: intel/{gm45,x4x},i82801{ix|jx}: Move enable_smbus() call ......................................................................
Patch Set 3: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/38268/3/src/northbridge/intel/x4x/r... File src/northbridge/intel/x4x/romstage.c:
https://review.coreboot.org/c/coreboot/+/38268/3/src/northbridge/intel/x4x/r... PS3, Line 37: enable_smbus(); you might want to swap the order of this patch with the one implementing this on i82801gx to make sure x4x is not broken on this patch when coupled with i82801gx.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38268 )
Change subject: intel/{gm45,x4x},i82801{ix|jx}: Move enable_smbus() call ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38268/3/src/northbridge/intel/x4x/r... File src/northbridge/intel/x4x/romstage.c:
https://review.coreboot.org/c/coreboot/+/38268/3/src/northbridge/intel/x4x/r... PS3, Line 37: enable_smbus();
you might want to swap the order of this patch with the one implementing this on i82801gx to make su […]
Done
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38268 )
Change subject: intel/{gm45,x4x},i82801{ix|jx}: Move enable_smbus() call ......................................................................
Patch Set 4: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/38268/4/src/southbridge/intel/i8280... File src/southbridge/intel/i82801jx/early_init.c:
https://review.coreboot.org/c/coreboot/+/38268/4/src/southbridge/intel/i8280... PS4, Line 84: if (ENV_ROMSTAGE) Not sure if it matters, but nobody calls i82801jx_early_init() other than x4x/romstage.c
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38268 )
Change subject: intel/{gm45,x4x},i82801{ix|jx}: Move enable_smbus() call ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38268/4/src/southbridge/intel/i8280... File src/southbridge/intel/i82801jx/early_init.c:
https://review.coreboot.org/c/coreboot/+/38268/4/src/southbridge/intel/i8280... PS4, Line 84: if (ENV_ROMSTAGE)
Not sure if it matters, but nobody calls i82801jx_early_init() other than x4x/romstage. […]
By keeping this there's some uniformity with the other chipsets, so the argument can go either way. If somebody objects violently with this, they can push a commit to change it :-)
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/38268 )
Change subject: intel/{gm45,x4x},i82801{ix|jx}: Move enable_smbus() call ......................................................................
intel/{gm45,x4x},i82801{ix|jx}: Move enable_smbus() call
Change-Id: Idc7631abb550b31af722ccf3b69afdc01fdb616e Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/38268 Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Arthur Heymans arthur@aheymans.xyz Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/northbridge/intel/gm45/raminit.c M src/northbridge/intel/x4x/romstage.c M src/southbridge/intel/i82801ix/early_init.c M src/southbridge/intel/i82801jx/early_init.c M src/southbridge/intel/i82801jx/i82801jx.h 5 files changed, 7 insertions(+), 6 deletions(-)
Approvals: build bot (Jenkins): Verified Arthur Heymans: Looks good to me, approved Angel Pons: Looks good to me, approved
diff --git a/src/northbridge/intel/gm45/raminit.c b/src/northbridge/intel/gm45/raminit.c index b1da177..5b8d1d8 100644 --- a/src/northbridge/intel/gm45/raminit.c +++ b/src/northbridge/intel/gm45/raminit.c @@ -1723,9 +1723,6 @@ while (!(read8((u8 *)0xfed40000) & (1 << 7))) {} }
- /* Enable SMBUS. */ - enable_smbus(); - /* Collect information about DIMMs and find common settings. */ collect_dimm_config(sysinfo);
diff --git a/src/northbridge/intel/x4x/romstage.c b/src/northbridge/intel/x4x/romstage.c index eae87f3..26d336b 100644 --- a/src/northbridge/intel/x4x/romstage.c +++ b/src/northbridge/intel/x4x/romstage.c @@ -34,8 +34,6 @@ u8 boot_path = 0; u8 s3_resume;
- enable_smbus(); - #if CONFIG(SOUTHBRIDGE_INTEL_I82801JX) i82801jx_early_init(); #elif CONFIG(SOUTHBRIDGE_INTEL_I82801GX) diff --git a/src/southbridge/intel/i82801ix/early_init.c b/src/southbridge/intel/i82801ix/early_init.c index 92db7d8..4ce4fbe 100644 --- a/src/southbridge/intel/i82801ix/early_init.c +++ b/src/southbridge/intel/i82801ix/early_init.c @@ -23,6 +23,9 @@ { const pci_devfn_t d31f0 = PCI_DEV(0, 0x1f, 0);
+ if (ENV_ROMSTAGE) + enable_smbus(); + /* Set up RCBA. */ pci_write_config32(d31f0, RCBA, (uintptr_t)DEFAULT_RCBA | 1);
diff --git a/src/southbridge/intel/i82801jx/early_init.c b/src/southbridge/intel/i82801jx/early_init.c index 1afc6b3..87831bb 100644 --- a/src/southbridge/intel/i82801jx/early_init.c +++ b/src/southbridge/intel/i82801jx/early_init.c @@ -81,6 +81,9 @@ { const pci_devfn_t d31f0 = PCI_DEV(0, 0x1f, 0);
+ if (ENV_ROMSTAGE) + enable_smbus(); + printk(BIOS_DEBUG, "Setting up static southbridge registers..."); i82801jx_setup_bars(); printk(BIOS_DEBUG, " done.\n"); diff --git a/src/southbridge/intel/i82801jx/i82801jx.h b/src/southbridge/intel/i82801jx/i82801jx.h index 26a99f4..2c5135e 100644 --- a/src/southbridge/intel/i82801jx/i82801jx.h +++ b/src/southbridge/intel/i82801jx/i82801jx.h @@ -225,8 +225,8 @@ } #define LPC_IS_MOBILE(dev) lpc_is_mobile(pci_read_config16(dev, PCI_DEVICE_ID))
-#if ENV_ROMSTAGE void enable_smbus(void); +#if ENV_ROMSTAGE int smbus_read_byte(unsigned int device, unsigned int address); int i2c_eeprom_read(unsigned int device, unsigned int cmd, unsigned int bytes, u8 *buf);
-
Angel Pons (Code Review) -
Arthur Heymans (Code Review)
-
Kyösti Mälkki (Code Review)
-
Patrick Georgi (Code Review)