coreboot-gerrit October 2022

coreboot-gerrit@coreboot.org

1 participants
5878 discussions

[S] Change in coreboot[master]: mb/google/dedede/var/beadrix: Update SoC gpio pin of USB camera
by Teddy Shih (Code Review) 06 Oct '22

06 Oct '22

Attention is currently required from: Henry Sun, Super Ni, Simon Yang, Ivan Chen. Teddy Shih has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68129 ) Change subject: mb/google/dedede/var/beadrix: Update SoC gpio pin of USB camera ...................................................................... Patch Set 2: (1 comment) File src/mainboard/google/dedede/variants/beadrix/gpio.c: https://review.coreboot.org/c/coreboot/+/68129/comment/14dc3562_caa775a5 PS2, Line 33: /* D13 : EN_PP2800_CAMERA */ > Could you double check the comment for D13 is match its purpose? Hi Simon, GPP_D13 is tested as expected HIGH. I also sync with ECS Owen with test result are correct as output HIGH with 3.3V. Then, we wait for re-Gerber HW done in the end of October to retest power resume less than 500ms. -- To view, visit https://review.coreboot.org/c/coreboot/+/68129 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id00cb85cdad900c03842ad69707966aa62410efd Gerrit-Change-Number: 68129 Gerrit-PatchSet: 2 Gerrit-Owner: Teddy Shih <teddyshih(a)ami.corp-partner.google.com> Gerrit-Reviewer: Henry Sun <henrysun(a)google.com> Gerrit-Reviewer: Ivan Chen <yulunchen(a)google.com> Gerrit-Reviewer: Simon Yang <simon1.yang(a)intel.corp-partner.google.com> Gerrit-Reviewer: Super Ni <super.ni(a)intel.corp-partner.google.com> Gerrit-Reviewer: Teddy Shih <teddyshih(a)ami.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Eddy Lu <eddylu(a)ami.corp-partner.google.com> Gerrit-CC: Jack Cheng <jack.cheng(a)ecs.corp-partner.google.com> Gerrit-CC: Raymond Chung <raymondchung(a)ami.corp-partner.google.com> Gerrit-CC: Simon Yang <simon1.yang(a)intel.com> Gerrit-Attention: Henry Sun <henrysun(a)google.com> Gerrit-Attention: Super Ni <super.ni(a)intel.corp-partner.google.com> Gerrit-Attention: Simon Yang <simon1.yang(a)intel.com> Gerrit-Attention: Simon Yang <simon1.yang(a)intel.corp-partner.google.com> Gerrit-Attention: Ivan Chen <yulunchen(a)google.com> Gerrit-Comment-Date: Thu, 06 Oct 2022 09:23:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Simon Yang <simon1.yang(a)intel.com> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: mb/google/brya/nivviks: Enable ISH driver and firmware name
by Meera Ravindranath (Code Review) 06 Oct '22

06 Oct '22

Meera Ravindranath has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/68164 ) Change subject: mb/google/brya/nivviks: Enable ISH driver and firmware name ...................................................................... mb/google/brya/nivviks: Enable ISH driver and firmware name BRANCH=none BUG=b:234776154 TEST=build and boot Nirwen UFS, copy ISH firmware to host file system /lib/firmware/intel/adln_ish.bin check "dmesg |grep ish", it should show: ish-loader: ISH firmware intel/adlnrvp_ish.bin loaded Signed-off-by: Meera Ravindranath <meera.ravindranath(a)intel.com> Change-Id: I89782b0b7dde1fca0130472a38628e72dfd5c26c --- M src/drivers/intel/ish/ish.c M src/include/device/pci_ids.h M src/mainboard/google/brya/Kconfig M src/mainboard/google/brya/variants/nivviks/overridetree.cb 4 files changed, 24 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/64/68164/1 diff --git a/src/drivers/intel/ish/ish.c b/src/drivers/intel/ish/ish.c index 78e2f6c..adc0042 100644 --- a/src/drivers/intel/ish/ish.c +++ b/src/drivers/intel/ish/ish.c @@ -56,6 +56,7 @@ PCI_DID_INTEL_CML_ISHB, PCI_DID_INTEL_TGL_ISHB, PCI_DID_INTEL_TGL_H_ISHB, + PCI_DID_INTEL_ADL_ISHB, 0 }; diff --git a/src/include/device/pci_ids.h b/src/include/device/pci_ids.h index 434449c..e2ba3f6 100644 --- a/src/include/device/pci_ids.h +++ b/src/include/device/pci_ids.h @@ -2182,6 +2182,7 @@ #define PCI_DID_INTEL_TGL_ISHB 0xa0fc #define PCI_DID_INTEL_TGL_H_ISHB 0x43fc #define PCI_DID_INTEL_MTL_ISHB 0x7e45 +#define PCI_DID_INTEL_ADL_ISHB 0x54fc /* Intel 82371FB (PIIX) */ #define PCI_DID_INTEL_82371FB_ISA 0x122e diff --git a/src/mainboard/google/brya/Kconfig b/src/mainboard/google/brya/Kconfig index 5934071..1d111d8 100644 --- a/src/mainboard/google/brya/Kconfig +++ b/src/mainboard/google/brya/Kconfig @@ -73,6 +73,7 @@ select SYSTEM_TYPE_LAPTOP select TPM_GOOGLE_TI50 select SOC_INTEL_COMMON_MMC_OVERRIDE + select DRIVERS_INTEL_ISH config BOARD_GOOGLE_BASEBOARD_SKOLAS def_bool n diff --git a/src/mainboard/google/brya/variants/nivviks/overridetree.cb b/src/mainboard/google/brya/variants/nivviks/overridetree.cb index d93095c..ef2d2f4 100644 --- a/src/mainboard/google/brya/variants/nivviks/overridetree.cb +++ b/src/mainboard/google/brya/variants/nivviks/overridetree.cb @@ -519,6 +519,10 @@ probe STORAGE STORAGE_EMMC end device ref ish on + chip drivers/intel/ish + register "firmware_name" = ""adln_ish.bin"" + device generic 0 on end + end probe STORAGE STORAGE_UFS end device ref ufs on -- To view, visit https://review.coreboot.org/c/coreboot/+/68164 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I89782b0b7dde1fca0130472a38628e72dfd5c26c Gerrit-Change-Number: 68164 Gerrit-PatchSet: 1 Gerrit-Owner: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-MessageType: newchange

1 0

[S] Change in coreboot[master]: mb/google/nissa/var/xivu: Change TPM I2C freqeuncy to 1 MHz
by Frank Wu (Code Review) 06 Oct '22

06 Oct '22

Attention is currently required from: Ian Feng, Kangheui Won, Tim Wawrzynczak, Reka Norman, Shou-Chieh Hsu. Frank Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68071 ) Change subject: mb/google/nissa/var/xivu: Change TPM I2C freqeuncy to 1 MHz ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/68071 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I33f712c14978b95f3a4da82d6f1f5fbae1283b17 Gerrit-Change-Number: 68071 Gerrit-PatchSet: 2 Gerrit-Owner: Ian Feng <ian_feng(a)compal.corp-partner.google.com> Gerrit-Reviewer: Frank Wu <frank_wu(a)compal.corp-partner.google.com> Gerrit-Reviewer: John Su <john_su(a)compal.corp-partner.google.com> Gerrit-Reviewer: Kangheui Won <khwon(a)chromium.org> Gerrit-Reviewer: Reka Norman <rekanorman(a)chromium.org> Gerrit-Reviewer: Shou-Chieh Hsu <shouchieh(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Amanda Hwang <amanda_hwang(a)compal.corp-partner.google.com> Gerrit-CC: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com> Gerrit-CC: EricKY Cheng <ericky_cheng(a)compal.corp-partner.google.com> Gerrit-CC: Van Chen <van_chen(a)compal.corp-partner.google.com> Gerrit-Attention: Ian Feng <ian_feng(a)compal.corp-partner.google.com> Gerrit-Attention: Kangheui Won <khwon(a)chromium.org> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Reka Norman <rekanorman(a)chromium.org> Gerrit-Attention: Shou-Chieh Hsu <shouchieh(a)google.com> Gerrit-Comment-Date: Thu, 06 Oct 2022 09:10:45 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[L] Change in coreboot[master]: mb/*/*/*.fmd: Start flash at 0
by Arthur Heymans (Code Review) 06 Oct '22

06 Oct '22

Attention is currently required from: Raul Rangel, Patrick Rudolph, Angel Pons, Jason Glenesk, Nico Huber, Martin L Roth, Subrata Banik, Marshall Dawson, Christian Walter, Tim Wawrzynczak, Alexander Couzens, Fred Reitberger, Felix Held. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63771 ) Change subject: mb/*/*/*.fmd: Start flash at 0 ...................................................................... Patch Set 10: (1 comment) File src/soc/amd/cezanne/Makefile.inc: https://review.coreboot.org/c/coreboot/+/63771/comment/22663ec6_b3a57ec5 PS10, Line 157: APOB_NV_SIZE=$(shell awk '$$2 == "FMAP_SECTION_RW_MRC_CACHE_SIZE" {print $$3}' $(obj)/fmap_config.h) : APOB_NV_BASE=$(shell awk '$$2 == "FMAP_SECTION_RW_MRC_CACHE_START" {print $$3}' $(obj)/fmap_config.h) This needs updating too -- To view, visit https://review.coreboot.org/c/coreboot/+/63771 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iac86ef9be6b14817a65bf3a7ccb624d205ca3f99 Gerrit-Change-Number: 63771 Gerrit-PatchSet: 10 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Erik van den Bogaert <ebogaert(a)eltan.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Stefan Ott <coreboot(a)desire.ch> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Alexander Couzens <lynxis(a)fe80.eu> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Thu, 06 Oct 2022 09:02:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: util/cbfstool: Add a new mechanism to provide a memory mapped
by Arthur Heymans (Code Review) 06 Oct '22

06 Oct '22

Attention is currently required from: Maximilian Brune, Angel Pons. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68160 ) Change subject: util/cbfstool: Add a new mechanism to provide a memory mapped ...................................................................... Patch Set 3: (2 comments) File util/cbfstool/cbfstool.c: https://review.coreboot.org/c/coreboot/+/68160/comment/5d7369d9_0fb052ae PS1, Line 313: // Should be enough for now > Please be consistent with the existing C-style comments. Done https://review.coreboot.org/c/coreboot/+/68160/comment/75b1b170_468762c4 PS1, Line 335: static int decode_mmap_arg(char *optarg) : { : if (optarg == NULL) : return 1; : : unsigned long flash_base; : unsigned long mmap_base; : unsigned long region_size; : char *suffix = NULL; : char *substring = strtok(optarg, ":"); : if (substring) : flash_base = strtol(substring, &suffix, 0); : if (!substring || (suffix && *suffix)) { : ERROR("Invalid flash base address '%s'.\n", : optarg); : return 1; : } : substring = strtok(NULL, ":"); : if (substring) : mmap_base = strtol(substring, &suffix, 0); : if (!substring || (suffix && *suffix)) { : ERROR("Invalid mmap base address '%s'.\n", : optarg); : return 1; : } : substring = strtok(NULL, ":"); : if (substring) : region_size = strtol(substring, &suffix, 0); : if (!substring || (suffix && *suffix)) { : ERROR("Invalid flash region size '%s'.\n", : optarg); : return 1; : } : : if (strtok(NULL, ":") != NULL) { : ERROR("Invalid argument, too many substrings '%s'.\n", : optarg); : : return 1; : } : : add_mmap_window(flash_base, mmap_base, region_size); : return 0; > In this case it doesn't seem like a helper function would actually reduce any code here. […] Done -- To view, visit https://review.coreboot.org/c/coreboot/+/68160 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I38ab4c369704497f711e14ecda3ff3a8cdc0d089 Gerrit-Change-Number: 68160 Gerrit-PatchSet: 3 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Thu, 06 Oct 2022 08:49:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Maximilian Brune <maximilian.brune(a)9elements.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: mb/google/dedede/var/beadrix: Update SoC gpio pin of USB camera
by Simon Yang (Code Review) 06 Oct '22

06 Oct '22

Attention is currently required from: Henry Sun, Super Ni, Simon Yang, Ivan Chen, Teddy Shih. Simon Yang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68129 ) Change subject: mb/google/dedede/var/beadrix: Update SoC gpio pin of USB camera ...................................................................... Patch Set 2: (2 comments) Patchset: PS1: > Hi Simon, […] ODM confirmed the GPP_D13 is set to GPO/high and don't care voltage level. File src/mainboard/google/dedede/variants/beadrix/gpio.c: https://review.coreboot.org/c/coreboot/+/68129/comment/6aa4faa7_e0417361 PS2, Line 33: /* D13 : EN_PP2800_CAMERA */ Could you double check the comment for D13 is match its purpose? -- To view, visit https://review.coreboot.org/c/coreboot/+/68129 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id00cb85cdad900c03842ad69707966aa62410efd Gerrit-Change-Number: 68129 Gerrit-PatchSet: 2 Gerrit-Owner: Teddy Shih <teddyshih(a)ami.corp-partner.google.com> Gerrit-Reviewer: Henry Sun <henrysun(a)google.com> Gerrit-Reviewer: Ivan Chen <yulunchen(a)google.com> Gerrit-Reviewer: Simon Yang <simon1.yang(a)intel.corp-partner.google.com> Gerrit-Reviewer: Super Ni <super.ni(a)intel.corp-partner.google.com> Gerrit-Reviewer: Teddy Shih <teddyshih(a)ami.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Eddy Lu <eddylu(a)ami.corp-partner.google.com> Gerrit-CC: Jack Cheng <jack.cheng(a)ecs.corp-partner.google.com> Gerrit-CC: Raymond Chung <raymondchung(a)ami.corp-partner.google.com> Gerrit-CC: Simon Yang <simon1.yang(a)intel.com> Gerrit-Attention: Henry Sun <henrysun(a)google.com> Gerrit-Attention: Super Ni <super.ni(a)intel.corp-partner.google.com> Gerrit-Attention: Simon Yang <simon1.yang(a)intel.corp-partner.google.com> Gerrit-Attention: Ivan Chen <yulunchen(a)google.com> Gerrit-Attention: Teddy Shih <teddyshih(a)ami.corp-partner.google.com> Gerrit-Comment-Date: Thu, 06 Oct 2022 08:47:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Simon Yang <simon1.yang(a)intel.com> Comment-In-Reply-To: Teddy Shih <teddyshih(a)ami.corp-partner.google.com> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: mb/google/dedede/var/beadrix: Update SoC gpio pin of USB camera
by Teddy Shih (Code Review) 06 Oct '22

06 Oct '22

Attention is currently required from: Henry Sun, Super Ni, Simon Yang, Ivan Chen. Teddy Shih has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68129 ) Change subject: mb/google/dedede/var/beadrix: Update SoC gpio pin of USB camera ...................................................................... Patch Set 2: This change is ready for review. -- To view, visit https://review.coreboot.org/c/coreboot/+/68129 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id00cb85cdad900c03842ad69707966aa62410efd Gerrit-Change-Number: 68129 Gerrit-PatchSet: 2 Gerrit-Owner: Teddy Shih <teddyshih(a)ami.corp-partner.google.com> Gerrit-Reviewer: Henry Sun <henrysun(a)google.com> Gerrit-Reviewer: Ivan Chen <yulunchen(a)google.com> Gerrit-Reviewer: Simon Yang <simon1.yang(a)intel.corp-partner.google.com> Gerrit-Reviewer: Super Ni <super.ni(a)intel.corp-partner.google.com> Gerrit-Reviewer: Teddy Shih <teddyshih(a)ami.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Eddy Lu <eddylu(a)ami.corp-partner.google.com> Gerrit-CC: Jack Cheng <jack.cheng(a)ecs.corp-partner.google.com> Gerrit-CC: Raymond Chung <raymondchung(a)ami.corp-partner.google.com> Gerrit-CC: Simon Yang <simon1.yang(a)intel.com> Gerrit-Attention: Henry Sun <henrysun(a)google.com> Gerrit-Attention: Super Ni <super.ni(a)intel.corp-partner.google.com> Gerrit-Attention: Simon Yang <simon1.yang(a)intel.com> Gerrit-Attention: Simon Yang <simon1.yang(a)intel.corp-partner.google.com> Gerrit-Attention: Ivan Chen <yulunchen(a)google.com> Gerrit-Comment-Date: Thu, 06 Oct 2022 08:46:17 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: util/cbfstool: Add a new mechanism to provide a memory mapped
by Maximilian Brune (Code Review) 06 Oct '22

06 Oct '22

Attention is currently required from: Angel Pons, Arthur Heymans. Maximilian Brune has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68160 ) Change subject: util/cbfstool: Add a new mechanism to provide a memory mapped ...................................................................... Patch Set 1: (1 comment) File util/cbfstool/cbfstool.c: https://review.coreboot.org/c/coreboot/+/68160/comment/27a0bb90_ef092f81 PS1, Line 335: static int decode_mmap_arg(char *optarg) : { : if (optarg == NULL) : return 1; : : unsigned long flash_base; : unsigned long mmap_base; : unsigned long region_size; : char *suffix = NULL; : char *substring = strtok(optarg, ":"); : if (substring) : flash_base = strtol(substring, &suffix, 0); : if (!substring || (suffix && *suffix)) { : ERROR("Invalid flash base address '%s'.\n", : optarg); : return 1; : } : substring = strtok(NULL, ":"); : if (substring) : mmap_base = strtol(substring, &suffix, 0); : if (!substring || (suffix && *suffix)) { : ERROR("Invalid mmap base address '%s'.\n", : optarg); : return 1; : } : substring = strtok(NULL, ":"); : if (substring) : region_size = strtol(substring, &suffix, 0); : if (!substring || (suffix && *suffix)) { : ERROR("Invalid flash region size '%s'.\n", : optarg); : return 1; : } : : if (strtok(NULL, ":") != NULL) { : ERROR("Invalid argument, too many substrings '%s'.\n", : optarg); : : return 1; : } : : add_mmap_window(flash_base, mmap_base, region_size); : return 0; > It doesn't look *too* bad, but please try introducing a helper function that gets called three times […] In this case it doesn't seem like a helper function would actually reduce any code here. But I would recommend to remove the "if (substring)" and just place the "flash_base = ..." after the error checking -- To view, visit https://review.coreboot.org/c/coreboot/+/68160 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I38ab4c369704497f711e14ecda3ff3a8cdc0d089 Gerrit-Change-Number: 68160 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Thu, 06 Oct 2022 08:36:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: util/cbfstool: Add a new mechanism to provide a memory mapped
by build bot (Jenkins) (Code Review) 06 Oct '22

06 Oct '22

Attention is currently required from: Arthur Heymans. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68160 ) Change subject: util/cbfstool: Add a new mechanism to provide a memory mapped ...................................................................... Patch Set 3: (5 comments) File util/cbfstool/cbfstool.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159802): https://review.coreboot.org/c/coreboot/+/68160/comment/65aa434c_0143b876 PS3, Line 393: * Default decode window lives just below 4G boundary in host space and maps up to a line length of 100 exceeds 96 columns Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159802): https://review.coreboot.org/c/coreboot/+/68160/comment/c402a065_6e8731ec PS3, Line 394: * maximum of 16MiB. If the window is smaller than 16MiB, the SPI flash window is mapped line length of 104 exceeds 96 columns Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159802): https://review.coreboot.org/c/coreboot/+/68160/comment/fa58adcd_d585c7d2 PS3, Line 397: add_mmap_window(std_window_flash_offset, DEFAULT_DECODE_WINDOW_TOP - std_window_size, std_window_size); line length of 119 exceeds 96 columns Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159802): https://review.coreboot.org/c/coreboot/+/68160/comment/b066d372_48ccf1c8 PS3, Line 406: ERROR("Flash space windows (base=0x%zx, limit=0x%zx) and (base=0x%zx, limit=0x%zx) overlap!\n", line length of 135 exceeds 96 columns Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-159802): https://review.coreboot.org/c/coreboot/+/68160/comment/a02d025d_4d0ec22d PS3, Line 416: ERROR("Host space windows (base=0x%zx, limit=0x%zx) and (base=0x%zx, limit=0x%zx) overlap!\n", line length of 134 exceeds 96 columns -- To view, visit https://review.coreboot.org/c/coreboot/+/68160 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I38ab4c369704497f711e14ecda3ff3a8cdc0d089 Gerrit-Change-Number: 68160 Gerrit-PatchSet: 3 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Thu, 06 Oct 2022 08:29:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: util/cbfstool: Add a new mechanism to provide a memory mapped
by Arthur Heymans (Code Review) 06 Oct '22

06 Oct '22

Attention is currently required from: Arthur Heymans. Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/68160 to look at the new patch set (#3). Change subject: util/cbfstool: Add a new mechanism to provide a memory mapped ...................................................................... util/cbfstool: Add a new mechanism to provide a memory mapped This replaces the mechanism with --ext-win-base --ext-win-size with a more generic mechanism where cbfstool can be provided with an arbitrary memory map. This will be useful for AMD platforms with flash sizes larger than 16M where only the lower 16M half gets memory mapped below 4G. Also on Intel system the IFD allows for a memory map where the "top of flash" != "below 4G". This is for instance the case by default on Intel APL. TEST: google/brya build for chromeos which used --ext-win-base remains the same after this change with BUILD_TIMELESS=1. Change-Id: I38ab4c369704497f711e14ecda3ff3a8cdc0d089 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/soc/intel/common/block/fast_spi/Makefile.inc M util/cbfstool/cbfstool.c 2 files changed, 139 insertions(+), 77 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/68160/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/68160 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I38ab4c369704497f711e14ecda3ff3a8cdc0d089 Gerrit-Change-Number: 68160 Gerrit-PatchSet: 3 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newpatchset

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit October 2022