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

[PATCH v16 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 v16:
  - Removed linux/mod_devicetable.h and an extra comma per Uwe's feedback.
  - Added the "packed" attribute to struct rpmsg_gpio_response and set
    "can_sleep = true", per Sashiko's review comments.
  - Removed the channel node from the rpmsg node in dts layout.

Changes in v15:
 - Update Kconfig description per AndrewD and Julian's feedback
 - Enable the GPIO driver even without a DT node per AndrewD's feedback
 - Update gpio-rpmsg.rst per Mathieu’s feedback
 - Code cleanup according the new gpio-rpmsg.rst and sashiko-bot's review
   feedback

Changes in v14:
 - Update gpio-rpmsg.rst per Mathieu’s feedback.
 - Align the rpmsg-gpio driver with the revised gpio-rpmsg.rst.
 - Modify rpmsg-core to enable prefix-based matching of RPMSG device IDs.

Changes in v13:
 - drop the support for legacy NXP firmware.
 - remove the fixed_up hooks from the rpmsg gpio driver.
 - code cleanup.

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
  rpmsg: core: match rpmsg device IDs by prefix
  gpio: rpmsg: add generic rpmsg GPIO driver
  arm64: dts: imx8ulp: Add rpmsg node under imx_rproc

 .../devicetree/bindings/gpio/gpio-rpmsg.yaml  |  55 ++
 .../bindings/remoteproc/fsl,imx-rproc.yaml    |  46 ++
 Documentation/driver-api/gpio/gpio-rpmsg.rst  | 242 ++++++++
 Documentation/driver-api/gpio/index.rst       |   1 +
 arch/arm64/boot/dts/freescale/imx8ulp.dtsi    |  23 +
 drivers/gpio/Kconfig                          |  17 +
 drivers/gpio/Makefile                         |   1 +
 drivers/gpio/gpio-rpmsg.c                     | 585 ++++++++++++++++++
 drivers/rpmsg/rpmsg_core.c                    |   4 +-
 9 files changed, 973 insertions(+), 1 deletion(-)
 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 v16 1/5] docs: driver-api: gpio: rpmsg gpio driver over rpmsg bus

[Shenwei Wang]

From: Shenwei Wang <shenwei.wang@nxp.com>

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 | 242 +++++++++++++++++++
 Documentation/driver-api/gpio/index.rst      |   1 +
 2 files changed, 243 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..9af75cc759ba
--- /dev/null
+++ b/Documentation/driver-api/gpio/gpio-rpmsg.rst
@@ -0,0 +1,242 @@
+.. 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 out message to the remote consists of a 8-byte packet with the
+following layout:
+
+.. code-block:: none
+
+   +------+------+------+------+------+------+------+------+
+   | 0x00 | 0x01 | 0x02 | 0x03 | 0x04 | 0x05 | 0x06 | 0x07 |
+   |     cmd     |    line     |           value           |
+   +------+------+------+------+------+------+------+------+
+
+- **cmd**: Little endian. Command type.
+
+- **line**: Little endian. The GPIO line (pin) index.
+
+- **value**: Little endian. See details in the command description below.
+
+
+The in message from the remote has the following layout:
+
+.. code-block:: none
+
+   +------+--------+--------+
+   | 0x00 |  0x01  |  0x02  |
+   | type |     payload     |
+   +------+--------+--------+
+
+- **type**: Message types.
+  - 0: Command reply messages.
+  - 1: Interrupt messages.
+
+- **payload**: See details below.
+
+GPIO Commands
+-------------
+
+Commands are specified in the **Cmd** field.
+
+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
+-ETIMEDOUT.
+
+GET_DIRECTION (Cmd=2)
+~~~~~~~~~~~~~~~~~~~~~
+
+**Request:**
+
+.. code-block:: none
+
+   +------+------+------+------+------+------+------+------+
+   | 0x00 | 0x01 | 0x02 | 0x03 | 0x04 | 0x05 | 0x06 | 0x07 |
+   |      2      |    line     |             0             |
+   +------+------+------+------+------+------+------+------+
+
+**Reply:**
+
+.. code-block:: none
+
+   +------+--------+--------+
+   | 0x00 |  0x01  |  0x02  |
+   |   0  | status | value  |
+   +------+--------+--------+
+
+- **status**:
+
+  - 0: Ok
+  - 1: Error
+
+- **value**: Direction.
+
+  - 0: None
+  - 1: Output
+  - 2: Input
+
+
+SET_DIRECTION (Cmd=3)
+~~~~~~~~~~~~~~~~~~~~~
+
+**Request:**
+
+.. code-block:: none
+
+   +------+------+------+------+------+------+------+------+
+   | 0x00 | 0x01 | 0x02 | 0x03 | 0x04 | 0x05 | 0x06 | 0x07 |
+   |      3      |    line     |           value           |
+   +------+------+------+------+------+------+------+------+
+
+- **value**: Direction.
+
+  - 0: None
+  - 1: Output
+  - 2: Input
+
+**Reply:**
+
+.. code-block:: none
+
+   +------+--------+--------+
+   | 0x00 |  0x01  |  0x02  |
+   |   0  | status |    0   |
+   +------+--------+--------+
+
+- **status**:
+
+  - 0: Ok
+  - 1: Error
+
+
+GET_VALUE (Cmd=4)
+~~~~~~~~~~~~~~~~~
+
+**Request:**
+
+.. code-block:: none
+
+   +------+------+------+------+------+------+------+------+
+   | 0x00 | 0x01 | 0x02 | 0x03 | 0x04 | 0x05 | 0x06 | 0x07 |
+   |      4      |    line     |             0             |
+   +------+------+------+------+------+------+------+------+
+
+**Reply:**
+
+.. code-block:: none
+
+   +------+--------+--------+
+   | 0x00 |  0x01  |  0x02  |
+   |   0  | status | value  |
+   +------+--------+--------+
+
+- **status**:
+
+  - 0: Ok
+  - 1: Error
+
+- **value**: Level.
+
+  - 0: Low
+  - 1: High
+
+
+SET_VALUE (Cmd=5)
+~~~~~~~~~~~~~~~~~
+
+**Request:**
+
+.. code-block:: none
+
+   +------+------+------+------+------+------+------+------+
+   | 0x00 | 0x01 | 0x02 | 0x03 | 0x04 | 0x05 | 0x06 | 0x07 |
+   |      5      |    line     |           value           |
+   +------+------+------+------+------+------+------+------+
+
+- **value**: Output level.
+
+  - 0: Low
+  - 1: High
+
+**Reply:**
+
+.. code-block:: none
+
+   +------+--------+--------+
+   | 0x00 |  0x01  |  0x02  |
+   |   0  | status |    0   |
+   +------+--------+--------+
+
+- **status**:
+
+  - 0: Ok
+  - 1: Error
+
+
+SET_IRQ_TYPE (Cmd=6)
+~~~~~~~~~~~~~~~~~~~~
+
+**Request:**
+
+.. code-block:: none
+
+   +------+------+------+------+------+------+------+------+
+   | 0x00 | 0x01 | 0x02 | 0x03 | 0x04 | 0x05 | 0x06 | 0x07 |
+   |      6      |    line     |           value           |
+   +------+------+------+------+------+------+------+------+
+
+- **value**: 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
+
+**Reply:**
+
+.. code-block:: none
+
+   +------+--------+--------+
+   | 0x00 |  0x01  |  0x02  |
+   |   0  | status |    0   |
+   +------+--------+--------+
+
+- **status**:
+
+  - 0: Ok
+  - 1: Error
+
+
+Interrupt Messages
+------------------
+
+Interrupt messages are sent by the remote core and they have
+**Type=1 (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.
+
+.. code-block:: none
+
+   +------+------+--------+
+   | 0x00 | 0x01 |  0x02  |
+   |   1  |     line      |
+   +------+------+--------+
+
+- **line**: Little endian. The GPIO line (pin) index.
+
diff --git a/Documentation/driver-api/gpio/index.rst b/Documentation/driver-api/gpio/index.rst
index bee58f709b9a..e5eb1f82f01f 100644
--- a/Documentation/driver-api/gpio/index.rst
+++ b/Documentation/driver-api/gpio/index.rst
@@ -16,6 +16,7 @@ Contents:
    drivers-on-gpio
    bt8xxgpio
    pca953x
+   gpio-rpmsg
 
 Core
 ====
-- 
2.43.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] gpio-rpmsg: lost edge interrupts due to SET_IRQ_TYPE unmasking
- [Medium] gpio-rpmsg: unaligned 16-bit line field in interrupt message
--

commit 2049dbbf3b708aaa525f92e4899cb8a65c3fd69f
Author: Shenwei Wang <shenwei.wang@nxp.com>

docs: driver-api: gpio: rpmsg gpio driver over rpmsg bus

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

> diff --git a/Documentation/driver-api/gpio/gpio-rpmsg.rst b/Documentation/driver-api/gpio/gpio-rpmsg.rst
> new file mode 100644
> index 0000000000000..9af75cc759ba7
> --- /dev/null
> +++ b/Documentation/driver-api/gpio/gpio-rpmsg.rst

[ ... ]

> +Interrupt Messages
> +------------------
> +
> +Interrupt messages are sent by the remote core and they have
> +**Type=1 (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.

[Severity: High]
Does this mechanism risk losing edge interrupts?

If a secondary edge interrupt occurs while the interrupt is masked during
processing on the Linux side, it will be latched by the remote hardware.

When the firmware receives the SET_IRQ_TYPE command as an unmask signal,
writing the trigger configuration to hardware will inadvertently clear this
latched edge on many platforms, such as the i.MX platforms this driver
targets.

Should the protocol separate MASK and UNMASK commands from configuration
commands to prevent clearing latched pending bits or resetting the
edge-detection state machine?

> +
> +.. code-block:: none
> +
> +   +------+------+--------+
> +   | 0x00 | 0x01 |  0x02  |
> +   |   1  |     line      |
> +   +------+------+--------+

[Severity: Medium]
Will this message layout cause hardware faults or performance penalties on
remote processors?

The layout places the 2-byte line field immediately after the 1-byte type
field, creating an unaligned multi-byte field. Without padding, this requires
packed structs on Linux and could cause hardware faults or performance
penalties on remote firmware processors lacking hardware unaligned access
support (like ARM Cortex-M0 or M0+).

Should a 1-byte padding field be introduced to ensure natural alignment and
cross-architecture compatibility?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260923184255.299034-1-shenwei.wang@oss.nxp.com?part=1

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

[Shenwei Wang]

From: Shenwei Wang <shenwei.wang@nxp.com>

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    | 46 ++++++++++++++++
 2 files changed, 101 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..41eb2e149942
--- /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"
+  - gpio-controller
+
+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 c18f71b64889..5c82255b92a5 100644
--- a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
@@ -88,6 +88,29 @@ 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 devices running on the bus.
+    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
 
@@ -150,5 +173,28 @@ examples:
                 &mu 3 1>;
       memory-region = <&vdev0buffer>, <&vdev0vring0>, <&vdev0vring1>, <&rsc_table>;
       syscon = <&src>;
+
+      rpmsg {
+        #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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dt-bindings: architecture and commit message mismatch
--

commit a7843220c66c7f04da43c050fb047567ab9e7e53
Author: Shenwei Wang <shenwei.wang@nxp.com>

dt-bindings: remoteproc: imx_rproc: Add "rpmsg" subnode support

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.

[Severity: Medium]
Does this description match the implemented schema?

The commit message explicitly claims a 3-level device tree hierarchy where the
channel node name strictly matches the remote processor's channel name, and
remote devices are defined as child nodes under the channel.

However, the schema implements a 2-level hierarchy where the remote device is a
direct child of the rpmsg node, uses standard DT unit addresses instead of
RPMSG channel names, and explicitly forbids child nodes.

> diff --git a/Documentation/devicetree/bindings/gpio/gpio-rpmsg.yaml b/Documentation/devicetree/bindings/gpio/gpio-rpmsg.yaml
> new file mode 100644
> index 0000000000000..41eb2e1499429
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-rpmsg.yaml

[ ... ]

> +allOf:
> +  - $ref: /schemas/gpio/gpio.yaml#
> +
> +unevaluatedProperties: false

[Severity: Medium]
Does this explicitly prevent the addition of remote devices as child nodes?

The lack of child node definitions combined with unevaluatedProperties: false
breaks the design described in the commit message where remote devices are
defined as child nodes.

> diff --git a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
> index c18f71b648890..5c82255b92a50 100644
> --- a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
> +++ b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml

[ ... ]

> @@ -88,6 +88,29 @@ 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 devices running on the bus.
> +    properties:
> +      '#address-cells':
> +        const: 1
> +
> +      '#size-cells':
> +        const: 0
> +
> +    patternProperties:
> +      "^gpio@[0-9a-f]+$":
> +        type: object
> +        $ref: /schemas/gpio/gpio-rpmsg.yaml#
> +        unevaluatedProperties: false

[Severity: Medium]
Does this enforce GPIO controllers as direct children of the rpmsg node?

By using standard DT unit addresses like gpio@0 directly under the rpmsg
node, this contradicts the claimed 3-level hierarchy and literal
channel-naming convention described in the commit message.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260923184255.299034-1-shenwei.wang@oss.nxp.com?part=2

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

[Conor Dooley]

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

On Wed, Sep 23, 2026 at 01:42:29PM -0500, Shenwei Wang wrote:
> From: Shenwei Wang <shenwei.wang@nxp.com>
> 
> 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    | 46 ++++++++++++++++
>  2 files changed, 101 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..41eb2e149942
> --- /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

Is this even used? Your dts patch doesn't have it.

> +          - const: rpmsg-gpio
> +      - const: rpmsg-gpio

Sounds like, if linux are the arbiters of what this protocol is,
"linux," should be used as a vendor prefix.

> +
> +  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"
> +  - gpio-controller
> +
> +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 c18f71b64889..5c82255b92a5 100644
> --- a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
> +++ b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
> @@ -88,6 +88,29 @@ 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 devices running on the bus.
> +    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
>  
> @@ -150,5 +173,28 @@ examples:
>                  &mu 3 1>;
>        memory-region = <&vdev0buffer>, <&vdev0vring0>, <&vdev0vring1>, <&rsc_table>;
>        syscon = <&src>;
> +
> +      rpmsg {
> +        #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
> 

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

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

[Rob Herring]

On Wed, Sep 23, 2026 at 01:42:29PM -0500, Shenwei Wang wrote:
> From: Shenwei Wang <shenwei.wang@nxp.com>
> 
> 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.

Sorry, but DT defines the names of nodes. If it's a gpio-controller, 
then 'gpio'.

I still don't understand where the unit address gets defined. 0 and 1 
look a bit made up. I'm sure you explained it before, but *this patch* 
needs to explain it. When you define a second protocol, the 0 and 1 
addresses are taken already, so you can't have 'clock-controller@0' for 
example.

Rob

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

[Mathieu Poirier]

On Mon, 5 Oct 2026 at 21:07, Rob Herring <robh@kernel.org> wrote:
>
> On Wed, Sep 23, 2026 at 01:42:29PM -0500, Shenwei Wang wrote:
> > From: Shenwei Wang <shenwei.wang@nxp.com>
> >
> > 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.
>
> Sorry, but DT defines the names of nodes. If it's a gpio-controller,
> then 'gpio'.
>
> I still don't understand where the unit address gets defined. 0 and 1
> look a bit made up. I'm sure you explained it before, but *this patch*
> needs to explain it. When you define a second protocol, the 0 and 1
> addresses are taken already, so you can't have 'clock-controller@0' for
> example.
>

I had a conversation with Krzysztof about this here in Prague.  We
have decided to adopt the bindings proposed by Francesco in this
thread [1].

[1].  https://lore.kernel.org/all/20260916-remoteproc_virtio_map-v1-8-dac8c5eb4aa9@valla.it/

> Rob

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

[Rob Herring]

On Tue, Oct 06, 2026 at 09:18:23AM -0600, Mathieu Poirier wrote:
> On Mon, 5 Oct 2026 at 21:07, Rob Herring <robh@kernel.org> wrote:
> >
> > On Wed, Sep 23, 2026 at 01:42:29PM -0500, Shenwei Wang wrote:
> > > From: Shenwei Wang <shenwei.wang@nxp.com>
> > >
> > > 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.
> >
> > Sorry, but DT defines the names of nodes. If it's a gpio-controller,
> > then 'gpio'.
> >
> > I still don't understand where the unit address gets defined. 0 and 1
> > look a bit made up. I'm sure you explained it before, but *this patch*
> > needs to explain it. When you define a second protocol, the 0 and 1
> > addresses are taken already, so you can't have 'clock-controller@0' for
> > example.
> >
> 
> I had a conversation with Krzysztof about this here in Prague.  We
> have decided to adopt the bindings proposed by Francesco in this
> thread [1].

Same question applies there. Where do the 0 and 1 addresses come from?  
Will reply on that thread.

Rob

> 
> [1].  https://lore.kernel.org/all/20260916-remoteproc_virtio_map-v1-8-dac8c5eb4aa9@valla.it/
> 
> > Rob

[PATCH v16 3/5] rpmsg: core: match rpmsg device IDs by prefix

[Shenwei Wang]

From: Shenwei Wang <shenwei.wang@nxp.com>

The current rpmsg_id_match() implementation requires an exact
string match between the driver id_table entry and the rpmsg
device name using strncmp() with RPMSG_NAME_SIZE.

This makes it impossible for a driver to match a group of
rpmsg devices sharing a common prefix (e.g. dynamically
suffixed channel names).

Update the matching logic to compare only the length of the
id->name string, allowing id_table entries to act as prefixes.
This enables drivers to bind to devices whose names start with
the specified id->name.

The implementation is copied from a reply by Mathieu.

Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
---
 drivers/rpmsg/rpmsg_core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c
index 04bfcc75f05b..97a4db52b040 100644
--- a/drivers/rpmsg/rpmsg_core.c
+++ b/drivers/rpmsg/rpmsg_core.c
@@ -386,7 +386,9 @@ ATTRIBUTE_GROUPS(rpmsg_dev);
 static inline int rpmsg_id_match(const struct rpmsg_device *rpdev,
 				  const struct rpmsg_device_id *id)
 {
-	return strncmp(id->name, rpdev->id.name, RPMSG_NAME_SIZE) == 0;
+	size_t len = strnlen(id->name, RPMSG_NAME_SIZE);
+
+	return strncmp(id->name, rpdev->id.name, len) == 0;
 }
 
 /* match rpmsg channel and rpmsg driver */
-- 
2.43.0

Re: [PATCH v16 3/5] rpmsg: core: match rpmsg device IDs by prefix

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] rpmsg: core: global prefix matching causes driver binding collisions
- [High] rpmsg: core: broken module autoloading for dynamically suffixed devices
--

commit b3b593160f090b0f9bdaa3003ed014a224f40238
Author: Shenwei Wang <shenwei.wang@nxp.com>

rpmsg: core: match rpmsg device IDs by prefix

The current rpmsg_id_match() implementation requires an exact string match
between the driver id_table entry and the rpmsg device name. This commit
updates the matching logic to compare only the length of the id->name string,
allowing id_table entries to act as prefixes for matching devices.

> diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c
> index 04bfcc75f05b4..97a4db52b0401 100644
> --- a/drivers/rpmsg/rpmsg_core.c
> +++ b/drivers/rpmsg/rpmsg_core.c
> @@ -386,7 +386,9 @@ ATTRIBUTE_GROUPS(rpmsg_dev);
>  static inline int rpmsg_id_match(const struct rpmsg_device *rpdev,
>  				  const struct rpmsg_device_id *id)
>  {
> -	return strncmp(id->name, rpdev->id.name, RPMSG_NAME_SIZE) == 0;
> +	size_t len = strnlen(id->name, RPMSG_NAME_SIZE);
> +
> +	return strncmp(id->name, rpdev->id.name, len) == 0;
>  }
>  
>  /* match rpmsg channel and rpmsg driver */

[Severity: High]
Since this unconditionally changes bus matching to prefix matching for all
drivers, will this cause a regression for existing drivers that rely on
exact match semantics?

For example, if a driver like rpmsg_wwan_ctrl expects an exact match:

drivers/net/wwan/rpmsg_wwan_ctrl.c:
static const struct rpmsg_device_id rpmsg_wwan_ctrl_id_table[] = {
    { .name = "DATA1", .driver_data = WWAN_PORT_AT },
    ...
};

Could this now erroneously bind to overlapping device names like "DATA10",
leading to unpredictable behavior? This breakage in exact match semantics
does not appear to be addressed in subsequent patches in the series.

[Severity: High]
With driver matching now operating by prefix, does this introduce a
regression for module autoloading with dynamically suffixed devices?

When the device is discovered, rpmsg_uevent() emits the full device name:

drivers/rpmsg/rpmsg_core.c:rpmsg_uevent() {
    ...
    return add_uevent_var(env, "MODALIAS=" RPMSG_DEVICE_MODALIAS_FMT,
                          rpdev->id.name);
}

However, scripts/mod/file2alias.c:do_rpmsg_entry() does not append a
wildcard when generating the alias:

scripts/mod/file2alias.c:do_rpmsg_entry() {
    ...
    module_alias_printf(mod, false, RPMSG_DEVICE_MODALIAS_FMT, *name);
}

Because the exported module alias is strictly exact (e.g., alias
rpmsg:rpmsg-io-), will udev fail to load the module for a device named
"rpmsg-io-0" since the required wildcard '*' is missing? This omission
doesn't seem to be fixed later in the patch series.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260923184255.299034-1-shenwei.wang@oss.nxp.com?part=3

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

[Shenwei Wang]

From: Shenwei Wang <shenwei.wang@nxp.com>

On an AMP platform, the system may include multiple processors:
	- MCUs 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 | 580 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 598 insertions(+)
 create mode 100644 drivers/gpio/gpio-rpmsg.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index f03c05288376..9fb8ab4d3616 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1954,6 +1954,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 Y here to enable GPIO controllers exposed over an RPMSG
+	  communication channel to a remote processor. GPIO operations
+	  and interrupt configuration requests are forwarded to the
+	  remote side through RPMSG, allowing Linux to control GPIOs
+	  implemented by a remote firmware.
+
+endmenu
+
 menu "SPI GPIO expanders"
 	depends on SPI_MASTER
 
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index fa14581e3995..89d26f1aaf1c 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -159,6 +159,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_GPIO_RTD1625)		+= gpio-rtd1625.o
 obj-$(CONFIG_ARCH_SA1100)		+= gpio-sa1100.o
diff --git a/drivers/gpio/gpio-rpmsg.c b/drivers/gpio/gpio-rpmsg.c
new file mode 100644
index 000000000000..076af781f3bf
--- /dev/null
+++ b/drivers/gpio/gpio-rpmsg.c
@@ -0,0 +1,580 @@
+// 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/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 GPIOS_PER_PORT_MAX		32
+#define RPMSG_TIMEOUT			1000
+
+/* GPIO Receive MSG Type */
+#define GPIO_RPMSG_REPLY	0
+#define GPIO_RPMSG_NOTIFY	1
+
+#define CHAN_NAME_PREFIX	"rpmsg-io-"
+#define GPIO_COMPAT_STR		"rpmsg-gpio"
+
+struct __packed rpmsg_gpio_response {
+	__u8 type;
+	union {
+		/* command reply */
+		struct {
+			__u8 status;
+			__u8 value;
+		};
+
+		/* interrupt notification */
+		__le16 line;
+	};
+};
+
+struct rpmsg_gpio_line {
+	u8 irq_shutdown;
+	u8 irq_unmask;
+	u8 irq_mask;
+	u32 irq_type;
+};
+
+struct rpmsg_gpio_port {
+	struct gpio_chip gc;
+	struct rpmsg_device *rpdev;
+	struct virtio_gpio_request *send_msg;
+	struct rpmsg_gpio_response *recv_msg;
+	struct completion cmd_complete;
+	struct mutex lock;
+	struct work_struct eoi_work;
+	DECLARE_BITMAP(pending_eoi, GPIOS_PER_PORT_MAX);
+	u32 ngpios;
+	u32 idx;
+	struct rpmsg_gpio_line lines[GPIOS_PER_PORT_MAX];
+};
+
+static int rpmsg_gpio_send_message(struct rpmsg_gpio_port *port)
+{
+	int ret;
+
+	reinit_completion(&port->cmd_complete);
+
+	ret = rpmsg_send(port->rpdev->ept, port->send_msg, sizeof(*port->send_msg));
+	if (ret) {
+		dev_err(&port->rpdev->dev, "rpmsg_send failed: cmd=%d ret=%d\n",
+			port->send_msg->type, ret);
+		return ret;
+	}
+
+	ret = wait_for_completion_timeout(&port->cmd_complete,
+					  msecs_to_jiffies(RPMSG_TIMEOUT));
+	if (ret == 0) {
+		dev_err(&port->rpdev->dev, "rpmsg_send timeout! cmd=%d\n",
+			port->send_msg->type);
+		return -ETIMEDOUT;
+	}
+
+	if (port->recv_msg->status != VIRTIO_GPIO_STATUS_OK) {
+		dev_err(&port->rpdev->dev, "remote core replies an error: cmd=%d!\n",
+			port->send_msg->type);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static struct virtio_gpio_request *
+rpmsg_gpio_msg_prepare(struct rpmsg_gpio_port *port, u16 line, u16 cmd, u32 val)
+{
+	struct virtio_gpio_request *msg = port->send_msg;
+
+	msg->type = cpu_to_le16(cmd);
+	msg->gpio = cpu_to_le16(line);
+	msg->value = cpu_to_le32(val);
+
+	return msg;
+}
+
+static int rpmsg_gpio_get(struct gpio_chip *gc, unsigned int line)
+{
+	struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
+	int ret;
+
+	guard(mutex)(&port->lock);
+
+	rpmsg_gpio_msg_prepare(port, line, VIRTIO_GPIO_MSG_GET_VALUE, 0);
+
+	ret = rpmsg_gpio_send_message(port);
+	return ret ? ret : port->recv_msg->value;
+}
+
+static int rpmsg_gpio_get_direction(struct gpio_chip *gc, unsigned int line)
+{
+	struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
+	int ret;
+
+	guard(mutex)(&port->lock);
+
+	rpmsg_gpio_msg_prepare(port, line, VIRTIO_GPIO_MSG_GET_DIRECTION, 0);
+
+	ret = rpmsg_gpio_send_message(port);
+	if (ret)
+		return ret;
+
+	switch (port->recv_msg->value) {
+	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);
+
+	guard(mutex)(&port->lock);
+
+	rpmsg_gpio_msg_prepare(port, line, VIRTIO_GPIO_MSG_SET_DIRECTION,
+			       VIRTIO_GPIO_DIRECTION_IN);
+
+	return rpmsg_gpio_send_message(port);
+}
+
+static int rpmsg_gpio_set(struct gpio_chip *gc, unsigned int line, int val)
+{
+	struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
+
+	guard(mutex)(&port->lock);
+
+	rpmsg_gpio_msg_prepare(port, line, VIRTIO_GPIO_MSG_SET_VALUE, val);
+
+	return rpmsg_gpio_send_message(port);
+}
+
+static int rpmsg_gpio_direction_output(struct gpio_chip *gc, unsigned int line, int val)
+{
+	struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
+	int ret;
+
+	guard(mutex)(&port->lock);
+
+	rpmsg_gpio_msg_prepare(port, line, VIRTIO_GPIO_MSG_SET_VALUE, val);
+	ret = rpmsg_gpio_send_message(port);
+	if (ret)
+		return ret;
+
+	rpmsg_gpio_msg_prepare(port, line, VIRTIO_GPIO_MSG_SET_DIRECTION,
+			       VIRTIO_GPIO_DIRECTION_OUT);
+	return rpmsg_gpio_send_message(port);
+}
+
+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;
+
+	switch (type) {
+	case IRQ_TYPE_EDGE_RISING:
+		type = VIRTIO_GPIO_IRQ_TYPE_EDGE_RISING;
+		break;
+	case IRQ_TYPE_EDGE_FALLING:
+		type = VIRTIO_GPIO_IRQ_TYPE_EDGE_FALLING;
+		break;
+	case IRQ_TYPE_EDGE_BOTH:
+		type = VIRTIO_GPIO_IRQ_TYPE_EDGE_BOTH;
+		break;
+	case IRQ_TYPE_LEVEL_LOW:
+		type = VIRTIO_GPIO_IRQ_TYPE_LEVEL_LOW;
+		break;
+	case IRQ_TYPE_LEVEL_HIGH:
+		type = VIRTIO_GPIO_IRQ_TYPE_LEVEL_HIGH;
+		break;
+	default:
+		dev_err(&port->rpdev->dev, "unsupported irq type: %u\n", type);
+		return -EINVAL;
+	}
+
+	port->lines[line].irq_type = type;
+
+	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_eoi_irq(struct irq_data *d)
+{
+	struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
+
+	set_bit(d->hwirq, port->pending_eoi);
+	schedule_work(&port->eoi_work);
+}
+
+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->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);
+	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->lock);
+		return;
+	}
+
+	if (port->lines[line].irq_shutdown) {
+		rpmsg_gpio_msg_prepare(port, line, VIRTIO_GPIO_MSG_IRQ_TYPE,
+				       VIRTIO_GPIO_IRQ_TYPE_NONE);
+		port->lines[line].irq_shutdown = 0;
+	} else {
+		rpmsg_gpio_msg_prepare(port, line, VIRTIO_GPIO_MSG_IRQ_TYPE,
+				       port->lines[line].irq_type);
+
+		if (port->lines[line].irq_unmask)
+			port->lines[line].irq_unmask = 0;
+	}
+
+	rpmsg_gpio_send_message(port);
+	mutex_unlock(&port->lock);
+}
+
+static const struct irq_chip gpio_rpmsg_irq_chip = {
+	.name = "rpmsg-gpio",
+	.irq_mask = gpio_rpmsg_mask_irq,
+	.irq_unmask = gpio_rpmsg_unmask_irq,
+	.irq_eoi = gpio_rpmsg_eoi_irq,
+	.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 gpio_rpmsg_eoi_work(struct work_struct *work)
+{
+	struct rpmsg_gpio_port *port =
+		container_of(work, struct rpmsg_gpio_port, eoi_work);
+	unsigned long pending[BITS_TO_LONGS(GPIOS_PER_PORT_MAX)];
+	unsigned int line;
+
+	guard(mutex)(&port->lock);
+
+	bitmap_copy(pending, port->pending_eoi, port->ngpios);
+	bitmap_zero(port->pending_eoi, port->ngpios);
+
+	for_each_set_bit(line, pending, port->ngpios) {
+		rpmsg_gpio_msg_prepare(port, line, VIRTIO_GPIO_MSG_IRQ_TYPE,
+				       port->lines[line].irq_type);
+
+		if (rpmsg_gpio_send_message(port))
+			dev_err(&port->rpdev->dev, "EOI error for line %u\n", line);
+	}
+}
+
+static int rpmsg_gpiochip_register(struct rpmsg_device *rpdev, u32 idx,
+				   struct device_node *np, const char *name)
+{
+	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 = devm_mutex_init(&rpdev->dev, &port->lock);
+	if (ret)
+		return ret;
+
+	ret = of_property_read_u32(np, "ngpios", &port->ngpios);
+	if (ret || port->ngpios > GPIOS_PER_PORT_MAX)
+		port->ngpios = GPIOS_PER_PORT_MAX;
+
+	port->send_msg = devm_kzalloc(&rpdev->dev,
+				      sizeof(*port->send_msg),
+				      GFP_KERNEL);
+
+	port->recv_msg = devm_kzalloc(&rpdev->dev,
+				      sizeof(*port->recv_msg),
+				      GFP_KERNEL);
+	if (!port->send_msg || !port->recv_msg)
+		return -ENOMEM;
+
+	INIT_WORK(&port->eoi_work, gpio_rpmsg_eoi_work);
+	init_completion(&port->cmd_complete);
+	port->rpdev = rpdev;
+	port->idx = idx;
+
+	gc = &port->gc;
+	gc->owner = THIS_MODULE;
+	gc->parent = &rpdev->dev;
+	gc->fwnode = of_fwnode_handle(np);
+	gc->ngpio = port->ngpios;
+	gc->can_sleep = true;
+	gc->base = -1;
+	gc->label = devm_kasprintf(&rpdev->dev, GFP_KERNEL, "%s-gpio%d",
+				   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->default_type = IRQ_TYPE_NONE;
+	girq->handler = handle_fasteoi_irq;
+
+	dev_set_drvdata(&rpdev->dev, port);
+
+	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_find_child_by_compat_reg(struct device_node *parent, const char *compat, u32 idx)
+{
+	struct device_node *child;
+	u32 reg;
+
+	for_each_available_child_of_node(parent, child) {
+		if (!of_device_is_compatible(child, compat))
+			continue;
+
+		if (of_property_read_u32(child, "reg", &reg))
+			continue;
+
+		if (reg == idx)
+			return child;
+	}
+
+	return NULL;
+}
+
+static struct device_node *
+rpmsg_get_gpio_ofnode(struct rpmsg_device *rpdev, const char *compat, u32 idx)
+{
+	struct device_node *np_chan, *np_rproc, *np_rpmsg;
+	struct rproc *rproc;
+
+	rproc = rproc_get_by_child(&rpdev->dev);
+	if (!rproc)
+		return NULL;
+
+	np_rproc = of_node_get(rproc->dev.of_node);
+	if (!np_rproc && rproc->dev.parent)
+		np_rproc = of_node_get(rproc->dev.parent->of_node);
+
+	if (!np_rproc)
+		return NULL;
+
+	np_rpmsg = of_get_child_by_name(np_rproc, "rpmsg");
+	of_node_put(np_rproc);
+	if (!np_rpmsg)
+		return NULL;
+
+	np_chan = rpmsg_find_child_by_compat_reg(np_rpmsg, compat, idx);
+	of_node_put(np_rpmsg);
+
+	return np_chan;
+}
+
+static int rpmsg_get_gpio_index(const char *name, const char *prefix)
+{
+	const char *p;
+	int base = 10;
+	int val;
+
+	if (!name)
+		return -EINVAL;
+
+	/* Ensure correct prefix */
+	if (!str_has_prefix(name, prefix))
+		return -EINVAL;
+
+	/* Find last '-' */
+	p = strrchr(name, '-');
+
+	if (!p || *(p + 1) == '\0')
+		return -EINVAL;
+
+	if (p[1] == '0' && (p[2] == 'x' || p[2] == 'X'))
+		base = 16;
+
+	if (kstrtoint(p + 1, base, &val))
+		return -EINVAL;
+
+	return val;
+}
+
+static int rpmsg_gpio_channel_callback(struct rpmsg_device *rpdev, void *data,
+				       int len, void *priv, u32 src)
+{
+	struct rpmsg_gpio_response *msg = data;
+	struct rpmsg_gpio_port *port = NULL;
+	u32 line;
+
+	port = dev_get_drvdata(&rpdev->dev);
+
+	if (!port) {
+		dev_err(&rpdev->dev, "port is null\n");
+		return -EINVAL;
+	}
+
+	if (msg->type == GPIO_RPMSG_REPLY) {
+		*port->recv_msg = *msg;
+		complete(&port->cmd_complete);
+	} else if (msg->type == GPIO_RPMSG_NOTIFY) {
+		line = le16_to_cpu(msg->line);
+		generic_handle_domain_irq_safe(port->gc.irq.domain, line);
+	} else {
+		dev_err(&rpdev->dev, "wrong message type (0x%x)\n", msg->type);
+	}
+
+	return 0;
+}
+
+static int rpmsg_gpio_channel_probe(struct rpmsg_device *rpdev)
+{
+	struct device *dev = &rpdev->dev;
+	struct device_node *np;
+	const char *rproc_name;
+	int idx;
+
+	idx = rpmsg_get_gpio_index(rpdev->id.name, CHAN_NAME_PREFIX);
+	if (idx < 0)
+		return -EINVAL;
+
+	if (!dev->of_node) {
+		np = rpmsg_get_gpio_ofnode(rpdev, GPIO_COMPAT_STR, idx);
+		if (np) {
+			dev->of_node = np;
+			set_primary_fwnode(dev, of_fwnode_handle(np));
+			return -EPROBE_DEFER;
+		}
+	}
+
+	rproc_name = rpmsg_get_rproc_node_name(rpdev);
+
+	return rpmsg_gpiochip_register(rpdev, idx, dev->of_node, rproc_name);
+}
+
+static void rpmsg_gpio_channel_remove(struct rpmsg_device *rpdev)
+{
+	struct rpmsg_gpio_port *port = dev_get_drvdata(&rpdev->dev);
+
+	cancel_work_sync(&port->eoi_work);
+}
+
+static struct rpmsg_device_id rpmsg_gpio_channel_id_table[] = {
+	{ .name = CHAN_NAME_PREFIX },
+	{ }
+};
+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,
+	.remove		= rpmsg_gpio_channel_remove,
+	.drv.name	= KBUILD_MODNAME,
+};
+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 v16 4/5] gpio: rpmsg: add generic rpmsg GPIO driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [High] gpio: rpmsg: non-atomic bitmap_zero() clobbers concurrent set_bit() updates
- [High] gpio: rpmsg: premature cancel_work_sync() causes UAF on port struct
- [High] gpio: rpmsg: RPC timeouts break request-response synchronization
- [High] gpio: rpmsg: early drvdata setup causes NULL pointer dereference in probe
- [Medium] gpio: rpmsg: out-of-bounds read in callback due to missing length check
- [Medium] gpio: rpmsg: eoi_work() unmasks interrupts that were just shut down
--

commit aea7aeffb6bed8da7df793b582974e6352af17db
Author: Shenwei Wang <shenwei.wang@nxp.com>
Subject: gpio: rpmsg: add generic rpmsg GPIO driver

This commit adds a generic RPMSG GPIO driver allowing Linux to control
remote processor GPIOs via RPMSG.

> diff --git a/drivers/gpio/gpio-rpmsg.c b/drivers/gpio/gpio-rpmsg.c
> new file mode 100644
> index 0000000000000..076af781f3bfb
> --- /dev/null
> +++ b/drivers/gpio/gpio-rpmsg.c
[ ... ]
> +static int rpmsg_gpio_send_message(struct rpmsg_gpio_port *port)
> +{
> +	int ret;
> +
> +	reinit_completion(&port->cmd_complete);
> +
> +	ret = rpmsg_send(port->rpdev->ept, port->send_msg, sizeof(*port->send_msg));
> +	if (ret) {
> +		dev_err(&port->rpdev->dev, "rpmsg_send failed: cmd=%d ret=%d\n",
> +			port->send_msg->type, ret);
> +		return ret;
> +	}
> +
> +	ret = wait_for_completion_timeout(&port->cmd_complete,
> +					  msecs_to_jiffies(RPMSG_TIMEOUT));
> +	if (ret == 0) {
> +		dev_err(&port->rpdev->dev, "rpmsg_send timeout! cmd=%d\n",
> +			port->send_msg->type);
> +		return -ETIMEDOUT;
> +	}

[Severity: High]
Is it possible for a delayed reply to break the request-response
synchronization?

If wait_for_completion_timeout() times out, the driver aborts the current
request but leaves the channel active. If a new request is subsequently
sent, could a late reply from the timed-out request overwrite *port->recv_msg
in rpmsg_gpio_channel_callback() and falsely wake up the completion for the
new request?

[ ... ]
> +static void gpio_rpmsg_eoi_work(struct work_struct *work)
> +{
> +	struct rpmsg_gpio_port *port =
> +		container_of(work, struct rpmsg_gpio_port, eoi_work);
> +	unsigned long pending[BITS_TO_LONGS(GPIOS_PER_PORT_MAX)];
> +	unsigned int line;
> +
> +	guard(mutex)(&port->lock);
> +
> +	bitmap_copy(pending, port->pending_eoi, port->ngpios);
> +	bitmap_zero(port->pending_eoi, port->ngpios);

[Severity: High]
Could bitmap_zero() clobber concurrent updates?

Since bitmap_zero() expands to a non-atomic memory store, if a new
interrupt arrives and calls set_bit() in gpio_rpmsg_eoi_irq() while this
zeroing is executing, won't the new hardware interrupt get permanently lost?

> +
> +	for_each_set_bit(line, pending, port->ngpios) {
> +		rpmsg_gpio_msg_prepare(port, line, VIRTIO_GPIO_MSG_IRQ_TYPE,
> +				       port->lines[line].irq_type);

[Severity: Medium]
Does this blindly re-enable interrupts that were just shut down?

If free_irq() calls gpio_rpmsg_irq_shutdown() to mask the interrupt right
before or while this async work is executing, could this
VIRTIO_GPIO_MSG_IRQ_TYPE command override the shutdown and cause spurious
wakeups on the remote end?

> +
> +		if (rpmsg_gpio_send_message(port))
> +			dev_err(&port->rpdev->dev, "EOI error for line %u\n", line);
> +	}
> +}
[ ... ]
> +static int rpmsg_gpiochip_register(struct rpmsg_device *rpdev, u32 idx,
> +				   struct device_node *np, const char *name)
> +{
[ ... ]
> +	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->default_type = IRQ_TYPE_NONE;
> +	girq->handler = handle_fasteoi_irq;
> +
> +	dev_set_drvdata(&rpdev->dev, port);
> +
> +	return devm_gpiochip_add_data(&rpdev->dev, gc, port);
> +}

[Severity: High]
Is there a risk of a NULL pointer dereference here if an early interrupt
arrives?

The call to dev_set_drvdata() exposes the port structure to the active RPMSG
endpoint before devm_gpiochip_add_data() initializes port->gc.irq.domain.

If the remote processor sends an interrupt notification at this exact moment,
rpmsg_gpio_channel_callback() could retrieve the port and pass a still-NULL
IRQ domain to generic_handle_domain_irq_safe().

[ ... ]
> +static int rpmsg_gpio_channel_callback(struct rpmsg_device *rpdev, void *data,
> +				       int len, void *priv, u32 src)
> +{
> +	struct rpmsg_gpio_response *msg = data;
> +	struct rpmsg_gpio_port *port = NULL;
> +	u32 line;
> +
> +	port = dev_get_drvdata(&rpdev->dev);
> +
> +	if (!port) {
> +		dev_err(&rpdev->dev, "port is null\n");
> +		return -EINVAL;
> +	}
> +
> +	if (msg->type == GPIO_RPMSG_REPLY) {
> +		*port->recv_msg = *msg;

[Severity: Medium]
Is there a missing bounds check on len?

If the remote processor sends a payload smaller than
sizeof(struct rpmsg_gpio_response), this assignment will perform an
out-of-bounds memory read, potentially processing garbage data. Should we
verify len before dereferencing msg?

> +		complete(&port->cmd_complete);
> +	} else if (msg->type == GPIO_RPMSG_NOTIFY) {
> +		line = le16_to_cpu(msg->line);
> +		generic_handle_domain_irq_safe(port->gc.irq.domain, line);
> +	} else {
> +		dev_err(&rpdev->dev, "wrong message type (0x%x)\n", msg->type);
> +	}
> +
> +	return 0;
> +}
[ ... ]
> +static void rpmsg_gpio_channel_remove(struct rpmsg_device *rpdev)
> +{
> +	struct rpmsg_gpio_port *port = dev_get_drvdata(&rpdev->dev);
> +
> +	cancel_work_sync(&port->eoi_work);
> +}

[Severity: High]
Could this lead to a use-after-free on the port struct when the driver is
unbound?

While cancel_work_sync() runs here, the RPMSG endpoint remains active until
devres tears it down later. If an interrupt arrives immediately after remove()
returns:

rpmsg_gpio_channel_callback()
  -> gpio_rpmsg_eoi_irq()
     -> schedule_work(&port->eoi_work);

This would queue work on a struct that devres is about to free, leaving a
freed work_struct linked in the kernel's workqueue. Should the endpoint be
explicitly disabled before canceling the work?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260923184255.299034-1-shenwei.wang@oss.nxp.com?part=4

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

[Andrew Davis]

On 9/23/26 1:42 PM, Shenwei Wang wrote:
> From: Shenwei Wang <shenwei.wang@nxp.com>
> 
> On an AMP platform, the system may include multiple processors:
> 	- MCUs 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 | 580 ++++++++++++++++++++++++++++++++++++++
>   3 files changed, 598 insertions(+)
>   create mode 100644 drivers/gpio/gpio-rpmsg.c
> 
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index f03c05288376..9fb8ab4d3616 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -1954,6 +1954,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 Y here to enable GPIO controllers exposed over an RPMSG
> +	  communication channel to a remote processor. GPIO operations
> +	  and interrupt configuration requests are forwarded to the
> +	  remote side through RPMSG, allowing Linux to control GPIOs
> +	  implemented by a remote firmware.
> +
> +endmenu
> +
>   menu "SPI GPIO expanders"
>   	depends on SPI_MASTER
>   
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index fa14581e3995..89d26f1aaf1c 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -159,6 +159,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_GPIO_RTD1625)		+= gpio-rtd1625.o
>   obj-$(CONFIG_ARCH_SA1100)		+= gpio-sa1100.o
> diff --git a/drivers/gpio/gpio-rpmsg.c b/drivers/gpio/gpio-rpmsg.c
> new file mode 100644
> index 000000000000..076af781f3bf
> --- /dev/null
> +++ b/drivers/gpio/gpio-rpmsg.c
> @@ -0,0 +1,580 @@
> +// 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/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 GPIOS_PER_PORT_MAX		32
> +#define RPMSG_TIMEOUT			1000
> +
> +/* GPIO Receive MSG Type */
> +#define GPIO_RPMSG_REPLY	0
> +#define GPIO_RPMSG_NOTIFY	1
> +
> +#define CHAN_NAME_PREFIX	"rpmsg-io-"
> +#define GPIO_COMPAT_STR		"rpmsg-gpio"
> +
> +struct __packed rpmsg_gpio_response {
> +	__u8 type;
> +	union {
> +		/* command reply */
> +		struct {
> +			__u8 status;
> +			__u8 value;
> +		};
> +
> +		/* interrupt notification */
> +		__le16 line;
> +	};
> +};
> +
> +struct rpmsg_gpio_line {
> +	u8 irq_shutdown;
> +	u8 irq_unmask;
> +	u8 irq_mask;
> +	u32 irq_type;
> +};
> +
> +struct rpmsg_gpio_port {
> +	struct gpio_chip gc;
> +	struct rpmsg_device *rpdev;
> +	struct virtio_gpio_request *send_msg;
> +	struct rpmsg_gpio_response *recv_msg;
> +	struct completion cmd_complete;
> +	struct mutex lock;
> +	struct work_struct eoi_work;
> +	DECLARE_BITMAP(pending_eoi, GPIOS_PER_PORT_MAX);
> +	u32 ngpios;
> +	u32 idx;
> +	struct rpmsg_gpio_line lines[GPIOS_PER_PORT_MAX];
> +};
> +
> +static int rpmsg_gpio_send_message(struct rpmsg_gpio_port *port)
> +{
> +	int ret;
> +
> +	reinit_completion(&port->cmd_complete);
> +
> +	ret = rpmsg_send(port->rpdev->ept, port->send_msg, sizeof(*port->send_msg));
> +	if (ret) {
> +		dev_err(&port->rpdev->dev, "rpmsg_send failed: cmd=%d ret=%d\n",
> +			port->send_msg->type, ret);
> +		return ret;
> +	}
> +
> +	ret = wait_for_completion_timeout(&port->cmd_complete,
> +					  msecs_to_jiffies(RPMSG_TIMEOUT));
> +	if (ret == 0) {
> +		dev_err(&port->rpdev->dev, "rpmsg_send timeout! cmd=%d\n",
> +			port->send_msg->type);
> +		return -ETIMEDOUT;
> +	}
> +
> +	if (port->recv_msg->status != VIRTIO_GPIO_STATUS_OK) {
> +		dev_err(&port->rpdev->dev, "remote core replies an error: cmd=%d!\n",
> +			port->send_msg->type);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static struct virtio_gpio_request *
> +rpmsg_gpio_msg_prepare(struct rpmsg_gpio_port *port, u16 line, u16 cmd, u32 val)
> +{
> +	struct virtio_gpio_request *msg = port->send_msg;
> +
> +	msg->type = cpu_to_le16(cmd);
> +	msg->gpio = cpu_to_le16(line);
> +	msg->value = cpu_to_le32(val);
> +
> +	return msg;
> +}
> +
> +static int rpmsg_gpio_get(struct gpio_chip *gc, unsigned int line)
> +{
> +	struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
> +	int ret;
> +
> +	guard(mutex)(&port->lock);
> +
> +	rpmsg_gpio_msg_prepare(port, line, VIRTIO_GPIO_MSG_GET_VALUE, 0);
> +
> +	ret = rpmsg_gpio_send_message(port);
> +	return ret ? ret : port->recv_msg->value;
> +}
> +
> +static int rpmsg_gpio_get_direction(struct gpio_chip *gc, unsigned int line)
> +{
> +	struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
> +	int ret;
> +
> +	guard(mutex)(&port->lock);
> +
> +	rpmsg_gpio_msg_prepare(port, line, VIRTIO_GPIO_MSG_GET_DIRECTION, 0);
> +
> +	ret = rpmsg_gpio_send_message(port);
> +	if (ret)
> +		return ret;
> +
> +	switch (port->recv_msg->value) {
> +	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);
> +
> +	guard(mutex)(&port->lock);
> +
> +	rpmsg_gpio_msg_prepare(port, line, VIRTIO_GPIO_MSG_SET_DIRECTION,
> +			       VIRTIO_GPIO_DIRECTION_IN);
> +
> +	return rpmsg_gpio_send_message(port);
> +}
> +
> +static int rpmsg_gpio_set(struct gpio_chip *gc, unsigned int line, int val)
> +{
> +	struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
> +
> +	guard(mutex)(&port->lock);
> +
> +	rpmsg_gpio_msg_prepare(port, line, VIRTIO_GPIO_MSG_SET_VALUE, val);
> +
> +	return rpmsg_gpio_send_message(port);
> +}
> +
> +static int rpmsg_gpio_direction_output(struct gpio_chip *gc, unsigned int line, int val)
> +{
> +	struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
> +	int ret;
> +
> +	guard(mutex)(&port->lock);
> +
> +	rpmsg_gpio_msg_prepare(port, line, VIRTIO_GPIO_MSG_SET_VALUE, val);
> +	ret = rpmsg_gpio_send_message(port);
> +	if (ret)
> +		return ret;
> +
> +	rpmsg_gpio_msg_prepare(port, line, VIRTIO_GPIO_MSG_SET_DIRECTION,
> +			       VIRTIO_GPIO_DIRECTION_OUT);
> +	return rpmsg_gpio_send_message(port);
> +}
> +
> +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;
> +
> +	switch (type) {
> +	case IRQ_TYPE_EDGE_RISING:
> +		type = VIRTIO_GPIO_IRQ_TYPE_EDGE_RISING;
> +		break;
> +	case IRQ_TYPE_EDGE_FALLING:
> +		type = VIRTIO_GPIO_IRQ_TYPE_EDGE_FALLING;
> +		break;
> +	case IRQ_TYPE_EDGE_BOTH:
> +		type = VIRTIO_GPIO_IRQ_TYPE_EDGE_BOTH;
> +		break;
> +	case IRQ_TYPE_LEVEL_LOW:
> +		type = VIRTIO_GPIO_IRQ_TYPE_LEVEL_LOW;
> +		break;
> +	case IRQ_TYPE_LEVEL_HIGH:
> +		type = VIRTIO_GPIO_IRQ_TYPE_LEVEL_HIGH;
> +		break;
> +	default:
> +		dev_err(&port->rpdev->dev, "unsupported irq type: %u\n", type);
> +		return -EINVAL;
> +	}
> +
> +	port->lines[line].irq_type = type;
> +
> +	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_eoi_irq(struct irq_data *d)
> +{
> +	struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> +
> +	set_bit(d->hwirq, port->pending_eoi);
> +	schedule_work(&port->eoi_work);
> +}
> +
> +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->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);
> +	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->lock);
> +		return;
> +	}
> +
> +	if (port->lines[line].irq_shutdown) {
> +		rpmsg_gpio_msg_prepare(port, line, VIRTIO_GPIO_MSG_IRQ_TYPE,
> +				       VIRTIO_GPIO_IRQ_TYPE_NONE);
> +		port->lines[line].irq_shutdown = 0;
> +	} else {
> +		rpmsg_gpio_msg_prepare(port, line, VIRTIO_GPIO_MSG_IRQ_TYPE,
> +				       port->lines[line].irq_type);
> +
> +		if (port->lines[line].irq_unmask)
> +			port->lines[line].irq_unmask = 0;
> +	}
> +
> +	rpmsg_gpio_send_message(port);
> +	mutex_unlock(&port->lock);
> +}
> +
> +static const struct irq_chip gpio_rpmsg_irq_chip = {
> +	.name = "rpmsg-gpio",
> +	.irq_mask = gpio_rpmsg_mask_irq,
> +	.irq_unmask = gpio_rpmsg_unmask_irq,
> +	.irq_eoi = gpio_rpmsg_eoi_irq,
> +	.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 gpio_rpmsg_eoi_work(struct work_struct *work)
> +{
> +	struct rpmsg_gpio_port *port =
> +		container_of(work, struct rpmsg_gpio_port, eoi_work);
> +	unsigned long pending[BITS_TO_LONGS(GPIOS_PER_PORT_MAX)];
> +	unsigned int line;
> +
> +	guard(mutex)(&port->lock);
> +
> +	bitmap_copy(pending, port->pending_eoi, port->ngpios);
> +	bitmap_zero(port->pending_eoi, port->ngpios);
> +
> +	for_each_set_bit(line, pending, port->ngpios) {
> +		rpmsg_gpio_msg_prepare(port, line, VIRTIO_GPIO_MSG_IRQ_TYPE,
> +				       port->lines[line].irq_type);
> +
> +		if (rpmsg_gpio_send_message(port))
> +			dev_err(&port->rpdev->dev, "EOI error for line %u\n", line);
> +	}
> +}
> +
> +static int rpmsg_gpiochip_register(struct rpmsg_device *rpdev, u32 idx,
> +				   struct device_node *np, const char *name)
> +{
> +	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 = devm_mutex_init(&rpdev->dev, &port->lock);
> +	if (ret)
> +		return ret;
> +
> +	ret = of_property_read_u32(np, "ngpios", &port->ngpios);
> +	if (ret || port->ngpios > GPIOS_PER_PORT_MAX)
> +		port->ngpios = GPIOS_PER_PORT_MAX;
> +
> +	port->send_msg = devm_kzalloc(&rpdev->dev,
> +				      sizeof(*port->send_msg),
> +				      GFP_KERNEL);
> +
> +	port->recv_msg = devm_kzalloc(&rpdev->dev,
> +				      sizeof(*port->recv_msg),
> +				      GFP_KERNEL);
> +	if (!port->send_msg || !port->recv_msg)
> +		return -ENOMEM;
> +
> +	INIT_WORK(&port->eoi_work, gpio_rpmsg_eoi_work);
> +	init_completion(&port->cmd_complete);
> +	port->rpdev = rpdev;
> +	port->idx = idx;
> +
> +	gc = &port->gc;
> +	gc->owner = THIS_MODULE;
> +	gc->parent = &rpdev->dev;
> +	gc->fwnode = of_fwnode_handle(np);
> +	gc->ngpio = port->ngpios;
> +	gc->can_sleep = true;
> +	gc->base = -1;
> +	gc->label = devm_kasprintf(&rpdev->dev, GFP_KERNEL, "%s-gpio%d",
> +				   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->default_type = IRQ_TYPE_NONE;
> +	girq->handler = handle_fasteoi_irq;
> +
> +	dev_set_drvdata(&rpdev->dev, port);
> +
> +	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_find_child_by_compat_reg(struct device_node *parent, const char *compat, u32 idx)
> +{
> +	struct device_node *child;
> +	u32 reg;
> +
> +	for_each_available_child_of_node(parent, child) {
> +		if (!of_device_is_compatible(child, compat))
> +			continue;
> +
> +		if (of_property_read_u32(child, "reg", &reg))
> +			continue;
> +
> +		if (reg == idx)
> +			return child;
> +	}
> +
> +	return NULL;
> +}
> +
> +static struct device_node *
> +rpmsg_get_gpio_ofnode(struct rpmsg_device *rpdev, const char *compat, u32 idx)
> +{
> +	struct device_node *np_chan, *np_rproc, *np_rpmsg;
> +	struct rproc *rproc;
> +
> +	rproc = rproc_get_by_child(&rpdev->dev);
> +	if (!rproc)
> +		return NULL;
> +
> +	np_rproc = of_node_get(rproc->dev.of_node);
> +	if (!np_rproc && rproc->dev.parent)
> +		np_rproc = of_node_get(rproc->dev.parent->of_node);
> +
> +	if (!np_rproc)
> +		return NULL;
> +
> +	np_rpmsg = of_get_child_by_name(np_rproc, "rpmsg");
> +	of_node_put(np_rproc);
> +	if (!np_rpmsg)
> +		return NULL;
> +
> +	np_chan = rpmsg_find_child_by_compat_reg(np_rpmsg, compat, idx);
> +	of_node_put(np_rpmsg);
> +
> +	return np_chan;
> +}
> +
> +static int rpmsg_get_gpio_index(const char *name, const char *prefix)
> +{
> +	const char *p;
> +	int base = 10;
> +	int val;
> +
> +	if (!name)
> +		return -EINVAL;
> +
> +	/* Ensure correct prefix */
> +	if (!str_has_prefix(name, prefix))
> +		return -EINVAL;
> +
> +	/* Find last '-' */
> +	p = strrchr(name, '-');
> +
> +	if (!p || *(p + 1) == '\0')
> +		return -EINVAL;
> +
> +	if (p[1] == '0' && (p[2] == 'x' || p[2] == 'X'))
> +		base = 16;
> +
> +	if (kstrtoint(p + 1, base, &val))
> +		return -EINVAL;
> +
> +	return val;
> +}
> +
> +static int rpmsg_gpio_channel_callback(struct rpmsg_device *rpdev, void *data,
> +				       int len, void *priv, u32 src)
> +{
> +	struct rpmsg_gpio_response *msg = data;
> +	struct rpmsg_gpio_port *port = NULL;
> +	u32 line;
> +
> +	port = dev_get_drvdata(&rpdev->dev);
> +
> +	if (!port) {
> +		dev_err(&rpdev->dev, "port is null\n");
> +		return -EINVAL;
> +	}
> +
> +	if (msg->type == GPIO_RPMSG_REPLY) {
> +		*port->recv_msg = *msg;
> +		complete(&port->cmd_complete);
> +	} else if (msg->type == GPIO_RPMSG_NOTIFY) {
> +		line = le16_to_cpu(msg->line);
> +		generic_handle_domain_irq_safe(port->gc.irq.domain, line);
> +	} else {
> +		dev_err(&rpdev->dev, "wrong message type (0x%x)\n", msg->type);
> +	}
> +
> +	return 0;
> +}
> +
> +static int rpmsg_gpio_channel_probe(struct rpmsg_device *rpdev)
> +{
> +	struct device *dev = &rpdev->dev;
> +	struct device_node *np;
> +	const char *rproc_name;
> +	int idx;
> +
> +	idx = rpmsg_get_gpio_index(rpdev->id.name, CHAN_NAME_PREFIX);
> +	if (idx < 0)
> +		return -EINVAL;
> +
> +	if (!dev->of_node) {
> +		np = rpmsg_get_gpio_ofnode(rpdev, GPIO_COMPAT_STR, idx);
> +		if (np) {
> +			dev->of_node = np;
> +			set_primary_fwnode(dev, of_fwnode_handle(np));
> +			return -EPROBE_DEFER;
> +		}
> +	}
> +
> +	rproc_name = rpmsg_get_rproc_node_name(rpdev);
> +
> +	return rpmsg_gpiochip_register(rpdev, idx, dev->of_node, rproc_name);

Why split your probe function in half like this? Where do you draw the
split line? Why not fetch rproc_name in the second half function, etc..

Should be one function.

> +}
> +
> +static void rpmsg_gpio_channel_remove(struct rpmsg_device *rpdev)
> +{
> +	struct rpmsg_gpio_port *port = dev_get_drvdata(&rpdev->dev);
> +
> +	cancel_work_sync(&port->eoi_work);

Init the work with devm_work_autocancel() and then you can drop the
remove function.

Andrew

> +}
> +
> +static struct rpmsg_device_id rpmsg_gpio_channel_id_table[] = {
> +	{ .name = CHAN_NAME_PREFIX },
> +	{ }
> +};
> +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,
> +	.remove		= rpmsg_gpio_channel_remove,
> +	.drv.name	= KBUILD_MODNAME,
> +};
> +module_rpmsg_driver(rpmsg_gpio_channel_client);
> +
> +MODULE_AUTHOR("Shenwei Wang <shenwei.wang@nxp.com>");
> +MODULE_DESCRIPTION("generic rpmsg gpio driver");
> +MODULE_LICENSE("GPL");

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

[Shenwei Wang]

From: Shenwei Wang <shenwei.wang@nxp.com>

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 | 23 ++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
index c6d1bb9edf38..81fb36b48f20 100644
--- a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
@@ -190,6 +190,29 @@ scmi_sensor: protocol@15 {
 	cm33: remoteproc-cm33 {
 		compatible = "fsl,imx8ulp-cm33";
 		status = "disabled";
+
+		rpmsg {
+			#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 v16 5/5] arm64: dts: imx8ulp: Add rpmsg node under imx_rproc

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] gpio: lost EOI due to race between set_bit() and bitmap_zero()
- [High] gpio: out-of-bounds read in rpmsg_gpio_channel_callback()
--

commit aea7aeffb6bed8da7df793b582974e6352af17db
Author: Shenwei Wang <shenwei.wang@nxp.com>

gpio: rpmsg: add generic rpmsg GPIO driver

On an AMP platform, the system may include multiple processors communicating
via the RPMSG protocol. This driver implements the standard GPIO interface,
allowing the Linux side to control GPIO controllers on the remote processor.

> diff --git a/drivers/gpio/gpio-rpmsg.c b/drivers/gpio/gpio-rpmsg.c
> new file mode 100644
> index 0000000000000..076af781f3bfb
> --- /dev/null
> +++ b/drivers/gpio/gpio-rpmsg.c

[ ... ]

> +static void gpio_rpmsg_eoi_irq(struct irq_data *d)
> +{
> +	struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> +
> +	set_bit(d->hwirq, port->pending_eoi);
> +	schedule_work(&port->eoi_work);
> +}

[ ... ]

> +static void gpio_rpmsg_eoi_work(struct work_struct *work)
> +{
> +	struct rpmsg_gpio_port *port =
> +		container_of(work, struct rpmsg_gpio_port, eoi_work);
> +	unsigned long pending[BITS_TO_LONGS(GPIOS_PER_PORT_MAX)];
> +	unsigned int line;
> +
> +	guard(mutex)(&port->lock);
> +
> +	bitmap_copy(pending, port->pending_eoi, port->ngpios);
> +	bitmap_zero(port->pending_eoi, port->ngpios);

[Severity: High]
Does this code lose EOI events in port->pending_eoi?

In gpio_rpmsg_eoi_irq(), set_bit() is used to atomically record an EOI for a
specific GPIO line in port->pending_eoi. However, bitmap_zero() performs a
non-atomic memory clear. 

If set_bit() executes after the workqueue reads the bitmap in bitmap_copy(),
but before or during the non-atomic bitmap_zero(), won't the newly set bit
be overwritten with zero and lost?

> +
> +	for_each_set_bit(line, pending, port->ngpios) {
> +		rpmsg_gpio_msg_prepare(port, line, VIRTIO_GPIO_MSG_IRQ_TYPE,
> +				       port->lines[line].irq_type);

[ ... ]

> +static int rpmsg_gpio_channel_callback(struct rpmsg_device *rpdev, void *data,
> +				       int len, void *priv, u32 src)
> +{
> +	struct rpmsg_gpio_response *msg = data;
> +	struct rpmsg_gpio_port *port = NULL;
> +	u32 line;
> +
> +	port = dev_get_drvdata(&rpdev->dev);
> +
> +	if (!port) {
> +		dev_err(&rpdev->dev, "port is null\n");
> +		return -EINVAL;
> +	}
> +
> +	if (msg->type == GPIO_RPMSG_REPLY) {
> +		*port->recv_msg = *msg;

[Severity: High]
Does this code read beyond the data buffer?

The callback casts the data pointer to struct rpmsg_gpio_response and
immediately accesses msg->type, or copies the entire struct.

Since the len parameter is not validated, if a truncated packet with a length
smaller than sizeof(struct rpmsg_gpio_response) is received, couldn't this
result in an out-of-bounds read?

> +		complete(&port->cmd_complete);
> +	} else if (msg->type == GPIO_RPMSG_NOTIFY) {
> +		line = le16_to_cpu(msg->line);
> +		generic_handle_domain_irq_safe(port->gc.irq.domain, line);
> +	} else {
> +		dev_err(&rpdev->dev, "wrong message type (0x%x)\n", msg->type);
> +	}
> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260923184255.299034-1-shenwei.wang@oss.nxp.com?part=5

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

[Mathieu Poirier]

On Wed, 23 Sept 2026 at 12:43, Shenwei Wang <shenwei.wang@oss.nxp.com> wrote:
>
> Support the remote devices on the remote processor via the RPMSG bus on
> i.MX platform.
>
> Changes in v16:
>   - Removed linux/mod_devicetable.h and an extra comma per Uwe's feedback.
>   - Added the "packed" attribute to struct rpmsg_gpio_response and set
>     "can_sleep = true", per Sashiko's review comments.
>   - Removed the channel node from the rpmsg node in dts layout.
>

I will not review any new patchset on this topic until an agreement is
reached on the DT bindings.  Rob and Krzysztof haven't replied to my
previous email, which leads me to believe they have questions or
comments from earlier versions that haven't been addressed.  I suggest
looking into that instead of spinning off new versions.


> Changes in v15:
>  - Update Kconfig description per AndrewD and Julian's feedback
>  - Enable the GPIO driver even without a DT node per AndrewD's feedback
>  - Update gpio-rpmsg.rst per Mathieu’s feedback
>  - Code cleanup according the new gpio-rpmsg.rst and sashiko-bot's review
>    feedback
>
> Changes in v14:
>  - Update gpio-rpmsg.rst per Mathieu’s feedback.
>  - Align the rpmsg-gpio driver with the revised gpio-rpmsg.rst.
>  - Modify rpmsg-core to enable prefix-based matching of RPMSG device IDs.
>
> Changes in v13:
>  - drop the support for legacy NXP firmware.
>  - remove the fixed_up hooks from the rpmsg gpio driver.
>  - code cleanup.
>
> 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
>   rpmsg: core: match rpmsg device IDs by prefix
>   gpio: rpmsg: add generic rpmsg GPIO driver
>   arm64: dts: imx8ulp: Add rpmsg node under imx_rproc
>
>  .../devicetree/bindings/gpio/gpio-rpmsg.yaml  |  55 ++
>  .../bindings/remoteproc/fsl,imx-rproc.yaml    |  46 ++
>  Documentation/driver-api/gpio/gpio-rpmsg.rst  | 242 ++++++++
>  Documentation/driver-api/gpio/index.rst       |   1 +
>  arch/arm64/boot/dts/freescale/imx8ulp.dtsi    |  23 +
>  drivers/gpio/Kconfig                          |  17 +
>  drivers/gpio/Makefile                         |   1 +
>  drivers/gpio/gpio-rpmsg.c                     | 585 ++++++++++++++++++
>  drivers/rpmsg/rpmsg_core.c                    |   4 +-
>  9 files changed, 973 insertions(+), 1 deletion(-)
>  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
>