Message-ID: <3f95eb2c-d9ea-4acd-a57f-3ffdd43fd505@ti.com>
Date: Mon, 21 Apr 2025 15:28:21 -0500
From: Andrew Davis <afd@...com>
To: Judith Mendez <jm@...com>, Bryan Brattlof <bb@...com>,
        Nishanth Menon
	<nm@...com>
CC: Vignesh Raghavendra <vigneshr@...com>, Tero Kristo <kristo@...nel.org>,
        Rob Herring <robh@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>,
        <linux-arm-kernel@...ts.infradead.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, Hari Nagalla
	<hnagalla@...com>,
        Beleswar Prasad <b-padhi@...com>,
        Markus
 Schneider-Pargmann <msp@...libre.com>,
        Devarsh Thakkar <devarsht@...com>
Subject: Re: [PATCH v7 06/11] arm64: dts: ti: k3-am62a7-sk: Enable IPC with
 remote processors

On 4/21/25 2:05 PM, Judith Mendez wrote:
> Hi Bryan,
> 
> On 4/21/25 11:26 AM, Bryan Brattlof wrote:
>> On April 21, 2025 thus sayeth Nishanth Menon:
>>> On 10:04-20250419, Bryan Brattlof wrote:
>>>> On April 15, 2025 thus sayeth Judith Mendez:
>>>>> From: Devarsh Thakkar <devarsht@...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@...com>
>>>>> Signed-off-by: Hari Nagalla <hnagalla@...com>
>>>>> Signed-off-by: Judith Mendez <jm@...com>
>>>>> Acked-by: Andrew Davis <afd@...com>
>>>>> Reviewed-by: Beleswar Padhi <b-padhi@...com>
>>>>> Reviewed-by: Jai Luthra <jai.luthra@...asonboard.com>
>>>>> ---
>>>>>   arch/arm64/boot/dts/ti/k3-am62a7-sk.dts | 96 +++++++++++++++++++++++--
>>>>>   1 file changed, 90 insertions(+), 6 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts b/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts
>>>>> index 1c9d95696c839..7d817b447c1d0 100644
>>>>> --- a/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts
>>>>> +++ b/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts
>>>>> @@ -52,6 +52,42 @@ linux,cma {
>>>>>               linux,cma-default;
>>>>>           };
>>>>> +        c7x_0_dma_memory_region: c7x-dma-memory@...00000 {
>>>>> +            compatible = "shared-dma-pool";
>>>>> +            reg = <0x00 0x99800000 0x00 0x100000>;
>>>>> +            no-map;
>>>>> +        };
>>>>> +
>>>>> +        c7x_0_memory_region: c7x-memory@...00000 {
>>>>> +            compatible = "shared-dma-pool";
>>>>> +            reg = <0x00 0x99900000 0x00 0xf00000>;
>>>>> +            no-map;
>>>>> +        };
>>>>> +
>>>>
>>>> I know this has been a push for our IPC and MCU+ teams for a couple
>>>> windows now, though I do want to point out that some AM62A devices
>>>> (AM62A12AQMSIAMBRQ1) will not even have a C7x.
>>>>
>>>> It's relatively easy to cut nodes out that describe the hardware in the
>>>> bootloaders, but once we start configuring them to demo something it
>>>> becomes impossible to unwind that during boot.
>>>>
>>>> We can clam we only support the superset devices but I just wanted to
>>>> make this email so I could point people to it when they inevitably ask
>>>> why their parts do not work out of the box with Linux.
>>>>
>>>> Naked-by: Bryan Brattlof <bb@...com>
>>>
>>>
>>> I am confused. I do not see support for AM62A1 in upstream. We have
>>> AM62A7-SK in upstream. I am not sure what direction you are suggesting
>>> here.
>>
>> All I'm trying to point out is for every part we upstream there are >10
>> times the number of parts that for one reason or another will not make
>> it to these upstream repositories.
>>
>> Most of these parts will have trivial changes like having lower CPU
>> counts, some will not have a GPU, MCU, PRU, or display, or maybe it's
>> just a package change and the thermal zones are different, or it's just
>> the speeds the IP can confidently run at, or it could be as simple as
>> DDR part changes. Each variant will be mostly the superset device with
>> one or two nodes disabled or modified in some way.
>>
>> For a while now, without configuring the remote cores to demo anything,
>> it's been relatively seamless to support these variants in the
>> bootloaders by disabling or modifying the nodes that do not exist so
>> Linux can at least boot to a shell and provides a great foundation for
>> others to start their development
>>
>> If we want to use these boards to demo a advanced usecases we can do
>> that but I worry it will come at the cost of supporting all the part
>> variants.
>>
>> My hope was we could define the board as minimally as possible here so
>> we can maximize their flexibility with what timers, mailboxes and memory
>> carve-outs each remote processor uses.
>>
> 
> Is it not agreed upon to support the superset device upstream? It seems
> like what we need is a detailed whitepaper on board bring-up for each
> part variant instead of NOT adding support for the superset device
> upstream approach.
> 

We would still support the superset device upstream, Bryan is simply
suggesting (correct me if I'm wrong) a different way of supporting
the superset device such that later adding support for the cutdowns
is less painful.

To this I 100% agree, and I've suggested the same before in
cases where we know a subset device is in the works. Sometimes
plans for a cutdown happen after we have added support and that
makes a bit of a mess (see J78424 / J742s2). Even when we know
this is going to happen we still make a mess of DT which we end
up having to clean up later (see AM62p / J722s).

Easy manipulation of the DT in this way was one of the promises
of "System Device Trees". Until we have that, the next best thing
is DT overlays. The solution here then is to factor out the components
we know will be conditional into a device tree overlays. These
overlays will be applied by default to form the superset DT as the
out-of-box supported device, while also allowing easy removal without
having the bootloader perform complex DT surgery. Win-Win

The plan we agreed on for this series (back in v2 IIRC) is to add
these items as part of the base board DT to match what is already
done for all our other devices. *then* we factor it out from all
devices in one go to make that series more coherent.

So if that sounds good, Bryan feel free to un-NAK this and we
we will be sure to make your task of supporting all these subset
devices much easier in the follow up series :)

Andrew

Powered by blists - more mailing lists