Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32997
Change subject: soc/intel/braswell: Don't use CAR_GLOBAL ......................................................................
soc/intel/braswell: Don't use CAR_GLOBAL
Now that this soc supports NO_CAR_GLOBAL_MIGRATION CAR_GLOBAL and car_get/set_x are not needed anymore.
Change-Id: Ia7fa97135a4b376ac0bd8b30093a77614cc2cf55 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/soc/intel/braswell/romstage/romstage.c 1 file changed, 16 insertions(+), 21 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/97/32997/1
diff --git a/src/soc/intel/braswell/romstage/romstage.c b/src/soc/intel/braswell/romstage/romstage.c index 22b0df2..054a49c 100644 --- a/src/soc/intel/braswell/romstage/romstage.c +++ b/src/soc/intel/braswell/romstage/romstage.c @@ -17,7 +17,6 @@
#include <cbmem.h> #include <stddef.h> -#include <arch/early_variables.h> #include <arch/io.h> #include <device/mmio.h> #include <device/pci_ops.h> @@ -105,47 +104,43 @@ cmos_init(rtc_failed); }
-static struct chipset_power_state power_state CAR_GLOBAL; +static struct chipset_power_state power_state;
static void migrate_power_state(int is_recovery) { struct chipset_power_state *ps_cbmem; - struct chipset_power_state *ps_car;
- ps_car = car_get_var_ptr(&power_state); ps_cbmem = cbmem_add(CBMEM_ID_POWER_STATE, sizeof(*ps_cbmem));
if (ps_cbmem == NULL) { printk(BIOS_DEBUG, "Not adding power state to cbmem!\n"); return; } - memcpy(ps_cbmem, ps_car, sizeof(*ps_cbmem)); + memcpy(ps_cbmem, &power_state, sizeof(*ps_cbmem)); } ROMSTAGE_CBMEM_INIT_HOOK(migrate_power_state);
struct chipset_power_state *fill_power_state(void) { - struct chipset_power_state *ps = car_get_var_ptr(&power_state); + power_state.pm1_sts = inw(ACPI_BASE_ADDRESS + PM1_STS); + power_state.pm1_en = inw(ACPI_BASE_ADDRESS + PM1_EN); + power_state.pm1_cnt = inl(ACPI_BASE_ADDRESS + PM1_CNT); + power_state.gpe0_sts = inl(ACPI_BASE_ADDRESS + GPE0_STS); + power_state.gpe0_en = inl(ACPI_BASE_ADDRESS + GPE0_EN); + power_state.tco_sts = inl(ACPI_BASE_ADDRESS + TCO_STS); + power_state.prsts = read32((void *)(PMC_BASE_ADDRESS + PRSTS)); + power_state.gen_pmcon1 = read32((void *)(PMC_BASE_ADDRESS + GEN_PMCON1)); + power_state.gen_pmcon2 = read32((void *)(PMC_BASE_ADDRESS + GEN_PMCON2));
- ps->pm1_sts = inw(ACPI_BASE_ADDRESS + PM1_STS); - ps->pm1_en = inw(ACPI_BASE_ADDRESS + PM1_EN); - ps->pm1_cnt = inl(ACPI_BASE_ADDRESS + PM1_CNT); - ps->gpe0_sts = inl(ACPI_BASE_ADDRESS + GPE0_STS); - ps->gpe0_en = inl(ACPI_BASE_ADDRESS + GPE0_EN); - ps->tco_sts = inl(ACPI_BASE_ADDRESS + TCO_STS); - ps->prsts = read32((void *)(PMC_BASE_ADDRESS + PRSTS)); - ps->gen_pmcon1 = read32((void *)(PMC_BASE_ADDRESS + GEN_PMCON1)); - ps->gen_pmcon2 = read32((void *)(PMC_BASE_ADDRESS + GEN_PMCON2)); - - ps->prev_sleep_state = chipset_prev_sleep_state(ps); + power_state.prev_sleep_state = chipset_prev_sleep_state(ps);
printk(BIOS_DEBUG, "pm1_sts: %04x pm1_en: %04x pm1_cnt: %08x\n", - ps->pm1_sts, ps->pm1_en, ps->pm1_cnt); + power_state.pm1_sts, power_state.pm1_en, power_state.pm1_cnt); printk(BIOS_DEBUG, "gpe0_sts: %08x gpe0_en: %08x tco_sts: %08x\n", - ps->gpe0_sts, ps->gpe0_en, ps->tco_sts); + power_state.gpe0_sts, power_state.gpe0_en, power_state.tco_sts); printk(BIOS_DEBUG, "prsts: %08x gen_pmcon1: %08x gen_pmcon2: %08x\n", - ps->prsts, ps->gen_pmcon1, ps->gen_pmcon2); - printk(BIOS_DEBUG, "prev_sleep_state %d\n", ps->prev_sleep_state); + power_state.prsts, power_state.gen_pmcon1, power_state.gen_pmcon2); + printk(BIOS_DEBUG, "prev_sleep_state %d\n", power_state.prev_sleep_state); return ps; }
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32997 )
Change subject: soc/intel/braswell: Don't use CAR_GLOBAL ......................................................................
Patch Set 1:
(4 comments)
https://review.coreboot.org/#/c/32997/1/src/soc/intel/braswell/romstage/roms... File src/soc/intel/braswell/romstage/romstage.c:
https://review.coreboot.org/#/c/32997/1/src/soc/intel/braswell/romstage/roms... PS1, Line 132: power_state.gen_pmcon1 = read32((void *)(PMC_BASE_ADDRESS + GEN_PMCON1)); line over 80 characters
https://review.coreboot.org/#/c/32997/1/src/soc/intel/braswell/romstage/roms... PS1, Line 133: power_state.gen_pmcon2 = read32((void *)(PMC_BASE_ADDRESS + GEN_PMCON2)); line over 80 characters
https://review.coreboot.org/#/c/32997/1/src/soc/intel/braswell/romstage/roms... PS1, Line 142: power_state.prsts, power_state.gen_pmcon1, power_state.gen_pmcon2); line over 80 characters
https://review.coreboot.org/#/c/32997/1/src/soc/intel/braswell/romstage/roms... PS1, Line 143: printk(BIOS_DEBUG, "prev_sleep_state %d\n", power_state.prev_sleep_state); line over 80 characters
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32997 )
Change subject: soc/intel/braswell: Don't use CAR_GLOBAL ......................................................................
Patch Set 2:
(4 comments)
https://review.coreboot.org/#/c/32997/2/src/soc/intel/braswell/romstage/roms... File src/soc/intel/braswell/romstage/romstage.c:
https://review.coreboot.org/#/c/32997/2/src/soc/intel/braswell/romstage/roms... PS2, Line 132: power_state.gen_pmcon1 = read32((void *)(PMC_BASE_ADDRESS + GEN_PMCON1)); line over 80 characters
https://review.coreboot.org/#/c/32997/2/src/soc/intel/braswell/romstage/roms... PS2, Line 133: power_state.gen_pmcon2 = read32((void *)(PMC_BASE_ADDRESS + GEN_PMCON2)); line over 80 characters
https://review.coreboot.org/#/c/32997/2/src/soc/intel/braswell/romstage/roms... PS2, Line 142: power_state.prsts, power_state.gen_pmcon1, power_state.gen_pmcon2); line over 80 characters
https://review.coreboot.org/#/c/32997/2/src/soc/intel/braswell/romstage/roms... PS2, Line 143: printk(BIOS_DEBUG, "prev_sleep_state %d\n", power_state.prev_sleep_state); line over 80 characters
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32997 )
Change subject: soc/intel/braswell: Don't use CAR_GLOBAL ......................................................................
Patch Set 3:
(4 comments)
https://review.coreboot.org/#/c/32997/3/src/soc/intel/braswell/romstage/roms... File src/soc/intel/braswell/romstage/romstage.c:
https://review.coreboot.org/#/c/32997/3/src/soc/intel/braswell/romstage/roms... PS3, Line 132: power_state.gen_pmcon1 = read32((void *)(PMC_BASE_ADDRESS + GEN_PMCON1)); line over 80 characters
https://review.coreboot.org/#/c/32997/3/src/soc/intel/braswell/romstage/roms... PS3, Line 133: power_state.gen_pmcon2 = read32((void *)(PMC_BASE_ADDRESS + GEN_PMCON2)); line over 80 characters
https://review.coreboot.org/#/c/32997/3/src/soc/intel/braswell/romstage/roms... PS3, Line 142: power_state.prsts, power_state.gen_pmcon1, power_state.gen_pmcon2); line over 80 characters
https://review.coreboot.org/#/c/32997/3/src/soc/intel/braswell/romstage/roms... PS3, Line 143: printk(BIOS_DEBUG, "prev_sleep_state %d\n", power_state.prev_sleep_state); line over 80 characters
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32997
to look at the new patch set (#4).
Change subject: soc/intel/braswell: Don't use CAR_GLOBAL ......................................................................
soc/intel/braswell: Don't use CAR_GLOBAL
Now that this soc supports NO_CAR_GLOBAL_MIGRATION CAR_GLOBAL and car_get/set_x are not needed anymore.
Change-Id: Ia7fa97135a4b376ac0bd8b30093a77614cc2cf55 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/soc/intel/braswell/romstage/romstage.c 1 file changed, 17 insertions(+), 22 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/97/32997/4
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32997 )
Change subject: soc/intel/braswell: Don't use CAR_GLOBAL ......................................................................
Patch Set 4:
(4 comments)
https://review.coreboot.org/#/c/32997/4/src/soc/intel/braswell/romstage/roms... File src/soc/intel/braswell/romstage/romstage.c:
https://review.coreboot.org/#/c/32997/4/src/soc/intel/braswell/romstage/roms... PS4, Line 132: power_state.gen_pmcon1 = read32((void *)(PMC_BASE_ADDRESS + GEN_PMCON1)); line over 80 characters
https://review.coreboot.org/#/c/32997/4/src/soc/intel/braswell/romstage/roms... PS4, Line 133: power_state.gen_pmcon2 = read32((void *)(PMC_BASE_ADDRESS + GEN_PMCON2)); line over 80 characters
https://review.coreboot.org/#/c/32997/4/src/soc/intel/braswell/romstage/roms... PS4, Line 142: power_state.prsts, power_state.gen_pmcon1, power_state.gen_pmcon2); line over 80 characters
https://review.coreboot.org/#/c/32997/4/src/soc/intel/braswell/romstage/roms... PS4, Line 143: printk(BIOS_DEBUG, "prev_sleep_state %d\n", power_state.prev_sleep_state); line over 80 characters
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32997 )
Change subject: soc/intel/braswell: Don't use CAR_GLOBAL ......................................................................
Patch Set 4: Code-Review+2
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32997 )
Change subject: soc/intel/braswell: Don't use CAR_GLOBAL ......................................................................
soc/intel/braswell: Don't use CAR_GLOBAL
Now that this soc supports NO_CAR_GLOBAL_MIGRATION CAR_GLOBAL and car_get/set_x are not needed anymore.
Change-Id: Ia7fa97135a4b376ac0bd8b30093a77614cc2cf55 Signed-off-by: Arthur Heymans arthur@aheymans.xyz Reviewed-on: https://review.coreboot.org/c/coreboot/+/32997 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/soc/intel/braswell/romstage/romstage.c 1 file changed, 17 insertions(+), 22 deletions(-)
Approvals: build bot (Jenkins): Verified Kyösti Mälkki: Looks good to me, approved
diff --git a/src/soc/intel/braswell/romstage/romstage.c b/src/soc/intel/braswell/romstage/romstage.c index 22b0df2..8dfc291 100644 --- a/src/soc/intel/braswell/romstage/romstage.c +++ b/src/soc/intel/braswell/romstage/romstage.c @@ -17,7 +17,6 @@
#include <cbmem.h> #include <stddef.h> -#include <arch/early_variables.h> #include <arch/io.h> #include <device/mmio.h> #include <device/pci_ops.h> @@ -105,48 +104,44 @@ cmos_init(rtc_failed); }
-static struct chipset_power_state power_state CAR_GLOBAL; +static struct chipset_power_state power_state;
static void migrate_power_state(int is_recovery) { struct chipset_power_state *ps_cbmem; - struct chipset_power_state *ps_car;
- ps_car = car_get_var_ptr(&power_state); ps_cbmem = cbmem_add(CBMEM_ID_POWER_STATE, sizeof(*ps_cbmem));
if (ps_cbmem == NULL) { printk(BIOS_DEBUG, "Not adding power state to cbmem!\n"); return; } - memcpy(ps_cbmem, ps_car, sizeof(*ps_cbmem)); + memcpy(ps_cbmem, &power_state, sizeof(*ps_cbmem)); } ROMSTAGE_CBMEM_INIT_HOOK(migrate_power_state);
struct chipset_power_state *fill_power_state(void) { - struct chipset_power_state *ps = car_get_var_ptr(&power_state); + power_state.pm1_sts = inw(ACPI_BASE_ADDRESS + PM1_STS); + power_state.pm1_en = inw(ACPI_BASE_ADDRESS + PM1_EN); + power_state.pm1_cnt = inl(ACPI_BASE_ADDRESS + PM1_CNT); + power_state.gpe0_sts = inl(ACPI_BASE_ADDRESS + GPE0_STS); + power_state.gpe0_en = inl(ACPI_BASE_ADDRESS + GPE0_EN); + power_state.tco_sts = inl(ACPI_BASE_ADDRESS + TCO_STS); + power_state.prsts = read32((void *)(PMC_BASE_ADDRESS + PRSTS)); + power_state.gen_pmcon1 = read32((void *)(PMC_BASE_ADDRESS + GEN_PMCON1)); + power_state.gen_pmcon2 = read32((void *)(PMC_BASE_ADDRESS + GEN_PMCON2));
- ps->pm1_sts = inw(ACPI_BASE_ADDRESS + PM1_STS); - ps->pm1_en = inw(ACPI_BASE_ADDRESS + PM1_EN); - ps->pm1_cnt = inl(ACPI_BASE_ADDRESS + PM1_CNT); - ps->gpe0_sts = inl(ACPI_BASE_ADDRESS + GPE0_STS); - ps->gpe0_en = inl(ACPI_BASE_ADDRESS + GPE0_EN); - ps->tco_sts = inl(ACPI_BASE_ADDRESS + TCO_STS); - ps->prsts = read32((void *)(PMC_BASE_ADDRESS + PRSTS)); - ps->gen_pmcon1 = read32((void *)(PMC_BASE_ADDRESS + GEN_PMCON1)); - ps->gen_pmcon2 = read32((void *)(PMC_BASE_ADDRESS + GEN_PMCON2)); - - ps->prev_sleep_state = chipset_prev_sleep_state(ps); + power_state.prev_sleep_state = chipset_prev_sleep_state(&power_state);
printk(BIOS_DEBUG, "pm1_sts: %04x pm1_en: %04x pm1_cnt: %08x\n", - ps->pm1_sts, ps->pm1_en, ps->pm1_cnt); + power_state.pm1_sts, power_state.pm1_en, power_state.pm1_cnt); printk(BIOS_DEBUG, "gpe0_sts: %08x gpe0_en: %08x tco_sts: %08x\n", - ps->gpe0_sts, ps->gpe0_en, ps->tco_sts); + power_state.gpe0_sts, power_state.gpe0_en, power_state.tco_sts); printk(BIOS_DEBUG, "prsts: %08x gen_pmcon1: %08x gen_pmcon2: %08x\n", - ps->prsts, ps->gen_pmcon1, ps->gen_pmcon2); - printk(BIOS_DEBUG, "prev_sleep_state %d\n", ps->prev_sleep_state); - return ps; + power_state.prsts, power_state.gen_pmcon1, power_state.gen_pmcon2); + printk(BIOS_DEBUG, "prev_sleep_state %d\n", power_state.prev_sleep_state); + return &power_state; }
/* Return 0, 3, or 5 to indicate the previous sleep state. */
-
Arthur Heymans (Code Review) -
build bot (Jenkins) (Code Review)
-
Kyösti Mälkki (Code Review)
-
Patrick Georgi (Code Review)