Martin Roth has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/42765 )
Change subject: soc/amd/common: Don't init SMIs or SCIs in psp_verstage ......................................................................
soc/amd/common: Don't init SMIs or SCIs in psp_verstage
We can't set the SMI or SCI flags in psp verstage, so skip them.
TEST=Build BUG=b:154142138
Signed-off-by: Martin Roth martin@coreboot.org Change-Id: I40eb464cde6b233607de1e177702c643ea2b4bb2 --- M src/soc/amd/common/block/gpio_banks/gpio.c 1 file changed, 12 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/42765/1
diff --git a/src/soc/amd/common/block/gpio_banks/gpio.c b/src/soc/amd/common/block/gpio_banks/gpio.c index 419f67a..3169e0e 100644 --- a/src/soc/amd/common/block/gpio_banks/gpio.c +++ b/src/soc/amd/common/block/gpio_banks/gpio.c @@ -180,6 +180,7 @@ int gevent_num; const struct soc_amd_event *gev_tbl; size_t gev_items; + const bool can_set_smi_flags = !(CONFIG(VBOOT_STARTS_BEFORE_BOOTBLOCK) && ENV_SEPARATE_VERSTAGE);
inter_master = gpio_get_bar() + GPIO_MASTER_SWITCH; direction = 0; @@ -198,7 +199,9 @@ */ mem_read_write32(inter_master, 0, GPIO_MASK_STS_EN | GPIO_INTERRUPT_EN);
- soc_get_gpio_event_table(&gev_tbl, &gev_items); + if (can_set_smi_flags) { + soc_get_gpio_event_table(&gev_tbl, &gev_items); + }
for (index = 0; index < size; index++) { gpio = gpio_list_ptr[index].gpio; @@ -234,11 +237,19 @@ AMD_GPIO_CONTROL_MASK); break; case GPIO_SMI_FLAG: + /* Can't set SMI flags from PSP */ + if (!can_set_smi_flags) + break; + mem_read_write32(gpio_ptr, control, INT_SCI_SMI_MASK); program_smi(control_flags, gevent_num); break; case GPIO_SCI_FLAG: + /* Can't set SCI flags from PSP */ + if (!can_set_smi_flags) + break; + mem_read_write32(gpio_ptr, control, INT_SCI_SMI_MASK); get_sci_config_bits(control_flags, &bit_edge,
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42765 )
Change subject: soc/amd/common: Don't init SMIs or SCIs in psp_verstage ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42765/1/src/soc/amd/common/block/gp... File src/soc/amd/common/block/gpio_banks/gpio.c:
https://review.coreboot.org/c/coreboot/+/42765/1/src/soc/amd/common/block/gp... PS1, Line 183: const bool can_set_smi_flags = !(CONFIG(VBOOT_STARTS_BEFORE_BOOTBLOCK) && ENV_SEPARATE_VERSTAGE); line over 96 characters
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42765
to look at the new patch set (#2).
Change subject: soc/amd/common: Don't init SMIs or SCIs in psp_verstage ......................................................................
soc/amd/common: Don't init SMIs or SCIs in psp_verstage
We can't set the SMI or SCI flags in psp verstage, so skip them.
TEST=Build BUG=b:154142138
Signed-off-by: Martin Roth martin@coreboot.org Change-Id: I40eb464cde6b233607de1e177702c643ea2b4bb2 --- M src/soc/amd/common/block/gpio_banks/gpio.c 1 file changed, 12 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/42765/2
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42765 )
Change subject: soc/amd/common: Don't init SMIs or SCIs in psp_verstage ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42765/2/src/soc/amd/common/block/gp... File src/soc/amd/common/block/gpio_banks/gpio.c:
https://review.coreboot.org/c/coreboot/+/42765/2/src/soc/amd/common/block/gp... PS2, Line 183: const bool can_set_smi_flags = !(CONFIG(VBOOT_STARTS_BEFORE_BOOTBLOCK) && ENV_SEPARATE_VERSTAGE); line over 96 characters
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42765 )
Change subject: soc/amd/common: Don't init SMIs or SCIs in psp_verstage ......................................................................
Patch Set 2: Code-Review+2
Hello build bot (Jenkins), Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42765
to look at the new patch set (#3).
Change subject: soc/amd/common: Don't init SMIs or SCIs in psp_verstage ......................................................................
soc/amd/common: Don't init SMIs or SCIs in psp_verstage
We can't set the SMI or SCI flags in psp verstage, so skip them.
TEST=Build BUG=b:154142138
Signed-off-by: Martin Roth martin@coreboot.org Change-Id: I40eb464cde6b233607de1e177702c643ea2b4bb2 --- M src/soc/amd/common/block/gpio_banks/gpio.c 1 file changed, 10 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/42765/3
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42765 )
Change subject: soc/amd/common: Don't init SMIs or SCIs in psp_verstage ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42765/3/src/soc/amd/common/block/gp... File src/soc/amd/common/block/gpio_banks/gpio.c:
https://review.coreboot.org/c/coreboot/+/42765/3/src/soc/amd/common/block/gp... PS3, Line 186: const bool can_set_smi_flags = !(CONFIG(VBOOT_STARTS_BEFORE_BOOTBLOCK) && ENV_SEPARATE_VERSTAGE); line over 96 characters
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42765 )
Change subject: soc/amd/common: Don't init SMIs or SCIs in psp_verstage ......................................................................
Patch Set 3: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/42765/3/src/soc/amd/common/block/gp... File src/soc/amd/common/block/gpio_banks/gpio.c:
https://review.coreboot.org/c/coreboot/+/42765/3/src/soc/amd/common/block/gp... PS3, Line 186: const bool can_set_smi_flags = !(CONFIG(VBOOT_STARTS_BEFORE_BOOTBLOCK) && ENV_SEPARATE_VERSTAGE);
line over 96 characters
Please fix.
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42765 )
Change subject: soc/amd/common: Don't init SMIs or SCIs in psp_verstage ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42765/3/src/soc/amd/common/block/gp... File src/soc/amd/common/block/gpio_banks/gpio.c:
https://review.coreboot.org/c/coreboot/+/42765/3/src/soc/amd/common/block/gp... PS3, Line 186: const bool can_set_smi_flags = !(CONFIG(VBOOT_STARTS_BEFORE_BOOTBLOCK) && ENV_SEPARATE_VERSTAGE);
Please fix.
Done
Hello build bot (Jenkins), Raul Rangel, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42765
to look at the new patch set (#4).
Change subject: soc/amd/common: Don't init SMIs or SCIs in psp_verstage ......................................................................
soc/amd/common: Don't init SMIs or SCIs in psp_verstage
We can't set the SMI or SCI flags in psp verstage, so skip them.
TEST=Build BUG=b:154142138
Signed-off-by: Martin Roth martin@coreboot.org Change-Id: I40eb464cde6b233607de1e177702c643ea2b4bb2 --- M src/soc/amd/common/block/gpio_banks/gpio.c 1 file changed, 13 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/42765/4
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42765 )
Change subject: soc/amd/common: Don't init SMIs or SCIs in psp_verstage ......................................................................
Patch Set 4: Code-Review+2
Hello build bot (Jenkins), Raul Rangel, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42765
to look at the new patch set (#5).
Change subject: soc/amd/common: Don't init SMIs or SCIs in psp_verstage ......................................................................
soc/amd/common: Don't init SMIs or SCIs in psp_verstage
We can't set the SMI or SCI flags in psp verstage, so skip them.
TEST=Build BUG=b:154142138
Signed-off-by: Martin Roth martin@coreboot.org Change-Id: I40eb464cde6b233607de1e177702c643ea2b4bb2 --- M src/soc/amd/common/block/gpio_banks/gpio.c 1 file changed, 13 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/42765/5
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42765 )
Change subject: soc/amd/common: Don't init SMIs or SCIs in psp_verstage ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42765/5/src/soc/amd/common/block/gp... File src/soc/amd/common/block/gpio_banks/gpio.c:
https://review.coreboot.org/c/coreboot/+/42765/5/src/soc/amd/common/block/gp... PS5, Line 230: if (control_flags & GPIO_FLAG_SMI) { suspect code indent for conditional statements (16, 32)
Hello build bot (Jenkins), Raul Rangel, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42765
to look at the new patch set (#6).
Change subject: soc/amd/common: Don't init SMIs or SCIs in psp_verstage ......................................................................
soc/amd/common: Don't init SMIs or SCIs in psp_verstage
We can't set the SMI or SCI flags in psp verstage, so skip them.
TEST=Build BUG=b:154142138
Signed-off-by: Martin Roth martin@coreboot.org Change-Id: I40eb464cde6b233607de1e177702c643ea2b4bb2 --- M src/soc/amd/common/block/gpio_banks/gpio.c 1 file changed, 13 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/42765/6
Hello build bot (Jenkins), Raul Rangel, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42765
to look at the new patch set (#7).
Change subject: soc/amd/common: Don't init SMIs or SCIs in psp_verstage ......................................................................
soc/amd/common: Don't init SMIs or SCIs in psp_verstage
We can't set the SMI or SCI flags in psp verstage, so skip them.
TEST=Build BUG=b:154142138
Signed-off-by: Martin Roth martin@coreboot.org Change-Id: I40eb464cde6b233607de1e177702c643ea2b4bb2 --- M src/soc/amd/common/block/gpio_banks/gpio.c 1 file changed, 12 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/42765/7
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42765 )
Change subject: soc/amd/common: Don't init SMIs or SCIs in psp_verstage ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42765/7/src/soc/amd/common/block/gp... File src/soc/amd/common/block/gpio_banks/gpio.c:
https://review.coreboot.org/c/coreboot/+/42765/7/src/soc/amd/common/block/gp... PS7, Line 221: With the refactor we can just return here:
if (!can_set_smi_flags) break;
Hello build bot (Jenkins), Raul Rangel, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42765
to look at the new patch set (#8).
Change subject: soc/amd/common: Don't init SMIs or SCIs in psp_verstage ......................................................................
soc/amd/common: Don't init SMIs or SCIs in psp_verstage
We can't set the SMI or SCI flags in psp verstage, so skip them.
TEST=Build BUG=b:154142138
Signed-off-by: Martin Roth martin@coreboot.org Change-Id: I40eb464cde6b233607de1e177702c643ea2b4bb2 --- M src/soc/amd/common/block/gpio_banks/gpio.c 1 file changed, 11 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/42765/8
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42765 )
Change subject: soc/amd/common: Don't init SMIs or SCIs in psp_verstage ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42765/7/src/soc/amd/common/block/gp... File src/soc/amd/common/block/gpio_banks/gpio.c:
https://review.coreboot.org/c/coreboot/+/42765/7/src/soc/amd/common/block/gp... PS7, Line 221:
With the refactor we can just return here: […]
done. (changed to a continue.)
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42765 )
Change subject: soc/amd/common: Don't init SMIs or SCIs in psp_verstage ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42765/8/src/soc/amd/common/block/gp... File src/soc/amd/common/block/gpio_banks/gpio.c:
https://review.coreboot.org/c/coreboot/+/42765/8/src/soc/amd/common/block/gp... PS8, Line 233: /* Can't set SMI flags from PSP */ Move the comment up to the if condition.
Hello build bot (Jenkins), Raul Rangel, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42765
to look at the new patch set (#9).
Change subject: soc/amd/common: Don't init SMIs or SCIs in psp_verstage ......................................................................
soc/amd/common: Don't init SMIs or SCIs in psp_verstage
We can't set the SMI or SCI flags in psp verstage, so skip them.
TEST=Build BUG=b:154142138
Signed-off-by: Martin Roth martin@coreboot.org Change-Id: I40eb464cde6b233607de1e177702c643ea2b4bb2 --- M src/soc/amd/common/block/gpio_banks/gpio.c 1 file changed, 10 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/42765/9
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42765 )
Change subject: soc/amd/common: Don't init SMIs or SCIs in psp_verstage ......................................................................
Patch Set 9: Code-Review+2
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42765 )
Change subject: soc/amd/common: Don't init SMIs or SCIs in psp_verstage ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42765/8/src/soc/amd/common/block/gp... File src/soc/amd/common/block/gpio_banks/gpio.c:
https://review.coreboot.org/c/coreboot/+/42765/8/src/soc/amd/common/block/gp... PS8, Line 233: /* Can't set SMI flags from PSP */
Move the comment up to the if condition.
Done
Martin Roth has submitted this change. ( https://review.coreboot.org/c/coreboot/+/42765 )
Change subject: soc/amd/common: Don't init SMIs or SCIs in psp_verstage ......................................................................
soc/amd/common: Don't init SMIs or SCIs in psp_verstage
We can't set the SMI or SCI flags in psp verstage, so skip them.
TEST=Build BUG=b:154142138
Signed-off-by: Martin Roth martin@coreboot.org Change-Id: I40eb464cde6b233607de1e177702c643ea2b4bb2 Reviewed-on: https://review.coreboot.org/c/coreboot/+/42765 Reviewed-by: Raul Rangel rrangel@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/amd/common/block/gpio_banks/gpio.c 1 file changed, 10 insertions(+), 2 deletions(-)
Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved
diff --git a/src/soc/amd/common/block/gpio_banks/gpio.c b/src/soc/amd/common/block/gpio_banks/gpio.c index 726ee1c..b9646b9 100644 --- a/src/soc/amd/common/block/gpio_banks/gpio.c +++ b/src/soc/amd/common/block/gpio_banks/gpio.c @@ -183,6 +183,8 @@ const struct soc_amd_event *gev_tbl; struct sci_trigger_regs sci_trigger_cfg = { 0 }; size_t gev_items; + const bool can_set_smi_flags = !(CONFIG(VBOOT_STARTS_BEFORE_BOOTBLOCK) && + ENV_SEPARATE_VERSTAGE);
/* * Disable blocking wake/interrupt status generation while updating @@ -196,7 +198,8 @@ */ master_switch_clr(GPIO_MASK_STS_EN | GPIO_INTERRUPT_EN);
- soc_get_gpio_event_table(&gev_tbl, &gev_items); + if (can_set_smi_flags) + soc_get_gpio_event_table(&gev_tbl, &gev_items);
for (index = 0; index < size; index++) { gpio = gpio_list_ptr[index].gpio; @@ -216,6 +219,10 @@ if (control_flags == 0) continue;
+ /* Can't set SMI flags from PSP */ + if (!can_set_smi_flags) + continue; + gevent_num = get_gpio_gevent(gpio, gev_tbl, gev_items); if (gevent_num < 0) { printk(BIOS_WARNING, "Warning: GPIO pin %d has no associated gevent!\n", @@ -241,7 +248,8 @@ master_switch_set(GPIO_INTERRUPT_EN);
/* Set all SCI trigger polarity (high/low) and level (edge/level). */ - set_sci_trigger(&sci_trigger_cfg); + if (can_set_smi_flags) + set_sci_trigger(&sci_trigger_cfg); }
int gpio_interrupt_status(gpio_t gpio)