coreboot-gerrit November 2019

coreboot-gerrit@coreboot.org

1 participants
2418 discussions

Change in coreboot[master]: soc/intel/tigerlake: Do initial SoC commit till ramstage
by build bot (Jenkins) (Code Review) 05 Nov '19

05 Nov '19

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36087 ) Change subject: soc/intel/tigerlake: Do initial SoC commit till ramstage ...................................................................... Patch Set 13: (1 comment) https://review.coreboot.org/c/coreboot/+/36087/13/src/soc/intel/tigerlake/i… File src/soc/intel/tigerlake/include/soc/ramstage.h: https://review.coreboot.org/c/coreboot/+/36087/13/src/soc/intel/tigerlake/i… PS13, Line 24: void mainboard_silicon_init_params(FSP_S_CONFIG *params); need consistent spacing around '*' (ctx:WxV) -- 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: 13 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: Tue, 05 Nov 2019 16:17:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

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

05 Nov '19

Subrata Banik has uploaded a new patch set (#13) to the change originally created by Ravishankar Sarawadi. ( https://review.coreboot.org/c/coreboot/+/36087 ) Change subject: soc/intel/tigerlake: Do initial SoC commit till ramstage ...................................................................... soc/intel/tigerlake: Do initial SoC commit till ramstage Clone entirely from Icelake List of changes on top off initial icelake clone 1. Replace "Icelake" with "Tigerlake" 2. Replace "icl" with "tgl" 3. Replace "icp" with "tgp" 4. Rename structure based on Icelake with Tigerlake 5. Remove and clean below files 5.a Clean up upd override in fsp_params.c, will be added once FSP available. 5.b Remove __weak functions from fsp_params.c 5.c Remove dGPU over PCIE enable Kconfig option 6. Add CPU/PCH/SA EDS document number and chapter number Tiger Lake specific changes will follow in subsequent patches. 1. Include GPIO controller delta over ICL 2. FSP-S related UPD overrides as applicable "The External Design Specification (EDS) is not published yet. It comes from an authoritative internal source. The patch has been tested on real hardware." Change-Id: Id95e2fa9b7a7c6b3b9233d2c438b25a6c4904bbb Signed-off-by: Ravi Sarawadi <ravishankar.sarawadi(a)intel.com> Signed-off-by: Subrata Banik <subrata.banik(a)intel.com> --- A src/soc/intel/tigerlake/Kconfig A src/soc/intel/tigerlake/Makefile.inc A src/soc/intel/tigerlake/acpi.c A src/soc/intel/tigerlake/chip.c A src/soc/intel/tigerlake/chip.h A src/soc/intel/tigerlake/cpu.c A src/soc/intel/tigerlake/elog.c A src/soc/intel/tigerlake/espi.c A src/soc/intel/tigerlake/finalize.c A src/soc/intel/tigerlake/fsp_params.c A src/soc/intel/tigerlake/gpio.c A src/soc/intel/tigerlake/graphics.c A src/soc/intel/tigerlake/gspi.c A src/soc/intel/tigerlake/i2c.c A src/soc/intel/tigerlake/include/soc/cpu.h A src/soc/intel/tigerlake/include/soc/gpe.h A src/soc/intel/tigerlake/include/soc/gpio.h A src/soc/intel/tigerlake/include/soc/gpio_defs.h A src/soc/intel/tigerlake/include/soc/gpio_soc_defs.h A src/soc/intel/tigerlake/include/soc/irq.h A src/soc/intel/tigerlake/include/soc/itss.h A src/soc/intel/tigerlake/include/soc/msr.h A src/soc/intel/tigerlake/include/soc/nvs.h A src/soc/intel/tigerlake/include/soc/pmc.h A src/soc/intel/tigerlake/include/soc/ramstage.h A src/soc/intel/tigerlake/include/soc/serialio.h A src/soc/intel/tigerlake/include/soc/smm.h A src/soc/intel/tigerlake/include/soc/usb.h A src/soc/intel/tigerlake/lockdown.c A src/soc/intel/tigerlake/p2sb.c A src/soc/intel/tigerlake/pmc.c A src/soc/intel/tigerlake/pmutil.c A src/soc/intel/tigerlake/reset.c A src/soc/intel/tigerlake/sd.c A src/soc/intel/tigerlake/smihandler.c A src/soc/intel/tigerlake/smmrelocate.c A src/soc/intel/tigerlake/spi.c A src/soc/intel/tigerlake/systemagent.c A src/soc/intel/tigerlake/uart.c 39 files changed, 4,765 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/36087/13 -- 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: 13 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-MessageType: newpatchset

1 0

Change in coreboot[master]: superio/winbond/w83627dhg/acpi: Fix IndexField(IndexName, DataName..)
by HAOUAS Elyes (Code Review) 05 Nov '19

05 Nov '19

HAOUAS Elyes has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36598 ) Change subject: superio/winbond/w83627dhg/acpi: Fix IndexField(IndexName, DataName..) ...................................................................... superio/winbond/w83627dhg/acpi: Fix IndexField(IndexName, DataName..) Change-Id: I64fdcbcbbd54334c1c551bc1346c6000ea82c97d Signed-off-by: Elyes HAOUAS <ehaouas(a)noos.fr> --- M src/superio/winbond/w83627dhg/acpi/superio.asl 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/98/36598/1 diff --git a/src/superio/winbond/w83627dhg/acpi/superio.asl b/src/superio/winbond/w83627dhg/acpi/superio.asl index 301ee94..588c476 100644 --- a/src/superio/winbond/w83627dhg/acpi/superio.asl +++ b/src/superio/winbond/w83627dhg/acpi/superio.asl @@ -65,7 +65,7 @@ PNP_ADDR_REG, 8, PNP_DATA_REG, 8 } - IndexField (ADDR, DATA, ByteAcc, NoLock, Preserve) + IndexField (PNP_ADDR_REG,PNP_DATA_REG, ByteAcc, NoLock, Preserve) { Offset (0x07), PNP_LOGICAL_DEVICE, 8, /* Logical device selector */ -- To view, visit https://review.coreboot.org/c/coreboot/+/36598 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I64fdcbcbbd54334c1c551bc1346c6000ea82c97d Gerrit-Change-Number: 36598 Gerrit-PatchSet: 1 Gerrit-Owner: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-MessageType: newchange

2 8

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

05 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 till ramstage ...................................................................... Patch Set 12: (3 comments) > Patch Set 12: > > > > > > > > HI Arthur/Philipp, > > > > > > > > > > > > > > Will you please consider your -2 with this new sets of split CL in place and we have icl base clean up cl also merged ? This will help us to land TGL SOC copy code ? > > > > > > > > > > > > This still has received no real review. It references many docs that I cannot find so how do you expect proper review to happen? > > > > > > > > > > I have access to the documents and would be willing to review. However, > > > > > I want to be fully honest: a review of copy-first patches will take weeks > > > > > if not months. It pushes a lot of work that should be done by the authors > > > > > to the reviewers and will create a bottle-neck. I estimate at least five > > > > > times more work for reviewers this way. Doesn't seem fair, and due to the > > > > > overhead of copy-first patch-later, it might even be less work for me if > > > > > I write the patches by myself. > > > > > > > > Again, document said to have TGL specific changes which are yet to land. this copy patches are placeholder from previous soc to help review only TGL specific changes as per logic. > > > > > > Again, again, again, it eases the review of differences, yes; but it also > > > increases the effort to confirm that everything else really stayed the same > > > by one or two orders of magnitude. > > > > Shouldn't the same we can confirm based on TGL specific CL's for an example: GPIO driver for TGL. If its an incremental change over original ICL gpio code then why should I write again a fresh code? thats the argument i have. > > I don't say you should. But I don't see how this information, that it > was an incremental change, is valuable in our Git history. > > > Given that we don't have trust on SOC vendors that they know what they are doing with new soc. in terms of IP. We are just asking help to make the code land and its matter to have 8-10 CL to make entire ICL to TGL transition. > > That 8-10 sounds bad to me. Few, big commits are 10 times harder to review > than many, small commits. > > > And i have shared the document number so u can verify whatever coming new makes sense or not. > > I think we should also verify that everything that we assume is unchanged > in hardware really didn't change. Otherwise, we'll create hard to track > bugs for the future. > > Let's stick to your GPIO example. In the current patch set, `gpio.c`. How > is one supposed to review this file? its ICL gpio.c is getting copied for TGL so we "assume" TGL is going to use "almost" similar GPIO structure hence this CL just copied the same. You should look for TGL GPIO changes. if you have issue with existing ICL GPIO structure that could have handle here already. Isn't the copy patch originally intended to save review time ? atleast since BSW i'm working in coreeboot, except APL (which was boot from star) we already had copied the previous generation soc. > > * To see if this is the final state of the file, I'd have to look ahead on the patch branch. Time wasted. > * It is not the final state, so I'd have to figure out for every line of the file if that line is in its final state. Incredibly much time wasted. > * If I have to comment on one line but surrounding lines are patched later, on which change shall I comment? => messy review, more time wasted. > > Another example `graphics.c`. This file is flawed, but not changed later > it seems. If we don't go through the pain of reviewing it as part of the > messy review, the errors will stick. Now, in a year or so, somebody might > run into a bug there, they might check the Git history for clues about > the code: Please feel free to fix graphics.c, i believe its same code getting copied over soc and common piece already landed into soc common. if you think something is wrong why to wait for new soc copy ? > > * How is one supposed to know if the code was already checked to be Tiger Lake compatible? I believe that is the work we are doing to do the delta analysis and landing the CL > * If somebody assumes that it was checked but it wasn't, more time gets wasted. Again there is part of SOC vendor work i believe to tell you what is coming new and what is getting copied. i agreed that incremental CL would be good but right now the time we have might not suitable for complete incremental code development from scratch. And based on your feedback i have already splited CL's into smaller group to ease of ICL copy patch review. please hold for TGL specific code review after we could able to land the basic CLs https://review.coreboot.org/c/coreboot/+/36087/12/src/soc/intel/tigerlake/M… File src/soc/intel/tigerlake/Makefile.inc: https://review.coreboot.org/c/coreboot/+/36087/12/src/soc/intel/tigerlake/M… PS12, Line 57: postcar-y += memmap.c > only needed in romstage anymore in master. Needs a rebase. Done https://review.coreboot.org/c/coreboot/+/36087/12/src/soc/intel/tigerlake/p… File src/soc/intel/tigerlake/pmutil.c: https://review.coreboot.org/c/coreboot/+/36087/12/src/soc/intel/tigerlake/p… PS12, Line 140: void > cast to uint8_t * Done https://review.coreboot.org/c/coreboot/+/36087/12/src/soc/intel/tigerlake/s… File src/soc/intel/tigerlake/smmrelocate.c: https://review.coreboot.org/c/coreboot/+/36087/12/src/soc/intel/tigerlake/s… PS12, Line 191: tseg_base > Check if aligned to tseg_size. isn't we did that in line 182 ? -- 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: 12 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: Tue, 05 Nov 2019 16:02:22 +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]: drivers/fsp2_0: drop support for FSP-T
by Nico Huber (Code Review) 05 Nov '19

05 Nov '19

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36622 ) Change subject: drivers/fsp2_0: drop support for FSP-T ...................................................................... Patch Set 4: > Thanks for the detailed explanation Nico, I appreciate that. > I see the problem now, I don't have a solution today, but FSP should address this issue. > In the short term is there any way to reduce the pain in coreboot without removing FSP-T from fsp2_0? > Your FIT idea, microcode address is written in that table. The idea is to put the structure that is passed to FSP-T into it's own section and patch the microcode-update in the final binary, i.e. read offset in CBFS, write that into the new section. Probably not worth the effort, if the FSP binaries can be fixed (and if they can't, I think that shows that FSP is not compatible to coreboot's quality expectations). In very short term, it would already be nice if you could turn it into a build-time error when CPU_MICROCODE_CBFS_LOC is unset and the FSP binary is known to crash. -- To view, visit https://review.coreboot.org/c/coreboot/+/36622 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib791b30b621730f4b7c0a5f668a3b6559245daf5 Gerrit-Change-Number: 36622 Gerrit-PatchSet: 4 Gerrit-Owner: Michael Niewöhner Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: David Guckian Gerrit-Reviewer: David Guckian <david.guckian(a)intel.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> 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: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Vanny E <vanessa.f.eusebio(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 05 Nov 2019 15:57:49 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: drivers/fsp2_0: drop support for FSP-T
by David Guckian (Code Review) 05 Nov '19

05 Nov '19

David Guckian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36622 ) Change subject: drivers/fsp2_0: drop support for FSP-T ...................................................................... Patch Set 4: > Patch Set 4: > > (1 comment) Thanks for the detailed explanation Nico, I appreciate that. I see the problem now, I don't have a solution today, but FSP should address this issue. In the short term is there any way to reduce the pain in coreboot without removing FSP-T from fsp2_0? Your FIT idea, microcode address is written in that table. -- To view, visit https://review.coreboot.org/c/coreboot/+/36622 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib791b30b621730f4b7c0a5f668a3b6559245daf5 Gerrit-Change-Number: 36622 Gerrit-PatchSet: 4 Gerrit-Owner: Michael Niewöhner Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: David Guckian Gerrit-Reviewer: David Guckian <david.guckian(a)intel.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> 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: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Vanny E <vanessa.f.eusebio(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 05 Nov 2019 15:42:56 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

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

05 Nov '19

Tim Wawrzynczak 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 17: (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_ > Seems like the latest patchset is still VB -- are you still going to change this? Yeah -- 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: 17 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: Tue, 05 Nov 2019 15:42:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Comment-In-Reply-To: Joel Kitching <kitching(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common: Make native and FSP-T CAR init mutually exclusive
by Arthur Heymans (Code Review) 05 Nov '19

05 Nov '19

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36168 ) Change subject: soc/intel/common: Make native and FSP-T CAR init mutually exclusive ...................................................................... soc/intel/common: Make native and FSP-T CAR init mutually exclusive Change-Id: Ie723412dcf09151cdbb41e357ad9c2e4f393cb47 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/soc/intel/common/block/cpu/Makefile.inc 1 file changed, 6 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/36168/1 diff --git a/src/soc/intel/common/block/cpu/Makefile.inc b/src/soc/intel/common/block/cpu/Makefile.inc index 3d6adce..c572928 100644 --- a/src/soc/intel/common/block/cpu/Makefile.inc +++ b/src/soc/intel/common/block/cpu/Makefile.inc @@ -1,10 +1,12 @@ +ifneq ($(CONFIG_FSP_CAR),y) bootblock-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CAR) += car/cache_as_ram.S bootblock-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CAR) += ../../../../../cpu/x86/early_reset.S -bootblock-$(CONFIG_FSP_CAR)+= car/cache_as_ram_fsp.S -bootblock-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CPU) += cpulib.c - postcar-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CAR) += car/exit_car.S -postcar-$(CONFIG_FSP_CAR) += car/exit_car_fsp.S +else +bootblock-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CPU)+= car/cache_as_ram_fsp.S +postcar-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CPU) += car/exit_car_fsp.S +endif +bootblock-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CPU) += cpulib.c ramstage-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CPU) += cpulib.c ramstage-$(CONFIG_SOC_INTEL_COMMON_BLOCK_CPU_MPINIT) += mp_init.c -- To view, visit https://review.coreboot.org/c/coreboot/+/36168 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie723412dcf09151cdbb41e357ad9c2e4f393cb47 Gerrit-Change-Number: 36168 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

4 16

Change in coreboot[master]: arch/x86/riscv: Don't link `stages.c` into ramstage
by Nico Huber (Code Review) 05 Nov '19

05 Nov '19

Hello Arthur Heymans, Jonathan Neuschäfer, Philipp Hug, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/36610 to review the following change. Change subject: arch/x86/riscv: Don't link `stages.c` into ramstage ...................................................................... arch/x86/riscv: Don't link `stages.c` into ramstage It's superseded by `ramstage.S`. Change-Id: I81648da2f2af3ad73b3b51471c6fa2daac0540b1 Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M src/arch/riscv/Makefile.inc 1 file changed, 0 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/36610/1 diff --git a/src/arch/riscv/Makefile.inc b/src/arch/riscv/Makefile.inc index 0039fab..16f160e 100644 --- a/src/arch/riscv/Makefile.inc +++ b/src/arch/riscv/Makefile.inc @@ -140,7 +140,6 @@ ramstage-y += misaligned.c ramstage-y += sbi.c ramstage-y += virtual_memory.c -ramstage-y += stages.c ramstage-y += misc.c ramstage-y += smp.c ramstage-y += boot.c -- To view, visit https://review.coreboot.org/c/coreboot/+/36610 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I81648da2f2af3ad73b3b51471c6fa2daac0540b1 Gerrit-Change-Number: 36610 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Jonathan Neuschäfer <j.neuschaefer(a)gmx.net> Gerrit-Reviewer: Philipp Hug <philipp(a)hug.cx> Gerrit-MessageType: newchange

4 4

Change in coreboot[master]: mb/solidrun/solidpc: Do initial commit
by Arthur Heymans (Code Review) 05 Nov '19

05 Nov '19

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/23027 ) Change subject: mb/solidrun/solidpc: Do initial commit ...................................................................... Patch Set 12: (8 comments) https://review.coreboot.org/c/coreboot/+/23027/12/src/mainboard/solidrun/so… File src/mainboard/solidrun/solidpc/Kconfig: https://review.coreboot.org/c/coreboot/+/23027/12/src/mainboard/solidrun/so… PS12, Line 6: ENABLE_BUILTIN_COM1 Not used on braswell (but should be implemented like on ~baytrail) https://review.coreboot.org/c/coreboot/+/23027/12/src/mainboard/solidrun/so… PS12, Line 28: 126 128? https://review.coreboot.org/c/coreboot/+/23027/12/src/mainboard/solidrun/so… File src/mainboard/solidrun/solidpc/Makefile.inc: https://review.coreboot.org/c/coreboot/+/23027/12/src/mainboard/solidrun/so… PS12, Line 20: romstage bootblock? https://review.coreboot.org/c/coreboot/+/23027/12/src/mainboard/solidrun/so… File src/mainboard/solidrun/solidpc/boardid.c: https://review.coreboot.org/c/coreboot/+/23027/12/src/mainboard/solidrun/so… PS12, Line 1: /* : * This file is part of the coreboot project. : * : * Copyright(C) 2013 Google Inc. : * Copyright (C) 2015 Intel Corp. : * Copyright (C) 2017 Andreas Galauner <andreas(a)galauner.de> : * Copyright (C) 2018 3mdeb Embedded Systems Consulting : * : * This program is free software; you can redistribute it and/or modify : * it under the terms of the GNU General Public License as published by : * the Free Software Foundation; version 2 of the License. : * : * This program is distributed in the hope that it will be useful, : * but WITHOUT ANY WARRANTY; without even the implied warranty of : * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the : * GNU General Public License for more details. : */ : : #include <boardid.h> : #include <stdlib.h> : : uint32_t board_id(void) : { : MAYBE_STATIC int id = -1; : : return (uint32_t) id; : } This is a weak function in coreboot_table.c to do this. https://review.coreboot.org/c/coreboot/+/23027/12/src/mainboard/solidrun/so… File src/mainboard/solidrun/solidpc/com_init.c: https://review.coreboot.org/c/coreboot/+/23027/12/src/mainboard/solidrun/so… PS12, Line 26: : uint32_t *pad_config_reg; : : /* Enable the UART hardware for COM1. */ : pci_write_config32(PCI_DEV(0, LPC_DEV, 0), UART_CONT, 1); : : /* : * Set up the pads to select the UART function : * AD12 SW16(UART1_DATAIN/UART0_DATAIN) - Setting Mode 2 for UART0_RXD : * AD10 SW20(UART1_DATAOUT/UART0_DATAOUT) - Setting Mode 2 for UART0_TXD : */ : pad_config_reg = gpio_pad_config_reg(GP_SOUTHWEST, UART1_RXD_PAD); : write32(pad_config_reg, SET_PAD_MODE_SELECTION(PAD_CONFIG0_DEFAULT0, : M2)); : : pad_config_reg = gpio_pad_config_reg(GP_SOUTHWEST, UART1_TXD_PAD); : write32(pad_config_reg, SET_PAD_MODE_SELECTION(PAD_CONFIG0_DEFAULT0, : M2)); This should probably not be in the MB dir. https://review.coreboot.org/c/coreboot/+/23027/12/src/mainboard/solidrun/so… File src/mainboard/solidrun/solidpc/dsdt.asl: https://review.coreboot.org/c/coreboot/+/23027/12/src/mainboard/solidrun/so… PS12, Line 45: Device (RP03) : { : Name (_ADR, 0x001C0002) // _ADR: Address : OperationRegion(RPXX, PCI_Config, 0x00, 0x10) : } copied? unused? https://review.coreboot.org/c/coreboot/+/23027/12/src/mainboard/solidrun/so… File src/mainboard/solidrun/solidpc/smihandler.c: https://review.coreboot.org/c/coreboot/+/23027/12/src/mainboard/solidrun/so… PS12, Line 32: int mainboard_io_trap_handler(int smif) : { : switch (smif) { : case 0x99: : printk(BIOS_DEBUG, "Sample\n"); : smm_get_gnvs()->smif = 0; : break; : default: : return 0; : } : : /* : * On success, the IO Trap Handler returns 0 : * On failure, the IO Trap Handler returns a value != 0 : * : * For now, we force the return value to 0 and log all traps to : * see what's going on. : */ : //gnvs->smif = 0; : return 1; : } : : /* : * The entire 32-bit ALT_GPIO_SMI register is passed as a parameter. Note, that : * this includes the enable bits in the lower 16 bits. : */ : void mainboard_smi_gpi(uint32_t alt_gpio_smi) : { : } remove? https://review.coreboot.org/c/coreboot/+/23027/12/src/mainboard/solidrun/so… PS12, Line 75: : int mainboard_smi_apmc(uint8_t apmc) : { : switch (apmc) { : case APM_CNT_ACPI_ENABLE: : break; : case APM_CNT_ACPI_DISABLE: : break; : } : return 0; : } remove -- To view, visit https://review.coreboot.org/c/coreboot/+/23027 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I00ff95313d74091e7411f6c8658d0d560a0e682b Gerrit-Change-Number: 23027 Gerrit-PatchSet: 12 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> 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: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Comment-Date: Tue, 05 Nov 2019 15:22:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2019