* [PATCH v14 0/5] Enable Remote GPIO over RPMSG on i.MX Platform
@ 2026-06-25 15:54 Shenwei Wang
2026-06-25 15:54 ` [PATCH v14 1/5] docs: driver-api: gpio: rpmsg gpio driver over rpmsg bus Shenwei Wang
` (4 more replies)
0 siblings, 5 replies; 11+ messages in thread
From: Shenwei Wang @ 2026-06-25 15:54 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Jonathan Corbet, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Mathieu Poirier, Frank Li, Sascha Hauer
Cc: Shuah Khan, linux-gpio, linux-doc, linux-kernel,
Pengutronix Kernel Team, Fabio Estevam, Shenwei Wang, Peng Fan,
devicetree, linux-remoteproc, imx, linux-arm-kernel, linux-imx,
Arnaud POULIQUEN, b-padhi, Andrew Lunn
From: Shenwei Wang <shenwei.wang@nxp.com>
Support the remote devices on the remote processor via the RPMSG bus on
i.MX platform.
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 | 53 ++
Documentation/driver-api/gpio/gpio-rpmsg.rst | 271 +++++++++
Documentation/driver-api/gpio/index.rst | 1 +
arch/arm64/boot/dts/freescale/imx8ulp.dtsi | 25 +
drivers/gpio/Kconfig | 17 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-rpmsg.c | 568 ++++++++++++++++++
drivers/rpmsg/rpmsg_core.c | 4 +-
9 files changed, 994 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
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v14 1/5] docs: driver-api: gpio: rpmsg gpio driver over rpmsg bus
2026-06-25 15:54 [PATCH v14 0/5] Enable Remote GPIO over RPMSG on i.MX Platform Shenwei Wang
@ 2026-06-25 15:54 ` Shenwei Wang
2026-06-25 16:03 ` sashiko-bot
2026-06-25 15:54 ` [PATCH v14 2/5] dt-bindings: remoteproc: imx_rproc: Add "rpmsg" subnode support Shenwei Wang
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Shenwei Wang @ 2026-06-25 15:54 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Jonathan Corbet, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Mathieu Poirier, Frank Li, Sascha Hauer
Cc: Shuah Khan, linux-gpio, linux-doc, linux-kernel,
Pengutronix Kernel Team, Fabio Estevam, Shenwei Wang, Peng Fan,
devicetree, linux-remoteproc, imx, linux-arm-kernel, linux-imx,
Arnaud POULIQUEN, b-padhi, Andrew Lunn
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 | 271 +++++++++++++++++++
Documentation/driver-api/gpio/index.rst | 1 +
2 files changed, 272 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..7d351ff0adb0
--- /dev/null
+++ b/Documentation/driver-api/gpio/gpio-rpmsg.rst
@@ -0,0 +1,271 @@
+.. 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 8-byte packet with the following layout:
+
+.. code-block:: none
+
+ +------+------+------+------+------+------+------+------+
+ | 0x00 | 0x01 | 0x02 | 0x03 | 0x04 | 0x05 | 0x06 | 0x07 |
+ | cmd | line | value |
+ +------+------+------+------+------+------+------+------+
+
+- **cmd**: Command code, used for GPIO_RPMSG_SEND messages.
+
+- **line**: The GPIO line (pin) index of the port.
+
+- **value**: See details in the command description 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
+-ETIMEOUT.
+
+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 |
+ | 1 | 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 |
+ | 1 | 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 |
+ | 1 | 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 |
+ | 1 | 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 |
+ | 1 | status | 0 |
+ +------+--------+--------+
+
+- **status**:
+
+ - 0: Ok
+ - 1: Error
+
+SET_WAKEUP (Cmd=16)
+~~~~~~~~~~~~~~~~~~~
+
+**Request:**
+
+.. code-block:: none
+
+ +------+------+------+------+------+------+------+------+
+ | 0x00 | 0x01 | 0x02 | 0x03 | 0x04 | 0x05 | 0x06 | 0x07 |
+ | 1 | line | value |
+ +------+------+------+------+------+------+------+------+
+
+- **value**: 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 |
+ | 1 | status | 0 |
+ +------+--------+--------+
+
+- **status**:
+
+ - 0: Ok
+ - 1: Error
+
+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 |
+ | 2 | line | trigger|
+ +------+------+--------+
+
+- **line**: The GPIO line (pin) index of the port.
+
+- **trigger**: 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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v14 2/5] dt-bindings: remoteproc: imx_rproc: Add "rpmsg" subnode support
2026-06-25 15:54 [PATCH v14 0/5] Enable Remote GPIO over RPMSG on i.MX Platform Shenwei Wang
2026-06-25 15:54 ` [PATCH v14 1/5] docs: driver-api: gpio: rpmsg gpio driver over rpmsg bus Shenwei Wang
@ 2026-06-25 15:54 ` Shenwei Wang
2026-06-25 16:00 ` sashiko-bot
2026-06-25 15:54 ` [PATCH v14 3/5] rpmsg: core: match rpmsg device IDs by prefix Shenwei Wang
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Shenwei Wang @ 2026-06-25 15:54 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Jonathan Corbet, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Mathieu Poirier, Frank Li, Sascha Hauer
Cc: Shuah Khan, linux-gpio, linux-doc, linux-kernel,
Pengutronix Kernel Team, Fabio Estevam, Shenwei Wang, Peng Fan,
devicetree, linux-remoteproc, imx, linux-arm-kernel, linux-imx,
Arnaud POULIQUEN, b-padhi, Andrew Lunn
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 | 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..aea33205a881 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:
+ 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 {
+ #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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v14 3/5] rpmsg: core: match rpmsg device IDs by prefix
2026-06-25 15:54 [PATCH v14 0/5] Enable Remote GPIO over RPMSG on i.MX Platform Shenwei Wang
2026-06-25 15:54 ` [PATCH v14 1/5] docs: driver-api: gpio: rpmsg gpio driver over rpmsg bus Shenwei Wang
2026-06-25 15:54 ` [PATCH v14 2/5] dt-bindings: remoteproc: imx_rproc: Add "rpmsg" subnode support Shenwei Wang
@ 2026-06-25 15:54 ` Shenwei Wang
2026-06-25 16:06 ` sashiko-bot
2026-06-25 15:54 ` [PATCH v14 4/5] gpio: rpmsg: add generic rpmsg GPIO driver Shenwei Wang
2026-06-25 15:54 ` [PATCH v14 5/5] arm64: dts: imx8ulp: Add rpmsg node under imx_rproc Shenwei Wang
4 siblings, 1 reply; 11+ messages in thread
From: Shenwei Wang @ 2026-06-25 15:54 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Jonathan Corbet, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Mathieu Poirier, Frank Li, Sascha Hauer
Cc: Shuah Khan, linux-gpio, linux-doc, linux-kernel,
Pengutronix Kernel Team, Fabio Estevam, Shenwei Wang, Peng Fan,
devicetree, linux-remoteproc, imx, linux-arm-kernel, linux-imx,
Arnaud POULIQUEN, b-padhi, Andrew Lunn
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 e7f7831d37f8..f95bfc9965d4 100644
--- a/drivers/rpmsg/rpmsg_core.c
+++ b/drivers/rpmsg/rpmsg_core.c
@@ -414,7 +414,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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v14 4/5] gpio: rpmsg: add generic rpmsg GPIO driver
2026-06-25 15:54 [PATCH v14 0/5] Enable Remote GPIO over RPMSG on i.MX Platform Shenwei Wang
` (2 preceding siblings ...)
2026-06-25 15:54 ` [PATCH v14 3/5] rpmsg: core: match rpmsg device IDs by prefix Shenwei Wang
@ 2026-06-25 15:54 ` Shenwei Wang
2026-06-25 16:04 ` sashiko-bot
2026-06-25 15:54 ` [PATCH v14 5/5] arm64: dts: imx8ulp: Add rpmsg node under imx_rproc Shenwei Wang
4 siblings, 1 reply; 11+ messages in thread
From: Shenwei Wang @ 2026-06-25 15:54 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Jonathan Corbet, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Mathieu Poirier, Frank Li, Sascha Hauer
Cc: Shuah Khan, linux-gpio, linux-doc, linux-kernel,
Pengutronix Kernel Team, Fabio Estevam, Shenwei Wang, Peng Fan,
devicetree, linux-remoteproc, imx, linux-arm-kernel, linux-imx,
Arnaud POULIQUEN, b-padhi, Andrew Lunn, Bartosz Golaszewski
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 | 568 ++++++++++++++++++++++++++++++++++++++
3 files changed, 586 insertions(+)
create mode 100644 drivers/gpio/gpio-rpmsg.c
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 020e51e30317..4ad299fe3c6f 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1917,6 +1917,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 b267598b517d..ee75c0e65b8b 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -157,6 +157,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..332e2925a830
--- /dev/null
+++ b/drivers/gpio/gpio-rpmsg.c
@@ -0,0 +1,568 @@
+// 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 GPIOS_PER_PORT_DEFAULT 32
+#define RPMSG_TIMEOUT 1000
+
+/* Additional commands beyond virtio-gpio */
+#define VIRTIO_GPIO_MSG_SET_WAKEUP 0x0010
+
+/* GPIO Receive MSG Type */
+#define GPIO_RPMSG_REPLY 1
+#define GPIO_RPMSG_NOTIFY 2
+
+#define CHAN_NAME_PREFIX "rpmsg-io-"
+#define GPIO_COMPAT_STR "rpmsg-gpio"
+
+struct rpmsg_gpio_response {
+ __u8 type;
+ union {
+ /* command reply */
+ struct {
+ __u8 status;
+ __u8 value;
+ };
+
+ /* interrupt notification */
+ struct {
+ __u8 line;
+ __u8 trigger; /* rising/falling/high/low */
+ };
+ };
+};
+
+struct rpmsg_gpio_line {
+ u8 irq_shutdown;
+ u8 irq_unmask;
+ u8 irq_mask;
+ u32 irq_wake_enable;
+ 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;
+ u32 ngpios;
+ u32 idx;
+ struct rpmsg_gpio_line lines[GPIOS_PER_PORT_DEFAULT];
+};
+
+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 (unlikely(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 = cmd;
+ msg->gpio = line;
+ msg->value = 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_DIRECTION,
+ VIRTIO_GPIO_DIRECTION_OUT);
+
+ ret = rpmsg_gpio_send_message(port);
+ if (ret)
+ return ret;
+
+ rpmsg_gpio_msg_prepare(port, line, VIRTIO_GPIO_MSG_SET_VALUE, val);
+
+ 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;
+ 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:
+ dev_err(&port->rpdev->dev, "unsupported irq type: %u\n", type);
+ return -EINVAL;
+ }
+
+ port->lines[line].irq_type = type;
+
+ return 0;
+}
+
+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->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;
+
+ rpmsg_gpio_msg_prepare(port, line, VIRTIO_GPIO_MSG_SET_WAKEUP,
+ port->lines[line].irq_wake_enable);
+ rpmsg_gpio_send_message(port);
+
+ /*
+ * 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 = {
+ .irq_mask = gpio_rpmsg_mask_irq,
+ .irq_unmask = gpio_rpmsg_unmask_irq,
+ .irq_set_wake = gpio_rpmsg_irq_set_wake,
+ .irq_set_type = gpio_rpmsg_irq_set_type,
+ .irq_shutdown = gpio_rpmsg_irq_shutdown,
+ .irq_bus_lock = gpio_rpmsg_irq_bus_lock,
+ .irq_bus_sync_unlock = gpio_rpmsg_irq_bus_sync_unlock,
+ .flags = IRQCHIP_IMMUTABLE,
+};
+
+static int rpmsg_gpiochip_register(struct rpmsg_device *rpdev,
+ 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 = of_property_read_u32(np, "reg", &port->idx);
+ if (ret)
+ return ret;
+
+ 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_DEFAULT)
+ port->ngpios = GPIOS_PER_PORT_DEFAULT;
+
+ 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_completion(&port->cmd_complete);
+ port->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",
+ 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_kstrdup(&rpdev->dev, gc->label, GFP_KERNEL);
+
+ 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", ®))
+ continue;
+
+ if (reg == idx)
+ return child;
+ }
+
+ return NULL;
+}
+
+static struct device_node *
+rpmsg_get_channel_ofnode(struct rpmsg_device *rpdev, const char *compat, u32 idx)
+{
+ 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);
+
+ if (np)
+ np_chan = rpmsg_find_child_by_compat_reg(np, compat, idx);
+
+ 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;
+
+ 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) {
+ generic_handle_domain_irq_safe(port->gc.irq.domain, msg->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_channel_ofnode(rpdev, GPIO_COMPAT_STR, idx);
+ if (!np)
+ return -ENODEV;
+
+ 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, dev->of_node, rproc_name);
+}
+
+static const struct of_device_id rpmsg_gpio_dt_ids[] = {
+ { .compatible = GPIO_COMPAT_STR },
+ { /* sentinel */ }
+};
+
+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,
+ .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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v14 5/5] arm64: dts: imx8ulp: Add rpmsg node under imx_rproc
2026-06-25 15:54 [PATCH v14 0/5] Enable Remote GPIO over RPMSG on i.MX Platform Shenwei Wang
` (3 preceding siblings ...)
2026-06-25 15:54 ` [PATCH v14 4/5] gpio: rpmsg: add generic rpmsg GPIO driver Shenwei Wang
@ 2026-06-25 15:54 ` Shenwei Wang
2026-06-25 16:06 ` sashiko-bot
4 siblings, 1 reply; 11+ messages in thread
From: Shenwei Wang @ 2026-06-25 15:54 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Jonathan Corbet, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Mathieu Poirier, Frank Li, Sascha Hauer
Cc: Shuah Khan, linux-gpio, linux-doc, linux-kernel,
Pengutronix Kernel Team, Fabio Estevam, Shenwei Wang, Peng Fan,
devicetree, linux-remoteproc, imx, linux-arm-kernel, linux-imx,
Arnaud POULIQUEN, b-padhi, Andrew Lunn
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 | 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 1de3ad60c6aa..f1b984eb1203 100644
--- a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
@@ -190,6 +190,31 @@ scmi_sensor: protocol@15 {
cm33: remoteproc-cm33 {
compatible = "fsl,imx8ulp-cm33";
status = "disabled";
+
+ rpmsg {
+ rpmsg-io {
+ #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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v14 2/5] dt-bindings: remoteproc: imx_rproc: Add "rpmsg" subnode support
2026-06-25 15:54 ` [PATCH v14 2/5] dt-bindings: remoteproc: imx_rproc: Add "rpmsg" subnode support Shenwei Wang
@ 2026-06-25 16:00 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-06-25 16:00 UTC (permalink / raw)
To: Shenwei Wang; +Cc: conor+dt, robh, Frank.Li, imx, devicetree
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] The `gpio-rpmsg.yaml` binding adds GPIO and interrupt controller provider capabilities but fails to list `gpio-controller` and `interrupt-controller` in its `required` properties array.
- [Medium] The `rpmsg` node schema hardcodes `rpmsg-io` as the only allowed subnode, contradicting the commit message's description of flexible channel names.
- [Low] The `patternProperties` regex used for matching GPIO subnodes in `fsl,imx-rproc.yaml` is missing a start-of-string anchor (`^`).
--
commit e02180a974397446bfafe8bc43641010b7365078
Author: Shenwei Wang <shenwei.wang@nxp.com>
dt-bindings: remoteproc: imx_rproc: Add "rpmsg" subnode support
This commit adds device tree binding support for RPMSG GPIO controllers
exposed by a remote processor. It introduces a generic RPMSG GPIO controller
schema and updates the fsl,imx-rproc binding to support a new rpmsg subnode
container for these channels.
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-rpmsg.yaml b/Documentation/devicetree/bindings/gpio/gpio-rpmsg.yaml
> new file mode 100644
> index 0000000000000..6c78b6850321c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-rpmsg.yaml
[ ... ]
> + "#interrupt-cells":
> + const: 2
> +
> +required:
> + - compatible
> + - reg
> + - "#gpio-cells"
> + - "#interrupt-cells"
[Severity: Medium]
Should gpio-controller and interrupt-controller be included in this required
list? The binding adds GPIO and interrupt controller provider capabilities,
so omitting them might allow incomplete device tree nodes to pass validation.
> +
> +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 ce8ec0119469c..aea33205a881a 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:
> + type: object
> + additionalProperties: false
[Severity: Medium]
The commit message states "The name of each subnode corresponds to the
channel name as defined by the remote processor".
Does hardcoding rpmsg-io as a property and setting additionalProperties to
false unintentionally restrict the schema from allowing flexible channel
names?
> + 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: Low]
Does this regex need a start-of-string anchor (^)? Without it, could this
unintentionally match any node name that ends in gpio@[0-9a-f]+?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260625155432.815185-1-shenwei.wang@oss.nxp.com?part=2
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v14 1/5] docs: driver-api: gpio: rpmsg gpio driver over rpmsg bus
2026-06-25 15:54 ` [PATCH v14 1/5] docs: driver-api: gpio: rpmsg gpio driver over rpmsg bus Shenwei Wang
@ 2026-06-25 16:03 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-06-25 16:03 UTC (permalink / raw)
To: Shenwei Wang; +Cc: robh, conor+dt, devicetree, Frank.Li, imx
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [High] Missing endianness specification and implementation for multi-byte protocol fields.
- [High] The driver's direction_output callback causes hardware glitches by setting the direction before the initial value.
- [Medium] Inconsistent field sizes for REPLY and NOTIFICATION messages truncate the 'line' field.
- [Low] Incorrect command code in the SET_WAKEUP request diagram.
- [Low] Incorrect Linux error code usage (-ETIMEOUT).
- [Low] Grammatical error in SET_WAKEUP description.
--
commit 7712498eeadf5080c07364375a209ec4b75b63fd
Author: Shenwei Wang <shenwei.wang@nxp.com>
docs: driver-api: gpio: rpmsg gpio driver over rpmsg bus
This commit introduces the documentation for the GPIO RPMSG transport
protocol used over the RPMSG bus. It details the message formats,
command codes, and notification structures for communicating with
remote processors.
> diff --git a/Documentation/driver-api/gpio/gpio-rpmsg.rst b/Documentation/driver-api/gpio/gpio-rpmsg.rst
> new file mode 100644
> index 0000000000000..7d351ff0adb0e
> --- /dev/null
> +++ b/Documentation/driver-api/gpio/gpio-rpmsg.rst
> @@ -0,0 +1,271 @@
> +.. SPDX-License-Identifier: GPL-2.0-or-later
> +
> +GPIO RPMSG (Remote Processor Messaging) Protocol
> +================================================
[ ... ]
> +Message Format
> +--------------
> +
> +The RPMSG message consists of a 8-byte packet with the following layout:
> +
> +.. code-block:: none
> +
> + +------+------+------+------+------+------+------+------+
> + | 0x00 | 0x01 | 0x02 | 0x03 | 0x04 | 0x05 | 0x06 | 0x07 |
> + | cmd | line | value |
> + +------+------+------+------+------+------+------+------+
[Severity: High]
Does this message format lack an explicit endianness specification for the
multi-byte fields (cmd, line, value)?
Without defining the endianness here, the driver implementation in
drivers/gpio/gpio-rpmsg.c:rpmsg_gpio_msg_prepare() assigns native integer types
directly:
msg->type = cmd;
msg->gpio = line;
msg->value = val;
Could this cause malformed packets when communicating between a big-endian host
and the remote processor?
> +
> +- **cmd**: Command code, used for GPIO_RPMSG_SEND messages.
[ ... ]
> +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
> +-ETIMEOUT.
[Severity: Low]
Should this use the standard Linux kernel error code -ETIMEDOUT instead of
-ETIMEOUT?
> +
> +GET_DIRECTION (Cmd=2)
[ ... ]
> +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
[Severity: High]
Does this command lack a parameter to specify the initial output level,
requiring a separate SET_VALUE call?
In drivers/gpio/gpio-rpmsg.c:rpmsg_gpio_direction_output(), the driver sends
the SET_DIRECTION command before the SET_VALUE command:
rpmsg_gpio_msg_prepare(port, line, VIRTIO_GPIO_MSG_SET_DIRECTION,
VIRTIO_GPIO_DIRECTION_OUT);
ret = rpmsg_gpio_send_message(port);
if (ret)
return ret;
rpmsg_gpio_msg_prepare(port, line, VIRTIO_GPIO_MSG_SET_VALUE, val);
return rpmsg_gpio_send_message(port);
Could this sequence cause a hardware glitch by driving an unintended default
state on the physical GPIO line during the IPC transit time, before the
SET_VALUE command is processed?
> +
> +**Reply:**
[ ... ]
> +SET_WAKEUP (Cmd=16)
> +~~~~~~~~~~~~~~~~~~~
> +
> +**Request:**
> +
> +.. code-block:: none
> +
> + +------+------+------+------+------+------+------+------+
> + | 0x00 | 0x01 | 0x02 | 0x03 | 0x04 | 0x05 | 0x06 | 0x07 |
> + | 1 | line | value |
> + +------+------+------+------+------+------+------+------+
[Severity: Low]
Does the command field in this diagram say '1' when it should say '16' or
'0x10' to match the SET_WAKEUP (Cmd=16) heading?
> +
> +- **value**: 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.
[Severity: Low]
Should "does not require to know" be rephrased to "does not need to know" for
better grammar?
> +
> + When the wakeup bit is set, the remote firmware should configure the line
[ ... ]
> +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 |
> + | 2 | line | trigger|
> + +------+------+--------+
[Severity: Medium]
Does this notification message truncate the line index to 8 bits?
The SEND message diagram earlier explicitly defines an 8-byte layout with a
16-bit line field (occupying bytes 0x02 and 0x03). However, this NOTIFICATION
diagram and the driver's struct rpmsg_gpio_response in
drivers/gpio/gpio-rpmsg.c restrict the line index to an 8-bit __u8:
struct rpmsg_gpio_response {
...
__u8 line;
...
};
If an interrupt occurs on a line index greater than 255, could this cause the
driver to mishandle the IRQ due to truncation?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260625155432.815185-1-shenwei.wang@oss.nxp.com?part=1
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v14 4/5] gpio: rpmsg: add generic rpmsg GPIO driver
2026-06-25 15:54 ` [PATCH v14 4/5] gpio: rpmsg: add generic rpmsg GPIO driver Shenwei Wang
@ 2026-06-25 16:04 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-06-25 16:04 UTC (permalink / raw)
To: Shenwei Wang; +Cc: devicetree, conor+dt, Frank.Li, imx, robh
Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider:
- [Critical] Writing to the `.name` field of the immutable `gpio_rpmsg_irq_chip` structure attempts to modify read-only memory, causing a kernel panic.
- [High] Host-endian integers are directly assigned to little-endian protocol fields, breaking communication on big-endian architectures.
- [High] An RPMSG timeout permanently desynchronizes the request/response state machine because the driver lacks transaction IDs to discard late replies.
- [High] The RPMSG callback blindly accesses the incoming message without validating that the payload length is sufficient.
- [High] The driver's GPIO callbacks can sleep, but it fails to set `gc->can_sleep = true`, incorrectly allowing them to be called from atomic contexts.
- [High] Interrupts will be permanently masked after firing once because the driver incorrectly relies on `irq_bus_sync_unlock` to send the unmask command during normal IRQ flow.
- [Medium] The driver leaks a reference to the `rproc` parent device node.
- [Medium] `rpmsg_gpio_direction_output()` configures the GPIO line as an output before setting the desired logical value, potentially causing a signal glitch.
--
commit e50fe4c0d14d5e1abdf1ce315d5d9511d01ee66e
Author: Shenwei Wang <shenwei.wang@nxp.com>
Subject: gpio: rpmsg: add generic rpmsg GPIO driver
This commit adds a generic GPIO driver to support controlling GPIO
controllers that reside on a remote processor. It translates standard
Linux GPIO calls into RPMSG protocol messages sent to the remote side.
> diff --git a/drivers/gpio/gpio-rpmsg.c b/drivers/gpio/gpio-rpmsg.c
> new file mode 100644
> index 0000000000000..332e2925a8309
> --- /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]
If rpmsg_send times out, can this permanently desynchronize the request and
response state machine?
If a timeout occurs, rpmsg_gpio_send_message() returns -ETIMEDOUT but leaves
the channel active. Since the rpmsg_gpio_response doesn't appear to contain a
transaction ID, if the remote processor sends a delayed reply to this timed-out
command, will rpmsg_gpio_channel_callback() mistakenly accept it as the
completion for the next unrelated command sent by the host?
> + }
> +
> + if (unlikely(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 = cmd;
> + msg->gpio = line;
> + msg->value = val;
[Severity: High]
Are these values being assigned with the correct endianness?
The virtio_gpio_request structure defines these fields as __le16 and __le32,
but this assigns native host-endian integers to them directly. On big-endian
architectures, could this result in the remote processor misinterpreting the
commands?
> +
> + return msg;
> +}
[ ... ]
> +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_DIRECTION,
> + VIRTIO_GPIO_DIRECTION_OUT);
> +
> + ret = rpmsg_gpio_send_message(port);
> + if (ret)
> + return ret;
> +
> + rpmsg_gpio_msg_prepare(port, line, VIRTIO_GPIO_MSG_SET_VALUE, val);
[Severity: Medium]
Could configuring the direction before setting the value cause a signal
glitch?
rpmsg_gpio_direction_output() sends VIRTIO_GPIO_MSG_SET_DIRECTION to the
remote processor before VIRTIO_GPIO_MSG_SET_VALUE. If the remote hardware
applies the direction immediately, could this drive the line to its default
or previous state for a short window before the intended new value is applied?
> +
> + return rpmsg_gpio_send_message(port);
> +}
[ ... ]
> +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;
> +}
[Severity: High]
Will interrupts remain permanently masked on the remote processor after they
fire once?
gpio_rpmsg_unmask_irq() updates local memory but does not send an RPMSG
command to unmask the interrupt remotely. While gpio_rpmsg_irq_bus_sync_unlock()
does send the command, the core IRQ subsystem (like handle_level_irq) masks
and unmasks interrupts without invoking bus lock/unlock callbacks during normal
interrupt dispatch. Does this mean the unmask command is never actually sent
after an interrupt is handled?
[ ... ]
> +static int rpmsg_gpiochip_register(struct rpmsg_device *rpdev,
> + 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 = of_property_read_u32(np, "reg", &port->idx);
> + if (ret)
> + return ret;
> +
> + 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_DEFAULT)
> + port->ngpios = GPIOS_PER_PORT_DEFAULT;
> +
> + 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_completion(&port->cmd_complete);
> + port->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",
> + name, port->idx);
[Severity: High]
Should gc->can_sleep be explicitly set to true for this gpio_chip?
The .set and .get callbacks in this driver acquire a mutex and wait for
completion, which will sleep. Without setting gc->can_sleep = true, the
gpiolib framework treats these GPIOs as atomic. If consumer drivers call
these GPIO operations from within an interrupt handler or a spinlock,
could this trigger a "scheduling while atomic" panic?
> +
> + 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_kstrdup(&rpdev->dev, gc->label, GFP_KERNEL);
[Severity: Critical]
Does this attempt to write to read-only memory?
gpio_irq_chip_set_chip() assigns the const structure gpio_rpmsg_irq_chip to
girq->chip. Modifying the .name field of a const structure placed in the
.rodata section will trigger a write-protection page fault and crash the
kernel. Should the irq_chip be dynamically allocated, or should the name be
set differently?
> +
> + dev_set_drvdata(&rpdev->dev, port);
> +
> + return devm_gpiochip_add_data(&rpdev->dev, gc, port);
> +}
[ ... ]
> +static struct device_node *
> +rpmsg_get_channel_ofnode(struct rpmsg_device *rpdev, const char *compat, u32 idx)
> +{
> + 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);
> +
> + if (np)
> + np_chan = rpmsg_find_child_by_compat_reg(np, compat, idx);
> +
> + return np_chan;
[Severity: Medium]
Does this leak the reference to the parent device node?
rpmsg_get_channel_ofnode() acquires a reference to np using of_node_get(),
but it appears to return without calling of_node_put(np) to release it.
> +}
[ ... ]
> +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;
> +
> + port = dev_get_drvdata(&rpdev->dev);
> +
> + if (!port) {
> + dev_err(&rpdev->dev, "port is null\n");
> + return -EINVAL;
> + }
> +
> + if (msg->type == GPIO_RPMSG_REPLY) {
[Severity: High]
Is there a risk of an out-of-bounds read here if the message is truncated?
rpmsg_gpio_channel_callback() receives a payload of size len, but blindly
dereferences the data pointer as struct rpmsg_gpio_response without
verifying that len >= sizeof(struct rpmsg_gpio_response). Could this read
out-of-bounds memory if the remote processor sends a malformed or small
message?
> + *port->recv_msg = *msg;
> + complete(&port->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 message type (0x%x)\n", msg->type);
> + }
> +
> + return 0;
> +}
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260625155432.815185-1-shenwei.wang@oss.nxp.com?part=4
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v14 5/5] arm64: dts: imx8ulp: Add rpmsg node under imx_rproc
2026-06-25 15:54 ` [PATCH v14 5/5] arm64: dts: imx8ulp: Add rpmsg node under imx_rproc Shenwei Wang
@ 2026-06-25 16:06 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-06-25 16:06 UTC (permalink / raw)
To: Shenwei Wang; +Cc: devicetree, robh, Frank.Li, conor+dt, imx
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The newly added `rpmsg-gpio` nodes are nested too deeply in the device tree for the `gpio-rpmsg` driver to discover them, causing an unconditional probe failure.
--
commit 9072997f8f4ba10a42ddca328c65744333555ac3
Author: Shenwei Wang <shenwei.wang@nxp.com>
arm64: dts: imx8ulp: Add rpmsg node under imx_rproc
This patch adds an RPMSG bus node and its GPIO subnodes to the device tree
for the i.MX platform. It aims to enable remote device communication and
GPIO control via RPMSG.
> diff --git a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
> index 1de3ad60c6aa7..f1b984eb1203a 100644
> --- a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
> @@ -190,6 +190,31 @@ scmi_sensor: protocol@15 {
> cm33: remoteproc-cm33 {
> compatible = "fsl,imx8ulp-cm33";
> status = "disabled";
> +
> + rpmsg {
> + rpmsg-io {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + rpmsg_gpioa: gpio@0 {
[Severity: Medium]
Does nesting these nodes cause the gpio-rpmsg driver to fail to probe?
Looking at rpmsg_find_child_by_compat_reg() in drivers/gpio/gpio-rpmsg.c,
the driver only searches the direct children of the remoteproc node:
for_each_available_child_of_node(parent, child) {
if (!of_device_is_compatible(child, compat))
continue;
Since for_each_available_child_of_node() is not recursive, the driver
fails to find the nested gpio@0 and gpio@1 nodes under rpmsg/rpmsg-io/.
This causes an -ENODEV return, preventing the driver from probing.
> + compatible = "rpmsg-gpio";
> + reg = <0>;
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260625155432.815185-1-shenwei.wang@oss.nxp.com?part=5
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v14 3/5] rpmsg: core: match rpmsg device IDs by prefix
2026-06-25 15:54 ` [PATCH v14 3/5] rpmsg: core: match rpmsg device IDs by prefix Shenwei Wang
@ 2026-06-25 16:06 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-06-25 16:06 UTC (permalink / raw)
To: Shenwei Wang; +Cc: imx, conor+dt, robh, Frank.Li, devicetree
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Forcing global prefix matching on all rpmsg drivers without a 'longest-prefix match' rule can cause unintended device binding and collisions.
- [Medium] Module auto-loading via udev is broken for dynamically suffixed devices because module alias generation remains exact and lacks a trailing wildcard.
--
commit 7cf465d01393f57a212484ddaf8a931c40ec4179
Author: Shenwei Wang <shenwei.wang@nxp.com>
rpmsg: core: match rpmsg device IDs by prefix
This commit updates the rpmsg device matching logic to compare only the
length of the id->name string. This allows id_table entries to act as
prefixes, enabling drivers to bind to dynamically suffixed devices.
> diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c
> index e7f7831d37f89..f95bfc9965d4c 100644
> --- a/drivers/rpmsg/rpmsg_core.c
> +++ b/drivers/rpmsg/rpmsg_core.c
> @@ -414,7 +414,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;
> }
[Severity: High]
Will this change cause unintended driver binding for existing drivers that
expect an exact match?
Because this modifies the global match behavior for all rpmsg drivers without
an opt-in flag or longest-prefix match logic, a shorter existing prefix could
hijack a new device.
For example, if a remote processor creates a channel named rpmsg-tty-control,
would it be incorrectly matched by an existing driver registered as rpmsg-tty?
Looking at rpmsg_dev_match(), the iteration returns on the first successful
match:
drivers/rpmsg/rpmsg_core.c:rpmsg_dev_match() {
...
for (i = 0; ids[i].name[0]; i++)
if (rpmsg_id_match(rpdev, &ids[i])) {
rpdev->id.driver_data = ids[i].driver_data;
return 1;
}
...
}
Could this result in a collision if multiple IDs share the same prefix?
[Severity: Medium]
Does this change break module auto-loading via udev for dynamically suffixed
devices?
When a dynamically suffixed device is registered, rpmsg_uevent() emits the
exact name:
drivers/rpmsg/rpmsg_core.c:rpmsg_uevent() {
...
return add_uevent_var(env, "MODALIAS=" RPMSG_DEVICE_MODALIAS_FMT,
rpdev->id.name);
}
However, the module alias generated by file2alias.c does not append a
wildcard for prefix matching:
scripts/mod/file2alias.c:do_rpmsg_entry() {
...
module_alias_printf(mod, false, RPMSG_DEVICE_MODALIAS_FMT, *name);
}
Since module_alias_printf() is called with append_wildcard set to false, the
generated alias will be an exact match (e.g., rpmsg:rpmsg-io-) which won't
match the uevent string (e.g., MODALIAS=rpmsg:rpmsg-io-1).
Will this prevent the module from loading automatically?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260625155432.815185-1-shenwei.wang@oss.nxp.com?part=3
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-06-25 16:06 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-25 15:54 [PATCH v14 0/5] Enable Remote GPIO over RPMSG on i.MX Platform Shenwei Wang
2026-06-25 15:54 ` [PATCH v14 1/5] docs: driver-api: gpio: rpmsg gpio driver over rpmsg bus Shenwei Wang
2026-06-25 16:03 ` sashiko-bot
2026-06-25 15:54 ` [PATCH v14 2/5] dt-bindings: remoteproc: imx_rproc: Add "rpmsg" subnode support Shenwei Wang
2026-06-25 16:00 ` sashiko-bot
2026-06-25 15:54 ` [PATCH v14 3/5] rpmsg: core: match rpmsg device IDs by prefix Shenwei Wang
2026-06-25 16:06 ` sashiko-bot
2026-06-25 15:54 ` [PATCH v14 4/5] gpio: rpmsg: add generic rpmsg GPIO driver Shenwei Wang
2026-06-25 16:04 ` sashiko-bot
2026-06-25 15:54 ` [PATCH v14 5/5] arm64: dts: imx8ulp: Add rpmsg node under imx_rproc Shenwei Wang
2026-06-25 16:06 ` sashiko-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox