HAOUAS Elyes has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36510 )
Change subject: sb/intel/common: Drop unused DO_NOT_TOUCH_DESCRIPTOR_REGION
......................................................................
sb/intel/common: Drop unused DO_NOT_TOUCH_DESCRIPTOR_REGION
Change-Id: I6ff071a00b2e185a8f3cc1f38fc35b6d55864e54
Signed-off-by: Elyes HAOUAS <ehaouas(a)noos.fr>
---
M src/southbridge/intel/common/firmware/Kconfig
1 file changed, 0 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/36510/1
diff --git a/src/southbridge/intel/common/firmware/Kconfig b/src/southbridge/intel/common/firmware/Kconfig
index eb63d34..b97d12f 100644
--- a/src/southbridge/intel/common/firmware/Kconfig
+++ b/src/southbridge/intel/common/firmware/Kconfig
@@ -147,15 +147,6 @@
help
This option allows you to protect flash regions.
-config DO_NOT_TOUCH_DESCRIPTOR_REGION
- bool "Use the preset values to protect the regions"
- help
- Read and write access permissions to different regions in the flash
- can be controlled via dedicated bitfields in the flash descriptor.
- These permissions can be modified with the Intel Flash Descriptor
- Tool (ifdtool). If you don't want to change these permissions and
- keep the ones provided in the initial descriptor, use this option.
-
config LOCK_MANAGEMENT_ENGINE
bool "Lock ME/TXE section"
help
--
To view, visit https://review.coreboot.org/c/coreboot/+/36510
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I6ff071a00b2e185a8f3cc1f38fc35b6d55864e54
Gerrit-Change-Number: 36510
Gerrit-PatchSet: 1
Gerrit-Owner: HAOUAS Elyes <ehaouas(a)noos.fr>
Gerrit-MessageType: newchange
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36087 )
Change subject: soc/intel/tigerlake: Do initial SoC commit
......................................................................
Patch Set 10:
> > > > That doesn't mean we shouldn't review the Tiger Lake addition. And
> > > > I think we all agreed already that a 8k LOC patch isn't reviewable.
> > > > And of course, if we had more common code already, it would be much
> > > > easier... and we would have a better code base if 3ee54bb hadn't
> > > > slipped in. Let's not repeat that error.
> > >
> > > Sure, i believe Arthur has done great work of taking pain to review this CL. we have address majority of his feedback and few more is there. will continue to address also will see if we can submit this CL into 4 part
> > >
> > > 1. Bootblock (soc/intel/tigerlake/bootblock/)
> > > 2. romstage (soc/intel/tigerlake/romstage/)
> > > 3. ramstage (soc/intel/tigerlake/)
> > > 4. acpi (soc/intel/tigerlake/acpi/)
> >
> > I still don't think "this CL" is a good idea, even if split up. Any
> > commit history that first copies stale code and then adapts it in
> > later commits (instead of adding the adapted code step by step) will
> > lose lots of information.
> >
> > For instance, I assume some developers have already looked at the
> > copied code and identified chunks that can stay the same. A copy
> > first, adapt later commit history will not contain this information.
> > It would contain details about what was changed, but not about what
> > wasn't. OTOH, if the commit that adds (pieces of) unchanged code
> > targets a single topic, and the commit message states that this was
> > confirmed to work on the new platform, nobody will have to doubt
> > if that was already checked. And the information will be in the Git
> > history and can help future maintenance.
>
> i understand you are suggesting for incremental patches to create TGL code in upstream. but the philosophy i'm referring is that, we have already reviewed ICL code many time and should't we allow ICL code as is when we are renaming as TGL (this is copy patch logic) and then add only 8 CL to make TGL code complete. Rather we have many CL (definitely more than 8) to even reach at any stage in incremental approach. I agree that incremental approach is easy to review entire code but do we really like to review ICL code again in TGL development?
Well, I don't think ICL code was much reviewed either. Every line got
a +2 at some point, yes. But that doesn't mean half of it wasn't rubber
stamped. And you seem to completely ignore that reviewing includes to
check available documentation (for the newer silicon and FSP).
If you want to catch subtle difference between ICL and TGL silicon
and FSP interface early, the incremental approach seems so much easier.
If you copy first, please mention in the commit message which parts
were identified to behave exactly the same and which will be adapted
in future commits. Then, reviewers can confirm that. However, if that
requires to look ahead in the patch queue for every other line of code,
you'll likely scare the best reviewers away.
--
To view, visit https://review.coreboot.org/c/coreboot/+/36087
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Id95e2fa9b7a7c6b3b9233d2c438b25a6c4904bbb
Gerrit-Change-Number: 36087
Gerrit-PatchSet: 10
Gerrit-Owner: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: John Zhao <john.zhao(a)intel.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Michael Niewöhner
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: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>
Gerrit-Reviewer: Raj Astekar <raj.astekar(a)intel.com>
Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-Comment-Date: Thu, 31 Oct 2019 15:28:21 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36462 )
Change subject: soc/intel/{cnl,icl,skl}: Remove unused SMI opregion
......................................................................
soc/intel/{cnl,icl,skl}: Remove unused SMI opregion
TEST=Able to build and boot Hatch and DE.
Change-Id: I6d63c005873fc5d67b4a44f42bb436628d7c1dc3
Signed-off-by: Subrata Banik <subrata.banik(a)intel.com>
---
M src/soc/intel/cannonlake/acpi/platform.asl
M src/soc/intel/icelake/acpi/platform.asl
M src/soc/intel/skylake/acpi/platform.asl
3 files changed, 0 insertions(+), 27 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/36462/1
diff --git a/src/soc/intel/cannonlake/acpi/platform.asl b/src/soc/intel/cannonlake/acpi/platform.asl
index 773cdcb..da61619 100644
--- a/src/soc/intel/cannonlake/acpi/platform.asl
+++ b/src/soc/intel/cannonlake/acpi/platform.asl
@@ -20,15 +20,6 @@
/* Generic indicator for sleep state */
#include <soc/intel/common/acpi/platform.asl>
-/* The APM port can be used for generating software SMIs */
-
-OperationRegion (APMP, SystemIO, 0xb2, 2)
-Field (APMP, ByteAcc, NoLock, Preserve)
-{
- APMC, 8, // APM command
- APMS, 8 // APM status
-}
-
/*
* The _PIC method is called by the OS to choose between interrupt
* routing via the i8259 interrupt controller or the APIC.
diff --git a/src/soc/intel/icelake/acpi/platform.asl b/src/soc/intel/icelake/acpi/platform.asl
index 080bf7b..b89fd46 100644
--- a/src/soc/intel/icelake/acpi/platform.asl
+++ b/src/soc/intel/icelake/acpi/platform.asl
@@ -18,15 +18,6 @@
/* Generic indicator for sleep state */
#include <soc/intel/common/acpi/platform.asl>
-/* The APM port can be used for generating software SMIs */
-
-OperationRegion (APMP, SystemIO, 0xb2, 2)
-Field (APMP, ByteAcc, NoLock, Preserve)
-{
- APMC, 8, // APM command
- APMS, 8 // APM status
-}
-
/*
* The _PIC method is called by the OS to choose between interrupt
* routing via the i8259 interrupt controller or the APIC.
diff --git a/src/soc/intel/skylake/acpi/platform.asl b/src/soc/intel/skylake/acpi/platform.asl
index 39debe1..19345b2 100644
--- a/src/soc/intel/skylake/acpi/platform.asl
+++ b/src/soc/intel/skylake/acpi/platform.asl
@@ -18,15 +18,6 @@
/* Enable ACPI _SWS methods */
#include <soc/intel/common/acpi/acpi_wake_source.asl>
-/* The APM port can be used for generating software SMIs */
-
-OperationRegion (APMP, SystemIO, 0xb2, 2)
-Field (APMP, ByteAcc, NoLock, Preserve)
-{
- APMC, 8, // APM command
- APMS, 8 // APM status
-}
-
/* Port 80 POST */
OperationRegion (POST, SystemIO, 0x80, 1)
--
To view, visit https://review.coreboot.org/c/coreboot/+/36462
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I6d63c005873fc5d67b4a44f42bb436628d7c1dc3
Gerrit-Change-Number: 36462
Gerrit-PatchSet: 1
Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-MessageType: newchange