Hello Alexander Couzens, Patrick Rudolph, Vanessa Eusebio, Huang Jin, York Yang, Philipp Deppenwiese, build bot (Jenkins), Martin Roth, David Guckian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/29986
to look at the new patch set (#2).
Change subject: ACPI: Rename FADT model and set it to zero
......................................................................
ACPI: Rename FADT model and set it to zero
INT_MODEL defined in ACPI 1.0 and renamed to reserved in upper
versions.
The value for this field is zero but 1 is allowed to maintain
compatibility with ACPI 1.0.
So set this value to zero as we are using ACPI upper than 1.0
Change-Id: I910ead4e5618c958a7989f4c309a3a4bb938e31a
Signed-off-by: Elyes HAOUAS <ehaouas(a)noos.fr>
---
M src/arch/x86/include/arch/acpi.h
M src/mainboard/aopen/dxplplusu/fadt.c
M src/mainboard/emulation/qemu-q35/acpi_tables.c
M src/mainboard/google/auron/fadt.c
M src/mainboard/google/cyan/fadt.c
M src/mainboard/google/jecht/fadt.c
M src/mainboard/google/rambi/fadt.c
M src/mainboard/intel/strago/fadt.c
M src/mainboard/intel/wtm2/fadt.c
M src/mainboard/lenovo/t400/fadt.c
M src/mainboard/lenovo/x200/fadt.c
M src/mainboard/purism/librem_bdw/fadt.c
M src/mainboard/roda/rk9/fadt.c
M src/soc/amd/stoneyridge/acpi.c
M src/soc/intel/fsp_baytrail/acpi.c
M src/soc/intel/fsp_broadwell_de/acpi.c
M src/southbridge/amd/agesa/hudson/fadt.c
M src/southbridge/amd/cimx/sb800/fadt.c
M src/southbridge/amd/pi/hudson/fadt.c
M src/southbridge/intel/bd82x6x/lpc.c
M src/southbridge/intel/fsp_rangeley/acpi.c
M src/southbridge/intel/i82801gx/lpc.c
M src/southbridge/intel/i82801jx/lpc.c
M src/southbridge/intel/ibexpeak/lpc.c
24 files changed, 24 insertions(+), 24 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/86/29986/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/29986
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I910ead4e5618c958a7989f4c309a3a4bb938e31a
Gerrit-Change-Number: 29986
Gerrit-PatchSet: 2
Gerrit-Owner: HAOUAS Elyes <ehaouas(a)noos.fr>
Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu>
Gerrit-Reviewer: David Guckian <david.guckian(a)intel.com>
Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr>
Gerrit-Reviewer: Huang Jin <huang.jin(a)intel.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>
Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com>
Gerrit-Reviewer: York Yang <york.yang(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: newpatchset
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31250 )
Change subject: soc/intel/cannonlake: Configure GPIOs again after FSP-S is done
......................................................................
soc/intel/cannonlake: Configure GPIOs again after FSP-S is done
FSP-S is currently configuring GPIOs that it should not. This results
in issues where mainboard devices don't behave as expected e.g. host
unable to receive TPM interrupts as the pad for the interrupt is
re-configured as something else.
Until FSP-S is fixed, this change adds a workaround by reconfiguring
GPIOs after FSP-S is run.
All mainboards need to call cnl_configure_pads instead of
gpio_configure_pads so that SoC code can maintain a reference to the
GPIO table and use that to re-configure GPIOs after FSP-S is run.
BUG=b:123721147
BRANCH=None
TEST=Verified that there are no TPM IRQ timeouts in boot log on hatch.
Change-Id: I7787aa8f185f633627bcedc7f23504bf4a5250b4
Signed-off-by: Furquan Shaikh <furquan(a)google.com>
Reviewed-on: https://review.coreboot.org/c/31250
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Duncan Laurie <dlaurie(a)chromium.org>
---
M src/soc/intel/cannonlake/chip.c
M src/soc/intel/cannonlake/include/soc/gpio.h
2 files changed, 35 insertions(+), 1 deletion(-)
Approvals:
build bot (Jenkins): Verified
Duncan Laurie: Looks good to me, approved
diff --git a/src/soc/intel/cannonlake/chip.c b/src/soc/intel/cannonlake/chip.c
index 4604d80..a643954 100644
--- a/src/soc/intel/cannonlake/chip.c
+++ b/src/soc/intel/cannonlake/chip.c
@@ -140,6 +140,33 @@
}
#endif
+/*
+ * TODO(furquan): Get rid of this workaround once FSP is fixed. Currently, FSP-S
+ * configures GPIOs when it should not and this results in coreboot GPIO
+ * configuration being overwritten. Until FSP is fixed, maintain the reference
+ * of GPIO config table from mainboard and use that to re-configure GPIOs after
+ * FSP-S is done.
+ */
+void cnl_configure_pads(const struct pad_config *cfg, size_t num_pads)
+{
+ static const struct pad_config *g_cfg;
+ static size_t g_num_pads;
+
+ /*
+ * If cfg and num_pads are passed in from mainboard, maintain a
+ * reference to the GPIO table.
+ */
+ if ((cfg == NULL) || (num_pads == 0)) {
+ cfg = g_cfg;
+ num_pads = g_num_pads;
+ } else {
+ g_cfg = cfg;
+ g_num_pads = num_pads;
+ }
+
+ gpio_configure_pads(cfg, num_pads);
+}
+
void soc_init_pre_device(void *chip_info)
{
/* Snapshot the current GPIO IRQ polarities. FSP is setting a
@@ -154,6 +181,9 @@
/* Restore GPIO IRQ polarities back to previous settings. */
itss_restore_irq_polarities(GPIO_IRQ_START, GPIO_IRQ_END);
+
+ /* TODO(furquan): Get rid of this workaround once FSP is fixed. */
+ cnl_configure_pads(NULL, 0);
}
static void pci_domain_set_resources(struct device *dev)
diff --git a/src/soc/intel/cannonlake/include/soc/gpio.h b/src/soc/intel/cannonlake/include/soc/gpio.h
index cbc230a..718372d 100644
--- a/src/soc/intel/cannonlake/include/soc/gpio.h
+++ b/src/soc/intel/cannonlake/include/soc/gpio.h
@@ -16,7 +16,6 @@
#ifndef _SOC_CANNONLAKE_GPIO_H_
#define _SOC_CANNONLAKE_GPIO_H_
-
#if IS_ENABLED(CONFIG_SOC_INTEL_CANNONLAKE_PCH_H)
#include <soc/gpio_defs_cnp_h.h>
#define CROS_GPIO_DEVICE_NAME "INT3450:00"
@@ -26,4 +25,9 @@
#endif
#include <intelblocks/gpio.h>
+#ifndef __ACPI__
+struct pad_config;
+void cnl_configure_pads(const struct pad_config *cfg, size_t num_pads);
+#endif
+
#endif
--
To view, visit https://review.coreboot.org/c/coreboot/+/31250
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7787aa8f185f633627bcedc7f23504bf4a5250b4
Gerrit-Change-Number: 31250
Gerrit-PatchSet: 5
Gerrit-Owner: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Shelley Chen <shchen(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29398 )
Change subject: src/soc/intel/braswell/southcluster.c: Correct serial IRQ support
......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/#/c/29398/5/src/soc/intel/braswell/Kconfig
File src/soc/intel/braswell/Kconfig:
https://review.coreboot.org/#/c/29398/5/src/soc/intel/braswell/Kconfig@137
PS5, Line 137:
: config ENABLE_SERIRQ
: bool "Enable Serial IRQ"
: default n
: help
: Enable Serial IRQ
:
: config SERIRQ_CONTINUOUS_MODE
: bool "Serial IRQ continuous mode"
: default n
: depends on ENABLE_SERIRQ
: help
: Enable Serial IRQ continuous mode
> Supposed as user configurable, so visible and configurable in menuconfig.
are there boards which use this for which you'd want to disable it conditionally? If not, setting it at the board level vs user configurable makes more sense
--
To view, visit https://review.coreboot.org/c/coreboot/+/29398
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7844cad69dc0563fa6109d779d0afb7c2edd7245
Gerrit-Change-Number: 29398
Gerrit-PatchSet: 5
Gerrit-Owner: Frans Hendriks <fhendriks(a)eltan.com>
Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com>
Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: HAOUAS Elyes <ehaouas(a)noos.fr>
Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Comment-Date: Thu, 07 Feb 2019 05:27:34 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Comment-In-Reply-To: Frans Hendriks <fhendriks(a)eltan.com>
Gerrit-MessageType: comment
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29394 )
Change subject: src/soc/intel/braswell/hda.c: Configure HDA codecs
......................................................................
Patch Set 8:
since LPE audio is the norm on braswell devices, might it be better to set a Kconfig for HDA audio and conditionally include hda_verb.c if set? That way you don't need to modify existing boards which don't use HDA audio.
--
To view, visit https://review.coreboot.org/c/coreboot/+/29394
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I5c23ec311e5b5a6dfd6f031aa19617407fe8ed63
Gerrit-Change-Number: 29394
Gerrit-PatchSet: 8
Gerrit-Owner: Frans Hendriks <fhendriks(a)eltan.com>
Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com>
Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com>
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-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-CC: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Comment-Date: Thu, 07 Feb 2019 05:24:23 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29284 )
Change subject: src/soc/intel/braswell/chip.c: Configure LPSS devices in correct mode
......................................................................
Patch Set 3:
is the default PCI mode? (since you're not explicitly setting it)
what are the consequences of not passing the correct mode to FSP?
--
To view, visit https://review.coreboot.org/c/coreboot/+/29284
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ie271d8cb9f30f0c0ba538f1530cfb82f1306fea8
Gerrit-Change-Number: 29284
Gerrit-PatchSet: 3
Gerrit-Owner: Frans Hendriks <fhendriks(a)eltan.com>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com>
Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Comment-Date: Thu, 07 Feb 2019 05:20:48 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29414 )
Change subject: src/soc/intel/braswell: Remove disabled LPE acpi code
......................................................................
Patch Set 4:
wouldn't it be a lot easier to just set the LPE device's _STA method to return the correct status based on device presence? You could do it either dynamically based on PCI device 0:15.0 being enabled or present, or statically based on a gnvs set in the mainboard config. This just seems like an overly complicated solution to telling the OS that the LPE hardware isn't present.
--
To view, visit https://review.coreboot.org/c/coreboot/+/29414
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic8acf9ea9e9b0ba9b272e20beb2023b7a4716a73
Gerrit-Change-Number: 29414
Gerrit-PatchSet: 4
Gerrit-Owner: Frans Hendriks <fhendriks(a)eltan.com>
Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com>
Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com>
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-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: HAOUAS Elyes <ehaouas(a)noos.fr>
Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Comment-Date: Thu, 07 Feb 2019 05:14:24 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment