Hello Duan huayang,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/46393
to review the following change.
Change subject: soc/mediatek/mt8192: Add dpm loader ......................................................................
soc/mediatek/mt8192: Add dpm loader
BUG=none TEST=Boots correctly on Asurada
Signed-off-by: Huayang Duan huayang.duan@mediatek.com Change-Id: I16b341ad63940b45b886c4a7fd733c1970624e40 --- M src/mainboard/google/asurada/mainboard.c M src/soc/mediatek/mt8192/Makefile.inc A src/soc/mediatek/mt8192/dpm.c M src/soc/mediatek/mt8192/include/soc/addressmap.h A src/soc/mediatek/mt8192/include/soc/dpm.h 5 files changed, 136 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/46393/1
diff --git a/src/mainboard/google/asurada/mainboard.c b/src/mainboard/google/asurada/mainboard.c index 7b3ef22..677e917 100644 --- a/src/mainboard/google/asurada/mainboard.c +++ b/src/mainboard/google/asurada/mainboard.c @@ -5,6 +5,7 @@ #include <device/device.h> #include <device/mmio.h> #include <lib.h> +#include <soc/dpm.h> #include <soc/gpio_common.h> #include <soc/spm.h> #include <soc/usb.h> @@ -79,6 +80,9 @@
register_reset_to_bl31();
+ if (dpm_init()) + printk(BIOS_ERR, "dpm init fail, system can't do DVFS switch\n"); + if (spm_init()) printk(BIOS_ERR, "spm init fail, system suspend may stuck\n"); } diff --git a/src/soc/mediatek/mt8192/Makefile.inc b/src/soc/mediatek/mt8192/Makefile.inc index 59d105c..5fc90a8 100644 --- a/src/soc/mediatek/mt8192/Makefile.inc +++ b/src/soc/mediatek/mt8192/Makefile.inc @@ -37,6 +37,7 @@ ramstage-y += ../common/gpio.c gpio.c ramstage-y += emi.c ramstage-$(CONFIG_SPI_FLASH) += ../common/spi.c spi.c +ramstage-y += dpm.c ramstage-y += spm.c ramstage-y += ../common/mmu_operations.c mmu_operations.c ramstage-y += ../common/mtcmos.c mtcmos.c @@ -48,6 +49,16 @@
MT8192_BLOB_DIR := 3rdparty/blobs/soc/mediatek/mt8192
+cbfs-files-y += dpm.dm +dpm.dm-file := $(MT8192_BLOB_DIR)/dpm.dm +dpm.dm-type := raw +dpm.dm-compression := $(CBFS_COMPRESS_FLAG) + +cbfs-files-y += dpm.pm +dpm.pm-file := $(MT8192_BLOB_DIR)/dpm.pm +dpm.pm-type := raw +dpm.pm-compression := $(CBFS_COMPRESS_FLAG) + cbfs-files-y += spm_firmware.bin spm_firmware.bin-file := $(MT8192_BLOB_DIR)/spm_firmware.bin spm_firmware.bin-type := raw diff --git a/src/soc/mediatek/mt8192/dpm.c b/src/soc/mediatek/mt8192/dpm.c new file mode 100644 index 0000000..7e018b5 --- /dev/null +++ b/src/soc/mediatek/mt8192/dpm.c @@ -0,0 +1,72 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <assert.h> +#include <cbfs.h> +#include <console/console.h> +#include <delay.h> +#include <device/mmio.h> +#include <soc/dpm.h> +#include <timer.h> + +#define DM_BUF_SIZE (2 * KiB) +#define PM_BUF_SIZE (20 * KiB) + +static u8 dpm_dm_bin[DM_BUF_SIZE] __aligned(8); +static u8 dpm_pm_bin[PM_BUF_SIZE] __aligned(8); +static void *dpm_pm_reg = (void *)DPM_PM_SRAM_BASE; +static void *dpm_dm_reg = (void *)DPM_DM_SRAM_BASE; + +static int dpm_load_firmware(void) +{ + const char *dm_file_name = "dpm.dm"; + const char *pm_file_name = "dpm.pm"; + size_t dm_file_size, pm_file_size; + + dm_file_size = cbfs_boot_load_file(dm_file_name, dpm_dm_bin, + sizeof(dpm_dm_bin), CBFS_TYPE_RAW); + if (dm_file_size == 0) { + printk(BIOS_ERR, "binary %s not found\n", dm_file_name); + return -1; + } + + pm_file_size = cbfs_boot_load_file(pm_file_name, dpm_pm_bin, + sizeof(dpm_pm_bin), CBFS_TYPE_RAW); + if (pm_file_size == 0) { + printk(BIOS_ERR, "binary %s not found\n", pm_file_name); + return -2; + } + printk(BIOS_INFO, "dm_file_size:%d, pm_file_size:%d\n", + (int)dm_file_size, (int)pm_file_size); + + /* config DPM SRAM layout */ + write32(&mtk_dpm->sw_rstn, read32(&mtk_dpm->sw_rstn) | 0x10000000); + + /* copy PM DM to DPM_SRAM */ + memcpy(dpm_pm_reg, dpm_pm_bin, pm_file_size); + memcpy(dpm_dm_reg, dpm_dm_bin, dm_file_size); + + /* 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) +{ + struct stopwatch sw; + stopwatch_init(&sw); + + if (dpm_load_firmware()) { + printk(BIOS_ERR, "DPM: firmware is not ready\n"); + return -1; + } + + printk(BIOS_INFO, "DPM: load finish in %ld msecs\n", + stopwatch_duration_msecs(&sw)); + + return 0; +} diff --git a/src/soc/mediatek/mt8192/include/soc/addressmap.h b/src/soc/mediatek/mt8192/include/soc/addressmap.h old mode 100755 new mode 100644 index 9864e5b..5f49fe5 --- a/src/soc/mediatek/mt8192/include/soc/addressmap.h +++ b/src/soc/mediatek/mt8192/include/soc/addressmap.h @@ -29,6 +29,9 @@ PMICSPI_MST_BASE = IO_PHYS + 0x00028000, SPMI_MST_BASE = IO_PHYS + 0x00029000, AUXADC_BASE = IO_PHYS + 0x01001000, + DPM_PM_SRAM_BASE = IO_PHYS + 0x00900000, + DPM_DM_SRAM_BASE = IO_PHYS + 0x00920000, + DPM_CFG_BASE = IO_PHYS + 0x00940000, UART0_BASE = IO_PHYS + 0x01002000, SPI0_BASE = IO_PHYS + 0x0100A000, SPI1_BASE = IO_PHYS + 0x01010000, diff --git a/src/soc/mediatek/mt8192/include/soc/dpm.h b/src/soc/mediatek/mt8192/include/soc/dpm.h new file mode 100644 index 0000000..5763dab --- /dev/null +++ b/src/soc/mediatek/mt8192/include/soc/dpm.h @@ -0,0 +1,46 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef __SOC_MEDIATEK_MT8192_DPM_H__ +#define __SOC_MEDIATEK_MT8192_DPM_H__ + +#include <soc/addressmap.h> +#include <stdint.h> +#include <types.h> + +#define DPM_DM_OFFSET 0x8000 /* 32KB */ + +struct dpm_regs { + u32 sw_rstn; + u32 rsvd_0[3072]; + u32 mclk_div; + u32 rsvd_1[3071]; + u32 twam_window_len; + u32 twam_mon_type; + u32 rsvd_2[1022]; + u32 low_power_cfg_0; + u32 low_power_cfg_1; + u32 rsvd_3[1]; + u32 fsm_out_ctrl_0; + u32 rsvd_4[8]; + u32 fsm_cfg_1; + u32 low_power_cfg_3; + u32 dfd_dbug_0; + u32 rsvd_5[28]; + u32 status_4; +}; + +check_member(dpm_regs, mclk_div, 0x3004); +check_member(dpm_regs, twam_window_len, 0x6004); +check_member(dpm_regs, low_power_cfg_0, 0x7004); +check_member(dpm_regs, low_power_cfg_1, 0x7008); +check_member(dpm_regs, fsm_out_ctrl_0, 0x7010); +check_member(dpm_regs, fsm_cfg_1, 0x7034); +check_member(dpm_regs, low_power_cfg_3, 0x7038); +check_member(dpm_regs, dfd_dbug_0, 0x703C); +check_member(dpm_regs, status_4, 0x70B0);; + +int dpm_init(void); + +#define mtk_dpm ((struct dpm_regs *)DPM_CFG_BASE) + +#endif /* __SOC_MEDIATEK_MT8192_DPM_H__ */
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46393 )
Change subject: soc/mediatek/mt8192: Add dpm loader ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46393/1/src/soc/mediatek/mt8192/inc... File src/soc/mediatek/mt8192/include/soc/dpm.h:
https://review.coreboot.org/c/coreboot/+/46393/1/src/soc/mediatek/mt8192/inc... PS1, Line 40: check_member(dpm_regs, status_4, 0x70B0);; Statements terminations use 1 semicolon
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46393 )
Change subject: soc/mediatek/mt8192: Add dpm loader ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46393/2/src/soc/mediatek/mt8192/inc... File src/soc/mediatek/mt8192/include/soc/dpm.h:
https://review.coreboot.org/c/coreboot/+/46393/2/src/soc/mediatek/mt8192/inc... PS2, Line 40: check_member(dpm_regs, status_4, 0x70B0);; Statements terminations use 1 semicolon
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46393 )
Change subject: soc/mediatek/mt8192: Add dpm loader ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46393/3/src/soc/mediatek/mt8192/inc... File src/soc/mediatek/mt8192/include/soc/dpm.h:
https://review.coreboot.org/c/coreboot/+/46393/3/src/soc/mediatek/mt8192/inc... PS3, Line 40: check_member(dpm_regs, status_4, 0x70B0);; Statements terminations use 1 semicolon
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46393 )
Change subject: soc/mediatek/mt8192: Add dpm loader ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46393/4/src/soc/mediatek/mt8192/inc... File src/soc/mediatek/mt8192/include/soc/dpm.h:
https://review.coreboot.org/c/coreboot/+/46393/4/src/soc/mediatek/mt8192/inc... PS4, Line 40: check_member(dpm_regs, status_4, 0x70B0);; Statements terminations use 1 semicolon
Xi Chen has uploaded a new patch set (#5) to the change originally created by Yidi Lin. ( https://review.coreboot.org/c/coreboot/+/46393 )
Change subject: soc/mediatek/mt8192: Add dpm loader ......................................................................
soc/mediatek/mt8192: Add dpm loader
BUG=none TEST=Boots correctly on Asurada
Signed-off-by: Huayang Duan huayang.duan@mediatek.com Change-Id: I16b341ad63940b45b886c4a7fd733c1970624e40 --- M src/mainboard/google/asurada/mainboard.c M src/soc/mediatek/mt8192/Makefile.inc A src/soc/mediatek/mt8192/dpm.c M src/soc/mediatek/mt8192/include/soc/addressmap.h A src/soc/mediatek/mt8192/include/soc/dpm.h 5 files changed, 136 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/46393/5
Xi Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46393 )
Change subject: soc/mediatek/mt8192: Add dpm loader ......................................................................
Patch Set 5:
(1 comment)
verify pass.
https://review.coreboot.org/c/coreboot/+/46393/4/src/soc/mediatek/mt8192/inc... File src/soc/mediatek/mt8192/include/soc/dpm.h:
https://review.coreboot.org/c/coreboot/+/46393/4/src/soc/mediatek/mt8192/inc... PS4, Line 40: check_member(dpm_regs, status_4, 0x70B0);;
Statements terminations use 1 semicolon
done
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46393 )
Change subject: soc/mediatek/mt8192: Add dpm loader ......................................................................
Patch Set 5:
(6 comments)
https://review.coreboot.org/c/coreboot/+/46393/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46393/5//COMMIT_MSG@8 PS5, Line 8: Please elaborate, what DPM is, what it is used for, and where it’s documented.
https://review.coreboot.org/c/coreboot/+/46393/5//COMMIT_MSG@10 PS5, Line 10: TEST=Boots correctly on Asurada Tested with what firmware version, and how much time did it add? Maybe add the related coreboot log messages.
https://review.coreboot.org/c/coreboot/+/46393/5/src/soc/mediatek/mt8192/dpm... File src/soc/mediatek/mt8192/dpm.c:
https://review.coreboot.org/c/coreboot/+/46393/5/src/soc/mediatek/mt8192/dpm... PS5, Line 18: Document the return values.
https://review.coreboot.org/c/coreboot/+/46393/5/src/soc/mediatek/mt8192/dpm... PS5, Line 23: size_t dm_file_size, pm_file_size; Is the unit bytes? Add it to the variable name?
https://review.coreboot.org/c/coreboot/+/46393/5/src/soc/mediatek/mt8192/dpm... PS5, Line 38: printk(BIOS_INFO, "dm_file_size:%d, pm_file_size:%d\n", Does the blob contain a version? Please print it out too.
https://review.coreboot.org/c/coreboot/+/46393/5/src/soc/mediatek/mt8192/dpm... PS5, Line 38: dm_file_size:%d, pm_file_size:%d Please add a space after the colon and the units.
Hello Xi Chen, build bot (Jenkins), Patrick Georgi, Martin Roth, Duan huayang,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46393
to look at the new patch set (#6).
Change subject: soc/mediatek/mt8192: Add dpm loader ......................................................................
soc/mediatek/mt8192: Add dpm loader
BUG=none TEST=Boots correctly on Asurada
Signed-off-by: Huayang Duan huayang.duan@mediatek.com Change-Id: I16b341ad63940b45b886c4a7fd733c1970624e40 --- M src/mainboard/google/asurada/mainboard.c M src/soc/mediatek/mt8192/Makefile.inc A src/soc/mediatek/mt8192/dpm.c M src/soc/mediatek/mt8192/include/soc/addressmap.h A src/soc/mediatek/mt8192/include/soc/dpm.h 5 files changed, 136 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/46393/6
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46393 )
Change subject: soc/mediatek/mt8192: Add dpm loader ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46393/6/src/soc/mediatek/mt8192/inc... File src/soc/mediatek/mt8192/include/soc/dpm.h:
https://review.coreboot.org/c/coreboot/+/46393/6/src/soc/mediatek/mt8192/inc... PS6, Line 40: check_member(dpm_regs, status_4, 0x70B0);; Statements terminations use 1 semicolon
Xixi Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46393 )
Change subject: soc/mediatek/mt8192: Add dpm loader ......................................................................
Patch Set 6:
(2 comments)
https://review.coreboot.org/c/coreboot/+/46393/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46393/5//COMMIT_MSG@8 PS5, Line 8:
Please elaborate, what DPM is, what it is used for, and where it’s documented.
DPM is used for power manager, which is mediatek proprietary, it's binary release. Sorry for that we can't document it.
https://review.coreboot.org/c/coreboot/+/46393/5//COMMIT_MSG@10 PS5, Line 10: TEST=Boots correctly on Asurada
Tested with what firmware version, and how much time did it add? Maybe add the related coreboot log […]
Dpm is tested pass with firmware 13457.0.
Dpm log is added already, for example: DPM: load finish in 37 msecs
Xi Chen has uploaded a new patch set (#7) to the change originally created by Yidi Lin. ( https://review.coreboot.org/c/coreboot/+/46393 )
Change subject: soc/mediatek/mt8192: Add dpm loader ......................................................................
soc/mediatek/mt8192: Add dpm loader
BUG=none TEST=Boots correctly on Asurada
Signed-off-by: Huayang Duan huayang.duan@mediatek.com Change-Id: I16b341ad63940b45b886c4a7fd733c1970624e40 --- M src/mainboard/google/asurada/mainboard.c M src/soc/mediatek/mt8192/Makefile.inc A src/soc/mediatek/mt8192/dpm.c M src/soc/mediatek/mt8192/include/soc/addressmap.h A src/soc/mediatek/mt8192/include/soc/dpm.h 5 files changed, 137 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/46393/7
Xixi Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46393 )
Change subject: soc/mediatek/mt8192: Add dpm loader ......................................................................
Patch Set 7:
(4 comments)
https://review.coreboot.org/c/coreboot/+/46393/5/src/soc/mediatek/mt8192/dpm... File src/soc/mediatek/mt8192/dpm.c:
https://review.coreboot.org/c/coreboot/+/46393/5/src/soc/mediatek/mt8192/dpm... PS5, Line 18:
Document the return values.
Ack
https://review.coreboot.org/c/coreboot/+/46393/5/src/soc/mediatek/mt8192/dpm... PS5, Line 23: size_t dm_file_size, pm_file_size;
Is the unit bytes? Add it to the variable name?
Yes, unit as bytes.
https://review.coreboot.org/c/coreboot/+/46393/5/src/soc/mediatek/mt8192/dpm... PS5, Line 38: printk(BIOS_INFO, "dm_file_size:%d, pm_file_size:%d\n",
Does the blob contain a version? Please print it out too.
The blob doesn't contain a version.
https://review.coreboot.org/c/coreboot/+/46393/5/src/soc/mediatek/mt8192/dpm... PS5, Line 38: dm_file_size:%d, pm_file_size:%d
Please add a space after the colon and the units.
Ack
Xi Chen has uploaded a new patch set (#8) to the change originally created by Yidi Lin. ( https://review.coreboot.org/c/coreboot/+/46393 )
Change subject: soc/mediatek/mt8192: Add dpm loader ......................................................................
soc/mediatek/mt8192: Add dpm loader
BUG=none TEST=Boots correctly on Asurada
Signed-off-by: Huayang Duan huayang.duan@mediatek.com Change-Id: I16b341ad63940b45b886c4a7fd733c1970624e40 --- M src/mainboard/google/asurada/mainboard.c M src/soc/mediatek/mt8192/Makefile.inc A src/soc/mediatek/mt8192/dpm.c M src/soc/mediatek/mt8192/include/soc/addressmap.h A src/soc/mediatek/mt8192/include/soc/dpm.h 5 files changed, 137 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/46393/8
Hello Xi Chen, build bot (Jenkins), Patrick Georgi, Martin Roth, Duan huayang,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46393
to look at the new patch set (#12).
Change subject: soc/mediatek/mt8192: Add dpm loader ......................................................................
soc/mediatek/mt8192: Add dpm loader
BUG=none TEST=Boots correctly on Asurada
Signed-off-by: Huayang Duan huayang.duan@mediatek.com Change-Id: I16b341ad63940b45b886c4a7fd733c1970624e40 --- M src/mainboard/google/asurada/mainboard.c M src/soc/mediatek/mt8192/Makefile.inc A src/soc/mediatek/mt8192/dpm.c M src/soc/mediatek/mt8192/include/soc/addressmap.h A src/soc/mediatek/mt8192/include/soc/dpm.h 5 files changed, 137 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/46393/12
Xi Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46393 )
Change subject: soc/mediatek/mt8192: Add dpm loader ......................................................................
Patch Set 15:
I think build bot may make a mistake: File src/soc/mediatek/mt8192/auxadc.c has one or more executable bits set in the file permissions. File src/soc/mediatek/mt8192/include/soc/auxadc.h has one or more executable bits set in the file permissions. File src/soc/mediatek/mt8192/include/soc/efuse.h has one or more executable bits set in the file permissions.
These files don't belong to this patch.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46393 )
Change subject: soc/mediatek/mt8192: Add dpm loader ......................................................................
Patch Set 15:
Patch Set 15:
I think build bot may make a mistake: File src/soc/mediatek/mt8192/auxadc.c has one or more executable bits set in the file permissions. File src/soc/mediatek/mt8192/include/soc/auxadc.h has one or more executable bits set in the file permissions. File src/soc/mediatek/mt8192/include/soc/efuse.h has one or more executable bits set in the file permissions.
These files don't belong to this patch.
Patch CB:46390 brings that issue in and this one depends on it. As soon as you fixed the file permissions there, the issue should be gone.
Hello Hung-Te Lin, Xi Chen, build bot (Jenkins), Patrick Georgi, Martin Roth, Yu-Ping Wu, Duan huayang,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46393
to look at the new patch set (#25).
Change subject: soc/mediatek/mt8192: Add dpm loader ......................................................................
soc/mediatek/mt8192: Add dpm loader
BUG=none TEST=Boots correctly on Asurada
Signed-off-by: Huayang Duan huayang.duan@mediatek.com Change-Id: I16b341ad63940b45b886c4a7fd733c1970624e40 --- M src/mainboard/google/asurada/mainboard.c M src/soc/mediatek/mt8192/Makefile.inc A src/soc/mediatek/mt8192/dpm.c M src/soc/mediatek/mt8192/include/soc/addressmap.h A src/soc/mediatek/mt8192/include/soc/dpm.h 5 files changed, 137 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/46393/25
Xixi Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46393 )
Change subject: soc/mediatek/mt8192: Add dpm loader ......................................................................
Patch Set 25:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46393/5/src/soc/mediatek/mt8192/dpm... File src/soc/mediatek/mt8192/dpm.c:
https://review.coreboot.org/c/coreboot/+/46393/5/src/soc/mediatek/mt8192/dpm... PS5, Line 38: printk(BIOS_INFO, "dm_file_size:%d, pm_file_size:%d\n",
The blob doesn't contain a version.
Done
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46393 )
Change subject: soc/mediatek/mt8192: Add dpm loader ......................................................................
Patch Set 28:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46393/28/src/soc/mediatek/mt8192/dp... File src/soc/mediatek/mt8192/dpm.c:
https://review.coreboot.org/c/coreboot/+/46393/28/src/soc/mediatek/mt8192/dp... PS28, Line 25: : dm_file_bytes = cbfs_boot_load_file(dm_file_name, dpm_dm_bin, : sizeof(dpm_dm_bin), CBFS_TYPE_RAW); : if (dm_file_bytes == 0) { : printk(BIOS_ERR, "binary %s not found\n", dm_file_name); : return -1; : } : : pm_file_bytes = cbfs_boot_load_file(pm_file_name, dpm_pm_bin, : sizeof(dpm_pm_bin), CBFS_TYPE_RAW); : if (pm_file_bytes == 0) { : printk(BIOS_ERR, "binary %s not found\n", pm_file_name); : return -2; : } why can't we use the ocmmon API?
Yidi Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46393 )
Change subject: soc/mediatek/mt8192: Add dpm loader ......................................................................
Patch Set 28:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46393/28/src/soc/mediatek/mt8192/dp... File src/soc/mediatek/mt8192/dpm.c:
https://review.coreboot.org/c/coreboot/+/46393/28/src/soc/mediatek/mt8192/dp... PS28, Line 25: : dm_file_bytes = cbfs_boot_load_file(dm_file_name, dpm_dm_bin, : sizeof(dpm_dm_bin), CBFS_TYPE_RAW); : if (dm_file_bytes == 0) { : printk(BIOS_ERR, "binary %s not found\n", dm_file_name); : return -1; : } : : pm_file_bytes = cbfs_boot_load_file(pm_file_name, dpm_pm_bin, : sizeof(dpm_pm_bin), CBFS_TYPE_RAW); : if (pm_file_bytes == 0) { : printk(BIOS_ERR, "binary %s not found\n", pm_file_name); : return -2; : }
why can't we use the ocmmon API?
I will squash CB:46932 into this patch in next patchset.
Hello Hung-Te Lin, Xi Chen, build bot (Jenkins), Patrick Georgi, Martin Roth, Yu-Ping Wu, Duan huayang,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46393
to look at the new patch set (#30).
Change subject: soc/mediatek/mt8192: Add dpm loader ......................................................................
soc/mediatek/mt8192: Add dpm loader
BUG=none TEST=Boots correctly on Asurada
Signed-off-by: Huayang Duan huayang.duan@mediatek.com Change-Id: I16b341ad63940b45b886c4a7fd733c1970624e40 --- M src/mainboard/google/asurada/mainboard.c M src/soc/mediatek/mt8192/Makefile.inc A src/soc/mediatek/mt8192/dpm.c M src/soc/mediatek/mt8192/include/soc/addressmap.h A src/soc/mediatek/mt8192/include/soc/dpm.h 5 files changed, 137 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/46393/30
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46393 )
Change subject: soc/mediatek/mt8192: Add dpm loader ......................................................................
Patch Set 32:
(13 comments)
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/dp... File src/soc/mediatek/mt8192/dpm.c:
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/dp... PS32, Line 27: Just one tab, or
dm_file_bytes = cbfs_boot_load_file( dm_file_name, dpm_dm_bin, sizeof(dpm_dm_bin), CBFS_TYPE_RAW);
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/dp... PS32, Line 29: binary %s not found Failed to load %s
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/dp... PS32, Line 34: sizeof(dpm_pm_bin), CBFS_TYPE_RAW); Same.
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/dp... PS32, Line 36: binary %s not found Failed to load %s
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/dp... PS32, Line 39: %d Use "%zu" (or "%#zx" if you prefer hex), and no need to cast it to "int".
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/dp... PS32, Line 40: (int) Please align with "BIOS_INFO".
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/dp... PS32, Line 43: write32(&mtk_dpm->sw_rstn, read32(&mtk_dpm->sw_rstn) | 0x10000000); Use clrsetbits32.
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/dp... PS32, Line 43: 0x10000000 Please define a macro for this.
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/dp... PS32, Line 54: 0x1 Please define a macro for this.
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/dp... PS32, Line 54: write32(&mtk_dpm->sw_rstn, read32(&mtk_dpm->sw_rstn) | 0x1); Use clrsetbits32.
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/in... File src/soc/mediatek/mt8192/include/soc/addressmap.h:
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/in... PS32, Line 30: AUXADC_BASE = IO_PHYS + 0x01001000, : DPM_PM_SRAM_BASE = IO_PHYS + 0x00900000, : DPM_DM_SRAM_BASE = IO_PHYS + 0x00920000, : DPM_CFG_BASE = IO_PHYS + 0x00940000, Please sort these by values.
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/in... File src/soc/mediatek/mt8192/include/soc/dpm.h:
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/in... PS32, Line 10: 0x8000 Use KiB?
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/in... PS32, Line 44: #define mtk_dpm ((struct dpm_regs *)DPM_CFG_BASE) Use global variable to be consistent with others.
Hello Hung-Te Lin, Xi Chen, build bot (Jenkins), Patrick Georgi, Martin Roth, Yu-Ping Wu, Duan huayang,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46393
to look at the new patch set (#33).
Change subject: soc/mediatek/mt8192: Add dpm loader ......................................................................
soc/mediatek/mt8192: Add dpm loader
BUG=none TEST=Boots correctly on Asurada
Signed-off-by: Huayang Duan huayang.duan@mediatek.com Change-Id: I16b341ad63940b45b886c4a7fd733c1970624e40 --- M src/mainboard/google/asurada/mainboard.c M src/soc/mediatek/mt8192/Makefile.inc A src/soc/mediatek/mt8192/dpm.c M src/soc/mediatek/mt8192/include/soc/addressmap.h A src/soc/mediatek/mt8192/include/soc/dpm.h 5 files changed, 109 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/46393/33
Yidi Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46393 )
Change subject: soc/mediatek/mt8192: Add dpm loader ......................................................................
Patch Set 33:
(16 comments)
https://review.coreboot.org/c/coreboot/+/46393/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46393/5//COMMIT_MSG@8 PS5, Line 8:
DPM is used for power manager, which is mediatek proprietary, it's binary release. […]
Done
https://review.coreboot.org/c/coreboot/+/46393/5//COMMIT_MSG@10 PS5, Line 10: TEST=Boots correctly on Asurada
Dpm is tested pass with firmware 13457.0. […]
Done
https://review.coreboot.org/c/coreboot/+/46393/28/src/soc/mediatek/mt8192/dp... File src/soc/mediatek/mt8192/dpm.c:
https://review.coreboot.org/c/coreboot/+/46393/28/src/soc/mediatek/mt8192/dp... PS28, Line 25: : dm_file_bytes = cbfs_boot_load_file(dm_file_name, dpm_dm_bin, : sizeof(dpm_dm_bin), CBFS_TYPE_RAW); : if (dm_file_bytes == 0) { : printk(BIOS_ERR, "binary %s not found\n", dm_file_name); : return -1; : } : : pm_file_bytes = cbfs_boot_load_file(pm_file_name, dpm_pm_bin, : sizeof(dpm_pm_bin), CBFS_TYPE_RAW); : if (pm_file_bytes == 0) { : printk(BIOS_ERR, "binary %s not found\n", pm_file_name); : return -2; : }
I will squash CB:46932 into this patch in next patchset.
Done
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/dp... File src/soc/mediatek/mt8192/dpm.c:
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/dp... PS32, Line 27:
Just one tab, or […]
Done
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/dp... PS32, Line 29: binary %s not found
Failed to load %s
Done
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/dp... PS32, Line 34: sizeof(dpm_pm_bin), CBFS_TYPE_RAW);
Same.
Please review new API proposed in CB:47895 CB:47896 CB:47897.
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/dp... PS32, Line 36: binary %s not found
Failed to load %s
ditto
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/dp... PS32, Line 39: %d
Use "%zu" (or "%#zx" if you prefer hex), and no need to cast it to "int".
ditto
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/dp... PS32, Line 40: (int)
Please align with "BIOS_INFO".
ditto
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/dp... PS32, Line 43: write32(&mtk_dpm->sw_rstn, read32(&mtk_dpm->sw_rstn) | 0x10000000);
Use clrsetbits32.
Done
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/dp... PS32, Line 43: 0x10000000
Please define a macro for this.
Done
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/dp... PS32, Line 54: write32(&mtk_dpm->sw_rstn, read32(&mtk_dpm->sw_rstn) | 0x1);
Use clrsetbits32.
Done
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/dp... PS32, Line 54: 0x1
Please define a macro for this.
Done
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/in... File src/soc/mediatek/mt8192/include/soc/addressmap.h:
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/in... PS32, Line 30: AUXADC_BASE = IO_PHYS + 0x01001000, : DPM_PM_SRAM_BASE = IO_PHYS + 0x00900000, : DPM_DM_SRAM_BASE = IO_PHYS + 0x00920000, : DPM_CFG_BASE = IO_PHYS + 0x00940000,
Please sort these by values.
Done
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/in... File src/soc/mediatek/mt8192/include/soc/dpm.h:
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/in... PS32, Line 10: 0x8000
Use KiB?
unused, removed.
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/in... PS32, Line 44: #define mtk_dpm ((struct dpm_regs *)DPM_CFG_BASE)
Use global variable to be consistent with others.
Done
Xixi Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46393 )
Change subject: soc/mediatek/mt8192: Add dpm loader ......................................................................
Patch Set 33:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46393/33/src/soc/mediatek/mt8192/dp... File src/soc/mediatek/mt8192/dpm.c:
https://review.coreboot.org/c/coreboot/+/46393/33/src/soc/mediatek/mt8192/dp... PS33, Line 18: if (load_blob_file(dm_file_name, DPM_DM_SRAM_BASE)) if errors, load_blob_file function should print errors, is it?
Yidi Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46393 )
Change subject: soc/mediatek/mt8192: Add dpm loader ......................................................................
Patch Set 33:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46393/33/src/soc/mediatek/mt8192/dp... File src/soc/mediatek/mt8192/dpm.c:
https://review.coreboot.org/c/coreboot/+/46393/33/src/soc/mediatek/mt8192/dp... PS33, Line 18: if (load_blob_file(dm_file_name, DPM_DM_SRAM_BASE))
if errors, load_blob_file function should print errors, is it?
load_blob_file will print error message if failed.
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46393 )
Change subject: soc/mediatek/mt8192: Add dpm loader ......................................................................
Patch Set 33: Code-Review+2
Hello Hung-Te Lin, Xi Chen, build bot (Jenkins), Patrick Georgi, Martin Roth, Yu-Ping Wu, Duan huayang,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46393
to look at the new patch set (#34).
Change subject: soc/mediatek/mt8192: Add dpm loader ......................................................................
soc/mediatek/mt8192: Add dpm loader
BUG=none TEST=Boots correctly on Asurada
Signed-off-by: Huayang Duan huayang.duan@mediatek.com Change-Id: I16b341ad63940b45b886c4a7fd733c1970624e40 --- M src/mainboard/google/asurada/mainboard.c M src/soc/mediatek/mt8192/Makefile.inc A src/soc/mediatek/mt8192/dpm.c M src/soc/mediatek/mt8192/include/soc/addressmap.h A src/soc/mediatek/mt8192/include/soc/dpm.h 5 files changed, 103 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/46393/34
Hello Hung-Te Lin, Xi Chen, build bot (Jenkins), Patrick Georgi, Martin Roth, Yu-Ping Wu, Duan huayang,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46393
to look at the new patch set (#35).
Change subject: soc/mediatek/mt8192: Add dpm loader ......................................................................
soc/mediatek/mt8192: Add dpm loader
BUG=none TEST=Boots correctly on Asurada
Signed-off-by: Huayang Duan huayang.duan@mediatek.com Change-Id: I16b341ad63940b45b886c4a7fd733c1970624e40 --- M src/mainboard/google/asurada/mainboard.c M src/soc/mediatek/mt8192/Makefile.inc A src/soc/mediatek/mt8192/dpm.c M src/soc/mediatek/mt8192/include/soc/addressmap.h A src/soc/mediatek/mt8192/include/soc/dpm.h 5 files changed, 103 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/46393/35
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46393 )
Change subject: soc/mediatek/mt8192: Add dpm loader ......................................................................
Patch Set 35:
(2 comments)
https://review.coreboot.org/c/coreboot/+/46393/35/src/soc/mediatek/mt8192/dp... File src/soc/mediatek/mt8192/dpm.c:
https://review.coreboot.org/c/coreboot/+/46393/35/src/soc/mediatek/mt8192/dp... PS35, Line 29: ret = -1; Simply write
return -1;
here, and remove the 'ret' variable.
https://review.coreboot.org/c/coreboot/+/46393/35/src/soc/mediatek/mt8192/dp... PS35, Line 31: ret = -2; return -2 (Actually I wonder if we can also return -1 here since mtlib_load_and_run() should have printed the file name in the error message)
Hello Hung-Te Lin, Xi Chen, build bot (Jenkins), Patrick Georgi, Martin Roth, Yu-Ping Wu, Duan huayang,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46393
to look at the new patch set (#36).
Change subject: soc/mediatek/mt8192: Add dpm loader ......................................................................
soc/mediatek/mt8192: Add dpm loader
BUG=none TEST=Boots correctly on Asurada
Signed-off-by: Huayang Duan huayang.duan@mediatek.com Change-Id: I16b341ad63940b45b886c4a7fd733c1970624e40 --- M src/mainboard/google/asurada/mainboard.c M src/soc/mediatek/mt8192/Kconfig M src/soc/mediatek/mt8192/Makefile.inc A src/soc/mediatek/mt8192/dpm.c M src/soc/mediatek/mt8192/include/soc/addressmap.h A src/soc/mediatek/mt8192/include/soc/dpm.h 6 files changed, 101 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/46393/36
Hello Hung-Te Lin, Xi Chen, build bot (Jenkins), Patrick Georgi, Martin Roth, Yu-Ping Wu, Duan huayang,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46393
to look at the new patch set (#37).
Change subject: soc/mediatek/mt8192: Add dpm loader ......................................................................
soc/mediatek/mt8192: Add dpm loader
BUG=none TEST=Boots correctly on Asurada
Signed-off-by: Huayang Duan huayang.duan@mediatek.com Change-Id: I16b341ad63940b45b886c4a7fd733c1970624e40 --- M src/mainboard/google/asurada/mainboard.c M src/soc/mediatek/mt8192/Kconfig M src/soc/mediatek/mt8192/Makefile.inc A src/soc/mediatek/mt8192/dpm.c M src/soc/mediatek/mt8192/include/soc/addressmap.h A src/soc/mediatek/mt8192/include/soc/dpm.h 6 files changed, 101 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/46393/37
Yidi Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46393 )
Change subject: soc/mediatek/mt8192: Add dpm loader ......................................................................
Patch Set 37:
(2 comments)
https://review.coreboot.org/c/coreboot/+/46393/35/src/soc/mediatek/mt8192/dp... File src/soc/mediatek/mt8192/dpm.c:
https://review.coreboot.org/c/coreboot/+/46393/35/src/soc/mediatek/mt8192/dp... PS35, Line 29: ret = -1;
Simply write […]
Done
https://review.coreboot.org/c/coreboot/+/46393/35/src/soc/mediatek/mt8192/dp... PS35, Line 31: ret = -2;
return -2 (Actually I wonder if we can also return -1 here since mtlib_load_and_run() should have pr […]
Done
Hello Hung-Te Lin, Xi Chen, build bot (Jenkins), Patrick Georgi, Martin Roth, Yu-Ping Wu, Duan huayang,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46393
to look at the new patch set (#38).
Change subject: soc/mediatek/mt8192: Add dpm loader ......................................................................
soc/mediatek/mt8192: Add dpm loader
BUG=none TEST=Boots correctly on Asurada
Signed-off-by: Huayang Duan huayang.duan@mediatek.com Change-Id: I16b341ad63940b45b886c4a7fd733c1970624e40 --- M src/mainboard/google/asurada/mainboard.c M src/soc/mediatek/mt8192/Kconfig M src/soc/mediatek/mt8192/Makefile.inc A src/soc/mediatek/mt8192/dpm.c M src/soc/mediatek/mt8192/include/soc/addressmap.h A src/soc/mediatek/mt8192/include/soc/dpm.h 6 files changed, 118 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/46393/38
Hello Hung-Te Lin, Xi Chen, build bot (Jenkins), Patrick Georgi, Martin Roth, Yu-Ping Wu, Duan huayang,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46393
to look at the new patch set (#39).
Change subject: soc/mediatek/mt8192: Add dpm loader ......................................................................
soc/mediatek/mt8192: Add dpm loader
BUG=none TEST=Boots correctly on Asurada
Signed-off-by: Huayang Duan huayang.duan@mediatek.com Change-Id: I16b341ad63940b45b886c4a7fd733c1970624e40 --- M src/mainboard/google/asurada/mainboard.c M src/soc/mediatek/mt8192/Kconfig M src/soc/mediatek/mt8192/Makefile.inc A src/soc/mediatek/mt8192/dpm.c M src/soc/mediatek/mt8192/include/soc/addressmap.h A src/soc/mediatek/mt8192/include/soc/dpm.h 6 files changed, 118 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/46393/39
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46393 )
Change subject: soc/mediatek/mt8192: Add dpm loader ......................................................................
Patch Set 39:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46393/39/src/soc/mediatek/mt8192/dp... File src/soc/mediatek/mt8192/dpm.c:
https://review.coreboot.org/c/coreboot/+/46393/39/src/soc/mediatek/mt8192/dp... PS39, Line 43: else remove this 'else' to make it more clear that we expect both [0] and [1] executed.
Hello Hung-Te Lin, Xi Chen, build bot (Jenkins), Patrick Georgi, Martin Roth, Yu-Ping Wu, Duan huayang,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46393
to look at the new patch set (#40).
Change subject: soc/mediatek/mt8192: Add dpm loader ......................................................................
soc/mediatek/mt8192: Add dpm loader
BUG=none TEST=Boots correctly on Asurada
Signed-off-by: Huayang Duan huayang.duan@mediatek.com Change-Id: I16b341ad63940b45b886c4a7fd733c1970624e40 --- M src/mainboard/google/asurada/mainboard.c M src/soc/mediatek/mt8192/Kconfig M src/soc/mediatek/mt8192/Makefile.inc A src/soc/mediatek/mt8192/dpm.c M src/soc/mediatek/mt8192/include/soc/addressmap.h A src/soc/mediatek/mt8192/include/soc/dpm.h 6 files changed, 119 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/46393/40
Yidi Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46393 )
Change subject: soc/mediatek/mt8192: Add dpm loader ......................................................................
Patch Set 40:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46393/39/src/soc/mediatek/mt8192/dp... File src/soc/mediatek/mt8192/dpm.c:
https://review.coreboot.org/c/coreboot/+/46393/39/src/soc/mediatek/mt8192/dp... PS39, Line 43: else
remove this 'else' to make it more clear that we expect both [0] and [1] executed.
Done
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46393 )
Change subject: soc/mediatek/mt8192: Add dpm loader ......................................................................
Patch Set 40:
(2 comments)
https://review.coreboot.org/c/coreboot/+/46393/40//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46393/40//COMMIT_MSG@8 PS40, Line 8: Add a short description on what is DPM.
https://review.coreboot.org/c/coreboot/+/46393/40/src/soc/mediatek/mt8192/dp... File src/soc/mediatek/mt8192/dpm.c:
https://review.coreboot.org/c/coreboot/+/46393/40/src/soc/mediatek/mt8192/dp... PS40, Line 33: dpm_mcu[0].load_buffer = _dram_dma; : dpm_mcu[0].buffer_size = REGION_SIZE(dram_dma); : dpm_mcu[1].load_buffer = _dram_dma; : dpm_mcu[1].buffer_size = REGION_SIZE(dram_dma); : : /* config DPM SRAM layout */ : clrsetbits32(&mtk_dpm->sw_rstn, DPM_MEM_RATIO_MASK, DPM_MEM_RATIO_CFG1); : : if (mtlib_init_mcu(&dpm_mcu[0])) : return -1; : if (mtlib_init_mcu(&dpm_mcu[1])) : return -1; int i; struct mtlib_mcu *dpm;
/* config DPM SRAM layout */ crsetbits32(&mtk_dpm->sw_rstn, DPM_MEM_RATIO_MASK, DPM_MEM_RATIO_CFG1);
for (i = 0; i < ARRAY_SIZE(dpm); i++) { dpm = &dpm_mcu[i]; dpm->load_buffer = _dram_dma; dpm->buffer_size = REGION_SIZE(dram_dma); if (mtlib_init_mcu(dpm)) return -1; }
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46393 )
Change subject: soc/mediatek/mt8192: Add dpm loader ......................................................................
Patch Set 40:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46393/40/src/soc/mediatek/mt8192/Ma... File src/soc/mediatek/mt8192/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/46393/40/src/soc/mediatek/mt8192/Ma... PS40, Line 56: $(CONFIG_DPM_DM_FIRMWARE) \ : $(CONFIG_DPM_PM_FIRMWARE) \ : $(CONFIG_MCUPM_FIRMWARE) \ No need to put \ in same column.
Just add
$(CONFIG_DPM_DM_FIRMWARE) \ $(CONFIG_DPM_PM_FIRMWARE) \
And not touching MCUPM line.
Hello Hung-Te Lin, Xi Chen, build bot (Jenkins), Patrick Georgi, Martin Roth, Yu-Ping Wu, Duan huayang,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46393
to look at the new patch set (#41).
Change subject: soc/mediatek/mt8192: Init DPM ......................................................................
soc/mediatek/mt8192: Init DPM
DPM is a hardware module for DRAM power management and for better power saving in low power mode.
BUG=none TEST=Boots correctly on Asurada
Signed-off-by: Huayang Duan huayang.duan@mediatek.com Change-Id: I16b341ad63940b45b886c4a7fd733c1970624e40 --- M src/mainboard/google/asurada/mainboard.c M src/soc/mediatek/mt8192/Kconfig M src/soc/mediatek/mt8192/Makefile.inc A src/soc/mediatek/mt8192/dpm.c M src/soc/mediatek/mt8192/include/soc/addressmap.h A src/soc/mediatek/mt8192/include/soc/dpm.h 6 files changed, 119 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/46393/41
Yidi Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46393 )
Change subject: soc/mediatek/mt8192: Init DPM ......................................................................
Patch Set 41:
(3 comments)
https://review.coreboot.org/c/coreboot/+/46393/40//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46393/40//COMMIT_MSG@8 PS40, Line 8:
Add a short description on what is DPM.
Done
https://review.coreboot.org/c/coreboot/+/46393/40/src/soc/mediatek/mt8192/Ma... File src/soc/mediatek/mt8192/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/46393/40/src/soc/mediatek/mt8192/Ma... PS40, Line 56: $(CONFIG_DPM_DM_FIRMWARE) \ : $(CONFIG_DPM_PM_FIRMWARE) \ : $(CONFIG_MCUPM_FIRMWARE) \
No need to put \ in same column. […]
Done
https://review.coreboot.org/c/coreboot/+/46393/40/src/soc/mediatek/mt8192/dp... File src/soc/mediatek/mt8192/dpm.c:
https://review.coreboot.org/c/coreboot/+/46393/40/src/soc/mediatek/mt8192/dp... PS40, Line 33: dpm_mcu[0].load_buffer = _dram_dma; : dpm_mcu[0].buffer_size = REGION_SIZE(dram_dma); : dpm_mcu[1].load_buffer = _dram_dma; : dpm_mcu[1].buffer_size = REGION_SIZE(dram_dma); : : /* config DPM SRAM layout */ : clrsetbits32(&mtk_dpm->sw_rstn, DPM_MEM_RATIO_MASK, DPM_MEM_RATIO_CFG1); : : if (mtlib_init_mcu(&dpm_mcu[0])) : return -1; : if (mtlib_init_mcu(&dpm_mcu[1])) : return -1;
int i; […]
Done
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46393 )
Change subject: soc/mediatek/mt8192: Init DPM ......................................................................
Patch Set 41: Code-Review+2
Hello Hung-Te Lin, Xi Chen, build bot (Jenkins), Patrick Georgi, Martin Roth, Yu-Ping Wu, Duan huayang,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46393
to look at the new patch set (#42).
Change subject: soc/mediatek/mt8192: Init DPM ......................................................................
soc/mediatek/mt8192: Init DPM
DPM is a hardware module for DRAM power management and for better power saving in low power mode.
BUG=none TEST=Boots correctly on Asurada
Signed-off-by: Huayang Duan huayang.duan@mediatek.com Change-Id: I16b341ad63940b45b886c4a7fd733c1970624e40 --- M src/mainboard/google/asurada/mainboard.c M src/soc/mediatek/mt8192/Kconfig M src/soc/mediatek/mt8192/Makefile.inc A src/soc/mediatek/mt8192/dpm.c M src/soc/mediatek/mt8192/include/soc/addressmap.h A src/soc/mediatek/mt8192/include/soc/dpm.h 6 files changed, 119 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/46393/42
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46393 )
Change subject: soc/mediatek/mt8192: Init DPM ......................................................................
Patch Set 42: Code-Review+2
Hello Hung-Te Lin, Xi Chen, build bot (Jenkins), Patrick Georgi, Martin Roth, Yu-Ping Wu, Duan huayang,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46393
to look at the new patch set (#44).
Change subject: soc/mediatek/mt8192: Init DPM ......................................................................
soc/mediatek/mt8192: Init DPM
DPM is a hardware module for DRAM power management and for better power saving in low power mode.
BUG=none TEST=Boots correctly on Asurada
Signed-off-by: Huayang Duan huayang.duan@mediatek.com Change-Id: I16b341ad63940b45b886c4a7fd733c1970624e40 --- M src/mainboard/google/asurada/mainboard.c M src/soc/mediatek/mt8192/Kconfig M src/soc/mediatek/mt8192/Makefile.inc A src/soc/mediatek/mt8192/dpm.c M src/soc/mediatek/mt8192/include/soc/addressmap.h A src/soc/mediatek/mt8192/include/soc/dpm.h 6 files changed, 119 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/46393/44
Hung-Te Lin has submitted this change. ( https://review.coreboot.org/c/coreboot/+/46393 )
Change subject: soc/mediatek/mt8192: Init DPM ......................................................................
soc/mediatek/mt8192: Init DPM
DPM is a hardware module for DRAM power management and for better power saving in low power mode.
BUG=none TEST=Boots correctly on Asurada
Signed-off-by: Huayang Duan huayang.duan@mediatek.com Change-Id: I16b341ad63940b45b886c4a7fd733c1970624e40 Reviewed-on: https://review.coreboot.org/c/coreboot/+/46393 Reviewed-by: Hung-Te Lin hungte@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/google/asurada/mainboard.c M src/soc/mediatek/mt8192/Kconfig M src/soc/mediatek/mt8192/Makefile.inc A src/soc/mediatek/mt8192/dpm.c M src/soc/mediatek/mt8192/include/soc/addressmap.h A src/soc/mediatek/mt8192/include/soc/dpm.h 6 files changed, 119 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Hung-Te Lin: Looks good to me, approved
diff --git a/src/mainboard/google/asurada/mainboard.c b/src/mainboard/google/asurada/mainboard.c index 0701c67..32bd407 100644 --- a/src/mainboard/google/asurada/mainboard.c +++ b/src/mainboard/google/asurada/mainboard.c @@ -4,6 +4,7 @@ #include <console/console.h> #include <device/device.h> #include <device/mmio.h> +#include <soc/dpm.h> #include <soc/gpio.h> #include <soc/regulator.h> #include <soc/spm.h> @@ -109,6 +110,9 @@
register_reset_to_bl31();
+ if (dpm_init()) + printk(BIOS_ERR, "dpm init fail, system can't do DVFS switch\n"); + if (spm_init()) printk(BIOS_ERR, "spm init fail, system suspend may stuck\n"); } diff --git a/src/soc/mediatek/mt8192/Kconfig b/src/soc/mediatek/mt8192/Kconfig index 36ad2e3..67e5a52 100644 --- a/src/soc/mediatek/mt8192/Kconfig +++ b/src/soc/mediatek/mt8192/Kconfig @@ -45,6 +45,18 @@ This option enables memory basic compare test to verify the DRAM read or write is as expected.
+config DPM_DM_FIRMWARE + string + default "dpm.dm" + help + The file name of the MediaTek DPM DM firmware + +config DPM_PM_FIRMWARE + string + default "dpm.pm" + help + The file name of the MediaTek DPM PM firmware + config MCUPM_FIRMWARE string default "mcupm.bin" diff --git a/src/soc/mediatek/mt8192/Makefile.inc b/src/soc/mediatek/mt8192/Makefile.inc index 2fc8f39..c02eabc 100644 --- a/src/soc/mediatek/mt8192/Makefile.inc +++ b/src/soc/mediatek/mt8192/Makefile.inc @@ -35,6 +35,7 @@ romstage-y += mt6359p.c
ramstage-y += ../common/auxadc.c +ramstage-y += dpm.c ramstage-y += flash_controller.c ramstage-y += ../common/gpio.c gpio.c ramstage-y += emi.c @@ -52,6 +53,8 @@ MT8192_BLOB_DIR := 3rdparty/blobs/soc/mediatek/mt8192
mcu-firmware-files := \ + $(CONFIG_DPM_DM_FIRMWARE) \ + $(CONFIG_DPM_PM_FIRMWARE) \ $(CONFIG_MCUPM_FIRMWARE) \ $(CONFIG_SPM_FIRMWARE)
diff --git a/src/soc/mediatek/mt8192/dpm.c b/src/soc/mediatek/mt8192/dpm.c new file mode 100644 index 0000000..7acdf68 --- /dev/null +++ b/src/soc/mediatek/mt8192/dpm.c @@ -0,0 +1,48 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <console/console.h> +#include <device/mmio.h> +#include <soc/dpm.h> +#include <soc/mcu_common.h> +#include <soc/symbols.h> + +static void reset_dpm(struct mtk_mcu *mcu) +{ + /* write bootargs */ + write32(&mtk_dpm->twam_window_len, 0x0); + write32(&mtk_dpm->twam_mon_type, 0x0); + + /* free RST */ + setbits32(&mtk_dpm->sw_rstn, DPM_SW_RSTN_RESET); +} + +static struct mtk_mcu dpm_mcu[] = { + { + .firmware_name = CONFIG_DPM_DM_FIRMWARE, + .run_address = (void *)DPM_DM_SRAM_BASE, + }, + { + .firmware_name = CONFIG_DPM_PM_FIRMWARE, + .run_address = (void *)DPM_PM_SRAM_BASE, + .reset = reset_dpm, + }, +}; + +int dpm_init(void) +{ + int i; + struct mtk_mcu *dpm; + + /* config DPM SRAM layout */ + clrsetbits32(&mtk_dpm->sw_rstn, DPM_MEM_RATIO_MASK, DPM_MEM_RATIO_CFG1); + + for (i = 0; i < ARRAY_SIZE(dpm_mcu); i++) { + dpm = &dpm_mcu[i]; + dpm->load_buffer = _dram_dma; + dpm->buffer_size = REGION_SIZE(dram_dma); + if (mtk_init_mcu(dpm)) + return -1; + } + + return 0; +} diff --git a/src/soc/mediatek/mt8192/include/soc/addressmap.h b/src/soc/mediatek/mt8192/include/soc/addressmap.h index c68403b..2e8ac9e 100644 --- a/src/soc/mediatek/mt8192/include/soc/addressmap.h +++ b/src/soc/mediatek/mt8192/include/soc/addressmap.h @@ -27,6 +27,9 @@ PMIF_SPMI_BASE = IO_PHYS + 0x00027000, PMICSPI_MST_BASE = IO_PHYS + 0x00028000, SPMI_MST_BASE = IO_PHYS + 0x00029000, + DPM_PM_SRAM_BASE = IO_PHYS + 0x00900000, + DPM_DM_SRAM_BASE = IO_PHYS + 0x00920000, + DPM_CFG_BASE = IO_PHYS + 0x00940000, AUXADC_BASE = IO_PHYS + 0x01001000, UART0_BASE = IO_PHYS + 0x01002000, SPI0_BASE = IO_PHYS + 0x0100A000, diff --git a/src/soc/mediatek/mt8192/include/soc/dpm.h b/src/soc/mediatek/mt8192/include/soc/dpm.h new file mode 100644 index 0000000..f5e704b --- /dev/null +++ b/src/soc/mediatek/mt8192/include/soc/dpm.h @@ -0,0 +1,49 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef __SOC_MEDIATEK_MT8192_DPM_H__ +#define __SOC_MEDIATEK_MT8192_DPM_H__ + +#include <soc/addressmap.h> +#include <stdint.h> +#include <types.h> + +struct dpm_regs { + u32 sw_rstn; + u32 rsvd_0[3072]; + u32 mclk_div; + u32 rsvd_1[3071]; + u32 twam_window_len; + u32 twam_mon_type; + u32 rsvd_2[1022]; + u32 low_power_cfg_0; + u32 low_power_cfg_1; + u32 rsvd_3[1]; + u32 fsm_out_ctrl_0; + u32 rsvd_4[8]; + u32 fsm_cfg_1; + u32 low_power_cfg_3; + u32 dfd_dbug_0; + u32 rsvd_5[28]; + u32 status_4; +}; + +check_member(dpm_regs, mclk_div, 0x3004); +check_member(dpm_regs, twam_window_len, 0x6004); +check_member(dpm_regs, low_power_cfg_0, 0x7004); +check_member(dpm_regs, low_power_cfg_1, 0x7008); +check_member(dpm_regs, fsm_out_ctrl_0, 0x7010); +check_member(dpm_regs, fsm_cfg_1, 0x7034); +check_member(dpm_regs, low_power_cfg_3, 0x7038); +check_member(dpm_regs, dfd_dbug_0, 0x703C); +check_member(dpm_regs, status_4, 0x70B0); + +#define DPM_SW_RSTN_RESET BIT(0) +#define DPM_MEM_RATIO_OFFSET 28 +#define DPM_MEM_RATIO_MASK (0x3 << DPM_MEM_RATIO_OFFSET) +#define DPM_MEM_RATIO_CFG1 (1 << DPM_MEM_RATIO_OFFSET) + +static struct dpm_regs *const mtk_dpm = (void *)DPM_CFG_BASE; + +int dpm_init(void); + +#endif /* __SOC_MEDIATEK_MT8192_DPM_H__ */
-
build bot (Jenkins) (Code Review) -
Felix Singer (Code Review)
-
Hung-Te Lin (Code Review)
-
Paul Menzel (Code Review)
-
Xi Chen (Code Review)
-
Xixi Chen (Code Review)
-
Yidi Lin (Code Review)
-
Yu-Ping Wu (Code Review)