On 04/02/2018 05:50 AM, Philipp Tomsich wrote:
On Tue, 27 Mar 2018, Kever Yang wrote:
dram_init_banksize() can be common used by all SoCs, move it into sdram_common.c
Signed-off-by: Kever Yang kever.yang@rock-chips.com
arch/arm/mach-rockchip/sdram_common.c | 63 ++++++++++++++++++++++++++++++++++- 1 file changed, 62 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-rockchip/sdram_common.c b/arch/arm/mach-rockchip/sdram_common.c index 3a71f09..ff86096 100644 --- a/arch/arm/mach-rockchip/sdram_common.c +++ b/arch/arm/mach-rockchip/sdram_common.c @@ -21,13 +21,74 @@ struct ddr_param {
#define PARAM_DRAM_INFO_OFFSET 0x2000000
+#define TRUST_PARAMETER_OFFSET (34 * 1024 * 1024)
This isn't covered by what you commit message states as content for this patch.
Will add it.
+struct tos_parameter_t { + u32 version; + u32 checksum; + struct { + char name[8]; + s64 phy_addr; + u32 size; + u32 flags; + } tee_mem; + struct { + char name[8]; + s64 phy_addr; + u32 size; + u32 flags; + } drm_mem; + s64 reserve[8]; +};
+int dram_init_banksize(void) +{ + size_t top = min((unsigned long)(gd->ram_size + CONFIG_SYS_SDRAM_BASE), + gd->ram_top);
+#ifdef CONFIG_ARM64 + /* Reserve 0x200000 for ATF bl31 */ + gd->bd->bi_dram[0].start = 0x200000; + gd->bd->bi_dram[0].size = top - gd->bd->bi_dram[0].start;
This should only be done when preparing to start a IH_OS_ARM_TRUSTED_FIRMWARE.
Yes, you are right, but ATF in ARM64 is not optional, it's mandatory, so I don't think we need a #if/#else here.
+#else +#ifdef CONFIG_SPL_OPTEE
I don't think that this CONFIG_SPL_OPTEE was what the comments in the OPTEE thread have arrived at... please check again.
Will update and use new MACRO, I leave it there because you have such a sloooow response and then a looong time discussion.
Thanks, - Kever
Just as an unreleated comment/reminder: you still need to revise the other series as per the comments and final decision on how to implement it.
+ struct tos_parameter_t *tos_parameter;
+ tos_parameter = (struct tos_parameter_t *)(CONFIG_SYS_SDRAM_BASE + + TRUST_PARAMETER_OFFSET);
+ if (tos_parameter->tee_mem.flags == 1) {
Please describe what this 'flags' member means and how it's encoded.
+ gd->bd->bi_dram[0].start = CONFIG_SYS_SDRAM_BASE; + gd->bd->bi_dram[0].size = tos_parameter->tee_mem.phy_addr + - CONFIG_SYS_SDRAM_BASE; + gd->bd->bi_dram[1].start = tos_parameter->tee_mem.phy_addr + + tos_parameter->tee_mem.size; + gd->bd->bi_dram[1].size = gd->bd->bi_dram[0].start + + top - gd->bd->bi_dram[1].start; + } else { + gd->bd->bi_dram[0].start = CONFIG_SYS_SDRAM_BASE; + gd->bd->bi_dram[0].size = 0x8400000; + /* Reserve 32M for OPTEE with TA */ + gd->bd->bi_dram[1].start = CONFIG_SYS_SDRAM_BASE + + gd->bd->bi_dram[0].size + 0x2000000; + gd->bd->bi_dram[1].size = gd->bd->bi_dram[0].start + + top - gd->bd->bi_dram[1].start; + }
This should again only be done, when the appropriate IH_OS_* is entered.
+#else + gd->bd->bi_dram[0].start = CONFIG_SYS_SDRAM_BASE; + gd->bd->bi_dram[0].size = top - gd->bd->bi_dram[0].start;
This should be the default (so you can remove the #if paths) once you make sure that the other paths are actually called for entering the particular IH_OS_* types.
+#endif +#endif
+ return 0; +}
size_t rockchip_sdram_size(phys_addr_t reg) { u32 rank, col, bk, cs0_row, cs1_row, bw, row_3_4; size_t chipsize_mb = 0; size_t size_mb = 0; u32 ch;
Ok, but not really needed (unless you want to do a separate 'cosmetic' or 'whitespace' patch. Touching an unrelated function is usually a bad idea.
u32 sys_reg = readl(reg); u32 ch_num = 1 + ((sys_reg >> SYS_REG_NUM_CH_SHIFT) & SYS_REG_NUM_CH_MASK);