Attention is currently required from: Arthur Heymans, Martin Roth.
Nicholas Chin has posted comments on this change by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/83973?usp=email )
Change subject: toolchain: Add support for sccache
......................................................................
Patch Set 1:
(3 comments)
File Makefile.mk:
https://review.coreboot.org/c/coreboot/+/83973/comment/d085f9c1_0eea2a6f?us… :
PS1, Line 87: sccace
sccache
```suggestion
printf "\nsccache statistics\n"; \
```
File src/Kconfig:
https://review.coreboot.org/c/coreboot/+/83973/comment/3764205c_c29f4a92?us… :
PS1, Line 119: config SCCACHE
Should this be mutually exclusive with CONFIG_CCACHE? Based on the logic in tookchain.mk I'm assuming sccache would take prececedence as it assigns CC_*, HOSTCC, and HOSTCXX after the ccache logic?
File toolchain.mk:
https://review.coreboot.org/c/coreboot/+/83973/comment/ebe43ef8_697bf095?us… :
PS1, Line 22: ccache
sccache
```suggestion
# sccache integration
```
--
To view, visit https://review.coreboot.org/c/coreboot/+/83973?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: Ia28e696dfe9eab0fc73ba8c7c6bdfc90cbdb790e
Gerrit-Change-Number: 83973
Gerrit-PatchSet: 1
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Comment-Date: Mon, 19 Aug 2024 12:38:38 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Nico Huber has submitted this change. ( https://review.coreboot.org/c/coreboot/+/83895?usp=email )
Change subject: lib/jpeg: avoid calling malloc and free
......................................................................
lib/jpeg: avoid calling malloc and free
Since commit 1d029b40c9de ("lib/jpeg: Replace decoder with Wuffs'
implementation"), a relatively large heap allocation is needed to decode
many JPEGs for use as work area. The prior decoder did not need this,
but also had many limitations in the JPEGs it could decode, was not as
memory-safe and quickly crashed under fuzzing.
This commit keeps using Wuffs' JPEG decoder, but it no longer requires
any heap allocation (and thus configuring the heap size depending on how
big a bootsplash image you want to support).
Change-Id: Ie4c52520cbce498539517c4898ff765365a6beba
Signed-off-by: Nigel Tao <nigeltao(a)golang.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/83895
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Nico Huber <nico.h(a)gmx.de>
Reviewed-by: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Reviewed-by: Jonathon Hall <jonathon.hall(a)puri.sm>
---
M src/lib/jpeg.c
1 file changed, 21 insertions(+), 15 deletions(-)
Approvals:
Felix Singer: Looks good to me, but someone else must approve
build bot (Jenkins): Verified
Jonathon Hall: Looks good to me, but someone else must approve
Nico Huber: Looks good to me, approved
diff --git a/src/lib/jpeg.c b/src/lib/jpeg.c
index 242cf0c..617ab0b 100644
--- a/src/lib/jpeg.c
+++ b/src/lib/jpeg.c
@@ -1,9 +1,7 @@
/* SPDX-License-Identifier: GPL-2.0-only */
/*
- * Provide a simple API around the Wuffs JPEG decoder
- * Uses the heap (and lots of it) for the image-size specific
- * work buffer, so ramstage-only.
+ * Provide a simple API around the Wuffs JPEG decoder.
*/
#include <stdint.h>
@@ -85,6 +83,24 @@
return JPEG_DECODE_FAILED;
}
+ /* Opting in to lower quality means that we can pass an empty slice as the
+ * "work buffer" argument to wuffs_jpeg__decoder__decode_frame below.
+ *
+ * Decoding progressive (not sequential) JPEGs would still require dynamic
+ * memory allocation (and the amount of work buffer required depends on the
+ * image dimensions), but we choose to just reject progressive JPEGs. It is
+ * simpler than sometimes calling malloc (which can fail, especially for
+ * large allocations) and free.
+ *
+ * More commentary about these quirks is at
+ * https://github.com/google/wuffs/blob/beaf45650085a16780b5f708b72daaeb1aa865…
+ */
+ wuffs_jpeg__decoder__set_quirk(
+ &dec, WUFFS_BASE__QUIRK_QUALITY,
+ WUFFS_BASE__QUIRK_QUALITY__VALUE__LOWER_QUALITY);
+ wuffs_jpeg__decoder__set_quirk(
+ &dec, WUFFS_JPEG__QUIRK_REJECT_PROGRESSIVE_JPEGS, 1);
+
wuffs_base__image_config imgcfg;
wuffs_base__io_buffer src = wuffs_base__ptr_u8__reader(filedata, filesize, true);
status = wuffs_jpeg__decoder__decode_image_config(&dec, &imgcfg, &src);
@@ -104,19 +120,9 @@
return JPEG_DECODE_FAILED;
}
- uint64_t workbuf_len_min_incl = wuffs_jpeg__decoder__workbuf_len(&dec).min_incl;
- uint8_t *workbuf_array = malloc(workbuf_len_min_incl);
- if ((workbuf_array == NULL) && workbuf_len_min_incl) {
- return JPEG_DECODE_FAILED;
- }
-
- wuffs_base__slice_u8 workbuf =
- wuffs_base__make_slice_u8(workbuf_array, workbuf_len_min_incl);
status = wuffs_jpeg__decoder__decode_frame(&dec, &pixbuf, &src,
- WUFFS_BASE__PIXEL_BLEND__SRC, workbuf, NULL);
-
- free(workbuf_array);
-
+ WUFFS_BASE__PIXEL_BLEND__SRC,
+ wuffs_base__empty_slice_u8(), NULL);
if (status.repr) {
return JPEG_DECODE_FAILED;
}
--
To view, visit https://review.coreboot.org/c/coreboot/+/83895?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ie4c52520cbce498539517c4898ff765365a6beba
Gerrit-Change-Number: 83895
Gerrit-PatchSet: 3
Gerrit-Owner: Nigel Tao <nigeltao(a)golang.org>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Jonathon Hall <jonathon.hall(a)puri.sm>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-CC: Patrick Georgi <patrick(a)coreboot.org>
Attention is currently required from: Martin L Roth, Nigel Tao, Paul Menzel.
Nico Huber has posted comments on this change by Nigel Tao. ( https://review.coreboot.org/c/coreboot/+/83895?usp=email )
Change subject: lib/jpeg: avoid calling malloc and free
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
> @nico.h@gmx.de thanks for CR+2 but I don't have permissions to submit. […]
Acknowledged
--
To view, visit https://review.coreboot.org/c/coreboot/+/83895?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: Ie4c52520cbce498539517c4898ff765365a6beba
Gerrit-Change-Number: 83895
Gerrit-PatchSet: 2
Gerrit-Owner: Nigel Tao <nigeltao(a)golang.org>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Jonathon Hall <jonathon.hall(a)puri.sm>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-CC: Patrick Georgi <patrick(a)coreboot.org>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nigel Tao <nigeltao(a)golang.org>
Gerrit-Comment-Date: Mon, 19 Aug 2024 12:32:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Nigel Tao <nigeltao(a)golang.org>
Nico Huber has submitted this change. ( https://review.coreboot.org/c/coreboot/+/83894?usp=email )
Change subject: vc/wuffs: upgrade to Wuffs 0.4.0-alpha.8
......................................................................
vc/wuffs: upgrade to Wuffs 0.4.0-alpha.8
We were previously at Wuffs 0.4.0-alpha.2. The C file was copied from
https://github.com/google/wuffs-mirror-release-c and its hash matches
https://github.com/google/wuffs-mirror-release-c/blob/90e4d81a6a8b7b601e8e5…
$ sha256sum src/vendorcode/wuffs/wuffs-v0.4.c
6c22caff4af929112601379a73f72461bc4719a5215366bcc90d599cbc442bb6 src/vendorcode/wuffs/wuffs-v0.4.c
Change-Id: Ie90d989384e0db2b23d7d1b3d9a57920ac8a95a2
Signed-off-by: Nigel Tao <nigeltao(a)golang.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/83894
Reviewed-by: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Reviewed-by: Nico Huber <nico.h(a)gmx.de>
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
---
M src/vendorcode/wuffs/wuffs-v0.4.c
1 file changed, 22,492 insertions(+), 5,930 deletions(-)
Approvals:
Nico Huber: Looks good to me, approved
build bot (Jenkins): Verified
Felix Singer: Looks good to me, but someone else must approve
--
To view, visit https://review.coreboot.org/c/coreboot/+/83894?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ie90d989384e0db2b23d7d1b3d9a57920ac8a95a2
Gerrit-Change-Number: 83894
Gerrit-PatchSet: 2
Gerrit-Owner: Nigel Tao <nigeltao(a)golang.org>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Attention is currently required from: Nigel Tao.
Nico Huber has posted comments on this change by Nigel Tao. ( https://review.coreboot.org/c/coreboot/+/83894?usp=email )
Change subject: vc/wuffs: upgrade to Wuffs 0.4.0-alpha.8
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
> @nico.h@gmx.de thanks for CR+2 but I don't have permissions to submit. […]
No worries, it's just common that we give other people a chance to have a final look.
and for some of us the weekend was already starting ;)
Thanks again for your patches!
--
To view, visit https://review.coreboot.org/c/coreboot/+/83894?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: Ie90d989384e0db2b23d7d1b3d9a57920ac8a95a2
Gerrit-Change-Number: 83894
Gerrit-PatchSet: 1
Gerrit-Owner: Nigel Tao <nigeltao(a)golang.org>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nigel Tao <nigeltao(a)golang.org>
Gerrit-Comment-Date: Mon, 19 Aug 2024 12:31:47 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nigel Tao <nigeltao(a)golang.org>
Attention is currently required from: Dinesh Gehlot, Jayvik Desai, Kapil Porwal, Nick Vaccaro, Rishika Raj.
Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/83975?usp=email )
Change subject: soc/intel/alderlake: Refactor eSOL for late CSE sync text message
......................................................................
soc/intel/alderlake: Refactor eSOL for late CSE sync text message
This patch extends the eSOL implementation on Alder Lake to render text
messages during late CSE sync (from ramstage).
Currently, the eSOL is limited to the early boot phase (until romstage)
and only displays FSP-M memory training warnings or messages during
early CSE sync (at romstage).
Platforms like Nissa/Nirul and Trulo, which use CSE sync from ramstage,
cannot display any eSOL messages, resulting in a brief black screen
during CSE firmware updates.
This patch implements the following logic to scale eSOL for late CSE
sync (at ramstage) without recompiling eSOL code for ramstage:
1. During boot, check if the MRC cache is available. This indicates the
need for memory/DRAM training and triggers an eSOL message.
2. For CSE lite SKUs (applicable to CrOS), leverage the
`is_cse_fw_update_required` API to check if the current CSE RW
firmware version differs from the CBFS metadata file version.
If so, trigger an eSOL message indicating a CSE sync is required.
3. If either condition #1 and/or #2 is true, the AP firmware renders
an eSOL text message using LibGfxInit for the Alder Lake platform.
BUG=b:359814797
TEST=eSOL text messages are displayed during CSE sync and FSP updates.
tirwen-rev3 ~ # elogtool list
0 | ... | Log area cleared | 4088
1 | ... | Early Sign of Life | MRC Early SOL Screen Shown
1 | ... | Early Sign of Life | CSE Sync Early SOL Screen Shown
2 | ... | System boot | 197
3 | ... | Memory Cache Update | Normal | Success
4 | ... | System boot | 198
5 | ... | Firmware Splash Screen | Enabled
Change-Id: I1c7d4475ed5cf6888df1beebab0641ee4203b497
Signed-off-by: Subrata Banik <subratabanik(a)google.com>
---
M src/soc/intel/alderlake/romstage/fsp_params.c
M src/soc/intel/alderlake/romstage/romstage.c
2 files changed, 45 insertions(+), 15 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/83975/1
diff --git a/src/soc/intel/alderlake/romstage/fsp_params.c b/src/soc/intel/alderlake/romstage/fsp_params.c
index a63b64c..95b9479 100644
--- a/src/soc/intel/alderlake/romstage/fsp_params.c
+++ b/src/soc/intel/alderlake/romstage/fsp_params.c
@@ -13,6 +13,7 @@
#include <gpio.h>
#include <intelbasecode/debug_feature.h>
#include <intelblocks/cpulib.h>
+#include <intelblocks/cse.h>
#include <intelblocks/pcie_rp.h>
#include <option.h>
#include <soc/iomap.h>
@@ -411,6 +412,47 @@
debug_get_pch_cpu_tracehub_modes(&mupd->CpuTraceHubMode, &mupd->PchTraceHubMode);
}
+static void fill_fspm_sign_of_life(FSP_M_CONFIG *m_cfg,
+ FSPM_ARCH_UPD *arch_upd)
+{
+ const char *name;
+ bool esol_required = false;
+
+ /*
+ * Memory training
+ *
+ * If valid MRC cache data is not found, FSP should perform a memory
+ * training. Memory training can take a while so let's inform the end
+ * user with an on-screen text message.
+ */
+ if (!arch_upd->NvsBufferPtr) {
+ esol_required = true;
+ name = "memory training";
+ elog_add_event_byte(ELOG_TYPE_FW_EARLY_SOL, ELOG_FW_EARLY_SOL_MRC);
+ }
+
+ /*
+ * CSE Sync
+ *
+ * If currently running CSE RW firmware version is different than CSE version
+ * packed as part of the CBFS then CSE sync will be triggered. CSE sync can take
+ * < 1-minute hence, let's inform the end user with an on-screen text message.
+ */
+ if (CONFIG(SOC_INTEL_CSE_LITE_SKU) && is_cse_fw_update_required()) {
+ if (esol_required) {
+ name = "memory training and CSE update";
+ } else {
+ name = "CSE update";
+ esol_required = true;
+ }
+
+ elog_add_event_byte(ELOG_TYPE_FW_EARLY_SOL, ELOG_FW_EARLY_SOL_CSE_SYNC);
+ }
+
+ if (esol_required)
+ ux_inform_user_of_update_operation(name);
+}
+
void platform_fsp_memory_init_params_cb(FSPM_UPD *mupd, uint32_t version)
{
const struct soc_intel_alderlake_config *config;
@@ -434,15 +476,9 @@
}
}
- /*
- * If valid MRC cache data is not found, FSP should perform a memory
- * training. Memory training can take a while so let's inform the end
- * user with an on-screen text message.
- */
- if (!arch_upd->NvsBufferPtr) {
- if (ux_inform_user_of_update_operation("memory training"))
- elog_add_event_byte(ELOG_TYPE_FW_EARLY_SOL, ELOG_FW_EARLY_SOL_MRC);
- }
+ if (CONFIG(MAINBOARD_HAS_EARLY_LIBGFXINIT))
+ fill_fspm_sign_of_life(m_cfg, arch_upd);
+
config = config_of_soc();
soc_memory_init_params(m_cfg, config);
diff --git a/src/soc/intel/alderlake/romstage/romstage.c b/src/soc/intel/alderlake/romstage/romstage.c
index f0c039e..b577e32 100644
--- a/src/soc/intel/alderlake/romstage/romstage.c
+++ b/src/soc/intel/alderlake/romstage/romstage.c
@@ -150,12 +150,6 @@
printk(BIOS_DEBUG, "%d DIMMs found\n", mem_info->dimm_cnt);
}
-void cse_fw_update_misc_oper(void)
-{
- if (ux_inform_user_of_update_operation("CSE update"))
- elog_add_event_byte(ELOG_TYPE_FW_EARLY_SOL, ELOG_FW_EARLY_SOL_CSE_SYNC);
-}
-
void cse_board_reset(void)
{
early_graphics_stop();
--
To view, visit https://review.coreboot.org/c/coreboot/+/83975?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: I1c7d4475ed5cf6888df1beebab0641ee4203b497
Gerrit-Change-Number: 83975
Gerrit-PatchSet: 1
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Rishika Raj <rishikaraj(a)google.com>
Gerrit-Attention: Jayvik Desai <jayvik(a)google.com>
Gerrit-Attention: Rishika Raj <rishikaraj(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/83974?usp=email )
Change subject: soc/intel/alderlake: Preserve eSOL for late CSE sync
......................................................................
soc/intel/alderlake: Preserve eSOL for late CSE sync
This patch prevents the eSOL screen from being wiped out on Alder Lake
platforms that use late CSE sync (from ramstage). This allows the eSOL
text message to remain visible until ramstage.
Currently, the eSOL only functions during the early boot phase (until
romstage), so platforms like Nissa/Nirul and Trulo, which use CSE sync
from ramstage, cannot display any eSOL messages to the user.
A future patch will ensure the eSOL remains relevant for CSE sync in
ramstage, but this patch is necessary to avoid tearing down the IGD text
mode when exiting romstage.
BUG=b:359814797
TEST=eSOL text mode is not torn down when exiting romstage.
Change-Id: I81548b4057ab95ce3da0dbc69703977baf0581f1
Signed-off-by: Subrata Banik <subratabanik(a)google.com>
---
M src/soc/intel/alderlake/romstage/romstage.c
1 file changed, 3 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/83974/1
diff --git a/src/soc/intel/alderlake/romstage/romstage.c b/src/soc/intel/alderlake/romstage/romstage.c
index 5ca16ae..f0c039e 100644
--- a/src/soc/intel/alderlake/romstage/romstage.c
+++ b/src/soc/intel/alderlake/romstage/romstage.c
@@ -227,7 +227,9 @@
* - Allow PEIM graphics driver to smoothly execute in ramstage if
* RUN_FSP_GOP is selected
*/
- early_graphics_stop();
+ if (!CONFIG(SOC_INTEL_CSE_LITE_SYNC_IN_RAMSTAGE))
+ /* Keep eSOL active if CSE sync in ramstage config is enabled */
+ early_graphics_stop();
if (CONFIG(ENABLE_EARLY_DMA_PROTECTION))
vtd_enable_dma_protection();
--
To view, visit https://review.coreboot.org/c/coreboot/+/83974?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: I81548b4057ab95ce3da0dbc69703977baf0581f1
Gerrit-Change-Number: 83974
Gerrit-PatchSet: 1
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Attention is currently required from: Hung-Te Lin, Jarried Lin, Paul Menzel, Yu-Ping Wu.
Yidi Lin has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/83922?usp=email )
Change subject: soc/mediatek/mt8196: Add GPIO driver
......................................................................
Patch Set 6:
(1 comment)
File src/soc/mediatek/mt8196/gpio.c:
https://review.coreboot.org/c/coreboot/+/83922/comment/cfeaa4cf_0ea805e7?us… :
PS6, Line 391: const struct gpio_drv_info *get_gpio_driving_info(uint32_t raw_id)
: {
: if (raw_id >= ARRAY_SIZE(gpio_driving_info)) {
: printk(BIOS_ERR, "Error: raw_id is out of range\n");
: return NULL;
: }
: return &gpio_driving_info[raw_id];
: }
:
: const struct gpio_drv_info *get_gpio_driving_adv_info(uint32_t raw_id)
: {
: if (raw_id >= ARRAY_SIZE(gpio_driving_adv_info)) {
: printk(BIOS_ERR, "Error: raw_id is out of range\n");
: return NULL;
: }
: return &gpio_driving_adv_info[raw_id];
: }
These two functions mainly serve the callers in `src/soc/mediatek/common/gpio.c`.
Add the log to the call sites. Otherwise, every SoC-specific implementation needs to add the duplicate logs, too.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83922?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: I6d1e6ef17660308c8de908697ffba6b5f17ff9ae
Gerrit-Change-Number: 83922
Gerrit-PatchSet: 6
Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Comment-Date: Mon, 19 Aug 2024 12:16:31 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Hung-Te Lin, Jarried Lin.
Paul Menzel has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/83923?usp=email )
Change subject: soc/mediatek/mt8196: Add NOR-Flash support
......................................................................
Patch Set 7:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/83923/comment/3bb46bb1_f18c04a5?us… :
PS5, Line 11: TEST=read nor flash data successfully.
> no, there is no benchmark data of nor flash.
How do you know, if the driver fully works then? (Anyway, feel free to ignore and mark as resolved.)
File src/soc/mediatek/mt8196/spi.c:
https://review.coreboot.org/c/coreboot/+/83923/comment/94dc484b_119b7728?us… :
PS5, Line 33: void mtk_snfc_init(void)
: {
: const struct pad_func *ptr;
:
: for (size_t i = 0; i < ARRAY_SIZE(nor_pinmux); i++) {
: ptr = &nor_pinmux[i];
:
: gpio_set_pull(ptr->gpio, GPIO_PULL_ENABLE, ptr->select);
: gpio_set_mode(ptr->gpio, ptr->func);
:
: if (gpio_set_driving(ptr->gpio, GPIO_DRV_14_MA) < 0)
: printk(BIOS_ERR,
: "%s: failed to set pin drive to 14 mA for %d\n",
: __func__, ptr->gpio.id);
: else
: printk(BIOS_DEBUG, "%s: got pin drive: %#x\n", __func__,
: gpio_get_driving(ptr->gpio));
: }
: }
> There are also differences in GPIO name and number. […]
1. It would be nice to make them as similar as possible.
2. Pass `nor_pinmux` and drive current to a common function?
--
To view, visit https://review.coreboot.org/c/coreboot/+/83923?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: Id0a19f0520020f16c4cf9d62da4228a5b0371b91
Gerrit-Change-Number: 83923
Gerrit-PatchSet: 7
Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com>
Gerrit-Comment-Date: Mon, 19 Aug 2024 11:45:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Comment-In-Reply-To: Jarried Lin <jarried.lin(a)mediatek.com>