coreboot-gerrit March 2024

coreboot-gerrit@coreboot.org

1 participants
3668 discussions

[XS] Change in coreboot[main]: src: Add missing SPDX license headers
by Nicholas Chin (Code Review) 22 Mar '24

22 Mar '24

Nicholas Chin has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/81421?usp=email ) Change subject: src: Add missing SPDX license headers ...................................................................... src: Add missing SPDX license headers Other files in the commits that added these files were licensed under GPL-2.0-only, and the project as a whole is GPL-2.0-only, so use that as the license for these files. Change-Id: I6c1a7ba582f61f98069ebf3857a8b5bdc8588c3e Signed-off-by: Nicholas Chin <nic.c3.14(a)gmail.com> --- M src/mainboard/google/myst/variants/baseboard/include/baseboard/port_descriptors.h M src/mainboard/google/skyrim/variants/baseboard/include/baseboard/port_descriptors.h M src/soc/sifive/fu740/include/soc/gpio.h 3 files changed, 6 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/81421/1 diff --git a/src/mainboard/google/myst/variants/baseboard/include/baseboard/port_descriptors.h b/src/mainboard/google/myst/variants/baseboard/include/baseboard/port_descriptors.h index 7742bed..bc92b69 100644 --- a/src/mainboard/google/myst/variants/baseboard/include/baseboard/port_descriptors.h +++ b/src/mainboard/google/myst/variants/baseboard/include/baseboard/port_descriptors.h @@ -1,3 +1,5 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + #ifndef __BASEBOARD_PORT_DESCRIPTORS_H__ #define __BASEBOARD_PORT_DESCRIPTORS_H__ diff --git a/src/mainboard/google/skyrim/variants/baseboard/include/baseboard/port_descriptors.h b/src/mainboard/google/skyrim/variants/baseboard/include/baseboard/port_descriptors.h index 40da0b4..1315379 100644 --- a/src/mainboard/google/skyrim/variants/baseboard/include/baseboard/port_descriptors.h +++ b/src/mainboard/google/skyrim/variants/baseboard/include/baseboard/port_descriptors.h @@ -1,3 +1,5 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + #ifndef __BASEBOARD_PORT_DESCRIPTORS_H__ #define __BASEBOARD_PORT_DESCRIPTORS_H__ diff --git a/src/soc/sifive/fu740/include/soc/gpio.h b/src/soc/sifive/fu740/include/soc/gpio.h index b0466cc..a4d4ecb 100644 --- a/src/soc/sifive/fu740/include/soc/gpio.h +++ b/src/soc/sifive/fu740/include/soc/gpio.h @@ -1,3 +1,5 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + #ifndef _SOC_SIFIVE_FU740_GPIO_H_ #define _SOC_SIFIVE_FU740_GPIO_H_ -- To view, visit https://review.coreboot.org/c/coreboot/+/81421?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I6c1a7ba582f61f98069ebf3857a8b5bdc8588c3e Gerrit-Change-Number: 81421 Gerrit-PatchSet: 1 Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-MessageType: newchange

1 0

[XL] Change in coreboot[main]: soc/intel/xeon_sp: Add GraniteRapids initial codes
by Nico Huber (Code Review) 22 Mar '24

22 Mar '24

Attention is currently required from: Arthur Heymans, Chen, Gang C, Christian Walter, David Hendricks, Jincheng Li, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Shuo Liu, TangYiwei, Tim Chu. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81316?usp=email ) Change subject: soc/intel/xeon_sp: Add GraniteRapids initial codes ...................................................................... Patch Set 21: (3 comments) File src/soc/intel/xeon_sp/chip_gen6.c: https://review.coreboot.org/c/coreboot/+/81316/comment/1a99123e_0c0217d8 : PS21, Line 34: } Note, you could avoid the late HOB lookup by adding the resources already when the domains are created (this is generally fine when the resources are not read from hardware). Basically what we did with the SPR IOAT domains. Of course, you could also read hardware registers instead, then we wouldn't rely that much on HOB information. https://review.coreboot.org/c/coreboot/+/81316/comment/dc003e92_56272c50 : PS21, Line 57: res->flags = IORESOURCE_SUBTRACTIVE; In hardware, there's virtually always only one subtractive resource of a type. Are you sure about this? it might conflict with the resource added below (line 73). Generally, all the semantic changes here compared to gen1 seem odd to me. Did this really change? Does domain0 always have subtractive resource windows (also mem and prefmem)? https://review.coreboot.org/c/coreboot/+/81316/comment/8c453bcd_bfd2d892 : PS21, Line 119: void iio_pci_domain_read_resources(struct device *dev) Why is this an external API? -- To view, visit https://review.coreboot.org/c/coreboot/+/81316?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I3084e1b5abf25d8d9504bebeaed2a15b916ed56b Gerrit-Change-Number: 81316 Gerrit-PatchSet: 21 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Jincheng Li <jincheng.li(a)intel.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: TangYiwei Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Attention: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Jincheng Li <jincheng.li(a)intel.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: TangYiwei Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Fri, 22 Mar 2024 16:01:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[L] Change in coreboot[main]: soc/intel/xeon_sp: Unshare Xeon-SP chip common codes
by Nico Huber (Code Review) 22 Mar '24

22 Mar '24

Attention is currently required from: Arthur Heymans, Christian Walter, David Hendricks, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Martin L Roth, Patrick Rudolph, Paul Menzel, Shuo Liu, TangYiwei, Tim Chu. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81312?usp=email ) Change subject: soc/intel/xeon_sp: Unshare Xeon-SP chip common codes ...................................................................... Patch Set 14: (1 comment) File src/soc/intel/xeon_sp/chip_common.c: https://review.coreboot.org/c/coreboot/+/81312/comment/69031015_bbfefc72 : PS14, Line 224: if (is_ubox_stack_res(ri)) : soc_create_ubox_domains(dn, root_bus, ri, seg); : else if (CONFIG(SOC_INTEL_HAS_CXL) && is_iio_cxl_stack_res(ri)) : soc_create_cxl_domains(dn, root_bus, ri, seg); : else if (is_pcie_iio_stack_res(ri)) : soc_create_pcie_domains(dn, root_bus, ri, seg); : else if (CONFIG(HAVE_IOAT_DOMAINS) && is_ioat_iio_stack_res(ri)) : soc_create_ioat_domains(dn, root_bus, ri, seg); These `if`s seem unnecessary for gen6. Moving them too, e.g. into a new API soc_create_domains(), could avoid having so many public APIs, and save some boilerplate for gen6. -- To view, visit https://review.coreboot.org/c/coreboot/+/81312?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Iab6acaa5e5c090c8d821bd7c2d3e0e0ad7486bdc Gerrit-Change-Number: 81312 Gerrit-PatchSet: 14 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: TangYiwei Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Martin L Roth <gaumless(a)gmail.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: TangYiwei Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Fri, 22 Mar 2024 15:48:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[L] Change in coreboot[main]: mb/hp: Add Pro 3500 series (Sandy/Ivy Bridge)
by Nicholas Chin (Code Review) 22 Mar '24

22 Mar '24

Attention is currently required from: Joel Linn, Matt DeVillier. Nicholas Chin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81368?usp=email ) Change subject: mb/hp: Add Pro 3500 series (Sandy/Ivy Bridge) ...................................................................... Patch Set 2: (2 comments) File src/mainboard/hp/pro_3500_series/Kconfig: https://review.coreboot.org/c/coreboot/+/81368/comment/e6a8eace_a2c61867 : PS2, Line 37: # FIXME: check this > I unfortunately have no EHCI debugger so I can't test this An EHCI debugger isn't necessary to determine this value, as all it does is indicate which EHCI controller in the chipset has its debug capable port physically routed to an accessible USB port. This can be done using the `util/find_usbdebug/find_usbdebug.sh` script. Basically, you run it, inserting a usb device into a usb port, and pressing any key (other than q for quit) to refresh the output. It will print out any device that it detects which is connected to an EHCI debug capable port. You can then move the device between ports until it shows up in the output, if at all. If it shows up under `PCI device 0000:00:1a.0`, then the index is 2; if it's under `PCI device 0000:00:1d.0`, then it's index 1. This mapping is defined by `src/southbridge/intel/common/usb_debug.c`. On the 6 series chipsets (like what the 3500 has), any USB 1.1 low speed or full speed (generally things like keyboards and mice) or USB 2.0 High Speed devices (generally things like flash drives) should work with the script. Older chipsets couldn't work with USB 1.1 devices since those were handled by a UHCI controller, whereas the 6 series translates USB 1.1 traffic into USB 2.0 traffic and handles everything using the EHCI controller. USB 3.0 devices plugged into a USB 2.0 port should also work since USB 3.0 retains USB 2.0 signals for backwards compatibility. Note that the script doesn't currently work with versions of lsusb 016 and newer, as the output format changed slightly. Feel free to check and fix this if you want, otherwise just mark the Gerrit comment as resolved and note that you haven't checked it. File src/mainboard/hp/pro_3500_series/early_init.c: https://review.coreboot.org/c/coreboot/+/81368/comment/f6e90035_d0057521 : PS2, Line 18: FIXME: Unknown current: RCBA(0x350c)=0x350c > Can you point me in some direction/to some documentation about the usb configuration? `struct southbridge_usb_port` is defined in `sb/intel/bd82x6x/pch.h`: ``` struct southbridge_usb_port { int enabled; int current; int oc_pin; }; ``` In particular, the current member defines an offset in the `currents[]` array in `sb/intel/bd82x6x/early_usb.c`: ``` const u32 currents[] = { USBIR_TXRX_GAIN_MOBILE_LOW, USBIR_TXRX_GAIN_DEFAULT, USBIR_TXRX_GAIN_HIGH, 0x20000f51, 0x2000094a, 0x2000035f, USBIR_TXRX_GAIN_DESKTOP_LOW, 0x20000357, 0x20000353 }; ``` The value that autoport found in the particular RCBA register doesn't match anything in the currents array. You can check the inteltool.log dump that autoport generated, for the actual value of that register. The address is the number inside `RCBA()` in the comment; there should be a section in inteltool.log for RCBA registers. Seems like autoport was intended to include the value after the `=` in the FIXME comment, but seems like that was overlooked and it's just reprinting the address. Unfortunately those current registers don't seem to be documented in the public 6 series chipset datasheets, so coreboot's code is probably all there really is in terms of documentation that you're likely able to obtain. New values seem to just be added to that array as they are seen in logs from vendor firmware, such as commit 216ad2170c (sb/intel/bd82x6x: Add new USB currents) -- To view, visit https://review.coreboot.org/c/coreboot/+/81368?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Idf793fe915096cf2553572964faec5c7f8526b9a Gerrit-Change-Number: 81368 Gerrit-PatchSet: 2 Gerrit-Owner: Joel Linn <jl_coreboot(a)conductive.de> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: Joel Linn <jl_coreboot(a)conductive.de> Gerrit-Comment-Date: Fri, 22 Mar 2024 15:14:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)gmail.com> Comment-In-Reply-To: Joel Linn <jl_coreboot(a)conductive.de> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: [for test] upgrade crossgcc
by Elyes Haouas (Code Review) 22 Mar '24

22 Mar '24

Attention is currently required from: Elyes Haouas, Martin L Roth. Hello Martin L Roth, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/81419?usp=email to look at the new patch set (#3). The following approvals got outdated and were removed: Verified-1 by build bot (Jenkins) Change subject: [for test] upgrade crossgcc ...................................................................... [for test] upgrade crossgcc upgarde LLVM to 18.1.2 & LD upgrade CMake to 3.29.0 upgrade IASL to G20240322 Change-Id: I463c303694c304bb3bf664bc1d914462e7af5dbb Signed-off-by: Elyes Haouas <ehaouas(a)noos.fr> --- M util/crossgcc/buildgcc D util/crossgcc/patches/acpica-R06_28_23_iasl.patch A util/crossgcc/sum/G20240322.tar.gz.cksum D util/crossgcc/sum/R06_28_23.tar.gz.cksum D util/crossgcc/sum/clang-17.0.6.src.tar.xz.cksum A util/crossgcc/sum/clang-18.1.2.src.tar.xz.cksum D util/crossgcc/sum/clang-tools-extra-17.0.6.src.tar.xz.cksum A util/crossgcc/sum/clang-tools-extra-18.1.2.src.tar.xz.cksum D util/crossgcc/sum/cmake-17.0.6.src.tar.xz.cksum A util/crossgcc/sum/cmake-18.1.2.src.tar.xz.cksum D util/crossgcc/sum/cmake-3.28.3.tar.gz.cksum A util/crossgcc/sum/cmake-3.29.0.tar.gz.cksum D util/crossgcc/sum/compiler-rt-17.0.6.src.tar.xz.cksum A util/crossgcc/sum/compiler-rt-18.1.2.src.tar.xz.cksum A util/crossgcc/sum/libunwind-18.1.2.src.tar.xz.cksum A util/crossgcc/sum/lld-18.1.2.src.tar.xz.cksum D util/crossgcc/sum/llvm-17.0.6.src.tar.xz.cksum A util/crossgcc/sum/llvm-18.1.2.src.tar.xz.cksum 18 files changed, 24 insertions(+), 41 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/19/81419/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/81419?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I463c303694c304bb3bf664bc1d914462e7af5dbb Gerrit-Change-Number: 81419 Gerrit-PatchSet: 3 Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-MessageType: newpatchset

1 0

[XS] Change in coreboot[main]: soc/intel/xeon_sp: Include soc_util.h in Xeon-SP common codes
by Nico Huber (Code Review) 22 Mar '24

22 Mar '24

Nico Huber has submitted this change. ( https://review.coreboot.org/c/coreboot/+/81229?usp=email ) ( 6 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: soc/intel/xeon_sp: Include soc_util.h in Xeon-SP common codes ...................................................................... soc/intel/xeon_sp: Include soc_util.h in Xeon-SP common codes Different SoC generations might have different FSP header files. It is recommended to put these uncommon header files in soc_util.h so that Xeon-SP codes refer to soc_util.h to include them in a clean way. TEST=intel/archercity CRB Change-Id: Icfc20921efe00bc69b0c16c665f65f5baae4c309 Signed-off-by: Shuo Liu <shuo.liu(a)intel.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/81229 Reviewed-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/soc/intel/xeon_sp/include/soc/chip_common.h M src/soc/intel/xeon_sp/include/soc/util.h 2 files changed, 2 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Patrick Rudolph: Looks good to me, approved diff --git a/src/soc/intel/xeon_sp/include/soc/chip_common.h b/src/soc/intel/xeon_sp/include/soc/chip_common.h index 2b2d5de..68dc4f9 100644 --- a/src/soc/intel/xeon_sp/include/soc/chip_common.h +++ b/src/soc/intel/xeon_sp/include/soc/chip_common.h @@ -5,7 +5,7 @@ #include <device/device.h> #include <device/path.h> -#include <hob_iiouds.h> +#include <soc/soc_util.h> union xeon_domain_path { unsigned int domain_path; diff --git a/src/soc/intel/xeon_sp/include/soc/util.h b/src/soc/intel/xeon_sp/include/soc/util.h index db66fcb..eff8c08 100644 --- a/src/soc/intel/xeon_sp/include/soc/util.h +++ b/src/soc/intel/xeon_sp/include/soc/util.h @@ -4,7 +4,7 @@ #define _XEON_SP_SOC_UTIL_H_ #include <cpu/x86/msr.h> -#include <hob_iiouds.h> +#include <soc/soc_util.h> #define MEM_ADDR_64MB_SHIFT_BITS 26 -- To view, visit https://review.coreboot.org/c/coreboot/+/81229?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Icfc20921efe00bc69b0c16c665f65f5baae4c309 Gerrit-Change-Number: 81229 Gerrit-PatchSet: 13 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: TangYiwei Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[L] Change in coreboot[main]: soc/intel/xeon_sp: Share DDR codes acorss Xeon-SP platforms
by Nico Huber (Code Review) 22 Mar '24

22 Mar '24

Attention is currently required from: Arthur Heymans, Christian Walter, Jincheng Li, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Shuo Liu, Tim Chu. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81219?usp=email ) The change is no longer submittable: All-Comments-Resolved is unsatisfied now. Change subject: soc/intel/xeon_sp: Share DDR codes acorss Xeon-SP platforms ...................................................................... Patch Set 12: Code-Review+1 (3 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/81219/comment/8828818e_ce2712e5 : PS12, Line 7: acorss ac*ro*ss File src/soc/intel/xeon_sp/ddr.c: https://review.coreboot.org/c/coreboot/+/81219/comment/8f23f83c_9fb49186 : PS12, Line 5: uint32_t get_ddr_voltage(uint8_t DdrVoltage) Not as part of this change, because you just moved it around. But would be nice to align with coreboot coding style. https://review.coreboot.org/c/coreboot/+/81219/comment/246be347_e90600a4 : PS12, Line 107: } Also just moved around. But why is this a function? Such information belongs into the devicetree. Also, do we expect gaps or different amount of slots per channel? otherwise, this could just be a `slot <= CONFIG_DIMM_MAX`. -- To view, visit https://review.coreboot.org/c/coreboot/+/81219?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I237d561003671d70dfaaa9823a0cf16d6e1f50cf Gerrit-Change-Number: 81219 Gerrit-PatchSet: 12 Gerrit-Owner: Jincheng Li <jincheng.li(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Attention: Jincheng Li <jincheng.li(a)intel.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Fri, 22 Mar 2024 14:35:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: soc/intel/xeon_sp: Remove unlock_pam_regions
by Arthur Heymans (Code Review) 22 Mar '24

22 Mar '24

Attention is currently required from: Christian Walter, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Patrick Rudolph, Shuo Liu, Tim Chu. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81349?usp=email ) Change subject: soc/intel/xeon_sp: Remove unlock_pam_regions ...................................................................... Patch Set 4: (1 comment) Patchset: PS4: Why do you want to leave PAM as default? Typically you want those directed to DRAM and not DMI, which is what this does. Coreboot even needs 0xc0000 to point to DRAM to do MP init. -- To view, visit https://review.coreboot.org/c/coreboot/+/81349?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I3fd1d806807449e6a4d9d4d2c8a47ce61ed53018 Gerrit-Change-Number: 81349 Gerrit-PatchSet: 4 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Fri, 22 Mar 2024 14:06:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: vc/intel/fsp/twinlake: Add FspProducerDataHeader.h header
by Felix Held (Code Review) 22 Mar '24

22 Mar '24

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/81037?usp=email ) ( 2 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: vc/intel/fsp/twinlake: Add FspProducerDataHeader.h header ...................................................................... vc/intel/fsp/twinlake: Add FspProducerDataHeader.h header This patch is to add FspProducerDataHeader.h header file to support MRC version Info in TWL. BUG=b:296433836 Change-Id: Ie33c681676d2a699b7aec8185dbdb90555ef8fe2 Signed-off-by: Ronak Kanabar <ronak.kanabar(a)intel.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/81037 Reviewed-by: V Sowmya <v.sowmya(a)intel.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Kapil Porwal <kapilporwal(a)google.com> --- A src/vendorcode/intel/fsp/fsp2_0/twinlake/FspProducerDataHeader.h 1 file changed, 78 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified V Sowmya: Looks good to me, approved Kapil Porwal: Looks good to me, approved diff --git a/src/vendorcode/intel/fsp/fsp2_0/twinlake/FspProducerDataHeader.h b/src/vendorcode/intel/fsp/fsp2_0/twinlake/FspProducerDataHeader.h new file mode 100644 index 0000000..2d75439 --- /dev/null +++ b/src/vendorcode/intel/fsp/fsp2_0/twinlake/FspProducerDataHeader.h @@ -0,0 +1,78 @@ +/** @file + Copyright (c) 2023, Intel Corporation. All rights reserved.<BR> + This program and the accompanying materials + are licensed and made available under the terms and conditions of the BSD License + which accompanies this distribution. The full text of the license may be found at + http://opensource.org/licenses/bsd-license.php + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. +**/ +#ifndef _FSP_PRODUCER_DATA_HEADER_H_ +#define _FSP_PRODUCER_DATA_HEADER_H_ + +#include <Guid/FspHeaderFile.h> + +#define BUILD_TIME_STAMP_SIZE 12 + +// +// FSP Header Data structure from FspHeader driver. +// +#pragma pack(1) +/// +/// FSP Producer Data Subtype - 1 +/// +typedef struct { + /// + /// Byte 0x00: Length of this FSP producer data type record. + /// + UINT16 Length; + /// + /// Byte 0x02: FSP producer data type. + /// + UINT8 Type; + /// + /// Byte 0x03: Revision of this FSP producer data type. + /// + UINT8 Revision; + /// + /// Byte 0x04: 4 byte field of RC version which is used to build this FSP image. + /// + UINT32 RcVersion; + /// + /// Byte 0x08: Represents the build time stamp "YYYYMMDDHHMM". + /// + UINT8 BuildTimeStamp[BUILD_TIME_STAMP_SIZE]; +} FSP_PRODUCER_DATA_TYPE1; + +/// +/// FSP Producer Data Subtype - 2 +/// +typedef struct { + /// + /// Byte 0x00: Length of this FSP producer data type record. + /// + UINT16 Length; + /// + /// Byte 0x02: FSP producer data type. + /// + UINT8 Type; + /// + /// Byte 0x03: Revision of this FSP producer data type. + /// + UINT8 Revision; + /// + /// Byte 0x04: 4 byte field of Mrc version which is used to build this FSP image. + /// + UINT8 MrcVersion [4]; +} FSP_PRODUCER_DATA_TYPE2; + +typedef struct { + FSP_INFO_HEADER FspInfoHeader; + FSP_INFO_EXTENDED_HEADER FspInfoExtendedHeader; + FSP_PRODUCER_DATA_TYPE1 FspProduceDataType1; + FSP_PRODUCER_DATA_TYPE2 FspProduceDataType2; + FSP_PATCH_TABLE FspPatchTable; +} FSP_PRODUCER_DATA_TABLES; +#pragma pack() + +#endif // _FSP_PRODUCER_DATA_HEADER_H -- To view, visit https://review.coreboot.org/c/coreboot/+/81037?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ie33c681676d2a699b7aec8185dbdb90555ef8fe2 Gerrit-Change-Number: 81037 Gerrit-PatchSet: 6 Gerrit-Owner: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[XS] Change in coreboot[main]: soc/intel/alderlake: select UDK_202111_BINDING for ADL-N
by Felix Held (Code Review) 22 Mar '24

22 Mar '24

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/80274?usp=email ) Change subject: soc/intel/alderlake: select UDK_202111_BINDING for ADL-N ...................................................................... soc/intel/alderlake: select UDK_202111_BINDING for ADL-N ADL-N FSP uses 202111 Edk2. select UDK_202111_BINDING Kconfig for ADL-N SoC. BUG=b:296433836 TEST=Able to build and boot google/crassk. Change-Id: If277ede4307515035389cd0e9d34c15cc80f278c Signed-off-by: Ronak Kanabar <ronak.kanabar(a)intel.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/80274 Reviewed-by: Arthur Heymans <arthur(a)aheymans.xyz> Reviewed-by: V Sowmya <v.sowmya(a)intel.com> Reviewed-by: Subrata Banik <subratabanik(a)google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/soc/intel/alderlake/Kconfig 1 file changed, 2 insertions(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Arthur Heymans: Looks good to me, approved V Sowmya: Looks good to me, approved Subrata Banik: Looks good to me, approved diff --git a/src/soc/intel/alderlake/Kconfig b/src/soc/intel/alderlake/Kconfig index 6cc7555..01d5f89 100644 --- a/src/soc/intel/alderlake/Kconfig +++ b/src/soc/intel/alderlake/Kconfig @@ -91,7 +91,8 @@ select SUPPORT_CPU_UCODE_IN_CBFS select TSC_MONOTONIC_TIMER select UDELAY_TSC - select UDK_202005_BINDING + select UDK_202111_BINDING if SOC_INTEL_ALDERLAKE_PCH_N + select UDK_202005_BINDING if !SOC_INTEL_ALDERLAKE_PCH_N select VBOOT_LIB select X86_CLFLUSH_CAR help -- To view, visit https://review.coreboot.org/c/coreboot/+/80274?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: If277ede4307515035389cd0e9d34c15cc80f278c Gerrit-Change-Number: 80274 Gerrit-PatchSet: 9 Gerrit-Owner: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit March 2024