cedarhouse1(a)comcast.net has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38734 )
Change subject: cpu/x86: Put guard around align for smm_save_state_size
......................................................................
cpu/x86: Put guard around align for smm_save_state_size
The STM support aligns the smm_save_state_size. However, this creates
issue for some platforms because of this value being hard coded to
0x400
Signed-off-by: Eugene D. Myers <edmyers(a)tycho.nsa.gov>
Change-Id: Ia584f7e9b86405a12eb6cbedc3a2615a8727f69e
---
M src/cpu/x86/mp_init.c
1 file changed, 3 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/38734/1
diff --git a/src/cpu/x86/mp_init.c b/src/cpu/x86/mp_init.c
index 331f3b5..aba50bf 100644
--- a/src/cpu/x86/mp_init.c
+++ b/src/cpu/x86/mp_init.c
@@ -1053,8 +1053,9 @@
* note: In the future, this will need to handle newer x86 processors
* that require alignment of the save state on 32K boundaries.
*/
- state->smm_save_state_size =
- ALIGN_UP(state->smm_save_state_size, 0x1000);
+ if (CONFIG(STM))
+ state->smm_save_state_size =
+ ALIGN_UP(state->smm_save_state_size, 0x1000);
/*
* Default to smm_initiate_relocation() if trigger callback isn't
--
To view, visit https://review.coreboot.org/c/coreboot/+/38734
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ia584f7e9b86405a12eb6cbedc3a2615a8727f69e
Gerrit-Change-Number: 38734
Gerrit-PatchSet: 1
Gerrit-Owner: cedarhouse1(a)comcast.net
Gerrit-MessageType: newchange
Hello Anjaneya "Reddy" Chagam,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/38727
to review the following change.
Change subject: device: Expose round()
......................................................................
device: Expose round()
This function is needed by Skylake-SP SOC code, so exposing it.
Signed-off-by: Jonathan Zhang <jonzhang(a)fb.com>
Signed-off-by: Reddy Chagam <anjaneya.chagam(a)intel.com>
Change-Id: I317dad18176fc2339072f02a5bcdcc8859596df9
---
M src/device/device.c
M src/include/device/device.h
2 files changed, 10 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/38727/1
diff --git a/src/device/device.c b/src/device/device.c
index 236b768..9bd8f45 100644
--- a/src/device/device.c
+++ b/src/device/device.c
@@ -162,7 +162,7 @@
* @param pow Alignment as a power of two.
* @return Rounded up number.
*/
-static resource_t round(resource_t val, unsigned long pow)
+resource_t round(resource_t val, unsigned long pow)
{
resource_t mask;
mask = (1ULL << pow) - 1ULL;
diff --git a/src/include/device/device.h b/src/include/device/device.h
index 2d7400b..9a7bacd 100644
--- a/src/include/device/device.h
+++ b/src/include/device/device.h
@@ -322,4 +322,13 @@
void scan_generic_bus(struct device *bus);
void scan_static_bus(struct device *bus);
+/**
+ * Round a number up to an alignment.
+ *
+ * @param val The starting value.
+ * @param pow Alignment as a power of two.
+ * @return Rounded up number.
+ */
+resource_t round(resource_t val, unsigned long pow);
+
#endif /* DEVICE_H */
--
To view, visit https://review.coreboot.org/c/coreboot/+/38727
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I317dad18176fc2339072f02a5bcdcc8859596df9
Gerrit-Change-Number: 38727
Gerrit-PatchSet: 1
Gerrit-Owner: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-Reviewer: Anjaneya "Reddy" Chagam <anjaneya.chagam(a)intel.com>
Gerrit-MessageType: newchange
Name of user not set #1002789 has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38722 )
Change subject: src/northbridge/intel/haswell/early_init.c: Fix type definition of dev in PCI_FUNC(dev)
......................................................................
src/northbridge/intel/haswell/early_init.c: Fix type definition of dev in PCI_FUNC(dev)
The type of dev in the PCI_FUNC(dev) is incorrect. Fix it using PCI_DEV2DEVFN()
function. Tested on a T440P, and necessary on this board to enable the dGPU.
Will submit subsequent patches to enable dGPU.
Change-Id: I3fb0f677cc98800f355f6af7d3172be3e59ce5c2
Signed-off-by: Chris Morgan <macromorgan(a)hotmail.com>
---
M src/northbridge/intel/haswell/early_init.c
1 file changed, 3 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/22/38722/1
diff --git a/src/northbridge/intel/haswell/early_init.c b/src/northbridge/intel/haswell/early_init.c
index 666bda2..6aad4a3 100644
--- a/src/northbridge/intel/haswell/early_init.c
+++ b/src/northbridge/intel/haswell/early_init.c
@@ -101,7 +101,7 @@
}
pci_update_config32(dev, 0xc24, ~(1 << 16), 1 << 5);
- printk(BIOS_DEBUG, "Started PEG1%d link training.\n", PCI_FUNC(dev));
+ printk(BIOS_DEBUG, "Started PEG1%d link training.\n", PCI_FUNC(PCI_DEV2DEVFN(dev)));
/*
* The PEG device is hidden while the MRC runs. This is because the
@@ -110,8 +110,8 @@
* to these configurations.
*/
pci_update_config32(PCI_DEV(0, 0, 0), DEVEN, ~mask, 0);
- peg_hidden[PCI_FUNC(dev)] = true;
- printk(BIOS_DEBUG, "Temporarily hiding PEG1%d.\n", PCI_FUNC(dev));
+ peg_hidden[PCI_FUNC(PCI_DEV2DEVFN(dev))] = true;
+ printk(BIOS_DEBUG, "Temporarily hiding PEG1%d.\n", PCI_FUNC(PCI_DEV2DEVFN(dev)));
}
void haswell_unhide_peg(void)
--
To view, visit https://review.coreboot.org/c/coreboot/+/38722
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I3fb0f677cc98800f355f6af7d3172be3e59ce5c2
Gerrit-Change-Number: 38722
Gerrit-PatchSet: 1
Gerrit-Owner: Name of user not set #1002789
Gerrit-MessageType: newchange
Divya Sasidharan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37870 )
Change subject: soc/intel/tigerlake: Add code for early tcss
......................................................................
Patch Set 22:
(2 comments)
https://review.coreboot.org/c/coreboot/+/37870/22/src/soc/intel/tigerlake/e…
File src/soc/intel/tigerlake/early_tcss.c:
https://review.coreboot.org/c/coreboot/+/37870/22/src/soc/intel/tigerlake/e…
PS22, Line 79: req_size = 2;
Suggestion: Add macros for the different request size for different commands. Line 92, 127, 156 has this variable.
https://review.coreboot.org/c/coreboot/+/37870/22/src/soc/intel/tigerlake/e…
PS22, Line 169:
Do you need to add condition for safe mode. If coreboot polls and reads safe mode from EC, what should be done.
--
To view, visit https://review.coreboot.org/c/coreboot/+/37870
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I45c3fe9d4a2ec2f2f51b78cca2bd7e623540c00e
Gerrit-Change-Number: 37870
Gerrit-PatchSet: 22
Gerrit-Owner: Shaunak Saha <shaunak.saha(a)intel.com>
Gerrit-Reviewer: Brandon Breitenstein <brandon.breitenstein(a)intel.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Keith Short <keithshort(a)chromium.org>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Raj Astekar <raj.astekar(a)intel.com>
Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com>
Gerrit-Reviewer: Shaunak Saha <shaunak.saha(a)intel.com>
Gerrit-Reviewer: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: caveh jalali <caveh(a)chromium.org>
Gerrit-CC: Divya Sasidharan <divya.s.sasidharan(a)intel.corp-partner.google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-CC: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com>
Gerrit-CC: Vijay P Hiremath <vijay.p.hiremath(a)intel.com>
Gerrit-Comment-Date: Thu, 06 Feb 2020 19:00:28 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37867 )
Change subject: src/ec/google/chromeec: Get Type-C Mux info from EC (TCPM)
......................................................................
Patch Set 14:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37867/11/src/ec/google/chromeec/ec…
File src/ec/google/chromeec/ec.c:
https://review.coreboot.org/c/coreboot/+/37867/11/src/ec/google/chromeec/ec…
PS11, Line 1542: /**
: * Check for the current mux state in EC
: * Flags representing mux state
: * USB_PD_MUX_USB_ENABLED BIT(0)
: * USB_PD_MUX_DP_ENABLED BIT(1)
: * USB_PD_MUX_POLARITY_INVERTED BIT(2)
: * USB_PD_MUX_HPD_IRQ BIT(3)
: * USB_PD_MUX_HPD_LVL BIT(4)
: * USB_PD_MUX_SAFE_MODE BIT(5)
: *
: */
> case in point: it's already out of sync from master :)
I will remove this and say the flags can be found in ec_commands.h
--
To view, visit https://review.coreboot.org/c/coreboot/+/37867
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I96bfb6036f4340ba42a078cfd7ecaae777a3ed00
Gerrit-Change-Number: 37867
Gerrit-PatchSet: 14
Gerrit-Owner: Shaunak Saha <shaunak.saha(a)intel.com>
Gerrit-Reviewer: Brandon Breitenstein <brandon.breitenstein(a)intel.com>
Gerrit-Reviewer: Divya S Sasidharan <divya.s.sasidharan(a)intel.com>
Gerrit-Reviewer: Keith Short <keithshort(a)chromium.org>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Raj Astekar <raj.astekar(a)intel.com>
Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com>
Gerrit-Reviewer: Shaunak Saha <shaunak.saha(a)intel.com>
Gerrit-Reviewer: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: caveh jalali <caveh(a)chromium.org>
Gerrit-CC: Furquan Shaikh <furquan(a)google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-CC: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com>
Gerrit-CC: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-CC: Vijay P Hiremath <vijay.p.hiremath(a)intel.com>
Gerrit-Comment-Date: Thu, 06 Feb 2020 17:45:29 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: caveh jalali <caveh(a)chromium.org>
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-MessageType: comment
Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37871 )
Change subject: soc/intel/common/block: Enable PMC IPC driver
......................................................................
Patch Set 16:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37871/16/src/soc/intel/common/bloc…
File src/soc/intel/common/block/include/intelblocks/pmclib.h:
https://review.coreboot.org/c/coreboot/+/37871/16/src/soc/intel/common/bloc…
PS16, Line 33: /* pmc_ipc_cmd struct declaration to store cmd data for IPC commands */
: struct pmc_ipc_cmd {
: uint32_t cmd:8;
: uint32_t msi:1;
: uint32_t res_1:3;
: uint32_t subcmd:4;
: uint32_t len:8;
: uint32_t res_2:8;
: };
> is it used?
it is the struct for cmd it is used from early tcss
--
To view, visit https://review.coreboot.org/c/coreboot/+/37871
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ibd3ed262fc700ccc891ec68997a108f5bfbaf9ed
Gerrit-Change-Number: 37871
Gerrit-PatchSet: 16
Gerrit-Owner: Shaunak Saha <shaunak.saha(a)intel.com>
Gerrit-Reviewer: Brandon Breitenstein <brandon.breitenstein(a)intel.com>
Gerrit-Reviewer: Keith Short <keithshort(a)chromium.org>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Raj Astekar <raj.astekar(a)intel.com>
Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com>
Gerrit-Reviewer: Shaunak Saha <shaunak.saha(a)intel.com>
Gerrit-Reviewer: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com>
Gerrit-Reviewer: Vijay P Hiremath <vijay.p.hiremath(a)intel.com>
Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: caveh jalali <caveh(a)chromium.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Furquan Shaikh <furquan(a)google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-CC: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com>
Gerrit-Comment-Date: Thu, 06 Feb 2020 17:45:11 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Vijay P Hiremath <vijay.p.hiremath(a)intel.com>
Gerrit-MessageType: comment
Jonathan Kollasch has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/22214 )
Change subject: nb/intel/sandybridge/raminit: Add ECC detection support
......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/22214/9/src/northbridge/intel/sand…
File src/northbridge/intel/sandybridge/raminit.c:
https://review.coreboot.org/c/coreboot/+/22214/9/src/northbridge/intel/sand…
PS9, Line 407: ctrl.ecc_forced ? "yes" : "no");
> I was wondering why we don't have to read SPDs on this path (see next
> line)... the only way to get here is to run through `if (!fastboot)` above,
> so we gathered/printed this information already. Code flow is a mess :-/
> it seems somebody tried to save indentation levels.
The argument about code structure and necessity of (re-?)reading the SPD is in my opinion outside the scope of this change.
It does appear that this message would be printed twice should the err path be taken, and can therefore be removed.
>
> Also, this seems informational, BIOS_INFO?
Almost all the other printks in this file are BIOS_DEBUG. I don't think this one is more significant than those.
There is a printk in #22215 (the one that indicates ECC is being enabled) that should probably be advanced from BIOS_DEBUG to BIOS_INFO however.
--
To view, visit https://review.coreboot.org/c/coreboot/+/22214
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I5b7599746195cfa996a48320404a8dbe6820483a
Gerrit-Change-Number: 22214
Gerrit-PatchSet: 9
Gerrit-Owner: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com>
Gerrit-Reviewer: Jonathan Kollasch <jakllsch(a)kollasch.net>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
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-Comment-Date: Thu, 06 Feb 2020 16:49:32 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
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/+/22214 )
Change subject: nb/intel/sandybridge/raminit: Add ECC detection support
......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/22214/9/src/northbridge/intel/sand…
File src/northbridge/intel/sandybridge/raminit.c:
https://review.coreboot.org/c/coreboot/+/22214/9/src/northbridge/intel/sand…
PS9, Line 407: ctrl.ecc_forced ? "yes" : "no");
I was wondering why we don't have to read SPDs on this path (see next
line)... the only way to get here is to run through `if (!fastboot)` above,
so we gathered/printed this information already. Code flow is a mess :-/
it seems somebody tried to save indentation levels.
Also, this seems informational, BIOS_INFO?
--
To view, visit https://review.coreboot.org/c/coreboot/+/22214
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I5b7599746195cfa996a48320404a8dbe6820483a
Gerrit-Change-Number: 22214
Gerrit-PatchSet: 9
Gerrit-Owner: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com>
Gerrit-Reviewer: Jonathan Kollasch <jakllsch(a)kollasch.net>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
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-Comment-Date: Thu, 06 Feb 2020 09:59:00 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment