coreboot-gerrit December 2024

coreboot-gerrit@coreboot.org

1 participants
3018 discussions

[M] Change in coreboot[main]: mb/erying/tgl: Cleanup the code
by Felix Singer (Code Review) 19 Dec '24

19 Dec '24

Attention is currently required from: Alicja Michalska. Felix Singer has posted comments on this change by Alicja Michalska. ( https://review.coreboot.org/c/coreboot/+/85641?usp=email ) Change subject: mb/erying/tgl: Cleanup the code ...................................................................... Patch Set 3: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/85641/comment/e1b21536_3c4b31ac?us… : PS3, Line 7: mb/erying/tgl: Cleanup the code > It's a bit more than that. Also, the description mentions a fix. I would split it up. -- To view, visit https://review.coreboot.org/c/coreboot/+/85641?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: I9e395220025fc13e0589472e4732ea1495668554 Gerrit-Change-Number: 85641 Gerrit-PatchSet: 3 Gerrit-Owner: Alicja Michalska <ahplka19(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Alicja Michalska <ahplka19(a)gmail.com> Gerrit-Comment-Date: Thu, 19 Dec 2024 20:19:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>

1 0

[M] Change in coreboot[main]: mb/erying/tgl: Cleanup the code
by Felix Singer (Code Review) 19 Dec '24

19 Dec '24

Attention is currently required from: Alicja Michalska. Felix Singer has posted comments on this change by Alicja Michalska. ( https://review.coreboot.org/c/coreboot/+/85641?usp=email ) Change subject: mb/erying/tgl: Cleanup the code ...................................................................... Patch Set 3: (2 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/85641/comment/e765fcd2_8c9c9699?us… : PS3, Line 7: mb/erying/tgl: Cleanup the code It's a bit more than that. Also, the description mentions a fix. https://review.coreboot.org/c/coreboot/+/85641/comment/53a64aad_e8708846?us… : PS3, Line 11: - Moving SuperIO configuration to bootblock to make devtree cleaner That is meant to be in the devicetree. -- To view, visit https://review.coreboot.org/c/coreboot/+/85641?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: I9e395220025fc13e0589472e4732ea1495668554 Gerrit-Change-Number: 85641 Gerrit-PatchSet: 3 Gerrit-Owner: Alicja Michalska <ahplka19(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Alicja Michalska <ahplka19(a)gmail.com> Gerrit-Comment-Date: Thu, 19 Dec 2024 20:18:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[M] Change in coreboot[main]: Documentation: Add Erying Polestar G613 Pro
by Alicja Michalska (Code Review) 19 Dec '24

19 Dec '24

Attention is currently required from: Angel Pons. Alicja Michalska has posted comments on this change by Alicja Michalska. ( https://review.coreboot.org/c/coreboot/+/81611?usp=email ) Change subject: Documentation: Add Erying Polestar G613 Pro ...................................................................... Patch Set 6: (5 comments) File Documentation/mainboard/erying/tgl_matx.md: https://review.coreboot.org/c/coreboot/+/81611/comment/6b022a4a_d8b93aca?us… : PS5, Line 21: | FD | Intel Flash Descriptor | Optional (if SI_BIOS)| > IFD is always needed, unless flashing the BIOS region only Done https://review.coreboot.org/c/coreboot/+/81611/comment/10920c44_84101fe9?us… : PS5, Line 28: overal > typo: overall […] Done https://review.coreboot.org/c/coreboot/+/81611/comment/435c2b01_673adbda?us… : PS5, Line 89: x3 > PCIe x3 might be a cute face, but I think it's a typo 😄 Yup :D https://review.coreboot.org/c/coreboot/+/81611/comment/406258f9_ef329865?us… : PS5, Line 119: bifurfication > Typo (bifurcation), trailing space and PCIe spelling: […] Acknowledged File Documentation/mainboard/erying/tgl_matx_board.jpg: PS5: > See if you can use something to optimise the image size. […] You were right (as usual) :) -- To view, visit https://review.coreboot.org/c/coreboot/+/81611?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: I5d60508dbde10373b0da2fb4ece0992760d3121c Gerrit-Change-Number: 81611 Gerrit-PatchSet: 6 Gerrit-Owner: Alicja Michalska <ahplka19(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: coreboot org <coreboot.org(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Thu, 19 Dec 2024 20:14:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>

1 0

[M] Change in coreboot[main]: Documentation: Add Erying Polestar G613 Pro
by Alicja Michalska (Code Review) 19 Dec '24

19 Dec '24

Attention is currently required from: Alicja Michalska. Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/81611?usp=email to look at the new patch set (#6). The following approvals got outdated and were removed: Verified-1 by build bot (Jenkins) Change subject: Documentation: Add Erying Polestar G613 Pro ...................................................................... Documentation: Add Erying Polestar G613 Pro Document the board and process of building/flashing coreboot on it. Change-Id: I5d60508dbde10373b0da2fb4ece0992760d3121c Signed-off-by: Alicja Michalska <ahplka19(a)gmail.com> --- A Documentation/mainboard/erying/tgl/tgl_matx.md A Documentation/mainboard/erying/tgl/tgl_matx_board.jpg M Documentation/mainboard/index.md 3 files changed, 176 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/81611/6 -- To view, visit https://review.coreboot.org/c/coreboot/+/81611?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: I5d60508dbde10373b0da2fb4ece0992760d3121c Gerrit-Change-Number: 81611 Gerrit-PatchSet: 6 Gerrit-Owner: Alicja Michalska <ahplka19(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: coreboot org <coreboot.org(a)gmail.com> Gerrit-Attention: Alicja Michalska <ahplka19(a)gmail.com>

1 0

[L] Change in coreboot[main]: drivers/amd/opensil/mpio: Factor out common MPIO symbols from vendorcode
by Felix Held (Code Review) 19 Dec '24

19 Dec '24

1 0

[M] Change in coreboot[main]: drivers/amd/opensil/romstage.c: Implement cbmem_top_chipset in driver
by Felix Held (Code Review) 19 Dec '24

19 Dec '24

Attention is currently required from: Nick Kochlowski. Felix Held has posted comments on this change by Nick Kochlowski. ( https://review.coreboot.org/c/coreboot/+/85635?usp=email ) Change subject: drivers/amd/opensil/romstage.c: Implement cbmem_top_chipset in driver ...................................................................... Patch Set 6: (1 comment) File src/drivers/amd/opensil/romstage.c: https://review.coreboot.org/c/coreboot/+/85635/comment/dd70b7bd_ad5b4b2f?us… : PS6, Line 13: size so > I know this was just moved, but it doesn't make sense. […] should be 'size to' -- To view, visit https://review.coreboot.org/c/coreboot/+/85635?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: Ibc79456b0429cdd3d8e3fa5c224799a05add8359 Gerrit-Change-Number: 85635 Gerrit-PatchSet: 6 Gerrit-Owner: Nick Kochlowski <nickkochlowski(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Attention: Nick Kochlowski <nickkochlowski(a)gmail.com> Gerrit-Comment-Date: Thu, 19 Dec 2024 17:53:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Martin Roth <martin.roth(a)amd.corp-partner.google.com>

1 0

[L] Change in coreboot[main]: drivers/amd/opensil/mpio: Factor out common MPIO symbols from vendorcode
by Felix Held (Code Review) 19 Dec '24

19 Dec '24

Attention is currently required from: Fred Reitberger, Jason Glenesk, Martin L Roth, Martin Roth, Matt DeVillier, Nick Kochlowski, Varshit Pandya. Felix Held has posted comments on this change by Nick Kochlowski. ( https://review.coreboot.org/c/coreboot/+/85632?usp=email ) Change subject: drivers/amd/opensil/mpio: Factor out common MPIO symbols from vendorcode ...................................................................... Patch Set 6: (5 comments) Patchset: PS6: > I wouldn't insist on it, but in my opinion, this would be better split into a few different patches […] splitting this change into multiple ones would probably be more difficult than necessary, since this is very intertwined. at least to me this patch still does one thing and it's still a reviewable patch size, so i'd keep it as one patch File src/drivers/amd/opensil/mpio/chip.c: https://review.coreboot.org/c/coreboot/+/85632/comment/bfa36c21_5ad1e8b1?us… : PS6, Line 8: struct chip_operations drivers_amd_opensil_mpio_ops = { : .name = "AMD MPIO", : }; > Why do these go in chip.c? Obviously the *can* go here, but that's not typically the case. […] i'd say that the chip.c is the right place for this specific one. if you want to rename the whole file to mpio.c, i won't oppose to that, but i don't see a strong reason to do that. beware that this specific chip driver works differently compared to most other chip drivers, but this was the cleanest solution i was able come up with. the previous solution ran this code not exactly at the time where it should have done that; now that's not a problem any more. it is indeed a bit odd chip driver though where the code doesn't get called from the chip ops, but the soc code calls into this. iirc the previous timing issue was when there were some other devices or chips below the pci device below the mpio chip, but since the soc code just calls this at the right time and the code then processes all mpio chip information, this should just work in any case File src/vendorcode/amd/opensil/Kconfig: https://review.coreboot.org/c/coreboot/+/85632/comment/8154aa16_a48b188e?us… : PS6, Line 28: ../../../../vendorcode > '$(top)/src/vendorcode' ? if this works, that would be preferrable. not sure if that will work, but would be great if you can test this File src/vendorcode/amd/opensil/genoa_poc/mpio/chip.c: https://review.coreboot.org/c/coreboot/+/85632/comment/a691452a_a8d46451?us… : PS6, Line 134: // /\* \*/ comment style https://review.coreboot.org/c/coreboot/+/85632/comment/7a353b75_177a5268?us… : PS6, Line 202: } i think this is missing the final newline -- To view, visit https://review.coreboot.org/c/coreboot/+/85632?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: I8b1f92f08565216dd93203a06015e3eec1e7bb69 Gerrit-Change-Number: 85632 Gerrit-PatchSet: 6 Gerrit-Owner: Nick Kochlowski <nickkochlowski(a)gmail.com> Gerrit-Reviewer: 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 L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Varshit Pandya <pandyavarshit(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: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Nick Kochlowski <nickkochlowski(a)gmail.com> Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Comment-Date: Thu, 19 Dec 2024 17:53:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Martin Roth <martin.roth(a)amd.corp-partner.google.com>

1 0

[M] Change in coreboot[main]: drivers/amd/opensil: Add openSIL timepoint calls
by Felix Held (Code Review) 19 Dec '24

19 Dec '24

Attention is currently required from: Fred Reitberger, Jason Glenesk, Martin L Roth, Matt DeVillier, Nick Kochlowski, Varshit Pandya. Felix Held has posted comments on this change by Nick Kochlowski. ( https://review.coreboot.org/c/coreboot/+/85631?usp=email ) Change subject: drivers/amd/opensil: Add openSIL timepoint calls ...................................................................... Patch Set 6: (3 comments) File src/drivers/amd/opensil/Kconfig: https://review.coreboot.org/c/coreboot/+/85631/comment/a93d8403_24c1d56b?us… : PS6, Line 5: depends on SOC_AMD_OPENSIL > I don't think this is needed, or actually even useful. […] i asked for that one to be put back in some earlier version, but i don't have a super strong opinion on that one. my reasoning was that in order to avoid the build failing when only one option is selected, it might be good to have the dependency there; not 100% convinced that selecting SOC_AMD_OPENSIL here would be a better solution, since selecting SOC_AMD_OPENSIL also requires selecting one of the SOC_AMD_OPENSIL_\* options and i don't think that turning the SOC_AMD_OPENSIL_\* into a kconfig choice with the stub being the default, since i think we can't easily add another choice option from site-local there File src/drivers/amd/opensil/opensil.h: https://review.coreboot.org/c/coreboot/+/85631/comment/58dc7d88_47178e37?us… : PS6, Line 3: _OPENSIL_DRIVER_H_ > Nit: maybe "DRIVER_AMD_OPENSIL", without the underscores? Or not. […] good point, now that i've thought about that one, i second that File src/soc/amd/phoenix/chip.c: https://review.coreboot.org/c/coreboot/+/85631/comment/8a659c10_80617e5b?us… : PS6, Line 3: /* TODO: Update for Phoenix */ > Not related to this patch, but maybe we can get rid of this todo soon? i probably just forgot to remove that one. but yes, that can be removed; probably something for a separate patch -- To view, visit https://review.coreboot.org/c/coreboot/+/85631?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: If0559fc0ff0ec55e9ef131e5ed20dfb5baa651da Gerrit-Change-Number: 85631 Gerrit-PatchSet: 6 Gerrit-Owner: Nick Kochlowski <nickkochlowski(a)gmail.com> Gerrit-Reviewer: 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 L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-CC: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Attention: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Nick Kochlowski <nickkochlowski(a)gmail.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Comment-Date: Thu, 19 Dec 2024 17:42:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Martin Roth <martin.roth(a)amd.corp-partner.google.com>

1 0

[M] Change in coreboot[main]: drivers/amd/opensil/romstage.c: Implement cbmem_top_chipset in driver
by Martin Roth (Code Review) 19 Dec '24

19 Dec '24

Attention is currently required from: Felix Held, Nick Kochlowski. Martin Roth has posted comments on this change by Nick Kochlowski. ( https://review.coreboot.org/c/coreboot/+/85635?usp=email ) Change subject: drivers/amd/opensil/romstage.c: Implement cbmem_top_chipset in driver ...................................................................... Patch Set 6: (1 comment) File src/drivers/amd/opensil/romstage.c: https://review.coreboot.org/c/coreboot/+/85635/comment/34b43418_45587e2e?us… : PS6, Line 13: size so I know this was just moved, but it doesn't make sense. Should this be "size, so" or "size to"? -- To view, visit https://review.coreboot.org/c/coreboot/+/85635?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: Ibc79456b0429cdd3d8e3fa5c224799a05add8359 Gerrit-Change-Number: 85635 Gerrit-PatchSet: 6 Gerrit-Owner: Nick Kochlowski <nickkochlowski(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-CC: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Attention: Nick Kochlowski <nickkochlowski(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Thu, 19 Dec 2024 17:09:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[S] Change in coreboot[main]: drivers/amd/opensil/acpi.c: Factor common ACPI calls to openSIL driver
by Martin Roth (Code Review) 19 Dec '24

19 Dec '24

Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk, Martin L Roth, Matt DeVillier, Nick Kochlowski, Varshit Pandya. Martin Roth has posted comments on this change by Nick Kochlowski. ( https://review.coreboot.org/c/coreboot/+/85633?usp=email ) Change subject: drivers/amd/opensil/acpi.c: Factor common ACPI calls to openSIL driver ...................................................................... Patch Set 6: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/85633?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: I9fa4f60164333ec7a268702fa3e94979a1b83594 Gerrit-Change-Number: 85633 Gerrit-PatchSet: 6 Gerrit-Owner: Nick Kochlowski <nickkochlowski(a)gmail.com> Gerrit-Reviewer: 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 L Roth <gaumless(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: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-Attention: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Nick Kochlowski <nickkochlowski(a)gmail.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Thu, 19 Dec 2024 17:05:01 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit December 2024