coreboot-gerrit November 2019

coreboot-gerrit@coreboot.org

1 participants
2418 discussions

Change in coreboot[master]: cpu/x86/tsc: Replace TSC_CONSTANT_RATE with UNKNOWN_TSC_RATE
by Kyösti Mälkki (Code Review) 01 Nov '19

01 Nov '19

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33928 ) Change subject: cpu/x86/tsc: Replace TSC_CONSTANT_RATE with UNKNOWN_TSC_RATE ...................................................................... Patch Set 13: (1 comment) https://review.coreboot.org/c/coreboot/+/33928/12//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/33928/12//COMMIT_MSG@15 PS12, Line 15: For via/epia-m850, switch romstage udelay() from UDELAY_IO to : UDELAY_TSC. > Should we wait for the next release? I think the via platform will be dropped. Well, via/epia-m850 should have been separate change, not squashed here. This commit should be only about flipping TSC_CONSTANT_RATE Kconfig and naming it better. -- To view, visit https://review.coreboot.org/c/coreboot/+/33928 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1690cb80295d6b006b75ed69edea28899b674b68 Gerrit-Change-Number: 33928 Gerrit-PatchSet: 13 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: David Guckian <david.guckian(a)intel.com> Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Huang Jin <huang.jin(a)intel.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Vanny E <vanessa.f.eusebio(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Fri, 01 Nov 2019 11:26:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/tigerlake: Do initial SoC commit
by Arthur Heymans (Code Review) 01 Nov '19

01 Nov '19

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36087 ) Change subject: soc/intel/tigerlake: Do initial SoC commit ...................................................................... Patch Set 10: (1 comment) https://review.coreboot.org/c/coreboot/+/36087/10/src/soc/intel/tigerlake/M… File src/soc/intel/tigerlake/Makefile.inc: https://review.coreboot.org/c/coreboot/+/36087/10/src/soc/intel/tigerlake/M… PS10, Line 1: feq ($(CONFIG_SOC_INTEL_TIGERLAKE),y) > one question. is that all means all stages like > > bootblock-y += somefile.c > verstage-y += somefile.c > romstage-y += somefile.c > postcar-y += somefile.c > ramstage-y += somefile.c > smm-y += somefile.c > > or it excludes smm files? The decision was made to exclude smm as it's not really a stage. So smm-y is still explicitly needed. -- To view, visit https://review.coreboot.org/c/coreboot/+/36087 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id95e2fa9b7a7c6b3b9233d2c438b25a6c4904bbb Gerrit-Change-Number: 36087 Gerrit-PatchSet: 10 Gerrit-Owner: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> 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: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: John Zhao <john.zhao(a)intel.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Michael Niewöhner Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Raj Astekar <raj.astekar(a)intel.com> Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Fri, 01 Nov 2019 10:57:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subrata.banik(a)intel.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/tigerlake: Do initial SoC commit
by Subrata Banik (Code Review) 01 Nov '19

01 Nov '19

1 0

Change in coreboot[master]: [TEST] test CB:36444
by Michael Niewöhner (Code Review) 01 Nov '19

01 Nov '19

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36541 ) Change subject: [TEST] test CB:36444 ...................................................................... Uploaded patch set 2: Patch Set 1 was rebased. -- To view, visit https://review.coreboot.org/c/coreboot/+/36541 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibd9f5e1dd4508bd6cec0875660b2d3d72f2ccecd Gerrit-Change-Number: 36541 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Niewöhner Gerrit-Reviewer: Michael Niewöhner Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Fri, 01 Nov 2019 10:41:50 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: device/Kconfig: hide display menu when NO_GFX_INIT is selected
by Michael Niewöhner (Code Review) 01 Nov '19

01 Nov '19

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36444 ) Change subject: device/Kconfig: hide display menu when NO_GFX_INIT is selected ...................................................................... Uploaded patch set 4. -- To view, visit https://review.coreboot.org/c/coreboot/+/36444 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iec5a47f84b8c776a45edc6f4b31a03b9ac714b4e Gerrit-Change-Number: 36444 Gerrit-PatchSet: 4 Gerrit-Owner: Michael Niewöhner Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Michael Niewöhner Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> 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)users.sourceforge.net> Gerrit-Comment-Date: Fri, 01 Nov 2019 10:41:50 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: device/Kconfig: hide display menu when NO_GFX_INIT is selected
by Michael Niewöhner (Code Review) 01 Nov '19

01 Nov '19

Hello Patrick Rudolph, Arthur Heymans, build bot (Jenkins), Nico Huber, Patrick Georgi, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/36444 to look at the new patch set (#4). Change subject: device/Kconfig: hide display menu when NO_GFX_INIT is selected ...................................................................... device/Kconfig: hide display menu when NO_GFX_INIT is selected Hide display menu when NO_GFX_INIT is selected. This also adds a config to make Jenkins test this. Change-Id: Iec5a47f84b8c776a45edc6f4b31a03b9ac714b4e Signed-off-by: Michael Niewöhner <foss(a)mniewoehner.de> --- A configs/config.no_gfx_init M src/device/Kconfig 2 files changed, 7 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/44/36444/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/36444 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iec5a47f84b8c776a45edc6f4b31a03b9ac714b4e Gerrit-Change-Number: 36444 Gerrit-PatchSet: 4 Gerrit-Owner: Michael Niewöhner Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Michael Niewöhner Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> 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)users.sourceforge.net> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: security/vboot: Build vboot library with same .a that depthcharge uses
by Joel Kitching (Code Review) 01 Nov '19

01 Nov '19

Joel Kitching has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36341 ) Change subject: security/vboot: Build vboot library with same .a that depthcharge uses ...................................................................... Patch Set 14: (1 comment) https://review.coreboot.org/c/coreboot/+/36341/14/src/security/vboot/Makefi… File src/security/vboot/Makefile.inc: https://review.coreboot.org/c/coreboot/+/36341/14/src/security/vboot/Makefi… PS14, Line 113: VB_ Can we call this VBOOT_LIB? -- To view, visit https://review.coreboot.org/c/coreboot/+/36341 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I905b781c3596965ec7ef45a2a7eafe15fdd4d9cc Gerrit-Change-Number: 36341 Gerrit-PatchSet: 14 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Joel Kitching <kitching(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Fri, 01 Nov 2019 10:26:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/tigerlake: Do initial SoC commit
by Subrata Banik (Code Review) 01 Nov '19

01 Nov '19

Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36087 ) Change subject: soc/intel/tigerlake: Do initial SoC commit ...................................................................... Patch Set 10: > Patch Set 10: > > > > > > That doesn't mean we shouldn't review the Tiger Lake addition. And > > > > > I think we all agreed already that a 8k LOC patch isn't reviewable. > > > > > And of course, if we had more common code already, it would be much > > > > > easier... and we would have a better code base if 3ee54bb hadn't > > > > > slipped in. Let's not repeat that error. > > > > > > > > Sure, i believe Arthur has done great work of taking pain to review this CL. we have address majority of his feedback and few more is there. will continue to address also will see if we can submit this CL into 4 part > > > > > > > > 1. Bootblock (soc/intel/tigerlake/bootblock/) > > > > 2. romstage (soc/intel/tigerlake/romstage/) > > > > 3. ramstage (soc/intel/tigerlake/) > > > > 4. acpi (soc/intel/tigerlake/acpi/) > > > > > > I still don't think "this CL" is a good idea, even if split up. Any > > > commit history that first copies stale code and then adapts it in > > > later commits (instead of adding the adapted code step by step) will > > > lose lots of information. > > > > > > For instance, I assume some developers have already looked at the > > > copied code and identified chunks that can stay the same. A copy > > > first, adapt later commit history will not contain this information. > > > It would contain details about what was changed, but not about what > > > wasn't. OTOH, if the commit that adds (pieces of) unchanged code > > > targets a single topic, and the commit message states that this was > > > confirmed to work on the new platform, nobody will have to doubt > > > if that was already checked. And the information will be in the Git > > > history and can help future maintenance. > > > > i understand you are suggesting for incremental patches to create TGL code in upstream. but the philosophy i'm referring is that, we have already reviewed ICL code many time and should't we allow ICL code as is when we are renaming as TGL (this is copy patch logic) and then add only 8 CL to make TGL code complete. Rather we have many CL (definitely more than 8) to even reach at any stage in incremental approach. I agree that incremental approach is easy to review entire code but do we really like to review ICL code again in TGL development? > > Well, I don't think ICL code was much reviewed either. Every line got > a +2 at some point, yes. But that doesn't mean half of it wasn't rubber > stamped. And you seem to completely ignore that reviewing includes to > check available documentation (for the newer silicon and FSP). Yes, you are right in terms of ICL code because of not active chrome product out. But ICL is kind of base for next generation platform hence targeting ICL code. Thanks to all reviewers help to make ICL code in shape, really appreciate all help here. > > If you want to catch subtle difference between ICL and TGL silicon > and FSP interface early, the incremental approach seems so much easier. > If you copy first, please mention in the commit message which parts > were identified to behave exactly the same and which will be adapted > in future commits. Then, reviewers can confirm that. However, if that > requires to look ahead in the patch queue for every other line of code, > you'll likely scare the best reviewers away. yes, will capture those increment changes expectation in commit msg so we know what we are expecting to get at each CL when we split this one in multiple smaller CL -- To view, visit https://review.coreboot.org/c/coreboot/+/36087 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id95e2fa9b7a7c6b3b9233d2c438b25a6c4904bbb Gerrit-Change-Number: 36087 Gerrit-PatchSet: 10 Gerrit-Owner: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> 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: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: John Zhao <john.zhao(a)intel.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Michael Niewöhner Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Raj Astekar <raj.astekar(a)intel.com> Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Fri, 01 Nov 2019 10:21:36 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/tigerlake: Do initial SoC commit
by Arthur Heymans (Code Review) 01 Nov '19

01 Nov '19

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36087 ) Change subject: soc/intel/tigerlake: Do initial SoC commit ...................................................................... Patch Set 10: (1 comment) https://review.coreboot.org/c/coreboot/+/36087/10/src/soc/intel/tigerlake/M… File src/soc/intel/tigerlake/Makefile.inc: https://review.coreboot.org/c/coreboot/+/36087/10/src/soc/intel/tigerlake/M… PS10, Line 1: feq ($(CONFIG_SOC_INTEL_TIGERLAKE),y) > Thanks and this is great. > > btw, do you want me to use alt from ICL or lets start using the same from TGL ? It's always nice to backport some improvements to older targets, but that is never a requirement per se. -- To view, visit https://review.coreboot.org/c/coreboot/+/36087 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id95e2fa9b7a7c6b3b9233d2c438b25a6c4904bbb Gerrit-Change-Number: 36087 Gerrit-PatchSet: 10 Gerrit-Owner: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> 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: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: John Zhao <john.zhao(a)intel.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Michael Niewöhner Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Raj Astekar <raj.astekar(a)intel.com> Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Fri, 01 Nov 2019 10:20:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subrata.banik(a)intel.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/tigerlake: Do initial SoC commit
by Subrata Banik (Code Review) 01 Nov '19

01 Nov '19

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2019