Hi Shenwei,
On Mon, Sep 22, 2025 at 03:04:12PM -0500, Shenwei Wang 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.
>+ */
...
>+
>+#include <linux/err.h>
>+#include <linux/gpio/driver.h>
>+#include <linux/rpmsg/imx_rpmsg.h>
>+#include <linux/init.h>
>+#include <linux/irqdomain.h>
>+#include <linux/of.h>
>+#include <linux/platform_device.h>
>+#include <linux/rpmsg.h>
The included headers should be sorted.
>+
>+#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 completion cmd_complete;
>+ struct mutex lock;
>+ void **port_store;
>+};
>+
>+struct imx_rpmsg_gpio_port {
>+ struct gpio_chip gc;
>+ 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");
"m4 image" -> "remote core"
>+ return -EINVAL;
>+ }
>+
>+ 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);
>+ 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, "rpmsg not ack %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_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;
>+
>+ guard(mutex)(&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;
>+
>+ 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;
>+
>+ guard(mutex)(&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;
>+
>+ return gpio_send_message(port, msg, true);
>+}
>+
>+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;
>+
>+ guard(mutex)(&port->info.lock);
>+
>+ msg = gpio_get_pin_msg(port, gpio);
>+ imx_rpmsg_gpio_direction_output_init(gc, gpio, val, msg);
>+
>+ return gpio_send_message(port, msg, true);
>+}
>+
>+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;
>+
>+ guard(mutex)(&port->info.lock);
>+
>+ msg = gpio_get_pin_msg(port, gpio);
>+ imx_rpmsg_gpio_direction_output_init(gc, gpio, val, msg);
>+
>+ return gpio_send_message(port, msg, true);
>+}
>+
>+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 const 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,
>+ .flags = IRQCHIP_IMMUTABLE,
>+};
>+
>+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;
>+ struct imx_rpmsg_gpio_port *port = NULL;
>+ struct imx_rpmsg_driver_data *drvdata;
>+ unsigned long flags;
>+
>+ 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);
Would you explain a bit on why need to disable IRQ on current core.
>+ generic_handle_domain_irq(port->gc.irq.domain, msg->pin_idx);
>+ local_irq_restore(flags);
>+ } else
>+ dev_err(&rpdev->dev, "wrong command type!\n");
>+
>+ return 0;
>+}
>+
>+static void imx_rpmsg_gpio_remove_action(void *data)
>+{
>+ struct imx_rpmsg_gpio_port *port = data;
>+
>+ port->info.port_store[port->idx] = 0;
>+}
>+
>+static int imx_rpmsg_gpio_probe(struct platform_device *pdev)
>+{
>+ struct imx_rpmsg_driver_data *pltdata = pdev->dev.platform_data;
>+ struct device_node *np = pdev->dev.of_node;
>+ struct imx_rpmsg_gpio_port *port;
>+ struct gpio_irq_chip *girq;
>+ struct gpio_chip *gc;
>+ 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);
"device_property_read_u32" should be better.
>+ 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 = 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->owner = THIS_MODULE;
>+ 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->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);
>+ girq = &gc->irq;
>+ gpio_irq_chip_set_chip(girq, &imx_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);
Align pltdata->rproc_name with the upper line '('.
>+
>+ devm_add_action_or_reset(&pdev->dev, imx_rpmsg_gpio_remove_action,
>port);
return value should be checked.
>+
>+ return devm_gpiochip_add_data(&pdev->dev, gc, port);
>+}
>+
Thanks,
Peng
