Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45884 )
Change subject: mb/clevo/l140cu: Align comment with rest of the devicetree
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/45884
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Idcaedd3d5b7e465644f79e5a882e42eff040fdbd
Gerrit-Change-Number: 45884
Gerrit-PatchSet: 2
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Thu, 01 Oct 2020 18:07:31 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38978 )
Change subject: [WIP] mb/acer: Add Acer Aspire ES1-572
......................................................................
Patch Set 7:
(4 comments)
https://review.coreboot.org/c/coreboot/+/38978/5/src/mainboard/acer/es1-572…
File src/mainboard/acer/es1-572/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/38978/5/src/mainboard/acer/es1-572…
PS5, Line 223: register "SendVrMbxCmd" = "2"
> I never found documentation on that PS4 exit issue; how did you verify this isn't required here?
I never verified that it was required in the first place. The issue seems to be made out of mystery meat.
https://review.coreboot.org/c/coreboot/+/38978/7/src/mainboard/acer/es1-572…
File src/mainboard/acer/es1-572/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/38978/7/src/mainboard/acer/es1-572…
PS7, Line 37: register "SataTestMode" = "1"
> huh? why is test mode on?
Because I want to test SATA? 😄
https://review.coreboot.org/c/coreboot/+/38978/7/src/mainboard/acer/es1-572…
PS7, Line 40: register "SataPortsDevSlp[0]" = "0"
: register "SataPortsDevSlp[1]" = "0"
:
> drop disabled options - or enable devsleep cap if the board supports it (haven't checked the schemat […]
the latter part is exactly why I've left these here.
https://review.coreboot.org/c/coreboot/+/38978/7/src/mainboard/acer/es1-572…
PS7, Line 108: device pci 04.0 off end # CPU Thermal Subsystem
> why off?
why on?
EC ACPI is missing, haven't done anything about it yet. I'll check if DPTF is enabled on vendor fw
--
To view, visit https://review.coreboot.org/c/coreboot/+/38978
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Id98788a2c5e54f70fd1cacbd70d636f5e63b2619
Gerrit-Change-Number: 38978
Gerrit-PatchSet: 7
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <felixsinger(a)posteo.net>
Gerrit-CC: Benjamin Doron <benjamin.doron00(a)gmail.com>
Gerrit-Comment-Date: Thu, 01 Oct 2020 18:05:07 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-MessageType: comment
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45885 )
Change subject: mb/clevo/l140cu: Fill SSDT table with information about USB ports
......................................................................
Patch Set 1:
This change is ready for review.
--
To view, visit https://review.coreboot.org/c/coreboot/+/45885
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I414d02cc9c16ed23ce12cba79bfd68e0e6486129
Gerrit-Change-Number: 45885
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Thu, 01 Oct 2020 17:35:19 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Angel Pons 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: 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.
Why not revert the change that made spd_tools output binaries instead? I'm not convinced using binaries would help with the issue at hand.
--
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: Thu, 01 Oct 2020 08:53:16 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45799 )
Change subject: mb/ocp/deltalake: Override smbios_fill_dimm_locator for type 17
......................................................................
Patch Set 5: Code-Review+1
(2 comments)
https://review.coreboot.org/c/coreboot/+/45799/5//COMMIT_MSG
Commit Message:
https://review.coreboot.org/c/coreboot/+/45799/5//COMMIT_MSG@10
PS5, Line 10: Also remove CONFIG(GENERATE_SMBIOS_TABLES) compile option because SMBIOS
: is always enabled and it makes the code cleaner.
Another approach is to move all SMBIOS stuff into a `smbios.c` file 😊
https://review.coreboot.org/c/coreboot/+/45799/5/src/mainboard/ocp/deltalak…
File src/mainboard/ocp/deltalake/ramstage.c:
https://review.coreboot.org/c/coreboot/+/45799/5/src/mainboard/ocp/deltalak…
PS5, Line 246: dimm_locator[dimm->channel_num]
I would just do:
'A' + dimm->channel_num
--
To view, visit https://review.coreboot.org/c/coreboot/+/45799
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I84531f9ee8bc76d9529aa983bc13e64f40c93138
Gerrit-Change-Number: 45799
Gerrit-PatchSet: 5
Gerrit-Owner: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Jingle Hsu <jingle_hsu(a)wiwynn.com>
Gerrit-Reviewer: John Looney <john.looney(a)gmail.com>
Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-Reviewer: Marc Jones <marc(a)marcjonesconsulting.com>
Gerrit-Reviewer: Morgan Jang <Morgan_Jang(a)wiwynn.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Thu, 01 Oct 2020 08:20:59 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Caveh Jalali 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:
This change is ready for review.
--
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: 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: Thu, 01 Oct 2020 07:07:01 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45888 )
Change subject: soc/intel/xeon_sp: Make use of ITSS common block [WIP]
......................................................................
soc/intel/xeon_sp: Make use of ITSS common block [WIP]
This patch ensures to have xeon SoC can use common functions
from block/itss.
This patch is needed to make use of ITSS common code.
Change-Id: I612fdbc8e2ae993de47656002d87b6c4d4f3affd
Signed-off-by: Subrata Banik <subrata.banik(a)intel.com>
---
M src/soc/intel/xeon_sp/Kconfig
1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/88/45888/1
diff --git a/src/soc/intel/xeon_sp/Kconfig b/src/soc/intel/xeon_sp/Kconfig
index 3b18741..25603b5 100644
--- a/src/soc/intel/xeon_sp/Kconfig
+++ b/src/soc/intel/xeon_sp/Kconfig
@@ -41,6 +41,7 @@
select INTEL_DESCRIPTOR_MODE_CAPABLE
select SOC_INTEL_COMMON_BLOCK
select SOC_INTEL_COMMON_BLOCK_CPU
+ select SOC_INTEL_COMMON_BLOCK_ITSS
select SOC_INTEL_COMMON_BLOCK_TIMER
select SOC_INTEL_COMMON_BLOCK_LPC
select SOC_INTEL_COMMON_BLOCK_RTC
--
To view, visit https://review.coreboot.org/c/coreboot/+/45888
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I612fdbc8e2ae993de47656002d87b6c4d4f3affd
Gerrit-Change-Number: 45888
Gerrit-PatchSet: 1
Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-MessageType: newchange
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44995 )
Change subject: acpi: Support MSDM table signature as SLIC
......................................................................
Patch Set 4: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/44995
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic3a1374c8a4880111a30662823c3be99008eedd3
Gerrit-Change-Number: 44995
Gerrit-PatchSet: 4
Gerrit-Owner: Benjamin Doron <benjamin.doron00(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Thu, 01 Oct 2020 01:47:29 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment