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

[PATCH v12 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 v12:
 - Fixed the "underline" warning reported by Randy.

Changes in v11:
 - Expand RPMSG for the first time per Shuah's review comment.

Changes in v10:
 - Update gpio-rpmsg.rst according to Daniel Baluta's review comments.
 - Add a kernel CONFIG for fixed up handlers and only enable it on
   i.MX products.
 - Fixed bugs reported by kernel test robot.

Changes in v9:
 - Reuse the gpio-virtio design for command and IRQ type definitions.
 - Remove msg_id, version, and vendor fields from the generic protocol.
 - Add fixed-up handlers to support legacy firmware.

Changes in v8:
 - Add "depends on REMOTEPROC" in Kconfig to fix the build error reported
   by the kernel test robot.
 - Move the .rst patch before the .yaml patch.
 - Handle the "ngpios" DT property based on Andrew's feedback.

Changes in v7:
 - Reworked the driver to use the rpmsg_driver framework instead of
   platform_driver, based on feedback from Bjorn and Arnaud.
 - Updated gpio-rpmsg.yaml and imx_rproc.yaml according to comments from
   Rob and Arnaud.
 - Further refinements to gpio-rpmsg.yaml per Arnaud's feedback.

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

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

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

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

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

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

 .../devicetree/bindings/gpio/gpio-rpmsg.yaml  |  55 ++
 .../bindings/remoteproc/fsl,imx-rproc.yaml    |  53 ++
 Documentation/driver-api/gpio/gpio-rpmsg.rst  | 266 +++++++
 Documentation/driver-api/gpio/index.rst       |   1 +
 arch/arm64/boot/dts/freescale/imx8ulp.dtsi    |  25 +
 drivers/gpio/Kconfig                          |  32 +
 drivers/gpio/Makefile                         |   1 +
 drivers/gpio/gpio-rpmsg.c                     | 743 ++++++++++++++++++
 8 files changed, 1176 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-rpmsg.yaml
 create mode 100644 Documentation/driver-api/gpio/gpio-rpmsg.rst
 create mode 100644 drivers/gpio/gpio-rpmsg.c

--
2.43.0

[PATCH v12 1/5] docs: driver-api: gpio: rpmsg gpio driver over rpmsg bus

[Shenwei Wang]

Describes the gpio rpmsg transport protocol over the rpmsg bus between
the remote system and Linux.

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

diff --git a/Documentation/driver-api/gpio/gpio-rpmsg.rst b/Documentation/driver-api/gpio/gpio-rpmsg.rst
new file mode 100644
index 000000000000..abfde68c9b0a
--- /dev/null
+++ b/Documentation/driver-api/gpio/gpio-rpmsg.rst
@@ -0,0 +1,266 @@
+.. SPDX-License-Identifier: GPL-2.0-or-later
+
+GPIO RPMSG (Remote Processor Messaging) Protocol
+================================================
+
+The GPIO RPMSG transport protocol is used for communication and interaction
+with GPIO controllers on remote processors via the RPMSG bus.
+
+Message Format
+--------------
+
+The RPMSG message consists of a 6-byte packet with the following layout:
+
+.. code-block:: none
+
+   +-----+-----+-----+-----+-----+----+
+   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05|
+   |type |cmd  |port |line |  data    |
+   +-----+-----+-----+-----+-----+----+
+
+- **type (Message Type)**: The message type can be one of:
+
+  - 0: GPIO_RPMSG_SEND
+  - 1: GPIO_RPMSG_REPLY
+  - 2: GPIO_RPMSG_NOTIFY
+
+- **cmd**: Command code, used for GPIO_RPMSG_SEND messages.
+
+- **port**: The GPIO port (bank) index.
+
+- **line**: The GPIO line (pin) index of the port.
+
+- **data**: See details in the command description below.
+
+- **reply err**: Error code from the remote core.
+
+  - 0: Success
+  - 1: General error (Early remote software only returns this unclassified error)
+  - 2: Not supported (A command is not supported by the remote firmware)
+  - 3: Resource not available (The resource is not allocated to Linux)
+  - 4: Resource busy (The resource is already in use)
+  - 5: Parameter error
+
+
+GPIO Commands
+-------------
+
+Commands are specified in the **Cmd** field for **GPIO_RPMSG_SEND** (Type=0) messages.
+
+The SEND message is always sent from Linux to the remote firmware. Each
+SEND corresponds to a single REPLY message. The GPIO driver should
+serialize messages and determine whether a REPLY message is required. If a
+REPLY message is expected but not received within the specified timeout
+period (currently 1 second in the Linux driver), the driver should return
+-ETIMEOUT.
+
+GET_DIRECTION (Cmd=2)
+~~~~~~~~~~~~~~~~~~~~~
+
+**Request:**
+
+.. code-block:: none
+
+   +-----+-----+-----+-----+-----+----+
+   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05|
+   | 0   | 2   |port |line | 0   | 0  |
+   +-----+-----+-----+-----+-----+----+
+
+**Reply:**
+
+.. code-block:: none
+
+   +-----+-----+-----+-----+-----+----+
+   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05|
+   | 1   | 2   |port |line | err | dir|
+   +-----+-----+-----+-----+-----+----+
+
+- **err**: See above for definitions.
+
+- **dir**: Direction.
+
+  - 0: None
+  - 1: Output
+  - 2: Input
+
+SET_DIRECTION (Cmd=3)
+~~~~~~~~~~~~~~~~~~~~~
+
+**Request:**
+
+.. code-block:: none
+
+   +-----+-----+-----+-----+-----+----+
+   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05|
+   | 0   | 3   |port |line | dir | 0  |
+   +-----+-----+-----+-----+-----+----+
+
+- **dir**: Direction.
+
+  - 0: None
+  - 1: Output
+  - 2: Input
+
+**Reply:**
+
+.. code-block:: none
+
+   +-----+-----+-----+-----+-----+----+
+   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05|
+   | 1   | 3   |port |line | err | 0  |
+   +-----+-----+-----+-----+-----+----+
+
+- **err**: See above for definitions.
+
+
+GET_VALUE (Cmd=4)
+~~~~~~~~~~~~~~~~~
+
+**Request:**
+
+.. code-block:: none
+
+   +-----+-----+-----+-----+-----+----+
+   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05|
+   | 0   | 4   |port |line | 0   | 0  |
+   +-----+-----+-----+-----+-----+----+
+
+**Reply:**
+
+.. code-block:: none
+
+   +-----+-----+-----+-----+-----+----+
+   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05|
+   | 1   | 4   |port |line | err | val|
+   +-----+-----+-----+-----+-----+----+
+
+- **err**: See above for definitions.
+
+- **val**: Line level.
+
+  - 0: Low
+  - 1: High
+
+SET_VALUE (Cmd=5)
+~~~~~~~~~~~~~~~~~
+
+**Request:**
+
+.. code-block:: none
+
+   +-----+-----+-----+-----+-----+----+
+   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05|
+   | 0   | 5   |port |line | val | 0  |
+   +-----+-----+-----+-----+-----+----+
+
+- **val**: Output level.
+
+  - 0: Low
+  - 1: High
+
+**Reply:**
+
+.. code-block:: none
+
+   +-----+-----+-----+-----+-----+----+
+   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05|
+   | 1   | 5   |port |line | err | 0  |
+   +-----+-----+-----+-----+-----+----+
+
+- **err**: See above for definitions.
+
+SET_IRQ_TYPE (Cmd=6)
+~~~~~~~~~~~~~~~~~~~~
+
+**Request:**
+
+.. code-block:: none
+
+   +-----+-----+-----+-----+-----+----+
+   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05|
+   | 0   | 6   |port |line | val | wk |
+   +-----+-----+-----+-----+-----+----+
+
+- **val**: IRQ types.
+
+  - 0: Interrupt disabled
+  - 1: Rising edge trigger
+  - 2: Falling edge trigger
+  - 3: Both edge trigger
+  - 4: High level trigger
+  - 8: Low level trigger
+
+- **wk**: Wakeup enable.
+
+  The remote system should always aim to stay in a power-efficient state by
+  shutting down or clock-gating the GPIO blocks that aren't in use. Since
+  the remoteproc driver is responsible for managing the power states of the
+  remote firmware, the GPIO driver does not require to know the firmware's
+  running states.
+
+  When the wakeup bit is set, the remote firmware should configure the line
+  as a wakeup source. The firmware should send the notification message to
+  Linux after it is woken from the GPIO line.
+
+  - 0: Disable wakeup from GPIO
+  - 1: Enable wakeup from GPIO
+
+**Reply:**
+
+.. code-block:: none
+
+   +-----+-----+-----+-----+-----+----+
+   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05|
+   | 1   | 6   |port |line | err | 0  |
+   +-----+-----+-----+-----+-----+----+
+
+- **err**: See above for definitions.
+
+NOTIFY_REPLY (Cmd=10)
+~~~~~~~~~~~~~~~~~~~~~
+The reply message for the notification is optional. The remote firmware can
+implement it to simulate the interrupt acknowledgment behavior.
+
+**Request:**
+
+.. code-block:: none
+
+   +-----+-----+-----+-----+-----+----+
+   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05|
+   | 0   | 10  |port |line |level| 0  |
+   +-----+-----+-----+-----+-----+----+
+
+- **port**: The GPIO port (bank) index.
+
+- **line**: The GPIO line (pin) index of the port.
+
+- **level**: GPIO line status.
+
+Notification Message
+--------------------
+
+Notifications are sent by the remote core and they have
+**Type=2 (GPIO_RPMSG_NOTIFY)**:
+
+When a GPIO line asserts an interrupt on the remote processor, the firmware
+should immediately mask the corresponding interrupt source and send a
+notification message to the Linux. Upon completion of the interrupt
+handling on the Linux side, the driver should issue a
+command **SET_IRQ_TYPE** to the firmware to unmask the interrupt.
+
+A Notification message can arrive between a SEND and its REPLY message,
+and the driver is expected to handle this scenario.
+
+.. code-block:: none
+
+   +-----+-----+-----+-----+-----+----+
+   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05|
+   | 2   | 0   |port |line |type | 0  |
+   +-----+-----+-----+-----+-----+----+
+
+- **port**: The GPIO port (bank) index.
+
+- **line**: The GPIO line (pin) index of the port.
+
+- **type**: Optional parameter to indicate the trigger event type.
+
diff --git a/Documentation/driver-api/gpio/index.rst b/Documentation/driver-api/gpio/index.rst
index bee58f709b9a..e5eb1f82f01f 100644
--- a/Documentation/driver-api/gpio/index.rst
+++ b/Documentation/driver-api/gpio/index.rst
@@ -16,6 +16,7 @@ Contents:
    drivers-on-gpio
    bt8xxgpio
    pca953x
+   gpio-rpmsg
 
 Core
 ====
-- 
2.43.0

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

[Shenwei Wang]

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

Define an RPMSG node to work as a container for a group of RPMSG channels
under the imx_rproc node. Each subnode within "rpmsg" represents an
individual RPMSG channel. The name of each subnode corresponds to the
channel name as defined by the remote processor.

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

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

diff --git a/Documentation/devicetree/bindings/gpio/gpio-rpmsg.yaml b/Documentation/devicetree/bindings/gpio/gpio-rpmsg.yaml
new file mode 100644
index 000000000000..6c78b6850321
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-rpmsg.yaml
@@ -0,0 +1,55 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/gpio/gpio-rpmsg.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Generic RPMSG GPIO Controller
+
+maintainers:
+  - Shenwei Wang <shenwei.wang@nxp.com>
+
+description:
+  On an AMP platform, some GPIO controllers are exposed by the remote processor
+  through the RPMSG bus. The RPMSG GPIO transport protocol defines the packet
+  structure and communication flow between Linux and the remote firmware. Those
+  controllers are managed via this transport protocol. For more details of the
+  protocol, check the document below.
+  Documentation/driver-api/gpio/gpio-rpmsg.rst
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+          - enum:
+              - fsl,rpmsg-gpio
+          - const: rpmsg-gpio
+      - const: rpmsg-gpio
+
+  reg:
+    description:
+      The reg property represents the index of the GPIO controllers. Since
+      the driver manages controllers on a remote system, this index tells
+      the remote system which controller to operate.
+    maxItems: 1
+
+  "#gpio-cells":
+    const: 2
+
+  gpio-controller: true
+
+  interrupt-controller: true
+
+  "#interrupt-cells":
+    const: 2
+
+required:
+  - compatible
+  - reg
+  - "#gpio-cells"
+  - "#interrupt-cells"
+
+allOf:
+  - $ref: /schemas/gpio/gpio.yaml#
+
+unevaluatedProperties: false
diff --git a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
index ce8ec0119469..88281ffc18ca 100644
--- a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
@@ -85,6 +85,34 @@ properties:
       This property is to specify the resource id of the remote processor in SoC
       which supports SCFW
 
+  rpmsg:
+    type: object
+    additionalProperties: false
+    description:
+      Represents the RPMSG bus between Linux and the remote system. Contains
+      a group of RPMSG channel devices running on the bus.
+
+    properties:
+      rpmsg-io-channel:
+        type: object
+        additionalProperties: false
+        properties:
+          '#address-cells':
+            const: 1
+
+          '#size-cells':
+            const: 0
+
+        patternProperties:
+          "gpio@[0-9a-f]+$":
+            type: object
+            $ref: /schemas/gpio/gpio-rpmsg.yaml#
+            unevaluatedProperties: false
+
+        required:
+          - '#address-cells'
+          - '#size-cells'
+
 required:
   - compatible
 
@@ -147,5 +175,30 @@ examples:
                 &mu 3 1>;
       memory-region = <&vdev0buffer>, <&vdev0vring0>, <&vdev0vring1>, <&rsc_table>;
       syscon = <&src>;
+
+      rpmsg {
+        rpmsg-io-channel {
+          #address-cells = <1>;
+          #size-cells = <0>;
+
+          gpio@0 {
+            compatible = "rpmsg-gpio";
+            reg = <0>;
+            gpio-controller;
+            #gpio-cells = <2>;
+            #interrupt-cells = <2>;
+            interrupt-controller;
+          };
+
+          gpio@1 {
+            compatible = "rpmsg-gpio";
+            reg = <1>;
+            gpio-controller;
+            #gpio-cells = <2>;
+            #interrupt-cells = <2>;
+            interrupt-controller;
+          };
+        };
+      };
     };
 ...
-- 
2.43.0

[PATCH v12 3/5] gpio: rpmsg: add generic rpmsg GPIO driver

[Shenwei Wang]

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

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

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

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index b45fb799e36c..cff0fda8a283 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1892,6 +1892,23 @@ config GPIO_SODAVILLE
 
 endmenu
 
+menu "RPMSG GPIO drivers"
+	depends on RPMSG
+
+config GPIO_RPMSG
+	tristate "Generic RPMSG GPIO support"
+	depends on OF && REMOTEPROC
+	select GPIOLIB_IRQCHIP
+	default REMOTEPROC
+	help
+	  Say yes here to support the generic GPIO functions over the RPMSG
+	  bus. Currently supported devices: i.MX7ULP, i.MX8ULP, i.MX8x, and
+	  i.MX9x.
+
+	  If unsure, say N.
+
+endmenu
+
 menu "SPI GPIO expanders"
 	depends on SPI_MASTER
 
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index c05f7d795c43..501aba56ad68 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -158,6 +158,7 @@ obj-$(CONFIG_GPIO_RDC321X)		+= gpio-rdc321x.o
 obj-$(CONFIG_GPIO_REALTEK_OTTO)		+= gpio-realtek-otto.o
 obj-$(CONFIG_GPIO_REG)			+= gpio-reg.o
 obj-$(CONFIG_GPIO_ROCKCHIP)	+= gpio-rockchip.o
+obj-$(CONFIG_GPIO_RPMSG)		+= gpio-rpmsg.o
 obj-$(CONFIG_GPIO_RTD)			+= gpio-rtd.o
 obj-$(CONFIG_ARCH_SA1100)		+= gpio-sa1100.o
 obj-$(CONFIG_GPIO_SAMA5D2_PIOBU)	+= gpio-sama5d2-piobu.o
diff --git a/drivers/gpio/gpio-rpmsg.c b/drivers/gpio/gpio-rpmsg.c
new file mode 100644
index 000000000000..9c609b55bc14
--- /dev/null
+++ b/drivers/gpio/gpio-rpmsg.c
@@ -0,0 +1,596 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2026 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/of_device.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/remoteproc.h>
+#include <linux/rpmsg.h>
+#include <linux/virtio_gpio.h>
+
+#define MAX_PORT_PER_CHANNEL    10
+#define GPIOS_PER_PORT_DEFAULT	32
+#define RPMSG_TIMEOUT		1000
+
+/* GPIO RPMSG Type */
+#define GPIO_RPMSG_SEND		0
+#define GPIO_RPMSG_REPLY	1
+#define GPIO_RPMSG_NOTIFY	2
+
+struct rpmsg_gpio_packet {
+	u8 type;	/* Message type */
+	u8 cmd;		/* Command code */
+	u8 port_idx;
+	u8 line;
+	u8 val1;
+	u8 val2;
+};
+
+struct rpmsg_gpio_line {
+	u8 irq_shutdown;
+	u8 irq_unmask;
+	u8 irq_mask;
+	u32 irq_wake_enable;
+	u32 irq_type;
+	struct rpmsg_gpio_packet msg;
+};
+
+struct rpmsg_gpio_info {
+	struct rpmsg_device *rpdev;
+	struct rpmsg_gpio_packet *reply_msg;
+	struct completion cmd_complete;
+	struct mutex lock;
+	void **port_store;
+};
+
+struct rpmsg_gpio_port {
+	struct gpio_chip gc;
+	struct rpmsg_gpio_line lines[GPIOS_PER_PORT_DEFAULT];
+	struct rpmsg_gpio_info info;
+	u32 ngpios;
+	u32 idx;
+};
+
+struct rpmsg_gpio_fixed_up {
+	int (*send_fixed_up)(struct rpmsg_gpio_info *info, struct rpmsg_gpio_packet *msg);
+	struct rpmsg_gpio_packet *(*recv_fixed_up)(struct rpmsg_device *rpdev, void *data);
+};
+
+/*
+ * @rproc_name: the name of the remote proc.
+ * @recv_pkt: a pointer to the received packet for protocol fix up.
+ * @protocol_fixed_up: optional callbacks to handle protocol mismatches.
+ * @channel_devices: an array of the devices related to the rpdev.
+ */
+struct rpdev_drvdata {
+	const char *rproc_name;
+	void *recv_pkt;
+	struct rpmsg_gpio_fixed_up *protocol_fixed_up;
+	void *channel_devices[MAX_PORT_PER_CHANNEL];
+};
+
+static int rpmsg_gpio_send_message(struct rpmsg_gpio_port *port,
+				   struct rpmsg_gpio_packet *msg,
+				   bool sync)
+{
+	struct rpmsg_gpio_info *info = &port->info;
+	struct rpdev_drvdata *drvdata;
+	int ret;
+
+	drvdata = dev_get_drvdata(&info->rpdev->dev);
+	reinit_completion(&info->cmd_complete);
+
+	if (drvdata->protocol_fixed_up)
+		ret = drvdata->protocol_fixed_up->send_fixed_up(info, msg);
+	else
+		ret = rpmsg_send(info->rpdev->ept, msg, sizeof(*msg));
+
+	if (ret) {
+		dev_err(&info->rpdev->dev, "rpmsg_send failed: %d\n", ret);
+		return ret;
+	}
+
+	if (sync) {
+		ret = wait_for_completion_timeout(&info->cmd_complete,
+						  msecs_to_jiffies(RPMSG_TIMEOUT));
+		if (ret == 0) {
+			dev_err(&info->rpdev->dev, "rpmsg_send timeout!\n");
+			return -ETIMEDOUT;
+		}
+
+		if (info->reply_msg->val1 != 0) {
+			dev_err(&info->rpdev->dev, "remote core replies an error: %d!\n",
+				info->reply_msg->val1);
+			return -EINVAL;
+		}
+
+		/* copy the reply message */
+		memcpy(&port->lines[info->reply_msg->line].msg,
+		       info->reply_msg, sizeof(*info->reply_msg));
+	}
+
+	return 0;
+}
+
+static struct rpmsg_gpio_packet *
+rpmsg_gpio_msg_init_common(struct rpmsg_gpio_port *port, unsigned int line, u8 cmd)
+{
+	struct rpmsg_gpio_packet *msg = &port->lines[line].msg;
+
+	memset(msg, 0, sizeof(struct rpmsg_gpio_packet));
+	msg->type = GPIO_RPMSG_SEND;
+	msg->cmd = cmd;
+	msg->port_idx = port->idx;
+	msg->line = line;
+
+	return msg;
+}
+
+static int rpmsg_gpio_get(struct gpio_chip *gc, unsigned int line)
+{
+	struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
+	struct rpmsg_gpio_packet *msg;
+	int ret;
+
+	guard(mutex)(&port->info.lock);
+
+	msg = rpmsg_gpio_msg_init_common(port, line, VIRTIO_GPIO_MSG_GET_VALUE);
+
+	ret = rpmsg_gpio_send_message(port, msg, true);
+	if (!ret)
+		ret = !!port->lines[line].msg.val2;
+
+	return ret;
+}
+
+static int rpmsg_gpio_get_direction(struct gpio_chip *gc, unsigned int line)
+{
+	struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
+	struct rpmsg_gpio_packet *msg;
+	int ret;
+
+	guard(mutex)(&port->info.lock);
+
+	msg = rpmsg_gpio_msg_init_common(port, line, VIRTIO_GPIO_MSG_GET_DIRECTION);
+
+	ret = rpmsg_gpio_send_message(port, msg, true);
+	if (ret)
+		return ret;
+
+	switch (port->lines[line].msg.val2) {
+	case VIRTIO_GPIO_DIRECTION_IN:
+		return GPIO_LINE_DIRECTION_IN;
+	case VIRTIO_GPIO_DIRECTION_OUT:
+		return GPIO_LINE_DIRECTION_OUT;
+	default:
+		break;
+	}
+
+	return -EINVAL;
+}
+
+static int rpmsg_gpio_direction_input(struct gpio_chip *gc, unsigned int line)
+{
+	struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
+	struct rpmsg_gpio_packet *msg;
+
+	guard(mutex)(&port->info.lock);
+
+	msg = rpmsg_gpio_msg_init_common(port, line, VIRTIO_GPIO_MSG_SET_DIRECTION);
+	msg->val1 = VIRTIO_GPIO_DIRECTION_IN;
+
+	return rpmsg_gpio_send_message(port, msg, true);
+}
+
+static int rpmsg_gpio_set(struct gpio_chip *gc, unsigned int line, int val)
+{
+	struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
+	struct rpmsg_gpio_packet *msg;
+
+	guard(mutex)(&port->info.lock);
+
+	msg = rpmsg_gpio_msg_init_common(port, line, VIRTIO_GPIO_MSG_SET_VALUE);
+	msg->val1 = val;
+
+	return rpmsg_gpio_send_message(port, msg, true);
+}
+
+static int rpmsg_gpio_direction_output(struct gpio_chip *gc, unsigned int line, int val)
+{
+	struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
+	struct rpmsg_gpio_packet *msg;
+	int ret;
+
+	guard(mutex)(&port->info.lock);
+
+	msg = rpmsg_gpio_msg_init_common(port, line, VIRTIO_GPIO_MSG_SET_DIRECTION);
+	msg->val1 = VIRTIO_GPIO_DIRECTION_OUT;
+
+	ret = rpmsg_gpio_send_message(port, msg, true);
+	if (ret)
+		return ret;
+
+	msg = rpmsg_gpio_msg_init_common(port, line, VIRTIO_GPIO_MSG_SET_VALUE);
+	msg->val1 = val;
+
+	return rpmsg_gpio_send_message(port, msg, true);
+}
+
+static int gpio_rpmsg_irq_set_type(struct irq_data *d, u32 type)
+{
+	struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
+	u32 line = d->hwirq;
+	int ret = 0;
+
+	switch (type) {
+	case IRQ_TYPE_EDGE_RISING:
+		type = VIRTIO_GPIO_IRQ_TYPE_EDGE_RISING;
+		irq_set_handler_locked(d, handle_simple_irq);
+		break;
+	case IRQ_TYPE_EDGE_FALLING:
+		type = VIRTIO_GPIO_IRQ_TYPE_EDGE_FALLING;
+		irq_set_handler_locked(d, handle_simple_irq);
+		break;
+	case IRQ_TYPE_EDGE_BOTH:
+		type = VIRTIO_GPIO_IRQ_TYPE_EDGE_BOTH;
+		irq_set_handler_locked(d, handle_simple_irq);
+		break;
+	case IRQ_TYPE_LEVEL_LOW:
+		type = VIRTIO_GPIO_IRQ_TYPE_LEVEL_LOW;
+		irq_set_handler_locked(d, handle_level_irq);
+		break;
+	case IRQ_TYPE_LEVEL_HIGH:
+		type = VIRTIO_GPIO_IRQ_TYPE_LEVEL_HIGH;
+		irq_set_handler_locked(d, handle_level_irq);
+		break;
+	default:
+		ret = -EINVAL;
+		irq_set_handler_locked(d, handle_bad_irq);
+		break;
+	}
+
+	port->lines[line].irq_type = type;
+
+	return ret;
+}
+
+static int gpio_rpmsg_irq_set_wake(struct irq_data *d, u32 enable)
+{
+	struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
+	u32 line = d->hwirq;
+
+	port->lines[line].irq_wake_enable = enable;
+
+	return 0;
+}
+
+/*
+ * This unmask/mask function is invoked in two situations:
+ *   - when an interrupt is being set up, and
+ *   - after an interrupt has occurred.
+ *
+ * The GPIO driver does not access hardware registers directly.
+ * Instead, it caches all relevant information locally, and then sends
+ * the accumulated state to the remote system at this stage.
+ */
+static void gpio_rpmsg_unmask_irq(struct irq_data *d)
+{
+	struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
+	u32 line = d->hwirq;
+
+	port->lines[line].irq_unmask = 1;
+}
+
+static void gpio_rpmsg_mask_irq(struct irq_data *d)
+{
+	struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
+	u32 line = d->hwirq;
+
+	/*
+	 * When an interrupt occurs, the remote system masks the interrupt
+	 * and then sends a notification to Linux. After Linux processes
+	 * that notification, it sends an RPMsg command back to the remote
+	 * system to unmask the interrupt again.
+	 */
+	port->lines[line].irq_mask = 1;
+}
+
+static void gpio_rpmsg_irq_shutdown(struct irq_data *d)
+{
+	struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
+	u32 line = d->hwirq;
+
+	port->lines[line].irq_shutdown = 1;
+}
+
+static void gpio_rpmsg_irq_bus_lock(struct irq_data *d)
+{
+	struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
+
+	mutex_lock(&port->info.lock);
+}
+
+static void gpio_rpmsg_irq_bus_sync_unlock(struct irq_data *d)
+{
+	struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
+	struct rpmsg_gpio_packet *msg;
+	u32 line = d->hwirq;
+
+	/*
+	 * For mask irq, do nothing here.
+	 * The remote system will mask interrupt after an interrupt occurs,
+	 * and then send a notification to Linux system. After Linux system
+	 * handles the notification, it sends an rpmsg back to the remote
+	 * system to unmask this interrupt again.
+	 */
+	if (port->lines[line].irq_mask && !port->lines[line].irq_unmask) {
+		port->lines[line].irq_mask = 0;
+		mutex_unlock(&port->info.lock);
+		return;
+	}
+
+	msg = rpmsg_gpio_msg_init_common(port, line, VIRTIO_GPIO_MSG_IRQ_TYPE);
+
+	if (port->lines[line].irq_shutdown) {
+		port->lines[line].irq_shutdown = 0;
+		msg->val1 = VIRTIO_GPIO_IRQ_TYPE_NONE;
+		msg->val2 = 0;
+	} else {
+		/* if irq type is not set, use low level trigger as default. */
+		msg->val1 = port->lines[line].irq_type;
+		if (!msg->val1)
+			msg->val1 = VIRTIO_GPIO_IRQ_TYPE_LEVEL_LOW;
+		if (port->lines[line].irq_unmask) {
+			msg->val2 = 0;
+			port->lines[line].irq_unmask = 0;
+		} else /* irq set wake */
+			msg->val2 = port->lines[line].irq_wake_enable;
+	}
+
+	rpmsg_gpio_send_message(port, msg, false);
+	mutex_unlock(&port->info.lock);
+}
+
+static const struct irq_chip gpio_rpmsg_irq_chip = {
+	.irq_mask = gpio_rpmsg_mask_irq,
+	.irq_unmask = gpio_rpmsg_unmask_irq,
+	.irq_set_wake = gpio_rpmsg_irq_set_wake,
+	.irq_set_type = gpio_rpmsg_irq_set_type,
+	.irq_shutdown = gpio_rpmsg_irq_shutdown,
+	.irq_bus_lock = gpio_rpmsg_irq_bus_lock,
+	.irq_bus_sync_unlock = gpio_rpmsg_irq_bus_sync_unlock,
+	.flags = IRQCHIP_IMMUTABLE,
+};
+
+static void rpmsg_gpio_remove_action(void *data)
+{
+	struct rpmsg_gpio_port *port = data;
+
+	port->info.port_store[port->idx] = NULL;
+}
+
+static int rpmsg_gpiochip_register(struct rpmsg_device *rpdev, struct device_node *np)
+{
+	struct rpdev_drvdata *drvdata = dev_get_drvdata(&rpdev->dev);
+	struct rpmsg_gpio_port *port;
+	struct gpio_irq_chip *girq;
+	struct gpio_chip *gc;
+	int ret;
+
+	port = devm_kzalloc(&rpdev->dev, sizeof(*port), GFP_KERNEL);
+	if (!port)
+		return -ENOMEM;
+
+	ret = of_property_read_u32(np, "reg", &port->idx);
+	if (ret)
+		return ret;
+
+	if (port->idx >= MAX_PORT_PER_CHANNEL)
+		return -EINVAL;
+
+	ret = devm_mutex_init(&rpdev->dev, &port->info.lock);
+	if (ret)
+		return ret;
+
+	ret = of_property_read_u32(np, "ngpios", &port->ngpios);
+	if (ret || port->ngpios > GPIOS_PER_PORT_DEFAULT)
+		port->ngpios = GPIOS_PER_PORT_DEFAULT;
+
+	port->info.reply_msg = devm_kzalloc(&rpdev->dev,
+					    sizeof(struct rpmsg_gpio_packet),
+					    GFP_KERNEL);
+	if (!port->info.reply_msg)
+		return -ENOMEM;
+
+	init_completion(&port->info.cmd_complete);
+	port->info.port_store = drvdata->channel_devices;
+	port->info.port_store[port->idx] = port;
+	port->info.rpdev = rpdev;
+
+	gc = &port->gc;
+	gc->owner = THIS_MODULE;
+	gc->parent = &rpdev->dev;
+	gc->fwnode = of_fwnode_handle(np);
+	gc->ngpio = port->ngpios;
+	gc->base = -1;
+	gc->label = devm_kasprintf(&rpdev->dev, GFP_KERNEL, "%s-gpio%d",
+				   drvdata->rproc_name, port->idx);
+
+	gc->direction_input = rpmsg_gpio_direction_input;
+	gc->direction_output = rpmsg_gpio_direction_output;
+	gc->get_direction = rpmsg_gpio_get_direction;
+	gc->get = rpmsg_gpio_get;
+	gc->set = rpmsg_gpio_set;
+
+	girq = &gc->irq;
+	gpio_irq_chip_set_chip(girq, &gpio_rpmsg_irq_chip);
+	girq->parent_handler = NULL;
+	girq->num_parents = 0;
+	girq->parents = NULL;
+	girq->chip->name = devm_kasprintf(&rpdev->dev, GFP_KERNEL, "%s-gpio%d",
+					  drvdata->rproc_name, port->idx);
+
+	ret = devm_add_action_or_reset(&rpdev->dev, rpmsg_gpio_remove_action, port);
+	if (ret)
+		return ret;
+
+	return devm_gpiochip_add_data(&rpdev->dev, gc, port);
+}
+
+static const char *rpmsg_get_rproc_node_name(struct rpmsg_device *rpdev)
+{
+	const char *name = NULL;
+	struct device_node *np;
+	struct rproc *rproc;
+
+	rproc = rproc_get_by_child(&rpdev->dev);
+	if (!rproc)
+		return NULL;
+
+	np = of_node_get(rproc->dev.of_node);
+	if (!np && rproc->dev.parent)
+		np = of_node_get(rproc->dev.parent->of_node);
+
+	if (np) {
+		name = devm_kstrdup(&rpdev->dev, np->name, GFP_KERNEL);
+		of_node_put(np);
+	}
+
+	return name;
+}
+
+static struct device_node *
+rpmsg_get_channel_ofnode(struct rpmsg_device *rpdev, char *chan_name)
+{
+	struct device_node *np_chan = NULL, *np;
+	struct rproc *rproc;
+
+	rproc = rproc_get_by_child(&rpdev->dev);
+	if (!rproc)
+		return NULL;
+
+	np = of_node_get(rproc->dev.of_node);
+	if (!np && rproc->dev.parent)
+		np = of_node_get(rproc->dev.parent->of_node);
+
+	/* The of_node_put() is performed by of_find_node_by_name(). */
+	if (np)
+		np_chan = of_find_node_by_name(np, chan_name);
+
+	return np_chan;
+}
+
+static int rpmsg_gpio_channel_callback(struct rpmsg_device *rpdev, void *data,
+				       int len, void *priv, u32 src)
+{
+	struct rpmsg_gpio_packet *msg = data;
+	struct rpmsg_gpio_port *port = NULL;
+	struct rpdev_drvdata *drvdata;
+
+	drvdata = dev_get_drvdata(&rpdev->dev);
+	if (drvdata && drvdata->protocol_fixed_up)
+		msg = drvdata->protocol_fixed_up->recv_fixed_up(rpdev, data);
+
+	if (!msg || !drvdata)
+		return -EINVAL;
+
+	if (msg->port_idx < MAX_PORT_PER_CHANNEL)
+		port = drvdata->channel_devices[msg->port_idx];
+
+	if (!port || msg->line >= port->ngpios) {
+		dev_err(&rpdev->dev, "wrong port index or line number. port:%d line:%d\n",
+			msg->port_idx, msg->line);
+		return -EINVAL;
+	}
+
+	if (msg->type == GPIO_RPMSG_REPLY) {
+		*port->info.reply_msg = *msg;
+		complete(&port->info.cmd_complete);
+	} else if (msg->type == GPIO_RPMSG_NOTIFY) {
+		generic_handle_domain_irq_safe(port->gc.irq.domain, msg->line);
+	} else {
+		dev_err(&rpdev->dev, "wrong command type (0x%x)\n", msg->type);
+	}
+
+	return 0;
+}
+
+static int rpmsg_gpio_channel_probe(struct rpmsg_device *rpdev)
+{
+	struct device *dev = &rpdev->dev;
+	struct rpdev_drvdata *drvdata;
+	struct device_node *np;
+	int ret = -ENODEV;
+
+	if (!dev->of_node) {
+		np = rpmsg_get_channel_ofnode(rpdev, rpdev->id.name);
+		if (np) {
+			dev->of_node = np;
+			set_primary_fwnode(dev, of_fwnode_handle(np));
+		}
+		return -EPROBE_DEFER;
+	}
+
+	drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
+	if (!drvdata)
+		return -ENOMEM;
+
+	drvdata->rproc_name = rpmsg_get_rproc_node_name(rpdev);
+	drvdata->protocol_fixed_up = (struct rpmsg_gpio_fixed_up *)rpdev->id.driver_data;
+	dev_set_drvdata(dev, drvdata);
+
+	for_each_child_of_node_scoped(dev->of_node, child) {
+		if (!of_device_is_available(child))
+			continue;
+
+		if (!of_match_node(dev->driver->of_match_table, child))
+			continue;
+
+		ret = rpmsg_gpiochip_register(rpdev, child);
+		if (ret < 0)
+			break;
+	}
+
+	return ret;
+}
+
+static const struct of_device_id rpmsg_gpio_dt_ids[] = {
+	{ .compatible = "rpmsg-gpio" },
+	{ /* sentinel */ }
+};
+
+static struct rpmsg_device_id rpmsg_gpio_channel_id_table[] = {
+	{ .name = "rpmsg-io" },
+	{ },
+};
+MODULE_DEVICE_TABLE(rpmsg, rpmsg_gpio_channel_id_table);
+
+static struct rpmsg_driver rpmsg_gpio_channel_client = {
+	.callback	= rpmsg_gpio_channel_callback,
+	.id_table	= rpmsg_gpio_channel_id_table,
+	.probe		= rpmsg_gpio_channel_probe,
+	.drv		= {
+		.name	= KBUILD_MODNAME,
+		.of_match_table = rpmsg_gpio_dt_ids,
+	},
+};
+module_rpmsg_driver(rpmsg_gpio_channel_client);
+
+MODULE_AUTHOR("Shenwei Wang <shenwei.wang@nxp.com>");
+MODULE_DESCRIPTION("generic rpmsg gpio driver");
+MODULE_LICENSE("GPL");
-- 
2.43.0

Re: [PATCH v12 3/5] gpio: rpmsg: add generic rpmsg GPIO driver

[Arnaud POULIQUEN]

Hello,

On 3/13/26 20:57, Shenwei Wang wrote:
> On an AMP platform, the system may include two processors:
> 	- An MCU running an RTOS
> 	- An MPU running Linux
> 
> These processors communicate via the RPMSG protocol.
> The driver implements the standard GPIO interface, allowing
> the Linux side to control GPIO controllers which reside in
> the remote processor via RPMSG protocol.
> 
> Cc: Bartosz Golaszewski <brgl@bgdev.pl>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
> ---
>   drivers/gpio/Kconfig      |  17 ++
>   drivers/gpio/Makefile     |   1 +
>   drivers/gpio/gpio-rpmsg.c | 596 ++++++++++++++++++++++++++++++++++++++
>   3 files changed, 614 insertions(+)
>   create mode 100644 drivers/gpio/gpio-rpmsg.c
> 
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index b45fb799e36c..cff0fda8a283 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -1892,6 +1892,23 @@ config GPIO_SODAVILLE
>   
>   endmenu
>   
> +menu "RPMSG GPIO drivers"
> +	depends on RPMSG
> +
> +config GPIO_RPMSG
> +	tristate "Generic RPMSG GPIO support"
> +	depends on OF && REMOTEPROC
> +	select GPIOLIB_IRQCHIP
> +	default REMOTEPROC
> +	help
> +	  Say yes here to support the generic GPIO functions over the RPMSG
> +	  bus. Currently supported devices: i.MX7ULP, i.MX8ULP, i.MX8x, and
> +	  i.MX9x.
> +
> +	  If unsure, say N.
> +
> +endmenu
> +
>   menu "SPI GPIO expanders"
>   	depends on SPI_MASTER
>   
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index c05f7d795c43..501aba56ad68 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -158,6 +158,7 @@ obj-$(CONFIG_GPIO_RDC321X)		+= gpio-rdc321x.o
>   obj-$(CONFIG_GPIO_REALTEK_OTTO)		+= gpio-realtek-otto.o
>   obj-$(CONFIG_GPIO_REG)			+= gpio-reg.o
>   obj-$(CONFIG_GPIO_ROCKCHIP)	+= gpio-rockchip.o
> +obj-$(CONFIG_GPIO_RPMSG)		+= gpio-rpmsg.o
>   obj-$(CONFIG_GPIO_RTD)			+= gpio-rtd.o
>   obj-$(CONFIG_ARCH_SA1100)		+= gpio-sa1100.o
>   obj-$(CONFIG_GPIO_SAMA5D2_PIOBU)	+= gpio-sama5d2-piobu.o
> diff --git a/drivers/gpio/gpio-rpmsg.c b/drivers/gpio/gpio-rpmsg.c
> new file mode 100644
> index 000000000000..9c609b55bc14
> --- /dev/null
> +++ b/drivers/gpio/gpio-rpmsg.c
> @@ -0,0 +1,596 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright 2026 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/of_device.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/remoteproc.h>
> +#include <linux/rpmsg.h>
> +#include <linux/virtio_gpio.h>

you still needed to include virtio_gpio.h?

> +
> +#define MAX_PORT_PER_CHANNEL    10
> +#define GPIOS_PER_PORT_DEFAULT	32
> +#define RPMSG_TIMEOUT		1000

I wonder if the timeout is not too high. but
not blocking.

> +
> +/* GPIO RPMSG Type */
> +#define GPIO_RPMSG_SEND		0
> +#define GPIO_RPMSG_REPLY	1
> +#define GPIO_RPMSG_NOTIFY	2
> +
> +struct rpmsg_gpio_packet {
> +	u8 type;	/* Message type */
> +	u8 cmd;		/* Command code */
> +	u8 port_idx;
> +	u8 line;
> +	u8 val1;
> +	u8 val2;
> +};
> +
> +struct rpmsg_gpio_line {
> +	u8 irq_shutdown;
> +	u8 irq_unmask;
> +	u8 irq_mask;
> +	u32 irq_wake_enable;
> +	u32 irq_type;
> +	struct rpmsg_gpio_packet msg;
> +};
> +
> +struct rpmsg_gpio_info {
> +	struct rpmsg_device *rpdev;
> +	struct rpmsg_gpio_packet *reply_msg;
> +	struct completion cmd_complete;
> +	struct mutex lock;
> +	void **port_store;
> +};

Except if I missunderstood Mathieu and Bjorn's request:
"reuse all the design-work done in the gpio-virtio"
We should find similar structures here to those defined
in virtio_gpio.h.
struct rpmsg_gpio_config {
	__le16 ngpio;
	__u8 padding[2];
	__le32 gpio_names_size;
};

/* Virtio GPIO Request / Response */
struct virtio_gpio_request {
	__le16 type;
	__le16 gpio;
	__le32 value;
};

struct rpmsg_gpio_response {
	__u8 status;
	__u8 value;
};

struct rpmsg_gpio_response_get_names {
	__u8 status;
	__u8 value[];
};

/* Virtio GPIO IRQ Request / Response */
struct rpmsg_gpio_irq_request {
	__le16 gpio;
};

struct rpmsg_gpio_irq_response {
	__u8 status;
};

> +
> +struct rpmsg_gpio_port {
> +	struct gpio_chip gc;
> +	struct rpmsg_gpio_line lines[GPIOS_PER_PORT_DEFAULT];
> +	struct rpmsg_gpio_info info;
> +	u32 ngpios;
> +	u32 idx;
> +};
> +
> +struct rpmsg_gpio_fixed_up {
> +	int (*send_fixed_up)(struct rpmsg_gpio_info *info, struct rpmsg_gpio_packet *msg);
> +	struct rpmsg_gpio_packet *(*recv_fixed_up)(struct rpmsg_device *rpdev, void *data);
> +};
> +
> +/*
> + * @rproc_name: the name of the remote proc.
> + * @recv_pkt: a pointer to the received packet for protocol fix up.
> + * @protocol_fixed_up: optional callbacks to handle protocol mismatches.
> + * @channel_devices: an array of the devices related to the rpdev.
> + */
> +struct rpdev_drvdata {
> +	const char *rproc_name;
> +	void *recv_pkt;
> +	struct rpmsg_gpio_fixed_up *protocol_fixed_up;
> +	void *channel_devices[MAX_PORT_PER_CHANNEL];
> +};
> +
> +static int rpmsg_gpio_send_message(struct rpmsg_gpio_port *port,
> +				   struct rpmsg_gpio_packet *msg,
> +				   bool sync)
> +{
> +	struct rpmsg_gpio_info *info = &port->info;
> +	struct rpdev_drvdata *drvdata;
> +	int ret;
> +
> +	drvdata = dev_get_drvdata(&info->rpdev->dev);
> +	reinit_completion(&info->cmd_complete);
> +
> +	if (drvdata->protocol_fixed_up)
> +		ret = drvdata->protocol_fixed_up->send_fixed_up(info, msg);

Seems not part of a generic implementation

> +	else
> +		ret = rpmsg_send(info->rpdev->ept, msg, sizeof(*msg));
> +
> +	if (ret) {
> +		dev_err(&info->rpdev->dev, "rpmsg_send failed: %d\n", ret);
> +		return ret;
> +	}
> +
> +	if (sync) {
> +		ret = wait_for_completion_timeout(&info->cmd_complete,
> +						  msecs_to_jiffies(RPMSG_TIMEOUT));
> +		if (ret == 0) {
> +			dev_err(&info->rpdev->dev, "rpmsg_send timeout!\n");
> +			return -ETIMEDOUT;
> +		}
> +
> +		if (info->reply_msg->val1 != 0) {
> +			dev_err(&info->rpdev->dev, "remote core replies an error: %d!\n",
> +				info->reply_msg->val1);
> +			return -EINVAL;
> +		}
> +
> +		/* copy the reply message */
> +		memcpy(&port->lines[info->reply_msg->line].msg,
> +		       info->reply_msg, sizeof(*info->reply_msg));
> +	}
> +
> +	return 0;
> +}
> +
> +static struct rpmsg_gpio_packet *
> +rpmsg_gpio_msg_init_common(struct rpmsg_gpio_port *port, unsigned int line, u8 cmd)
> +{
> +	struct rpmsg_gpio_packet *msg = &port->lines[line].msg;
> +
> +	memset(msg, 0, sizeof(struct rpmsg_gpio_packet));
> +	msg->type = GPIO_RPMSG_SEND;
> +	msg->cmd = cmd;
> +	msg->port_idx = port->idx;
> +	msg->line = line;
> +
> +	return msg;
> +}
> +
> +static int rpmsg_gpio_get(struct gpio_chip *gc, unsigned int line)
> +{
> +	struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
> +	struct rpmsg_gpio_packet *msg;
> +	int ret;
> +
> +	guard(mutex)(&port->info.lock);
> +
> +	msg = rpmsg_gpio_msg_init_common(port, line, VIRTIO_GPIO_MSG_GET_VALUE);
> +
> +	ret = rpmsg_gpio_send_message(port, msg, true);
> +	if (!ret)
> +		ret = !!port->lines[line].msg.val2;
> +
> +	return ret;
> +}
> +
> +static int rpmsg_gpio_get_direction(struct gpio_chip *gc, unsigned int line)
> +{
> +	struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
> +	struct rpmsg_gpio_packet *msg;
> +	int ret;
> +
> +	guard(mutex)(&port->info.lock);
> +
> +	msg = rpmsg_gpio_msg_init_common(port, line, VIRTIO_GPIO_MSG_GET_DIRECTION);
> +
> +	ret = rpmsg_gpio_send_message(port, msg, true);
> +	if (ret)
> +		return ret;
> +
> +	switch (port->lines[line].msg.val2) {
> +	case VIRTIO_GPIO_DIRECTION_IN:
> +		return GPIO_LINE_DIRECTION_IN;
> +	case VIRTIO_GPIO_DIRECTION_OUT:
> +		return GPIO_LINE_DIRECTION_OUT;
> +	default:
> +		break;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int rpmsg_gpio_direction_input(struct gpio_chip *gc, unsigned int line)
> +{
> +	struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
> +	struct rpmsg_gpio_packet *msg;
> +
> +	guard(mutex)(&port->info.lock);
> +
> +	msg = rpmsg_gpio_msg_init_common(port, line, VIRTIO_GPIO_MSG_SET_DIRECTION);
> +	msg->val1 = VIRTIO_GPIO_DIRECTION_IN;
> +
> +	return rpmsg_gpio_send_message(port, msg, true);
> +}
> +
> +static int rpmsg_gpio_set(struct gpio_chip *gc, unsigned int line, int val)
> +{
> +	struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
> +	struct rpmsg_gpio_packet *msg;
> +
> +	guard(mutex)(&port->info.lock);
> +
> +	msg = rpmsg_gpio_msg_init_common(port, line, VIRTIO_GPIO_MSG_SET_VALUE);
> +	msg->val1 = val;
> +
> +	return rpmsg_gpio_send_message(port, msg, true);
> +}
> +
> +static int rpmsg_gpio_direction_output(struct gpio_chip *gc, unsigned int line, int val)
> +{
> +	struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
> +	struct rpmsg_gpio_packet *msg;
> +	int ret;
> +
> +	guard(mutex)(&port->info.lock);
> +
> +	msg = rpmsg_gpio_msg_init_common(port, line, VIRTIO_GPIO_MSG_SET_DIRECTION);
> +	msg->val1 = VIRTIO_GPIO_DIRECTION_OUT;
> +
> +	ret = rpmsg_gpio_send_message(port, msg, true);
> +	if (ret)
> +		return ret;
> +
> +	msg = rpmsg_gpio_msg_init_common(port, line, VIRTIO_GPIO_MSG_SET_VALUE);
> +	msg->val1 = val;
> +
> +	return rpmsg_gpio_send_message(port, msg, true);
> +}
> +
> +static int gpio_rpmsg_irq_set_type(struct irq_data *d, u32 type)
> +{
> +	struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> +	u32 line = d->hwirq;
> +	int ret = 0;
> +
> +	switch (type) {
> +	case IRQ_TYPE_EDGE_RISING:
> +		type = VIRTIO_GPIO_IRQ_TYPE_EDGE_RISING;
> +		irq_set_handler_locked(d, handle_simple_irq);
> +		break;
> +	case IRQ_TYPE_EDGE_FALLING:
> +		type = VIRTIO_GPIO_IRQ_TYPE_EDGE_FALLING;
> +		irq_set_handler_locked(d, handle_simple_irq);
> +		break;
> +	case IRQ_TYPE_EDGE_BOTH:
> +		type = VIRTIO_GPIO_IRQ_TYPE_EDGE_BOTH;
> +		irq_set_handler_locked(d, handle_simple_irq);
> +		break;
> +	case IRQ_TYPE_LEVEL_LOW:
> +		type = VIRTIO_GPIO_IRQ_TYPE_LEVEL_LOW;
> +		irq_set_handler_locked(d, handle_level_irq);
> +		break;
> +	case IRQ_TYPE_LEVEL_HIGH:
> +		type = VIRTIO_GPIO_IRQ_TYPE_LEVEL_HIGH;
> +		irq_set_handler_locked(d, handle_level_irq);
> +		break;
> +	default:
> +		ret = -EINVAL;
> +		irq_set_handler_locked(d, handle_bad_irq);
> +		break;
> +	}
> +
> +	port->lines[line].irq_type = type;
> +
> +	return ret;
> +}
> +
> +static int gpio_rpmsg_irq_set_wake(struct irq_data *d, u32 enable)
> +{
> +	struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> +	u32 line = d->hwirq;
> +
> +	port->lines[line].irq_wake_enable = enable;
> +
> +	return 0;
> +}
> +
> +/*
> + * This unmask/mask function is invoked in two situations:
> + *   - when an interrupt is being set up, and
> + *   - after an interrupt has occurred.
> + *
> + * The GPIO driver does not access hardware registers directly.
> + * Instead, it caches all relevant information locally, and then sends
> + * the accumulated state to the remote system at this stage.
> + */
> +static void gpio_rpmsg_unmask_irq(struct irq_data *d)
> +{
> +	struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> +	u32 line = d->hwirq;
> +
> +	port->lines[line].irq_unmask = 1;
> +}
> +
> +static void gpio_rpmsg_mask_irq(struct irq_data *d)
> +{
> +	struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> +	u32 line = d->hwirq;
> +
> +	/*
> +	 * When an interrupt occurs, the remote system masks the interrupt
> +	 * and then sends a notification to Linux. After Linux processes
> +	 * that notification, it sends an RPMsg command back to the remote
> +	 * system to unmask the interrupt again.
> +	 */
> +	port->lines[line].irq_mask = 1;
> +}
> +
> +static void gpio_rpmsg_irq_shutdown(struct irq_data *d)
> +{
> +	struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> +	u32 line = d->hwirq;
> +
> +	port->lines[line].irq_shutdown = 1;
> +}
> +
> +static void gpio_rpmsg_irq_bus_lock(struct irq_data *d)
> +{
> +	struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> +
> +	mutex_lock(&port->info.lock);
> +}
> +
> +static void gpio_rpmsg_irq_bus_sync_unlock(struct irq_data *d)
> +{
> +	struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> +	struct rpmsg_gpio_packet *msg;
> +	u32 line = d->hwirq;
> +
> +	/*
> +	 * For mask irq, do nothing here.
> +	 * The remote system will mask interrupt after an interrupt occurs,
> +	 * and then send a notification to Linux system. After Linux system
> +	 * handles the notification, it sends an rpmsg back to the remote
> +	 * system to unmask this interrupt again.
> +	 */
> +	if (port->lines[line].irq_mask && !port->lines[line].irq_unmask) {
> +		port->lines[line].irq_mask = 0;
> +		mutex_unlock(&port->info.lock);
> +		return;
> +	}
> +
> +	msg = rpmsg_gpio_msg_init_common(port, line, VIRTIO_GPIO_MSG_IRQ_TYPE);
> +
> +	if (port->lines[line].irq_shutdown) {
> +		port->lines[line].irq_shutdown = 0;
> +		msg->val1 = VIRTIO_GPIO_IRQ_TYPE_NONE;
> +		msg->val2 = 0;
> +	} else {
> +		/* if irq type is not set, use low level trigger as default. */
> +		msg->val1 = port->lines[line].irq_type;
> +		if (!msg->val1)
> +			msg->val1 = VIRTIO_GPIO_IRQ_TYPE_LEVEL_LOW;
> +		if (port->lines[line].irq_unmask) {
> +			msg->val2 = 0;
> +			port->lines[line].irq_unmask = 0;
> +		} else /* irq set wake */
> +			msg->val2 = port->lines[line].irq_wake_enable;
> +	}
> +
> +	rpmsg_gpio_send_message(port, msg, false);
> +	mutex_unlock(&port->info.lock);
> +}
> +
> +static const struct irq_chip gpio_rpmsg_irq_chip = {
> +	.irq_mask = gpio_rpmsg_mask_irq,
> +	.irq_unmask = gpio_rpmsg_unmask_irq,
> +	.irq_set_wake = gpio_rpmsg_irq_set_wake,
> +	.irq_set_type = gpio_rpmsg_irq_set_type,
> +	.irq_shutdown = gpio_rpmsg_irq_shutdown,
> +	.irq_bus_lock = gpio_rpmsg_irq_bus_lock,
> +	.irq_bus_sync_unlock = gpio_rpmsg_irq_bus_sync_unlock,
> +	.flags = IRQCHIP_IMMUTABLE,
> +};
> +
> +static void rpmsg_gpio_remove_action(void *data)
> +{
> +	struct rpmsg_gpio_port *port = data;
> +
> +	port->info.port_store[port->idx] = NULL;
> +}
> +
> +static int rpmsg_gpiochip_register(struct rpmsg_device *rpdev, struct device_node *np)
> +{
> +	struct rpdev_drvdata *drvdata = dev_get_drvdata(&rpdev->dev);
> +	struct rpmsg_gpio_port *port;
> +	struct gpio_irq_chip *girq;
> +	struct gpio_chip *gc;
> +	int ret;
> +
> +	port = devm_kzalloc(&rpdev->dev, sizeof(*port), GFP_KERNEL);
> +	if (!port)
> +		return -ENOMEM;
> +
> +	ret = of_property_read_u32(np, "reg", &port->idx);
> +	if (ret)
> +		return ret;
> +
> +	if (port->idx >= MAX_PORT_PER_CHANNEL)
> +		return -EINVAL;
> +
> +	ret = devm_mutex_init(&rpdev->dev, &port->info.lock);
> +	if (ret)
> +		return ret;
> +
> +	ret = of_property_read_u32(np, "ngpios", &port->ngpios);

The number of GPIOs should be obtained from the remote side, as done in 
virtio_gpio. In virtio_gpio, this is retrieved via a get_config 
operation. Here, you could implement a specific RPMsg to retrieve the 
remote topology.

> +	if (ret || port->ngpios > GPIOS_PER_PORT_DEFAULT)
> +		port->ngpios = GPIOS_PER_PORT_DEFAULT;
> +
> +	port->info.reply_msg = devm_kzalloc(&rpdev->dev,
> +					    sizeof(struct rpmsg_gpio_packet),
> +					    GFP_KERNEL);
> +	if (!port->info.reply_msg)
> +		return -ENOMEM;
> +
> +	init_completion(&port->info.cmd_complete);
> +	port->info.port_store = drvdata->channel_devices;
> +	port->info.port_store[port->idx] = port;
> +	port->info.rpdev = rpdev;
> +
> +	gc = &port->gc;
> +	gc->owner = THIS_MODULE;
> +	gc->parent = &rpdev->dev;
> +	gc->fwnode = of_fwnode_handle(np);
> +	gc->ngpio = port->ngpios;
> +	gc->base = -1;
> +	gc->label = devm_kasprintf(&rpdev->dev, GFP_KERNEL, "%s-gpio%d",
> +				   drvdata->rproc_name, port->idx);
> +
> +	gc->direction_input = rpmsg_gpio_direction_input;
> +	gc->direction_output = rpmsg_gpio_direction_output;
> +	gc->get_direction = rpmsg_gpio_get_direction;
> +	gc->get = rpmsg_gpio_get;
> +	gc->set = rpmsg_gpio_set;
> +
> +	girq = &gc->irq;
> +	gpio_irq_chip_set_chip(girq, &gpio_rpmsg_irq_chip);
> +	girq->parent_handler = NULL;
> +	girq->num_parents = 0;
> +	girq->parents = NULL;
> +	girq->chip->name = devm_kasprintf(&rpdev->dev, GFP_KERNEL, "%s-gpio%d",
> +					  drvdata->rproc_name, port->idx);
> +
> +	ret = devm_add_action_or_reset(&rpdev->dev, rpmsg_gpio_remove_action, port);
> +	if (ret)
> +		return ret;
> +
> +	return devm_gpiochip_add_data(&rpdev->dev, gc, port);
> +}
> +
> +static const char *rpmsg_get_rproc_node_name(struct rpmsg_device *rpdev)
> +{
> +	const char *name = NULL;
> +	struct device_node *np;
> +	struct rproc *rproc;
> +
> +	rproc = rproc_get_by_child(&rpdev->dev);
> +	if (!rproc)
> +		return NULL;
> +
> +	np = of_node_get(rproc->dev.of_node);
> +	if (!np && rproc->dev.parent)
> +		np = of_node_get(rproc->dev.parent->of_node);
> +
> +	if (np) {
> +		name = devm_kstrdup(&rpdev->dev, np->name, GFP_KERNEL);
> +		of_node_put(np);
> +	}
> +
> +	return name;
> +}
> +
> +static struct device_node *
> +rpmsg_get_channel_ofnode(struct rpmsg_device *rpdev, char *chan_name)
> +{
> +	struct device_node *np_chan = NULL, *np;
> +	struct rproc *rproc;
> +
> +	rproc = rproc_get_by_child(&rpdev->dev);
> +	if (!rproc)
> +		return NULL;
> +
> +	np = of_node_get(rproc->dev.of_node);
> +	if (!np && rproc->dev.parent)
> +		np = of_node_get(rproc->dev.parent->of_node);
> +
> +	/* The of_node_put() is performed by of_find_node_by_name(). */
> +	if (np)
> +		np_chan = of_find_node_by_name(np, chan_name);
> +
> +	return np_chan;
> +}
> +
> +static int rpmsg_gpio_channel_callback(struct rpmsg_device *rpdev, void *data,
> +				       int len, void *priv, u32 src)
> +{
> +	struct rpmsg_gpio_packet *msg = data;
> +	struct rpmsg_gpio_port *port = NULL;
> +	struct rpdev_drvdata *drvdata;
> +
> +	drvdata = dev_get_drvdata(&rpdev->dev);
> +	if (drvdata && drvdata->protocol_fixed_up)
> +		msg = drvdata->protocol_fixed_up->recv_fixed_up(rpdev, data);
> +
> +	if (!msg || !drvdata)
> +		return -EINVAL;
> +
> +	if (msg->port_idx < MAX_PORT_PER_CHANNEL)
> +		port = drvdata->channel_devices[msg->port_idx];
> +
> +	if (!port || msg->line >= port->ngpios) {
> +		dev_err(&rpdev->dev, "wrong port index or line number. port:%d line:%d\n",
> +			msg->port_idx, msg->line);
> +		return -EINVAL;
> +	}
> +
> +	if (msg->type == GPIO_RPMSG_REPLY) {
> +		*port->info.reply_msg = *msg;
> +		complete(&port->info.cmd_complete);
> +	} else if (msg->type == GPIO_RPMSG_NOTIFY) {
> +		generic_handle_domain_irq_safe(port->gc.irq.domain, msg->line);
> +	} else {
> +		dev_err(&rpdev->dev, "wrong command type (0x%x)\n", msg->type);
> +	}
> +
> +	return 0;
> +}
> +
> +static int rpmsg_gpio_channel_probe(struct rpmsg_device *rpdev)
> +{
> +	struct device *dev = &rpdev->dev;
> +	struct rpdev_drvdata *drvdata;
> +	struct device_node *np;
> +	int ret = -ENODEV;
> +
> +	if (!dev->of_node) {
> +		np = rpmsg_get_channel_ofnode(rpdev, rpdev->id.name);
> +		if (np) {
> +			dev->of_node = np;
> +			set_primary_fwnode(dev, of_fwnode_handle(np));
> +		}
> +		return -EPROBE_DEFER;

Here is it a bug ? else you should explain in a comment why you perform 
some actions when np != 0 but return -EPROBE_DEFER

Regards,
Arnaud

> +	}
> +
> +	drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
> +	if (!drvdata)
> +		return -ENOMEM;
> +
> +	drvdata->rproc_name = rpmsg_get_rproc_node_name(rpdev);
> +	drvdata->protocol_fixed_up = (struct rpmsg_gpio_fixed_up *)rpdev->id.driver_data;
> +	dev_set_drvdata(dev, drvdata);
> +
> +	for_each_child_of_node_scoped(dev->of_node, child) {
> +		if (!of_device_is_available(child))
> +			continue;
> +
> +		if (!of_match_node(dev->driver->of_match_table, child))
> +			continue;
> +
> +		ret = rpmsg_gpiochip_register(rpdev, child);
> +		if (ret < 0)
> +			break;
> +	}
> +
> +	return ret;
> +}
> +
> +static const struct of_device_id rpmsg_gpio_dt_ids[] = {
> +	{ .compatible = "rpmsg-gpio" },
> +	{ /* sentinel */ }
> +};
> +
> +static struct rpmsg_device_id rpmsg_gpio_channel_id_table[] = {
> +	{ .name = "rpmsg-io" },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(rpmsg, rpmsg_gpio_channel_id_table);
> +
> +static struct rpmsg_driver rpmsg_gpio_channel_client = {
> +	.callback	= rpmsg_gpio_channel_callback,
> +	.id_table	= rpmsg_gpio_channel_id_table,
> +	.probe		= rpmsg_gpio_channel_probe,
> +	.drv		= {
> +		.name	= KBUILD_MODNAME,
> +		.of_match_table = rpmsg_gpio_dt_ids,
> +	},
> +};
> +module_rpmsg_driver(rpmsg_gpio_channel_client);
> +
> +MODULE_AUTHOR("Shenwei Wang <shenwei.wang@nxp.com>");
> +MODULE_DESCRIPTION("generic rpmsg gpio driver");
> +MODULE_LICENSE("GPL");

Re: [PATCH v12 3/5] gpio: rpmsg: add generic rpmsg GPIO driver

[Andrew Lunn]

> > +struct rpmsg_gpio_info {
> > +	struct rpmsg_device *rpdev;
> > +	struct rpmsg_gpio_packet *reply_msg;
> > +	struct completion cmd_complete;
> > +	struct mutex lock;
> > +	void **port_store;
> > +};
> 
> Except if I missunderstood Mathieu and Bjorn's request:
> "reuse all the design-work done in the gpio-virtio"
> We should find similar structures here to those defined
> in virtio_gpio.h.
> struct rpmsg_gpio_config {
> 	__le16 ngpio;
> 	__u8 padding[2];
> 	__le32 gpio_names_size;
> };
> 
> /* Virtio GPIO Request / Response */
> struct virtio_gpio_request {
> 	__le16 type;
> 	__le16 gpio;
> 	__le32 value;
> };

The core of the issue is that Shenwei is stone walling any change
which makes it hard to keep the legacy firmware. It is possible to use
these structures, but it makes the extra code Shenwei needs to
translate this protocol to the legacy protocol more difficult. It
might need to keep state, etc. 

Two points...

The firmware implements more than GPIO. There is definitely I2C as
well, the first version of the patch has bits of I2C code. Looking at:

https://lwn.net/ml/all/20250922200413.309707-3-shenwei.wang@nxp.com/

There is also RTC, and a few other things which don't directly map to
Linux subsystems, but maybe do have Linux drivers?

Give how much pushback there has been on the existing protocol for
GPIO, it would be wise to assume that I2C, and RTC is going to get the
same amount of pushback. If any of these three, GPIO, I2C, or RTC
decide that only a new, clean protocol will be accepted, no legacy
shims, the firmware has to change, breaking compatibility to legacy
protocols, and the accepted shims become pointless Maintenance burden.

Point two is that the customers who are pushing for these drivers to
be added to Mainline probably know that nearly nothing gets into
Mainline without some changes. There is some short term pain to
swapping to Mainline because of these changes, in this case, firmware
upgrades. But in the long run, it is worth the pain to be able to use
Mainline. And those customers who don't want to upgrade the firmware
can keep with the out of tree drives.

So, what are our choices?

1) We accept the code as it is now, with the shim?

2) We keep pushing for the virtio protocol, with the shim?

3) We keep pushing for the virtio protocol, no shim, firmware changes

4) We pause GPIO where it is today, and restart all the arguments with
   the I2C driver. We can come back to the GPIO driver in a few months
   time once we have a better idea how I2C is going. And maybe we also
   need to see the watchdog driver, and argue about its protocol.

I also understand ST has a generic I2C driver nearly ready, if that
gets merged first, that probably kills the NXP I2C protocol, and maybe
the NXP GPIO and RTC protocols.

My vote is for 3. If not 3, then 4.

     Andrew

Re: [PATCH v12 3/5] gpio: rpmsg: add generic rpmsg GPIO driver

[Shenwei Wang]

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Tuesday, March 17, 2026 9:12 AM
> To: Arnaud POULIQUEN <arnaud.pouliquen@foss.st.com>
> Cc: Shenwei Wang <shenwei.wang@nxp.com>; Linus Walleij
> <linusw@kernel.org>; Bartosz Golaszewski <brgl@kernel.org>; Jonathan Corbet
> <corbet@lwn.net>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski
> <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Bjorn Andersson
> <andersson@kernel.org>; Mathieu Poirier <mathieu.poirier@linaro.org>; Frank Li
> <frank.li@nxp.com>; Sascha Hauer <s.hauer@pengutronix.de>; Shuah Khan
> <skhan@linuxfoundation.org>; linux-gpio@vger.kernel.org; linux-
> doc@vger.kernel.org; linux-kernel@vger.kernel.org; Pengutronix Kernel Team
> <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; Peng Fan
> <peng.fan@nxp.com>; devicetree@vger.kernel.org; linux-
> remoteproc@vger.kernel.org; imx@lists.linux.dev; linux-arm-
> kernel@lists.infradead.org; dl-linux-imx <linux-imx@nxp.com>; Bartosz
> Golaszewski <brgl@bgdev.pl>
> Subject: [EXT] Re: [PATCH v12 3/5] gpio: rpmsg: add generic rpmsg GPIO driver
> > > +struct rpmsg_gpio_info {
> > > +   struct rpmsg_device *rpdev;
> > > +   struct rpmsg_gpio_packet *reply_msg;
> > > +   struct completion cmd_complete;
> > > +   struct mutex lock;
> > > +   void **port_store;
> > > +};
> >
> > Except if I missunderstood Mathieu and Bjorn's request:
> > "reuse all the design-work done in the gpio-virtio"
> > We should find similar structures here to those defined in
> > virtio_gpio.h.
> > struct rpmsg_gpio_config {
> >       __le16 ngpio;
> >       __u8 padding[2];
> >       __le32 gpio_names_size;
> > };
> >
> > /* Virtio GPIO Request / Response */
> > struct virtio_gpio_request {
> >       __le16 type;
> >       __le16 gpio;
> >       __le32 value;
> > };
> 
> The core of the issue is that Shenwei is stone walling any change which makes it
> hard to keep the legacy firmware. It is possible to use these structures, but it
> makes the extra code Shenwei needs to translate this protocol to the legacy
> protocol more difficult. It might need to keep state, etc.
> 

I’m fully open to reasonable changes, but duplicating these structures is not helpful.
The whole point of aligning with gpio‑virtio is to keep the low‑level command and information 
exchange identical, so the behavior on both sides could remain consistent. This makes it 
possible to reuse the backend implementation on the other side easily.

> Two points...
> 
> The firmware implements more than GPIO. There is definitely I2C as well, the
> first version of the patch has bits of I2C code. Looking at:
> 

Please keep the discussion focused on the GPIO interface.
In the current implementation, there is nothing beyond GPIO, and you will not find 
any information or indication of other interfaces such as I2C here.

Shenwei

> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flwn.net%2
> Fml%2Fall%2F20250922200413.309707-3-
> shenwei.wang%40nxp.com%2F&data=05%7C02%7Cshenwei.wang%40nxp.com
> %7C249345db2cda4e35e09c08de842f2ac2%7C686ea1d3bc2b4c6fa92cd99c5c30
> 1635%7C0%7C0%7C639093535285629301%7CUnknown%7CTWFpbGZsb3d8eyJ
> FbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWF
> pbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=NlmSowKXXGqdOtAKEWXRz
> AdQZaU19yJtyJyfIm%2B8ZQ0%3D&reserved=0
> 
> There is also RTC, and a few other things which don't directly map to Linux
> subsystems, but maybe do have Linux drivers?
> 
> Give how much pushback there has been on the existing protocol for GPIO, it
> would be wise to assume that I2C, and RTC is going to get the same amount of
> pushback. If any of these three, GPIO, I2C, or RTC decide that only a new, clean
> protocol will be accepted, no legacy shims, the firmware has to change, breaking
> compatibility to legacy protocols, and the accepted shims become pointless
> Maintenance burden.
> 
> Point two is that the customers who are pushing for these drivers to be added to
> Mainline probably know that nearly nothing gets into Mainline without some
> changes. There is some short term pain to swapping to Mainline because of these
> changes, in this case, firmware upgrades. But in the long run, it is worth the pain
> to be able to use Mainline. And those customers who don't want to upgrade the
> firmware can keep with the out of tree drives.
> 
> So, what are our choices?
> 
> 1) We accept the code as it is now, with the shim?
> 
> 2) We keep pushing for the virtio protocol, with the shim?
> 
> 3) We keep pushing for the virtio protocol, no shim, firmware changes
> 
> 4) We pause GPIO where it is today, and restart all the arguments with
>    the I2C driver. We can come back to the GPIO driver in a few months
>    time once we have a better idea how I2C is going. And maybe we also
>    need to see the watchdog driver, and argue about its protocol.
> 
> I also understand ST has a generic I2C driver nearly ready, if that gets merged
> first, that probably kills the NXP I2C protocol, and maybe the NXP GPIO and RTC
> protocols.
> 
> My vote is for 3. If not 3, then 4.
> 
>      Andrew

Re: [PATCH v12 3/5] gpio: rpmsg: add generic rpmsg GPIO driver

[Mathieu Poirier]

On Tue, 17 Mar 2026 at 08:11, Andrew Lunn <andrew@lunn.ch> wrote:
>
> > > +struct rpmsg_gpio_info {
> > > +   struct rpmsg_device *rpdev;
> > > +   struct rpmsg_gpio_packet *reply_msg;
> > > +   struct completion cmd_complete;
> > > +   struct mutex lock;
> > > +   void **port_store;
> > > +};
> >
> > Except if I missunderstood Mathieu and Bjorn's request:
> > "reuse all the design-work done in the gpio-virtio"
> > We should find similar structures here to those defined
> > in virtio_gpio.h.
> > struct rpmsg_gpio_config {
> >       __le16 ngpio;
> >       __u8 padding[2];
> >       __le32 gpio_names_size;
> > };
> >
> > /* Virtio GPIO Request / Response */
> > struct virtio_gpio_request {
> >       __le16 type;
> >       __le16 gpio;
> >       __le32 value;
> > };
>
> The core of the issue is that Shenwei is stone walling any change
> which makes it hard to keep the legacy firmware. It is possible to use
> these structures, but it makes the extra code Shenwei needs to
> translate this protocol to the legacy protocol more difficult. It
> might need to keep state, etc.
>

I agree with everything Andrew points out above.

> Two points...
>
> The firmware implements more than GPIO. There is definitely I2C as
> well, the first version of the patch has bits of I2C code. Looking at:
>
> https://lwn.net/ml/all/20250922200413.309707-3-shenwei.wang@nxp.com/
>
> There is also RTC, and a few other things which don't directly map to
> Linux subsystems, but maybe do have Linux drivers?
>
> Give how much pushback there has been on the existing protocol for
> GPIO, it would be wise to assume that I2C, and RTC is going to get the
> same amount of pushback. If any of these three, GPIO, I2C, or RTC
> decide that only a new, clean protocol will be accepted, no legacy
> shims, the firmware has to change, breaking compatibility to legacy
> protocols, and the accepted shims become pointless Maintenance burden.
>

I have made this point clear before: modeling legacy protocols in
mainline doesn't scale.  Mainline uses a single generic protocol, and
yes, it means breaking legacy protocols.  This is the cost of moving
to a mainline kernel.  If people want to use the legacy firmware, they
must stick with a legacy kernel.

> Point two is that the customers who are pushing for these drivers to
> be added to Mainline probably know that nearly nothing gets into
> Mainline without some changes. There is some short term pain to
> swapping to Mainline because of these changes, in this case, firmware
> upgrades. But in the long run, it is worth the pain to be able to use
> Mainline. And those customers who don't want to upgrade the firmware
> can keep with the out of tree drives.
>
> So, what are our choices?
>
> 1) We accept the code as it is now, with the shim?
>

NAK

> 2) We keep pushing for the virtio protocol, with the shim?
>

NAK

> 3) We keep pushing for the virtio protocol, no shim, firmware changes
>

Nothing will get merged in the RPMSG subsystem that includes support
for the legacy protocol.  Not today, not in a month, not in 5 years.

> 4) We pause GPIO where it is today, and restart all the arguments with
>    the I2C driver. We can come back to the GPIO driver in a few months
>    time once we have a better idea how I2C is going. And maybe we also
>    need to see the watchdog driver, and argue about its protocol.
>
> I also understand ST has a generic I2C driver nearly ready, if that
> gets merged first, that probably kills the NXP I2C protocol, and maybe
> the NXP GPIO and RTC protocols.
>
> My vote is for 3. If not 3, then 4.
>

Strong vote for 3.

>      Andrew
>

Re: [PATCH v12 3/5] gpio: rpmsg: add generic rpmsg GPIO driver

[Shenwei Wang]

> -----Original Message-----
> From: Mathieu Poirier <mathieu.poirier@linaro.org>
> Sent: Wednesday, March 18, 2026 11:03 AM
> To: Andrew Lunn <andrew@lunn.ch>
> Cc: Arnaud POULIQUEN <arnaud.pouliquen@foss.st.com>; Shenwei Wang
> <shenwei.wang@nxp.com>; Linus Walleij <linusw@kernel.org>; Bartosz
> Golaszewski <brgl@kernel.org>; Jonathan Corbet <corbet@lwn.net>; Rob Herring
> <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
> <conor+dt@kernel.org>; Bjorn Andersson <andersson@kernel.org>; Frank Li
> <frank.li@nxp.com>; Sascha Hauer <s.hauer@pengutronix.de>; Shuah Khan
> <skhan@linuxfoundation.org>; linux-gpio@vger.kernel.org; linux-
> doc@vger.kernel.org; linux-kernel@vger.kernel.org; Pengutronix Kernel Team
> <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; Peng Fan
> <peng.fan@nxp.com>; devicetree@vger.kernel.org; linux-
> remoteproc@vger.kernel.org; imx@lists.linux.dev; linux-arm-
> kernel@lists.infradead.org; dl-linux-imx <linux-imx@nxp.com>; Bartosz
> Golaszewski <brgl@bgdev.pl>
> Subject: [EXT] Re: [PATCH v12 3/5] gpio: rpmsg: add generic rpmsg GPIO driver
> On Tue, 17 Mar 2026 at 08:11, Andrew Lunn <andrew@lunn.ch> wrote:
> >
> > > > +struct rpmsg_gpio_info {
> > > > +   struct rpmsg_device *rpdev;
> > > > +   struct rpmsg_gpio_packet *reply_msg;
> > > > +   struct completion cmd_complete;
> > > > +   struct mutex lock;
> > > > +   void **port_store;
> > > > +};
> > >
> > > Except if I missunderstood Mathieu and Bjorn's request:
> > > "reuse all the design-work done in the gpio-virtio"
> > > We should find similar structures here to those defined in
> > > virtio_gpio.h.
> > > struct rpmsg_gpio_config {
> > >       __le16 ngpio;
> > >       __u8 padding[2];
> > >       __le32 gpio_names_size;
> > > };
> > >
> > > /* Virtio GPIO Request / Response */ struct virtio_gpio_request {
> > >       __le16 type;
> > >       __le16 gpio;
> > >       __le32 value;
> > > };
> >
> > The core of the issue is that Shenwei is stone walling any change
> > which makes it hard to keep the legacy firmware. It is possible to use
> > these structures, but it makes the extra code Shenwei needs to
> > translate this protocol to the legacy protocol more difficult. It
> > might need to keep state, etc.
> >
> 
> I agree with everything Andrew points out above.
> 
> > Two points...
> >
> > The firmware implements more than GPIO. There is definitely I2C as
> > well, the first version of the patch has bits of I2C code. Looking at:
> >
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flwn.
> > net%2Fml%2Fall%2F20250922200413.309707-3-
> shenwei.wang%40nxp.com%2F&dat
> >
> a=05%7C02%7Cshenwei.wang%40nxp.com%7C4b8879a9c89a4a831cf508de850
> 7de18%
> >
> 7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C639094465992371367%
> 7CUnkn
> >
> own%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIl
> AiOiJX
> >
> aW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=s8tl8n
> m3eD
> > 9l%2FetyyE%2FPWwJh4wQalaaHr4OEwzpQ7NY%3D&reserved=0
> >
> > There is also RTC, and a few other things which don't directly map to
> > Linux subsystems, but maybe do have Linux drivers?
> >
> > Give how much pushback there has been on the existing protocol for
> > GPIO, it would be wise to assume that I2C, and RTC is going to get the
> > same amount of pushback. If any of these three, GPIO, I2C, or RTC
> > decide that only a new, clean protocol will be accepted, no legacy
> > shims, the firmware has to change, breaking compatibility to legacy
> > protocols, and the accepted shims become pointless Maintenance burden.
> >
> 
> I have made this point clear before: modeling legacy protocols in mainline doesn't
> scale.  Mainline uses a single generic protocol, and yes, it means breaking legacy
> protocols.  This is the cost of moving to a mainline kernel.  If people want to use
> the legacy firmware, they must stick with a legacy kernel.
> 
> > Point two is that the customers who are pushing for these drivers to
> > be added to Mainline probably know that nearly nothing gets into
> > Mainline without some changes. There is some short term pain to
> > swapping to Mainline because of these changes, in this case, firmware
> > upgrades. But in the long run, it is worth the pain to be able to use
> > Mainline. And those customers who don't want to upgrade the firmware
> > can keep with the out of tree drives.
> >
> > So, what are our choices?
> >
> > 1) We accept the code as it is now, with the shim?
> >
> 
> NAK
> 
> > 2) We keep pushing for the virtio protocol, with the shim?
> >
> 
> NAK
> 
> > 3) We keep pushing for the virtio protocol, no shim, firmware changes
> >
> 
> Nothing will get merged in the RPMSG subsystem that includes support for the
> legacy protocol.  Not today, not in a month, not in 5 years.
> 

@Mathieu, 
Your tone is unnecessary. If you believe this driver must
comply with a specific virtio protocol, then please point to the exact
specification instead of making blanket statements.

If virtio is the direction you prefer, you are of course free to propose
and implement such support yourself.

My patches are contributed in good faith to improve the ecosystem, and
this work clearly belongs to the GPIO subsystem. I don't understand why
you are asserting authority here without providing any technical
justification.

@Linus Walleij,
From a technical standpoint, this GPIO driver is no different from
gpio-mxc, gpio-omap, or gpio-rda. If the concern is simply the use of
the word “generic” in the name, I’m perfectly fine reverting it to an
NXP‑specific driver.

If maintaining a private GPIO driver is no longer acceptable going
forward, that’s also fine — we can stop the discussion here. If you think
there are still technical limitations in the driver itself, I’m more than
willing to continue improving it.

But the goal is not to create a driver for another protocol that someone 
claims perfect. 

Thanks,
Shenwei

> > 4) We pause GPIO where it is today, and restart all the arguments with
> >    the I2C driver. We can come back to the GPIO driver in a few months
> >    time once we have a better idea how I2C is going. And maybe we also
> >    need to see the watchdog driver, and argue about its protocol.
> >
> > I also understand ST has a generic I2C driver nearly ready, if that
> > gets merged first, that probably kills the NXP I2C protocol, and maybe
> > the NXP GPIO and RTC protocols.
> >
> > My vote is for 3. If not 3, then 4.
> >
> 
> Strong vote for 3.
> 
> >      Andrew
> >

Re: [PATCH v12 3/5] gpio: rpmsg: add generic rpmsg GPIO driver

[Mathieu Poirier]

On Wed, 18 Mar 2026 at 12:46, Shenwei Wang <shenwei.wang@nxp.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Mathieu Poirier <mathieu.poirier@linaro.org>
> > Sent: Wednesday, March 18, 2026 11:03 AM
> > To: Andrew Lunn <andrew@lunn.ch>
> > Cc: Arnaud POULIQUEN <arnaud.pouliquen@foss.st.com>; Shenwei Wang
> > <shenwei.wang@nxp.com>; Linus Walleij <linusw@kernel.org>; Bartosz
> > Golaszewski <brgl@kernel.org>; Jonathan Corbet <corbet@lwn.net>; Rob Herring
> > <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
> > <conor+dt@kernel.org>; Bjorn Andersson <andersson@kernel.org>; Frank Li
> > <frank.li@nxp.com>; Sascha Hauer <s.hauer@pengutronix.de>; Shuah Khan
> > <skhan@linuxfoundation.org>; linux-gpio@vger.kernel.org; linux-
> > doc@vger.kernel.org; linux-kernel@vger.kernel.org; Pengutronix Kernel Team
> > <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; Peng Fan
> > <peng.fan@nxp.com>; devicetree@vger.kernel.org; linux-
> > remoteproc@vger.kernel.org; imx@lists.linux.dev; linux-arm-
> > kernel@lists.infradead.org; dl-linux-imx <linux-imx@nxp.com>; Bartosz
> > Golaszewski <brgl@bgdev.pl>
> > Subject: [EXT] Re: [PATCH v12 3/5] gpio: rpmsg: add generic rpmsg GPIO driver
> > On Tue, 17 Mar 2026 at 08:11, Andrew Lunn <andrew@lunn.ch> wrote:
> > >
> > > > > +struct rpmsg_gpio_info {
> > > > > +   struct rpmsg_device *rpdev;
> > > > > +   struct rpmsg_gpio_packet *reply_msg;
> > > > > +   struct completion cmd_complete;
> > > > > +   struct mutex lock;
> > > > > +   void **port_store;
> > > > > +};
> > > >
> > > > Except if I missunderstood Mathieu and Bjorn's request:
> > > > "reuse all the design-work done in the gpio-virtio"
> > > > We should find similar structures here to those defined in
> > > > virtio_gpio.h.
> > > > struct rpmsg_gpio_config {
> > > >       __le16 ngpio;
> > > >       __u8 padding[2];
> > > >       __le32 gpio_names_size;
> > > > };
> > > >
> > > > /* Virtio GPIO Request / Response */ struct virtio_gpio_request {
> > > >       __le16 type;
> > > >       __le16 gpio;
> > > >       __le32 value;
> > > > };
> > >
> > > The core of the issue is that Shenwei is stone walling any change
> > > which makes it hard to keep the legacy firmware. It is possible to use
> > > these structures, but it makes the extra code Shenwei needs to
> > > translate this protocol to the legacy protocol more difficult. It
> > > might need to keep state, etc.
> > >
> >
> > I agree with everything Andrew points out above.
> >
> > > Two points...
> > >
> > > The firmware implements more than GPIO. There is definitely I2C as
> > > well, the first version of the patch has bits of I2C code. Looking at:
> > >
> > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flwn.
> > > net%2Fml%2Fall%2F20250922200413.309707-3-
> > shenwei.wang%40nxp.com%2F&dat
> > >
> > a=05%7C02%7Cshenwei.wang%40nxp.com%7C4b8879a9c89a4a831cf508de850
> > 7de18%
> > >
> > 7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C639094465992371367%
> > 7CUnkn
> > >
> > own%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIl
> > AiOiJX
> > >
> > aW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=s8tl8n
> > m3eD
> > > 9l%2FetyyE%2FPWwJh4wQalaaHr4OEwzpQ7NY%3D&reserved=0
> > >
> > > There is also RTC, and a few other things which don't directly map to
> > > Linux subsystems, but maybe do have Linux drivers?
> > >
> > > Give how much pushback there has been on the existing protocol for
> > > GPIO, it would be wise to assume that I2C, and RTC is going to get the
> > > same amount of pushback. If any of these three, GPIO, I2C, or RTC
> > > decide that only a new, clean protocol will be accepted, no legacy
> > > shims, the firmware has to change, breaking compatibility to legacy
> > > protocols, and the accepted shims become pointless Maintenance burden.
> > >
> >
> > I have made this point clear before: modeling legacy protocols in mainline doesn't
> > scale.  Mainline uses a single generic protocol, and yes, it means breaking legacy
> > protocols.  This is the cost of moving to a mainline kernel.  If people want to use
> > the legacy firmware, they must stick with a legacy kernel.
> >
> > > Point two is that the customers who are pushing for these drivers to
> > > be added to Mainline probably know that nearly nothing gets into
> > > Mainline without some changes. There is some short term pain to
> > > swapping to Mainline because of these changes, in this case, firmware
> > > upgrades. But in the long run, it is worth the pain to be able to use
> > > Mainline. And those customers who don't want to upgrade the firmware
> > > can keep with the out of tree drives.
> > >
> > > So, what are our choices?
> > >
> > > 1) We accept the code as it is now, with the shim?
> > >
> >
> > NAK
> >
> > > 2) We keep pushing for the virtio protocol, with the shim?
> > >
> >
> > NAK
> >
> > > 3) We keep pushing for the virtio protocol, no shim, firmware changes
> > >
> >
> > Nothing will get merged in the RPMSG subsystem that includes support for the
> > legacy protocol.  Not today, not in a month, not in 5 years.
> >
>
> @Mathieu,
> Your tone is unnecessary. If you believe this driver must
> comply with a specific virtio protocol, then please point to the exact
> specification instead of making blanket statements.
>
> If virtio is the direction you prefer, you are of course free to propose
> and implement such support yourself.
>
> My patches are contributed in good faith to improve the ecosystem, and
> this work clearly belongs to the GPIO subsystem. I don't understand why
> you are asserting authority here without providing any technical
> justification.
>

All arguments have already been presented to you, we are now going in circles.

I am happy to look at a new revision of this work that complies with
the comments Andrew, Arnaud and I provided.  I will not engage with
you or your work until that time comes.

> @Linus Walleij,
> From a technical standpoint, this GPIO driver is no different from
> gpio-mxc, gpio-omap, or gpio-rda. If the concern is simply the use of
> the word “generic” in the name, I’m perfectly fine reverting it to an
> NXP‑specific driver.
>
> If maintaining a private GPIO driver is no longer acceptable going
> forward, that’s also fine — we can stop the discussion here. If you think
> there are still technical limitations in the driver itself, I’m more than
> willing to continue improving it.
>
> But the goal is not to create a driver for another protocol that someone
> claims perfect.
>
> Thanks,
> Shenwei
>
> > > 4) We pause GPIO where it is today, and restart all the arguments with
> > >    the I2C driver. We can come back to the GPIO driver in a few months
> > >    time once we have a better idea how I2C is going. And maybe we also
> > >    need to see the watchdog driver, and argue about its protocol.
> > >
> > > I also understand ST has a generic I2C driver nearly ready, if that
> > > gets merged first, that probably kills the NXP I2C protocol, and maybe
> > > the NXP GPIO and RTC protocols.
> > >
> > > My vote is for 3. If not 3, then 4.
> > >
> >
> > Strong vote for 3.
> >
> > >      Andrew
> > >

Re: [PATCH v12 3/5] gpio: rpmsg: add generic rpmsg GPIO driver

[Andrew Lunn]

> @Linus Walleij,
> From a technical standpoint, this GPIO driver is no different from
> gpio-mxc, gpio-omap, or gpio-rda.

Yes it is different. The example you list are all silicon GPIO blocks.
This driver is not for silicon, is a protocol spoken over rpmsg.  The
concept of two CPUs connected by rpmsg in a SoC is used by a number of
vendors. Look in Documentation/devicetree/bindings/remoteproc you see:

amlogic, fsl, ingenic, mtk, qcom, reneses, st, ti, wkup and xlnx.

We want one generic protocol/implementation of GPIO over rpmsg which
all these vendors will use. That means we have one GPIO driver, not 10
drivers to maintain for the next 10-20 years. It also means those 10
vendors have 1/10 of a driver they need to maintain for the next 10-20
years. And likely less bugs to deal with, since the driver is more
heavily tested by 10 vendors, etc.

     Andrew

Re: [PATCH v12 3/5] gpio: rpmsg: add generic rpmsg GPIO driver

[Shenwei Wang]

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Wednesday, March 18, 2026 3:11 PM
> To: Shenwei Wang <shenwei.wang@nxp.com>
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>; Arnaud POULIQUEN
> <arnaud.pouliquen@foss.st.com>; Linus Walleij <linusw@kernel.org>; Bartosz
> Golaszewski <brgl@kernel.org>; Jonathan Corbet <corbet@lwn.net>; Rob Herring
> <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
> <conor+dt@kernel.org>; Bjorn Andersson <andersson@kernel.org>; Frank Li
> <frank.li@nxp.com>; Sascha Hauer <s.hauer@pengutronix.de>; Shuah Khan
> <skhan@linuxfoundation.org>; linux-gpio@vger.kernel.org; linux-
> doc@vger.kernel.org; linux-kernel@vger.kernel.org; Pengutronix Kernel Team
> <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; Peng Fan
> <peng.fan@nxp.com>; devicetree@vger.kernel.org; linux-
> remoteproc@vger.kernel.org; imx@lists.linux.dev; linux-arm-
> kernel@lists.infradead.org; dl-linux-imx <linux-imx@nxp.com>; Bartosz
> Golaszewski <brgl@bgdev.pl>
> Subject: [EXT] Re: [PATCH v12 3/5] gpio: rpmsg: add generic rpmsg GPIO driver
> 
> > @Linus Walleij,
> > From a technical standpoint, this GPIO driver is no different from
> > gpio-mxc, gpio-omap, or gpio-rda.
> 
> Yes it is different. The example you list are all silicon GPIO blocks.
> This driver is not for silicon, is a protocol spoken over rpmsg.  The concept of two

A GPIO driver only needs to follow its defined interface, whether that interface is exposed 
via registers or via rpmsg. It doesn't need to know whether the backend implementation is 
hardware or firmware, and stable firmware can behave just like hardened logic from the 
driver's perspective.

> CPUs connected by rpmsg in a SoC is used by a number of vendors. Look in
> Documentation/devicetree/bindings/remoteproc you see:
> 
> amlogic, fsl, ingenic, mtk, qcom, reneses, st, ti, wkup and xlnx.
> 
> We want one generic protocol/implementation of GPIO over rpmsg which all

If you believe the current GPIO driver isn't generic, could you point to a real example 
that shows where it doesn't fit?

Thanks,
Shenwei 

> these vendors will use. That means we have one GPIO driver, not 10 drivers to
> maintain for the next 10-20 years. It also means those 10 vendors have 1/10 of a
> driver they need to maintain for the next 10-20 years. And likely less bugs to deal
> with, since the driver is more heavily tested by 10 vendors, etc.
> 
>      Andrew

Re: [PATCH v12 3/5] gpio: rpmsg: add generic rpmsg GPIO driver

[Linus Walleij]

On Wed, Mar 18, 2026 at 5:03 PM Mathieu Poirier
<mathieu.poirier@linaro.org> wrote:

> I have made this point clear before: modeling legacy protocols in
> mainline doesn't scale.  Mainline uses a single generic protocol, and
> yes, it means breaking legacy protocols.  This is the cost of moving
> to a mainline kernel.  If people want to use the legacy firmware, they
> must stick with a legacy kernel.

I mostly agree with this stance.

But it is under the assumption that the contributor is coming from the
same legal body that can define and change the firmware in question.

For example: the mainline Linux kernel supports a whole slew of
funky Apple rpmsg-like protocols. c.f. drivers/soc/apple/rtkit.c

We cannot go and tell the Asahi contributors to change the Apple
firmware to use rpmsg like everyone else, because they are not Apple,
they just want to run Linux on someone else's hardware.

In this case, the contributor is coming from the same legal body as
the one doing the firmware. I know and sympathize with the fact
that sometimes working inside a company to make changes happen
can be as hard as working on the outside, and internal structures
can be as resistant to pressure change as Microsoft, or Apple.
Additionally they hit the contributor on the head with "just get this
done, now, fast".

So it is pretty important in this situation that it is NXP that we address.
The contributor is just a representative of that legal body
in this case.

If someone *outside* of NXP, say an OpenWrt hobbyist contributor
was pushing the same patches based on code drops and reverse
engineering, the response would be *different*.

In a way the situation is a bit icky. As the Linux community we often
see all contributions as personal, individual. And the discussion here
is that of standards committee, which we are not.

So if we are addressing NXP, then we need to be explicit about that,
and careful not to put their representative in the crosshairs. It's unfair,
he's just the messenger.

Yours,
Linus Walleij

Re: [PATCH v12 3/5] gpio: rpmsg: add generic rpmsg GPIO driver

[Andrew Lunn]

> > +static int rpmsg_gpio_send_message(struct rpmsg_gpio_port *port,
> > +				   struct rpmsg_gpio_packet *msg,
> > +				   bool sync)
> > +{
> > +	struct rpmsg_gpio_info *info = &port->info;
> > +	struct rpdev_drvdata *drvdata;
> > +	int ret;
> > +
> > +	drvdata = dev_get_drvdata(&info->rpdev->dev);
> > +	reinit_completion(&info->cmd_complete);
> > +
> > +	if (drvdata->protocol_fixed_up)
> > +		ret = drvdata->protocol_fixed_up->send_fixed_up(info, msg);
> 
> Seems not part of a generic implementation

Agreed. Lets have a clean generic implementation first, and then
patches on top for legacy stuff.

> > +	ret = of_property_read_u32(np, "ngpios", &port->ngpios);
> 
> The number of GPIOs should be obtained from the remote side, as done in
> virtio_gpio. In virtio_gpio, this is retrieved via a get_config operation.
> Here, you could implement a specific RPMsg to retrieve the remote topology.

The DT property ngpios is pretty standard, so i think it makes a lot
of sense to have. But i also agree that asking the other side makes
sense. What is being implemented here with rpmsg is not that different
to greybus, and the greybus GPIO driver also ask the other side how
many GPIO lines it has. So yes, it should be part of the protocol, but
maybe optional?

	  Andrew

Re: [PATCH v12 3/5] gpio: rpmsg: add generic rpmsg GPIO driver

[Shenwei Wang]

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Tuesday, March 17, 2026 9:22 AM
> To: Arnaud POULIQUEN <arnaud.pouliquen@foss.st.com>
> Cc: Shenwei Wang <shenwei.wang@nxp.com>; Linus Walleij
> <linusw@kernel.org>; Bartosz Golaszewski <brgl@kernel.org>; Jonathan Corbet
> <corbet@lwn.net>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski
> <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Bjorn Andersson
> <andersson@kernel.org>; Mathieu Poirier <mathieu.poirier@linaro.org>; Frank Li
> <frank.li@nxp.com>; Sascha Hauer <s.hauer@pengutronix.de>; Shuah Khan
> <skhan@linuxfoundation.org>; linux-gpio@vger.kernel.org; linux-
> doc@vger.kernel.org; linux-kernel@vger.kernel.org; Pengutronix Kernel Team
> <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; Peng Fan
> <peng.fan@nxp.com>; devicetree@vger.kernel.org; linux-
> remoteproc@vger.kernel.org; imx@lists.linux.dev; linux-arm-
> kernel@lists.infradead.org; dl-linux-imx <linux-imx@nxp.com>; Bartosz
> Golaszewski <brgl@bgdev.pl>
> Subject: [EXT] Re: [PATCH v12 3/5] gpio: rpmsg: add generic rpmsg GPIO driver
> 
> > > +static int rpmsg_gpio_send_message(struct rpmsg_gpio_port *port,
> > > +                              struct rpmsg_gpio_packet *msg,
> > > +                              bool sync) {
> > > +   struct rpmsg_gpio_info *info = &port->info;
> > > +   struct rpdev_drvdata *drvdata;
> > > +   int ret;
> > > +
> > > +   drvdata = dev_get_drvdata(&info->rpdev->dev);
> > > +   reinit_completion(&info->cmd_complete);
> > > +
> > > +   if (drvdata->protocol_fixed_up)
> > > +           ret = drvdata->protocol_fixed_up->send_fixed_up(info,
> > > + msg);
> >
> > Seems not part of a generic implementation
> 
> Agreed. Lets have a clean generic implementation first, and then patches on top
> for legacy stuff.
> 

Adding fixed_up hooks doesn't make the implementation non-generic-this pattern is 
widely used in Linux drivers.

Shenwei

> > > +   ret = of_property_read_u32(np, "ngpios", &port->ngpios);
> >
> > The number of GPIOs should be obtained from the remote side, as done
> > in virtio_gpio. In virtio_gpio, this is retrieved via a get_config operation.
> > Here, you could implement a specific RPMsg to retrieve the remote topology.
> 
> The DT property ngpios is pretty standard, so i think it makes a lot of sense to
> have. But i also agree that asking the other side makes sense. What is being
> implemented here with rpmsg is not that different to greybus, and the greybus
> GPIO driver also ask the other side how many GPIO lines it has. So yes, it should
> be part of the protocol, but maybe optional?
> 
>           Andrew

Re: [PATCH v12 3/5] gpio: rpmsg: add generic rpmsg GPIO driver

[Shenwei Wang]

> -----Original Message-----
> From: Arnaud POULIQUEN <arnaud.pouliquen@foss.st.com>
> Sent: Tuesday, March 17, 2026 3:57 AM
> To: Shenwei Wang <shenwei.wang@nxp.com>; Linus Walleij
> <linusw@kernel.org>; Bartosz Golaszewski <brgl@kernel.org>; Jonathan Corbet
> <corbet@lwn.net>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski
> <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Bjorn Andersson
> <andersson@kernel.org>; Mathieu Poirier <mathieu.poirier@linaro.org>; Frank Li
> <frank.li@nxp.com>; Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Shuah Khan <skhan@linuxfoundation.org>; linux-gpio@vger.kernel.org; linux-
> doc@vger.kernel.org; linux-kernel@vger.kernel.org; Pengutronix Kernel Team
> <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; Peng Fan
> <peng.fan@nxp.com>; devicetree@vger.kernel.org; linux-
> remoteproc@vger.kernel.org; imx@lists.linux.dev; linux-arm-
> kernel@lists.infradead.org; dl-linux-imx <linux-imx@nxp.com>; Bartosz
> Golaszewski <brgl@bgdev.pl>; Andrew Lunn <andrew@lunn.ch>
> Subject: [EXT] Re: [PATCH v12 3/5] gpio: rpmsg: add generic rpmsg GPIO driver
> Hello,
> 
> On 3/13/26 20:57, Shenwei Wang wrote:
> > On an AMP platform, the system may include two processors:
> >       - An MCU running an RTOS
> >       - An MPU running Linux
> >
> > +#include <linux/platform_device.h>
> > +#include <linux/remoteproc.h>
> > +#include <linux/rpmsg.h>
> > +#include <linux/virtio_gpio.h>
> 
> you still needed to include virtio_gpio.h?
> 

That's way to re-use the design of virtio-gpio.

> > +
> > +#define MAX_PORT_PER_CHANNEL    10
> > +#define GPIOS_PER_PORT_DEFAULT       32
> > +#define RPMSG_TIMEOUT                1000
> 
> I wonder if the timeout is not too high. but not blocking.
> 
> > +
> > +/* GPIO RPMSG Type */
> > +#define GPIO_RPMSG_SEND              0
> > +#define GPIO_RPMSG_REPLY     1
> > +#define GPIO_RPMSG_NOTIFY    2
> > +
> > +struct rpmsg_gpio_packet {
> > +     u8 type;        /* Message type */
> > +     u8 cmd;         /* Command code */
> > +     u8 port_idx;
> > +     u8 line;
> > +     u8 val1;
> > +     u8 val2;
> > +};
> > +
> > +struct rpmsg_gpio_line {
> > +     u8 irq_shutdown;
> > +     u8 irq_unmask;
> > +     u8 irq_mask;
> > +     u32 irq_wake_enable;
> > +     u32 irq_type;
> > +     struct rpmsg_gpio_packet msg;
> > +};
> > +
> > +struct rpmsg_gpio_info {
> > +     struct rpmsg_device *rpdev;
> > +     struct rpmsg_gpio_packet *reply_msg;
> > +     struct completion cmd_complete;
> > +     struct mutex lock;
> > +     void **port_store;
> > +};
> 
> Except if I missunderstood Mathieu and Bjorn's request:
> "reuse all the design-work done in the gpio-virtio"
> We should find similar structures here to those defined in virtio_gpio.h.
> struct rpmsg_gpio_config {
>         __le16 ngpio;
>         __u8 padding[2];
>         __le32 gpio_names_size;
> };
> 
> /* Virtio GPIO Request / Response */
> struct virtio_gpio_request {
>         __le16 type;
>         __le16 gpio;
>         __le32 value;
> };
> 
> struct rpmsg_gpio_response {
>         __u8 status;
>         __u8 value;
> };
> 
> struct rpmsg_gpio_response_get_names {
>         __u8 status;
>         __u8 value[];
> };
> 
> /* Virtio GPIO IRQ Request / Response */ struct rpmsg_gpio_irq_request {
>         __le16 gpio;
> };
> 
> struct rpmsg_gpio_irq_response {
>         __u8 status;
> };
> 

There’s no need to replicate the virtio‑gpio structs.
virtio‑gpio is much more complex than this rpmsg‑gpio implementation, and 
copying its structures does not have benefits here.
Matching the command set and behavior is the way to re-use its design so that
enables the backend on the other side to be reused easily.

> > +
> > +struct rpmsg_gpio_port {
> > +     struct gpio_chip gc;
> > +     struct rpmsg_gpio_line lines[GPIOS_PER_PORT_DEFAULT];
> > +     struct rpmsg_gpio_info info;
> > +     u32 ngpios;
> > +     u32 idx;
> > +};
> > +
> > +struct rpmsg_gpio_fixed_up {
> > +     int (*send_fixed_up)(struct rpmsg_gpio_info *info, struct
> rpmsg_gpio_packet *msg);
> > +     struct rpmsg_gpio_packet *(*recv_fixed_up)(struct rpmsg_device
> > +*rpdev, void *data); };
> > +
> > +/*
> > + * @rproc_name: the name of the remote proc.
> > + * @recv_pkt: a pointer to the received packet for protocol fix up.
> > + * @protocol_fixed_up: optional callbacks to handle protocol mismatches.
> > + * @channel_devices: an array of the devices related to the rpdev.
> > + */
> > +struct rpdev_drvdata {
> > +     const char *rproc_name;
> > +     void *recv_pkt;
> > +     struct rpmsg_gpio_fixed_up *protocol_fixed_up;
> > +     void *channel_devices[MAX_PORT_PER_CHANNEL];
> > +};
> > +
> > +static int rpmsg_gpio_send_message(struct rpmsg_gpio_port *port,
> > +                                struct rpmsg_gpio_packet *msg,
> > +                                bool sync) {
> > +     struct rpmsg_gpio_info *info = &port->info;
> > +     struct rpdev_drvdata *drvdata;
> > +     int ret;
> > +
> > +     drvdata = dev_get_drvdata(&info->rpdev->dev);
> > +     reinit_completion(&info->cmd_complete);
> > +
> > +     if (drvdata->protocol_fixed_up)
> > +             ret = drvdata->protocol_fixed_up->send_fixed_up(info,
> > + msg);
> 
> Seems not part of a generic implementation
>

Can you explain in what way this is not generic?
I’d like to understand the specific concern.

I remember we had the agreement before this new patch.

------------------------------
https://lore.kernel.org/imx/64ef5dbf-6264-4758-a5d8-d8c52c359fcc@foss.st.com/

> Arnaud, if you agree with the points above, my proposal is the following:
>   - Remove the fields you mentioned in the protocol and update the driver accordingly so
> that we can establish a true baseline for a generic implementation we all agree.
>   - Then prepare a separate patch to add support for existing NXP platforms by introducing
> the necessary fix‑up functions.
> 
> Please let me know if this approach works for you. My goal is to find a solution that works for
> everyone — even though I know that pleasing everyone is almost impossible.
This looks reasonable to me, but I am not a maintainer, so I will let 
maintainers share their opinions on your proposition.

-------------------------------
 
> > +     else
> > +             ret = rpmsg_send(info->rpdev->ept, msg, sizeof(*msg));
> > +
> > +     if (ret) {
> > +             dev_err(&info->rpdev->dev, "rpmsg_send failed: %d\n", ret);
> > +             return ret;
> > +     }
> > +
> > +     if (sync) {
> > +             ret = wait_for_completion_timeout(&info->cmd_complete,
> > +                                               msecs_to_jiffies(RPMSG_TIMEOUT));
> > +             if (ret == 0) {
> > +                     dev_err(&info->rpdev->dev, "rpmsg_send timeout!\n");
> > +                     return -ETIMEDOUT;
> > +             }
> > +
> > +             if (info->reply_msg->val1 != 0) {
> > +                     dev_err(&info->rpdev->dev, "remote core replies an error: %d!\n",
> > +                             info->reply_msg->val1);
> > +                     return -EINVAL;
> > +             }
> > +
> > +             /* copy the reply message */
> > +             memcpy(&port->lines[info->reply_msg->line].msg,
> > +                    info->reply_msg, sizeof(*info->reply_msg));
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +static struct rpmsg_gpio_packet *
> > +rpmsg_gpio_msg_init_common(struct rpmsg_gpio_port *port, unsigned int
> > +line, u8 cmd) {
> > +     struct rpmsg_gpio_packet *msg = &port->lines[line].msg;
> > +
> > +     memset(msg, 0, sizeof(struct rpmsg_gpio_packet));
> > +     msg->type = GPIO_RPMSG_SEND;
> > +     msg->cmd = cmd;
> > +     msg->port_idx = port->idx;
> > +     msg->line = line;
> > +
> > +     return msg;
> > +}
> > +
> > +static int rpmsg_gpio_get(struct gpio_chip *gc, unsigned int line) {
> > +     struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
> > +     struct rpmsg_gpio_packet *msg;
> > +     int ret;
> > +
> > +     guard(mutex)(&port->info.lock);
> > +
> > +     msg = rpmsg_gpio_msg_init_common(port, line,
> > + VIRTIO_GPIO_MSG_GET_VALUE);
> > +
> > +     ret = rpmsg_gpio_send_message(port, msg, true);
> > +     if (!ret)
> > +             ret = !!port->lines[line].msg.val2;
> > +
> > +     return ret;
> > +}
> > +
> > +static int rpmsg_gpio_get_direction(struct gpio_chip *gc, unsigned
> > +int line) {
> > +     struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
> > +     struct rpmsg_gpio_packet *msg;
> > +     int ret;
> > +
> > +     guard(mutex)(&port->info.lock);
> > +
> > +     msg = rpmsg_gpio_msg_init_common(port, line,
> > + VIRTIO_GPIO_MSG_GET_DIRECTION);
> > +
> > +     ret = rpmsg_gpio_send_message(port, msg, true);
> > +     if (ret)
> > +             return ret;
> > +
> > +     switch (port->lines[line].msg.val2) {
> > +     case VIRTIO_GPIO_DIRECTION_IN:
> > +             return GPIO_LINE_DIRECTION_IN;
> > +     case VIRTIO_GPIO_DIRECTION_OUT:
> > +             return GPIO_LINE_DIRECTION_OUT;
> > +     default:
> > +             break;
> > +     }
> > +
> > +     return -EINVAL;
> > +}
> > +
> > +static int rpmsg_gpio_direction_input(struct gpio_chip *gc, unsigned
> > +int line) {
> > +     struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
> > +     struct rpmsg_gpio_packet *msg;
> > +
> > +     guard(mutex)(&port->info.lock);
> > +
> > +     msg = rpmsg_gpio_msg_init_common(port, line,
> VIRTIO_GPIO_MSG_SET_DIRECTION);
> > +     msg->val1 = VIRTIO_GPIO_DIRECTION_IN;
> > +
> > +     return rpmsg_gpio_send_message(port, msg, true); }
> > +
> > +static int rpmsg_gpio_set(struct gpio_chip *gc, unsigned int line,
> > +int val) {
> > +     struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
> > +     struct rpmsg_gpio_packet *msg;
> > +
> > +     guard(mutex)(&port->info.lock);
> > +
> > +     msg = rpmsg_gpio_msg_init_common(port, line,
> VIRTIO_GPIO_MSG_SET_VALUE);
> > +     msg->val1 = val;
> > +
> > +     return rpmsg_gpio_send_message(port, msg, true); }
> > +
> > +static int rpmsg_gpio_direction_output(struct gpio_chip *gc, unsigned
> > +int line, int val) {
> > +     struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
> > +     struct rpmsg_gpio_packet *msg;
> > +     int ret;
> > +
> > +     guard(mutex)(&port->info.lock);
> > +
> > +     msg = rpmsg_gpio_msg_init_common(port, line,
> VIRTIO_GPIO_MSG_SET_DIRECTION);
> > +     msg->val1 = VIRTIO_GPIO_DIRECTION_OUT;
> > +
> > +     ret = rpmsg_gpio_send_message(port, msg, true);
> > +     if (ret)
> > +             return ret;
> > +
> > +     msg = rpmsg_gpio_msg_init_common(port, line,
> VIRTIO_GPIO_MSG_SET_VALUE);
> > +     msg->val1 = val;
> > +
> > +     return rpmsg_gpio_send_message(port, msg, true); }
> > +
> > +static int gpio_rpmsg_irq_set_type(struct irq_data *d, u32 type) {
> > +     struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> > +     u32 line = d->hwirq;
> > +     int ret = 0;
> > +
> > +     switch (type) {
> > +     case IRQ_TYPE_EDGE_RISING:
> > +             type = VIRTIO_GPIO_IRQ_TYPE_EDGE_RISING;
> > +             irq_set_handler_locked(d, handle_simple_irq);
> > +             break;
> > +     case IRQ_TYPE_EDGE_FALLING:
> > +             type = VIRTIO_GPIO_IRQ_TYPE_EDGE_FALLING;
> > +             irq_set_handler_locked(d, handle_simple_irq);
> > +             break;
> > +     case IRQ_TYPE_EDGE_BOTH:
> > +             type = VIRTIO_GPIO_IRQ_TYPE_EDGE_BOTH;
> > +             irq_set_handler_locked(d, handle_simple_irq);
> > +             break;
> > +     case IRQ_TYPE_LEVEL_LOW:
> > +             type = VIRTIO_GPIO_IRQ_TYPE_LEVEL_LOW;
> > +             irq_set_handler_locked(d, handle_level_irq);
> > +             break;
> > +     case IRQ_TYPE_LEVEL_HIGH:
> > +             type = VIRTIO_GPIO_IRQ_TYPE_LEVEL_HIGH;
> > +             irq_set_handler_locked(d, handle_level_irq);
> > +             break;
> > +     default:
> > +             ret = -EINVAL;
> > +             irq_set_handler_locked(d, handle_bad_irq);
> > +             break;
> > +     }
> > +
> > +     port->lines[line].irq_type = type;
> > +
> > +     return ret;
> > +}
> > +
> > +static int gpio_rpmsg_irq_set_wake(struct irq_data *d, u32 enable) {
> > +     struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> > +     u32 line = d->hwirq;
> > +
> > +     port->lines[line].irq_wake_enable = enable;
> > +
> > +     return 0;
> > +}
> > +
> > +/*
> > + * This unmask/mask function is invoked in two situations:
> > + *   - when an interrupt is being set up, and
> > + *   - after an interrupt has occurred.
> > + *
> > + * The GPIO driver does not access hardware registers directly.
> > + * Instead, it caches all relevant information locally, and then
> > +sends
> > + * the accumulated state to the remote system at this stage.
> > + */
> > +static void gpio_rpmsg_unmask_irq(struct irq_data *d) {
> > +     struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> > +     u32 line = d->hwirq;
> > +
> > +     port->lines[line].irq_unmask = 1; }
> > +
> > +static void gpio_rpmsg_mask_irq(struct irq_data *d) {
> > +     struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> > +     u32 line = d->hwirq;
> > +
> > +     /*
> > +      * When an interrupt occurs, the remote system masks the interrupt
> > +      * and then sends a notification to Linux. After Linux processes
> > +      * that notification, it sends an RPMsg command back to the remote
> > +      * system to unmask the interrupt again.
> > +      */
> > +     port->lines[line].irq_mask = 1;
> > +}
> > +
> > +static void gpio_rpmsg_irq_shutdown(struct irq_data *d) {
> > +     struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> > +     u32 line = d->hwirq;
> > +
> > +     port->lines[line].irq_shutdown = 1; }
> > +
> > +static void gpio_rpmsg_irq_bus_lock(struct irq_data *d) {
> > +     struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> > +
> > +     mutex_lock(&port->info.lock);
> > +}
> > +
> > +static void gpio_rpmsg_irq_bus_sync_unlock(struct irq_data *d) {
> > +     struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> > +     struct rpmsg_gpio_packet *msg;
> > +     u32 line = d->hwirq;
> > +
> > +     /*
> > +      * For mask irq, do nothing here.
> > +      * The remote system will mask interrupt after an interrupt occurs,
> > +      * and then send a notification to Linux system. After Linux system
> > +      * handles the notification, it sends an rpmsg back to the remote
> > +      * system to unmask this interrupt again.
> > +      */
> > +     if (port->lines[line].irq_mask && !port->lines[line].irq_unmask) {
> > +             port->lines[line].irq_mask = 0;
> > +             mutex_unlock(&port->info.lock);
> > +             return;
> > +     }
> > +
> > +     msg = rpmsg_gpio_msg_init_common(port, line,
> > + VIRTIO_GPIO_MSG_IRQ_TYPE);
> > +
> > +     if (port->lines[line].irq_shutdown) {
> > +             port->lines[line].irq_shutdown = 0;
> > +             msg->val1 = VIRTIO_GPIO_IRQ_TYPE_NONE;
> > +             msg->val2 = 0;
> > +     } else {
> > +             /* if irq type is not set, use low level trigger as default. */
> > +             msg->val1 = port->lines[line].irq_type;
> > +             if (!msg->val1)
> > +                     msg->val1 = VIRTIO_GPIO_IRQ_TYPE_LEVEL_LOW;
> > +             if (port->lines[line].irq_unmask) {
> > +                     msg->val2 = 0;
> > +                     port->lines[line].irq_unmask = 0;
> > +             } else /* irq set wake */
> > +                     msg->val2 = port->lines[line].irq_wake_enable;
> > +     }
> > +
> > +     rpmsg_gpio_send_message(port, msg, false);
> > +     mutex_unlock(&port->info.lock);
> > +}
> > +
> > +static const struct irq_chip gpio_rpmsg_irq_chip = {
> > +     .irq_mask = gpio_rpmsg_mask_irq,
> > +     .irq_unmask = gpio_rpmsg_unmask_irq,
> > +     .irq_set_wake = gpio_rpmsg_irq_set_wake,
> > +     .irq_set_type = gpio_rpmsg_irq_set_type,
> > +     .irq_shutdown = gpio_rpmsg_irq_shutdown,
> > +     .irq_bus_lock = gpio_rpmsg_irq_bus_lock,
> > +     .irq_bus_sync_unlock = gpio_rpmsg_irq_bus_sync_unlock,
> > +     .flags = IRQCHIP_IMMUTABLE,
> > +};
> > +
> > +static void rpmsg_gpio_remove_action(void *data) {
> > +     struct rpmsg_gpio_port *port = data;
> > +
> > +     port->info.port_store[port->idx] = NULL; }
> > +
> > +static int rpmsg_gpiochip_register(struct rpmsg_device *rpdev, struct
> > +device_node *np) {
> > +     struct rpdev_drvdata *drvdata = dev_get_drvdata(&rpdev->dev);
> > +     struct rpmsg_gpio_port *port;
> > +     struct gpio_irq_chip *girq;
> > +     struct gpio_chip *gc;
> > +     int ret;
> > +
> > +     port = devm_kzalloc(&rpdev->dev, sizeof(*port), GFP_KERNEL);
> > +     if (!port)
> > +             return -ENOMEM;
> > +
> > +     ret = of_property_read_u32(np, "reg", &port->idx);
> > +     if (ret)
> > +             return ret;
> > +
> > +     if (port->idx >= MAX_PORT_PER_CHANNEL)
> > +             return -EINVAL;
> > +
> > +     ret = devm_mutex_init(&rpdev->dev, &port->info.lock);
> > +     if (ret)
> > +             return ret;
> > +
> > +     ret = of_property_read_u32(np, "ngpios", &port->ngpios);
> 
> The number of GPIOs should be obtained from the remote side, as done in
> virtio_gpio. In virtio_gpio, this is retrieved via a get_config operation. Here, you
> could implement a specific RPMsg to retrieve the remote topology.
> 
> > +     if (ret || port->ngpios > GPIOS_PER_PORT_DEFAULT)
> > +             port->ngpios = GPIOS_PER_PORT_DEFAULT;
> > +
> > +     port->info.reply_msg = devm_kzalloc(&rpdev->dev,
> > +                                         sizeof(struct rpmsg_gpio_packet),
> > +                                         GFP_KERNEL);
> > +     if (!port->info.reply_msg)
> > +             return -ENOMEM;
> > +
> > +     init_completion(&port->info.cmd_complete);
> > +     port->info.port_store = drvdata->channel_devices;
> > +     port->info.port_store[port->idx] = port;
> > +     port->info.rpdev = rpdev;
> > +
> > +     gc = &port->gc;
> > +     gc->owner = THIS_MODULE;
> > +     gc->parent = &rpdev->dev;
> > +     gc->fwnode = of_fwnode_handle(np);
> > +     gc->ngpio = port->ngpios;
> > +     gc->base = -1;
> > +     gc->label = devm_kasprintf(&rpdev->dev, GFP_KERNEL, "%s-gpio%d",
> > +                                drvdata->rproc_name, port->idx);
> > +
> > +     gc->direction_input = rpmsg_gpio_direction_input;
> > +     gc->direction_output = rpmsg_gpio_direction_output;
> > +     gc->get_direction = rpmsg_gpio_get_direction;
> > +     gc->get = rpmsg_gpio_get;
> > +     gc->set = rpmsg_gpio_set;
> > +
> > +     girq = &gc->irq;
> > +     gpio_irq_chip_set_chip(girq, &gpio_rpmsg_irq_chip);
> > +     girq->parent_handler = NULL;
> > +     girq->num_parents = 0;
> > +     girq->parents = NULL;
> > +     girq->chip->name = devm_kasprintf(&rpdev->dev, GFP_KERNEL, "%s-
> gpio%d",
> > +                                       drvdata->rproc_name,
> > + port->idx);
> > +
> > +     ret = devm_add_action_or_reset(&rpdev->dev,
> rpmsg_gpio_remove_action, port);
> > +     if (ret)
> > +             return ret;
> > +
> > +     return devm_gpiochip_add_data(&rpdev->dev, gc, port); }
> > +
> > +static const char *rpmsg_get_rproc_node_name(struct rpmsg_device
> > +*rpdev) {
> > +     const char *name = NULL;
> > +     struct device_node *np;
> > +     struct rproc *rproc;
> > +
> > +     rproc = rproc_get_by_child(&rpdev->dev);
> > +     if (!rproc)
> > +             return NULL;
> > +
> > +     np = of_node_get(rproc->dev.of_node);
> > +     if (!np && rproc->dev.parent)
> > +             np = of_node_get(rproc->dev.parent->of_node);
> > +
> > +     if (np) {
> > +             name = devm_kstrdup(&rpdev->dev, np->name, GFP_KERNEL);
> > +             of_node_put(np);
> > +     }
> > +
> > +     return name;
> > +}
> > +
> > +static struct device_node *
> > +rpmsg_get_channel_ofnode(struct rpmsg_device *rpdev, char *chan_name)
> > +{
> > +     struct device_node *np_chan = NULL, *np;
> > +     struct rproc *rproc;
> > +
> > +     rproc = rproc_get_by_child(&rpdev->dev);
> > +     if (!rproc)
> > +             return NULL;
> > +
> > +     np = of_node_get(rproc->dev.of_node);
> > +     if (!np && rproc->dev.parent)
> > +             np = of_node_get(rproc->dev.parent->of_node);
> > +
> > +     /* The of_node_put() is performed by of_find_node_by_name(). */
> > +     if (np)
> > +             np_chan = of_find_node_by_name(np, chan_name);
> > +
> > +     return np_chan;
> > +}
> > +
> > +static int rpmsg_gpio_channel_callback(struct rpmsg_device *rpdev, void
> *data,
> > +                                    int len, void *priv, u32 src) {
> > +     struct rpmsg_gpio_packet *msg = data;
> > +     struct rpmsg_gpio_port *port = NULL;
> > +     struct rpdev_drvdata *drvdata;
> > +
> > +     drvdata = dev_get_drvdata(&rpdev->dev);
> > +     if (drvdata && drvdata->protocol_fixed_up)
> > +             msg = drvdata->protocol_fixed_up->recv_fixed_up(rpdev,
> > + data);
> > +
> > +     if (!msg || !drvdata)
> > +             return -EINVAL;
> > +
> > +     if (msg->port_idx < MAX_PORT_PER_CHANNEL)
> > +             port = drvdata->channel_devices[msg->port_idx];
> > +
> > +     if (!port || msg->line >= port->ngpios) {
> > +             dev_err(&rpdev->dev, "wrong port index or line number. port:%d
> line:%d\n",
> > +                     msg->port_idx, msg->line);
> > +             return -EINVAL;
> > +     }
> > +
> > +     if (msg->type == GPIO_RPMSG_REPLY) {
> > +             *port->info.reply_msg = *msg;
> > +             complete(&port->info.cmd_complete);
> > +     } else if (msg->type == GPIO_RPMSG_NOTIFY) {
> > +             generic_handle_domain_irq_safe(port->gc.irq.domain, msg->line);
> > +     } else {
> > +             dev_err(&rpdev->dev, "wrong command type (0x%x)\n", msg->type);
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +static int rpmsg_gpio_channel_probe(struct rpmsg_device *rpdev) {
> > +     struct device *dev = &rpdev->dev;
> > +     struct rpdev_drvdata *drvdata;
> > +     struct device_node *np;
> > +     int ret = -ENODEV;
> > +
> > +     if (!dev->of_node) {
> > +             np = rpmsg_get_channel_ofnode(rpdev, rpdev->id.name);
> > +             if (np) {
> > +                     dev->of_node = np;
> > +                     set_primary_fwnode(dev, of_fwnode_handle(np));
> > +             }
> > +             return -EPROBE_DEFER;
> 
> Here is it a bug ? else you should explain in a comment why you perform some
> actions when np != 0 but return -EPROBE_DEFER
> 

-EPROBE_DEFER is to let the system to probe the driver again. 

Shenwei 

> Regards,
> Arnaud
> 
> > +     }
> > +
> > +     drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
> > +     if (!drvdata)
> > +             return -ENOMEM;
> > +
> > +     drvdata->rproc_name = rpmsg_get_rproc_node_name(rpdev);
> > +     drvdata->protocol_fixed_up = (struct rpmsg_gpio_fixed_up *)rpdev-
> >id.driver_data;
> > +     dev_set_drvdata(dev, drvdata);
> > +
> > +     for_each_child_of_node_scoped(dev->of_node, child) {
> > +             if (!of_device_is_available(child))
> > +                     continue;
> > +
> > +             if (!of_match_node(dev->driver->of_match_table, child))
> > +                     continue;
> > +
> > +             ret = rpmsg_gpiochip_register(rpdev, child);
> > +             if (ret < 0)
> > +                     break;
> > +     }
> > +
> > +     return ret;
> > +}
> > +
> > +static const struct of_device_id rpmsg_gpio_dt_ids[] = {
> > +     { .compatible = "rpmsg-gpio" },
> > +     { /* sentinel */ }
> > +};
> > +
> > +static struct rpmsg_device_id rpmsg_gpio_channel_id_table[] = {
> > +     { .name = "rpmsg-io" },
> > +     { },
> > +};
> > +MODULE_DEVICE_TABLE(rpmsg, rpmsg_gpio_channel_id_table);
> > +
> > +static struct rpmsg_driver rpmsg_gpio_channel_client = {
> > +     .callback       = rpmsg_gpio_channel_callback,
> > +     .id_table       = rpmsg_gpio_channel_id_table,
> > +     .probe          = rpmsg_gpio_channel_probe,
> > +     .drv            = {
> > +             .name   = KBUILD_MODNAME,
> > +             .of_match_table = rpmsg_gpio_dt_ids,
> > +     },
> > +};
> > +module_rpmsg_driver(rpmsg_gpio_channel_client);
> > +
> > +MODULE_AUTHOR("Shenwei Wang <shenwei.wang@nxp.com>");
> > +MODULE_DESCRIPTION("generic rpmsg gpio driver");
> > +MODULE_LICENSE("GPL");

RE: [PATCH v12 3/5] gpio: rpmsg: add generic rpmsg GPIO driver

[Shenwei Wang]

Hi Bjorn,

> -----Original Message-----
> From: Shenwei Wang
> Sent: Friday, March 13, 2026 2:59 PM
> To: Linus Walleij <linusw@kernel.org>; Bartosz Golaszewski <brgl@kernel.org>;
> Jonathan Corbet <corbet@lwn.net>; Rob Herring <robh@kernel.org>; Krzysztof
> Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Bjorn
> Andersson <andersson@kernel.org>; Mathieu Poirier
> <mathieu.poirier@linaro.org>; Frank Li <frank.li@nxp.com>; Sascha Hauer
> <s.hauer@pengutronix.de>; arnaud.pouliquen@foss.st.com
> Cc: Shuah Khan <skhan@linuxfoundation.org>; linux-gpio@vger.kernel.org; linux-
> doc@vger.kernel.org; linux-kernel@vger.kernel.org; Pengutronix Kernel Team
> <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; Shenwei
> Wang <shenwei.wang@nxp.com>; Peng Fan <peng.fan@nxp.com>;
> devicetree@vger.kernel.org; linux-remoteproc@vger.kernel.org;
> imx@lists.linux.dev; linux-arm-kernel@lists.infradead.org; dl-linux-imx <linux-
> imx@nxp.com>; Bartosz Golaszewski <brgl@bgdev.pl>; Andrew Lunn
> <andrew@lunn.ch>
> Subject: [PATCH v12 3/5] gpio: rpmsg: add generic rpmsg GPIO driver
> 

Since I rewrote this version based on your earlier feedback 
---
"My expectation is that it will be better to just have two separate
drivers - but reuse all the design-work done in the gpio-virtio."
---
I'd glad to hear what you think about this patch.

Thanks,
Shenwei

> On an AMP platform, the system may include two processors:
> 	- An MCU running an RTOS
> 	- An MPU running Linux
> 
> These processors communicate via the RPMSG protocol.
> The driver implements the standard GPIO interface, allowing the Linux side to
> control GPIO controllers which reside in the remote processor via RPMSG
> protocol.
> 
> Cc: Bartosz Golaszewski <brgl@bgdev.pl>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
> ---
>  drivers/gpio/Kconfig      |  17 ++
>  drivers/gpio/Makefile     |   1 +
>  drivers/gpio/gpio-rpmsg.c | 596 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 614 insertions(+)
>  create mode 100644 drivers/gpio/gpio-rpmsg.c
> 
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index
> b45fb799e36c..cff0fda8a283 100644

[PATCH v12 4/5] gpio: rpmsg: add support for NXP legacy firmware protocol

[Shenwei Wang]

Implement fixed-up message handlers to maintain compatibility with
existing i.MX devices that rely on the NXP legacy RPMSG firmware and
its transport protocol. This ensures backward compatibility and preserves
functionality for deployed NXP systems.

Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
---
 drivers/gpio/Kconfig      |  15 ++++
 drivers/gpio/gpio-rpmsg.c | 147 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 162 insertions(+)

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index cff0fda8a283..cd0ac5bf4443 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1907,6 +1907,21 @@ config GPIO_RPMSG
 
 	  If unsure, say N.
 
+if GPIO_RPMSG
+
+config GPIO_RPMSG_NXP_LEGACY
+	bool "Support for the NXP legacy firmware"
+	depends on GPIO_RPMSG && ARCH_MXC
+	default y
+	help
+	  Enable support for the legacy NXP firmware protocol used by older
+	  i.MX products. This option provides compatibility for systems
+	  that still rely on the nxp legacy message format and allows
+	  existing deployments to continue functioning without requiring
+	  firmware changes.
+
+endif
+
 endmenu
 
 menu "SPI GPIO expanders"
diff --git a/drivers/gpio/gpio-rpmsg.c b/drivers/gpio/gpio-rpmsg.c
index 9c609b55bc14..be263c09a1f0 100644
--- a/drivers/gpio/gpio-rpmsg.c
+++ b/drivers/gpio/gpio-rpmsg.c
@@ -84,6 +84,147 @@ struct rpdev_drvdata {
 	void *channel_devices[MAX_PORT_PER_CHANNEL];
 };
 
+#ifdef CONFIG_GPIO_RPMSG_NXP_LEGACY
+/* NXP I.MX Legacy GPIO RPMSG protocol */
+#define IMX_RPMSG_CONFIG_INPUT		0
+#define IMX_RPMSG_CONFIG_OUTPUT		1
+#define IMX_RPMSG_GET_LEVEL		2
+#define IMX_RPMSG_GET_DIRECTION		3
+#define IMX_RPMSG_CMD_UNKNOWN		0x7F
+
+#define IMX_RPMSG_TRI_LOW_LEVEL		4
+#define IMX_RPMSG_TRI_HIGH_LEVEL	5
+
+#define IMX_RPMSG_ID		5
+#define IMX_RPMSG_VENDOR	1
+#define IMX_RPMSG_VERSION	0
+
+struct rpmsg_gpio_nxp_packet {
+	u8 id;		/* Message ID Code */
+	u8 vendor;	/* Vendor ID number */
+	u8 version;	/* Protocol version number */
+	u8 type;	/* Message type */
+	u8 cmd;		/* Command code */
+	u8 reserved[5];
+	u8 line;
+	u8 port_idx;
+	u8 val1;
+	u8 val2;
+};
+
+static struct rpmsg_gpio_packet *
+rpmsg_gpio_imx_recv_fixed_up(struct rpmsg_device *rpdev, void *data)
+{
+	struct rpmsg_gpio_nxp_packet *imx_msg = data;
+	struct rpmsg_gpio_packet *msg;
+	struct rpdev_drvdata *drvdata;
+
+	if (!imx_msg)
+		return NULL;
+
+	drvdata = dev_get_drvdata(&rpdev->dev);
+	if (!drvdata->recv_pkt)
+		drvdata->recv_pkt = devm_kzalloc(&rpdev->dev, sizeof(*msg), GFP_ATOMIC);
+
+	if (!drvdata->recv_pkt)
+		return NULL;
+
+	msg = drvdata->recv_pkt;
+
+	msg->type = imx_msg->type;
+	msg->cmd = imx_msg->cmd;
+	msg->port_idx = imx_msg->port_idx;
+	msg->line = imx_msg->line;
+	msg->val1 = imx_msg->val1;
+	msg->val2 = imx_msg->val2;
+
+	switch (imx_msg->cmd) {
+	case IMX_RPMSG_GET_LEVEL:
+		msg->cmd = VIRTIO_GPIO_MSG_GET_VALUE;
+		break;
+
+	case IMX_RPMSG_GET_DIRECTION:
+		msg->cmd = VIRTIO_GPIO_MSG_GET_DIRECTION;
+		break;
+
+	case IMX_RPMSG_CONFIG_OUTPUT:
+		msg->cmd = VIRTIO_GPIO_MSG_SET_DIRECTION;
+		msg->val2 = VIRTIO_GPIO_DIRECTION_OUT;
+		break;
+
+	case IMX_RPMSG_CONFIG_INPUT:
+		msg->cmd = VIRTIO_GPIO_MSG_SET_DIRECTION;
+		msg->val2 = VIRTIO_GPIO_DIRECTION_IN;
+		break;
+
+	default:
+		break;
+	}
+
+	return msg;
+}
+
+static const int imx_std_cmd_map[] = {
+	IMX_RPMSG_CMD_UNKNOWN,
+	IMX_RPMSG_CMD_UNKNOWN,		/* VIRTIO_GPIO_MSG_GET_NAMES */
+	IMX_RPMSG_GET_DIRECTION,	/* VIRTIO_GPIO_MSG_GET_DIRECTION */
+	IMX_RPMSG_CONFIG_INPUT,		/* VIRTIO_GPIO_MSG_SET_DIRECTION */
+	IMX_RPMSG_GET_LEVEL,		/* VIRTIO_GPIO_MSG_GET_VALUE */
+	IMX_RPMSG_CONFIG_OUTPUT,	/* VIRTIO_GPIO_MSG_SET_VALUE */
+	IMX_RPMSG_CONFIG_INPUT		/* VIRTIO_GPIO_MSG_IRQ_TYPE */
+};
+
+static int rpmsg_gpio_imx_send_fixed_up(struct rpmsg_gpio_info *info,
+					struct rpmsg_gpio_packet *msg)
+{
+	struct rpmsg_gpio_nxp_packet imx_msg;
+
+	if (msg->cmd >= ARRAY_SIZE(imx_std_cmd_map))
+		return -EINVAL;
+
+	imx_msg.id = IMX_RPMSG_ID;
+	imx_msg.vendor = IMX_RPMSG_VENDOR;
+	imx_msg.version = IMX_RPMSG_VERSION;
+	imx_msg.type = msg->type;
+	imx_msg.cmd = imx_std_cmd_map[msg->cmd];
+	imx_msg.port_idx = msg->port_idx;
+	imx_msg.line = msg->line;
+	imx_msg.val1 = msg->val1;
+	imx_msg.val2 = msg->val2;
+
+	switch (msg->cmd) {
+	case VIRTIO_GPIO_MSG_IRQ_TYPE:
+		switch (msg->val1) {
+		case VIRTIO_GPIO_IRQ_TYPE_LEVEL_HIGH:
+			imx_msg.val1 = IMX_RPMSG_TRI_HIGH_LEVEL;
+			break;
+		case VIRTIO_GPIO_IRQ_TYPE_LEVEL_LOW:
+			imx_msg.val1 = IMX_RPMSG_TRI_LOW_LEVEL;
+			break;
+		default:
+			break;
+		}
+		break;
+
+	case VIRTIO_GPIO_MSG_SET_DIRECTION:
+		imx_msg.val1 = 0;
+		if (msg->val1 == VIRTIO_GPIO_DIRECTION_OUT)
+			imx_msg.cmd = IMX_RPMSG_CONFIG_OUTPUT;
+		break;
+
+	default:
+		break;
+	}
+
+	return rpmsg_send(info->rpdev->ept, &imx_msg, sizeof(imx_msg));
+}
+
+static const struct rpmsg_gpio_fixed_up imx_fixed_up_data = {
+	.recv_fixed_up = rpmsg_gpio_imx_recv_fixed_up,
+	.send_fixed_up = rpmsg_gpio_imx_send_fixed_up,
+};
+#endif
+
 static int rpmsg_gpio_send_message(struct rpmsg_gpio_port *port,
 				   struct rpmsg_gpio_packet *msg,
 				   bool sync)
@@ -576,6 +717,12 @@ static const struct of_device_id rpmsg_gpio_dt_ids[] = {
 
 static struct rpmsg_device_id rpmsg_gpio_channel_id_table[] = {
 	{ .name = "rpmsg-io" },
+#ifdef CONFIG_GPIO_RPMSG_NXP_LEGACY
+	{
+		.name = "rpmsg-io-channel",
+		.driver_data = (kernel_ulong_t)&imx_fixed_up_data
+	},
+#endif
 	{ },
 };
 MODULE_DEVICE_TABLE(rpmsg, rpmsg_gpio_channel_id_table);
-- 
2.43.0

[PATCH v12 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 | 25 ++++++++++++++++++++++
 1 file changed, 25 insertions(+)

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

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

[Linus Walleij]

Hi Shenwei,

On Fri, Mar 13, 2026 at 8:58 PM Shenwei Wang <shenwei.wang@nxp.com> wrote:

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

I think v12 looks pretty good, if Arnaud gives his ACK on this patch
series I think it's ripe for merge.

Yours,
Linus Walleij

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

[Mathieu Poirier]

[Adding Andrew Lunn]

On Mon, 16 Mar 2026 at 08:23, Linus Walleij <linusw@kernel.org> wrote:
>
> Hi Shenwei,
>
> On Fri, Mar 13, 2026 at 8:58 PM Shenwei Wang <shenwei.wang@nxp.com> wrote:
>
> > Support the remote devices on the remote processor via the RPMSG bus on
> > i.MX platform.
>
> I think v12 looks pretty good, if Arnaud gives his ACK on this patch
> series I think it's ripe for merge.

Please wait until Andrew and I have provided our RBs before merging.

>
> Yours,
> Linus Walleij

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

[Linus Walleij]

On Mon, Mar 16, 2026 at 5:01 PM Mathieu Poirier
<mathieu.poirier@linaro.org> wrote:
> [Adding Andrew Lunn]
>
> On Mon, 16 Mar 2026 at 08:23, Linus Walleij <linusw@kernel.org> wrote:
> >
> > Hi Shenwei,
> >
> > On Fri, Mar 13, 2026 at 8:58 PM Shenwei Wang <shenwei.wang@nxp.com> wrote:
> >
> > > Support the remote devices on the remote processor via the RPMSG bus on
> > > i.MX platform.
> >
> > I think v12 looks pretty good, if Arnaud gives his ACK on this patch
> > series I think it's ripe for merge.
>
> Please wait until Andrew and I have provided our RBs before merging.

Fair enough, I think either you will all agree or none of you anyway.

Yours,
Linus Walleij