Attention is currently required from: Arthur Heymans, Christian Walter, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Patrick Rudolph, Shuo Liu, Tim Chu.
Nico Huber has posted comments on this change by Shuo Liu. ( https://review.coreboot.org/c/coreboot/+/83538?usp=email )
Change subject: soc/intel/xeon_sp: Reserve FSP MMIO high window
......................................................................
Patch Set 1:
(6 comments)
Patchset:
PS1:
> Some critical log diffs: […]
Just for the future, please disable CONSOLE_USE_ANSI_ESCAPES and use triple backticks \`\`\` for quote blocks.
Also, what you pasted is not the MTRR assignment but the requirements.
Commit Message:
https://review.coreboot.org/c/coreboot/+/83538/comment/edaa428b_6c4eb88b?us… :
PS1, Line 7: soc/intel/xeon_sp: Reserve FSP MMIO high window
Only for SPR, right? btw. why?
https://review.coreboot.org/c/coreboot/+/83538/comment/0de08812_32eaf051?us… :
PS1, Line 9: Xeon-SP supports MMIO high window, a.k.a. MMIO window above 4G.
TBH, I don't know what this means; or what platform doesn't support it?
https://review.coreboot.org/c/coreboot/+/83538/comment/58821c4e_7aa00c89?us… :
PS1, Line 19: is especially important
: on systems with 2 or more sockets, where each socket has multiple
: domains.
How so? In what scenario is this important? What part of coreboot
or your payload has such requirements?
File src/soc/intel/xeon_sp/uncore.c:
https://review.coreboot.org/c/coreboot/+/83538/comment/d10ee5ad_5e0830c5?us… :
PS1, Line 368: * Add MMIO high window.
That's what the code says. Please don't state the obvious in comments.
What a reader needs to know here is *why* we do it here in set_resources().
https://review.coreboot.org/c/coreboot/+/83538/comment/231d4481_8bfd6293?us… :
PS1, Line 375: index++;
This not stable because mmapvtd_read_resources() mixes generic PCI
resources and the static ones counted from 0. The latter start at
0x10, IIRC. Does the `mmapvtd` device really have standard PCI
resources?
If so, I would choose some higher constant and document that.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83538?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: Ib2a0e1f1f13e797c1fab6aca589d060c4d3fa15b
Gerrit-Change-Number: 83538
Gerrit-PatchSet: 1
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: Nico Huber <nico.h(a)gmx.de>
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: 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, 19 Jul 2024 14:47:04 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Shuo Liu <shuo.liu(a)intel.com>
Attention is currently required from: Dinesh Gehlot, Kapil Porwal, Nick Vaccaro, Subrata Banik.
Michał Żygowski has posted comments on this change by Michał Żygowski. ( https://review.coreboot.org/c/coreboot/+/83356?usp=email )
Change subject: soc/intel/alderlake/tcss: Add definition of IOM_READY bit
......................................................................
Patch Set 4:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/83356/comment/37b187e0_dc93d74f?us… :
PS3, Line 14: of
> drop one `of`, and missing line break
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/83356?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: If868a77852468ebb73526b1571191cbdeb1804b9
Gerrit-Change-Number: 83356
Gerrit-PatchSet: 4
Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Subrata Banik <subratabanik(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-Comment-Date: Fri, 19 Jul 2024 14:34:51 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Attention is currently required from: Dinesh Gehlot, Kapil Porwal, Nick Vaccaro, Subrata Banik.
Hello Dinesh Gehlot, Kapil Porwal, Nick Vaccaro, Nico Huber, Subrata Banik, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/83356?usp=email
to look at the new patch set (#4).
Change subject: soc/intel/alderlake/tcss: Add definition of IOM_READY bit
......................................................................
soc/intel/alderlake/tcss: Add definition of IOM_READY bit
Add definition of the IOM_READY bit in the IOM_TYPEC_STATUS_1
register. Needed by Protectli VP66XX boards to poll for this bit
for about 2 seconds before FSP Silicon Init to have USB functionality.
ME is supposed to start fetching and executing the TCSS IPs FW right
after DRAM Init Done message, which happens after MRC. For most
platforms the time interval between the end of MemoryInit and start of
SiliconInit is enough for IOM_READY to get set.
TEST=Poll the IOM_READY bit on VP66XX platform and observe the
TCSS XHCI is up in lspci.
Change-Id: If868a77852468ebb73526b1571191cbdeb1804b9
Signed-off-by: Michał Żygowski <michal.zygowski(a)3mdeb.com>
---
M src/soc/intel/alderlake/include/soc/tcss.h
1 file changed, 4 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/56/83356/4
--
To view, visit https://review.coreboot.org/c/coreboot/+/83356?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: If868a77852468ebb73526b1571191cbdeb1804b9
Gerrit-Change-Number: 83356
Gerrit-PatchSet: 4
Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Subrata Banik <subratabanik(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>
Attention is currently required from: Dinesh Gehlot, Kapil Porwal, Nick Vaccaro, Subrata Banik.
Michał Żygowski has posted comments on this change by Michał Żygowski. ( https://review.coreboot.org/c/coreboot/+/83356?usp=email )
Change subject: soc/intel/alderlake/tcss: Add definition of IOM_READY bit
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS1:
> Wild thought, because you mentioned fetching fw: Is your flash fast enough? e.g. […]
ifdtool says:
FLCOMP 0x093030f5
Dual Output Fast Read Support: not supported
Read ID/Read Status Clock Frequency: 50MHz
Write/Erase Clock Frequency: 50MHz
Fast Read Clock Frequency: 50MHz
Fast Read Support: supported
Read eSPI/EC Bus Frequency: 20MHz
Component 2 Density: UNUSED
Component 1 Density: 16MB
So, I think fast read is configured well.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83356?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: If868a77852468ebb73526b1571191cbdeb1804b9
Gerrit-Change-Number: 83356
Gerrit-PatchSet: 3
Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Subrata Banik <subratabanik(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-Comment-Date: Fri, 19 Jul 2024 14:34:27 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Attention is currently required from: Julius Werner, Jérémy Compostella, Nico Huber, Philipp Hug, Ron Minnich.
Arthur Heymans has posted comments on this change by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/83574?usp=email )
Change subject: Makefile.mk: Add a common link_stage function and use it
......................................................................
Patch Set 2:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/83574/comment/3f60c9bc_561f61c4?us… :
PS2, Line 16: - the x86 --oformat is dropped as it is not needed.
> How would that look like? it's not just one linker script, right?
>
> Actually, it feels like this is xcompile's job.
It looks like ARM and ARM64 feature this.
Something like:
OUTPUT_FORMAT(elf32-i386)
OUTPUT_ARCH(i386)
needs to be added to header.ld of multiple arch.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83574?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: I64f7686894c99732d06972e7ba327061db6d7c44
Gerrit-Change-Number: 83574
Gerrit-PatchSet: 2
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Philipp Hug <philipp(a)hug.cx>
Gerrit-Reviewer: Ron Minnich <rminnich(a)gmail.com>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Philipp Hug <philipp(a)hug.cx>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Ron Minnich <rminnich(a)gmail.com>
Gerrit-Comment-Date: Fri, 19 Jul 2024 14:21:16 +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>
Attention is currently required from: Hung-Te Lin, Jarried Lin, Paul Menzel.
Yu-Ping Wu has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/83572?usp=email )
The change is no longer submittable: All-Comments-Resolved is unsatisfied now.
Change subject: soc/mediatek/mt8196: Add a stub implementation of the MT8196 SoC
......................................................................
Patch Set 5:
(1 comment)
File src/soc/mediatek/mt8196/Makefile.mk:
https://review.coreboot.org/c/coreboot/+/83572/comment/573b533a_2334dbb6?us… :
PS5, Line 1: ## SPDX-License-Identifier: GPL-2.0-only
one blank line below
--
To view, visit https://review.coreboot.org/c/coreboot/+/83572?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: I8190253ed000db879b04a806ca0bdf29c14be806
Gerrit-Change-Number: 83572
Gerrit-PatchSet: 5
Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com>
Gerrit-Comment-Date: Fri, 19 Jul 2024 13:54:52 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Hung-Te Lin, Jarried Lin, Xi Chen.
Yu-Ping Wu has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/83570?usp=email )
Change subject: soc/mediatek: Move symbols.h into MediaTek common directory
......................................................................
Patch Set 6: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/83570?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: Iad1c9f520cdc5c6ad2b55e8f4ec6149fa47b17b1
Gerrit-Change-Number: 83570
Gerrit-PatchSet: 6
Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Xi Chen <xixi.chen(a)mediatek.com>
Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Attention: Xi Chen <xixi.chen(a)mediatek.com>
Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com>
Gerrit-Comment-Date: Fri, 19 Jul 2024 13:52:40 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes