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.y...@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); >> > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot