All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ARM: imx: Add OCRAM_S into iMX8M MMU tables
@ 2021-02-25 20:52 Marek Vasut
  2021-02-26  7:15 ` [EXT] " Ye Li
  2021-04-08 20:58 ` sbabic at denx.de
  0 siblings, 2 replies; 6+ messages in thread
From: Marek Vasut @ 2021-02-25 20:52 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [EXT] [PATCH] ARM: imx: Add OCRAM_S into iMX8M MMU tables
  2021-02-25 20:52 [PATCH] ARM: imx: Add OCRAM_S into iMX8M MMU tables Marek Vasut
@ 2021-02-26  7:15 ` Ye Li
  2021-02-26 12:44   ` Marek Vasut
  2021-04-08 20:58 ` sbabic at denx.de
  1 sibling, 1 reply; 6+ messages in thread
From: Ye Li @ 2021-02-26  7:15 UTC (permalink / raw)
  To: u-boot

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. It is better not?
use it in u-boot to avoid any DDR issue.
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.

Best regards,
Ye Li

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [EXT] [PATCH] ARM: imx: Add OCRAM_S into iMX8M MMU tables
  2021-02-26  7:15 ` [EXT] " Ye Li
@ 2021-02-26 12:44   ` Marek Vasut
  2021-02-27  6:26     ` Ye Li
  0 siblings, 1 reply; 6+ messages in thread
From: Marek Vasut @ 2021-02-26 12:44 UTC (permalink / raw)
  To: u-boot

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 ?

> 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 .

> 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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [EXT] [PATCH] ARM: imx: Add OCRAM_S into iMX8M MMU tables
  2021-02-26 12:44   ` Marek Vasut
@ 2021-02-27  6:26     ` Ye Li
  2021-02-27 12:57       ` Marek Vasut
  0 siblings, 1 reply; 6+ messages in thread
From: Ye Li @ 2021-02-27  6:26 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [EXT] [PATCH] ARM: imx: Add OCRAM_S into iMX8M MMU tables
  2021-02-27  6:26     ` Ye Li
@ 2021-02-27 12:57       ` Marek Vasut
  0 siblings, 0 replies; 6+ messages in thread
From: Marek Vasut @ 2021-02-27 12:57 UTC (permalink / raw)
  To: u-boot

On 2/27/21 7:26 AM, Ye Li wrote:
> Hi Marek,

Hi,

[...]
>>>> 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

So, this just defines some sort of value. Note that some boards do 
override this value to NOT point into OCRAM_S .

I think what you wanted to point me to is drivers/ddr/imx/imx8m/ddr_init.c:
248         dram_config_save(dram_timing, CONFIG_SAVED_DRAM_TIMING_BASE);
which writes regular memory. And that regular memory can very well be 
cacheable.

>>> 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.

Not necessarily, use git grep SAVED_DRAM_TIMING_BASE configs/ and see 
how some boards change the default in configs/ .

>>> 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;

OK, then this hard-coding of random offset is nasty and should be fixed.
For example iterate over imx8m_mem_map[] and detect entry which has PA 
 >= 0x40000000 , that's your DRAM.

>  ????????????????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;

[...]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] ARM: imx: Add OCRAM_S into iMX8M MMU tables
  2021-02-25 20:52 [PATCH] ARM: imx: Add OCRAM_S into iMX8M MMU tables Marek Vasut
  2021-02-26  7:15 ` [EXT] " Ye Li
@ 2021-04-08 20:58 ` sbabic at denx.de
  1 sibling, 0 replies; 6+ messages in thread
From: sbabic at denx.de @ 2021-04-08 20:58 UTC (permalink / raw)
  To: u-boot

> 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>
Applied to u-boot-imx, master, thanks !

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-04-08 20:58 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-25 20:52 [PATCH] ARM: imx: Add OCRAM_S into iMX8M MMU tables Marek Vasut
2021-02-26  7:15 ` [EXT] " Ye Li
2021-02-26 12:44   ` Marek Vasut
2021-02-27  6:26     ` Ye Li
2021-02-27 12:57       ` Marek Vasut
2021-04-08 20:58 ` sbabic at denx.de

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.