Re: [PATCH v4 6/9] arm64: dts: ti: k3-am62p5-sk: Enable IPC with remote processors

[Judith Mendez <jm@ti.com>]

Hi Andrew,

On 2/7/25 10:58 AM, Andrew Davis wrote:
> On 2/6/25 5:51 PM, Judith Mendez wrote:
>> 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 <hnagalla@ti.com>
>> Signed-off-by: Judith Mendez <jm@ti.com>
>> ---
>> Changes since v3:
>> - Add SRAM child node for am62p MCU R5 core 0
>> ---
>>   .../dts/ti/k3-am62p-j722s-common-mcu.dtsi     | 13 +++++
>>   arch/arm64/boot/dts/ti/k3-am62p5-sk.dts       | 50 ++++++++++++++++---
>>   2 files changed, 57 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/ti/k3-am62p-j722s-common-mcu.dtsi 
>> b/arch/arm64/boot/dts/ti/k3-am62p-j722s-common-mcu.dtsi
>> index b33aff0d65c9d..0be3463bc21c5 100644
>> --- a/arch/arm64/boot/dts/ti/k3-am62p-j722s-common-mcu.dtsi
>> +++ b/arch/arm64/boot/dts/ti/k3-am62p-j722s-common-mcu.dtsi
>> @@ -6,6 +6,18 @@
>>    */
>>   &cbass_mcu {
>> +    mcu_ram: sram@79100000 {
>> +        compatible = "mmio-sram";
>> +        reg = <0x00 0x79100000 0x00 0x80000>;
>> +        ranges = <0x00 0x00 0x79100000 0x80000>;
>> +        #address-cells = <1>;
>> +        #size-cells = <1>;
>> +
>> +        mcu_sram1@0 {
> 
> What does this node do for us? Seems you reserve the whole SRAM
> area from the start, but shouldn't the phandle in mcu_r5fss0_core0
> point to this node? Or better it would use the normal SRAM API
> to request an allocation from this region.
> 
> Since this is still not resolved, and you don't mention it in
> the commit message, might be good to drop this SRAM part of this
> patch and deal with this in a later series.


Ok, since we do not have a better way to do this for now, will drop the
SRAM node.

Will respin the series one more time. Anyways I forgot to include
another patch so will add that for v5.

Thanks,
Judith



> 
> Andrew
> 
>> +            reg = <0x0 0x80000>;
>> +        };
>> +    };
>> +
>>       mcu_pmx0: pinctrl@4084000 {
>>           compatible = "pinctrl-single";
>>           reg = <0x00 0x04084000 0x00 0x88>;
>> @@ -213,6 +225,7 @@ mcu_r5fss0_core0: r5f@79000000 {
>>               ti,atcm-enable = <0>;
>>               ti,btcm-enable = <1>;
>>               ti,loczrama = <0>;
>> +            sram = <&mcu_ram>;
>>           };
>>       };
>>   };
>> 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;
>> +        };
>> +
>> +        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>;
>> +            no-map;
>> +        };
>> +
>>           secure_tfa_ddr: tfa@9e780000 {
>>               reg = <0x00 0x9e780000 0x00 0x80000>;
>>               no-map;
>> @@ -57,12 +81,6 @@ secure_ddr: optee@9e800000 {
>>               reg = <0x00 0x9e800000 0x00 0x01800000>; /* for OP-TEE */
>>               no-map;
>>           };
>> -
>> -        wkup_r5fss0_core0_memory_region: r5f-dma-memory@9c900000 {
>> -            compatible = "shared-dma-pool";
>> -            reg = <0x00 0x9c900000 0x00 0x01e00000>;
>> -            no-map;
>> -        };
>>       };
>>       vmain_pd: regulator-0 {
>> @@ -638,6 +656,26 @@ mbox_mcu_r5_0: mbox-mcu-r5-0 {
>>       };
>>   };
>> +&wkup_r5fss0 {
>> +    status = "okay";
>> +};
>> +
>> +&wkup_r5fss0_core0 {
>> +    mboxes = <&mailbox0_cluster0 &mbox_r5_0>;
>> +    memory-region = <&wkup_r5fss0_core0_dma_memory_region>,
>> +            <&wkup_r5fss0_core0_memory_region>;
>> +};
>> +
>> +&mcu_r5fss0 {
>> +    status = "okay";
>> +};
>> +
>> +&mcu_r5fss0_core0 {
>> +    mboxes = <&mailbox0_cluster1 &mbox_mcu_r5_0>;
>> +    memory-region = <&mcu_r5fss0_core0_dma_memory_region>,
>> +            <&mcu_r5fss0_core0_memory_region>;
>> +};
>> +
>>   &main_uart0 {
>>       pinctrl-names = "default";
>>       pinctrl-0 = <&main_uart0_pins_default>;