Attention is currently required from: Ashish Kumar Mishra, Dinesh Gehlot, Elyes Haouas, Eran Mitrani, Felix Singer, Jakub Czapiga, Jérémy Compostella, Kapil Porwal, Subrata Banik, Tarun.
Saurabh Mishra has posted comments on this change by Saurabh Mishra. ( https://review.coreboot.org/c/coreboot/+/83354?usp=email )
Change subject: soc/intel/ptl: Do initial Panther Lake SoC commit till bootblock
......................................................................
Patch Set 45:
(2 comments)
File src/soc/intel/pantherlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/83354/comment/ca33ad7d_44c930c2?us… :
PS36, Line 133: 10
> as per Intel doc (731941), i read as `6 ports USB 2. […]
Hi, as per EDS v0.7 #815002, chapter 21.0, support for up
to 8 USB 2.0.
I am not able to find the doc "731941".
File src/soc/intel/pantherlake/include/soc/iomap.h:
https://review.coreboot.org/c/coreboot/+/83354/comment/02036545_240de705?us… :
PS32, Line 19: #define SAF_BASE_ADDRESS 0xfa000000
> Sure Subrata, let me check with the team and i will update on the crosbug.
Hi Subrata, due to document v1.1 is not released externally yet, i am working with Intel internal team to make it sharebale over corsbug. Meanwhile, can we add "TO-DO" comment to the SAF Addr, and unblock this patch?
--
To view, visit https://review.coreboot.org/c/coreboot/+/83354?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: Ibcfe71eec27cebf04f10ec343a73dd92f1272aca
Gerrit-Change-Number: 83354
Gerrit-PatchSet: 45
Gerrit-Owner: Saurabh Mishra <mishra.saurabh(a)intel.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tarun <tstuli(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-CC: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com>
Gerrit-CC: Balaji Manigandan <balaji.manigandan(a)intel.com>
Gerrit-CC: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-CC: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-CC: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-CC: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-CC: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com>
Gerrit-CC: Sanju Jose Thottan <sanjujose.thottan(a)intel.com>
Gerrit-CC: Saurabh Mishra <mishra.saurabh(a)intel.corp-partner.google.com>
Gerrit-CC: Vikrant L Jadeja <vikrant.l.jadeja(a)intel.com>
Gerrit-CC: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com>
Gerrit-Attention: Eran Mitrani <mitrani(a)google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Tarun <tstuli(a)gmail.com>
Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Comment-Date: Mon, 05 Aug 2024 13:08:07 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Saurabh Mishra <mishra.saurabh(a)intel.com>
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Attention is currently required from: Karthik Ramasubramanian, Nick Vaccaro.
Sumeet R Pawnikar has posted comments on this change by Karthik Ramasubramanian. ( https://review.coreboot.org/c/coreboot/+/83755?usp=email )
Change subject: mb/google/brox: Tune Touchpad I2C parameters
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/83755?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: I0006bfb9bb5839ffa1248d9f2ea055160ed0936e
Gerrit-Change-Number: 83755
Gerrit-PatchSet: 1
Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Bob Moragues <moragues(a)google.com>
Gerrit-Reviewer: Jon Murphy <jpmurphy(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Comment-Date: Mon, 05 Aug 2024 12:11:46 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Daniel Peng, Derek Huang, Dinesh Gehlot, Eric Lai, Kapil Porwal, Nick Vaccaro, Rishika Raj, Simon Yang, Subrata Banik, Sumeet R Pawnikar.
Hello Daniel Peng, Derek Huang, Dinesh Gehlot, Eric Lai, Kapil Porwal, Nick Vaccaro, Rishika Raj, Subrata Banik, Sumeet R Pawnikar, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/82682?usp=email
to look at the new patch set (#12).
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: soc/intel/alderlake: Add Vccin Aux Imon Iccmax setting
......................................................................
soc/intel/alderlake: Add Vccin Aux Imon Iccmax setting
According to RDC#646929 Power Map, there are two expected values of
VccInAuxImonIccImax and the value has to align with HW design.
But in current code, vccin_aux_imon_iccmax is hard code to 27000 (27A),
hence, provide a config for projects modification.
BUG=b:330117043
BRANCH=firmware-nissa-15217.B
TEST=Modify the register and add a printk to output a debug message
to observe whether the value is changing as expected.
Change-Id: I0651f0eb8a5c32b27c524e43bbf6f2a184b95657
Signed-off-by: Simon Yang <simon1.yang(a)intel.com>
---
M src/soc/intel/alderlake/chip.h
M src/soc/intel/alderlake/fsp_params.c
2 files changed, 10 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/82/82682/12
--
To view, visit https://review.coreboot.org/c/coreboot/+/82682?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I0651f0eb8a5c32b27c524e43bbf6f2a184b95657
Gerrit-Change-Number: 82682
Gerrit-PatchSet: 12
Gerrit-Owner: Simon Yang <simon1.yang(a)intel.com>
Gerrit-Reviewer: Daniel Peng <daniel_peng(a)pegatron.corp-partner.google.com>
Gerrit-Reviewer: Derek Huang <derekhuang(a)google.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Rishika Raj <rishikaraj(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Eric Lai <ericllai(a)google.com>
Gerrit-Attention: Rishika Raj <rishikaraj(a)google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Derek Huang <derekhuang(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: Simon Yang <simon1.yang(a)intel.com>
Gerrit-Attention: Daniel Peng <daniel_peng(a)pegatron.corp-partner.google.com>
Attention is currently required from: Andrey Petrov, Felix Held, Felix Singer, Julius Werner, Jérémy Compostella, Martin L Roth, Maximilian Brune, Ronak Kanabar.
Nico Huber has posted comments on this change by Nico Huber. ( https://review.coreboot.org/c/coreboot/+/79905?usp=email )
Change subject: region: Introduce region_create() functions
......................................................................
Patch Set 8:
(3 comments)
File src/include/cpu/x86/smm.h:
https://review.coreboot.org/c/coreboot/+/79905/comment/111ff428_87e3b7c6?us… :
PS7, Line 147: if (region_create_untrusted(&r, (uintptr_t)ptr, len))
> region_create_untrusted returns enum cb_err, so i'd prefer checking the return value against CB_SUCC […]
Done
File util/cbfstool/cbfstool.c:
https://review.coreboot.org/c/coreboot/+/79905/comment/fd71724f_678d28dc?us… :
PS7, Line 334: if (region_create_untrusted(
> i'd prefer to check the return values for the two region_create_untrusted calls against CB_SUCCESS h […]
Done
Actually, don't like how it looks, but don't know how to do better and
be consistent.
File util/cbfstool/cse_serger.c:
https://review.coreboot.org/c/coreboot/+/79905/comment/a10b0c7e_1ab0a98b?us… :
PS7, Line 833: if (region_create_untrusted(r, offset, size)) {
> same here
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/79905?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: I4ae3e6274c981c9ab4fb1263c2a72fa68ef1c32b
Gerrit-Change-Number: 79905
Gerrit-PatchSet: 8
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Mon, 05 Aug 2024 09:23:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>
Attention is currently required from: Andrey Petrov, Felix Singer, Julius Werner, Jérémy Compostella, Martin L Roth, Maximilian Brune, Nico Huber, Ronak Kanabar.
Hello Andrey Petrov, Julius Werner, Jérémy Compostella, Martin L Roth, Maximilian Brune, Ronak Kanabar, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/79905?usp=email
to look at the new patch set (#8).
The following approvals got outdated and were removed:
Code-Review+2 by Julius Werner, Code-Review+2 by Maximilian Brune, Verified+1 by build bot (Jenkins)
Change subject: region: Introduce region_create() functions
......................................................................
region: Introduce region_create() functions
We introduce two new functions to create region objects. They allow us
to check for integer overflows (region_create_untrusted()) or assert
their absence (region_create()).
This fixes potential overflows in region_overlap() checks in SMI
handlers, where we would wrongfully report MMIO as *not* overlapping
SMRAM.
FIT payload support is left out, as it doesn't use the region API
(only the struct).
Change-Id: I4ae3e6274c981c9ab4fb1263c2a72fa68ef1c32b
Ticket: https://ticket.coreboot.org/issues/522
Found-by: Vadim Zaliva <lord(a)digamma.ai>
Signed-off-by: Nico Huber <nico.h(a)gmx.de>
---
M src/commonlib/include/commonlib/region.h
M src/cpu/x86/smm/smm_module_handler.c
M src/cpu/x86/smm/smm_module_loader.c
M src/drivers/intel/fsp1_1/fsp_report.c
M src/drivers/intel/fsp2_0/fspt_report.c
M src/drivers/spi/spi_flash.c
M src/drivers/spi/winbond.c
M src/include/cpu/x86/smm.h
M src/lib/fmap.c
M src/soc/qualcomm/common/qclib.c
M util/cbfstool/cbfstool.c
M util/cbfstool/cse_serger.c
12 files changed, 95 insertions(+), 75 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/79905/8
--
To view, visit https://review.coreboot.org/c/coreboot/+/79905?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I4ae3e6274c981c9ab4fb1263c2a72fa68ef1c32b
Gerrit-Change-Number: 79905
Gerrit-PatchSet: 8
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Attention is currently required from: Xixi Chen, Yidi Lin, Yu-Ping Wu.
Paul Menzel has posted comments on this change by Xixi Chen. ( https://review.coreboot.org/c/blobs/+/83748?usp=email )
Change subject: soc/mediatek/mt8186: Update DRAM binary from 0.1.0 to 0.1.1
......................................................................
Patch Set 5:
(5 comments)
Commit Message:
https://review.coreboot.org/c/blobs/+/83748/comment/64cfb84f_b72e3205?usp=e… :
PS5, Line 9: readed
read
https://review.coreboot.org/c/blobs/+/83748/comment/7d7700d6_45f57d47?usp=e… :
PS5, Line 12: let
make?
https://review.coreboot.org/c/blobs/+/83748/comment/4cb390e6_e4781fac?usp=e… :
PS5, Line 12: value(from full-k reference)
Please add a space before (.
https://review.coreboot.org/c/blobs/+/83748/comment/54b05dbf_57c0cd9b?usp=e… :
PS5, Line 9: For fast-k RX flow, Vref value is readed from the MRC_CACHE, but the
: preferred RX Vref value 0xE is set, with no re-calibration. But some
: DRAM vendor may use higher RX Vref value, increase the default RX
: Vref value(from full-k reference) to let different DRAM RX Vref
: compatible.
:
Please make use of 72 characters per line.
https://review.coreboot.org/c/blobs/+/83748/comment/d2fe5fb8_2d1142c3?usp=e… :
PS5, Line 16: TEST=Check the fast-k RX Vref value is normal
How? What DRAM vendor/model?
--
To view, visit https://review.coreboot.org/c/blobs/+/83748?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: blobs
Gerrit-Branch: main
Gerrit-Change-Id: Id7df502346d590d3cf3827f48d868da021f6ec9d
Gerrit-Change-Number: 83748
Gerrit-PatchSet: 5
Gerrit-Owner: Xixi Chen <xixi.chen(a)mediatek.corp-partner.google.com>
Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-CC: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com>
Gerrit-CC: Frank Wu <frank_wu(a)compal.corp-partner.google.com>
Gerrit-CC: John Su <john_su(a)compal.corp-partner.google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Xixi Chen <xixi.chen(a)mediatek.corp-partner.google.com>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Attention: Yidi Lin <yidilin(a)google.com>
Gerrit-Comment-Date: Mon, 05 Aug 2024 09:02:36 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Nico Huber has submitted this change. ( https://review.coreboot.org/c/coreboot/+/82404?usp=email )
Change subject: util/autoport: Use sudo to call log-making programs
......................................................................
util/autoport: Use sudo to call log-making programs
Running autoport as root has the annoying side effect of making all
generated files owned by root. Prevent this by using sudo to invoke
log-making programs (lspci, dmidecode, acpidump, inteltool, ectool,
superiotool). These programs either need to be run as root or allow
collecting more information if run as root (lspci).
In case there's a valid reason not to use sudo, provide a prompt to
let autoport run the programs directly, as it originally did. There
might be someone trying to run autoport from an OS that lacks sudo.
Change-Id: I4bf4ddf8dd2cb930e9b7303e2ea986d8c072aa7a
Signed-off-by: Angel Pons <th3fanbus(a)gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/82404
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Nico Huber <nico.h(a)gmx.de>
---
M util/autoport/log_maker.go
1 file changed, 19 insertions(+), 5 deletions(-)
Approvals:
build bot (Jenkins): Verified
Nico Huber: Looks good to me, approved
diff --git a/util/autoport/log_maker.go b/util/autoport/log_maker.go
index ed157c7..6183fb4d 100644
--- a/util/autoport/log_maker.go
+++ b/util/autoport/log_maker.go
@@ -18,14 +18,22 @@
args []string
}
-func (prog LogMakingProgram) TryRunAndSave(output string, prefix string) error {
+func ExecCommand(sudo bool, name string, arg []string) *exec.Cmd {
+ if sudo {
+ return exec.Command("sudo", append([]string{name}, arg...)...)
+ } else {
+ return exec.Command(name, arg...)
+ }
+}
+
+func (prog LogMakingProgram) TryRunAndSave(output string, sudo bool, prefix string) error {
f, err := os.Create(output)
if err != nil {
log.Fatal(err)
}
defer f.Close()
- cmd := exec.Command(prefix+prog.name, prog.args...)
+ cmd := ExecCommand(sudo, prefix+prog.name, prog.args)
cmd.Stdout = f
cmd.Stderr = f
@@ -36,7 +44,7 @@
return cmd.Wait()
}
-func (prog LogMakingProgram) RunAndSave(outDir string) {
+func (prog LogMakingProgram) RunAndSave(outDir string, sudo bool) {
output := fmt.Sprintf("%s/%s.log", outDir, prog.name)
cmdline := strings.Join(append([]string{prog.name}, prog.args...), " ")
@@ -44,7 +52,7 @@
var sb strings.Builder
for _, prefix := range prog.prefixes {
- err := prog.TryRunAndSave(output, prefix)
+ err := prog.TryRunAndSave(output, sudo, prefix)
if err == nil {
return
}
@@ -157,6 +165,12 @@
func MakeLogs(outDir string) {
os.MkdirAll(outDir, 0700)
+ sudo := PromptUserBool("Should autoport use sudo to run the commands to make the logs? "+
+ "This is recommended over running autoport as root, since the generated files "+
+ "won't be owned by root. If running as root already because sudo isn't available, "+
+ "choose 'no'. Otherwise, run autoport as a regular (non-root) user and choose 'yes'.",
+ true)
+
probeGFX := PromptUserBool("WARNING: Running inteltool MAY cause your system to hang when it attempts "+
"to probe for graphics registers. Having the graphics registers will help create a better port. "+
"Should autoport probe these registers?",
@@ -202,7 +216,7 @@
fmt.Println("Making logs...")
for _, prog := range programs {
- prog.RunAndSave(outDir)
+ prog.RunAndSave(outDir, sudo)
}
SysSound := "/sys/class/sound/"
--
To view, visit https://review.coreboot.org/c/coreboot/+/82404?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: I4bf4ddf8dd2cb930e9b7303e2ea986d8c072aa7a
Gerrit-Change-Number: 82404
Gerrit-PatchSet: 6
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Nico Huber has submitted this change. ( https://review.coreboot.org/c/coreboot/+/82403?usp=email )
Change subject: util/autoport: Streamline external program invocation
......................................................................
util/autoport: Streamline external program invocation
The original approach to call external programs was rather convoluted
and would fall back to running executables inside the current working
directory if running them from the location specified in the code did
not succeed, swallowing any errors from the first invocation.
Rewrite the system around the `LogMakingProgram` concept, a struct to
represent a program. Each program has a name, prefixes to try running
it from and the arguments to pass to it (if any). Plus, collect error
information from failed executions, but only show it when none of the
prefixes resulted in a successful invocation.
In addition, look for programs in PATH instead of CWD: it is unlikely
that all utils will be in the CWD, but utils can be in the PATH after
one installs them (`sudo make install`). For coreboot utils, look for
them in the utils folder first as the installed versions might not be
up-to-date.
Furthermore, print out the command about to be executed, as there are
some commands (e.g. `ectool` on boards without an EC) that can take a
very long time to complete.
Change-Id: I144bdf609e0aebd8f6ddebc0eb1216bedebfa313
Signed-off-by: Angel Pons <th3fanbus(a)gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/82403
Reviewed-by: Nico Huber <nico.h(a)gmx.de>
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
---
M util/autoport/log_maker.go
1 file changed, 72 insertions(+), 25 deletions(-)
Approvals:
build bot (Jenkins): Verified
Nico Huber: Looks good to me, approved
diff --git a/util/autoport/log_maker.go b/util/autoport/log_maker.go
index 5ca8284..ed157c7 100644
--- a/util/autoport/log_maker.go
+++ b/util/autoport/log_maker.go
@@ -12,14 +12,20 @@
"bytes"
)
-func TryRunAndSave(output string, name string, arg []string) error {
- cmd := exec.Command(name, arg...)
+type LogMakingProgram struct {
+ name string
+ prefixes []string
+ args []string
+}
+func (prog LogMakingProgram) TryRunAndSave(output string, prefix string) error {
f, err := os.Create(output)
if err != nil {
log.Fatal(err)
}
+ defer f.Close()
+ cmd := exec.Command(prefix+prog.name, prog.args...)
cmd.Stdout = f
cmd.Stderr = f
@@ -27,25 +33,35 @@
if err != nil {
return err
}
- cmd.Wait()
- return nil
+ return cmd.Wait()
}
-func RunAndSave(output string, name string, arg ...string) {
- err := TryRunAndSave(output, name, arg)
- if err == nil {
- return
+func (prog LogMakingProgram) RunAndSave(outDir string) {
+ output := fmt.Sprintf("%s/%s.log", outDir, prog.name)
+ cmdline := strings.Join(append([]string{prog.name}, prog.args...), " ")
+
+ fmt.Println("Running: "+cmdline)
+
+ var sb strings.Builder
+ for _, prefix := range prog.prefixes {
+ err := prog.TryRunAndSave(output, prefix)
+ if err == nil {
+ return
+ }
+ sb.WriteString("\nError running '"+prefix+cmdline+"': "+err.Error()+"\n")
+ data, ferr := os.ReadFile(output)
+ if ferr != nil {
+ sb.WriteString("<failed to open log>\n")
+ } else {
+ if len(data) > 0 {
+ sb.WriteString("Program output:\n\n")
+ sb.WriteString(string(data))
+ }
+ }
}
- idx := strings.LastIndex(name, "/")
- relname := name
- if idx >= 0 {
- relname = name[idx+1:]
- }
- relname = "./" + relname
- err = TryRunAndSave(output, relname, arg)
- if err != nil {
- log.Fatal(err)
- }
+
+ fmt.Println("\nCould not run program: '"+cmdline+"'")
+ log.Fatal(sb.String())
}
const MAXPROMPTRETRY = 3
@@ -140,11 +156,8 @@
func MakeLogs(outDir string) {
os.MkdirAll(outDir, 0700)
- RunAndSave(outDir+"/lspci.log", "lspci", "-nnvvvxxxx")
- RunAndSave(outDir+"/dmidecode.log", "dmidecode")
- RunAndSave(outDir+"/acpidump.log", "acpidump")
- probeGFX := PromptUserBool("WARNING: The following tool MAY cause your system to hang when it attempts "+
+ probeGFX := PromptUserBool("WARNING: Running inteltool MAY cause your system to hang when it attempts "+
"to probe for graphics registers. Having the graphics registers will help create a better port. "+
"Should autoport probe these registers?",
true)
@@ -154,9 +167,43 @@
inteltoolArgs += "f"
}
- RunAndSave(outDir+"/inteltool.log", "../inteltool/inteltool", inteltoolArgs)
- RunAndSave(outDir+"/ectool.log", "../ectool/ectool", "-pd")
- RunAndSave(outDir+"/superiotool.log", "../superiotool/superiotool", "-ade")
+ var programs = []LogMakingProgram {
+ LogMakingProgram {
+ name: "lspci",
+ prefixes: []string{""},
+ args: []string{"-nnvvvxxxx"},
+ },
+ LogMakingProgram {
+ name: "dmidecode",
+ prefixes: []string{""},
+ args: []string{},
+ },
+ LogMakingProgram {
+ name: "acpidump",
+ prefixes: []string{""},
+ args: []string{},
+ },
+ LogMakingProgram {
+ name: "inteltool",
+ prefixes: []string{"../inteltool/", ""},
+ args: []string{inteltoolArgs},
+ },
+ LogMakingProgram {
+ name: "ectool",
+ prefixes: []string{"../ectool/", ""},
+ args: []string{"-pd"},
+ },
+ LogMakingProgram {
+ name: "superiotool",
+ prefixes: []string{"../superiotool/", ""},
+ args: []string{"-ade"},
+ },
+ }
+
+ fmt.Println("Making logs...")
+ for _, prog := range programs {
+ prog.RunAndSave(outDir)
+ }
SysSound := "/sys/class/sound/"
card := ""
--
To view, visit https://review.coreboot.org/c/coreboot/+/82403?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: I144bdf609e0aebd8f6ddebc0eb1216bedebfa313
Gerrit-Change-Number: 82403
Gerrit-PatchSet: 7
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Attention is currently required from: Dinesh Gehlot, Kapil Porwal, Pranava Y N.
Subrata Banik has posted comments on this change by Pranava Y N. ( https://review.coreboot.org/c/coreboot/+/83767?usp=email )
Change subject: MAINTAINERS: Add google/fatcat entry and update maintainers
......................................................................
Patch Set 1: Code-Review+2
(1 comment)
Patchset:
PS1:
can you please submit one more CL for PTL SoC ?
```
INTEL METEORLAKE SOC
M: Subrata Banik <subratabanik(a)google.com>
M: Kapil Porwal <kapilporwal(a)google.com>
M: Pranava Y N <pranavayn(a)google.com>
S: Maintained
F: src/soc/intel/pantherlake/
```
--
To view, visit https://review.coreboot.org/c/coreboot/+/83767?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: I5ae0f0d24d43e91c2097c68446bb64b9ae507e2e
Gerrit-Change-Number: 83767
Gerrit-PatchSet: 1
Gerrit-Owner: Pranava Y N <pranavayn(a)google.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Pranava Y N <pranavayn(a)google.com>
Gerrit-Comment-Date: Mon, 05 Aug 2024 08:54:57 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes