Sean Rhodes has posted comments on this change by Sean Rhodes. ( https://review.coreboot.org/c/coreboot/+/81409?usp=email )
Change subject: i2c/drivers/generic: Add support for including a CDM
......................................................................
Patch Set 12:
(1 comment)
Patchset:
PS2:
> Could you please provide links for both, if possible?
AMI CRB is protected by about 50 million NDAs
--
To view, visit https://review.coreboot.org/c/coreboot/+/81409?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: If70c25288d835df7064b4051c43abeb2d6531f3b
Gerrit-Change-Number: 81409
Gerrit-PatchSet: 12
Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
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-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Wed, 07 Aug 2024 10:44:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Sean Rhodes <sean(a)starlabs.systems>
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)gmail.com>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Attention is currently required from: Sean Rhodes.
Angel Pons has posted comments on this change by Sean Rhodes. ( https://review.coreboot.org/c/coreboot/+/81409?usp=email )
Change subject: i2c/drivers/generic: Add support for including a CDM
......................................................................
Patch Set 12:
(1 comment)
Patchset:
PS2:
> I added that to Linux […]
Could you please provide links for both, if possible?
--
To view, visit https://review.coreboot.org/c/coreboot/+/81409?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: If70c25288d835df7064b4051c43abeb2d6531f3b
Gerrit-Change-Number: 81409
Gerrit-PatchSet: 12
Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
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-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Comment-Date: Wed, 07 Aug 2024 10:33:34 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Sean Rhodes <sean(a)starlabs.systems>
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)gmail.com>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Attention is currently required from: Felix Singer, Jérémy Compostella, yuchi.chen(a)intel.com.
Shuo Liu has posted comments on this change by yuchi.chen(a)intel.com. ( https://review.coreboot.org/c/coreboot/+/83317?usp=email )
Change subject: soc/intel/common/block/gpmr: Allow soc to have specific gpmr definition
......................................................................
Patch Set 9:
(1 comment)
File src/soc/intel/common/block/include/intelblocks/gpmr.h:
https://review.coreboot.org/c/coreboot/+/83317/comment/8eb6ad5e_1d48a54f?us… :
PS3, Line 10: #include <soc/gpmr.h>
> Can we rename to, e.g. USE_SOC_PCR_GPMR_DEFINITION and handle in pcr_gpmr.h? […]
To remove the word 'PCR' in the config item might make sense (e.g. USE_SOC_GPMR_DEFS), since the same mechanism could also fit ioc_gpmr.h (though at the moment no need to change).
--
To view, visit https://review.coreboot.org/c/coreboot/+/83317?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: I94797a72af75fc96ab2cacb1d46b581605a15387
Gerrit-Change-Number: 83317
Gerrit-PatchSet: 9
Gerrit-Owner: yuchi.chen(a)intel.com
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.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>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Attention: yuchi.chen(a)intel.com
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Comment-Date: Wed, 07 Aug 2024 10:25:46 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: yuchi.chen(a)intel.com
Comment-In-Reply-To: Shuo Liu <shuo.liu(a)intel.com>
Sean Rhodes has posted comments on this change by Sean Rhodes. ( https://review.coreboot.org/c/coreboot/+/81409?usp=email )
Change subject: i2c/drivers/generic: Add support for including a CDM
......................................................................
Patch Set 12:
(1 comment)
Patchset:
PS2:
> @sean@starlabs. […]
I added that to Linux
In the DSDT
--
To view, visit https://review.coreboot.org/c/coreboot/+/81409?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: If70c25288d835df7064b4051c43abeb2d6531f3b
Gerrit-Change-Number: 81409
Gerrit-PatchSet: 12
Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
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-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Wed, 07 Aug 2024 10:21:12 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Sean Rhodes <sean(a)starlabs.systems>
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)gmail.com>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
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 4:
(3 comments)
File util/cbfstool/cbfs-payload-linux.c:
https://review.coreboot.org/c/coreboot/+/83615/comment/9a3d8fde_b4c7a7d0?us… :
PS4, Line 58: }
> This shouldn't get lost.
Right.
I moved the error handling out of `bzp_init` in the latest patchset.
I will probably move the error handling to `bzp_output_segment` in CB:83617 since it is the only place where we actually use the compression function.
https://review.coreboot.org/c/coreboot/+/83615/comment/00238104_81d6902f?us… :
PS4, Line 180: * size (ie. incompressible data)?
> Does this still make sense?
That is being dealt with in the last patch of the patchtrain.
https://review.coreboot.org/c/coreboot/+/83615/comment/803eb819_dc2ffff7?us… :
PS4, Line 190:
> I'd do the compression_function() lookup here, right away. […]
I did that before, but I need the `algo` parameter in `bzp_output_segment` because I need to set the compression attribute of the segment to `algo`. And it seemed overkill to give `bzp_output_segment` both the algorithm as well as the compression function as argument.
--
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: 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: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Comment-Date: Wed, 07 Aug 2024 10:17:42 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
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 (#5).
The following approvals got outdated and were removed:
Code-Review+1 by Angel Pons, Code-Review+1 by Nico Huber, 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, 24 insertions(+), 21 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/83615/5
--
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: 5
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: Sean Rhodes.
Angel Pons has posted comments on this change by Sean Rhodes. ( https://review.coreboot.org/c/coreboot/+/81409?usp=email )
Change subject: i2c/drivers/generic: Add support for including a CDM
......................................................................
Patch Set 12:
(1 comment)
Patchset:
PS2:
> Looks like the Linux driver only checks `ROTM`: https://github. […]
@sean@starlabs.systems Where do you see this in the CRB?
--
To view, visit https://review.coreboot.org/c/coreboot/+/81409?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: If70c25288d835df7064b4051c43abeb2d6531f3b
Gerrit-Change-Number: 81409
Gerrit-PatchSet: 12
Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
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-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Comment-Date: Wed, 07 Aug 2024 10:16:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Sean Rhodes <sean(a)starlabs.systems>
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)gmail.com>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>