This patch introduces a basic RPMSG Ethernet driver skeleton. It adds
support for creating virtual Ethernet devices over RPMSG channels,
allowing user-space programs to send and receive messages using a
standard Ethernet protocol. The driver includes message handling,
probe, and remove functions, along with necessary data structures.

Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
---
 drivers/net/ethernet/Kconfig     |  10 +++
 drivers/net/ethernet/Makefile    |   1 +
 drivers/net/ethernet/rpmsg_eth.c | 128 +++++++++++++++++++++++++++++++
 drivers/net/ethernet/rpmsg_eth.h |  75 ++++++++++++++++++
 4 files changed, 214 insertions(+)
 create mode 100644 drivers/net/ethernet/rpmsg_eth.c
 create mode 100644 drivers/net/ethernet/rpmsg_eth.h

diff --git a/drivers/net/ethernet/Kconfig b/drivers/net/ethernet/Kconfig
index f86d4557d8d7..a73a45a9ef3d 100644
--- a/drivers/net/ethernet/Kconfig
+++ b/drivers/net/ethernet/Kconfig
@@ -170,6 +170,16 @@ config OA_TC6
 	  To know the implementation details, refer documentation in
 	  <file:Documentation/networking/oa-tc6-framework.rst>.
 
+config RPMSG_ETH
+	tristate "RPMsg Based Virtual Ethernet driver"
+	depends on RPMSG
+	help
+	  This makes it possible for user-space programs to send and receive
+	  rpmsg messages as a standard eth protocol.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called rpmsg_eth.
+
 source "drivers/net/ethernet/packetengines/Kconfig"
 source "drivers/net/ethernet/pasemi/Kconfig"
 source "drivers/net/ethernet/pensando/Kconfig"
diff --git a/drivers/net/ethernet/Makefile b/drivers/net/ethernet/Makefile
index 67182339469a..aebd15993e3c 100644
--- a/drivers/net/ethernet/Makefile
+++ b/drivers/net/ethernet/Makefile
@@ -107,3 +107,4 @@ obj-$(CONFIG_NET_VENDOR_XIRCOM) += xircom/
 obj-$(CONFIG_NET_VENDOR_SYNOPSYS) += synopsys/
 obj-$(CONFIG_NET_VENDOR_PENSANDO) += pensando/
 obj-$(CONFIG_OA_TC6) += oa_tc6.o
+obj-$(CONFIG_RPMSG_ETH) += rpmsg_eth.o
diff --git a/drivers/net/ethernet/rpmsg_eth.c b/drivers/net/ethernet/rpmsg_eth.c
new file mode 100644
index 000000000000..9a51619f9313
--- /dev/null
+++ b/drivers/net/ethernet/rpmsg_eth.c
@@ -0,0 +1,128 @@
+// SPDX-License-Identifier: GPL-2.0
+/* RPMsg Based Virtual Ethernet Driver
+ *
+ * Copyright (C) 2024 Texas Instruments Incorporated - https://www.ti.com/
+ */
+
+#include <linux/of.h>
+#include "rpmsg_eth.h"
+
+static int rpmsg_eth_rpmsg_cb(struct rpmsg_device *rpdev, void *data, int len,
+			      void *priv, u32 src)
+{
+	struct rpmsg_eth_common *common = dev_get_drvdata(&rpdev->dev);
+	struct message *msg = (struct message *)data;
+	u32 msg_type = msg->msg_hdr.msg_type;
+	int ret = 0;
+
+	switch (msg_type) {
+	case RPMSG_ETH_REQUEST_MSG:
+	case RPMSG_ETH_RESPONSE_MSG:
+	case RPMSG_ETH_NOTIFY_MSG:
+		dev_dbg(common->dev, "Msg type = %d, Src Id = %d\n",
+			msg_type, msg->msg_hdr.src_id);
+		break;
+	default:
+		dev_err(common->dev, "Invalid msg type\n");
+		ret = -EINVAL;
+		break;
+	}
+	return ret;
+}
+
+/**
+ * rpmsg_eth_get_shm_info - Get shared memory info from device tree
+ * @common: Pointer to rpmsg_eth_common structure
+ *
+ * Return: 0 on success, negative error code on failure
+ */
+static int rpmsg_eth_get_shm_info(struct rpmsg_eth_common *common)
+{
+	struct device_node *peer;
+	const __be32 *reg;
+	u64 start_address;
+	int prop_size;
+	int reg_len;
+	u64 size;
+
+	peer = of_find_node_by_name(NULL, "virtual-eth-shm");
+	if (!peer) {
+		dev_err(common->dev, "Couldn't get shared mem node");
+		return -ENODEV;
+	}
+
+	reg = of_get_property(peer, "reg", &prop_size);
+	if (!reg) {
+		dev_err(common->dev, "Couldn't get reg property");
+		return -ENODEV;
+	}
+
+	reg_len = prop_size / sizeof(u32);
+
+	if (reg_len == 2) {
+		/* 32-bit address space */
+		start_address = be32_to_cpu(reg[0]);
+		size = be32_to_cpu(reg[1]);
+	} else if (reg_len == 4) {
+		/* 64-bit address space */
+		start_address = ((u64)be32_to_cpu(reg[0]) << 32) |
+				 be32_to_cpu(reg[1]);
+		size = ((u64)be32_to_cpu(reg[2]) << 32) |
+			be32_to_cpu(reg[3]);
+	} else {
+		dev_err(common->dev, "Invalid reg_len: %d\n", reg_len);
+		return -EINVAL;
+	}
+
+	common->port->buf_start_addr = start_address;
+	common->port->buf_size = size;
+
+	return 0;
+}
+
+static int rpmsg_eth_probe(struct rpmsg_device *rpdev)
+{
+	struct device *dev = &rpdev->dev;
+	struct rpmsg_eth_common *common;
+	int ret = 0;
+
+	common = devm_kzalloc(&rpdev->dev, sizeof(*common), GFP_KERNEL);
+	if (!common)
+		return -ENOMEM;
+
+	dev_set_drvdata(dev, common);
+
+	common->port = devm_kzalloc(dev, sizeof(*common->port), GFP_KERNEL);
+	common->dev = dev;
+	common->rpdev = rpdev;
+
+	ret = rpmsg_eth_get_shm_info(common);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static void rpmsg_eth_rpmsg_remove(struct rpmsg_device *rpdev)
+{
+	dev_dbg(&rpdev->dev, "rpmsg-eth client driver is removed\n");
+}
+
+static struct rpmsg_device_id rpmsg_eth_rpmsg_id_table[] = {
+	{ .name = "shm-eth" },
+	{},
+};
+MODULE_DEVICE_TABLE(rpmsg, rpmsg_eth_rpmsg_id_table);
+
+static struct rpmsg_driver rpmsg_eth_rpmsg_client = {
+	.drv.name = KBUILD_MODNAME,
+	.id_table = rpmsg_eth_rpmsg_id_table,
+	.probe = rpmsg_eth_probe,
+	.callback = rpmsg_eth_rpmsg_cb,
+	.remove = rpmsg_eth_rpmsg_remove,
+};
+module_rpmsg_driver(rpmsg_eth_rpmsg_client);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("MD Danish Anwar <danishanwar@ti.com>");
+MODULE_DESCRIPTION("RPMsg Based Virtual Ethernet driver");
diff --git a/drivers/net/ethernet/rpmsg_eth.h b/drivers/net/ethernet/rpmsg_eth.h
new file mode 100644
index 000000000000..56dabd139643
--- /dev/null
+++ b/drivers/net/ethernet/rpmsg_eth.h
@@ -0,0 +1,75 @@
+/* SPDX-License-Identifier: GPL-2.0
+ * Texas Instruments K3 Inter Core Virtual Ethernet Driver common header
+ *
+ * Copyright (C) 2024 Texas Instruments Incorporated - https://www.ti.com/
+ */
+
+#ifndef __RPMSG_ETH_H__
+#define __RPMSG_ETH_H__
+
+#include <linux/errno.h>
+#include <linux/etherdevice.h>
+#include <linux/if_ether.h>
+#include <linux/if_vlan.h>
+#include <linux/jiffies.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/netdevice.h>
+#include <linux/rpmsg.h>
+
+#define RPMSG_ETH_SHM_MAGIC_NUM 0xABCDABCD
+
+enum rpmsg_eth_msg_type {
+	RPMSG_ETH_REQUEST_MSG = 0,
+	RPMSG_ETH_RESPONSE_MSG,
+	RPMSG_ETH_NOTIFY_MSG,
+};
+
+/**
+ * struct message_header - message header structure for RPMSG Ethernet
+ * @src_id: Source endpoint ID
+ * @msg_type: Message type
+ */
+struct message_header {
+	u32 src_id;
+	u32 msg_type;
+} __packed;
+
+/**
+ * struct message - RPMSG Ethernet message structure
+ *
+ * @msg_hdr: Message header contains source and destination endpoint and
+ *          the type of message
+ *
+ * This structure is used to send and receive messages between the RPMSG
+ * Ethernet ports.
+ */
+struct message {
+	struct message_header msg_hdr;
+} __packed;
+
+/**
+ * struct rpmsg_eth_common - common structure for RPMSG Ethernet
+ * @rpdev: RPMSG device
+ * @port: Ethernet port
+ * @dev: Device
+ */
+struct rpmsg_eth_common {
+	struct rpmsg_device *rpdev;
+	struct rpmsg_eth_port *port;
+	struct device *dev;
+};
+
+/**
+ * struct rpmsg_eth_port - Ethernet port structure for RPMSG Ethernet
+ * @common: Pointer to the common RPMSG Ethernet structure
+ * @buf_start_addr: Start address of the shared memory buffer for this port
+ * @buf_size: Size (in bytes) of the shared memory buffer for this port
+ */
+struct rpmsg_eth_port {
+	struct rpmsg_eth_common *common;
+	u32 buf_start_addr;
+	u32 buf_size;
+};
+
+#endif /* __RPMSG_ETH_H__ */
-- 
2.34.1

On 23/07/2025 10:03, MD Danish Anwar wrote:
> This patch introduces a basic RPMSG Ethernet driver skeleton. It adds

Please do not use "This commit/patch/change", but imperative mood. See
longer explanation here:
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95

> support for creating virtual Ethernet devices over RPMSG channels,
> allowing user-space programs to send and receive messages using a
> standard Ethernet protocol. The driver includes message handling,
> probe, and remove functions, along with necessary data structures.
> 


...

> +
> +/**
> + * rpmsg_eth_get_shm_info - Get shared memory info from device tree
> + * @common: Pointer to rpmsg_eth_common structure
> + *
> + * Return: 0 on success, negative error code on failure
> + */
> +static int rpmsg_eth_get_shm_info(struct rpmsg_eth_common *common)
> +{
> +	struct device_node *peer;
> +	const __be32 *reg;
> +	u64 start_address;
> +	int prop_size;
> +	int reg_len;
> +	u64 size;
> +
> +	peer = of_find_node_by_name(NULL, "virtual-eth-shm");


This is new ABI and I do not see earlier patch documenting it.

You cannot add undocumented ABI... but even if you documented it, I am
sorry, but I am pretty sure it is wrong. Why are you choosing random
nodes just because their name by pure coincidence is "virtual-eth-shm"?
I cannot name my ethernet like that?

Best regards,
Krzysztof

Hi Krzysztof,

On 25/07/25 12:48 am, Krzysztof Kozlowski wrote:
> On 23/07/2025 10:03, MD Danish Anwar wrote:
>> This patch introduces a basic RPMSG Ethernet driver skeleton. It adds
> 
> Please do not use "This commit/patch/change", but imperative mood. See
> longer explanation here:
> https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95
> 

Sure. I will fix this in v2.

>> support for creating virtual Ethernet devices over RPMSG channels,
>> allowing user-space programs to send and receive messages using a
>> standard Ethernet protocol. The driver includes message handling,
>> probe, and remove functions, along with necessary data structures.
>>
> 
> 
> ...
> 
>> +
>> +/**
>> + * rpmsg_eth_get_shm_info - Get shared memory info from device tree
>> + * @common: Pointer to rpmsg_eth_common structure
>> + *
>> + * Return: 0 on success, negative error code on failure
>> + */
>> +static int rpmsg_eth_get_shm_info(struct rpmsg_eth_common *common)
>> +{
>> +	struct device_node *peer;
>> +	const __be32 *reg;
>> +	u64 start_address;
>> +	int prop_size;
>> +	int reg_len;
>> +	u64 size;
>> +
>> +	peer = of_find_node_by_name(NULL, "virtual-eth-shm");
> 
> 
> This is new ABI and I do not see earlier patch documenting it.
> 
> You cannot add undocumented ABI... but even if you documented it, I am
> sorry, but I am pretty sure it is wrong. Why are you choosing random
> nodes just because their name by pure coincidence is "virtual-eth-shm"?
> I cannot name my ethernet like that?
> 

This series adds a new virtual ethernet driver. The tx / rx happens in a
shared memory block. I need to have a way for the driver to know what is
the address / size of this block. This driver can be used by any
vendors. The vendors can create a new node in their dt and specify the
base address / size of the shared memory block.

I wanted to keep the name of the node constant so that the driver can
just look for this name and then grab the address and size.

I can create a new binding file for this but I didn't create thinking
it's a virtual device not a physical and I wasn't sure if bindings can
be created for virtual devices.

In my use case, I am reserving this shared memory and during reserving I
named the node "virtual-eth-shm". The memory is reserved by the
ti_k3_r5_remoteproc.c driver. The DT change is not part of this series
but can be found
https://gist.github.com/danish-ti/cdd10525ad834fdb20871ab411ff94fb

The idea is any vendor who want to use this driver, should name their dt
node as "virtual-eth-shm" (if they also need to reserve the memory) so
that the driver can take the address from DT and use it for tx / rx.

If this is not the correct way, can you please let me know of some other
way to handle this.

One idea I had was to create a new binding for this node, and use
compatible string to access the node in driver. But the device is
virtual and not physical so I thought that might not be the way to go so
I went with the current approach.

> Best regards,
> Krzysztof

-- 
Thanks and Regards,
Danish

On 28/07/2025 10:10, MD Danish Anwar wrote:
> Hi Krzysztof,
> 
> On 25/07/25 12:48 am, Krzysztof Kozlowski wrote:
>> On 23/07/2025 10:03, MD Danish Anwar wrote:
>>> This patch introduces a basic RPMSG Ethernet driver skeleton. It adds
>>
>> Please do not use "This commit/patch/change", but imperative mood. See
>> longer explanation here:
>> https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95
>>
> 
> Sure. I will fix this in v2.
> 
>>> support for creating virtual Ethernet devices over RPMSG channels,
>>> allowing user-space programs to send and receive messages using a
>>> standard Ethernet protocol. The driver includes message handling,
>>> probe, and remove functions, along with necessary data structures.
>>>
>>
>>
>> ...
>>
>>> +
>>> +/**
>>> + * rpmsg_eth_get_shm_info - Get shared memory info from device tree
>>> + * @common: Pointer to rpmsg_eth_common structure
>>> + *
>>> + * Return: 0 on success, negative error code on failure
>>> + */
>>> +static int rpmsg_eth_get_shm_info(struct rpmsg_eth_common *common)
>>> +{
>>> +	struct device_node *peer;
>>> +	const __be32 *reg;
>>> +	u64 start_address;
>>> +	int prop_size;
>>> +	int reg_len;
>>> +	u64 size;
>>> +
>>> +	peer = of_find_node_by_name(NULL, "virtual-eth-shm");
>>
>>
>> This is new ABI and I do not see earlier patch documenting it.
>>
>> You cannot add undocumented ABI... but even if you documented it, I am
>> sorry, but I am pretty sure it is wrong. Why are you choosing random
>> nodes just because their name by pure coincidence is "virtual-eth-shm"?
>> I cannot name my ethernet like that?
>>
> 
> This series adds a new virtual ethernet driver. The tx / rx happens in a
> shared memory block. I need to have a way for the driver to know what is
> the address / size of this block. This driver can be used by any
> vendors. The vendors can create a new node in their dt and specify the
> base address / size of the shared memory block.
> 
> I wanted to keep the name of the node constant so that the driver can
> just look for this name and then grab the address and size.

You should not.

> 
> I can create a new binding file for this but I didn't create thinking
> it's a virtual device not a physical and I wasn't sure if bindings can
> be created for virtual devices.

So you added undocumented ABI intentionally, sorry, that's a no go.

> 
> In my use case, I am reserving this shared memory and during reserving I
> named the node "virtual-eth-shm". The memory is reserved by the
> ti_k3_r5_remoteproc.c driver. The DT change is not part of this series
> but can be found
> https://gist.github.com/danish-ti/cdd10525ad834fdb20871ab411ff94fb
> 
> The idea is any vendor who want to use this driver, should name their dt
> node as "virtual-eth-shm" (if they also need to reserve the memory) so
> that the driver can take the address from DT and use it for tx / rx.
> 
> If this is not the correct way, can you please let me know of some other
> way to handle this.
> 
> One idea I had was to create a new binding for this node, and use
> compatible string to access the node in driver. But the device is
> virtual and not physical so I thought that might not be the way to go so
> I went with the current approach.

virtual devices do not go to DTS anyway. How do you imagine this works?
You add it to DTS but you do not add bindings and you expect checks to
succeed?

Provide details how you checked your DTS compliance.



Best regards,
Krzysztof

On 28/07/25 6:10 pm, Krzysztof Kozlowski wrote:
> On 28/07/2025 10:10, MD Danish Anwar wrote:
>> Hi Krzysztof,
>>
>> On 25/07/25 12:48 am, Krzysztof Kozlowski wrote:
>>> On 23/07/2025 10:03, MD Danish Anwar wrote:
>>>> This patch introduces a basic RPMSG Ethernet driver skeleton. It adds
>>>
>>> Please do not use "This commit/patch/change", but imperative mood. See
>>> longer explanation here:
>>> https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95
>>>
>>
>> Sure. I will fix this in v2.
>>
>>>> support for creating virtual Ethernet devices over RPMSG channels,
>>>> allowing user-space programs to send and receive messages using a
>>>> standard Ethernet protocol. The driver includes message handling,
>>>> probe, and remove functions, along with necessary data structures.
>>>>
>>>
>>>
>>> ...
>>>
>>>> +
>>>> +/**
>>>> + * rpmsg_eth_get_shm_info - Get shared memory info from device tree
>>>> + * @common: Pointer to rpmsg_eth_common structure
>>>> + *
>>>> + * Return: 0 on success, negative error code on failure
>>>> + */
>>>> +static int rpmsg_eth_get_shm_info(struct rpmsg_eth_common *common)
>>>> +{
>>>> +	struct device_node *peer;
>>>> +	const __be32 *reg;
>>>> +	u64 start_address;
>>>> +	int prop_size;
>>>> +	int reg_len;
>>>> +	u64 size;
>>>> +
>>>> +	peer = of_find_node_by_name(NULL, "virtual-eth-shm");
>>>
>>>
>>> This is new ABI and I do not see earlier patch documenting it.
>>>
>>> You cannot add undocumented ABI... but even if you documented it, I am
>>> sorry, but I am pretty sure it is wrong. Why are you choosing random
>>> nodes just because their name by pure coincidence is "virtual-eth-shm"?
>>> I cannot name my ethernet like that?
>>>
>>
>> This series adds a new virtual ethernet driver. The tx / rx happens in a
>> shared memory block. I need to have a way for the driver to know what is
>> the address / size of this block. This driver can be used by any
>> vendors. The vendors can create a new node in their dt and specify the
>> base address / size of the shared memory block.
>>
>> I wanted to keep the name of the node constant so that the driver can
>> just look for this name and then grab the address and size.
> 
> You should not.
> 
>>
>> I can create a new binding file for this but I didn't create thinking
>> it's a virtual device not a physical and I wasn't sure if bindings can
>> be created for virtual devices.
> 
> So you added undocumented ABI intentionally, sorry, that's a no go.
> 
>>
>> In my use case, I am reserving this shared memory and during reserving I
>> named the node "virtual-eth-shm". The memory is reserved by the
>> ti_k3_r5_remoteproc.c driver. The DT change is not part of this series
>> but can be found
>> https://gist.github.com/danish-ti/cdd10525ad834fdb20871ab411ff94fb
>>
>> The idea is any vendor who want to use this driver, should name their dt
>> node as "virtual-eth-shm" (if they also need to reserve the memory) so
>> that the driver can take the address from DT and use it for tx / rx.
>>
>> If this is not the correct way, can you please let me know of some other
>> way to handle this.
>>
>> One idea I had was to create a new binding for this node, and use
>> compatible string to access the node in driver. But the device is
>> virtual and not physical so I thought that might not be the way to go so
>> I went with the current approach.
> 
> virtual devices do not go to DTS anyway. How do you imagine this works?
> You add it to DTS but you do not add bindings and you expect checks to
> succeed?
> 
> Provide details how you checked your DTS compliance.
> 
> 

This is my device tree patch [1]. I ran these two commands before and
after applying the patch and checked the diff.

	make dt_binding_check
	make dtbs_check

I didn't see any new error / warning getting introduced due to the patch

After applying the patch I also ran,

	make CHECK_DTBS=y ti/k3-am642-evm.dtb

I still don't see any warnings / error.


If you look at the DT patch, you'll see I am adding a new node in the
`reserved-memory`. I am not creating a completely new undocumented node.
Instead I am creating a new node under reserved-memory as the shared
memory used by rpmsg-eth driver needs to be reserved first. This memory
is reserved by the ti_k3_r5_remoteproc driver by k3_reserved_mem_init().

It's just that I am naming this node as "virtual-eth-shm@a0400000" and
then using the same name in driver to get the base_address and size
mentioned in this node.


> 
> Best regards,
> Krzysztof


[1]
https://gist.githubusercontent.com/danish-ti/fd3e630227ae5b165e12eabd91b0dc9d/raw/67d7c15cd1c47a29c0cfd3674d7cd6233ef1bea5/0001-arch-arm64-dts-k3-am64-Add-shared-memory-node.patch

-- 
Thanks and Regards,
Danish

On 29/07/2025 11:46, MD Danish Anwar wrote:
>>>
>>> One idea I had was to create a new binding for this node, and use
>>> compatible string to access the node in driver. But the device is
>>> virtual and not physical so I thought that might not be the way to go so
>>> I went with the current approach.
>>
>> virtual devices do not go to DTS anyway. How do you imagine this works?
>> You add it to DTS but you do not add bindings and you expect checks to
>> succeed?
>>
>> Provide details how you checked your DTS compliance.
>>
>>
> 
> This is my device tree patch [1]. I ran these two commands before and
> after applying the patch and checked the diff.
> 
> 	make dt_binding_check
> 	make dtbs_check
> 
> I didn't see any new error / warning getting introduced due to the patch
> 
> After applying the patch I also ran,
> 
> 	make CHECK_DTBS=y ti/k3-am642-evm.dtb
> 
> I still don't see any warnings / error.
> 
> 
> If you look at the DT patch, you'll see I am adding a new node in the

I see. This is so odd syntax... You have the phandle there, so you do
not need to do any node name checking. I did not really expect you will
be checking node name for reserved memory!!!

Obviously this will be fine with dt bindings, because such ABI should
never be constructed.


> `reserved-memory`. I am not creating a completely new undocumented node.
> Instead I am creating a new node under reserved-memory as the shared
> memory used by rpmsg-eth driver needs to be reserved first. This memory
> is reserved by the ti_k3_r5_remoteproc driver by k3_reserved_mem_init().
> 
> It's just that I am naming this node as "virtual-eth-shm@a0400000" and
> then using the same name in driver to get the base_address and size
> mentioned in this node.

And how your driver will work with:

s/virtual-eth-shm@a0400000/whatever@a0400000/

? It will not.

Best regards,
Krzysztof

On 29/07/25 6:02 pm, Krzysztof Kozlowski wrote:
> On 29/07/2025 11:46, MD Danish Anwar wrote:
>>>>
>>>> One idea I had was to create a new binding for this node, and use
>>>> compatible string to access the node in driver. But the device is
>>>> virtual and not physical so I thought that might not be the way to go so
>>>> I went with the current approach.
>>>
>>> virtual devices do not go to DTS anyway. How do you imagine this works?
>>> You add it to DTS but you do not add bindings and you expect checks to
>>> succeed?
>>>
>>> Provide details how you checked your DTS compliance.
>>>
>>>
>>
>> This is my device tree patch [1]. I ran these two commands before and
>> after applying the patch and checked the diff.
>>
>> 	make dt_binding_check
>> 	make dtbs_check
>>
>> I didn't see any new error / warning getting introduced due to the patch
>>
>> After applying the patch I also ran,
>>
>> 	make CHECK_DTBS=y ti/k3-am642-evm.dtb
>>
>> I still don't see any warnings / error.
>>
>>
>> If you look at the DT patch, you'll see I am adding a new node in the
> 
> I see. This is so odd syntax... You have the phandle there, so you do
> not need to do any node name checking. I did not really expect you will
> be checking node name for reserved memory!!!
> 

I don't have access to the phandle in my function. The reserved memory
is reserved by ti_k3_r5_remoteproc driver. That driver has the phandle.

I am writing a new driver rpmsg_eth, this driver only has the rpdev
structure. This driver doesn't have any dt node or phandle and because
of this I am doing `peer = of_find_node_by_name(NULL,
"virtual-eth-shm");` to get the access to this node here.

I couldn't find any way to access the dt node of reserved memory from
this (rpmsg_eth) driver. Please let me know if there is any way I can
access that.

> Obviously this will be fine with dt bindings, because such ABI should
> never be constructed.
> 
> 
>> `reserved-memory`. I am not creating a completely new undocumented node.
>> Instead I am creating a new node under reserved-memory as the shared
>> memory used by rpmsg-eth driver needs to be reserved first. This memory
>> is reserved by the ti_k3_r5_remoteproc driver by k3_reserved_mem_init().
>>
>> It's just that I am naming this node as "virtual-eth-shm@a0400000" and
>> then using the same name in driver to get the base_address and size
>> mentioned in this node.
> 
> And how your driver will work with:
> 
> s/virtual-eth-shm@a0400000/whatever@a0400000/
> 


It won't. The driver imposes a restriction with the node name. The node
name should always be "virtual-eth-shm"

For other vendors who want to use this driver, they need to reserve
memory for their shared block and name the node `virtual-eth-shm@XXXXXXXX`

> ? It will not.
> 
> Best regards,
> Krzysztof

-- 
Thanks and Regards,
Danish

On 30/07/2025 08:01, MD Danish Anwar wrote:
>>
>>> `reserved-memory`. I am not creating a completely new undocumented node.
>>> Instead I am creating a new node under reserved-memory as the shared
>>> memory used by rpmsg-eth driver needs to be reserved first. This memory
>>> is reserved by the ti_k3_r5_remoteproc driver by k3_reserved_mem_init().
>>>
>>> It's just that I am naming this node as "virtual-eth-shm@a0400000" and
>>> then using the same name in driver to get the base_address and size
>>> mentioned in this node.
>>
>> And how your driver will work with:
>>
>> s/virtual-eth-shm@a0400000/whatever@a0400000/
>>
> 
> 
> It won't. The driver imposes a restriction with the node name. The node
> name should always be "virtual-eth-shm"

Drivers cannot impose the restriction. I don't think you understand the
problem. What stops me from renaming the node? Nothing.

You keep explaining this broken code, but sorry, this is a no-go. Shall
I NAK it to make it obvious?


Best regards,
Krzysztof

On 7/30/2025 11:43 AM, Krzysztof Kozlowski wrote:
> On 30/07/2025 08:01, MD Danish Anwar wrote:
>>>
>>>> `reserved-memory`. I am not creating a completely new undocumented node.
>>>> Instead I am creating a new node under reserved-memory as the shared
>>>> memory used by rpmsg-eth driver needs to be reserved first. This memory
>>>> is reserved by the ti_k3_r5_remoteproc driver by k3_reserved_mem_init().
>>>>
>>>> It's just that I am naming this node as "virtual-eth-shm@a0400000" and
>>>> then using the same name in driver to get the base_address and size
>>>> mentioned in this node.
>>>
>>> And how your driver will work with:
>>>
>>> s/virtual-eth-shm@a0400000/whatever@a0400000/
>>>
>>
>>
>> It won't. The driver imposes a restriction with the node name. The node
>> name should always be "virtual-eth-shm"
> 
> Drivers cannot impose the restriction. I don't think you understand the
> problem. What stops me from renaming the node? Nothing.
> 
> You keep explaining this broken code, but sorry, this is a no-go. Shall
> I NAK it to make it obvious?
> 

Krzysztof, I understand this can't be accepted. This wasn't my first
approach. The first approach was that the firmware running on the
remotecore will share the base-address using rpmsg. But that was
discouraged by Andrew.

So I came up with this DT approach to read the base-address from linux only.

Andrew, Since rpmsg-eth is a virtual device and we can't have DT node
for it. Using the reserved memory node and then search the same using
node name in the driver is also not acceptable as per Krzysztof. What do
you suggest should be done here?

Can we revisit the first approach (firmware sharing the address)? Can we
use module params to pass the base-address? or Do you have any other
ideas on how to handle this?

Please let me know.

> 
> Best regards,
> Krzysztof

-- 
Thanks and Regards,
Md Danish Anwar

Hi Krzysztof, Andrew,

On 30/07/25 8:41 pm, Anwar, Md Danish wrote:
> 
> 
> On 7/30/2025 11:43 AM, Krzysztof Kozlowski wrote:
>> On 30/07/2025 08:01, MD Danish Anwar wrote:
>>>>
>>>>> `reserved-memory`. I am not creating a completely new undocumented node.
>>>>> Instead I am creating a new node under reserved-memory as the shared
>>>>> memory used by rpmsg-eth driver needs to be reserved first. This memory
>>>>> is reserved by the ti_k3_r5_remoteproc driver by k3_reserved_mem_init().
>>>>>
>>>>> It's just that I am naming this node as "virtual-eth-shm@a0400000" and
>>>>> then using the same name in driver to get the base_address and size
>>>>> mentioned in this node.
>>>>
>>>> And how your driver will work with:
>>>>
>>>> s/virtual-eth-shm@a0400000/whatever@a0400000/
>>>>
>>>
>>>
>>> It won't. The driver imposes a restriction with the node name. The node
>>> name should always be "virtual-eth-shm"
>>
>> Drivers cannot impose the restriction. I don't think you understand the
>> problem. What stops me from renaming the node? Nothing.
>>
>> You keep explaining this broken code, but sorry, this is a no-go. Shall
>> I NAK it to make it obvious?
>>
> 
> Krzysztof, I understand this can't be accepted. This wasn't my first
> approach. The first approach was that the firmware running on the
> remotecore will share the base-address using rpmsg. But that was
> discouraged by Andrew.
> 
> So I came up with this DT approach to read the base-address from linux only.
> 
> Andrew, Since rpmsg-eth is a virtual device and we can't have DT node
> for it. Using the reserved memory node and then search the same using
> node name in the driver is also not acceptable as per Krzysztof. What do
> you suggest should be done here?
> 
> Can we revisit the first approach (firmware sharing the address)? Can we
> use module params to pass the base-address? or Do you have any other
> ideas on how to handle this?
> 
> Please let me know.
> 

This is what I came up with after few discussions offline with Andrew. I
will post v2 soon with the below changes

1. Similar to qcom,glink-edge.yaml and google,cros-ec.yaml - I will
create a new binding named ti,rpmsg-eth.yaml this binding will describe
the rpmsg eth node. This node will have a memory region.
2. The rpmsg-eth node will be a child node of the rproc device. In this
case `r5f@78000000`. I will modify the rproc binding
`ti,k3-r5f-rproc.yaml` to describe the same.
3. Other vendors who wish to use RPMSG_ETH, can create a rpmsg-eth node
as a child of their rproc device.

This approach is very similar to what's done by qcom,glink-edge.yaml
/google,cros-ec.yaml and their users.

-- 
Thanks and Regards,
Danish