coreboot-gerrit May 2021

coreboot-gerrit@coreboot.org

1 participants
6048 discussions

Change in coreboot[master]: option: Add API for mainboard provided options
by Nico Huber (Code Review) 01 May '21

01 May '21

Attention is currently required from: Bill XIE, Patrick Rudolph, Christian Walter, Julius Werner, Kyösti Mälkki. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52601 ) Change subject: option: Add API for mainboard provided options ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: I think this is not a bad idea in general, but it doesn't seem like what is needed in the follow up. If the board isn't using the CMOS option table (I've not found any cmos.layout), it would not need arbitration between the two option backends. Instead we could have a single active backend (decided at compile time) and that it is called is simply decided by what files are included into the build. Just a thought, it wouldn't need this if (CONFIG(X)) call x, else if... -- To view, visit https://review.coreboot.org/c/coreboot/+/52601 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic5bbb5b3353d862c5533f90ced4824eec27ca3b9 Gerrit-Change-Number: 52601 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Comment-Date: Sat, 01 May 2021 15:14:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: [RFC] device: Introduce new method for setting device states
by Michael Niewöhner (Code Review) 01 May '21

01 May '21

Attention is currently required from: Felix Singer, Nico Huber, Raul Rangel, Furquan Shaikh, Matt DeVillier, Angel Pons, Patrick Rudolph, Felix Held. Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52493 ) Change subject: [RFC] device: Introduce new method for setting device states ...................................................................... Patch Set 17: (1 comment) File src/soc/intel/skylake/fspdevmap.c: https://review.coreboot.org/c/coreboot/+/52493/comment/7477952d_49a2ba54 PS13, Line 15: devmap[i].inverted ^ dev->enabled > Please don't squabble. did we? :-) > This is something every author should decide for themself. What I meant with "personal preference" - so let's have Felix decide that :-) -- To view, visit https://review.coreboot.org/c/coreboot/+/52493 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I70fde306c65a8881f565c5f923be20f380ea64d3 Gerrit-Change-Number: 52493 Gerrit-PatchSet: 17 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Sat, 01 May 2021 14:30:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: src/cpu/x86/smm: remove debug message; not thread safe
by Angel Pons (Code Review) 01 May '21

01 May '21

Attention is currently required from: David Hendricks, Rocky Phagura, Arthur Heymans. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52518 ) Change subject: src/cpu/x86/smm: remove debug message; not thread safe ...................................................................... Patch Set 3: (5 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/52518/comment/161b5713_b71af88d PS1, Line 9: moves the console init code > No. This patch moves a check for an error condition. […] Ack Commit Message: https://review.coreboot.org/c/coreboot/+/52518/comment/0246c1fe_13df12b2 PS3, Line 16: . one period too many? File src/cpu/x86/smm/smm_module_handler.c: https://review.coreboot.org/c/coreboot/+/52518/comment/3be9482a_0a9ffb8e PS1, Line 149: if (cpu > CONFIG_MAX_CPUS) { > Yes, if attempting to print the message doesn't work, we might as well not do it. Ack https://review.coreboot.org/c/coreboot/+/52518/comment/17a5d679_d46dfa99 PS1, Line 149: cpu > CONFIG_MAX_CPUS > The `=` in the check was lost. This should be: […] Ack https://review.coreboot.org/c/coreboot/+/52518/comment/f32d2e42_37e49a85 PS1, Line 157: console_init(); > Do you mean placing the `if (cpu >= CONFIG_MAX_CPUS)` check here? Note that only one thread executes […] Ack -- To view, visit https://review.coreboot.org/c/coreboot/+/52518 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7e8af35d1cde78b327144b6a9da528ae7870e874 Gerrit-Change-Number: 52518 Gerrit-PatchSet: 3 Gerrit-Owner: Rocky Phagura Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Rocky Phagura <rphagura(a)fb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Rocky Phagura Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Sat, 01 May 2021 14:29:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: David Hendricks <david.hendricks(a)gmail.com> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Rocky Phagura Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: src/cpu/x86/smm: remove debug message; not thread safe
by Nico Huber (Code Review) 01 May '21

01 May '21

Attention is currently required from: Angel Pons, Rocky Phagura. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52518 ) Change subject: src/cpu/x86/smm: remove debug message; not thread safe ...................................................................... Patch Set 3: (1 comment) Patchset: PS3: Angel, please check if all your comments have been addressed. -- To view, visit https://review.coreboot.org/c/coreboot/+/52518 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7e8af35d1cde78b327144b6a9da528ae7870e874 Gerrit-Change-Number: 52518 Gerrit-PatchSet: 3 Gerrit-Owner: Rocky Phagura Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Rocky Phagura <rphagura(a)fb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Rocky Phagura Gerrit-Comment-Date: Sat, 01 May 2021 14:28:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/cannonlake: Set MAX_CPUS to 16
by Nico Huber (Code Review) 01 May '21

01 May '21

Attention is currently required from: Felix Singer, Patrick Rudolph, Jeremy Soller, Christian Walter, Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52607 ) Change subject: soc/intel/cannonlake: Set MAX_CPUS to 16 ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > > It doesn't make much sense to provide such a high default for the ULT SKUs. […] Impact on (loaded) binary sizes is expected, not boot times. How much space could be saved depends on future code development and is unpredictable in my experience. -- To view, visit https://review.coreboot.org/c/coreboot/+/52607 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib56fdcfe770ef736a2c5e183481d9f9966570e6d Gerrit-Change-Number: 52607 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Jeremy Soller <jeremy(a)system76.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Sat, 01 May 2021 14:26:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: nb/intel/common: Drop deprecated fixed BAR accessors
by Nico Huber (Code Review) 01 May '21

01 May '21

Attention is currently required from: Angel Pons, Arthur Heymans, Patrick Rudolph. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51885 ) Change subject: nb/intel/common: Drop deprecated fixed BAR accessors ...................................................................... Patch Set 8: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/51885/comment/210c671d_fe23e9d7 PS8, Line 7: nb/intel/common: Drop deprecated fixed BAR accessors : : Now that all code has been switched to make use of the new accessors, : the old ones can be dropped. Follow-ups will clean up bitwise accessors. > So, what do I do? 😄 Well, a) click Edit, write a commit message that matches the changes, click Done, Submit. Or b) split the changes, re-review, ... -- To view, visit https://review.coreboot.org/c/coreboot/+/51885 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib4cb24ca71f3c3717ea50d147ddca74aaf0288fa Gerrit-Change-Number: 51885 Gerrit-PatchSet: 8 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Sat, 01 May 2021 14:17:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: [RFC] device: Introduce new method for setting device states
by Nico Huber (Code Review) 01 May '21

01 May '21

Attention is currently required from: Felix Singer, Raul Rangel, Furquan Shaikh, Matt DeVillier, Angel Pons, Michael Niewöhner, Patrick Rudolph, Felix Held. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52493 ) Change subject: [RFC] device: Introduce new method for setting device states ...................................................................... Patch Set 17: (2 comments) File src/soc/intel/skylake/chip.c: https://review.coreboot.org/c/coreboot/+/52493/comment/a14e55ed_3b35eb25 PS13, Line 461: if (!xdci_can_enable()) : params->XdciEnable = 0; > Agreed, I simply missed that we set it before. […] Oh, no. Now you triggered the software engineer in me... const struct device_enable_map devmap[] = { { &params->XdciEnable, PCH_DEVFN_USBOTG, .filter = xdci_can_enable }, { &params->SomeDisable, PCH_DEVFN_STH, .filter = devmap_invert }, }; i.e. we'd actually only need a single abstract post-processing step. File src/soc/intel/skylake/fspdevmap.c: https://review.coreboot.org/c/coreboot/+/52493/comment/e800e28a_ffae8f35 PS13, Line 15: devmap[i].inverted ^ dev->enabled > `works like` != `is the same` ;) I don't really care... […] Please don't squabble. I merely wanted to point out that there's also an argument for the current order `inverted ^ enabled`. I don't think there is a single correct answer. This is something every author should decide for themself. -- To view, visit https://review.coreboot.org/c/coreboot/+/52493 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I70fde306c65a8881f565c5f923be20f380ea64d3 Gerrit-Change-Number: 52493 Gerrit-PatchSet: 17 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Sat, 01 May 2021 14:13:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: sandybridge,haswell,broadwell: Use DIV_ROUND_CLOSEST macro
by Angel Pons (Code Review) 01 May '21

01 May '21

Attention is currently required from: Nico Huber, Arthur Heymans, Patrick Rudolph. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52651 ) Change subject: sandybridge,haswell,broadwell: Use DIV_ROUND_CLOSEST macro ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/52651/comment/e5d0f4b9_0684dc23 PS2, Line 13: > Do binaries change? I didn't check -- To view, visit https://review.coreboot.org/c/coreboot/+/52651 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I251af82da15049a3a2aa6ea712ae8c9fe859caf6 Gerrit-Change-Number: 52651 Gerrit-PatchSet: 2 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Sat, 01 May 2021 14:06:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: sandybridge,haswell,broadwell: Use DIV_ROUND_CLOSEST macro
by Nico Huber (Code Review) 01 May '21

01 May '21

Attention is currently required from: Angel Pons, Arthur Heymans, Patrick Rudolph. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52651 ) Change subject: sandybridge,haswell,broadwell: Use DIV_ROUND_CLOSEST macro ...................................................................... Patch Set 2: Code-Review+2 (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/52651/comment/0585d272_f3eb6cd6 PS2, Line 13: Do binaries change? -- To view, visit https://review.coreboot.org/c/coreboot/+/52651 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I251af82da15049a3a2aa6ea712ae8c9fe859caf6 Gerrit-Change-Number: 52651 Gerrit-PatchSet: 2 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Sat, 01 May 2021 14:05:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: nb/intel/haswell: Move PEG registers to a separate header
by Nico Huber (Code Review) 01 May '21

01 May '21

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit May 2021