Attention is currently required from: Poornima Tom.
Hello Poornima Tom,
I'd like you to do a code review.
Please visit
https://review.coreboot.org/c/coreboot/+/84060?usp=email
to review the following change.
Change subject: [skolas]: Add reference to wifi_sar_0.hex file
......................................................................
[skolas]: Add reference to wifi_sar_0.hex file
Note: To test
Change-Id: Ic3bb2088f970e829f43a49e0e3497049eccf2b6d
Signed-off-by: Poornima Tom <poornima.tom(a)intel.corp-partner.google.com>
---
M src/mainboard/google/brya/variants/skolas/variant.c
1 file changed, 5 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/84060/1
diff --git a/src/mainboard/google/brya/variants/skolas/variant.c b/src/mainboard/google/brya/variants/skolas/variant.c
index e9ae51e..c09ea97 100644
--- a/src/mainboard/google/brya/variants/skolas/variant.c
+++ b/src/mainboard/google/brya/variants/skolas/variant.c
@@ -4,6 +4,11 @@
#include <fw_config.h>
#include <baseboard/variants.h>
+const char *get_wifi_sar_cbfs_filename(void)
+{
+ return "wifi_sar_0.hex";
+}
+
void variant_update_soc_chip_config(struct soc_intel_alderlake_config *config)
{
config->cnvi_bt_audio_offload = fw_config_probe(FW_CONFIG(AUDIO,
--
To view, visit https://review.coreboot.org/c/coreboot/+/84060?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ic3bb2088f970e829f43a49e0e3497049eccf2b6d
Gerrit-Change-Number: 84060
Gerrit-PatchSet: 1
Gerrit-Owner: Poornima Tom <poornima.tom(a)intel.com>
Gerrit-Reviewer: Poornima Tom <poornima.tom(a)intel.corp-partner.google.com>
Gerrit-Attention: Poornima Tom <poornima.tom(a)intel.corp-partner.google.com>
Attention is currently required from: Angel Pons, Arthur Heymans.
Nico Huber has posted comments on this change by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/84059?usp=email )
Change subject: drivers/spi: Stop using a variable-length array
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
Patchset:
PS1:
Let's wait for the release and try to test it better this time...
--
To view, visit https://review.coreboot.org/c/coreboot/+/84059?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I157ecec69c049ead06467b0328efd7d1a09bd268
Gerrit-Change-Number: 84059
Gerrit-PatchSet: 1
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Fri, 23 Aug 2024 15:57:26 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Attention is currently required from: Arthur Heymans.
Nico Huber has posted comments on this change by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/84042?usp=email )
Change subject: soc/intel/pmclib.c: Work around compiler bug -Werror=stringop-overread
......................................................................
Patch Set 8:
(1 comment)
File src/soc/intel/common/block/pmc/pmclib.c:
https://review.coreboot.org/c/coreboot/+/84042/comment/9684f45b_8474d5e7?us… :
PS7, Line 104: for (i = num_bits - 1; i >= 0; i--) {
> > I don't get it, it should never enter the loop body with `num_bits == 0`, right? […]
Maybe also check the compiled code? If it warns, maybe not the warning is wrong,
but the optimized code.
--
To view, visit https://review.coreboot.org/c/coreboot/+/84042?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ieee6e9bddc4e738eb560dd0e69dc3087ac9f5da6
Gerrit-Change-Number: 84042
Gerrit-PatchSet: 8
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Fri, 23 Aug 2024 15:54:31 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Attention is currently required from: Angel Pons.
Hello Angel Pons,
I'd like you to do a code review.
Please visit
https://review.coreboot.org/c/coreboot/+/84059?usp=email
to review the following change.
Change subject: drivers/spi: Stop using a variable-length array
......................................................................
drivers/spi: Stop using a variable-length array
Only the call in `spi_flash_cmd_write_page_program` uses non-constant
values for the array length. However, the value for `data_len` has an
upper bound: `flash->page_size` is set to `1U << vi->page_size_shift`
which depends on the flash chip vendor info, and the largest value it
can currently have is 8. Thus, the maximum page size is currently 256.
Define the `MAX_FLASH_CMD_DATA_SIZE` macro to place an upper bound on
the amount of data that can be written in one command. Then, use this
value to allocate a fixed-size buffer in `spi_flash_cmd_write`. Also,
add a check to prevent buffer overflow problems. Finally, ensure that
the `spi_flash_cmd_write_page_program` function always writes no more
than 256 bytes of data when using the `spi_flash_cmd_write` function.
The buffer is placed in .bss so that it does not increase stack usage in
some use cases.
Tested on Asrock B85M Pro4 (Winbond W25Q64FV), MRC cache still works.
Repost of https://review.coreboot.org/c/coreboot/+/50480 with 'static'
so that the buffer is in bss.
Change-Id: I157ecec69c049ead06467b0328efd7d1a09bd268
Signed-off-by: Angel Pons <th3fanbus(a)gmail.com>
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
---
M src/drivers/spi/spi_flash.c
M src/drivers/spi/spi_flash_internal.h
2 files changed, 9 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/84059/1
diff --git a/src/drivers/spi/spi_flash.c b/src/drivers/spi/spi_flash.c
index 11597c6..2ad1fe9 100644
--- a/src/drivers/spi/spi_flash.c
+++ b/src/drivers/spi/spi_flash.c
@@ -129,17 +129,16 @@
return ret;
}
-/* TODO: This code is quite possibly broken and overflowing stacks. Fix ASAP! */
-#pragma GCC diagnostic push
-#if defined(__GNUC__) && !defined(__clang__)
-#pragma GCC diagnostic ignored "-Wstack-usage="
-#endif
-#pragma GCC diagnostic ignored "-Wvla"
int spi_flash_cmd_write(const struct spi_slave *spi, const u8 *cmd,
size_t cmd_len, const void *data, size_t data_len)
{
int ret;
- u8 buff[cmd_len + data_len];
+ /* Put the buffer in .bss to not put strain the stack (it's limited in SMM) */
+ static u8 buff[4 + MAX_FLASH_CMD_DATA_SIZE];
+
+ if (ARRAY_SIZE(buff) < cmd_len + data_len)
+ return -1;
+
memcpy(buff, cmd, cmd_len);
memcpy(buff + cmd_len, data, data_len);
@@ -151,7 +150,6 @@
return ret;
}
-#pragma GCC diagnostic pop
/* Perform the read operation honoring spi controller fifo size, reissuing
* the read command until the full request completed. */
@@ -316,6 +314,7 @@
byte_addr = offset % page_size;
chunk_len = MIN(len - actual, page_size - byte_addr);
chunk_len = spi_crop_chunk(&flash->spi, sizeof(cmd), chunk_len);
+ chunk_len = MIN(MAX_FLASH_CMD_DATA_SIZE, chunk_len);
spi_flash_addr(offset, cmd);
if (CONFIG(DEBUG_SPI_FLASH)) {
diff --git a/src/drivers/spi/spi_flash_internal.h b/src/drivers/spi/spi_flash_internal.h
index 3aea914..9ce2f6a 100644
--- a/src/drivers/spi/spi_flash_internal.h
+++ b/src/drivers/spi/spi_flash_internal.h
@@ -27,6 +27,8 @@
/* Common status */
#define STATUS_WIP 0x01
+#define MAX_FLASH_CMD_DATA_SIZE 256
+
/* Send a single-byte command to the device and read the response */
int spi_flash_cmd(const struct spi_slave *spi, u8 cmd, void *response, size_t len);
--
To view, visit https://review.coreboot.org/c/coreboot/+/84059?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I157ecec69c049ead06467b0328efd7d1a09bd268
Gerrit-Change-Number: 84059
Gerrit-PatchSet: 1
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Attention is currently required from: Angel Pons, Jason Glenesk, Martin L Roth, Matt DeVillier, Nicholas Chin.
Jason Glenesk has posted comments on this change by Martin L Roth. ( https://review.coreboot.org/c/coreboot/+/84046?usp=email )
Change subject: Docs/releases: Update 24.08 release notes
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/84046?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I449e8490d72976c8f723dc3b5ab3b77d7b16e3a0
Gerrit-Change-Number: 84046
Gerrit-PatchSet: 2
Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Comment-Date: Fri, 23 Aug 2024 15:21:40 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Angel Pons, Jason Glenesk, Martin L Roth, Matt DeVillier, Nicholas Chin.
Felix Singer has posted comments on this change by Martin L Roth. ( https://review.coreboot.org/c/coreboot/+/84046?usp=email )
Change subject: Docs/releases: Update 24.08 release notes
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/84046?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I449e8490d72976c8f723dc3b5ab3b77d7b16e3a0
Gerrit-Change-Number: 84046
Gerrit-PatchSet: 2
Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Comment-Date: Fri, 23 Aug 2024 15:05:57 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes