From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ye Li <ye.li@nxp.com>
Date: Sat, 27 Feb 2021 06:26:54 +0000
Subject: [EXT] [PATCH] ARM: imx: Add OCRAM_S into iMX8M MMU tables
In-Reply-To: <872dc185-856f-ee55-b58c-838fb1ee6593@denx.de>
References: <20210225205226.728164-1-marex@denx.de>
 <1614323728.111070.5.camel@nxp.com>
 <872dc185-856f-ee55-b58c-838fb1ee6593@denx.de>
Message-ID: <1614340964.112899.26.camel@nxp.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Marek,

On Fri, 2021-02-26 at 13:44 +0100, Marek Vasut wrote:
> Caution: EXT Email
> 
> On 2/26/21 8:15 AM, Ye Li wrote:
> > 
> > Hi Marek,
> > 
> > On Thu, 2021-02-25 at 21:52 +0100, Marek Vasut wrote:
> > > 
> > > Caution: EXT Email
> > > 
> > > The OCRAM_S is regular memory, just like the OCRAM, add it to the
> > > MMU
> > > tables so it can be used and cached.
> > > 
> > > Signed-off-by: Marek Vasut <marex@denx.de>
> > > Cc: Fabio Estevam <festevam@gmail.com>
> > > Cc: Peng Fan <peng.fan@nxp.com>
> > > Cc: Stefano Babic <sbabic@denx.de>
> > > ---
> > > ? arch/arm/mach-imx/imx8m/soc.c | 7 +++++++
> > > ? 1 file changed, 7 insertions(+)
> > > 
> > > diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-
> > > imx/imx8m/soc.c
> > > index 5456c10fb17..225e4e12500 100644
> > > --- a/arch/arm/mach-imx/imx8m/soc.c
> > > +++ b/arch/arm/mach-imx/imx8m/soc.c
> > > @@ -104,6 +104,13 @@ static struct mm_region imx8m_mem_map[] = {
> > > ?????????????????.attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
> > > ??????????????????????????PTE_BLOCK_NON_SHARE |
> > > ??????????????????????????PTE_BLOCK_PXN | PTE_BLOCK_UXN
> > > +???????}, {
> > > +???????????????/* OCRAM_S */
> > > +???????????????.virt = 0x180000UL,
> > > +???????????????.phys = 0x180000UL,
> > > +???????????????.size = 0x8000UL,
> > > +???????????????.attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
> > > +????????????????????????PTE_BLOCK_OUTER_SHARE
> > > ?????????}, {
> > > ?????????????????/* TCM */
> > > ?????????????????.virt = 0x7C0000UL,
> > > --
> > > 2.30.0
> > > 
> > OCRAM_S is used by ATF and SPL to pass DDR CSR data.
> Where is this implemented ?

See below definition in?drivers/ddr/imx/imx8m/Kconfig

config SAVED_DRAM_TIMING_BASE
	hex "Define the base address for saved dram timing"
	help
	??after DRAM is trained, need to save the dram related timming
	??info into memory for low power use. OCRAM_S is used for this
	??purpose on i.MX8MM.
	default 0x180000
> 
> > 
> > It is better not
> > use it in u-boot to avoid any DDR issue.
> The MMU table entry does not trigger any IO to the OCRAM_S , it
> merely
> makes it cacheable .
> 
That's fine to add a map, just remind to use it carefully since it
already used by ATF.

> > 
> > And this imx8m_mem_map will be modified at runtime to get rid of
> > optee
> > memory. When OCRAM_S is added, the index used in enable_caches and
> > dram_init need update as well.
> I'm not sure I understand this. What kind of modification are you
> talking about ? The DRAM entry offset should be determined
> automatically, so there shouldn't be any need to hand-tune ad-hoc
> offsets.

You also need below change, the index for DRAM1 is used in codes to
help remove the OPTEE space from MMU table.

--- a/arch/arm/mach-imx/imx8m/soc.c
+++ b/arch/arm/mach-imx/imx8m/soc.c
@@ -167,10 +167,10 @@ void enable_caches(void)
?????????????????* please make sure that entry initial value matches
?????????????????* imx8m_mem_map for DRAM1
?????????????????*/
-???????????????int entry = 5;
+???????????????int entry = 6;
????????????????u64 attrs = imx8m_mem_map[entry].attrs;
?
-???????????????while (i < CONFIG_NR_DRAM_BANKS && entry < 8) {
+???????????????while (i < CONFIG_NR_DRAM_BANKS && entry < 9) {
????????????????????????if (gd->bd->bi_dram[i].start == 0)
????????????????????????????????break;
????????????????????????imx8m_mem_map[entry].phys = gd->bd-
>bi_dram[i].start;
@@ -212,7 +212,7 @@ int dram_init(void)
????????????????gd->ram_size = sdram_size;
?
????????/* also update the SDRAM size in the mem_map used externally */
-???????imx8m_mem_map[5].size = sdram_size;
+???????imx8m_mem_map[6].size = sdram_size;
?
?#ifdef PHYS_SDRAM_2_SIZE
????????gd->ram_size += PHYS_SDRAM_2_SIZE;

Best regards,
Ye Li