Re: [PATCH v8 03/11] arm64: dts: ti: k3-am62a-mcu: Add R5F remote proc node

[Andrew Davis <afd@ti.com>]

On 5/5/25 11:00 AM, Daniel Schultz wrote:
> Hey,
> 
> On 5/5/25 17:22, Andrew Davis wrote:
>> On 5/5/25 10:05 AM, Mendez, Judith wrote:
>>> Hi Daniel,
>>>
>>> On 5/5/2025 4:55 AM, Daniel Schultz wrote:
>>>> Hi,
>>>>
>>>> I'm unable to load the latest TI firmware (98efd20ec71f8c1c8f909d34ab656731) with this patch.
>>>>
>>>> [    7.012889] remoteproc remoteproc1: 79000000.r5f is available
>>>> [    7.032640] remoteproc remoteproc1: powering up 79000000.r5f
>>>> [    7.038626] remoteproc remoteproc1: Booting fw image am62a-mcu- r5f0_0-fw, size 53140
>>>> [    7.057209] remoteproc remoteproc1: bad phdr da 0x79100000 mem 0x47ea0
>>
>> So this looks like the firmware has sections in the SRAM region. That would be the
>> issue here.
>>
>>>> [    7.064716] remoteproc remoteproc1: Failed to load program segments: -22
>>>>
>>>> I figured out that the mcu sram node disappeared in v5. Apparently adding it back manually doesn't solve this problem. Any idea what's wrong?
>>>
>>> For am62ax, there should be several items changed with this v8
>>> series in order for remoteproc to work with the TI default firmware:
> What firmware did you use? I was using the latest public default firmware from ti-linux-firmware.
>>>
>>> 1. memory carveouts were reduced to 15MB [0] & edge-ai memory
>>> carveouts are not included here
>>
>> This shouldn't be an issue, the default firmware doesn't
>> use the extended carveouts.
> Yes, this is just the echo firmware.
>>
>>> 2. mcu_sram1 node removed [2]
>>>
>>
>> So when you say you added back the SRAM node, did you also add the
>> sram = <&mcu_ram>; in the core node?
> 
> With that property added, I can load the firmware again! So, what's the problem with adding this sram node and did you remove it?
> 

Good to hear that fixed it.

And we removed the SRAM node as I was unhappy with how we were handling reserving it.
The firmware should declare usage of shared resources like this in its resource table,
and the RProc driver should dynamically request the same from a normal SRAM pool.
What we were doing before was to statically block out the whole SRAM node for use by
the R5, and the driver would unconditionally map it, even if it was not used by the
loaded firmware at all.

I wanted us to fix the above before upstreaming it so we left it out of this
series. Next cycle we should have the better solution ready and posted for
upstream.

Andrew

> - Daniel
> 
>>
>> Andrew
>>
>>> If you want to catch up on the general direction for this series,
>>> please refer to [3]. atm remoteproc can fail with the default FW,
>>> but we are trying to move away from that firmware and this is the
>>> first step in that direction.
>>>
>>> [0] https://lore.kernel.org/linux-devicetree/0ab5c5ec-cde3-41f1-8adf-2419b31497c1@ti.com/
>>> [1] https://lore.kernel.org/linux-devicetree/04e77daf-e775-44fa-82bf-8b6ebf73bcef@ti.com/
>>> [2] https://lore.kernel.org/linux-devicetree/32358aa1-0c02-4f4d-9782-2d8376c0d9fc@ti.com/
>>> [3] https://lore.kernel.org/linux-devicetree/e131298f-3713-482a-a740-ff89709270b4@ti.com/
>>>
>>> ~ Judith
>>>
>>>>
>>>> On 5/3/25 00:03, Judith Mendez wrote:
>>>>> From: Hari Nagalla <hnagalla@ti.com>
>>>>>
>>>>> AM62A SoCs have a single R5F core in the MCU voltage domain.
>>>>> Add the R5FSS node with the child node for core0 in MCU voltage
>>>>> domain .dtsi file.
>>>>>
>>>>> Signed-off-by: Hari Nagalla <hnagalla@ti.com>
>>>>> Signed-off-by: Judith Mendez <jm@ti.com>
>>>>> Acked-by: Andrew Davis <afd@ti.com>
>>>>> ---
>>>>>   arch/arm64/boot/dts/ti/k3-am62a-mcu.dtsi | 25 ++++++++++++++++++++++++
>>>>>   1 file changed, 25 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/ti/k3-am62a-mcu.dtsi b/arch/arm64/ boot/dts/ti/k3-am62a-mcu.dtsi
>>>>> index 9ed9d703ff24..ee961ced7208 100644
>>>>> --- a/arch/arm64/boot/dts/ti/k3-am62a-mcu.dtsi
>>>>> +++ b/arch/arm64/boot/dts/ti/k3-am62a-mcu.dtsi
>>>>> @@ -174,4 +174,29 @@ mcu_mcan1: can@4e18000 {
>>>>>           bosch,mram-cfg = <0x0 128 64 64 64 64 32 32>;
>>>>>           status = "disabled";
>>>>>       };
>>>>> +
>>>>> +    mcu_r5fss0: r5fss@79000000 {
>>>>> +        compatible = "ti,am62-r5fss";
>>>>> +        #address-cells = <1>;
>>>>> +        #size-cells = <1>;
>>>>> +        ranges = <0x79000000 0x00 0x79000000 0x8000>,
>>>>> +             <0x79020000 0x00 0x79020000 0x8000>;
>>>>> +        power-domains = <&k3_pds 7 TI_SCI_PD_EXCLUSIVE>;
>>>>> +        status = "disabled";
>>>>> +
>>>>> +        mcu_r5fss0_core0: r5f@79000000 {
>>>>> +            compatible = "ti,am62-r5f";
>>>>> +            reg = <0x79000000 0x00008000>,
>>>>> +                  <0x79020000 0x00008000>;
>>>>> +            reg-names = "atcm", "btcm";
>>>>> +            resets = <&k3_reset 9 1>;
>>>>> +            firmware-name = "am62a-mcu-r5f0_0-fw";
>>>>> +            ti,atcm-enable = <0>;
>>>>> +            ti,btcm-enable = <1>;
>>>>> +            ti,loczrama = <0>;
>>>>> +            ti,sci = <&dmsc>;
>>>>> +            ti,sci-dev-id = <9>;
>>>>> +            ti,sci-proc-ids = <0x03 0xff>;
>>>>> +        };
>>>>> +    };
>>>>>   };
>>>