Attention is currently required from: Nico Huber, Tim Wawrzynczak, Angel Pons, Arthur Heymans, Patrick Rudolph.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56107 )
Change subject: sb/intel/common: Hide IFD options if !HAVE_IFD_BIN
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/56107
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I8736f32b4c695efbd68adf551e1376726c718b56
Gerrit-Change-Number: 56107
Gerrit-PatchSet: 1
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Tue, 06 Jul 2021 12:05:01 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Tim Wawrzynczak, Arthur Heymans, Patrick Rudolph.
Hello Nico Huber, Tim Wawrzynczak, Arthur Heymans, Patrick Rudolph,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/56107
to review the following change.
Change subject: sb/intel/common: Hide IFD options if !HAVE_IFD_BIN
......................................................................
sb/intel/common: Hide IFD options if !HAVE_IFD_BIN
When `HAVE_IFD_BIN` is not enabled, do not show IFD-related options.
Change-Id: I8736f32b4c695efbd68adf551e1376726c718b56
Signed-off-by: Angel Pons <th3fanbus(a)gmail.com>
---
M src/southbridge/intel/common/firmware/Kconfig
1 file changed, 3 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/07/56107/1
diff --git a/src/southbridge/intel/common/firmware/Kconfig b/src/southbridge/intel/common/firmware/Kconfig
index 36dde56..a1026e8 100644
--- a/src/southbridge/intel/common/firmware/Kconfig
+++ b/src/southbridge/intel/common/firmware/Kconfig
@@ -57,6 +57,7 @@
config ME_REGION_ALLOW_CPU_READ_ACCESS
bool "Allows HOST/CPU read access to ME region"
+ depends on HAVE_IFD_BIN
default y if SOC_INTEL_CSE_LITE_SKU
default n
help
@@ -137,8 +138,8 @@
default "3rdparty/blobs/mainboard/\$(MAINBOARDDIR)/ec.bin"
choice
- prompt "Protect flash regions"
- default UNLOCK_FLASH_REGIONS
+ prompt "Protect flash regions" if HAVE_IFD_BIN
+ default UNLOCK_FLASH_REGIONS if HAVE_IFD_BIN
help
This option allows you to protect flash regions.
--
To view, visit https://review.coreboot.org/c/coreboot/+/56107
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I8736f32b4c695efbd68adf551e1376726c718b56
Gerrit-Change-Number: 56107
Gerrit-PatchSet: 1
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-MessageType: newchange
Attention is currently required from: Jan Tatje, Paul Menzel, Stefan Reinauer.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56070 )
Change subject: util/ifdtool: Add sklkbl to IFDv2 platforms
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/56070
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I3f92b090e929336b5c18b442d1504ee1000f5594
Gerrit-Change-Number: 56070
Gerrit-PatchSet: 2
Gerrit-Owner: Jan Tatje <jan(a)jnt.io>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
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)mailbox.org>
Gerrit-Attention: Jan Tatje <jan(a)jnt.io>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Comment-Date: Tue, 06 Jul 2021 11:57:02 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Subrata Banik.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/54305 )
Change subject: util/ifdtool: Use -p platform name to detect IFDv2 platform and chipset
......................................................................
Patch Set 10:
(1 comment)
Patchset:
PS10:
> > > Earlier if your platform was entering into earlier if loop and touching bit 7:0 then it was doin […]
I checked CB:56070 against document 551377 (KBL-H SPI Programming Guide) and all the implications of using IFD v2 are correct, i.e. the registers will be decoded properly.
--
To view, visit https://review.coreboot.org/c/coreboot/+/54305
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I25f69ce775454409974056d8326c02e29038ec8a
Gerrit-Change-Number: 54305
Gerrit-PatchSet: 10
Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Angel Pons <angel.pons(a)9elements.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Lean Sheng Tan <lean.sheng.tan(a)intel.com>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com>
Gerrit-CC: Jan Tatje <jan(a)jnt.io>
Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Subrata Banik <subi.banik(a)gmail.com>
Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Comment-Date: Tue, 06 Jul 2021 11:54:47 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Jan Tatje <jan(a)jnt.io>
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Comment-In-Reply-To: Subrata Banik <subrata.banik(a)intel.com>
Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-MessageType: comment
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/54305 )
Change subject: util/ifdtool: Use -p platform name to detect IFDv2 platform and chipset
......................................................................
Patch Set 10:
(1 comment)
Patchset:
PS10:
> > Earlier if your platform was entering into earlier if loop and touching bit 7:0 then it was doing wrong programming.
>
> I either don't understand that sentence correctly or it is the opposite, the else clause is what touches bit 7:0.
Its exactly opposite what i said, SKL SPI programming guide says, Bit 7:0 are reserved and other IFDv2 chipset has extended ranges hence it might need to protect.
if (ifd_version >= IFD_VERSION_2) {
/* Clear non-reserved bits */
fmba->flmstr1 &= 0xff;
fmba->flmstr2 &= 0xff;
fmba->flmstr3 &= 0xff;
fmba->flmstr5 &= 0xff;
} else {
wr_shift = FLMSTR_WR_SHIFT_V1;
rd_shift = FLMSTR_RD_SHIFT_V1;
fmba->flmstr1 = 0;
fmba->flmstr2 = 0;
/* Requestor ID */
fmba->flmstr3 = 0x118;
}
>
> If ifd_version is set to IFD_VERSION_2 like it was before this change, then it will not touch bits 7:0 and set read and write permissions to true. In the else-clause of the condition, which it uses now, both 7:0 reserved bits are changed and (I think this is what breaks boot) read permission bits 15:8 are set to 0.
if you could download the SPI programming guide from this link
https://www.intel.com/content/www/us/en/search.html?ws=text#q=550696&t=All
and go to page 33, Flash Descriptor Master Section, you will see 7:0 are reserved and what I could sense is that, existing code itself had issue
https://review.coreboot.org/c/coreboot/+/56070/1/util/ifdtool/ifdtool.c#1278 should be as below to protect bits 15:8 as well.
fmba->flmstr1 = 0xffffff00;
fmba->flmstr2 = 0xffffff00;
>
> > I would say, it was wrong that SKL was considered as IFDv2.
> Even if IFDv2 is supposed to mean that 7:0 of the flash master are used for Extended Region Access Permissions somewhere else, in ifdtool it just decides whether FLMSTR uses 31:16 or 31:8 for permissions.
>
> IMHO adding SKLKBL to the ifd_2_platforms list is the easiest solution to fix this issue. This is the change I applied to make it work for me: https://review.coreboot.org/c/coreboot/+/56070
Adding SKL into IFDv2 would be an easy W/A as SKL flash descriptor master register offsets are very close to IFDv2 (except the last byte).
Have you verified other fields as well with CB:56070 for skl platform?
--
To view, visit https://review.coreboot.org/c/coreboot/+/54305
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I25f69ce775454409974056d8326c02e29038ec8a
Gerrit-Change-Number: 54305
Gerrit-PatchSet: 10
Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Angel Pons <angel.pons(a)9elements.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Lean Sheng Tan <lean.sheng.tan(a)intel.com>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com>
Gerrit-CC: Jan Tatje <jan(a)jnt.io>
Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Subrata Banik <subi.banik(a)gmail.com>
Gerrit-Comment-Date: Tue, 06 Jul 2021 11:43:52 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Jan Tatje <jan(a)jnt.io>
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Comment-In-Reply-To: Subrata Banik <subrata.banik(a)intel.com>
Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-MessageType: comment
Attention is currently required from: Paul Menzel, Stefan Reinauer.
Jan Tatje has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56070 )
Change subject: util/ifdtool: Add sklkbl to IFDv2 platforms
......................................................................
Patch Set 2:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/56070/comment/a19ce27c_7f1c58f2
PS1, Line 12:
> Please add a Fixes: tag: […]
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/56070
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I3f92b090e929336b5c18b442d1504ee1000f5594
Gerrit-Change-Number: 56070
Gerrit-PatchSet: 2
Gerrit-Owner: Jan Tatje <jan(a)jnt.io>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Comment-Date: Tue, 06 Jul 2021 11:30:56 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: comment
Attention is currently required from: Jan Tatje, Stefan Reinauer.
Hello build bot (Jenkins), Stefan Reinauer, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/56070
to look at the new patch set (#2).
Change subject: util/ifdtool: Add sklkbl to IFDv2 platforms
......................................................................
util/ifdtool: Add sklkbl to IFDv2 platforms
Currently ifdtool breaks the descriptor because it treats it as IFDv1.
This change adds it to the list of IFDv2 platforms.
Fixes boot for X11SSH-LN4F.
Fixes: 8c082e5fef ("util/ifdtool: Use -p platform name to detect IFDv2 platform and chipset")
Change-Id: I3f92b090e929336b5c18b442d1504ee1000f5594
Signed-off-by: Jan Tatje <jan(a)jnt.io>
---
M util/ifdtool/ifdtool.c
1 file changed, 4 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/56070/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/56070
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I3f92b090e929336b5c18b442d1504ee1000f5594
Gerrit-Change-Number: 56070
Gerrit-PatchSet: 2
Gerrit-Owner: Jan Tatje <jan(a)jnt.io>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jan Tatje <jan(a)jnt.io>
Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Jan Tatje, Stefan Reinauer.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56070 )
Change subject: util/ifdtool: Add sklkbl to IFDv2 platforms
......................................................................
Patch Set 1: Code-Review+2
(1 comment)
Patchset:
PS1:
> I'll check this against the SPI programming guide.
IFD v2 is the right choice.
--
To view, visit https://review.coreboot.org/c/coreboot/+/56070
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I3f92b090e929336b5c18b442d1504ee1000f5594
Gerrit-Change-Number: 56070
Gerrit-PatchSet: 1
Gerrit-Owner: Jan Tatje <jan(a)jnt.io>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jan Tatje <jan(a)jnt.io>
Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Comment-Date: Tue, 06 Jul 2021 11:24:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Jan Tatje, Stefan Reinauer.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56070 )
Change subject: util/ifdtool: Add sklkbl to IFDv2 platforms
......................................................................
Patch Set 1:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/56070/comment/c4a50467_5b4a1426
PS1, Line 12:
Please add a Fixes: tag:
> Fixes: 8c082e5fef ("util/ifdtool: Use -p platform name to detect IFDv2 platform and chipset")
--
To view, visit https://review.coreboot.org/c/coreboot/+/56070
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I3f92b090e929336b5c18b442d1504ee1000f5594
Gerrit-Change-Number: 56070
Gerrit-PatchSet: 1
Gerrit-Owner: Jan Tatje <jan(a)jnt.io>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jan Tatje <jan(a)jnt.io>
Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Comment-Date: Tue, 06 Jul 2021 11:21:22 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Kevin Chang.
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/56100
to look at the new patch set (#2).
Change subject: grunt/treeya: add Realtek ALC5682 codec support
......................................................................
grunt/treeya: add Realtek ALC5682 codec support
ALC5682 i2c address: 0x1A
BUG=b:185972050
BRANCH=master
TEST=emerge-grunt coreboot
Signed-off-by: Kevin Chang <kevin.chang(a)lcfc.corp-partner.google.com>
Change-Id: I49c673fd944b2c2a79c4283eee941a16596ba7fa
---
M src/mainboard/google/kahlee/variants/treeya/Makefile.inc
A src/mainboard/google/kahlee/variants/treeya/audio.c
M src/mainboard/google/kahlee/variants/treeya/devicetree.cb
A src/mainboard/google/kahlee/variants/treeya/include/variant/sku.h
4 files changed, 85 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/00/56100/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/56100
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I49c673fd944b2c2a79c4283eee941a16596ba7fa
Gerrit-Change-Number: 56100
Gerrit-PatchSet: 2
Gerrit-Owner: Kevin Chang <kevin.chang(a)lcfc.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Kevin Chang <kevin.chang(a)lcfc.corp-partner.google.com>
Gerrit-MessageType: newpatchset