Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44559 )
Change subject: mb/google/volteer: Update SLP_Sx assertion widths and PwrCycDur
......................................................................
Patch Set 3:
(4 comments)
https://review.coreboot.org/c/coreboot/+/44559/2/src/mainboard/google/volte…
File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/44559/2/src/mainboard/google/volte…
PS2, Line 152: 3
> Tim, with current common code implementation, I think this definition cannot be used directly here t […]
Ah right, it was on my to-do list to move those to a header file. Sorry.
https://review.coreboot.org/c/coreboot/+/44559/2/src/mainboard/google/volte…
PS2, Line 153: 1
> Same as above.
Ack
https://review.coreboot.org/c/coreboot/+/44559/2/src/mainboard/google/volte…
PS2, Line 154: 3
> Same as above.
Ack
https://review.coreboot.org/c/coreboot/+/44559/2/src/mainboard/google/volte…
PS2, Line 155: 3
> Same as above.
Ack
--
To view, visit https://review.coreboot.org/c/coreboot/+/44559
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Idf4e0c3a60b4ac59e31df1357f2ff28f195ff17f
Gerrit-Change-Number: 44559
Gerrit-PatchSet: 3
Gerrit-Owner: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.corp-partner.google.com>
Gerrit-Reviewer: Raj Astekar <raj.astekar(a)intel.com>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com>
Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Comment-Date: Tue, 29 Sep 2020 19:25:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Jamie Ryu <jamie.m.ryu(a)intel.com>
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-MessageType: comment
Rob Barnes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44775 )
Change subject: treewide: stop using hexdumps for SPD files
......................................................................
Patch Set 27: Code-Review+1
> Patch Set 27:
>
> > I'd vote for not checking binaries into the coreboot source tree at all. Personally, I'd like to get rid of the vbt binaries and have a tool to compile them as well. My understanding was that there wasn't documentation for the VBT fields, but it seems that maybe this is no longer the case. [1]
>
> In general I agree, but it seems hard to get authoritative sources on the precise format, even for SPD dumps - that ought to be standardized but get repurposed in lots of "wonderful" ways by memory reference code. As far as we know, the "spec" to these files is the reference code, and that's not public.
>
> Some of the SPD tools we use on newer chipsets (for lp4x?) may come close to that. In which case: why ship hex dumps or binaries at all?
>
> > As far as vendors adding custom flags, I don't see how binary vs text solves any problem there.
> The transition here removes the hex-to-bin translation that's been fragile. Michael offers a patch that fixes things for him - that I'm relatively sure breaks building that part of the tree on other UNIX systems, since I vaguely remember having had these issues myself. My guess would be Solaris but it might be some odd BSD as well.
>
> The text files we ship are plain hex dumps. They don't add anything of value, do they? So why should we pretend that they're "source" and make our live miserable?
>
> My proposal is to get this in to remove the pain point that this is dealing with (the fragile hex-to-bin translation) and whoever is motivated to do so looks into providing a higher level description for these files and translating things to that, removing the opaque datasets we have (no matter the format) from the tree.
spd_tools was already updated to output binary SPD files, so now spd_tools is not usable for adding parts until this change goes in. So this is a 2nd vote for getting this in as is to unblock spd_tools.
spd_tools generates SPD binaries from a json input. This could certainly be made part of the build so the generated SPD binaries do not need to be checked in. The only blocker is golang support in the build or rewriting the tool in a supported language.
--
To view, visit https://review.coreboot.org/c/coreboot/+/44775
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I0f24183a872924cddcfdf7587cc0c126da900f91
Gerrit-Change-Number: 44775
Gerrit-PatchSet: 27
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: David Guckian <david.guckian(a)intel.com>
Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.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: Piotr Król <piotr.krol(a)3mdeb.com>
Gerrit-Reviewer: Rob Barnes <robbarnes(a)google.com>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com>
Gerrit-Reviewer: Wim Vervoorn <wvervoorn(a)eltan.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-CC: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Tue, 29 Sep 2020 17:18:06 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Michael Niewöhner has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/45826 )
Change subject: soc/intel/icl: enable common CPU code
......................................................................
Abandoned
duplicate of CB:45037
--
To view, visit https://review.coreboot.org/c/coreboot/+/45826
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I58e86021687fc0a836324f70071f7ea80242b3cb
Gerrit-Change-Number: 45826
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: abandon
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45826 )
Change subject: soc/intel/icl: enable common CPU code
......................................................................
Patch Set 3: Code-Review-2
I just saw CB:45037, which does the same already
--
To view, visit https://review.coreboot.org/c/coreboot/+/45826
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I58e86021687fc0a836324f70071f7ea80242b3cb
Gerrit-Change-Number: 45826
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Tue, 29 Sep 2020 16:42:41 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Hello build bot (Jenkins), Subrata Banik, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45826
to look at the new patch set (#3).
Change subject: soc/intel/icl: enable common CPU code
......................................................................
soc/intel/icl: enable common CPU code
Enable CPU_INTEL_COMMON to make common CPU code available to CNL, which
gets used in CB:45535 and CB:45536 for CPPC entries generation.
Note: This also retrieves the VMX Kconfig and enables it by default,
like done for SKL,ICL,... already.
Change-Id: I58e86021687fc0a836324f70071f7ea80242b3cb
Signed-off-by: Michael Niewöhner <foss(a)mniewoehner.de>
---
M src/soc/intel/icelake/Kconfig
1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/45826/3
--
To view, visit https://review.coreboot.org/c/coreboot/+/45826
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I58e86021687fc0a836324f70071f7ea80242b3cb
Gerrit-Change-Number: 45826
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: newpatchset