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

[PATCH v5 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 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

Cc: Bartosz Golaszewski <brgl@bgdev.pl>
Cc: Randy Dunlap <rdunlap@infradead.org>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Arnaud POULIQUEN <arnaud.pouliquen@foss.st.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: linux-gpio@vger.kernel.org

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

 .../bindings/remoteproc/fsl,imx-rproc.yaml    | 123 +++++
 Documentation/staging/gpio-rpmsg.rst          | 202 ++++++++
 Documentation/staging/index.rst               |   1 +
 arch/arm64/boot/dts/freescale/imx8ulp.dtsi    |  27 +
 drivers/gpio/Kconfig                          |  17 +
 drivers/gpio/Makefile                         |   1 +
 drivers/gpio/gpio-imx-rpmsg.c                 | 475 ++++++++++++++++++
 drivers/remoteproc/imx_rproc.c                | 146 ++++++
 include/linux/rpmsg/imx_rpmsg.h               |  48 ++
 9 files changed, 1040 insertions(+)
 create mode 100644 Documentation/staging/gpio-rpmsg.rst
 create mode 100644 drivers/gpio/gpio-imx-rpmsg.c
 create mode 100644 include/linux/rpmsg/imx_rpmsg.h

--
2.43.0

[PATCH v5 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>
---
 .../bindings/remoteproc/fsl,imx-rproc.yaml    | 123 ++++++++++++++++++
 1 file changed, 123 insertions(+)

diff --git a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
index 57d75acb0b5e..897a16c4f7db 100644
--- a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
@@ -84,6 +84,92 @@ 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
+            unevaluatedProperties: false
+            properties:
+              compatible:
+                enum:
+                  - fsl,imx-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#
+              - $ref: /schemas/interrupt-controller.yaml#
+
+        required:
+          - '#address-cells'
+          - '#size-cells'
+
+      rpmsg-i2c-channel:
+        type: object
+        unevaluatedProperties: false
+        properties:
+          '#address-cells':
+            const: 1
+
+          '#size-cells':
+            const: 0
+
+        patternProperties:
+          "i2c@[0-9a-f]+$":
+            type: object
+            unevaluatedProperties: false
+            properties:
+              compatible:
+                enum:
+                  - fsl,imx-rpmsg-i2c
+
+              reg:
+                maxItems: 1
+
+            required:
+              - compatible
+              - reg
+
+            allOf:
+              - $ref: /schemas/i2c/i2c-controller.yaml#
+
+        required:
+          - '#address-cells'
+          - '#size-cells'
+
 required:
   - compatible
 
@@ -146,5 +232,42 @@ 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 = "fsl,imx-rpmsg-gpio";
+            reg = <0>;
+            gpio-controller;
+            #gpio-cells = <2>;
+            #interrupt-cells = <2>;
+            interrupt-controller;
+            interrupt-parent = <&rpmsg_gpioa>;
+          };
+
+          gpio@1 {
+            compatible = "fsl,imx-rpmsg-gpio";
+            reg = <1>;
+            gpio-controller;
+            #gpio-cells = <2>;
+            #interrupt-cells = <2>;
+            interrupt-controller;
+            interrupt-parent = <&rpmsg_gpiob>;
+          };
+        };
+
+        rpmsg-i2c-channel {
+          #address-cells = <1>;
+          #size-cells = <0>;
+
+          i2c@0 {
+            compatible = "fsl,imx-rpmsg-i2c";
+            reg = <0>;
+          };
+        };
+      };
     };
 ...
-- 
2.43.0

[PATCH v5 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>;
				};

				...
			};

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

			...
		};
	};

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

diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
index a6eef0080ca9..e21a7980c490 100644
--- a/drivers/remoteproc/imx_rproc.c
+++ b/drivers/remoteproc/imx_rproc.c
@@ -8,6 +8,7 @@
 #include <linux/clk.h>
 #include <linux/err.h>
 #include <linux/firmware/imx/sci.h>
+#include <linux/rpmsg/imx_rpmsg.h>
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
 #include <linux/mailbox_client.h>
@@ -15,6 +16,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 +25,7 @@
 #include <linux/reboot.h>
 #include <linux/regmap.h>
 #include <linux/remoteproc.h>
+#include <linux/rpmsg.h>
 #include <linux/workqueue.h>
 
 #include "imx_rproc.h"
@@ -1084,6 +1088,144 @@ static int imx_rproc_sys_off_handler(struct sys_off_data *data)
 	return NOTIFY_DONE;
 }
 
+struct imx_rpmsg_driver {
+	struct rpmsg_driver rpdrv;
+	void *driver_data;
+};
+
+static char *channel_device_map[][2] = {
+	{"rpmsg-io-channel", "fsl,imx-rpmsg-gpio"},
+	{"rpmsg-i2c-channel", "fsl,imx-rpmsg-i2c"},
+};
+
+static int imx_rpmsg_endpoint_cb(struct rpmsg_device *rpdev, void *data,
+				 int len, void *priv, u32 src)
+{
+	struct imx_rpmsg_driver_data *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 imx_rpmsg_driver_data *drvdata;
+	struct imx_rpmsg_driver *imx_rpdrv;
+	struct device *dev = &rpdev->dev;
+	struct of_dev_auxdata *auxdata;
+	struct rpmsg_driver *rpdrv;
+	int i;
+
+	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;
+
+	i = drvdata->map_idx;
+	if (i >= ARRAY_SIZE(channel_device_map))
+		return -ENODEV;
+
+	auxdata = devm_kzalloc(dev, sizeof(*auxdata) * 2, GFP_KERNEL);
+	if (!auxdata)
+		return -ENOMEM;
+
+	drvdata->rpdev = rpdev;
+	auxdata[0].compatible = channel_device_map[i][1];
+	auxdata[0].platform_data = drvdata;
+	dev_set_drvdata(dev, drvdata);
+
+	of_platform_populate(drvdata->channel_node, NULL, auxdata, dev);
+	of_node_put(drvdata->channel_node);
+
+	return 0;
+}
+
+static int 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 i;
+	}
+
+	return -1;
+}
+
+static int imx_of_rpmsg_register_rpdriver(struct device_node *channel,
+					  struct device *dev, int idx)
+{
+	struct imx_rpmsg_driver_data *driver_data;
+	struct imx_rpmsg_driver *rp_driver;
+	struct rpmsg_device_id *rpdev_id;
+
+	rpdev_id = devm_kzalloc(dev, sizeof(*rpdev_id) * 2, GFP_KERNEL);
+	if (!rpdev_id)
+		return -ENOMEM;
+
+	strscpy(rpdev_id[0].name, channel_device_map[idx][0], 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;
+	driver_data->map_idx = idx;
+
+	rp_driver->rpdrv.drv.name = channel_device_map[idx][0];
+	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;
+
+	register_rpmsg_driver(&rp_driver->rpdrv);
+
+	return 0;
+}
+
+static int imx_of_rpmsg_node_init(struct platform_device *pdev)
+{
+	struct device_node *np __free(device_node);
+	struct device *dev = &pdev->dev;
+	int idx, ret;
+
+	np = of_get_child_by_name(dev->of_node, "rpmsg");
+	if (!np)
+		return 0;
+
+	for_each_child_of_node_scoped(np, child) {
+		idx = imx_of_rpmsg_is_in_map(child->name);
+		if (idx < 0)
+			ret = of_platform_default_populate(child, NULL, dev);
+		else
+			ret = imx_of_rpmsg_register_rpdriver(child, dev, idx);
+
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
 static int imx_rproc_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -1177,6 +1319,10 @@ static int imx_rproc_probe(struct platform_device *pdev)
 		goto err_put_clk;
 	}
 
+	ret = imx_of_rpmsg_node_init(pdev);
+	if (ret < 0)
+		dev_info(dev, "populating 'rpmsg' node failed\n");
+
 	return 0;
 
 err_put_clk:
diff --git a/include/linux/rpmsg/imx_rpmsg.h b/include/linux/rpmsg/imx_rpmsg.h
new file mode 100644
index 000000000000..04a5ad2d4a1d
--- /dev/null
+++ b/include/linux/rpmsg/imx_rpmsg.h
@@ -0,0 +1,48 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright 2025 NXP */
+
+/*
+ * @file linux/imx_rpmsg.h
+ *
+ * @brief Global header file for iMX RPMSG
+ *
+ * @ingroup RPMSG
+ */
+#ifndef __LINUX_IMX_RPMSG_H__
+#define __LINUX_IMX_RPMSG_H__
+
+/* Category define */
+#define IMX_RMPSG_LIFECYCLE	1
+#define IMX_RPMSG_PMIC		2
+#define IMX_RPMSG_AUDIO		3
+#define IMX_RPMSG_KEY		4
+#define IMX_RPMSG_GPIO		5
+#define IMX_RPMSG_RTC		6
+#define IMX_RPMSG_SENSOR	7
+
+/* rpmsg version */
+#define IMX_RMPSG_MAJOR		1
+#define IMX_RMPSG_MINOR		0
+
+#define MAX_DEV_PER_CHANNEL	10
+
+struct imx_rpmsg_head {
+	u8 cate;	/* Category */
+	u8 major;	/* Major version */
+	u8 minor;	/* Minor version */
+	u8 type;	/* Message type */
+	u8 cmd;		/* Command code */
+	u8 reserved[5];
+} __packed;
+
+struct imx_rpmsg_driver_data {
+	int map_idx;
+	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_IMX_RPMSG_H__ */
-- 
2.43.0

[PATCH v5 3/5] docs: staging: gpio-rpmsg: gpio 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/staging/gpio-rpmsg.rst | 202 +++++++++++++++++++++++++++
 Documentation/staging/index.rst      |   1 +
 2 files changed, 203 insertions(+)
 create mode 100644 Documentation/staging/gpio-rpmsg.rst

diff --git a/Documentation/staging/gpio-rpmsg.rst b/Documentation/staging/gpio-rpmsg.rst
new file mode 100644
index 000000000000..ad6207a3093f
--- /dev/null
+++ b/Documentation/staging/gpio-rpmsg.rst
@@ -0,0 +1,202 @@
+.. 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|
+   |cate |major |minor |type |cmd  |reserved[5] |line |port |  data    |
+   +-----+------+------+-----+-----+------------+-----+-----+-----+----+
+
+- **Cate (Category field)**: Indicates the category of the message, such as GPIO, I2C, PMIC, AUDIO, etc.
+
+  Defined categories:
+
+  - 1: RPMSG_LIFECYCLE
+  - 2: RPMSG_PMIC
+  - 3: RPMSG_AUDIO
+  - 4: RPMSG_KEY
+  - 5: RPMSG_GPIO
+  - 6: RPMSG_RTC
+  - 7: RPMSG_SENSOR
+  - 8: RPMSG_AUTO
+  - 9: RPMSG_CATEGORY
+  - A: RPMSG_PWM
+  - B: RPMSG_UART
+
+- **Major**: Major version number.
+
+- **Minor**: Minor version number.
+
+- **Type (Message Type)**: For the GPIO category, 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.
+
+- **line**: The GPIO line index.
+
+- **port**: The GPIO controller index.
+
+GPIO Commands
+-------------
+
+Commands are specified in the **Cmd** field for **GPIO_RPMSG_SETUP** (Type=0) messages.
+
+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.
+
+  - 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
+  - 3: Resource not available
+  - 4: Resource busy
+  - 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)**:
+
+.. 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.
+
diff --git a/Documentation/staging/index.rst b/Documentation/staging/index.rst
index 77bae5e5328b..fbb212e26007 100644
--- a/Documentation/staging/index.rst
+++ b/Documentation/staging/index.rst
@@ -7,6 +7,7 @@ Unsorted Documentation
    :maxdepth: 2
 
    crc32
+   gpio-rpmsg
    lzo
    magic-number
    remoteproc
-- 
2.43.0

[PATCH v5 4/5] gpio: imx-rpmsg: add imx-rpmsg GPIO driver

[Shenwei Wang]

On i.MX SoCs, 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          |  17 ++
 drivers/gpio/Makefile         |   1 +
 drivers/gpio/gpio-imx-rpmsg.c | 475 ++++++++++++++++++++++++++++++++++
 3 files changed, 493 insertions(+)
 create mode 100644 drivers/gpio/gpio-imx-rpmsg.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index a437fe652dbc..97eda94b0ba1 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1847,6 +1847,23 @@ config GPIO_SODAVILLE

 endmenu

+menu "RPMSG GPIO drivers"
+	depends on RPMSG
+
+config GPIO_IMX_RPMSG
+	tristate "NXP i.MX SoC RPMSG GPIO support"
+	depends on IMX_REMOTEPROC
+	select GPIOLIB_IRQCHIP
+	default IMX_REMOTEPROC
+	help
+	  Say yes here to support the RPMSG GPIO functions on i.MX SoC based
+	  platform.  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 379f55e9ed1e..e01465c03431 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -83,6 +83,7 @@ obj-$(CONFIG_GPIO_I8255)		+= gpio-i8255.o
 obj-$(CONFIG_GPIO_ICH)			+= gpio-ich.o
 obj-$(CONFIG_GPIO_IDIO_16)		+= gpio-idio-16.o
 obj-$(CONFIG_GPIO_IDT3243X)		+= gpio-idt3243x.o
+obj-$(CONFIG_GPIO_IMX_RPMSG)		+= gpio-imx-rpmsg.o
 obj-$(CONFIG_GPIO_IMX_SCU)		+= gpio-imx-scu.o
 obj-$(CONFIG_GPIO_IT87)			+= gpio-it87.o
 obj-$(CONFIG_GPIO_IXP4XX)		+= gpio-ixp4xx.o
diff --git a/drivers/gpio/gpio-imx-rpmsg.c b/drivers/gpio/gpio-imx-rpmsg.c
new file mode 100644
index 000000000000..888c3fdeaa04
--- /dev/null
+++ b/drivers/gpio/gpio-imx-rpmsg.c
@@ -0,0 +1,475 @@
+// 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/imx_rpmsg.h>
+
+#define IMX_RPMSG_GPIO_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_data {
+	struct imx_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 imx_rpmsg_gpio_pin {
+	u8 irq_shutdown;
+	u8 irq_unmask;
+	u8 irq_mask;
+	u32 irq_wake_enable;
+	u32 irq_type;
+	struct gpio_rpmsg_data msg;
+};
+
+struct imx_gpio_rpmsg_info {
+	struct rpmsg_device *rpdev;
+	struct gpio_rpmsg_data *notify_msg;
+	struct gpio_rpmsg_data *reply_msg;
+	struct completion cmd_complete;
+	struct mutex lock;
+	void **port_store;
+};
+
+struct imx_rpmsg_gpio_port {
+	struct gpio_chip gc;
+	struct imx_rpmsg_gpio_pin gpio_pins[IMX_RPMSG_GPIO_PER_PORT];
+	struct imx_gpio_rpmsg_info info;
+	int idx;
+};
+
+static int gpio_send_message(struct imx_rpmsg_gpio_port *port,
+			     struct gpio_rpmsg_data *msg,
+			     bool sync)
+{
+	struct imx_gpio_rpmsg_info *info = &port->info;
+	int err;
+
+	if (!info->rpdev) {
+		dev_dbg(&info->rpdev->dev,
+			"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_data));
+	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_data *gpio_setup_msg_header(struct imx_rpmsg_gpio_port *port,
+						     unsigned int offset,
+						     u8 cmd)
+{
+	struct gpio_rpmsg_data *msg = &port->gpio_pins[offset].msg;
+
+	memset(msg, 0, sizeof(struct gpio_rpmsg_data));
+	msg->header.cate = IMX_RPMSG_GPIO;
+	msg->header.major = IMX_RMPSG_MAJOR;
+	msg->header.minor = IMX_RMPSG_MINOR;
+	msg->header.type = GPIO_RPMSG_SETUP;
+	msg->header.cmd = cmd;
+	msg->pin_idx = offset;
+	msg->port_idx = port->idx;
+
+	return msg;
+};
+
+static int imx_rpmsg_gpio_get(struct gpio_chip *gc, unsigned int gpio)
+{
+	struct imx_rpmsg_gpio_port *port = gpiochip_get_data(gc);
+	struct gpio_rpmsg_data *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 imx_rpmsg_gpio_get_direction(struct gpio_chip *gc, unsigned int gpio)
+{
+	struct imx_rpmsg_gpio_port *port = gpiochip_get_data(gc);
+	struct gpio_rpmsg_data *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 imx_rpmsg_gpio_direction_input(struct gpio_chip *gc,
+					  unsigned int gpio)
+{
+	struct imx_rpmsg_gpio_port *port = gpiochip_get_data(gc);
+	struct gpio_rpmsg_data *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 imx_rpmsg_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
+{
+	struct imx_rpmsg_gpio_port *port = gpiochip_get_data(gc);
+	struct gpio_rpmsg_data *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 imx_rpmsg_gpio_direction_output(struct gpio_chip *gc,
+					unsigned int gpio, int val)
+{
+
+	return imx_rpmsg_gpio_set(gc, gpio, val);
+}
+
+static int imx_rpmsg_irq_set_type(struct irq_data *d, u32 type)
+{
+	struct imx_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;
+		break;
+	case IRQ_TYPE_EDGE_FALLING:
+		edge = GPIO_RPMSG_TRI_FALLING;
+		break;
+	case IRQ_TYPE_EDGE_BOTH:
+		edge = GPIO_RPMSG_TRI_BOTH_EDGE;
+		break;
+	case IRQ_TYPE_LEVEL_LOW:
+		edge = GPIO_RPMSG_TRI_LOW_LEVEL;
+		break;
+	case IRQ_TYPE_LEVEL_HIGH:
+		edge = GPIO_RPMSG_TRI_HIGH_LEVEL;
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	port->gpio_pins[gpio_idx].irq_type = edge;
+
+	return ret;
+}
+
+static int imx_rpmsg_irq_set_wake(struct irq_data *d, u32 enable)
+{
+	struct imx_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 imx_rpmsg_unmask_irq(struct irq_data *d)
+{
+	struct imx_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 imx_rpmsg_mask_irq(struct irq_data *d)
+{
+	struct imx_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 imx_rpmsg_irq_shutdown(struct irq_data *d)
+{
+	struct imx_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 imx_rpmsg_irq_bus_lock(struct irq_data *d)
+{
+	struct imx_rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
+
+	mutex_lock(&port->info.lock);
+}
+
+static void imx_rpmsg_irq_bus_sync_unlock(struct irq_data *d)
+{
+	struct imx_rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
+	struct gpio_rpmsg_data *msg = NULL;
+	u32 gpio_idx = d->hwirq;
+
+	if (port == NULL) {
+		mutex_unlock(&port->info.lock);
+		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 imx_rpmsg_irq_chip = {
+	.irq_mask = imx_rpmsg_mask_irq,
+	.irq_unmask = imx_rpmsg_unmask_irq,
+	.irq_set_wake = imx_rpmsg_irq_set_wake,
+	.irq_set_type = imx_rpmsg_irq_set_type,
+	.irq_shutdown = imx_rpmsg_irq_shutdown,
+	.irq_bus_lock = imx_rpmsg_irq_bus_lock,
+	.irq_bus_sync_unlock = imx_rpmsg_irq_bus_sync_unlock,
+	.flags = IRQCHIP_IMMUTABLE,
+};
+
+static int imx_rpmsg_gpio_callback(struct rpmsg_device *rpdev,
+	void *data, int len, void *priv, u32 src)
+{
+	struct gpio_rpmsg_data *msg = (struct gpio_rpmsg_data *)data;
+	struct imx_rpmsg_gpio_port *port = NULL;
+	struct imx_rpmsg_driver_data *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 imx_rpmsg_gpio_remove_action(void *data)
+{
+	struct imx_rpmsg_gpio_port *port = data;
+
+	port->info.port_store[port->idx] = NULL;
+}
+
+static int imx_rpmsg_gpio_probe(struct platform_device *pdev)
+{
+	struct imx_rpmsg_driver_data *pltdata = pdev->dev.platform_data;
+	struct imx_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 = imx_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 = IMX_RPMSG_GPIO_PER_PORT;
+	gc->base = -1;
+
+	gc->direction_input = imx_rpmsg_gpio_direction_input;
+	gc->direction_output = imx_rpmsg_gpio_direction_output;
+	gc->get_direction = imx_rpmsg_gpio_get_direction;
+	gc->get = imx_rpmsg_gpio_get;
+	gc->set = imx_rpmsg_gpio_set;
+
+	platform_set_drvdata(pdev, port);
+	girq = &gc->irq;
+	gpio_irq_chip_set_chip(girq, &imx_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, imx_rpmsg_gpio_remove_action, port);
+	if (ret)
+		return ret;
+
+	return devm_gpiochip_add_data(&pdev->dev, gc, port);
+}
+
+static const struct of_device_id imx_rpmsg_gpio_dt_ids[] = {
+	{ .compatible = "fsl,imx-rpmsg-gpio" },
+	{ /* sentinel */ }
+};
+
+static struct platform_driver imx_rpmsg_gpio_driver = {
+	.driver	= {
+		.name = "gpio-imx-rpmsg",
+		.of_match_table = imx_rpmsg_gpio_dt_ids,
+	},
+	.probe = imx_rpmsg_gpio_probe,
+};
+
+module_platform_driver(imx_rpmsg_gpio_driver);
+
+MODULE_AUTHOR("Shenwei Wang <shenwei.wang@nxp.com>");
+MODULE_DESCRIPTION("NXP i.MX SoC rpmsg gpio driver");
+MODULE_LICENSE("GPL");
--
2.43.0

[PATCH v5 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..6ab1c12a3bc1 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 = "fsl,imx-rpmsg-gpio";
+					reg = <0>;
+					gpio-controller;
+					#gpio-cells = <2>;
+					#interrupt-cells = <2>;
+					interrupt-controller;
+					interrupt-parent = <&rpmsg_gpioa>;
+				};
+
+				rpmsg_gpiob: gpio@1 {
+					compatible = "fsl,imx-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 v5 0/5] Enable Remote GPIO over RPMSG on i.MX Platform

[Peng Fan]

Hi Shenwei

> Subject: [PATCH v5 0/5] Enable Remote GPIO over RPMSG on i.MX
> Platform
> 
> Support the remote devices on the remote processor via the RPMSG
> bus on i.MX platform.
> 

I have not look into the details of new version, but before that,
just want to check, have we agreed on what Arnaud suggested?
or continue to proceed to be this as i.MX specific?

Thanks
Peng.

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

[Arnaud POULIQUEN]

Hi,

On 11/5/25 02:12, Peng Fan wrote:
> Hi Shenwei
> 
>> Subject: [PATCH v5 0/5] Enable Remote GPIO over RPMSG on i.MX
>> Platform
>>
>> Support the remote devices on the remote processor via the RPMSG
>> bus on i.MX platform.
>>
> 
> I have not look into the details of new version, but before that,
> just want to check, have we agreed on what Arnaud suggested?
> or continue to proceed to be this as i.MX specific?

Thanks, Peng, for pointing this out. Regarding the V3 discussions,
it seems that I am not the only one suggesting a generic driver.

Thanks,
Arnaud

> 
> Thanks
> Peng.

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

[Shenwei Wang]

Hi Arnaud,

> -----Original Message-----
> From: Arnaud POULIQUEN <arnaud.pouliquen@foss.st.com>
> Sent: Wednesday, November 5, 2025 4:26 AM
> To: Peng Fan <peng.fan@nxp.com>; Shenwei Wang <shenwei.wang@nxp.com>;
> Bjorn Andersson <andersson@kernel.org>; Mathieu Poirier
> <mathieu.poirier@linaro.org>; Rob Herring <robh@kernel.org>; Krzysztof
> Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Shawn
> Guo <shawnguo@kernel.org>; Sascha Hauer <s.hauer@pengutronix.de>;
> Jonathan Corbet <corbet@lwn.net>; Linus Walleij <linus.walleij@linaro.org>;
> Bartosz Golaszewski <brgl@bgdev.pl>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>; Fabio Estevam
> <festevam@gmail.com>; linux-remoteproc@vger.kernel.org;
> devicetree@vger.kernel.org; imx@lists.linux.dev; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; linux-
> doc@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Randy Dunlap
> <rdunlap@infradead.org>; Andrew Lunn <andrew@lunn.ch>; linux-
> gpio@vger.kernel.org
> Subject: [EXT] Re: [PATCH v5 0/5] Enable Remote GPIO over RPMSG on i.MX
> Platform
> Hi,
> 
> On 11/5/25 02:12, Peng Fan wrote:
> > Hi Shenwei
> >
> >> Subject: [PATCH v5 0/5] Enable Remote GPIO over RPMSG on i.MX
> >> Platform
> >>
> >> Support the remote devices on the remote processor via the RPMSG bus
> >> on i.MX platform.
> >>
> >
> > I have not look into the details of new version, but before that, just
> > want to check, have we agreed on what Arnaud suggested?
> > or continue to proceed to be this as i.MX specific?
> 
> Thanks, Peng, for pointing this out. Regarding the V3 discussions, it seems that I
> am not the only one suggesting a generic driver.
> 

As I mentioned before, the only i.MX-specific part is the transport protocol over the RPMSG bus. 
In this v5 patches, we’ve included detailed documentation for the protocol in a separate file. Any 
platform that follows the same protocol should work right out of the box. 

If you spot anything that could be improved, please let me know!

Thanks,
Shenwei 

> Thanks,
> Arnaud
> 
> >
> > Thanks
> > Peng.

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

[Peng Fan]

Hi Shenwei,

It looks good to me, only a few minor comments.

On Tue, Nov 04, 2025 at 02:33:12PM -0600, 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>;
>				};

I think better drop "imx" 

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

i2c channel could be removed, it is not included in this patchset.

>				};
>			};
>
>			...
>+

....

>+static char *channel_device_map[][2] = {
>+	{"rpmsg-io-channel", "fsl,imx-rpmsg-gpio"},
>+	{"rpmsg-i2c-channel", "fsl,imx-rpmsg-i2c"},

only keep gpio and drop imx.

>+};
>+
>+static int imx_rpmsg_endpoint_cb(struct rpmsg_device *rpdev, void *data,
>+				 int len, void *priv, u32 src)
>+{
>+	struct imx_rpmsg_driver_data *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 imx_rpmsg_driver_data *drvdata;
>+	struct imx_rpmsg_driver *imx_rpdrv;
>+	struct device *dev = &rpdev->dev;
>+	struct of_dev_auxdata *auxdata;
>+	struct rpmsg_driver *rpdrv;
>+	int i;
>+
>+	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;
>+
>+	i = drvdata->map_idx;
>+	if (i >= ARRAY_SIZE(channel_device_map))
>+		return -ENODEV;
>+
>+	auxdata = devm_kzalloc(dev, sizeof(*auxdata) * 2, GFP_KERNEL);
>+	if (!auxdata)
>+		return -ENOMEM;
>+
>+	drvdata->rpdev = rpdev;
>+	auxdata[0].compatible = channel_device_map[i][1];
>+	auxdata[0].platform_data = drvdata;
>+	dev_set_drvdata(dev, drvdata);
>+
>+	of_platform_populate(drvdata->channel_node, NULL, auxdata, dev);
>+	of_node_put(drvdata->channel_node);
>+
>+	return 0;
>+}
>+
>+static int 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 i;
>+	}
>+
>+	return -1;
>+}
>+
>+static int imx_of_rpmsg_register_rpdriver(struct device_node *channel,
>+					  struct device *dev, int idx)
>+{
>+	struct imx_rpmsg_driver_data *driver_data;
>+	struct imx_rpmsg_driver *rp_driver;
>+	struct rpmsg_device_id *rpdev_id;
>+
>+	rpdev_id = devm_kzalloc(dev, sizeof(*rpdev_id) * 2, GFP_KERNEL);
>+	if (!rpdev_id)
>+		return -ENOMEM;
>+
>+	strscpy(rpdev_id[0].name, channel_device_map[idx][0], 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;
>+	driver_data->map_idx = idx;
>+
>+	rp_driver->rpdrv.drv.name = channel_device_map[idx][0];
>+	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;
>+
>+	register_rpmsg_driver(&rp_driver->rpdrv);
>+
>+	return 0;
>+}
>+
>+static int imx_of_rpmsg_node_init(struct platform_device *pdev)
>+{
>+	struct device_node *np __free(device_node);
>+	struct device *dev = &pdev->dev;
>+	int idx, ret;
>+
>+	np = of_get_child_by_name(dev->of_node, "rpmsg");
>+	if (!np)
>+		return 0;
>+
>+	for_each_child_of_node_scoped(np, child) {
>+		idx = imx_of_rpmsg_is_in_map(child->name);
>+		if (idx < 0)
>+			ret = of_platform_default_populate(child, NULL, dev);
>+		else
>+			ret = imx_of_rpmsg_register_rpdriver(child, dev, idx);
>+
>+		if (ret < 0)
>+			return ret;
>+	}
>+
>+	return 0;
>+}
>+
> static int imx_rproc_probe(struct platform_device *pdev)
> {
> 	struct device *dev = &pdev->dev;
>@@ -1177,6 +1319,10 @@ static int imx_rproc_probe(struct platform_device *pdev)
> 		goto err_put_clk;
> 	}
> 
>+	ret = imx_of_rpmsg_node_init(pdev);
>+	if (ret < 0)
>+		dev_info(dev, "populating 'rpmsg' node failed\n");

In best case to make this generic enough, an generic function
could be provided saying rproc_of_rpmsg_node_init() in remoteproc core
or rpmsg core. But for now, it should be ok to keep it in imx_rproc.c.

Regards
Peng

Re: [PATCH v5 4/5] gpio: imx-rpmsg: add imx-rpmsg GPIO driver

[Peng Fan]

Hi Shenwei,

On Tue, Nov 04, 2025 at 02:33:14PM -0600, Shenwei Wang wrote:
>On i.MX SoCs, 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          |  17 ++
> drivers/gpio/Makefile         |   1 +
> drivers/gpio/gpio-imx-rpmsg.c | 475 ++++++++++++++++++++++++++++++++++

The patch looks good to me, but since the protocol is already generic
enough, I would prefer to drop "imx" in this patch.

Thanks,
Peng

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

[Arnaud POULIQUEN]

Hi Shenwei,

On 11/5/25 15:12, Shenwei Wang wrote:
> Hi Arnaud,
> 
>> -----Original Message-----
>> From: Arnaud POULIQUEN <arnaud.pouliquen@foss.st.com>
>> Sent: Wednesday, November 5, 2025 4:26 AM
>> To: Peng Fan <peng.fan@nxp.com>; Shenwei Wang <shenwei.wang@nxp.com>;
>> Bjorn Andersson <andersson@kernel.org>; Mathieu Poirier
>> <mathieu.poirier@linaro.org>; Rob Herring <robh@kernel.org>; Krzysztof
>> Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Shawn
>> Guo <shawnguo@kernel.org>; Sascha Hauer <s.hauer@pengutronix.de>;
>> Jonathan Corbet <corbet@lwn.net>; Linus Walleij <linus.walleij@linaro.org>;
>> Bartosz Golaszewski <brgl@bgdev.pl>
>> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>; Fabio Estevam
>> <festevam@gmail.com>; linux-remoteproc@vger.kernel.org;
>> devicetree@vger.kernel.org; imx@lists.linux.dev; linux-arm-
>> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; linux-
>> doc@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Randy Dunlap
>> <rdunlap@infradead.org>; Andrew Lunn <andrew@lunn.ch>; linux-
>> gpio@vger.kernel.org
>> Subject: [EXT] Re: [PATCH v5 0/5] Enable Remote GPIO over RPMSG on i.MX
>> Platform
>> Hi,
>>
>> On 11/5/25 02:12, Peng Fan wrote:
>>> Hi Shenwei
>>>
>>>> Subject: [PATCH v5 0/5] Enable Remote GPIO over RPMSG on i.MX
>>>> Platform
>>>>
>>>> Support the remote devices on the remote processor via the RPMSG bus
>>>> on i.MX platform.
>>>>
>>>
>>> I have not look into the details of new version, but before that, just
>>> want to check, have we agreed on what Arnaud suggested?
>>> or continue to proceed to be this as i.MX specific?
>>
>> Thanks, Peng, for pointing this out. Regarding the V3 discussions, it seems that I
>> am not the only one suggesting a generic driver.
>>
> 
> As I mentioned before, the only i.MX-specific part is the transport protocol over the RPMSG bus.
> In this v5 patches, we’ve included detailed documentation for the protocol in a separate file. Any
> platform that follows the same protocol should work right out of the box.
> 
> If you spot anything that could be improved, please let me know!

My concerns remain the same as those shared previously:

1) The simpler one: gpio-imx-rpmsg.c should be renamed to gpio-rpmsg.c.

2) The more complex one: the driver should be independent of the 
remoteproc driver. The rpmsg protocol relies on virtio and can be used 
in contexts other than remoteproc. In other words, the struct 
rpmsg_driver and its associated operations should be defined within the 
rpmsg-gpio driver, not in the remoteproc driver.


Thanks,
Arnaud

> 
> Thanks,
> Shenwei
> 
>> Thanks,
>> Arnaud
>>
>>>
>>> Thanks
>>> Peng.
> 

Re: [PATCH v5 4/5] gpio: imx-rpmsg: add imx-rpmsg GPIO driver

[Zhongqiu Han]

On 11/5/2025 4:33 AM, Shenwei Wang wrote:
> On i.MX SoCs, 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          |  17 ++
>   drivers/gpio/Makefile         |   1 +
>   drivers/gpio/gpio-imx-rpmsg.c | 475 ++++++++++++++++++++++++++++++++++
>   3 files changed, 493 insertions(+)
>   create mode 100644 drivers/gpio/gpio-imx-rpmsg.c
> 
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index a437fe652dbc..97eda94b0ba1 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -1847,6 +1847,23 @@ config GPIO_SODAVILLE
> 
>   endmenu
> 
> +menu "RPMSG GPIO drivers"
> +	depends on RPMSG
> +
> +config GPIO_IMX_RPMSG
> +	tristate "NXP i.MX SoC RPMSG GPIO support"
> +	depends on IMX_REMOTEPROC
> +	select GPIOLIB_IRQCHIP
> +	default IMX_REMOTEPROC
> +	help
> +	  Say yes here to support the RPMSG GPIO functions on i.MX SoC based
> +	  platform.  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 379f55e9ed1e..e01465c03431 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -83,6 +83,7 @@ obj-$(CONFIG_GPIO_I8255)		+= gpio-i8255.o
>   obj-$(CONFIG_GPIO_ICH)			+= gpio-ich.o
>   obj-$(CONFIG_GPIO_IDIO_16)		+= gpio-idio-16.o
>   obj-$(CONFIG_GPIO_IDT3243X)		+= gpio-idt3243x.o
> +obj-$(CONFIG_GPIO_IMX_RPMSG)		+= gpio-imx-rpmsg.o
>   obj-$(CONFIG_GPIO_IMX_SCU)		+= gpio-imx-scu.o
>   obj-$(CONFIG_GPIO_IT87)			+= gpio-it87.o
>   obj-$(CONFIG_GPIO_IXP4XX)		+= gpio-ixp4xx.o
> diff --git a/drivers/gpio/gpio-imx-rpmsg.c b/drivers/gpio/gpio-imx-rpmsg.c
> new file mode 100644
> index 000000000000..888c3fdeaa04
> --- /dev/null
> +++ b/drivers/gpio/gpio-imx-rpmsg.c
> @@ -0,0 +1,475 @@
> +// 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/imx_rpmsg.h>
> +
> +#define IMX_RPMSG_GPIO_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_data {
> +	struct imx_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 imx_rpmsg_gpio_pin {
> +	u8 irq_shutdown;
> +	u8 irq_unmask;
> +	u8 irq_mask;
> +	u32 irq_wake_enable;
> +	u32 irq_type;
> +	struct gpio_rpmsg_data msg;
> +};
> +
> +struct imx_gpio_rpmsg_info {
> +	struct rpmsg_device *rpdev;
> +	struct gpio_rpmsg_data *notify_msg;
> +	struct gpio_rpmsg_data *reply_msg;
> +	struct completion cmd_complete;
> +	struct mutex lock;
> +	void **port_store;
> +};
> +
> +struct imx_rpmsg_gpio_port {
> +	struct gpio_chip gc;
> +	struct imx_rpmsg_gpio_pin gpio_pins[IMX_RPMSG_GPIO_PER_PORT];
> +	struct imx_gpio_rpmsg_info info;
> +	int idx;
> +};
> +

Hello Shenwei,
I'd like to go over a few aspects of this patch.


> +static int gpio_send_message(struct imx_rpmsg_gpio_port *port,
> +			     struct gpio_rpmsg_data *msg,
> +			     bool sync)
> +{
> +	struct imx_gpio_rpmsg_info *info = &port->info;
> +	int err;
> +
> +	if (!info->rpdev) {
> +		dev_dbg(&info->rpdev->dev,

1.NULL pointer dereference here.

> +			"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_data));
> +	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_data *gpio_setup_msg_header(struct imx_rpmsg_gpio_port *port,
> +						     unsigned int offset,
> +						     u8 cmd)
> +{
> +	struct gpio_rpmsg_data *msg = &port->gpio_pins[offset].msg;
> +
> +	memset(msg, 0, sizeof(struct gpio_rpmsg_data));
> +	msg->header.cate = IMX_RPMSG_GPIO;
> +	msg->header.major = IMX_RMPSG_MAJOR;
> +	msg->header.minor = IMX_RMPSG_MINOR;
> +	msg->header.type = GPIO_RPMSG_SETUP;
> +	msg->header.cmd = cmd;
> +	msg->pin_idx = offset;
> +	msg->port_idx = port->idx;
> +
> +	return msg;
> +};
> +
> +static int imx_rpmsg_gpio_get(struct gpio_chip *gc, unsigned int gpio)
> +{
> +	struct imx_rpmsg_gpio_port *port = gpiochip_get_data(gc);
> +	struct gpio_rpmsg_data *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 imx_rpmsg_gpio_get_direction(struct gpio_chip *gc, unsigned int gpio)
> +{
> +	struct imx_rpmsg_gpio_port *port = gpiochip_get_data(gc);
> +	struct gpio_rpmsg_data *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 imx_rpmsg_gpio_direction_input(struct gpio_chip *gc,
> +					  unsigned int gpio)
> +{
> +	struct imx_rpmsg_gpio_port *port = gpiochip_get_data(gc);
> +	struct gpio_rpmsg_data *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 imx_rpmsg_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
> +{
> +	struct imx_rpmsg_gpio_port *port = gpiochip_get_data(gc);
> +	struct gpio_rpmsg_data *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 imx_rpmsg_gpio_direction_output(struct gpio_chip *gc,
> +					unsigned int gpio, int val)
> +{
> +
> +	return imx_rpmsg_gpio_set(gc, gpio, val);
> +}
> +
> +static int imx_rpmsg_irq_set_type(struct irq_data *d, u32 type)
> +{
> +	struct imx_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;
> +		break;
> +	case IRQ_TYPE_EDGE_FALLING:
> +		edge = GPIO_RPMSG_TRI_FALLING;
> +		break;
> +	case IRQ_TYPE_EDGE_BOTH:
> +		edge = GPIO_RPMSG_TRI_BOTH_EDGE;
> +		break;
> +	case IRQ_TYPE_LEVEL_LOW:
> +		edge = GPIO_RPMSG_TRI_LOW_LEVEL;
> +		break;
> +	case IRQ_TYPE_LEVEL_HIGH:
> +		edge = GPIO_RPMSG_TRI_HIGH_LEVEL;
> +		break;
> +	default:
> +		ret = -EINVAL;
> +		break;
> +	}
> +
> +	port->gpio_pins[gpio_idx].irq_type = edge;
> +
> +	return ret;
> +}
> +
> +static int imx_rpmsg_irq_set_wake(struct irq_data *d, u32 enable)
> +{
> +	struct imx_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 imx_rpmsg_unmask_irq(struct irq_data *d)
> +{
> +	struct imx_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 imx_rpmsg_mask_irq(struct irq_data *d)
> +{
> +	struct imx_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 imx_rpmsg_irq_shutdown(struct irq_data *d)
> +{
> +	struct imx_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 imx_rpmsg_irq_bus_lock(struct irq_data *d)
> +{
> +	struct imx_rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> +
> +	mutex_lock(&port->info.lock);
> +}
> +
> +static void imx_rpmsg_irq_bus_sync_unlock(struct irq_data *d)
> +{
> +	struct imx_rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> +	struct gpio_rpmsg_data *msg = NULL;
> +	u32 gpio_idx = d->hwirq;
> +
> +	if (port == NULL) {
> +		mutex_unlock(&port->info.lock);
> +		return;
> +	}
> +

2.Unlocking port->info.lock when port is NULL will crash due to a NULL
pointer dereference. Please fix the logic as well.> +	/*
> +	 * 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 imx_rpmsg_irq_chip = {
> +	.irq_mask = imx_rpmsg_mask_irq,
> +	.irq_unmask = imx_rpmsg_unmask_irq,
> +	.irq_set_wake = imx_rpmsg_irq_set_wake,
> +	.irq_set_type = imx_rpmsg_irq_set_type,
> +	.irq_shutdown = imx_rpmsg_irq_shutdown,
> +	.irq_bus_lock = imx_rpmsg_irq_bus_lock,
> +	.irq_bus_sync_unlock = imx_rpmsg_irq_bus_sync_unlock,
> +	.flags = IRQCHIP_IMMUTABLE,
> +};
> +
> +static int imx_rpmsg_gpio_callback(struct rpmsg_device *rpdev,
> +	void *data, int len, void *priv, u32 src)
> +{
> +	struct gpio_rpmsg_data *msg = (struct gpio_rpmsg_data *)data;
> +	struct imx_rpmsg_gpio_port *port = NULL;
> +	struct imx_rpmsg_driver_data *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 imx_rpmsg_gpio_remove_action(void *data)
> +{
> +	struct imx_rpmsg_gpio_port *port = data;
> +
> +	port->info.port_store[port->idx] = NULL;
> +}
> +
> +static int imx_rpmsg_gpio_probe(struct platform_device *pdev)
> +{
> +	struct imx_rpmsg_driver_data *pltdata = pdev->dev.platform_data;
> +	struct imx_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 = imx_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 = IMX_RPMSG_GPIO_PER_PORT;
> +	gc->base = -1;
> +
> +	gc->direction_input = imx_rpmsg_gpio_direction_input;
> +	gc->direction_output = imx_rpmsg_gpio_direction_output;
> +	gc->get_direction = imx_rpmsg_gpio_get_direction;
> +	gc->get = imx_rpmsg_gpio_get;
> +	gc->set = imx_rpmsg_gpio_set;
> +
> +	platform_set_drvdata(pdev, port);
> +	girq = &gc->irq;
> +	gpio_irq_chip_set_chip(girq, &imx_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, imx_rpmsg_gpio_remove_action, port);
> +	if (ret)
> +		return ret;
> +
> +	return devm_gpiochip_add_data(&pdev->dev, gc, port);
> +}
> +
> +static const struct of_device_id imx_rpmsg_gpio_dt_ids[] = {
> +	{ .compatible = "fsl,imx-rpmsg-gpio" },
> +	{ /* sentinel */ }
> +};
> +
> +static struct platform_driver imx_rpmsg_gpio_driver = {
> +	.driver	= {
> +		.name = "gpio-imx-rpmsg",
> +		.of_match_table = imx_rpmsg_gpio_dt_ids,
> +	},
> +	.probe = imx_rpmsg_gpio_probe,
> +};
> +
> +module_platform_driver(imx_rpmsg_gpio_driver);
> +
> +MODULE_AUTHOR("Shenwei Wang <shenwei.wang@nxp.com>");
> +MODULE_DESCRIPTION("NXP i.MX SoC rpmsg gpio driver");
> +MODULE_LICENSE("GPL");
> --
> 2.43.0
> 
> 


-- 
Thx and BRs,
Zhongqiu Han

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

[Shenwei Wang]

Hi Arnaud,

> -----Original Message-----
> From: Arnaud POULIQUEN <arnaud.pouliquen@foss.st.com>
> Sent: Thursday, November 6, 2025 4:17 AM
> To: Shenwei Wang <shenwei.wang@nxp.com>; Peng Fan <peng.fan@nxp.com>;
> Bjorn Andersson <andersson@kernel.org>; Mathieu Poirier
> <mathieu.poirier@linaro.org>; Rob Herring <robh@kernel.org>; Krzysztof
> Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Shawn
> Guo <shawnguo@kernel.org>; Sascha Hauer <s.hauer@pengutronix.de>;
> Jonathan Corbet <corbet@lwn.net>; Linus Walleij <linus.walleij@linaro.org>;
> Bartosz Golaszewski <brgl@bgdev.pl>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>; Fabio Estevam
> <festevam@gmail.com>; linux-remoteproc@vger.kernel.org;
> devicetree@vger.kernel.org; imx@lists.linux.dev; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; linux-
> doc@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Randy Dunlap
> <rdunlap@infradead.org>; Andrew Lunn <andrew@lunn.ch>; linux-
> gpio@vger.kernel.org
> Subject: [EXT] Re: [PATCH v5 0/5] Enable Remote GPIO over RPMSG on i.MX
> Platform
> 
> Hi Shenwei,
> 
> On 11/5/25 15:12, Shenwei Wang wrote:
> > Hi Arnaud,
> >
> >> -----Original Message-----
> >>
> >> On 11/5/25 02:12, Peng Fan wrote:
> >>> Hi Shenwei
> >>>
> >>>> Subject: [PATCH v5 0/5] Enable Remote GPIO over RPMSG on i.MX
> >>>> Platform
> >>>>
> >>>> Support the remote devices on the remote processor via the RPMSG
> >>>> bus on i.MX platform.
> >>>>
> >>>
> >>> I have not look into the details of new version, but before that,
> >>> just want to check, have we agreed on what Arnaud suggested?
> >>> or continue to proceed to be this as i.MX specific?
> >>
> >> Thanks, Peng, for pointing this out. Regarding the V3 discussions, it
> >> seems that I am not the only one suggesting a generic driver.
> >>
> >
> > As I mentioned before, the only i.MX-specific part is the transport protocol over
> the RPMSG bus.
> > In this v5 patches, we’ve included detailed documentation for the
> > protocol in a separate file. Any platform that follows the same protocol should
> work right out of the box.
> >
> > If you spot anything that could be improved, please let me know!
> 
> My concerns remain the same as those shared previously:
> 
> 1) The simpler one: gpio-imx-rpmsg.c should be renamed to gpio-rpmsg.c.
> 

Agree. Will fix it in the next version.

> 2) The more complex one: the driver should be independent of the remoteproc
> driver. The rpmsg protocol relies on virtio and can be used in contexts other than
> remoteproc. In other words, the struct rpmsg_driver and its associated
> operations should be defined within the rpmsg-gpio driver, not in the remoteproc
> driver.
> 

The GPIO driver operates independently of the remoteproc driver. It functions based 
on the defined GPIO-RPMSG transport protocol. Any remoteproc that supports 
this protocol can exchange data with the GPIO driver via the underlying rpmsg bus. 
Placing the rpmsg_driver (which manages the rpmsg channel) within the remoteproc 
driver is more logical, as rpmsg channels run on the rpmsg bus. This bus is defined inside 
the remoteproc device tree node and is populated by the corresponding remoteproc driver. 

Thanks,
Shenwei

> 
> Thanks,
> Arnaud
> 
> >
> > Thanks,
> > Shenwei
> >
> >> Thanks,
> >> Arnaud
> >>
> >>>
> >>> Thanks
> >>> Peng.
> >

Re: [PATCH v5 4/5] gpio: imx-rpmsg: add imx-rpmsg GPIO driver

[Shenwei Wang]

> -----Original Message-----
> From: Zhongqiu Han <zhongqiu.han@oss.qualcomm.com>
> Sent: Thursday, November 6, 2025 6:31 AM
> To: Shenwei Wang <shenwei.wang@nxp.com>; Bjorn Andersson
> <andersson@kernel.org>; Mathieu Poirier <mathieu.poirier@linaro.org>; Rob
> Herring <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor
> Dooley <conor+dt@kernel.org>; Shawn Guo <shawnguo@kernel.org>; Sascha
> Hauer <s.hauer@pengutronix.de>; Jonathan Corbet <corbet@lwn.net>; Linus
> Walleij <linus.walleij@linaro.org>; Bartosz Golaszewski <brgl@bgdev.pl>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>; Fabio Estevam
> <festevam@gmail.com>; Peng Fan <peng.fan@nxp.com>; linux-
> remoteproc@vger.kernel.org; devicetree@vger.kernel.org; imx@lists.linux.dev;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; linux-
> doc@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Andrew Lunn
> <andrew@lunn.ch>; zhongqiu.han@oss.qualcomm.com
> Subject: [EXT] Re: [PATCH v5 4/5] gpio: imx-rpmsg: add imx-rpmsg GPIO driver
> On 11/5/2025 4:33 AM, Shenwei Wang wrote:
> > On i.MX SoCs, the system may include two processors:
> >       - An MCU running an RTOS
> >       - An MPU running Linux
> >
> > +
> > +struct imx_rpmsg_gpio_port {
> > +     struct gpio_chip gc;
> > +     struct imx_rpmsg_gpio_pin gpio_pins[IMX_RPMSG_GPIO_PER_PORT];
> > +     struct imx_gpio_rpmsg_info info;
> > +     int idx;
> > +};
> > +
> 
> Hello Shenwei,
> I'd like to go over a few aspects of this patch.
> 

Hi Zhongqiu,

Thank you for the review. I'll address the two issues you reported below in the next revision.

Thanks,
Shenwei

> 
> > +static int gpio_send_message(struct imx_rpmsg_gpio_port *port,
> > +                          struct gpio_rpmsg_data *msg,
> > +                          bool sync)
> > +{
> > +     struct imx_gpio_rpmsg_info *info = &port->info;
> > +     int err;
> > +
> > +     if (!info->rpdev) {
> > +             dev_dbg(&info->rpdev->dev,
> 
> 1.NULL pointer dereference here.
> 
> > +                     "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_data));
> > +static void imx_rpmsg_irq_bus_sync_unlock(struct irq_data *d) {
> > +     struct imx_rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> > +     struct gpio_rpmsg_data *msg = NULL;
> > +     u32 gpio_idx = d->hwirq;
> > +
> > +     if (port == NULL) {
> > +             mutex_unlock(&port->info.lock);
> > +             return;
> > +     }
> > +
> 
> 2.Unlocking port->info.lock when port is NULL will crash due to a NULL
> pointer dereference. Please fix the logic as well.> +   /*
> > +      * 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;
> > +     }
> > +
> > +
> > +MODULE_AUTHOR("Shenwei Wang <shenwei.wang@nxp.com>");
> > +MODULE_DESCRIPTION("NXP i.MX SoC rpmsg gpio driver");
> > +MODULE_LICENSE("GPL");
> > --
> > 2.43.0
> >
> >
> 
> 
> --
> Thx and BRs,
> Zhongqiu Han

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

[Andrew Lunn]

On Tue, Nov 04, 2025 at 02:33:11PM -0600, 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>
> ---
>  .../bindings/remoteproc/fsl,imx-rproc.yaml    | 123 ++++++++++++++++++
>  1 file changed, 123 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
> index 57d75acb0b5e..897a16c4f7db 100644
> --- a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
> +++ b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
> @@ -84,6 +84,92 @@ 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
> +            unevaluatedProperties: false
> +            properties:
> +              compatible:
> +                enum:
> +                  - fsl,imx-rpmsg-gpio

The compatible should be generic. Or at least, you need a generic
fallback, since the one driver should be able to handle all
implementation.

> +
> +              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#
> +              - $ref: /schemas/interrupt-controller.yaml#

The gpio part should really be pulled out into a separate .yaml file,
because it should be identical for all implementation. You just need
to reference it here.

	Andrew

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

[Andrew Lunn]

> >			rpmsg-i2c-channel {
> >				i2c@0 {
> >					compatible = "fsl,imx-rpmsg-i2c";
> >					reg = <0>;
> 
> i2c channel could be removed, it is not included in this patchset.

And i also expect a similar sort of generic driver for i2c. There
should not be anything IMX specific needed.

	Andrew

Re: [PATCH v5 3/5] docs: staging: gpio-rpmsg: gpio over rpmsg bus

[Andrew Lunn]

On Tue, Nov 04, 2025 at 02:33:13PM -0600, 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/staging/gpio-rpmsg.rst | 202 +++++++++++++++++++++++++++
>  Documentation/staging/index.rst      |   1 +
>  2 files changed, 203 insertions(+)
>  create mode 100644 Documentation/staging/gpio-rpmsg.rst
> 
> diff --git a/Documentation/staging/gpio-rpmsg.rst b/Documentation/staging/gpio-rpmsg.rst
> new file mode 100644
> index 000000000000..ad6207a3093f
> --- /dev/null
> +++ b/Documentation/staging/gpio-rpmsg.rst
> @@ -0,0 +1,202 @@
> +.. 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|
> +   |cate |major |minor |type |cmd  |reserved[5] |line |port |  data    |
> +   +-----+------+------+-----+-----+------------+-----+-----+-----+----+
> +
> +- **Cate (Category field)**: Indicates the category of the message, such as GPIO, I2C, PMIC, AUDIO, etc.

We know it is a GPIO message, this document is titled "GPIO RPMSG
Protocol". So i don't see the need for cate. I can however understand
that your device does support multiple functions, but to make this
generic, it would be better if each function had its own channel.

> +
> +  Defined categories:
> +
> +  - 1: RPMSG_LIFECYCLE
> +  - 2: RPMSG_PMIC
> +  - 3: RPMSG_AUDIO
> +  - 4: RPMSG_KEY
> +  - 5: RPMSG_GPIO
> +  - 6: RPMSG_RTC
> +  - 7: RPMSG_SENSOR
> +  - 8: RPMSG_AUTO
> +  - 9: RPMSG_CATEGORY
> +  - A: RPMSG_PWM
> +  - B: RPMSG_UART
> +
> +- **Major**: Major version number.
> +
> +- **Minor**: Minor version number.

What is the purpose of Major and Minor? What values are valid. What
should happen if an invalid value is passed?

What you should think about is, if you gave this specification to
somebody else, could they implement it? 

> +
> +- **Type (Message Type)**: For the GPIO category, can be one of:
> +
> +  - 0: GPIO_RPMSG_SETUP
> +  - 1: GPIO_RPMSG_REPLY
> +  - 2: GPIO_RPMSG_NOTIFY

Is _SETUP always from Linux to the firmware? Is a setup always
followed by a _REPLY? Do you need to wait for the _REPLY before
sending the next _SETUP? If there is no _REPLY within X seconds, should
Linux retry? Can an _NOTIFY arrive between a _SETUP and a _REPLY?

> +
> +- **Cmd**: Command code, used for GPIO_RPMSG_SETUP messages.
> +
> +- **reserved[5]**: Reserved bytes.

Why are these in the middle. It is more normal to have reserved bytes
at the end.

You should also specify these bytes should be set to 0. If you don't
it will be hard to use them in the future, because they will contain
42, or some other random values.

Is there a relationship between major:minor and reserved?

> +
> +- **line**: The GPIO line index.
> +
> +- **port**: The GPIO controller index.

data? 

> +GPIO Commands
> +-------------

This is a GPIO specification, so i would only expect GPIO commands...

> +
> +Commands are specified in the **Cmd** field for **GPIO_RPMSG_SETUP** (Type=0) messages.
> +
> +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.
> +
> +  - 0: Disable wakeup from GPIO
> +  - 1: Enable wakeup from GPIO

What do you mean by wakeup? 

> +
> +**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
> +  - 3: Resource not available
> +  - 4: Resource busy
> +  - 5: Parameter error

It would be good to give some examples of when these should be used.

Say the hardware does not support both edges. Is that 2? Why would a
resource be busy? How is busy different to not available?

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

Maybe make a comment about the order. Some GPIO controllers suffer from
glitches when you swap them from input to output. While it is an
input, you first need to set the output value, and then configure the
pin for output. 

> +Notification Message
> +--------------------
> +
> +Notifications are sent with **Type=2 (GPIO_RPMSG_NOTIFY)**:
> +
> +.. 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.

There is no need to acknowledge the notification? How do level
interrupts work?

	Andrew

Re: [PATCH v5 3/5] docs: staging: gpio-rpmsg: gpio over rpmsg bus

[Shenwei Wang]

Hi Andrew,

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Thursday, November 6, 2025 1:06 PM
> To: Shenwei Wang <shenwei.wang@nxp.com>
> Cc: Bjorn Andersson <andersson@kernel.org>; Mathieu Poirier
> <mathieu.poirier@linaro.org>; Rob Herring <robh@kernel.org>; Krzysztof
> Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Shawn
> Guo <shawnguo@kernel.org>; Sascha Hauer <s.hauer@pengutronix.de>;
> Jonathan Corbet <corbet@lwn.net>; Linus Walleij <linus.walleij@linaro.org>;
> Bartosz Golaszewski <brgl@bgdev.pl>; Pengutronix Kernel Team
> <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; Peng Fan
> <peng.fan@nxp.com>; linux-remoteproc@vger.kernel.org;
> devicetree@vger.kernel.org; imx@lists.linux.dev; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; linux-
> doc@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>
> Subject: [EXT] Re: [PATCH v5 3/5] docs: staging: gpio-rpmsg: gpio over rpmsg bus
> > +   +-----+------+------+-----+-----+------------+-----+-----+-----+----+
> > +   |0x00 |0x01  |0x02  |0x03 |0x04 |0x05..0x09  |0x0A |0x0B |0x0C |0x0D|
> > +   |cate |major |minor |type |cmd  |reserved[5] |line |port |  data    |
> > +
> > + +-----+------+------+-----+-----+------------+-----+-----+-----+----
> > + +
> > +
> > +- **Cate (Category field)**: Indicates the category of the message, such as
> GPIO, I2C, PMIC, AUDIO, etc.
> 
> We know it is a GPIO message, this document is titled "GPIO RPMSG Protocol". So
> i don't see the need for cate. I can however understand that your device does
> support multiple functions, but to make this generic, it would be better if each
> function had its own channel.
> 

These details are defined in the message header to support multiple functions. For 
the GPIO-specific implementation, the header values are fixed and do not require 
modification, provided the transport protocol version remains unchanged.

Then I should remove those unrelated definitions from this document. Is my understanding
correct?

> > +
> > +  Defined categories:
> > +
> > +  - 1: RPMSG_LIFECYCLE
> > +  - 2: RPMSG_PMIC
> > +  - 3: RPMSG_AUDIO
> > +  - 4: RPMSG_KEY
> > +  - 5: RPMSG_GPIO
> > +  - 6: RPMSG_RTC
> > +  - 7: RPMSG_SENSOR
> > +  - 8: RPMSG_AUTO
> > +  - 9: RPMSG_CATEGORY
> > +  - A: RPMSG_PWM
> > +  - B: RPMSG_UART
> > +
> > +- **Major**: Major version number.
> > +
> > +- **Minor**: Minor version number.
> 
> What is the purpose of Major and Minor? What values are valid. What should
> happen if an invalid value is passed?
> 
> What you should think about is, if you gave this specification to somebody else,
> could they implement it?
> 

Okay. Will change those fields to fixed number and remove the above definitions.

> > +
> > +- **Type (Message Type)**: For the GPIO category, can be one of:
> > +
> > +  - 0: GPIO_RPMSG_SETUP
> > +  - 1: GPIO_RPMSG_REPLY
> > +  - 2: GPIO_RPMSG_NOTIFY
> 
> Is _SETUP always from Linux to the firmware? Is a setup always followed by a
> _REPLY? Do you need to wait for the _REPLY before sending the next _SETUP? If
> there is no _REPLY within X seconds, should Linux retry? Can an _NOTIFY arrive
> between a _SETUP and a _REPLY?
> 

Yes, the SETUP message is always sent from Linux to the remote firmware. Each SETUP 
corresponds to a single REPLY message. The GPIO driver serializes messages and determines 
whether a REPLY is required. If a REPLY is expected but not received within the timeout 
period (currently 1 second in the driver), the driver returns -ETIMEOUT.

A NOTIFY message can arrive between a SETUP and the REPLY, and the driver is designed to handle this scenario.

> > +
> > +- **Cmd**: Command code, used for GPIO_RPMSG_SETUP messages.
> > +
> > +- **reserved[5]**: Reserved bytes.
> 
> Why are these in the middle. It is more normal to have reserved bytes at the end.
> 

The reserved[5] bytes may be used for other type of functions. For GPIO, all should be 0.

> You should also specify these bytes should be set to 0. If you don't it will be hard
> to use them in the future, because they will contain 42, or some other random
> values.
> 
> Is there a relationship between major:minor and reserved?
> 

No so far. Maybe it could be related if the protocol is updated in the future.

> > +
> > +- **line**: The GPIO line index.
> > +
> > +- **port**: The GPIO controller index.
> 
> data?
> 

Data is explained in the commands below.

> > +GPIO Commands
> > +-------------
> 
> This is a GPIO specification, so i would only expect GPIO commands...
> 
> > +
> > +Commands are specified in the **Cmd** field for **GPIO_RPMSG_SETUP**
> (Type=0) messages.
> > +
> > +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.
> > +
> > +  - 0: Disable wakeup from GPIO
> > +  - 1: Enable wakeup from GPIO
> 
> What do you mean by wakeup?
> 

The gpio line can be enabled as an wakeup source for the system.

> > +
> > +**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
> > +  - 3: Resource not available
> > +  - 4: Resource busy
> > +  - 5: Parameter error
> 
> It would be good to give some examples of when these should be used.
> 
> Say the hardware does not support both edges. Is that 2? Why would a resource
> be busy? How is busy different to not available?
> 
> > +
> > +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
> 
> Maybe make a comment about the order. Some GPIO controllers suffer from
> glitches when you swap them from input to output. While it is an input, you first
> need to set the output value, and then configure the pin for output.
> 

These are platform-dependent details that should be managed by the remote firmware. 
In the Linux driver, we simply request the remote side to configure the GPIO direction.

> > +Notification Message
> > +--------------------
> > +
> > +Notifications are sent with **Type=2 (GPIO_RPMSG_NOTIFY)**:
> > +
> > +.. 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.
> 
> There is no need to acknowledge the notification? How do level interrupts work?
> 

Currently, there is no need to acknowledge the message, as the interrupt is managed entirely 
by the remote firmware. On the Linux side, a single notification message is received when an 
interrupt is triggered.

Thanks,
Shenwei

>         Andrew

Re: [PATCH v5 3/5] docs: staging: gpio-rpmsg: gpio over rpmsg bus

[Andrew Lunn]

On Thu, Nov 06, 2025 at 08:40:05PM +0000, Shenwei Wang wrote:
> Hi Andrew,
> 
> > -----Original Message-----
> > From: Andrew Lunn <andrew@lunn.ch>
> > Sent: Thursday, November 6, 2025 1:06 PM
> > To: Shenwei Wang <shenwei.wang@nxp.com>
> > Cc: Bjorn Andersson <andersson@kernel.org>; Mathieu Poirier
> > <mathieu.poirier@linaro.org>; Rob Herring <robh@kernel.org>; Krzysztof
> > Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Shawn
> > Guo <shawnguo@kernel.org>; Sascha Hauer <s.hauer@pengutronix.de>;
> > Jonathan Corbet <corbet@lwn.net>; Linus Walleij <linus.walleij@linaro.org>;
> > Bartosz Golaszewski <brgl@bgdev.pl>; Pengutronix Kernel Team
> > <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; Peng Fan
> > <peng.fan@nxp.com>; linux-remoteproc@vger.kernel.org;
> > devicetree@vger.kernel.org; imx@lists.linux.dev; linux-arm-
> > kernel@lists.infradead.org; linux-kernel@vger.kernel.org; linux-
> > doc@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>
> > Subject: [EXT] Re: [PATCH v5 3/5] docs: staging: gpio-rpmsg: gpio over rpmsg bus
> > > +   +-----+------+------+-----+-----+------------+-----+-----+-----+----+
> > > +   |0x00 |0x01  |0x02  |0x03 |0x04 |0x05..0x09  |0x0A |0x0B |0x0C |0x0D|
> > > +   |cate |major |minor |type |cmd  |reserved[5] |line |port |  data    |
> > > +
> > > + +-----+------+------+-----+-----+------------+-----+-----+-----+----
> > > + +
> > > +
> > > +- **Cate (Category field)**: Indicates the category of the message, such as
> > GPIO, I2C, PMIC, AUDIO, etc.
> > 
> > We know it is a GPIO message, this document is titled "GPIO RPMSG Protocol". So
> > i don't see the need for cate. I can however understand that your device does
> > support multiple functions, but to make this generic, it would be better if each
> > function had its own channel.
> > 
> 
> These details are defined in the message header to support multiple functions. For 
> the GPIO-specific implementation, the header values are fixed and do not require 
> modification, provided the transport protocol version remains unchanged.
> 
> Then I should remove those unrelated definitions from this document. Is my understanding
> correct?

I think we need to know more about your system. Why does cate exist?
Why are you mixing different functions onto one channel, rather than
having a channel per function?

I think you should remove cate from the GPIO protocol.

> > > +  Defined categories:
> > > +
> > > +  - 1: RPMSG_LIFECYCLE
> > > +  - 2: RPMSG_PMIC
> > > +  - 3: RPMSG_AUDIO
> > > +  - 4: RPMSG_KEY
> > > +  - 5: RPMSG_GPIO
> > > +  - 6: RPMSG_RTC
> > > +  - 7: RPMSG_SENSOR
> > > +  - 8: RPMSG_AUTO
> > > +  - 9: RPMSG_CATEGORY
> > > +  - A: RPMSG_PWM
> > > +  - B: RPMSG_UART
> > > +
> > > +- **Major**: Major version number.
> > > +
> > > +- **Minor**: Minor version number.
> > 
> > What is the purpose of Major and Minor? What values are valid. What should
> > happen if an invalid value is passed?
> > 
> > What you should think about is, if you gave this specification to somebody else,
> > could they implement it?
> > 
> 
> Okay. Will change those fields to fixed number and remove the above definitions.

What does not answer my question: What should happen if an invalid
value is passed?

You must have major:minor for a reason. You expect to change
them. Then what happens? How should forward/backwards compatibility
work? Must version 0:0 always be implemented, were as 0:1 is optional?
How do you find out if 0:1 is implemented?

> > > +- **Type (Message Type)**: For the GPIO category, can be one of:
> > > +
> > > +  - 0: GPIO_RPMSG_SETUP
> > > +  - 1: GPIO_RPMSG_REPLY
> > > +  - 2: GPIO_RPMSG_NOTIFY
> > 
> > Is _SETUP always from Linux to the firmware? Is a setup always followed by a
> > _REPLY? Do you need to wait for the _REPLY before sending the next _SETUP? If
> > there is no _REPLY within X seconds, should Linux retry? Can an _NOTIFY arrive
> > between a _SETUP and a _REPLY?
> > 
> 
> Yes, the SETUP message is always sent from Linux to the remote firmware. Each SETUP 
> corresponds to a single REPLY message. The GPIO driver serializes messages and determines 
> whether a REPLY is required. If a REPLY is expected but not received within the timeout 
> period (currently 1 second in the driver), the driver returns -ETIMEOUT.

You need to specify this in the protocol. Looking at the messages, i
don't see why i could not send off a batch of requests with different
line values, and later expect a batch of replies with different line
values. The linux driver can then match the replies to the request and
know they are all answered. The current specification allows that.

You really need to forget about your implementation. Look at the
specification, and think about all the different ways it can be
implemented, and conform to the specification. If there is only one
possible implementation, your specification is good. If it can be
implemented in multiple ways, you need to improve the specification.

> A NOTIFY message can arrive between a SETUP and the REPLY, and the driver is designed to handle this scenario.

Please state that. 

> 
> > > +
> > > +- **Cmd**: Command code, used for GPIO_RPMSG_SETUP messages.
> > > +
> > > +- **reserved[5]**: Reserved bytes.
> > 
> > Why are these in the middle. It is more normal to have reserved bytes at the end.
> > 
> 
> The reserved[5] bytes may be used for other type of functions. For GPIO, all should be 0.

We don't care about other functions. This is all about GPIO.

> > You should also specify these bytes should be set to 0. If you don't it will be hard
> > to use them in the future, because they will contain 42, or some other random
> > values.
> > 
> > Is there a relationship between major:minor and reserved?
> > 
> 
> No so far. Maybe it could be related if the protocol is updated in the future.

That implies that you cannot look at major:minor and know if the
reserved bits are reserved, or have some actual meaning?

Again, think about forward/backwards compatibility. How do you turn a
reserved bit into a used bit? Is the specification sufficient to make
that possible.

> > > +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.
> > > +
> > > +  - 0: Disable wakeup from GPIO
> > > +  - 1: Enable wakeup from GPIO
> > 
> > What do you mean by wakeup?
> > 
> 
> The gpio line can be enabled as an wakeup source for the system.

Keep going.....

Does that imply if none of the lines have wakeup enabled, the GPIO
controller can be suspended when Linux suspends? How does the GPIO
controller know it can suspend? There is no message for that. How does
it know to come out of suspension?

> > > +Notification Message
> > > +--------------------
> > > +
> > > +Notifications are sent with **Type=2 (GPIO_RPMSG_NOTIFY)**:
> > > +
> > > +.. 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.
> > 
> > There is no need to acknowledge the notification? How do level interrupts work?
> > 
> 
> Currently, there is no need to acknowledge the message, as the interrupt is managed entirely 
> by the remote firmware. On the Linux side, a single notification message is received when an 
> interrupt is triggered.

At sounds broken.

A level interrupt is not cleared until the level changes. The typical
flow is:

Interrupt fires.

Interrupt is masked

Interrupt handler is called, which reads/write registers in the device
who pin is connected to the GPIO

Interrupt is unmasked



At the unmask point, the interrupt will fire again, if the level is
still in the active state. You need this because while reading/writing
registers in the device, another event can happen, which needs
handling. Generally, the interrupt output of a device is an OR of many
different sources. You only release the interrupt when all sources
have been cleared.

So for the protocol, you need to acknowledge the notification after
the interrupt handler has executed. And that could cause another
notification to be immediately sent if the line is still active.

I'm not too sure how edge interrupts should work. If we are still busy
in an edge interrupt handler, do we want to know about the next edge?
Should edge interrupts be disabled until the previous edge has been
handled? I'm not too familiar with edge interrupts, most of the device
i've handled are level.

	Andrew

Re: [PATCH v5 3/5] docs: staging: gpio-rpmsg: gpio over rpmsg bus

[Shenwei Wang]

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Thursday, November 6, 2025 3:33 PM
> To: Shenwei Wang <shenwei.wang@nxp.com>
> Cc: Bjorn Andersson <andersson@kernel.org>; Mathieu Poirier
> <mathieu.poirier@linaro.org>; Rob Herring <robh@kernel.org>; Krzysztof
> Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Shawn
> Guo <shawnguo@kernel.org>; Sascha Hauer <s.hauer@pengutronix.de>;
> Jonathan Corbet <corbet@lwn.net>; Linus Walleij <linus.walleij@linaro.org>;
> Bartosz Golaszewski <brgl@bgdev.pl>; Pengutronix Kernel Team
> <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; Peng Fan
> <peng.fan@nxp.com>; linux-remoteproc@vger.kernel.org;
> devicetree@vger.kernel.org; imx@lists.linux.dev; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; linux-
> doc@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>
> Subject: [EXT] Re: [PATCH v5 3/5] docs: staging: gpio-rpmsg: gpio over rpmsg bus
> > > > +
> > > > +- **Cate (Category field)**: Indicates the category of the
> > > > +message, such as
> > > GPIO, I2C, PMIC, AUDIO, etc.
> > >
> > > We know it is a GPIO message, this document is titled "GPIO RPMSG
> > > Protocol". So i don't see the need for cate. I can however
> > > understand that your device does support multiple functions, but to
> > > make this generic, it would be better if each function had its own channel.
> > >
> >
> > These details are defined in the message header to support multiple
> > functions. For the GPIO-specific implementation, the header values are
> > fixed and do not require modification, provided the transport protocol version
> remains unchanged.
> >
> > Then I should remove those unrelated definitions from this document.
> > Is my understanding correct?
> 
> I think we need to know more about your system. Why does cate exist?
> Why are you mixing different functions onto one channel, rather than having a
> channel per function?
> 

At the protocol layer, there is no restriction of one function per channel because the 
Cate field in the message header is designed to differentiate multiple functions within 
the same channel. This provides flexibility for implementations that need to support 
multiple functionalities over a single communication path.

However, for a specific implementation, you may choose to enforce a one-function-per-channel 
approach by assigning a fixed value to the Cate field.

> I think you should remove cate from the GPIO protocol.
> 


The original proposal was to use a single transport message header to support multiple functions. 
Removing the Cate field would require significant changes to the current remote firmware. 
Therefore, I recommend keeping this field.

For implementations that do not need to support multiple functions, a fixed value (e.g., 5) can be used 
for the Cate field. This approach can run smoothly with the existing system and give the option for systems
that doesn't want to support multiple functions. 

> > > > +  Defined categories:
> > > > +
> > > > +  - 1: RPMSG_LIFECYCLE
> > > > +  - 2: RPMSG_PMIC
> > > > +  - 3: RPMSG_AUDIO
> > > > +  - 4: RPMSG_KEY
> > > > +  - 5: RPMSG_GPIO
> > > > +  - 6: RPMSG_RTC
> > > > +  - 7: RPMSG_SENSOR
> > > > +  - 8: RPMSG_AUTO
> > > > +  - 9: RPMSG_CATEGORY
> > > > +  - A: RPMSG_PWM
> > > > +  - B: RPMSG_UART
> > > > +
> > > > +- **Major**: Major version number.
> > > > +
> > > > +- **Minor**: Minor version number.
> > >
> > > What is the purpose of Major and Minor? What values are valid. What
> > > should happen if an invalid value is passed?
> > >
> > > What you should think about is, if you gave this specification to
> > > somebody else, could they implement it?
> > >
> >
> > Okay. Will change those fields to fixed number and remove the above
> definitions.
> 
> What does not answer my question: What should happen if an invalid value is
> passed?
> 

That is an implementation detail of the remote firmware. If it chooses to support a 
specific protocol version, it can return an error when the version is not supported. 
For example, the existing i.MX remote firmware returns a "not supported" error in such cases.

> You must have major:minor for a reason. You expect to change them. Then what
> happens? How should forward/backwards compatibility work? Must version 0:0
> always be implemented, were as 0:1 is optional?
> How do you find out if 0:1 is implemented?
> 

The remote firmware decides how to respond to the protocol version. By default, it 
should return a "not supported" error for any unknown version.

> > > > +- **Type (Message Type)**: For the GPIO category, can be one of:
> > > > +
> > > > +  - 0: GPIO_RPMSG_SETUP
> > > > +  - 1: GPIO_RPMSG_REPLY
> > > > +  - 2: GPIO_RPMSG_NOTIFY
> > >
> > > Is _SETUP always from Linux to the firmware? Is a setup always
> > > followed by a _REPLY? Do you need to wait for the _REPLY before
> > > sending the next _SETUP? If there is no _REPLY within X seconds,
> > > should Linux retry? Can an _NOTIFY arrive between a _SETUP and a _REPLY?
> > >
> >
> > Yes, the SETUP message is always sent from Linux to the remote
> > firmware. Each SETUP corresponds to a single REPLY message. The GPIO
> > driver serializes messages and determines whether a REPLY is required.
> > If a REPLY is expected but not received within the timeout period (currently 1
> second in the driver), the driver returns -ETIMEOUT.
> 
> You need to specify this in the protocol. Looking at the messages, i don't see why i
> could not send off a batch of requests with different line values, and later expect
> a batch of replies with different line values. The linux driver can then match the
> replies to the request and know they are all answered. The current specification
> allows that.
> 
> You really need to forget about your implementation. Look at the specification,
> and think about all the different ways it can be implemented, and conform to the
> specification. If there is only one possible implementation, your specification is
> good. If it can be implemented in multiple ways, you need to improve the
> specification.
> 
> > A NOTIFY message can arrive between a SETUP and the REPLY, and the driver
> is designed to handle this scenario.
> 
> Please state that.
> 
> >
> > > > +
> > > > +- **Cmd**: Command code, used for GPIO_RPMSG_SETUP messages.
> > > > +
> > > > +- **reserved[5]**: Reserved bytes.
> > >
> > > Why are these in the middle. It is more normal to have reserved bytes at the
> end.
> > >
> >
> > The reserved[5] bytes may be used for other type of functions. For GPIO, all
> should be 0.
> 
> We don't care about other functions. This is all about GPIO.
> 
> > > You should also specify these bytes should be set to 0. If you don't
> > > it will be hard to use them in the future, because they will contain
> > > 42, or some other random values.
> > >
> > > Is there a relationship between major:minor and reserved?
> > >
> >
> > No so far. Maybe it could be related if the protocol is updated in the future.
> 
> That implies that you cannot look at major:minor and know if the reserved bits
> are reserved, or have some actual meaning?
> 
> Again, think about forward/backwards compatibility. How do you turn a reserved
> bit into a used bit? Is the specification sufficient to make that possible.
> 
> > > > +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.
> > > > +
> > > > +  - 0: Disable wakeup from GPIO
> > > > +  - 1: Enable wakeup from GPIO
> > >
> > > What do you mean by wakeup?
> > >
> >
> > The gpio line can be enabled as an wakeup source for the system.
> 
> Keep going.....
> 
> Does that imply if none of the lines have wakeup enabled, the GPIO controller can
> be suspended when Linux suspends? How does the GPIO controller know it can
> suspend? There is no message for that. How does it know to come out of
> suspension?
> 

The power state of the remote GPIO controller is entirely managed by the remote firmware. 
The remote firmware operates as an independent system from Linux, with its own power states 
and policies for transitioning between modes. The wakeup field is solely intended to inform the 
remote firmware whether the GPIO line should be used as a wakeup source for the Linux system.

> > > > +Notification Message
> > > > +--------------------
> > > > +
> > > > +Notifications are sent with **Type=2 (GPIO_RPMSG_NOTIFY)**:
> > > > +
> > > > +.. 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.
> > >
> > > There is no need to acknowledge the notification? How do level interrupts
> work?
> > >
> >
> > Currently, there is no need to acknowledge the message, as the
> > interrupt is managed entirely by the remote firmware. On the Linux
> > side, a single notification message is received when an interrupt is triggered.
> 
> At sounds broken.
> 
> A level interrupt is not cleared until the level changes. The typical flow is:
> 
> Interrupt fires.
> 
> Interrupt is masked
> 
> Interrupt handler is called, which reads/write registers in the device who pin is
> connected to the GPIO
> 
> Interrupt is unmasked
> 

The sequences you mentioned above are managed entirely by the remote firmware. On the Linux 
side, it only receives a notification message when a GPIO line is triggered, which then invokes the 
corresponding interrupt handler.

Since the interrupt handling sequences are implemented in the remote firmware, the Linux driver 
can treat level-triggered and edge-triggered types in the same manner.

Thanks,
Shenwei

> 
> 
> At the unmask point, the interrupt will fire again, if the level is still in the active
> state. You need this because while reading/writing registers in the device,
> another event can happen, which needs handling. Generally, the interrupt output
> of a device is an OR of many different sources. You only release the interrupt
> when all sources have been cleared.
> 
> So for the protocol, you need to acknowledge the notification after the interrupt
> handler has executed. And that could cause another notification to be
> immediately sent if the line is still active.
> 
> I'm not too sure how edge interrupts should work. If we are still busy in an edge
> interrupt handler, do we want to know about the next edge?
> Should edge interrupts be disabled until the previous edge has been handled? I'm
> not too familiar with edge interrupts, most of the device i've handled are level.
> 
>         Andrew

Re: [PATCH v5 3/5] docs: staging: gpio-rpmsg: gpio over rpmsg bus

[Andrew Lunn]

> > > > > +- **wk**: Wakeup enable.
> > > > > +
> > > > > +  - 0: Disable wakeup from GPIO
> > > > > +  - 1: Enable wakeup from GPIO
> > > >
> > > > What do you mean by wakeup?
> > > >
> > >
> > > The gpio line can be enabled as an wakeup source for the system.
> > 
> > Keep going.....
> > 
> > Does that imply if none of the lines have wakeup enabled, the GPIO controller can
> > be suspended when Linux suspends? How does the GPIO controller know it can
> > suspend? There is no message for that. How does it know to come out of
> > suspension?
> > 
> 
> The power state of the remote GPIO controller is entirely managed by the remote firmware. 
> The remote firmware operates as an independent system from Linux, with its own power states 
> and policies for transitioning between modes. The wakeup field is solely intended to inform the 
> remote firmware whether the GPIO line should be used as a wakeup source for the Linux system.

O.K. How does the firmware use this information? How should it change
its behaviour? 

> > > > > +Notification Message
> > > > > +--------------------
> > > > > +
> > > > > +Notifications are sent with **Type=2 (GPIO_RPMSG_NOTIFY)**:
> > > > > +
> > > > > +.. 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.
> > > >
> > > > There is no need to acknowledge the notification? How do level interrupts
> > work?
> > > >
> > >
> > > Currently, there is no need to acknowledge the message, as the
> > > interrupt is managed entirely by the remote firmware. On the Linux
> > > side, a single notification message is received when an interrupt is triggered.
> > 
> > That sounds broken.
> > 
> > A level interrupt is not cleared until the level changes. The typical flow is:
> > 
> > Interrupt fires.
> > 
> > Interrupt is masked
> > 
> > Interrupt handler is called, which reads/write registers in the device who pin is
> > connected to the GPIO
> > 
> > Interrupt is unmasked
> > 
> 
> The sequences you mentioned above are managed entirely by the remote firmware. On the Linux 
> side, it only receives a notification message when a GPIO line is triggered, which then invokes the 
> corresponding interrupt handler.
> 
> Since the interrupt handling sequences are implemented in the remote firmware, the Linux driver 
> can treat level-triggered and edge-triggered types in the same manner.

That is wrong. Edge and level are different and need different
handling. That is why the GPIO framework and the interrupt core
handles them differently.

The devices i mostly deal with are Ethernet PHYs. These are level
devices, the interrupt is active low. Within the PHY there are
multiple interrupt sources, which all get logically NORed together to
form the interrupt output line. Talking to the PHY over MDIO is
slow. Sometimes you need to read multiple registers to find out what
caused the interrupt and clear it. So your initial read suggests
interrupt source Y triggered the interrupt. While you are clearing Y,
source X becomes active. After you have cleared Y, the NORed interrupt
line is still active, because of X. The interrupt handler exits, the
IRQ core reenabled the interrupt, and you expect it to fire again so
that you go handle source X. If it does not fire again, you have lost
an interrupt, and potentially the hardware stops working.

There are also other use cases of level interrupts. You sometimes see
two PHY devices sharing one level interrupt. You get the same sort of
race condition. PHY #1 pulls the interrupt low, triggering an
interrupt. While handling it, PHY #2 also pulls it low. When the
handler exits, it has only handled the interrupt from PHY #1. PHY #2
is still pulling the interrupt low, and needs its handler calling. So
it is required the interrupt fires again when it is re-enabled.

Given the protocol you have defined, how do you tell the firmware that
Linux has finished handling the interrupt, and it should notify Linux
again if the interrupt is still active?

	Andrew

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

[Arnaud POULIQUEN]

hi Shenwei

On 11/6/25 17:26, Shenwei Wang wrote:
> Hi Arnaud,
> 
>> -----Original Message-----
>> From: Arnaud POULIQUEN <arnaud.pouliquen@foss.st.com>
>> Sent: Thursday, November 6, 2025 4:17 AM
>> To: Shenwei Wang <shenwei.wang@nxp.com>; Peng Fan <peng.fan@nxp.com>;
>> Bjorn Andersson <andersson@kernel.org>; Mathieu Poirier
>> <mathieu.poirier@linaro.org>; Rob Herring <robh@kernel.org>; Krzysztof
>> Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Shawn
>> Guo <shawnguo@kernel.org>; Sascha Hauer <s.hauer@pengutronix.de>;
>> Jonathan Corbet <corbet@lwn.net>; Linus Walleij <linus.walleij@linaro.org>;
>> Bartosz Golaszewski <brgl@bgdev.pl>
>> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>; Fabio Estevam
>> <festevam@gmail.com>; linux-remoteproc@vger.kernel.org;
>> devicetree@vger.kernel.org; imx@lists.linux.dev; linux-arm-
>> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; linux-
>> doc@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Randy Dunlap
>> <rdunlap@infradead.org>; Andrew Lunn <andrew@lunn.ch>; linux-
>> gpio@vger.kernel.org
>> Subject: [EXT] Re: [PATCH v5 0/5] Enable Remote GPIO over RPMSG on i.MX
>> Platform
>>
>> Hi Shenwei,
>>
>> On 11/5/25 15:12, Shenwei Wang wrote:
>>> Hi Arnaud,
>>>
>>>> -----Original Message-----
>>>>
>>>> On 11/5/25 02:12, Peng Fan wrote:
>>>>> Hi Shenwei
>>>>>
>>>>>> Subject: [PATCH v5 0/5] Enable Remote GPIO over RPMSG on i.MX
>>>>>> Platform
>>>>>>
>>>>>> Support the remote devices on the remote processor via the RPMSG
>>>>>> bus on i.MX platform.
>>>>>>
>>>>>
>>>>> I have not look into the details of new version, but before that,
>>>>> just want to check, have we agreed on what Arnaud suggested?
>>>>> or continue to proceed to be this as i.MX specific?
>>>>
>>>> Thanks, Peng, for pointing this out. Regarding the V3 discussions, it
>>>> seems that I am not the only one suggesting a generic driver.
>>>>
>>>
>>> As I mentioned before, the only i.MX-specific part is the transport protocol over
>> the RPMSG bus.
>>> In this v5 patches, we’ve included detailed documentation for the
>>> protocol in a separate file. Any platform that follows the same protocol should
>> work right out of the box.
>>>
>>> If you spot anything that could be improved, please let me know!
>>
>> My concerns remain the same as those shared previously:
>>
>> 1) The simpler one: gpio-imx-rpmsg.c should be renamed to gpio-rpmsg.c.
>>
> 
> Agree. Will fix it in the next version.
> 
>> 2) The more complex one: the driver should be independent of the remoteproc
>> driver. The rpmsg protocol relies on virtio and can be used in contexts other than
>> remoteproc. In other words, the struct rpmsg_driver and its associated
>> operations should be defined within the rpmsg-gpio driver, not in the remoteproc
>> driver.
>>
> 
> The GPIO driver operates independently of the remoteproc driver. 

The channel_device_map table in imx_rproc.c would give me the opposite 
feeling

It functions based
> on the defined GPIO-RPMSG transport protocol. Any remoteproc that supports
> this protocol can exchange data with the GPIO driver via the underlying rpmsg bus.
> Placing the rpmsg_driver (which manages the rpmsg channel) within the remoteproc
> driver is more logical, as rpmsg channels run on the rpmsg bus. This bus is defined inside
> the remoteproc device tree node and is populated by the corresponding remoteproc driver.


Regarding imx_of_rpmsg_node_init(), It seems you rely on device tree in 
the rproc platform to register rpmsg drivers. This implies you are 
creating drivers based on a device description. To me, this does not 
appear to be a valid implementation, but perhaps such an approach 
already exists in the Linux kernel?


For your information, I'm facing a similar issue with my remoteproc_tee 
series [1]. The advice I received was to look at the PCIe DT 
implementation (I haven't had time to explore it yet). This advice also 
seems relevant to your series.

Do you also have a look to rpmsg_virtio_bus ? it seems a good candidate 
to match the device tree properties with the rpmsg device?

In the end, this is my point of view. Perhaps it is better to wait for 
others before deciding on the direction...

Thanks,
Arnaud

[1]https://lists.infradead.org/pipermail/linux-arm-kernel/2025-October/1069154.html

> 
> Thanks,
> Shenwei
> 
>>
>> Thanks,
>> Arnaud
>>
>>>
>>> Thanks,
>>> Shenwei
>>>
>>>> Thanks,
>>>> Arnaud
>>>>
>>>>>
>>>>> Thanks
>>>>> Peng.
>>>
> 

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

[Andrew Lunn]

> For your information, I'm facing a similar issue with my remoteproc_tee
> series [1]. The advice I received was to look at the PCIe DT implementation
> (I haven't had time to explore it yet). This advice also seems relevant to
> your series.
> 
> Do you also have a look to rpmsg_virtio_bus ? it seems a good candidate to
> match the device tree properties with the rpmsg device?
> 
> In the end, this is my point of view. Perhaps it is better to wait for
> others before deciding on the direction...

There might also be some ideas which can be take from greybus. It also
implements remote devices over a communication medium. 

	Andrew

Re: [PATCH v5 3/5] docs: staging: gpio-rpmsg: gpio over rpmsg bus

[Shenwei Wang]

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Thursday, November 6, 2025 6:32 PM
> To: Shenwei Wang <shenwei.wang@nxp.com>
> Cc: Bjorn Andersson <andersson@kernel.org>; Mathieu Poirier
> <mathieu.poirier@linaro.org>; Rob Herring <robh@kernel.org>; Krzysztof
> Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Shawn
> Guo <shawnguo@kernel.org>; Sascha Hauer <s.hauer@pengutronix.de>;
> Jonathan Corbet <corbet@lwn.net>; Linus Walleij <linus.walleij@linaro.org>;
> Bartosz Golaszewski <brgl@bgdev.pl>; Pengutronix Kernel Team
> <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; Peng Fan
> <peng.fan@nxp.com>; linux-remoteproc@vger.kernel.org;
> devicetree@vger.kernel.org; imx@lists.linux.dev; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; linux-
> doc@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>
> Subject: [EXT] Re: [PATCH v5 3/5] docs: staging: gpio-rpmsg: gpio over rpmsg bus
> >
> > The power state of the remote GPIO controller is entirely managed by the
> remote firmware.
> > The remote firmware operates as an independent system from Linux, with
> > its own power states and policies for transitioning between modes. The
> > wakeup field is solely intended to inform the remote firmware whether the
> GPIO line should be used as a wakeup source for the Linux system.
> 
> O.K. How does the firmware use this information? How should it change its
> behaviour?
> 

The remote system should always aim to stay in a power-efficient state by shutting down 
or clock-gating any blocks that aren't in use. In this wakeup scenario, if no GPIO lines are 
requested or marked as wakeup sources for Linux, the remote firmware should put the 
GPIO controller into a low-power state.

> > > > > > +Notification Message
> > > > > > +--------------------
> > > > > > +
> > > > > > +Notifications are sent with **Type=2 (GPIO_RPMSG_NOTIFY)**:
> > > > > > +
> > > > > > +.. 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.
> > > > >
> > > > > There is no need to acknowledge the notification? How do level
> > > > > interrupts
> > > work?
> > > > >
> > > >
> > > > Currently, there is no need to acknowledge the message, as the
> > > > interrupt is managed entirely by the remote firmware. On the Linux
> > > > side, a single notification message is received when an interrupt is triggered.
> > >
> > > That sounds broken.
> > >
> > > A level interrupt is not cleared until the level changes. The typical flow is:
> > >
> > > Interrupt fires.
> > >
> > > Interrupt is masked
> > >
> > > Interrupt handler is called, which reads/write registers in the
> > > device who pin is connected to the GPIO
> > >
> > > Interrupt is unmasked
> > >
> >
> > The sequences you mentioned above are managed entirely by the remote
> > firmware. On the Linux side, it only receives a notification message
> > when a GPIO line is triggered, which then invokes the corresponding interrupt
> handler.
> >
> > Since the interrupt handling sequences are implemented in the remote
> > firmware, the Linux driver can treat level-triggered and edge-triggered types in
> the same manner.
> 
> That is wrong. Edge and level are different and need different handling. That is
> why the GPIO framework and the interrupt core handles them differently.
> 
> The devices i mostly deal with are Ethernet PHYs. These are level devices, the
> interrupt is active low. Within the PHY there are multiple interrupt sources, which
> all get logically NORed together to form the interrupt output line. Talking to the
> PHY over MDIO is slow. Sometimes you need to read multiple registers to find out
> what caused the interrupt and clear it. So your initial read suggests interrupt
> source Y triggered the interrupt. While you are clearing Y, source X becomes
> active. After you have cleared Y, the NORed interrupt line is still active, because
> of X. The interrupt handler exits, the IRQ core reenabled the interrupt, and you
> expect it to fire again so that you go handle source X. If it does not fire again, you
> have lost an interrupt, and potentially the hardware stops working.
> 
> There are also other use cases of level interrupts. You sometimes see two PHY
> devices sharing one level interrupt. You get the same sort of race condition. PHY
> #1 pulls the interrupt low, triggering an interrupt. While handling it, PHY #2 also
> pulls it low. When the handler exits, it has only handled the interrupt from PHY
> #1. PHY #2 is still pulling the interrupt low, and needs its handler calling. So it is
> required the interrupt fires again when it is re-enabled.
> 
> Given the protocol you have defined, how do you tell the firmware that Linux has
> finished handling the interrupt, and it should notify Linux again if the interrupt is
> still active?
> 

Okay. To fully simulate a level-triggered interrupt, a notification reply message is required.

Remote firmware sequence:
Receive the level-triggered GPIO interrupt.
Mask the interrupt for the corresponding line.
Send a notification message to Linux.
Wait for the notification reply, then unmask the interrupt for the line.

Linux sequence:
Receive the notification message.
Invoke the interrupt handler for the line.
Send a notification reply to the remote firmware to indicate End of Interrupt (EOI).

Thanks,
Shenwei

>         Andrew

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

[Shenwei Wang]

> -----Original Message-----
> From: Arnaud POULIQUEN <arnaud.pouliquen@foss.st.com>
> Sent: Friday, November 7, 2025 11:18 AM
> To: Shenwei Wang <shenwei.wang@nxp.com>; Peng Fan <peng.fan@nxp.com>;
> Bjorn Andersson <andersson@kernel.org>; Mathieu Poirier
> <mathieu.poirier@linaro.org>; Rob Herring <robh@kernel.org>; Krzysztof
> Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Shawn
> Guo <shawnguo@kernel.org>; Sascha Hauer <s.hauer@pengutronix.de>;
> Jonathan Corbet <corbet@lwn.net>; Linus Walleij <linus.walleij@linaro.org>;
> Bartosz Golaszewski <brgl@bgdev.pl>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>; Fabio Estevam
> <festevam@gmail.com>; linux-remoteproc@vger.kernel.org;
> devicetree@vger.kernel.org; imx@lists.linux.dev; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; linux-
> doc@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Randy Dunlap
> <rdunlap@infradead.org>; Andrew Lunn <andrew@lunn.ch>; linux-
> gpio@vger.kernel.org
> Subject: [EXT] Re: [PATCH v5 0/5] Enable Remote GPIO over RPMSG on i.MX
> Platform
> >>>>> I have not look into the details of new version, but before that,
> >>>>> just want to check, have we agreed on what Arnaud suggested?
> >>>>> or continue to proceed to be this as i.MX specific?
> >>>>
> >>>> Thanks, Peng, for pointing this out. Regarding the V3 discussions,
> >>>> it seems that I am not the only one suggesting a generic driver.
> >>>>
> >>>
> >>> As I mentioned before, the only i.MX-specific part is the transport
> >>> protocol over
> >> the RPMSG bus.
> >>> In this v5 patches, we've included detailed documentation for the
> >>> protocol in a separate file. Any platform that follows the same
> >>> protocol should
> >> work right out of the box.
> >>>
> >>> If you spot anything that could be improved, please let me know!
> >>
> >> My concerns remain the same as those shared previously:
> >>
> >> 1) The simpler one: gpio-imx-rpmsg.c should be renamed to gpio-rpmsg.c.
> >>
> >
> > Agree. Will fix it in the next version.
> >
> >> 2) The more complex one: the driver should be independent of the
> >> remoteproc driver. The rpmsg protocol relies on virtio and can be
> >> used in contexts other than remoteproc. In other words, the struct
> >> rpmsg_driver and its associated operations should be defined within
> >> the rpmsg-gpio driver, not in the remoteproc driver.
> >>
> >
> > The GPIO driver operates independently of the remoteproc driver.
>
> The channel_device_map table in imx_rproc.c would give me the opposite feeling
>
> It functions based

The channel_device_map is used to control the order in which devices are populated.
This mechanism is key to implementing the probe-defer feature for the GPIO driver.
For those who don't care about the probe DEFER, you don't need to put the devices in the map.

> > on the defined GPIO-RPMSG transport protocol. Any remoteproc that
> > supports this protocol can exchange data with the GPIO driver via the
> underlying rpmsg bus.
> > Placing the rpmsg_driver (which manages the rpmsg channel) within the
> > remoteproc driver is more logical, as rpmsg channels run on the rpmsg
> > bus. This bus is defined inside the remoteproc device tree node and is
> populated by the corresponding remoteproc driver.
>
>
> Regarding imx_of_rpmsg_node_init(), It seems you rely on device tree in the
> rproc platform to register rpmsg drivers. This implies you are creating drivers
> based on a device description. To me, this does not appear to be a valid
> implementation, but perhaps such an approach already exists in the Linux kernel?
>

This follows a standard DTS population approach. Regardless of how the code is organized,
you need the logic somewhere to populate the devices. Placing the "rpmsg" bus under the
remoteproc node is more straightforward because the bus depends on the remote processor.

>
> For your information, I'm facing a similar issue with my remoteproc_tee series
> [1]. The advice I received was to look at the PCIe DT implementation (I haven't
> had time to explore it yet). This advice also seems relevant to your series.
>
> Do you also have a look to rpmsg_virtio_bus ? it seems a good candidate to
> match the device tree properties with the rpmsg device?
>

The implementation is actually based on the rpmsg_virtio_bus. The rpmsg_send
Will finally call virtio_rpmsg_send.

Thanks,
Shenwei

> In the end, this is my point of view. Perhaps it is better to wait for others before
> deciding on the direction...
>
> Thanks,
> Arnaud
>
> [1]https://lists.infr/
> adead.org%2Fpipermail%2Flinux-arm-kernel%2F2025-
> October%2F1069154.html&data=05%7C02%7Cshenwei.wang%40nxp.com%7C0
> 2b24edd459f4d349b2608de1e21b45d%7C686ea1d3bc2b4c6fa92cd99c5c301635
> %7C0%7C0%7C638981327288728022%7CUnknown%7CTWFpbGZsb3d8eyJFbXB
> 0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCI
> sIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=KHeu3GVR%2BYvFnNh%2FgwizEr
> bu%2B0UnwkB5vf%2BuLrRQReE%3D&reserved=0
>
> >
> > Thanks,
> > Shenwei
> >
> >>
> >> Thanks,
> >> Arnaud
> >>
> >>>
> >>> Thanks,
> >>> Shenwei
> >>>
> >>>> Thanks,
> >>>> Arnaud
> >>>>
> >>>>>
> >>>>> Thanks
> >>>>> Peng.
> >>>
> >

Re: [PATCH v5 3/5] docs: staging: gpio-rpmsg: gpio over rpmsg bus

[Andrew Lunn]

> > > The power state of the remote GPIO controller is entirely managed by the
> > remote firmware.
> > > The remote firmware operates as an independent system from Linux, with
> > > its own power states and policies for transitioning between modes. The
> > > wakeup field is solely intended to inform the remote firmware whether the
> > GPIO line should be used as a wakeup source for the Linux system.
> > 
> > O.K. How does the firmware use this information? How should it change its
> > behaviour?
> > 
> 
> The remote system should always aim to stay in a power-efficient state by shutting down 
> or clock-gating any blocks that aren't in use. In this wakeup scenario, if no GPIO lines are 
> requested or marked as wakeup sources for Linux, the remote firmware should put the 
> GPIO controller into a low-power state.

There are no messages defined to tell the GPIO controller Linux is
suspended.

Since the firmware has no idea Linux is asleep, the firmware is
performing all the usual processing, driving output pins, monitoring
input pins, delivering interrupt notifications. If no pins are marked
as wakeup, it can then enter some sort of low-power state, which
allows it to do all this work, plus save power? How?

I've also been thinking about what a wake up source actually
means. I've been looking at this from one use case i know, an Ethernet
PHY performing Wake on LAN? What normally happens is that Linux
suspends, but leaves the main SoC interrupt controller enabled, and
parts of the GPIO controller. The GPIO controller has a hard wired
connection to the interrupt controller. When the PHY indicated WoL by
driving its output pin low, triggering an interrupt, the GPIO triggers
the main interrupt controller, which wakes the CPU.

How does this work here, in a message passing system? Linux is
asleep. While asleep, does it still process all remote proc messages?
How? Does it wake up for each message and go back to sleep once it
finds it is a non wake notification? Since the firmware does not know
Linux is asleep, it will still be sending notifications for non-wake
interrupts. How does Linux actually know to wake up? Do you require
that the low level remote proc mechanism is also wake capable? So in
effect, Linux needs to go up the device chain and enable wake source
not only in the GPIO layer but also the remote proc layer? And
whatever mechanism that is based on, until you get to an interrupt
which can actually wake the system?

> Okay. To fully simulate a level-triggered interrupt, a notification reply message is required.

I would not word it like that. All you currently have is edge. To
simulate level triggered interrupts you need a notification reply
message.

> Remote firmware sequence:
> Receive the level-triggered GPIO interrupt.
> Mask the interrupt for the corresponding line.
> Send a notification message to Linux.
> Wait for the notification reply, then unmask the interrupt for the line.
> 
> Linux sequence:
> Receive the notification message.
> Invoke the interrupt handler for the line.
> Send a notification reply to the remote firmware to indicate End of Interrupt (EOI).

That sounds more reasonable.

	Andrew

Re: [PATCH v5 3/5] docs: staging: gpio-rpmsg: gpio over rpmsg bus

[Shenwei Wang]

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Saturday, November 8, 2025 11:46 AM
> To: Shenwei Wang <shenwei.wang@nxp.com>
> Cc: Bjorn Andersson <andersson@kernel.org>; Mathieu Poirier
> <mathieu.poirier@linaro.org>; Rob Herring <robh@kernel.org>; Krzysztof
> Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Shawn
> Guo <shawnguo@kernel.org>; Sascha Hauer <s.hauer@pengutronix.de>;
> Jonathan Corbet <corbet@lwn.net>; Linus Walleij <linus.walleij@linaro.org>;
> Bartosz Golaszewski <brgl@bgdev.pl>; Pengutronix Kernel Team
> <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; Peng Fan
> <peng.fan@nxp.com>; linux-remoteproc@vger.kernel.org;
> devicetree@vger.kernel.org; imx@lists.linux.dev; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; linux-
> doc@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>
> Subject: [EXT] Re: [PATCH v5 3/5] docs: staging: gpio-rpmsg: gpio over rpmsg bus
> > The remote system should always aim to stay in a power-efficient state
> > by shutting down or clock-gating any blocks that aren't in use. In
> > this wakeup scenario, if no GPIO lines are requested or marked as
> > wakeup sources for Linux, the remote firmware should put the GPIO controller
> into a low-power state.
> 
> There are no messages defined to tell the GPIO controller Linux is suspended.
> 
> Since the firmware has no idea Linux is asleep, the firmware is performing all the
> usual processing, driving output pins, monitoring input pins, delivering interrupt
> notifications. If no pins are marked as wakeup, it can then enter some sort of
> low-power state, which allows it to do all this work, plus save power? How?
> 
> I've also been thinking about what a wake up source actually means. I've been
> looking at this from one use case i know, an Ethernet PHY performing Wake on
> LAN? What normally happens is that Linux suspends, but leaves the main SoC
> interrupt controller enabled, and parts of the GPIO controller. The GPIO controller
> has a hard wired connection to the interrupt controller. When the PHY indicated
> WoL by driving its output pin low, triggering an interrupt, the GPIO triggers the
> main interrupt controller, which wakes the CPU.
> 
> How does this work here, in a message passing system? Linux is asleep. While
> asleep, does it still process all remote proc messages?
> How? Does it wake up for each message and go back to sleep once it finds it is a
> non wake notification? Since the firmware does not know Linux is asleep, it will
> still be sending notifications for non-wake interrupts. How does Linux actually
> know to wake up? Do you require that the low level remote proc mechanism is
> also wake capable? So in effect, Linux needs to go up the device chain and enable
> wake source not only in the GPIO layer but also the remote proc layer? And
> whatever mechanism that is based on, until you get to an interrupt which can
> actually wake the system?
> 

The remote firmware does not need to know whether Linux is asleep. The GPIO is not used 
to wake Linux directly; instead, it serves as a wake-up source for the remote firmware if configured 
accordingly. Once the remote firmware is awake, it sends a notification message to Linux. This 
notification is the actual event that wakes Linux.

This works because there is always a physical interface connecting Linux and the remote firmware. 
On i.MX platforms, this interface is the MU block. When the remoteproc driver is running, the MU 
block is automatically configured as a wake-up source for Linux by default. As a result, the notification 
message can wake the Linux system if it is asleep.

Thanks,
Shenwei

> > Okay. To fully simulate a level-triggered interrupt, a notification reply message
> is required.
> 
> I would not word it like that. All you currently have is edge. To simulate level
> triggered interrupts you need a notification reply message.
> 
> > Remote firmware sequence:
> > Receive the level-triggered GPIO interrupt.
> > Mask the interrupt for the corresponding line.
> > Send a notification message to Linux.
> > Wait for the notification reply, then unmask the interrupt for the line.
> >
> > Linux sequence:
> > Receive the notification message.
> > Invoke the interrupt handler for the line.
> > Send a notification reply to the remote firmware to indicate End of Interrupt
> (EOI).
> 
> That sounds more reasonable.
> 
>         Andrew

Re: [PATCH v5 3/5] docs: staging: gpio-rpmsg: gpio over rpmsg bus

[Andrew Lunn]

> The remote firmware does not need to know whether Linux is asleep. The GPIO is not used 
> to wake Linux directly; instead, it serves as a wake-up source for the remote firmware if configured 
> accordingly. Once the remote firmware is awake, it sends a notification message to Linux. This 
> notification is the actual event that wakes Linux.
> 
> This works because there is always a physical interface connecting Linux and the remote firmware. 
> On i.MX platforms, this interface is the MU block. When the remoteproc driver is running, the MU 
> block is automatically configured as a wake-up source for Linux by default. As a result, the notification 
> message can wake the Linux system if it is asleep.

You need to add a lot more documentation to the specification to make
this clear. As you said, the firmware and Linux have different
sleep/wake life cycles. How does the firmware know it is safe to go to
sleep, if it has no idea Linux is running or suspended?

	Andrew

Re: [PATCH v5 3/5] docs: staging: gpio-rpmsg: gpio over rpmsg bus

[Shenwei Wang]

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Monday, November 10, 2025 9:59 AM
> To: Shenwei Wang <shenwei.wang@nxp.com>
> Cc: Bjorn Andersson <andersson@kernel.org>; Mathieu Poirier
> <mathieu.poirier@linaro.org>; Rob Herring <robh@kernel.org>; Krzysztof
> Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Shawn
> Guo <shawnguo@kernel.org>; Sascha Hauer <s.hauer@pengutronix.de>;
> Jonathan Corbet <corbet@lwn.net>; Linus Walleij <linus.walleij@linaro.org>;
> Bartosz Golaszewski <brgl@bgdev.pl>; Pengutronix Kernel Team
> <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; Peng Fan
> <peng.fan@nxp.com>; linux-remoteproc@vger.kernel.org;
> devicetree@vger.kernel.org; imx@lists.linux.dev; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; linux-
> doc@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>
> Subject: [EXT] Re: [PATCH v5 3/5] docs: staging: gpio-rpmsg: gpio over rpmsg bus
> > The remote firmware does not need to know whether Linux is asleep. The
> > GPIO is not used to wake Linux directly; instead, it serves as a
> > wake-up source for the remote firmware if configured accordingly. Once
> > the remote firmware is awake, it sends a notification message to Linux. This
> notification is the actual event that wakes Linux.
> >
> > This works because there is always a physical interface connecting Linux and
> the remote firmware.
> > On i.MX platforms, this interface is the MU block. When the remoteproc
> > driver is running, the MU block is automatically configured as a
> > wake-up source for Linux by default. As a result, the notification message can
> wake the Linux system if it is asleep.
> 
> You need to add a lot more documentation to the specification to make this
> clear. As you said, the firmware and Linux have different sleep/wake life cycles.
> How does the firmware know it is safe to go to sleep, if it has no idea Linux is
> running or suspended?
> 

The remoteproc driver is responsible for managing the remote firmware. The GPIO driver 
operates independently of this process and functions transparently on top of it. 
So the GPIO driver does not require to know the firmware's running states.

Thanks,
Shenwei

>         Andrew