[PATCH net-next 0/5] Add RPMSG Ethernet Driver

[PATCH net-next 0/5] Add RPMSG Ethernet Driver

[MD Danish Anwar]

This patch series introduces the RPMSG Ethernet driver, which provides a
virtual Ethernet interface for communication between a host processor and
a remote processor using the RPMSG framework. The driver enables
Ethernet-like packet transmission and reception over shared memory,
facilitating inter-core communication in systems with heterogeneous
processors.

This series is a rework of [1]. There was comment from Andrew Lunn
<andrew@lunn.ch> to modify this driver and make it generic so that other
vendors can also use it.

I have tried to generalize the driver. Since there has been lots of changes
since [1], I am posting this as a new series.

The series includes the following patches:

1. Documentation:
  - Adds comprehensive documentation for the RPMSG Ethernet driver.
  - Details the shared memory layout, communication protocol, and usage
    instructions.
  - Provides a guide for vendors to develop compatible firmware.

2. Basic RPMSG Skeleton:
  - Introduces the basic RPMSG Ethernet driver skeleton.
  - Implements probe, remove, and callback functions.
  - Sets up the foundation for RPMSG communication.

3. Netdev Registration:
  - Registers the RPMSG Ethernet device as a netdev.
  - Enhances the RPMSG callback to handle shared memory for TX and RX
    buffers.
  - Introduces shared memory structures and initializes the netdev.

4. Netdev Operations:
  - Implements netdev operations such as `ndo_open`, `ndo_stop`,
    `ndo_start_xmit`, and `ndo_set_mac_address`.
  - Adds support for NAPI-based RX processing and a timer-based RX
    polling mechanism.
  - Introduces a state machine to manage the driver's state transitions.

5. Multicast Filtering:
  - Adds support for multicast address filtering.
  - Implements the `ndo_set_rx_mode` callback to manage multicast
    addresses.
  - Introduces a workqueue-based mechanism for asynchronous RX mode
    updates.

Key Features:
- Virtual Ethernet interface using RPMSG.
- Shared memory-based packet transmission and reception.
- Support for multicast address management.
- Dynamic MAC address assignment.
- Efficient packet processing using NAPI.
- State machine for managing interface states.

This driver is designed to be generic and vendor-agnostic. Vendors can
develop firmware for the remote processor to make it compatible with this
driver by adhering to the shared memory layout and communication protocol
described in the documentation.

This patch series has been tested on a TI AM64xx platform with a compatible
remote processor firmware. Feedback and suggestions for improvement are
welcome.

[1] https://lore.kernel.org/all/20240531064006.1223417-1-y-mallik@ti.com/

MD Danish Anwar (5):
  net: rpmsg-eth: Add Documentation for RPMSG-ETH Driver
  net: rpmsg-eth: Add basic rpmsg skeleton
  net: rpmsg-eth: Register device as netdev
  net: rpmsg-eth: Add netdev ops
  net: rpmsg-eth: Add support for multicast filtering

 .../device_drivers/ethernet/index.rst         |   1 +
 .../device_drivers/ethernet/rpmsg_eth.rst     | 339 ++++++++
 drivers/net/ethernet/Kconfig                  |  10 +
 drivers/net/ethernet/Makefile                 |   1 +
 drivers/net/ethernet/rpmsg_eth.c              | 743 ++++++++++++++++++
 drivers/net/ethernet/rpmsg_eth.h              | 305 +++++++
 6 files changed, 1399 insertions(+)
 create mode 100644 Documentation/networking/device_drivers/ethernet/rpmsg_eth.rst
 create mode 100644 drivers/net/ethernet/rpmsg_eth.c
 create mode 100644 drivers/net/ethernet/rpmsg_eth.h


base-commit: 56613001dfc9b2e35e2d6ba857cbc2eb0bac4272
-- 
2.34.1

[PATCH net-next 1/5] net: rpmsg-eth: Add Documentation for RPMSG-ETH Driver

[MD Danish Anwar]

Add documentation for the RPMSG Based Virtual Ethernet Driver (rpmsg-eth).
The documentation describes the driver's architecture, shared memory
layout, RPMSG communication protocol, and requirements for vendor firmware
to interoperate with the driver. It details the use of a magic number for
shared memory validation, outlines the information exchanged between the
host and remote processor, and provides a how-to guide for vendors to
implement compatible firmware.

Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
---
 .../device_drivers/ethernet/index.rst         |   1 +
 .../device_drivers/ethernet/rpmsg_eth.rst     | 339 ++++++++++++++++++
 2 files changed, 340 insertions(+)
 create mode 100644 Documentation/networking/device_drivers/ethernet/rpmsg_eth.rst

diff --git a/Documentation/networking/device_drivers/ethernet/index.rst b/Documentation/networking/device_drivers/ethernet/index.rst
index 40ac552641a3..941f60585ee4 100644
--- a/Documentation/networking/device_drivers/ethernet/index.rst
+++ b/Documentation/networking/device_drivers/ethernet/index.rst
@@ -61,6 +61,7 @@ Contents:
    wangxun/txgbevf
    wangxun/ngbe
    wangxun/ngbevf
+   rpmsg_eth
 
 .. only::  subproject and html
 
diff --git a/Documentation/networking/device_drivers/ethernet/rpmsg_eth.rst b/Documentation/networking/device_drivers/ethernet/rpmsg_eth.rst
new file mode 100644
index 000000000000..70c13deb31ea
--- /dev/null
+++ b/Documentation/networking/device_drivers/ethernet/rpmsg_eth.rst
@@ -0,0 +1,339 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+===================================
+RPMSG Based Virtual Ethernet Driver
+===================================
+
+Overview
+========
+
+The RPMSG Based Virtual Ethernet Driver provides a virtual Ethernet interface for
+communication between a host processor and a remote processor using the RPMSG
+framework. This driver enables Ethernet-like packet transmission and reception
+over shared memory, facilitating inter-core communication in systems with
+heterogeneous processors.
+
+The driver is designed to work with the RPMSG framework, which is part of the
+Linux Remote Processor (remoteproc) subsystem. It uses shared memory for data
+exchange and supports features like multicast address management, dynamic MAC
+address assignment, and efficient packet processing using NAPI.
+
+This driver is generic and can be used by any vendor. Vendors can develop their
+own firmware for the remote processor to make it compatible with this driver.
+The firmware must adhere to the shared memory layout, RPMSG communication
+protocol, and data exchange requirements described in this documentation.
+
+Key Features
+============
+
+- Virtual Ethernet interface using RPMSG.
+- Shared memory-based packet transmission and reception.
+- Support for multicast address management.
+- Dynamic MAC address assignment.
+- NAPI (New API) support for efficient packet processing.
+- State machine for managing interface states.
+- Workqueue-based asynchronous operations.
+- Support for notifications and responses from the remote processor.
+
+Magic Number
+============
+
+A **magic number** is used in the shared memory layout to validate that the
+memory region is correctly initialized and accessible by both the host and the
+remote processor. This value is a unique constant (for example,
+``0xABCDABCD``) that is written to specific locations (such as the head and
+tail structures) in the shared memory by the firmware and checked by the Linux
+driver during the handshake process.
+
+Purpose of the Magic Number
+---------------------------
+
+- **Validation:** Ensures that the shared memory region has been properly set up
+  and is not corrupted or uninitialized.
+- **Synchronization:** Both the host and remote processor must agree on the
+  magic number value, which helps detect mismatches in memory layout or protocol
+  version.
+- **Error Detection:** If the driver detects an incorrect magic number during
+  initialization or runtime, it can abort the handshake and report an error,
+  preventing undefined behavior.
+
+Implementation Details
+----------------------
+
+- The magic number is defined as a macro in the driver source (e.g.,
+  ``#define RPMSG_ETH_SHM_MAGIC_NUM 0xABCDABCD``).
+- The firmware must write this value to the ``magic_num`` field of the head and
+  tail structures in the shared memory region.
+- During the handshake, the Linux driver reads these fields and compares them to
+  the expected value. If any mismatch is detected, the driver will log an error
+  and refuse to proceed.
+
+Example Usage in Shared Memory
+------------------------------
+
+.. code-block:: text
+
+      Shared Memory Layout:
+      ---------------------------
+      |   MAGIC_NUM (0xABCDABCD) |   <-- rpmsg_eth_shm_head
+      |          HEAD            |
+      ---------------------------
+      |   MAGIC_NUM (0xABCDABCD) |   <-- rpmsg_eth_shm_tail
+      |          TAIL            |
+      ---------------------------
+
+The magic number must be present in both the head and tail structures for the
+handshake to succeed.
+
+Firmware developers must ensure that the correct magic number is written to the
+appropriate locations in shared memory before the Linux driver attempts to
+initialize the interface.
+
+Shared Memory Layout
+====================
+
+The RPMSG Based Virtual Ethernet Driver uses a shared memory region to exchange
+data between the host and the remote processor. The shared memory is divided
+into transmit and receive regions, each with its own `head` and `tail` pointers
+to track the buffer state.
+
+Shared Memory Parameters
+------------------------
+
+The following parameters are exchanged between the host and the firmware to
+configure the shared memory layout:
+
+1. **num_pkt_bufs**:
+
+   - The total number of packet buffers available in the shared memory.
+   - This determines the maximum number of packets that can be stored in the
+     shared memory at any given time.
+
+2. **buff_slot_size**:
+
+   - The size of each buffer slot in the shared memory.
+   - This includes space for the packet length, metadata, and the actual packet
+     data.
+
+3. **base_addr**:
+
+   - The base address of the shared memory region.
+   - This is the starting point for accessing the shared memory.
+
+4. **tx_offset**:
+
+   - The offset from the `base_addr` where the transmit buffers begin.
+   - This is used by the host to write packets for transmission.
+
+5. **rx_offset**:
+
+   - The offset from the `base_addr` where the receive buffers begin.
+   - This is used by the host to read packets received from the remote
+     processor.
+
+Shared Memory Structure
+-----------------------
+
+The shared memory layout is as follows:
+
+.. code-block:: text
+
+      Shared Memory Layout:
+      ---------------------------
+      |        MAGIC_NUM        |   rpmsg_eth_shm_head
+      |          HEAD           |
+      ---------------------------
+      |        MAGIC_NUM        |
+      |        PKT_1_LEN        |
+      |          PKT_1          |
+      ---------------------------
+      |           ...           |
+      ---------------------------
+      |        MAGIC_NUM        |
+      |          TAIL           |   rpmsg_eth_shm_tail
+      ---------------------------
+
+1. **MAGIC_NUM**:
+
+   - A unique identifier used to validate the shared memory region.
+   - Ensures that the memory region is correctly initialized and accessible.
+
+2. **HEAD Pointer**:
+
+   - Tracks the start of the buffer for packet transmission or reception.
+   - Updated by the producer (host or remote processor) after writing a packet.
+
+3. **TAIL Pointer**:
+
+   - Tracks the end of the buffer for packet transmission or reception.
+   - Updated by the consumer (host or remote processor) after reading a packet.
+
+4. **Packet Buffers**:
+
+   - Each packet buffer contains:
+
+      - **Packet Length**: A 4-byte field indicating the size of the packet.
+      - **Packet Data**: The actual Ethernet frame data.
+
+5. **Buffer Size**:
+
+   - Each buffer has a fixed size defined by `RPMSG_ETH_BUFFER_SIZE`, which
+     includes space for the packet length and data.
+
+Buffer Management
+-----------------
+
+- The host and remote processor use a circular buffer mechanism to manage the shared memory.
+- The `head` and `tail` pointers are used to determine the number of packets available for processing:
+
+   .. code-block:: c
+
+         num_pkts = head - tail;
+         num_pkts = num_pkts >= 0 ? num_pkts : (num_pkts + max_buffers);
+
+- The producer writes packets to the buffer and increments the `head` pointer.
+- The consumer reads packets from the buffer and increments the `tail` pointer.
+
+RPMSG Communication
+===================
+
+The driver uses RPMSG channels to exchange control messages between the host and
+the remote processor. These messages are used to manage the state of the
+Ethernet interface, configure settings, notify events, and exchange runtime
+information.
+
+Information Exchanged Between RPMSG Channels
+--------------------------------------------
+
+1. **Requests from Host to Remote Processor**:
+
+   - `RPMSG_ETH_REQ_SHM_INFO`: Request shared memory information, such as
+     ``num_pkt_bufs``, ``buff_slot_size``, ``base_addr``, ``tx_offset``, and
+     ``rx_offset``.
+   - `RPMSG_ETH_REQ_SET_MAC_ADDR`: Set the MAC address of the Ethernet
+     interface.
+   - `RPMSG_ETH_REQ_ADD_MC_ADDR`: Add a multicast address to the remote
+     processor's filter list.
+   - `RPMSG_ETH_REQ_DEL_MC_ADDR`: Remove a multicast address from the remote
+     processor's filter list.
+
+2. **Responses from Remote Processor to Host**:
+
+   - `RPMSG_ETH_RESP_SET_MAC_ADDR`: Acknowledge the MAC address configuration.
+   - `RPMSG_ETH_RESP_ADD_MC_ADDR`: Acknowledge the addition of a multicast
+     address.
+   - `RPMSG_ETH_RESP_DEL_MC_ADDR`: Acknowledge the removal of a multicast
+     address.
+   - `RPMSG_ETH_RESP_SHM_INFO`: Respond with shared memory information such as
+     ``num_pkt_bufs``, ``buff_slot_size``, ``base_addr``, ``tx_offset``, and
+     ``rx_offset``.
+
+3. **Notifications from Remote Processor to Host**:
+
+   - `RPMSG_ETH_NOTIFY_PORT_UP`: Notify that the Ethernet port is up and ready
+     for communication.
+   - `RPMSG_ETH_NOTIFY_PORT_DOWN`: Notify that the Ethernet port is down.
+   - `RPMSG_ETH_NOTIFY_PORT_READY`: Notify that the Ethernet port is ready for
+     configuration.
+   - `RPMSG_ETH_NOTIFY_REMOTE_READY`: Notify that the remote processor is ready
+     for communication.
+
+4. **Runtime Information Exchanged**:
+
+   - **Link State**: Notifications about link state changes (e.g., link up or
+     link down).
+   - **Statistics**: Runtime statistics such as transmitted/received packets,
+     errors, and dropped packets.
+   - **Error Notifications**: Notifications about errors like buffer overflows
+     or invalid packets.
+   - **Configuration Updates**: Notifications about changes in configuration,
+     such as updated MTU or VLAN settings.
+
+How-To Guide for Vendors
+========================
+
+This section provides a guide for vendors to develop firmware for the remote
+processor that is compatible with the RPMSG Based Virtual Ethernet Driver.
+
+1. **Implement Shared Memory Layout**:
+
+   - Allocate a shared memory region for packet transmission and reception.
+   - Initialize the `MAGIC_NUM`, `num_pkt_bufs`, `buff_slot_size`, `base_addr`,
+     `tx_offset`, and `rx_offset`.
+
+2. **Magic Number Requirements**
+
+   - The firmware must write a unique magic number (for example, ``0xABCDABCD``)
+     to the `magic_num` field of both the head and tail structures in the shared
+     memory region.
+   - This magic number is used by the Linux driver to validate that the shared
+     memory region is correctly initialized and accessible.
+   - If the driver detects an incorrect magic number during the handshake, it
+     will abort initialization and report an error.
+   - Vendors must ensure the magic number matches the value expected by the
+     Linux driver (see the `RPMSG_ETH_SHM_MAGIC_NUM` macro in the driver
+     source).
+
+3. **Handle RPMSG Requests**:
+
+   - Implement handlers for the following RPMSG requests:
+
+      - `RPMSG_ETH_REQ_SHM_INFO`
+      - `RPMSG_ETH_REQ_SET_MAC_ADDR`
+      - `RPMSG_ETH_REQ_ADD_MC_ADDR`
+      - `RPMSG_ETH_REQ_DEL_MC_ADDR`
+
+4. **Send RPMSG Notifications**:
+
+   - Notify the host about the state of the Ethernet interface using the
+     notifications described above.
+
+5. **Send Runtime Information**:
+
+   - Implement mechanisms to send runtime information such as link state
+     changes, statistics, and error notifications.
+
+6. **Implement Packet Processing**:
+
+   - Process packets in the shared memory transmit and receive buffers.
+
+7. **Test the Firmware**:
+
+   - Use the RPMSG Based Virtual Ethernet Driver on the host to test packet
+     transmission and reception.
+
+Configuration
+=============
+
+The driver relies on the device tree for configuration. The shared memory region
+is specified using the `virtual-eth-shm` node in the device tree.
+
+Example Device Tree Node
+------------------------
+
+.. code-block:: dts
+
+   virtual-eth-shm {
+           compatible = "rpmsg,virtual-eth-shm";
+           reg = <0x80000000 0x10000>; /* Base address and size of shared memory */
+   };
+
+Limitations
+===========
+
+- The driver assumes a specific shared memory layout and may not work with other
+  configurations.
+- Multicast address filtering is limited to the capabilities of the underlying
+  RPMSG framework.
+- The driver currently supports only one transmit and one receive queue.
+
+References
+==========
+
+- RPMSG Framework Documentation: https://www.kernel.org/doc/html/latest/rpmsg.html
+- Linux Networking Documentation: https://www.kernel.org/doc/html/latest/networking/index.html
+
+Authors
+=======
+
+- MD Danish Anwar <danishanwar@ti.com>
-- 
2.34.1

[PATCH net-next 2/5] net: rpmsg-eth: Add basic rpmsg skeleton

[MD Danish Anwar]

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

[PATCH net-next 3/5] net: rpmsg-eth: Register device as netdev

[MD Danish Anwar]

Register the rpmsg-eth device as a netdev and enhance the rpmsg callback
function to handle shared memory for tx and rx buffers. Introduce
structures for shared memory layout, including head, buffer, and tail
indices. Add initialization for the netdev, including setting up MAC
address, MTU, and netdev operations. Allocate memory for tx and rx
buffers and map shared memory regions. Update the probe function to
initialize the netdev and set the device state. Add necessary headers,
constants, and enums for shared memory and state management. Define
shared memory layout and buffer structures for efficient data handling.
Implement helper macros for accessing private data and shared memory
buffers. Ensure proper error handling during memory allocation and
device registration.

Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
---
 drivers/net/ethernet/rpmsg_eth.c | 239 ++++++++++++++++++++++++++++++-
 drivers/net/ethernet/rpmsg_eth.h | 216 ++++++++++++++++++++++++++++
 2 files changed, 452 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/rpmsg_eth.c b/drivers/net/ethernet/rpmsg_eth.c
index 9a51619f9313..26f9eee6aeec 100644
--- a/drivers/net/ethernet/rpmsg_eth.c
+++ b/drivers/net/ethernet/rpmsg_eth.c
@@ -7,20 +7,173 @@
 #include <linux/of.h>
 #include "rpmsg_eth.h"
 
+/**
+ * rpmsg_eth_validate_handshake - Validate handshake parameters from remote
+ * @port: Pointer to rpmsg_eth_port structure
+ * @shm_info: Pointer to shared memory info received from remote
+ *
+ * Checks the magic numbers, base address, and TX/RX offsets in the handshake
+ * response to ensure they match expected values and are within valid ranges.
+ *
+ * Return: 0 on success, -EINVAL on validation failure.
+ */
+static int rpmsg_eth_validate_handshake(struct rpmsg_eth_port *port,
+					struct rpmsg_eth_shm *shm_info)
+{
+	if (port->tx_buffer->head->magic_num != RPMSG_ETH_SHM_MAGIC_NUM ||
+	    port->tx_buffer->tail->magic_num != RPMSG_ETH_SHM_MAGIC_NUM ||
+	    port->rx_buffer->head->magic_num != RPMSG_ETH_SHM_MAGIC_NUM ||
+	    port->rx_buffer->tail->magic_num != RPMSG_ETH_SHM_MAGIC_NUM) {
+		dev_err(port->common->dev, "Magic number mismatch in handshake: tx_head=0x%x, tx_tail=0x%x, rx_head=0x%x, rx_tail=0x%x\n",
+			port->tx_buffer->head->magic_num,
+			port->tx_buffer->tail->magic_num,
+			port->rx_buffer->head->magic_num,
+			port->rx_buffer->tail->magic_num);
+		return -EINVAL;
+	}
+
+	if (shm_info->base_addr != port->buf_start_addr) {
+		dev_err(port->common->dev, "Base address mismatch in handshake: expected=0x%x, received=0x%x\n",
+			port->buf_start_addr,
+			shm_info->base_addr);
+		return -EINVAL;
+	}
+
+	if (shm_info->tx_offset >= port->buf_size ||
+	    shm_info->rx_offset >= port->buf_size) {
+		dev_err(port->common->dev, "TX/RX offset out of range in handshake: tx_offset=0x%x, rx_offset=0x%x, size=0x%x\n",
+			shm_info->tx_offset,
+			shm_info->rx_offset,
+			port->buf_size);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static void rpmsg_eth_map_buffers(struct rpmsg_eth_port *port,
+				  struct message *msg)
+{
+	port->tx_buffer->head =
+		(struct rpmsg_eth_shm_index __force *)
+		 (ioremap(msg->resp_msg.shm_info.base_addr +
+			  msg->resp_msg.shm_info.tx_offset,
+			  sizeof(*port->tx_buffer->head)));
+
+	port->tx_buffer->buf->base_addr =
+		ioremap((msg->resp_msg.shm_info.base_addr +
+			 msg->resp_msg.shm_info.tx_offset +
+			 sizeof(*port->tx_buffer->head)),
+			 (msg->resp_msg.shm_info.num_pkt_bufs *
+			  msg->resp_msg.shm_info.buff_slot_size));
+
+	port->tx_buffer->tail =
+		(struct rpmsg_eth_shm_index __force *)
+		 (ioremap(msg->resp_msg.shm_info.base_addr +
+			  msg->resp_msg.shm_info.tx_offset +
+			  sizeof(*port->tx_buffer->head) +
+			  (msg->resp_msg.shm_info.num_pkt_bufs *
+			   msg->resp_msg.shm_info.buff_slot_size),
+			  sizeof(*port->tx_buffer->tail)));
+
+	port->rx_buffer->head =
+		(struct rpmsg_eth_shm_index __force *)
+		 (ioremap(msg->resp_msg.shm_info.base_addr +
+			  msg->resp_msg.shm_info.rx_offset,
+			  sizeof(*port->rx_buffer->head)));
+
+	port->rx_buffer->buf->base_addr =
+		ioremap(msg->resp_msg.shm_info.base_addr +
+			msg->resp_msg.shm_info.rx_offset +
+			sizeof(*port->rx_buffer->head),
+			(msg->resp_msg.shm_info.num_pkt_bufs *
+			 msg->resp_msg.shm_info.buff_slot_size));
+
+	port->rx_buffer->tail =
+		(struct rpmsg_eth_shm_index __force *)
+		 (ioremap(msg->resp_msg.shm_info.base_addr +
+			  msg->resp_msg.shm_info.rx_offset +
+			  sizeof(*port->rx_buffer->head) +
+			  (msg->resp_msg.shm_info.num_pkt_bufs *
+			   msg->resp_msg.shm_info.buff_slot_size),
+			  sizeof(*port->rx_buffer->tail)));
+}
+
+static void rpmsg_eth_unmap_buffers(struct rpmsg_eth_port *port)
+{
+	if (port->tx_buffer && port->tx_buffer->head) {
+		iounmap((void __iomem *)port->tx_buffer->head);
+		port->tx_buffer->head = NULL;
+	}
+	if (port->tx_buffer && port->tx_buffer->buf &&
+	    port->tx_buffer->buf->base_addr) {
+		iounmap((void __iomem *)port->tx_buffer->buf->base_addr);
+		port->tx_buffer->buf->base_addr = NULL;
+	}
+	if (port->tx_buffer && port->tx_buffer->tail) {
+		iounmap((void __iomem *)port->tx_buffer->tail);
+		port->tx_buffer->tail = NULL;
+	}
+
+	if (port->rx_buffer && port->rx_buffer->head) {
+		iounmap((void __iomem *)port->rx_buffer->head);
+		port->rx_buffer->head = NULL;
+	}
+	if (port->rx_buffer && port->rx_buffer->buf &&
+	    port->rx_buffer->buf->base_addr) {
+		iounmap((void __iomem *)port->rx_buffer->buf->base_addr);
+		port->rx_buffer->buf->base_addr = NULL;
+	}
+	if (port->rx_buffer && port->rx_buffer->tail) {
+		iounmap((void __iomem *)port->rx_buffer->tail);
+		port->rx_buffer->tail = NULL;
+	}
+}
+
 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;
+	struct rpmsg_eth_port *port = common->port;
 	u32 msg_type = msg->msg_hdr.msg_type;
+	u32 rpmsg_type;
 	int ret = 0;
 
 	switch (msg_type) {
 	case RPMSG_ETH_REQUEST_MSG:
+		rpmsg_type = msg->req_msg.type;
+		dev_dbg(common->dev, "Msg type = %d, RPMsg type = %d, Src Id = %d, Msg Id = %d\n",
+			msg_type, rpmsg_type, msg->msg_hdr.src_id, msg->req_msg.id);
+		break;
 	case RPMSG_ETH_RESPONSE_MSG:
+		rpmsg_type = msg->resp_msg.type;
+		dev_dbg(common->dev, "Msg type = %d, RPMsg type = %d, Src Id = %d, Msg Id = %d\n",
+			msg_type, rpmsg_type, msg->msg_hdr.src_id, msg->resp_msg.id);
+		switch (rpmsg_type) {
+		case RPMSG_ETH_RESP_SHM_INFO:
+			/* Retrieve Tx and Rx shared memory info from msg */
+			rpmsg_eth_map_buffers(port, msg);
+
+			port->rpmsg_eth_tx_max_buffers =
+				msg->resp_msg.shm_info.num_pkt_bufs;
+			port->rpmsg_eth_rx_max_buffers =
+				msg->resp_msg.shm_info.num_pkt_bufs;
+
+			/* Handshake validation */
+			ret = rpmsg_eth_validate_handshake(port, &msg->resp_msg.shm_info);
+			if (ret) {
+				dev_err(common->dev, "RPMSG handshake failed %d\n", ret);
+				rpmsg_eth_unmap_buffers(port);
+				return ret;
+			}
+			break;
+		}
+		break;
 	case RPMSG_ETH_NOTIFY_MSG:
-		dev_dbg(common->dev, "Msg type = %d, Src Id = %d\n",
-			msg_type, msg->msg_hdr.src_id);
+		rpmsg_type = msg->notify_msg.type;
+		dev_dbg(common->dev, "Msg type = %d, RPMsg type = %d, Src Id = %d, Msg Id = %d\n",
+			msg_type, rpmsg_type, msg->msg_hdr.src_id, msg->notify_msg.id);
 		break;
 	default:
 		dev_err(common->dev, "Invalid msg type\n");
@@ -80,6 +233,76 @@ static int rpmsg_eth_get_shm_info(struct rpmsg_eth_common *common)
 	return 0;
 }
 
+static int rpmsg_eth_init_ndev(struct rpmsg_eth_common *common)
+{
+	struct device *dev = &common->rpdev->dev;
+	struct rpmsg_eth_ndev_priv *ndev_priv;
+	struct rpmsg_eth_port *port;
+	static u32 port_id;
+	int err;
+
+	port = common->port;
+	port->common = common;
+	port->port_id = port_id++;
+
+	port->ndev = devm_alloc_etherdev_mqs(common->dev, sizeof(*ndev_priv),
+					     RPMSG_ETH_MAX_TX_QUEUES,
+					     RPMSG_ETH_MAX_RX_QUEUES);
+
+	if (!port->ndev) {
+		dev_err(dev, "error allocating net_device\n");
+		return -ENOMEM;
+	}
+
+	ndev_priv = netdev_priv(port->ndev);
+	ndev_priv->port = port;
+	SET_NETDEV_DEV(port->ndev, dev);
+
+	port->ndev->min_mtu = RPMSG_ETH_MIN_PACKET_SIZE;
+	port->ndev->max_mtu = MAX_MTU;
+
+	if (!is_valid_ether_addr(port->ndev->dev_addr)) {
+		eth_hw_addr_random(port->ndev);
+		dev_dbg(dev, "Using random MAC address %pM\n", port->ndev->dev_addr);
+	}
+
+	/* Allocate memory to test without actual RPMsg handshaking */
+	port->tx_buffer =
+		devm_kzalloc(dev, sizeof(*port->tx_buffer), GFP_KERNEL);
+	if (!port->tx_buffer) {
+		dev_err(dev, "Memory not available\n");
+		return -ENOMEM;
+	}
+
+	port->tx_buffer->buf =
+		devm_kzalloc(dev, sizeof(*port->tx_buffer->buf), GFP_KERNEL);
+	if (!port->tx_buffer->buf) {
+		dev_err(dev, "Memory not available\n");
+		return -ENOMEM;
+	}
+
+	port->rx_buffer =
+		devm_kzalloc(dev, sizeof(*port->rx_buffer), GFP_KERNEL);
+	if (!port->rx_buffer) {
+		dev_err(dev, "Memory not available\n");
+		return -ENOMEM;
+	}
+
+	port->rx_buffer->buf =
+		devm_kzalloc(dev, sizeof(*port->rx_buffer->buf), GFP_KERNEL);
+	if (!port->rx_buffer->buf) {
+		dev_err(dev, "Memory not available\n");
+		return -ENOMEM;
+	}
+	netif_carrier_off(port->ndev);
+
+	err = register_netdev(port->ndev);
+
+	if (err)
+		dev_err(dev, "error registering rpmsg_eth net device %d\n", err);
+	return 0;
+}
+
 static int rpmsg_eth_probe(struct rpmsg_device *rpdev)
 {
 	struct device *dev = &rpdev->dev;
@@ -95,17 +318,27 @@ static int rpmsg_eth_probe(struct rpmsg_device *rpdev)
 	common->port = devm_kzalloc(dev, sizeof(*common->port), GFP_KERNEL);
 	common->dev = dev;
 	common->rpdev = rpdev;
+	common->state = RPMSG_ETH_STATE_PROBE;
 
 	ret = rpmsg_eth_get_shm_info(common);
 	if (ret)
 		return ret;
 
+	/* Register the network device */
+	ret = rpmsg_eth_init_ndev(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");
+	struct rpmsg_eth_common *common = dev_get_drvdata(&rpdev->dev);
+	struct rpmsg_eth_port *port = common->port;
+
+	/* Unmap ioremap'd regions */
+	rpmsg_eth_unmap_buffers(port);
 }
 
 static struct rpmsg_device_id rpmsg_eth_rpmsg_id_table[] = {
diff --git a/drivers/net/ethernet/rpmsg_eth.h b/drivers/net/ethernet/rpmsg_eth.h
index 56dabd139643..aa43030f3d72 100644
--- a/drivers/net/ethernet/rpmsg_eth.h
+++ b/drivers/net/ethernet/rpmsg_eth.h
@@ -18,6 +18,27 @@
 #include <linux/rpmsg.h>
 
 #define RPMSG_ETH_SHM_MAGIC_NUM 0xABCDABCD
+#define RPMSG_ETH_MIN_PACKET_SIZE ETH_ZLEN
+#define RPMSG_ETH_PACKET_BUFFER_SIZE   1540
+#define MAX_MTU   (RPMSG_ETH_PACKET_BUFFER_SIZE - (ETH_HLEN + ETH_FCS_LEN + VLAN_HLEN))
+
+#define RPMSG_ETH_MAX_TX_QUEUES 1
+#define RPMSG_ETH_MAX_RX_QUEUES 1
+#define PKT_LEN_SIZE_TYPE sizeof(u32)
+#define MAGIC_NUM_SIZE_TYPE sizeof(u32)
+
+/* 4 bytes to hold packet length and RPMSG_ETH_PACKET_BUFFER_SIZE to hold packet */
+#define RPMSG_ETH_BUFFER_SIZE \
+	(RPMSG_ETH_PACKET_BUFFER_SIZE + PKT_LEN_SIZE_TYPE + MAGIC_NUM_SIZE_TYPE)
+
+#define RX_POLL_TIMEOUT_JIFFIES usecs_to_jiffies(1000)
+#define RX_POLL_JIFFIES (jiffies + RX_POLL_TIMEOUT_JIFFIES)
+#define STATE_MACHINE_TIME_JIFFIES msecs_to_jiffies(100)
+#define RPMSG_ETH_REQ_TIMEOUT_JIFFIES msecs_to_jiffies(100)
+
+#define rpmsg_eth_ndev_to_priv(ndev) ((struct rpmsg_eth_ndev_priv *)netdev_priv(ndev))
+#define rpmsg_eth_ndev_to_port(ndev) (rpmsg_eth_ndev_to_priv(ndev)->port)
+#define rpmsg_eth_ndev_to_common(ndev) (rpmsg_eth_ndev_to_port(ndev)->common)
 
 enum rpmsg_eth_msg_type {
 	RPMSG_ETH_REQUEST_MSG = 0,
@@ -25,6 +46,89 @@ enum rpmsg_eth_msg_type {
 	RPMSG_ETH_NOTIFY_MSG,
 };
 
+enum rpmsg_eth_rpmsg_type {
+	/* Request types */
+	RPMSG_ETH_REQ_SHM_INFO = 0,
+	RPMSG_ETH_REQ_SET_MAC_ADDR,
+
+	/* Response types */
+	RPMSG_ETH_RESP_SHM_INFO,
+	RPMSG_ETH_RESP_SET_MAC_ADDR,
+
+	/* Notification types */
+	RPMSG_ETH_NOTIFY_PORT_UP,
+	RPMSG_ETH_NOTIFY_PORT_DOWN,
+	RPMSG_ETH_NOTIFY_PORT_READY,
+	RPMSG_ETH_NOTIFY_REMOTE_READY,
+};
+
+/**
+ * struct rpmsg_eth_shm - Shared memory layout for RPMsg Ethernet
+ * @num_pkt_bufs: Number of packet buffers available in the shared memory
+ * @buff_slot_size: Size of each buffer slot in bytes
+ * @base_addr: Base address of the shared memory region
+ * @tx_offset: Offset for the transmit buffer region within the shared memory
+ * @rx_offset: Offset for the receive buffer region within the shared memory
+ *
+ * This structure defines the layout of the shared memory used for
+ * communication between the host and the remote processor in an RPMsg
+ * Ethernet driver. It specifies the configuration and memory offsets
+ * required for transmitting and receiving Ethernet packets.
+ */
+struct rpmsg_eth_shm {
+	u32 num_pkt_bufs;
+	u32 buff_slot_size;
+	u32 base_addr;
+	u32 tx_offset;
+	u32 rx_offset;
+} __packed;
+
+/**
+ * struct rpmsg_eth_mac_addr - MAC address information for RPMSG Ethernet
+ * @addr: MAC address
+ */
+struct rpmsg_eth_mac_addr {
+	char addr[ETH_ALEN];
+} __packed;
+
+/**
+ * struct request_message - request message structure for RPMSG Ethernet
+ * @type: Request Type
+ * @id: Request ID
+ * @mac_addr: MAC address (if request type is MAC address related)
+ */
+struct request_message {
+	u32 type;
+	u32 id;
+	union {
+		struct rpmsg_eth_mac_addr mac_addr;
+	};
+} __packed;
+
+/**
+ * struct response_message - response message structure for RPMSG Ethernet
+ * @type: Response Type
+ * @id: Response ID
+ * @shm_info: rpmsg shared memory info
+ */
+struct response_message {
+	u32 type;
+	u32 id;
+	union {
+		struct rpmsg_eth_shm shm_info;
+	};
+} __packed;
+
+/**
+ * struct notify_message - notification message structure for RPMSG Ethernet
+ * @type: Notify Type
+ * @id: Notify ID
+ */
+struct notify_message {
+	u32 type;
+	u32 id;
+} __packed;
+
 /**
  * struct message_header - message header structure for RPMSG Ethernet
  * @src_id: Source endpoint ID
@@ -40,22 +144,116 @@ struct message_header {
  *
  * @msg_hdr: Message header contains source and destination endpoint and
  *          the type of message
+ * @req_msg: Request message structure contains the request type and ID
+ * @resp_msg: Response message structure contains the response type and ID
+ * @notify_msg: Notification message structure contains the notify type and ID
  *
  * This structure is used to send and receive messages between the RPMSG
  * Ethernet ports.
  */
 struct message {
 	struct message_header msg_hdr;
+	union {
+		struct request_message req_msg;
+		struct response_message resp_msg;
+		struct notify_message notify_msg;
+	};
+} __packed;
+
+/*      Shared Memory Layout
+ *
+ *	---------------------------	*****************
+ *	|        MAGIC_NUM        |	 rpmsg_eth_shm_head
+ *	|          HEAD           |
+ *	---------------------------	*****************
+ *	|        MAGIC_NUM        |
+ *	|        PKT_1_LEN        |
+ *	|          PKT_1          |
+ *	---------------------------
+ *	|        MAGIC_NUM        |
+ *	|        PKT_2_LEN        |	 rpmsg_eth_shm_buf
+ *	|          PKT_2          |
+ *	---------------------------
+ *	|           .             |
+ *	|           .             |
+ *	---------------------------
+ *	|        MAGIC_NUM        |
+ *	|        PKT_N_LEN        |
+ *	|          PKT_N          |
+ *	---------------------------	****************
+ *	|        MAGIC_NUM        |      rpmsg_eth_shm_tail
+ *	|          TAIL           |
+ *	---------------------------	****************
+ */
+
+struct rpmsg_eth_shm_index {
+	u32 magic_num;
+	u32 index;
+}  __packed;
+
+/**
+ * struct rpmsg_eth_shm_buf - shared memory buffer structure for RPMSG Ethernet
+ * @base_addr: Base address of the buffer
+ * @magic_num: Magic number for buffer validation
+ */
+struct rpmsg_eth_shm_buf {
+	void __iomem *base_addr;
+	u32 magic_num;
+} __packed;
+
+/**
+ * struct rpmsg_eth_shared_mem - shared memory structure for RPMSG Ethernet
+ * @head: Head of the shared memory
+ * @buf: Buffer of the shared memory
+ * @tail: Tail of the shared memory
+ */
+struct rpmsg_eth_shared_mem {
+	struct rpmsg_eth_shm_index *head;
+	struct rpmsg_eth_shm_buf *buf;
+	struct rpmsg_eth_shm_index *tail;
 } __packed;
 
+enum rpmsg_eth_state {
+	RPMSG_ETH_STATE_PROBE,
+	RPMSG_ETH_STATE_OPEN,
+	RPMSG_ETH_STATE_CLOSE,
+	RPMSG_ETH_STATE_READY,
+	RPMSG_ETH_STATE_RUNNING,
+
+};
+
 /**
  * struct rpmsg_eth_common - common structure for RPMSG Ethernet
  * @rpdev: RPMSG device
+ * @send_msg: Send message
+ * @recv_msg: Receive message
  * @port: Ethernet port
  * @dev: Device
+ * @state: Interface state
+ * @state_work: Delayed work for state machine
  */
 struct rpmsg_eth_common {
 	struct rpmsg_device *rpdev;
+	/** @send_msg_lock: Lock for sending RPMSG */
+	spinlock_t send_msg_lock;
+	/** @recv_msg_lock: Lock for receiving RPMSG */
+	spinlock_t recv_msg_lock;
+	struct message send_msg;
+	struct message recv_msg;
+	struct rpmsg_eth_port *port;
+	struct device *dev;
+	enum rpmsg_eth_state state;
+	/** @state_lock: Lock for changing interface state */
+	struct mutex state_lock;
+	struct delayed_work state_work;
+};
+
+/**
+ * struct rpmsg_eth_ndev_priv - private structure for RPMSG Ethernet net device
+ * @port: Ethernet port
+ * @dev: Device
+ */
+struct rpmsg_eth_ndev_priv {
 	struct rpmsg_eth_port *port;
 	struct device *dev;
 };
@@ -65,11 +263,29 @@ struct rpmsg_eth_common {
  * @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
+ * @tx_buffer: Write buffer for data to be consumed by remote side
+ * @rx_buffer: Read buffer for data to be consumed by this driver
+ * @rx_timer: Timer for rx polling
+ * @rx_napi: NAPI structure for rx polling
+ * @local_mac_addr: Local MAC address
+ * @ndev: Network device
+ * @rpmsg_eth_tx_max_buffers: Maximum number of tx buffers
+ * @rpmsg_eth_rx_max_buffers: Maximum number of rx buffers
+ * @port_id: Port ID
  */
 struct rpmsg_eth_port {
 	struct rpmsg_eth_common *common;
 	u32 buf_start_addr;
 	u32 buf_size;
+	struct rpmsg_eth_shared_mem *tx_buffer;
+	struct rpmsg_eth_shared_mem *rx_buffer;
+	struct timer_list rx_timer;
+	struct napi_struct rx_napi;
+	u8 local_mac_addr[ETH_ALEN];
+	struct net_device *ndev;
+	u32 rpmsg_eth_tx_max_buffers;
+	u32 rpmsg_eth_rx_max_buffers;
+	u32 port_id;
 };
 
 #endif /* __RPMSG_ETH_H__ */
-- 
2.34.1

[PATCH net-next 4/5] net: rpmsg-eth: Add netdev ops

[MD Danish Anwar]

Add netdev ops for rpmsg-eth driver. This patch introduces the netdev
operations for the rpmsg-eth driver, enabling the driver to interact
with the Linux networking stack. The following functionalities are
implemented:

1. `ndo_open` and `ndo_stop`:
	- Handles the initialization and cleanup of the network device
	  during open and stop operations.
	- Manages the state transitions of the rpmsg-eth driver.

2. `ndo_start_xmit`:
	- Implements the transmit functionality by copying data from the
	  skb to the shared memory buffer and updating the head index.

3. `ndo_set_mac_address`:
	- Allows setting the MAC address of the network device and sends
	  the updated MAC address to the remote processor.

4. RX Path:
	- Adds a timer-based mechanism to poll for received packets in
	  shared memory.
	- Implements NAPI-based packet processing to handle received
	  packets efficiently.

5. State Machine:
	- Introduces a state machine to manage the driver's state
	  transitions, such as PROBE, OPEN, READY, and RUNNING.

6. Initialization:
	- Adds necessary initialization for locks, timers, and work
	  structures.
	- Registers the network device and sets up NAPI and RX timer.

7. Cleanup:
	- Ensures proper cleanup of resources during driver removal,
	  including NAPI and timers.

This patch enhances the rpmsg-eth driver to function as a fully
operational network device in the Linux kernel.

Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
---
 drivers/net/ethernet/rpmsg_eth.c | 319 +++++++++++++++++++++++++++++++
 drivers/net/ethernet/rpmsg_eth.h |   2 +
 2 files changed, 321 insertions(+)

diff --git a/drivers/net/ethernet/rpmsg_eth.c b/drivers/net/ethernet/rpmsg_eth.c
index 26f9eee6aeec..4efa9b634f8b 100644
--- a/drivers/net/ethernet/rpmsg_eth.c
+++ b/drivers/net/ethernet/rpmsg_eth.c
@@ -130,6 +130,109 @@ static void rpmsg_eth_unmap_buffers(struct rpmsg_eth_port *port)
 	}
 }
 
+static int create_request(struct rpmsg_eth_common *common,
+			  enum rpmsg_eth_rpmsg_type rpmsg_type)
+{
+	struct message *msg = &common->send_msg;
+	int ret = 0;
+
+	msg->msg_hdr.src_id = common->port->port_id;
+	msg->req_msg.type = rpmsg_type;
+
+	switch (rpmsg_type) {
+	case RPMSG_ETH_REQ_SHM_INFO:
+		msg->msg_hdr.msg_type = RPMSG_ETH_REQUEST_MSG;
+		break;
+	case RPMSG_ETH_REQ_SET_MAC_ADDR:
+		msg->msg_hdr.msg_type = RPMSG_ETH_REQUEST_MSG;
+		ether_addr_copy(msg->req_msg.mac_addr.addr,
+				common->port->ndev->dev_addr);
+		break;
+	case RPMSG_ETH_NOTIFY_PORT_UP:
+	case RPMSG_ETH_NOTIFY_PORT_DOWN:
+		msg->msg_hdr.msg_type = RPMSG_ETH_NOTIFY_MSG;
+		break;
+	default:
+		ret = -EINVAL;
+		dev_err(common->dev, "Invalid RPMSG request\n");
+	}
+	return ret;
+}
+
+static int rpmsg_eth_create_send_request(struct rpmsg_eth_common *common,
+					 enum rpmsg_eth_rpmsg_type rpmsg_type,
+					 bool wait)
+{
+	unsigned long flags;
+	int ret = 0;
+
+	if (wait)
+		reinit_completion(&common->sync_msg);
+
+	spin_lock_irqsave(&common->send_msg_lock, flags);
+	ret = create_request(common, rpmsg_type);
+	if (ret)
+		goto release_lock;
+
+	ret = rpmsg_send(common->rpdev->ept, (void *)(&common->send_msg),
+			 sizeof(common->send_msg));
+	if (ret) {
+		dev_err(common->dev, "Failed to send RPMSG message\n");
+		goto release_lock;
+	}
+
+	spin_unlock_irqrestore(&common->send_msg_lock, flags);
+	if (wait) {
+		ret = wait_for_completion_timeout(&common->sync_msg,
+						  RPMSG_ETH_REQ_TIMEOUT_JIFFIES);
+
+		if (!ret) {
+			dev_err(common->dev, "Failed to receive response within %ld jiffies\n",
+				RPMSG_ETH_REQ_TIMEOUT_JIFFIES);
+			return -ETIMEDOUT;
+		}
+		ret = 0;
+	}
+	return ret;
+release_lock:
+	spin_unlock_irqrestore(&common->send_msg_lock, flags);
+	return ret;
+}
+
+static void rpmsg_eth_state_machine(struct work_struct *work)
+{
+	struct delayed_work *dwork = to_delayed_work(work);
+	struct rpmsg_eth_common *common;
+	struct rpmsg_eth_port *port;
+	int ret;
+
+	common = container_of(dwork, struct rpmsg_eth_common, state_work);
+	port = common->port;
+
+	mutex_lock(&common->state_lock);
+
+	switch (common->state) {
+	case RPMSG_ETH_STATE_PROBE:
+		break;
+	case RPMSG_ETH_STATE_OPEN:
+		rpmsg_eth_create_send_request(common, RPMSG_ETH_REQ_SHM_INFO, false);
+		break;
+	case RPMSG_ETH_STATE_CLOSE:
+		break;
+	case RPMSG_ETH_STATE_READY:
+		ret = rpmsg_eth_create_send_request(common, RPMSG_ETH_REQ_SET_MAC_ADDR, false);
+		if (!ret) {
+			napi_enable(&port->rx_napi);
+			netif_carrier_on(port->ndev);
+			mod_timer(&port->rx_timer, RX_POLL_TIMEOUT_JIFFIES);
+		}
+		break;
+	case RPMSG_ETH_STATE_RUNNING:
+		break;
+	}
+	mutex_unlock(&common->state_lock);
+}
+
 static int rpmsg_eth_rpmsg_cb(struct rpmsg_device *rpdev, void *data, int len,
 			      void *priv, u32 src)
 {
@@ -167,6 +270,17 @@ static int rpmsg_eth_rpmsg_cb(struct rpmsg_device *rpdev, void *data, int len,
 				rpmsg_eth_unmap_buffers(port);
 				return ret;
 			}
+
+			mutex_lock(&common->state_lock);
+			common->state = RPMSG_ETH_STATE_READY;
+			mutex_unlock(&common->state_lock);
+
+			mod_delayed_work(system_wq,
+					 &common->state_work,
+					 STATE_MACHINE_TIME_JIFFIES);
+
+			break;
+		case RPMSG_ETH_RESP_SET_MAC_ADDR:
 			break;
 		}
 		break;
@@ -174,6 +288,20 @@ static int rpmsg_eth_rpmsg_cb(struct rpmsg_device *rpdev, void *data, int len,
 		rpmsg_type = msg->notify_msg.type;
 		dev_dbg(common->dev, "Msg type = %d, RPMsg type = %d, Src Id = %d, Msg Id = %d\n",
 			msg_type, rpmsg_type, msg->msg_hdr.src_id, msg->notify_msg.id);
+		switch (rpmsg_type) {
+		case RPMSG_ETH_NOTIFY_REMOTE_READY:
+			mutex_lock(&common->state_lock);
+			common->state = RPMSG_ETH_STATE_RUNNING;
+			mutex_unlock(&common->state_lock);
+
+			mod_delayed_work(system_wq,
+					 &common->state_work,
+					 STATE_MACHINE_TIME_JIFFIES);
+			break;
+		case RPMSG_ETH_NOTIFY_PORT_UP:
+		case RPMSG_ETH_NOTIFY_PORT_DOWN:
+			break;
+		}
 		break;
 	default:
 		dev_err(common->dev, "Invalid msg type\n");
@@ -233,6 +361,185 @@ static int rpmsg_eth_get_shm_info(struct rpmsg_eth_common *common)
 	return 0;
 }
 
+static void rpmsg_eth_rx_timer(struct timer_list *timer)
+{
+	struct rpmsg_eth_port *port = timer_container_of(port, timer, rx_timer);
+	struct napi_struct *napi;
+	int num_pkts = 0;
+	u32 head, tail;
+
+	head = port->rx_buffer->head->index;
+	tail = port->rx_buffer->tail->index;
+
+	num_pkts = tail - head;
+	num_pkts = num_pkts >= 0 ? num_pkts :
+				   (num_pkts + port->rpmsg_eth_rx_max_buffers);
+
+	napi = &port->rx_napi;
+	if (num_pkts && likely(napi_schedule_prep(napi)))
+		__napi_schedule(napi);
+	else
+		mod_timer(&port->rx_timer, RX_POLL_JIFFIES);
+}
+
+static int rpmsg_eth_rx_packets(struct napi_struct *napi, int budget)
+{
+	struct rpmsg_eth_port *port = container_of(napi, struct rpmsg_eth_port, rx_napi);
+	u32 count, process_pkts;
+	struct sk_buff *skb;
+	u32 head, tail;
+	int num_pkts;
+	u32 pkt_len;
+
+	head = port->rx_buffer->head->index;
+	tail = port->rx_buffer->tail->index;
+
+	num_pkts = head - tail;
+
+	num_pkts = num_pkts >= 0 ? num_pkts :
+				   (num_pkts + port->rpmsg_eth_rx_max_buffers);
+	process_pkts = min(num_pkts, budget);
+	count = 0;
+	while (count < process_pkts) {
+		memcpy_fromio((void *)&pkt_len,
+			      (void __iomem *)(port->rx_buffer->buf->base_addr +
+			      MAGIC_NUM_SIZE_TYPE +
+			      (((tail + count) % port->rpmsg_eth_rx_max_buffers) *
+			      RPMSG_ETH_BUFFER_SIZE)),
+			      PKT_LEN_SIZE_TYPE);
+		/* Start building the skb */
+		skb = napi_alloc_skb(napi, pkt_len);
+		if (!skb) {
+			port->ndev->stats.rx_dropped++;
+			goto rx_dropped;
+		}
+
+		skb->dev = port->ndev;
+		skb_put(skb, pkt_len);
+		memcpy_fromio((void *)skb->data,
+			      (void __iomem *)(port->rx_buffer->buf->base_addr +
+			      PKT_LEN_SIZE_TYPE + MAGIC_NUM_SIZE_TYPE +
+			      (((tail + count) % port->rpmsg_eth_rx_max_buffers) *
+			      RPMSG_ETH_BUFFER_SIZE)),
+			      pkt_len);
+
+		skb->protocol = eth_type_trans(skb, port->ndev);
+
+		/* Push skb into network stack */
+		napi_gro_receive(napi, skb);
+
+		count++;
+		port->ndev->stats.rx_packets++;
+		port->ndev->stats.rx_bytes += skb->len;
+	}
+
+rx_dropped:
+
+	if (num_pkts) {
+		port->rx_buffer->tail->index =
+			(port->rx_buffer->tail->index + count) %
+			port->rpmsg_eth_rx_max_buffers;
+
+		if (num_pkts < budget && napi_complete_done(napi, count))
+			mod_timer(&port->rx_timer, RX_POLL_TIMEOUT_JIFFIES);
+	}
+
+	return count;
+}
+
+static int rpmsg_eth_ndo_open(struct net_device *ndev)
+{
+	struct rpmsg_eth_common *common = rpmsg_eth_ndev_to_common(ndev);
+
+	mutex_lock(&common->state_lock);
+	common->state = RPMSG_ETH_STATE_OPEN;
+	mutex_unlock(&common->state_lock);
+	mod_delayed_work(system_wq, &common->state_work, msecs_to_jiffies(100));
+
+	return 0;
+}
+
+static int rpmsg_eth_ndo_stop(struct net_device *ndev)
+{
+	struct rpmsg_eth_common *common = rpmsg_eth_ndev_to_common(ndev);
+	struct rpmsg_eth_port *port = rpmsg_eth_ndev_to_port(ndev);
+
+	mutex_lock(&common->state_lock);
+	common->state = RPMSG_ETH_STATE_CLOSE;
+	mutex_unlock(&common->state_lock);
+
+	netif_carrier_off(port->ndev);
+
+	cancel_delayed_work_sync(&common->state_work);
+	timer_delete_sync(&port->rx_timer);
+	napi_disable(&port->rx_napi);
+
+	return 0;
+}
+
+static netdev_tx_t rpmsg_eth_start_xmit(struct sk_buff *skb, struct net_device *ndev)
+{
+	struct rpmsg_eth_port *port = rpmsg_eth_ndev_to_port(ndev);
+	u32 head, tail;
+	int num_pkts;
+	u32 len;
+
+	len = skb_headlen(skb);
+	head = port->tx_buffer->head->index;
+	tail = port->tx_buffer->tail->index;
+
+	/* If the buffer queue is full, then drop packet */
+	num_pkts = head - tail;
+	num_pkts = num_pkts >= 0 ? num_pkts :
+				   (num_pkts + port->rpmsg_eth_tx_max_buffers);
+
+	if ((num_pkts + 1) == port->rpmsg_eth_tx_max_buffers) {
+		netdev_warn(ndev, "Tx buffer full %d\n", num_pkts);
+		goto ring_full;
+	}
+	/* Copy length */
+	memcpy_toio((void __iomem *)port->tx_buffer->buf->base_addr +
+			    MAGIC_NUM_SIZE_TYPE +
+			    (port->tx_buffer->head->index * RPMSG_ETH_BUFFER_SIZE),
+		    (void *)&len, PKT_LEN_SIZE_TYPE);
+	/* Copy data to shared mem */
+	memcpy_toio((void __iomem *)(port->tx_buffer->buf->base_addr +
+			     MAGIC_NUM_SIZE_TYPE + PKT_LEN_SIZE_TYPE +
+			     (port->tx_buffer->head->index * RPMSG_ETH_BUFFER_SIZE)),
+		    (void *)skb->data, len);
+	port->tx_buffer->head->index =
+		(port->tx_buffer->head->index + 1) % port->rpmsg_eth_tx_max_buffers;
+
+	ndev->stats.tx_packets++;
+	ndev->stats.tx_bytes += skb->len;
+
+	dev_consume_skb_any(skb);
+	return NETDEV_TX_OK;
+
+ring_full:
+	return NETDEV_TX_BUSY;
+}
+
+static int rpmsg_eth_set_mac_address(struct net_device *ndev, void *addr)
+{
+	struct rpmsg_eth_common *common = rpmsg_eth_ndev_to_common(ndev);
+	int ret;
+
+	ret = eth_mac_addr(ndev, addr);
+
+	if (ret < 0)
+		return ret;
+	ret = rpmsg_eth_create_send_request(common, RPMSG_ETH_REQ_SET_MAC_ADDR, false);
+	return ret;
+}
+
+static const struct net_device_ops rpmsg_eth_netdev_ops = {
+	.ndo_open = rpmsg_eth_ndo_open,
+	.ndo_stop = rpmsg_eth_ndo_stop,
+	.ndo_start_xmit = rpmsg_eth_start_xmit,
+	.ndo_set_mac_address = rpmsg_eth_set_mac_address,
+};
+
 static int rpmsg_eth_init_ndev(struct rpmsg_eth_common *common)
 {
 	struct device *dev = &common->rpdev->dev;
@@ -256,6 +563,7 @@ static int rpmsg_eth_init_ndev(struct rpmsg_eth_common *common)
 
 	ndev_priv = netdev_priv(port->ndev);
 	ndev_priv->port = port;
+	port->ndev->netdev_ops = &rpmsg_eth_netdev_ops;
 	SET_NETDEV_DEV(port->ndev, dev);
 
 	port->ndev->min_mtu = RPMSG_ETH_MIN_PACKET_SIZE;
@@ -296,6 +604,8 @@ static int rpmsg_eth_init_ndev(struct rpmsg_eth_common *common)
 	}
 	netif_carrier_off(port->ndev);
 
+	netif_napi_add(port->ndev, &port->rx_napi, rpmsg_eth_rx_packets);
+	timer_setup(&port->rx_timer, rpmsg_eth_rx_timer, 0);
 	err = register_netdev(port->ndev);
 
 	if (err)
@@ -324,6 +634,12 @@ static int rpmsg_eth_probe(struct rpmsg_device *rpdev)
 	if (ret)
 		return ret;
 
+	spin_lock_init(&common->send_msg_lock);
+	spin_lock_init(&common->recv_msg_lock);
+	mutex_init(&common->state_lock);
+	INIT_DELAYED_WORK(&common->state_work, rpmsg_eth_state_machine);
+	init_completion(&common->sync_msg);
+
 	/* Register the network device */
 	ret = rpmsg_eth_init_ndev(common);
 	if (ret)
@@ -339,6 +655,9 @@ static void rpmsg_eth_rpmsg_remove(struct rpmsg_device *rpdev)
 
 	/* Unmap ioremap'd regions */
 	rpmsg_eth_unmap_buffers(port);
+
+	netif_napi_del(&port->rx_napi);
+	timer_delete_sync(&port->rx_timer);
 }
 
 static struct rpmsg_device_id rpmsg_eth_rpmsg_id_table[] = {
diff --git a/drivers/net/ethernet/rpmsg_eth.h b/drivers/net/ethernet/rpmsg_eth.h
index aa43030f3d72..d7e4d53c8de4 100644
--- a/drivers/net/ethernet/rpmsg_eth.h
+++ b/drivers/net/ethernet/rpmsg_eth.h
@@ -231,6 +231,7 @@ enum rpmsg_eth_state {
  * @dev: Device
  * @state: Interface state
  * @state_work: Delayed work for state machine
+ * @sync_msg: Completion for synchronous message
  */
 struct rpmsg_eth_common {
 	struct rpmsg_device *rpdev;
@@ -246,6 +247,7 @@ struct rpmsg_eth_common {
 	/** @state_lock: Lock for changing interface state */
 	struct mutex state_lock;
 	struct delayed_work state_work;
+	struct completion sync_msg;
 };
 
 /**
-- 
2.34.1

[PATCH net-next 5/5] net: rpmsg-eth: Add support for multicast filtering

[MD Danish Anwar]

Add support for multicast filtering for ICVE driver. Implement the
ndo_set_rx_mode callback as icve_set_rx_mode() API. rx_mode_workqueue is
initialized in icve_rpmsg_probe() and queued in icve_set_rx_mode().

Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
---
 drivers/net/ethernet/rpmsg_eth.c | 63 ++++++++++++++++++++++++++++++++
 drivers/net/ethernet/rpmsg_eth.h | 12 ++++++
 2 files changed, 75 insertions(+)

diff --git a/drivers/net/ethernet/rpmsg_eth.c b/drivers/net/ethernet/rpmsg_eth.c
index 4efa9b634f8b..a77fc4f3f769 100644
--- a/drivers/net/ethernet/rpmsg_eth.c
+++ b/drivers/net/ethernet/rpmsg_eth.c
@@ -148,6 +148,11 @@ static int create_request(struct rpmsg_eth_common *common,
 		ether_addr_copy(msg->req_msg.mac_addr.addr,
 				common->port->ndev->dev_addr);
 		break;
+	case RPMSG_ETH_REQ_ADD_MC_ADDR:
+	case RPMSG_ETH_REQ_DEL_MC_ADDR:
+		ether_addr_copy(msg->req_msg.mac_addr.addr,
+				common->mcast_addr);
+		break;
 	case RPMSG_ETH_NOTIFY_PORT_UP:
 	case RPMSG_ETH_NOTIFY_PORT_DOWN:
 		msg->msg_hdr.msg_type = RPMSG_ETH_NOTIFY_MSG;
@@ -199,6 +204,22 @@ static int rpmsg_eth_create_send_request(struct rpmsg_eth_common *common,
 	return ret;
 }
 
+static int rpmsg_eth_add_mc_addr(struct net_device *ndev, const u8 *addr)
+{
+	struct rpmsg_eth_common *common = rpmsg_eth_ndev_to_common(ndev);
+
+	ether_addr_copy(common->mcast_addr, addr);
+	return rpmsg_eth_create_send_request(common, RPMSG_ETH_REQ_ADD_MC_ADDR, true);
+}
+
+static int rpmsg_eth_del_mc_addr(struct net_device *ndev, const u8 *addr)
+{
+	struct rpmsg_eth_common *common = rpmsg_eth_ndev_to_common(ndev);
+
+	ether_addr_copy(common->mcast_addr, addr);
+	return rpmsg_eth_create_send_request(common, RPMSG_ETH_REQ_DEL_MC_ADDR, true);
+}
+
 static void rpmsg_eth_state_machine(struct work_struct *work)
 {
 	struct delayed_work *dwork = to_delayed_work(work);
@@ -282,6 +303,10 @@ static int rpmsg_eth_rpmsg_cb(struct rpmsg_device *rpdev, void *data, int len,
 			break;
 		case RPMSG_ETH_RESP_SET_MAC_ADDR:
 			break;
+		case RPMSG_ETH_RESP_ADD_MC_ADDR:
+		case RPMSG_ETH_RESP_DEL_MC_ADDR:
+			complete(&common->sync_msg);
+			break;
 		}
 		break;
 	case RPMSG_ETH_NOTIFY_MSG:
@@ -470,10 +495,15 @@ static int rpmsg_eth_ndo_stop(struct net_device *ndev)
 
 	netif_carrier_off(port->ndev);
 
+	__dev_mc_unsync(ndev, rpmsg_eth_del_mc_addr);
+	__hw_addr_init(&common->mc_list);
+
 	cancel_delayed_work_sync(&common->state_work);
 	timer_delete_sync(&port->rx_timer);
 	napi_disable(&port->rx_napi);
 
+	cancel_work_sync(&common->rx_mode_work);
+
 	return 0;
 }
 
@@ -533,10 +563,35 @@ static int rpmsg_eth_set_mac_address(struct net_device *ndev, void *addr)
 	return ret;
 }
 
+static void rpmsg_eth_ndo_set_rx_mode_work(struct work_struct *work)
+{
+	struct rpmsg_eth_common *common;
+	struct net_device *ndev;
+
+	common = container_of(work, struct rpmsg_eth_common, rx_mode_work);
+	ndev = common->port->ndev;
+
+	/* make a mc list copy */
+	netif_addr_lock_bh(ndev);
+	__hw_addr_sync(&common->mc_list, &ndev->mc, ndev->addr_len);
+	netif_addr_unlock_bh(ndev);
+
+	__hw_addr_sync_dev(&common->mc_list, ndev, rpmsg_eth_add_mc_addr,
+			   rpmsg_eth_del_mc_addr);
+}
+
+static void rpmsg_eth_set_rx_mode(struct net_device *ndev)
+{
+	struct rpmsg_eth_common *common = rpmsg_eth_ndev_to_common(ndev);
+
+	queue_work(common->cmd_wq, &common->rx_mode_work);
+}
+
 static const struct net_device_ops rpmsg_eth_netdev_ops = {
 	.ndo_open = rpmsg_eth_ndo_open,
 	.ndo_stop = rpmsg_eth_ndo_stop,
 	.ndo_start_xmit = rpmsg_eth_start_xmit,
+	.ndo_set_rx_mode = rpmsg_eth_set_rx_mode,
 	.ndo_set_mac_address = rpmsg_eth_set_mac_address,
 };
 
@@ -640,6 +695,13 @@ static int rpmsg_eth_probe(struct rpmsg_device *rpdev)
 	INIT_DELAYED_WORK(&common->state_work, rpmsg_eth_state_machine);
 	init_completion(&common->sync_msg);
 
+	__hw_addr_init(&common->mc_list);
+	INIT_WORK(&common->rx_mode_work, rpmsg_eth_ndo_set_rx_mode_work);
+	common->cmd_wq = create_singlethread_workqueue("rpmsg_eth_rx_work");
+	if (!common->cmd_wq) {
+		dev_err(dev, "Failure requesting workqueue\n");
+		return -ENOMEM;
+	}
 	/* Register the network device */
 	ret = rpmsg_eth_init_ndev(common);
 	if (ret)
@@ -658,6 +720,7 @@ static void rpmsg_eth_rpmsg_remove(struct rpmsg_device *rpdev)
 
 	netif_napi_del(&port->rx_napi);
 	timer_delete_sync(&port->rx_timer);
+	destroy_workqueue(common->cmd_wq);
 }
 
 static struct rpmsg_device_id rpmsg_eth_rpmsg_id_table[] = {
diff --git a/drivers/net/ethernet/rpmsg_eth.h b/drivers/net/ethernet/rpmsg_eth.h
index d7e4d53c8de4..5ff1a0e57c37 100644
--- a/drivers/net/ethernet/rpmsg_eth.h
+++ b/drivers/net/ethernet/rpmsg_eth.h
@@ -50,10 +50,14 @@ enum rpmsg_eth_rpmsg_type {
 	/* Request types */
 	RPMSG_ETH_REQ_SHM_INFO = 0,
 	RPMSG_ETH_REQ_SET_MAC_ADDR,
+	RPMSG_ETH_REQ_ADD_MC_ADDR,
+	RPMSG_ETH_REQ_DEL_MC_ADDR,
 
 	/* Response types */
 	RPMSG_ETH_RESP_SHM_INFO,
 	RPMSG_ETH_RESP_SET_MAC_ADDR,
+	RPMSG_ETH_RESP_ADD_MC_ADDR,
+	RPMSG_ETH_RESP_DEL_MC_ADDR,
 
 	/* Notification types */
 	RPMSG_ETH_NOTIFY_PORT_UP,
@@ -232,6 +236,10 @@ enum rpmsg_eth_state {
  * @state: Interface state
  * @state_work: Delayed work for state machine
  * @sync_msg: Completion for synchronous message
+ * @rx_mode_work: Work structure for rx mode
+ * @cmd_wq: Workqueue for commands
+ * @mc_list: List of multicast addresses
+ * @mcast_addr: Multicast address filter
  */
 struct rpmsg_eth_common {
 	struct rpmsg_device *rpdev;
@@ -248,6 +256,10 @@ struct rpmsg_eth_common {
 	struct mutex state_lock;
 	struct delayed_work state_work;
 	struct completion sync_msg;
+	struct work_struct rx_mode_work;
+	struct workqueue_struct *cmd_wq;
+	struct netdev_hw_addr_list mc_list;
+	u8 mcast_addr[ETH_ALEN];
 };
 
 /**
-- 
2.34.1

Re: [PATCH net-next 1/5] net: rpmsg-eth: Add Documentation for RPMSG-ETH Driver

[Jakub Kicinski]

On Wed, 23 Jul 2025 13:33:18 +0530 MD Danish Anwar wrote:
> +   - Vendors must ensure the magic number matches the value expected by the
> +     Linux driver (see the `RPMSG_ETH_SHM_MAGIC_NUM` macro in the driver
> +     source).

For some reason this trips up make coccicheck:

EXN: Failure("unexpected paren order") in /home/cocci/testing/Documentation/networking/device_drivers/ethernet/rpmsg_eth.rst

If I replace the brackets with a comma it works:

   - Vendors must ensure the magic number matches the value expected by the
     Linux driver, see the `RPMSG_ETH_SHM_MAGIC_NUM` macro in the driver
     source.

Could you make that change in the next revision to avoid the problem?

Julia, is there an easy way to make coccinelle ignore files which
don't end with .c or .h when using --use-patch-diff ?
-- 
pw-bot: cr

Re: [PATCH net-next 1/5] net: rpmsg-eth: Add Documentation for RPMSG-ETH Driver

[Andrew Lunn]

> --- a/Documentation/networking/device_drivers/ethernet/index.rst
> +++ b/Documentation/networking/device_drivers/ethernet/index.rst
> @@ -61,6 +61,7 @@ Contents:
>     wangxun/txgbevf
>     wangxun/ngbe
>     wangxun/ngbevf
> +   rpmsg_eth

This list is sorted. Please insert at the right location. I made the
same comment to somebody else this week as well....

> +This driver is generic and can be used by any vendor. Vendors can develop their
> +own firmware for the remote processor to make it compatible with this driver.
> +The firmware must adhere to the shared memory layout, RPMSG communication
> +protocol, and data exchange requirements described in this documentation.

Could you add a link to TIs firmware? It would be a good reference
implementation. But i guess that needs to wait until the driver is
merged and the ABI is stable.

> +Implementation Details
> +----------------------
> +
> +- The magic number is defined as a macro in the driver source (e.g.,
> +  ``#define RPMSG_ETH_SHM_MAGIC_NUM 0xABCDABCD``).
> +- The firmware must write this value to the ``magic_num`` field of the head and
> +  tail structures in the shared memory region.
> +- During the handshake, the Linux driver reads these fields and compares them to
> +  the expected value. If any mismatch is detected, the driver will log an error
> +  and refuse to proceed.

So the firmware always takes the role of "primary" and Linux is
"secondary"? With the current implementation, you cannot have Linux on
both ends?

I don't see this as a problem, but maybe it is worth stating as a
current limitation.

> +Shared Memory Layout
> +====================
> +
> +The RPMSG Based Virtual Ethernet Driver uses a shared memory region to exchange
> +data between the host and the remote processor. The shared memory is divided
> +into transmit and receive regions, each with its own `head` and `tail` pointers
> +to track the buffer state.
> +
> +Shared Memory Parameters
> +------------------------
> +
> +The following parameters are exchanged between the host and the firmware to
> +configure the shared memory layout:

So the host tells the firmware this? Maybe this is explained later,
but is the flow something like:

Linux makes an RPC call to the firmware with the parameters you list
below. Upon receiving that RPC, the firmware puts the magic numbers in
place. It then ACKs the RPC call? Linux then checks the magic numbers?

> +1. **num_pkt_bufs**:
> +
> +   - The total number of packet buffers available in the shared memory.
> +   - This determines the maximum number of packets that can be stored in the
> +     shared memory at any given time.
> +
> +2. **buff_slot_size**:
> +
> +   - The size of each buffer slot in the shared memory.
> +   - This includes space for the packet length, metadata, and the actual packet
> +     data.
> +
> +3. **base_addr**:
> +
> +   - The base address of the shared memory region.
> +   - This is the starting point for accessing the shared memory.

So this is the base address in the Linux address space? How should the
firmware convert this into a base address in its address space?

> +4. **tx_offset**:
> +
> +   - The offset from the `base_addr` where the transmit buffers begin.
> +   - This is used by the host to write packets for transmission.
> +
> +5. **rx_offset**:
> +
> +   - The offset from the `base_addr` where the receive buffers begin.
> +   - This is used by the host to read packets received from the remote
> +     processor.

Maybe change 'host' to 'Linux'? Or some other name, 'primary' and
'secondary'. The naming should be consistent throughout the
documentation and driver.

Part of the issue here is that you pass this information from Linux to
the firmware. When the firmware receives it, it has the complete
opposite meaning. It uses "tx_offset" to receive packets, and
"rx_offset" to send packets. This can quickly get confusing. If you
used names like "linux_tx_offset", the added context with avoid
confusion.

> +Shared Memory Structure
> +-----------------------
> +
> +The shared memory layout is as follows:
> +
> +.. code-block:: text
> +
> +      Shared Memory Layout:
> +      ---------------------------
> +      |        MAGIC_NUM        |   rpmsg_eth_shm_head
> +      |          HEAD           |
> +      ---------------------------
> +      |        MAGIC_NUM        |
> +      |        PKT_1_LEN        |
> +      |          PKT_1          |
> +      ---------------------------
> +      |           ...           |
> +      ---------------------------
> +      |        MAGIC_NUM        |
> +      |          TAIL           |   rpmsg_eth_shm_tail
> +      ---------------------------
> +
> +1. **MAGIC_NUM**:
> +
> +   - A unique identifier used to validate the shared memory region.
> +   - Ensures that the memory region is correctly initialized and accessible.
> +
> +2. **HEAD Pointer**:
> +
> +   - Tracks the start of the buffer for packet transmission or reception.
> +   - Updated by the producer (host or remote processor) after writing a packet.

Is this a pointer, or an offset from the base address? Pointers get
messy when you have multiple address spaces involved. An offset is
simpler to work with. Given that the buffers are fixed size, it could
even be an index.

> +Information Exchanged Between RPMSG Channels
> +--------------------------------------------
> +
> +1. **Requests from Host to Remote Processor**:

Another place where consistent naming would be good. Here it is the
remote processor, not firmware used earlier.

> +
> +   - `RPMSG_ETH_REQ_SHM_INFO`: Request shared memory information, such as
> +     ``num_pkt_bufs``, ``buff_slot_size``, ``base_addr``, ``tx_offset``, and
> +     ``rx_offset``.

Is this requested, or telling? I suppose the text above uses "between"
which is ambiguous.

> +3. **Notifications from Remote Processor to Host**:
> +
> +   - `RPMSG_ETH_NOTIFY_PORT_UP`: Notify that the Ethernet port is up and ready
> +     for communication.
> +   - `RPMSG_ETH_NOTIFY_PORT_DOWN`: Notify that the Ethernet port is down.
> +   - `RPMSG_ETH_NOTIFY_PORT_READY`: Notify that the Ethernet port is ready for
> +     configuration.

That needs more explanation. Why would it not be ready? 

> +   - `RPMSG_ETH_NOTIFY_REMOTE_READY`: Notify that the remote processor is ready
> +     for communication.

How does this differ from PORT_READY?

> +How-To Guide for Vendors
> +========================
> +
> +This section provides a guide for vendors to develop firmware for the remote
> +processor that is compatible with the RPMSG Based Virtual Ethernet Driver.
> +
> +1. **Implement Shared Memory Layout**:
> +
> +   - Allocate a shared memory region for packet transmission and reception.
> +   - Initialize the `MAGIC_NUM`, `num_pkt_bufs`, `buff_slot_size`, `base_addr`,
> +     `tx_offset`, and `rx_offset`.
> +
> +2. **Magic Number Requirements**
> +
> +   - The firmware must write a unique magic number (for example, ``0xABCDABCD``)

Why "for example"? Do you have a use case where some other value
should be used? Or can we just make this magic value part of the
specification?

> +- The driver assumes a specific shared memory layout and may not work with other
> +  configurations.
> +- Multicast address filtering is limited to the capabilities of the underlying
> +  RPMSG framework.

I don't think there is anything special here. The network stack always
does perfect address filtering. The driver can help out, by also doing
perfect address filtering, or imperfect address filtering, and letting
more through than actually wanted. Or it can go into promiscuous mode.

> +- The driver currently supports only one transmit and one receive queue.
> +
> +References
> +==========
> +
> +- RPMSG Framework Documentation: https://www.kernel.org/doc/html/latest/rpmsg.html

This results in 404 Not Found.

     Andrew

Re: [PATCH net-next 1/5] net: rpmsg-eth: Add Documentation for RPMSG-ETH Driver

[MD Danish Anwar]

Hi Jakub,

On 23/07/25 7:19 pm, Jakub Kicinski wrote:
> On Wed, 23 Jul 2025 13:33:18 +0530 MD Danish Anwar wrote:
>> +   - Vendors must ensure the magic number matches the value expected by the
>> +     Linux driver (see the `RPMSG_ETH_SHM_MAGIC_NUM` macro in the driver
>> +     source).
> 
> For some reason this trips up make coccicheck:
> 
> EXN: Failure("unexpected paren order") in /home/cocci/testing/Documentation/networking/device_drivers/ethernet/rpmsg_eth.rst
> 
> If I replace the brackets with a comma it works:
> 
>    - Vendors must ensure the magic number matches the value expected by the
>      Linux driver, see the `RPMSG_ETH_SHM_MAGIC_NUM` macro in the driver
>      source.
> 
> Could you make that change in the next revision to avoid the problem?
> 

Sure. I'll do this change in v2.

> Julia, is there an easy way to make coccinelle ignore files which
> don't end with .c or .h when using --use-patch-diff ?

-- 
Thanks and Regards,
Danish

Re: [PATCH net-next 1/5] net: rpmsg-eth: Add Documentation for RPMSG-ETH Driver

[MD Danish Anwar]

Hi Andrew,

On 23/07/25 9:54 pm, Andrew Lunn wrote:
>> --- a/Documentation/networking/device_drivers/ethernet/index.rst
>> +++ b/Documentation/networking/device_drivers/ethernet/index.rst
>> @@ -61,6 +61,7 @@ Contents:
>>     wangxun/txgbevf
>>     wangxun/ngbe
>>     wangxun/ngbevf
>> +   rpmsg_eth
> 
> This list is sorted. Please insert at the right location. I made the
> same comment to somebody else this week as well....
> 

Sure. I will re-order this.

>> +This driver is generic and can be used by any vendor. Vendors can develop their
>> +own firmware for the remote processor to make it compatible with this driver.
>> +The firmware must adhere to the shared memory layout, RPMSG communication
>> +protocol, and data exchange requirements described in this documentation.
> 
> Could you add a link to TIs firmware? It would be a good reference
> implementation. But i guess that needs to wait until the driver is
> merged and the ABI is stable.
> 

Currently TIs firmware is not open source. Once the firmware is
available in open source I can update the documentation to have a link
to that.

>> +Implementation Details
>> +----------------------
>> +
>> +- The magic number is defined as a macro in the driver source (e.g.,
>> +  ``#define RPMSG_ETH_SHM_MAGIC_NUM 0xABCDABCD``).
>> +- The firmware must write this value to the ``magic_num`` field of the head and
>> +  tail structures in the shared memory region.
>> +- During the handshake, the Linux driver reads these fields and compares them to
>> +  the expected value. If any mismatch is detected, the driver will log an error
>> +  and refuse to proceed.
> 
> So the firmware always takes the role of "primary" and Linux is
> "secondary"? With the current implementation, you cannot have Linux on
> both ends?
> 

Yes the firmware is primary and Linux is secondary. Linux can not be at
both ends.

> I don't see this as a problem, but maybe it is worth stating as a
> current limitation.
> 

Sure, I will mention that in the documentation in v2.

>> +Shared Memory Layout
>> +====================
>> +
>> +The RPMSG Based Virtual Ethernet Driver uses a shared memory region to exchange
>> +data between the host and the remote processor. The shared memory is divided
>> +into transmit and receive regions, each with its own `head` and `tail` pointers
>> +to track the buffer state.
>> +
>> +Shared Memory Parameters
>> +------------------------
>> +
>> +The following parameters are exchanged between the host and the firmware to
>> +configure the shared memory layout:
> 
> So the host tells the firmware this? Maybe this is explained later,
> but is the flow something like:
> 
> Linux makes an RPC call to the firmware with the parameters you list
> below. Upon receiving that RPC, the firmware puts the magic numbers in
> place. It then ACKs the RPC call? Linux then checks the magic numbers?
> 

Let me explain the flow,

Linux first send a rpmsg request with msg type = RPMSG_ETH_REQ_SHM_INFO
i.e. requesting for the shared memory info.

Once firmware recieves this request it sends response with below fields,

	num_pkt_bufs, buff_slot_size, base_addr, tx_offset, rx_offset

In the device tree, while reserving the shared memory for rpmsg_eth
driver, the base address and the size of the shared memory block is
mentioned. I have mentioned that in the documentation as well

+Configuration
+=============
+
+The driver relies on the device tree for configuration. The shared
memory region
+is specified using the `virtual-eth-shm` node in the device tree.
+
+Example Device Tree Node
+------------------------
+
+.. code-block:: dts
+
+   virtual-eth-shm {
+           compatible = "rpmsg,virtual-eth-shm";
+           reg = <0x80000000 0x10000>; /* Base address and size of
shared memory */
+   };

Now once Linux recieves (num_pkt_bufs, buff_slot_size, base_addr,
tx_offset, rx_offset) from firmware, the driver does a handshake
validation rpmsg_eth_validate_handshake() where the driver checks if the
base address recieved from firmware matches the base address resevred in
DT, if the tx and rx offsets are within the range, if the magic number
in tx and rx buffers are same as the magic number defined in driver etc.

Based on this validation the callback function returns success / failure.

If needed, I can add this detail also in the documentation.

>> +1. **num_pkt_bufs**:
>> +
>> +   - The total number of packet buffers available in the shared memory.
>> +   - This determines the maximum number of packets that can be stored in the
>> +     shared memory at any given time.
>> +
>> +2. **buff_slot_size**:
>> +
>> +   - The size of each buffer slot in the shared memory.
>> +   - This includes space for the packet length, metadata, and the actual packet
>> +     data.
>> +
>> +3. **base_addr**:
>> +
>> +   - The base address of the shared memory region.
>> +   - This is the starting point for accessing the shared memory.
> 
> So this is the base address in the Linux address space? How should the
> firmware convert this into a base address in its address space?
> 

The `base_addr` is the physical address of the shared memory. This is
reserved in the Linux DT. I haven't added the DT patch in this series,
but if you want I can add in v2 for your reference.

The same `base_addr` is used by firmware for the shared memory. During
the rpmsg callback, firmware shares this `base_addr` and during
rpmsg_eth_validate_handshake() driver checks if the base_addr shared by
firmware is same as the one described in DT or not. Driver only proceeds
if it's same.

After that the driver maps this base_addr and the tx / rx offsets in the
Linux virtual address space and use the same virtual address for writing
/ reading.

The firmware also maps this base_addr into it's vitual address space.
How firmware maps it is upto the firmware. The only requiremnt is that
the physical base_addr used by firmware and driver should be the same.

>> +4. **tx_offset**:
>> +
>> +   - The offset from the `base_addr` where the transmit buffers begin.
>> +   - This is used by the host to write packets for transmission.
>> +
>> +5. **rx_offset**:
>> +
>> +   - The offset from the `base_addr` where the receive buffers begin.
>> +   - This is used by the host to read packets received from the remote
>> +     processor.
> 
> Maybe change 'host' to 'Linux'? Or some other name, 'primary' and
> 'secondary'. The naming should be consistent throughout the
> documentation and driver.
> 

Sure I will change 'host' to 'Linux' and keep it consistent throughout
driver and firmware.

> Part of the issue here is that you pass this information from Linux to
> the firmware. When the firmware receives it, it has the complete
> opposite meaning. It uses "tx_offset" to receive packets, and
> "rx_offset" to send packets. This can quickly get confusing. If you
> used names like "linux_tx_offset", the added context with avoid
> confusion.
> 

Sure. Will do this.

>> +Shared Memory Structure
>> +-----------------------
>> +
>> +The shared memory layout is as follows:
>> +
>> +.. code-block:: text
>> +
>> +      Shared Memory Layout:
>> +      ---------------------------
>> +      |        MAGIC_NUM        |   rpmsg_eth_shm_head
>> +      |          HEAD           |
>> +      ---------------------------
>> +      |        MAGIC_NUM        |
>> +      |        PKT_1_LEN        |
>> +      |          PKT_1          |
>> +      ---------------------------
>> +      |           ...           |
>> +      ---------------------------
>> +      |        MAGIC_NUM        |
>> +      |          TAIL           |   rpmsg_eth_shm_tail
>> +      ---------------------------
>> +
>> +1. **MAGIC_NUM**:
>> +
>> +   - A unique identifier used to validate the shared memory region.
>> +   - Ensures that the memory region is correctly initialized and accessible.
>> +
>> +2. **HEAD Pointer**:
>> +
>> +   - Tracks the start of the buffer for packet transmission or reception.
>> +   - Updated by the producer (host or remote processor) after writing a packet.
> 
> Is this a pointer, or an offset from the base address? Pointers get
> messy when you have multiple address spaces involved. An offset is
> simpler to work with. Given that the buffers are fixed size, it could
> even be an index.
> 

Below are the structure definitions.

struct rpmsg_eth_shared_mem {
	struct rpmsg_eth_shm_index *head;
	struct rpmsg_eth_shm_buf *buf;
	struct rpmsg_eth_shm_index *tail;
} __packed;

struct rpmsg_eth_shm_index {
	u32 magic_num;
	u32 index;
}  __packed;

Head is pointer and it is mapped as below based on the information
shared by firmware

	port->tx_buffer->head =
		(struct rpmsg_eth_shm_index __force *)
		 (ioremap(msg->resp_msg.shm_info.base_addr +
			  msg->resp_msg.shm_info.tx_offset,
			  sizeof(*port->tx_buffer->head)));

>> +Information Exchanged Between RPMSG Channels
>> +--------------------------------------------
>> +
>> +1. **Requests from Host to Remote Processor**:
> 
> Another place where consistent naming would be good. Here it is the
> remote processor, not firmware used earlier.
> 

Sure I will rename it.

>> +
>> +   - `RPMSG_ETH_REQ_SHM_INFO`: Request shared memory information, such as
>> +     ``num_pkt_bufs``, ``buff_slot_size``, ``base_addr``, ``tx_offset``, and
>> +     ``rx_offset``.
> 
> Is this requested, or telling? I suppose the text above uses "between"
> which is ambiguous.

It's requested. The Linux driver requests firmware for these info.

> 
>> +3. **Notifications from Remote Processor to Host**:
>> +
>> +   - `RPMSG_ETH_NOTIFY_PORT_UP`: Notify that the Ethernet port is up and ready
>> +     for communication.
>> +   - `RPMSG_ETH_NOTIFY_PORT_DOWN`: Notify that the Ethernet port is down.
>> +   - `RPMSG_ETH_NOTIFY_PORT_READY`: Notify that the Ethernet port is ready for
>> +     configuration.
> 
> That needs more explanation. Why would it not be ready? 
> 

This actually not needed RPMSG_ETH_NOTIFY_PORT_UP and
RPMSG_ETH_NOTIFY_PORT_DOWN are enough. I will drop
RPMSG_ETH_NOTIFY_PORT_READY

>> +   - `RPMSG_ETH_NOTIFY_REMOTE_READY`: Notify that the remote processor is ready
>> +     for communication.
> 
> How does this differ from PORT_READY?

RPMSG_ETH_NOTIFY_REMOTE_READY implies that the remote processor i.e. the
firmware is ready where as RPMSG_ETH_NOTIFY_PORT_UP implies that the
ethernet port (Linux driver) is ready.

PORT_READY is not needed and can be dropped.


> 
>> +How-To Guide for Vendors
>> +========================
>> +
>> +This section provides a guide for vendors to develop firmware for the remote
>> +processor that is compatible with the RPMSG Based Virtual Ethernet Driver.
>> +
>> +1. **Implement Shared Memory Layout**:
>> +
>> +   - Allocate a shared memory region for packet transmission and reception.
>> +   - Initialize the `MAGIC_NUM`, `num_pkt_bufs`, `buff_slot_size`, `base_addr`,
>> +     `tx_offset`, and `rx_offset`.
>> +
>> +2. **Magic Number Requirements**
>> +
>> +   - The firmware must write a unique magic number (for example, ``0xABCDABCD``)
> 
> Why "for example"? Do you have a use case where some other value
> should be used? Or can we just make this magic value part of the
> specification?

No the magic number is always the same. I will update the doucmentation
to state the same.
> 
>> +- The driver assumes a specific shared memory layout and may not work with other
>> +  configurations.
>> +- Multicast address filtering is limited to the capabilities of the underlying
>> +  RPMSG framework.
> 
> I don't think there is anything special here. The network stack always
> does perfect address filtering. The driver can help out, by also doing
> perfect address filtering, or imperfect address filtering, and letting
> more through than actually wanted. Or it can go into promiscuous mode.
> 

Sure. I will drop this.

>> +- The driver currently supports only one transmit and one receive queue.
>> +
>> +References
>> +==========
>> +
>> +- RPMSG Framework Documentation: https://www.kernel.org/doc/html/latest/rpmsg.html
> 
> This results in 404 Not Found.
> 

I see the url is wrong. I will update the correct url
https://docs.kernel.org/staging/rpmsg.html

>      Andrew

-- 
Thanks and Regards,
Danish

Re: [PATCH net-next 1/5] net: rpmsg-eth: Add Documentation for RPMSG-ETH Driver

[Andrew Lunn]

> Linux first send a rpmsg request with msg type = RPMSG_ETH_REQ_SHM_INFO
> i.e. requesting for the shared memory info.
> 
> Once firmware recieves this request it sends response with below fields,
> 
> 	num_pkt_bufs, buff_slot_size, base_addr, tx_offset, rx_offset
> 
> In the device tree, while reserving the shared memory for rpmsg_eth
> driver, the base address and the size of the shared memory block is
> mentioned. I have mentioned that in the documentation as well

If it is in device tree, why should Linux ask for the base address and
length? That just seems like a source of errors, and added complexity.

In general, we just trust DT. It is a source of truth. So i would
delete all this backwards and forwards and just use the values from
DT. Just check the magic numbers are in place.

> The same `base_addr` is used by firmware for the shared memory. During
> the rpmsg callback, firmware shares this `base_addr` and during
> rpmsg_eth_validate_handshake() driver checks if the base_addr shared by
> firmware is same as the one described in DT or not. Driver only proceeds
> if it's same.

So there is a big assumption here. That both are sharing the same MMU,
or maybe IOMMU. Or both CPUs have configured their MMU/IOMMU so that
the pages appear at the same physical address. I think this is a
problem, and the design should avoid anything which makes this
assumptions. The data structures within the share memory should only
refer to offsets from the base of the shared memory, not absolute
values. Or an index into the table of buffers, 0..N.

> >> +2. **HEAD Pointer**:
> >> +
> >> +   - Tracks the start of the buffer for packet transmission or reception.
> >> +   - Updated by the producer (host or remote processor) after writing a packet.
> > 
> > Is this a pointer, or an offset from the base address? Pointers get
> > messy when you have multiple address spaces involved. An offset is
> > simpler to work with. Given that the buffers are fixed size, it could
> > even be an index.
> > 
> 
> Below are the structure definitions.
> 
> struct rpmsg_eth_shared_mem {
> 	struct rpmsg_eth_shm_index *head;
> 	struct rpmsg_eth_shm_buf *buf;
> 	struct rpmsg_eth_shm_index *tail;
> } __packed;
> 
> struct rpmsg_eth_shm_index {
> 	u32 magic_num;
> 	u32 index;
> }  __packed;

So index is the index into the array of fixed size buffers. That is
fine, it is not a pointer, so you don't need to worry about address
spaces. However, head and tail are pointers, so for those you do need
to worry about address spaces. But why do you even need them? Just put
the indexes directly into rpmsg_eth_shared_mem. The four index values
can be in the first few words of the shared memory, fixed offset from
the beginning, KISS.

	Andrew

Re: [PATCH net-next 2/5] net: rpmsg-eth: Add basic rpmsg skeleton

[Krzysztof Kozlowski]

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

Re: [PATCH net-next 1/5] net: rpmsg-eth: Add Documentation for RPMSG-ETH Driver

[Anwar, Md Danish]

Hi Andrew,

On 7/24/2025 10:07 PM, Andrew Lunn wrote:
>> Linux first send a rpmsg request with msg type = RPMSG_ETH_REQ_SHM_INFO
>> i.e. requesting for the shared memory info.
>>
>> Once firmware recieves this request it sends response with below fields,
>>
>> 	num_pkt_bufs, buff_slot_size, base_addr, tx_offset, rx_offset
>>
>> In the device tree, while reserving the shared memory for rpmsg_eth
>> driver, the base address and the size of the shared memory block is
>> mentioned. I have mentioned that in the documentation as well
> 
> If it is in device tree, why should Linux ask for the base address and
> length? That just seems like a source of errors, and added complexity.
> 
> In general, we just trust DT. It is a source of truth. So i would
> delete all this backwards and forwards and just use the values from
> DT. Just check the magic numbers are in place.
> 

Sure, I will not check the base_addr and trust the info we get from
device tree. Just check the offsets we are getting from firmware is
within the shared memory block or not (using base_addr + size)

>> The same `base_addr` is used by firmware for the shared memory. During
>> the rpmsg callback, firmware shares this `base_addr` and during
>> rpmsg_eth_validate_handshake() driver checks if the base_addr shared by
>> firmware is same as the one described in DT or not. Driver only proceeds
>> if it's same.
> 
> So there is a big assumption here. That both are sharing the same MMU,
> or maybe IOMMU. Or both CPUs have configured their MMU/IOMMU so that
> the pages appear at the same physical address. I think this is a
> problem, and the design should avoid anything which makes this
> assumptions. The data structures within the share memory should only
> refer to offsets from the base of the shared memory, not absolute
> values. Or an index into the table of buffers, 0..N.
> 

Sure I will try to do the same.

>>>> +2. **HEAD Pointer**:
>>>> +
>>>> +   - Tracks the start of the buffer for packet transmission or reception.
>>>> +   - Updated by the producer (host or remote processor) after writing a packet.
>>>
>>> Is this a pointer, or an offset from the base address? Pointers get
>>> messy when you have multiple address spaces involved. An offset is
>>> simpler to work with. Given that the buffers are fixed size, it could
>>> even be an index.
>>>
>>
>> Below are the structure definitions.
>>
>> struct rpmsg_eth_shared_mem {
>> 	struct rpmsg_eth_shm_index *head;
>> 	struct rpmsg_eth_shm_buf *buf;
>> 	struct rpmsg_eth_shm_index *tail;
>> } __packed;
>>
>> struct rpmsg_eth_shm_index {
>> 	u32 magic_num;
>> 	u32 index;
>> }  __packed;
> 
> So index is the index into the array of fixed size buffers. That is
> fine, it is not a pointer, so you don't need to worry about address
> spaces. However, head and tail are pointers, so for those you do need
> to worry about address spaces. But why do you even need them? Just put
> the indexes directly into rpmsg_eth_shared_mem. The four index values
> can be in the first few words of the shared memory, fixed offset from
> the beginning, KISS.
> 

Sure I will try to move everything to offsets and not use pointers.

> 	Andrew

-- 
Thanks and Regards,
Md Danish Anwar

Re: [PATCH net-next 2/5] net: rpmsg-eth: Add basic rpmsg skeleton

[MD Danish Anwar]

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

Re: [PATCH net-next 2/5] net: rpmsg-eth: Add basic rpmsg skeleton

[Krzysztof Kozlowski]

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

Re: [PATCH net-next 2/5] net: rpmsg-eth: Add basic rpmsg skeleton

[MD Danish Anwar]

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

Re: [PATCH net-next 2/5] net: rpmsg-eth: Add basic rpmsg skeleton

[Krzysztof Kozlowski]

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

Re: [PATCH net-next 2/5] net: rpmsg-eth: Add basic rpmsg skeleton

[MD Danish Anwar]

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

Re: [PATCH net-next 2/5] net: rpmsg-eth: Add basic rpmsg skeleton

[Krzysztof Kozlowski]

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

Re: [PATCH net-next 2/5] net: rpmsg-eth: Add basic rpmsg skeleton

[Anwar, Md Danish]

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

Re: [cocci] [PATCH net-next 1/5] net: rpmsg-eth: Add Documentation for RPMSG-ETH Driver

[Julia Lawall]

On Wed, 23 Jul 2025, Jakub Kicinski wrote:

> On Wed, 23 Jul 2025 13:33:18 +0530 MD Danish Anwar wrote:
> > +   - Vendors must ensure the magic number matches the value expected by the
> > +     Linux driver (see the `RPMSG_ETH_SHM_MAGIC_NUM` macro in the driver
> > +     source).
>
> For some reason this trips up make coccicheck:
>
> EXN: Failure("unexpected paren order") in /home/cocci/testing/Documentation/networking/device_drivers/ethernet/rpmsg_eth.rst
>
> If I replace the brackets with a comma it works:
>
>    - Vendors must ensure the magic number matches the value expected by the
>      Linux driver, see the `RPMSG_ETH_SHM_MAGIC_NUM` macro in the driver
>      source.
>
> Could you make that change in the next revision to avoid the problem?
>
> Julia, is there an easy way to make coccinelle ignore files which
> don't end with .c or .h when using --use-patch-diff ?

Perhaps not.  I can adjust it.

julia

Re: [PATCH net-next 2/5] net: rpmsg-eth: Add basic rpmsg skeleton

[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

Re: [PATCH net-next 1/5] net: rpmsg-eth: Add Documentation for RPMSG-ETH Driver

[MD Danish Anwar]

On 24/07/25 10:07 pm, Andrew Lunn wrote:
>> Linux first send a rpmsg request with msg type = RPMSG_ETH_REQ_SHM_INFO
>> i.e. requesting for the shared memory info.
>>
>> Once firmware recieves this request it sends response with below fields,
>>
>> 	num_pkt_bufs, buff_slot_size, base_addr, tx_offset, rx_offset
>>
>> In the device tree, while reserving the shared memory for rpmsg_eth
>> driver, the base address and the size of the shared memory block is
>> mentioned. I have mentioned that in the documentation as well
> 
> If it is in device tree, why should Linux ask for the base address and
> length? That just seems like a source of errors, and added complexity.
> 
> In general, we just trust DT. It is a source of truth. So i would
> delete all this backwards and forwards and just use the values from
> DT. Just check the magic numbers are in place.
> 

Sure. I will make this change in v2.

>> The same `base_addr` is used by firmware for the shared memory. During
>> the rpmsg callback, firmware shares this `base_addr` and during
>> rpmsg_eth_validate_handshake() driver checks if the base_addr shared by
>> firmware is same as the one described in DT or not. Driver only proceeds
>> if it's same.
> 
> So there is a big assumption here. That both are sharing the same MMU,
> or maybe IOMMU. Or both CPUs have configured their MMU/IOMMU so that
> the pages appear at the same physical address. I think this is a
> problem, and the design should avoid anything which makes this
> assumptions. The data structures within the share memory should only
> refer to offsets from the base of the shared memory, not absolute
> values. Or an index into the table of buffers, 0..N.
> 
>>>> +2. **HEAD Pointer**:
>>>> +
>>>> +   - Tracks the start of the buffer for packet transmission or reception.
>>>> +   - Updated by the producer (host or remote processor) after writing a packet.
>>>
>>> Is this a pointer, or an offset from the base address? Pointers get
>>> messy when you have multiple address spaces involved. An offset is
>>> simpler to work with. Given that the buffers are fixed size, it could
>>> even be an index.
>>>
>>
>> Below are the structure definitions.
>>
>> struct rpmsg_eth_shared_mem {
>> 	struct rpmsg_eth_shm_index *head;
>> 	struct rpmsg_eth_shm_buf *buf;
>> 	struct rpmsg_eth_shm_index *tail;
>> } __packed;
>>
>> struct rpmsg_eth_shm_index {
>> 	u32 magic_num;
>> 	u32 index;
>> }  __packed;
> 
> So index is the index into the array of fixed size buffers. That is
> fine, it is not a pointer, so you don't need to worry about address
> spaces. However, head and tail are pointers, so for those you do need
> to worry about address spaces. But why do you even need them? Just put
> the indexes directly into rpmsg_eth_shared_mem. The four index values
> can be in the first few words of the shared memory, fixed offset from
> the beginning, KISS.
> 

I will drop all these pointers and use a offset based approach in v2.

Thanks for the feedback.

-- 
Thanks and Regards,
Danish

Re: [PATCH net-next 1/5] net: rpmsg-eth: Add Documentation for RPMSG-ETH Driver

[Bagas Sanjaya]

[-- Attachment #1: Type: text/plain, Size: 579 bytes --]

On Wed, Jul 23, 2025 at 01:33:18PM +0530, MD Danish Anwar wrote:
> +References
> +==========
> +
> +- RPMSG Framework Documentation: https://www.kernel.org/doc/html/latest/rpmsg.html
> +- Linux Networking Documentation: https://www.kernel.org/doc/html/latest/networking/index.html

Instead of using external link to kernel docs, you can also use internal
cross-references, simply by mentioning the docs path
(Documentation/staging/rpmsg.rst and Documentation/networking/index.rst in this
case).

Thanks.

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]