Re: [PATCH net-next v2 2/8] dt-bindings: remoteproc: k3-r5f: Add rpmsg-eth subnode

[Krzysztof Kozlowski <krzk@kernel.org>]

On 03/09/2025 16:06, Andrew Lunn wrote:
>>>>  	mboxes = <&mailbox0_cluster2 &mbox_main_r5fss0_core0>;
>>>>  	memory-region = <&main_r5fss0_core0_dma_memory_region>,
>>>>  			<&main_r5fss0_core0_memory_region>;
>>>> +	rpmsg-eth-region = <&main_r5fss0_core0_memory_region_shm>;
>>>
>>> You already have here memory-region, so use that one.
>>>
>>
>> There is a problem with using memory-region. If I add
>> `main_r5fss0_core0_memory_region_shm` to memory region, to get this
>> phandle from driver I would have to use
>> 	
>> 	of_parse_phandle(np, "memory-region", 2)
>>
>> Where 2 is the index for this region. But the problem is how would the
>> driver know this index. This index can vary for different vendors and
>> their rproc device.
>>
>> If some other vendor tries to use this driver but their memory-region
>> has 3 existing entries. so this this entry will be the 4th one.
> 
> Just adding to this, there is nothing really TI specific in this
> system. We want the design so that any vendor can use it, just by
> adding the needed nodes to their rpmsg node, indicating there is a
> compatible implementation on the other end, and an indication of where
> the shared memory is.

I don't know your drivers, but I still do not see here a problem with
'memory-region'. You just need to tell this common code which
memory-region phandle by index or name is the one for rpmsg.

> 
> Logically, it is a different shared memory. memory-region above is for
> the rpmsg mechanism itself. A second shared memory is used for the
> Ethernet drivers where it can place Ethernet frames. The Ethernet
> frames themselves are not transported over rpmsg. The rpmsg is just
> used for the control path, not the data path.

It is still "shared-dma-pool", right? Nothing in the bindings says that
all memory-region phandles are equal or the same. Just like phandles for
clocks. Some clocks need to be enabled for entire lifetime of the
device, some are toggled on-off during runtime PM.

Best regards,
Krzysztof