[PATCH] arch: arm: mach-k3: Delete tifs node in DT fixup

[PATCH] arch: arm: mach-k3: Delete tifs node in DT fixup

[Udit Kumar]

This patch deletes tifs DT node as part of fixup.

TISCI API reported msmc_size, does not include
64KB reserved size for tifs aka MSMC comms memory.

As part of fixup, original code uses TISCI API
reported msmc_size as size for sram DT node.

tifs node is similar to l3-cache, which should
hold address above msms_size, and should be deleted
before passing control to OS.

Documentation
https://software-dl.ti.com/tisci/esd/latest/2_tisci_msgs/general/core.html?highlight=msmc#tisci-msg-query-msmc

Signed-off-by: Udit Kumar <u-kumar1@ti.com>
---
 arch/arm/mach-k3/common.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-k3/common.c b/arch/arm/mach-k3/common.c
index a2adb791f6..4651744821 100644
--- a/arch/arm/mach-k3/common.c
+++ b/arch/arm/mach-k3/common.c
@@ -349,9 +349,11 @@ int fdt_fixup_msmc_ram(void *blob, char *parent_path, char *node_name)
 		size = fdt_read_number(sub_reg, 1);
 		debug("%s: subnode = %d, addr = 0x%x. size = 0x%x\n", __func__,
 		      subnode, addr, size);
+
 		if (addr + size > msmc_size ||
 		    !strncmp(fdt_get_name(blob, subnode, &len), "sysfw", 5) ||
-		    !strncmp(fdt_get_name(blob, subnode, &len), "l3cache", 7)) {
+		    !strncmp(fdt_get_name(blob, subnode, &len), "l3cache", 7) ||
+		    !strncmp(fdt_get_name(blob, subnode, &len), "tifs", 4)) {
 			fdt_del_node(blob, subnode);
 			debug("%s: deleting subnode %d\n", __func__, subnode);
 			if (!prev_node)
-- 
2.34.1

Re: [PATCH] arch: arm: mach-k3: Delete tifs node in DT fixup

[Nishanth Menon]

On 11:43-20230419, Udit Kumar wrote:
> This patch deletes tifs DT node as part of fixup.
> 
> TISCI API reported msmc_size, does not include
> 64KB reserved size for tifs aka MSMC comms memory.
> 
> As part of fixup, original code uses TISCI API
> reported msmc_size as size for sram DT node.
> 
> tifs node is similar to l3-cache, which should
> hold address above msms_size, and should be deleted
> before passing control to OS.
> 
> Documentation
> https://software-dl.ti.com/tisci/esd/latest/2_tisci_msgs/general/core.html?highlight=msmc#tisci-msg-query-msmc
> 
> Signed-off-by: Udit Kumar <u-kumar1@ti.com>
> ---
>  arch/arm/mach-k3/common.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-k3/common.c b/arch/arm/mach-k3/common.c
> index a2adb791f6..4651744821 100644
> --- a/arch/arm/mach-k3/common.c
> +++ b/arch/arm/mach-k3/common.c
> @@ -349,9 +349,11 @@ int fdt_fixup_msmc_ram(void *blob, char *parent_path, char *node_name)
>  		size = fdt_read_number(sub_reg, 1);
>  		debug("%s: subnode = %d, addr = 0x%x. size = 0x%x\n", __func__,
>  		      subnode, addr, size);
> +
>  		if (addr + size > msmc_size ||
>  		    !strncmp(fdt_get_name(blob, subnode, &len), "sysfw", 5) ||
> -		    !strncmp(fdt_get_name(blob, subnode, &len), "l3cache", 7)) {
> +		    !strncmp(fdt_get_name(blob, subnode, &len), "l3cache", 7) ||
> +		    !strncmp(fdt_get_name(blob, subnode, &len), "tifs", 4)) {
I would probably move the tifs check above the l3cache check.

>  			fdt_del_node(blob, subnode);
>  			debug("%s: deleting subnode %d\n", __func__, subnode);
>  			if (!prev_node)
> -- 
> 2.34.1
> 

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

Re: [PATCH] arch: arm: mach-k3: Delete tifs node in DT fixup

[Kumar, Udit]

Thanks for review

On 4/19/2023 9:22 PM, Nishanth Menon wrote:
> On 11:43-20230419, Udit Kumar wrote:
>> This patch deletes tifs DT node as part of fixup.
>>
>> TISCI API reported msmc_size, does not include
>> 64KB reserved size for tifs aka MSMC comms memory.
>>
>> As part of fixup, original code uses TISCI API
>> reported msmc_size as size for sram DT node.
>>
>> tifs node is similar to l3-cache, which should
>> hold address above msms_size, and should be deleted
>> before passing control to OS.
>>
>> Documentation
>> https://software-dl.ti.com/tisci/esd/latest/2_tisci_msgs/general/core.html?highlight=msmc#tisci-msg-query-msmc
>>
>> Signed-off-by: Udit Kumar <u-kumar1@ti.com>
>> ---
>>   arch/arm/mach-k3/common.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/mach-k3/common.c b/arch/arm/mach-k3/common.c
>> index a2adb791f6..4651744821 100644
>> --- a/arch/arm/mach-k3/common.c
>> +++ b/arch/arm/mach-k3/common.c
>> @@ -349,9 +349,11 @@ int fdt_fixup_msmc_ram(void *blob, char *parent_path, char *node_name)
>>   		size = fdt_read_number(sub_reg, 1);
>>   		debug("%s: subnode = %d, addr = 0x%x. size = 0x%x\n", __func__,
>>   		      subnode, addr, size);
>> +
>>   		if (addr + size > msmc_size ||
>>   		    !strncmp(fdt_get_name(blob, subnode, &len), "sysfw", 5) ||
>> -		    !strncmp(fdt_get_name(blob, subnode, &len), "l3cache", 7)) {
>> +		    !strncmp(fdt_get_name(blob, subnode, &len), "l3cache", 7) ||
>> +		    !strncmp(fdt_get_name(blob, subnode, &len), "tifs", 4)) {
> I would probably move the tifs check above the l3cache check.

I will move this in v2.

you want to stay close to DT nodes order. or there is some other reason ?


>
>>   			fdt_del_node(blob, subnode);
>>   			debug("%s: deleting subnode %d\n", __func__, subnode);
>>   			if (!prev_node)
>> -- 
>> 2.34.1
>>

[PATCH v2] arch: arm: mach-k3: Delete tifs node in DT fixup

[Udit Kumar]

This patch deletes tifs DT node as part of fixup.

TISCI API reported msmc_size, does not include
64KB reserved size for tifs aka MSMC comms memory.

As part of fixup, original code uses TISCI API
reported msmc_size as size for sram DT node.

tifs node is similar to l3-cache, which should
hold address above msms_size, and should be deleted
before passing control to OS.

Documentation
https://software-dl.ti.com/tisci/esd/latest/2_tisci_msgs/general/core.html?highlight=msmc#tisci-msg-query-msmc

Signed-off-by: Udit Kumar <u-kumar1@ti.com>
---
Changes since v1:
https://lore.kernel.org/all/20230419061352.3156023-1-u-kumar1@ti.com/
- moved tifs check above l3 as

 arch/arm/mach-k3/common.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-k3/common.c b/arch/arm/mach-k3/common.c
index a2adb791f6..33b1f10d58 100644
--- a/arch/arm/mach-k3/common.c
+++ b/arch/arm/mach-k3/common.c
@@ -351,6 +351,7 @@ int fdt_fixup_msmc_ram(void *blob, char *parent_path, char *node_name)
 		      subnode, addr, size);
 		if (addr + size > msmc_size ||
 		    !strncmp(fdt_get_name(blob, subnode, &len), "sysfw", 5) ||
+		    !strncmp(fdt_get_name(blob, subnode, &len), "tifs", 4)  ||
 		    !strncmp(fdt_get_name(blob, subnode, &len), "l3cache", 7)) {
 			fdt_del_node(blob, subnode);
 			debug("%s: deleting subnode %d\n", __func__, subnode);
-- 
2.34.1

Re: [PATCH v2] arch: arm: mach-k3: Delete tifs node in DT fixup

[Neha Malcom Francis]

Hi Udit

On 20/04/23 13:41, Udit Kumar wrote:
> This patch deletes tifs DT node as part of fixup.
> 
> TISCI API reported msmc_size, does not include
> 64KB reserved size for tifs aka MSMC comms memory.
> 
> As part of fixup, original code uses TISCI API
> reported msmc_size as size for sram DT node.
> 
> tifs node is similar to l3-cache, which should
> hold address above msms_size, and should be deleted
> before passing control to OS.
> 
> Documentation
> https://software-dl.ti.com/tisci/esd/latest/2_tisci_msgs/general/core.html?highlight=msmc#tisci-msg-query-msmc
> 
> Signed-off-by: Udit Kumar <u-kumar1@ti.com>
> ---
> Changes since v1:
> https://lore.kernel.org/all/20230419061352.3156023-1-u-kumar1@ti.com/
> - moved tifs check above l3 as
> 
>   arch/arm/mach-k3/common.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/mach-k3/common.c b/arch/arm/mach-k3/common.c
> index a2adb791f6..33b1f10d58 100644
> --- a/arch/arm/mach-k3/common.c
> +++ b/arch/arm/mach-k3/common.c
> @@ -351,6 +351,7 @@ int fdt_fixup_msmc_ram(void *blob, char *parent_path, char *node_name)
>   		      subnode, addr, size);
>   		if (addr + size > msmc_size ||
>   		    !strncmp(fdt_get_name(blob, subnode, &len), "sysfw", 5) ||
> +		    !strncmp(fdt_get_name(blob, subnode, &len), "tifs", 4)  ||
>   		    !strncmp(fdt_get_name(blob, subnode, &len), "l3cache", 7)) {
>   			fdt_del_node(blob, subnode);
>   			debug("%s: deleting subnode %d\n", __func__, subnode);

Reviewed-by: Neha Malcom Francis <n-francis@ti.com>

I do have a general doubt; why do we have only atf-sram sub-node in 
msmc_sram in all other devices (j721e, j7200 and am65) except j721s2?

-- 
Thanking You
Neha Malcom Francis

Re: [PATCH v2] arch: arm: mach-k3: Delete tifs node in DT fixup

[Kumar, Udit]

Hi Neha

On 4/26/2023 2:56 PM, Neha Malcom Francis wrote:
> Hi Udit
>
> On 20/04/23 13:41, Udit Kumar wrote:
>> This patch deletes tifs DT node as part of fixup.
>>
>> TISCI API reported msmc_size, does not include
>> 64KB reserved size for tifs aka MSMC comms memory.
>>
>> As part of fixup, original code uses TISCI API
>> reported msmc_size as size for sram DT node.
>>
>> tifs node is similar to l3-cache, which should
>> hold address above msms_size, and should be deleted
>> before passing control to OS.
>>
>> Documentation
>> https://software-dl.ti.com/tisci/esd/latest/2_tisci_msgs/general/core.html?highlight=msmc#tisci-msg-query-msmc 
>>
>>
>> Signed-off-by: Udit Kumar <u-kumar1@ti.com>
>> ---
>> Changes since v1:
>> https://lore.kernel.org/all/20230419061352.3156023-1-u-kumar1@ti.com/
>> - moved tifs check above l3 as
>>
>>   arch/arm/mach-k3/common.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm/mach-k3/common.c b/arch/arm/mach-k3/common.c
>> index a2adb791f6..33b1f10d58 100644
>> --- a/arch/arm/mach-k3/common.c
>> +++ b/arch/arm/mach-k3/common.c
>> @@ -351,6 +351,7 @@ int fdt_fixup_msmc_ram(void *blob, char 
>> *parent_path, char *node_name)
>>                 subnode, addr, size);
>>           if (addr + size > msmc_size ||
>>               !strncmp(fdt_get_name(blob, subnode, &len), "sysfw", 5) ||
>> +            !strncmp(fdt_get_name(blob, subnode, &len), "tifs", 4)  ||
>>               !strncmp(fdt_get_name(blob, subnode, &len), "l3cache", 
>> 7)) {
>>               fdt_del_node(blob, subnode);
>>               debug("%s: deleting subnode %d\n", __func__, subnode);
>
> Reviewed-by: Neha Malcom Francis <n-francis@ti.com>
>
> I do have a general doubt; why do we have only atf-sram sub-node in 
> msmc_sram in all other devices (j721e, j7200 and am65) except j721s2?

let me know, which source code you are referring to

I could see for j721s2 as well, in uboot[0] and Linux[1]

[0]

https://elixir.bootlin.com/linux/latest/source/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi#L16 


[1]

https://elixir.bootlin.com/linux/latest/source/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi#L16 

Re: [PATCH v2] arch: arm: mach-k3: Delete tifs node in DT fixup

[Neha Malcom Francis]

Hi Udit,

On 26/04/23 15:35, Kumar, Udit wrote:
> Hi Neha
> 
> On 4/26/2023 2:56 PM, Neha Malcom Francis wrote:
>> Hi Udit
>>
>> On 20/04/23 13:41, Udit Kumar wrote:
>>> This patch deletes tifs DT node as part of fixup.
>>>
>>> TISCI API reported msmc_size, does not include
>>> 64KB reserved size for tifs aka MSMC comms memory.
>>>
>>> As part of fixup, original code uses TISCI API
>>> reported msmc_size as size for sram DT node.
>>>
>>> tifs node is similar to l3-cache, which should
>>> hold address above msms_size, and should be deleted
>>> before passing control to OS.
>>>
>>> Documentation
>>> https://software-dl.ti.com/tisci/esd/latest/2_tisci_msgs/general/core.html?highlight=msmc#tisci-msg-query-msmc
>>>
>>> Signed-off-by: Udit Kumar <u-kumar1@ti.com>
>>> ---
>>> Changes since v1:
>>> https://lore.kernel.org/all/20230419061352.3156023-1-u-kumar1@ti.com/
>>> - moved tifs check above l3 as
>>>
>>>   arch/arm/mach-k3/common.c | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/arch/arm/mach-k3/common.c b/arch/arm/mach-k3/common.c
>>> index a2adb791f6..33b1f10d58 100644
>>> --- a/arch/arm/mach-k3/common.c
>>> +++ b/arch/arm/mach-k3/common.c
>>> @@ -351,6 +351,7 @@ int fdt_fixup_msmc_ram(void *blob, char 
>>> *parent_path, char *node_name)
>>>                 subnode, addr, size);
>>>           if (addr + size > msmc_size ||
>>>               !strncmp(fdt_get_name(blob, subnode, &len), "sysfw", 5) ||
>>> +            !strncmp(fdt_get_name(blob, subnode, &len), "tifs", 4)  ||
>>>               !strncmp(fdt_get_name(blob, subnode, &len), "l3cache", 
>>> 7)) {
>>>               fdt_del_node(blob, subnode);
>>>               debug("%s: deleting subnode %d\n", __func__, subnode);
>>
>> Reviewed-by: Neha Malcom Francis <n-francis@ti.com>
>>
>> I do have a general doubt; why do we have only atf-sram sub-node in 
>> msmc_sram in all other devices (j721e, j7200 and am65) except j721s2?
> 
> let me know, which source code you are referring to
> 

In U-Boot, for j721e, j7200 and am65; they *only* contain atf-sram?

> I could see for j721s2 as well, in uboot[0] and Linux[1]
> 
> [0]
> 
> https://elixir.bootlin.com/linux/latest/source/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi#L16
> 
> [1]
> 
> https://elixir.bootlin.com/linux/latest/source/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi#L16
> 
> 

-- 
Thanking You
Neha Malcom Francis

RE: [PATCH v2] arch: arm: mach-k3: Delete tifs node in DT fixup

[Kumar, Udit]

Hi Neha, 

> Hi Udit,

[..]

>>>
>>> I do have a general doubt; why do we have only atf-sram sub-node in
>>> msmc_sram in all other devices (j721e, j7200 and am65) except j721s2?
>>
>> let me know, which source code you are referring to
>>
>
>In U-Boot, for j721e, j7200 and am65; they *only* contain atf-sram?

For u-boot please see 

https://elixir.bootlin.com/u-boot/latest/source/arch/arm/dts/k3-j721s2-main.dtsi#L16 

>> I could see for j721s2 as well, in uboot[0] and Linux[1]
[..]

>Thanking You
>Neha Malcom Francis

Re: [PATCH v2] arch: arm: mach-k3: Delete tifs node in DT fixup

[Neha Malcom Francis]

Hi Udit

On 26/04/23 16:09, Kumar, Udit wrote:
> Hi Neha,
> 
>> Hi Udit,
> 
> [..]
> 
>>>>
>>>> I do have a general doubt; why do we have only atf-sram sub-node in
>>>> msmc_sram in all other devices (j721e, j7200 and am65) except j721s2?
>>>
>>> let me know, which source code you are referring to
>>>
>>
>> In U-Boot, for j721e, j7200 and am65; they *only* contain atf-sram?
> 
> For u-boot please see
> 
> https://elixir.bootlin.com/u-boot/latest/source/arch/arm/dts/k3-j721s2-main.dtsi#L16
> 
>>> I could see for j721s2 as well, in uboot[0] and Linux[1]
> [..]
> 

What I mean to ask is, why aren't there tifs or l3cache subnodes in 
j721e, j7200 and am65?

>> Thanking You
>> Neha Malcom Francis

-- 
Thanking You
Neha Malcom Francis

Re: [PATCH v2] arch: arm: mach-k3: Delete tifs node in DT fixup

[Nishanth Menon]

On 17:31-20230426, Neha Malcom Francis wrote:
> Hi Udit
> 
> On 26/04/23 16:09, Kumar, Udit wrote:
> > Hi Neha,
> > 
> > > Hi Udit,
> > 
> > [..]
> > 
> > > > > 
> > > > > I do have a general doubt; why do we have only atf-sram sub-node in
> > > > > msmc_sram in all other devices (j721e, j7200 and am65) except j721s2?
> > > > 
> > > > let me know, which source code you are referring to
> > > > 
> > > 
> > > In U-Boot, for j721e, j7200 and am65; they *only* contain atf-sram?
> > 
> > For u-boot please see
> > 
> > https://elixir.bootlin.com/u-boot/latest/source/arch/arm/dts/k3-j721s2-main.dtsi#L16
> > 
> > > > I could see for j721s2 as well, in uboot[0] and Linux[1]
> > [..]
> > 
> 
> What I mean to ask is, why aren't there tifs or l3cache subnodes in j721e,
> j7200 and am65?
> 

am65x has it
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/ti/k3-am65-main.dtsi#n17

J7200 and j721e is missing it
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi#n27
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi#n15


we need to fix it.
-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

Re: [PATCH v2] arch: arm: mach-k3: Delete tifs node in DT fixup

[Kumar, Udit]

Hi Neha,

On 4/26/2023 5:31 PM, Neha Malcom Francis wrote:
> Hi Udit
>
> On 26/04/23 16:09, Kumar, Udit wrote:
>> Hi Neha,
>>
>>> Hi Udit,
>>
>> [..]
>>
>>>>>
>>>>> I do have a general doubt; why do we have only atf-sram sub-node in
>>>>> msmc_sram in all other devices (j721e, j7200 and am65) except j721s2?
>>>>
>>>> let me know, which source code you are referring to
>>>>
>>>
>>> In U-Boot, for j721e, j7200 and am65; they *only* contain atf-sram?
>>
>> For u-boot please see
>>
>> https://elixir.bootlin.com/u-boot/latest/source/arch/arm/dts/k3-j721s2-main.dtsi#L16 
>>
>>
>>>> I could see for j721s2 as well, in uboot[0] and Linux[1]
>> [..]
>>
>
> What I mean to ask is, why aren't there tifs or l3cache subnodes in 
> j721e, j7200 and am65?
>
I think,  above platform is doing in right way,

AFAIK,  if we have to provide then we can provide size of this.

l3-cache can not be addressable.

But in any case, u-boot removes this code before passing to OS.

https://elixir.bootlin.com/u-boot/latest/source/arch/arm/mach-k3/common.c#L354 


>>> Thanking You
>>> Neha Malcom Francis
>

Re: [PATCH v2] arch: arm: mach-k3: Delete tifs node in DT fixup

[Andrew Davis]

On 4/26/23 9:13 AM, Kumar, Udit wrote:
> Hi Neha,
> 
> On 4/26/2023 5:31 PM, Neha Malcom Francis wrote:
>> Hi Udit
>>
>> On 26/04/23 16:09, Kumar, Udit wrote:
>>> Hi Neha,
>>>
>>>> Hi Udit,
>>>
>>> [..]
>>>
>>>>>>
>>>>>> I do have a general doubt; why do we have only atf-sram sub-node in
>>>>>> msmc_sram in all other devices (j721e, j7200 and am65) except j721s2?
>>>>>
>>>>> let me know, which source code you are referring to
>>>>>
>>>>
>>>> In U-Boot, for j721e, j7200 and am65; they *only* contain atf-sram?
>>>
>>> For u-boot please see
>>>
>>> https://elixir.bootlin.com/u-boot/latest/source/arch/arm/dts/k3-j721s2-main.dtsi#L16
>>>
>>>>> I could see for j721s2 as well, in uboot[0] and Linux[1]
>>> [..]
>>>
>>
>> What I mean to ask is, why aren't there tifs or l3cache subnodes in j721e, j7200 and am65?
>>
> I think,  above platform is doing in right way,
> 
> AFAIK,  if we have to provide then we can provide size of this.
> 
> l3-cache can not be addressable.
> 


So the history here is we used to have the SRAM node in DT sized
to the actual size in hardware. L3 cache size can be set at boot
time (in SYSFW board-config file), and that uses up some of the
SRAM, so the end address moves in. We could represent this as
a reserved node inside the full SRAM node, or by shrinking the
SRAM node and hiding this. Same story for TIFS and ATF, they
use some variable amount of the end of SRAM.

I'd prefer being explicit and keep these nodes.

Andrew


> But in any case, u-boot removes this code before passing to OS.
> 
> https://elixir.bootlin.com/u-boot/latest/source/arch/arm/mach-k3/common.c#L354
> 
>>>> Thanking You
>>>> Neha Malcom Francis
>>

Re: [PATCH v2] arch: arm: mach-k3: Delete tifs node in DT fixup

[Neha Malcom Francis]

Hi Andrew

On 01/05/23 20:16, Andrew Davis wrote:
> On 4/26/23 9:13 AM, Kumar, Udit wrote:
>> Hi Neha,
>>
>> On 4/26/2023 5:31 PM, Neha Malcom Francis wrote:
>>> Hi Udit
>>>
>>> On 26/04/23 16:09, Kumar, Udit wrote:
>>>> Hi Neha,
>>>>
>>>>> Hi Udit,
>>>>
>>>> [..]
>>>>
>>>>>>>
>>>>>>> I do have a general doubt; why do we have only atf-sram sub-node in
>>>>>>> msmc_sram in all other devices (j721e, j7200 and am65) except 
>>>>>>> j721s2?
>>>>>>
>>>>>> let me know, which source code you are referring to
>>>>>>
>>>>>
>>>>> In U-Boot, for j721e, j7200 and am65; they *only* contain atf-sram?
>>>>
>>>> For u-boot please see
>>>>
>>>> https://elixir.bootlin.com/u-boot/latest/source/arch/arm/dts/k3-j721s2-main.dtsi#L16
>>>>
>>>>>> I could see for j721s2 as well, in uboot[0] and Linux[1]
>>>> [..]
>>>>
>>>
>>> What I mean to ask is, why aren't there tifs or l3cache subnodes in 
>>> j721e, j7200 and am65?
>>>
>> I think,  above platform is doing in right way,
>>
>> AFAIK,  if we have to provide then we can provide size of this.
>>
>> l3-cache can not be addressable.
>>
> 
> 
> So the history here is we used to have the SRAM node in DT sized
> to the actual size in hardware. L3 cache size can be set at boot
> time (in SYSFW board-config file), and that uses up some of the
> SRAM, so the end address moves in. We could represent this as
> a reserved node inside the full SRAM node, or by shrinking the
> SRAM node and hiding this. Same story for TIFS and ATF, they
> use some variable amount of the end of SRAM.
> 
> I'd prefer being explicit and keep these nodes.
> 
> Andrew
> 

Thanks for the explanation, it does seem better to keep these nodes 
giving the complete description.

> 
>> But in any case, u-boot removes this code before passing to OS.
>>
>> https://elixir.bootlin.com/u-boot/latest/source/arch/arm/mach-k3/common.c#L354
>>
>>>>> Thanking You
>>>>> Neha Malcom Francis
>>>

-- 
Thanking You
Neha Malcom Francis

Re: [PATCH v2] arch: arm: mach-k3: Delete tifs node in DT fixup

[Kumar, Udit]

On 5/1/2023 8:16 PM, Andrew Davis wrote:
> On 4/26/23 9:13 AM, Kumar, Udit wrote:
>> Hi Neha,
>>
>> On 4/26/2023 5:31 PM, Neha Malcom Francis wrote:
>>> Hi Udit
>>>
>>> On 26/04/23 16:09, Kumar, Udit wrote:
>>>> Hi Neha,
>>>>
>>>>> Hi Udit,
>>>>
>>>> [..]
>>>>
>>>>>>>
>>>>>>> I do have a general doubt; why do we have only atf-sram sub-node in
>>>>>>> msmc_sram in all other devices (j721e, j7200 and am65) except 
>>>>>>> j721s2?
>>>>>>
>>>>>> let me know, which source code you are referring to
>>>>>>
>>>>>
>>>>> In U-Boot, for j721e, j7200 and am65; they *only* contain atf-sram?
>>>>
>>>> For u-boot please see
>>>>
>>>> https://elixir.bootlin.com/u-boot/latest/source/arch/arm/dts/k3-j721s2-main.dtsi#L16 
>>>>
>>>>
>>>>>> I could see for j721s2 as well, in uboot[0] and Linux[1]
>>>> [..]
>>>>
>>>
>>> What I mean to ask is, why aren't there tifs or l3cache subnodes in 
>>> j721e, j7200 and am65?
>>>
>> I think,  above platform is doing in right way,
>>
>> AFAIK,  if we have to provide then we can provide size of this.
>>
>> l3-cache can not be addressable.
>>
>
>
> So the history here is we used to have the SRAM node in DT sized
> to the actual size in hardware. L3 cache size can be set at boot
> time (in SYSFW board-config file), and that uses up some of the
> SRAM, so the end address moves in. We could represent this as
> a reserved node inside the full SRAM node, or by shrinking the
> SRAM node and hiding this. Same story for TIFS and ATF, they
> use some variable amount of the end of SRAM.
>
Ah, I have other view.

We shrunk SRAM size already, having reserved node on top of SRAM

is good as removing this.

> I'd prefer being explicit and keep these nodes.
>
> Andrew
>
>
>> But in any case, u-boot removes this code before passing to OS.
>>
>> https://elixir.bootlin.com/u-boot/latest/source/arch/arm/mach-k3/common.c#L354 
>>
>>
>>>>> Thanking You
>>>>> Neha Malcom Francis
>>>

Re: [PATCH v2] arch: arm: mach-k3: Delete tifs node in DT fixup

[Nishanth Menon]

On 12:57-20230502, Kumar, Udit wrote:
> 
> On 5/1/2023 8:16 PM, Andrew Davis wrote:
> > On 4/26/23 9:13 AM, Kumar, Udit wrote:
> > > Hi Neha,
> > > 
> > > On 4/26/2023 5:31 PM, Neha Malcom Francis wrote:
> > > > Hi Udit
> > > > 
> > > > On 26/04/23 16:09, Kumar, Udit wrote:
> > > > > Hi Neha,
> > > > > 
> > > > > > Hi Udit,
> > > > > 
> > > > > [..]
> > > > > 
> > > > > > > > 
> > > > > > > > I do have a general doubt; why do we have only atf-sram sub-node in
> > > > > > > > msmc_sram in all other devices (j721e, j7200 and
> > > > > > > > am65) except j721s2?
> > > > > > > 
> > > > > > > let me know, which source code you are referring to
> > > > > > > 
> > > > > > 
> > > > > > In U-Boot, for j721e, j7200 and am65; they *only* contain atf-sram?
> > > > > 
> > > > > For u-boot please see
> > > > > 
> > > > > https://elixir.bootlin.com/u-boot/latest/source/arch/arm/dts/k3-j721s2-main.dtsi#L16
> > > > > 
> > > > > 
> > > > > > > I could see for j721s2 as well, in uboot[0] and Linux[1]
> > > > > [..]
> > > > > 
> > > > 
> > > > What I mean to ask is, why aren't there tifs or l3cache subnodes
> > > > in j721e, j7200 and am65?
> > > > 
> > > I think,  above platform is doing in right way,
> > > 
> > > AFAIK,  if we have to provide then we can provide size of this.
> > > 
> > > l3-cache can not be addressable.
> > > 
> > 
> > 
> > So the history here is we used to have the SRAM node in DT sized
> > to the actual size in hardware. L3 cache size can be set at boot
> > time (in SYSFW board-config file), and that uses up some of the
> > SRAM, so the end address moves in. We could represent this as
> > a reserved node inside the full SRAM node, or by shrinking the
> > SRAM node and hiding this. Same story for TIFS and ATF, they
> > use some variable amount of the end of SRAM.
> > 
> Ah, I have other view.
> 
> We shrunk SRAM size already, having reserved node on top of SRAM
> 
> is good as removing this.

How about we do this:
a) Start by discussing in k.org with a patch as to how we think it
   should be and what the rationale is.

b) SRAM size fixup is a consequence of firmware being flexible.. Since,
   the tifs reserved sram etc, base description exists even after
   "hardware reconfiguration", u-boot may adjust, but not delete such nodes.
   "reserved" is part of complete description and indication that this
   specific OS is not supposed to use this region. That region is protected by
   firewall and mechanisms to make such access fail, but that is the
   point of "reserved" nodes.

c) Standardize this across the SoCs that use MSMC (WITHOUT BREAKING
   FORWARD AND BACKWARD COMPATIBILITY of u-boot vs dtb).

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

Re: [PATCH v2] arch: arm: mach-k3: Delete tifs node in DT fixup

[Kumar, Udit]

Hi Nishanth,

On 5/3/2023 4:30 AM, Nishanth Menon wrote:
> On 12:57-20230502, Kumar, Udit wrote:
>> On 5/1/2023 8:16 PM, Andrew Davis wrote:
>>> On 4/26/23 9:13 AM, Kumar, Udit wrote:
>>>> Hi Neha,
>>>>
>>>> On 4/26/2023 5:31 PM, Neha Malcom Francis wrote:
>>>>> Hi Udit
>>>>>
>>>>> On 26/04/23 16:09, Kumar, Udit wrote:
>>>>>> Hi Neha,
>>>>>>
>>>>>>> Hi Udit,
>>>>>> [..]
>>>>>> [..]
>>>>>>
>>>>> What I mean to ask is, why aren't there tifs or l3cache subnodes
>>>>> in j721e, j7200 and am65?
>>>>>
>>>> I think,  above platform is doing in right way,
>>>>
>>>> AFAIK,  if we have to provide then we can provide size of this.
>>>>
>>>> l3-cache can not be addressable.
>>>>
>>>
>>> So the history here is we used to have the SRAM node in DT sized
>>> to the actual size in hardware. L3 cache size can be set at boot
>>> time (in SYSFW board-config file), and that uses up some of the
>>> SRAM, so the end address moves in. We could represent this as
>>> a reserved node inside the full SRAM node, or by shrinking the
>>> SRAM node and hiding this. Same story for TIFS and ATF, they
>>> use some variable amount of the end of SRAM.
>>>
>> Ah, I have other view.
>>
>> We shrunk SRAM size already, having reserved node on top of SRAM
>>
>> is good as removing this.
> How about we do this:
> a) Start by discussing in k.org with a patch as to how we think it
>     should be and what the rationale is.
ok
> b) SRAM size fixup is a consequence of firmware being flexible.. Since,
>     the tifs reserved sram etc, base description exists even after
>     "hardware reconfiguration", u-boot may adjust, but not delete such nodes.
>     "reserved" is part of complete description and indication that this
>     specific OS is not supposed to use this region. That region is protected by
>     firewall and mechanisms to make such access fail, but that is the
>     point of "reserved" nodes.

you mean , keep full view of SRAM and update size of reserved node.

BTW, L3-cache and tifs will be reserved by default.

> c) Standardize this across the SoCs that use MSMC (WITHOUT BREAKING
>     FORWARD AND BACKWARD COMPATIBILITY of u-boot vs dtb).
>