| Message ID | 1522142971-20739-18-git-send-email-kever.yang@rock-chips.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Philipp Tomsich |
| Headers | show |
| Series | rockchip: clean up board file for rockchip SoCs | expand |
> 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(-) > Acked-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
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. > + > +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. > +#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. 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); >
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); >> >
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) + +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; +#else +#ifdef CONFIG_SPL_OPTEE + 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) { + 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; + } +#else + gd->bd->bi_dram[0].start = CONFIG_SYS_SDRAM_BASE; + gd->bd->bi_dram[0].size = top - gd->bd->bi_dram[0].start; +#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; - u32 sys_reg = readl(reg); u32 ch_num = 1 + ((sys_reg >> SYS_REG_NUM_CH_SHIFT) & SYS_REG_NUM_CH_MASK);
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(-)