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
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
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
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 >
© 2016 - 2025 Red Hat, Inc.