Thread (20 messages) 20 messages, 4 authors, 2025-09-05

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

From: Krzysztof Kozlowski <krzk@kernel.org>
Date: 2025-09-03 14:19:59
Also in: linux-arm-kernel, linux-devicetree, linux-remoteproc, lkml, netdev

On 03/09/2025 15:32, Anwar, Md Danish wrote:

On 9/3/2025 6:24 PM, Krzysztof Kozlowski wrote:
quoted
On 03/09/2025 09:57, MD Danish Anwar wrote:
quoted
quoted
quoted
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.


...
quoted
@@ -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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help