Attention is currently required from: Maximilian Brune.
Felix Singer has posted comments on this change by Maximilian Brune. ( https://review.coreboot.org/c/coreboot/+/83313?usp=email )
Change subject: Makefile.mk: Remove bc dependency
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/83313/comment/04c8128c_4b80f5bb?us… :
PS1, Line 9: commit:
: 229e021110: Makefile.inc: Add left shift macro
Please rewrite to this, so that Gerrit makes it a link. Also makes the link in line 17 superfluous. I would just add that to this one.
```
commit 229e021110 ("Makefile.inc: Add left shift macro")
```
--
To view, visit https://review.coreboot.org/c/coreboot/+/83313?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I6ab4bc2bd7a45e84b923d4fe7ec473e6c7db2146
Gerrit-Change-Number: 83313
Gerrit-PatchSet: 1
Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Comment-Date: Fri, 05 Jul 2024 22:39:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Attention is currently required from: Felix Singer, Nick Vaccaro.
Matt DeVillier has posted comments on this change by Felix Singer. ( https://review.coreboot.org/c/coreboot/+/83253?usp=email )
Change subject: tgl mainboards: Move PCIe root port settings into their device scope
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/83253?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I110cc95d536cb0fd3b5db85b84cca7a96e31401c
Gerrit-Change-Number: 83253
Gerrit-PatchSet: 5
Gerrit-Owner: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Comment-Date: Fri, 05 Jul 2024 20:49:55 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Felix Singer has submitted this change. ( https://review.coreboot.org/c/coreboot/+/83344?usp=email )
Change subject: autoport: Print location of generated sources
......................................................................
autoport: Print location of generated sources
Autoport determines the mainboard vendor and board names based on DMI
entries, which sometimes doesn't result in the most obvious name. In
addition, newcomers may not be familiar with coreboot's directory
structure and have no idea where to look. Print out the absolute patch
of the generated sources once autoport finishes so that it is easier to
locate the files.
Change-Id: I4ba00484ac57355d7539fa6e36e0e6df62719f8a
Signed-off-by: Nicholas Chin <nic.c3.14(a)gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/83344
Reviewed-by: Angel Pons <th3fanbus(a)gmail.com>
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
---
M util/autoport/main.go
1 file changed, 3 insertions(+), 0 deletions(-)
Approvals:
build bot (Jenkins): Verified
Angel Pons: Looks good to me, approved
Felix Singer: Looks good to me, approved
diff --git a/util/autoport/main.go b/util/autoport/main.go
index b7130b2..18fb694 100644
--- a/util/autoport/main.go
+++ b/util/autoport/main.go
@@ -8,6 +8,7 @@
"fmt"
"log"
"os"
+ "path/filepath"
"sort"
"strings"
)
@@ -903,4 +904,6 @@
end GMA.Mainboard;
`)
}
+ outputPath, _ := filepath.Abs(ctx.BaseDirectory)
+ fmt.Printf("Done! Generated sources are in %s\n", outputPath)
}
--
To view, visit https://review.coreboot.org/c/coreboot/+/83344?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I4ba00484ac57355d7539fa6e36e0e6df62719f8a
Gerrit-Change-Number: 83344
Gerrit-PatchSet: 2
Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Attention is currently required from: Nicholas Chin.
Felix Singer has posted comments on this change by Nicholas Chin. ( https://review.coreboot.org/c/coreboot/+/83344?usp=email )
Change subject: autoport: Print location of generated sources
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/83344?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I4ba00484ac57355d7539fa6e36e0e6df62719f8a
Gerrit-Change-Number: 83344
Gerrit-PatchSet: 1
Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Comment-Date: Fri, 05 Jul 2024 19:48:51 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Alexander Couzens, David Hendricks, Evgeny Zinoviev, Felix Held, Jeremy Soller, Jonathon Hall, Martin L Roth, Michael Niewöhner, Michał Żygowski, Philipp Hug, Piotr Król, Ron Minnich, Sean Rhodes, Tim Crawford.
Felix Singer has posted comments on this change by Martin L Roth. ( https://review.coreboot.org/c/coreboot/+/83119?usp=email )
Change subject: mb: source site-local mainboard Kconfig.name and Kconfig files
......................................................................
Patch Set 2: Code-Review-1
(1 comment)
Patchset:
PS2:
> I don't think we allow symlinks to be checked into the coreboot repo, though I do see some in submod […]
I wasn't saying that symlinks should be checked in the coreboot repo, but this is what the symlink target from the Makefile does, as far as I can tell. I don't know how it works, but I assume this is only done locally when the user intends to use site-local. So, that's why I've asked why that doesn't work anymore.
Are any more changes expected or even planned in order to integrate downstream code into what's available on the main branch? I feel this is heavily working around git branches, which also should work on Windows.
I understand that people from here intend to upstream their changes at some point, but I feel this might encourage others to not upstream changes since it becomes more comfortable to work downstream. Well, here it's "just"(?) mainboards from site-local which are integrated into the upstream infrastructure. I dislike the fact that upstream should provide infrastructure for whatever is done downstream somewhere. git branches seem more appropriate to me. That's why I give a -1 to this.
As an alternative to this, but not sure if this makes it better or worse or no change at all, I'm thinking of a dedicated site-local mainboard vendor directory, which hooks up mainboards from site-local. It still does the same thing as what is proposed here, but then it's limited to that directory and not applied to the general infrastructure, which might be less encouraging to keep changes downstream.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83119?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I4106fea7197c979e6648ebbbbaa107070c916727
Gerrit-Change-Number: 83119
Gerrit-PatchSet: 2
Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Evgeny Zinoviev <me(a)ch1p.io>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Reviewer: Jonathon Hall <jonathon.hall(a)puri.sm>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Philipp Hug <philipp(a)hug.cx>
Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com>
Gerrit-Reviewer: Ron Minnich <rminnich(a)gmail.com>
Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Varshit Pandya <pandyavarshit(a)gmail.com>
Gerrit-Attention: Philipp Hug <philipp(a)hug.cx>
Gerrit-Attention: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Attention: Jonathon Hall <jonathon.hall(a)puri.sm>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Attention: Piotr Król <piotr.krol(a)3mdeb.com>
Gerrit-Attention: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Attention: Alexander Couzens <lynxis(a)fe80.eu>
Gerrit-Attention: Evgeny Zinoviev <me(a)ch1p.io>
Gerrit-Attention: Ron Minnich <rminnich(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Fri, 05 Jul 2024 19:23:27 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Comment-In-Reply-To: Martin L Roth <gaumless(a)gmail.com>
Comment-In-Reply-To: David Hendricks <david.hendricks(a)gmail.com>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Comment-In-Reply-To: Jonathon Hall <jonathon.hall(a)puri.sm>
Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de>
Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>
Attention is currently required from: Jakub Czapiga, Jan Dabros, Martin L Roth.
Nico Huber has posted comments on this change by Jakub Czapiga. ( https://review.coreboot.org/c/coreboot/+/70701?usp=email )
Change subject: util/sconfig: Allow to specify multiple chip base paths
......................................................................
Patch Set 8:
(1 comment)
Patchset:
PS8:
Not sure if I fully understood the problem. In case this is still an issue:
We used to have macro checks in header files to exclude certain things for
specific languages (C / assembler / ASL). Maybe worth looking into.
Also, if FSP etc. headers are leaking into general coreboot code, that
should probably be avoided locally.
--
To view, visit https://review.coreboot.org/c/coreboot/+/70701?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I2be9d6bebd271f32effd1b14e6edcf47664f30a5
Gerrit-Change-Number: 70701
Gerrit-PatchSet: 8
Gerrit-Owner: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Jan Dabros <jsd(a)semihalf.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Attention: Jan Dabros <jsd(a)semihalf.com>
Gerrit-Comment-Date: Fri, 05 Jul 2024 18:31:47 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Jakub Czapiga, Martin L Roth.
Nico Huber has posted comments on this change by Jakub Czapiga. ( https://review.coreboot.org/c/coreboot/+/70526?usp=email )
Change subject: util/sconfig: Simplify file opening and error handling
......................................................................
Patch Set 5:
(2 comments)
File util/sconfig/main.c:
https://review.coreboot.org/c/coreboot/+/70526/comment/5049bcd5_05a9b6f0?us… :
PS5, Line 2137: goto end;
Not sure why the compiler doesn't warn here. AIUI, this skips the initialization
of the other variables, hence we can't count on them being NULL. Individual jump
labels are the canonical way, I guess. i.e. `fclose()` in the reverse order and
let the first `goto` jump to the last `fclose()`, etc.
https://review.coreboot.org/c/coreboot/+/70526/comment/2abc8a9e_f7956ded?us… :
PS5, Line 2183: return 0;
We shouldn't `return 0` on failure.
--
To view, visit https://review.coreboot.org/c/coreboot/+/70526?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ib315b50cdc89ace414ce0b78d3ece02e81b028ad
Gerrit-Change-Number: 70526
Gerrit-PatchSet: 5
Gerrit-Owner: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Jan Dabros <jsd(a)semihalf.com>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Comment-Date: Fri, 05 Jul 2024 18:22:31 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Jakub Czapiga, Martin L Roth.
Nico Huber has posted comments on this change by Jakub Czapiga. ( https://review.coreboot.org/c/coreboot/+/70525?usp=email )
Change subject: util/sconfig: Remove unnecessary strdup() calls
......................................................................
Patch Set 5: Code-Review+2
(1 comment)
Patchset:
PS5:
Sorry, looks like I missed the call back then. Looks good.
--
To view, visit https://review.coreboot.org/c/coreboot/+/70525?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ie5a27f64077af1c04b06732cd601145b8becacfd
Gerrit-Change-Number: 70525
Gerrit-PatchSet: 5
Gerrit-Owner: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Jan Dabros <jsd(a)semihalf.com>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Comment-Date: Fri, 05 Jul 2024 18:15:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Attention is currently required from: Alexander Couzens, Evgeny Zinoviev, Felix Held, Felix Singer, Jeremy Soller, Martin L Roth, Michael Niewöhner, Michał Żygowski, Philipp Hug, Piotr Król, Ron Minnich, Sean Rhodes, Tim Crawford.
Jonathon Hall has posted comments on this change by Martin L Roth. ( https://review.coreboot.org/c/coreboot/+/83119?usp=email )
Change subject: mb: source site-local mainboard Kconfig.name and Kconfig files
......................................................................
Patch Set 2: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/83119?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I4106fea7197c979e6648ebbbbaa107070c916727
Gerrit-Change-Number: 83119
Gerrit-PatchSet: 2
Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Evgeny Zinoviev <me(a)ch1p.io>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Reviewer: Jonathon Hall <jonathon.hall(a)puri.sm>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Philipp Hug <philipp(a)hug.cx>
Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com>
Gerrit-Reviewer: Ron Minnich <rminnich(a)gmail.com>
Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Varshit Pandya <pandyavarshit(a)gmail.com>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Philipp Hug <philipp(a)hug.cx>
Gerrit-Attention: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Attention: Piotr Król <piotr.krol(a)3mdeb.com>
Gerrit-Attention: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Alexander Couzens <lynxis(a)fe80.eu>
Gerrit-Attention: Evgeny Zinoviev <me(a)ch1p.io>
Gerrit-Attention: Ron Minnich <rminnich(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Fri, 05 Jul 2024 17:57:55 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes