Extend the Texas Instruments K3 R5F remoteproc device tree bindings to
include a 'rpmsg-eth' subnode.

This extension allows the RPMsg Ethernet to be defined as a subnode of
K3 R5F remoteproc nodes, enabling the configuration of shared memory-based
Ethernet communication between the host and remote processors.

Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
---
 .../devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml     | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml
index a492f74a8608..4dbd708ec8ee 100644
--- a/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml
@@ -210,6 +210,12 @@ patternProperties:
           should be defined as per the generic bindings in,
           Documentation/devicetree/bindings/sram/sram.yaml
 
+      rpmsg-eth:
+        $ref: /schemas/net/ti,rpmsg-eth.yaml
+        description:
+          RPMsg Ethernet subnode which represents the communication interface
+          between host processor and remote processor.
+
     required:
       - compatible
       - reg
-- 
2.34.1

On Tue, Sep 02, 2025 at 02:37:40PM +0530, MD Danish Anwar wrote:
> Extend the Texas Instruments K3 R5F remoteproc device tree bindings to
> include a 'rpmsg-eth' subnode.
> 
> This extension allows the RPMsg Ethernet to be defined as a subnode of
> K3 R5F remoteproc nodes, enabling the configuration of shared memory-based
> Ethernet communication between the host and remote processors.
> 
> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
> ---
>  .../devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml     | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml
> index a492f74a8608..4dbd708ec8ee 100644
> --- a/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml
> +++ b/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml
> @@ -210,6 +210,12 @@ patternProperties:
>            should be defined as per the generic bindings in,
>            Documentation/devicetree/bindings/sram/sram.yaml
>  
> +      rpmsg-eth:
> +        $ref: /schemas/net/ti,rpmsg-eth.yaml

No, not a separate device. Please read slides from my DT for beginners
talk from OSSE25. This is EXACTLY the case I covered there - what not to
do.

Best regards,
Krzysztof

On 03/09/25 12:49 pm, Krzysztof Kozlowski wrote:
> On Tue, Sep 02, 2025 at 02:37:40PM +0530, MD Danish Anwar wrote:
>> Extend the Texas Instruments K3 R5F remoteproc device tree bindings to
>> include a 'rpmsg-eth' subnode.
>>
>> This extension allows the RPMsg Ethernet to be defined as a subnode of
>> K3 R5F remoteproc nodes, enabling the configuration of shared memory-based
>> Ethernet communication between the host and remote processors.
>>
>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
>> ---
>>  .../devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml     | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml
>> index a492f74a8608..4dbd708ec8ee 100644
>> --- a/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml
>> +++ b/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml
>> @@ -210,6 +210,12 @@ patternProperties:
>>            should be defined as per the generic bindings in,
>>            Documentation/devicetree/bindings/sram/sram.yaml
>>  
>> +      rpmsg-eth:
>> +        $ref: /schemas/net/ti,rpmsg-eth.yaml
> 
> No, not a separate device. Please read slides from my DT for beginners

I had synced with Andrew and we came to the conclusion that including
rpmsg-eth this way will follow the DT guidelines and should be okay.

I have another approach to handle this.

Instead of a new binding and node. I can just add a new phandle to the
rproc binding. Phandle name `shared-mem-region` or `rpmsg-eth-region`

Below is the device tree and dt binding diff for the same.

diff --git
a/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml
b/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml
index a492f74a8608..c02c99a5a768 100644
--- a/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml
@@ -210,6 +210,16 @@ patternProperties:
           should be defined as per the generic bindings in,
           Documentation/devicetree/bindings/sram/sram.yaml

+      rpmsg-eth-region:
+        $ref: /schemas/types.yaml#/definitions/phandle
+        description: |
+          phandle to the reserved memory nodes to be associated with the
+          remoteproc device for rpmsg eth communication. The reserved
memory
+          nodes should be carveout nodes, and should be defined with a
"no-map"
+          property as per the bindings in
+
Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
+        additionalItems: true
+
     required:
       - compatible
       - reg
diff --git a/arch/arm64/boot/dts/ti/k3-am642-evm.dts
b/arch/arm64/boot/dts/ti/k3-am642-evm.dts
index e01866372293..e70dc542c6be 100644
--- a/arch/arm64/boot/dts/ti/k3-am642-evm.dts
+++ b/arch/arm64/boot/dts/ti/k3-am642-evm.dts
@@ -61,7 +61,13 @@ main_r5fss0_core0_dma_memory_region:
r5f-dma-memory@a0000000 {

 		main_r5fss0_core0_memory_region: r5f-memory@a0100000 {
 			compatible = "shared-dma-pool";
-			reg = <0x00 0xa0100000 0x00 0xf00000>;
+			reg = <0x00 0xa0100000 0x00 0x300000>;
+			no-map;
+		};
+
+		main_r5fss0_core0_memory_region_shm: r5f-shm-memory@a0400000 {
+			compatible = "shared-dma-pool";
+			reg = <0x00 0xa0400000 0x00 0xc00000>;
 			no-map;
 		};

@@ -768,6 +774,7 @@ &main_r5fss0_core0 {
 	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>;
 };

 &main_r5fss0_core1 {


In this approach I am creating a new phandle to a memory region that
will be used by my device.

Can you please let me know if this approach looks okay to you? Or it you
have any other suggestion on how to handle this?

> talk from OSSE25. This is EXACTLY the case I covered there - what not to
> do.

Sure I will have a look at that.

-- 
Thanks and Regards,
Danish

On 03/09/2025 09:57, MD Danish Anwar wrote:
>>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
>>> ---
>>>  .../devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml     | 6 ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml
>>> index a492f74a8608..4dbd708ec8ee 100644
>>> --- a/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml
>>> +++ b/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml
>>> @@ -210,6 +210,12 @@ patternProperties:
>>>            should be defined as per the generic bindings in,
>>>            Documentation/devicetree/bindings/sram/sram.yaml
>>>  
>>> +      rpmsg-eth:
>>> +        $ref: /schemas/net/ti,rpmsg-eth.yaml
>>
>> No, not a separate device. Please read slides from my DT for beginners
> 
> I had synced with Andrew and we came to the conclusion that including
> rpmsg-eth this way will follow the DT guidelines and should be okay.

... and did you check the guidelines? Instead of repeating something not
related to my comment rather bring argument matching the comment.


...

> @@ -768,6 +774,7 @@ &main_r5fss0_core0 {
>  	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.

>  };
> 
>  &main_r5fss0_core1 {
> 
> 
> In this approach I am creating a new phandle to a memory region that
> will be used by my device.



Best regards,
Krzysztof

On 9/3/2025 6:24 PM, Krzysztof Kozlowski wrote:
> On 03/09/2025 09:57, MD Danish Anwar wrote:
>>>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
>>>> ---
>>>>  .../devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml     | 6 ++++++
>>>>  1 file changed, 6 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml
>>>> index a492f74a8608..4dbd708ec8ee 100644
>>>> --- a/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml
>>>> +++ b/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml
>>>> @@ -210,6 +210,12 @@ patternProperties:
>>>>            should be defined as per the generic bindings in,
>>>>            Documentation/devicetree/bindings/sram/sram.yaml
>>>>  
>>>> +      rpmsg-eth:
>>>> +        $ref: /schemas/net/ti,rpmsg-eth.yaml
>>>
>>> No, not a separate device. Please read slides from my DT for beginners
>>
>> I had synced with Andrew and we came to the conclusion that including
>> rpmsg-eth this way will follow the DT guidelines and should be okay.
> 
> ... and did you check the guidelines? Instead of repeating something not
> related to my comment rather bring argument matching the comment.
> 
> 
> ...
> 
>> @@ -768,6 +774,7 @@ &main_r5fss0_core0 {
>>  	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.

But the driver code won't work for this. We need to have a way to know
which index to look for in existing memory-region which can defer from
vendor to vendor.

So to avoid this, I thought of using a new memory region. Which will
have only 1 entry specifically for this case, and the driver can always

	of_parse_phandle(np, "rpmsg-eth-region", 0)

to get the memory region.

>>  };
>>
>>  &main_r5fss0_core1 {
>>
>>
>> In this approach I am creating a new phandle to a memory region that
>> will be used by my device.
> 
> 
> 
> Best regards,
> Krzysztof

-- 
Thanks and Regards,
Md Danish Anwar

On 03/09/2025 15:32, Anwar, Md Danish wrote:
> 
> 
> On 9/3/2025 6:24 PM, Krzysztof Kozlowski wrote:
>> On 03/09/2025 09:57, MD Danish Anwar wrote:
>>>>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
>>>>> ---
>>>>>  .../devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml     | 6 ++++++
>>>>>  1 file changed, 6 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml
>>>>> index a492f74a8608..4dbd708ec8ee 100644
>>>>> --- a/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml
>>>>> +++ b/Documentation/devicetree/bindings/remoteproc/ti,k3-r5f-rproc.yaml
>>>>> @@ -210,6 +210,12 @@ patternProperties:
>>>>>            should be defined as per the generic bindings in,
>>>>>            Documentation/devicetree/bindings/sram/sram.yaml
>>>>>  
>>>>> +      rpmsg-eth:
>>>>> +        $ref: /schemas/net/ti,rpmsg-eth.yaml
>>>>
>>>> No, not a separate device. Please read slides from my DT for beginners
>>>
>>> I had synced with Andrew and we came to the conclusion that including
>>> rpmsg-eth this way will follow the DT guidelines and should be okay.
>>
>> ... and did you check the guidelines? Instead of repeating something not
>> related to my comment rather bring argument matching the comment.
>>
>>
>> ...
>>
>>> @@ -768,6 +774,7 @@ &main_r5fss0_core0 {
>>>  	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.

Index is fixed, cannot be anything else. Cannot vary.


> 
> 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.

None of these are reasons to add completely new node in DT. You use
arguments of drivers in hardware description. Really, can you read the
slides I asked for already?

> 
> But the driver code won't work for this. We need to have a way to know

Driver code can easily work with this. Multiple choices from using names
up to having driver match data with index.

> which index to look for in existing memory-region which can defer from
> vendor to vendor.
> 
> So to avoid this, I thought of using a new memory region. Which will
> have only 1 entry specifically for this case, and the driver can always
> 
> 	of_parse_phandle(np, "rpmsg-eth-region", 0)
> 
> to get the memory region.

Please don't drag the discussion. Look:

Q: I need a child node for my device to instantiate Linux driver"
A: NO

Q: I need new “vendor,foo-prop” property
A: Please look at existing common properties from common schemas or
devices representing similar class

Or actually let's start with most important:

"What Could You Put into DTS?"
Answers:
1. "Not the Linux Device Driver model"
2. "No Linux driver choices"

And that's exactly what you do and how you argue.

Best regards,
Krzysztof

> >>  	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.

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.

	Andrew

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

On 03/09/25 7:53 pm, Krzysztof Kozlowski wrote:
> 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.
> 

I am able to pass this index as driver_data in the `rpmsg_device_id`.

I can work with just adding this reserved memory region to the
'memory-region'. No need to create additional node in dt.

This will be the code,

static const struct rpmsg_eth_data ti_rpmsg_eth_data = {
	.shm_region_index = 2,
};

static struct rpmsg_device_id rpmsg_eth_rpmsg_id_table[] = {
	{ .name = "ti.shm-eth", .driver_data =
(kernel_ulong_t)&ti_rpmsg_eth_data },
	{},
};

Other vendors can add separate entry to rpmsg_eth_rpmsg_id_table with
their index based on their device tree.

I am keeping the data name `ti_rpmsg_eth_data` vendor specific so that
other vendors can create their data filed and add entry to
`rpmsg_eth_rpmsg_id_table` with driver_data pointing to their data
structure.

Andrew, does this sound ok to you. I will send out a v3 once you confirm.

Krzysztof, Andrew thanks for the feedback.

-- 
Thanks and Regards,
Danish