Yidi Lin has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47895 )
Change subject: WIP: mediatek/common/mtlib.c: refactor load_blob_file API ......................................................................
WIP: mediatek/common/mtlib.c: refactor load_blob_file API
Add reset_mcu callback and rename to mtlib_load_and_run.
Change-Id: Iebae1485e87940013413a82b362fc361f4a5503e Signed-off-by: Yidi Lin yidi.lin@mediatek.com --- M src/soc/mediatek/common/include/soc/mtlib_common.h M src/soc/mediatek/common/mtlib.c M src/soc/mediatek/mt8192/dpm.c M src/soc/mediatek/mt8192/mcupm.c M src/soc/mediatek/mt8192/sspm.c 5 files changed, 52 insertions(+), 50 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/47895/1
diff --git a/src/soc/mediatek/common/include/soc/mtlib_common.h b/src/soc/mediatek/common/include/soc/mtlib_common.h index 3210468..a2eba62 100644 --- a/src/soc/mediatek/common/include/soc/mtlib_common.h +++ b/src/soc/mediatek/common/include/soc/mtlib_common.h @@ -3,6 +3,6 @@ #ifndef SOC_MEDIATEK_MTLIB_COMMON_H #define SOC_MEDIATEK_MTLIB_COMMON_H
-int load_blob_file(const char *name, unsigned long addr); +int mtlib_load_and_run(const char *fw_name, void *addr, void(*reset_mcu)(void));
#endif /* SOC_MEDIATEK_MTLIB_COMMON_H */ diff --git a/src/soc/mediatek/common/mtlib.c b/src/soc/mediatek/common/mtlib.c index f5b7f4a..8fe5d4b 100644 --- a/src/soc/mediatek/common/mtlib.c +++ b/src/soc/mediatek/common/mtlib.c @@ -4,33 +4,34 @@ #include <cbfs.h> #include <console/console.h> #include <soc/mtlib_common.h> +#include <soc/symbols.h> #include <timer.h>
-#define BUF_SIZE (192 * KiB) -static u8 blob_data[BUF_SIZE] __aligned(8); - -int load_blob_file(const char *name, unsigned long addr) +int mtlib_load_and_run(const char *fw_name, void *addr, void (*reset_mcu)(void)) { struct stopwatch sw; size_t blob_bytes;
stopwatch_init(&sw);
- blob_bytes = cbfs_boot_load_file(name, blob_data, sizeof(blob_data), CBFS_TYPE_RAW); + /* Assume MCU firmwares are loaded after DRAM is ready. */ + blob_bytes = cbfs_boot_load_file(fw_name, _dram_dma, REGION_SIZE(dram_dma), + CBFS_TYPE_RAW); if (blob_bytes == 0) { - printk(BIOS_ERR, "binary %s not found\n", name); + printk(BIOS_ERR, "%s: Failed to load %s.\n", __func__, fw_name); return CB_ERR; }
- memcpy((void *)addr, blob_data, blob_bytes); + memcpy(addr, _dram_dma, blob_bytes);
- /* Memory barrier to ensure that all firmware code is loaded - * before we release the reset pin. - */ + /* Memory barrier to ensure data is flushed before we release the reset pin. */ mb();
- printk(BIOS_DEBUG, "%s: Load %s in %ld msecs, size %ld bytes\n", - __func__, name, stopwatch_duration_msecs(&sw), blob_bytes); + if (reset_mcu != NULL) + reset_mcu(); + + printk(BIOS_DEBUG, "%s: Load %s in %ld msecs, size %zd bytes\n", + __func__, fw_name, stopwatch_duration_msecs(&sw), blob_bytes);
return CB_SUCCESS; } diff --git a/src/soc/mediatek/mt8192/dpm.c b/src/soc/mediatek/mt8192/dpm.c index 912f153..4f2aa46 100644 --- a/src/soc/mediatek/mt8192/dpm.c +++ b/src/soc/mediatek/mt8192/dpm.c @@ -4,44 +4,36 @@ #include <device/mmio.h> #include <soc/dpm.h> #include <soc/mtlib_common.h> -#include <timer.h>
+#define DPM_DM_FW_FILE "dpm.dm" +#define DPM_PM_FW_FILE "dpm.pm"
-/* Return Value: 0 successful, < 0 means failed */ -static int dpm_load_firmware(void) +static void reset_dpm(void) { - const char *dm_file_name = "dpm.dm"; - const char *pm_file_name = "dpm.pm"; - - /* config DPM SRAM layout */ - write32(&mtk_dpm->sw_rstn, read32(&mtk_dpm->sw_rstn) | 0x10000000); - - if (load_blob_file(dm_file_name, DPM_DM_SRAM_BASE)) { - printk(BIOS_ERR, "binary %s not found\n", dm_file_name); - return -1; - } - - if (load_blob_file(pm_file_name, DPM_PM_SRAM_BASE)) { - printk(BIOS_ERR, "binary %s not found\n", pm_file_name); - return -2; - } - /* write bootargs */ write32(&mtk_dpm->twam_window_len, 0x0); write32(&mtk_dpm->twam_mon_type, 0x0);
/* free RST */ write32(&mtk_dpm->sw_rstn, read32(&mtk_dpm->sw_rstn) | 0x1); - - return 0; }
int dpm_init(void) { - if (dpm_load_firmware()) { - printk(BIOS_ERR, "DPM: firmware is not ready\n"); - return -1; - } + /* config DPM SRAM layout */ + write32(&mtk_dpm->sw_rstn, read32(&mtk_dpm->sw_rstn) | 0x10000000);
- return 0; + int ret; + + if (mtlib_load_and_run(DPM_DM_FW_FILE, (void *)DPM_DM_SRAM_BASE, NULL)) + ret = -1; + else if (mtlib_load_and_run(DPM_PM_FW_FILE, (void *)DPM_PM_SRAM_BASE, reset_dpm)) + ret = -2; + else + ret = 0; + + if (ret) + printk(BIOS_ERR, "%s() failed (%d)\n", __func__, ret); + + return ret; } diff --git a/src/soc/mediatek/mt8192/mcupm.c b/src/soc/mediatek/mt8192/mcupm.c index c7500e0..a9cff7e 100644 --- a/src/soc/mediatek/mt8192/mcupm.c +++ b/src/soc/mediatek/mt8192/mcupm.c @@ -6,16 +6,19 @@ #include <soc/mtlib_common.h>
#define ABNORMALBOOT_REG (0x0C55FAA0) -#define MCUPM_BLOB_FILE "mcupm.bin" +#define MCUPM_FW_FILE "mcupm.bin" + +static void reset_mcupm(void) +{ + /* Clear abnormal boot register */ + write32((void *)ABNORMALBOOT_REG, 0x0); + write32(&mt8192_mcupm->sw_rstn, 0x1); +}
void mcupm_init(void) { write32(&mt8192_mcupm->sw_rstn, 0x0);
- if (load_blob_file(MCUPM_BLOB_FILE, MCUPM_SRAM_BASE)) - die("MCUPM file :%s not found.", MCUPM_BLOB_FILE); - - /* Clear abnormal boot register */ - write32((void *)ABNORMALBOOT_REG, 0x0); - write32(&mt8192_mcupm->sw_rstn, 0x1); + if (mtlib_load_and_run(MCUPM_FW_FILE, (void *)MCUPM_SRAM_BASE, reset_mcupm)) + die("%s() failed\n", __func__); } diff --git a/src/soc/mediatek/mt8192/sspm.c b/src/soc/mediatek/mt8192/sspm.c index a555d76..0245e3b 100644 --- a/src/soc/mediatek/mt8192/sspm.c +++ b/src/soc/mediatek/mt8192/sspm.c @@ -1,14 +1,20 @@ /* SPDX-License-Identifier: GPL-2.0-only */
+#include <console/console.h> #include <device/mmio.h> #include <soc/mtlib_common.h> #include <soc/sspm.h>
+#define SSPM_FW_FILE "sspm.bin" + +static void reset_sspm(void) +{ + write32(&mt8192_sspm->sw_rstn, 0x1); +} + void sspm_init(void) { - const char *sspm_file_name = "sspm.bin"; + if (mtlib_load_and_run(SSPM_FW_FILE, (void *)SSPM_SRAM_BASE, reset_sspm)) + printk(BIOS_ERR, "%s() failed\n", __func__);
- if (load_blob_file(sspm_file_name, SSPM_SRAM_BASE) == CB_SUCCESS) { - write32(&mt8192_sspm->sw_rstn, 0x1); - } }
Hello Hung-Te Lin, build bot (Jenkins), Paul Menzel, Yu-Ping Wu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47895
to look at the new patch set (#6).
Change subject: WIP: mediatek/common/mtlib.c: refactor load_blob_file API ......................................................................
WIP: mediatek/common/mtlib.c: refactor load_blob_file API
Add reset_mcu callback and rename to mtlib_load_and_run.
Change-Id: Iebae1485e87940013413a82b362fc361f4a5503e Signed-off-by: Yidi Lin yidi.lin@mediatek.com --- M src/soc/mediatek/common/include/soc/mtlib_common.h M src/soc/mediatek/common/mtlib.c M src/soc/mediatek/mt8192/dpm.c M src/soc/mediatek/mt8192/mcupm.c M src/soc/mediatek/mt8192/sspm.c 5 files changed, 53 insertions(+), 46 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/47895/6
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47895 )
Change subject: WIP: mediatek/common/mtlib.c: refactor load_blob_file API ......................................................................
Patch Set 6:
(8 comments)
https://review.coreboot.org/c/coreboot/+/47895/6/src/soc/mediatek/common/mtl... File src/soc/mediatek/common/mtlib.c:
https://review.coreboot.org/c/coreboot/+/47895/6/src/soc/mediatek/common/mtl... PS6, Line 10: mcu Is mcu a general term, suitable for sspm?
https://review.coreboot.org/c/coreboot/+/47895/6/src/soc/mediatek/common/mtl... PS6, Line 10: fw_name blob_name (since you're using 'blob_bytes')
https://review.coreboot.org/c/coreboot/+/47895/6/src/soc/mediatek/common/mtl... PS6, Line 13: blob_bytes I'd prefer 'blob_size'.
https://review.coreboot.org/c/coreboot/+/47895/6/src/soc/mediatek/common/mtl... PS6, Line 21: . Remove this
https://review.coreboot.org/c/coreboot/+/47895/6/src/soc/mediatek/common/mtl... PS6, Line 33: , size %zd bytes Either "(%zd bytes)" or "(size = %zd)"
https://review.coreboot.org/c/coreboot/+/47895/6/src/soc/mediatek/mt8192/dpm... File src/soc/mediatek/mt8192/dpm.c:
https://review.coreboot.org/c/coreboot/+/47895/6/src/soc/mediatek/mt8192/dpm... PS6, Line 36: printk(BIOS_ERR, "%s() failed (%d)\n", __func__, ret); Same. Maybe no need to repeat the error message here.
https://review.coreboot.org/c/coreboot/+/47895/6/src/soc/mediatek/mt8192/mcu... File src/soc/mediatek/mt8192/mcupm.c:
https://review.coreboot.org/c/coreboot/+/47895/6/src/soc/mediatek/mt8192/mcu... PS6, Line 23: die("%s() failed\n", __func__); Why do we die here, but only print an error message in sspm_init()? Is this failure more severe than the other?
https://review.coreboot.org/c/coreboot/+/47895/6/src/soc/mediatek/mt8192/ssp... File src/soc/mediatek/mt8192/sspm.c:
https://review.coreboot.org/c/coreboot/+/47895/6/src/soc/mediatek/mt8192/ssp... PS6, Line 18: printk(BIOS_ERR, "%s() failed\n", __func__); An error message should have been printed in mtlib_load_and_run(). Maybe no need to repeat it here.
Yidi Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47895 )
Change subject: WIP: mediatek/common/mtlib.c: refactor load_blob_file API ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47895/6/src/soc/mediatek/common/mtl... File src/soc/mediatek/common/mtlib.c:
https://review.coreboot.org/c/coreboot/+/47895/6/src/soc/mediatek/common/mtl... PS6, Line 10: mcu
Is mcu a general term, suitable for sspm?
yes, mcu stands for micro controller unit. Each firmware is loaded and running on its own MCU.
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47895 )
Change subject: WIP: mediatek/common/mtlib.c: refactor load_blob_file API ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47895/6/src/soc/mediatek/common/mtl... File src/soc/mediatek/common/mtlib.c:
https://review.coreboot.org/c/coreboot/+/47895/6/src/soc/mediatek/common/mtl... PS6, Line 10: mcu
yes, mcu stands for micro controller unit. […]
Ack
Yidi Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47895 )
Change subject: WIP: mediatek/common/mtlib.c: refactor load_blob_file API ......................................................................
Patch Set 6:
(7 comments)
This patch is squashed into CB:46930. I will abandon it later.
https://review.coreboot.org/c/coreboot/+/47895/6/src/soc/mediatek/common/mtl... File src/soc/mediatek/common/mtlib.c:
https://review.coreboot.org/c/coreboot/+/47895/6/src/soc/mediatek/common/mtl... PS6, Line 10: fw_name
blob_name (since you're using 'blob_bytes')
Done
https://review.coreboot.org/c/coreboot/+/47895/6/src/soc/mediatek/common/mtl... PS6, Line 13: blob_bytes
I'd prefer 'blob_size'.
Done
https://review.coreboot.org/c/coreboot/+/47895/6/src/soc/mediatek/common/mtl... PS6, Line 21: .
Remove this
Done
https://review.coreboot.org/c/coreboot/+/47895/6/src/soc/mediatek/common/mtl... PS6, Line 33: , size %zd bytes
Either "(%zd bytes)" or "(size = %zd)"
Done
https://review.coreboot.org/c/coreboot/+/47895/6/src/soc/mediatek/mt8192/dpm... File src/soc/mediatek/mt8192/dpm.c:
https://review.coreboot.org/c/coreboot/+/47895/6/src/soc/mediatek/mt8192/dpm... PS6, Line 36: printk(BIOS_ERR, "%s() failed (%d)\n", __func__, ret);
Same. Maybe no need to repeat the error message here.
Done
https://review.coreboot.org/c/coreboot/+/47895/6/src/soc/mediatek/mt8192/mcu... File src/soc/mediatek/mt8192/mcupm.c:
https://review.coreboot.org/c/coreboot/+/47895/6/src/soc/mediatek/mt8192/mcu... PS6, Line 23: die("%s() failed\n", __func__);
Why do we die here, but only print an error message in sspm_init()? Is this failure more severe than […]
Yes, mcupm firmware effects suspend/resume flow.
https://review.coreboot.org/c/coreboot/+/47895/6/src/soc/mediatek/mt8192/ssp... File src/soc/mediatek/mt8192/sspm.c:
https://review.coreboot.org/c/coreboot/+/47895/6/src/soc/mediatek/mt8192/ssp... PS6, Line 18: printk(BIOS_ERR, "%s() failed\n", __func__);
An error message should have been printed in mtlib_load_and_run(). Maybe no need to repeat it here.
Done
Yidi Lin has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/47895 )
Change subject: WIP: mediatek/common/mtlib.c: refactor load_blob_file API ......................................................................
Abandoned
-
Yidi Lin (Code Review) -
Yu-Ping Wu (Code Review)