Attention is currently required from: Raul Rangel, Eric Peers, Felix Held.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56230 )
Change subject: lib/thread: Add mutex
......................................................................
Patch Set 7: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/56230
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ife1ac95ec064ebcdd00fcaacec37a06ac52885ff
Gerrit-Change-Number: 56230
Gerrit-PatchSet: 7
Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Eric Peers <epeers(a)google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Eric Peers <epeers(a)google.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Fri, 16 Jul 2021 00:11:58 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Raul Rangel.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56357 )
Change subject: lib/thread: Rename thread_cooperate and thread_prevent_coop
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/56357
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I1d70c18965f53e733e871ca03107270612efa4fc
Gerrit-Change-Number: 56357
Gerrit-PatchSet: 1
Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Comment-Date: Fri, 16 Jul 2021 00:10:55 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Raul Rangel, Furquan Shaikh.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56350 )
Change subject: lib/thread: Allow nesting thread_cooperate and thread_prevent_coop
......................................................................
Patch Set 2: Code-Review+2
(2 comments)
File src/lib/thread.c:
https://review.coreboot.org/c/coreboot/+/56350/comment/b1597399_ef373e70
PS1, Line 217: thread_cooperate();
> I think moving the `initialized = 1` after the `idle_thread_init` solves that problem. […]
Good point.
https://review.coreboot.org/c/coreboot/+/56350/comment/db20970c_f19b4b0b
PS1, Line 34: return (t != NULL && t->can_yield > 0);
> Normally `can_yield` will be 1. It's setup by `prepare_thread`. […]
Yeah, I mean I think `can_yield` is just not a good name for a counting variable in the first place. I think "zero or non-zero" for the differentiation would be more intuitive than "one, or either zero or a negative number". It's not a big deal either way though.
--
To view, visit https://review.coreboot.org/c/coreboot/+/56350
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I325ab6181b17c5c084ca1e2c181b4df235020557
Gerrit-Change-Number: 56350
Gerrit-PatchSet: 2
Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Comment-Date: Fri, 16 Jul 2021 00:10:18 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Raul Rangel <rrangel(a)chromium.org>
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Raul Rangel.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56356 )
Change subject: lib/thread: Clean up initialization sequence
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/56356
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7f1d6afac3b0622612565b37c61fbd2cd2481552
Gerrit-Change-Number: 56356
Gerrit-PatchSet: 1
Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Comment-Date: Fri, 16 Jul 2021 00:10:11 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Jason Glenesk, Raul Rangel, Furquan Shaikh, Marshall Dawson, Eric Peers, Karthik Ramasubramanian, Felix Held.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56228 )
Change subject: soc/amd/common/block/lpc/spi_dma: Implement SPI DMA functionality
......................................................................
Patch Set 7: Code-Review+1
(1 comment)
File src/soc/amd/common/block/lpc/Kconfig:
https://review.coreboot.org/c/coreboot/+/56228/comment/f193b3fe_03cfa4d4
PS6, Line 15: The SPI DMA controller
: is only functional on Cezanne, Renoir or later SoCs.
> Done
I think checking a handful of specific AMD SoCs in AMD common code should be fine. If this was generic code in src/lib I'd agree it might be better to avoid.
The alternative here would be to do
config SOC_AMD_COMMON_BLOCK_LPC_SPI_DMA
bool
default y
select X86_CUSTOM_BOOTMEDIA
help
...
and then in src/soc/amd/picasso/Kconfig and friends do
config SOC_AMD_COMMON_BLOCK_LPC_SPI_DMA
bool
default n
...which also works. But it's a bit more complicated to follow, so for such a simple and localized case (where we don't expect the exception list to grow much larger than those two SoCs), I think this is fine.
--
To view, visit https://review.coreboot.org/c/coreboot/+/56228
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I0be555956581fd82bbe1482d8afa8828c61aaa01
Gerrit-Change-Number: 56228
Gerrit-PatchSet: 7
Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Eric Peers <epeers(a)google.com>
Gerrit-CC: Furquan Shaikh <furquan(a)google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Eric Peers <epeers(a)google.com>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Fri, 16 Jul 2021 00:04:26 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Raul Rangel <rrangel(a)chromium.org>
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Raul Rangel, Eric Peers, Felix Held.
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56229 )
Change subject: lib/thread: Add thread_handle
......................................................................
Patch Set 7:
(1 comment)
File src/include/thread.h:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-124352):
https://review.coreboot.org/c/coreboot/+/56229/comment/917dbb64_eff62dde
PS7, Line 36: enum cb_err (*entry)(void *);
function definition argument 'void *' should also have an identifier name
--
To view, visit https://review.coreboot.org/c/coreboot/+/56229
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ie6f64d0c5a5acad4431a605f0b0b5100dc5358ff
Gerrit-Change-Number: 56229
Gerrit-PatchSet: 7
Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Eric Peers <epeers(a)google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Eric Peers <epeers(a)google.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Thu, 15 Jul 2021 23:59:55 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson, Felix Held.
Karthik Ramasubramanian has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/56362 )
Change subject: soc/amd/common/elog: Update elog_pm1_status function signature
......................................................................
soc/amd/common/elog: Update elog_pm1_status function signature
The common event log library is used from both Romstage and SMM stage.
Previous Sleep state is logged as part of logging PM1_STS. But it uses
acpi_is_wakeup_s3 which is supported only in Romstage. Previous sleep
state is already read from the PM1_CNT register and is stored as part of
acpi_pm_gpe_state structure. Use the previous state from that structure
and update the elog_pm1_status function signature accordingly.
BUG=None
TEST=Build and boot to OS in Guybrush.
Change-Id: I492d2928fa94e5245101a463341d1f170efb5254
Signed-off-by: Karthikeyan Ramasubramanian <kramasub(a)google.com>
---
M src/soc/amd/common/block/acpi/acpi.c
M src/soc/amd/common/block/elog/elog.c
M src/soc/amd/common/block/include/amdblocks/elog.h
3 files changed, 6 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/56362/1
diff --git a/src/soc/amd/common/block/acpi/acpi.c b/src/soc/amd/common/block/acpi/acpi.c
index d9807eb..2ce9ed6 100644
--- a/src/soc/amd/common/block/acpi/acpi.c
+++ b/src/soc/amd/common/block/acpi/acpi.c
@@ -84,7 +84,7 @@
return;
state = &ps->gpe_state;
- elog_pm1_status(state->pm1_sts);
+ elog_pm1_status(state);
print_pm1_status(state->pm1_sts);
elog_gpe_events(state);
}
diff --git a/src/soc/amd/common/block/elog/elog.c b/src/soc/amd/common/block/elog/elog.c
index a8c2e44..79e9825 100644
--- a/src/soc/amd/common/block/elog/elog.c
+++ b/src/soc/amd/common/block/elog/elog.c
@@ -4,14 +4,15 @@
#include <elog.h>
#include <soc/southbridge.h>
-void elog_pm1_status(uint16_t pm1_sts)
+void elog_pm1_status(const struct acpi_pm_gpe_state *state)
{
+ uint16_t pm1_sts = state->pm1_sts;
+
if (!CONFIG(ELOG))
return;
if (pm1_sts & WAK_STS)
- elog_add_event_byte(ELOG_TYPE_ACPI_WAKE,
- acpi_is_wakeup_s3() ? ACPI_S3 : ACPI_S5);
+ elog_add_event_byte(ELOG_TYPE_ACPI_WAKE, state->previous_sx_state);
if (pm1_sts & PWRBTN_STS)
elog_add_event_wake(ELOG_WAKE_SOURCE_PWRBTN, 0);
diff --git a/src/soc/amd/common/block/include/amdblocks/elog.h b/src/soc/amd/common/block/include/amdblocks/elog.h
index 8b1e8fa..d75a6f0 100644
--- a/src/soc/amd/common/block/include/amdblocks/elog.h
+++ b/src/soc/amd/common/block/include/amdblocks/elog.h
@@ -5,7 +5,7 @@
#include <amdblocks/acpi.h>
-void elog_pm1_status(uint16_t pm1_sts);
+void elog_pm1_status(const struct acpi_pm_gpe_state *state);
void elog_gpe_events(const struct acpi_pm_gpe_state *state);
--
To view, visit https://review.coreboot.org/c/coreboot/+/56362
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I492d2928fa94e5245101a463341d1f170efb5254
Gerrit-Change-Number: 56362
Gerrit-PatchSet: 1
Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: newchange