Attention is currently required from: Dinesh Gehlot.
Julius Werner has posted comments on this change by Dinesh Gehlot. ( https://review.coreboot.org/c/coreboot/+/84120?usp=email )
Change subject: vc/google/chromeos: Skip boot info logging if cse sync at payload
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS2:
> We need this change to log boot information accurately. […]
Apologies for missing this, I lost track of some emails in the last few weeks. But honestly, I'm still not convinced this is the right move. We have always had additional resets in depthcharge as well (e.g. due to EC sync or TCPC sync). We picked the end of ramstage for this because it was the most convenient point to implement it (it already has an elog driver that is always used on all platforms anyway), not because it's a huge issue to occasionally log this twice. Depthcharge does not normally write to elog on every boot, so by adding this you are adding the overhead of linking the elog code and initializing the driver (reading the area from flash) a second time to your critical boot path.
But my main concern here is that you are making something more complicated that doesn't deserve to be that complicated. Introducing such a difference between platforms is always an ongoing maintenance burden, even if you were to move the code to commonlib. People will always have to think about where and how exactly the logging works for each particular platform, and may get confused because it works different on some than on others. There is value in keeping things simple.
--
To view, visit https://review.coreboot.org/c/coreboot/+/84120?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: Ia29ec350facc6850c04bb988027ecb146e648a50
Gerrit-Change-Number: 84120
Gerrit-PatchSet: 3
Gerrit-Owner: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Comment-Date: Tue, 17 Sep 2024 00:07:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Dinesh Gehlot <digehlot(a)google.com>
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Attention is currently required from: Kapil Porwal, Paul Menzel, Pranava Y N.
Cliff Huang has posted comments on this change by Cliff Huang. ( https://review.coreboot.org/c/coreboot/+/84297?usp=email )
Change subject: soc/intel/ptl: Define GPE1 macros and register fields
......................................................................
Patch Set 10:
(1 comment)
File src/soc/intel/pantherlake/include/soc/pm.h:
PS9:
> Split the reformatting out into a separate commit?
The actual event bits for GPE1 are PMC I/O registers, which are defines in pm.h. Hence, I keep this header in the same CL. However, I modified the title.
--
To view, visit https://review.coreboot.org/c/coreboot/+/84297?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: Iebf6f6d02b37cc9702e4ee07c1ec0017b6628836
Gerrit-Change-Number: 84297
Gerrit-PatchSet: 10
Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Pranava Y N <pranavayn(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Pranava Y N <pranavayn(a)google.com>
Gerrit-Comment-Date: Mon, 16 Sep 2024 23:52:59 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Attention is currently required from: Bora Guvendik, Cliff Huang.
Hello Bora Guvendik, Subrata Banik, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/84392?usp=email
to look at the new patch set (#2).
Change subject: soc/intel/common/block/acpi: Fix GPE1 blocks to ACPI FADT table
......................................................................
soc/intel/common/block/acpi: Fix GPE1 blocks to ACPI FADT table
Use CONFIG_SOC_INTEL_COMMON_BLOCK_ACPI_HAVE_GPE1 to add GPE1 block
rather than checking if GPE1_STS(0) is '0'.
BUG:362310295
TEST=with the flag, boot google/fatcat or intel/ptlrvp to OS and check
that FADT table includes GPE1. FADT should have:
GPE1 Block Address : 00001810
GPE1 Block Length : 18
GPE1 Base Offset : 80
Without the flag, boot to OS and check that FADT table does not include
GPE1. FADT should have:
GPE1 Block Address : 0
GPE1 Block Length : 0
GPE1 Base Offset : 0
Signed-off-by: Cliff Huang <cliff.huang(a)intel.com>
Change-Id: Idd8115044faff3161ea6bd1cae6c0fe8aa0ff8d7
---
M src/soc/intel/common/block/acpi/acpi.c
1 file changed, 3 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/92/84392/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/84392?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: Idd8115044faff3161ea6bd1cae6c0fe8aa0ff8d7
Gerrit-Change-Number: 84392
Gerrit-PatchSet: 2
Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-CC: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com>
Attention is currently required from: Arthur Heymans, Cliff Huang, Elyes Haouas, Hung-Te Lin, Jérémy Compostella, Lance Zhao, Tim Wawrzynczak, Yidi Lin, Yu-Ping Wu.
Julius Werner has posted comments on this change by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/84208?usp=email )
Change subject: soc/mt6366: Work around GCC LTO build
......................................................................
Patch Set 8:
(1 comment)
File src/soc/mediatek/mt8186/mt6366.c:
https://review.coreboot.org/c/coreboot/+/84208/comment/0aab3012_921fbcf2?us… :
PS8, Line 10: #define NO_BUILDTIME_ASSERT
> > just because one assert statement in a file had this issue the other statements in that file are s […]
Sorry, I'm still trying to understand why this file is somehow more likely to have this problem than others. So you're saying that it is because `mt6366_set_voltage()` contains a big switch-case over different functions that all check different bounds for their argument, and that that specific pattern is what seems to trigger this bug? Okay, fair enough, I guess it makes sense to guard all those checks in advance then.
I still feel like that's a property of a few specific functions that happen to be grouped in this file (together with other assertions that don't fall into this class, e.g. the one in `pmic_get_efuse_votrim()`) and doesn't really mean that we should generally always treat this as a per-file problem. But maybe that's not worth arguing about as long as this is the only file where we're seeing this.
I think the most important thing is that we report this and will hopefully not have to carry the workaround for too long anyway.
--
To view, visit https://review.coreboot.org/c/coreboot/+/84208?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: I6185e87a374f8722dba545d6bbce1c3a8de53e7e
Gerrit-Change-Number: 84208
Gerrit-PatchSet: 8
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com>
Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.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-CC: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com>
Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Attention: Yidi Lin <yidilin(a)google.com>
Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr>
Gerrit-Comment-Date: Mon, 16 Sep 2024 20:47:41 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com>
Attention is currently required from: Arthur Heymans, Christian Walter, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Patrick Rudolph, Shuo Liu, Tim Chu.
Alicja Michalska has posted comments on this change by Alicja Michalska. ( https://review.coreboot.org/c/coreboot/+/84301?usp=email )
Change subject: soc/intel/xeon_sp: Handle GPIO SMIs
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS1:
> After fixing unstable builds.. […]
Added a condition to enable handler only for Lewisburg for now, enabling it for other families will requite a bit more work due to different name/location of gpio.c 😊
Not the best way to handle it, but it will do for now.
--
To view, visit https://review.coreboot.org/c/coreboot/+/84301?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: I906978d2bb2c5e055143aab656e40e06c24ace8d
Gerrit-Change-Number: 84301
Gerrit-PatchSet: 2
Gerrit-Owner: Alicja Michalska <ahplka19(a)gmail.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: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-CC: 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: Mon, 16 Sep 2024 20:33:15 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Christian Walter <christian.walter(a)9elements.com>
Attention is currently required from: Alicja Michalska, Arthur Heymans, Christian Walter, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Patrick Rudolph, Shuo Liu, Tim Chu.
Hello Arthur Heymans, Christian Walter, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Patrick Rudolph, Shuo Liu, Tim Chu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/84301?usp=email
to look at the new patch set (#2).
The following approvals got outdated and were removed:
Code-Review+1 by Christian Walter
Change subject: soc/intel/xeon_sp: Handle GPIO SMIs
......................................................................
soc/intel/xeon_sp: Handle GPIO SMIs
This patch implements GPIO_SMI_STS handler on Lewisburg.
Change-Id: I906978d2bb2c5e055143aab656e40e06c24ace8d
Signed-off-by: Alicja Michalska <alicja.michalska(a)9elements.com>
---
M src/soc/intel/xeon_sp/lbg/Makefile.mk
M src/soc/intel/xeon_sp/smihandler.c
2 files changed, 5 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/84301/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/84301?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: I906978d2bb2c5e055143aab656e40e06c24ace8d
Gerrit-Change-Number: 84301
Gerrit-PatchSet: 2
Gerrit-Owner: Alicja Michalska <ahplka19(a)gmail.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: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Alicja Michalska <ahplka19(a)gmail.com>
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>
Attention is currently required from: Alicja Michalska, Arthur Heymans, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Patrick Rudolph, Shuo Liu, Tim Chu.
Christian Walter has posted comments on this change by Alicja Michalska. ( https://review.coreboot.org/c/coreboot/+/84301?usp=email )
Change subject: soc/intel/xeon_sp: Handle GPIO SMIs
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
Patchset:
PS1:
After fixing unstable builds.. :)
--
To view, visit https://review.coreboot.org/c/coreboot/+/84301?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: I906978d2bb2c5e055143aab656e40e06c24ace8d
Gerrit-Change-Number: 84301
Gerrit-PatchSet: 1
Gerrit-Owner: Alicja Michalska <ahplka19(a)gmail.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: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Alicja Michalska <ahplka19(a)gmail.com>
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: 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: Mon, 16 Sep 2024 20:15:11 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Attention is currently required from: Maximilian Brune.
Felix Held has posted comments on this change by Maximilian Brune. ( https://review.coreboot.org/c/coreboot/+/84376?usp=email )
Change subject: soc/amd/glinda/chipset.cb: Update for glinda
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
> the devicetrees for birman and birman plus with glinda soc also need to be updated to match these ch […]
ah, that's also why the build fails
--
To view, visit https://review.coreboot.org/c/coreboot/+/84376?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: I1138f27bfd47f6fa70a0c2afcc65a5553a609d57
Gerrit-Change-Number: 84376
Gerrit-PatchSet: 2
Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Comment-Date: Mon, 16 Sep 2024 20:14:35 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>
Attention is currently required from: Maximilian Brune.
Felix Held has posted comments on this change by Maximilian Brune. ( https://review.coreboot.org/c/coreboot/+/84376?usp=email )
Change subject: soc/amd/glinda/chipset.cb: Update for glinda
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
the devicetrees for birman and birman plus with glinda soc also need to be updated to match these changes; haven't checked all the details, but the xhci_0 controller is now on a different bus in the chipset devicetree and the mainboard devicetree
--
To view, visit https://review.coreboot.org/c/coreboot/+/84376?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: I1138f27bfd47f6fa70a0c2afcc65a5553a609d57
Gerrit-Change-Number: 84376
Gerrit-PatchSet: 2
Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Comment-Date: Mon, 16 Sep 2024 20:13:47 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No