Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk, Matt DeVillier, ritul guru.
Martin Roth has posted comments on this change by Felix Held. ( https://review.coreboot.org/c/coreboot/+/83777?usp=email )
Change subject: soc/amd/common/psp_smi_flash: implement SPI read/write/erase command
......................................................................
Patch Set 3: Code-Review+2
(1 comment)
File src/soc/amd/common/block/psp/psp_smi_flash.c:
https://review.coreboot.org/c/coreboot/+/83777/comment/584f5dd1_75faac9c?us… :
PS1, Line 237: }
> updated the commit message of the previous patch since that's the one that first uses spi_controller […]
Sounds good.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83777?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: I4957a6d316015cc7037acf52facb6cc69188d446
Gerrit-Change-Number: 83777
Gerrit-PatchSet: 3
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: ritul guru <ritul.bits(a)gmail.com>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: ritul guru <ritul.bits(a)gmail.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Tue, 06 Aug 2024 17:04:22 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>
Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk, Matt DeVillier, ritul guru.
Martin Roth has posted comments on this change by Felix Held. ( https://review.coreboot.org/c/coreboot/+/83775?usp=email )
Change subject: soc/amd/common/psp_smi_flash: add spi_controller_available
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/83775?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: I49218e03a5dd555b2b2d34eaad86673e9fc908c3
Gerrit-Change-Number: 83775
Gerrit-PatchSet: 2
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: ritul guru <ritul.bits(a)gmail.com>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: ritul guru <ritul.bits(a)gmail.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Tue, 06 Aug 2024 17:03:00 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk, ritul guru.
Martin Roth has posted comments on this change by Felix Held. ( https://review.coreboot.org/c/coreboot/+/83740?usp=email )
Change subject: soc/amd/common/psp_smi: implement P2C mailbox handling
......................................................................
Patch Set 5: Code-Review+2
(2 comments)
Patchset:
PS4:
> i'd like to keep this in this folder, since it belongs thematically to the other files in there and […]
Hrmph. I disagree, but it's your decision.
File src/soc/amd/common/block/psp/psp_smi.c:
https://review.coreboot.org/c/coreboot/+/83740/comment/e9ca93ee_d3617959?us… :
PS4, Line 80: u32
> the existing parts of the psp code in coreboot already use a mix of both; i can look into using only […]
Sounds good.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83740?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: I50479bed2332addae652026c6818460eeb6403af
Gerrit-Change-Number: 83740
Gerrit-PatchSet: 5
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: ritul guru <ritul.bits(a)gmail.com>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: ritul guru <ritul.bits(a)gmail.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Tue, 06 Aug 2024 17:02:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>
Attention is currently required from: Angel Pons, Felix Singer, Julius Werner, Lean Sheng Tan, Nico Huber.
Maximilian Brune has posted comments on this change by Maximilian Brune. ( https://review.coreboot.org/c/coreboot/+/83615?usp=email )
Change subject: util/cbfstool/cbfs-payload-linux: Do not compress bzImage
......................................................................
Patch Set 3:
(3 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/83615/comment/90ac96c3_fdfcbef2?us… :
PS3, Line 12: erros
> typo: errors
Done
File util/cbfstool/cbfs-payload-linux.c:
https://review.coreboot.org/c/coreboot/+/83615/comment/1622c488_d23a4766?us… :
PS3, Line 159: comp_func_ptr compress_func = compression_function(algo);
> Looks like `algo` only needs to be different when `type == PAYLOAD_SEGMENT_CODE`. […]
In my opinion that makes it look like PAYLOAD_SEGMENT_CODE is the deciding factor upon which to decide whether to compress or not. But PAYLOAD_SEGMENT_CODE doesn't really have anything to do with the decision whether or not to compress. It is mostly just coincidence, that the bzImage is the code segment and other ones aren't.
I am also a fan of removing it from the global `struct bzpayload` state, since global states are at least for me usually harder to keep track of and read. And it is just used in this single place so I don't see a reason to have a global state for it in the first place.
If you don't mind I would leave it like that.
https://review.coreboot.org/c/coreboot/+/83615/comment/5e4e862e_6ad6f63c?us… :
PS3, Line 294: // There is no point in compressing the bzImage (it is already compressed)
> nit: consistent comment style
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/83615?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: I022982667515ce721d98af534414d9e336b5f35a
Gerrit-Change-Number: 83615
Gerrit-PatchSet: 3
Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Comment-Date: Tue, 06 Aug 2024 16:56:52 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Attention is currently required from: Sean Rhodes.
Martin Roth has posted comments on this change by Sean Rhodes. ( https://review.coreboot.org/c/coreboot/+/83623?usp=email )
Change subject: mb/starlabs/starbook/rpl: Add USB ACPi to devicetree
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS3:
ping
--
To view, visit https://review.coreboot.org/c/coreboot/+/83623?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: Iab8b6e03c8c05e459fb354bc008109c873a4846f
Gerrit-Change-Number: 83623
Gerrit-PatchSet: 3
Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Comment-Date: Tue, 06 Aug 2024 16:55:55 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Angel Pons, Felix Singer, Julius Werner, Lean Sheng Tan, Maximilian Brune, Nico Huber.
Hello Angel Pons, Felix Singer, Julius Werner, Lean Sheng Tan, Nico Huber, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/83615?usp=email
to look at the new patch set (#4).
The following approvals got outdated and were removed:
Code-Review+1 by Angel Pons, Verified+1 by build bot (Jenkins)
Change subject: util/cbfstool/cbfs-payload-linux: Do not compress bzImage
......................................................................
util/cbfstool/cbfs-payload-linux: Do not compress bzImage
Compressing the already compressed bzImage does not yield any
fruit. If you are lucky it actually makes the image a little bit
smaller. If you are unlucky the image actually gets bigger and since the
compressing function is not checked for any errors, coreboot just builds
successfully even though the payload is broken through compression.
Before this patch you could possibly get this error during compilation:
```
E: LZMA: LzmaEnc_Encode failed 9.
```
and your linux payload would end up something like this in CBFS:
```
FMAP REGION: COREBOOT
Name Offset Type Size Comp
....
fallback/payload 0x1c9c0 simple elf 511 none
....
```
That doesn't stop coreboot from finishing the build though, since we
currently don't check for errors from the compression. That is an issue
for another patch though.
Tested:
Build and run QEMU-Q35 with Linux bzImage as payload.
Change-Id: I022982667515ce721d98af534414d9e336b5f35a
Signed-off-by: Maximilian Brune <maximilian.brune(a)9elements.com>
---
M util/cbfstool/cbfs-payload-linux.c
1 file changed, 19 insertions(+), 21 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/83615/4
--
To view, visit https://review.coreboot.org/c/coreboot/+/83615?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: I022982667515ce721d98af534414d9e336b5f35a
Gerrit-Change-Number: 83615
Gerrit-PatchSet: 4
Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk, Martin Roth, ritul guru.
Matt DeVillier has posted comments on this change by Felix Held. ( https://review.coreboot.org/c/coreboot/+/83740?usp=email )
Change subject: soc/amd/common/psp_smi: implement P2C mailbox handling
......................................................................
Patch Set 5: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/83740?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: I50479bed2332addae652026c6818460eeb6403af
Gerrit-Change-Number: 83740
Gerrit-PatchSet: 5
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: ritul guru <ritul.bits(a)gmail.com>
Gerrit-CC: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: ritul guru <ritul.bits(a)gmail.com>
Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Tue, 06 Aug 2024 16:53:47 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Martin Roth has submitted this change. ( https://review.coreboot.org/c/coreboot/+/83315?usp=email )
Change subject: soc/intel/common/intelblocks/gpio.h: Allow specifying the pad ownership
......................................................................
soc/intel/common/intelblocks/gpio.h: Allow specifying the pad ownership
Add pad_own_reg_0 to `struct pad_community`. Pad ownership indicates
whether the GPIO is owned by host or Intel Management Engine. If owned
by host, then host ownership indicates whether the GPIO is owned by ACPI
or driver.
Change-Id: I30a934fd00a7a42cb156341da1954e4e4b1231d8
Signed-off-by: Yuchi Chen <yuchi.chen(a)intel.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/83315
Reviewed-by: Shuo Liu <shuo.liu(a)intel.com>
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
---
M src/soc/intel/common/block/include/intelblocks/gpio.h
1 file changed, 2 insertions(+), 1 deletion(-)
Approvals:
Shuo Liu: Looks good to me, approved
build bot (Jenkins): Verified
diff --git a/src/soc/intel/common/block/include/intelblocks/gpio.h b/src/soc/intel/common/block/include/intelblocks/gpio.h
index a7bb332..8e60a16 100644
--- a/src/soc/intel/common/block/include/intelblocks/gpio.h
+++ b/src/soc/intel/common/block/include/intelblocks/gpio.h
@@ -121,7 +121,8 @@
Number of pads bit mapped in each GPI status/en and Host Own Reg */
gpio_t first_pad; /* first pad in community */
gpio_t last_pad; /* last pad in community */
- uint16_t host_own_reg_0; /* offset to Host Ownership Reg 0 */
+ uint16_t pad_own_reg_0; /* offset to Pad Ownership (host or CSME) Reg 0 */
+ uint16_t host_own_reg_0; /* offset to Host Ownership (ACPI or driver) Reg 0 */
uint16_t gpi_int_sts_reg_0; /* offset to GPI Int STS Reg 0 */
uint16_t gpi_int_en_reg_0; /* offset to GPI Int Enable Reg 0 */
uint16_t gpi_smi_sts_reg_0; /* offset to GPI SMI STS Reg 0 */
--
To view, visit https://review.coreboot.org/c/coreboot/+/83315?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I30a934fd00a7a42cb156341da1954e4e4b1231d8
Gerrit-Change-Number: 83315
Gerrit-PatchSet: 9
Gerrit-Owner: yuchi.chen(a)intel.com
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>