Tristan Hsieh has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34872 )
Change subject: google/kukui: Load and run dram init blob ......................................................................
google/kukui: Load and run dram init blob
Load dram.bin and run it for full dram calibration. If any error occurred, try default dram init flow.
BUG=b:134351649 BRANCH=none TEST=emerge-kukui coreboot
Change-Id: Iad05a77e6353f37cd5ea897203d528762f44176e Signed-off-by: Tristan Shieh tristan.shieh@mediatek.com --- M src/mainboard/google/kukui/Makefile.inc M src/mainboard/google/kukui/romstage.c M src/soc/mediatek/mt8183/include/soc/memlayout.ld 3 files changed, 45 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/34872/1
diff --git a/src/mainboard/google/kukui/Makefile.inc b/src/mainboard/google/kukui/Makefile.inc index a0556c1..d7621f1 100644 --- a/src/mainboard/google/kukui/Makefile.inc +++ b/src/mainboard/google/kukui/Makefile.inc @@ -25,3 +25,8 @@ ramstage-y += mainboard.c ramstage-y += memlayout.ld ramstage-y += reset.c + +cbfs-files-y += dram.bin +dram.bin-file := 3rdparty/blobs/soc/mediatek/mt8183/dram.bin +dram.bin-type := raw +dram.bin-compression := $(CBFS_COMPRESS_FLAG) diff --git a/src/mainboard/google/kukui/romstage.c b/src/mainboard/google/kukui/romstage.c index 1465243..8521cf8 100644 --- a/src/mainboard/google/kukui/romstage.c +++ b/src/mainboard/google/kukui/romstage.c @@ -19,9 +19,45 @@ #include <soc/mt6358.h> #include <soc/pll.h> #include <soc/rtc.h> +#include <string.h>
#include "early_init.h"
+#include <cbfs.h> +#include <console/console.h> + +DECLARE_REGION(dram_init_code) + +static int dram_blob_load_and_run(void) +{ + int (*dram_init)(void *); + size_t blob_size, region_size; + + region_size = _edram_init_code - _dram_init_code; + blob_size = cbfs_boot_load_file("dram.bin", _dram_init_code, + region_size, CBFS_TYPE_RAW); + if (blob_size == 0) { + printk(BIOS_ERR, "dram.bin not found\n"); + return -1; + } + + memset(_dram_init_code + blob_size, 0, region_size - blob_size); + + dram_init = (int (*)(void *))_dram_init_code; + + return dram_init(NULL); +} + +static void dram_config(void) +{ + int err = dram_blob_load_and_run(); + if (err == 0) + return; + + printk(BIOS_ERR, "dram_blob error:%d\n", err); + mt_mem_init(get_sdram_config()); +} + void platform_romstage_main(void) { /* This will be done in verstage if CONFIG_VBOOT is enabled. */ @@ -33,6 +69,6 @@ pmic_set_vsim2_cali(2700); mt_pll_raise_ca53_freq(1989 * MHz); rtc_boot(); - mt_mem_init(get_sdram_config()); + dram_config(); mtk_mmu_after_dram(); } diff --git a/src/soc/mediatek/mt8183/include/soc/memlayout.ld b/src/soc/mediatek/mt8183/include/soc/memlayout.ld index 7da282e..9a71645 100644 --- a/src/soc/mediatek/mt8183/include/soc/memlayout.ld +++ b/src/soc/mediatek/mt8183/include/soc/memlayout.ld @@ -24,6 +24,8 @@ */ #define SRAM_L2C_START(addr) SYMBOL(sram_l2c, addr) #define SRAM_L2C_END(addr) SYMBOL(esram_l2c, addr) +#define DRAM_INIT_CODE(addr, size) \ + REGION(dram_init_code, addr, size, 4)
SECTIONS { @@ -43,6 +45,7 @@ DECOMPRESSOR(0x00201000, 28K) BOOTBLOCK(0x00208000, 64K) OVERLAP_VERSTAGE_ROMSTAGE(0x00218000, 160K) + DRAM_INIT_CODE(0x00240000, 256K) SRAM_L2C_END(0x00280000)
DRAM_START(0x40000000)
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34872 )
Change subject: google/kukui: Load and run dram init blob ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34872/1/src/mainboard/google/kukui/... File src/mainboard/google/kukui/romstage.c:
https://review.coreboot.org/c/coreboot/+/34872/1/src/mainboard/google/kukui/... PS1, Line 33: int (*dram_init)(void *); function definition argument 'void *' should also have an identifier name
Tristan Hsieh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34872 )
Change subject: google/kukui: Load and run dram init blob ......................................................................
Patch Set 1:
This change is ready for review.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34872 )
Change subject: google/kukui: Load and run dram init blob ......................................................................
Patch Set 2:
(1 comment)
This change is ready for review.
https://review.coreboot.org/c/coreboot/+/34872/1/src/mainboard/google/kukui/... File src/mainboard/google/kukui/romstage.c:
https://review.coreboot.org/c/coreboot/+/34872/1/src/mainboard/google/kukui/... PS1, Line 44: memset(_dram_init_code + blob_size, 0, region_size - blob_size);
To initial .bss to zero. […]
Guys, why are we inventing our own loading conventions? Please provide the DRAM blob as an ELF, not a raw .bin, and then treat it as a stage in CBFS. That way, .bss initialization will be handled automatically.
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34872 )
Change subject: google/kukui: Load and run dram init blob ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34872/1/src/mainboard/google/kukui/... File src/mainboard/google/kukui/romstage.c:
https://review.coreboot.org/c/coreboot/+/34872/1/src/mainboard/google/kukui/... PS1, Line 44: memset(_dram_init_code + blob_size, 0, region_size - blob_size);
Guys, why are we inventing our own loading conventions? Please provide the DRAM blob as an ELF, not […]
I thought there was no pure ELF (non-stage, non-payload) support in Coreboot? But maybe I missed some APIs.
For stage - can we call a stage that was not built from Coreboot source (and layout), and return back to where it was called?
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34872 )
Change subject: google/kukui: Load and run dram init blob ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34872/1/src/mainboard/google/kukui/... File src/mainboard/google/kukui/romstage.c:
https://review.coreboot.org/c/coreboot/+/34872/1/src/mainboard/google/kukui/... PS1, Line 44: memset(_dram_init_code + blob_size, 0, region_size - blob_size);
I thought there was no pure ELF (non-stage, non-payload) support in Coreboot? But maybe I missed som […]
Both stages and payloads get built as ELFs. cbfstool converts them to the respective stage or SELF format. It can do that just as well with any other ELF that fits the requirements (e.g. for stages all program data from .text through .bss needs to be contiguous in memory, which I assume we can do for this blob?). See src/soc/qualcomm/common/qclib.c for an example where we treat a Qualcomm blob as a stage.
Returning from a stage is the same as returning from a blob, makes no difference ("stage" just means that the binary is encoded in CBFS in a special way so that we can use our existing CBFS and prog_loader functions on it and they can handle things like zeroing out BSS automatically).
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34872 )
Change subject: google/kukui: Load and run dram init blob ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34872/3/src/mainboard/google/kukui/... File src/mainboard/google/kukui/romstage.c:
https://review.coreboot.org/c/coreboot/+/34872/3/src/mainboard/google/kukui/... PS3, Line 55: printk(BIOS_ERR, "dram_blob error:%d\n", err); Please add a space after the colon, and rephrase the message, so that it understandable for the user.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34872 )
Change subject: google/kukui: Load and run dram init blob ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34872/3/src/mainboard/google/kukui/... File src/mainboard/google/kukui/romstage.c:
https://review.coreboot.org/c/coreboot/+/34872/3/src/mainboard/google/kukui/... PS3, Line 37: dram.bin Which error codes can be returned by this binary, and what do they mean? It would be good to document that somewhere.
Hello Yu-Ping Wu, Julius Werner, CK HU, You-Cheng Syu, Hung-Te Lin, Huayang Duan, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34872
to look at the new patch set (#4).
Change subject: google/kukui: Load and run dram init blob ......................................................................
google/kukui: Load and run dram init blob
Load dram.bin and run it for full dram calibration. If any error occurred, try default dram init flow.
BUG=b:134351649 BRANCH=none TEST=emerge-kukui coreboot
Change-Id: Iad05a77e6353f37cd5ea897203d528762f44176e Signed-off-by: Tristan Shieh tristan.shieh@mediatek.com --- M src/mainboard/google/kukui/Makefile.inc M src/mainboard/google/kukui/romstage.c M src/soc/mediatek/mt8183/include/soc/memlayout.ld 3 files changed, 43 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/34872/4
Tristan Hsieh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34872 )
Change subject: google/kukui: Load and run dram init blob ......................................................................
Patch Set 4:
(3 comments)
https://review.coreboot.org/c/coreboot/+/34872/1/src/mainboard/google/kukui/... File src/mainboard/google/kukui/romstage.c:
https://review.coreboot.org/c/coreboot/+/34872/1/src/mainboard/google/kukui/... PS1, Line 44: memset(_dram_init_code + blob_size, 0, region_size - blob_size);
Both stages and payloads get built as ELFs. […]
Done
https://review.coreboot.org/c/coreboot/+/34872/3/src/mainboard/google/kukui/... File src/mainboard/google/kukui/romstage.c:
https://review.coreboot.org/c/coreboot/+/34872/3/src/mainboard/google/kukui/... PS3, Line 37: dram.bin
Which error codes can be returned by this binary, and what do they mean? It would be good to documen […]
We treat the blob as a prog (stage type / elf format), and prog_run() return void now. But, you are right. We should document the result of this program somewhere. In the nearly future, I guess we will get the return value from dram_params and we should document the detail in front of that struct declaration.
https://review.coreboot.org/c/coreboot/+/34872/3/src/mainboard/google/kukui/... PS3, Line 55: printk(BIOS_ERR, "dram_blob error:%d\n", err);
Please add a space after the colon, and rephrase the message, so that it understandable for the user […]
Done
Hello Yu-Ping Wu, Julius Werner, CK HU, You-Cheng Syu, Hung-Te Lin, Huayang Duan, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34872
to look at the new patch set (#5).
Change subject: google/kukui: Load and run dram init blob ......................................................................
google/kukui: Load and run dram init blob
Load dram.bin and run it for full dram calibration. If any error occurred, try default dram init flow.
BUG=b:134351649 BRANCH=none TEST=emerge-kukui coreboot
Change-Id: Iad05a77e6353f37cd5ea897203d528762f44176e Signed-off-by: Tristan Shieh tristan.shieh@mediatek.com --- M src/mainboard/google/kukui/Makefile.inc M src/mainboard/google/kukui/romstage.c M src/soc/mediatek/mt8183/include/soc/memlayout.ld 3 files changed, 43 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/34872/5
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34872 )
Change subject: google/kukui: Load and run dram init blob ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34872/3/src/mainboard/google/kukui/... File src/mainboard/google/kukui/romstage.c:
https://review.coreboot.org/c/coreboot/+/34872/3/src/mainboard/google/kukui/... PS3, Line 37: dram.bin
We treat the blob as a prog (stage type / elf format), and prog_run() return void now. […]
Ack, thank you for taking this into account
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34872 )
Change subject: google/kukui: Load and run dram init blob ......................................................................
Patch Set 5:
you can also change to few more params,
blob(int *result, void *buffer, size_t length);
so it's very clear the execution result is in result, and the blob can access the buffer[length].
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34872 )
Change subject: google/kukui: Load and run dram init blob ......................................................................
Patch Set 5:
... on the other hand, it's probably also not a bad idea to put everything into the buffer, say
struct dram_calibration_param { u32 result; u32 length; u8 data[]; }
This may be easier to prevent ABI issues
Tristan Hsieh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34872 )
Change subject: google/kukui: Load and run dram init blob ......................................................................
Patch Set 5:
Patch Set 5:
... on the other hand, it's probably also not a bad idea to put everything into the buffer, say
struct dram_calibration_param { u32 result; u32 length; u8 data[]; }
This may be easier to prevent ABI issues
I think we should do it this way (with struct). Since we use PROG_RUN(), it passes only one argument to entry.
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34872 )
Change subject: google/kukui: Load and run dram init blob ......................................................................
Patch Set 20:
(3 comments)
https://review.coreboot.org/c/coreboot/+/34872/14/src/mainboard/google/kukui... File src/mainboard/google/kukui/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/34872/14/src/mainboard/google/kukui... PS14, Line 38: $(CBFS_COMPRESS_FLAG) I thought this must be raw?
https://review.coreboot.org/c/coreboot/+/34872/14/src/mainboard/google/kukui... PS14, Line 40: cbfs-files-y += $(DRAM_CBFS) add one level of indent since it's inside ifndeq
https://review.coreboot.org/c/coreboot/+/34872/14/src/mainboard/google/kukui... File src/mainboard/google/kukui/romstage.c:
https://review.coreboot.org/c/coreboot/+/34872/14/src/mainboard/google/kukui... PS14, Line 40: remove the blank line
huayang duan has uploaded a new patch set (#22) to the change originally created by Tristan Hsieh. ( https://review.coreboot.org/c/coreboot/+/34872 )
Change subject: google/kukui: Load and run dram init blob ......................................................................
google/kukui: Load and run dram init blob
Load dram.bin and run it for full dram calibration. If any error occurred, try default dram init flow.
BUG=b:134351649 BRANCH=none TEST=emerge-kukui coreboot
Change-Id: Iad05a77e6353f37cd5ea897203d528762f44176e Signed-off-by: Tristan Shieh tristan.shieh@mediatek.com --- M src/mainboard/google/kukui/Makefile.inc M src/mainboard/google/kukui/romstage.c M src/soc/mediatek/mt8183/include/soc/memlayout.ld 3 files changed, 43 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/34872/22
Yu-Ping Wu has uploaded a new patch set (#23) to the change originally created by Tristan Hsieh. ( https://review.coreboot.org/c/coreboot/+/34872 )
Change subject: google/kukui: Load and run DRAM init blob ......................................................................
google/kukui: Load and run DRAM init blob
Load dram.bin and run it for full DRAM calibration. If any error occurred, try default DRAM init flow.
BUG=b:134351649 BRANCH=none TEST=emerge-kukui coreboot
Change-Id: Iad05a77e6353f37cd5ea897203d528762f44176e Signed-off-by: Tristan Shieh tristan.shieh@mediatek.com --- M src/mainboard/google/kukui/Makefile.inc M src/mainboard/google/kukui/romstage.c M src/soc/mediatek/mt8183/include/soc/memlayout.ld 3 files changed, 42 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/34872/23
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34872 )
Change subject: google/kukui: Load and run DRAM init blob ......................................................................
Patch Set 23:
(3 comments)
https://review.coreboot.org/c/coreboot/+/34872/14/src/mainboard/google/kukui... File src/mainboard/google/kukui/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/34872/14/src/mainboard/google/kukui... PS14, Line 38: $(CBFS_COMPRESS_FLAG)
I thought this must be raw?
Changed to "none".
https://review.coreboot.org/c/coreboot/+/34872/14/src/mainboard/google/kukui... PS14, Line 40: cbfs-files-y += $(DRAM_CBFS)
add one level of indent since it's inside ifndeq
Done
https://review.coreboot.org/c/coreboot/+/34872/14/src/mainboard/google/kukui... File src/mainboard/google/kukui/romstage.c:
https://review.coreboot.org/c/coreboot/+/34872/14/src/mainboard/google/kukui... PS14, Line 40:
remove the blank line
Done
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34872 )
Change subject: google/kukui: Load and run DRAM init blob ......................................................................
Patch Set 23:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34872/1/src/mainboard/google/kukui/... File src/mainboard/google/kukui/romstage.c:
https://review.coreboot.org/c/coreboot/+/34872/1/src/mainboard/google/kukui/... PS1, Line 53: int err = dram_blob_load_and_run();
No, we don't. (You are right.) […]
@hungte Are you okay with that?
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34872 )
Change subject: google/kukui: Load and run DRAM init blob ......................................................................
Patch Set 23:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34872/23/src/mainboard/google/kukui... File src/mainboard/google/kukui/romstage.c:
https://review.coreboot.org/c/coreboot/+/34872/23/src/mainboard/google/kukui... PS23, Line 28: dram_blob_load_and_run static int dram_full_calibration(void) { /* Load and run the provided blob for full-calibration if available. */
https://review.coreboot.org/c/coreboot/+/34872/23/src/mainboard/google/kukui... PS23, Line 30: uint32_t any reason to use uint32_t instead of uint8_t [4096]?
I think uint8_t helps reading the size easier, unless if you have some good reasons.
huayang duan has uploaded a new patch set (#24) to the change originally created by Tristan Hsieh. ( https://review.coreboot.org/c/coreboot/+/34872 )
Change subject: google/kukui: Load and run DRAM init blob ......................................................................
google/kukui: Load and run DRAM init blob
Load dram.bin and run it for full DRAM calibration. If any error occurred, try default DRAM init flow.
BUG=b:134351649 BRANCH=none TEST=emerge-kukui coreboot
Change-Id: Iad05a77e6353f37cd5ea897203d528762f44176e Signed-off-by: Tristan Shieh tristan.shieh@mediatek.com --- M src/mainboard/google/kukui/Makefile.inc M src/mainboard/google/kukui/romstage.c M src/soc/mediatek/mt8183/include/soc/memlayout.ld 3 files changed, 46 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/34872/24
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34872 )
Change subject: google/kukui: Load and run DRAM init blob ......................................................................
Patch Set 24:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34872/24/src/mainboard/google/kukui... File src/mainboard/google/kukui/romstage.c:
https://review.coreboot.org/c/coreboot/+/34872/24/src/mainboard/google/kukui... PS24, Line 55: printk(BIOS_ERR, "failed to do full calibration(%d), fall back to load default sdram param\n", err); line over 96 characters
Huayang Duan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34872 )
Change subject: google/kukui: Load and run DRAM init blob ......................................................................
Patch Set 24:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34872/23/src/mainboard/google/kukui... File src/mainboard/google/kukui/romstage.c:
https://review.coreboot.org/c/coreboot/+/34872/23/src/mainboard/google/kukui... PS23, Line 28: dram_blob_load_and_run
static int dram_full_calibration(void) […]
Done
https://review.coreboot.org/c/coreboot/+/34872/23/src/mainboard/google/kukui... PS23, Line 30: uint32_t
any reason to use uint32_t instead of uint8_t [4096]? […]
Done
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34872 )
Change subject: google/kukui: Load and run DRAM init blob ......................................................................
Patch Set 24:
(5 comments)
https://review.coreboot.org/c/coreboot/+/34872/24/src/mainboard/google/kukui... File src/mainboard/google/kukui/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/34872/24/src/mainboard/google/kukui... PS24, Line 36: $(DRAM_CBFS)-file := 3rdparty/blobs/soc/mediatek/mt8183/dram.elf Should the path be configurable in Kconfig?
https://review.coreboot.org/c/coreboot/+/34872/24/src/mainboard/google/kukui... File src/mainboard/google/kukui/romstage.c:
https://review.coreboot.org/c/coreboot/+/34872/24/src/mainboard/google/kukui... PS24, Line 30: static int dram_run_full_calibration(struct sdram_params *freq_params) Please document the return values.
https://review.coreboot.org/c/coreboot/+/34872/24/src/mainboard/google/kukui... PS24, Line 36: return -1; Please add debug messages.
https://review.coreboot.org/c/coreboot/+/34872/24/src/mainboard/google/kukui... PS24, Line 39: return -2; Please add debug messages.
https://review.coreboot.org/c/coreboot/+/34872/24/src/mainboard/google/kukui... PS24, Line 55: printk(BIOS_ERR, "failed to do full calibration(%d), fall back to load default sdram param\n", err); 1. Please add a space before the opening bracket (. 2. *F*ailed 3. As this is more user visible, please add some more information.
The system will still work, but might not work with the best performance.
(No idea, if that is good.)
Huayang Duan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34872 )
Change subject: google/kukui: Load and run DRAM init blob ......................................................................
Patch Set 24:
(5 comments)
https://review.coreboot.org/c/coreboot/+/34872/14/src/mainboard/google/kukui... File src/mainboard/google/kukui/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/34872/14/src/mainboard/google/kukui... PS14, Line 38: $(CBFS_COMPRESS_FLAG)
Changed to "none".
Done
https://review.coreboot.org/c/coreboot/+/34872/1/src/mainboard/google/kukui/... File src/mainboard/google/kukui/romstage.c:
https://review.coreboot.org/c/coreboot/+/34872/1/src/mainboard/google/kukui/... PS1, Line 53: int err = dram_blob_load_and_run();
@hungte Are you okay with that?
only when not found full-calibration param will execute this function in new patch. https://review.coreboot.org/c/coreboot/+/35110/21/src/mainboard/google/kukui...
https://review.coreboot.org/c/coreboot/+/34872/24/src/mainboard/google/kukui... File src/mainboard/google/kukui/romstage.c:
https://review.coreboot.org/c/coreboot/+/34872/24/src/mainboard/google/kukui... PS24, Line 36: return -1;
Please add debug messages.
we already print the error value at below position, so I think no need add debug msg here. https://review.coreboot.org/c/coreboot/+/34872/24/src/mainboard/google/kukui...
https://review.coreboot.org/c/coreboot/+/34872/24/src/mainboard/google/kukui... PS24, Line 39: return -2;
Please add debug messages.
we already print the error value at below position, so I think no need add debug msg here. https://review.coreboot.org/c/coreboot/+/34872/24/src/mainboard/google/kukui...
https://review.coreboot.org/c/coreboot/+/34872/24/src/mainboard/google/kukui... PS24, Line 55: printk(BIOS_ERR, "failed to do full calibration(%d), fall back to load default sdram param\n", err);
- Please add a space before the opening bracket (. […]
we add some comment at below position, is this ok? https://review.coreboot.org/c/coreboot/+/35110/21/src/mainboard/google/kukui...
huayang duan has uploaded a new patch set (#25) to the change originally created by Tristan Hsieh. ( https://review.coreboot.org/c/coreboot/+/34872 )
Change subject: google/kukui: Load and run DRAM init blob ......................................................................
google/kukui: Load and run DRAM init blob
Load dram.bin and run it for full DRAM calibration. If any error occurred, try default DRAM init flow.
BUG=b:134351649 BRANCH=none TEST=emerge-kukui coreboot
Change-Id: Iad05a77e6353f37cd5ea897203d528762f44176e Signed-off-by: Tristan Shieh tristan.shieh@mediatek.com --- M src/mainboard/google/kukui/Makefile.inc M src/mainboard/google/kukui/romstage.c M src/soc/mediatek/mt8183/include/soc/memlayout.ld 3 files changed, 47 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/34872/25
Huayang Duan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34872 )
Change subject: google/kukui: Load and run DRAM init blob ......................................................................
Patch Set 25: Code-Review-1
changes in this patch have porting to https://review.coreboot.org/c/coreboot/+/35110/25
No need this patch, please drop this patch!
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34872 )
Change subject: google/kukui: Load and run DRAM init blob ......................................................................
Patch Set 25:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34872/1/src/mainboard/google/kukui/... File src/mainboard/google/kukui/romstage.c:
https://review.coreboot.org/c/coreboot/+/34872/1/src/mainboard/google/kukui/... PS1, Line 53: int err = dram_blob_load_and_run();
only when not found full-calibration param will execute this function in new patch. […]
Ack
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34872 )
Change subject: google/kukui: Load and run DRAM init blob ......................................................................
Patch Set 25:
This has been deprecated by https://review.coreboot.org/c/coreboot/+/35110. Please abandon this change.
huayang duan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34872 )
Change subject: google/kukui: Load and run DRAM init blob ......................................................................
Patch Set 25:
Patch Set 25:
This has been deprecated by https://review.coreboot.org/c/coreboot/+/35110. Please abandon this change.
this patch's owner is Tristan Hsieh ,so I not have permission to abandon this patch.
Tristan Hsieh has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/34872 )
Change subject: google/kukui: Load and run DRAM init blob ......................................................................
Abandoned
-
Angel Pons (Code Review) -
build bot (Jenkins) (Code Review)
-
Huayang Duan (Code Review)
-
huayang duan (Code Review)
-
Hung-Te Lin (Code Review)
-
Julius Werner (Code Review)
-
Paul Menzel (Code Review)
-
Tristan Hsieh (Code Review)
-
Yu-Ping Wu (Code Review)