Attention is currently required from: Jakub Czapiga.
Elyes Haouas has posted comments on this change by Elyes Haouas. ( https://review.coreboot.org/c/coreboot/+/86162?usp=email )
Change subject: tree: Use the default standard for C
......................................................................
Patch Set 1: Code-Review-1
--
To view, visit https://review.coreboot.org/c/coreboot/+/86162?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: I250700f989c033fcb63550218c7af8bbfaf01247
Gerrit-Change-Number: 86162
Gerrit-PatchSet: 1
Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Comment-Date: Mon, 27 Jan 2025 04:12:39 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Subrata Banik.
Matt DeVillier has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/85161?usp=email )
Change subject: soc/intel: Assert if `pmc_/gpe0_dwX` values are not unique
......................................................................
Patch Set 5:
(1 comment)
Patchset:
PS5:
> Yes, this is an option for sure but this is still a W/A because 0 means some GPIO community as well that you are programming as Tire-1 GPE. So, each DWORD register sets the 0 values means all GPE will be masked.
if dw0/dw1/dw2 are all zero when passed to `pmc_gpe_init()`, then the POR values are used, so zeros are not programmed. 0/0/0 isn't a valid GPE routing config anyway, so we can use that to fall back on the POR values, and still check if someone has misconfigured via the assert - just carve out one exception
--
To view, visit https://review.coreboot.org/c/coreboot/+/85161?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: I6b4f2f90a858b9ec85145bce0542f1ce61d080be
Gerrit-Change-Number: 85161
Gerrit-PatchSet: 5
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: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Lean Sheng Tan <tanleansheng(a)outlook.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Pranava Y N <pranavayn(a)google.com>
Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tarun <tstuli(a)gmail.com>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com>
Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Comment-Date: Mon, 27 Jan 2025 04:04:17 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)gmail.com>
Attention is currently required from: Hung-Te Lin, Jarried Lin.
Yu-Ping Wu has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/86159?usp=email )
Change subject: soc/mediatek/commmon: Set mcupm mcufw_reserved region to non-cacheable
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/86159?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: I886effd59006e5ad4bfe5bdbc14f057520304835
Gerrit-Change-Number: 86159
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-Attention: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com>
Gerrit-Comment-Date: Mon, 27 Jan 2025 03:53:39 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Subrata Banik has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/85161?usp=email )
Change subject: soc/intel: Assert if `pmc_/gpe0_dwX` values are not unique
......................................................................
Patch Set 5:
(1 comment)
Patchset:
PS5:
> > HI Matt,
> >
> > Can you help me understand how this affects you? If, as you say, lots of devices are failing due to static assertions, why did the build-bots mark this CL as V+1? Shouldn't a static assertion cause the build to fail?
> >
> > Suggestion, if you are thinking avoiding assertion check for undeclared GPE (with default value 0) is good enough ? But note, as per GPE PMC description, zero is also a valid GPE group assignment.
>
> It's manifesting as a run-time assertion for me - as an example, the starlabs/byte-adl doesn't define any GPE routes, builds fine, but boots/halts when the check is called at the end of ramstage. I scanned the tree and found a handful of other ADL-N boards which likewise did not define any GPE routes.
>
> I think we could skip the assertion if dw0 == dw1 == dw2 == 0, and that would resolve the issue
Yes, this is an option for sure but this is still a W/A because 0 means some GPIO community as well that you are programming as Tire-1 GPE. So, each DWORD register sets the 0 values means all GPE will be masked.
--
To view, visit https://review.coreboot.org/c/coreboot/+/85161?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: I6b4f2f90a858b9ec85145bce0542f1ce61d080be
Gerrit-Change-Number: 85161
Gerrit-PatchSet: 5
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: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Lean Sheng Tan <tanleansheng(a)outlook.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Pranava Y N <pranavayn(a)google.com>
Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tarun <tstuli(a)gmail.com>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com>
Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Comment-Date: Mon, 27 Jan 2025 03:44:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)gmail.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/+/86159?usp=email )
Change subject: soc/mediatek/commmon: Set mcupm mcufw_reserved region to non-cacheable
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/86159?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: I886effd59006e5ad4bfe5bdbc14f057520304835
Gerrit-Change-Number: 86159
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-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: Mon, 27 Jan 2025 03:28:06 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
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/+/86159?usp=email )
Change subject: soc/mediatek/commmon: Set mcupm mcufw_reserved region to non-cacheable
......................................................................
Patch Set 5:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/86159/comment/098ff644_5c780de2?us… :
PS4, Line 7: soc/mediatek/mt8196: Set mcupm mcufw_reserved region to non-cacheable
> It's MTK common code, not just 8196.
Done
File src/soc/mediatek/common/mmu_operations.c:
https://review.coreboot.org/c/coreboot/+/86159/comment/2f34397b_58f423b1?us… :
PS4, Line 39: /* mtk_soc_mcufw_reserved set mcufw_reserved to non-cacheable */
> No such "mtk_soc*" function.
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/86159?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: I886effd59006e5ad4bfe5bdbc14f057520304835
Gerrit-Change-Number: 86159
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-Attention: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Attention: Yidi Lin <yidilin(a)google.com>
Gerrit-Comment-Date: Mon, 27 Jan 2025 03:04:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com>
Attention is currently required from: Hung-Te Lin, Jarried 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/+/86159?usp=email
to look at the new patch set (#5).
The following approvals got outdated and were removed:
Code-Review+2 by Yidi Lin, Verified+1 by build bot (Jenkins)
Change subject: soc/mediatek/commmon: Set mcupm mcufw_reserved region to non-cacheable
......................................................................
soc/mediatek/commmon: Set mcupm mcufw_reserved region to non-cacheable
Set mcufw_reserved region to non-cacheable and remove cache operation in
dvfs.c.
TEST=Build pass, boot ok.
Check MMU List by CVD (Codeviser):
0x00113000--0x00123FFF = I:non-cacheable O:non-cacheable
BUG=b:390334489
Change-Id: I886effd59006e5ad4bfe5bdbc14f057520304835
Signed-off-by: Jarried Lin <jarried.lin(a)mediatek.corp-partner.google.com>
---
M src/soc/mediatek/common/include/soc/symbols.h
M src/soc/mediatek/common/mmu_operations.c
M src/soc/mediatek/mt8196/dvfs.c
3 files changed, 7 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/86159/5
--
To view, visit https://review.coreboot.org/c/coreboot/+/86159?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: I886effd59006e5ad4bfe5bdbc14f057520304835
Gerrit-Change-Number: 86159
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-Attention: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Attention: Yidi Lin <yidilin(a)google.com>
Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com>
Attention is currently required from: Subrata Banik.
Matt DeVillier has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/85161?usp=email )
Change subject: soc/intel: Assert if `pmc_/gpe0_dwX` values are not unique
......................................................................
Patch Set 5:
(1 comment)
Patchset:
PS5:
> HI Matt,
>
> Can you help me understand how this affects you? If, as you say, lots of devices are failing due to static assertions, why did the build-bots mark this CL as V+1? Shouldn't a static assertion cause the build to fail?
>
> Suggestion, if you are thinking avoiding assertion check for undeclared GPE (with default value 0) is good enough ? But note, as per GPE PMC description, zero is also a valid GPE group assignment.
It's manifesting as a run-time assertion for me - as an example, the starlabs/byte-adl doesn't define any GPE routes, builds fine, but boots/halts when the check is called at the end of ramstage. I scanned the tree and found a handful of other ADL-N boards which likewise did not define any GPE routes.
I think we could skip the assertion if dw0 == dw1 == dw2 == 0, and that would resolve the issue
--
To view, visit https://review.coreboot.org/c/coreboot/+/85161?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: I6b4f2f90a858b9ec85145bce0542f1ce61d080be
Gerrit-Change-Number: 85161
Gerrit-PatchSet: 5
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: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Lean Sheng Tan <tanleansheng(a)outlook.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Pranava Y N <pranavayn(a)google.com>
Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tarun <tstuli(a)gmail.com>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com>
Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Comment-Date: Mon, 27 Jan 2025 02:56:43 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)gmail.com>
Subrata Banik has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/85161?usp=email )
Change subject: soc/intel: Assert if `pmc_/gpe0_dwX` values are not unique
......................................................................
Patch Set 5:
(1 comment)
Patchset:
PS5:
> FYI, this breaks a ton of devices which use the default GPE routing, which previously worked fine but now assert since no routes defined in devicetree
HI Matt,
Can you help me understand how this affects you? If, as you say, lots of devices are failing due to static assertions, why did the build-bots mark this CL as V+1? Shouldn't a static assertion cause the build to fail?
Suggestion, if you are thinking avoiding assertion check for undeclared GPE (with default value 0) is good enough ? But note, as per GPE PMC description, zero is also a valid GPE group assignment.
--
To view, visit https://review.coreboot.org/c/coreboot/+/85161?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: I6b4f2f90a858b9ec85145bce0542f1ce61d080be
Gerrit-Change-Number: 85161
Gerrit-PatchSet: 5
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: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Lean Sheng Tan <tanleansheng(a)outlook.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Pranava Y N <pranavayn(a)google.com>
Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tarun <tstuli(a)gmail.com>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com>
Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Comment-Date: Mon, 27 Jan 2025 02:34:05 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)gmail.com>
Attention is currently required from: Hung-Te Lin, Jarried Lin.
Yu-Ping Wu has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/86161?usp=email )
Change subject: soc/mediatek/mt8196: Add pi_img loader in ramstage
......................................................................
Patch Set 7: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/86161?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: I571243c3115f5cd005fac88eb740c643e936fca9
Gerrit-Change-Number: 86161
Gerrit-PatchSet: 7
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-Comment-Date: Mon, 27 Jan 2025 00:21:06 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes