Hello Maulik V Vaghela,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/31640
to review the following change.
Change subject: mb/google/hatch: Add correct GPIO programming for GPP_C0 to GPP_C7
......................................................................
mb/google/hatch: Add correct GPIO programming for GPP_C0 to GPP_C7
Coreboot did not program all GPIOs from C0 to C7 correctly which are
SMBUS GPIO for CSME. Some of the GPIOs are left in default mode which is
native function but we need to configure as GPIO mode and provide proper
configuration as per schematic.
After fixing GPIO, CSME power gating issue also gets fixed since all
SMBUS GPIOs are configured properly
BUG=b:123702553
BRANCH=none
TEST=Check on hatch board. CSME was not getting power gated for s0ix.
After applying this patch CSME is power gated now
Change-Id: I5c6b9310dcc7bade0023abd5524781ce71df28be
Signed-off-by: Maulik V Vaghela <maulik.v.vaghela(a)intel.corp-partner.google.com>
---
M src/mainboard/google/hatch/variants/baseboard/gpio.c
1 file changed, 14 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/31640/1
diff --git a/src/mainboard/google/hatch/variants/baseboard/gpio.c b/src/mainboard/google/hatch/variants/baseboard/gpio.c
index 769d2ab..7387f92 100644
--- a/src/mainboard/google/hatch/variants/baseboard/gpio.c
+++ b/src/mainboard/google/hatch/variants/baseboard/gpio.c
@@ -45,10 +45,22 @@
PAD_CFG_NF(GPP_B17, NONE, DEEP, NF1),
/* H1_SLAVE_SPI_MOSI_R */
PAD_CFG_NF(GPP_B18, NONE, DEEP, NF1),
- /* TOUCHSCREEN_DIS_L */
- PAD_CFG_GPO(GPP_C4, 0, DEEP),
+ /* GPP_C0 => NC */
+ PAD_NC(GPP_C0, NONE),
/* PCIE_14_WLAN_WAKE_ODL */
PAD_CFG_GPI_SCI_LOW(GPP_C1, NONE, DEEP, EDGE_SINGLE),
+ /* GPP_C2 => NC */
+ PAD_NC(GPP_C2, NONE),
+ /* WLAN_OFF_L */
+ PAD_CFG_GPO(GPP_C3, 1, DEEP),
+ /* TOUCHSCREEN_DIS_L */
+ PAD_CFG_GPO(GPP_C4, 1, DEEP),
+ /* GPP_C5 => NC */
+ PAD_NC(GPP_C5, NONE),
+ /* PEN_PDCT_OD_L */
+ PAD_CFG_GPI(GPP_C6, NONE, DEEP),
+ /* PEN_IRQ_OD_L */
+ PAD_CFG_GPI(GPP_C7, NONE, DEEP),
/* GPP_C10_TP */
PAD_NC(GPP_C10, DN_20K),
/* GPP_C11_TP */
--
To view, visit https://review.coreboot.org/c/coreboot/+/31640
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I5c6b9310dcc7bade0023abd5524781ce71df28be
Gerrit-Change-Number: 31640
Gerrit-PatchSet: 1
Gerrit-Owner: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.corp-partner.google.com>
Gerrit-MessageType: newchange
Furquan Shaikh has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31633
Change subject: soc/intel/cannonlake: Disable ACPI mode on BS_DEV_INIT exit
......................................................................
soc/intel/cannonlake: Disable ACPI mode on BS_DEV_INIT exit
Change ac8c60e (soc/intel/cannonlake: Disable ACPI mode as part of
pmc_soc_init) moved disabling of ACPI mode to pmc_soc_init to keep it
more aligned with the behavior on other Intel SoCs. However, as the
PMC device is hidden, it never gets enumerated and so init function
does not get called for it. This change moves the call to disable ACPI
mode to exit of BS_DEV_INIT instead.
BUG=b:126016602
TEST=Verified that:
1. pmc_set_acpi_mode is actually getting called.
2. EC panic event gets logged to eventlog correctly.
Change-Id: Ie7025e322fa0abc21367a520184a4c7741eba1e6
Signed-off-by: Furquan Shaikh <furquan(a)google.com>
---
M src/soc/intel/cannonlake/pmc.c
1 file changed, 10 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/31633/1
diff --git a/src/soc/intel/cannonlake/pmc.c b/src/soc/intel/cannonlake/pmc.c
index 24ddfee..84bfba0 100644
--- a/src/soc/intel/cannonlake/pmc.c
+++ b/src/soc/intel/cannonlake/pmc.c
@@ -158,20 +158,26 @@
*/
BOOT_STATE_INIT_ENTRY(BS_DEV_INIT_CHIPS, BS_ON_EXIT, pmc_init, NULL);
-void pmc_soc_init(struct device *dev)
+static void soc_acpi_mode_init(void *unused)
{
/*
* PMC initialization happens earlier for this SoC because FSP-Silicon
* init hides PMC from PCI bus. However, pmc_set_acpi_mode, which
* disables ACPI mode doesn't need to happen that early and can be
- * delayed till typical pmc_soc_init callback. This ensures that ACPI
- * mode disabling happens the same way for all SoCs and hence the
- * ordering of events is the same.
+ * delayed till typical BS_DEV_INIT. This ensures that ACPI mode
+ * disabling happens the same way for all SoCs and hence the ordering of
+ * events is the same.
*
* This is important to ensure that the ordering does not break the
* assumptions of any other drivers (e.g. ChromeEC) which could be
* taking different actions based on disabling of ACPI (e.g. flushing of
* all EC hostevent bits).
+ *
+ * P.S.: This cannot be done as part of pmc_soc_init as PMC device is
+ * hidden and hence the PMC driver never gets enumerated and so init is
+ * not called for it.
*/
pmc_set_acpi_mode();
}
+
+BOOT_STATE_INIT_ENTRY(BS_DEV_INIT, BS_ON_EXIT, soc_acpi_mode_init, NULL);
--
To view, visit https://review.coreboot.org/c/coreboot/+/31633
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ie7025e322fa0abc21367a520184a4c7741eba1e6
Gerrit-Change-Number: 31633
Gerrit-PatchSet: 1
Gerrit-Owner: Furquan Shaikh <furquan(a)google.com>
Gerrit-MessageType: newchange
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31529 )
Change subject: vendorcode/intel/fsp/fsp2_0/cml: Add FSP header files for Cometlake
......................................................................
Patch Set 3:
> > @Nico/Patrick: Can you please reconsider your -1 on this CL?
>
> Why? A -1 merely states that we'd prefer something else. It's
> no problem to merge something with -1, especially not in a
> situation like this with FSP involved. I assume that you have
> to work with what you get from the people building FSP.
>
> If you want, you can take the -1 as a reminder to tell those
> people that they are dragging Intel down. (I mean, if I built
> something with the wrong numbers, I'd just rebuild it when
> somebody tells me about it. But that'd probably just be me.)
Got it Nico, i just don't want to merge this CL without knowing your concern that was the intention for me asking you to consider. We will merge it now.
--
To view, visit https://review.coreboot.org/c/coreboot/+/31529
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I734316445dda5b1feb4098ce3c58b6dd8ce2d272
Gerrit-Change-Number: 31529
Gerrit-PatchSet: 3
Gerrit-Owner: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.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: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Shelley Chen <shchen(a)google.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik(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: Thu, 28 Feb 2019 13:33:34 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29661 )
Change subject: soc/intel/braswell: Add support for FSP MR2
......................................................................
Patch Set 5:
> Patch Set 5:
>
> > Patch Set 5:
> >
> > Why don't you use the 3rdparty/fsp repo instead, like done in https://review.coreboot.org/#/c/coreboot/+/30742/ ?
>
> That would solve the problems with different headers. But the binary present in current FSP repo still has different signature and version which needs to be changed in coreboot according to used FSP blob revision, i.e. MR1 or MR2, thus the Kconfig option.
This problem occurs when 3rdparty/fsp is used. This FSP signature is not in sync with coreboot. coreboot expects 'old' signature (used in MR1).
--
To view, visit https://review.coreboot.org/c/coreboot/+/29661
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Id40b5d46ddda93845d9739b56aaf7ad24ee89246
Gerrit-Change-Number: 29661
Gerrit-PatchSet: 5
Gerrit-Owner: Frans Hendriks <fhendriks(a)eltan.com>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: David Guckian
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: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Nathaniel L Desimone <nathaniel.l.desimone(a)intel.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
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-CC: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-Comment-Date: Thu, 28 Feb 2019 13:30:59 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29661 )
Change subject: soc/intel/braswell: Add support for FSP MR2
......................................................................
Patch Set 5:
> Patch Set 5:
>
> Why don't you use the 3rdparty/fsp repo instead, like done in https://review.coreboot.org/#/c/coreboot/+/30742/ ?
That would solve the problems with different headers. But the binary present in current FSP repo still has different signature and version which needs to be changed in coreboot according to used FSP blob revision, i.e. MR1 or MR2, thus the Kconfig option.
--
To view, visit https://review.coreboot.org/c/coreboot/+/29661
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Id40b5d46ddda93845d9739b56aaf7ad24ee89246
Gerrit-Change-Number: 29661
Gerrit-PatchSet: 5
Gerrit-Owner: Frans Hendriks <fhendriks(a)eltan.com>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: David Guckian
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: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Nathaniel L Desimone <nathaniel.l.desimone(a)intel.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
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-CC: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-Comment-Date: Thu, 28 Feb 2019 13:29:15 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Philipp Deppenwiese has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31647 )
Change subject: drivers/intel/fsp2_0: Add more EFI return status into FSP2.0 driver
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/31647
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9f040e7b9232b05dfc34971afa190cc3cbd7192a
Gerrit-Change-Number: 31647
Gerrit-PatchSet: 1
Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com>
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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Thu, 28 Feb 2019 13:26:11 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment