[PATCH v6 0/5] Enable Remote GPIO over RPMSG on i.MX Platform

[PATCH v6 0/5] Enable Remote GPIO over RPMSG on i.MX Platform

[Shenwei Wang]

Support the remote devices on the remote processor via the RPMSG bus on
i.MX platform.

Changes in v6:
 - make the driver more generic with the actions below:
     rename the driver file to gpio-rpmsg.c
     remove the imx related info in the function and variable names
     rename the imx_rpmsg.h to rpdev_info.h
     create a gpio-rpmsg.yaml and refer it in imx_rproc.yaml
 - update the gpio-rpmsg.rst according to the feedback from Andrew and
   move the source file to driver-api/gpio
 - fix the bug reported by Zhongqiu Han
 - remove the I2C related info

Changes in v5:
 - move the gpio-rpmsg.rst from admin-guide to staging directory after
   discussion with Randy Dunlap.
 - add include files with some code improvements per Bartosz's comments.

Changes in v4:
 - add a documentation to describe the transport protocol per Andrew's
   comments.
 - add a new handler to get the gpio direction.

Changes in v3:
 - fix various format issue and return value check per Peng 's review
   comments.
 - add the logic to also populate the subnodes which are not in the
   device map per Arnaud's request. (in imx_rproc.c)
 - update the yaml per Frank's review comments.

Changes in v2:
 - re-implemented the gpio driver per Linus Walleij's feedback by using
   GPIOLIB_IRQCHIP helper library.
 - fix various format issue per Mathieu/Peng 's review comments.
 - update the yaml doc per Rob's feedback

Shenwei Wang (5):
  dt-bindings: remoteproc: imx_rproc: Add "rpmsg" subnode support
  remoteproc: imx_rproc: Populate devices under "rpmsg" subnode
  docs: driver-api: gpio: generic gpio driver over rpmsg bus
  gpio: rpmsg: add generic rpmsg GPIO driver
  arm64: dts: imx8ulp: Add rpmsg node under imx_rproc

 .../devicetree/bindings/gpio/gpio-rpmsg.yaml  |  49 ++
 .../bindings/remoteproc/fsl,imx-rproc.yaml    |  54 ++
 Documentation/driver-api/gpio/gpio-rpmsg.rst  | 232 +++++++++
 Documentation/driver-api/gpio/index.rst       |   1 +
 arch/arm64/boot/dts/freescale/imx8ulp.dtsi    |  27 +
 drivers/gpio/Kconfig                          |  16 +
 drivers/gpio/Makefile                         |   1 +
 drivers/gpio/gpio-rpmsg.c                     | 490 ++++++++++++++++++
 drivers/remoteproc/imx_rproc.c                | 143 +++++
 include/linux/rpmsg/rpdev_info.h              |  33 ++
 10 files changed, 1046 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-rpmsg.yaml
 create mode 100644 Documentation/driver-api/gpio/gpio-rpmsg.rst
 create mode 100644 drivers/gpio/gpio-rpmsg.c
 create mode 100644 include/linux/rpmsg/rpdev_info.h

--
2.43.0

[PATCH v6 1/5] dt-bindings: remoteproc: imx_rproc: Add "rpmsg" subnode support

[Shenwei Wang]

Remote processors may announce multiple devices (e.g., I2C, GPIO) over
an RPMSG channel. These devices may require corresponding device tree
nodes, especially when acting as providers, to supply phandles for their
consumers.

Define an RPMSG node to work as a container for a group of RPMSG channels
under the imx_rproc node.

Each subnode within "rpmsg" represents an individual RPMSG channel. The
name of each subnode corresponds to the channel name as defined by the
remote processor.

All remote devices associated with a given channel are defined as child
nodes under the corresponding channel node.

Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
---
 .../devicetree/bindings/gpio/gpio-rpmsg.yaml  | 49 +++++++++++++++++
 .../bindings/remoteproc/fsl,imx-rproc.yaml    | 54 +++++++++++++++++++
 2 files changed, 103 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-rpmsg.yaml

diff --git a/Documentation/devicetree/bindings/gpio/gpio-rpmsg.yaml b/Documentation/devicetree/bindings/gpio/gpio-rpmsg.yaml
new file mode 100644
index 000000000000..b3e1a5dbf731
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-rpmsg.yaml
@@ -0,0 +1,49 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/gpio/gpio-rpmsg.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Generic GPIO driver over RPMSG
+
+maintainers:
+  - Shenwei Wang <shenwei.wang@nxp.com>
+
+description:
+  On an AMP platform, some GPIO controllers are exposed by the remote processor
+  through the RPMSG bus. The RPMSG GPIO transport protocol defines the packet
+  structure and communication flow between Linux and the remote firmware. Those
+  controllers are managed via this transport protocol.
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+          - enum:
+              - fsl,rpmsg-gpio
+          - const: rpmsg-gpio
+      - const: rpmsg-gpio
+
+  reg:
+    maxItems: 1
+
+  "#gpio-cells":
+    const: 2
+
+  gpio-controller: true
+
+  interrupt-controller: true
+
+  "#interrupt-cells":
+    const: 2
+
+required:
+  - compatible
+  - reg
+  - "#gpio-cells"
+  - "#interrupt-cells"
+
+allOf:
+  - $ref: /schemas/gpio/gpio.yaml#
+
+unevaluatedProperties: false
diff --git a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
index 57d75acb0b5e..fd8e5a61a459 100644
--- a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
@@ -84,6 +84,33 @@ properties:
       This property is to specify the resource id of the remote processor in SoC
       which supports SCFW
 
+  rpmsg:
+    type: object
+    additionalProperties: false
+    description:
+      Present a group of RPMSG channel devices.
+
+    properties:
+      rpmsg-io-channel:
+        type: object
+        additionalProperties: false
+        properties:
+          '#address-cells':
+            const: 1
+
+          '#size-cells':
+            const: 0
+
+        patternProperties:
+          "gpio@[0-9a-f]+$":
+            type: object
+            $ref: /schemas/gpio/gpio-rpmsg.yaml#
+            unevaluatedProperties: false
+
+        required:
+          - '#address-cells'
+          - '#size-cells'
+
 required:
   - compatible
 
@@ -146,5 +173,32 @@ examples:
                 &mu 3 1>;
       memory-region = <&vdev0buffer>, <&vdev0vring0>, <&vdev0vring1>, <&rsc_table>;
       syscon = <&src>;
+
+      rpmsg {
+        rpmsg-io-channel {
+          #address-cells = <1>;
+          #size-cells = <0>;
+
+          gpio@0 {
+            compatible = "rpmsg-gpio";
+            reg = <0>;
+            gpio-controller;
+            #gpio-cells = <2>;
+            #interrupt-cells = <2>;
+            interrupt-controller;
+            interrupt-parent = <&rpmsg_gpioa>;
+          };
+
+          gpio@1 {
+            compatible = "rpmsg-gpio";
+            reg = <1>;
+            gpio-controller;
+            #gpio-cells = <2>;
+            #interrupt-cells = <2>;
+            interrupt-controller;
+            interrupt-parent = <&rpmsg_gpiob>;
+          };
+        };
+      };
     };
 ...
-- 
2.43.0

[PATCH v6 2/5] remoteproc: imx_rproc: Populate devices under "rpmsg" subnode

[Shenwei Wang]

Register the RPMsg channel driver and populate remote devices defined
under the "rpmsg" subnode upon receiving their notification messages.

The following illustrates the expected DTS layout structure:

	cm33: remoteproc-cm33 {
		compatible = "fsl,imx8ulp-cm33";

		rpmsg {
			rpmsg-io-channel {
				gpio@0 {
					compatible = "fsl,imx-rpmsg-gpio";
					reg = <0>;
				};

				gpio@1 {
					compatible = "fsl,imx-rpmsg-gpio";
					reg = <1>;
				};

				...
			};

			...
		};
	};

Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
---
 drivers/remoteproc/imx_rproc.c   | 143 +++++++++++++++++++++++++++++++
 include/linux/rpmsg/rpdev_info.h |  33 +++++++
 2 files changed, 176 insertions(+)
 create mode 100644 include/linux/rpmsg/rpdev_info.h

diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
index 33f21ab24c92..65ee16fd66d1 100644
--- a/drivers/remoteproc/imx_rproc.c
+++ b/drivers/remoteproc/imx_rproc.c
@@ -15,6 +15,8 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/of_platform.h>
 #include <linux/of_reserved_mem.h>
 #include <linux/platform_device.h>
 #include <linux/pm_domain.h>
@@ -22,6 +24,8 @@
 #include <linux/reboot.h>
 #include <linux/regmap.h>
 #include <linux/remoteproc.h>
+#include <linux/rpmsg.h>
+#include <linux/rpmsg/rpdev_info.h>
 #include <linux/workqueue.h>
 
 #include "imx_rproc.h"
@@ -1016,6 +1020,141 @@ static void imx_rproc_destroy_workqueue(void *data)
 	destroy_workqueue(workqueue);
 }
 
+struct imx_rpmsg_driver {
+	struct rpmsg_driver rpdrv;
+	const char *compat;
+	void *driver_data;
+};
+
+static const char *channel_device_map[][2] = {
+	{"rpmsg-io-channel", "rpmsg-gpio"},
+};
+
+static int imx_rpmsg_endpoint_cb(struct rpmsg_device *rpdev, void *data,
+				 int len, void *priv, u32 src)
+{
+	struct rpdev_platform_info *drvdata;
+
+	drvdata = dev_get_drvdata(&rpdev->dev);
+	if (drvdata && drvdata->rx_callback)
+		return drvdata->rx_callback(rpdev, data, len, priv, src);
+
+	return 0;
+}
+
+static void imx_rpmsg_endpoint_remove(struct rpmsg_device *rpdev)
+{
+	of_platform_depopulate(&rpdev->dev);
+}
+
+static int imx_rpmsg_endpoint_probe(struct rpmsg_device *rpdev)
+{
+	struct rpdev_platform_info *drvdata;
+	struct imx_rpmsg_driver *imx_rpdrv;
+	struct device *dev = &rpdev->dev;
+	struct of_dev_auxdata *auxdata;
+	struct rpmsg_driver *rpdrv;
+
+	rpdrv = container_of(dev->driver, struct rpmsg_driver, drv);
+	imx_rpdrv = container_of(rpdrv, struct imx_rpmsg_driver, rpdrv);
+
+	if (!imx_rpdrv->driver_data)
+		return -EINVAL;
+
+	drvdata = devm_kmemdup(dev, imx_rpdrv->driver_data, sizeof(*drvdata), GFP_KERNEL);
+	if (!drvdata)
+		return -ENOMEM;
+
+	auxdata = devm_kzalloc(dev, sizeof(*auxdata) * 2, GFP_KERNEL);
+	if (!auxdata)
+		return -ENOMEM;
+
+	drvdata->rpdev = rpdev;
+	auxdata[0].compatible = devm_kstrdup(dev, imx_rpdrv->compat, GFP_KERNEL);
+	auxdata[0].platform_data = drvdata;
+	dev_set_drvdata(dev, drvdata);
+
+	of_platform_populate(drvdata->channel_node, NULL, auxdata, dev);
+
+	return 0;
+}
+
+static const char *imx_of_rpmsg_is_in_map(const char *name)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(channel_device_map); i++) {
+		if (strcmp(name, channel_device_map[i][0]) == 0)
+			return channel_device_map[i][1];
+	}
+
+	return NULL;
+}
+
+static int imx_of_rpmsg_register_rpdriver(struct device_node *channel,
+					  struct device *dev,
+					  const char *name,
+					  const char *compat)
+{
+	struct rpdev_platform_info *driver_data;
+	struct imx_rpmsg_driver *rp_driver;
+	struct rpmsg_device_id *rpdev_id;
+
+	/* rpmsg_device_id is a NULL terminated array */
+	rpdev_id = devm_kzalloc(dev, sizeof(*rpdev_id) * 2, GFP_KERNEL);
+	if (!rpdev_id)
+		return -ENOMEM;
+
+	strscpy(rpdev_id[0].name, name, RPMSG_NAME_SIZE);
+
+	rp_driver = devm_kzalloc(dev, sizeof(*rp_driver), GFP_KERNEL);
+	if (!rp_driver)
+		return -ENOMEM;
+
+	driver_data = devm_kzalloc(dev, sizeof(*driver_data), GFP_KERNEL);
+	if (!driver_data)
+		return -ENOMEM;
+
+	driver_data->rproc_name = dev->of_node->name;
+	driver_data->channel_node = channel;
+
+	rp_driver->rpdrv.drv.name = name;
+	rp_driver->rpdrv.id_table = rpdev_id;
+	rp_driver->rpdrv.probe = imx_rpmsg_endpoint_probe;
+	rp_driver->rpdrv.remove = imx_rpmsg_endpoint_remove;
+	rp_driver->rpdrv.callback = imx_rpmsg_endpoint_cb;
+	rp_driver->driver_data = driver_data;
+	rp_driver->compat = compat;
+
+	register_rpmsg_driver(&rp_driver->rpdrv);
+
+	return 0;
+}
+
+static int rproc_of_rpmsg_node_init(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	const char *compat;
+	int ret;
+
+	struct device_node *np __free(device_node) = of_get_child_by_name(dev->of_node, "rpmsg");
+	if (!np)
+		return 0;
+
+	for_each_child_of_node_scoped(np, child) {
+		compat = imx_of_rpmsg_is_in_map(child->name);
+		if (!compat)
+			ret = of_platform_default_populate(child, NULL, dev);
+		else
+			ret = imx_of_rpmsg_register_rpdriver(child, dev, child->name, compat);
+
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
 static int imx_rproc_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -1114,6 +1253,10 @@ static int imx_rproc_probe(struct platform_device *pdev)
 		goto err_put_pm;
 	}
 
+	ret = rproc_of_rpmsg_node_init(pdev);
+	if (ret < 0)
+		dev_info(dev, "populating 'rpmsg' node failed\n");
+
 	return 0;
 
 err_put_pm:
diff --git a/include/linux/rpmsg/rpdev_info.h b/include/linux/rpmsg/rpdev_info.h
new file mode 100644
index 000000000000..13e020cd028b
--- /dev/null
+++ b/include/linux/rpmsg/rpdev_info.h
@@ -0,0 +1,33 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright 2025 NXP */
+
+/*
+ * @file linux/rpdev_info.h
+ *
+ * @brief Global header file for RPDEV Info
+ *
+ * @ingroup RPMSG
+ */
+#ifndef __LINUX_RPDEV_INFO_H__
+#define __LINUX_RPDEV_INFO_H__
+
+#define MAX_DEV_PER_CHANNEL    10
+
+/**
+ * rpdev_platform_info - store the platform information of rpdev
+ * @rproc_name: the name of the remote proc.
+ * @rpdev: rpmsg channel device
+ * @device_node: pointer to the device node of the rpdev.
+ * @rx_callback: rx callback handler of the rpdev.
+ * @channel_devices: an array of the devices related to the rpdev.
+ */
+struct rpdev_platform_info {
+	const char *rproc_name;
+	struct rpmsg_device *rpdev;
+	struct device_node *channel_node;
+	int (*rx_callback)(struct rpmsg_device *rpdev, void *data,
+			   int len, void *priv, u32 src);
+	void *channel_devices[MAX_DEV_PER_CHANNEL];
+};
+
+#endif /* __LINUX_RPDEV_INFO_H__ */
-- 
2.43.0

[PATCH v6 3/5] docs: driver-api: gpio: generic gpio driver over rpmsg bus

[Shenwei Wang]

Describes the gpio rpmsg transport protocol over the rpmsg bus between
the cores.

Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
---
 Documentation/driver-api/gpio/gpio-rpmsg.rst | 232 +++++++++++++++++++
 Documentation/driver-api/gpio/index.rst      |   1 +
 2 files changed, 233 insertions(+)
 create mode 100644 Documentation/driver-api/gpio/gpio-rpmsg.rst

diff --git a/Documentation/driver-api/gpio/gpio-rpmsg.rst b/Documentation/driver-api/gpio/gpio-rpmsg.rst
new file mode 100644
index 000000000000..c78d10a9a85c
--- /dev/null
+++ b/Documentation/driver-api/gpio/gpio-rpmsg.rst
@@ -0,0 +1,232 @@
+.. SPDX-License-Identifier: GPL-2.0-or-later
+
+GPIO RPMSG Protocol
+===================
+
+The GPIO RPMSG transport protocol is used for communication and interaction
+with GPIO controllers located on remote cores on the RPMSG bus.
+
+Message Format
+--------------
+
+The RPMSG message consists of a 14-byte packet with the following layout:
+
+.. code-block:: none
+
+   +-----+-------+--------+-----+-----+------------+-----+-----+-----+----+
+   |0x00 |0x01   |0x02    |0x03 |0x04 |0x05..0x09  |0x0A |0x0B |0x0C |0x0D|
+   | ID  |vendor |version |type |cmd  |reserved[5] |line |port |  data    |
+   +-----+-------+--------+-----+-----+------------+-----+-----+-----+----+
+
+- **ID (Message Identification Code)**: Always be 0x5. Indicates the GPIO message.
+
+- **Vendor**: Vendor ID number.
+  - 0: Reserved
+  - 1: NXP
+
+- **Version**: Vendor-specific version number (such as software release).
+
+- **Type (Message Type)**: The message type can be one of:
+
+  - 0: GPIO_RPMSG_SETUP
+  - 1: GPIO_RPMSG_REPLY
+  - 2: GPIO_RPMSG_NOTIFY
+
+- **Cmd**: Command code, used for GPIO_RPMSG_SETUP messages.
+
+- **reserved[5]**: Reserved bytes. Should always be 0.
+
+- **line**: The GPIO line index.
+
+- **port**: The GPIO controller index.
+
+- **data**: See details in the command description below.
+
+GPIO Commands
+-------------
+
+Commands are specified in the **Cmd** field for **GPIO_RPMSG_SETUP** (Type=0) messages.
+
+The SETUP message is always sent from Linux to the remote firmware. Each
+SETUP corresponds to a single REPLY message. The GPIO driver should
+serialize messages and determine whether a REPLY message is required. If a
+REPLY message is expected but not received within the specified timeout
+period (currently 1 second in the Linux driver), the driver should return
+-ETIMEOUT.
+
+GPIO_RPMSG_INPUT_INIT (Cmd=0)
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+**Request:**
+
+.. code-block:: none
+
+   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
+   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05..0x09 |0x0A |0x0B |0x0C |0x0D|
+   | 5   | 1   | 0   | 0   | 0   |  0        |line |port | val | wk |
+   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
+
+- **val**: Interrupt trigger type.
+
+  - 0: Interrupt disabled
+  - 1: Rising edge trigger
+  - 2: Falling edge trigger
+  - 3: Both edge trigger
+  - 4: Low level trigger
+  - 5: High level trigger
+
+- **wk**: Wakeup enable.
+
+  The remote system should always aim to stay in a power-efficient state by
+  shutting down or clock-gating the GPIO blocks that aren't in use. Since
+  the remoteproc driver is responsibe for managing the power states of the
+  remote firmware, the GPIO driver does not require to konow the firmware's
+  running states.
+
+  When the wakeup bit is set, the remote firmware should configure the line
+  as a wakeup source. The firmware should send the notification message to
+  Linux after it is woken from the GPIO line.
+
+  - 0: Disable wakeup from GPIO
+  - 1: Enable wakeup from GPIO
+
+**Reply:**
+
+.. code-block:: none
+
+   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
+   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05..0x09 |0x0A |0x0B |0x0C |0x0D|
+   | 5   | 1   | 0   | 1   | 1   |  0        |line |port | err | 0  |
+   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
+
+- **err**: Error code from the remote core.
+
+  - 0: Success
+  - 1: General error (early remote software only returns this unclassified error)
+  - 2: Not supported (A command is not supported by the remote firmware)
+  - 3: Resource not available (The resource is not allocated to the Linux)
+  - 4: Resource busy (The resource is already used)
+  - 5: Parameter error
+
+GPIO_RPMSG_OUTPUT_INIT (Cmd=1)
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+**Request:**
+
+.. code-block:: none
+
+   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
+   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05..0x09 |0x0A |0x0B |0x0C |0x0D|
+   | 5   | 1   | 0   | 0   | 1   |  0        |line |port | val | 0  |
+   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
+
+- **val**: Output level.
+
+  - 0: Low
+  - 1: High
+
+**Reply:**
+
+.. code-block:: none
+
+   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
+   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05..0x09 |0x0A |0x0B |0x0C |0x0D|
+   | 5   | 1   | 0   | 1   | 1   |  0        |line |port | err | 0  |
+   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
+
+- **err**: See above for definitions.
+
+GPIO_RPMSG_INPUT_GET (Cmd=2)
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+**Request:**
+
+.. code-block:: none
+
+   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
+   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05..0x09 |0x0A |0x0B |0x0C |0x0D|
+   | 5   | 1   | 0   | 0   | 2   |  0        |line |port | 0   | 0  |
+   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
+
+**Reply:**
+
+.. code-block:: none
+
+   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+-----+
+   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05..0x09 |0x0A |0x0B |0x0C |0x0D |
+   | 5   | 1   | 0   | 1   | 2   |  0        |line |port | err |level|
+   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+-----+
+
+- **err**: See above for definitions.
+
+- **level**: Input level.
+
+  - 0: Low
+  - 1: High
+
+GPIO_RPMSG_GET_DIRECTION (Cmd=3)
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+**Request:**
+
+.. code-block:: none
+
+   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
+   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05..0x09 |0x0A |0x0B |0x0C |0x0D|
+   | 5   | 1   | 0   | 0   | 3   |  0        |line |port | 0   | 0  |
+   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
+
+**Reply:**
+
+.. code-block:: none
+
+   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+-----+
+   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05..0x09 |0x0A |0x0B |0x0C |0x0D |
+   | 5   | 1   | 0   | 1   | 3   |  0        |line |port | err | dir |
+   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+-----+
+
+- **err**: See above for definitions.
+
+- **dir**: Direction.
+
+  - 0: Output
+  - 1: Input
+
+Notification Message
+--------------------
+
+Notifications are sent with **Type=2 (GPIO_RPMSG_NOTIFY)**:
+
+When a GPIO line asserts an interrupt on the remote processor, the firmware
+should immediately mask the corresponding interrupt source and send a
+notification message to the Linux. Upon completion of the interrupt
+handling on the Linux side, the driver should issue a
+**GPIO_RPMSG_INPUT_INIT** command to the firmware to unmask the interrupt.
+
+A Notification message can arrive between a SETUP and its REPLY message,
+and the driver is expected to handle this scenario.
+
+.. code-block:: none
+
+   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
+   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05..0x09 |0x0A |0x0B |0x0C |0x0D|
+   | 5   | 1   | 0   | 2   | 0   |  0        |line |port | 0   | 0  |
+   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
+
+- **line**: The GPIO line index.
+- **port**: The GPIO controller index.
+
+The reply message for the notification is optional. The remote firmware can
+implement it to simulate the interrupt acknowledgment behavior.
+
+The notification reply is sent with the byte index 0x4=1.
+
+.. code-block:: none
+
+   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
+   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05..0x09 |0x0A |0x0B |0x0C |0x0D|
+   | 5   | 1   | 0   | 2   | 1   |  0        |line |port | 0   | 0  |
+   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
+
+- **line**: The GPIO line index.
+- **port**: The GPIO controller index.
diff --git a/Documentation/driver-api/gpio/index.rst b/Documentation/driver-api/gpio/index.rst
index bee58f709b9a..e5eb1f82f01f 100644
--- a/Documentation/driver-api/gpio/index.rst
+++ b/Documentation/driver-api/gpio/index.rst
@@ -16,6 +16,7 @@ Contents:
    drivers-on-gpio
    bt8xxgpio
    pca953x
+   gpio-rpmsg
 
 Core
 ====
-- 
2.43.0

[PATCH v6 4/5] gpio: rpmsg: add generic rpmsg GPIO driver

[Shenwei Wang]

On an AMP platform, the system may include two processors:
	- An MCU running an RTOS
	- An MPU running Linux

These processors communicate via the RPMSG protocol.
The driver implements the standard GPIO interface, allowing
the Linux side to control GPIO controllers which reside in
the remote processor via RPMSG protocol.

Cc: Bartosz Golaszewski <brgl@bgdev.pl>
Cc: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
---
 drivers/gpio/Kconfig      |  16 ++
 drivers/gpio/Makefile     |   1 +
 drivers/gpio/gpio-rpmsg.c | 490 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 507 insertions(+)
 create mode 100644 drivers/gpio/gpio-rpmsg.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index bd185482a7fd..7a72b5dbd4a9 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1883,6 +1883,22 @@ config GPIO_SODAVILLE
 
 endmenu
 
+menu "RPMSG GPIO drivers"
+	depends on RPMSG
+
+config GPIO_RPMSG
+	tristate "Generic RPMSG GPIO support"
+	select GPIOLIB_IRQCHIP
+	default REMOTEPROC
+	help
+	  Say yes here to support the generic GPIO functions over the RPMSG
+	  bus. Currently supported devices: i.MX7ULP, i.MX8ULP, i.MX8x,and
+	  i.MX9x.
+
+	  If unsure, say N.
+
+endmenu
+
 menu "SPI GPIO expanders"
 	depends on SPI_MASTER
 
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 2421a8fd3733..b1373ec274c8 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -156,6 +156,7 @@ obj-$(CONFIG_GPIO_RDC321X)		+= gpio-rdc321x.o
 obj-$(CONFIG_GPIO_REALTEK_OTTO)		+= gpio-realtek-otto.o
 obj-$(CONFIG_GPIO_REG)			+= gpio-reg.o
 obj-$(CONFIG_GPIO_ROCKCHIP)	+= gpio-rockchip.o
+obj-$(CONFIG_GPIO_RPMSG)		+= gpio-rpmsg.o
 obj-$(CONFIG_GPIO_RTD)			+= gpio-rtd.o
 obj-$(CONFIG_ARCH_SA1100)		+= gpio-sa1100.o
 obj-$(CONFIG_GPIO_SAMA5D2_PIOBU)	+= gpio-sama5d2-piobu.o
diff --git a/drivers/gpio/gpio-rpmsg.c b/drivers/gpio/gpio-rpmsg.c
new file mode 100644
index 000000000000..cf10e2958374
--- /dev/null
+++ b/drivers/gpio/gpio-rpmsg.c
@@ -0,0 +1,490 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2025 NXP
+ *
+ * The driver exports a standard gpiochip interface to control
+ * the GPIO controllers via RPMSG on a remote processor.
+ */
+#include <linux/completion.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/gpio/driver.h>
+#include <linux/init.h>
+#include <linux/irqdomain.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/rpmsg.h>
+#include <linux/rpmsg/rpdev_info.h>
+
+#define RPMSG_GPIO_ID		5
+#define RPMSG_VENDOR		1
+#define RPMSG_VERSION		0
+
+#define GPIOS_PER_PORT		32
+#define RPMSG_TIMEOUT		1000
+
+enum gpio_input_trigger_type {
+	GPIO_RPMSG_TRI_IGNORE,
+	GPIO_RPMSG_TRI_RISING,
+	GPIO_RPMSG_TRI_FALLING,
+	GPIO_RPMSG_TRI_BOTH_EDGE,
+	GPIO_RPMSG_TRI_LOW_LEVEL,
+	GPIO_RPMSG_TRI_HIGH_LEVEL,
+};
+
+enum gpio_rpmsg_header_type {
+	GPIO_RPMSG_SETUP,
+	GPIO_RPMSG_REPLY,
+	GPIO_RPMSG_NOTIFY,
+};
+
+enum gpio_rpmsg_header_cmd {
+	GPIO_RPMSG_INPUT_INIT,
+	GPIO_RPMSG_OUTPUT_INIT,
+	GPIO_RPMSG_INPUT_GET,
+	GPIO_RPMSG_DIRECTION_GET,
+};
+
+struct gpio_rpmsg_head {
+	u8 id;		/* Message ID Code */
+	u8 vendor;	/* Vendor ID number */
+	u8 version;	/* Vendor-specific version number */
+	u8 type;	/* Message type */
+	u8 cmd;		/* Command code */
+	u8 reserved[5];
+} __packed;
+
+struct gpio_rpmsg_packet {
+	struct gpio_rpmsg_head header;
+	u8 pin_idx;
+	u8 port_idx;
+	union {
+		u8 event;
+		u8 retcode;
+		u8 value;
+	} out;
+	union {
+		u8 wakeup;
+		u8 value;
+	} in;
+} __packed __aligned(8);
+
+struct gpio_rpmsg_pin {
+	u8 irq_shutdown;
+	u8 irq_unmask;
+	u8 irq_mask;
+	u32 irq_wake_enable;
+	u32 irq_type;
+	struct gpio_rpmsg_packet msg;
+};
+
+struct gpio_rpmsg_info {
+	struct rpmsg_device *rpdev;
+	struct gpio_rpmsg_packet *notify_msg;
+	struct gpio_rpmsg_packet *reply_msg;
+	struct completion cmd_complete;
+	struct mutex lock;
+	void **port_store;
+};
+
+struct rpmsg_gpio_port {
+	struct gpio_chip gc;
+	struct gpio_rpmsg_pin gpio_pins[GPIOS_PER_PORT];
+	struct gpio_rpmsg_info info;
+	int idx;
+};
+
+static int gpio_send_message(struct rpmsg_gpio_port *port,
+			     struct gpio_rpmsg_packet *msg,
+			     bool sync)
+{
+	struct gpio_rpmsg_info *info = &port->info;
+	int err;
+
+	if (!info->rpdev) {
+		pr_err("rpmsg channel doesn't exist, is remote core ready?\n");
+		return -EINVAL;
+	}
+
+	reinit_completion(&info->cmd_complete);
+	err = rpmsg_send(info->rpdev->ept, (void *)msg,
+			 sizeof(struct gpio_rpmsg_packet));
+	if (err) {
+		dev_err(&info->rpdev->dev, "rpmsg_send failed: %d\n", err);
+		return err;
+	}
+
+	if (sync) {
+		err = wait_for_completion_timeout(&info->cmd_complete,
+						  msecs_to_jiffies(RPMSG_TIMEOUT));
+		if (!err) {
+			dev_err(&info->rpdev->dev, "rpmsg_send timeout!\n");
+			return -ETIMEDOUT;
+		}
+
+		if (info->reply_msg->out.retcode != 0) {
+			dev_err(&info->rpdev->dev, "remote core replies an error: %d!\n",
+				info->reply_msg->out.retcode);
+			return -EINVAL;
+		}
+
+		/* copy the reply message */
+		memcpy(&port->gpio_pins[info->reply_msg->pin_idx].msg,
+		       info->reply_msg, sizeof(*info->reply_msg));
+	}
+
+	return 0;
+}
+
+static struct gpio_rpmsg_packet *gpio_setup_msg_header(struct rpmsg_gpio_port *port,
+						       unsigned int offset,
+						       u8 cmd)
+{
+	struct gpio_rpmsg_packet *msg = &port->gpio_pins[offset].msg;
+
+	memset(msg, 0, sizeof(struct gpio_rpmsg_packet));
+	msg->header.id = RPMSG_GPIO_ID;
+	msg->header.vendor = RPMSG_VENDOR;
+	msg->header.version = RPMSG_VERSION;
+	msg->header.type = GPIO_RPMSG_SETUP;
+	msg->header.cmd = cmd;
+	msg->pin_idx = offset;
+	msg->port_idx = port->idx;
+
+	return msg;
+};
+
+static int rpmsg_gpio_get(struct gpio_chip *gc, unsigned int gpio)
+{
+	struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
+	struct gpio_rpmsg_packet *msg = NULL;
+	int ret;
+
+	guard(mutex)(&port->info.lock);
+
+	msg = gpio_setup_msg_header(port, gpio, GPIO_RPMSG_INPUT_GET);
+
+	ret = gpio_send_message(port, msg, true);
+	if (!ret)
+		ret = !!port->gpio_pins[gpio].msg.in.value;
+
+	return ret;
+}
+
+static int rpmsg_gpio_get_direction(struct gpio_chip *gc, unsigned int gpio)
+{
+	struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
+	struct gpio_rpmsg_packet *msg = NULL;
+	int ret;
+
+	guard(mutex)(&port->info.lock);
+
+	msg = gpio_setup_msg_header(port, gpio, GPIO_RPMSG_DIRECTION_GET);
+
+	ret = gpio_send_message(port, msg, true);
+	if (!ret)
+		ret = !!port->gpio_pins[gpio].msg.in.value;
+
+	return ret;
+}
+
+static int rpmsg_gpio_direction_input(struct gpio_chip *gc, unsigned int gpio)
+{
+	struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
+	struct gpio_rpmsg_packet *msg = NULL;
+
+	guard(mutex)(&port->info.lock);
+
+	msg = gpio_setup_msg_header(port, gpio, GPIO_RPMSG_INPUT_INIT);
+
+	return gpio_send_message(port, msg, true);
+}
+
+static int rpmsg_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
+{
+	struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
+	struct gpio_rpmsg_packet *msg = NULL;
+
+	guard(mutex)(&port->info.lock);
+
+	msg = gpio_setup_msg_header(port, gpio, GPIO_RPMSG_OUTPUT_INIT);
+	msg->out.value = val;
+
+	return gpio_send_message(port, msg, true);
+}
+
+static int rpmsg_gpio_direction_output(struct gpio_chip *gc,
+				       unsigned int gpio,
+				       int val)
+{
+
+	return rpmsg_gpio_set(gc, gpio, val);
+}
+
+static int gpio_rpmsg_irq_set_type(struct irq_data *d, u32 type)
+{
+	struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
+	u32 gpio_idx = d->hwirq;
+	int edge = 0;
+	int ret = 0;
+
+	switch (type) {
+	case IRQ_TYPE_EDGE_RISING:
+		edge = GPIO_RPMSG_TRI_RISING;
+		irq_set_handler_locked(d, handle_simple_irq);
+		break;
+	case IRQ_TYPE_EDGE_FALLING:
+		edge = GPIO_RPMSG_TRI_FALLING;
+		irq_set_handler_locked(d, handle_simple_irq);
+		break;
+	case IRQ_TYPE_EDGE_BOTH:
+		edge = GPIO_RPMSG_TRI_BOTH_EDGE;
+		irq_set_handler_locked(d, handle_simple_irq);
+		break;
+	case IRQ_TYPE_LEVEL_LOW:
+		edge = GPIO_RPMSG_TRI_LOW_LEVEL;
+		irq_set_handler_locked(d, handle_level_irq);
+		break;
+	case IRQ_TYPE_LEVEL_HIGH:
+		edge = GPIO_RPMSG_TRI_HIGH_LEVEL;
+		irq_set_handler_locked(d, handle_level_irq);
+		break;
+	default:
+		ret = -EINVAL;
+		irq_set_handler_locked(d, handle_bad_irq);
+		break;
+	}
+
+	port->gpio_pins[gpio_idx].irq_type = edge;
+
+	return ret;
+}
+
+static int gpio_rpmsg_irq_set_wake(struct irq_data *d, u32 enable)
+{
+	struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
+	u32 gpio_idx = d->hwirq;
+
+	port->gpio_pins[gpio_idx].irq_wake_enable = enable;
+
+	return 0;
+}
+
+/*
+ * This function will be called at:
+ *  - one interrupt setup.
+ *  - the end of one interrupt happened
+ * The gpio over rpmsg driver will not write the real register, so save
+ * all infos before this function and then send all infos to M core in this
+ * step.
+ */
+static void gpio_rpmsg_unmask_irq(struct irq_data *d)
+{
+	struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
+	u32 gpio_idx = d->hwirq;
+
+	port->gpio_pins[gpio_idx].irq_unmask = 1;
+}
+
+static void gpio_rpmsg_mask_irq(struct irq_data *d)
+{
+	struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
+	u32 gpio_idx = d->hwirq;
+	/*
+	 * No need to implement the callback at A core side.
+	 * M core will mask interrupt after a interrupt occurred, and then
+	 * sends a notify to A core.
+	 * After A core dealt with the notify, A core will send a rpmsg to
+	 * M core to unmask this interrupt again.
+	 */
+	port->gpio_pins[gpio_idx].irq_mask = 1;
+}
+
+static void gpio_rpmsg_irq_shutdown(struct irq_data *d)
+{
+	struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
+	u32 gpio_idx = d->hwirq;
+
+	port->gpio_pins[gpio_idx].irq_shutdown = 1;
+}
+
+static void gpio_rpmsg_irq_bus_lock(struct irq_data *d)
+{
+	struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
+
+	mutex_lock(&port->info.lock);
+}
+
+static void gpio_rpmsg_irq_bus_sync_unlock(struct irq_data *d)
+{
+	struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
+	struct gpio_rpmsg_packet *msg = NULL;
+	u32 gpio_idx = d->hwirq;
+
+	if (!port)
+		return;
+
+	/*
+	 * For mask irq, do nothing here.
+	 * M core will mask interrupt after a interrupt occurred, and then
+	 * sends a notify to A core.
+	 * After A core dealt with the notify, A core will send a rpmsg to
+	 * M core to unmask this interrupt again.
+	 */
+
+	if (port->gpio_pins[gpio_idx].irq_mask && !port->gpio_pins[gpio_idx].irq_unmask) {
+		port->gpio_pins[gpio_idx].irq_mask = 0;
+		mutex_unlock(&port->info.lock);
+		return;
+	}
+
+	msg = gpio_setup_msg_header(port, gpio_idx, GPIO_RPMSG_INPUT_INIT);
+
+	if (port->gpio_pins[gpio_idx].irq_shutdown) {
+		msg->out.event = GPIO_RPMSG_TRI_IGNORE;
+		msg->in.wakeup = 0;
+		port->gpio_pins[gpio_idx].irq_shutdown = 0;
+	} else {
+		 /* if not set irq type, then use low level as trigger type */
+		msg->out.event = port->gpio_pins[gpio_idx].irq_type;
+		if (!msg->out.event)
+			msg->out.event = GPIO_RPMSG_TRI_LOW_LEVEL;
+		if (port->gpio_pins[gpio_idx].irq_unmask) {
+			msg->in.wakeup = 0;
+			port->gpio_pins[gpio_idx].irq_unmask = 0;
+		} else /* irq set wake */
+			msg->in.wakeup = port->gpio_pins[gpio_idx].irq_wake_enable;
+	}
+
+	gpio_send_message(port, msg, false);
+	mutex_unlock(&port->info.lock);
+}
+
+static const struct irq_chip gpio_rpmsg_irq_chip = {
+	.irq_mask = gpio_rpmsg_mask_irq,
+	.irq_unmask = gpio_rpmsg_unmask_irq,
+	.irq_set_wake = gpio_rpmsg_irq_set_wake,
+	.irq_set_type = gpio_rpmsg_irq_set_type,
+	.irq_shutdown = gpio_rpmsg_irq_shutdown,
+	.irq_bus_lock = gpio_rpmsg_irq_bus_lock,
+	.irq_bus_sync_unlock = gpio_rpmsg_irq_bus_sync_unlock,
+	.flags = IRQCHIP_IMMUTABLE,
+};
+
+static int rpmsg_gpio_callback(struct rpmsg_device *rpdev,
+			       void *data, int len, void *priv, u32 src)
+{
+	struct gpio_rpmsg_packet *msg = (struct gpio_rpmsg_packet *)data;
+	struct rpmsg_gpio_port *port = NULL;
+	struct rpdev_platform_info *drvdata;
+
+	drvdata = dev_get_drvdata(&rpdev->dev);
+	if (msg)
+		port = drvdata->channel_devices[msg->port_idx];
+
+	if (!port)
+		return -ENODEV;
+
+	if (msg->header.type == GPIO_RPMSG_REPLY) {
+		port->info.reply_msg = msg;
+		complete(&port->info.cmd_complete);
+	} else if (msg->header.type == GPIO_RPMSG_NOTIFY) {
+		port->info.notify_msg = msg;
+		generic_handle_domain_irq_safe(port->gc.irq.domain, msg->pin_idx);
+	} else
+		dev_err(&rpdev->dev, "wrong command type!\n");
+
+	return 0;
+}
+
+static void rpmsg_gpio_remove_action(void *data)
+{
+	struct rpmsg_gpio_port *port = data;
+
+	port->info.port_store[port->idx] = NULL;
+}
+
+static int rpmsg_gpio_probe(struct platform_device *pdev)
+{
+	struct rpdev_platform_info *pltdata = pdev->dev.platform_data;
+	struct rpmsg_gpio_port *port;
+	struct gpio_irq_chip *girq;
+	struct gpio_chip *gc;
+	int ret;
+
+	if (!pltdata)
+		return -EPROBE_DEFER;
+
+	port = devm_kzalloc(&pdev->dev, sizeof(*port), GFP_KERNEL);
+	if (!port)
+		return -ENOMEM;
+
+	ret = device_property_read_u32(&pdev->dev, "reg", &port->idx);
+	if (ret)
+		return ret;
+
+	if (port->idx > MAX_DEV_PER_CHANNEL)
+		return -EINVAL;
+
+	ret = devm_mutex_init(&pdev->dev, &port->info.lock);
+	if (ret)
+		return ret;
+
+	init_completion(&port->info.cmd_complete);
+	port->info.rpdev = pltdata->rpdev;
+	port->info.port_store = pltdata->channel_devices;
+	port->info.port_store[port->idx] = port;
+	if (!pltdata->rx_callback)
+		pltdata->rx_callback = rpmsg_gpio_callback;
+
+	gc = &port->gc;
+	gc->owner = THIS_MODULE;
+	gc->parent = &pdev->dev;
+	gc->label = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s-gpio%d",
+				   pltdata->rproc_name, port->idx);
+	gc->ngpio = GPIOS_PER_PORT;
+	gc->base = -1;
+
+	gc->direction_input = rpmsg_gpio_direction_input;
+	gc->direction_output = rpmsg_gpio_direction_output;
+	gc->get_direction = rpmsg_gpio_get_direction;
+	gc->get = rpmsg_gpio_get;
+	gc->set = rpmsg_gpio_set;
+
+	platform_set_drvdata(pdev, port);
+	girq = &gc->irq;
+	gpio_irq_chip_set_chip(girq, &gpio_rpmsg_irq_chip);
+	girq->parent_handler = NULL;
+	girq->num_parents = 0;
+	girq->parents = NULL;
+	girq->chip->name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s-gpio%d",
+					  pltdata->rproc_name, port->idx);
+
+	ret = devm_add_action_or_reset(&pdev->dev, rpmsg_gpio_remove_action, port);
+	if (ret)
+		return ret;
+
+	return devm_gpiochip_add_data(&pdev->dev, gc, port);
+}
+
+static const struct of_device_id rpmsg_gpio_dt_ids[] = {
+	{ .compatible = "rpmsg-gpio" },
+	{ /* sentinel */ }
+};
+
+static struct platform_driver rpmsg_gpio_driver = {
+	.driver	= {
+		.name = "gpio-rpmsg",
+		.of_match_table = rpmsg_gpio_dt_ids,
+	},
+	.probe = rpmsg_gpio_probe,
+};
+
+module_platform_driver(rpmsg_gpio_driver);
+
+MODULE_AUTHOR("Shenwei Wang <shenwei.wang@nxp.com>");
+MODULE_DESCRIPTION("generic rpmsg gpio driver");
+MODULE_LICENSE("GPL");
-- 
2.43.0

[PATCH v6 5/5] arm64: dts: imx8ulp: Add rpmsg node under imx_rproc

[Shenwei Wang]

Add the RPMSG bus node along with its GPIO subnodes to the device
tree.

Enable remote device communication and GPIO control via RPMSG on
the i.MX platform.

Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8ulp.dtsi | 27 ++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
index 13b01f3aa2a4..93846435c6c1 100644
--- a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
@@ -191,6 +191,33 @@ scmi_sensor: protocol@15 {
 	cm33: remoteproc-cm33 {
 		compatible = "fsl,imx8ulp-cm33";
 		status = "disabled";
+
+		rpmsg {
+			rpmsg-io-channel {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				rpmsg_gpioa: gpio@0 {
+					compatible = "rpmsg-gpio";
+					reg = <0>;
+					gpio-controller;
+					#gpio-cells = <2>;
+					#interrupt-cells = <2>;
+					interrupt-controller;
+					interrupt-parent = <&rpmsg_gpioa>;
+				};
+
+				rpmsg_gpiob: gpio@1 {
+					compatible = "rpmsg-gpio";
+					reg = <1>;
+					gpio-controller;
+					#gpio-cells = <2>;
+					#interrupt-cells = <2>;
+					interrupt-controller;
+					interrupt-parent = <&rpmsg_gpiob>;
+				};
+			};
+		};
 	};
 
 	soc: soc@0 {
-- 
2.43.0

Re: [PATCH v6 1/5] dt-bindings: remoteproc: imx_rproc: Add "rpmsg" subnode support

[Rob Herring]

On Fri, Dec 12, 2025 at 01:43:37PM -0600, Shenwei Wang wrote:
> Remote processors may announce multiple devices (e.g., I2C, GPIO) over
> an RPMSG channel. 

Which channel does that happen on?

> These devices may require corresponding device tree
> nodes, especially when acting as providers, to supply phandles for their
> consumers.
> 
> Define an RPMSG node to work as a container for a group of RPMSG channels
> under the imx_rproc node.
> 
> Each subnode within "rpmsg" represents an individual RPMSG channel. The
> name of each subnode corresponds to the channel name as defined by the
> remote processor.
> 
> All remote devices associated with a given channel are defined as child
> nodes under the corresponding channel node.
> 
> Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
> ---
>  .../devicetree/bindings/gpio/gpio-rpmsg.yaml  | 49 +++++++++++++++++
>  .../bindings/remoteproc/fsl,imx-rproc.yaml    | 54 +++++++++++++++++++
>  2 files changed, 103 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-rpmsg.yaml
> 
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-rpmsg.yaml b/Documentation/devicetree/bindings/gpio/gpio-rpmsg.yaml
> new file mode 100644
> index 000000000000..b3e1a5dbf731
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-rpmsg.yaml
> @@ -0,0 +1,49 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/gpio/gpio-rpmsg.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Generic GPIO driver over RPMSG

The driver doesn't go over RPMSG. It's a GPIO provider or protocol.

> +
> +maintainers:
> +  - Shenwei Wang <shenwei.wang@nxp.com>
> +
> +description:
> +  On an AMP platform, some GPIO controllers are exposed by the remote processor
> +  through the RPMSG bus. The RPMSG GPIO transport protocol defines the packet
> +  structure and communication flow between Linux and the remote firmware. Those
> +  controllers are managed via this transport protocol.

Got a reference to where any of this is defined?

> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - items:
> +          - enum:
> +              - fsl,rpmsg-gpio
> +          - const: rpmsg-gpio
> +      - const: rpmsg-gpio
> +
> +  reg:
> +    maxItems: 1

I still don't understand how 'reg' is determined. You may have explained 
it previously, but *this* patch needs to make me understand.

> +
> +  "#gpio-cells":
> +    const: 2
> +
> +  gpio-controller: true
> +
> +  interrupt-controller: true
> +
> +  "#interrupt-cells":
> +    const: 2
> +
> +required:
> +  - compatible
> +  - reg
> +  - "#gpio-cells"
> +  - "#interrupt-cells"
> +
> +allOf:
> +  - $ref: /schemas/gpio/gpio.yaml#
> +
> +unevaluatedProperties: false
> diff --git a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
> index 57d75acb0b5e..fd8e5a61a459 100644
> --- a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
> +++ b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
> @@ -84,6 +84,33 @@ properties:
>        This property is to specify the resource id of the remote processor in SoC
>        which supports SCFW
>  
> +  rpmsg:
> +    type: object
> +    additionalProperties: false
> +    description:
> +      Present a group of RPMSG channel devices.
> +
> +    properties:
> +      rpmsg-io-channel:
> +        type: object
> +        additionalProperties: false
> +        properties:
> +          '#address-cells':
> +            const: 1
> +
> +          '#size-cells':
> +            const: 0
> +
> +        patternProperties:
> +          "gpio@[0-9a-f]+$":
> +            type: object
> +            $ref: /schemas/gpio/gpio-rpmsg.yaml#
> +            unevaluatedProperties: false
> +
> +        required:
> +          - '#address-cells'
> +          - '#size-cells'
> +
>  required:
>    - compatible
>  
> @@ -146,5 +173,32 @@ examples:
>                  &mu 3 1>;
>        memory-region = <&vdev0buffer>, <&vdev0vring0>, <&vdev0vring1>, <&rsc_table>;
>        syscon = <&src>;
> +
> +      rpmsg {

What's the purpose of this node? Is it going to contain things other 
than "rpmsg io channels"?

> +        rpmsg-io-channel {
> +          #address-cells = <1>;
> +          #size-cells = <0>;
> +
> +          gpio@0 {
> +            compatible = "rpmsg-gpio";
> +            reg = <0>;
> +            gpio-controller;
> +            #gpio-cells = <2>;
> +            #interrupt-cells = <2>;
> +            interrupt-controller;
> +            interrupt-parent = <&rpmsg_gpioa>;
> +          };
> +
> +          gpio@1 {
> +            compatible = "rpmsg-gpio";
> +            reg = <1>;
> +            gpio-controller;
> +            #gpio-cells = <2>;
> +            #interrupt-cells = <2>;
> +            interrupt-controller;
> +            interrupt-parent = <&rpmsg_gpiob>;
> +          };
> +        };
> +      };
>      };
>  ...
> -- 
> 2.43.0
> 

Re: [PATCH v6 1/5] dt-bindings: remoteproc: imx_rproc: Add "rpmsg" subnode support

[Shenwei Wang]

> -----Original Message-----
> Subject: [EXT] Re: [PATCH v6 1/5] dt-bindings: remoteproc: imx_rproc: Add
> "rpmsg" subnode support
> On Fri, Dec 12, 2025 at 01:43:37PM -0600, Shenwei Wang wrote:
> > Remote processors may announce multiple devices (e.g., I2C, GPIO) over
> > an RPMSG channel.
> 
> Which channel does that happen on?

It runs on the RPMSG  Control channel.

> 
> > These devices may require corresponding device tree nodes, especially
> > when acting as providers, to supply phandles for their consumers.
> >
> > Define an RPMSG node to work as a container for a group of RPMSG
> > channels under the imx_rproc node.
> >
> > Each subnode within "rpmsg" represents an individual RPMSG channel.
> > The name of each subnode corresponds to the channel name as defined by
> > the remote processor.
> >
> > All remote devices associated with a given channel are defined as
> > child nodes under the corresponding channel node.
> >
> > Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
> > ---
> >  .../devicetree/bindings/gpio/gpio-rpmsg.yaml  | 49 +++++++++++++++++
> >  .../bindings/remoteproc/fsl,imx-rproc.yaml    | 54 +++++++++++++++++++
> >  2 files changed, 103 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/gpio/gpio-rpmsg.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/gpio/gpio-rpmsg.yaml
> > b/Documentation/devicetree/bindings/gpio/gpio-rpmsg.yaml
> > new file mode 100644
> > index 000000000000..b3e1a5dbf731
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/gpio/gpio-rpmsg.yaml
> > @@ -0,0 +1,49 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > +---
> > +
> > +title: Generic GPIO driver over RPMSG
> 
> The driver doesn't go over RPMSG. It's a GPIO provider or protocol.
> 

Yes, it might be clearer to phrase it as 'Generic RPMSG GPIO Controller,' since this is 
a GPIO controller driver that operates over the RPMsg bus.

> > +
> > +maintainers:
> > +  - Shenwei Wang <shenwei.wang@nxp.com>
> > +
> > +description:
> > +  On an AMP platform, some GPIO controllers are exposed by the remote
> > +processor
> > +  through the RPMSG bus. The RPMSG GPIO transport protocol defines
> > +the packet
> > +  structure and communication flow between Linux and the remote
> > +firmware. Those
> > +  controllers are managed via this transport protocol.
> 
> Got a reference to where any of this is defined?

Will add a ref to the gpio-rpmsg.rst doc.

> 
> > +
> > +properties:
> > +  compatible:
> > +    oneOf:
> > +      - items:
> > +          - enum:
> > +              - fsl,rpmsg-gpio
> > +          - const: rpmsg-gpio
> > +      - const: rpmsg-gpio
> > +
> > +  reg:
> > +    maxItems: 1
> 
> I still don't understand how 'reg' is determined. You may have explained it
> previously, but *this* patch needs to make me understand.
> 

I see. Will add a description for this item.

> > +
> > +  "#gpio-cells":
> > +    const: 2
> > +
> > +  gpio-controller: true
> >        memory-region = <&vdev0buffer>, <&vdev0vring0>, <&vdev0vring1>,
> <&rsc_table>;
> >        syscon = <&src>;
> > +
> > +      rpmsg {
> 
> What's the purpose of this node? Is it going to contain things other than "rpmsg io
> channels"?

It represents the RPMSG bus between Linux and the remote processor, and will eventually 
host additional channels such as rpmsg-i2c-channel and rpmsg-pwm-channel.

Thanks,
Shenwei

> 
> > +        rpmsg-io-channel {
> > +          #address-cells = <1>;
> > +          #size-cells = <0>;
> > +
> > +          gpio@0 {
> > +            compatible = "rpmsg-gpio";
> > +            reg = <0>;
> > +            gpio-controller;
> > +            #gpio-cells = <2>;
> > +            #interrupt-cells = <2>;
> > +            interrupt-controller;
> > +            interrupt-parent = <&rpmsg_gpioa>;
> > +          };
> > +
> > +          gpio@1 {
> > +            compatible = "rpmsg-gpio";
> > +            reg = <1>;
> > +            gpio-controller;
> > +            #gpio-cells = <2>;
> > +            #interrupt-cells = <2>;
> > +            interrupt-controller;
> > +            interrupt-parent = <&rpmsg_gpiob>;
> > +          };
> > +        };
> > +      };
> >      };
> >  ...
> > --
> > 2.43.0
> >

Re: [PATCH v6 1/5] dt-bindings: remoteproc: imx_rproc: Add "rpmsg" subnode support

[Arnaud POULIQUEN]

Hello Shenwei,


On 12/12/25 20:43, Shenwei Wang wrote:
> Remote processors may announce multiple devices (e.g., I2C, GPIO) over
> an RPMSG channel. These devices may require corresponding device tree
> nodes, especially when acting as providers, to supply phandles for their
> consumers.
> 
> Define an RPMSG node to work as a container for a group of RPMSG channels
> under the imx_rproc node.
> 
> Each subnode within "rpmsg" represents an individual RPMSG channel. The
> name of each subnode corresponds to the channel name as defined by the
> remote processor.
> 
> All remote devices associated with a given channel are defined as child
> nodes under the corresponding channel node.
> 
> Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
> ---
>   .../devicetree/bindings/gpio/gpio-rpmsg.yaml  | 49 +++++++++++++++++
>   .../bindings/remoteproc/fsl,imx-rproc.yaml    | 54 +++++++++++++++++++
>   2 files changed, 103 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/gpio/gpio-rpmsg.yaml
> 
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-rpmsg.yaml b/Documentation/devicetree/bindings/gpio/gpio-rpmsg.yaml
> new file mode 100644
> index 000000000000..b3e1a5dbf731
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-rpmsg.yaml
> @@ -0,0 +1,49 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/gpio/gpio-rpmsg.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Generic GPIO driver over RPMSG
> +
> +maintainers:
> +  - Shenwei Wang <shenwei.wang@nxp.com>
> +
> +description:
> +  On an AMP platform, some GPIO controllers are exposed by the remote processor
> +  through the RPMSG bus. The RPMSG GPIO transport protocol defines the packet
> +  structure and communication flow between Linux and the remote firmware. Those
> +  controllers are managed via this transport protocol.
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - items:
> +          - enum:
> +              - fsl,rpmsg-gpio
> +          - const: rpmsg-gpio
> +      - const: rpmsg-gpio
> +
> +  reg:
> +    maxItems: 1
> +
> +  "#gpio-cells":
> +    const: 2
> +
> +  gpio-controller: true
> +
> +  interrupt-controller: true
> +
> +  "#interrupt-cells":
> +    const: 2
> +
> +required:
> +  - compatible
> +  - reg
> +  - "#gpio-cells"
> +  - "#interrupt-cells"
> +
> +allOf:
> +  - $ref: /schemas/gpio/gpio.yaml#
> +
> +unevaluatedProperties: false
> diff --git a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
> index 57d75acb0b5e..fd8e5a61a459 100644
> --- a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
> +++ b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
> @@ -84,6 +84,33 @@ properties:
>         This property is to specify the resource id of the remote processor in SoC
>         which supports SCFW
>   
> +  rpmsg:
> +    type: object
> +    additionalProperties: false
> +    description:
> +      Present a group of RPMSG channel devices.
> +
> +    properties:
> +      rpmsg-io-channel:
> +        type: object
> +        additionalProperties: false
> +        properties:
> +          '#address-cells':
> +            const: 1
> +
> +          '#size-cells':
> +            const: 0
> +
> +        patternProperties:
> +          "gpio@[0-9a-f]+$":
> +            type: object
> +            $ref: /schemas/gpio/gpio-rpmsg.yaml#
> +            unevaluatedProperties: false
> +
> +        required:
> +          - '#address-cells'
> +          - '#size-cells'
> +
>   required:
>     - compatible
>   
> @@ -146,5 +173,32 @@ examples:
>                   &mu 3 1>;
>         memory-region = <&vdev0buffer>, <&vdev0vring0>, <&vdev0vring1>, <&rsc_table>;
>         syscon = <&src>;
> +
> +      rpmsg {
> +        rpmsg-io-channel {
> +          #address-cells = <1>;
> +          #size-cells = <0>;
> +
> +          gpio@0 {
> +            compatible = "rpmsg-gpio";
> +            reg = <0>;
> +            gpio-controller;
> +            #gpio-cells = <2>;
> +            #interrupt-cells = <2>;
> +            interrupt-controller;
> +            interrupt-parent = <&rpmsg_gpioa>;


"rpmsg_gpioa" seems not defined, or I missed something?

Regards,
Arnaud

> +          };
> +
> +          gpio@1 {
> +            compatible = "rpmsg-gpio";
> +            reg = <1>;
> +            gpio-controller;
> +            #gpio-cells = <2>;
> +            #interrupt-cells = <2>;
> +            interrupt-controller;
> +            interrupt-parent = <&rpmsg_gpiob>;
> +          };
> +        };
> +      };
>       };
>   ...

Re: [PATCH v6 3/5] docs: driver-api: gpio: generic gpio driver over rpmsg bus

[Arnaud POULIQUEN]

Hello Shenwei,

On 12/12/25 20:43, Shenwei Wang wrote:
> Describes the gpio rpmsg transport protocol over the rpmsg bus between
> the cores.
> 
> Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
> ---
>   Documentation/driver-api/gpio/gpio-rpmsg.rst | 232 +++++++++++++++++++
>   Documentation/driver-api/gpio/index.rst      |   1 +
>   2 files changed, 233 insertions(+)
>   create mode 100644 Documentation/driver-api/gpio/gpio-rpmsg.rst
> 
> diff --git a/Documentation/driver-api/gpio/gpio-rpmsg.rst b/Documentation/driver-api/gpio/gpio-rpmsg.rst
> new file mode 100644
> index 000000000000..c78d10a9a85c
> --- /dev/null
> +++ b/Documentation/driver-api/gpio/gpio-rpmsg.rst
> @@ -0,0 +1,232 @@
> +.. SPDX-License-Identifier: GPL-2.0-or-later
> +
> +GPIO RPMSG Protocol
> +===================
> +
> +The GPIO RPMSG transport protocol is used for communication and interaction
> +with GPIO controllers located on remote cores on the RPMSG bus.
> +
> +Message Format
> +--------------
> +
> +The RPMSG message consists of a 14-byte packet with the following layout:
> +
> +.. code-block:: none
> +
> +   +-----+-------+--------+-----+-----+------------+-----+-----+-----+----+
> +   |0x00 |0x01   |0x02    |0x03 |0x04 |0x05..0x09  |0x0A |0x0B |0x0C |0x0D|
> +   | ID  |vendor |version |type |cmd  |reserved[5] |line |port |  data    |
> +   +-----+-------+--------+-----+-----+------------+-----+-----+-----+----+
> +
> +- **ID (Message Identification Code)**: Always be 0x5. Indicates the GPIO message.
> +
> +- **Vendor**: Vendor ID number.
> +  - 0: Reserved
> +  - 1: NXP

These two fields above seem useless for the rpmsg-gpio. Is there any 
reason to keep them?

> +
> +- **Version**: Vendor-specific version number (such as software release).
> +
> +- **Type (Message Type)**: The message type can be one of:
> +
> +  - 0: GPIO_RPMSG_SETUP
> +  - 1: GPIO_RPMSG_REPLY
> +  - 2: GPIO_RPMSG_NOTIFY
> +
> +- **Cmd**: Command code, used for GPIO_RPMSG_SETUP messages.
> +
> +- **reserved[5]**: Reserved bytes. Should always be 0.
> +
> +- **line**: The GPIO line index.
> +
> +- **port**: The GPIO controller index.

The description of port and line should be OS-agnostic.
The notion of a GPIO controller index makes sense from a Linux 
perspective, but here you should provide a hardware description.
Additionally, I suggest reversing the order of port and line, as a line 
is an instance within a port.

Suggested definitions:
   **port**: The GPIO port(bank) index.
   **line**: The GPIO line(pin) index of the port.


> +
> +- **data**: See details in the command description below.
> +
> +GPIO Commands
> +-------------
> +
> +Commands are specified in the **Cmd** field for **GPIO_RPMSG_SETUP** (Type=0) messages.
> +
> +The SETUP message is always sent from Linux to the remote firmware. Each
> +SETUP corresponds to a single REPLY message. The GPIO driver should
> +serialize messages and determine whether a REPLY message is required. If a
> +REPLY message is expected but not received within the specified timeout
> +period (currently 1 second in the Linux driver), the driver should return
> +-ETIMEOUT.
> +
> +GPIO_RPMSG_INPUT_INIT (Cmd=0)
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +**Request:**
> +
> +.. code-block:: none
> +
> +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
> +   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05..0x09 |0x0A |0x0B |0x0C |0x0D|
> +   | 5   | 1   | 0   | 0   | 0   |  0        |line |port | val | wk |
> +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
> +
> +- **val**: Interrupt trigger type.
> +
> +  - 0: Interrupt disabled
> +  - 1: Rising edge trigger
> +  - 2: Falling edge trigger
> +  - 3: Both edge trigger
> +  - 4: Low level trigger
> +  - 5: High level trigger
> +
> +- **wk**: Wakeup enable.
> +
> +  The remote system should always aim to stay in a power-efficient state by
> +  shutting down or clock-gating the GPIO blocks that aren't in use. Since
> +  the remoteproc driver is responsibe for managing the power states of the

s/responsibe/responsible

> +  remote firmware, the GPIO driver does not require to konow the firmware's

s/konow/know/

> +  running states.
> +
> +  When the wakeup bit is set, the remote firmware should configure the line
> +  as a wakeup source. The firmware should send the notification message to
> +  Linux after it is woken from the GPIO line.

What about the other direction? The remote could also need to disable 
message from Linux, right?
In such case the remote might need a message to get the GPIO value on 
wake-up.

> +
> +  - 0: Disable wakeup from GPIO
> +  - 1: Enable wakeup from GPIO
> +
> +**Reply:**
> +
> +.. code-block:: none
> +
> +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
> +   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05..0x09 |0x0A |0x0B |0x0C |0x0D|
> +   | 5   | 1   | 0   | 1   | 1   |  0        |line |port | err | 0  |
> +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
> +
> +- **err**: Error code from the remote core.
> +
> +  - 0: Success
> +  - 1: General error (early remote software only returns this unclassified error)
> +  - 2: Not supported (A command is not supported by the remote firmware)
> +  - 3: Resource not available (The resource is not allocated to the Linux)
> +  - 4: Resource busy (The resource is already used)
> +  - 5: Parameter error
> +
> +GPIO_RPMSG_OUTPUT_INIT (Cmd=1)
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Does this means that we can not change the output level during runtime?
else this should be renamed GPIO_RPMSG_OUTPUT_SET

> +
> +**Request:**
> +
> +.. code-block:: none
> +
> +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
> +   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05..0x09 |0x0A |0x0B |0x0C |0x0D|
> +   | 5   | 1   | 0   | 0   | 1   |  0        |line |port | val | 0  |
> +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
> +
> +- **val**: Output level.
> +
> +  - 0: Low
> +  - 1: High
> +
> +**Reply:**
> +
> +.. code-block:: none
> +
> +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
> +   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05..0x09 |0x0A |0x0B |0x0C |0x0D|
> +   | 5   | 1   | 0   | 1   | 1   |  0        |line |port | err | 0  |
> +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
> +
> +- **err**: See above for definitions.
> +
> +GPIO_RPMSG_INPUT_GET (Cmd=2)
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +**Request:**
> +
> +.. code-block:: none
> +
> +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
> +   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05..0x09 |0x0A |0x0B |0x0C |0x0D|
> +   | 5   | 1   | 0   | 0   | 2   |  0        |line |port | 0   | 0  |
> +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
> +
> +**Reply:**
> +
> +.. code-block:: none
> +
> +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+-----+
> +   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05..0x09 |0x0A |0x0B |0x0C |0x0D |
> +   | 5   | 1   | 0   | 1   | 2   |  0        |line |port | err |level|
> +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+-----+
> +
> +- **err**: See above for definitions.
> +
> +- **level**: Input level.
> +
> +  - 0: Low
> +  - 1: High
> +
> +GPIO_RPMSG_GET_DIRECTION (Cmd=3)
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +**Request:**
> +
> +.. code-block:: none
> +
> +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
> +   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05..0x09 |0x0A |0x0B |0x0C |0x0D|
> +   | 5   | 1   | 0   | 0   | 3   |  0        |line |port | 0   | 0  |
> +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
> +
> +**Reply:**
> +
> +.. code-block:: none
> +
> +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+-----+
> +   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05..0x09 |0x0A |0x0B |0x0C |0x0D |
> +   | 5   | 1   | 0   | 1   | 3   |  0        |line |port | err | dir |
> +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+-----+
> +
> +- **err**: See above for definitions.
> +
> +- **dir**: Direction.
> +
> +  - 0: Output
> +  - 1: Input

So here if I well understand, the list of GPIO are defined in DT and
This command should be use to check the direction during the probe.
Could you document its usage?

> +
> +Notification Message
> +--------------------
> +
> +Notifications are sent with **Type=2 (GPIO_RPMSG_NOTIFY)**:
> +
> +When a GPIO line asserts an interrupt on the remote processor, the firmware
> +should immediately mask the corresponding interrupt source and send a
> +notification message to the Linux. Upon completion of the interrupt
> +handling on the Linux side, the driver should issue a
> +**GPIO_RPMSG_INPUT_INIT** command to the firmware to unmask the interrupt.
> +
> +A Notification message can arrive between a SETUP and its REPLY message,
> +and the driver is expected to handle this scenario.
> +
> +.. code-block:: none
> +
> +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
> +   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05..0x09 |0x0A |0x0B |0x0C |0x0D|
> +   | 5   | 1   | 0   | 2   | 0   |  0        |line |port | 0   | 0  |
> +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
> +
> +- **line**: The GPIO line index.
> +- **port**: The GPIO controller index.
> +
> +The reply message for the notification is optional. The remote firmware can
> +implement it to simulate the interrupt acknowledgment behavior.
> +
> +The notification reply is sent with the byte index 0x4=1.
> +
> +.. code-block:: none
> +
> +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
> +   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05..0x09 |0x0A |0x0B |0x0C |0x0D|
> +   | 5   | 1   | 0   | 2   | 1   |  0        |line |port | 0   | 0  |
> +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
> +
> +- **line**: The GPIO line index.
> +- **port**: The GPIO controller index.

The type seems strange here, it is a reply but tagged as notification, 
what about adding a type 4 GPIO_RPMSG_NOTIFY_REPLY ?


It might be useful to specify the GPIO level as parameter, especially 
for  "3: Both edge trigger"

Thanks,
Arnaud

> diff --git a/Documentation/driver-api/gpio/index.rst b/Documentation/driver-api/gpio/index.rst
> index bee58f709b9a..e5eb1f82f01f 100644
> --- a/Documentation/driver-api/gpio/index.rst
> +++ b/Documentation/driver-api/gpio/index.rst
> @@ -16,6 +16,7 @@ Contents:
>      drivers-on-gpio
>      bt8xxgpio
>      pca953x
> +   gpio-rpmsg
>   
>   Core
>   ====

Re: [PATCH v6 2/5] remoteproc: imx_rproc: Populate devices under "rpmsg" subnode

[Arnaud POULIQUEN]

Hello,

On 12/12/25 20:43, Shenwei Wang wrote:
> Register the RPMsg channel driver and populate remote devices defined
> under the "rpmsg" subnode upon receiving their notification messages.
> 
> The following illustrates the expected DTS layout structure:
> 
> 	cm33: remoteproc-cm33 {
> 		compatible = "fsl,imx8ulp-cm33";
> 
> 		rpmsg {
> 			rpmsg-io-channel {
> 				gpio@0 {
> 					compatible = "fsl,imx-rpmsg-gpio";
> 					reg = <0>;
> 				};
> 
> 				gpio@1 {
> 					compatible = "fsl,imx-rpmsg-gpio";
> 					reg = <1>;
> 				};
> 
> 				...
> 			};
> 
> 			...
> 		};
> 	};
> 
> Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
> ---
>   drivers/remoteproc/imx_rproc.c   | 143 +++++++++++++++++++++++++++++++
>   include/linux/rpmsg/rpdev_info.h |  33 +++++++
>   2 files changed, 176 insertions(+)
>   create mode 100644 include/linux/rpmsg/rpdev_info.h
> 
> diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> index 33f21ab24c92..65ee16fd66d1 100644
> --- a/drivers/remoteproc/imx_rproc.c
> +++ b/drivers/remoteproc/imx_rproc.c
> @@ -15,6 +15,8 @@
>   #include <linux/module.h>
>   #include <linux/of.h>
>   #include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_platform.h>
>   #include <linux/of_reserved_mem.h>
>   #include <linux/platform_device.h>
>   #include <linux/pm_domain.h>
> @@ -22,6 +24,8 @@
>   #include <linux/reboot.h>
>   #include <linux/regmap.h>
>   #include <linux/remoteproc.h>
> +#include <linux/rpmsg.h>
> +#include <linux/rpmsg/rpdev_info.h>
>   #include <linux/workqueue.h>
>   
>   #include "imx_rproc.h"
> @@ -1016,6 +1020,141 @@ static void imx_rproc_destroy_workqueue(void *data)
>   	destroy_workqueue(workqueue);
>   }
>   
> +struct imx_rpmsg_driver {
> +	struct rpmsg_driver rpdrv;
> +	const char *compat;
> +	void *driver_data;
> +};
> +
> +static const char *channel_device_map[][2] = {
> +	{"rpmsg-io-channel", "rpmsg-gpio"},
> +};
> +
> +static int imx_rpmsg_endpoint_cb(struct rpmsg_device *rpdev, void *data,
> +				 int len, void *priv, u32 src)
> +{
> +	struct rpdev_platform_info *drvdata;
> +
> +	drvdata = dev_get_drvdata(&rpdev->dev);
> +	if (drvdata && drvdata->rx_callback)
> +		return drvdata->rx_callback(rpdev, data, len, priv, src);
> +
> +	return 0;
> +}
> +
> +static void imx_rpmsg_endpoint_remove(struct rpmsg_device *rpdev)
> +{
> +	of_platform_depopulate(&rpdev->dev);
> +}
> +
> +static int imx_rpmsg_endpoint_probe(struct rpmsg_device *rpdev)
> +{
> +	struct rpdev_platform_info *drvdata;
> +	struct imx_rpmsg_driver *imx_rpdrv;
> +	struct device *dev = &rpdev->dev;
> +	struct of_dev_auxdata *auxdata;
> +	struct rpmsg_driver *rpdrv;
> +
> +	rpdrv = container_of(dev->driver, struct rpmsg_driver, drv);
> +	imx_rpdrv = container_of(rpdrv, struct imx_rpmsg_driver, rpdrv);
> +
> +	if (!imx_rpdrv->driver_data)
> +		return -EINVAL;
> +
> +	drvdata = devm_kmemdup(dev, imx_rpdrv->driver_data, sizeof(*drvdata), GFP_KERNEL);
> +	if (!drvdata)
> +		return -ENOMEM;
> +
> +	auxdata = devm_kzalloc(dev, sizeof(*auxdata) * 2, GFP_KERNEL);
> +	if (!auxdata)
> +		return -ENOMEM;
> +
> +	drvdata->rpdev = rpdev;
> +	auxdata[0].compatible = devm_kstrdup(dev, imx_rpdrv->compat, GFP_KERNEL);
> +	auxdata[0].platform_data = drvdata;
> +	dev_set_drvdata(dev, drvdata);
> +
> +	of_platform_populate(drvdata->channel_node, NULL, auxdata, dev);
> +
> +	return 0;
> +}
> +
> +static const char *imx_of_rpmsg_is_in_map(const char *name)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(channel_device_map); i++) {
> +		if (strcmp(name, channel_device_map[i][0]) == 0)
> +			return channel_device_map[i][1];
> +	}
> +
> +	return NULL;
> +}
> +
> +static int imx_of_rpmsg_register_rpdriver(struct device_node *channel,
> +					  struct device *dev,
> +					  const char *name,
> +					  const char *compat)
> +{
> +	struct rpdev_platform_info *driver_data;
> +	struct imx_rpmsg_driver *rp_driver;
> +	struct rpmsg_device_id *rpdev_id;
> +
> +	/* rpmsg_device_id is a NULL terminated array */
> +	rpdev_id = devm_kzalloc(dev, sizeof(*rpdev_id) * 2, GFP_KERNEL);
> +	if (!rpdev_id)
> +		return -ENOMEM;
> +
> +	strscpy(rpdev_id[0].name, name, RPMSG_NAME_SIZE);
> +
> +	rp_driver = devm_kzalloc(dev, sizeof(*rp_driver), GFP_KERNEL);
> +	if (!rp_driver)
> +		return -ENOMEM;
> +
> +	driver_data = devm_kzalloc(dev, sizeof(*driver_data), GFP_KERNEL);
> +	if (!driver_data)
> +		return -ENOMEM;
> +
> +	driver_data->rproc_name = dev->of_node->name;
> +	driver_data->channel_node = channel;
> +
> +	rp_driver->rpdrv.drv.name = name;
> +	rp_driver->rpdrv.id_table = rpdev_id;
> +	rp_driver->rpdrv.probe = imx_rpmsg_endpoint_probe;
> +	rp_driver->rpdrv.remove = imx_rpmsg_endpoint_remove;
> +	rp_driver->rpdrv.callback = imx_rpmsg_endpoint_cb;
> +	rp_driver->driver_data = driver_data;
> +	rp_driver->compat = compat;
> +
> +	register_rpmsg_driver(&rp_driver->rpdrv);


I still believe that creating a dependency between remoteproc and rpmsg 
is not suitable.

Please correct me if I’m wrong, but since you define gpio@0 and gpio@1 
in your device tree, you call register_rpmsg_driver() twice, creating 
two instances of the same driver. To differentiate both, you fill the 
rpdrv.id_table with the node names "gpio@0" and "gpio@1".

In a topology with two remote processors, each needing rpmsg-gpio, the 
same situation would not work because you would have two "gpio@0" entries.

What about using the ns-announcement mechanism on the remote side to 
address GPIO port/bank instances?

In the rpmsg-gpio driver, you could define:

static struct rpmsg_device_id rpmsg_gpio_id_table[] = {
     { .name = "rpmsg-gpio" },
     { .name = "rpmsg_gpio-0" },
     { .name = "rpmsg_gpio-1" },
     { .name = "rpmsg_gpio-2" },
     { .name = "rpmsg_gpio-3" },
     { },
};

Then the match between the device tree and the rpmsg bus could be done 
in the rpmsg-gpio driver by matching the rpmsg name with the compatible 
property plus the reg value.

Example device tree snippet:

cm33: remoteproc-cm33 {
     compatible = "fsl,imx8ulp-cm33";

     rpmsg {
         rpmsg-io-channel {
             gpio@0 {
                 compatible = "rpmsg_gpio";
                 reg = <0>;
             };

             gpio@1 {
                 compatible = "rpmsg_gpio";
                 reg = <1>;
             };

             ...
         };

         ...
     };
};

With this approach, rpmsg management could be handled entirely within 
the rpmsg-gpio driver, avoiding the need to register multiple instances 
of the rpmsg_gpio driver.

Regards,
Arnaud

> +
> +	return 0;
> +}
> +
> +static int rproc_of_rpmsg_node_init(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	const char *compat;
> +	int ret;
> +
> +	struct device_node *np __free(device_node) = of_get_child_by_name(dev->of_node, "rpmsg");
> +	if (!np)
> +		return 0;
> +
> +	for_each_child_of_node_scoped(np, child) {
> +		compat = imx_of_rpmsg_is_in_map(child->name);
> +		if (!compat)
> +			ret = of_platform_default_populate(child, NULL, dev);
> +		else
> +			ret = imx_of_rpmsg_register_rpdriver(child, dev, child->name, compat);
> +
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
>   static int imx_rproc_probe(struct platform_device *pdev)
>   {
>   	struct device *dev = &pdev->dev;
> @@ -1114,6 +1253,10 @@ static int imx_rproc_probe(struct platform_device *pdev)
>   		goto err_put_pm;
>   	}
>   
> +	ret = rproc_of_rpmsg_node_init(pdev);
> +	if (ret < 0)
> +		dev_info(dev, "populating 'rpmsg' node failed\n");
> +
>   	return 0;
>   
>   err_put_pm:
> diff --git a/include/linux/rpmsg/rpdev_info.h b/include/linux/rpmsg/rpdev_info.h
> new file mode 100644
> index 000000000000..13e020cd028b
> --- /dev/null
> +++ b/include/linux/rpmsg/rpdev_info.h
> @@ -0,0 +1,33 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright 2025 NXP */
> +
> +/*
> + * @file linux/rpdev_info.h
> + *
> + * @brief Global header file for RPDEV Info
> + *
> + * @ingroup RPMSG
> + */
> +#ifndef __LINUX_RPDEV_INFO_H__
> +#define __LINUX_RPDEV_INFO_H__
> +
> +#define MAX_DEV_PER_CHANNEL    10
> +
> +/**
> + * rpdev_platform_info - store the platform information of rpdev
> + * @rproc_name: the name of the remote proc.
> + * @rpdev: rpmsg channel device
> + * @device_node: pointer to the device node of the rpdev.
> + * @rx_callback: rx callback handler of the rpdev.
> + * @channel_devices: an array of the devices related to the rpdev.
> + */
> +struct rpdev_platform_info {
> +	const char *rproc_name;
> +	struct rpmsg_device *rpdev;
> +	struct device_node *channel_node;
> +	int (*rx_callback)(struct rpmsg_device *rpdev, void *data,
> +			   int len, void *priv, u32 src);
> +	void *channel_devices[MAX_DEV_PER_CHANNEL];
> +};
> +
> +#endif /* __LINUX_RPDEV_INFO_H__ */

Re: [PATCH v6 1/5] dt-bindings: remoteproc: imx_rproc: Add "rpmsg" subnode support

[Shenwei Wang]

> -----Original Message-----
> From: Arnaud POULIQUEN <arnaud.pouliquen@foss.st.com>
> Sent: Thursday, December 18, 2025 4:30 AM
> To: Shenwei Wang <shenwei.wang@nxp.com>; Linus Walleij
> <linusw@kernel.org>; Bartosz Golaszewski <brgl@kernel.org>; Rob Herring
> <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
> <conor+dt@kernel.org>; Bjorn Andersson <andersson@kernel.org>; Mathieu
> Poirier <mathieu.poirier@linaro.org>; Shawn Guo <shawnguo@kernel.org>;
> Sascha Hauer <s.hauer@pengutronix.de>; Jonathan Corbet <corbet@lwn.net>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>; Fabio Estevam
> <festevam@gmail.com>; Peng Fan <peng.fan@nxp.com>; linux-
> gpio@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-remoteproc@vger.kernel.org; imx@lists.linux.dev;
> linux-arm-kernel@lists.infradead.org; linux-doc@vger.kernel.org; dl-linux-imx
> <linux-imx@nxp.com>
> Subject: [EXT] Re: [PATCH v6 1/5] dt-bindings: remoteproc: imx_rproc: Add
> "rpmsg" subnode support
>         syscon = <&src>;
> > +
> > +      rpmsg {
> > +        rpmsg-io-channel {
> > +          #address-cells = <1>;
> > +          #size-cells = <0>;
> > +
> > +          gpio@0 {
> > +            compatible = "rpmsg-gpio";
> > +            reg = <0>;
> > +            gpio-controller;
> > +            #gpio-cells = <2>;
> > +            #interrupt-cells = <2>;
> > +            interrupt-controller;
> > +            interrupt-parent = <&rpmsg_gpioa>;
> 
> 
> "rpmsg_gpioa" seems not defined, or I missed something?
> 

That's just a reference alias in the DTS example, so you can ignore it. I can also remove 
it in the next version to avoid any confusion.

Regards,
Shenwei

> Regards,
> Arnaud
> 
> > +          };
> > +
> > +          gpio@1 {
> > +            compatible = "rpmsg-gpio";
> > +            reg = <1>;
> > +            gpio-controller;
> > +            #gpio-cells = <2>;
> > +            #interrupt-cells = <2>;
> > +            interrupt-controller;
> > +            interrupt-parent = <&rpmsg_gpiob>;
> > +          };
> > +        };
> > +      };
> >       };
> >   ...

Re: [PATCH v6 2/5] remoteproc: imx_rproc: Populate devices under "rpmsg" subnode

[Shenwei Wang]

> -----Original Message-----
> From: Arnaud POULIQUEN <arnaud.pouliquen@foss.st.com>
> Sent: Thursday, December 18, 2025 5:04 AM
> To: Shenwei Wang <shenwei.wang@nxp.com>; Linus Walleij
> <linusw@kernel.org>; Bartosz Golaszewski <brgl@kernel.org>; Rob Herring
> <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
> <conor+dt@kernel.org>; Bjorn Andersson <andersson@kernel.org>; Mathieu
> Poirier <mathieu.poirier@linaro.org>; Shawn Guo <shawnguo@kernel.org>;
> Sascha Hauer <s.hauer@pengutronix.de>; Jonathan Corbet <corbet@lwn.net>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>; Fabio Estevam
> <festevam@gmail.com>; Peng Fan <peng.fan@nxp.com>; linux-
> gpio@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-remoteproc@vger.kernel.org; imx@lists.linux.dev;
> linux-arm-kernel@lists.infradead.org; linux-doc@vger.kernel.org; dl-linux-imx
> <linux-imx@nxp.com>
> Subject: [EXT] Re: [PATCH v6 2/5] remoteproc: imx_rproc: Populate devices
> under "rpmsg" subnode
> 
> > +     rp_driver->rpdrv.drv.name = name;
> > +     rp_driver->rpdrv.id_table = rpdev_id;
> > +     rp_driver->rpdrv.probe = imx_rpmsg_endpoint_probe;
> > +     rp_driver->rpdrv.remove = imx_rpmsg_endpoint_remove;
> > +     rp_driver->rpdrv.callback = imx_rpmsg_endpoint_cb;
> > +     rp_driver->driver_data = driver_data;
> > +     rp_driver->compat = compat;
> > +
> > +     register_rpmsg_driver(&rp_driver->rpdrv);
> 
> 
> I still believe that creating a dependency between remoteproc and rpmsg is not
> suitable.
> 
> Please correct me if I’m wrong, but since you define gpio@0 and gpio@1 in your
> device tree, you call register_rpmsg_driver() twice, creating two instances of the
> same driver. To differentiate both, you fill the rpdrv.id_table with the node
> names "gpio@0" and "gpio@1".
> 

Nope. The function of register_rpmsg_driver is invoked only once per channel.

> In a topology with two remote processors, each needing rpmsg-gpio, the same
> situation would not work because you would have two "gpio@0" entries.
> 

No, it’s working. The gpio-rpmsg driver is designed to support multiple instances. In fact, that’s the reason 
we added the "rpmsg" bus node under the rproc node. 
Please note that you cannot use duplicate labels within the same channel. 

> What about using the ns-announcement mechanism on the remote side to
> address GPIO port/bank instances?
> 
> In the rpmsg-gpio driver, you could define:
> 
> static struct rpmsg_device_id rpmsg_gpio_id_table[] = {
>      { .name = "rpmsg-gpio" },
>      { .name = "rpmsg_gpio-0" },
>      { .name = "rpmsg_gpio-1" },
>      { .name = "rpmsg_gpio-2" },
>      { .name = "rpmsg_gpio-3" },
>      { },
> };
> 

These definitions are redundant. 
The current implementation already supports multiple instances without requiring this 
information to be hardcoded into the driver source.

Regards,
Shenwei

> Then the match between the device tree and the rpmsg bus could be done in the
> rpmsg-gpio driver by matching the rpmsg name with the compatible property plus
> the reg value.
> 
> Example device tree snippet:
> 
> cm33: remoteproc-cm33 {
>      compatible = "fsl,imx8ulp-cm33";
> 
>      rpmsg {
>          rpmsg-io-channel {
>              gpio@0 {
>                  compatible = "rpmsg_gpio";
>                  reg = <0>;
>              };
> 
>              gpio@1 {
>                  compatible = "rpmsg_gpio";
>                  reg = <1>;
>              };
> 
>              ...
>          };
> 
>          ...
>      };
> };
> 
> With this approach, rpmsg management could be handled entirely within the
> rpmsg-gpio driver, avoiding the need to register multiple instances of the
> rpmsg_gpio driver.
> 
> Regards,
> Arnaud
> 
> > +
> > +     return 0;
> > +}
> > +
> > +static int rproc_of_rpmsg_node_init(struct platform_device *pdev) {
> > +     struct device *dev = &pdev->dev;
> > +     const char *compat;
> > +     int ret;
> > +
> > +     struct device_node *np __free(device_node) = of_get_child_by_name(dev-
> >of_node, "rpmsg");
> > +     if (!np)
> > +             return 0;
> > +
> > +     for_each_child_of_node_scoped(np, child) {
> > +             compat = imx_of_rpmsg_is_in_map(child->name);
> > +             if (!compat)
> > +                     ret = of_platform_default_populate(child, NULL, dev);
> > +             else
> > +                     ret = imx_of_rpmsg_register_rpdriver(child, dev,
> > + child->name, compat);
> > +
> > +             if (ret < 0)
> > +                     return ret;
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> >   static int imx_rproc_probe(struct platform_device *pdev)
> >   {
> >       struct device *dev = &pdev->dev; @@ -1114,6 +1253,10 @@ static
> > int imx_rproc_probe(struct platform_device *pdev)
> >               goto err_put_pm;
> >       }
> >
> > +     ret = rproc_of_rpmsg_node_init(pdev);
> > +     if (ret < 0)
> > +             dev_info(dev, "populating 'rpmsg' node failed\n");
> > +
> >       return 0;
> >
> >   err_put_pm:
> > diff --git a/include/linux/rpmsg/rpdev_info.h
> > b/include/linux/rpmsg/rpdev_info.h
> > new file mode 100644
> > index 000000000000..13e020cd028b
> > --- /dev/null
> > +++ b/include/linux/rpmsg/rpdev_info.h
> > @@ -0,0 +1,33 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/* Copyright 2025 NXP */
> > +
> > +/*
> > + * @file linux/rpdev_info.h
> > + *
> > + * @brief Global header file for RPDEV Info
> > + *
> > + * @ingroup RPMSG
> > + */
> > +#ifndef __LINUX_RPDEV_INFO_H__
> > +#define __LINUX_RPDEV_INFO_H__
> > +
> > +#define MAX_DEV_PER_CHANNEL    10
> > +
> > +/**
> > + * rpdev_platform_info - store the platform information of rpdev
> > + * @rproc_name: the name of the remote proc.
> > + * @rpdev: rpmsg channel device
> > + * @device_node: pointer to the device node of the rpdev.
> > + * @rx_callback: rx callback handler of the rpdev.
> > + * @channel_devices: an array of the devices related to the rpdev.
> > + */
> > +struct rpdev_platform_info {
> > +     const char *rproc_name;
> > +     struct rpmsg_device *rpdev;
> > +     struct device_node *channel_node;
> > +     int (*rx_callback)(struct rpmsg_device *rpdev, void *data,
> > +                        int len, void *priv, u32 src);
> > +     void *channel_devices[MAX_DEV_PER_CHANNEL];
> > +};
> > +
> > +#endif /* __LINUX_RPDEV_INFO_H__ */

Re: [PATCH v6 3/5] docs: driver-api: gpio: generic gpio driver over rpmsg bus

[Shenwei Wang]

> -----Original Message-----
> From: Arnaud POULIQUEN <arnaud.pouliquen@foss.st.com>
> Sent: Thursday, December 18, 2025 4:45 AM
> To: Shenwei Wang <shenwei.wang@nxp.com>; Linus Walleij
> <linusw@kernel.org>; Bartosz Golaszewski <brgl@kernel.org>; Rob Herring
> <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
> <conor+dt@kernel.org>; Bjorn Andersson <andersson@kernel.org>; Mathieu
> Poirier <mathieu.poirier@linaro.org>; Shawn Guo <shawnguo@kernel.org>;
> Sascha Hauer <s.hauer@pengutronix.de>; Jonathan Corbet <corbet@lwn.net>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>; Fabio Estevam
> <festevam@gmail.com>; Peng Fan <peng.fan@nxp.com>; linux-
> gpio@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-remoteproc@vger.kernel.org; imx@lists.linux.dev;
> linux-arm-kernel@lists.infradead.org; linux-doc@vger.kernel.org; dl-linux-imx
> <linux-imx@nxp.com>
> Subject: [EXT] Re: [PATCH v6 3/5] docs: driver-api: gpio: generic gpio driver over
> rpmsg bus
> > +
> > + +-----+-------+--------+-----+-----+------------+-----+-----+-----+-
> > + ---+
> > +
> > +- **ID (Message Identification Code)**: Always be 0x5. Indicates the GPIO
> message.
> > +
> > +- **Vendor**: Vendor ID number.
> > +  - 0: Reserved
> > +  - 1: NXP
> 
> These two fields above seem useless for the rpmsg-gpio. Is there any reason to
> keep them?
> 

They are not used so far.
And they are reserved for implementing workarounds that might be required by 
different vendors or different firmware versions.

> > +
> > +- **Version**: Vendor-specific version number (such as software release).
> > +
> > +- **Type (Message Type)**: The message type can be one of:
> > +
> > +  - 0: GPIO_RPMSG_SETUP
> > +  - 1: GPIO_RPMSG_REPLY
> > +  - 2: GPIO_RPMSG_NOTIFY
> > +
> > +- **Cmd**: Command code, used for GPIO_RPMSG_SETUP messages.
> > +
> > +- **reserved[5]**: Reserved bytes. Should always be 0.
> > +
> > +- **line**: The GPIO line index.
> > +
> > +- **port**: The GPIO controller index.
> 
> The description of port and line should be OS-agnostic.
> The notion of a GPIO controller index makes sense from a Linux perspective, but
> here you should provide a hardware description.
> Additionally, I suggest reversing the order of port and line, as a line is an instance
> within a port.
> 
> Suggested definitions:
>    **port**: The GPIO port(bank) index.
>    **line**: The GPIO line(pin) index of the port.
> 

Okay, I’ll use your suggested descriptions in the next version. 
However, I’ll keep the current byte order since it’s the only implementation we’ve been able to test.

> 
> > +
> > +- **data**: See details in the command description below.
> > +
> > +GPIO Commands
> > +-------------
> > +
> > +Commands are specified in the **Cmd** field for **GPIO_RPMSG_SETUP**
> (Type=0) messages.
> > +
> > +The SETUP message is always sent from Linux to the remote firmware.
> > +Each SETUP corresponds to a single REPLY message. The GPIO driver
> > +should serialize messages and determine whether a REPLY message is
> > +required. If a REPLY message is expected but not received within the
> > +specified timeout period (currently 1 second in the Linux driver),
> > +the driver should return -ETIMEOUT.
> > +
> > +GPIO_RPMSG_INPUT_INIT (Cmd=0)
> > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > +
> > +**Request:**
> > +
> > +.. code-block:: none
> > +
> > +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
> > +   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05..0x09 |0x0A |0x0B |0x0C |0x0D|
> > +   | 5   | 1   | 0   | 0   | 0   |  0        |line |port | val | wk |
> > +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
> > +
> > +- **val**: Interrupt trigger type.
> > +
> > +  - 0: Interrupt disabled
> > +  - 1: Rising edge trigger
> > +  - 2: Falling edge trigger
> > +  - 3: Both edge trigger
> > +  - 4: Low level trigger
> > +  - 5: High level trigger
> > +
> > +- **wk**: Wakeup enable.
> > +
> > +  The remote system should always aim to stay in a power-efficient
> > + state by  shutting down or clock-gating the GPIO blocks that aren't
> > + in use. Since  the remoteproc driver is responsibe for managing the
> > + power states of the
> 
> s/responsibe/responsible
> 
> > +  remote firmware, the GPIO driver does not require to konow the
> > + firmware's
> 
> s/konow/know/
> 
> > +  running states.
> > +
> > +  When the wakeup bit is set, the remote firmware should configure
> > + the line  as a wakeup source. The firmware should send the
> > + notification message to  Linux after it is woken from the GPIO line.
> 
> What about the other direction? The remote could also need to disable message
> from Linux, right?

There are no restrictions on that.

> In such case the remote might need a message to get the GPIO value on wake-
> up.
> 

The remote is actually managing the GPIO controller. It can get any information 
regarding the GPIO controller if it wants. 

> > +
> > +  - 0: Disable wakeup from GPIO
> > +  - 1: Enable wakeup from GPIO
> > +
> > +**Reply:**
> > +
> > +.. code-block:: none
> > +
> > +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
> > +   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05..0x09 |0x0A |0x0B |0x0C |0x0D|
> > +   | 5   | 1   | 0   | 1   | 1   |  0        |line |port | err | 0  |
> > +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
> > +
> > +- **err**: Error code from the remote core.
> > +
> > +  - 0: Success
> > +  - 1: General error (early remote software only returns this
> > + unclassified error)
> > +  - 2: Not supported (A command is not supported by the remote
> > + firmware)
> > +  - 3: Resource not available (The resource is not allocated to the
> > + Linux)
> > +  - 4: Resource busy (The resource is already used)
> > +  - 5: Parameter error
> > +
> > +GPIO_RPMSG_OUTPUT_INIT (Cmd=1)
> > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Does this means that we can not change the output level during runtime?
> else this should be renamed GPIO_RPMSG_OUTPUT_SET
> 

No. You can run the INIT cmd multiple times.

> > +
> > +**Request:**
> > +
> > +.. code-block:: none
> > +
> > +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
> > +   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05..0x09 |0x0A |0x0B |0x0C |0x0D|
> > +   | 5   | 1   | 0   | 0   | 1   |  0        |line |port | val | 0  |
> > +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
> > +
> > +- **val**: Output level.
> > +
> > +  - 0: Low
> > +  - 1: High
> > +
> > +**Reply:**
> > +
> > +.. code-block:: none
> > +
> > +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
> > +   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05..0x09 |0x0A |0x0B |0x0C |0x0D|
> > +   | 5   | 1   | 0   | 1   | 1   |  0        |line |port | err | 0  |
> > +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
> > +
> > +- **err**: See above for definitions.
> > +
> > +GPIO_RPMSG_INPUT_GET (Cmd=2)
> > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > +
> > +**Request:**
> > +
> > +.. code-block:: none
> > +
> > +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
> > +   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05..0x09 |0x0A |0x0B |0x0C |0x0D|
> > +   | 5   | 1   | 0   | 0   | 2   |  0        |line |port | 0   | 0  |
> > +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
> > +
> > +**Reply:**
> > +
> > +.. code-block:: none
> > +
> > +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+-----+
> > +   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05..0x09 |0x0A |0x0B |0x0C |0x0D |
> > +   | 5   | 1   | 0   | 1   | 2   |  0        |line |port | err |level|
> > +
> > + +-----+-----+-----+-----+-----+-----------+-----+-----+-----+-----+
> > +
> > +- **err**: See above for definitions.
> > +
> > +- **level**: Input level.
> > +
> > +  - 0: Low
> > +  - 1: High
> > +
> > +GPIO_RPMSG_GET_DIRECTION (Cmd=3)
> > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > +
> > +**Request:**
> > +
> > +.. code-block:: none
> > +
> > +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
> > +   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05..0x09 |0x0A |0x0B |0x0C |0x0D|
> > +   | 5   | 1   | 0   | 0   | 3   |  0        |line |port | 0   | 0  |
> > +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
> > +
> > +**Reply:**
> > +
> > +.. code-block:: none
> > +
> > +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+-----+
> > +   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05..0x09 |0x0A |0x0B |0x0C |0x0D |
> > +   | 5   | 1   | 0   | 1   | 3   |  0        |line |port | err | dir |
> > +
> > + +-----+-----+-----+-----+-----+-----------+-----+-----+-----+-----+
> > +
> > +- **err**: See above for definitions.
> > +
> > +- **dir**: Direction.
> > +
> > +  - 0: Output
> > +  - 1: Input
> 
> So here if I well understand, the list of GPIO are defined in DT and This command
> should be use to check the direction during the probe.
> Could you document its usage?
> 

As the cmd name suggests, it’s used to retrieve the current GPIO line direction. This cmd
can run during the probe phase as well as at other times.

> > +
> > +Notification Message
> > +--------------------
> > +
> > +Notifications are sent with **Type=2 (GPIO_RPMSG_NOTIFY)**:
> > +
> > +When a GPIO line asserts an interrupt on the remote processor, the
> > +firmware should immediately mask the corresponding interrupt source
> > +and send a notification message to the Linux. Upon completion of the
> > +interrupt handling on the Linux side, the driver should issue a
> > +**GPIO_RPMSG_INPUT_INIT** command to the firmware to unmask the
> interrupt.
> > +
> > +A Notification message can arrive between a SETUP and its REPLY
> > +message, and the driver is expected to handle this scenario.
> > +
> > +.. code-block:: none
> > +
> > +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
> > +   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05..0x09 |0x0A |0x0B |0x0C |0x0D|
> > +   | 5   | 1   | 0   | 2   | 0   |  0        |line |port | 0   | 0  |
> > +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
> > +
> > +- **line**: The GPIO line index.
> > +- **port**: The GPIO controller index.
> > +
> > +The reply message for the notification is optional. The remote
> > +firmware can implement it to simulate the interrupt acknowledgment
> behavior.
> > +
> > +The notification reply is sent with the byte index 0x4=1.
> > +
> > +.. code-block:: none
> > +
> > +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
> > +   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05..0x09 |0x0A |0x0B |0x0C |0x0D|
> > +   | 5   | 1   | 0   | 2   | 1   |  0        |line |port | 0   | 0  |
> > +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
> > +
> > +- **line**: The GPIO line index.
> > +- **port**: The GPIO controller index.
> 
> The type seems strange here, it is a reply but tagged as notification, what about
> adding a type 4 GPIO_RPMSG_NOTIFY_REPLY ?
> 

Seems reasonable. I will update it in next version.

> 
> It might be useful to specify the GPIO level as parameter, especially for  "3: Both
> edge trigger"
> 

Okay. Will add a "triggered event type" as a parameter in the notification message in the next version.

Regards,
Shenwei

> Thanks,
> Arnaud
> 
> > diff --git a/Documentation/driver-api/gpio/index.rst
> > b/Documentation/driver-api/gpio/index.rst
> > index bee58f709b9a..e5eb1f82f01f 100644
> > --- a/Documentation/driver-api/gpio/index.rst
> > +++ b/Documentation/driver-api/gpio/index.rst
> > @@ -16,6 +16,7 @@ Contents:
> >      drivers-on-gpio
> >      bt8xxgpio
> >      pca953x
> > +   gpio-rpmsg
> >
> >   Core
> >   ====

Re: [PATCH v6 4/5] gpio: rpmsg: add generic rpmsg GPIO driver

[Bjorn Andersson]

On Fri, Dec 12, 2025 at 01:43:40PM -0600, Shenwei Wang wrote:
> On an AMP platform, the system may include two processors:

We have many examples where there's N systems and it's certainly not
unreasonable to have multiple remote processors expose GPIOs in this
fashion.

> 	- An MCU running an RTOS
> 	- An MPU running Linux
> 
> These processors communicate via the RPMSG protocol.
> The driver implements the standard GPIO interface, allowing
> the Linux side to control GPIO controllers which reside in
> the remote processor via RPMSG protocol.
> 
> Cc: Bartosz Golaszewski <brgl@bgdev.pl>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
> ---
>  drivers/gpio/Kconfig      |  16 ++
>  drivers/gpio/Makefile     |   1 +
>  drivers/gpio/gpio-rpmsg.c | 490 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 507 insertions(+)
>  create mode 100644 drivers/gpio/gpio-rpmsg.c
> 
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index bd185482a7fd..7a72b5dbd4a9 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -1883,6 +1883,22 @@ config GPIO_SODAVILLE
>  
>  endmenu
>  
> +menu "RPMSG GPIO drivers"
> +	depends on RPMSG
> +
> +config GPIO_RPMSG
> +	tristate "Generic RPMSG GPIO support"
> +	select GPIOLIB_IRQCHIP
> +	default REMOTEPROC
> +	help
> +	  Say yes here to support the generic GPIO functions over the RPMSG
> +	  bus. Currently supported devices: i.MX7ULP, i.MX8ULP, i.MX8x,and
> +	  i.MX9x.
> +
> +	  If unsure, say N.
> +
> +endmenu
> +
>  menu "SPI GPIO expanders"
>  	depends on SPI_MASTER
>  
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index 2421a8fd3733..b1373ec274c8 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -156,6 +156,7 @@ obj-$(CONFIG_GPIO_RDC321X)		+= gpio-rdc321x.o
>  obj-$(CONFIG_GPIO_REALTEK_OTTO)		+= gpio-realtek-otto.o
>  obj-$(CONFIG_GPIO_REG)			+= gpio-reg.o
>  obj-$(CONFIG_GPIO_ROCKCHIP)	+= gpio-rockchip.o
> +obj-$(CONFIG_GPIO_RPMSG)		+= gpio-rpmsg.o
>  obj-$(CONFIG_GPIO_RTD)			+= gpio-rtd.o
>  obj-$(CONFIG_ARCH_SA1100)		+= gpio-sa1100.o
>  obj-$(CONFIG_GPIO_SAMA5D2_PIOBU)	+= gpio-sama5d2-piobu.o
> diff --git a/drivers/gpio/gpio-rpmsg.c b/drivers/gpio/gpio-rpmsg.c
> new file mode 100644
> index 000000000000..cf10e2958374
> --- /dev/null
> +++ b/drivers/gpio/gpio-rpmsg.c
> @@ -0,0 +1,490 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright 2025 NXP
> + *
> + * The driver exports a standard gpiochip interface to control
> + * the GPIO controllers via RPMSG on a remote processor.
> + */
> +#include <linux/completion.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/init.h>
> +#include <linux/irqdomain.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/rpmsg.h>
> +#include <linux/rpmsg/rpdev_info.h>
> +
> +#define RPMSG_GPIO_ID		5
> +#define RPMSG_VENDOR		1
> +#define RPMSG_VERSION		0
> +
> +#define GPIOS_PER_PORT		32
> +#define RPMSG_TIMEOUT		1000
> +
> +enum gpio_input_trigger_type {
> +	GPIO_RPMSG_TRI_IGNORE,

These aren't enumerations, they are well defined constants of the
protocol. I think #define is better.

> +	GPIO_RPMSG_TRI_RISING,
> +	GPIO_RPMSG_TRI_FALLING,
> +	GPIO_RPMSG_TRI_BOTH_EDGE,
> +	GPIO_RPMSG_TRI_LOW_LEVEL,
> +	GPIO_RPMSG_TRI_HIGH_LEVEL,
> +};
> +
> +enum gpio_rpmsg_header_type {
> +	GPIO_RPMSG_SETUP,
> +	GPIO_RPMSG_REPLY,
> +	GPIO_RPMSG_NOTIFY,
> +};
> +
> +enum gpio_rpmsg_header_cmd {
> +	GPIO_RPMSG_INPUT_INIT,
> +	GPIO_RPMSG_OUTPUT_INIT,
> +	GPIO_RPMSG_INPUT_GET,
> +	GPIO_RPMSG_DIRECTION_GET,
> +};
> +
> +struct gpio_rpmsg_head {
> +	u8 id;		/* Message ID Code */
> +	u8 vendor;	/* Vendor ID number */
> +	u8 version;	/* Vendor-specific version number */
> +	u8 type;	/* Message type */
> +	u8 cmd;		/* Command code */
> +	u8 reserved[5];
> +} __packed;
> +
> +struct gpio_rpmsg_packet {
> +	struct gpio_rpmsg_head header;
> +	u8 pin_idx;
> +	u8 port_idx;
> +	union {
> +		u8 event;
> +		u8 retcode;
> +		u8 value;
> +	} out;
> +	union {
> +		u8 wakeup;
> +		u8 value;
> +	} in;
> +} __packed __aligned(8);
> +
> +struct gpio_rpmsg_pin {
> +	u8 irq_shutdown;
> +	u8 irq_unmask;
> +	u8 irq_mask;
> +	u32 irq_wake_enable;
> +	u32 irq_type;
> +	struct gpio_rpmsg_packet msg;
> +};
> +
> +struct gpio_rpmsg_info {
> +	struct rpmsg_device *rpdev;
> +	struct gpio_rpmsg_packet *notify_msg;
> +	struct gpio_rpmsg_packet *reply_msg;
> +	struct completion cmd_complete;
> +	struct mutex lock;
> +	void **port_store;
> +};
> +
> +struct rpmsg_gpio_port {
> +	struct gpio_chip gc;
> +	struct gpio_rpmsg_pin gpio_pins[GPIOS_PER_PORT];
> +	struct gpio_rpmsg_info info;
> +	int idx;
> +};
> +
> +static int gpio_send_message(struct rpmsg_gpio_port *port,
> +			     struct gpio_rpmsg_packet *msg,
> +			     bool sync)
> +{
> +	struct gpio_rpmsg_info *info = &port->info;
> +	int err;
> +
> +	if (!info->rpdev) {
> +		pr_err("rpmsg channel doesn't exist, is remote core ready?\n");

How is this possible? You're creating and destroying the platform_device
based on the presence of the rpmsg channel/endpoint, in what case would
you end up here without a valid rpdev?

And if this is to deal with the race during removal, I guess the error
message is wrong and rpdev might go away before you access it below?

> +		return -EINVAL;
> +	}
> +
> +	reinit_completion(&info->cmd_complete);
> +	err = rpmsg_send(info->rpdev->ept, (void *)msg,
> +			 sizeof(struct gpio_rpmsg_packet));
> +	if (err) {
> +		dev_err(&info->rpdev->dev, "rpmsg_send failed: %d\n", err);
> +		return err;
> +	}
> +
> +	if (sync) {
> +		err = wait_for_completion_timeout(&info->cmd_complete,
> +						  msecs_to_jiffies(RPMSG_TIMEOUT));
> +		if (!err) {
> +			dev_err(&info->rpdev->dev, "rpmsg_send timeout!\n");
> +			return -ETIMEDOUT;
> +		}
> +
> +		if (info->reply_msg->out.retcode != 0) {
> +			dev_err(&info->rpdev->dev, "remote core replies an error: %d!\n",
> +				info->reply_msg->out.retcode);
> +			return -EINVAL;
> +		}
> +
> +		/* copy the reply message */
> +		memcpy(&port->gpio_pins[info->reply_msg->pin_idx].msg,
> +		       info->reply_msg, sizeof(*info->reply_msg));
> +	}
> +
> +	return 0;
> +}
> +
> +static struct gpio_rpmsg_packet *gpio_setup_msg_header(struct rpmsg_gpio_port *port,
> +						       unsigned int offset,
> +						       u8 cmd)
> +{
> +	struct gpio_rpmsg_packet *msg = &port->gpio_pins[offset].msg;
> +
> +	memset(msg, 0, sizeof(struct gpio_rpmsg_packet));
> +	msg->header.id = RPMSG_GPIO_ID;
> +	msg->header.vendor = RPMSG_VENDOR;
> +	msg->header.version = RPMSG_VERSION;
> +	msg->header.type = GPIO_RPMSG_SETUP;
> +	msg->header.cmd = cmd;
> +	msg->pin_idx = offset;
> +	msg->port_idx = port->idx;
> +
> +	return msg;
> +};
> +
> +static int rpmsg_gpio_get(struct gpio_chip *gc, unsigned int gpio)
> +{
> +	struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
> +	struct gpio_rpmsg_packet *msg = NULL;

There's no reason to initialize msg here, the first reference is an
assignment.

> +	int ret;
> +
> +	guard(mutex)(&port->info.lock);
> +
> +	msg = gpio_setup_msg_header(port, gpio, GPIO_RPMSG_INPUT_GET);
> +
> +	ret = gpio_send_message(port, msg, true);
> +	if (!ret)
> +		ret = !!port->gpio_pins[gpio].msg.in.value;
> +
> +	return ret;
> +}
> +
> +static int rpmsg_gpio_get_direction(struct gpio_chip *gc, unsigned int gpio)
> +{
> +	struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
> +	struct gpio_rpmsg_packet *msg = NULL;
> +	int ret;
> +
> +	guard(mutex)(&port->info.lock);
> +
> +	msg = gpio_setup_msg_header(port, gpio, GPIO_RPMSG_DIRECTION_GET);
> +
> +	ret = gpio_send_message(port, msg, true);
> +	if (!ret)
> +		ret = !!port->gpio_pins[gpio].msg.in.value;
> +
> +	return ret;
> +}
> +
> +static int rpmsg_gpio_direction_input(struct gpio_chip *gc, unsigned int gpio)
> +{
> +	struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
> +	struct gpio_rpmsg_packet *msg = NULL;
> +
> +	guard(mutex)(&port->info.lock);
> +
> +	msg = gpio_setup_msg_header(port, gpio, GPIO_RPMSG_INPUT_INIT);
> +
> +	return gpio_send_message(port, msg, true);
> +}
> +
> +static int rpmsg_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
> +{
> +	struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
> +	struct gpio_rpmsg_packet *msg = NULL;
> +
> +	guard(mutex)(&port->info.lock);
> +
> +	msg = gpio_setup_msg_header(port, gpio, GPIO_RPMSG_OUTPUT_INIT);
> +	msg->out.value = val;
> +
> +	return gpio_send_message(port, msg, true);
> +}
> +
> +static int rpmsg_gpio_direction_output(struct gpio_chip *gc,
> +				       unsigned int gpio,
> +				       int val)
> +{
> +
> +	return rpmsg_gpio_set(gc, gpio, val);
> +}
> +
> +static int gpio_rpmsg_irq_set_type(struct irq_data *d, u32 type)
> +{
> +	struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> +	u32 gpio_idx = d->hwirq;
> +	int edge = 0;
> +	int ret = 0;
> +
> +	switch (type) {
> +	case IRQ_TYPE_EDGE_RISING:
> +		edge = GPIO_RPMSG_TRI_RISING;
> +		irq_set_handler_locked(d, handle_simple_irq);
> +		break;
> +	case IRQ_TYPE_EDGE_FALLING:
> +		edge = GPIO_RPMSG_TRI_FALLING;
> +		irq_set_handler_locked(d, handle_simple_irq);
> +		break;
> +	case IRQ_TYPE_EDGE_BOTH:
> +		edge = GPIO_RPMSG_TRI_BOTH_EDGE;
> +		irq_set_handler_locked(d, handle_simple_irq);
> +		break;
> +	case IRQ_TYPE_LEVEL_LOW:
> +		edge = GPIO_RPMSG_TRI_LOW_LEVEL;
> +		irq_set_handler_locked(d, handle_level_irq);
> +		break;
> +	case IRQ_TYPE_LEVEL_HIGH:
> +		edge = GPIO_RPMSG_TRI_HIGH_LEVEL;
> +		irq_set_handler_locked(d, handle_level_irq);
> +		break;
> +	default:
> +		ret = -EINVAL;
> +		irq_set_handler_locked(d, handle_bad_irq);
> +		break;
> +	}
> +
> +	port->gpio_pins[gpio_idx].irq_type = edge;
> +
> +	return ret;
> +}
> +
> +static int gpio_rpmsg_irq_set_wake(struct irq_data *d, u32 enable)
> +{
> +	struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> +	u32 gpio_idx = d->hwirq;
> +
> +	port->gpio_pins[gpio_idx].irq_wake_enable = enable;
> +
> +	return 0;
> +}
> +
> +/*
> + * This function will be called at:
> + *  - one interrupt setup.
> + *  - the end of one interrupt happened
> + * The gpio over rpmsg driver will not write the real register, so save
> + * all infos before this function and then send all infos to M core in this
> + * step.
> + */
> +static void gpio_rpmsg_unmask_irq(struct irq_data *d)
> +{
> +	struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> +	u32 gpio_idx = d->hwirq;
> +
> +	port->gpio_pins[gpio_idx].irq_unmask = 1;
> +}
> +
> +static void gpio_rpmsg_mask_irq(struct irq_data *d)
> +{
> +	struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> +	u32 gpio_idx = d->hwirq;
> +	/*
> +	 * No need to implement the callback at A core side.
> +	 * M core will mask interrupt after a interrupt occurred, and then
> +	 * sends a notify to A core.
> +	 * After A core dealt with the notify, A core will send a rpmsg to
> +	 * M core to unmask this interrupt again.

There's nothing in this scheme that dictates that we have A cores and M
cores, or that we have a single core system on both sides, or that they
are Arm cores, please describe things in terms of Linux system and
"remote system".

> +	 */
> +	port->gpio_pins[gpio_idx].irq_mask = 1;
> +}
> +
> +static void gpio_rpmsg_irq_shutdown(struct irq_data *d)
> +{
> +	struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> +	u32 gpio_idx = d->hwirq;
> +
> +	port->gpio_pins[gpio_idx].irq_shutdown = 1;
> +}
> +
> +static void gpio_rpmsg_irq_bus_lock(struct irq_data *d)
> +{
> +	struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> +
> +	mutex_lock(&port->info.lock);
> +}
> +
> +static void gpio_rpmsg_irq_bus_sync_unlock(struct irq_data *d)
> +{
> +	struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> +	struct gpio_rpmsg_packet *msg = NULL;
> +	u32 gpio_idx = d->hwirq;
> +
> +	if (!port)
> +		return;
> +
> +	/*
> +	 * For mask irq, do nothing here.
> +	 * M core will mask interrupt after a interrupt occurred, and then
> +	 * sends a notify to A core.
> +	 * After A core dealt with the notify, A core will send a rpmsg to
> +	 * M core to unmask this interrupt again.
> +	 */
> +
> +	if (port->gpio_pins[gpio_idx].irq_mask && !port->gpio_pins[gpio_idx].irq_unmask) {
> +		port->gpio_pins[gpio_idx].irq_mask = 0;
> +		mutex_unlock(&port->info.lock);
> +		return;
> +	}
> +
> +	msg = gpio_setup_msg_header(port, gpio_idx, GPIO_RPMSG_INPUT_INIT);
> +
> +	if (port->gpio_pins[gpio_idx].irq_shutdown) {
> +		msg->out.event = GPIO_RPMSG_TRI_IGNORE;
> +		msg->in.wakeup = 0;
> +		port->gpio_pins[gpio_idx].irq_shutdown = 0;
> +	} else {
> +		 /* if not set irq type, then use low level as trigger type */
> +		msg->out.event = port->gpio_pins[gpio_idx].irq_type;
> +		if (!msg->out.event)
> +			msg->out.event = GPIO_RPMSG_TRI_LOW_LEVEL;
> +		if (port->gpio_pins[gpio_idx].irq_unmask) {
> +			msg->in.wakeup = 0;
> +			port->gpio_pins[gpio_idx].irq_unmask = 0;
> +		} else /* irq set wake */
> +			msg->in.wakeup = port->gpio_pins[gpio_idx].irq_wake_enable;
> +	}
> +
> +	gpio_send_message(port, msg, false);
> +	mutex_unlock(&port->info.lock);
> +}
> +
> +static const struct irq_chip gpio_rpmsg_irq_chip = {
> +	.irq_mask = gpio_rpmsg_mask_irq,
> +	.irq_unmask = gpio_rpmsg_unmask_irq,
> +	.irq_set_wake = gpio_rpmsg_irq_set_wake,
> +	.irq_set_type = gpio_rpmsg_irq_set_type,
> +	.irq_shutdown = gpio_rpmsg_irq_shutdown,
> +	.irq_bus_lock = gpio_rpmsg_irq_bus_lock,
> +	.irq_bus_sync_unlock = gpio_rpmsg_irq_bus_sync_unlock,
> +	.flags = IRQCHIP_IMMUTABLE,
> +};
> +
> +static int rpmsg_gpio_callback(struct rpmsg_device *rpdev,
> +			       void *data, int len, void *priv, u32 src)
> +{
> +	struct gpio_rpmsg_packet *msg = (struct gpio_rpmsg_packet *)data;
> +	struct rpmsg_gpio_port *port = NULL;
> +	struct rpdev_platform_info *drvdata;
> +
> +	drvdata = dev_get_drvdata(&rpdev->dev);
> +	if (msg)
> +		port = drvdata->channel_devices[msg->port_idx];
> +
> +	if (!port)
> +		return -ENODEV;
> +
> +	if (msg->header.type == GPIO_RPMSG_REPLY) {
> +		port->info.reply_msg = msg;

As soon as you return from this function, the msg buffer is put back
into the virtqueue, so you can't just stash a reference to it here and
hope that it's still available when gpio_send_message() tries to read
it.

> +		complete(&port->info.cmd_complete);
> +	} else if (msg->header.type == GPIO_RPMSG_NOTIFY) {
> +		port->info.notify_msg = msg;

Ditto.

Although notify_msg is assigned, but I never see any further access to
it.

> +		generic_handle_domain_irq_safe(port->gc.irq.domain, msg->pin_idx);
> +	} else
> +		dev_err(&rpdev->dev, "wrong command type!\n");
> +
> +	return 0;
> +}
> +
> +static void rpmsg_gpio_remove_action(void *data)
> +{
> +	struct rpmsg_gpio_port *port = data;
> +
> +	port->info.port_store[port->idx] = NULL;
> +}
> +
> +static int rpmsg_gpio_probe(struct platform_device *pdev)
> +{
> +	struct rpdev_platform_info *pltdata = pdev->dev.platform_data;
> +	struct rpmsg_gpio_port *port;
> +	struct gpio_irq_chip *girq;
> +	struct gpio_chip *gc;
> +	int ret;
> +
> +	if (!pltdata)
> +		return -EPROBE_DEFER;

EPROBE_DEFER would imply that if we try again a bit later, platform_data
is suddenly non-NULL, that seems unlikely.

> +
> +	port = devm_kzalloc(&pdev->dev, sizeof(*port), GFP_KERNEL);
> +	if (!port)
> +		return -ENOMEM;
> +
> +	ret = device_property_read_u32(&pdev->dev, "reg", &port->idx);
> +	if (ret)
> +		return ret;
> +
> +	if (port->idx > MAX_DEV_PER_CHANNEL)
> +		return -EINVAL;
> +
> +	ret = devm_mutex_init(&pdev->dev, &port->info.lock);
> +	if (ret)
> +		return ret;
> +
> +	init_completion(&port->info.cmd_complete);
> +	port->info.rpdev = pltdata->rpdev;
> +	port->info.port_store = pltdata->channel_devices;
> +	port->info.port_store[port->idx] = port;
> +	if (!pltdata->rx_callback)
> +		pltdata->rx_callback = rpmsg_gpio_callback;

What happens if you rmmod your rpmsg gpio driver and then trigger an
interrupt?

> +
> +	gc = &port->gc;
> +	gc->owner = THIS_MODULE;
> +	gc->parent = &pdev->dev;
> +	gc->label = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s-gpio%d",
> +				   pltdata->rproc_name, port->idx);
> +	gc->ngpio = GPIOS_PER_PORT;
> +	gc->base = -1;
> +
> +	gc->direction_input = rpmsg_gpio_direction_input;
> +	gc->direction_output = rpmsg_gpio_direction_output;
> +	gc->get_direction = rpmsg_gpio_get_direction;
> +	gc->get = rpmsg_gpio_get;
> +	gc->set = rpmsg_gpio_set;
> +
> +	platform_set_drvdata(pdev, port);
> +	girq = &gc->irq;
> +	gpio_irq_chip_set_chip(girq, &gpio_rpmsg_irq_chip);
> +	girq->parent_handler = NULL;
> +	girq->num_parents = 0;
> +	girq->parents = NULL;
> +	girq->chip->name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s-gpio%d",
> +					  pltdata->rproc_name, port->idx);
> +
> +	ret = devm_add_action_or_reset(&pdev->dev, rpmsg_gpio_remove_action, port);
> +	if (ret)
> +		return ret;
> +
> +	return devm_gpiochip_add_data(&pdev->dev, gc, port);
> +}
> +
> +static const struct of_device_id rpmsg_gpio_dt_ids[] = {
> +	{ .compatible = "rpmsg-gpio" },
> +	{ /* sentinel */ }
> +};
> +
> +static struct platform_driver rpmsg_gpio_driver = {

It's an "rpmsg gpio driver", but it's a platform_driver...

I don't think this is the correct design, but if it is then this needs
to be well documented.

Same thing as platform_data forms a strong ABI between some other driver
and this platform_driver, this needs to be well documented (but should
be avoided).

Regards,
Bjorn

> +	.driver	= {
> +		.name = "gpio-rpmsg",
> +		.of_match_table = rpmsg_gpio_dt_ids,
> +	},
> +	.probe = rpmsg_gpio_probe,
> +};
> +
> +module_platform_driver(rpmsg_gpio_driver);
> +
> +MODULE_AUTHOR("Shenwei Wang <shenwei.wang@nxp.com>");
> +MODULE_DESCRIPTION("generic rpmsg gpio driver");
> +MODULE_LICENSE("GPL");
> -- 
> 2.43.0
> 

Re: [PATCH v6 2/5] remoteproc: imx_rproc: Populate devices under "rpmsg" subnode

[Bjorn Andersson]

On Fri, Dec 12, 2025 at 01:43:38PM -0600, Shenwei Wang wrote:
> Register the RPMsg channel driver and populate remote devices defined
> under the "rpmsg" subnode upon receiving their notification messages.

Please provide a proper description of what "problem" this patch solves.

> 
> The following illustrates the expected DTS layout structure:
> 
> 	cm33: remoteproc-cm33 {
> 		compatible = "fsl,imx8ulp-cm33";
> 
> 		rpmsg {
> 			rpmsg-io-channel {
> 				gpio@0 {
> 					compatible = "fsl,imx-rpmsg-gpio";
> 					reg = <0>;

Surely there needs to be some "gpio-controller" and "#gpio-cells" here?
Would be useful if the example is somewhat complete, to give a picture
of what's actually going on.

> 				};
> 
> 				gpio@1 {
> 					compatible = "fsl,imx-rpmsg-gpio";
> 					reg = <1>;
> 				};
> 
> 				...
> 			};
> 
> 			...
> 		};
> 	};
> 
> Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
> ---
>  drivers/remoteproc/imx_rproc.c   | 143 +++++++++++++++++++++++++++++++
>  include/linux/rpmsg/rpdev_info.h |  33 +++++++
>  2 files changed, 176 insertions(+)
>  create mode 100644 include/linux/rpmsg/rpdev_info.h
> 
> diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> index 33f21ab24c92..65ee16fd66d1 100644
> --- a/drivers/remoteproc/imx_rproc.c
> +++ b/drivers/remoteproc/imx_rproc.c
> @@ -15,6 +15,8 @@
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_platform.h>
>  #include <linux/of_reserved_mem.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_domain.h>
> @@ -22,6 +24,8 @@
>  #include <linux/reboot.h>
>  #include <linux/regmap.h>
>  #include <linux/remoteproc.h>
> +#include <linux/rpmsg.h>
> +#include <linux/rpmsg/rpdev_info.h>
>  #include <linux/workqueue.h>
>  
>  #include "imx_rproc.h"
> @@ -1016,6 +1020,141 @@ static void imx_rproc_destroy_workqueue(void *data)
>  	destroy_workqueue(workqueue);
>  }
>  
> +struct imx_rpmsg_driver {
> +	struct rpmsg_driver rpdrv;
> +	const char *compat;
> +	void *driver_data;
> +};
> +
> +static const char *channel_device_map[][2] = {
> +	{"rpmsg-io-channel", "rpmsg-gpio"},
> +};
> +
> +static int imx_rpmsg_endpoint_cb(struct rpmsg_device *rpdev, void *data,
> +				 int len, void *priv, u32 src)
> +{
> +	struct rpdev_platform_info *drvdata;
> +
> +	drvdata = dev_get_drvdata(&rpdev->dev);
> +	if (drvdata && drvdata->rx_callback)
> +		return drvdata->rx_callback(rpdev, data, len, priv, src);
> +
> +	return 0;
> +}
> +
> +static void imx_rpmsg_endpoint_remove(struct rpmsg_device *rpdev)
> +{
> +	of_platform_depopulate(&rpdev->dev);
> +}
> +
> +static int imx_rpmsg_endpoint_probe(struct rpmsg_device *rpdev)
> +{
> +	struct rpdev_platform_info *drvdata;
> +	struct imx_rpmsg_driver *imx_rpdrv;
> +	struct device *dev = &rpdev->dev;
> +	struct of_dev_auxdata *auxdata;
> +	struct rpmsg_driver *rpdrv;
> +
> +	rpdrv = container_of(dev->driver, struct rpmsg_driver, drv);
> +	imx_rpdrv = container_of(rpdrv, struct imx_rpmsg_driver, rpdrv);
> +
> +	if (!imx_rpdrv->driver_data)
> +		return -EINVAL;
> +
> +	drvdata = devm_kmemdup(dev, imx_rpdrv->driver_data, sizeof(*drvdata), GFP_KERNEL);
> +	if (!drvdata)
> +		return -ENOMEM;
> +
> +	auxdata = devm_kzalloc(dev, sizeof(*auxdata) * 2, GFP_KERNEL);
> +	if (!auxdata)
> +		return -ENOMEM;
> +
> +	drvdata->rpdev = rpdev;
> +	auxdata[0].compatible = devm_kstrdup(dev, imx_rpdrv->compat, GFP_KERNEL);
> +	auxdata[0].platform_data = drvdata;
> +	dev_set_drvdata(dev, drvdata);
> +
> +	of_platform_populate(drvdata->channel_node, NULL, auxdata, dev);

auxiliary_bus would be a better choice, but I don't understand why you
probe a rpmsg_device for each "gpio channel" and then from that create a
platform_device.

Why don't you just make the rpmsg_device register the gpio controller
directly?

> +
> +	return 0;
> +}
> +
> +static const char *imx_of_rpmsg_is_in_map(const char *name)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(channel_device_map); i++) {
> +		if (strcmp(name, channel_device_map[i][0]) == 0)
> +			return channel_device_map[i][1];
> +	}
> +
> +	return NULL;
> +}
> +
> +static int imx_of_rpmsg_register_rpdriver(struct device_node *channel,
> +					  struct device *dev,
> +					  const char *name,
> +					  const char *compat)
> +{
> +	struct rpdev_platform_info *driver_data;
> +	struct imx_rpmsg_driver *rp_driver;
> +	struct rpmsg_device_id *rpdev_id;
> +
> +	/* rpmsg_device_id is a NULL terminated array */
> +	rpdev_id = devm_kzalloc(dev, sizeof(*rpdev_id) * 2, GFP_KERNEL);
> +	if (!rpdev_id)
> +		return -ENOMEM;
> +
> +	strscpy(rpdev_id[0].name, name, RPMSG_NAME_SIZE);
> +
> +	rp_driver = devm_kzalloc(dev, sizeof(*rp_driver), GFP_KERNEL);
> +	if (!rp_driver)
> +		return -ENOMEM;
> +
> +	driver_data = devm_kzalloc(dev, sizeof(*driver_data), GFP_KERNEL);
> +	if (!driver_data)
> +		return -ENOMEM;
> +
> +	driver_data->rproc_name = dev->of_node->name;
> +	driver_data->channel_node = channel;
> +
> +	rp_driver->rpdrv.drv.name = name;
> +	rp_driver->rpdrv.id_table = rpdev_id;
> +	rp_driver->rpdrv.probe = imx_rpmsg_endpoint_probe;
> +	rp_driver->rpdrv.remove = imx_rpmsg_endpoint_remove;
> +	rp_driver->rpdrv.callback = imx_rpmsg_endpoint_cb;
> +	rp_driver->driver_data = driver_data;
> +	rp_driver->compat = compat;
> +
> +	register_rpmsg_driver(&rp_driver->rpdrv);

This would then also imply that it's the gpio driver that registers the
rpmsg_driver.

> +
> +	return 0;
> +}
> +
> +static int rproc_of_rpmsg_node_init(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	const char *compat;
> +	int ret;
> +
> +	struct device_node *np __free(device_node) = of_get_child_by_name(dev->of_node, "rpmsg");
> +	if (!np)
> +		return 0;
> +
> +	for_each_child_of_node_scoped(np, child) {
> +		compat = imx_of_rpmsg_is_in_map(child->name);
> +		if (!compat)
> +			ret = of_platform_default_populate(child, NULL, dev);

So if you don't recognize the child device node name you just register
platform_devices for each of the children?

> +		else
> +			ret = imx_of_rpmsg_register_rpdriver(child, dev, child->name, compat);
> +
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
>  static int imx_rproc_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -1114,6 +1253,10 @@ static int imx_rproc_probe(struct platform_device *pdev)
>  		goto err_put_pm;
>  	}
>  
> +	ret = rproc_of_rpmsg_node_init(pdev);
> +	if (ret < 0)
> +		dev_info(dev, "populating 'rpmsg' node failed\n");
> +
>  	return 0;
>  
>  err_put_pm:
> diff --git a/include/linux/rpmsg/rpdev_info.h b/include/linux/rpmsg/rpdev_info.h
> new file mode 100644
> index 000000000000..13e020cd028b
> --- /dev/null
> +++ b/include/linux/rpmsg/rpdev_info.h
> @@ -0,0 +1,33 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright 2025 NXP */
> +
> +/*
> + * @file linux/rpdev_info.h
> + *
> + * @brief Global header file for RPDEV Info
> + *
> + * @ingroup RPMSG
> + */
> +#ifndef __LINUX_RPDEV_INFO_H__
> +#define __LINUX_RPDEV_INFO_H__
> +
> +#define MAX_DEV_PER_CHANNEL    10
> +
> +/**
> + * rpdev_platform_info - store the platform information of rpdev
> + * @rproc_name: the name of the remote proc.
> + * @rpdev: rpmsg channel device
> + * @device_node: pointer to the device node of the rpdev.
> + * @rx_callback: rx callback handler of the rpdev.
> + * @channel_devices: an array of the devices related to the rpdev.
> + */
> +struct rpdev_platform_info {

I don't understand what this structure represents. Why is this glue
between the rpmsg_device and a made up platform_device needed?

> +	const char *rproc_name;

You don't need this, because you can rproc_get_by_child(&self) and then
get the remoteproc name from that.

> +	struct rpmsg_device *rpdev;
> +	struct device_node *channel_node;
> +	int (*rx_callback)(struct rpmsg_device *rpdev, void *data,
> +			   int len, void *priv, u32 src);
> +	void *channel_devices[MAX_DEV_PER_CHANNEL];

Why 10? What does it mean?

I think this becomes the list of the 10 grandchildren of the remoteproc
(per child node). So what happens if those matches against two different
drivers, what will rx_callback point to?

> +};

Regards,
Bjorn

> +
> +#endif /* __LINUX_RPDEV_INFO_H__ */
> -- 
> 2.43.0
> 

Re: [PATCH v6 2/5] remoteproc: imx_rproc: Populate devices under "rpmsg" subnode

[Shenwei Wang]

> -----Original Message-----
> From: Bjorn Andersson <andersson@kernel.org>
> Sent: Thursday, December 18, 2025 8:24 PM
> To: Shenwei Wang <shenwei.wang@nxp.com>
> Cc: Linus Walleij <linusw@kernel.org>; Bartosz Golaszewski <brgl@kernel.org>;
> Rob Herring <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>;
> Conor Dooley <conor+dt@kernel.org>; Mathieu Poirier
> <mathieu.poirier@linaro.org>; Shawn Guo <shawnguo@kernel.org>; Sascha
> Hauer <s.hauer@pengutronix.de>; Jonathan Corbet <corbet@lwn.net>;
> Pengutronix Kernel Team <kernel@pengutronix.de>; Fabio Estevam
> <festevam@gmail.com>; Peng Fan <peng.fan@nxp.com>; linux-
> gpio@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-remoteproc@vger.kernel.org; imx@lists.linux.dev;
> linux-arm-kernel@lists.infradead.org; linux-doc@vger.kernel.org; dl-linux-imx
> <linux-imx@nxp.com>
> Subject: [EXT] Re: [PATCH v6 2/5] remoteproc: imx_rproc: Populate devices
> under "rpmsg" subnode
> 
> Caution: This is an external email. Please take care when clicking links or opening
> attachments. When in doubt, report the message using the 'Report this email'
> button
> 
> 
> On Fri, Dec 12, 2025 at 01:43:38PM -0600, Shenwei Wang wrote:
> > Register the RPMsg channel driver and populate remote devices defined
> > under the "rpmsg" subnode upon receiving their notification messages.
> 
> Please provide a proper description of what "problem" this patch solves.
> 
> >
> > The following illustrates the expected DTS layout structure:
> >
> >       cm33: remoteproc-cm33 {
> >               compatible = "fsl,imx8ulp-cm33";
> >
> >               rpmsg {
> >                       rpmsg-io-channel {
> >                               gpio@0 {
> >                                       compatible = "fsl,imx-rpmsg-gpio";
> >                                       reg = <0>;
> 
> Surely there needs to be some "gpio-controller" and "#gpio-cells" here?
> Would be useful if the example is somewhat complete, to give a picture of what's
> actually going on.
> 

Okay. Will add those in next version.

> >                               };
> >
> >                               gpio@1 {
> >                                       compatible = "fsl,imx-rpmsg-gpio";
> >                                       reg = <1>;
> >                               };
> >
> >                               ...
> >                       };
> >
> >                       ...
> >               };
> >       };
> >
> > +     drvdata->rpdev = rpdev;
> > +     auxdata[0].compatible = devm_kstrdup(dev, imx_rpdrv->compat,
> GFP_KERNEL);
> > +     auxdata[0].platform_data = drvdata;
> > +     dev_set_drvdata(dev, drvdata);
> > +
> > +     of_platform_populate(drvdata->channel_node, NULL, auxdata, dev);
> 
> auxiliary_bus would be a better choice, but I don't understand why you probe a
> rpmsg_device for each "gpio channel" and then from that create a
> platform_device.
> 
> Why don't you just make the rpmsg_device register the gpio controller directly?
> 

The "GPIO channel" is just one example-there are also "PWM channel", "I2C channel", and other channels. 
The goal is to manage all these channels under a common logic, which helps avoid redundant code and keeps 
the implementation consistent.

> > +
> > +     return 0;
> > +}
> > +
> > +     rp_driver->rpdrv.drv.name = name;
> > +     rp_driver->rpdrv.id_table = rpdev_id;
> > +     rp_driver->rpdrv.probe = imx_rpmsg_endpoint_probe;
> > +     rp_driver->rpdrv.remove = imx_rpmsg_endpoint_remove;
> > +     rp_driver->rpdrv.callback = imx_rpmsg_endpoint_cb;
> > +     rp_driver->driver_data = driver_data;
> > +     rp_driver->compat = compat;
> > +
> > +     register_rpmsg_driver(&rp_driver->rpdrv);
> 
> This would then also imply that it's the gpio driver that registers the
> rpmsg_driver.
> 
> > +
> > +     return 0;
> > +}
> > +
> > +static int rproc_of_rpmsg_node_init(struct platform_device *pdev) {
> > +     struct device *dev = &pdev->dev;
> > +     const char *compat;
> > +     int ret;
> > +
> > +     struct device_node *np __free(device_node) = of_get_child_by_name(dev-
> >of_node, "rpmsg");
> > +     if (!np)
> > +             return 0;
> > +
> > +     for_each_child_of_node_scoped(np, child) {
> > +             compat = imx_of_rpmsg_is_in_map(child->name);
> > +             if (!compat)
> > +                     ret = of_platform_default_populate(child, NULL,
> > + dev);
> 
> So if you don't recognize the child device node name you just register
> platform_devices for each of the children?
> 

Yes. That would register platform_devices without the platform_data.

> > +             else
> > +                     ret = imx_of_rpmsg_register_rpdriver(child, dev,
> > + child->name, compat);
> > +
> > +             if (ret < 0)
> > +                     return ret;
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> >  static int imx_rproc_probe(struct platform_device *pdev)  {
> >       struct device *dev = &pdev->dev; @@ -1114,6 +1253,10 @@ static
> > int imx_rproc_probe(struct platform_device *pdev)
> >               goto err_put_pm;
> >       }
> >
> > +     ret = rproc_of_rpmsg_node_init(pdev);
> > +     if (ret < 0)
> > +             dev_info(dev, "populating 'rpmsg' node failed\n");
> > +
> >       return 0;
> >
> >  err_put_pm:
> > diff --git a/include/linux/rpmsg/rpdev_info.h
> > b/include/linux/rpmsg/rpdev_info.h
> > new file mode 100644
> > index 000000000000..13e020cd028b
> > --- /dev/null
> > +++ b/include/linux/rpmsg/rpdev_info.h
> > @@ -0,0 +1,33 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/* Copyright 2025 NXP */
> > +
> > +/*
> > + * @file linux/rpdev_info.h
> > + *
> > + * @brief Global header file for RPDEV Info
> > + *
> > + * @ingroup RPMSG
> > + */
> > +#ifndef __LINUX_RPDEV_INFO_H__
> > +#define __LINUX_RPDEV_INFO_H__
> > +
> > +#define MAX_DEV_PER_CHANNEL    10
> > +
> > +/**
> > + * rpdev_platform_info - store the platform information of rpdev
> > + * @rproc_name: the name of the remote proc.
> > + * @rpdev: rpmsg channel device
> > + * @device_node: pointer to the device node of the rpdev.
> > + * @rx_callback: rx callback handler of the rpdev.
> > + * @channel_devices: an array of the devices related to the rpdev.
> > + */
> > +struct rpdev_platform_info {
> 
> I don't understand what this structure represents. Why is this glue between the
> rpmsg_device and a made up platform_device needed?
> 

The purpose is to have a shared array that can be accessed by all devices within 
the same channel.

> > +     const char *rproc_name;
> 
> You don't need this, because you can rproc_get_by_child(&self) and then get the
> remoteproc name from that.
> 

Good to know. Will try it in the next version.

> > +     struct rpmsg_device *rpdev;
> > +     struct device_node *channel_node;
> > +     int (*rx_callback)(struct rpmsg_device *rpdev, void *data,
> > +                        int len, void *priv, u32 src);
> > +     void *channel_devices[MAX_DEV_PER_CHANNEL];
> 
> Why 10? What does it mean?
> 

This is based on practical experience. For example, on the i.MX platform, we typically don't have 
more than eight same devices on the remote system.

> I think this becomes the list of the 10 grandchildren of the remoteproc (per child
> node). So what happens if those matches against two different drivers, what will
> rx_callback point to?
> 

This is the limitation. That's why I used the map to populate the known child device for one specific channel.

Thanks,
Shenwei

> > +};
> 
> Regards,
> Bjorn
> 
> > +
> > +#endif /* __LINUX_RPDEV_INFO_H__ */
> > --
> > 2.43.0
> >

Re: [PATCH v6 4/5] gpio: rpmsg: add generic rpmsg GPIO driver

[Shenwei Wang]

> -----Original Message-----
> From: Bjorn Andersson <andersson@kernel.org>
> Sent: Thursday, December 18, 2025 9:58 AM
> To: Shenwei Wang <shenwei.wang@nxp.com>
> Cc: Linus Walleij <linusw@kernel.org>; Bartosz Golaszewski <brgl@kernel.org>;
> Rob Herring <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>;
> Conor Dooley <conor+dt@kernel.org>; Mathieu Poirier
> <mathieu.poirier@linaro.org>; Shawn Guo <shawnguo@kernel.org>; Sascha
> Hauer <s.hauer@pengutronix.de>; Jonathan Corbet <corbet@lwn.net>;
> Pengutronix Kernel Team <kernel@pengutronix.de>; Fabio Estevam
> On Fri, Dec 12, 2025 at 01:43:40PM -0600, Shenwei Wang wrote:
> > On an AMP platform, the system may include two processors:
> 
> We have many examples where there's N systems and it's certainly not
> unreasonable to have multiple remote processors expose GPIOs in this fashion.
> 

Agreed. There's no restriction on that.

> >       - An MCU running an RTOS
> >       - An MPU running Linux
> >
> > These processors communicate via the RPMSG protocol.
> > The driver implements the standard GPIO interface, allowing the Linux
> > side to control GPIO controllers which reside in the remote processor
> > +
> > +#define GPIOS_PER_PORT               32
> > +#define RPMSG_TIMEOUT                1000
> > +
> > +enum gpio_input_trigger_type {
> > +     GPIO_RPMSG_TRI_IGNORE,
> 
> These aren't enumerations, they are well defined constants of the protocol. I
> think #define is better.
> 
> > +     GPIO_RPMSG_TRI_RISING,
> > +     GPIO_RPMSG_TRI_FALLING,
> > +     GPIO_RPMSG_TRI_BOTH_EDGE,
> > +     GPIO_RPMSG_TRI_LOW_LEVEL,
> > +     GPIO_RPMSG_TRI_HIGH_LEVEL,
> > +};
> > +
> > +enum gpio_rpmsg_header_type {
> > +     GPIO_RPMSG_SETUP,
> > +     GPIO_RPMSG_REPLY,
> > +     GPIO_RPMSG_NOTIFY,
> > +};
> > +
> > +enum gpio_rpmsg_header_cmd {
> > +     GPIO_RPMSG_INPUT_INIT,
> > +     GPIO_RPMSG_OUTPUT_INIT,
> > +     GPIO_RPMSG_INPUT_GET,
> > +     GPIO_RPMSG_DIRECTION_GET,
> > +};
> > +
> > +struct gpio_rpmsg_head {
> > +     u8 id;          /* Message ID Code */
> > +     u8 vendor;      /* Vendor ID number */
> > +     u8 version;     /* Vendor-specific version number */
> > +     u8 type;        /* Message type */
> > +     u8 cmd;         /* Command code */
> > +     u8 reserved[5];
> > +} __packed;
> > +
> > +struct gpio_rpmsg_packet {
> > +     struct gpio_rpmsg_head header;
> > +     u8 pin_idx;
> > +     u8 port_idx;
> > +     union {
> > +             u8 event;
> > +             u8 retcode;
> > +             u8 value;
> > +     } out;
> > +     union {
> > +             u8 wakeup;
> > +             u8 value;
> > +     } in;
> > +} __packed __aligned(8);
> > +
> > +struct gpio_rpmsg_pin {
> > +     u8 irq_shutdown;
> > +     u8 irq_unmask;
> > +     u8 irq_mask;
> > +     u32 irq_wake_enable;
> > +     u32 irq_type;
> > +     struct gpio_rpmsg_packet msg;
> > +};
> > +
> > +struct gpio_rpmsg_info {
> > +     struct rpmsg_device *rpdev;
> > +     struct gpio_rpmsg_packet *notify_msg;
> > +     struct gpio_rpmsg_packet *reply_msg;
> > +     struct completion cmd_complete;
> > +     struct mutex lock;
> > +     void **port_store;
> > +};
> > +
> > +struct rpmsg_gpio_port {
> > +     struct gpio_chip gc;
> > +     struct gpio_rpmsg_pin gpio_pins[GPIOS_PER_PORT];
> > +     struct gpio_rpmsg_info info;
> > +     int idx;
> > +};
> > +
> > +static int gpio_send_message(struct rpmsg_gpio_port *port,
> > +                          struct gpio_rpmsg_packet *msg,
> > +                          bool sync)
> > +{
> > +     struct gpio_rpmsg_info *info = &port->info;
> > +     int err;
> > +
> > +     if (!info->rpdev) {
> > +             pr_err("rpmsg channel doesn't exist, is remote core
> > + ready?\n");
> 
> How is this possible? You're creating and destroying the platform_device based
> on the presence of the rpmsg channel/endpoint, in what case would you end up
> here without a valid rpdev?
> 
> And if this is to deal with the race during removal, I guess the error message is
> wrong and rpdev might go away before you access it below?
> 

Yes, the error message should be improved.

> > +             return -EINVAL;
> > +     }
> > +
> > +     reinit_completion(&info->cmd_complete);
> > +     err = rpmsg_send(info->rpdev->ept, (void *)msg,
> > +                      sizeof(struct gpio_rpmsg_packet));
> > +     if (err) {
> > +             dev_err(&info->rpdev->dev, "rpmsg_send failed: %d\n", err);
> > +             return err;
> > +     }
> > +
> > +     if (sync) {
> > +             err = wait_for_completion_timeout(&info->cmd_complete,
> > +                                               msecs_to_jiffies(RPMSG_TIMEOUT));
> > +             if (!err) {
> > +                     dev_err(&info->rpdev->dev, "rpmsg_send timeout!\n");
> > +                     return -ETIMEDOUT;
> > +             }
> > +
> > +             if (info->reply_msg->out.retcode != 0) {
> > +                     dev_err(&info->rpdev->dev, "remote core replies an error: %d!\n",
> > +                             info->reply_msg->out.retcode);
> > +                     return -EINVAL;
> > +             }
> > +
> > +             /* copy the reply message */
> > +             memcpy(&port->gpio_pins[info->reply_msg->pin_idx].msg,
> > +                    info->reply_msg, sizeof(*info->reply_msg));
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +static struct gpio_rpmsg_packet *gpio_setup_msg_header(struct
> rpmsg_gpio_port *port,
> > +                                                    unsigned int offset,
> > +                                                    u8 cmd) {
> > +     struct gpio_rpmsg_packet *msg = &port->gpio_pins[offset].msg;
> > +
> > +     memset(msg, 0, sizeof(struct gpio_rpmsg_packet));
> > +     msg->header.id = RPMSG_GPIO_ID;
> > +     msg->header.vendor = RPMSG_VENDOR;
> > +     msg->header.version = RPMSG_VERSION;
> > +     msg->header.type = GPIO_RPMSG_SETUP;
> > +     msg->header.cmd = cmd;
> > +     msg->pin_idx = offset;
> > +     msg->port_idx = port->idx;
> > +
> > +     return msg;
> > +};
> > +
> > +static int rpmsg_gpio_get(struct gpio_chip *gc, unsigned int gpio) {
> > +     struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
> > +     struct gpio_rpmsg_packet *msg = NULL;
> 
> There's no reason to initialize msg here, the first reference is an assignment.
> 
> > +     int ret;
> > +
> > +     guard(mutex)(&port->info.lock);
> > +
> > +     msg = gpio_setup_msg_header(port, gpio, GPIO_RPMSG_INPUT_GET);
> > +
> > +     ret = gpio_send_message(port, msg, true);
> > +     if (!ret)
> > +             ret = !!port->gpio_pins[gpio].msg.in.value;
> > +
> > +     return ret;
> > +}
> > +
> > +static int rpmsg_gpio_get_direction(struct gpio_chip *gc, unsigned
> > +int gpio) {
> > +     struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
> > +     struct gpio_rpmsg_packet *msg = NULL;
> > +     int ret;
> > +
> > +     guard(mutex)(&port->info.lock);
> > +
> > +     msg = gpio_setup_msg_header(port, gpio,
> > + GPIO_RPMSG_DIRECTION_GET);
> > +
> > +     ret = gpio_send_message(port, msg, true);
> > +     if (!ret)
> > +             ret = !!port->gpio_pins[gpio].msg.in.value;
> > +
> > +     return ret;
> > +}
> > +
> > +static int rpmsg_gpio_direction_input(struct gpio_chip *gc, unsigned
> > +int gpio) {
> > +     struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
> > +     struct gpio_rpmsg_packet *msg = NULL;
> > +
> > +     guard(mutex)(&port->info.lock);
> > +
> > +     msg = gpio_setup_msg_header(port, gpio, GPIO_RPMSG_INPUT_INIT);
> > +
> > +     return gpio_send_message(port, msg, true); }
> > +
> > +static int rpmsg_gpio_set(struct gpio_chip *gc, unsigned int gpio,
> > +int val) {
> > +     struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
> > +     struct gpio_rpmsg_packet *msg = NULL;
> > +
> > +     guard(mutex)(&port->info.lock);
> > +
> > +     msg = gpio_setup_msg_header(port, gpio, GPIO_RPMSG_OUTPUT_INIT);
> > +     msg->out.value = val;
> > +
> > +     return gpio_send_message(port, msg, true); }
> > +
> > +static int rpmsg_gpio_direction_output(struct gpio_chip *gc,
> > +                                    unsigned int gpio,
> > +                                    int val) {
> > +
> > +     return rpmsg_gpio_set(gc, gpio, val); }
> > +
> > +static int gpio_rpmsg_irq_set_type(struct irq_data *d, u32 type) {
> > +     struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> > +     u32 gpio_idx = d->hwirq;
> > +     int edge = 0;
> > +     int ret = 0;
> > +
> > +     switch (type) {
> > +     case IRQ_TYPE_EDGE_RISING:
> > +             edge = GPIO_RPMSG_TRI_RISING;
> > +             irq_set_handler_locked(d, handle_simple_irq);
> > +             break;
> > +     case IRQ_TYPE_EDGE_FALLING:
> > +             edge = GPIO_RPMSG_TRI_FALLING;
> > +             irq_set_handler_locked(d, handle_simple_irq);
> > +             break;
> > +     case IRQ_TYPE_EDGE_BOTH:
> > +             edge = GPIO_RPMSG_TRI_BOTH_EDGE;
> > +             irq_set_handler_locked(d, handle_simple_irq);
> > +             break;
> > +     case IRQ_TYPE_LEVEL_LOW:
> > +             edge = GPIO_RPMSG_TRI_LOW_LEVEL;
> > +             irq_set_handler_locked(d, handle_level_irq);
> > +             break;
> > +     case IRQ_TYPE_LEVEL_HIGH:
> > +             edge = GPIO_RPMSG_TRI_HIGH_LEVEL;
> > +             irq_set_handler_locked(d, handle_level_irq);
> > +             break;
> > +     default:
> > +             ret = -EINVAL;
> > +             irq_set_handler_locked(d, handle_bad_irq);
> > +             break;
> > +     }
> > +
> > +     port->gpio_pins[gpio_idx].irq_type = edge;
> > +
> > +     return ret;
> > +}
> > +
> > +static int gpio_rpmsg_irq_set_wake(struct irq_data *d, u32 enable) {
> > +     struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> > +     u32 gpio_idx = d->hwirq;
> > +
> > +     port->gpio_pins[gpio_idx].irq_wake_enable = enable;
> > +
> > +     return 0;
> > +}
> > +
> > +/*
> > + * This function will be called at:
> > + *  - one interrupt setup.
> > + *  - the end of one interrupt happened
> > + * The gpio over rpmsg driver will not write the real register, so
> > +save
> > + * all infos before this function and then send all infos to M core
> > +in this
> > + * step.
> > + */
> > +static void gpio_rpmsg_unmask_irq(struct irq_data *d) {
> > +     struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> > +     u32 gpio_idx = d->hwirq;
> > +
> > +     port->gpio_pins[gpio_idx].irq_unmask = 1; }
> > +
> > +static void gpio_rpmsg_mask_irq(struct irq_data *d) {
> > +     struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> > +     u32 gpio_idx = d->hwirq;
> > +     /*
> > +      * No need to implement the callback at A core side.
> > +      * M core will mask interrupt after a interrupt occurred, and then
> > +      * sends a notify to A core.
> > +      * After A core dealt with the notify, A core will send a rpmsg to
> > +      * M core to unmask this interrupt again.
> 
> There's nothing in this scheme that dictates that we have A cores and M cores, or
> that we have a single core system on both sides, or that they are Arm cores,
> please describe things in terms of Linux system and "remote system".
> 

Good point. I will reframe the description in next version.

> > +      */
> > +     port->gpio_pins[gpio_idx].irq_mask = 1; }
> > +
> > +static void gpio_rpmsg_irq_shutdown(struct irq_data *d) {
> > +     struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> > +     u32 gpio_idx = d->hwirq;
> > +
> > +     port->gpio_pins[gpio_idx].irq_shutdown = 1; }
> > +
> > +static void gpio_rpmsg_irq_bus_lock(struct irq_data *d) {
> > +     struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> > +
> > +     mutex_lock(&port->info.lock);
> > +}
> > +
> > +static void gpio_rpmsg_irq_bus_sync_unlock(struct irq_data *d) {
> > +     struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> > +     struct gpio_rpmsg_packet *msg = NULL;
> > +     u32 gpio_idx = d->hwirq;
> > +
> > +     if (!port)
> > +             return;
> > +
> > +     /*
> > +      * For mask irq, do nothing here.
> > +      * M core will mask interrupt after a interrupt occurred, and then
> > +      * sends a notify to A core.
> > +      * After A core dealt with the notify, A core will send a rpmsg to
> > +      * M core to unmask this interrupt again.
> > +      */
> > +
> > +     if (port->gpio_pins[gpio_idx].irq_mask && !port-
> >gpio_pins[gpio_idx].irq_unmask) {
> > +             port->gpio_pins[gpio_idx].irq_mask = 0;
> > +             mutex_unlock(&port->info.lock);
> > +             return;
> > +     }
> > +
> > +     msg = gpio_setup_msg_header(port, gpio_idx,
> > + GPIO_RPMSG_INPUT_INIT);
> > +
> > +     if (port->gpio_pins[gpio_idx].irq_shutdown) {
> > +             msg->out.event = GPIO_RPMSG_TRI_IGNORE;
> > +             msg->in.wakeup = 0;
> > +             port->gpio_pins[gpio_idx].irq_shutdown = 0;
> > +     } else {
> > +              /* if not set irq type, then use low level as trigger type */
> > +             msg->out.event = port->gpio_pins[gpio_idx].irq_type;
> > +             if (!msg->out.event)
> > +                     msg->out.event = GPIO_RPMSG_TRI_LOW_LEVEL;
> > +             if (port->gpio_pins[gpio_idx].irq_unmask) {
> > +                     msg->in.wakeup = 0;
> > +                     port->gpio_pins[gpio_idx].irq_unmask = 0;
> > +             } else /* irq set wake */
> > +                     msg->in.wakeup = port->gpio_pins[gpio_idx].irq_wake_enable;
> > +     }
> > +
> > +     gpio_send_message(port, msg, false);
> > +     mutex_unlock(&port->info.lock);
> > +}
> > +
> > +static const struct irq_chip gpio_rpmsg_irq_chip = {
> > +     .irq_mask = gpio_rpmsg_mask_irq,
> > +     .irq_unmask = gpio_rpmsg_unmask_irq,
> > +     .irq_set_wake = gpio_rpmsg_irq_set_wake,
> > +     .irq_set_type = gpio_rpmsg_irq_set_type,
> > +     .irq_shutdown = gpio_rpmsg_irq_shutdown,
> > +     .irq_bus_lock = gpio_rpmsg_irq_bus_lock,
> > +     .irq_bus_sync_unlock = gpio_rpmsg_irq_bus_sync_unlock,
> > +     .flags = IRQCHIP_IMMUTABLE,
> > +};
> > +
> > +static int rpmsg_gpio_callback(struct rpmsg_device *rpdev,
> > +                            void *data, int len, void *priv, u32 src)
> > +{
> > +     struct gpio_rpmsg_packet *msg = (struct gpio_rpmsg_packet *)data;
> > +     struct rpmsg_gpio_port *port = NULL;
> > +     struct rpdev_platform_info *drvdata;
> > +
> > +     drvdata = dev_get_drvdata(&rpdev->dev);
> > +     if (msg)
> > +             port = drvdata->channel_devices[msg->port_idx];
> > +
> > +     if (!port)
> > +             return -ENODEV;
> > +
> > +     if (msg->header.type == GPIO_RPMSG_REPLY) {
> > +             port->info.reply_msg = msg;
> 
> As soon as you return from this function, the msg buffer is put back into the
> virtqueue, so you can't just stash a reference to it here and hope that it's still
> available when gpio_send_message() tries to read it.
> 

Will resove it in the next version.

> > +             complete(&port->info.cmd_complete);
> > +     } else if (msg->header.type == GPIO_RPMSG_NOTIFY) {
> > +             port->info.notify_msg = msg;
> 
> Ditto.
> 
> Although notify_msg is assigned, but I never see any further access to it.
> 
> > +             generic_handle_domain_irq_safe(port->gc.irq.domain, msg->pin_idx);
> > +     } else
> > +             dev_err(&rpdev->dev, "wrong command type!\n");
> > +
> > +     return 0;
> > +}
> > +
> > +static void rpmsg_gpio_remove_action(void *data) {
> > +     struct rpmsg_gpio_port *port = data;
> > +
> > +     port->info.port_store[port->idx] = NULL; }
> > +
> > +static int rpmsg_gpio_probe(struct platform_device *pdev) {
> > +     struct rpdev_platform_info *pltdata = pdev->dev.platform_data;
> > +     struct rpmsg_gpio_port *port;
> > +     struct gpio_irq_chip *girq;
> > +     struct gpio_chip *gc;
> > +     int ret;
> > +
> > +     if (!pltdata)
> > +             return -EPROBE_DEFER;
> 
> EPROBE_DEFER would imply that if we try again a bit later, platform_data is
> suddenly non-NULL, that seems unlikely.
> 
> > +
> > +     port = devm_kzalloc(&pdev->dev, sizeof(*port), GFP_KERNEL);
> > +     if (!port)
> > +             return -ENOMEM;
> > +
> > +     ret = device_property_read_u32(&pdev->dev, "reg", &port->idx);
> > +     if (ret)
> > +             return ret;
> > +
> > +     if (port->idx > MAX_DEV_PER_CHANNEL)
> > +             return -EINVAL;
> > +
> > +     ret = devm_mutex_init(&pdev->dev, &port->info.lock);
> > +     if (ret)
> > +             return ret;
> > +
> > +     init_completion(&port->info.cmd_complete);
> > +     port->info.rpdev = pltdata->rpdev;
> > +     port->info.port_store = pltdata->channel_devices;
> > +     port->info.port_store[port->idx] = port;
> > +     if (!pltdata->rx_callback)
> > +             pltdata->rx_callback = rpmsg_gpio_callback;
> 
> What happens if you rmmod your rpmsg gpio driver and then trigger an interrupt?
> 

The driver has a rpmsg_gpio_remove_action which will clear the devices in the pltdata->channel_devices[].
In the rpmsg callback function, it will just return -NODEV error.

     if (msg)
             port = drvdata->channel_devices[msg->port_idx];

     if (!port)
             return -ENODEV;

> > +
> > +     gc = &port->gc;
> > +     gc->owner = THIS_MODULE;
> > +     gc->parent = &pdev->dev;
> > +     gc->label = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s-gpio%d",
> > +                                pltdata->rproc_name, port->idx);
> > +     gc->ngpio = GPIOS_PER_PORT;
> > +     gc->base = -1;
> > +
> > +     gc->direction_input = rpmsg_gpio_direction_input;
> > +     gc->direction_output = rpmsg_gpio_direction_output;
> > +     gc->get_direction = rpmsg_gpio_get_direction;
> > +     gc->get = rpmsg_gpio_get;
> > +     gc->set = rpmsg_gpio_set;
> > +
> > +     platform_set_drvdata(pdev, port);
> > +     girq = &gc->irq;
> > +     gpio_irq_chip_set_chip(girq, &gpio_rpmsg_irq_chip);
> > +     girq->parent_handler = NULL;
> > +     girq->num_parents = 0;
> > +     girq->parents = NULL;
> > +     girq->chip->name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s-
> gpio%d",
> > +                                       pltdata->rproc_name,
> > + port->idx);
> > +
> > +     ret = devm_add_action_or_reset(&pdev->dev, rpmsg_gpio_remove_action,
> port);
> > +     if (ret)
> > +             return ret;
> > +
> > +     return devm_gpiochip_add_data(&pdev->dev, gc, port); }
> > +
> > +static const struct of_device_id rpmsg_gpio_dt_ids[] = {
> > +     { .compatible = "rpmsg-gpio" },
> > +     { /* sentinel */ }
> > +};
> > +
> > +static struct platform_driver rpmsg_gpio_driver = {
> 
> It's an "rpmsg gpio driver", but it's a platform_driver...
> 
> I don't think this is the correct design, but if it is then this needs to be well
> documented.
> 
> Same thing as platform_data forms a strong ABI between some other driver and
> this platform_driver, this needs to be well documented (but should be avoided).
> 

Are you suggesting to use "rpmsg_driver" framework here?

Thank you very much for the review.
Shenwei

> Regards,
> Bjorn
> 
> > +     .driver = {
> > +             .name = "gpio-rpmsg",
> > +             .of_match_table = rpmsg_gpio_dt_ids,
> > +     },
> > +     .probe = rpmsg_gpio_probe,
> > +};
> > +
> > +module_platform_driver(rpmsg_gpio_driver);
> > +
> > +MODULE_AUTHOR("Shenwei Wang <shenwei.wang@nxp.com>");
> > +MODULE_DESCRIPTION("generic rpmsg gpio driver");
> > +MODULE_LICENSE("GPL");
> > --
> > 2.43.0
> >

Re: [PATCH v6 4/5] gpio: rpmsg: add generic rpmsg GPIO driver

[Bjorn Andersson]

On Tue, Dec 23, 2025 at 08:20:49PM +0000, Shenwei Wang wrote:
> 
> 
> > -----Original Message-----
> > From: Bjorn Andersson <andersson@kernel.org>
> > Sent: Thursday, December 18, 2025 9:58 AM
> > To: Shenwei Wang <shenwei.wang@nxp.com>
> > Cc: Linus Walleij <linusw@kernel.org>; Bartosz Golaszewski <brgl@kernel.org>;
> > Rob Herring <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>;
> > Conor Dooley <conor+dt@kernel.org>; Mathieu Poirier
> > <mathieu.poirier@linaro.org>; Shawn Guo <shawnguo@kernel.org>; Sascha
> > Hauer <s.hauer@pengutronix.de>; Jonathan Corbet <corbet@lwn.net>;
> > Pengutronix Kernel Team <kernel@pengutronix.de>; Fabio Estevam
> > On Fri, Dec 12, 2025 at 01:43:40PM -0600, Shenwei Wang wrote:
> > > On an AMP platform, the system may include two processors:
[..]
> > > +static int rpmsg_gpio_callback(struct rpmsg_device *rpdev,
> > > +                            void *data, int len, void *priv, u32 src)
> > > +{
[..]
> > > +     init_completion(&port->info.cmd_complete);
> > > +     port->info.rpdev = pltdata->rpdev;
> > > +     port->info.port_store = pltdata->channel_devices;
> > > +     port->info.port_store[port->idx] = port;
> > > +     if (!pltdata->rx_callback)
> > > +             pltdata->rx_callback = rpmsg_gpio_callback;
> > 
> > What happens if you rmmod your rpmsg gpio driver and then trigger an interrupt?
> > 
> 
> The driver has a rpmsg_gpio_remove_action which will clear the devices in the pltdata->channel_devices[].
> In the rpmsg callback function, it will just return -NODEV error.

When you rmmod the rpmsg_gpio driver, the code will go away, so at the
address of rpmsg_gpio_callback there's going to be some undefined data.

> 
>      if (msg)
>              port = drvdata->channel_devices[msg->port_idx];

So this code is no longer going to be there, or maybe there's some
remnants of it? But jumping to the address of rpmsg_gpio_callback (not
the actual rpmsg_gpio_callback) is best case going to result in a crash.

> 
>      if (!port)
>              return -ENODEV;
> 
> > > +
[..]
> > > +static const struct of_device_id rpmsg_gpio_dt_ids[] = {
> > > +     { .compatible = "rpmsg-gpio" },
> > > +     { /* sentinel */ }
> > > +};
> > > +
> > > +static struct platform_driver rpmsg_gpio_driver = {
> > 
> > It's an "rpmsg gpio driver", but it's a platform_driver...
> > 
> > I don't think this is the correct design, but if it is then this needs to be well
> > documented.
> > 
> > Same thing as platform_data forms a strong ABI between some other driver and
> > this platform_driver, this needs to be well documented (but should be avoided).
> > 
> 
> Are you suggesting to use "rpmsg_driver" framework here?
> 

Yes, making the functional driver bind to the particular rpmsg channel
is exactly what I would expect from a "rpmsg gpio driver".

Regards,
Bjorn

> Thank you very much for the review.
> Shenwei
> 
> > Regards,
> > Bjorn
> > 
> > > +     .driver = {
> > > +             .name = "gpio-rpmsg",
> > > +             .of_match_table = rpmsg_gpio_dt_ids,
> > > +     },
> > > +     .probe = rpmsg_gpio_probe,
> > > +};
> > > +
> > > +module_platform_driver(rpmsg_gpio_driver);
> > > +
> > > +MODULE_AUTHOR("Shenwei Wang <shenwei.wang@nxp.com>");
> > > +MODULE_DESCRIPTION("generic rpmsg gpio driver");
> > > +MODULE_LICENSE("GPL");
> > > --
> > > 2.43.0
> > >

Re: [PATCH v6 2/5] remoteproc: imx_rproc: Populate devices under "rpmsg" subnode

[Bjorn Andersson]

On Tue, Dec 23, 2025 at 07:47:31PM +0000, Shenwei Wang wrote:
> 
> 
> > -----Original Message-----
> > From: Bjorn Andersson <andersson@kernel.org>
> > Sent: Thursday, December 18, 2025 8:24 PM
> > To: Shenwei Wang <shenwei.wang@nxp.com>
> > Cc: Linus Walleij <linusw@kernel.org>; Bartosz Golaszewski <brgl@kernel.org>;
> > Rob Herring <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>;
> > Conor Dooley <conor+dt@kernel.org>; Mathieu Poirier
> > <mathieu.poirier@linaro.org>; Shawn Guo <shawnguo@kernel.org>; Sascha
> > Hauer <s.hauer@pengutronix.de>; Jonathan Corbet <corbet@lwn.net>;
> > Pengutronix Kernel Team <kernel@pengutronix.de>; Fabio Estevam
> > <festevam@gmail.com>; Peng Fan <peng.fan@nxp.com>; linux-
> > gpio@vger.kernel.org; devicetree@vger.kernel.org; linux-
> > kernel@vger.kernel.org; linux-remoteproc@vger.kernel.org; imx@lists.linux.dev;
> > linux-arm-kernel@lists.infradead.org; linux-doc@vger.kernel.org; dl-linux-imx
> > <linux-imx@nxp.com>
> > Subject: [EXT] Re: [PATCH v6 2/5] remoteproc: imx_rproc: Populate devices
> > under "rpmsg" subnode
> > 
> > Caution: This is an external email. Please take care when clicking links or opening
> > attachments. When in doubt, report the message using the 'Report this email'
> > button
> > 
> > 
> > On Fri, Dec 12, 2025 at 01:43:38PM -0600, Shenwei Wang wrote:
> > > Register the RPMsg channel driver and populate remote devices defined
> > > under the "rpmsg" subnode upon receiving their notification messages.
> > 
> > Please provide a proper description of what "problem" this patch solves.
> > 
> > >
> > > The following illustrates the expected DTS layout structure:
> > >
> > >       cm33: remoteproc-cm33 {
> > >               compatible = "fsl,imx8ulp-cm33";
> > >
> > >               rpmsg {
> > >                       rpmsg-io-channel {
> > >                               gpio@0 {
> > >                                       compatible = "fsl,imx-rpmsg-gpio";
> > >                                       reg = <0>;
> > 
> > Surely there needs to be some "gpio-controller" and "#gpio-cells" here?
> > Would be useful if the example is somewhat complete, to give a picture of what's
> > actually going on.
> > 
> 
> Okay. Will add those in next version.
> 
> > >                               };
> > >
> > >                               gpio@1 {
> > >                                       compatible = "fsl,imx-rpmsg-gpio";
> > >                                       reg = <1>;
> > >                               };
> > >
> > >                               ...
> > >                       };
> > >
> > >                       ...
> > >               };
> > >       };
> > >
> > > +     drvdata->rpdev = rpdev;
> > > +     auxdata[0].compatible = devm_kstrdup(dev, imx_rpdrv->compat,
> > GFP_KERNEL);
> > > +     auxdata[0].platform_data = drvdata;
> > > +     dev_set_drvdata(dev, drvdata);
> > > +
> > > +     of_platform_populate(drvdata->channel_node, NULL, auxdata, dev);
> > 
> > auxiliary_bus would be a better choice, but I don't understand why you probe a
> > rpmsg_device for each "gpio channel" and then from that create a
> > platform_device.
> > 
> > Why don't you just make the rpmsg_device register the gpio controller directly?
> > 
> 
> The "GPIO channel" is just one example-there are also "PWM channel", "I2C channel", and other channels. 
> The goal is to manage all these channels under a common logic, which helps avoid redundant code and keeps 
> the implementation consistent.
> 

If you make rpmsg_drivers for each of these channels/functions, then all
the common code should already be in the rpmsg framework.

What are you missing there?

> > > +
> > > +     return 0;
> > > +}
> > > +
> > > +     rp_driver->rpdrv.drv.name = name;
> > > +     rp_driver->rpdrv.id_table = rpdev_id;
> > > +     rp_driver->rpdrv.probe = imx_rpmsg_endpoint_probe;
> > > +     rp_driver->rpdrv.remove = imx_rpmsg_endpoint_remove;
> > > +     rp_driver->rpdrv.callback = imx_rpmsg_endpoint_cb;
> > > +     rp_driver->driver_data = driver_data;
> > > +     rp_driver->compat = compat;
> > > +
> > > +     register_rpmsg_driver(&rp_driver->rpdrv);
> > 
> > This would then also imply that it's the gpio driver that registers the
> > rpmsg_driver.
> > 
> > > +
> > > +     return 0;
> > > +}
> > > +
> > > +static int rproc_of_rpmsg_node_init(struct platform_device *pdev) {
> > > +     struct device *dev = &pdev->dev;
> > > +     const char *compat;
> > > +     int ret;
> > > +
> > > +     struct device_node *np __free(device_node) = of_get_child_by_name(dev-
> > >of_node, "rpmsg");
> > > +     if (!np)
> > > +             return 0;
> > > +
> > > +     for_each_child_of_node_scoped(np, child) {
> > > +             compat = imx_of_rpmsg_is_in_map(child->name);
> > > +             if (!compat)
> > > +                     ret = of_platform_default_populate(child, NULL,
> > > + dev);
> > 
> > So if you don't recognize the child device node name you just register
> > platform_devices for each of the children?
> > 
> 
> Yes. That would register platform_devices without the platform_data.
> 
> > > +             else
> > > +                     ret = imx_of_rpmsg_register_rpdriver(child, dev,
> > > + child->name, compat);
> > > +
> > > +             if (ret < 0)
> > > +                     return ret;
> > > +     }
> > > +
> > > +     return 0;
> > > +}
> > > +
> > >  static int imx_rproc_probe(struct platform_device *pdev)  {
> > >       struct device *dev = &pdev->dev; @@ -1114,6 +1253,10 @@ static
> > > int imx_rproc_probe(struct platform_device *pdev)
> > >               goto err_put_pm;
> > >       }
> > >
> > > +     ret = rproc_of_rpmsg_node_init(pdev);
> > > +     if (ret < 0)
> > > +             dev_info(dev, "populating 'rpmsg' node failed\n");
> > > +
> > >       return 0;
> > >
> > >  err_put_pm:
> > > diff --git a/include/linux/rpmsg/rpdev_info.h
> > > b/include/linux/rpmsg/rpdev_info.h
> > > new file mode 100644
> > > index 000000000000..13e020cd028b
> > > --- /dev/null
> > > +++ b/include/linux/rpmsg/rpdev_info.h
> > > @@ -0,0 +1,33 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > +/* Copyright 2025 NXP */
> > > +
> > > +/*
> > > + * @file linux/rpdev_info.h
> > > + *
> > > + * @brief Global header file for RPDEV Info
> > > + *
> > > + * @ingroup RPMSG
> > > + */
> > > +#ifndef __LINUX_RPDEV_INFO_H__
> > > +#define __LINUX_RPDEV_INFO_H__
> > > +
> > > +#define MAX_DEV_PER_CHANNEL    10
> > > +
> > > +/**
> > > + * rpdev_platform_info - store the platform information of rpdev
> > > + * @rproc_name: the name of the remote proc.
> > > + * @rpdev: rpmsg channel device
> > > + * @device_node: pointer to the device node of the rpdev.
> > > + * @rx_callback: rx callback handler of the rpdev.
> > > + * @channel_devices: an array of the devices related to the rpdev.
> > > + */
> > > +struct rpdev_platform_info {
> > 
> > I don't understand what this structure represents. Why is this glue between the
> > rpmsg_device and a made up platform_device needed?
> > 
> 
> The purpose is to have a shared array that can be accessed by all devices within 
> the same channel.
> 

What does this mean? How are multiple functions multiplexed over a
single rpmsg channel/endpoint?

Please provide a concrete description of how a device with some gpios
and PWMs would actually look in this model.

> > > +     const char *rproc_name;
> > 
> > You don't need this, because you can rproc_get_by_child(&self) and then get the
> > remoteproc name from that.
> > 
> 
> Good to know. Will try it in the next version.
> 
> > > +     struct rpmsg_device *rpdev;
> > > +     struct device_node *channel_node;
> > > +     int (*rx_callback)(struct rpmsg_device *rpdev, void *data,
> > > +                        int len, void *priv, u32 src);
> > > +     void *channel_devices[MAX_DEV_PER_CHANNEL];
> > 
> > Why 10? What does it mean?
> > 
> 
> This is based on practical experience. For example, on the i.MX platform, we typically don't have 
> more than eight same devices on the remote system.
> 
> > I think this becomes the list of the 10 grandchildren of the remoteproc (per child
> > node). So what happens if those matches against two different drivers, what will
> > rx_callback point to?
> > 
> 
> This is the limitation. That's why I used the map to populate the known child device for one specific channel.
> 

So for each rpdev_platform_info there can only be one type of client
driver. Where is this limitation defined? What happens if I put a PWM
and a GPIO controller under my rpmsg-io-channel?

Regards,
Bjorn

> Thanks,
> Shenwei
> 
> > > +};
> > 
> > Regards,
> > Bjorn
> > 
> > > +
> > > +#endif /* __LINUX_RPDEV_INFO_H__ */
> > > --
> > > 2.43.0
> > >