* [PATCH 0/4] Enable Remote GPIO over RPMSG on i.MX Platform
@ 2025-08-18 20:44 Shenwei Wang
2025-08-18 20:44 ` [PATCH 1/4] dt-bindings: remoteproc: imx_rproc: Add "rpmsg" subnode support Shenwei Wang
` (4 more replies)
0 siblings, 5 replies; 19+ messages in thread
From: Shenwei Wang @ 2025-08-18 20:44 UTC (permalink / raw)
To: Bjorn Andersson, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
Linus Walleij, Bartosz Golaszewski
Cc: Pengutronix Kernel Team, Fabio Estevam, Shenwei Wang, Peng Fan,
linux-remoteproc, devicetree, imx, linux-arm-kernel, linux-kernel,
linux-imx
Support the remote devices on the remote processor via the RPMSG bus on
i.MX platform.
The expected DTS layout structure is following:
cm33: remoteproc-cm33 {
compatible = "fsl,imx8ulp-cm33";
rpmsg {
rpmsg-io-channel {
gpio@0 {
compatible = "fsl,imx-rpmsg-gpio";
reg = <0>;
};
gpio@1 {
compatible = "fsl,imx-rpmsg-gpio";
reg = <1>;
};
...
};
rpmsg-i2c-channel {
i2c@0 {
compatible = "fsl,imx-rpmsg-i2c";
reg = <0>;
};
};
...
};
};
Shenwei Wang (4):
dt-bindings: remoteproc: imx_rproc: Add "rpmsg" subnode support
remoteproc: imx_rproc: Populate devices under "rpmsg" subnode
gpio: imx-rpmsg: add imx-rpmsg GPIO driver
arm64: dts: imx8ulp: Add rpmsg node under imx_rproc
.../bindings/remoteproc/fsl,imx-rproc.yaml | 117 ++++
arch/arm64/boot/dts/freescale/imx8ulp.dtsi | 27 +
drivers/gpio/Kconfig | 11 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-imx-rpmsg.c | 559 ++++++++++++++++++
drivers/remoteproc/imx_rproc.c | 125 ++++
include/linux/imx_rpmsg.h | 55 ++
7 files changed, 895 insertions(+)
create mode 100644 drivers/gpio/gpio-imx-rpmsg.c
create mode 100644 include/linux/imx_rpmsg.h
--
2.43.0
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 1/4] dt-bindings: remoteproc: imx_rproc: Add "rpmsg" subnode support
2025-08-18 20:44 [PATCH 0/4] Enable Remote GPIO over RPMSG on i.MX Platform Shenwei Wang
@ 2025-08-18 20:44 ` Shenwei Wang
2025-08-26 14:50 ` Frank Li
2025-08-26 20:09 ` Rob Herring
2025-08-18 20:44 ` [PATCH 2/4] remoteproc: imx_rproc: Populate devices under "rpmsg" subnode Shenwei Wang
` (3 subsequent siblings)
4 siblings, 2 replies; 19+ messages in thread
From: Shenwei Wang @ 2025-08-18 20:44 UTC (permalink / raw)
To: Bjorn Andersson, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
Linus Walleij, Bartosz Golaszewski
Cc: Pengutronix Kernel Team, Fabio Estevam, Shenwei Wang, Peng Fan,
linux-remoteproc, devicetree, imx, linux-arm-kernel, linux-kernel,
linux-imx
Remote processors may announce multiple devices (e.g., I2C, GPIO) over
an RPMSG channel. These devices may require corresponding device tree
nodes, especially when acting as providers, to supply phandles for their
consumers.
Define an RPMSG node to work as a container for a group of RPMSG channels
under the imx_rproc node.
Each subnode within "rpmsg" represents an individual RPMSG channel. The
name of each subnode corresponds to the channel name as defined by the
remote processor.
All remote devices associated with a given channel are defined as child
nodes under the corresponding channel node.
Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
---
.../bindings/remoteproc/fsl,imx-rproc.yaml | 117 ++++++++++++++++++
1 file changed, 117 insertions(+)
diff --git a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
index 57d75acb0b5e..a105aac798d0 100644
--- a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
@@ -84,6 +84,86 @@ properties:
This property is to specify the resource id of the remote processor in SoC
which supports SCFW
+ rpmsg:
+ type: object
+ additionalProperties: false
+ description:
+ Present a group of RPMSG channel devices.
+
+ properties:
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
+ rpmsg-io-channel:
+ type: object
+ unevaluatedProperties: false
+ properties:
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
+ patternProperties:
+ "gpio@[0-9a-f]+$":
+ type: object
+ unevaluatedProperties: false
+ properties:
+ compatible:
+ enum:
+ - fsl,imx-rpmsg-gpio
+
+ reg:
+ maxItems: 1
+
+ required:
+ - compatible
+ - reg
+
+ allOf:
+ - $ref: /schemas/gpio/gpio.yaml#
+ - $ref: /schemas/interrupt-controller.yaml#
+
+ required:
+ - '#address-cells'
+ - '#size-cells'
+
+ rpmsg-i2c-channel:
+ type: object
+ unevaluatedProperties: false
+ properties:
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
+ patternProperties:
+ "i2c@[0-9a-f]+$":
+ type: object
+ unevaluatedProperties: false
+ properties:
+ compatible:
+ enum:
+ - fsl,imx-rpmsg-i2c
+
+ reg:
+ maxItems: 1
+
+ required:
+ - compatible
+ - reg
+
+ allOf:
+ - $ref: /schemas/i2c/i2c-controller.yaml#
+
+ required:
+ - '#address-cells'
+ - '#size-cells'
+
required:
- compatible
@@ -146,5 +226,42 @@ examples:
&mu 3 1>;
memory-region = <&vdev0buffer>, <&vdev0vring0>, <&vdev0vring1>, <&rsc_table>;
syscon = <&src>;
+
+ rpmsg {
+ rpmsg-io-channel {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ gpio@0 {
+ compatible = "fsl,imx-rpmsg-gpio";
+ reg = <0>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ #interrupt-cells = <2>;
+ interrupt-controller;
+ interrupt-parent = <&rpmsg_gpioa>;
+ };
+
+ gpio@1 {
+ compatible = "fsl,imx-rpmsg-gpio";
+ reg = <1>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ #interrupt-cells = <2>;
+ interrupt-controller;
+ interrupt-parent = <&rpmsg_gpiob>;
+ };
+ };
+
+ rpmsg-i2c-channel {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ i2c@0 {
+ compatible = "fsl,imx-rpmsg-i2c";
+ reg = <0>;
+ };
+ };
+ };
};
...
--
2.43.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 2/4] remoteproc: imx_rproc: Populate devices under "rpmsg" subnode
2025-08-18 20:44 [PATCH 0/4] Enable Remote GPIO over RPMSG on i.MX Platform Shenwei Wang
2025-08-18 20:44 ` [PATCH 1/4] dt-bindings: remoteproc: imx_rproc: Add "rpmsg" subnode support Shenwei Wang
@ 2025-08-18 20:44 ` Shenwei Wang
2025-08-21 11:02 ` Peng Fan
2025-08-18 20:44 ` [PATCH 3/4] gpio: imx-rpmsg: add imx-rpmsg GPIO driver Shenwei Wang
` (2 subsequent siblings)
4 siblings, 1 reply; 19+ messages in thread
From: Shenwei Wang @ 2025-08-18 20:44 UTC (permalink / raw)
To: Bjorn Andersson, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
Linus Walleij, Bartosz Golaszewski
Cc: Pengutronix Kernel Team, Fabio Estevam, Shenwei Wang, Peng Fan,
linux-remoteproc, devicetree, imx, linux-arm-kernel, linux-kernel,
linux-imx
Register the RPMsg channel driver and populate remote devices defined
under the "rpmsg" subnode upon receiving their notification messages.
The following illustrates the expected DTS layout structure:
cm33: remoteproc-cm33 {
compatible = "fsl,imx8ulp-cm33";
rpmsg {
rpmsg-io-channel {
gpio@0 {
compatible = "fsl,imx-rpmsg-gpio";
reg = <0>;
};
gpio@1 {
compatible = "fsl,imx-rpmsg-gpio";
reg = <1>;
};
...
};
rpmsg-i2c-channel {
i2c@0 {
compatible = "fsl,imx-rpmsg-i2c";
reg = <0>;
};
};
...
};
};
Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
---
drivers/remoteproc/imx_rproc.c | 125 +++++++++++++++++++++++++++++++++
include/linux/imx_rpmsg.h | 55 +++++++++++++++
2 files changed, 180 insertions(+)
create mode 100644 include/linux/imx_rpmsg.h
diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
index a6eef0080ca9..9b3396f3f1ec 100644
--- a/drivers/remoteproc/imx_rproc.c
+++ b/drivers/remoteproc/imx_rproc.c
@@ -8,6 +8,7 @@
#include <linux/clk.h>
#include <linux/err.h>
#include <linux/firmware/imx/sci.h>
+#include <linux/imx_rpmsg.h>
#include <linux/interrupt.h>
#include <linux/kernel.h>
#include <linux/mailbox_client.h>
@@ -15,6 +16,8 @@
#include <linux/module.h>
#include <linux/of.h>
#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/of_platform.h>
#include <linux/of_reserved_mem.h>
#include <linux/platform_device.h>
#include <linux/pm_domain.h>
@@ -22,6 +25,7 @@
#include <linux/reboot.h>
#include <linux/regmap.h>
#include <linux/remoteproc.h>
+#include <linux/rpmsg.h>
#include <linux/workqueue.h>
#include "imx_rproc.h"
@@ -1084,6 +1088,126 @@ static int imx_rproc_sys_off_handler(struct sys_off_data *data)
return NOTIFY_DONE;
}
+struct imx_rpmsg_driver {
+ struct rpmsg_driver rpdrv;
+ void *driver_data;
+};
+
+static char *channel_device_map[][2] = {
+ {"rpmsg-io-channel", "fsl,imx-rpmsg-gpio"},
+ {"rpmsg-i2c-channel", "fsl,imx-rpmsg-i2c"},
+};
+
+static int imx_rpmsg_endpoint_cb(struct rpmsg_device *rpdev,
+ void *data, int len, void *priv, u32 src)
+{
+ struct imx_rpmsg_driver_data *drvdata;
+
+ drvdata = dev_get_drvdata(&rpdev->dev);
+ if (drvdata && drvdata->rx_callback)
+ return drvdata->rx_callback(rpdev, data, len, priv, src);
+
+ return 0;
+}
+
+static void imx_rpmsg_endpoint_remove(struct rpmsg_device *rpdev)
+{
+ of_platform_depopulate(&rpdev->dev);
+}
+
+static int imx_rpmsg_endpoint_probe(struct rpmsg_device *rpdev)
+{
+ struct imx_rpmsg_driver_data *drvdata;
+ struct imx_rpmsg_driver *imx_rpdrv;
+ struct device *dev = &rpdev->dev;
+ struct of_dev_auxdata *auxdata;
+ struct rpmsg_driver *rpdrv;
+ int i;
+
+ rpdrv = container_of(dev->driver, struct rpmsg_driver, drv);
+ imx_rpdrv = container_of(rpdrv, struct imx_rpmsg_driver, rpdrv);
+
+ if (!imx_rpdrv->driver_data)
+ return -EINVAL;
+
+ drvdata = devm_kmemdup(dev, imx_rpdrv->driver_data, sizeof(*drvdata), GFP_KERNEL);
+ if (!drvdata)
+ return -ENOMEM;
+
+ i = drvdata->map_idx;
+ if (i >= ARRAY_SIZE(channel_device_map))
+ return -ENODEV;
+
+ auxdata = devm_kzalloc(dev, sizeof(*auxdata)*2, GFP_KERNEL);
+ if (!auxdata)
+ return -ENOMEM;
+
+ drvdata->rpdev = rpdev;
+ auxdata[0].compatible = channel_device_map[i][1];
+ auxdata[0].platform_data = drvdata;
+ dev_set_drvdata(dev, drvdata);
+
+ of_platform_populate(drvdata->channel_node, NULL, auxdata, dev);
+ of_node_put(drvdata->channel_node);
+
+ return 0;
+}
+
+static int imx_of_rpmsg_node_init(struct platform_device *pdev)
+{
+ struct device_node *np __free(device_node), *channel;
+ struct imx_rpmsg_driver_data *driver_data;
+ struct imx_rpmsg_driver *rp_driver;
+ struct rpmsg_device_id *rpdev_id;
+ int i, ret;
+
+ int count = ARRAY_SIZE(channel_device_map);
+ struct device *dev = &pdev->dev;
+
+ np = of_get_child_by_name(dev->of_node, "rpmsg");
+ if (!np)
+ return 0;
+
+ for (i = 0; i < count; i++) {
+ ret = -ENOMEM;
+ channel = of_get_child_by_name(np, channel_device_map[i][0]);
+ if (!channel)
+ continue;
+
+ rpdev_id = devm_kzalloc(dev, sizeof(*rpdev_id)*2, GFP_KERNEL);
+ if (!rpdev_id)
+ break;
+ strscpy(rpdev_id[0].name, channel_device_map[i][0], RPMSG_NAME_SIZE);
+
+ rp_driver = devm_kzalloc(dev, sizeof(*rp_driver), GFP_KERNEL);
+ if (!rp_driver)
+ break;
+
+ driver_data = devm_kzalloc(dev, sizeof(*driver_data), GFP_KERNEL);
+ if (!driver_data)
+ break;
+
+ ret = 0;
+ driver_data->rproc_name = dev->of_node->name;
+ driver_data->channel_node = channel;
+ driver_data->map_idx = i;
+
+ rp_driver->rpdrv.drv.name = channel_device_map[i][0];
+ rp_driver->rpdrv.id_table = rpdev_id;
+ rp_driver->rpdrv.probe = imx_rpmsg_endpoint_probe;
+ rp_driver->rpdrv.remove = imx_rpmsg_endpoint_remove;
+ rp_driver->rpdrv.callback = imx_rpmsg_endpoint_cb;
+ rp_driver->driver_data = driver_data;
+
+ register_rpmsg_driver(&rp_driver->rpdrv);
+ }
+
+ if ((ret < 0) && channel)
+ of_node_put(channel);
+
+ return ret;
+}
+
static int imx_rproc_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -1177,6 +1301,7 @@ static int imx_rproc_probe(struct platform_device *pdev)
goto err_put_clk;
}
+ imx_of_rpmsg_node_init(pdev);
return 0;
err_put_clk:
diff --git a/include/linux/imx_rpmsg.h b/include/linux/imx_rpmsg.h
new file mode 100644
index 000000000000..300ada6237be
--- /dev/null
+++ b/include/linux/imx_rpmsg.h
@@ -0,0 +1,55 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2025 NXP.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+/*
+ * @file linux/imx_rpmsg.h
+ *
+ * @brief Global header file for iMX RPMSG
+ *
+ * @ingroup RPMSG
+ */
+#ifndef __LINUX_IMX_RPMSG_H__
+#define __LINUX_IMX_RPMSG_H__
+
+#include <linux/completion.h>
+#include <linux/mutex.h>
+
+/* Category define */
+#define IMX_RMPSG_LIFECYCLE 1
+#define IMX_RPMSG_PMIC 2
+#define IMX_RPMSG_AUDIO 3
+#define IMX_RPMSG_KEY 4
+#define IMX_RPMSG_GPIO 5
+#define IMX_RPMSG_RTC 6
+#define IMX_RPMSG_SENSOR 7
+/* rpmsg version */
+#define IMX_RMPSG_MAJOR 1
+#define IMX_RMPSG_MINOR 0
+
+#define MAX_DEV_PER_CHANNEL 10
+
+struct imx_rpmsg_head {
+ u8 cate;
+ u8 major;
+ u8 minor;
+ u8 type;
+ u8 cmd;
+ u8 reserved[5];
+} __packed;
+
+struct imx_rpmsg_driver_data {
+ int map_idx;
+ const char *rproc_name;
+ struct rpmsg_device *rpdev;
+ struct device_node *channel_node;
+ int (*rx_callback)(struct rpmsg_device *, void *, int, void *, u32);
+ void *channel_devices[MAX_DEV_PER_CHANNEL];
+};
+
+#endif /* __LINUX_IMX_RPMSG_H__ */
--
2.43.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 3/4] gpio: imx-rpmsg: add imx-rpmsg GPIO driver
2025-08-18 20:44 [PATCH 0/4] Enable Remote GPIO over RPMSG on i.MX Platform Shenwei Wang
2025-08-18 20:44 ` [PATCH 1/4] dt-bindings: remoteproc: imx_rproc: Add "rpmsg" subnode support Shenwei Wang
2025-08-18 20:44 ` [PATCH 2/4] remoteproc: imx_rproc: Populate devices under "rpmsg" subnode Shenwei Wang
@ 2025-08-18 20:44 ` Shenwei Wang
2025-08-21 9:01 ` Linus Walleij
2025-08-21 14:11 ` kernel test robot
2025-08-18 20:44 ` [PATCH 4/4] arm64: dts: imx8ulp: Add rpmsg node under imx_rproc Shenwei Wang
2025-08-20 20:49 ` [PATCH 0/4] Enable Remote GPIO over RPMSG on i.MX Platform Mathieu Poirier
4 siblings, 2 replies; 19+ messages in thread
From: Shenwei Wang @ 2025-08-18 20:44 UTC (permalink / raw)
To: Bjorn Andersson, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
Linus Walleij, Bartosz Golaszewski
Cc: Pengutronix Kernel Team, Fabio Estevam, Shenwei Wang, Peng Fan,
linux-remoteproc, devicetree, imx, linux-arm-kernel, linux-kernel,
linux-imx
On i.MX SoCs, the system may include two processors:
- An MCU running an RTOS
- An MPU running Linux
These processors communicate via the RPMSG protocol.
The driver implements the standard GPIO interface, allowing
the Linux side to control GPIO controllers which reside in
the remote processor via RPMSG protocol.
Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
---
drivers/gpio/Kconfig | 11 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-imx-rpmsg.c | 559 ++++++++++++++++++++++++++++++++++
3 files changed, 571 insertions(+)
create mode 100644 drivers/gpio/gpio-imx-rpmsg.c
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index a437fe652dbc..2ce4e9b5225e 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -402,6 +402,17 @@ config GPIO_ICH
If unsure, say N.
+config GPIO_IMX_RPMSG
+ tristate "NXP i.MX SoC RPMSG GPIO support"
+ depends on IMX_REMOTEPROC && RPMSG && GPIOLIB
+ default IMX_REMOTEPROC
+ help
+ Say yes here to support the RPMSG GPIO functions on i.MX SoC based
+ platform. Currently supported devices: i.MX7ULP, i.MX8ULP, i.MX8x,
+ and i.MX9x.
+
+ If unsure, say N.
+
config GPIO_IMX_SCU
def_bool y
depends on IMX_SCU
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 379f55e9ed1e..e01465c03431 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -83,6 +83,7 @@ obj-$(CONFIG_GPIO_I8255) += gpio-i8255.o
obj-$(CONFIG_GPIO_ICH) += gpio-ich.o
obj-$(CONFIG_GPIO_IDIO_16) += gpio-idio-16.o
obj-$(CONFIG_GPIO_IDT3243X) += gpio-idt3243x.o
+obj-$(CONFIG_GPIO_IMX_RPMSG) += gpio-imx-rpmsg.o
obj-$(CONFIG_GPIO_IMX_SCU) += gpio-imx-scu.o
obj-$(CONFIG_GPIO_IT87) += gpio-it87.o
obj-$(CONFIG_GPIO_IXP4XX) += gpio-ixp4xx.o
diff --git a/drivers/gpio/gpio-imx-rpmsg.c b/drivers/gpio/gpio-imx-rpmsg.c
new file mode 100644
index 000000000000..0f9c5ceec651
--- /dev/null
+++ b/drivers/gpio/gpio-imx-rpmsg.c
@@ -0,0 +1,559 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2025 NXP
+ *
+ * The driver exports a standard gpiochip interface to control
+ * the GPIO controllers via RPMSG on a remote processor.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/bitops.h>
+#include <linux/err.h>
+#include <linux/gpio/driver.h>
+#include <linux/imx_rpmsg.h>
+#include <linux/init.h>
+#include <linux/irqdomain.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/pm_qos.h>
+#include <linux/rpmsg.h>
+#include <linux/virtio.h>
+#include <linux/workqueue.h>
+
+#define IMX_RPMSG_GPIO_PER_PORT 32
+#define RPMSG_TIMEOUT 1000
+
+enum gpio_input_trigger_type {
+ GPIO_RPMSG_TRI_IGNORE,
+ GPIO_RPMSG_TRI_RISING,
+ GPIO_RPMSG_TRI_FALLING,
+ GPIO_RPMSG_TRI_BOTH_EDGE,
+ GPIO_RPMSG_TRI_LOW_LEVEL,
+ GPIO_RPMSG_TRI_HIGH_LEVEL,
+};
+
+enum gpio_rpmsg_header_type {
+ GPIO_RPMSG_SETUP,
+ GPIO_RPMSG_REPLY,
+ GPIO_RPMSG_NOTIFY,
+};
+
+enum gpio_rpmsg_header_cmd {
+ GPIO_RPMSG_INPUT_INIT,
+ GPIO_RPMSG_OUTPUT_INIT,
+ GPIO_RPMSG_INPUT_GET,
+};
+
+struct gpio_rpmsg_data {
+ struct imx_rpmsg_head header;
+ u8 pin_idx;
+ u8 port_idx;
+ union {
+ u8 event;
+ u8 retcode;
+ u8 value;
+ } out;
+ union {
+ u8 wakeup;
+ u8 value;
+ } in;
+} __packed __aligned(8);
+
+struct imx_rpmsg_gpio_pin {
+ u8 irq_shutdown;
+ u8 irq_unmask;
+ u8 irq_mask;
+ u32 irq_wake_enable;
+ u32 irq_type;
+ struct gpio_rpmsg_data msg;
+};
+
+struct imx_gpio_rpmsg_info {
+ struct rpmsg_device *rpdev;
+ struct gpio_rpmsg_data *notify_msg;
+ struct gpio_rpmsg_data *reply_msg;
+ struct pm_qos_request pm_qos_req;
+ struct completion cmd_complete;
+ struct mutex lock;
+ struct imx_rpmsg_gpio_port **port_store;
+};
+
+struct imx_rpmsg_gpio_port {
+ struct gpio_chip gc;
+ struct irq_chip chip;
+ struct device *dev;
+ struct irq_domain *domain;
+ struct imx_rpmsg_gpio_pin gpio_pins[IMX_RPMSG_GPIO_PER_PORT];
+ struct imx_gpio_rpmsg_info info;
+ int idx;
+};
+
+static int gpio_send_message(struct imx_rpmsg_gpio_port *port,
+ struct gpio_rpmsg_data *msg,
+ bool sync)
+{
+ struct imx_gpio_rpmsg_info *info = &port->info;
+ int err;
+
+ if (!info->rpdev) {
+ dev_dbg(&info->rpdev->dev,
+ "rpmsg channel not ready, m4 image ready?\n");
+ return -EINVAL;
+ }
+
+ cpu_latency_qos_add_request(&info->pm_qos_req, 0);
+ reinit_completion(&info->cmd_complete);
+ err = rpmsg_send(info->rpdev->ept, (void *)msg,
+ sizeof(struct gpio_rpmsg_data));
+ if (err) {
+ dev_err(&info->rpdev->dev, "rpmsg_send failed: %d\n", err);
+ goto err_out;
+ }
+
+ if (sync) {
+ err = wait_for_completion_timeout(&info->cmd_complete,
+ msecs_to_jiffies(RPMSG_TIMEOUT));
+ if (!err) {
+ dev_err(&info->rpdev->dev, "rpmsg_send timeout!\n");
+ err = -ETIMEDOUT;
+ goto err_out;
+ }
+
+ if (info->reply_msg->out.retcode != 0) {
+ dev_err(&info->rpdev->dev, "rpmsg not ack %d!\n",
+ info->reply_msg->out.retcode);
+ err = -EINVAL;
+ goto err_out;
+ }
+
+ /* copy the reply message */
+ memcpy(&port->gpio_pins[info->reply_msg->pin_idx].msg,
+ info->reply_msg, sizeof(*info->reply_msg));
+
+ err = 0;
+ }
+
+err_out:
+ cpu_latency_qos_remove_request(&info->pm_qos_req);
+
+ return err;
+}
+
+static struct gpio_rpmsg_data *gpio_get_pin_msg(struct imx_rpmsg_gpio_port *port,
+ unsigned int offset)
+{
+ struct gpio_rpmsg_data *msg = &port->gpio_pins[offset].msg;
+
+ memset(msg, 0, sizeof(struct gpio_rpmsg_data));
+
+ return msg;
+};
+
+static int imx_rpmsg_gpio_get(struct gpio_chip *gc, unsigned int gpio)
+{
+ struct imx_rpmsg_gpio_port *port = gpiochip_get_data(gc);
+ struct gpio_rpmsg_data *msg = NULL;
+ int ret;
+
+ mutex_lock(&port->info.lock);
+
+ msg = gpio_get_pin_msg(port, gpio);
+ msg->header.cate = IMX_RPMSG_GPIO;
+ msg->header.major = IMX_RMPSG_MAJOR;
+ msg->header.minor = IMX_RMPSG_MINOR;
+ msg->header.type = GPIO_RPMSG_SETUP;
+ msg->header.cmd = GPIO_RPMSG_INPUT_GET;
+ msg->pin_idx = gpio;
+ msg->port_idx = port->idx;
+
+ ret = gpio_send_message(port, msg, true);
+ if (!ret)
+ ret = !!port->gpio_pins[gpio].msg.in.value;
+
+ mutex_unlock(&port->info.lock);
+
+ return ret;
+}
+
+static int imx_rpmsg_gpio_direction_input(struct gpio_chip *gc,
+ unsigned int gpio)
+{
+ struct imx_rpmsg_gpio_port *port = gpiochip_get_data(gc);
+ struct gpio_rpmsg_data *msg = NULL;
+ int ret;
+
+ mutex_lock(&port->info.lock);
+
+ msg = gpio_get_pin_msg(port, gpio);
+ msg->header.cate = IMX_RPMSG_GPIO;
+ msg->header.major = IMX_RMPSG_MAJOR;
+ msg->header.minor = IMX_RMPSG_MINOR;
+ msg->header.type = GPIO_RPMSG_SETUP;
+ msg->header.cmd = GPIO_RPMSG_INPUT_INIT;
+ msg->pin_idx = gpio;
+ msg->port_idx = port->idx;
+
+ msg->out.event = GPIO_RPMSG_TRI_IGNORE;
+ msg->in.wakeup = 0;
+
+ ret = gpio_send_message(port, msg, true);
+
+ mutex_unlock(&port->info.lock);
+
+ return ret;
+}
+
+static inline void imx_rpmsg_gpio_direction_output_init(struct gpio_chip *gc,
+ unsigned int gpio, int val, struct gpio_rpmsg_data *msg)
+{
+ struct imx_rpmsg_gpio_port *port = gpiochip_get_data(gc);
+
+ msg->header.cate = IMX_RPMSG_GPIO;
+ msg->header.major = IMX_RMPSG_MAJOR;
+ msg->header.minor = IMX_RMPSG_MINOR;
+ msg->header.type = GPIO_RPMSG_SETUP;
+ msg->header.cmd = GPIO_RPMSG_OUTPUT_INIT;
+ msg->pin_idx = gpio;
+ msg->port_idx = port->idx;
+ msg->out.value = val;
+}
+
+static int imx_rpmsg_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
+{
+ struct imx_rpmsg_gpio_port *port = gpiochip_get_data(gc);
+ struct gpio_rpmsg_data *msg = NULL;
+ int ret;
+
+ mutex_lock(&port->info.lock);
+
+ msg = gpio_get_pin_msg(port, gpio);
+ imx_rpmsg_gpio_direction_output_init(gc, gpio, val, msg);
+ ret = gpio_send_message(port, msg, true);
+
+ mutex_unlock(&port->info.lock);
+
+ return ret;
+}
+
+static int imx_rpmsg_gpio_direction_output(struct gpio_chip *gc,
+ unsigned int gpio, int val)
+{
+ struct imx_rpmsg_gpio_port *port = gpiochip_get_data(gc);
+ struct gpio_rpmsg_data *msg = NULL;
+ int ret;
+
+ mutex_lock(&port->info.lock);
+
+ msg = gpio_get_pin_msg(port, gpio);
+ imx_rpmsg_gpio_direction_output_init(gc, gpio, val, msg);
+ ret = gpio_send_message(port, msg, true);
+
+ mutex_unlock(&port->info.lock);
+
+ return ret;
+}
+
+static int imx_rpmsg_irq_set_type(struct irq_data *d, u32 type)
+{
+ struct imx_rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
+ u32 gpio_idx = d->hwirq;
+ int edge = 0;
+ int ret = 0;
+
+ switch (type) {
+ case IRQ_TYPE_EDGE_RISING:
+ edge = GPIO_RPMSG_TRI_RISING;
+ break;
+ case IRQ_TYPE_EDGE_FALLING:
+ edge = GPIO_RPMSG_TRI_FALLING;
+ break;
+ case IRQ_TYPE_EDGE_BOTH:
+ edge = GPIO_RPMSG_TRI_BOTH_EDGE;
+ break;
+ case IRQ_TYPE_LEVEL_LOW:
+ edge = GPIO_RPMSG_TRI_LOW_LEVEL;
+ break;
+ case IRQ_TYPE_LEVEL_HIGH:
+ edge = GPIO_RPMSG_TRI_HIGH_LEVEL;
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+
+ port->gpio_pins[gpio_idx].irq_type = edge;
+
+ return ret;
+}
+
+static int imx_rpmsg_irq_set_wake(struct irq_data *d, u32 enable)
+{
+ struct imx_rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
+ u32 gpio_idx = d->hwirq;
+
+ port->gpio_pins[gpio_idx].irq_wake_enable = enable;
+
+ return 0;
+}
+
+/*
+ * This function will be called at:
+ * - one interrupt setup.
+ * - the end of one interrupt happened
+ * The gpio over rpmsg driver will not write the real register, so save
+ * all infos before this function and then send all infos to M core in this
+ * step.
+ */
+static void imx_rpmsg_unmask_irq(struct irq_data *d)
+{
+ struct imx_rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
+ u32 gpio_idx = d->hwirq;
+
+ port->gpio_pins[gpio_idx].irq_unmask = 1;
+}
+
+static void imx_rpmsg_mask_irq(struct irq_data *d)
+{
+ struct imx_rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
+ u32 gpio_idx = d->hwirq;
+ /*
+ * No need to implement the callback at A core side.
+ * M core will mask interrupt after a interrupt occurred, and then
+ * sends a notify to A core.
+ * After A core dealt with the notify, A core will send a rpmsg to
+ * M core to unmask this interrupt again.
+ */
+ port->gpio_pins[gpio_idx].irq_mask = 1;
+}
+
+static void imx_rpmsg_irq_shutdown(struct irq_data *d)
+{
+ struct imx_rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
+ u32 gpio_idx = d->hwirq;
+
+ port->gpio_pins[gpio_idx].irq_shutdown = 1;
+}
+
+static void imx_rpmsg_irq_bus_lock(struct irq_data *d)
+{
+ struct imx_rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
+
+ mutex_lock(&port->info.lock);
+}
+
+static void imx_rpmsg_irq_bus_sync_unlock(struct irq_data *d)
+{
+ struct imx_rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
+ struct gpio_rpmsg_data *msg = NULL;
+ u32 gpio_idx = d->hwirq;
+
+ if (port == NULL) {
+ mutex_unlock(&port->info.lock);
+ return;
+ }
+
+ /*
+ * For mask irq, do nothing here.
+ * M core will mask interrupt after a interrupt occurred, and then
+ * sends a notify to A core.
+ * After A core dealt with the notify, A core will send a rpmsg to
+ * M core to unmask this interrupt again.
+ */
+
+ if (port->gpio_pins[gpio_idx].irq_mask && !port->gpio_pins[gpio_idx].irq_unmask) {
+ port->gpio_pins[gpio_idx].irq_mask = 0;
+ mutex_unlock(&port->info.lock);
+ return;
+ }
+
+ msg = gpio_get_pin_msg(port, gpio_idx);
+ msg->header.cate = IMX_RPMSG_GPIO;
+ msg->header.major = IMX_RMPSG_MAJOR;
+ msg->header.minor = IMX_RMPSG_MINOR;
+ msg->header.type = GPIO_RPMSG_SETUP;
+ msg->header.cmd = GPIO_RPMSG_INPUT_INIT;
+ msg->pin_idx = gpio_idx;
+ msg->port_idx = port->idx;
+
+ if (port->gpio_pins[gpio_idx].irq_shutdown) {
+ msg->out.event = GPIO_RPMSG_TRI_IGNORE;
+ msg->in.wakeup = 0;
+ port->gpio_pins[gpio_idx].irq_shutdown = 0;
+ } else {
+ /* if not set irq type, then use low level as trigger type */
+ msg->out.event = port->gpio_pins[gpio_idx].irq_type;
+ if (!msg->out.event)
+ msg->out.event = GPIO_RPMSG_TRI_LOW_LEVEL;
+ if (port->gpio_pins[gpio_idx].irq_unmask) {
+ msg->in.wakeup = 0;
+ port->gpio_pins[gpio_idx].irq_unmask = 0;
+ } else /* irq set wake */
+ msg->in.wakeup = port->gpio_pins[gpio_idx].irq_wake_enable;
+ }
+
+ gpio_send_message(port, msg, false);
+ mutex_unlock(&port->info.lock);
+}
+
+static struct irq_chip imx_rpmsg_irq_chip = {
+ .irq_mask = imx_rpmsg_mask_irq,
+ .irq_unmask = imx_rpmsg_unmask_irq,
+ .irq_set_wake = imx_rpmsg_irq_set_wake,
+ .irq_set_type = imx_rpmsg_irq_set_type,
+ .irq_shutdown = imx_rpmsg_irq_shutdown,
+ .irq_bus_lock = imx_rpmsg_irq_bus_lock,
+ .irq_bus_sync_unlock = imx_rpmsg_irq_bus_sync_unlock,
+};
+
+static int imx_rpmsg_gpio_callback(struct rpmsg_device *rpdev,
+ void *data, int len, void *priv, u32 src)
+{
+ struct gpio_rpmsg_data *msg = (struct gpio_rpmsg_data *)data;
+ unsigned long flags;
+ struct imx_rpmsg_gpio_port *port;
+ struct imx_rpmsg_driver_data *drvdata;
+
+ drvdata = dev_get_drvdata(&rpdev->dev);
+ if (msg)
+ port = drvdata->channel_devices[msg->port_idx];
+ if (!port)
+ return -ENODEV;
+
+ if (msg->header.type == GPIO_RPMSG_REPLY) {
+ port->info.reply_msg = msg;
+ complete(&port->info.cmd_complete);
+ } else if (msg->header.type == GPIO_RPMSG_NOTIFY) {
+ port->info.notify_msg = msg;
+ local_irq_save(flags);
+ generic_handle_domain_irq(port->domain, msg->pin_idx);
+ local_irq_restore(flags);
+ } else
+ dev_err(&rpdev->dev, "wrong command type!\n");
+
+ return 0;
+}
+
+static int imx_rpmsg_gpio_to_irq(struct gpio_chip *gc, unsigned int gpio)
+{
+ struct imx_rpmsg_gpio_port *port = gpiochip_get_data(gc);
+ int irq;
+
+ irq = irq_find_mapping(port->domain, gpio);
+ if (irq > 0) {
+ irq_set_chip_data(irq, port);
+ irq_set_chip_and_handler(irq, &port->chip, handle_level_irq);
+ }
+
+ return irq;
+}
+
+static void imx_rpmsg_gpio_remove_action(void *data)
+{
+ struct imx_rpmsg_gpio_port *port = data;
+
+ irq_domain_remove(port->domain);
+ port->info.port_store[port->idx] = 0;
+}
+
+static int imx_rpmsg_gpio_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct imx_rpmsg_driver_data *pltdata = dev->platform_data;
+ struct device_node *np = dev->of_node;
+ struct imx_rpmsg_gpio_port *port;
+ struct gpio_chip *gc;
+ int irq_base;
+ int ret;
+
+ if (!pltdata)
+ return -EPROBE_DEFER;
+
+ port = devm_kzalloc(&pdev->dev, sizeof(*port), GFP_KERNEL);
+ if (!port)
+ return -ENOMEM;
+
+ ret = of_property_read_u32(np, "reg", &port->idx);
+ if (ret)
+ return ret;
+
+ if (port->idx > MAX_DEV_PER_CHANNEL)
+ return -EINVAL;
+
+ mutex_init(&port->info.lock);
+ init_completion(&port->info.cmd_complete);
+ port->info.rpdev = pltdata->rpdev;
+ port->info.port_store = (struct imx_rpmsg_gpio_port **)pltdata->channel_devices;
+ port->info.port_store[port->idx] = port;
+ if (!pltdata->rx_callback)
+ pltdata->rx_callback = imx_rpmsg_gpio_callback;
+
+ gc = &port->gc;
+ gc->parent = &pltdata->rpdev->dev;
+ gc->label = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s-gpio%d",
+ pltdata->rproc_name, port->idx);
+ gc->ngpio = IMX_RPMSG_GPIO_PER_PORT;
+ gc->base = -1;
+
+ gc->to_irq = imx_rpmsg_gpio_to_irq;
+ gc->direction_input = imx_rpmsg_gpio_direction_input;
+ gc->direction_output = imx_rpmsg_gpio_direction_output;
+ gc->get = imx_rpmsg_gpio_get;
+ gc->set = imx_rpmsg_gpio_set;
+
+ platform_set_drvdata(pdev, port);
+ ret = devm_gpiochip_add_data(dev, gc, port);
+ if (ret < 0)
+ return ret;
+
+ devm_add_action_or_reset(&pdev->dev, imx_rpmsg_gpio_remove_action, port);
+
+ /* create an irq domain */
+ port->chip = imx_rpmsg_irq_chip;
+ port->chip.name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s-gpio%d",
+ pltdata->rproc_name, port->idx);
+ port->dev = &pdev->dev;
+
+ irq_base = devm_irq_alloc_descs(&pdev->dev, -1, 0, IMX_RPMSG_GPIO_PER_PORT,
+ numa_node_id());
+ if (irq_base < 0) {
+ dev_err(&pdev->dev, "Failed to alloc irq_descs\n");
+ return irq_base;
+ }
+
+ port->domain = irq_domain_create_legacy(of_node_to_fwnode(np),
+ IMX_RPMSG_GPIO_PER_PORT,
+ irq_base, 0,
+ &irq_domain_simple_ops, port);
+ if (!port->domain) {
+ dev_err(&pdev->dev, "Failed to allocate IRQ domain\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static const struct of_device_id imx_rpmsg_gpio_dt_ids[] = {
+ { .compatible = "fsl,imx-rpmsg-gpio" },
+ { /* sentinel */ }
+};
+
+static struct platform_driver imx_rpmsg_gpio_driver = {
+ .driver = {
+ .name = "gpio-imx-rpmsg",
+ .of_match_table = imx_rpmsg_gpio_dt_ids,
+ },
+ .probe = imx_rpmsg_gpio_probe,
+};
+
+static int __init gpio_imx_rpmsg_init(void)
+{
+ return platform_driver_register(&imx_rpmsg_gpio_driver);
+}
+
+device_initcall(gpio_imx_rpmsg_init);
+
+MODULE_AUTHOR("NXP Semiconductor");
+MODULE_DESCRIPTION("NXP i.MX SoC rpmsg gpio driver");
+MODULE_LICENSE("GPL");
--
2.43.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 4/4] arm64: dts: imx8ulp: Add rpmsg node under imx_rproc
2025-08-18 20:44 [PATCH 0/4] Enable Remote GPIO over RPMSG on i.MX Platform Shenwei Wang
` (2 preceding siblings ...)
2025-08-18 20:44 ` [PATCH 3/4] gpio: imx-rpmsg: add imx-rpmsg GPIO driver Shenwei Wang
@ 2025-08-18 20:44 ` Shenwei Wang
2025-08-20 20:49 ` [PATCH 0/4] Enable Remote GPIO over RPMSG on i.MX Platform Mathieu Poirier
4 siblings, 0 replies; 19+ messages in thread
From: Shenwei Wang @ 2025-08-18 20:44 UTC (permalink / raw)
To: Bjorn Andersson, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
Linus Walleij, Bartosz Golaszewski
Cc: Pengutronix Kernel Team, Fabio Estevam, Shenwei Wang, Peng Fan,
linux-remoteproc, devicetree, imx, linux-arm-kernel, linux-kernel,
linux-imx
Add the RPMSG bus node along with its GPIO subnodes to the device
tree.
Enable remote device communication and GPIO control via RPMSG on
the i.MX platform.
Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
---
arch/arm64/boot/dts/freescale/imx8ulp.dtsi | 27 ++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
index 13b01f3aa2a4..6ab1c12a3bc1 100644
--- a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
@@ -191,6 +191,33 @@ scmi_sensor: protocol@15 {
cm33: remoteproc-cm33 {
compatible = "fsl,imx8ulp-cm33";
status = "disabled";
+
+ rpmsg {
+ rpmsg-io-channel {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ rpmsg_gpioa: gpio@0 {
+ compatible = "fsl,imx-rpmsg-gpio";
+ reg = <0>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ #interrupt-cells = <2>;
+ interrupt-controller;
+ interrupt-parent = <&rpmsg_gpioa>;
+ };
+
+ rpmsg_gpiob: gpio@1 {
+ compatible = "fsl,imx-rpmsg-gpio";
+ reg = <1>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ #interrupt-cells = <2>;
+ interrupt-controller;
+ interrupt-parent = <&rpmsg_gpiob>;
+ };
+ };
+ };
};
soc: soc@0 {
--
2.43.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 0/4] Enable Remote GPIO over RPMSG on i.MX Platform
2025-08-18 20:44 [PATCH 0/4] Enable Remote GPIO over RPMSG on i.MX Platform Shenwei Wang
` (3 preceding siblings ...)
2025-08-18 20:44 ` [PATCH 4/4] arm64: dts: imx8ulp: Add rpmsg node under imx_rproc Shenwei Wang
@ 2025-08-20 20:49 ` Mathieu Poirier
2025-08-20 22:29 ` Shenwei Wang
4 siblings, 1 reply; 19+ messages in thread
From: Mathieu Poirier @ 2025-08-20 20:49 UTC (permalink / raw)
To: Shenwei Wang
Cc: Bjorn Andersson, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Shawn Guo, Sascha Hauer, Linus Walleij, Bartosz Golaszewski,
Pengutronix Kernel Team, Fabio Estevam, Peng Fan,
linux-remoteproc, devicetree, imx, linux-arm-kernel, linux-kernel,
linux-imx
Did you send me a patchset that doesn't clear checkpatch.pl?
On Mon, Aug 18, 2025 at 03:44:16PM -0500, Shenwei Wang wrote:
> Support the remote devices on the remote processor via the RPMSG bus on
> i.MX platform.
>
> The expected DTS layout structure is following:
>
> cm33: remoteproc-cm33 {
> compatible = "fsl,imx8ulp-cm33";
>
> rpmsg {
> rpmsg-io-channel {
> gpio@0 {
> compatible = "fsl,imx-rpmsg-gpio";
> reg = <0>;
> };
>
> gpio@1 {
> compatible = "fsl,imx-rpmsg-gpio";
> reg = <1>;
> };
>
> ...
> };
>
> rpmsg-i2c-channel {
> i2c@0 {
> compatible = "fsl,imx-rpmsg-i2c";
> reg = <0>;
> };
> };
>
> ...
> };
> };
>
>
> Shenwei Wang (4):
> dt-bindings: remoteproc: imx_rproc: Add "rpmsg" subnode support
> remoteproc: imx_rproc: Populate devices under "rpmsg" subnode
> gpio: imx-rpmsg: add imx-rpmsg GPIO driver
> arm64: dts: imx8ulp: Add rpmsg node under imx_rproc
>
> .../bindings/remoteproc/fsl,imx-rproc.yaml | 117 ++++
> arch/arm64/boot/dts/freescale/imx8ulp.dtsi | 27 +
> drivers/gpio/Kconfig | 11 +
> drivers/gpio/Makefile | 1 +
> drivers/gpio/gpio-imx-rpmsg.c | 559 ++++++++++++++++++
> drivers/remoteproc/imx_rproc.c | 125 ++++
> include/linux/imx_rpmsg.h | 55 ++
> 7 files changed, 895 insertions(+)
> create mode 100644 drivers/gpio/gpio-imx-rpmsg.c
> create mode 100644 include/linux/imx_rpmsg.h
>
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/4] Enable Remote GPIO over RPMSG on i.MX Platform
2025-08-20 20:49 ` [PATCH 0/4] Enable Remote GPIO over RPMSG on i.MX Platform Mathieu Poirier
@ 2025-08-20 22:29 ` Shenwei Wang
0 siblings, 0 replies; 19+ messages in thread
From: Shenwei Wang @ 2025-08-20 22:29 UTC (permalink / raw)
To: Mathieu Poirier
Cc: Bjorn Andersson, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Shawn Guo, Sascha Hauer, Linus Walleij, Bartosz Golaszewski,
Pengutronix Kernel Team, Fabio Estevam, Peng Fan,
linux-remoteproc@vger.kernel.org, devicetree@vger.kernel.org,
imx@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, dl-linux-imx
> -----Original Message-----
> From: Mathieu Poirier <mathieu.poirier@linaro.org>
> Sent: Wednesday, August 20, 2025 3:49 PM
> To: Shenwei Wang <shenwei.wang@nxp.com>
> Cc: Bjorn Andersson <andersson@kernel.org>; Rob Herring <robh@kernel.org>;
> Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
> <conor+dt@kernel.org>; Shawn Guo <shawnguo@kernel.org>; Sascha Hauer
> <s.hauer@pengutronix.de>; Linus Walleij <linus.walleij@linaro.org>; Bartosz
> Golaszewski <brgl@bgdev.pl>; Pengutronix Kernel Team
> <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; Peng Fan
> <peng.fan@nxp.com>; linux-remoteproc@vger.kernel.org;
> devicetree@vger.kernel.org; imx@lists.linux.dev; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; dl-linux-imx <linux-
> imx@nxp.com>
> Subject: [EXT] Re: [PATCH 0/4] Enable Remote GPIO over RPMSG on i.MX
> Platform
>
>
> Did you send me a patchset that doesn't clear checkpatch.pl?
>
There is one known warning:
WARNING: function definition argument 'struct rpmsg_device *' should also have an identifier name
#268: FILE: include/linux/imx_rpmsg.h:51:
+ int (*rx_callback)(struct rpmsg_device *, void *, int, void *, u32);
I just kept the format as the existing style. If it is required to be fixed, I will update it in next version.
Regards,
Shenwei
> On Mon, Aug 18, 2025 at 03:44:16PM -0500, Shenwei Wang wrote:
> > Support the remote devices on the remote processor via the RPMSG bus
> > on i.MX platform.
> >
> > The expected DTS layout structure is following:
> >
> > cm33: remoteproc-cm33 {
> > compatible = "fsl,imx8ulp-cm33";
> >
> > rpmsg {
> > rpmsg-io-channel {
> > gpio@0 {
> > compatible = "fsl,imx-rpmsg-gpio";
> > reg = <0>;
> > };
> >
> > gpio@1 {
> > compatible = "fsl,imx-rpmsg-gpio";
> > reg = <1>;
> > };
> >
> > ...
> > };
> >
> > rpmsg-i2c-channel {
> > i2c@0 {
> > compatible = "fsl,imx-rpmsg-i2c";
> > reg = <0>;
> > };
> > };
> >
> > ...
> > };
> > };
> >
> >
> > Shenwei Wang (4):
> > dt-bindings: remoteproc: imx_rproc: Add "rpmsg" subnode support
> > remoteproc: imx_rproc: Populate devices under "rpmsg" subnode
> > gpio: imx-rpmsg: add imx-rpmsg GPIO driver
> > arm64: dts: imx8ulp: Add rpmsg node under imx_rproc
> >
> > .../bindings/remoteproc/fsl,imx-rproc.yaml | 117 ++++
> > arch/arm64/boot/dts/freescale/imx8ulp.dtsi | 27 +
> > drivers/gpio/Kconfig | 11 +
> > drivers/gpio/Makefile | 1 +
> > drivers/gpio/gpio-imx-rpmsg.c | 559 ++++++++++++++++++
> > drivers/remoteproc/imx_rproc.c | 125 ++++
> > include/linux/imx_rpmsg.h | 55 ++
> > 7 files changed, 895 insertions(+)
> > create mode 100644 drivers/gpio/gpio-imx-rpmsg.c create mode 100644
> > include/linux/imx_rpmsg.h
> >
> > --
> > 2.43.0
> >
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/4] gpio: imx-rpmsg: add imx-rpmsg GPIO driver
2025-08-18 20:44 ` [PATCH 3/4] gpio: imx-rpmsg: add imx-rpmsg GPIO driver Shenwei Wang
@ 2025-08-21 9:01 ` Linus Walleij
2025-09-01 7:26 ` Arnaud POULIQUEN
2025-08-21 14:11 ` kernel test robot
1 sibling, 1 reply; 19+ messages in thread
From: Linus Walleij @ 2025-08-21 9:01 UTC (permalink / raw)
To: Shenwei Wang, Bjorn Andersson, Mathieu Poirier
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer, Bartosz Golaszewski, Pengutronix Kernel Team,
Fabio Estevam, Peng Fan, linux-remoteproc, devicetree, imx,
linux-arm-kernel, linux-kernel, linux-imx
Hi Shenwei,
thanks for your patch!
On Mon, Aug 18, 2025 at 10:45 PM Shenwei Wang <shenwei.wang@nxp.com> wrote:
> On i.MX SoCs, the system may include two processors:
> - An MCU running an RTOS
> - An MPU running Linux
>
> These processors communicate via the RPMSG protocol.
> The driver implements the standard GPIO interface, allowing
> the Linux side to control GPIO controllers which reside in
> the remote processor via RPMSG protocol.
>
> Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
Since this is a first RPMSG GPIO driver, I'd like if Björn and/or
Mathieu have a look at it so I'm sure it is RPMSG-proper!
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index a437fe652dbc..2ce4e9b5225e 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -402,6 +402,17 @@ config GPIO_ICH
>
> If unsure, say N.
>
> +config GPIO_IMX_RPMSG
> + tristate "NXP i.MX SoC RPMSG GPIO support"
> + depends on IMX_REMOTEPROC && RPMSG && GPIOLIB
> + default IMX_REMOTEPROC
> + help
> + Say yes here to support the RPMSG GPIO functions on i.MX SoC based
> + platform. Currently supported devices: i.MX7ULP, i.MX8ULP, i.MX8x,
> + and i.MX9x.
> +
> + If unsure, say N.
This is sorted under memory-mapped GPIO, but it isn't.
Create a new submenu:
menu "RPMSG GPIO drivers"
depends on RPMSG
And put it here as the first such driver.
No need to have a dependency on RPMSG in the GPIO_IMX_RPMSG
Kconfig entry after this.
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/bitops.h>
bitops.h or just bits.h? Check which one you actually use.
> +#include <linux/err.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/imx_rpmsg.h>
> +#include <linux/init.h>
> +#include <linux/irqdomain.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_qos.h>
Are you really using pm_qos?
> +#include <linux/rpmsg.h>
> +#include <linux/virtio.h>
> +#include <linux/workqueue.h>
(...)
> +struct imx_rpmsg_gpio_port {
> + struct gpio_chip gc;
> + struct irq_chip chip;
This irqchip doesn't look very immutable.
Look at other patches rewriting irqchips to be immutable
and break this out to a static const struct irq_chip with
IRQCHIP_IMMUTABLE set instead.
> +static int imx_rpmsg_gpio_get(struct gpio_chip *gc, unsigned int gpio)
> +{
> + struct imx_rpmsg_gpio_port *port = gpiochip_get_data(gc);
> + struct gpio_rpmsg_data *msg = NULL;
> + int ret;
> +
> + mutex_lock(&port->info.lock);
Please use guards for all the mutexes:
#include <linux/cleanup.h>
guard(mutex)(&port->info.lock);
and it will be released as you exit the function.
> +static int imx_rpmsg_gpio_direction_input(struct gpio_chip *gc,
> + unsigned int gpio)
> +{
> + struct imx_rpmsg_gpio_port *port = gpiochip_get_data(gc);
> + struct gpio_rpmsg_data *msg = NULL;
> + int ret;
> +
> + mutex_lock(&port->info.lock);
Dito for all these instances.
(Saves you a bunch of lines!)
> +static void imx_rpmsg_irq_bus_lock(struct irq_data *d)
> +{
> + struct imx_rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> +
> + mutex_lock(&port->info.lock);
> +}
Here you need to keep the classic mutex_lock() though,
because of the irqchip locking abstraction helper.
> +static struct irq_chip imx_rpmsg_irq_chip = {
const
> + .irq_mask = imx_rpmsg_mask_irq,
> + .irq_unmask = imx_rpmsg_unmask_irq,
> + .irq_set_wake = imx_rpmsg_irq_set_wake,
> + .irq_set_type = imx_rpmsg_irq_set_type,
> + .irq_shutdown = imx_rpmsg_irq_shutdown,
> + .irq_bus_lock = imx_rpmsg_irq_bus_lock,
> + .irq_bus_sync_unlock = imx_rpmsg_irq_bus_sync_unlock,
.flags = IRQCHIP_IMMUTABLE,
probably also:
GPIOCHIP_IRQ_RESOURCE_HELPERS,
?
I think you want to properly mark GPIO lines as used for
IRQs!
> +static int imx_rpmsg_gpio_to_irq(struct gpio_chip *gc, unsigned int gpio)
> +{
> + struct imx_rpmsg_gpio_port *port = gpiochip_get_data(gc);
> + int irq;
> +
> + irq = irq_find_mapping(port->domain, gpio);
> + if (irq > 0) {
> + irq_set_chip_data(irq, port);
> + irq_set_chip_and_handler(irq, &port->chip, handle_level_irq);
> + }
> +
> + return irq;
> +}
Ugh we try to to use custom to_irq() if we can...
Do you have to?
Can't you use
select GPIOLIB_IRQCHIP
and be inspired by other chips using the irqchip
helper library?
We almost always use that these days.
> + /* create an irq domain */
> + port->chip = imx_rpmsg_irq_chip;
> + port->chip.name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s-gpio%d",
> + pltdata->rproc_name, port->idx);
> + port->dev = &pdev->dev;
> +
> + irq_base = devm_irq_alloc_descs(&pdev->dev, -1, 0, IMX_RPMSG_GPIO_PER_PORT,
> + numa_node_id());
> + if (irq_base < 0) {
> + dev_err(&pdev->dev, "Failed to alloc irq_descs\n");
> + return irq_base;
> + }
> +
> + port->domain = irq_domain_create_legacy(of_node_to_fwnode(np),
> + IMX_RPMSG_GPIO_PER_PORT,
> + irq_base, 0,
> + &irq_domain_simple_ops, port);
> + if (!port->domain) {
> + dev_err(&pdev->dev, "Failed to allocate IRQ domain\n");
> + return -EINVAL;
> + }
This also looks unnecessarily custom.
Try to use GPIOLIB_IRQCHIP.
> +static struct platform_driver imx_rpmsg_gpio_driver = {
> + .driver = {
> + .name = "gpio-imx-rpmsg",
> + .of_match_table = imx_rpmsg_gpio_dt_ids,
> + },
> + .probe = imx_rpmsg_gpio_probe,
> +};
> +
> +static int __init gpio_imx_rpmsg_init(void)
> +{
> + return platform_driver_register(&imx_rpmsg_gpio_driver);
> +}
> +
> +device_initcall(gpio_imx_rpmsg_init);
No please just do:
module_platform_driver(imx_rpmsg_gpio_driver);
Fix up these things to begin with and then we can
look at details!
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/4] remoteproc: imx_rproc: Populate devices under "rpmsg" subnode
2025-08-18 20:44 ` [PATCH 2/4] remoteproc: imx_rproc: Populate devices under "rpmsg" subnode Shenwei Wang
@ 2025-08-21 11:02 ` Peng Fan
2025-08-21 18:51 ` Shenwei Wang
0 siblings, 1 reply; 19+ messages in thread
From: Peng Fan @ 2025-08-21 11:02 UTC (permalink / raw)
To: Shenwei Wang
Cc: Bjorn Andersson, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
Linus Walleij, Bartosz Golaszewski, Pengutronix Kernel Team,
Fabio Estevam, Peng Fan, linux-remoteproc, devicetree, imx,
linux-arm-kernel, linux-kernel, linux-imx
Hi Shenwei,
On Mon, Aug 18, 2025 at 03:44:18PM -0500, Shenwei Wang wrote:
>Register the RPMsg channel driver and populate remote devices defined
>under the "rpmsg" subnode upon receiving their notification messages.
>
>The following illustrates the expected DTS layout structure:
>
> cm33: remoteproc-cm33 {
> compatible = "fsl,imx8ulp-cm33";
>
> rpmsg {
> rpmsg-io-channel {
> gpio@0 {
> compatible = "fsl,imx-rpmsg-gpio";
> reg = <0>;
> };
>
> gpio@1 {
> compatible = "fsl,imx-rpmsg-gpio";
> reg = <1>;
> };
>
> ...
> };
>
> rpmsg-i2c-channel {
> i2c@0 {
> compatible = "fsl,imx-rpmsg-i2c";
> reg = <0>;
> };
> };
>
> ...
> };
Need dt-binding maintainers to review the binding part first.
Good to see this feature, so I will still give a look on the driver changes.
> };
>
>Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
>---
> drivers/remoteproc/imx_rproc.c | 125 +++++++++++++++++++++++++++++++++
> include/linux/imx_rpmsg.h | 55 +++++++++++++++
> 2 files changed, 180 insertions(+)
> create mode 100644 include/linux/imx_rpmsg.h
>
>diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
>index a6eef0080ca9..9b3396f3f1ec 100644
>--- a/drivers/remoteproc/imx_rproc.c
>+++ b/drivers/remoteproc/imx_rproc.c
>@@ -8,6 +8,7 @@
> #include <linux/clk.h>
> #include <linux/err.h>
> #include <linux/firmware/imx/sci.h>
>+#include <linux/imx_rpmsg.h>
> #include <linux/interrupt.h>
> #include <linux/kernel.h>
> #include <linux/mailbox_client.h>
>@@ -15,6 +16,8 @@
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/of_address.h>
>+#include <linux/of_irq.h>
>+#include <linux/of_platform.h>
> #include <linux/of_reserved_mem.h>
> #include <linux/platform_device.h>
> #include <linux/pm_domain.h>
>@@ -22,6 +25,7 @@
> #include <linux/reboot.h>
> #include <linux/regmap.h>
> #include <linux/remoteproc.h>
>+#include <linux/rpmsg.h>
> #include <linux/workqueue.h>
>
> #include "imx_rproc.h"
>@@ -1084,6 +1088,126 @@ static int imx_rproc_sys_off_handler(struct sys_off_data *data)
> return NOTIFY_DONE;
> }
>
Just wonder could the changes be moved to drivers/rpmsg?
>+struct imx_rpmsg_driver {
>+ struct rpmsg_driver rpdrv;
>+ void *driver_data;
>+};
>+
>+static char *channel_device_map[][2] = {
>+ {"rpmsg-io-channel", "fsl,imx-rpmsg-gpio"},
>+ {"rpmsg-i2c-channel", "fsl,imx-rpmsg-i2c"},
>+};
>+
>+static int imx_rpmsg_endpoint_cb(struct rpmsg_device *rpdev,
>+ void *data, int len, void *priv, u32 src)
>+{
>+ struct imx_rpmsg_driver_data *drvdata;
>+
>+ drvdata = dev_get_drvdata(&rpdev->dev);
>+ if (drvdata && drvdata->rx_callback)
>+ return drvdata->rx_callback(rpdev, data, len, priv, src);
>+
>+ return 0;
>+}
>+
>+static void imx_rpmsg_endpoint_remove(struct rpmsg_device *rpdev)
>+{
>+ of_platform_depopulate(&rpdev->dev);
>+}
>+
>+static int imx_rpmsg_endpoint_probe(struct rpmsg_device *rpdev)
>+{
>+ struct imx_rpmsg_driver_data *drvdata;
>+ struct imx_rpmsg_driver *imx_rpdrv;
>+ struct device *dev = &rpdev->dev;
>+ struct of_dev_auxdata *auxdata;
>+ struct rpmsg_driver *rpdrv;
>+ int i;
>+
>+ rpdrv = container_of(dev->driver, struct rpmsg_driver, drv);
>+ imx_rpdrv = container_of(rpdrv, struct imx_rpmsg_driver, rpdrv);
>+
>+ if (!imx_rpdrv->driver_data)
>+ return -EINVAL;
>+
>+ drvdata = devm_kmemdup(dev, imx_rpdrv->driver_data, sizeof(*drvdata), GFP_KERNEL);
>+ if (!drvdata)
>+ return -ENOMEM;
>+
>+ i = drvdata->map_idx;
>+ if (i >= ARRAY_SIZE(channel_device_map))
>+ return -ENODEV;
>+
>+ auxdata = devm_kzalloc(dev, sizeof(*auxdata)*2, GFP_KERNEL);
>+ if (!auxdata)
>+ return -ENOMEM;
>+
>+ drvdata->rpdev = rpdev;
>+ auxdata[0].compatible = channel_device_map[i][1];
>+ auxdata[0].platform_data = drvdata;
>+ dev_set_drvdata(dev, drvdata);
>+
>+ of_platform_populate(drvdata->channel_node, NULL, auxdata, dev);
>+ of_node_put(drvdata->channel_node);
>+
>+ return 0;
>+}
>+
>+static int imx_of_rpmsg_node_init(struct platform_device *pdev)
>+{
>+ struct device_node *np __free(device_node), *channel;
>+ struct imx_rpmsg_driver_data *driver_data;
>+ struct imx_rpmsg_driver *rp_driver;
>+ struct rpmsg_device_id *rpdev_id;
>+ int i, ret;
>+
>+ int count = ARRAY_SIZE(channel_device_map);
>+ struct device *dev = &pdev->dev;
>+
>+ np = of_get_child_by_name(dev->of_node, "rpmsg");
>+ if (!np)
>+ return 0;
>+
>+ for (i = 0; i < count; i++) {
>+ ret = -ENOMEM;
>+ channel = of_get_child_by_name(np, channel_device_map[i][0]);
>+ if (!channel)
>+ continue;
>+
>+ rpdev_id = devm_kzalloc(dev, sizeof(*rpdev_id)*2, GFP_KERNEL);
sizeof(*rpdev_id) * 2
>+ if (!rpdev_id)
>+ break;
>+ strscpy(rpdev_id[0].name, channel_device_map[i][0], RPMSG_NAME_SIZE);
>+
>+ rp_driver = devm_kzalloc(dev, sizeof(*rp_driver), GFP_KERNEL);
>+ if (!rp_driver)
>+ break;
>+
>+ driver_data = devm_kzalloc(dev, sizeof(*driver_data), GFP_KERNEL);
>+ if (!driver_data)
>+ break;
>+
>+ ret = 0;
>+ driver_data->rproc_name = dev->of_node->name;
>+ driver_data->channel_node = channel;
>+ driver_data->map_idx = i;
>+
>+ rp_driver->rpdrv.drv.name = channel_device_map[i][0];
>+ rp_driver->rpdrv.id_table = rpdev_id;
>+ rp_driver->rpdrv.probe = imx_rpmsg_endpoint_probe;
>+ rp_driver->rpdrv.remove = imx_rpmsg_endpoint_remove;
>+ rp_driver->rpdrv.callback = imx_rpmsg_endpoint_cb;
>+ rp_driver->driver_data = driver_data;
>+
>+ register_rpmsg_driver(&rp_driver->rpdrv);
>+ }
>+
>+ if ((ret < 0) && channel)
>+ of_node_put(channel);
>+
>+ return ret;
>+}
>+
> static int imx_rproc_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
>@@ -1177,6 +1301,7 @@ static int imx_rproc_probe(struct platform_device *pdev)
> goto err_put_clk;
> }
>
>+ imx_of_rpmsg_node_init(pdev);
blank line
> return 0;
>
> err_put_clk:
>diff --git a/include/linux/imx_rpmsg.h b/include/linux/imx_rpmsg.h
This file should be put under include/linux/rpmsg/
>new file mode 100644
>index 000000000000..300ada6237be
>--- /dev/null
>+++ b/include/linux/imx_rpmsg.h
>@@ -0,0 +1,55 @@
>+/* SPDX-License-Identifier: GPL-2.0 */
>+/*
>+ * Copyright (C) 2025 NXP.
Copyright 2025 NXP
>+ *
>+ * This program is free software; you can redistribute it and/or modify
>+ * it under the terms of the GNU General Public License version 2 as
>+ * published by the Free Software Foundation.
Drop this duplicated License
>+ */
>+
>+/*
>+ * @file linux/imx_rpmsg.h
>+ *
>+ * @brief Global header file for iMX RPMSG
>+ *
>+ * @ingroup RPMSG
>+ */
>+#ifndef __LINUX_IMX_RPMSG_H__
>+#define __LINUX_IMX_RPMSG_H__
>+
>+#include <linux/completion.h>
>+#include <linux/mutex.h>
The including headers should be completed for what used in this file.
>+
>+/* Category define */
>+#define IMX_RMPSG_LIFECYCLE 1
>+#define IMX_RPMSG_PMIC 2
>+#define IMX_RPMSG_AUDIO 3
>+#define IMX_RPMSG_KEY 4
>+#define IMX_RPMSG_GPIO 5
>+#define IMX_RPMSG_RTC 6
>+#define IMX_RPMSG_SENSOR 7
>+/* rpmsg version */
>+#define IMX_RMPSG_MAJOR 1
>+#define IMX_RMPSG_MINOR 0
>+
>+#define MAX_DEV_PER_CHANNEL 10
>+
>+struct imx_rpmsg_head {
>+ u8 cate;
>+ u8 major;
>+ u8 minor;
>+ u8 type;
>+ u8 cmd;
>+ u8 reserved[5];
>+} __packed;
A comment should be added to describe each member.
Regards,
Peng
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/4] gpio: imx-rpmsg: add imx-rpmsg GPIO driver
2025-08-18 20:44 ` [PATCH 3/4] gpio: imx-rpmsg: add imx-rpmsg GPIO driver Shenwei Wang
2025-08-21 9:01 ` Linus Walleij
@ 2025-08-21 14:11 ` kernel test robot
1 sibling, 0 replies; 19+ messages in thread
From: kernel test robot @ 2025-08-21 14:11 UTC (permalink / raw)
To: Shenwei Wang, Bjorn Andersson, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
Linus Walleij, Bartosz Golaszewski
Cc: llvm, oe-kbuild-all, Pengutronix Kernel Team, Fabio Estevam,
Shenwei Wang, Peng Fan, linux-remoteproc, devicetree, imx,
linux-arm-kernel, linux-kernel, linux-imx
Hi Shenwei,
kernel test robot noticed the following build warnings:
[auto build test WARNING on remoteproc/rproc-next]
[also build test WARNING on brgl/gpio/for-next shawnguo/for-next linus/master v6.17-rc2 next-20250820]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Shenwei-Wang/dt-bindings-remoteproc-imx_rproc-Add-rpmsg-subnode-support/20250819-044803
base: https://git.kernel.org/pub/scm/linux/kernel/git/remoteproc/linux.git rproc-next
patch link: https://lore.kernel.org/r/20250818204420.794554-4-shenwei.wang%40nxp.com
patch subject: [PATCH 3/4] gpio: imx-rpmsg: add imx-rpmsg GPIO driver
config: arm64-allmodconfig (https://download.01.org/0day-ci/archive/20250821/202508212119.gamkDcXG-lkp@intel.com/config)
compiler: clang version 19.1.7 (https://github.com/llvm/llvm-project cd708029e0b2869e80abe31ddb175f7c35361f90)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250821/202508212119.gamkDcXG-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202508212119.gamkDcXG-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/gpio/gpio-imx-rpmsg.c:419:6: warning: variable 'port' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
419 | if (msg)
| ^~~
drivers/gpio/gpio-imx-rpmsg.c:421:7: note: uninitialized use occurs here
421 | if (!port)
| ^~~~
drivers/gpio/gpio-imx-rpmsg.c:419:2: note: remove the 'if' if its condition is always true
419 | if (msg)
| ^~~~~~~~
420 | port = drvdata->channel_devices[msg->port_idx];
drivers/gpio/gpio-imx-rpmsg.c:415:34: note: initialize the variable 'port' to silence this warning
415 | struct imx_rpmsg_gpio_port *port;
| ^
| = NULL
drivers/gpio/gpio-imx-rpmsg.c:503:10: error: incompatible function pointer types assigning to 'void (*)(struct gpio_chip *, unsigned int, int)' from 'int (struct gpio_chip *, unsigned int, int)' [-Wincompatible-function-pointer-types]
503 | gc->set = imx_rpmsg_gpio_set;
| ^ ~~~~~~~~~~~~~~~~~~
1 warning and 1 error generated.
vim +419 drivers/gpio/gpio-imx-rpmsg.c
409
410 static int imx_rpmsg_gpio_callback(struct rpmsg_device *rpdev,
411 void *data, int len, void *priv, u32 src)
412 {
413 struct gpio_rpmsg_data *msg = (struct gpio_rpmsg_data *)data;
414 unsigned long flags;
415 struct imx_rpmsg_gpio_port *port;
416 struct imx_rpmsg_driver_data *drvdata;
417
418 drvdata = dev_get_drvdata(&rpdev->dev);
> 419 if (msg)
420 port = drvdata->channel_devices[msg->port_idx];
421 if (!port)
422 return -ENODEV;
423
424 if (msg->header.type == GPIO_RPMSG_REPLY) {
425 port->info.reply_msg = msg;
426 complete(&port->info.cmd_complete);
427 } else if (msg->header.type == GPIO_RPMSG_NOTIFY) {
428 port->info.notify_msg = msg;
429 local_irq_save(flags);
430 generic_handle_domain_irq(port->domain, msg->pin_idx);
431 local_irq_restore(flags);
432 } else
433 dev_err(&rpdev->dev, "wrong command type!\n");
434
435 return 0;
436 }
437
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [PATCH 2/4] remoteproc: imx_rproc: Populate devices under "rpmsg" subnode
2025-08-21 11:02 ` Peng Fan
@ 2025-08-21 18:51 ` Shenwei Wang
0 siblings, 0 replies; 19+ messages in thread
From: Shenwei Wang @ 2025-08-21 18:51 UTC (permalink / raw)
To: Peng Fan (OSS)
Cc: Bjorn Andersson, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
Linus Walleij, Bartosz Golaszewski, Pengutronix Kernel Team,
Fabio Estevam, Peng Fan, linux-remoteproc@vger.kernel.org,
devicetree@vger.kernel.org, imx@lists.linux.dev,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, dl-linux-imx
> -----Original Message-----
> From: Peng Fan (OSS) <peng.fan@oss.nxp.com>
> Sent: Thursday, August 21, 2025 6:03 AM
> To: Shenwei Wang <shenwei.wang@nxp.com>
> Cc: Bjorn Andersson <andersson@kernel.org>; Mathieu Poirier
> <mathieu.poirier@linaro.org>; Rob Herring <robh@kernel.org>; Krzysztof
> Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Shawn
> Guo <shawnguo@kernel.org>; Sascha Hauer <s.hauer@pengutronix.de>; Linus
> Walleij <linus.walleij@linaro.org>; Bartosz Golaszewski <brgl@bgdev.pl>;
> Pengutronix Kernel Team <kernel@pengutronix.de>; Fabio Estevam
> <festevam@gmail.com>; Peng Fan <peng.fan@nxp.com>; linux-
> remoteproc@vger.kernel.org; devicetree@vger.kernel.org; imx@lists.linux.dev;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; dl-linux-imx
> <linux-imx@nxp.com>
> Subject: Re: [PATCH 2/4] remoteproc: imx_rproc: Populate devices under
> "rpmsg" subnode
> >
> > #include "imx_rproc.h"
> >@@ -1084,6 +1088,126 @@ static int imx_rproc_sys_off_handler(struct
> sys_off_data *data)
> > return NOTIFY_DONE;
> > }
> >
>
> Just wonder could the changes be moved to drivers/rpmsg?
Since the DTS node is under imx_rproc, the most straightforward approach is to implement the
feature here. Moving the implementation to drivers/rpmsg would introduce additional complexity
and might be less direct.
Thanks,
Shenwei
>
> >+struct imx_rpmsg_driver {
> >+ struct rpmsg_driver rpdrv;
> >+ void *driver_data;
> >+};
> >+
> >+ i = drvdata->map_idx;
> >+ if (i >= ARRAY_SIZE(channel_device_map))
> >+ return -ENODEV;
> >+
> >+ auxdata = devm_kzalloc(dev, sizeof(*auxdata)*2, GFP_KERNEL);
> >+ if (!auxdata)
> >+ return -ENOMEM;
> >+
> >+ u8 major;
> >+ u8 minor;
> >+ u8 type;
> >+ u8 cmd;
> >+ u8 reserved[5];
> >+} __packed;
>
> A comment should be added to describe each member.
>
> Regards,
> Peng
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/4] dt-bindings: remoteproc: imx_rproc: Add "rpmsg" subnode support
2025-08-18 20:44 ` [PATCH 1/4] dt-bindings: remoteproc: imx_rproc: Add "rpmsg" subnode support Shenwei Wang
@ 2025-08-26 14:50 ` Frank Li
2025-08-26 20:09 ` Rob Herring
1 sibling, 0 replies; 19+ messages in thread
From: Frank Li @ 2025-08-26 14:50 UTC (permalink / raw)
To: Shenwei Wang
Cc: Bjorn Andersson, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
Linus Walleij, Bartosz Golaszewski, Pengutronix Kernel Team,
Fabio Estevam, Peng Fan, linux-remoteproc, devicetree, imx,
linux-arm-kernel, linux-kernel, linux-imx
On Mon, Aug 18, 2025 at 03:44:17PM -0500, Shenwei Wang wrote:
> Remote processors may announce multiple devices (e.g., I2C, GPIO) over
> an RPMSG channel. These devices may require corresponding device tree
> nodes, especially when acting as providers, to supply phandles for their
> consumers.
>
> Define an RPMSG node to work as a container for a group of RPMSG channels
> under the imx_rproc node.
>
> Each subnode within "rpmsg" represents an individual RPMSG channel. The
> name of each subnode corresponds to the channel name as defined by the
> remote processor.
>
> All remote devices associated with a given channel are defined as child
> nodes under the corresponding channel node.
>
> Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
> ---
> .../bindings/remoteproc/fsl,imx-rproc.yaml | 117 ++++++++++++++++++
> 1 file changed, 117 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
> index 57d75acb0b5e..a105aac798d0 100644
> --- a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
> +++ b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
> @@ -84,6 +84,86 @@ properties:
> This property is to specify the resource id of the remote processor in SoC
> which supports SCFW
>
> + rpmsg:
> + type: object
> + additionalProperties: false
> + description:
> + Present a group of RPMSG channel devices.
> +
> + properties:
> + '#address-cells':
> + const: 1
> +
> + '#size-cells':
> + const: 0
> +
> + rpmsg-io-channel:
> + type: object
> + unevaluatedProperties: false
> + properties:
> + '#address-cells':
> + const: 1
> +
> + '#size-cells':
> + const: 0
> +
> + patternProperties:
> + "gpio@[0-9a-f]+$":
> + type: object
> + unevaluatedProperties: false
> + properties:
> + compatible:
> + enum:
> + - fsl,imx-rpmsg-gpio
> +
> + reg:
> + maxItems: 1
> +
> + required:
> + - compatible
> + - reg
> +
> + allOf:
> + - $ref: /schemas/gpio/gpio.yaml#
> + - $ref: /schemas/interrupt-controller.yaml#
> +
> + required:
> + - '#address-cells'
> + - '#size-cells'
> +
> + rpmsg-i2c-channel:
> + type: object
> + unevaluatedProperties: false
> + properties:
> + '#address-cells':
> + const: 1
> +
> + '#size-cells':
> + const: 0
> +
> + patternProperties:
> + "i2c@[0-9a-f]+$":
> + type: object
> + unevaluatedProperties: false
> + properties:
> + compatible:
> + enum:
> + - fsl,imx-rpmsg-i2c
> +
> + reg:
> + maxItems: 1
> +
> + required:
> + - compatible
> + - reg
> +
> + allOf:
> + - $ref: /schemas/i2c/i2c-controller.yaml#
> +
> + required:
> + - '#address-cells'
> + - '#size-cells'
> +
> required:
> - compatible
>
> @@ -146,5 +226,42 @@ examples:
> &mu 3 1>;
> memory-region = <&vdev0buffer>, <&vdev0vring0>, <&vdev0vring1>, <&rsc_table>;
> syscon = <&src>;
> +
> + rpmsg {
> + rpmsg-io-channel {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + gpio@0 {
> + compatible = "fsl,imx-rpmsg-gpio";
> + reg = <0>;
> + gpio-controller;
> + #gpio-cells = <2>;
> + #interrupt-cells = <2>;
> + interrupt-controller;
> + interrupt-parent = <&rpmsg_gpioa>;
> + };
> +
> + gpio@1 {
> + compatible = "fsl,imx-rpmsg-gpio";
> + reg = <1>;
> + gpio-controller;
> + #gpio-cells = <2>;
> + #interrupt-cells = <2>;
> + interrupt-controller;
> + interrupt-parent = <&rpmsg_gpiob>;
> + };
> + };
> +
> + rpmsg-i2c-channel {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + i2c@0 {
> + compatible = "fsl,imx-rpmsg-i2c";
> + reg = <0>;
> + };
> + };
> + };
> };
> ...
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/4] dt-bindings: remoteproc: imx_rproc: Add "rpmsg" subnode support
2025-08-18 20:44 ` [PATCH 1/4] dt-bindings: remoteproc: imx_rproc: Add "rpmsg" subnode support Shenwei Wang
2025-08-26 14:50 ` Frank Li
@ 2025-08-26 20:09 ` Rob Herring
2025-08-27 14:49 ` Shenwei Wang
1 sibling, 1 reply; 19+ messages in thread
From: Rob Herring @ 2025-08-26 20:09 UTC (permalink / raw)
To: Shenwei Wang
Cc: Bjorn Andersson, Mathieu Poirier, Krzysztof Kozlowski,
Conor Dooley, Shawn Guo, Sascha Hauer, Linus Walleij,
Bartosz Golaszewski, Pengutronix Kernel Team, Fabio Estevam,
Peng Fan, linux-remoteproc, devicetree, imx, linux-arm-kernel,
linux-kernel, linux-imx
On Mon, Aug 18, 2025 at 03:44:17PM -0500, Shenwei Wang wrote:
> Remote processors may announce multiple devices (e.g., I2C, GPIO) over
> an RPMSG channel. These devices may require corresponding device tree
> nodes, especially when acting as providers, to supply phandles for their
> consumers.
>
> Define an RPMSG node to work as a container for a group of RPMSG channels
> under the imx_rproc node.
>
> Each subnode within "rpmsg" represents an individual RPMSG channel. The
> name of each subnode corresponds to the channel name as defined by the
> remote processor.
>
> All remote devices associated with a given channel are defined as child
> nodes under the corresponding channel node.
How is each channel addressed? Are they really grouped by type first
(i2c, gpio, etc.) then an address within the group? Or is there some
flat channel numbering? If the latter, then the addresses in the DT
shoulc match the channel number.
>
> Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
> ---
> .../bindings/remoteproc/fsl,imx-rproc.yaml | 117 ++++++++++++++++++
> 1 file changed, 117 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
> index 57d75acb0b5e..a105aac798d0 100644
> --- a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
> +++ b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
> @@ -84,6 +84,86 @@ properties:
> This property is to specify the resource id of the remote processor in SoC
> which supports SCFW
>
> + rpmsg:
> + type: object
> + additionalProperties: false
> + description:
> + Present a group of RPMSG channel devices.
> +
> + properties:
> + '#address-cells':
> + const: 1
> +
> + '#size-cells':
> + const: 0
> +
> + rpmsg-io-channel:
> + type: object
> + unevaluatedProperties: false
> + properties:
> + '#address-cells':
> + const: 1
> +
> + '#size-cells':
> + const: 0
> +
> + patternProperties:
> + "gpio@[0-9a-f]+$":
> + type: object
> + unevaluatedProperties: false
> + properties:
> + compatible:
> + enum:
> + - fsl,imx-rpmsg-gpio
> +
> + reg:
> + maxItems: 1
> +
> + required:
> + - compatible
> + - reg
> +
> + allOf:
> + - $ref: /schemas/gpio/gpio.yaml#
> + - $ref: /schemas/interrupt-controller.yaml#
This is not needed nor sufficient. You need to define the cell sizes for
gpio and interrupt cells.
> +
> + required:
> + - '#address-cells'
> + - '#size-cells'
> +
> + rpmsg-i2c-channel:
> + type: object
> + unevaluatedProperties: false
> + properties:
> + '#address-cells':
> + const: 1
> +
> + '#size-cells':
> + const: 0
> +
> + patternProperties:
> + "i2c@[0-9a-f]+$":
> + type: object
> + unevaluatedProperties: false
> + properties:
> + compatible:
> + enum:
> + - fsl,imx-rpmsg-i2c
> +
> + reg:
> + maxItems: 1
> +
> + required:
> + - compatible
> + - reg
> +
> + allOf:
> + - $ref: /schemas/i2c/i2c-controller.yaml#
> +
> + required:
> + - '#address-cells'
> + - '#size-cells'
> +
> required:
> - compatible
>
> @@ -146,5 +226,42 @@ examples:
> &mu 3 1>;
> memory-region = <&vdev0buffer>, <&vdev0vring0>, <&vdev0vring1>, <&rsc_table>;
> syscon = <&src>;
> +
> + rpmsg {
> + rpmsg-io-channel {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + gpio@0 {
> + compatible = "fsl,imx-rpmsg-gpio";
> + reg = <0>;
> + gpio-controller;
> + #gpio-cells = <2>;
> + #interrupt-cells = <2>;
> + interrupt-controller;
> + interrupt-parent = <&rpmsg_gpioa>;
> + };
> +
> + gpio@1 {
> + compatible = "fsl,imx-rpmsg-gpio";
> + reg = <1>;
> + gpio-controller;
> + #gpio-cells = <2>;
> + #interrupt-cells = <2>;
> + interrupt-controller;
> + interrupt-parent = <&rpmsg_gpiob>;
> + };
> + };
> +
> + rpmsg-i2c-channel {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + i2c@0 {
> + compatible = "fsl,imx-rpmsg-i2c";
> + reg = <0>;
> + };
> + };
> + };
> };
> ...
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/4] dt-bindings: remoteproc: imx_rproc: Add "rpmsg" subnode support
2025-08-26 20:09 ` Rob Herring
@ 2025-08-27 14:49 ` Shenwei Wang
2025-08-29 15:06 ` Rob Herring
0 siblings, 1 reply; 19+ messages in thread
From: Shenwei Wang @ 2025-08-27 14:49 UTC (permalink / raw)
To: Rob Herring
Cc: Bjorn Andersson, Mathieu Poirier, Krzysztof Kozlowski,
Conor Dooley, Shawn Guo, Sascha Hauer, Linus Walleij,
Bartosz Golaszewski, Pengutronix Kernel Team, Fabio Estevam,
Peng Fan, linux-remoteproc@vger.kernel.org,
devicetree@vger.kernel.org, imx@lists.linux.dev,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, dl-linux-imx
> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: Tuesday, August 26, 2025 3:09 PM
> To: Shenwei Wang <shenwei.wang@nxp.com>
> Cc: Bjorn Andersson <andersson@kernel.org>; Mathieu Poirier
> <mathieu.poirier@linaro.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor
> Dooley <conor+dt@kernel.org>; Shawn Guo <shawnguo@kernel.org>; Sascha
> Hauer <s.hauer@pengutronix.de>; Linus Walleij <linus.walleij@linaro.org>;
> Bartosz Golaszewski <brgl@bgdev.pl>; Pengutronix Kernel Team
> <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; Peng Fan
> <peng.fan@nxp.com>; linux-remoteproc@vger.kernel.org;
> devicetree@vger.kernel.org; imx@lists.linux.dev; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; dl-linux-imx <linux-
> imx@nxp.com>
> Subject: [EXT] Re: [PATCH 1/4] dt-bindings: remoteproc: imx_rproc: Add "rpmsg"
> subnode support
> > 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.
>
> How is each channel addressed? Are they really grouped by type first (i2c, gpio,
> etc.) then an address within the group? Or is there some flat channel numbering?
> If the latter, then the addresses in the DT shoulc match the channel number.
>
Yes, the channels are grouped by type and identified by unique channel names assigned
by the remote processor.
The RPMSG bus dynamically assigns addresses to each channel at runtime. Because these
addresses are not static, they cannot be pre-defined in the dts.
Thanks,
Shenwei
> >
> > Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
> > ---
> > .../bindings/remoteproc/fsl,imx-rproc.yaml | 117 ++++++++++++++++++
> > 1 file changed, 117 insertions(+)
> >
.43.0
> >
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/4] dt-bindings: remoteproc: imx_rproc: Add "rpmsg" subnode support
2025-08-27 14:49 ` Shenwei Wang
@ 2025-08-29 15:06 ` Rob Herring
2025-08-30 17:44 ` Shenwei Wang
0 siblings, 1 reply; 19+ messages in thread
From: Rob Herring @ 2025-08-29 15:06 UTC (permalink / raw)
To: Shenwei Wang
Cc: Bjorn Andersson, Mathieu Poirier, Krzysztof Kozlowski,
Conor Dooley, Shawn Guo, Sascha Hauer, Linus Walleij,
Bartosz Golaszewski, Pengutronix Kernel Team, Fabio Estevam,
Peng Fan, linux-remoteproc@vger.kernel.org,
devicetree@vger.kernel.org, imx@lists.linux.dev,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, dl-linux-imx
On Wed, Aug 27, 2025 at 02:49:54PM +0000, Shenwei Wang wrote:
>
>
> > -----Original Message-----
> > From: Rob Herring <robh@kernel.org>
> > Sent: Tuesday, August 26, 2025 3:09 PM
> > To: Shenwei Wang <shenwei.wang@nxp.com>
> > Cc: Bjorn Andersson <andersson@kernel.org>; Mathieu Poirier
> > <mathieu.poirier@linaro.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor
> > Dooley <conor+dt@kernel.org>; Shawn Guo <shawnguo@kernel.org>; Sascha
> > Hauer <s.hauer@pengutronix.de>; Linus Walleij <linus.walleij@linaro.org>;
> > Bartosz Golaszewski <brgl@bgdev.pl>; Pengutronix Kernel Team
> > <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; Peng Fan
> > <peng.fan@nxp.com>; linux-remoteproc@vger.kernel.org;
> > devicetree@vger.kernel.org; imx@lists.linux.dev; linux-arm-
> > kernel@lists.infradead.org; linux-kernel@vger.kernel.org; dl-linux-imx <linux-
> > imx@nxp.com>
> > Subject: [EXT] Re: [PATCH 1/4] dt-bindings: remoteproc: imx_rproc: Add "rpmsg"
> > subnode support
> > > 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.
> >
> > How is each channel addressed? Are they really grouped by type first (i2c, gpio,
> > etc.) then an address within the group? Or is there some flat channel numbering?
> > If the latter, then the addresses in the DT shoulc match the channel number.
> >
>
> Yes, the channels are grouped by type and identified by unique channel names assigned
> by the remote processor.
>
> The RPMSG bus dynamically assigns addresses to each channel at runtime. Because these
> addresses are not static, they cannot be pre-defined in the dts.
But you did define addresses. How do you know which channel 'gpio@1'
corresponds to if RPMSG dynamically assigns addresses?
Rob
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/4] dt-bindings: remoteproc: imx_rproc: Add "rpmsg" subnode support
2025-08-29 15:06 ` Rob Herring
@ 2025-08-30 17:44 ` Shenwei Wang
0 siblings, 0 replies; 19+ messages in thread
From: Shenwei Wang @ 2025-08-30 17:44 UTC (permalink / raw)
To: Rob Herring
Cc: Bjorn Andersson, Mathieu Poirier, Krzysztof Kozlowski,
Conor Dooley, Shawn Guo, Sascha Hauer, Linus Walleij,
Bartosz Golaszewski, Pengutronix Kernel Team, Fabio Estevam,
Peng Fan, linux-remoteproc@vger.kernel.org,
devicetree@vger.kernel.org, imx@lists.linux.dev,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, dl-linux-imx
> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: Friday, August 29, 2025 10:07 AM
> To: Shenwei Wang <shenwei.wang@nxp.com>
> Cc: Bjorn Andersson <andersson@kernel.org>; Mathieu Poirier
> <mathieu.poirier@linaro.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor
> Dooley <conor+dt@kernel.org>; Shawn Guo <shawnguo@kernel.org>; Sascha
> Hauer <s.hauer@pengutronix.de>; Linus Walleij <linus.walleij@linaro.org>;
> Bartosz Golaszewski <brgl@bgdev.pl>; Pengutronix Kernel Team
> <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; Peng Fan
> <peng.fan@nxp.com>; linux-remoteproc@vger.kernel.org;
> devicetree@vger.kernel.org; imx@lists.linux.dev; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; dl-linux-imx <linux-
> imx@nxp.com>
> Subject: [EXT] Re: [PATCH 1/4] dt-bindings: remoteproc: imx_rproc: Add "rpmsg"
> subnode support
> > > >
> > > > All remote devices associated with a given channel are defined as
> > > > child nodes under the corresponding channel node.
> > >
> > > How is each channel addressed? Are they really grouped by type first
> > > (i2c, gpio,
> > > etc.) then an address within the group? Or is there some flat channel
> numbering?
> > > If the latter, then the addresses in the DT shoulc match the channel number.
> > >
> >
> > Yes, the channels are grouped by type and identified by unique channel
> > names assigned by the remote processor.
> >
> > The RPMSG bus dynamically assigns addresses to each channel at
> > runtime. Because these addresses are not static, they cannot be pre-defined in
> the dts.
>
> But you did define addresses. How do you know which channel 'gpio@1'
> corresponds to if RPMSG dynamically assigns addresses?
>
The remote devices within the rpmsg channels use pre-defined addresses specified in
the DTS. For example, the gpio@0 and gpio@1 nodes represent devices inside the
rpmsg-io-channel, each assigned a unique address within that channel.
The rpmsg-io-channel itself does not have a fixed address, as its address is dynamically
allocated by the RPMSG bus. Therefore, we only use the unique channel name here.
rpmsg {
rpmsg-io-channel {
gpio@0 {
compatible = "fsl,imx-rpmsg-gpio";
reg = <0>;
};
gpio@1 {
compatible = "fsl,imx-rpmsg-gpio";
reg = <1>;
};
...
};
...
};
Thanks,
Shenwei
> Rob
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/4] gpio: imx-rpmsg: add imx-rpmsg GPIO driver
2025-08-21 9:01 ` Linus Walleij
@ 2025-09-01 7:26 ` Arnaud POULIQUEN
2025-09-01 17:22 ` Shenwei Wang
0 siblings, 1 reply; 19+ messages in thread
From: Arnaud POULIQUEN @ 2025-09-01 7:26 UTC (permalink / raw)
To: Linus Walleij, Shenwei Wang, Bjorn Andersson, Mathieu Poirier
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer, Bartosz Golaszewski, Pengutronix Kernel Team,
Fabio Estevam, Peng Fan, linux-remoteproc, devicetree, imx,
linux-arm-kernel, linux-kernel, linux-imx
Hello,
On 8/21/25 11:01, Linus Walleij wrote:
> Hi Shenwei,
>
> thanks for your patch!
>
> On Mon, Aug 18, 2025 at 10:45 PM Shenwei Wang <shenwei.wang@nxp.com> wrote:
>
>> On i.MX SoCs, the system may include two processors:
>> - An MCU running an RTOS
>> - An MPU running Linux
>>
>> These processors communicate via the RPMSG protocol.
>> The driver implements the standard GPIO interface, allowing
>> the Linux side to control GPIO controllers which reside in
>> the remote processor via RPMSG protocol.
>>
>> Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
> Since this is a first RPMSG GPIO driver, I'd like if Björn and/or
> Mathieu have a look at it so I'm sure it is RPMSG-proper!
Could this driver be generic (platform independent) ?
Perhaps i missed something, but it seems to me that there is no IMX
specific code.
Making it generic would allow other platforms to reuse it instead of
duplicating it.
Thanks,
Arnaud
>
>> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
>> index a437fe652dbc..2ce4e9b5225e 100644
>> --- a/drivers/gpio/Kconfig
>> +++ b/drivers/gpio/Kconfig
>> @@ -402,6 +402,17 @@ config GPIO_ICH
>>
>> If unsure, say N.
>>
>> +config GPIO_IMX_RPMSG
>> + tristate "NXP i.MX SoC RPMSG GPIO support"
>> + depends on IMX_REMOTEPROC && RPMSG && GPIOLIB
>> + default IMX_REMOTEPROC
>> + help
>> + Say yes here to support the RPMSG GPIO functions on i.MX SoC based
>> + platform. Currently supported devices: i.MX7ULP, i.MX8ULP, i.MX8x,
>> + and i.MX9x.
>> +
>> + If unsure, say N.
> This is sorted under memory-mapped GPIO, but it isn't.
>
> Create a new submenu:
>
> menu "RPMSG GPIO drivers"
> depends on RPMSG
>
> And put it here as the first such driver.
>
> No need to have a dependency on RPMSG in the GPIO_IMX_RPMSG
> Kconfig entry after this.
>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/bitops.h>
> bitops.h or just bits.h? Check which one you actually use.
>
>> +#include <linux/err.h>
>> +#include <linux/gpio/driver.h>
>> +#include <linux/imx_rpmsg.h>
>> +#include <linux/init.h>
>> +#include <linux/irqdomain.h>
>> +#include <linux/of.h>
>> +#include <linux/of_device.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/pm_qos.h>
> Are you really using pm_qos?
>
>> +#include <linux/rpmsg.h>
>> +#include <linux/virtio.h>
>> +#include <linux/workqueue.h>
> (...)
>
>> +struct imx_rpmsg_gpio_port {
>> + struct gpio_chip gc;
>> + struct irq_chip chip;
> This irqchip doesn't look very immutable.
>
> Look at other patches rewriting irqchips to be immutable
> and break this out to a static const struct irq_chip with
> IRQCHIP_IMMUTABLE set instead.
>
>> +static int imx_rpmsg_gpio_get(struct gpio_chip *gc, unsigned int gpio)
>> +{
>> + struct imx_rpmsg_gpio_port *port = gpiochip_get_data(gc);
>> + struct gpio_rpmsg_data *msg = NULL;
>> + int ret;
>> +
>> + mutex_lock(&port->info.lock);
> Please use guards for all the mutexes:
>
> #include <linux/cleanup.h>
>
> guard(mutex)(&port->info.lock);
>
> and it will be released as you exit the function.
>
>> +static int imx_rpmsg_gpio_direction_input(struct gpio_chip *gc,
>> + unsigned int gpio)
>> +{
>> + struct imx_rpmsg_gpio_port *port = gpiochip_get_data(gc);
>> + struct gpio_rpmsg_data *msg = NULL;
>> + int ret;
>> +
>> + mutex_lock(&port->info.lock);
> Dito for all these instances.
> (Saves you a bunch of lines!)
>
>> +static void imx_rpmsg_irq_bus_lock(struct irq_data *d)
>> +{
>> + struct imx_rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
>> +
>> + mutex_lock(&port->info.lock);
>> +}
> Here you need to keep the classic mutex_lock() though,
> because of the irqchip locking abstraction helper.
>
>> +static struct irq_chip imx_rpmsg_irq_chip = {
> const
>
>> + .irq_mask = imx_rpmsg_mask_irq,
>> + .irq_unmask = imx_rpmsg_unmask_irq,
>> + .irq_set_wake = imx_rpmsg_irq_set_wake,
>> + .irq_set_type = imx_rpmsg_irq_set_type,
>> + .irq_shutdown = imx_rpmsg_irq_shutdown,
>> + .irq_bus_lock = imx_rpmsg_irq_bus_lock,
>> + .irq_bus_sync_unlock = imx_rpmsg_irq_bus_sync_unlock,
> .flags = IRQCHIP_IMMUTABLE,
>
> probably also:
>
> GPIOCHIP_IRQ_RESOURCE_HELPERS,
>
> ?
>
> I think you want to properly mark GPIO lines as used for
> IRQs!
>
>> +static int imx_rpmsg_gpio_to_irq(struct gpio_chip *gc, unsigned int gpio)
>> +{
>> + struct imx_rpmsg_gpio_port *port = gpiochip_get_data(gc);
>> + int irq;
>> +
>> + irq = irq_find_mapping(port->domain, gpio);
>> + if (irq > 0) {
>> + irq_set_chip_data(irq, port);
>> + irq_set_chip_and_handler(irq, &port->chip, handle_level_irq);
>> + }
>> +
>> + return irq;
>> +}
> Ugh we try to to use custom to_irq() if we can...
>
> Do you have to?
>
> Can't you use
> select GPIOLIB_IRQCHIP
> and be inspired by other chips using the irqchip
> helper library?
>
> We almost always use that these days.
>
>> + /* create an irq domain */
>> + port->chip = imx_rpmsg_irq_chip;
>> + port->chip.name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s-gpio%d",
>> + pltdata->rproc_name, port->idx);
>> + port->dev = &pdev->dev;
>> +
>> + irq_base = devm_irq_alloc_descs(&pdev->dev, -1, 0, IMX_RPMSG_GPIO_PER_PORT,
>> + numa_node_id());
>> + if (irq_base < 0) {
>> + dev_err(&pdev->dev, "Failed to alloc irq_descs\n");
>> + return irq_base;
>> + }
>> +
>> + port->domain = irq_domain_create_legacy(of_node_to_fwnode(np),
>> + IMX_RPMSG_GPIO_PER_PORT,
>> + irq_base, 0,
>> + &irq_domain_simple_ops, port);
>> + if (!port->domain) {
>> + dev_err(&pdev->dev, "Failed to allocate IRQ domain\n");
>> + return -EINVAL;
>> + }
> This also looks unnecessarily custom.
>
> Try to use GPIOLIB_IRQCHIP.
>
>
>> +static struct platform_driver imx_rpmsg_gpio_driver = {
>> + .driver = {
>> + .name = "gpio-imx-rpmsg",
>> + .of_match_table = imx_rpmsg_gpio_dt_ids,
>> + },
>> + .probe = imx_rpmsg_gpio_probe,
>> +};
>> +
>> +static int __init gpio_imx_rpmsg_init(void)
>> +{
>> + return platform_driver_register(&imx_rpmsg_gpio_driver);
>> +}
>> +
>> +device_initcall(gpio_imx_rpmsg_init);
> No please just do:
>
> module_platform_driver(imx_rpmsg_gpio_driver);
>
> Fix up these things to begin with and then we can
> look at details!
>
> Yours,
> Linus Walleij
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/4] gpio: imx-rpmsg: add imx-rpmsg GPIO driver
2025-09-01 7:26 ` Arnaud POULIQUEN
@ 2025-09-01 17:22 ` Shenwei Wang
2025-09-02 12:34 ` Arnaud POULIQUEN
0 siblings, 1 reply; 19+ messages in thread
From: Shenwei Wang @ 2025-09-01 17:22 UTC (permalink / raw)
To: Arnaud POULIQUEN, Linus Walleij, Bjorn Andersson, Mathieu Poirier
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer, Bartosz Golaszewski, Pengutronix Kernel Team,
Fabio Estevam, Peng Fan, linux-remoteproc@vger.kernel.org,
devicetree@vger.kernel.org, imx@lists.linux.dev,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, dl-linux-imx
> -----Original Message-----
> From: Arnaud POULIQUEN <arnaud.pouliquen@foss.st.com>
> Sent: Monday, September 1, 2025 2:27 AM
> To: Linus Walleij <linus.walleij@linaro.org>; Shenwei Wang
> <shenwei.wang@nxp.com>; Bjorn Andersson <andersson@kernel.org>; Mathieu
> Poirier <mathieu.poirier@linaro.org>
> Cc: Rob Herring <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>;
> Conor Dooley <conor+dt@kernel.org>; Shawn Guo <shawnguo@kernel.org>;
> Sascha Hauer <s.hauer@pengutronix.de>; Bartosz Golaszewski <brgl@bgdev.pl>;
> Pengutronix Kernel Team <kernel@pengutronix.de>; Fabio Estevam
> <festevam@gmail.com>; Peng Fan <peng.fan@nxp.com>; linux-
> remoteproc@vger.kernel.org; devicetree@vger.kernel.org; imx@lists.linux.dev;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; dl-linux-imx
> <linux-imx@nxp.com>
> Subject: [EXT] Re: [PATCH 3/4] gpio: imx-rpmsg: add imx-rpmsg GPIO driver
>
>
> Hello,
>
> On 8/21/25 11:01, Linus Walleij wrote:
> > Hi Shenwei,
> >
> > thanks for your patch!
> >
> > On Mon, Aug 18, 2025 at 10:45 PM Shenwei Wang <shenwei.wang@nxp.com>
> wrote:
> >
> >> On i.MX SoCs, the system may include two processors:
> >> - An MCU running an RTOS
> >> - An MPU running Linux
> >>
> >> These processors communicate via the RPMSG protocol.
> >> The driver implements the standard GPIO interface, allowing the Linux
> >> side to control GPIO controllers which reside in the remote processor
> >> via RPMSG protocol.
> >>
> >> Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
> > Since this is a first RPMSG GPIO driver, I'd like if Björn and/or
> > Mathieu have a look at it so I'm sure it is RPMSG-proper!
>
> Could this driver be generic (platform independent) ?
> Perhaps i missed something, but it seems to me that there is no IMX specific
> code.
> Making it generic would allow other platforms to reuse it instead of duplicating it.
>
The driver uses the RPMSG channel just as a transport layer, so the implementation is actually
platform-independent. However, it requires the remote side to implement the same communication
protocol and behavior for the GPIO controller.
Thanks,
Shenwei
> Thanks,
> Arnaud
>
> >
> >> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index
> >> a437fe652dbc..2ce4e9b5225e 100644
> >> --- a/drivers/gpio/Kconfig
> >> +++ b/drivers/gpio/Kconfig
> >> @@ -402,6 +402,17 @@ config GPIO_ICH
> >>
> >> If unsure, say N.
> >>
> >> +config GPIO_IMX_RPMSG
> >> + tristate "NXP i.MX SoC RPMSG GPIO support"
> >> + depends on IMX_REMOTEPROC && RPMSG && GPIOLIB
> >> + default IMX_REMOTEPROC
> >> + help
> >> + Say yes here to support the RPMSG GPIO functions on i.MX SoC based
> >> + platform. Currently supported devices: i.MX7ULP, i.MX8ULP, i.MX8x,
> >> + and i.MX9x.
> >> +
> >> + If unsure, say N.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/4] gpio: imx-rpmsg: add imx-rpmsg GPIO driver
2025-09-01 17:22 ` Shenwei Wang
@ 2025-09-02 12:34 ` Arnaud POULIQUEN
0 siblings, 0 replies; 19+ messages in thread
From: Arnaud POULIQUEN @ 2025-09-02 12:34 UTC (permalink / raw)
To: Shenwei Wang, Linus Walleij, Bjorn Andersson, Mathieu Poirier
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer, Bartosz Golaszewski, Pengutronix Kernel Team,
Fabio Estevam, Peng Fan, linux-remoteproc@vger.kernel.org,
devicetree@vger.kernel.org, imx@lists.linux.dev,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, dl-linux-imx
On 9/1/25 19:22, Shenwei Wang wrote:
>
>> -----Original Message-----
>> From: Arnaud POULIQUEN <arnaud.pouliquen@foss.st.com>
>> Sent: Monday, September 1, 2025 2:27 AM
>> To: Linus Walleij <linus.walleij@linaro.org>; Shenwei Wang
>> <shenwei.wang@nxp.com>; Bjorn Andersson <andersson@kernel.org>; Mathieu
>> Poirier <mathieu.poirier@linaro.org>
>> Cc: Rob Herring <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>;
>> Conor Dooley <conor+dt@kernel.org>; Shawn Guo <shawnguo@kernel.org>;
>> Sascha Hauer <s.hauer@pengutronix.de>; Bartosz Golaszewski <brgl@bgdev.pl>;
>> Pengutronix Kernel Team <kernel@pengutronix.de>; Fabio Estevam
>> <festevam@gmail.com>; Peng Fan <peng.fan@nxp.com>; linux-
>> remoteproc@vger.kernel.org; devicetree@vger.kernel.org; imx@lists.linux.dev;
>> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; dl-linux-imx
>> <linux-imx@nxp.com>
>> Subject: [EXT] Re: [PATCH 3/4] gpio: imx-rpmsg: add imx-rpmsg GPIO driver
>>
>>
>> Hello,
>>
>> On 8/21/25 11:01, Linus Walleij wrote:
>>> Hi Shenwei,
>>>
>>> thanks for your patch!
>>>
>>> On Mon, Aug 18, 2025 at 10:45 PM Shenwei Wang <shenwei.wang@nxp.com>
>> wrote:
>>>> On i.MX SoCs, the system may include two processors:
>>>> - An MCU running an RTOS
>>>> - An MPU running Linux
>>>>
>>>> These processors communicate via the RPMSG protocol.
>>>> The driver implements the standard GPIO interface, allowing the Linux
>>>> side to control GPIO controllers which reside in the remote processor
>>>> via RPMSG protocol.
>>>>
>>>> Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
>>> Since this is a first RPMSG GPIO driver, I'd like if Björn and/or
>>> Mathieu have a look at it so I'm sure it is RPMSG-proper!
>> Could this driver be generic (platform independent) ?
>> Perhaps i missed something, but it seems to me that there is no IMX specific
>> code.
>> Making it generic would allow other platforms to reuse it instead of duplicating it.
>>
> The driver uses the RPMSG channel just as a transport layer, so the implementation is actually
> platform-independent. However, it requires the remote side to implement the same communication
> protocol and behavior for the GPIO controller.
>
Yes, pending implementation is needed on the remote processor, as is
already the case for the rpmsg_char and
rpmsg_tty Linux drivers. If a generic remote implementation is needed
for this series, then it could be
implemented in the OpenAMP project as done for the rpmsg_tty and
rpmsg_char [1].
The idea here would be to have an equivalent of the virtio-gpio driver
but based on the RPMsg protocol instead of the
virtio one.
[1]
https://github.com/OpenAMP/openamp-system-reference/tree/main/examples/zephyr/rpmsg_multi_services
Thanks,
Arnaud
>
> Thanks,
> Shenwei
>
>> Thanks,
>> Arnaud
>>
>>>> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index
>>>> a437fe652dbc..2ce4e9b5225e 100644
>>>> --- a/drivers/gpio/Kconfig
>>>> +++ b/drivers/gpio/Kconfig
>>>> @@ -402,6 +402,17 @@ config GPIO_ICH
>>>>
>>>> If unsure, say N.
>>>>
>>>> +config GPIO_IMX_RPMSG
>>>> + tristate "NXP i.MX SoC RPMSG GPIO support"
>>>> + depends on IMX_REMOTEPROC && RPMSG && GPIOLIB
>>>> + default IMX_REMOTEPROC
>>>> + help
>>>> + Say yes here to support the RPMSG GPIO functions on i.MX SoC based
>>>> + platform. Currently supported devices: i.MX7ULP, i.MX8ULP, i.MX8x,
>>>> + and i.MX9x.
>>>> +
>>>> + If unsure, say N.
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2025-09-02 12:37 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-18 20:44 [PATCH 0/4] Enable Remote GPIO over RPMSG on i.MX Platform Shenwei Wang
2025-08-18 20:44 ` [PATCH 1/4] dt-bindings: remoteproc: imx_rproc: Add "rpmsg" subnode support Shenwei Wang
2025-08-26 14:50 ` Frank Li
2025-08-26 20:09 ` Rob Herring
2025-08-27 14:49 ` Shenwei Wang
2025-08-29 15:06 ` Rob Herring
2025-08-30 17:44 ` Shenwei Wang
2025-08-18 20:44 ` [PATCH 2/4] remoteproc: imx_rproc: Populate devices under "rpmsg" subnode Shenwei Wang
2025-08-21 11:02 ` Peng Fan
2025-08-21 18:51 ` Shenwei Wang
2025-08-18 20:44 ` [PATCH 3/4] gpio: imx-rpmsg: add imx-rpmsg GPIO driver Shenwei Wang
2025-08-21 9:01 ` Linus Walleij
2025-09-01 7:26 ` Arnaud POULIQUEN
2025-09-01 17:22 ` Shenwei Wang
2025-09-02 12:34 ` Arnaud POULIQUEN
2025-08-21 14:11 ` kernel test robot
2025-08-18 20:44 ` [PATCH 4/4] arm64: dts: imx8ulp: Add rpmsg node under imx_rproc Shenwei Wang
2025-08-20 20:49 ` [PATCH 0/4] Enable Remote GPIO over RPMSG on i.MX Platform Mathieu Poirier
2025-08-20 22:29 ` Shenwei Wang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).