Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58306 )
Change subject: [WIP] soc/amd/common/block/uart: implement read_resource
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
Furquan: since you worked on the resource allocator some months ago, I wonder if you know if a MMIO device with fixed address should have IORESOURCE_RESERVE set or not? The Intel UART uses that in the ACPI mode case which the AMD SoC UARTs always use, but the existing code for the AMD SoC I2C controllers doesn't set this flag; see the next patch. Also wouldn't it be good if the resource allocator knows all fixed MMIO regions no matter if the device is enabled or not?
--
To view, visit https://review.coreboot.org/c/coreboot/+/58306
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I4ffddee3f5f4281aca98ddfcefa639dfb7a38dae
Gerrit-Change-Number: 58306
Gerrit-PatchSet: 1
Gerrit-Owner: 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-CC: build bot (Jenkins) <no-reply(a)coreboot.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-Comment-Date: Wed, 13 Oct 2021 18:39:38 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/58306 )
Change subject: [WIP] soc/amd/common/block/uart: implement read_resource
......................................................................
[WIP] soc/amd/common/block/uart: implement read_resource
Implement the read_resources function for the UART devices so that the
resource allocator knows about their fixed MMIO resources when enabled.
TODO: find out if we need the IORESOURCE_RESERVE flag and run more tests
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: I4ffddee3f5f4281aca98ddfcefa639dfb7a38dae
---
M src/soc/amd/picasso/uart.c
1 file changed, 11 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/58306/1
diff --git a/src/soc/amd/picasso/uart.c b/src/soc/amd/picasso/uart.c
index b8532aa..6a65048 100644
--- a/src/soc/amd/picasso/uart.c
+++ b/src/soc/amd/picasso/uart.c
@@ -103,8 +103,18 @@
}
}
+static void uart_read_resources(struct device *dev)
+{
+ struct resource *res;
+
+ res = new_resource(dev, 0);
+ res->base = dev->path.mmio.addr;
+ res->size = 4 * KiB;
+ res->flags = IORESOURCE_MEM | IORESOURCE_ASSIGNED | IORESOURCE_FIXED;
+}
+
struct device_operations picasso_uart_mmio_ops = {
- .read_resources = noop_read_resources,
+ .read_resources = uart_read_resources,
.set_resources = noop_set_resources,
.scan_bus = scan_static_bus,
.acpi_name = uart_acpi_name,
--
To view, visit https://review.coreboot.org/c/coreboot/+/58306
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I4ffddee3f5f4281aca98ddfcefa639dfb7a38dae
Gerrit-Change-Number: 58306
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: newchange
Attention is currently required from: Paul Menzel, Nikolai Vyssotski.
Rob Barnes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58282 )
Change subject: mb/google/guybrush: Disable HAVE_ACPI_RESUME / S3
......................................................................
Patch Set 2:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/58282/comment/d53ace80_ab570503
PS1, Line 9: S3 is not currently functional on Guybrush.
> What does that mean exactly? Where does it fail? […]
It's not a regression, S3 support on Cezanne is still under development by AMD. The details are outside the scope of this CL and are detailed in b/181766974. S0i3 is working, so S3 support is low priority.
--
To view, visit https://review.coreboot.org/c/coreboot/+/58282
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9ed3e051f7f2e411670649ac2528a6f40229bdc6
Gerrit-Change-Number: 58282
Gerrit-PatchSet: 2
Gerrit-Owner: Rob Barnes <robbarnes(a)google.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Nikolai Vyssotski <nikolai.vyssotski(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nikolai Vyssotski <nikolai.vyssotski(a)amd.corp-partner.google.com>
Gerrit-Comment-Date: Wed, 13 Oct 2021 18:33:27 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: comment
Attention is currently required from: Nikolai Vyssotski, Rob Barnes.
Hello build bot (Jenkins), Raul Rangel, Nikolai Vyssotski, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/58282
to look at the new patch set (#2).
Change subject: mb/google/guybrush: Disable HAVE_ACPI_RESUME / S3
......................................................................
mb/google/guybrush: Disable HAVE_ACPI_RESUME / S3
S3 is not currently functional on Guybrush. Remove support from ACPI.
BUG=b:202401767 b:181766974
TEST=Boot Guybrush
Confirm 'deep' is not in /sys/power/mem_sleep
Confirm S0ix suspend/resume still works
BRANCH=None
Change-Id: I9ed3e051f7f2e411670649ac2528a6f40229bdc6
Signed-off-by: Rob Barnes <robbarnes(a)google.com>
---
M src/mainboard/google/guybrush/Kconfig
1 file changed, 0 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/82/58282/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/58282
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9ed3e051f7f2e411670649ac2528a6f40229bdc6
Gerrit-Change-Number: 58282
Gerrit-PatchSet: 2
Gerrit-Owner: Rob Barnes <robbarnes(a)google.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Nikolai Vyssotski <nikolai.vyssotski(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nikolai Vyssotski <nikolai.vyssotski(a)amd.corp-partner.google.com>
Gerrit-Attention: Rob Barnes <robbarnes(a)google.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Felix Singer, Jason Glenesk, Raul Rangel, Nico Huber, Furquan Shaikh, Marshall Dawson, Tim Wawrzynczak, Michael Niewöhner, Patrick Rudolph, Felix Held.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58118 )
Change subject: acpigen,soc/amd/cezanne,intel/{common,skl}: rework CPPC table passing
......................................................................
Patch Set 9:
(1 comment)
Patchset:
PS8:
> >> I think it makes sense to set it where the table is set. Also, the three-liner […]
s/An/Another in my previous message then 😊
--
To view, visit https://review.coreboot.org/c/coreboot/+/58118
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I26c5e80c2a16a50ed73245c7c32d61b17e45c22a
Gerrit-Change-Number: 58118
Gerrit-PatchSet: 9
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Lance Zhao
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Wed, 13 Oct 2021 18:22:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Nico Huber, Martin Roth, Angel Pons, Julius Werner.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56410 )
Change subject: [RFC] kconfig_lint: Drop overly restrictive rules about choice configs
......................................................................
Patch Set 5: Code-Review+1
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/56410/comment/e47d1c13_3f2412fb
PS4, Line 19: On top, the linter treats every occurence of a `config` entry as a
: symbol declaration, even when it's just setting a default or adding
: selects.
Martin, I don't see any direct offense in your direction, here. It's just facts and a try to make things better.
> If anyone would like to take a stab at writing a better linter, I'd welcome the effort.
> Again, if the coreboot community doesn't think that linting the Kconfigs is a good idea,
Nobody said that. My parent change - that is merged already - covers parts of the stuff that is dropped here.
> I actually supported the idea of using kconfiglib, which can do what this linter does and more, but that idea was shot down.
Well, push an RFC and let's discuss it. I'd be happy to take a look and would probably even support it. (Even though, I know some people hating python... I don't.)
@ALL Let's come back to a objective discussion on that topic. This is still blocking development of others and we should start to solve that problem. Thank you all.
--
To view, visit https://review.coreboot.org/c/coreboot/+/56410
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I48a17f6403470251be6b6d44bb82a8bdcbefe9f6
Gerrit-Change-Number: 56410
Gerrit-PatchSet: 5
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Martin Roth <martinroth(a)google.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Comment-Date: Wed, 13 Oct 2021 18:21:16 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Martin Roth <martinroth(a)google.com>
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Furquan Shaikh, Tim Wawrzynczak, Michael Niewöhner, Patrick Rudolph.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58211 )
Change subject: soc/intel/common/acpi: drop `RTC_EN` from static wake bits mask
......................................................................
Patch Set 3: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/58211
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ia0ae71f0a472513233bc0fd5625faf15bf86beaf
Gerrit-Change-Number: 58211
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Wed, 13 Oct 2021 18:18:34 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Tim Wawrzynczak, Angel Pons, Patrick Rudolph.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56727 )
Change subject: {adl,tgl}: Change crashlog Kconfig dependencies
......................................................................
Patch Set 5: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/56727
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Idc3ca88a8d4b665a4c954183757542cd9779066d
Gerrit-Change-Number: 56727
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Wed, 13 Oct 2021 18:17:55 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment