Attention is currently required from: Kapil Porwal.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/82045?usp=email )
Change subject: drivers/self_test: coreboot self test
......................................................................
Patch Set 6:
(2 comments)
File src/drivers/self_test/Kconfig:
https://review.coreboot.org/c/coreboot/+/82045/comment/d4ea6c5e_012bdf2e :
PS6, Line 22: in a CBMEM region.
Please also document the command how to read it.
https://review.coreboot.org/c/coreboot/+/82045/comment/04894410_4e3bd314 :
PS6, Line 29: Size of the buffer.
… in bytes.
--
To view, visit https://review.coreboot.org/c/coreboot/+/82045?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: I8d7813d972167592b8dc9e9e7e9b0eb1f50c3da6
Gerrit-Change-Number: 82045
Gerrit-PatchSet: 6
Gerrit-Owner: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Comment-Date: Wed, 24 Apr 2024 09:22:06 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Kapil Porwal.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/82046?usp=email )
Change subject: drivers/self_test/intel_cpu: Add self test submodule for Intel CPU
......................................................................
Patch Set 6:
(4 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/82046/comment/2d27dea1_9c72ea7e :
PS6, Line 9: submodule
submodule might already be associated with git submodules?
https://review.coreboot.org/c/coreboot/+/82046/comment/f236fbc4_eed66ff9 :
PS6, Line 23: 00001000
How does an error look like?
Patchset:
PS6:
Shouldn’t this be done in “normal” coreboot code? Or is it just an (simple, unneeded) example, how the self test framework can be used.
File src/drivers/self_test/modules/intel_cpu/Kconfig:
https://review.coreboot.org/c/coreboot/+/82046/comment/34966382_9b8bcfa7 :
PS6, Line 8: Enable tests for Intel CPU
Please describe the test in more detail.
--
To view, visit https://review.coreboot.org/c/coreboot/+/82046?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: Ie2e40e60609d4b5f2f0ea6dca0f6b51987890877
Gerrit-Change-Number: 82046
Gerrit-PatchSet: 6
Gerrit-Owner: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Comment-Date: Wed, 24 Apr 2024 09:18:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Kapil Porwal.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/82045?usp=email )
Change subject: drivers/self_test: coreboot self test
......................................................................
Patch Set 6:
(1 comment)
Patchset:
PS6:
I also recommend to bring this up on the coreboot mailing list.
--
To view, visit https://review.coreboot.org/c/coreboot/+/82045?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: I8d7813d972167592b8dc9e9e7e9b0eb1f50c3da6
Gerrit-Change-Number: 82045
Gerrit-PatchSet: 6
Gerrit-Owner: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Comment-Date: Wed, 24 Apr 2024 09:16:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Kapil Porwal.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/82045?usp=email )
Change subject: drivers/self_test: coreboot self test
......................................................................
Patch Set 6:
(3 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/82045/comment/ff57c184_1a7a1d47 :
PS6, Line 7: coreboot self test
:
Please make it a statement.
https://review.coreboot.org/c/coreboot/+/82045/comment/d1f367b1_5cd0c04c :
PS6, Line 9: Add a self test infrastructure with below features -
Please add an introduction, what self-tests are, and why they are needed in coreboot (and for example not in a payload or OS).
https://review.coreboot.org/c/coreboot/+/82045/comment/d9c94e7b_ae2337f1 :
PS6, Line 12: - Where results can be dumped during payload or the OS.
Please elaborate. Currently it looks like CBMEM?
--
To view, visit https://review.coreboot.org/c/coreboot/+/82045?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: I8d7813d972167592b8dc9e9e7e9b0eb1f50c3da6
Gerrit-Change-Number: 82045
Gerrit-PatchSet: 6
Gerrit-Owner: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Comment-Date: Wed, 24 Apr 2024 09:16:27 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Alicja Michalska, Felix Singer, Michał Kopeć, Michał Żygowski, Nicholas Chin, Paul Menzel.
Maxim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80853?usp=email )
Change subject: mb/erying: Add Erying Polestar G613 Pro (TGL-H)
......................................................................
Patch Set 8:
(1 comment)
File src/mainboard/erying/tgl/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/80853/comment/252267bd_ecbc9be9 :
PS7, Line 125: device ref pch_espi on
> In Intel PCH, the I/O Range is configured using registers in the LPC/eSPI controller: […]
The ITE it8613e chip is also used in `AOOSTAR R1` motherboard - port CB:82010 . Pay attention to these values: https://review.coreboot.org/c/coreboot/+/82010/8/src/mainboard/aoostar/wtr_…
--
To view, visit https://review.coreboot.org/c/coreboot/+/80853?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: Iffb9e357da2eb686bdcd9a9837df8a60fa94011e
Gerrit-Change-Number: 80853
Gerrit-PatchSet: 8
Gerrit-Owner: Alicja Michalska <ahplka19(a)gmail.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Maxim <max.senia.poliak(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-CC: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-CC: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Alicja Michalska <ahplka19(a)gmail.com>
Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Michał Kopeć <michal.kopec(a)3mdeb.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Comment-Date: Wed, 24 Apr 2024 08:52:01 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Alicja Michalska <ahplka19(a)gmail.com>
Comment-In-Reply-To: Maxim <max.senia.poliak(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Angel Pons, Arthur Heymans, Chen, Gang C, Christian Walter, David Hendricks, Felix Held, Jincheng Li, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Nico Huber, Patrick Rudolph, Paul Menzel, Shuo Liu, TangYiwei, Tim Chu.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81316?usp=email )
Change subject: soc/intel/xeon_sp: Add Granite Rapids initial codes
......................................................................
Patch Set 53:
(1 comment)
File src/soc/intel/xeon_sp/gnr/chipset.cb:
https://review.coreboot.org/c/coreboot/+/81316/comment/9bbb3ffd_7d0f8407 :
PS53, Line 10: .chipset_lockdown = CHIPSET_LOCKDOWN_FSP,
Just wondering. Why is that the default? coreboot does the lockdown for all other Intel platforms.
--
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: 53
Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.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: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: TangYiwei
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
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: Jincheng Li <jincheng.li(a)intel.com>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Comment-Date: Wed, 24 Apr 2024 08:14:37 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Fei Yan, Hung-Te Lin, Yidi Lin.
Hello Hung-Te Lin, Yidi Lin, Yu-Ping Wu, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/82076?usp=email
to look at the new patch set (#3).
The following approvals got outdated and were removed:
Code-Review+1 by Fei Yan, Verified+1 by build bot (Jenkins)
Change subject: soc/mediatek/mt8188: devapc: set devapc permission for MFG
......................................................................
soc/mediatek/mt8188: devapc: set devapc permission for MFG
Set MFG to domain 6.
Set MFG remap in infra and sub infra, so that MFG can switch to protect mode by MFG register.
Set MFG slave MFG_S_S-2 and MFG_S_S-5 permission from NO_PROTECTION to SEC_RW_ONLY for domain 0,
so that only AP in secure mode can access MFG_S_S-2 and MFG_S_S-5.
BUG=b:313855815
TEST=emerge-geralt coreboot
Change-Id: Ic6fb7d85bf9d4d92946a045a274b274abc440e1d
Signed-off-by: Fei Yan <fei.yan(a)mediatek.corp-partner.google.com>
---
M src/soc/mediatek/mt8188/devapc.c
M src/soc/mediatek/mt8188/include/soc/addressmap.h
M src/soc/mediatek/mt8188/include/soc/devapc.h
3 files changed, 55 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/76/82076/3
--
To view, visit https://review.coreboot.org/c/coreboot/+/82076?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: Ic6fb7d85bf9d4d92946a045a274b274abc440e1d
Gerrit-Change-Number: 82076
Gerrit-PatchSet: 3
Gerrit-Owner: Fei Yan <fei.yan(a)mediatek.corp-partner.google.com>
Gerrit-Reviewer: Fei Yan <fei.yan(a)mediatek.corp-partner.google.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-Attention: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Attention: Fei Yan <fei.yan(a)mediatek.corp-partner.google.com>
Gerrit-Attention: Yidi Lin <yidilin(a)google.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Nico Huber.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/libgfxinit/+/81854?usp=email )
Change subject: gfxtest: Introduce `From_RGBA` constructor for `Pixel_Type`
......................................................................
Patch Set 1:
(3 comments)
Commit Message:
https://review.coreboot.org/c/libgfxinit/+/81854/comment/ffa00fee_2d9624a0 :
PS1, Line 7: gfxtest: Fix out-of-order components, permanently
> Technically, there is nothing to fix, how about: […]
Done
File gfxtest/hw-gfx-gma-gfx_test.adb:
https://review.coreboot.org/c/libgfxinit/+/81854/comment/bb3b775f_d48ac364 :
PS1, Line 60: for Pixel_Type use record
: Blue at 0 range 0 .. 7;
: Green at 1 range 0 .. 7;
: Red at 2 range 0 .. 7;
: Alpha at 3 range 0 .. 7;
: end record;
> Could replace this with a `with Packed` for the record then.
Used a `with Pack`, if this record's size ever happens to not be 32 bits, at least the `Unchecked_Conversion` to `Word32` will cause a warning.
https://review.coreboot.org/c/libgfxinit/+/81854/comment/86d9a4c4_adecfcee :
PS1, Line 67: RGB
> or `From_RGBA`?
Done
--
To view, visit https://review.coreboot.org/c/libgfxinit/+/81854?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: libgfxinit
Gerrit-Branch: main
Gerrit-Change-Id: I77dbdcd6c235e411585585779c31777adcef57d0
Gerrit-Change-Number: 81854
Gerrit-PatchSet: 1
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Comment-Date: Wed, 24 Apr 2024 08:08:47 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: comment
Attention is currently required from: Angel Pons, Nico Huber.
Hello Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/libgfxinit/+/81854?usp=email
to look at the new patch set (#2).
The following approvals got outdated and were removed:
Code-Review+1 by Nico Huber
Change subject: gfxtest: Introduce `From_RGBA` constructor for `Pixel_Type`
......................................................................
gfxtest: Introduce `From_RGBA` constructor for `Pixel_Type`
The `Pixel_Type` record uses a different order for its components, as
the hardware expects the data bytes in BGRA order but most people are
used to RGBA order for color components. However, this approach makes
the compiler complain:
warning: component clause out of order with respect to declaration [-gnatw_r]
This prevents building gfxtest since the warning gets promoted to an
error. While the issue is easy to work around (use BGRA order in the
declaration), *having* to work around this problem is most certainly
not ideal.
Introduce the `From_RGBA` function which maps 3 or 4 RGB(A) bytes to
a `Pixel_Type` record. Alpha is optional for convenience, as most of
the pixel handling uses fully opaque colors anyway.
Use this new function to replace positional aggregate initialization
to preserve colors (red is red, blue is blue). Omitting `255` alphas
makes up for the (slightly) increased verbosity of the code.
TEST=Run this and make sure all builds pass:
for f in configs/*
do
make distclean
make DEBUG=1 cnf=$f gfx_test -j$(nproc)
done
Change-Id: I77dbdcd6c235e411585585779c31777adcef57d0
Signed-off-by: Angel Pons <th3fanbus(a)gmail.com>
---
M gfxtest/hw-gfx-gma-gfx_test.adb
1 file changed, 13 insertions(+), 17 deletions(-)
git pull ssh://review.coreboot.org:29418/libgfxinit refs/changes/54/81854/2
--
To view, visit https://review.coreboot.org/c/libgfxinit/+/81854?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: libgfxinit
Gerrit-Branch: main
Gerrit-Change-Id: I77dbdcd6c235e411585585779c31777adcef57d0
Gerrit-Change-Number: 81854
Gerrit-PatchSet: 2
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: newpatchset