Attention is currently required from: Kapil Porwal, Nick Vaccaro, sridhar siricilla.
Hello Kapil Porwal, Nick Vaccaro, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/80362?usp=email
to look at the new patch set (#4).
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: soc/intel/cmn/sa: Add APIs into System Agent (SA) common code
......................................................................
soc/intel/cmn/sa: Add APIs into System Agent (SA) common code
This commit streamlines code and strengthens common code robustness
by moving the following SoC-layer functions to the common layer:
- sa_get_mmcfg_size: Retrieves the MMIO (Memory-Mapped I/O)
configuration space size by reading offset
0x60 of the PCI Host Bridge (D0:F0).
- sa_get_dsm_size: Calculates the size of the DSM (Device Stolen
Memory) by reading offset 0x50 of the PCI
Host Bridge (D0:F0) to determine pre-allocated
memory for the IGD (Integrated Graphics Device).
- sa_get_gsm_size: Calculates the size of the GSM (Graphics Stolen
Memory) by reading offset 0x52 of the PCI Host
Bridge (D0:F0).
- sa_get_dpr_size: Determines the size of the DMA Protection
Range (DPR) by reading offset 0x5C of the PCI
Host Bridge (D0:F0).
TEST= Build and boot successful on google/screebo.
Change-Id: Ic00e001563ec6f0d737a445964c716b45db43327
Signed-off-by: Subrata Banik <subratabanik(a)google.com>
---
M src/soc/intel/common/block/include/intelblocks/systemagent.h
M src/soc/intel/common/block/systemagent/systemagent.c
M src/soc/intel/common/block/systemagent/systemagent_def.h
3 files changed, 131 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/80362/4
--
To view, visit https://review.coreboot.org/c/coreboot/+/80362?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: Ic00e001563ec6f0d737a445964c716b45db43327
Gerrit-Change-Number: 80362
Gerrit-PatchSet: 4
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: sridhar siricilla <siricillasridhar(a)gmail.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: sridhar siricilla <siricillasridhar(a)gmail.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Kapil Porwal, Nick Vaccaro, Tarun, sridhar siricilla.
Hello Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Kapil Porwal, Nick Vaccaro, Tarun, build bot (Jenkins), sridhar siricilla,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/80361?usp=email
to look at the new patch set (#4).
The following approvals got outdated and were removed:
Code-Review+2 by sridhar siricilla, Verified+1 by build bot (Jenkins)
The change is no longer submittable: Code-Review and Verified are unsatisfied now.
Change subject: soc/intel/cmn/sa: Refactor SA common code
......................................................................
soc/intel/cmn/sa: Refactor SA common code
Leverages common SA header definitions for Host Bridge registers.
Renames DSM_BASE_ADDR_REG to BDSM and DPR_REG to DPR for brevity.
Additionally, made some minor code alignment corrections while
adding newer macros in the header file.
TEST= Build and boot successful on google/screebo.
Change-Id: I476f213d75a0978336b3749a5ba1499107eb2238
Signed-off-by: Subrata Banik <subratabanik(a)google.com>
---
M src/soc/intel/alderlake/include/soc/systemagent.h
M src/soc/intel/alderlake/systemagent.c
M src/soc/intel/common/block/include/intelblocks/systemagent.h
M src/soc/intel/common/block/systemagent/systemagent_def.h
M src/soc/intel/meteorlake/include/soc/systemagent.h
M src/soc/intel/meteorlake/systemagent.c
6 files changed, 32 insertions(+), 41 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/80361/4
--
To view, visit https://review.coreboot.org/c/coreboot/+/80361?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: I476f213d75a0978336b3749a5ba1499107eb2238
Gerrit-Change-Number: 80361
Gerrit-PatchSet: 4
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Tarun <tstuli(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: sridhar siricilla <siricillasridhar(a)gmail.com>
Gerrit-Attention: Eran Mitrani <mitrani(a)google.com>
Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: sridhar siricilla <siricillasridhar(a)gmail.com>
Gerrit-Attention: Tarun <tstuli(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Kapil Porwal, Nick Vaccaro, Tarun.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80361?usp=email )
Change subject: soc/intel/cmn/sa: Refactor SA common code
......................................................................
Patch Set 3:
(1 comment)
File src/soc/intel/common/block/include/intelblocks/systemagent.h:
https://review.coreboot.org/c/coreboot/+/80361/comment/f25bf719_4c43c886 :
PS3, Line 36:
: /* DMA Protected Range Register */
: #define DPR 0x5C
> nit: My miss to add this comment earlier, may be this definition can be moved to above line#14 in th […]
Acknowledged
--
To view, visit https://review.coreboot.org/c/coreboot/+/80361?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: I476f213d75a0978336b3749a5ba1499107eb2238
Gerrit-Change-Number: 80361
Gerrit-PatchSet: 3
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Tarun <tstuli(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: sridhar siricilla <siricillasridhar(a)gmail.com>
Gerrit-Attention: Eran Mitrani <mitrani(a)google.com>
Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: Tarun <tstuli(a)gmail.com>
Gerrit-Comment-Date: Thu, 08 Feb 2024 18:05:20 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: sridhar siricilla <siricillasridhar(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Reto Buerki.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80261?usp=email )
Change subject: mb/up/squared: Make mini PCIe port mode configurable
......................................................................
Patch Set 2:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/80261/comment/5d1ed07c_9bef6a8f :
PS2, Line 9: Add ENABLE_MSATA config knob and pad configuration to put Mini PCIe port
: into mSATA mode.
> Hmm, I was about to suggest to disable the PCIe root port based on the […]
I think we should figure out if PCIe still works, so that we don't create confusion. Unfortunately, I don't have a board to help testing. I will see if I can get one.
File src/mainboard/up/squared/Kconfig:
https://review.coreboot.org/c/coreboot/+/80261/comment/f648c079_91dd3b8d :
PS2, Line 77: config ENABLE_MSATA
: bool "Use mini-PCIe port for mSATA"
: default n
> I used `mb/compulab/intense_pc` as a template, which provides the same `ENABLE_MSATA` knob. […]
Let's wait with that until we figured out what's going on with that GPIO.
--
To view, visit https://review.coreboot.org/c/coreboot/+/80261?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: Ic2da1dd4252ebb5e373bc65418e321f566d4c10f
Gerrit-Change-Number: 80261
Gerrit-PatchSet: 2
Gerrit-Owner: Reto Buerki <reet(a)codelabs.ch>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Reto Buerki <reet(a)codelabs.ch>
Gerrit-Comment-Date: Thu, 08 Feb 2024 17:15:30 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Reto Buerki <reet(a)codelabs.ch>
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans, Felix Held.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80409?usp=email )
Change subject: include/device/device: fix soft_reserved_ram_resource macro
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
> > So we really merged the soft-reserved stuff... […]
Erm, well, see its review, I guess. It uses some vendor-reserved memory tag. Basically
something from the range that is do-not-upstream by spec.
--
To view, visit https://review.coreboot.org/c/coreboot/+/80409?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: I6b454175c6530e539aa24dffb771368b0aea6da9
Gerrit-Change-Number: 80409
Gerrit-PatchSet: 2
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Thu, 08 Feb 2024 17:13:22 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment
Attention is currently required from: Eric Lai, Ivy Jian, Nick Vaccaro, Paul Menzel.
Shelley Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80300?usp=email )
Change subject: mb/google/brox: Initialize TCHSCR_RST_L to 0
......................................................................
Patch Set 5:
(5 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/80300/comment/496f2314_c6a231e9 :
PS4, Line 10: was causing some leakage
> Was it measured?
Yes it was.
https://review.coreboot.org/c/coreboot/+/80300/comment/a1c51525_90159227 :
PS4, Line 10: Configuring it to 0 initially in
: romstage should fix this.
> s/should fix/fixes/ […]
Done
https://review.coreboot.org/c/coreboot/+/80300/comment/6d2af1ed_9464f8c0 :
PS4, Line 11: Also, make sure that EN_PP3300_TCHSCR is
: initialized in romstage as well.
> Why?
Because if the reset value is initialized the enable value should be initialized to a proper value.
https://review.coreboot.org/c/coreboot/+/80300/comment/76ee5287_7c955daa :
PS4, Line 13:
> What does the schematic say?
I do not understand what you are referring to. What does the schematic say about what?
Patchset:
PS4:
> This is from our HW engineer (Charlie Costakis): "My understanding is that PLT_RST_L is actually the […]
Marking resolved unless there is still something in question?
--
To view, visit https://review.coreboot.org/c/coreboot/+/80300?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: I5bf1901a3a40a38237b950abcb758f96aebcc1cf
Gerrit-Change-Number: 80300
Gerrit-PatchSet: 5
Gerrit-Owner: Shelley Chen <shchen(a)google.com>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Ivy Jian <ivy.jian(a)quanta.corp-partner.google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Eric Lai <ericllai(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Ivy Jian <ivy.jian(a)quanta.corp-partner.google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Comment-Date: Thu, 08 Feb 2024 16:58:28 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Shelley Chen <shchen(a)google.com>
Comment-In-Reply-To: Eric Lai <ericllai(a)google.com>
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: comment
Attention is currently required from: Angel Pons, Arthur Heymans, Jérémy Compostella, Karthik Ramasubramanian, Paul Menzel, Simon Glass, Stefan Reinauer.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77712?usp=email )
Change subject: Introduce a coreboot Control Block (CCB)
......................................................................
Patch Set 10:
(1 comment)
Patchset:
PS10:
haven't really gotten around looking at the code, but thought a bit more about a possible clean implementation that doesn't add some parallel infrastructure or is too hacky and the two options is see are a file in cbfs and using an fmap section for this; the latter will be sort-of equivalent to the fixed flash location approach, but integrates way better into coreboot. when using an fmap section you'll get a fixed position in flash which you can use in the code by including <fmap_config.h> which gets generated during the build. the contents of that section can then just be put in there during build
--
To view, visit https://review.coreboot.org/c/coreboot/+/77712?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: I04e946b33035a493e833500351a0483761252613
Gerrit-Change-Number: 77712
Gerrit-PatchSet: 10
Gerrit-Owner: Simon Glass <sjg(a)chromium.org>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Simon Glass <sjg(a)chromium.org>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Simon Glass <sjg(a)chromium.org>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Comment-Date: Thu, 08 Feb 2024 16:43:12 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Alper Nebi Yasak, Philipp Hug, ron minnich.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80378?usp=email )
The change is no longer submittable: All-Comments-Resolved is unsatisfied now.
Change subject: mainboard/qemu-riscv: Add PCI support
......................................................................
Patch Set 1:
(2 comments)
File src/mainboard/emulation/qemu-riscv/include/mainboard/addressmap.h:
https://review.coreboot.org/c/coreboot/+/80378/comment/c616edca_d5f94975 :
PS1, Line 12: #define QEMU_VIRT_PCIE_ECAM_BASE 0x30000000
: #define QEMU_VIRT_PCIE_ECAM_SIZE 0x3fffffff
probably not needed, since there are CONFIG_ECAM_MMCONF_BASE_ADDRESS and CONFIG_ECAM_MMCONF_LENGTH. also the last one isn't a size, but a last address, right?
File src/mainboard/emulation/qemu-riscv/mainboard.c:
https://review.coreboot.org/c/coreboot/+/80378/comment/2e09d69b_51fa6f61 :
PS1, Line 47: mmio_range(dev, index++, QEMU_VIRT_PCIE_ECAM_BASE, QEMU_VIRT_PCIE_ECAM_SIZE);
can mmconf_resource be used instead?
--
To view, visit https://review.coreboot.org/c/coreboot/+/80378?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: Ie57b35620af82c681d1f0d78fa8e514e4df0d2ac
Gerrit-Change-Number: 80378
Gerrit-PatchSet: 1
Gerrit-Owner: Alper Nebi Yasak <alpernebiyasak(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Philipp Hug <philipp(a)hug.cx>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: ron minnich <rminnich(a)gmail.com>
Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Attention: Philipp Hug <philipp(a)hug.cx>
Gerrit-Attention: Alper Nebi Yasak <alpernebiyasak(a)gmail.com>
Gerrit-Attention: ron minnich <rminnich(a)gmail.com>
Gerrit-Comment-Date: Thu, 08 Feb 2024 15:32:40 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment