Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/48851 )
Change subject: soc/amd/picasso,stoneyridge: Change NVS parameter passing ......................................................................
soc/amd/picasso,stoneyridge: Change NVS parameter passing
Change-Id: I80f92bed737904e6ffc858b45459405fe76f1d04 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/soc/amd/common/block/acpi/acpi.c M src/soc/amd/common/block/include/amdblocks/acpi.h M src/soc/amd/picasso/fch.c M src/soc/amd/stoneyridge/southbridge.c 4 files changed, 9 insertions(+), 13 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/48851/1
diff --git a/src/soc/amd/common/block/acpi/acpi.c b/src/soc/amd/common/block/acpi/acpi.c index 4ca6599..3c108d9 100644 --- a/src/soc/amd/common/block/acpi/acpi.c +++ b/src/soc/amd/common/block/acpi/acpi.c @@ -3,12 +3,14 @@ #include <amdblocks/acpimmio.h> #include <amdblocks/acpi.h> #include <acpi/acpi.h> +#include <acpi/acpi_gnvs.h> #include <bootmode.h> #include <console/console.h> #include <elog.h> #include <halt.h> #include <security/vboot/vboot_common.h> #include <soc/southbridge.h> +#include <soc/nvs.h>
void poweroff(void) { @@ -138,9 +140,12 @@ return i; }
-void acpi_fill_gnvs(struct global_nvs *gnvs, const struct acpi_pm_gpe_state *state) +void acpi_fill_gnvs(const struct acpi_pm_gpe_state *state) { int index; + struct global_nvs *gnvs = acpi_get_gnvs(); + if (gnvs == NULL) + return;
index = get_index_bit(state->pm1_sts & state->pm1_en, PM1_LIMIT); if (index < 0) diff --git a/src/soc/amd/common/block/include/amdblocks/acpi.h b/src/soc/amd/common/block/include/amdblocks/acpi.h index c6e242a..65929af 100644 --- a/src/soc/amd/common/block/include/amdblocks/acpi.h +++ b/src/soc/amd/common/block/include/amdblocks/acpi.h @@ -4,7 +4,6 @@ #define AMD_BLOCK_ACPI_H
#include <types.h> -#include <soc/nvs.h>
/* ACPI MMIO registers 0xfed80800 */ #define MMIO_ACPI_PM1_STS 0x00 @@ -34,7 +33,7 @@ /* Clear PM and GPE status registers. */ void acpi_clear_pm_gpe_status(void); /* Fill GNVS object from PM GPE object. */ -void acpi_fill_gnvs(struct global_nvs *gnvs, const struct acpi_pm_gpe_state *state); +void acpi_fill_gnvs(const struct acpi_pm_gpe_state *state);
/* * If a system reset is about to be requested, modify the PM1 register so it diff --git a/src/soc/amd/picasso/fch.c b/src/soc/amd/picasso/fch.c index d5278cb..65cc86e 100644 --- a/src/soc/amd/picasso/fch.c +++ b/src/soc/amd/picasso/fch.c @@ -147,16 +147,12 @@ static void set_nvs_sws(void *unused) { struct chipset_state *state; - struct global_nvs *gnvs;
state = cbmem_find(CBMEM_ID_POWER_STATE); if (state == NULL) return; - gnvs = acpi_get_gnvs(); - if (gnvs == NULL) - return;
- acpi_fill_gnvs(gnvs, &state->gpe_state); + acpi_fill_gnvs(&state->gpe_state); }
BOOT_STATE_INIT_ENTRY(BS_OS_RESUME, BS_ON_ENTRY, set_nvs_sws, NULL); diff --git a/src/soc/amd/stoneyridge/southbridge.c b/src/soc/amd/stoneyridge/southbridge.c index 4436e0e..cc36aa1 100644 --- a/src/soc/amd/stoneyridge/southbridge.c +++ b/src/soc/amd/stoneyridge/southbridge.c @@ -404,16 +404,12 @@ static void set_nvs_sws(void *unused) { struct acpi_pm_gpe_state *state; - struct global_nvs *gnvs;
state = cbmem_find(CBMEM_ID_POWER_STATE); if (state == NULL) return; - gnvs = acpi_get_gnvs(); - if (gnvs == NULL) - return;
- acpi_fill_gnvs(gnvs, state); + acpi_fill_gnvs(state); }
BOOT_STATE_INIT_ENTRY(BS_OS_RESUME, BS_ON_ENTRY, set_nvs_sws, NULL);
Hello build bot (Jenkins), Jason Glenesk, Marshall Dawson, Felix Held,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48851
to look at the new patch set (#2).
Change subject: soc/amd: Rename to pm_fill_gnvs() ......................................................................
soc/amd: Rename to pm_fill_gnvs()
Change-Id: I80f92bed737904e6ffc858b45459405fe76f1d04 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/soc/amd/common/block/acpi/acpi.c M src/soc/amd/common/block/include/amdblocks/acpi.h M src/soc/amd/picasso/fch.c M src/soc/amd/stoneyridge/southbridge.c 4 files changed, 9 insertions(+), 13 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/48851/2
Attention is currently required from: Kyösti Mälkki. Jason Glenesk has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48851 )
Change subject: soc/amd: Rename to pm_fill_gnvs() ......................................................................
Patch Set 2: Code-Review+1
Attention is currently required from: Kyösti Mälkki. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48851 )
Change subject: soc/amd: Rename to pm_fill_gnvs() ......................................................................
Patch Set 2: Code-Review+2
(1 comment)
File src/soc/amd/stoneyridge/southbridge.c:
https://review.coreboot.org/c/coreboot/+/48851/comment/b047c57a_4e1c146f PS2, Line 408: state = cbmem_find(CBMEM_ID_POWER_STATE); Idea for a future patch: Unify `set_nvs_sws` between stoney and picasso.
I'm just writing it down here because I will most likely end up forgetting about it. If someone else wants to try, I'll be happy to review.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48851 )
Change subject: soc/amd: Rename to pm_fill_gnvs() ......................................................................
Patch Set 2:
(1 comment)
File src/soc/amd/stoneyridge/southbridge.c:
https://review.coreboot.org/c/coreboot/+/48851/comment/345bdb5b_df784936 PS2, Line 408: state = cbmem_find(CBMEM_ID_POWER_STATE);
Idea for a future patch: Unify `set_nvs_sws` between stoney and picasso. […]
CB:48555 or CB:48856 ?
Attention is currently required from: Kyösti Mälkki. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48851 )
Change subject: soc/amd: Rename to pm_fill_gnvs() ......................................................................
Patch Set 2:
(1 comment)
File src/soc/amd/stoneyridge/southbridge.c:
https://review.coreboot.org/c/coreboot/+/48851/comment/fbca8e80_405c8a67 PS2, Line 408: state = cbmem_find(CBMEM_ID_POWER_STATE);
CB:48555 or CB:48856 ?
CB:48855 and CB:48856
Angel Pons has submitted this change. ( https://review.coreboot.org/c/coreboot/+/48851 )
Change subject: soc/amd: Rename to pm_fill_gnvs() ......................................................................
soc/amd: Rename to pm_fill_gnvs()
Change-Id: I80f92bed737904e6ffc858b45459405fe76f1d04 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/48851 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Jason Glenesk jason.glenesk@gmail.com Reviewed-by: Angel Pons th3fanbus@gmail.com --- M src/soc/amd/common/block/acpi/acpi.c M src/soc/amd/common/block/include/amdblocks/acpi.h M src/soc/amd/picasso/fch.c M src/soc/amd/stoneyridge/southbridge.c 4 files changed, 9 insertions(+), 13 deletions(-)
Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved Jason Glenesk: Looks good to me, but someone else must approve
diff --git a/src/soc/amd/common/block/acpi/acpi.c b/src/soc/amd/common/block/acpi/acpi.c index 4ca6599..572faa5 100644 --- a/src/soc/amd/common/block/acpi/acpi.c +++ b/src/soc/amd/common/block/acpi/acpi.c @@ -3,12 +3,14 @@ #include <amdblocks/acpimmio.h> #include <amdblocks/acpi.h> #include <acpi/acpi.h> +#include <acpi/acpi_gnvs.h> #include <bootmode.h> #include <console/console.h> #include <elog.h> #include <halt.h> #include <security/vboot/vboot_common.h> #include <soc/southbridge.h> +#include <soc/nvs.h>
void poweroff(void) { @@ -138,9 +140,12 @@ return i; }
-void acpi_fill_gnvs(struct global_nvs *gnvs, const struct acpi_pm_gpe_state *state) +void pm_fill_gnvs(const struct acpi_pm_gpe_state *state) { int index; + struct global_nvs *gnvs = acpi_get_gnvs(); + if (gnvs == NULL) + return;
index = get_index_bit(state->pm1_sts & state->pm1_en, PM1_LIMIT); if (index < 0) diff --git a/src/soc/amd/common/block/include/amdblocks/acpi.h b/src/soc/amd/common/block/include/amdblocks/acpi.h index c6e242a..0512ad6 100644 --- a/src/soc/amd/common/block/include/amdblocks/acpi.h +++ b/src/soc/amd/common/block/include/amdblocks/acpi.h @@ -4,7 +4,6 @@ #define AMD_BLOCK_ACPI_H
#include <types.h> -#include <soc/nvs.h>
/* ACPI MMIO registers 0xfed80800 */ #define MMIO_ACPI_PM1_STS 0x00 @@ -34,7 +33,7 @@ /* Clear PM and GPE status registers. */ void acpi_clear_pm_gpe_status(void); /* Fill GNVS object from PM GPE object. */ -void acpi_fill_gnvs(struct global_nvs *gnvs, const struct acpi_pm_gpe_state *state); +void pm_fill_gnvs(const struct acpi_pm_gpe_state *state);
/* * If a system reset is about to be requested, modify the PM1 register so it diff --git a/src/soc/amd/picasso/fch.c b/src/soc/amd/picasso/fch.c index 4c8c584..7479a35 100644 --- a/src/soc/amd/picasso/fch.c +++ b/src/soc/amd/picasso/fch.c @@ -148,16 +148,12 @@ static void set_nvs_sws(void *unused) { struct chipset_state *state; - struct global_nvs *gnvs;
state = cbmem_find(CBMEM_ID_POWER_STATE); if (state == NULL) return; - gnvs = acpi_get_gnvs(); - if (gnvs == NULL) - return;
- acpi_fill_gnvs(gnvs, &state->gpe_state); + pm_fill_gnvs(&state->gpe_state); }
BOOT_STATE_INIT_ENTRY(BS_OS_RESUME, BS_ON_ENTRY, set_nvs_sws, NULL); diff --git a/src/soc/amd/stoneyridge/southbridge.c b/src/soc/amd/stoneyridge/southbridge.c index 4436e0e..3aecbb4 100644 --- a/src/soc/amd/stoneyridge/southbridge.c +++ b/src/soc/amd/stoneyridge/southbridge.c @@ -404,16 +404,12 @@ static void set_nvs_sws(void *unused) { struct acpi_pm_gpe_state *state; - struct global_nvs *gnvs;
state = cbmem_find(CBMEM_ID_POWER_STATE); if (state == NULL) return; - gnvs = acpi_get_gnvs(); - if (gnvs == NULL) - return;
- acpi_fill_gnvs(gnvs, state); + pm_fill_gnvs(state); }
BOOT_STATE_INIT_ENTRY(BS_OS_RESUME, BS_ON_ENTRY, set_nvs_sws, NULL);
-
Angel Pons (Code Review) -
Jason Glenesk (Code Review)
-
Kyösti Mälkki (Code Review)