Attention is currently required from: Hung-Te Lin, Jarried Lin, Yu-Ping Wu.
Yidi Lin has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/83924?usp=email )
Change subject: mb/google/rauru: Initialize flash controller in bootblock
......................................................................
Patch Set 2:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/83924/comment/08bd79c8_6e55109d?us… :
PS2, Line 9: NOR-Flash
SPI NOR Flash Controler(SNFC)
--
To view, visit https://review.coreboot.org/c/coreboot/+/83924?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: I88960ce7a50f67ea6f402884b714cb205836a6d8
Gerrit-Change-Number: 83924
Gerrit-PatchSet: 2
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-Attention: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Comment-Date: Thu, 15 Aug 2024 14:36:41 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Hung-Te Lin, Jarried Lin, Yu-Ping Wu.
Yidi Lin has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/83922?usp=email )
Change subject: soc/mediatek/mt8196: Add GPIO drivers
......................................................................
Patch Set 2:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/83922/comment/cb15a22f_5f2f8c20?us… :
PS2, Line 7: s
remove `s`
--
To view, visit https://review.coreboot.org/c/coreboot/+/83922?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: I6d1e6ef17660308c8de908697ffba6b5f17ff9ae
Gerrit-Change-Number: 83922
Gerrit-PatchSet: 2
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-Attention: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Comment-Date: Thu, 15 Aug 2024 14:35:33 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Alexander Couzens, Keith Hui, Martin L Roth, Nicholas Chin.
Angel Pons has posted comments on this change by Keith Hui. ( https://review.coreboot.org/c/coreboot/+/79025?usp=email )
Change subject: nb/intel/haswell: Move SPD addresses to devicetree
......................................................................
Patch Set 4:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/79025/comment/9822745b_11c88302?us… :
PS4, Line 14: Patch also covers recently added Z97 boards using Broadwell MRC.
I had to make a separate follow-up commit for Sandy Bridge because the original refactoring commits had already been submitted. See CB:49088 as to how I'd handle it.
Yes, it's go code, but it's not that hard to read. Plus, the changes done are in line with what is done to the mainboards, so reviewing them is not that hard. I'd prefer to include the autoport changes in this commit, and briefly mention so in the commit message, e.g.:
> Also, update util/autoport accordingly.
File src/northbridge/intel/haswell/broadwell_mrc/raminit.c:
https://review.coreboot.org/c/coreboot/+/79025/comment/9a28cde2_43fe140e?us… :
PS4, Line 377: struct spd_info spdi = {0};
: if (CONFIG(HAVE_SPD_IN_CBFS)) {
: /* Obtain the SPD addresses from mainboard code */
: mb_get_spd_map(&spdi);
: } else {
: /* Boards without memory down: Obtain the SPD addresses from devicetree */
: memcpy(spdi.addresses, cfg->spd_addresses, ARRAY_SIZE(spdi.addresses));
: }
:
> Nice idea, but the code are separate enough this helper function will need to go into a separate C f […]
Yeah, it's to avoid having redundant copies of the source code that may end up out of sync. I will also move things like `report_memory_config()` (or whatever it's called).
The file could be named something like `raminit_shared.c`, for example (IIRC, Sandybridge uses this name for stuff shared between MRC and NRI).
--
To view, visit https://review.coreboot.org/c/coreboot/+/79025?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: I574aec9cb6a47c8aaf275ae06c7e1fb695534b34
Gerrit-Change-Number: 79025
Gerrit-PatchSet: 4
Gerrit-Owner: Keith Hui <buurin(a)gmail.com>
Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: coreboot org <coreboot.org(a)gmail.com>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Attention: Alexander Couzens <lynxis(a)fe80.eu>
Gerrit-Attention: Keith Hui <buurin(a)gmail.com>
Gerrit-Comment-Date: Thu, 15 Aug 2024 13:26:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Comment-In-Reply-To: Keith Hui <buurin(a)gmail.com>
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/83864?usp=email
to look at the new patch set (#2).
Change subject: [TEST ONLY] src: Log boot info before cse sync
......................................................................
[TEST ONLY] src: Log boot info before cse sync
This patch logs the current boot information before proceeding to CSE
sync, as CSE sync could potentially trigger a reboot.
BUG=b:354435327
TEST=Verified elog boot info prior to cse sync
Change-Id: I40abfe870d6b3302005b898efbc7ebcae42ed52c
Signed-off-by: Dinesh Gehlot <digehlot(a)google.com>
---
M src/drivers/elog/elog.c
M src/include/elog.h
M src/soc/intel/common/block/cse/cse_lite.c
M src/vendorcode/google/chromeos/elog.c
4 files changed, 10 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/64/83864/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/83864?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: I40abfe870d6b3302005b898efbc7ebcae42ed52c
Gerrit-Change-Number: 83864
Gerrit-PatchSet: 2
Gerrit-Owner: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Dinesh Gehlot has posted comments on this change by Dinesh Gehlot. ( https://review.coreboot.org/c/coreboot/+/83864?usp=email )
Change subject: src: Log boot info before cse sync
......................................................................
Patch Set 1:
(1 comment)
File src/soc/intel/common/block/cse/cse_lite.c:
https://review.coreboot.org/c/coreboot/+/83864/comment/dc68255e_0128de2d?us… :
PS1, Line 1434: elog_add_vboot_info();
> You can't just remove these from platform-independent files and move them into an Intel-only file, y […]
Yes, agree it should not be move to platform specific code. I missed to mark the changes `TEST only`.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83864?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: I40abfe870d6b3302005b898efbc7ebcae42ed52c
Gerrit-Change-Number: 83864
Gerrit-PatchSet: 1
Gerrit-Owner: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Gerrit-Comment-Date: Thu, 15 Aug 2024 12:08:29 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Attention is currently required from: Alexander Couzens, Angel Pons, Martin L Roth, Nicholas Chin.
Keith Hui has posted comments on this change by Keith Hui. ( https://review.coreboot.org/c/coreboot/+/79025?usp=email )
Change subject: nb/intel/haswell: Move SPD addresses to devicetree
......................................................................
Patch Set 4:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/79025/comment/8c5cebdd_3f9bbaef?us… :
PS4, Line 14: Patch also covers recently added Z97 boards using Broadwell MRC.
> IIRC autoport support for Haswell also landed recently, but this change doesn't seem to update autop […]
This is in a separate patch on this train.
Since it involves Go, the best I can do is emulate what you did for sandybridge. And since I cannot test because I have no Haswell boards, I prefer it be reviewed separately.
File src/northbridge/intel/haswell/broadwell_mrc/raminit.c:
https://review.coreboot.org/c/coreboot/+/79025/comment/51b86f99_b0380d0d?us… :
PS4, Line 377: struct spd_info spdi = {0};
: if (CONFIG(HAVE_SPD_IN_CBFS)) {
: /* Obtain the SPD addresses from mainboard code */
: mb_get_spd_map(&spdi);
: } else {
: /* Boards without memory down: Obtain the SPD addresses from devicetree */
: memcpy(spdi.addresses, cfg->spd_addresses, ARRAY_SIZE(spdi.addresses));
: }
:
> Would be nice to avoid having this pattern repeated three times. […]
Nice idea, but the code are separate enough this helper function will need to go into a separate C file with all the overhead. In the end only one copy will get compiled into the binary. I can try it still.
--
To view, visit https://review.coreboot.org/c/coreboot/+/79025?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: I574aec9cb6a47c8aaf275ae06c7e1fb695534b34
Gerrit-Change-Number: 79025
Gerrit-PatchSet: 4
Gerrit-Owner: Keith Hui <buurin(a)gmail.com>
Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: coreboot org <coreboot.org(a)gmail.com>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Attention: Alexander Couzens <lynxis(a)fe80.eu>
Gerrit-Comment-Date: Thu, 15 Aug 2024 12:02:20 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Attention is currently required from: Alexander Couzens, Keith Hui, Martin L Roth, Nicholas Chin.
Angel Pons has posted comments on this change by Keith Hui. ( https://review.coreboot.org/c/coreboot/+/79025?usp=email )
Change subject: nb/intel/haswell: Move SPD addresses to devicetree
......................................................................
Patch Set 4:
(4 comments)
Patchset:
PS4:
I'm still not fond of having SPD settings spread between devicetree and code.
Commit Message:
https://review.coreboot.org/c/coreboot/+/79025/comment/7ef92cd8_f50d1394?us… :
PS4, Line 14: Patch also covers recently added Z97 boards using Broadwell MRC.
IIRC autoport support for Haswell also landed recently, but this change doesn't seem to update autoport.
File src/mainboard/asrock/h81m-hds/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/79025/comment/f2037813_a6367358?us… :
PS4, Line 5: register "spd_addresses" = "{0x50, 0, 0x52, 0}"
nit: I would prefer an initialiser with indices:
```suggestion
register "spd_addresses" = "{ [0] = 0x50, [2] = 0x52 }"
```
If I get bored enough, I'll replace this with a struct:
```
register "spd_addresses" = "{
.ch0 = { .slot0 = 0x50 },
.ch1 = { .slot0 = 0x52 },
}"
```
File src/northbridge/intel/haswell/broadwell_mrc/raminit.c:
https://review.coreboot.org/c/coreboot/+/79025/comment/df3b34d6_e4298e38?us… :
PS4, Line 377: struct spd_info spdi = {0};
: if (CONFIG(HAVE_SPD_IN_CBFS)) {
: /* Obtain the SPD addresses from mainboard code */
: mb_get_spd_map(&spdi);
: } else {
: /* Boards without memory down: Obtain the SPD addresses from devicetree */
: memcpy(spdi.addresses, cfg->spd_addresses, ARRAY_SIZE(spdi.addresses));
: }
:
Would be nice to avoid having this pattern repeated three times. How about adding a helper function?
```
struct spd_info get_spd_info(void)
{
struct spd_info spdi = {0};
if (CONFIG(HAVE_SPD_IN_CBFS)) {
/* Obtain the SPD addresses from mainboard code */
mb_get_spd_map(&spdi);
} else {
/* Boards without memory down: Obtain the SPD addresses from devicetree */
memcpy(spdi.addresses, cfg->spd_addresses, ARRAY_SIZE(spdi.addresses));
}
return spdi;
}
```
--
To view, visit https://review.coreboot.org/c/coreboot/+/79025?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: I574aec9cb6a47c8aaf275ae06c7e1fb695534b34
Gerrit-Change-Number: 79025
Gerrit-PatchSet: 4
Gerrit-Owner: Keith Hui <buurin(a)gmail.com>
Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: coreboot org <coreboot.org(a)gmail.com>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Attention: Alexander Couzens <lynxis(a)fe80.eu>
Gerrit-Attention: Keith Hui <buurin(a)gmail.com>
Gerrit-Comment-Date: Thu, 15 Aug 2024 11:06:52 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Hung-Te Lin, Yidi Lin, Yu-Ping Wu.
Jarried Lin has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/83924?usp=email )
Change subject: mb/google/rauru: Initialize flash controller in bootblock
......................................................................
Patch Set 2:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/83924/comment/dc81c8b2_11254830?us… :
PS1, Line 7: Add NOR-Flash support
> Initialize flash controller in bootblock
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/83924?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: I88960ce7a50f67ea6f402884b714cb205836a6d8
Gerrit-Change-Number: 83924
Gerrit-PatchSet: 2
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-Attention: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Attention: Yidi Lin <yidilin(a)google.com>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Comment-Date: Thu, 15 Aug 2024 09:51:05 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Yidi Lin <yidilin(a)google.com>
Attention is currently required from: Hung-Te Lin, Jarried Lin, Yu-Ping Wu.
Yidi Lin has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/83928?usp=email )
Change subject: soc/mediatek/mt8196: Fix timer reset in BL31
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/83928?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: I520986b81ca153ec3ce56558a80619448cfc0c59
Gerrit-Change-Number: 83928
Gerrit-PatchSet: 2
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-Attention: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Comment-Date: Thu, 15 Aug 2024 09:50:42 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Hung-Te Lin, Jarried Lin, Yu-Ping Wu.
Yidi Lin has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/83927?usp=email )
Change subject: soc/mediatek/mt8196: Add I2C driver support
......................................................................
Patch Set 1:
(1 comment)
File src/soc/mediatek/mt8196/include/soc/i2c.h:
https://review.coreboot.org/c/coreboot/+/83927/comment/4489ce9a_8743dbf2?us… :
PS1, Line 74:
remove one blank line
--
To view, visit https://review.coreboot.org/c/coreboot/+/83927?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: I617ad8a43ce8b492b1a0e5dc06c1f0ffe7d92b5e
Gerrit-Change-Number: 83927
Gerrit-PatchSet: 1
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: ot_hao.han(a)mediatek.com <ot_hao.han(a)mediatek.corp-partner.google.com>
Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Comment-Date: Thu, 15 Aug 2024 09:49:50 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No