Change in coreboot[master]: soc/mediatek/mt8183: Transfer ddr geometry type to dram blob
Duan huayang has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43796 )
Change subject: soc/mediatek/mt8183: Transfer ddr geometry type to dram blob ......................................................................
soc/mediatek/mt8183: Transfer ddr geometry type to dram blob
BUG=none BRANCH=kukui TEST=Boots correctly on Kukui
Change-Id: I3a677195f5036321939c60c8f9f1bace7c4a2e3f Signed-off-by: Huayang Duan huayang.duan@mediatek.com --- M src/soc/mediatek/mt8183/memory.c 1 file changed, 3 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/43796/1
diff --git a/src/soc/mediatek/mt8183/memory.c b/src/soc/mediatek/mt8183/memory.c index ff30f67..395a885 100644 --- a/src/soc/mediatek/mt8183/memory.c +++ b/src/soc/mediatek/mt8183/memory.c @@ -99,6 +99,7 @@
static int dram_run_full_calibration(struct dramc_param *dparam, u16 config) { + const struct sdram_params * sdram_cfg = get_sdram_config(); initialize_dramc_param(dparam, config);
/* Load and run the provided blob for full-calibration if available */ @@ -111,6 +112,8 @@ return -2;
dparam->do_putc = do_putchar; + dparam->freq_params[0].ddr_geometry = sdram_cfg->ddr_geometry; + printk(BIOS_INFO, "ddr_geometry:%d\n", sdram_cfg->ddr_geometry); prog_set_entry(&dram, prog_entry(&dram), dparam); prog_run(&dram);
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43796 )
Change subject: soc/mediatek/mt8183: Transfer ddr geometry type to dram blob ......................................................................
Patch Set 1:
(4 comments)
https://review.coreboot.org/c/coreboot/+/43796/1/src/soc/mediatek/mt8183/mem... File src/soc/mediatek/mt8183/memory.c:
https://review.coreboot.org/c/coreboot/+/43796/1/src/soc/mediatek/mt8183/mem... PS1, Line 102: const struct sdram_params * sdram_cfg = get_sdram_config(); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43796/1/src/soc/mediatek/mt8183/mem... PS1, Line 102: const struct sdram_params * sdram_cfg = get_sdram_config(); "foo * bar" should be "foo *bar"
https://review.coreboot.org/c/coreboot/+/43796/1/src/soc/mediatek/mt8183/mem... PS1, Line 115: dparam->freq_params[0].ddr_geometry = sdram_cfg->ddr_geometry; please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43796/1/src/soc/mediatek/mt8183/mem... PS1, Line 116: printk(BIOS_INFO, "ddr_geometry:%d\n", sdram_cfg->ddr_geometry); please, no spaces at the start of a line
Hello build bot (Jenkins), Julius Werner,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43796
to look at the new patch set (#3).
Change subject: soc/mediatek/mt8183: Transfer ddr geometry type to dram blob ......................................................................
soc/mediatek/mt8183: Transfer ddr geometry type to dram blob
BUG=none BRANCH=kukui TEST=Boots correctly on Kukui
Change-Id: I3a677195f5036321939c60c8f9f1bace7c4a2e3f Signed-off-by: Huayang Duan huayang.duan@mediatek.com --- M src/soc/mediatek/mt8183/memory.c 1 file changed, 3 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/43796/3
Hello build bot (Jenkins), Julius Werner,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43796
to look at the new patch set (#4).
Change subject: soc/mediatek/mt8183: Transfer ddr geometry type to dram blob ......................................................................
soc/mediatek/mt8183: Transfer ddr geometry type to dram blob
BUG=none BRANCH=kukui TEST=Boots correctly on Kukui
Change-Id: I3a677195f5036321939c60c8f9f1bace7c4a2e3f Signed-off-by: Huayang Duan huayang.duan@mediatek.com --- M src/soc/mediatek/mt8183/memory.c 1 file changed, 3 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/43796/4
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43796 )
Change subject: soc/mediatek/mt8183: Transfer ddr geometry type to dram blob ......................................................................
Patch Set 4:
(4 comments)
https://review.coreboot.org/c/coreboot/+/43796/4/src/soc/mediatek/mt8183/mem... File src/soc/mediatek/mt8183/memory.c:
https://review.coreboot.org/c/coreboot/+/43796/4/src/soc/mediatek/mt8183/mem... PS4, Line 102: const struct sdram_params * sdram_cfg = get_sdram_config(); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43796/4/src/soc/mediatek/mt8183/mem... PS4, Line 102: const struct sdram_params * sdram_cfg = get_sdram_config(); "foo * bar" should be "foo *bar"
https://review.coreboot.org/c/coreboot/+/43796/4/src/soc/mediatek/mt8183/mem... PS4, Line 115: dparam->freq_params[0].ddr_geometry = sdram_cfg->ddr_geometry; please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43796/4/src/soc/mediatek/mt8183/mem... PS4, Line 116: printk(BIOS_INFO, "ddr_geometry:%d\n", sdram_cfg->ddr_geometry); please, no spaces at the start of a line
Hello build bot (Jenkins), Julius Werner,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43796
to look at the new patch set (#5).
Change subject: soc/mediatek/mt8183: Transfer ddr geometry type to dram blob ......................................................................
soc/mediatek/mt8183: Transfer ddr geometry type to dram blob
BUG=none BRANCH=kukui TEST=Boots correctly on Kukui
Change-Id: I3a677195f5036321939c60c8f9f1bace7c4a2e3f Signed-off-by: Huayang Duan huayang.duan@mediatek.com --- M src/soc/mediatek/mt8183/memory.c 1 file changed, 3 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/43796/5
Hello build bot (Jenkins), Julius Werner,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43796
to look at the new patch set (#8).
Change subject: soc/mediatek/mt8183: Transfer ddr geometry type to dram blob ......................................................................
soc/mediatek/mt8183: Transfer ddr geometry type to dram blob
BUG=none BRANCH=kukui TEST=Boots correctly on Kukui
Change-Id: I3a677195f5036321939c60c8f9f1bace7c4a2e3f Signed-off-by: Huayang Duan huayang.duan@mediatek.com --- M src/soc/mediatek/mt8183/memory.c 1 file changed, 3 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/43796/8
Hello build bot (Jenkins), Julius Werner,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43796
to look at the new patch set (#9).
Change subject: soc/mediatek/mt8183: Transfer ddr geometry type to dram blob ......................................................................
soc/mediatek/mt8183: Transfer ddr geometry type to dram blob
BUG=none BRANCH=kukui TEST=Boots correctly on Kukui
Change-Id: I3a677195f5036321939c60c8f9f1bace7c4a2e3f Signed-off-by: Huayang Duan huayang.duan@mediatek.com --- M src/soc/mediatek/mt8183/memory.c 1 file changed, 3 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/43796/9
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43796 )
Change subject: soc/mediatek/mt8183: Transfer ddr geometry type to dram blob ......................................................................
Patch Set 12:
(2 comments)
https://review.coreboot.org/c/coreboot/+/43796/12/src/soc/mediatek/mt8183/me... File src/soc/mediatek/mt8183/memory.c:
https://review.coreboot.org/c/coreboot/+/43796/12/src/soc/mediatek/mt8183/me... PS12, Line 100: ) since you only need geometry, we should change this to
static int dram_run_...(..., u32 ddr_geometry, u16 config) {
... dram->freq_params[0].ddr_geometry = ddr_geometry;
https://review.coreboot.org/c/coreboot/+/43796/12/src/soc/mediatek/mt8183/me... PS12, Line 188: } given sdram_config is needed by both full-k and partial-k, we should read it here:
const struct sdram_params *sdram_cfg = get_sdram_config();
And then pas sdram_cfg->ddr_geometry when calling dram_full_full_calibration.
Hello build bot (Jenkins), Julius Werner, Yu-Ping Wu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43796
to look at the new patch set (#13).
Change subject: soc/mediatek/mt8183: Transfer ddr geometry type to dram blob ......................................................................
soc/mediatek/mt8183: Transfer ddr geometry type to dram blob
BUG=none BRANCH=kukui TEST=Boots correctly on Kukui
Change-Id: I3a677195f5036321939c60c8f9f1bace7c4a2e3f Signed-off-by: Huayang Duan huayang.duan@mediatek.com --- M src/soc/mediatek/mt8183/memory.c 1 file changed, 8 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/43796/13
Duan huayang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43796 )
Change subject: soc/mediatek/mt8183: Transfer ddr geometry type to dram blob ......................................................................
Patch Set 13:
(2 comments)
https://review.coreboot.org/c/coreboot/+/43796/12/src/soc/mediatek/mt8183/me... File src/soc/mediatek/mt8183/memory.c:
https://review.coreboot.org/c/coreboot/+/43796/12/src/soc/mediatek/mt8183/me... PS12, Line 100: )
since you only need geometry, we should change this to […]
Done
https://review.coreboot.org/c/coreboot/+/43796/12/src/soc/mediatek/mt8183/me... PS12, Line 188: }
given sdram_config is needed by both full-k and partial-k, we should read it here: […]
Done
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43796 )
Change subject: soc/mediatek/mt8183: Transfer ddr geometry type to dram blob ......................................................................
Patch Set 13:
(1 comment)
thanks - very close, please fix the last one.
https://review.coreboot.org/c/coreboot/+/43796/13/src/soc/mediatek/mt8183/me... File src/soc/mediatek/mt8183/memory.c:
https://review.coreboot.org/c/coreboot/+/43796/13/src/soc/mediatek/mt8183/me... PS13, Line 101: u32 ddr_geometry, u16 config) please indent this with previous line, e.g.
static int dram_run_full_calibration(struct dramc_param *dparam, u32 ddr_geometry, u16 config)
Duan huayang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43796 )
Change subject: soc/mediatek/mt8183: Transfer ddr geometry type to dram blob ......................................................................
Patch Set 13:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43796/13/src/soc/mediatek/mt8183/me... File src/soc/mediatek/mt8183/memory.c:
https://review.coreboot.org/c/coreboot/+/43796/13/src/soc/mediatek/mt8183/me... PS13, Line 101: u32 ddr_geometry, u16 config)
please indent this with previous line, e.g. […]
does this have some indent rule? must align with previous line?
Hello build bot (Jenkins), Julius Werner, Yu-Ping Wu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43796
to look at the new patch set (#14).
Change subject: soc/mediatek/mt8183: Transfer ddr geometry type to dram blob ......................................................................
soc/mediatek/mt8183: Transfer ddr geometry type to dram blob
BUG=none BRANCH=kukui TEST=Boots correctly on Kukui
Change-Id: I3a677195f5036321939c60c8f9f1bace7c4a2e3f Signed-off-by: Huayang Duan huayang.duan@mediatek.com --- M src/soc/mediatek/mt8183/memory.c 1 file changed, 8 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/43796/14
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43796 )
Change subject: soc/mediatek/mt8183: Transfer ddr geometry type to dram blob ......................................................................
Patch Set 14:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43796/13/src/soc/mediatek/mt8183/me... File src/soc/mediatek/mt8183/memory.c:
https://review.coreboot.org/c/coreboot/+/43796/13/src/soc/mediatek/mt8183/me... PS13, Line 101: u32 ddr_geometry, u16 config)
does this have some indent rule? […]
kernel coding style & coreboot coding style didn't clearly define this, but as you can find in most files, they all follow this:
https://google.github.io/styleguide/cppguide.html#Function_Declarations_and_...
align with previous parenthese
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43796 )
Change subject: soc/mediatek/mt8183: Transfer ddr geometry type to dram blob ......................................................................
Patch Set 14: Code-Review+2
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43796 )
Change subject: soc/mediatek/mt8183: Transfer ddr geometry type to dram blob ......................................................................
Patch Set 14:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43796/13/src/soc/mediatek/mt8183/me... File src/soc/mediatek/mt8183/memory.c:
https://review.coreboot.org/c/coreboot/+/43796/13/src/soc/mediatek/mt8183/me... PS13, Line 101: u32 ddr_geometry, u16 config)
kernel coding style & coreboot coding style didn't clearly define this, but as you can find in most […]
Ack
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43796 )
Change subject: soc/mediatek/mt8183: Transfer ddr geometry type to dram blob ......................................................................
Patch Set 14:
(2 comments)
https://review.coreboot.org/c/coreboot/+/43796/14/src/soc/mediatek/mt8183/me... File src/soc/mediatek/mt8183/memory.c:
https://review.coreboot.org/c/coreboot/+/43796/14/src/soc/mediatek/mt8183/me... PS14, Line 116: : Can we add a space after ":"?
ddr_geometry: %d, config: %#x
https://review.coreboot.org/c/coreboot/+/43796/14/src/soc/mediatek/mt8183/me... PS14, Line 116: %d %#x
Hello Hung-Te Lin, build bot (Jenkins), Julius Werner, Yu-Ping Wu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43796
to look at the new patch set (#15).
Change subject: soc/mediatek/mt8183: Transfer ddr geometry type to dram blob ......................................................................
soc/mediatek/mt8183: Transfer ddr geometry type to dram blob
BUG=none BRANCH=kukui TEST=Boots correctly on Kukui
Change-Id: I3a677195f5036321939c60c8f9f1bace7c4a2e3f Signed-off-by: Huayang Duan huayang.duan@mediatek.com --- M src/soc/mediatek/mt8183/memory.c 1 file changed, 8 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/43796/15
Duan huayang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43796 )
Change subject: soc/mediatek/mt8183: Transfer ddr geometry type to dram blob ......................................................................
Patch Set 15:
(2 comments)
https://review.coreboot.org/c/coreboot/+/43796/14/src/soc/mediatek/mt8183/me... File src/soc/mediatek/mt8183/memory.c:
https://review.coreboot.org/c/coreboot/+/43796/14/src/soc/mediatek/mt8183/me... PS14, Line 116: %d
%#x
Done
https://review.coreboot.org/c/coreboot/+/43796/14/src/soc/mediatek/mt8183/me... PS14, Line 116: :
Can we add a space after ":"? […]
Done
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43796 )
Change subject: soc/mediatek/mt8183: Transfer ddr geometry type to dram blob ......................................................................
Patch Set 15: Code-Review+2
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43796 )
Change subject: soc/mediatek/mt8183: Transfer ddr geometry type to dram blob ......................................................................
Patch Set 15: Code-Review+2
Hung-Te Lin has submitted this change. ( https://review.coreboot.org/c/coreboot/+/43796 )
Change subject: soc/mediatek/mt8183: Transfer ddr geometry type to dram blob ......................................................................
soc/mediatek/mt8183: Transfer ddr geometry type to dram blob
BUG=none BRANCH=kukui TEST=Boots correctly on Kukui
Change-Id: I3a677195f5036321939c60c8f9f1bace7c4a2e3f Signed-off-by: Huayang Duan huayang.duan@mediatek.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/43796 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Hung-Te Lin hungte@chromium.org Reviewed-by: Yu-Ping Wu yupingso@google.com --- M src/soc/mediatek/mt8183/memory.c 1 file changed, 8 insertions(+), 3 deletions(-)
Approvals: build bot (Jenkins): Verified Hung-Te Lin: Looks good to me, approved Yu-Ping Wu: Looks good to me, approved
diff --git a/src/soc/mediatek/mt8183/memory.c b/src/soc/mediatek/mt8183/memory.c index ff30f67..6b80a43 100644 --- a/src/soc/mediatek/mt8183/memory.c +++ b/src/soc/mediatek/mt8183/memory.c @@ -97,7 +97,8 @@ return 0; }
-static int dram_run_full_calibration(struct dramc_param *dparam, u16 config) +static int dram_run_full_calibration(struct dramc_param *dparam, + u32 ddr_geometry, u16 config) { initialize_dramc_param(dparam, config);
@@ -111,6 +112,8 @@ return -2;
dparam->do_putc = do_putchar; + dparam->freq_params[0].ddr_geometry = ddr_geometry; + printk(BIOS_INFO, "ddr_geometry: %d, config: %#x\n", ddr_geometry, config); prog_set_entry(&dram, prog_entry(&dram), dparam); prog_run(&dram);
@@ -184,9 +187,11 @@ "Failed to read calibration data from flash\n"); }
+ const struct sdram_params *sdram_cfg = get_sdram_config(); + /* Run full calibration */ printk(BIOS_INFO, "DRAM-K: Full Calibration\n"); - int err = dram_run_full_calibration(dparam, config); + int err = dram_run_full_calibration(dparam, sdram_cfg->ddr_geometry, config); if (err == 0) { printk(BIOS_INFO, "Successfully loaded DRAM blobs and " "ran DRAM calibration\n"); @@ -211,7 +216,7 @@
/* Init params from sdram configs and run partial calibration */ printk(BIOS_INFO, "DRAM-K: Partial Calibration\n"); - init_sdram_params(dparam->freq_params, get_sdram_config()); + init_sdram_params(dparam->freq_params, sdram_cfg); if (mt_set_emi(dparam) != 0) die("Set emi failed with params from sdram config\n"); if (mt_mem_test() != 0)
-
build bot (Jenkins) (Code Review) -
Duan huayang (Code Review)
-
Hung-Te Lin (Code Review)
-
Yu-Ping Wu (Code Review)