Attention is currently required from: Hung-Te Lin, Jarried Lin, Yidi Lin, Zhanzhan Ge.
Hello Hung-Te Lin, Yidi Lin, Yu-Ping Wu, Zhanzhan Ge, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/83928?usp=email
to look at the new patch set (#17).
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: soc/mediatek/mt8196: Fix timer reset in BL31
......................................................................
soc/mediatek/mt8196: Fix timer reset in BL31
1. Set systimer compensation to version 2.0.
2. The system does not need to serve pending IRQ from systimer
after rebooting. Therefore we clear systimer IRQ pending bit
at early booting.
TEST=Build pass and timestamp is not reset in ATF and payload
BUG=b:343881008
Change-Id: I520986b81ca153ec3ce56558a80619448cfc0c59
Signed-off-by: Zhanzhan Ge <zhanzhan.ge(a)mediatek.corp-partner.google.com>
---
M src/soc/mediatek/mt8196/Makefile.mk
M src/soc/mediatek/mt8196/include/soc/timer.h
M src/soc/mediatek/mt8196/timer.c
A src/soc/mediatek/mt8196/timer_prepare.c
4 files changed, 61 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/28/83928/17
--
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: newpatchset
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I520986b81ca153ec3ce56558a80619448cfc0c59
Gerrit-Change-Number: 83928
Gerrit-PatchSet: 17
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: Zhanzhan Ge <zhanzhan.ge(a)mediatek.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Attention: Zhanzhan Ge <zhanzhan.ge(a)mediatek.corp-partner.google.com>
Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com>
Gerrit-Attention: Yidi Lin <yidilin(a)google.com>
Jędrzej Ciupis has posted comments on this change by Jędrzej Ciupis. ( https://review.coreboot.org/c/coreboot/+/83885?usp=email )
Change subject: mb/google/dedede: enable Intel CrashLog
......................................................................
Patch Set 3:
(1 comment)
File src/mainboard/google/dedede/variants/baseboard/devicetree.cb:
PS2:
> i was wondering why the alias is defined in the mainboard devicetree and why it doesn't use the alia […]
Yes, exactly.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83885?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: Ib0487bd6a5bfdad2a80fd0787e009e48f4527d38
Gerrit-Change-Number: 83885
Gerrit-PatchSet: 3
Gerrit-Owner: Jędrzej Ciupis <jciupis(a)google.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Fri, 23 Aug 2024 07:02:45 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>
Jędrzej Ciupis has posted comments on this change by Jędrzej Ciupis. ( https://review.coreboot.org/c/coreboot/+/83884?usp=email )
Change subject: soc/intel/jasperlake: Add CrashLog implementation for Intel JSL
......................................................................
Patch Set 3:
(1 comment)
File src/soc/intel/jasperlake/crashlog_lib.c:
PS2:
> haven't compared this with the code in the other socs, but i wonder if some code in there is actuall […]
Thanks for the comment, it's a fair point. I considered reworking the code when preparing the patch. However, I believe my understanding of this feature is insufficient to redesign it reliably. I wouldn't know if a given code can be made common or rather should be kept separate for forward compatibility and easier extensibility for future SoCs.
Personally, I'd prefer to keep this change as it is and leave such a rework to domain experts from Intel.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83884?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: Ia18a79d8de849d556b4b8fd0e6b43090311eb23f
Gerrit-Change-Number: 83884
Gerrit-PatchSet: 3
Gerrit-Owner: Jędrzej Ciupis <jciupis(a)google.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Fri, 23 Aug 2024 07:02:41 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>
Attention is currently required from: Felix Singer, Jason Glenesk, Martin L Roth.
Angel Pons has posted comments on this change by Martin L Roth. ( https://review.coreboot.org/c/coreboot/+/84046?usp=email )
Change subject: Docs/releases: Update 24.08 release notes
......................................................................
Patch Set 1: Code-Review+1
(4 comments)
File Documentation/releases/coreboot-24.08-relnotes.md:
https://review.coreboot.org/c/coreboot/+/84046/comment/b4d1d7cc_8ae5e067?us… :
PS1, Line 23: ## Introduce region_create() functions
:
: We introduce two new functions to create region objects. They allow us to check
: for integer overflows (region_create_untrusted()) or assert their absence
: (region_create()).
:
: This fixes potential overflows in region_overlap() checks in SMI handlers, where
: we would wrongfully report MMIO as *not* overlapping SMRAM.
:
: Also, two cases of strtol() in parse_region() (cbfstool), where the results were
: implicitly converted to `size_t`, are replaced with the unsigned strtoul().
nit: use inline code blocks for function names? `like this`
https://review.coreboot.org/c/coreboot/+/84046/comment/c46f8ddf_2331510a?us… :
PS1, Line 39: https://review.coreboot.org/79905
If possible, would be nice to replace these links with commit hashes once submitted
https://review.coreboot.org/c/coreboot/+/84046/comment/9c2d8b41_7c50b9cb?us… :
PS1, Line 65: which were mostly fixed
Grammar: which (the issue) *was* mostly fixed
```suggestion
and had this issue, which was mostly fixed by using exception handlers in the
```
If the intent is to say that "the mainboards were mostly fixed", I would word this differently but it still sounds odd to me:
> and had this issue, **but they** were mostly fixed by ...
https://review.coreboot.org/c/coreboot/+/84046/comment/c70b29ef_6d1bc9f8?us… :
PS1, Line 287: * src/soc/intel/pantherlake
I think a verb is missing?
```suggestion
* Added src/soc/intel/pantherlake
```
--
To view, visit https://review.coreboot.org/c/coreboot/+/84046?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: I449e8490d72976c8f723dc3b5ab3b77d7b16e3a0
Gerrit-Change-Number: 84046
Gerrit-PatchSet: 1
Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
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: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Comment-Date: Fri, 23 Aug 2024 06:50:33 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
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 5:
(1 comment)
File src/northbridge/intel/haswell/raminit.h:
https://review.coreboot.org/c/coreboot/+/79025/comment/a8c26523_d88ffd2d?us… :
PS5, Line 19: get_spd_addresses
> I used this name trying to make this code self-documenting. […]
Yes, because this function fills in a `struct spd_info`, which contains more than SPD addresses.
--
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: 5
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: Fri, 23 Aug 2024 06:41:46 +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>
Attention is currently required from: Felix Singer, Jason Glenesk, Martin L Roth.
Angel Pons has posted comments on this change by Jason Glenesk. ( https://review.coreboot.org/c/coreboot/+/84036?usp=email )
The change is no longer submittable: All-Comments-Resolved is unsatisfied now.
Change subject: Documentation/releases: Add 24.11 release notes template
......................................................................
Patch Set 2: Code-Review+2
(1 comment)
File Documentation/releases/index.md:
https://review.coreboot.org/c/coreboot/+/84036/comment/dad73156_c25b41b8?us… :
PS2, Line 25: 24.08 - August 2024 <coreboot-24.08-relnotes.md>
nit: Oh no, the alignment!
```suggestion
24.08 - August 2024 <coreboot-24.08-relnotes.md>
```
--
To view, visit https://review.coreboot.org/c/coreboot/+/84036?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: I1e524f1db0090bf8815b08315f9cbc9894965af7
Gerrit-Change-Number: 84036
Gerrit-PatchSet: 2
Gerrit-Owner: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Jason Glenesk <jason.glenesk(a)amd.corp-partner.google.com>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Comment-Date: Fri, 23 Aug 2024 06:39:43 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Attention is currently required from: Martin L Roth, Matt DeVillier.
Angel Pons has posted comments on this change by Matt DeVillier. ( https://review.coreboot.org/c/coreboot/+/84034?usp=email )
The change is no longer submittable: All-Comments-Resolved is unsatisfied now.
Change subject: documentation/release: Update release checklist
......................................................................
Patch Set 2: Code-Review+1
(2 comments)
File Documentation/releases/checklist.md:
https://review.coreboot.org/c/coreboot/+/84034/comment/41c6e6ad_bd5715eb?us… :
PS2, Line 76: Freezing
nit:
```suggestion
- [ ] Freeze toolchain state. Only relevant fixes are allowed from this point on.
```
https://review.coreboot.org/c/coreboot/+/84034/comment/41dddff9_43c01cdd?us… :
PS2, Line 77: - [ ] Schedule release meetings
nit: try to be consistent with trailing period?
--
To view, visit https://review.coreboot.org/c/coreboot/+/84034?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: Id49b3f38d3501382b7fb7ac791190c0cacd58a11
Gerrit-Change-Number: 84034
Gerrit-PatchSet: 2
Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
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: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Comment-Date: Fri, 23 Aug 2024 06:38:32 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Attention is currently required from: Jakub Czapiga, Maximilian Brune, Nico Huber, Philipp Hug, ron minnich.
Julius Werner has posted comments on this change by Nico Huber. ( https://review.coreboot.org/c/coreboot/+/79907?usp=email )
Change subject: [RFC] region: Hide struct region members
......................................................................
Patch Set 8:
(1 comment)
File src/arch/arm/fit_payload.c:
https://review.coreboot.org/c/coreboot/+/79907/comment/4628acc7_cbcd7ff6?us… :
PS8, Line 3: /* FIXME: should use the high-level region api */
> >> I don't think we have an actual maintainer for the FIT payload support. […]
The main point of FIT is to be able to bundle a flattened device tree (FDT) together with a kernel image, which is required for modern Arm Linux. FIT is the only way to boot straight into the kernel from coreboot on Arm(64). I would rather we not remove it if possible.
I don't really have time (as usual) but am familiar enough with this code to review things if I get around to it. It shouldn't be too hard to convert it to use region_create(), I hope.
--
To view, visit https://review.coreboot.org/c/coreboot/+/79907?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: I713be9cf0bab4c2e21113b55e7229ab50f06c6cf
Gerrit-Change-Number: 79907
Gerrit-PatchSet: 8
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com>
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: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: coreboot org <coreboot.org(a)gmail.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Philipp Hug <philipp(a)hug.cx>
Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Attention: ron minnich <rminnich(a)gmail.com>
Gerrit-Comment-Date: Fri, 23 Aug 2024 05:29:03 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: coreboot org <coreboot.org(a)gmail.com>
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Maximilian Brune <maximilian.brune(a)9elements.com>