Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/33141
Change subject: mb/*/{x201,ms2290}/mainboard.c: Remove superfluous ramstage code ......................................................................
mb/*/{x201,ms2290}/mainboard.c: Remove superfluous ramstage code
Change-Id: I0270c50dea2a2ce6c8e6114ed708f06be9d33c0e Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/mainboard/lenovo/x201/mainboard.c M src/mainboard/packardbell/ms2290/mainboard.c 2 files changed, 0 insertions(+), 26 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/33141/1
diff --git a/src/mainboard/lenovo/x201/mainboard.c b/src/mainboard/lenovo/x201/mainboard.c index 54acca3..ad878db 100644 --- a/src/mainboard/lenovo/x201/mainboard.c +++ b/src/mainboard/lenovo/x201/mainboard.c @@ -70,19 +70,6 @@ dev->ops->init = mainboard_init; dev->ops->acpi_fill_ssdt_generator = fill_ssdt;
- pmbase = pci_read_config32(pcidev_on_root(0x1f, 0), - PMBASE) & 0xff80; - - printk(BIOS_SPEW, " ... pmbase = 0x%04x\n", pmbase); - - outl(0, pmbase + SMI_EN); - - enable_lapic(); - pci_write_config32(pcidev_on_root(0x1f, 0), GPIO_BASE, - DEFAULT_GPIOBASE | 1); - pci_write_config8(pcidev_on_root(0x1f, 0), GPIO_CNTL, - 0x10); - /* If we're resuming from suspend, blink suspend LED */ if (acpi_is_wakeup_s3()) ec_write(0x0c, 0xc7); diff --git a/src/mainboard/packardbell/ms2290/mainboard.c b/src/mainboard/packardbell/ms2290/mainboard.c index 28d3bb0..d532a48 100644 --- a/src/mainboard/packardbell/ms2290/mainboard.c +++ b/src/mainboard/packardbell/ms2290/mainboard.c @@ -83,19 +83,6 @@ for (i = 0; i < 256; i++) ec_write (i, dmp[i]);
- pmbase = pci_read_config32(pcidev_on_root(0x1f, 0), - PMBASE) & 0xff80; - - printk(BIOS_SPEW, " ... pmbase = 0x%04x\n", pmbase); - - outl(0, pmbase + SMI_EN); - - enable_lapic(); - pci_write_config32(pcidev_on_root(0x1f, 0), GPIO_BASE, - DEFAULT_GPIOBASE | 1); - pci_write_config8(pcidev_on_root(0x1f, 0), GPIO_CNTL, - 0x10); - install_intel_vga_int15_handler(GMA_INT15_ACTIVE_LFP_INT_LVDS, GMA_INT15_PANEL_FIT_DEFAULT, GMA_INT15_BOOT_DISPLAY_LFP, 2);
/* This sneaked in here, because EasyNote has no SuperIO chip.
Hello Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33141
to look at the new patch set (#2).
Change subject: mb/*/{x201,ms2290}/mainboard.c: Remove superfluous ramstage code ......................................................................
mb/*/{x201,ms2290}/mainboard.c: Remove superfluous ramstage code
Change-Id: I0270c50dea2a2ce6c8e6114ed708f06be9d33c0e Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/mainboard/lenovo/x201/mainboard.c M src/mainboard/packardbell/ms2290/mainboard.c 2 files changed, 0 insertions(+), 30 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/33141/2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33141 )
Change subject: mb/*/{x201,ms2290}/mainboard.c: Remove superfluous ramstage code ......................................................................
Patch Set 2: Code-Review+1
(1 comment)
https://review.coreboot.org/#/c/33141/2/src/mainboard/lenovo/x201/mainboard.... File src/mainboard/lenovo/x201/mainboard.c:
https://review.coreboot.org/#/c/33141/2/src/mainboard/lenovo/x201/mainboard.... PS2, Line 81: : : : Aren't these needed? Maybe they should go to a more common place, though.
Hello Patrick Rudolph, Angel Pons, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33141
to look at the new patch set (#3).
Change subject: mb/*/{x201,ms2290}/mainboard.c: Remove superfluous ramstage code ......................................................................
mb/*/{x201,ms2290}/mainboard.c: Remove superfluous ramstage code
Change-Id: I0270c50dea2a2ce6c8e6114ed708f06be9d33c0e Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/mainboard/lenovo/x201/mainboard.c M src/mainboard/packardbell/ms2290/mainboard.c 2 files changed, 0 insertions(+), 26 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/33141/3
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33141 )
Change subject: mb/*/{x201,ms2290}/mainboard.c: Remove superfluous ramstage code ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/#/c/33141/3/src/mainboard/lenovo/x201/mainboard.... File src/mainboard/lenovo/x201/mainboard.c:
https://review.coreboot.org/#/c/33141/3/src/mainboard/lenovo/x201/mainboard.... PS3, Line 68: u16 pmbase; Unused
Hello Patrick Rudolph, Angel Pons, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33141
to look at the new patch set (#4).
Change subject: mb/*/{x201,ms2290}/mainboard.c: Remove superfluous ramstage code ......................................................................
mb/*/{x201,ms2290}/mainboard.c: Remove superfluous ramstage code
Change-Id: I0270c50dea2a2ce6c8e6114ed708f06be9d33c0e Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/mainboard/lenovo/x201/mainboard.c M src/mainboard/packardbell/ms2290/mainboard.c 2 files changed, 0 insertions(+), 30 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/33141/4
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33141 )
Change subject: mb/*/{x201,ms2290}/mainboard.c: Remove superfluous ramstage code ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/#/c/33141/2/src/mainboard/lenovo/x201/mainboard.... File src/mainboard/lenovo/x201/mainboard.c:
https://review.coreboot.org/#/c/33141/2/src/mainboard/lenovo/x201/mainboard.... PS2, Line 81: : : :
Aren't these needed? Maybe they should go to a more common place, though.
Done in the romstage
https://review.coreboot.org/#/c/33141/2/src/mainboard/lenovo/x201/mainboard.... PS2, Line 71: /* If we're resuming from suspend, blink suspend LED */ : if (acpi_is_wakeup_s3()) : ec_write(0x0c, 0xc7); This is done in _SST. could likely be removed too
Hello Patrick Rudolph, Angel Pons, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33141
to look at the new patch set (#6).
Change subject: mb/*/{x201,ms2290}/mainboard.c: Remove superfluous ramstage code ......................................................................
mb/*/{x201,ms2290}/mainboard.c: Remove superfluous ramstage code
Change-Id: I0270c50dea2a2ce6c8e6114ed708f06be9d33c0e Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/mainboard/lenovo/x201/mainboard.c M src/mainboard/packardbell/ms2290/mainboard.c 2 files changed, 1 insertion(+), 30 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/33141/6
Hello Alexander Couzens, Patrick Rudolph, Angel Pons, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33141
to look at the new patch set (#7).
Change subject: mb/*/{x201,ms2290}/mainboard.c: Remove superfluous ramstage code ......................................................................
mb/*/{x201,ms2290}/mainboard.c: Remove superfluous ramstage code
Change-Id: I0270c50dea2a2ce6c8e6114ed708f06be9d33c0e Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/mainboard/lenovo/x201/mainboard.c M src/mainboard/packardbell/ms2290/mainboard.c 2 files changed, 0 insertions(+), 30 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/33141/7
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33141 )
Change subject: mb/*/{x201,ms2290}/mainboard.c: Remove superfluous ramstage code ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33141/7/src/mainboard/lenovo/x201/m... File src/mainboard/lenovo/x201/mainboard.c:
https://review.coreboot.org/c/coreboot/+/33141/7/src/mainboard/lenovo/x201/m... PS7, Line 47: Do we have equivalent of this later in ramstage?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33141 )
Change subject: mb/*/{x201,ms2290}/mainboard.c: Remove superfluous ramstage code ......................................................................
Patch Set 7: Code-Review+1
(2 comments)
Looks rather good
https://review.coreboot.org/c/coreboot/+/33141/2/src/mainboard/lenovo/x201/m... File src/mainboard/lenovo/x201/mainboard.c:
https://review.coreboot.org/c/coreboot/+/33141/2/src/mainboard/lenovo/x201/m... PS2, Line 81: : : :
Done in the romstage
Done
https://review.coreboot.org/c/coreboot/+/33141/3/src/mainboard/lenovo/x201/m... File src/mainboard/lenovo/x201/mainboard.c:
https://review.coreboot.org/c/coreboot/+/33141/3/src/mainboard/lenovo/x201/m... PS3, Line 68: u16 pmbase;
Unused
Done
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33141 )
Change subject: mb/*/{x201,ms2290}/mainboard.c: Remove superfluous ramstage code ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33141/7/src/mainboard/lenovo/x201/m... File src/mainboard/lenovo/x201/mainboard.c:
https://review.coreboot.org/c/coreboot/+/33141/7/src/mainboard/lenovo/x201/m... PS7, Line 47:
Do we have equivalent of this later in ramstage?
Properly setting this up is done in southbridge_smm_init() in southbridge/intel/common/smi.c afaict.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33141 )
Change subject: mb/*/{x201,ms2290}/mainboard.c: Remove superfluous ramstage code ......................................................................
Patch Set 7: Code-Review+2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33141 )
Change subject: mb/*/{x201,ms2290}/mainboard.c: Remove superfluous ramstage code ......................................................................
Patch Set 7: Code-Review+2
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33141 )
Change subject: mb/*/{x201,ms2290}/mainboard.c: Remove superfluous ramstage code ......................................................................
Patch Set 7:
(2 comments)
https://review.coreboot.org/c/coreboot/+/33141/2/src/mainboard/lenovo/x201/m... File src/mainboard/lenovo/x201/mainboard.c:
https://review.coreboot.org/c/coreboot/+/33141/2/src/mainboard/lenovo/x201/m... PS2, Line 71: /* If we're resuming from suspend, blink suspend LED */ : if (acpi_is_wakeup_s3()) : ec_write(0x0c, 0xc7);
This is done in _SST. […]
Ack
https://review.coreboot.org/c/coreboot/+/33141/7/src/mainboard/lenovo/x201/m... File src/mainboard/lenovo/x201/mainboard.c:
https://review.coreboot.org/c/coreboot/+/33141/7/src/mainboard/lenovo/x201/m... PS7, Line 47: outl(0, pmbase + SMI_EN);
Properly setting this up is done in southbridge_smm_init() in southbridge/intel/common/smi.c afaict.
Ack
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/33141 )
Change subject: mb/*/{x201,ms2290}/mainboard.c: Remove superfluous ramstage code ......................................................................
mb/*/{x201,ms2290}/mainboard.c: Remove superfluous ramstage code
Change-Id: I0270c50dea2a2ce6c8e6114ed708f06be9d33c0e Signed-off-by: Arthur Heymans arthur@aheymans.xyz Reviewed-on: https://review.coreboot.org/c/coreboot/+/33141 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-by: Angel Pons th3fanbus@gmail.com --- M src/mainboard/lenovo/x201/mainboard.c M src/mainboard/packardbell/ms2290/mainboard.c 2 files changed, 0 insertions(+), 30 deletions(-)
Approvals: build bot (Jenkins): Verified Kyösti Mälkki: Looks good to me, approved Angel Pons: Looks good to me, approved
diff --git a/src/mainboard/lenovo/x201/mainboard.c b/src/mainboard/lenovo/x201/mainboard.c index 96033f8..56c439b 100644 --- a/src/mainboard/lenovo/x201/mainboard.c +++ b/src/mainboard/lenovo/x201/mainboard.c @@ -35,23 +35,8 @@
static void mainboard_enable(struct device *dev) { - u16 pmbase; - dev->ops->acpi_fill_ssdt_generator = fill_ssdt;
- pmbase = pci_read_config32(pcidev_on_root(0x1f, 0), - PMBASE) & 0xff80; - - printk(BIOS_SPEW, " ... pmbase = 0x%04x\n", pmbase); - - outl(0, pmbase + SMI_EN); - - enable_lapic(); - pci_write_config32(pcidev_on_root(0x1f, 0), GPIO_BASE, - DEFAULT_GPIOBASE | 1); - pci_write_config8(pcidev_on_root(0x1f, 0), GPIO_CNTL, - 0x10); - /* If we're resuming from suspend, blink suspend LED */ if (acpi_is_wakeup_s3()) ec_write(0x0c, 0xc7); diff --git a/src/mainboard/packardbell/ms2290/mainboard.c b/src/mainboard/packardbell/ms2290/mainboard.c index 508488a..809ccea 100644 --- a/src/mainboard/packardbell/ms2290/mainboard.c +++ b/src/mainboard/packardbell/ms2290/mainboard.c @@ -32,8 +32,6 @@
static void mainboard_enable(struct device *dev) { - u16 pmbase; - int i; const u8 dmp[256] = { 0x00, 0x20, 0x00, 0x00, 0x00, 0x02, 0x89, 0xe4, 0x30, 0x00, 0x40, 0x14, 0x00, 0x00, 0x00, 0x11, @@ -57,19 +55,6 @@ for (i = 0; i < 256; i++) ec_write (i, dmp[i]);
- pmbase = pci_read_config32(pcidev_on_root(0x1f, 0), - PMBASE) & 0xff80; - - printk(BIOS_SPEW, " ... pmbase = 0x%04x\n", pmbase); - - outl(0, pmbase + SMI_EN); - - enable_lapic(); - pci_write_config32(pcidev_on_root(0x1f, 0), GPIO_BASE, - DEFAULT_GPIOBASE | 1); - pci_write_config8(pcidev_on_root(0x1f, 0), GPIO_CNTL, - 0x10); - install_intel_vga_int15_handler(GMA_INT15_ACTIVE_LFP_INT_LVDS, GMA_INT15_PANEL_FIT_DEFAULT, GMA_INT15_BOOT_DISPLAY_LFP, 2);
/* This sneaked in here, because EasyNote has no SuperIO chip.