[PATCH v6 4/5] gpio: rpmsg: add generic rpmsg GPIO driver
Bjorn Andersson
andersson at kernel.org
Thu Dec 18 07:58:13 PST 2025
On Fri, Dec 12, 2025 at 01:43:40PM -0600, Shenwei Wang wrote:
> On an AMP platform, the system may include two processors:
We have many examples where there's N systems and it's certainly not
unreasonable to have multiple remote processors expose GPIOs in this
fashion.
> - An MCU running an RTOS
> - An MPU running Linux
>
> These processors communicate via the RPMSG protocol.
> The driver implements the standard GPIO interface, allowing
> the Linux side to control GPIO controllers which reside in
> the remote processor via RPMSG protocol.
>
> Cc: Bartosz Golaszewski <brgl at bgdev.pl>
> Cc: Andrew Lunn <andrew at lunn.ch>
> Signed-off-by: Shenwei Wang <shenwei.wang at nxp.com>
> ---
> drivers/gpio/Kconfig | 16 ++
> drivers/gpio/Makefile | 1 +
> drivers/gpio/gpio-rpmsg.c | 490 ++++++++++++++++++++++++++++++++++++++
> 3 files changed, 507 insertions(+)
> create mode 100644 drivers/gpio/gpio-rpmsg.c
>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index bd185482a7fd..7a72b5dbd4a9 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -1883,6 +1883,22 @@ config GPIO_SODAVILLE
>
> endmenu
>
> +menu "RPMSG GPIO drivers"
> + depends on RPMSG
> +
> +config GPIO_RPMSG
> + tristate "Generic RPMSG GPIO support"
> + select GPIOLIB_IRQCHIP
> + default REMOTEPROC
> + help
> + Say yes here to support the generic GPIO functions over the RPMSG
> + bus. Currently supported devices: i.MX7ULP, i.MX8ULP, i.MX8x,and
> + i.MX9x.
> +
> + If unsure, say N.
> +
> +endmenu
> +
> menu "SPI GPIO expanders"
> depends on SPI_MASTER
>
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index 2421a8fd3733..b1373ec274c8 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -156,6 +156,7 @@ obj-$(CONFIG_GPIO_RDC321X) += gpio-rdc321x.o
> obj-$(CONFIG_GPIO_REALTEK_OTTO) += gpio-realtek-otto.o
> obj-$(CONFIG_GPIO_REG) += gpio-reg.o
> obj-$(CONFIG_GPIO_ROCKCHIP) += gpio-rockchip.o
> +obj-$(CONFIG_GPIO_RPMSG) += gpio-rpmsg.o
> obj-$(CONFIG_GPIO_RTD) += gpio-rtd.o
> obj-$(CONFIG_ARCH_SA1100) += gpio-sa1100.o
> obj-$(CONFIG_GPIO_SAMA5D2_PIOBU) += gpio-sama5d2-piobu.o
> diff --git a/drivers/gpio/gpio-rpmsg.c b/drivers/gpio/gpio-rpmsg.c
> new file mode 100644
> index 000000000000..cf10e2958374
> --- /dev/null
> +++ b/drivers/gpio/gpio-rpmsg.c
> @@ -0,0 +1,490 @@
> +// 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/completion.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/init.h>
> +#include <linux/irqdomain.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/rpmsg.h>
> +#include <linux/rpmsg/rpdev_info.h>
> +
> +#define RPMSG_GPIO_ID 5
> +#define RPMSG_VENDOR 1
> +#define RPMSG_VERSION 0
> +
> +#define GPIOS_PER_PORT 32
> +#define RPMSG_TIMEOUT 1000
> +
> +enum gpio_input_trigger_type {
> + GPIO_RPMSG_TRI_IGNORE,
These aren't enumerations, they are well defined constants of the
protocol. I think #define is better.
> + 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,
> + GPIO_RPMSG_DIRECTION_GET,
> +};
> +
> +struct gpio_rpmsg_head {
> + u8 id; /* Message ID Code */
> + u8 vendor; /* Vendor ID number */
> + u8 version; /* Vendor-specific version number */
> + u8 type; /* Message type */
> + u8 cmd; /* Command code */
> + u8 reserved[5];
> +} __packed;
> +
> +struct gpio_rpmsg_packet {
> + struct gpio_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 gpio_rpmsg_pin {
> + u8 irq_shutdown;
> + u8 irq_unmask;
> + u8 irq_mask;
> + u32 irq_wake_enable;
> + u32 irq_type;
> + struct gpio_rpmsg_packet msg;
> +};
> +
> +struct gpio_rpmsg_info {
> + struct rpmsg_device *rpdev;
> + struct gpio_rpmsg_packet *notify_msg;
> + struct gpio_rpmsg_packet *reply_msg;
> + struct completion cmd_complete;
> + struct mutex lock;
> + void **port_store;
> +};
> +
> +struct rpmsg_gpio_port {
> + struct gpio_chip gc;
> + struct gpio_rpmsg_pin gpio_pins[GPIOS_PER_PORT];
> + struct gpio_rpmsg_info info;
> + int idx;
> +};
> +
> +static int gpio_send_message(struct rpmsg_gpio_port *port,
> + struct gpio_rpmsg_packet *msg,
> + bool sync)
> +{
> + struct gpio_rpmsg_info *info = &port->info;
> + int err;
> +
> + if (!info->rpdev) {
> + pr_err("rpmsg channel doesn't exist, is remote core ready?\n");
How is this possible? You're creating and destroying the platform_device
based on the presence of the rpmsg channel/endpoint, in what case would
you end up here without a valid rpdev?
And if this is to deal with the race during removal, I guess the error
message is wrong and rpdev might go away before you access it below?
> + return -EINVAL;
> + }
> +
> + reinit_completion(&info->cmd_complete);
> + err = rpmsg_send(info->rpdev->ept, (void *)msg,
> + sizeof(struct gpio_rpmsg_packet));
> + if (err) {
> + dev_err(&info->rpdev->dev, "rpmsg_send failed: %d\n", err);
> + return err;
> + }
> +
> + 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");
> + return -ETIMEDOUT;
> + }
> +
> + if (info->reply_msg->out.retcode != 0) {
> + dev_err(&info->rpdev->dev, "remote core replies an error: %d!\n",
> + info->reply_msg->out.retcode);
> + return -EINVAL;
> + }
> +
> + /* copy the reply message */
> + memcpy(&port->gpio_pins[info->reply_msg->pin_idx].msg,
> + info->reply_msg, sizeof(*info->reply_msg));
> + }
> +
> + return 0;
> +}
> +
> +static struct gpio_rpmsg_packet *gpio_setup_msg_header(struct rpmsg_gpio_port *port,
> + unsigned int offset,
> + u8 cmd)
> +{
> + struct gpio_rpmsg_packet *msg = &port->gpio_pins[offset].msg;
> +
> + memset(msg, 0, sizeof(struct gpio_rpmsg_packet));
> + msg->header.id = RPMSG_GPIO_ID;
> + msg->header.vendor = RPMSG_VENDOR;
> + msg->header.version = RPMSG_VERSION;
> + msg->header.type = GPIO_RPMSG_SETUP;
> + msg->header.cmd = cmd;
> + msg->pin_idx = offset;
> + msg->port_idx = port->idx;
> +
> + return msg;
> +};
> +
> +static int rpmsg_gpio_get(struct gpio_chip *gc, unsigned int gpio)
> +{
> + struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
> + struct gpio_rpmsg_packet *msg = NULL;
There's no reason to initialize msg here, the first reference is an
assignment.
> + int ret;
> +
> + guard(mutex)(&port->info.lock);
> +
> + msg = gpio_setup_msg_header(port, gpio, GPIO_RPMSG_INPUT_GET);
> +
> + ret = gpio_send_message(port, msg, true);
> + if (!ret)
> + ret = !!port->gpio_pins[gpio].msg.in.value;
> +
> + return ret;
> +}
> +
> +static int rpmsg_gpio_get_direction(struct gpio_chip *gc, unsigned int gpio)
> +{
> + struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
> + struct gpio_rpmsg_packet *msg = NULL;
> + int ret;
> +
> + guard(mutex)(&port->info.lock);
> +
> + msg = gpio_setup_msg_header(port, gpio, GPIO_RPMSG_DIRECTION_GET);
> +
> + ret = gpio_send_message(port, msg, true);
> + if (!ret)
> + ret = !!port->gpio_pins[gpio].msg.in.value;
> +
> + return ret;
> +}
> +
> +static int rpmsg_gpio_direction_input(struct gpio_chip *gc, unsigned int gpio)
> +{
> + struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
> + struct gpio_rpmsg_packet *msg = NULL;
> +
> + guard(mutex)(&port->info.lock);
> +
> + msg = gpio_setup_msg_header(port, gpio, GPIO_RPMSG_INPUT_INIT);
> +
> + return gpio_send_message(port, msg, true);
> +}
> +
> +static int rpmsg_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
> +{
> + struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
> + struct gpio_rpmsg_packet *msg = NULL;
> +
> + guard(mutex)(&port->info.lock);
> +
> + msg = gpio_setup_msg_header(port, gpio, GPIO_RPMSG_OUTPUT_INIT);
> + msg->out.value = val;
> +
> + return gpio_send_message(port, msg, true);
> +}
> +
> +static int rpmsg_gpio_direction_output(struct gpio_chip *gc,
> + unsigned int gpio,
> + int val)
> +{
> +
> + return rpmsg_gpio_set(gc, gpio, val);
> +}
> +
> +static int gpio_rpmsg_irq_set_type(struct irq_data *d, u32 type)
> +{
> + struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> + u32 gpio_idx = d->hwirq;
> + int edge = 0;
> + int ret = 0;
> +
> + switch (type) {
> + case IRQ_TYPE_EDGE_RISING:
> + edge = GPIO_RPMSG_TRI_RISING;
> + irq_set_handler_locked(d, handle_simple_irq);
> + break;
> + case IRQ_TYPE_EDGE_FALLING:
> + edge = GPIO_RPMSG_TRI_FALLING;
> + irq_set_handler_locked(d, handle_simple_irq);
> + break;
> + case IRQ_TYPE_EDGE_BOTH:
> + edge = GPIO_RPMSG_TRI_BOTH_EDGE;
> + irq_set_handler_locked(d, handle_simple_irq);
> + break;
> + case IRQ_TYPE_LEVEL_LOW:
> + edge = GPIO_RPMSG_TRI_LOW_LEVEL;
> + irq_set_handler_locked(d, handle_level_irq);
> + break;
> + case IRQ_TYPE_LEVEL_HIGH:
> + edge = GPIO_RPMSG_TRI_HIGH_LEVEL;
> + irq_set_handler_locked(d, handle_level_irq);
> + break;
> + default:
> + ret = -EINVAL;
> + irq_set_handler_locked(d, handle_bad_irq);
> + break;
> + }
> +
> + port->gpio_pins[gpio_idx].irq_type = edge;
> +
> + return ret;
> +}
> +
> +static int gpio_rpmsg_irq_set_wake(struct irq_data *d, u32 enable)
> +{
> + struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> + u32 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 gpio_rpmsg_unmask_irq(struct irq_data *d)
> +{
> + struct 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 gpio_rpmsg_mask_irq(struct irq_data *d)
> +{
> + struct 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.
There's nothing in this scheme that dictates that we have A cores and M
cores, or that we have a single core system on both sides, or that they
are Arm cores, please describe things in terms of Linux system and
"remote system".
> + */
> + port->gpio_pins[gpio_idx].irq_mask = 1;
> +}
> +
> +static void gpio_rpmsg_irq_shutdown(struct irq_data *d)
> +{
> + struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> + u32 gpio_idx = d->hwirq;
> +
> + port->gpio_pins[gpio_idx].irq_shutdown = 1;
> +}
> +
> +static void gpio_rpmsg_irq_bus_lock(struct irq_data *d)
> +{
> + struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> +
> + mutex_lock(&port->info.lock);
> +}
> +
> +static void gpio_rpmsg_irq_bus_sync_unlock(struct irq_data *d)
> +{
> + struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> + struct gpio_rpmsg_packet *msg = NULL;
> + u32 gpio_idx = d->hwirq;
> +
> + if (!port)
> + 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_setup_msg_header(port, gpio_idx, GPIO_RPMSG_INPUT_INIT);
> +
> + 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 const struct irq_chip gpio_rpmsg_irq_chip = {
> + .irq_mask = gpio_rpmsg_mask_irq,
> + .irq_unmask = gpio_rpmsg_unmask_irq,
> + .irq_set_wake = gpio_rpmsg_irq_set_wake,
> + .irq_set_type = gpio_rpmsg_irq_set_type,
> + .irq_shutdown = gpio_rpmsg_irq_shutdown,
> + .irq_bus_lock = gpio_rpmsg_irq_bus_lock,
> + .irq_bus_sync_unlock = gpio_rpmsg_irq_bus_sync_unlock,
> + .flags = IRQCHIP_IMMUTABLE,
> +};
> +
> +static int rpmsg_gpio_callback(struct rpmsg_device *rpdev,
> + void *data, int len, void *priv, u32 src)
> +{
> + struct gpio_rpmsg_packet *msg = (struct gpio_rpmsg_packet *)data;
> + struct rpmsg_gpio_port *port = NULL;
> + struct rpdev_platform_info *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;
As soon as you return from this function, the msg buffer is put back
into the virtqueue, so you can't just stash a reference to it here and
hope that it's still available when gpio_send_message() tries to read
it.
> + complete(&port->info.cmd_complete);
> + } else if (msg->header.type == GPIO_RPMSG_NOTIFY) {
> + port->info.notify_msg = msg;
Ditto.
Although notify_msg is assigned, but I never see any further access to
it.
> + generic_handle_domain_irq_safe(port->gc.irq.domain, msg->pin_idx);
> + } else
> + dev_err(&rpdev->dev, "wrong command type!\n");
> +
> + return 0;
> +}
> +
> +static void rpmsg_gpio_remove_action(void *data)
> +{
> + struct rpmsg_gpio_port *port = data;
> +
> + port->info.port_store[port->idx] = NULL;
> +}
> +
> +static int rpmsg_gpio_probe(struct platform_device *pdev)
> +{
> + struct rpdev_platform_info *pltdata = pdev->dev.platform_data;
> + struct rpmsg_gpio_port *port;
> + struct gpio_irq_chip *girq;
> + struct gpio_chip *gc;
> + int ret;
> +
> + if (!pltdata)
> + return -EPROBE_DEFER;
EPROBE_DEFER would imply that if we try again a bit later, platform_data
is suddenly non-NULL, that seems unlikely.
> +
> + port = devm_kzalloc(&pdev->dev, sizeof(*port), GFP_KERNEL);
> + if (!port)
> + return -ENOMEM;
> +
> + ret = device_property_read_u32(&pdev->dev, "reg", &port->idx);
> + if (ret)
> + return ret;
> +
> + if (port->idx > MAX_DEV_PER_CHANNEL)
> + return -EINVAL;
> +
> + ret = devm_mutex_init(&pdev->dev, &port->info.lock);
> + if (ret)
> + return ret;
> +
> + init_completion(&port->info.cmd_complete);
> + port->info.rpdev = pltdata->rpdev;
> + port->info.port_store = pltdata->channel_devices;
> + port->info.port_store[port->idx] = port;
> + if (!pltdata->rx_callback)
> + pltdata->rx_callback = rpmsg_gpio_callback;
What happens if you rmmod your rpmsg gpio driver and then trigger an
interrupt?
> +
> + gc = &port->gc;
> + gc->owner = THIS_MODULE;
> + gc->parent = &pdev->dev;
> + gc->label = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s-gpio%d",
> + pltdata->rproc_name, port->idx);
> + gc->ngpio = GPIOS_PER_PORT;
> + gc->base = -1;
> +
> + gc->direction_input = rpmsg_gpio_direction_input;
> + gc->direction_output = rpmsg_gpio_direction_output;
> + gc->get_direction = rpmsg_gpio_get_direction;
> + gc->get = rpmsg_gpio_get;
> + gc->set = rpmsg_gpio_set;
> +
> + platform_set_drvdata(pdev, port);
> + girq = &gc->irq;
> + gpio_irq_chip_set_chip(girq, &gpio_rpmsg_irq_chip);
> + girq->parent_handler = NULL;
> + girq->num_parents = 0;
> + girq->parents = NULL;
> + girq->chip->name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s-gpio%d",
> + pltdata->rproc_name, port->idx);
> +
> + ret = devm_add_action_or_reset(&pdev->dev, rpmsg_gpio_remove_action, port);
> + if (ret)
> + return ret;
> +
> + return devm_gpiochip_add_data(&pdev->dev, gc, port);
> +}
> +
> +static const struct of_device_id rpmsg_gpio_dt_ids[] = {
> + { .compatible = "rpmsg-gpio" },
> + { /* sentinel */ }
> +};
> +
> +static struct platform_driver rpmsg_gpio_driver = {
It's an "rpmsg gpio driver", but it's a platform_driver...
I don't think this is the correct design, but if it is then this needs
to be well documented.
Same thing as platform_data forms a strong ABI between some other driver
and this platform_driver, this needs to be well documented (but should
be avoided).
Regards,
Bjorn
> + .driver = {
> + .name = "gpio-rpmsg",
> + .of_match_table = rpmsg_gpio_dt_ids,
> + },
> + .probe = rpmsg_gpio_probe,
> +};
> +
> +module_platform_driver(rpmsg_gpio_driver);
> +
> +MODULE_AUTHOR("Shenwei Wang <shenwei.wang at nxp.com>");
> +MODULE_DESCRIPTION("generic rpmsg gpio driver");
> +MODULE_LICENSE("GPL");
> --
> 2.43.0
>
More information about the linux-arm-kernel
mailing list