Hello Felix Singer, V Sowmya, Nico Huber, Furquan Shaikh, Tim Wawrzynczak, Subrata Banik, Arthur Heymans, Michael Niewöhner, Patrick Rudolph,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/48575
to review the following change.
Change subject: soc/intel/skylake: Drop always-zero PowerLimit4 dt setting
......................................................................
soc/intel/skylake: Drop always-zero PowerLimit4 dt setting
Unset devicetree settings default to zero, so the devicetree setting can
be removed. Looks like no one needs it anyway.
Change-Id: Iad94538c5465347b37a99c6c9f20988168661593
Signed-off-by: Angel Pons <th3fanbus(a)gmail.com>
---
M src/mainboard/intel/kblrvp/variants/rvp11/overridetree.cb
M src/soc/intel/skylake/chip.c
M src/soc/intel/skylake/chip.h
3 files changed, 1 insertion(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/48575/1
diff --git a/src/mainboard/intel/kblrvp/variants/rvp11/overridetree.cb b/src/mainboard/intel/kblrvp/variants/rvp11/overridetree.cb
index 782f3dc..ad7c4ab 100644
--- a/src/mainboard/intel/kblrvp/variants/rvp11/overridetree.cb
+++ b/src/mainboard/intel/kblrvp/variants/rvp11/overridetree.cb
@@ -105,9 +105,6 @@
.tdp_pl2_override = 60,
}"
- # Power Limit Related
- register "PowerLimit4" = "0"
-
# Lock Down
register "common_soc_config" = "{
.chipset_lockdown = CHIPSET_LOCKDOWN_COREBOOT,
diff --git a/src/soc/intel/skylake/chip.c b/src/soc/intel/skylake/chip.c
index 3eb72fa..c9519cd 100644
--- a/src/soc/intel/skylake/chip.c
+++ b/src/soc/intel/skylake/chip.c
@@ -303,7 +303,7 @@
tconfig->PchLockDownGlobalSmi = config->LockDownConfigGlobalSmi;
tconfig->PchLockDownRtcLock = config->LockDownConfigRtcLock;
- tconfig->PowerLimit4 = config->PowerLimit4;
+ tconfig->PowerLimit4 = 0;
/*
* To disable HECI, the Psf needs to be left unlocked
* by FSP till end of post sequence. Based on the devicetree
diff --git a/src/soc/intel/skylake/chip.h b/src/soc/intel/skylake/chip.h
index 8b545ea..8d93ac0 100644
--- a/src/soc/intel/skylake/chip.h
+++ b/src/soc/intel/skylake/chip.h
@@ -87,9 +87,6 @@
/* TCC activation offset */
uint32_t tcc_offset;
- /* Package PL4 power limit in Watts */
- u32 PowerLimit4;
-
/* Whether to ignore VT-d support of the SKU */
int ignore_vtd;
--
To view, visit https://review.coreboot.org/c/coreboot/+/48575
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iad94538c5465347b37a99c6c9f20988168661593
Gerrit-Change-Number: 48575
Gerrit-PatchSet: 1
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com>
Gerrit-MessageType: newchange
Hello Felix Singer, V Sowmya, Nico Huber, Furquan Shaikh, Tim Wawrzynczak, Subrata Banik, Arthur Heymans, Michael Niewöhner, Patrick Rudolph,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/48574
to review the following change.
Change subject: soc/intel/skylake: Drop never-set DdrFreqLimit dt setting
......................................................................
soc/intel/skylake: Drop never-set DdrFreqLimit dt setting
Only Google Eve uses a non-zero value, but it overwrites in C code.
Drop the devicetree setting, since no mainboard uses it.
Change-Id: I14e0e0cb9baa2b1f8f795e6bc6ffbee300f2243d
Signed-off-by: Angel Pons <th3fanbus(a)gmail.com>
---
M src/soc/intel/skylake/chip.h
M src/soc/intel/skylake/romstage/romstage.c
2 files changed, 1 insertion(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/48574/1
diff --git a/src/soc/intel/skylake/chip.h b/src/soc/intel/skylake/chip.h
index 710c08d..8b545ea 100644
--- a/src/soc/intel/skylake/chip.h
+++ b/src/soc/intel/skylake/chip.h
@@ -93,12 +93,6 @@
/* Whether to ignore VT-d support of the SKU */
int ignore_vtd;
- /*
- * DDR Frequency Limit
- * 0(Auto), 1067, 1333, 1600, 1867, 2133, 2400
- */
- u16 DdrFreqLimit;
-
/* Probeless Trace function */
u8 ProbelessTrace;
diff --git a/src/soc/intel/skylake/romstage/romstage.c b/src/soc/intel/skylake/romstage/romstage.c
index a7ce2f8..79fb464 100644
--- a/src/soc/intel/skylake/romstage/romstage.c
+++ b/src/soc/intel/skylake/romstage/romstage.c
@@ -221,7 +221,7 @@
m_cfg->UserBd = BOARD_TYPE_ULT_ULX;
m_cfg->RMT = config->Rmt;
m_cfg->CmdTriStateDis = config->CmdTriStateDis;
- m_cfg->DdrFreqLimit = config->DdrFreqLimit;
+ m_cfg->DdrFreqLimit = 0;
m_cfg->VmxEnable = CONFIG(ENABLE_VMX);
m_cfg->PrmrrSize = get_valid_prmrr_size();
for (i = 0; i < ARRAY_SIZE(config->PcieRpEnable); i++) {
--
To view, visit https://review.coreboot.org/c/coreboot/+/48574
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I14e0e0cb9baa2b1f8f795e6bc6ffbee300f2243d
Gerrit-Change-Number: 48574
Gerrit-PatchSet: 1
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com>
Gerrit-MessageType: newchange
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48583 )
Change subject: soc/intel: hook up new gpio device in the soc chips
......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48583/7/src/soc/intel/alderlake/ch…
File src/soc/intel/alderlake/chip.c:
https://review.coreboot.org/c/coreboot/+/48583/7/src/soc/intel/alderlake/ch…
PS7, Line 202: else if (dev->path.type == DEVICE_PATH_GPIO)
: dev->ops = &soc_gpio_ops;
> Ah thanks, that could indeed work - I tried this before but wasn't sure how we get the ops assigned […]
Uhm, wait... no. That will *not* work....
Example:
chip soc/intel/whateverlake
dev gpio 0 alias soc_gpio on end
chip this/is/another/chip
dev gpio 0 alias chip_gpio on end
end
chip blah/bmc
use chip_gpio as gpio
end
chip no/idea
use soc_gpio as gpio
end
end
With your code the block gpio driver will get assigned to both gpio devices which is obviously wrong.
--
To view, visit https://review.coreboot.org/c/coreboot/+/48583
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I81dbbf5397b28ffa7537465c53332779245b39f6
Gerrit-Change-Number: 48583
Gerrit-PatchSet: 7
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(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: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Mon, 14 Dec 2020 09:02:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com>
Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-MessageType: comment