Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45912 )
Change subject: mb/google/dedede/var/waddledee: Enable GPIO based I2C Multiplexer
......................................................................
Patch Set 3:
This change is ready for review.
--
To view, visit https://review.coreboot.org/c/coreboot/+/45912
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9b09e063b4377587019ade9e6e194f4aadcdd312
Gerrit-Change-Number: 45912
Gerrit-PatchSet: 3
Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Evan Green <evgreen(a)chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Justin TerAvest <teravest(a)chromium.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Comment-Date: Fri, 02 Oct 2020 07:11:09 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45889 )
Change subject: mb/google/volteer: Expand WP_RO region to 8MB in fmap
......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45889/2/src/mainboard/google/volte…
File src/mainboard/google/volteer/chromeos.fmd:
https://review.coreboot.org/c/coreboot/+/45889/2/src/mainboard/google/volte…
PS2, Line 9: # SPI flash only the top 16MiB actually gets memory mapped.
> That is correct, x86 only memory maps the top 16 MiB of SPI flash, so in this case RW_SECTION_A ends […]
+1 to what Julius and Tim said. The RW_SECTIONs need to stay above 16MiB so that they are memory mapped.
About reducing the RW sizes by 2MiB each - I think it might be really difficult given the size of CSME RW binary. Has any one actually performed an analysis of how much we are actually using in each of these sections and what the worst case sizes would be? Karthik put together a nice worksheet for JSL(dedede): https://docs.google.com/spreadsheets/d/1FtivbWNQ78ozU5b5Ds10q-jZ09jRfa8Qb5G…. We need something similar for TGL(volteer) as well to understand the actual usage.
--
To view, visit https://review.coreboot.org/c/coreboot/+/45889
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I72fe8b6a1f91390c218230c0c20825769ebd1e0b
Gerrit-Change-Number: 45889
Gerrit-PatchSet: 2
Gerrit-Owner: Caveh Jalali <caveh(a)chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(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, 02 Oct 2020 07:05:52 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Gerrit-MessageType: comment
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45131 )
Change subject: lib/Makefile.inc: fail build when SPD would be empty
......................................................................
Patch Set 14:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45131/14/src/lib/Makefile.inc
File src/lib/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/45131/14/src/lib/Makefile.inc@360
PS14, Line 360: test -n "$(SPD_SOURCES)" || \
: (echo "HAVE_SPD_BIN_IN_CBFS is set but SPD_SOURCES is empty" && exit 1)
In my opinion, we should allow a board to have empty SPD_SOURCES even when HAVE_SPD_IN_CBFS is set. This is very useful for variants of a board which start out with minimal changes (or use templates to add the initial support) and follow up with more variant-specific changes to enable other bits. What we really want to ensure here is that we don't accidentally drop SPD ROM data if SPD_SOURCES is not empty.
Thus it should be left upto the mainboard/variant to control when it is ready to add the SPD files.
Thoughts?
--
To view, visit https://review.coreboot.org/c/coreboot/+/45131
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic6db1dbe5fed5f242e408bcad4f36dda1b1fa1b4
Gerrit-Change-Number: 45131
Gerrit-PatchSet: 14
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Caveh Jalali <caveh(a)chromium.org>
Gerrit-Reviewer: David Wu <david_wu(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Paul Fagerburg <pfagerburg(a)chromium.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com>
Gerrit-Comment-Date: Fri, 02 Oct 2020 01:14:15 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment