coreboot-gerrit November 2021

coreboot-gerrit@coreboot.org

1 participants
6394 discussions

Change in coreboot[master]: cbfs: Add helper functions to look up size and type of a file
by Werner Zeh (Code Review) 17 Nov '21

17 Nov '21

Werner Zeh has submitted this change. ( https://review.coreboot.org/c/coreboot/+/59312 ) Change subject: cbfs: Add helper functions to look up size and type of a file ...................................................................... cbfs: Add helper functions to look up size and type of a file This patch adds cbfs_get_size() and cbfs_get_type() helper functions (and _ro_ variations) to look up the size or type of a CBFS file without loading it. Generally, use of these should be discouraged because that tends to mean that the file needs to be looked up more than once, and cbfs_alloc() or cbfs_type_load() are usually the more efficient alternative... but sometimes they're unavoidable, so we might as well offer them. Also remove the <cbfs_private.h> header which had already become sort of unnecessary with previous changes. cbfs_boot_lookup() is now exported in <cbfs.h> for use in inlines, but should not be used directly by other files (and is prefixed with an underscore to highlight that). Signed-off-by: Julius Werner <jwerner(a)chromium.org> Change-Id: I8092d8f6e04bdfb4df6c626dc7d42b402fe0a8ba Reviewed-on: https://review.coreboot.org/c/coreboot/+/59312 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Werner Zeh <werner.zeh(a)siemens.com> Reviewed-by: Jakub Czapiga <jacz(a)semihalf.com> Reviewed-by: Lean Sheng Tan <lean.sheng.tan(a)intel.com> --- M src/include/cbfs.h D src/include/cbfs_private.h M src/lib/cbfs.c 3 files changed, 80 insertions(+), 31 deletions(-) Approvals: build bot (Jenkins): Verified Werner Zeh: Looks good to me, approved Lean Sheng Tan: Looks good to me, approved Jakub Czapiga: Looks good to me, approved diff --git a/src/include/cbfs.h b/src/include/cbfs.h index f6309a3..43d8123 100644 --- a/src/include/cbfs.h +++ b/src/include/cbfs.h @@ -7,6 +7,7 @@ #include <commonlib/bsd/cbfs_mdata.h> #include <commonlib/cbfs.h> #include <commonlib/mem_pool.h> +#include <commonlib/region.h> #include <program_loading.h> #include <types.h> #include <vb2_sha.h> @@ -119,6 +120,20 @@ /* Load stage into memory filling in prog. Return 0 on success. < 0 on error. */ int cbfs_prog_stage_load(struct prog *prog); +/* Returns the size of a CBFS file, or 0 on error. Avoid using this function to allocate space, + and instead use cbfs_alloc() so the file only needs to be looked up once. */ +static inline size_t cbfs_get_size(const char *name); +static inline size_t cbfs_ro_get_size(const char *name); + +/* Returns the type of a CBFS file, or CBFS_TYPE_NULL on error. Use cbfs_type_load() instead of + this where possible to avoid looking up the file more than once. */ +static inline enum cbfs_type cbfs_get_type(const char *name); +static inline enum cbfs_type cbfs_ro_get_type(const char *name); + +/* Check whether a CBFS file exists. */ +static inline bool cbfs_file_exists(const char *name); +static inline bool cbfs_ro_file_exists(const char *name); + /********************************************************************************************** * BOOT DEVICE HELPER APIs * @@ -173,6 +188,9 @@ /********************************************************************************************** * INTERNAL HELPERS FOR INLINES, DO NOT USE. * **********************************************************************************************/ +cb_err_t _cbfs_boot_lookup(const char *name, bool force_ro, + union cbfs_mdata *mdata, struct region_device *rdev); + void *_cbfs_alloc(const char *name, cbfs_allocator_t allocator, void *arg, size_t *size_out, bool force_ro, enum cbfs_type *type); @@ -287,4 +305,58 @@ size_out, type); } +static inline size_t cbfs_get_size(const char *name) +{ + union cbfs_mdata mdata; + struct region_device rdev; + if (_cbfs_boot_lookup(name, false, &mdata, &rdev) != CB_SUCCESS) + return 0; + return be32toh(mdata.h.len); +} + +static inline size_t cbfs_ro_get_size(const char *name) +{ + union cbfs_mdata mdata; + struct region_device rdev; + if (_cbfs_boot_lookup(name, true, &mdata, &rdev) != CB_SUCCESS) + return 0; + return be32toh(mdata.h.len); +} + +static inline enum cbfs_type cbfs_get_type(const char *name) +{ + union cbfs_mdata mdata; + struct region_device rdev; + if (_cbfs_boot_lookup(name, false, &mdata, &rdev) != CB_SUCCESS) + return CBFS_TYPE_NULL; + return be32toh(mdata.h.type); +} + +static inline enum cbfs_type cbfs_ro_get_type(const char *name) +{ + union cbfs_mdata mdata; + struct region_device rdev; + if (_cbfs_boot_lookup(name, true, &mdata, &rdev) != CB_SUCCESS) + return CBFS_TYPE_NULL; + return be32toh(mdata.h.type); +} + +static inline bool cbfs_file_exists(const char *name) +{ + union cbfs_mdata mdata; + struct region_device rdev; + if (_cbfs_boot_lookup(name, false, &mdata, &rdev) != CB_SUCCESS) + return false; + return true; +} + +static inline bool cbfs_ro_file_exists(const char *name) +{ + union cbfs_mdata mdata; + struct region_device rdev; + if (_cbfs_boot_lookup(name, true, &mdata, &rdev) != CB_SUCCESS) + return false; + return true; +} + #endif diff --git a/src/include/cbfs_private.h b/src/include/cbfs_private.h deleted file mode 100644 index 8e98036..0000000 --- a/src/include/cbfs_private.h +++ /dev/null @@ -1,23 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -#ifndef _CBFS_PRIVATE_H_ -#define _CBFS_PRIVATE_H_ - -#include <commonlib/bsd/cbfs_private.h> -#include <commonlib/region.h> -#include <types.h> - -/* - * This header contains low-level CBFS APIs that should only be used by code - * that really needs this level of access. Most code (particularly platform - * code) should use the higher-level CBFS APIs in <cbfs.h>. Code using these - * APIs needs to take special care to ensure CBFS file data is verified (in a - * TOCTOU-safe manner) before access (TODO: add details on how to do this once - * file verification code is in). - */ - -/* Find by name, load metadata into |mdata| and chain file data to |rdev|. */ -cb_err_t cbfs_boot_lookup(const char *name, bool force_ro, - union cbfs_mdata *mdata, struct region_device *rdev); - -#endif diff --git a/src/lib/cbfs.c b/src/lib/cbfs.c index dbb3b1a..1ffc695 100644 --- a/src/lib/cbfs.c +++ b/src/lib/cbfs.c @@ -3,8 +3,8 @@ #include <assert.h> #include <boot_device.h> #include <cbfs.h> -#include <cbfs_private.h> #include <cbmem.h> +#include <commonlib/bsd/cbfs_private.h> #include <commonlib/bsd/compression.h> #include <commonlib/endian.h> #include <console/console.h> @@ -35,8 +35,8 @@ } ROMSTAGE_CBMEM_INIT_HOOK(switch_to_postram_cache); -cb_err_t cbfs_boot_lookup(const char *name, bool force_ro, - union cbfs_mdata *mdata, struct region_device *rdev) +cb_err_t _cbfs_boot_lookup(const char *name, bool force_ro, + union cbfs_mdata *mdata, struct region_device *rdev) { const struct cbfs_boot_device *cbd = cbfs_get_boot_device(force_ro); if (!cbd) @@ -65,7 +65,7 @@ if (CONFIG(VBOOT_ENABLE_CBFS_FALLBACK) && !force_ro && err == CB_CBFS_NOT_FOUND) { printk(BIOS_INFO, "CBFS: Fall back to RO region for %s\n", name); - return cbfs_boot_lookup(name, true, mdata, rdev); + return _cbfs_boot_lookup(name, true, mdata, rdev); } if (err) { if (err == CB_CBFS_NOT_FOUND) @@ -90,7 +90,7 @@ int cbfs_boot_locate(struct cbfsf *fh, const char *name, uint32_t *type) { - if (cbfs_boot_lookup(name, false, &fh->mdata, &fh->data)) + if (_cbfs_boot_lookup(name, false, &fh->mdata, &fh->data)) return -1; size_t msize = be32toh(fh->mdata.h.offset); @@ -330,7 +330,7 @@ DEBUG("%s(name='%s')\n", __func__, name); - if (cbfs_boot_lookup(name, force_ro, &mdata, &rdev)) + if (_cbfs_boot_lookup(name, force_ro, &mdata, &rdev)) return; size = region_device_sz(&rdev); @@ -422,7 +422,7 @@ DEBUG("%s(name='%s', alloc=%p(%p), force_ro=%s, type=%d)\n", __func__, name, allocator, arg, force_ro ? "true" : "false", type ? *type : -1); - if (cbfs_boot_lookup(name, force_ro, &mdata, &rdev)) + if (_cbfs_boot_lookup(name, force_ro, &mdata, &rdev)) return NULL; if (type) { @@ -525,7 +525,7 @@ prog_locate_hook(pstage); - if ((err = cbfs_boot_lookup(prog_name(pstage), false, &mdata, &rdev))) + if ((err = _cbfs_boot_lookup(prog_name(pstage), false, &mdata, &rdev))) return err; assert(be32toh(mdata.h.type) == CBFS_TYPE_STAGE); -- To view, visit https://review.coreboot.org/c/coreboot/+/59312 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8092d8f6e04bdfb4df6c626dc7d42b402fe0a8ba Gerrit-Change-Number: 59312 Gerrit-PatchSet: 2 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Reviewer: Lean Sheng Tan <lean.sheng.tan(a)intel.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in blobs[master]: 3rdparty/blobs/mainboard/teleplatforms/D4E4S16P8: Create new folder f...
by Angel Pons (Code Review) 17 Nov '21

17 Nov '21

Attention is currently required from: Дмитрий Понаморев, Paul Menzel, Kyösti Mälkki. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/blobs/+/57196 ) Change subject: 3rdparty/blobs/mainboard/teleplatforms/D4E4S16P8: Create new folder for teleplatforms/D4E4S16P8 CRB. ...................................................................... Patch Set 4: (1 comment) Commit Message: https://review.coreboot.org/c/blobs/+/57196/comment/c5947321_add74316 PS4, Line 11: > Please mention somewhere, where you got the permissions to distribute the blobs from. […] Blobs must be accompanied by a distribution license, which must allow unlimited redistribution. README.md (inside the root of the blobs repo) contains our binary policy. Also, licenses with clauses like this are frowned upon (at least by me): By loading or using the Software, you agree to the terms of this Agreement. Why? Well, I'm not a lawyer, so I'm probably wrong. As I understand it, if the blobs repo contains a file under such a license (the license is also in the blobs repo), wouldn't someone cloning the blobs repo (which can happen automatically as part of the build process) implicitly and unknowingly agree to the license before being able to read it? Yes, one could read the license before cloning, (e.g. view the license text from https://github.com/coreboot/blobs using a web browser), but it feels excessively forcing. Especially when one is never going to use that binary. -- To view, visit https://review.coreboot.org/c/blobs/+/57196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: blobs Gerrit-Branch: master Gerrit-Change-Id: I71de576aa283fe6738138ad023d70c282045f5cc Gerrit-Change-Number: 57196 Gerrit-PatchSet: 4 Gerrit-Owner: Дмитрий Понаморев <dponamorev(a)gmail.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Дмитрий Понаморев <dponamorev(a)gmail.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Comment-Date: Wed, 17 Nov 2021 12:44:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

Change in blobs[master]: 3rdparty/blobs/mainboard/teleplatforms/D4E4S16P8: Create new folder f...
by Angel Pons (Code Review) 17 Nov '21

17 Nov '21

Attention is currently required from: Дмитрий Понаморев, Kyösti Mälkki. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/blobs/+/57196 ) Change subject: 3rdparty/blobs/mainboard/teleplatforms/D4E4S16P8: Create new folder for teleplatforms/D4E4S16P8 CRB. ...................................................................... Patch Set 4: (1 comment) Patchset: PS4: I see you've included FSP binaries. I suppose they're just the DNV-NS FSP from https://github.com/intel/FSP with UPD settings applied using BCT. This is not how coreboot does it because it's hard to keep track of the settings each board uses. Moreover, if a new FSP version gets released, one needs to transfer the UPD settings from the old FSP binaries to the new ones. The preferred method to set FSP UPDs is to do so at runtime in coreboot code. The values can come from various places, such as: - Kconfig options, e.g. `soc/intel/denverton_ns/romstage.c`: m_cfg->PcdEnableIQAT = CONFIG(IQAT_ENABLE); - Devicetree devices' on/off state, e.g. `soc/intel/skylake/chip.c`: params->PchHdaEnable = is_devfn_enabled(PCH_DEVFN_HDA); - Devicetree chip "registers", e.g. `soc/intel/skylake/chip.c`: params->SsicPortEnable = config->SsicPortEnable; - Mainboard code, e.g. the `mainboard_silicon_init_params()` function. Compared to other Intel FSP platforms, only a few DNV-NS UPDs are hooked up to coreboot code. This is most likely because upstream DNV-NS development is basically non-existent. The only upstream DNV-NS development I'm aware of is scaleway/tagada and your teleplatforms/D4E4S16P8. Exposing FSP UPDs as devicetree "registers" as-is, while common, is subpar: besides the ugly CamelCase names (coreboot uses snake_case), FSP UPDs often depend on other UPDs, can be cumbersome to use (e.g. individual UPDs instead of an array) and can also have other pitfalls. Also, unspecified devicetree "registers" default to 0, which can be a valid value for an UPD but not a reasonable default. For example, a 0 in the `PcieClkSrcUsage` CFL FSP UPD means "clock source used for RP #1", but it would make more sense to default to 0xff aka "clock source is unused". There's some code to remap the values, but it's ugly: CB:49172 touches that code. IMHO, the best way to deal with cumbersome FSP UPDs is to "hide" them behind the devicetree: provide easy-to-use devicetree settings and calculate the corresponding UPD values in SoC code. That being said, I'm aware that implementing this takes a significant amount of time, so I'm fine if you prefer to directly set FSP UPDs from mainboard code. In any case, here's an example of what could be done to improve FSP integration. DNV-NS FSP-S UPDs for PCIe root port settings (de-emphasis, link speed, ASPM, lane reversal) depend on PCIe controller bifurcation settings, which depends on whether the PCIe controllers are enabled. Moreover, all of this depends on FIA MUX configuration done by FSP-M. It doesn't make sense to specify de-emphasis, link speed, ASPM or lane reversal for a disabled and/or unavailable PCIe root port, nor does specifying bifurcation settings for a disabled PCIe controller. Another annoying thing is that FSP UPDs aren't arrays, but it doesn't matter much because it makes more sense to group the root port settings (de-emphasis, link speed, ASPM, lane reversal) on a per-port basis, e.g. using a struct type. Still, the devicetree can use an array of structs to expose these settings (array index would be root port number). Furthermore, coreboot can skip programming the settings for disabled root ports into the FSP UPDs. -- To view, visit https://review.coreboot.org/c/blobs/+/57196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: blobs Gerrit-Branch: master Gerrit-Change-Id: I71de576aa283fe6738138ad023d70c282045f5cc Gerrit-Change-Number: 57196 Gerrit-PatchSet: 4 Gerrit-Owner: Дмитрий Понаморев <dponamorev(a)gmail.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Дмитрий Понаморев <dponamorev(a)gmail.com> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Comment-Date: Wed, 17 Nov 2021 12:21:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/dedede: Create beadrix variant
by Henry Sun (Code Review) 17 Nov '21

17 Nov '21

Attention is currently required from: Raymond Chung, Aseda Aboagye, Paul Fagerburg, Teddy Shih, Karthik Ramasubramanian. Henry Sun has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59302 ) Change subject: mb/google/dedede: Create beadrix variant ...................................................................... Patch Set 1: Code-Review+1 (1 comment) Patchset: PS1: LGTM, differ to Karthik to approve. -- To view, visit https://review.coreboot.org/c/coreboot/+/59302 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie08cbc19967eca8ba31ea3203e71c4e1fef044d6 Gerrit-Change-Number: 59302 Gerrit-PatchSet: 1 Gerrit-Owner: Teddy Shih <teddyshih(a)ami.corp-partner.google.com> Gerrit-Reviewer: Aseda Aboagye <aaboagye(a)google.com> Gerrit-Reviewer: Henry Sun <henrysun(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Paul Fagerburg <pfagerburg(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Raymond Chung <raymondchung(a)ami.corp-partner.google.com> Gerrit-Attention: Raymond Chung <raymondchung(a)ami.corp-partner.google.com> Gerrit-Attention: Aseda Aboagye <aaboagye(a)google.com> Gerrit-Attention: Paul Fagerburg <pfagerburg(a)google.com> Gerrit-Attention: Teddy Shih <teddyshih(a)ami.corp-partner.google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Wed, 17 Nov 2021 12:14:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: ec/starlabs: Add standardised ITE EC support
by build bot (Jenkins) (Code Review) 17 Nov '21

17 Nov '21

Attention is currently required from: Andy Pont, Paul Menzel, Angel Pons, Felix Held. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58343 ) Change subject: ec/starlabs: Add standardised ITE EC support ...................................................................... Patch Set 48: (1 comment) File src/ec/starlabs/merlin/ec.h: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-133634): https://review.coreboot.org/c/coreboot/+/58343/comment/352c2917_dce2aa47 PS48, Line 37: /* EC RAM commmon offsets */ 'commmon' may be misspelled - perhaps 'common'? -- To view, visit https://review.coreboot.org/c/coreboot/+/58343 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8023c26de23c874c84106fda96e64dcfa0c5ba32 Gerrit-Change-Number: 58343 Gerrit-PatchSet: 48 Gerrit-Owner: Sean Rhodes <admin(a)starlabs.systems> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Andy Pont <andy.pont(a)sdcsystems.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Andy Pont <andy.pont(a)sdcsystems.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Wed, 17 Nov 2021 12:07:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/intel/adlrvp: Fix S0ix regression
by Subrata Banik (Code Review) 17 Nov '21

17 Nov '21

Attention is currently required from: Meera Ravindranath. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58178 ) Change subject: mb/intel/adlrvp: Fix S0ix regression ...................................................................... Patch Set 5: (1 comment) Patchset: PS5: > @Meera, this CL caused boot from CPU PCIe failed regression due to CLKSRC_3 is not set to disable. […] I have verified this regression on LP4 ADL-P RVP. Kindly refer to ADL-P RVP HAS it has details why do should keep this RP and CLKSRC enabled. # Enable CPU PCIE RP 2 using CLK 3 register "cpu_pcie_rp[CPU_RP(2)]" = "{ .clk_req = 3, .clk_src = 3, }" Prior to your CL: Clock[3] Usage= 41 Clock[3] ClkReq= 3 With your CL: Clock[3] Usage= FF Clock[3] ClkReq= FF -- To view, visit https://review.coreboot.org/c/coreboot/+/58178 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0b8b76b5527d8b80776cb7588ce6b12281af7882 Gerrit-Change-Number: 58178 Gerrit-PatchSet: 5 Gerrit-Owner: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-Reviewer: Archana Patni <archana.patni(a)intel.com> Gerrit-Reviewer: Baieswara Reddy Sagili <baieswara.reddy.sagili(a)intel.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Ravindra <ravindra(a)intel.com> Gerrit-Reviewer: Sonam Sanju <sonam.sanju(a)intel.com> Gerrit-Reviewer: Usha P <usha.p(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: vagdevi.p(a)intel.com Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com> Gerrit-CC: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-Comment-Date: Wed, 17 Nov 2021 11:48:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subrata.banik(a)intel.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: ec/starlabs: Add standardised ITE EC support
by build bot (Jenkins) (Code Review) 17 Nov '21

17 Nov '21

Attention is currently required from: Andy Pont, Paul Menzel, Angel Pons, Felix Held. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58343 ) Change subject: ec/starlabs: Add standardised ITE EC support ...................................................................... Patch Set 44: (1 comment) File src/ec/starlabs/merlin/ec.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-133627): https://review.coreboot.org/c/coreboot/+/58343/comment/c5f52e15_daf4f0cf PS44, Line 47: static u16 ite_get_chip_id(unsigned int port) open brace '{' following function definitions go on the next line -- To view, visit https://review.coreboot.org/c/coreboot/+/58343 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8023c26de23c874c84106fda96e64dcfa0c5ba32 Gerrit-Change-Number: 58343 Gerrit-PatchSet: 44 Gerrit-Owner: Sean Rhodes <admin(a)starlabs.systems> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Andy Pont <andy.pont(a)sdcsystems.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Andy Pont <andy.pont(a)sdcsystems.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Wed, 17 Nov 2021 11:47:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/intel/adlrvp: Fix S0ix regression
by Subrata Banik (Code Review) 17 Nov '21

17 Nov '21

Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58178 ) Change subject: mb/intel/adlrvp: Fix S0ix regression ...................................................................... Patch Set 5: (1 comment) Patchset: PS5: @Meera, this CL caused boot from CPU PCIe failed regression due to CLKSRC_3 is not set to disable. Please consider to either submit a revert CL or fix it proper. -- To view, visit https://review.coreboot.org/c/coreboot/+/58178 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0b8b76b5527d8b80776cb7588ce6b12281af7882 Gerrit-Change-Number: 58178 Gerrit-PatchSet: 5 Gerrit-Owner: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-Reviewer: Archana Patni <archana.patni(a)intel.com> Gerrit-Reviewer: Baieswara Reddy Sagili <baieswara.reddy.sagili(a)intel.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Ravindra <ravindra(a)intel.com> Gerrit-Reviewer: Sonam Sanju <sonam.sanju(a)intel.com> Gerrit-Reviewer: Usha P <usha.p(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: vagdevi.p(a)intel.com Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com> Gerrit-CC: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Comment-Date: Wed, 17 Nov 2021 11:46:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: ec/starlabs: Add standardised ITE EC support
by Sean Rhodes (Code Review) 17 Nov '21

17 Nov '21

Attention is currently required from: Andy Pont, Paul Menzel, Angel Pons, Felix Held. Sean Rhodes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58343 ) Change subject: ec/starlabs: Add standardised ITE EC support ...................................................................... Patch Set 45: (2 comments) File src/ec/starlabs/merlin/ec.c: https://review.coreboot.org/c/coreboot/+/58343/comment/7b4fde98_30905c62 PS44, Line 14: u16 it_get_version(void) > This file seems to have developed a mismatched use of type definitions i.e we have u16 and uint8_t. […] Done https://review.coreboot.org/c/coreboot/+/58343/comment/558089ae_71e89f25 PS44, Line 47: static u16 ite_get_chip_id(unsigned int port) : { : return (pnp_read_index(port, ITE_CHIPID1) << 8) | : pnp_read_index(port, ITE_CHIPID2); : } : > This probably needs moving up to the top next to the it_get_version() function. Done -- To view, visit https://review.coreboot.org/c/coreboot/+/58343 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8023c26de23c874c84106fda96e64dcfa0c5ba32 Gerrit-Change-Number: 58343 Gerrit-PatchSet: 45 Gerrit-Owner: Sean Rhodes <admin(a)starlabs.systems> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Andy Pont <andy.pont(a)sdcsystems.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Andy Pont <andy.pont(a)sdcsystems.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Wed, 17 Nov 2021 11:07:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Andy Pont <andy.pont(a)sdcsystems.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/cannonlake: Set MAX_CPUS to 16
by Angel Pons (Code Review) 17 Nov '21

17 Nov '21

Attention is currently required from: Felix Singer, Patrick Rudolph, Jeremy Soller, Christian Walter. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52607 ) Change subject: soc/intel/cannonlake: Set MAX_CPUS to 16 ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: Any updates? -- To view, visit https://review.coreboot.org/c/coreboot/+/52607 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib56fdcfe770ef736a2c5e183481d9f9966570e6d Gerrit-Change-Number: 52607 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Jeremy Soller <jeremy(a)system76.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Wed, 17 Nov 2021 11:06:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2021