Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson.
Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/60119 )
Change subject: soc/amd/common/block/spi/fch_spi_ctrl: handle failure in execute_command
......................................................................
soc/amd/common/block/spi/fch_spi_ctrl: handle failure in execute_command
When wait_for_ready returned a timeout, execute_command still ended up
returning success. Fix this be returning a failure in this case.
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: Id012e74e26065c12d003793322dcdd448df758b0
---
M src/soc/amd/common/block/spi/fch_spi_ctrl.c
1 file changed, 4 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/19/60119/1
diff --git a/src/soc/amd/common/block/spi/fch_spi_ctrl.c b/src/soc/amd/common/block/spi/fch_spi_ctrl.c
index aad8de7..33e1139 100644
--- a/src/soc/amd/common/block/spi/fch_spi_ctrl.c
+++ b/src/soc/amd/common/block/spi/fch_spi_ctrl.c
@@ -97,9 +97,10 @@
spi_write8(SPI_CMD_TRIGGER, SPI_CMD_TRIGGER_EXECUTE);
- if (wait_for_ready())
- printk(BIOS_ERR,
- "FCH_SC Error: Timeout executing command\n");
+ if (wait_for_ready()) {
+ printk(BIOS_ERR, "FCH_SC Error: Timeout executing command\n");
+ return -1;
+ }
dump_state(SPI_DUMP_STATE_AFTER_CMD);
--
To view, visit https://review.coreboot.org/c/coreboot/+/60119
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Id012e74e26065c12d003793322dcdd448df758b0
Gerrit-Change-Number: 60119
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-MessageType: newchange
Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson.
Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/60117 )
Change subject: soc/amd/common/include/spi: add Cezanne-specific comment
......................................................................
soc/amd/common/include/spi: add Cezanne-specific comment
The Cezanne PPR #56569 Rev 3.03 has one more SPI FIFO bytes defined
compared to the previous generations. It is unclear if adding some
special handling for Cezanne would be worth the effort, since the
current code just doesn't use the last byte which should be safe to do,
since this only affects the maximum number of bytes that can be used for
one SPI transaction. Having another byte to use on Cezanne wouldn't
reduce the number of SPI transactions to write a 256 byte data block.
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: Ic730f4fe838f59066120c811833995c132c84c1c
---
M src/soc/amd/common/block/include/amdblocks/spi.h
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/60117/1
diff --git a/src/soc/amd/common/block/include/amdblocks/spi.h b/src/soc/amd/common/block/include/amdblocks/spi.h
index 4efed68..4be3739 100644
--- a/src/soc/amd/common/block/include/amdblocks/spi.h
+++ b/src/soc/amd/common/block/include/amdblocks/spi.h
@@ -71,7 +71,7 @@
#define SPI_RD4DW_EN_HOST BIT(15)
#define SPI_FIFO 0x80
-#define SPI_FIFO_LAST_BYTE 0xc6
+#define SPI_FIFO_LAST_BYTE 0xc6 /* 0xc7 for Cezanne */
#define SPI_FIFO_DEPTH (SPI_FIFO_LAST_BYTE - SPI_FIFO + 1)
struct spi_config {
--
To view, visit https://review.coreboot.org/c/coreboot/+/60117
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic730f4fe838f59066120c811833995c132c84c1c
Gerrit-Change-Number: 60117
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-MessageType: newchange
Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/60116 )
Change subject: soc/amd/common/include/spi: fix SPI_FIFO_LAST_BYTE define
......................................................................
soc/amd/common/include/spi: fix SPI_FIFO_LAST_BYTE define
The last byte of the SPI FIFO SPI_FIFO_LAST_BYTE is at offset 0xc6 of
the SPI controller's MMIO region for Stoneyridge and Picasso. Both
SPI_FIFO_LAST_BYTE and SPI_FIFO_DEPTH had an off-by-one error that ended
up cancelling out each other, so the resulting value for SPI_FIFO_DEPTH
isn't changed.
TEST=Timeless build results in identical image for Mandolin.
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: I1676be902ccf57e2e9f69d81251b4315866a0628
---
M src/soc/amd/common/block/include/amdblocks/spi.h
1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/60116/1
diff --git a/src/soc/amd/common/block/include/amdblocks/spi.h b/src/soc/amd/common/block/include/amdblocks/spi.h
index 5c3bd0e..4efed68 100644
--- a/src/soc/amd/common/block/include/amdblocks/spi.h
+++ b/src/soc/amd/common/block/include/amdblocks/spi.h
@@ -71,8 +71,8 @@
#define SPI_RD4DW_EN_HOST BIT(15)
#define SPI_FIFO 0x80
-#define SPI_FIFO_LAST_BYTE 0xc7
-#define SPI_FIFO_DEPTH (SPI_FIFO_LAST_BYTE - SPI_FIFO)
+#define SPI_FIFO_LAST_BYTE 0xc6
+#define SPI_FIFO_DEPTH (SPI_FIFO_LAST_BYTE - SPI_FIFO + 1)
struct spi_config {
/*
--
To view, visit https://review.coreboot.org/c/coreboot/+/60116
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I1676be902ccf57e2e9f69d81251b4315866a0628
Gerrit-Change-Number: 60116
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: newchange
Attention is currently required from: Reka Norman, Krishna P Bhat D, Usha P, Kangheui Won.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59915 )
Change subject: mb/intel/adlrvp_n: Add initial code for adl-n variant board
......................................................................
Patch Set 7: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/59915
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I4abf3bf62ec0398ae75e21575a2fab0d44b5c7ad
Gerrit-Change-Number: 59915
Gerrit-PatchSet: 7
Gerrit-Owner: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Kangheui Won <khwon(a)chromium.org>
Gerrit-Reviewer: Kangheui Won <khwon(a)google.com>
Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Reviewer: Reka Norman <rekanorman(a)chromium.org>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Usha P <usha.p(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Reka Norman <rekanorman(a)chromium.org>
Gerrit-Attention: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com>
Gerrit-Attention: Usha P <usha.p(a)intel.com>
Gerrit-Attention: Kangheui Won <khwon(a)google.com>
Gerrit-Comment-Date: Tue, 14 Dec 2021 21:55:53 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Julius Werner has submitted this change. ( https://review.coreboot.org/c/coreboot/+/60085 )
Change subject: cbfstool: Clean up remnants of locate action
......................................................................
cbfstool: Clean up remnants of locate action
`cbfstool locate` and the associated -T switch were removed a looong
time ago (2015 in CB:11671). However, getopt and the help text weren't
cleaned up correctly. Fix that.
Signed-off-by: Julius Werner <jwerner(a)chromium.org>
Change-Id: Ib098278d68df65d348528fbfd2496b5737ca6246
Reviewed-on: https://review.coreboot.org/c/coreboot/+/60085
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Raul Rangel <rrangel(a)chromium.org>
---
M util/cbfstool/cbfstool.c
1 file changed, 1 insertion(+), 6 deletions(-)
Approvals:
build bot (Jenkins): Verified
Raul Rangel: Looks good to me, approved
diff --git a/util/cbfstool/cbfstool.c b/util/cbfstool/cbfstool.c
index a84601e..608cd32 100644
--- a/util/cbfstool/cbfstool.c
+++ b/util/cbfstool/cbfstool.c
@@ -1771,7 +1771,6 @@
{"pow2page", no_argument, 0, 'Q' },
{"ucode-region", required_argument, 0, 'q' },
{"size", required_argument, 0, 's' },
- {"top-aligned", required_argument, 0, 'T' },
{"type", required_argument, 0, 't' },
{"verbose", no_argument, 0, 'v' },
{"with-readonly", no_argument, 0, 'w' },
@@ -1927,9 +1926,6 @@
"Create a legacy ROM file with CBFS master header*\n"
" create -M flashmap [-r list,of,regions,containing,cbfses] "
"Create a new-style partitioned firmware image\n"
- " locate [-r image,regions] -f FILE -n NAME [-P page-size] \\\n"
- " [-a align] [-T] "
- "Find a place for a file of that size\n"
" layout [-w] "
"List mutable (or, with -w, readable) image regions\n"
" print [-r image,regions] [-k] "
@@ -1964,8 +1960,7 @@
" specifying the location of this FMAP itself and a '%s'\n"
" section describing the primary CBFS. It should also be noted\n"
" that, when working with such images, the -F and -r switches\n"
- " default to '%s' for convenience, and both the -b switch to\n"
- " CBFS operations and the output of the locate action become\n"
+ " default to '%s' for convenience, and the -b switch becomes\n"
" relative to the selected CBFS region's lowest address.\n"
" The one exception to this rule is the top-aligned address,\n"
" which is always relative to the end of the entire image\n"
--
To view, visit https://review.coreboot.org/c/coreboot/+/60085
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib098278d68df65d348528fbfd2496b5737ca6246
Gerrit-Change-Number: 60085
Gerrit-PatchSet: 2
Gerrit-Owner: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
Julius Werner has submitted this change. ( https://review.coreboot.org/c/coreboot/+/60084 )
Change subject: cbfstool: Use converted buffer size for do_cbfs_locate()
......................................................................
cbfstool: Use converted buffer size for do_cbfs_locate()
The whole point of moving do_cbfs_locate() later (CB:59877) was that it
could use the file size that is actually going to be inserted into CBFS,
rather than the on-disk file size. Unfortunately, after all that work I
forgot to actually make it do that. This patch fixes that.
Since there is no more use case for do_cbfs_locate() having to figure
out the file size on its own, and that generally seems to be a bad idea
(as the original issue shows), also remove that part of it completely
and make the data_size parameter mandatory.
Signed-off-by: Julius Werner <jwerner(a)chromium.org>
Change-Id: I1af35e8e388f78aae3593c029afcfb4e510d2b8f
Reviewed-on: https://review.coreboot.org/c/coreboot/+/60084
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Raul Rangel <rrangel(a)chromium.org>
---
M util/cbfstool/cbfstool.c
1 file changed, 6 insertions(+), 18 deletions(-)
Approvals:
build bot (Jenkins): Verified
Raul Rangel: Looks good to me, approved
diff --git a/util/cbfstool/cbfstool.c b/util/cbfstool/cbfstool.c
index 77460d1..a84601e 100644
--- a/util/cbfstool/cbfstool.c
+++ b/util/cbfstool/cbfstool.c
@@ -511,11 +511,6 @@
{
uint32_t metadata_size = 0;
- if (!param.filename) {
- ERROR("You need to specify -f/--filename.\n");
- return 1;
- }
-
if (!param.name) {
ERROR("You need to specify -n/--name.\n");
return 1;
@@ -530,17 +525,10 @@
WARN("'%s' already in CBFS.\n", param.name);
if (!data_size) {
- struct buffer buffer;
- if (buffer_from_file(&buffer, param.filename) != 0) {
- ERROR("Cannot load %s.\n", param.filename);
- return 1;
- }
- data_size = buffer.size;
- buffer_delete(&buffer);
+ ERROR("File '%s' is empty?\n", param.name);
+ return 1;
}
- DEBUG("File size is %zd (0x%zx)\n", data_size, data_size);
-
/* Compute required page size */
if (param.force_pow2_pagesize) {
param.pagesize = 1;
@@ -571,8 +559,8 @@
param.alignment, metadata_size);
if (address < 0) {
- ERROR("'%s' can't fit in CBFS for page-size %#x, align %#x.\n",
- param.name, param.pagesize, param.alignment);
+ ERROR("'%s'(%u + %zu) can't fit in CBFS for page-size %#x, align %#x.\n",
+ param.name, metadata_size, data_size, param.pagesize, param.alignment);
return 1;
}
@@ -917,7 +905,7 @@
/* This needs to run after convert() to take compression into account. */
if (!offset && param.alignment)
- if (do_cbfs_locate(&offset, 0))
+ if (do_cbfs_locate(&offset, buffer_size(&buffer)))
goto error;
/* This needs to run after convert() to hash the actual final file data. */
@@ -1076,7 +1064,7 @@
if (param.stage_xip) {
if (!param.baseaddress_assigned) {
param.alignment = 4*1024;
- if (do_cbfs_locate(offset, 0))
+ if (do_cbfs_locate(offset, buffer_size(buffer)))
return -1;
}
assert(!IS_HOST_SPACE_ADDRESS(*offset));
--
To view, visit https://review.coreboot.org/c/coreboot/+/60084
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I1af35e8e388f78aae3593c029afcfb4e510d2b8f
Gerrit-Change-Number: 60084
Gerrit-PatchSet: 2
Gerrit-Owner: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
Attention is currently required from: Nick Vaccaro, Zhuohao Lee.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/60100 )
Change subject: mb/google/brya/variants/brask: disabled autonomous GPIO power management
......................................................................
Patch Set 1: Code-Review+2
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/60100/comment/2cd319e8_4fa44c14
PS1, Line 7: disabled
`Disable`
--
To view, visit https://review.coreboot.org/c/coreboot/+/60100
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I5f18fea5bc28493107c6d4951805de640a0b8ae5
Gerrit-Change-Number: 60100
Gerrit-PatchSet: 1
Gerrit-Owner: Zhuohao Lee <zhuohao(a)chromium.org>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Attention: Zhuohao Lee <zhuohao(a)chromium.org>
Gerrit-Comment-Date: Tue, 14 Dec 2021 21:01:16 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Kevin Chiu, Robert Chen, Wisley Chen.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/60099 )
Change subject: mb/google/brya/var/vell: update overridetree for SSD setting
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
Can this also be squashed into 59304?
--
To view, visit https://review.coreboot.org/c/coreboot/+/60099
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I4d452eaa690a91814739cc1b80966fc3a9f1be37
Gerrit-Change-Number: 60099
Gerrit-PatchSet: 1
Gerrit-Owner: Robert Chen <robert.chen(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Kevin Chiu <kevin.chiu.17802(a)gmail.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Wisley Chen <wisley.chen(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Shon Wang <shon.wang(a)quanta.corp-partner.google.com>
Gerrit-Attention: Kevin Chiu <kevin.chiu.17802(a)gmail.com>
Gerrit-Attention: Robert Chen <robert.chen(a)quanta.corp-partner.google.com>
Gerrit-Attention: Wisley Chen <wisley.chen(a)quanta.corp-partner.google.com>
Gerrit-Comment-Date: Tue, 14 Dec 2021 21:00:30 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment