Hi Devarsh,

On 2/27/25 6:05 AM, Devarsh Thakkar wrote:

Hi Judith,

Thanks for the patch.

On 18/02/25 23:21, Judith Mendez wrote:

quoted

Hi Andrew,


On 2/18/25 10:38 AM, Andrew Davis wrote:

quoted

On 2/10/25 4:15 PM, Judith Mendez wrote:

quoted

From: Devarsh Thakkar <devarsht@ti.com>

For each remote proc, reserve memory for IPC and bind the mailbox
assignments. Two memory regions are reserved for each remote processor.
The first region of 1MB of memory is used for Vring shared buffers
and the second region is used as external memory to the remote processor
for the resource table and for tracebuffer allocations.

Signed-off-by: Devarsh Thakkar <devarsht@ti.com>
Signed-off-by: Hari Nagalla <redacted>
Signed-off-by: Judith Mendez <redacted>
---
Changes since v4:
- Drop SRAM node for am62px MCU R5fSS0 core0
---
   arch/arm64/boot/dts/ti/k3-am62p5-sk.dts | 50 ++++++++++++++++++++++---
   1 file changed, 44 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/boot/dts/ti/k3-am62p5-sk.dts

b/arch/arm64/boot/dts/ti/k3-am62p5-sk.dts
index ad71d2f27f538..9609727d042d3 100644

--- a/arch/arm64/boot/dts/ti/k3-am62p5-sk.dts
+++ b/arch/arm64/boot/dts/ti/k3-am62p5-sk.dts

@@ -48,6 +48,30 @@ reserved-memory {
           #size-cells = <2>;
           ranges;
+        mcu_r5fss0_core0_dma_memory_region:

mcu-r5fss-dma-memory-region@9b800000 {
+            compatible = "shared-dma-pool";
+            reg = <0x00 0x9b800000 0x00 0x100000>;
+            no-map;
+        };
+

I believe you are testing these carveouts against the default firmwares
shipped with AM62P SDK (compiled from meta-arago), With the same firmwares,
each remote core also does inter-processor communication with each other
(RTOS<->RTOS) on bootup, so you need to reserve the regions for the same too
as done here [1].

This is how I originally had the patch Devarsh, if you see earlier
review, we removed the SRAM nodes and the rtos-to-rtos memory carveouts.

quoted

quoted

quoted

+        mcu_r5fss0_core0_memory_region: mcu-r5fss-memory-region@9b900000 {
+            compatible = "shared-dma-pool";
+            reg = <0x00 0x9b900000 0x00 0xf00000>;
+            no-map;
+        };
+
+        wkup_r5fss0_core0_dma_memory_region: r5f-dma-memory@9c800000 {
+            compatible = "shared-dma-pool";
+            reg = <0x00 0x9c800000 0x00 0x100000>;
+            no-map;
+        };
+
+        wkup_r5fss0_core0_memory_region: r5f-memory@9c900000 {
+            compatible = "shared-dma-pool";
+            reg = <0x00 0x9c900000 0x00 0x1e00000>;

0x1e00000?

Yes I know you didn't add this and are just coping it from below, but it
is still an issue. I see the same problem for the next patch, the R5F memory
size is 0xc00000??

Every remote core gets 15MB (0xf00000), this has been true for all K3, and
all cores, DSP, R5F, M4, etc.. You even do it correct for the MCU R5F above,
but the WKUP R5F on AM62P and AM62 are just randomly given 30M and 12MB?

Not sure why FW requires 30MB here, I have reached out to FW team to
investigate this, will respond back here soon.

You will need an alignment with the firmware team to make sure that it doesn't
break with the default firmwares shipped with the AM62Px SDK. Also just FYI,
this will leave a gap of 14 MiB between the wakeup R5 and the next component
i.e. ATF, ideally we should have avoided this gap but seems like ATF nodes are
already upstream [2], so probably can't do much, nevertheless I hope that 14
MiB will be claimed/used by Linux in some manner.

I did a sanity boot test with the default firmwares shipped for am62px
SDK, no error with am62px boot so far. Changes are: removed SRAM node, 
reduced wkup r5 memory carveout, no rtos-to-rtos memory carveout).

But I realize this is not a complete test. I believe there may be
potentially memory corruption with these changes if all implemented.

Andrew, I am not sure we are going in a good direction here, unless we
have a different reduced/fixed FW in the am62px SDK, we may have memory
corruption issues on our hands.

~ Judith

Soumya,
Please provide an ACK for this, if the DM R5 firmware is exceeding 15 MiB,
then you will need to update your linker scripts and regenerate the ipc echo
test firmwares to make sure the wakeup R5 code/data does not exceed to what is
being proposed here (15 MiB).

[1]:
https://git.ti.com/cgit/ti-linux-kernel/ti-linux-kernel/tree/arch/arm64/boot/dts/ti/k3-am62p5-sk.dts?h=11.00.05#n72

[2]:
https://web.git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm64/boot/dts/ti/k3-am62p5-sk.dts?h=next-20250227#n51

Regards
Devarsh