Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37579 )
Change subject: vendorcode/intel: Remove Ice Lake FSP Bindings
......................................................................
Patch Set 9: Code-Review+1
(4 comments)
https://review.coreboot.org/c/coreboot/+/37579/9//COMMIT_MSG
Commit Message:
https://review.coreboot.org/c/coreboot/+/37579/9//COMMIT_MSG@15
PS9, Line 15: has been
: removed by Intel
That's speculation. We cannot know if the headers pushed to vendorcode/
were unaltered. It's more likely that they added it to the vendorcode/
version, because it didn't compile, and never added it to the official
FSP headers. Suggesting:
that are missing in the official headers.
https://review.coreboot.org/c/coreboot/+/37579/9/src/drivers/intel/fsp2_0/i…
File src/drivers/intel/fsp2_0/include/fsp/soc_binding.h:
https://review.coreboot.org/c/coreboot/+/37579/9/src/drivers/intel/fsp2_0/i…
PS9, Line 20: /*
: * This file is a implementation specific header. i.e. different
: * FSP implementations for different chipsets.
: */
Does somebody understand this comment???
Was it maybe referring to one of the includes? Because this file
(soc_binding.h) is obviously not implementation specific, it draws in
implementation specific headers, though.
Please keep the comment where it was (directly above Base.h).
https://review.coreboot.org/c/coreboot/+/37579/9/src/drivers/intel/fsp2_0/i…
PS9, Line 24:
Please add a comment that the below block needs to be here to fix
missing includes in the FSP headers files.
https://review.coreboot.org/c/coreboot/+/37579/6/src/soc/intel/tigerlake/ro…
File src/soc/intel/tigerlake/romstage/romstage.c:
https://review.coreboot.org/c/coreboot/+/37579/6/src/soc/intel/tigerlake/ro…
PS6, Line 43:
> I reduced this commit to icelake where I can verify things
What I meant is that at the time you pushed this, the whole soc/intel/tigerlake
dir wasn't build tested by Jenkins. So you didn't notice that this change
actually broke it.
--
To view, visit https://review.coreboot.org/c/coreboot/+/37579
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7d5520dcd30f4a68af325125052e16e867e91ec9
Gerrit-Change-Number: 37579
Gerrit-PatchSet: 9
Gerrit-Owner: Mimoja <coreboot(a)mimoja.de>
Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: David Guckian <david.guckian(a)intel.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Mimoja <coreboot(a)mimoja.de>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Vanessa Eusebio <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: Sun, 12 Jan 2020 15:07:00 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Mimoja <coreboot(a)mimoja.de>
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: comment
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/16308 )
Change subject: drivers/intel/fsp2_0: Make FSP Headers Consumable out of Box
......................................................................
Patch Set 15:
(1 comment)
https://review.coreboot.org/c/coreboot/+/16308/15/src/drivers/intel/fsp2_0/…
File src/drivers/intel/fsp2_0/include/fsp/soc_binding.h:
https://review.coreboot.org/c/coreboot/+/16308/15/src/drivers/intel/fsp2_0/…
PS15, Line 20: /*
: * This file is a implementation specific header. i.e. different
: * FSP implementations for different chipsets.
: */
Does anybody know what this comment refers to? It does neither apply to
this file (`soc_binding.h`) nor `Base.h`.
--
To view, visit https://review.coreboot.org/c/coreboot/+/16308
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I10739dca1b6da3f15bd850adf06238f7c51508f7
Gerrit-Change-Number: 16308
Gerrit-PatchSet: 15
Gerrit-Owner: Brandon Breitenstein <brandon.breitenstein(a)intel.com>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Reviewer: Brandon Breitenstein <brandon.breitenstein(a)intel.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Lee Leahy <leroy.p.leahy(a)intel.com>
Gerrit-Reviewer: Lijian Zhao <lijian.zhao(a)intel.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-Comment-Date: Sun, 12 Jan 2020 15:00:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Mike Banon has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38353 )
Change subject: [TESTME]asus/f2a85-m: move the rest of romstage.c code to bootblock.c
......................................................................
[TESTME]asus/f2a85-m: move the rest of romstage.c code to bootblock.c
Move the rest of romstage.c code to bootblock.c. Based on a
similar change which worked fine for ASUS A88XM-E at CB:30987.
Signed-off-by: Mike Banon <mikebdp2(a)gmail.com>
Change-Id: I921ed188fe227148d6c48e476b2efff3de94b429
---
M src/mainboard/asus/f2a85-m/bootblock.c
D src/mainboard/asus/f2a85-m/romstage.c
2 files changed, 19 insertions(+), 41 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/53/38353/1
diff --git a/src/mainboard/asus/f2a85-m/bootblock.c b/src/mainboard/asus/f2a85-m/bootblock.c
index 0472877..b4b0987 100644
--- a/src/mainboard/asus/f2a85-m/bootblock.c
+++ b/src/mainboard/asus/f2a85-m/bootblock.c
@@ -17,11 +17,27 @@
#include <bootblock_common.h>
#include <device/pnp_type.h>
#include <amdblocks/acpimmio.h>
-#include <stdint.h>
#include <superio/ite/common/ite.h>
#include <superio/ite/it8728f/it8728f.h>
#include <superio/nuvoton/common/nuvoton.h>
#include <superio/nuvoton/nct6779d/nct6779d.h>
+#include <southbridge/amd/agesa/hudson/smbus.h>
+
+static void smbus_setup(void)
+{
+ /* turn on secondary smbus at b20 */
+ u32 reg32;
+ reg32 = misc_read32(0x28);
+ reg32 |= 0x00000001;
+ misc_write32(0x28, reg32);
+
+ /* set DDR3 voltage */
+ reg32 = CONFIG_BOARD_ASUS_F2A85_M_DDR3_VOLT_VAL;
+
+ /* default is reg32 = 0x0, so no need to set it in this case */
+ if (reg32)
+ do_smbus_write_byte(0xb20, 0x15, 0x3, reg32);
+}
static void sbxxx_enable_48mhzout(void)
{
@@ -56,6 +72,8 @@
void bootblock_mainboard_early_init(void)
{
+ smbus_setup();
+
/* enable SIO clock */
sbxxx_enable_48mhzout();
diff --git a/src/mainboard/asus/f2a85-m/romstage.c b/src/mainboard/asus/f2a85-m/romstage.c
deleted file mode 100644
index 3aa29c8..0000000
--- a/src/mainboard/asus/f2a85-m/romstage.c
+++ /dev/null
@@ -1,40 +0,0 @@
-/*
- * This file is part of the coreboot project.
- *
- * Copyright (C) 2012 Advanced Micro Devices, Inc.
- * Copyright (C) 2012 Rudolf Marek <r.marek(a)assembler.cz>
- *
- * 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 <arch/io.h>
-#include <northbridge/amd/agesa/state_machine.h>
-#include <southbridge/amd/agesa/hudson/smbus.h>
-#include <stdint.h>
-
-void board_BeforeAgesa(struct sysinfo *cb)
-{
- u8 byte;
-
- post_code(0x30);
-
- /* turn on secondary smbus at b20 */
- outb(0x28, 0xcd6);
- byte = inb(0xcd7);
- byte |= 1;
- outb(byte, 0xcd7);
-
- /* set DDR3 voltage */
- byte = CONFIG_BOARD_ASUS_F2A85_M_DDR3_VOLT_VAL;
-
- /* default is byte = 0x0, so no need to set it in this case */
- if (byte)
- do_smbus_write_byte(0xb20, 0x15, 0x3, byte);
-}
--
To view, visit https://review.coreboot.org/c/coreboot/+/38353
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I921ed188fe227148d6c48e476b2efff3de94b429
Gerrit-Change-Number: 38353
Gerrit-PatchSet: 1
Gerrit-Owner: Mike Banon <mikebdp2(a)gmail.com>
Gerrit-MessageType: newchange
Hello Paul Menzel, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/26894
to look at the new patch set (#5).
Change subject: mb/gigabyte/ga-g41m-es2l/acpi_tables.c: Remove unneeded includes
......................................................................
mb/gigabyte/ga-g41m-es2l/acpi_tables.c: Remove unneeded includes
Change-Id: I4b3b2d801698305dc6c214c58d367772ea2096a3
Signed-off-by: Elyes HAOUAS <ehaouas(a)noos.fr>
---
M src/mainboard/gigabyte/ga-g41m-es2l/acpi_tables.c
1 file changed, 0 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/94/26894/5
--
To view, visit https://review.coreboot.org/c/coreboot/+/26894
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I4b3b2d801698305dc6c214c58d367772ea2096a3
Gerrit-Change-Number: 26894
Gerrit-PatchSet: 5
Gerrit-Owner: HAOUAS Elyes <ehaouas(a)noos.fr>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: newpatchset
Hello Paul Menzel, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/26892
to look at the new patch set (#4).
Change subject: mb/gigabyte/ga-b75m-d3h/acpi_tables.c: Remove unneeded includes
......................................................................
mb/gigabyte/ga-b75m-d3h/acpi_tables.c: Remove unneeded includes
Change-Id: Ic94e60188dbb9cdee959ecfa5ef14c92f125e3f2
Signed-off-by: Elyes HAOUAS <ehaouas(a)noos.fr>
---
M src/mainboard/gigabyte/ga-b75m-d3h/acpi_tables.c
1 file changed, 0 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/92/26892/4
--
To view, visit https://review.coreboot.org/c/coreboot/+/26892
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic94e60188dbb9cdee959ecfa5ef14c92f125e3f2
Gerrit-Change-Number: 26892
Gerrit-PatchSet: 4
Gerrit-Owner: HAOUAS Elyes <ehaouas(a)noos.fr>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: newpatchset