coreboot-gerrit June 2024

coreboot-gerrit@coreboot.org

1 participants
3527 discussions

[S] Change in coreboot[main]: commonlib/bsd/lz4_wrapper.c: Fix misaligned access
by Angel Pons (Code Review) 21 Jun '24

21 Jun '24

Attention is currently required from: Arthur Heymans, Julius Werner, Maximilian Brune, Nico Huber, Philipp Hug, Ron Minnich. Angel Pons has posted comments on this change by Maximilian Brune. ( https://review.coreboot.org/c/coreboot/+/81910?usp=email ) Change subject: commonlib/bsd/lz4_wrapper.c: Fix misaligned access ...................................................................... Patch Set 5: Code-Review+1 (1 comment) File src/commonlib/bsd/lz4_wrapper.c: https://review.coreboot.org/c/coreboot/+/81910/comment/748ac5cc_339ff531?us… : PS5, Line 14: static uint16_t LZ4_readLE16(const void *src) : { : return *((const uint8_t *)src + 1) << 8 : | *(const uint8_t *)src; : } : : static uint32_t LZ4_readLE32(const void *src) : { : return *((const uint8_t *)src + 3) << 24 : | *((const uint8_t *)src + 2) << 16 : | *((const uint8_t *)src + 1) << 8 : | *(const uint8_t *)src; : } > +1 to Angel's suggested edit Given Nico's tests, I don't think there's any need to overcomplicate this. -- To view, visit https://review.coreboot.org/c/coreboot/+/81910?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: Id165829bfd35be2bce2bbb019c208a304f627add Gerrit-Change-Number: 81910 Gerrit-PatchSet: 5 Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Philipp Hug <philipp(a)hug.cx> Gerrit-Reviewer: Ron Minnich <rminnich(a)gmail.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: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Philipp Hug <philipp(a)hug.cx> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Ron Minnich <rminnich(a)gmail.com> Gerrit-Comment-Date: Sat, 22 Jun 2024 00:03:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>

1 0

[S] Change in coreboot[main]: commonlib/bsd/lz4_wrapper.c: Fix misaligned access
by Julius Werner (Code Review) 21 Jun '24

21 Jun '24

Attention is currently required from: Angel Pons, Arthur Heymans, Maximilian Brune, Nico Huber, Philipp Hug, Ron Minnich. Julius Werner has posted comments on this change by Maximilian Brune. ( https://review.coreboot.org/c/coreboot/+/81910?usp=email ) Change subject: commonlib/bsd/lz4_wrapper.c: Fix misaligned access ...................................................................... Patch Set 5: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/81910/comment/28a16b0c_8d53312c?us… : PS1, Line 18: The coreboot LZ4 decompression code does some misaligned access during : decompression > > So, AIUI, the two implementations produce different machine code, but only for architectures where […] Hmmm... fine. I'm not sure I really feel comfortable about relying on so much compiler magic, but I'll admit I can't find a relevant toolchain version that isn't able to do it. -- To view, visit https://review.coreboot.org/c/coreboot/+/81910?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: Id165829bfd35be2bce2bbb019c208a304f627add Gerrit-Change-Number: 81910 Gerrit-PatchSet: 5 Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Philipp Hug <philipp(a)hug.cx> Gerrit-Reviewer: Ron Minnich <rminnich(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Philipp Hug <philipp(a)hug.cx> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Ron Minnich <rminnich(a)gmail.com> Gerrit-Comment-Date: Fri, 21 Jun 2024 23:41:06 +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: Julius Werner <jwerner(a)chromium.org> Comment-In-Reply-To: Maximilian Brune <maximilian.brune(a)9elements.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>

1 0

[M] Change in coreboot[main]: region: Introduce region_create() functions
by Felix Held (Code Review) 21 Jun '24

21 Jun '24

Attention is currently required from: Andrey Petrov, Felix Singer, Jérémy Compostella, Martin L Roth, Nico Huber, Ronak Kanabar. Felix Held has posted comments on this change by Nico Huber. ( https://review.coreboot.org/c/coreboot/+/79905?usp=email ) The change is no longer submittable: All-Comments-Resolved is unsatisfied now. Change subject: region: Introduce region_create() functions ...................................................................... Patch Set 7: (4 comments) Patchset: PS7: i like this change, but find that the currently rather implicit check of the return value being CB_SUCCESS or something else makes it more difficult to read compared to checking the region_create_untrusted return value against CB_SUCCESS File src/include/cpu/x86/smm.h: https://review.coreboot.org/c/coreboot/+/79905/comment/0cfb6685_c5bbcb6b?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_SUCCESS. if i didn't mess up the logic here, that would be: if (region_create_untrusted(&r, (uintptr_t)ptr, len) != CB_SUCCESS) File util/cbfstool/cbfstool.c: https://review.coreboot.org/c/coreboot/+/79905/comment/1216d010_de3b6225?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 here too File util/cbfstool/cse_serger.c: https://review.coreboot.org/c/coreboot/+/79905/comment/8d007579_42e4d345?us… : PS7, Line 833: if (region_create_untrusted(r, offset, size)) { same here -- 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: 7 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: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Comment-Date: Fri, 21 Jun 2024 22:50:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[XS] Change in coreboot[main]: Makefile.mk: Put site-local path first
by Felix Held (Code Review) 21 Jun '24

21 Jun '24

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/68799?usp=email ) Change subject: Makefile.mk: Put site-local path first ...................................................................... Makefile.mk: Put site-local path first "site-local" Makfile(s) may need to override some of the macros/paths used elsewhere in src/* Makefiles. If we include it last src/* Makefile.mk will have already been processed. MAINBOARD_BLOBS_DIR is an example where the path needs to be overwritten in site-local requiring it to be included first before src/mainboard/* Makefile.mk is processed. Change-Id: I8ea865cd73aba5092a628b0422e5c4121b32fb4d Signed-off-by: Nikolai Vyssotski <nikolai.vyssotski(a)amd.corp-partner.google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/68799 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Felix Held <felix-coreboot(a)felixheld.de> --- M Makefile.mk 1 file changed, 3 insertions(+), 2 deletions(-) Approvals: Felix Held: Looks good to me, approved build bot (Jenkins): Verified diff --git a/Makefile.mk b/Makefile.mk index 3cfd97c..301a3bc 100644 --- a/Makefile.mk +++ b/Makefile.mk @@ -96,6 +96,9 @@ ####################################################################### # root source directories of coreboot +# site-local Makefile.mk must go first to override default locations (for binaries etc.) +subdirs-y := site-local + subdirs-y := src/lib src/commonlib/ src/console src/device src/acpi src/superio/common subdirs-$(CONFIG_EC_ACPI) += src/ec/intel subdirs-y += src/ec/acpi $(wildcard src/ec/*/*) $(wildcard src/southbridge/*/*) @@ -112,8 +115,6 @@ subdirs-y += src/security subdirs-y += payloads payloads/external subdirs-$(CONFIG_SBOM) += src/sbom - -subdirs-y += site-local subdirs-y += util/checklist util/testing ####################################################################### -- To view, visit https://review.coreboot.org/c/coreboot/+/68799?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: I8ea865cd73aba5092a628b0422e5c4121b32fb4d Gerrit-Change-Number: 68799 Gerrit-PatchSet: 6 Gerrit-Owner: Nikolai Vyssotski <nikolai.vyssotski(a)amd.corp-partner.google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>

1 0

[XS] Change in coreboot[main]: Makefile.mk: Put site-local path first
by Felix Held (Code Review) 21 Jun '24

21 Jun '24

Attention is currently required from: Felix Singer, Martin L Roth, Matt DeVillier, Nikolai Vyssotski, Paul Menzel. Felix Held has posted comments on this change by Nikolai Vyssotski. ( https://review.coreboot.org/c/coreboot/+/68799?usp=email ) Change subject: Makefile.mk: Put site-local path first ...................................................................... Patch Set 5: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/68799?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: I8ea865cd73aba5092a628b0422e5c4121b32fb4d Gerrit-Change-Number: 68799 Gerrit-PatchSet: 5 Gerrit-Owner: Nikolai Vyssotski <nikolai.vyssotski(a)amd.corp-partner.google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nikolai Vyssotski <nikolai.vyssotski(a)amd.corp-partner.google.com> Gerrit-Comment-Date: Fri, 21 Jun 2024 20:37:18 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[XS] Change in coreboot[main]: Kconfig: Remove prompt for FW_CONFIG_SOURCE_CHROMEEC_CBI
by Martin Roth (Code Review) 21 Jun '24

21 Jun '24

Attention is currently required from: Julius Werner, Martin L Roth, Nico Huber. Martin Roth has posted comments on this change by Martin L Roth. ( https://review.coreboot.org/c/coreboot/+/83118?usp=email ) Change subject: Kconfig: Remove prompt for FW_CONFIG_SOURCE_CHROMEEC_CBI ...................................................................... Patch Set 1: (1 comment) File src/Kconfig: https://review.coreboot.org/c/coreboot/+/83118/comment/32c931f5_8e34e7c7?us… : PS1, Line 509: bool "Firmware Configuration Probing" > This looks spurious too. […] Agreed. It might be that these were left with prompts so they can be updated with saved config files instead of being selected in the mainboard's Kconfig, or maybe these are just left over from initial testing. -- To view, visit https://review.coreboot.org/c/coreboot/+/83118?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: Icf170dc2ef790d6f5a897a9c7c2ea64033bf1dc9 Gerrit-Change-Number: 83118 Gerrit-PatchSet: 1 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Comment-Date: Fri, 21 Jun 2024 20:03:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>

1 0

[XS] Change in coreboot[main]: libpayload: add cse_*info pointer to lib_sysinfo
by Martin Roth (Code Review) 21 Jun '24

21 Jun '24

Martin Roth has submitted this change. ( https://review.coreboot.org/c/coreboot/+/82790?usp=email ) Change subject: libpayload: add cse_*info pointer to lib_sysinfo ...................................................................... libpayload: add cse_*info pointer to lib_sysinfo Set a pointer in lib_sysinfo for CSE_BP_INFO and CSE_INFO. BUG=b:343022317 TEST=Verified CBMEM data in depthcharge on Screebo Signed-off-by: Eran Mitrani <mitrani(a)google.com> Change-Id: I3aa64d1e439a0596e732a3c0608d60913cefd19f Reviewed-on: https://review.coreboot.org/c/coreboot/+/82790 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Subrata Banik <subratabanik(a)google.com> --- M payloads/libpayload/include/sysinfo.h M payloads/libpayload/libc/coreboot.c 2 files changed, 8 insertions(+), 0 deletions(-) Approvals: Subrata Banik: Looks good to me, approved build bot (Jenkins): Verified diff --git a/payloads/libpayload/include/sysinfo.h b/payloads/libpayload/include/sysinfo.h index d5f5073..cbfc7bf 100644 --- a/payloads/libpayload/include/sysinfo.h +++ b/payloads/libpayload/include/sysinfo.h @@ -114,6 +114,8 @@ uintptr_t acpi_cnvs; uintptr_t acpi_rsdp; uintptr_t smbios; + uintptr_t cse_bp_info; + uintptr_t cse_info; #define UNDEFINED_STRAPPING_ID (~0) #define UNDEFINED_FW_CONFIG ~((uint64_t)0) diff --git a/payloads/libpayload/libc/coreboot.c b/payloads/libpayload/libc/coreboot.c index 3c49484..7873426 100644 --- a/payloads/libpayload/libc/coreboot.c +++ b/payloads/libpayload/libc/coreboot.c @@ -262,6 +262,12 @@ case CBMEM_ID_MEM_CHIP_INFO: info->mem_chip_base = cbmem_entry->address; break; + case CBMEM_ID_CSE_BP_INFO: + info->cse_bp_info = cbmem_entry->address; + break; + case CBMEM_ID_CSE_INFO: + info->cse_info = cbmem_entry->address; + break; default: break; } -- To view, visit https://review.coreboot.org/c/coreboot/+/82790?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: I3aa64d1e439a0596e732a3c0608d60913cefd19f Gerrit-Change-Number: 82790 Gerrit-PatchSet: 10 Gerrit-Owner: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: Pranava Y N <pranavayn(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-CC: Kapil Porwal <kapilporwal(a)google.com>

1 0

[XS] Change in coreboot[main]: coreboot-sdk/Dockerfile: Remove explicit install of 'm4'
by Martin Roth (Code Review) 21 Jun '24

21 Jun '24

Martin Roth has submitted this change. ( https://review.coreboot.org/c/coreboot/+/82512?usp=email ) ( 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: coreboot-sdk/Dockerfile: Remove explicit install of 'm4' ...................................................................... coreboot-sdk/Dockerfile: Remove explicit install of 'm4' Remove m4 as an explicity installed package as it will be installed automatically by flex and bison. Change-Id: Ic4f1c5e6f3324429914bf593047d802dfcc0cb30 Signed-off-by: Elyes Haouas <ehaouas(a)noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/82512 Reviewed-by: Martin L Roth <gaumless(a)gmail.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M util/docker/coreboot-sdk/Dockerfile 1 file changed, 0 insertions(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Martin L Roth: Looks good to me, approved diff --git a/util/docker/coreboot-sdk/Dockerfile b/util/docker/coreboot-sdk/Dockerfile index d6e1e7c..5c4f9c4 100644 --- a/util/docker/coreboot-sdk/Dockerfile +++ b/util/docker/coreboot-sdk/Dockerfile @@ -61,7 +61,6 @@ libusb-1.0-0-dev \ libxml2-dev \ libyaml-dev \ - m4 \ make \ meson \ msitools \ -- To view, visit https://review.coreboot.org/c/coreboot/+/82512?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: Ic4f1c5e6f3324429914bf593047d802dfcc0cb30 Gerrit-Change-Number: 82512 Gerrit-PatchSet: 4 Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

[XS] Change in coreboot[main]: inteltool/lpc.c: Add PCI_DEVICE_ID_INTEL_H67
by Angel Pons (Code Review) 21 Jun '24

21 Jun '24

Attention is currently required from: Elyes Haouas, Pratikkumar V Prajapati. Angel Pons has posted comments on this change by Elyes Haouas. ( https://review.coreboot.org/c/coreboot/+/83110?usp=email ) Change subject: inteltool/lpc.c: Add PCI_DEVICE_ID_INTEL_H67 ...................................................................... Patch Set 1: (4 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/83110/comment/39e2d9e8_d50c2556?us… : PS1, Line 9: bh67bl Did you mean `DH67BL`? https://review.coreboot.org/c/coreboot/+/83110/comment/57566d8d_78ccd60c?us… : PS1, Line 11: ./inteltool -l Do we need the inteltool output in the commit message? File util/inteltool/lpc.c: https://review.coreboot.org/c/coreboot/+/83110/comment/34bd6a41_bd2c8acb?us… : PS1, Line 90: case PCI_DEVICE_ID_INTEL_H67: Why not add at least the rest of CPT IDs? https://review.coreboot.org/c/coreboot/+/83110/comment/07c43960_23ed1512?us… : PS1, Line 120: bc = pci_read_long(dev, SUNRISE_LPC_BC); Does this actually exist on CPT? I would prefer to have a special case for older platforms that only support LPC, just like the rest of the codebase does. -- To view, visit https://review.coreboot.org/c/coreboot/+/83110?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: I142393bd323418b8723d5bb364e919e1ece1e54d Gerrit-Change-Number: 83110 Gerrit-PatchSet: 1 Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Comment-Date: Fri, 21 Jun 2024 18:11:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[L] Change in coreboot[main]: mainboard/dell: Add new mainboard XPS 8300 (Sandy Bridge)
by Felix Singer (Code Review) 21 Jun '24

21 Jun '24

Attention is currently required from: Elyes Haouas, Paul Menzel, Ronald Claveau. Felix Singer has posted comments on this change by Ronald Claveau. ( https://review.coreboot.org/c/coreboot/+/83104?usp=email ) Change subject: mainboard/dell: Add new mainboard XPS 8300 (Sandy Bridge) ...................................................................... Patch Set 12: (1 comment) File src/mainboard/dell/xps_8300/devicetree.cb: https://review.coreboot.org/c/coreboot/+/83104/comment/955322e5_ac773f68?us… : PS12, Line 5: device domain 0x0 on All of the devices use the same subsystem ID. So you can do the following and remove the subsystemid line from the devices. Also, most devices can be reduced to one line then, like shown here. ``` device ref host_bridge on end ``` ```suggestion device domain 0x0 on subsystemid 0x1028 0x04aa inherit ``` -- To view, visit https://review.coreboot.org/c/coreboot/+/83104?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: I7d394794fec580bc7aed3f6396ceb47d4a6fd059 Gerrit-Change-Number: 83104 Gerrit-PatchSet: 12 Gerrit-Owner: Ronald Claveau <sousmangoosta(a)aliel.fr> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Attention: Ronald Claveau <sousmangoosta(a)aliel.fr> Gerrit-Comment-Date: Fri, 21 Jun 2024 18:08:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit June 2024