> -----Original Message-----
> From: Arnaud POULIQUEN <[email protected]>
> Sent: Tuesday, March 17, 2026 3:57 AM
> To: Shenwei Wang <[email protected]>; Linus Walleij
> <[email protected]>; Bartosz Golaszewski <[email protected]>; Jonathan Corbet
> <[email protected]>; Rob Herring <[email protected]>; Krzysztof Kozlowski
> <[email protected]>; Conor Dooley <[email protected]>; Bjorn Andersson
> <[email protected]>; Mathieu Poirier <[email protected]>; Frank Li
> <[email protected]>; Sascha Hauer <[email protected]>
> Cc: Shuah Khan <[email protected]>; [email protected]; linux-
> [email protected]; [email protected]; Pengutronix Kernel Team
> <[email protected]>; Fabio Estevam <[email protected]>; Peng Fan
> <[email protected]>; [email protected]; linux-
> [email protected]; [email protected]; linux-arm-
> [email protected]; dl-linux-imx <[email protected]>; Bartosz
> Golaszewski <[email protected]>; Andrew Lunn <[email protected]>
> Subject: [EXT] Re: [PATCH v12 3/5] gpio: rpmsg: add generic rpmsg GPIO driver
> Hello,
>
> On 3/13/26 20:57, Shenwei Wang wrote:
> > On an AMP platform, the system may include two processors:
> > - An MCU running an RTOS
> > - An MPU running Linux
> >
> > +#include <linux/platform_device.h>
> > +#include <linux/remoteproc.h>
> > +#include <linux/rpmsg.h>
> > +#include <linux/virtio_gpio.h>
>
> you still needed to include virtio_gpio.h?
>
That's way to re-use the design of virtio-gpio.
> > +
> > +#define MAX_PORT_PER_CHANNEL 10
> > +#define GPIOS_PER_PORT_DEFAULT 32
> > +#define RPMSG_TIMEOUT 1000
>
> I wonder if the timeout is not too high. but not blocking.
>
> > +
> > +/* GPIO RPMSG Type */
> > +#define GPIO_RPMSG_SEND 0
> > +#define GPIO_RPMSG_REPLY 1
> > +#define GPIO_RPMSG_NOTIFY 2
> > +
> > +struct rpmsg_gpio_packet {
> > + u8 type; /* Message type */
> > + u8 cmd; /* Command code */
> > + u8 port_idx;
> > + u8 line;
> > + u8 val1;
> > + u8 val2;
> > +};
> > +
> > +struct rpmsg_gpio_line {
> > + u8 irq_shutdown;
> > + u8 irq_unmask;
> > + u8 irq_mask;
> > + u32 irq_wake_enable;
> > + u32 irq_type;
> > + struct rpmsg_gpio_packet msg;
> > +};
> > +
> > +struct rpmsg_gpio_info {
> > + struct rpmsg_device *rpdev;
> > + struct rpmsg_gpio_packet *reply_msg;
> > + struct completion cmd_complete;
> > + struct mutex lock;
> > + void **port_store;
> > +};
>
> Except if I missunderstood Mathieu and Bjorn's request:
> "reuse all the design-work done in the gpio-virtio"
> We should find similar structures here to those defined in virtio_gpio.h.
> struct rpmsg_gpio_config {
> __le16 ngpio;
> __u8 padding[2];
> __le32 gpio_names_size;
> };
>
> /* Virtio GPIO Request / Response */
> struct virtio_gpio_request {
> __le16 type;
> __le16 gpio;
> __le32 value;
> };
>
> struct rpmsg_gpio_response {
> __u8 status;
> __u8 value;
> };
>
> struct rpmsg_gpio_response_get_names {
> __u8 status;
> __u8 value[];
> };
>
> /* Virtio GPIO IRQ Request / Response */ struct rpmsg_gpio_irq_request {
> __le16 gpio;
> };
>
> struct rpmsg_gpio_irq_response {
> __u8 status;
> };
>
There’s no need to replicate the virtio‑gpio structs.
virtio‑gpio is much more complex than this rpmsg‑gpio implementation, and
copying its structures does not have benefits here.
Matching the command set and behavior is the way to re-use its design so that
enables the backend on the other side to be reused easily.
> > +
> > +struct rpmsg_gpio_port {
> > + struct gpio_chip gc;
> > + struct rpmsg_gpio_line lines[GPIOS_PER_PORT_DEFAULT];
> > + struct rpmsg_gpio_info info;
> > + u32 ngpios;
> > + u32 idx;
> > +};
> > +
> > +struct rpmsg_gpio_fixed_up {
> > + int (*send_fixed_up)(struct rpmsg_gpio_info *info, struct
> rpmsg_gpio_packet *msg);
> > + struct rpmsg_gpio_packet *(*recv_fixed_up)(struct rpmsg_device
> > +*rpdev, void *data); };
> > +
> > +/*
> > + * @rproc_name: the name of the remote proc.
> > + * @recv_pkt: a pointer to the received packet for protocol fix up.
> > + * @protocol_fixed_up: optional callbacks to handle protocol mismatches.
> > + * @channel_devices: an array of the devices related to the rpdev.
> > + */
> > +struct rpdev_drvdata {
> > + const char *rproc_name;
> > + void *recv_pkt;
> > + struct rpmsg_gpio_fixed_up *protocol_fixed_up;
> > + void *channel_devices[MAX_PORT_PER_CHANNEL];
> > +};
> > +
> > +static int rpmsg_gpio_send_message(struct rpmsg_gpio_port *port,
> > + struct rpmsg_gpio_packet *msg,
> > + bool sync) {
> > + struct rpmsg_gpio_info *info = &port->info;
> > + struct rpdev_drvdata *drvdata;
> > + int ret;
> > +
> > + drvdata = dev_get_drvdata(&info->rpdev->dev);
> > + reinit_completion(&info->cmd_complete);
> > +
> > + if (drvdata->protocol_fixed_up)
> > + ret = drvdata->protocol_fixed_up->send_fixed_up(info,
> > + msg);
>
> Seems not part of a generic implementation
>
Can you explain in what way this is not generic?
I’d like to understand the specific concern.
I remember we had the agreement before this new patch.
------------------------------
https://lore.kernel.org/imx/[email protected]/
> Arnaud, if you agree with the points above, my proposal is the following:
> - Remove the fields you mentioned in the protocol and update the driver
> accordingly so
> that we can establish a true baseline for a generic implementation we all
> agree.
> - Then prepare a separate patch to add support for existing NXP platforms
> by introducing
> the necessary fix‑up functions.
>
> Please let me know if this approach works for you. My goal is to find a
> solution that works for
> everyone — even though I know that pleasing everyone is almost impossible.
This looks reasonable to me, but I am not a maintainer, so I will let
maintainers share their opinions on your proposition.
-------------------------------
> > + else
> > + ret = rpmsg_send(info->rpdev->ept, msg, sizeof(*msg));
> > +
> > + if (ret) {
> > + dev_err(&info->rpdev->dev, "rpmsg_send failed: %d\n", ret);
> > + return ret;
> > + }
> > +
> > + if (sync) {
> > + ret = wait_for_completion_timeout(&info->cmd_complete,
> > +
> > msecs_to_jiffies(RPMSG_TIMEOUT));
> > + if (ret == 0) {
> > + dev_err(&info->rpdev->dev, "rpmsg_send timeout!\n");
> > + return -ETIMEDOUT;
> > + }
> > +
> > + if (info->reply_msg->val1 != 0) {
> > + dev_err(&info->rpdev->dev, "remote core replies an
> > error: %d!\n",
> > + info->reply_msg->val1);
> > + return -EINVAL;
> > + }
> > +
> > + /* copy the reply message */
> > + memcpy(&port->lines[info->reply_msg->line].msg,
> > + info->reply_msg, sizeof(*info->reply_msg));
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static struct rpmsg_gpio_packet *
> > +rpmsg_gpio_msg_init_common(struct rpmsg_gpio_port *port, unsigned int
> > +line, u8 cmd) {
> > + struct rpmsg_gpio_packet *msg = &port->lines[line].msg;
> > +
> > + memset(msg, 0, sizeof(struct rpmsg_gpio_packet));
> > + msg->type = GPIO_RPMSG_SEND;
> > + msg->cmd = cmd;
> > + msg->port_idx = port->idx;
> > + msg->line = line;
> > +
> > + return msg;
> > +}
> > +
> > +static int rpmsg_gpio_get(struct gpio_chip *gc, unsigned int line) {
> > + struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
> > + struct rpmsg_gpio_packet *msg;
> > + int ret;
> > +
> > + guard(mutex)(&port->info.lock);
> > +
> > + msg = rpmsg_gpio_msg_init_common(port, line,
> > + VIRTIO_GPIO_MSG_GET_VALUE);
> > +
> > + ret = rpmsg_gpio_send_message(port, msg, true);
> > + if (!ret)
> > + ret = !!port->lines[line].msg.val2;
> > +
> > + return ret;
> > +}
> > +
> > +static int rpmsg_gpio_get_direction(struct gpio_chip *gc, unsigned
> > +int line) {
> > + struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
> > + struct rpmsg_gpio_packet *msg;
> > + int ret;
> > +
> > + guard(mutex)(&port->info.lock);
> > +
> > + msg = rpmsg_gpio_msg_init_common(port, line,
> > + VIRTIO_GPIO_MSG_GET_DIRECTION);
> > +
> > + ret = rpmsg_gpio_send_message(port, msg, true);
> > + if (ret)
> > + return ret;
> > +
> > + switch (port->lines[line].msg.val2) {
> > + case VIRTIO_GPIO_DIRECTION_IN:
> > + return GPIO_LINE_DIRECTION_IN;
> > + case VIRTIO_GPIO_DIRECTION_OUT:
> > + return GPIO_LINE_DIRECTION_OUT;
> > + default:
> > + break;
> > + }
> > +
> > + return -EINVAL;
> > +}
> > +
> > +static int rpmsg_gpio_direction_input(struct gpio_chip *gc, unsigned
> > +int line) {
> > + struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
> > + struct rpmsg_gpio_packet *msg;
> > +
> > + guard(mutex)(&port->info.lock);
> > +
> > + msg = rpmsg_gpio_msg_init_common(port, line,
> VIRTIO_GPIO_MSG_SET_DIRECTION);
> > + msg->val1 = VIRTIO_GPIO_DIRECTION_IN;
> > +
> > + return rpmsg_gpio_send_message(port, msg, true); }
> > +
> > +static int rpmsg_gpio_set(struct gpio_chip *gc, unsigned int line,
> > +int val) {
> > + struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
> > + struct rpmsg_gpio_packet *msg;
> > +
> > + guard(mutex)(&port->info.lock);
> > +
> > + msg = rpmsg_gpio_msg_init_common(port, line,
> VIRTIO_GPIO_MSG_SET_VALUE);
> > + msg->val1 = val;
> > +
> > + return rpmsg_gpio_send_message(port, msg, true); }
> > +
> > +static int rpmsg_gpio_direction_output(struct gpio_chip *gc, unsigned
> > +int line, int val) {
> > + struct rpmsg_gpio_port *port = gpiochip_get_data(gc);
> > + struct rpmsg_gpio_packet *msg;
> > + int ret;
> > +
> > + guard(mutex)(&port->info.lock);
> > +
> > + msg = rpmsg_gpio_msg_init_common(port, line,
> VIRTIO_GPIO_MSG_SET_DIRECTION);
> > + msg->val1 = VIRTIO_GPIO_DIRECTION_OUT;
> > +
> > + ret = rpmsg_gpio_send_message(port, msg, true);
> > + if (ret)
> > + return ret;
> > +
> > + msg = rpmsg_gpio_msg_init_common(port, line,
> VIRTIO_GPIO_MSG_SET_VALUE);
> > + msg->val1 = val;
> > +
> > + return rpmsg_gpio_send_message(port, msg, true); }
> > +
> > +static int gpio_rpmsg_irq_set_type(struct irq_data *d, u32 type) {
> > + struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> > + u32 line = d->hwirq;
> > + int ret = 0;
> > +
> > + switch (type) {
> > + case IRQ_TYPE_EDGE_RISING:
> > + type = VIRTIO_GPIO_IRQ_TYPE_EDGE_RISING;
> > + irq_set_handler_locked(d, handle_simple_irq);
> > + break;
> > + case IRQ_TYPE_EDGE_FALLING:
> > + type = VIRTIO_GPIO_IRQ_TYPE_EDGE_FALLING;
> > + irq_set_handler_locked(d, handle_simple_irq);
> > + break;
> > + case IRQ_TYPE_EDGE_BOTH:
> > + type = VIRTIO_GPIO_IRQ_TYPE_EDGE_BOTH;
> > + irq_set_handler_locked(d, handle_simple_irq);
> > + break;
> > + case IRQ_TYPE_LEVEL_LOW:
> > + type = VIRTIO_GPIO_IRQ_TYPE_LEVEL_LOW;
> > + irq_set_handler_locked(d, handle_level_irq);
> > + break;
> > + case IRQ_TYPE_LEVEL_HIGH:
> > + type = VIRTIO_GPIO_IRQ_TYPE_LEVEL_HIGH;
> > + irq_set_handler_locked(d, handle_level_irq);
> > + break;
> > + default:
> > + ret = -EINVAL;
> > + irq_set_handler_locked(d, handle_bad_irq);
> > + break;
> > + }
> > +
> > + port->lines[line].irq_type = type;
> > +
> > + 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 line = d->hwirq;
> > +
> > + port->lines[line].irq_wake_enable = enable;
> > +
> > + return 0;
> > +}
> > +
> > +/*
> > + * This unmask/mask function is invoked in two situations:
> > + * - when an interrupt is being set up, and
> > + * - after an interrupt has occurred.
> > + *
> > + * The GPIO driver does not access hardware registers directly.
> > + * Instead, it caches all relevant information locally, and then
> > +sends
> > + * the accumulated state to the remote system at this stage.
> > + */
> > +static void gpio_rpmsg_unmask_irq(struct irq_data *d) {
> > + struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> > + u32 line = d->hwirq;
> > +
> > + port->lines[line].irq_unmask = 1; }
> > +
> > +static void gpio_rpmsg_mask_irq(struct irq_data *d) {
> > + struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> > + u32 line = d->hwirq;
> > +
> > + /*
> > + * When an interrupt occurs, the remote system masks the interrupt
> > + * and then sends a notification to Linux. After Linux processes
> > + * that notification, it sends an RPMsg command back to the remote
> > + * system to unmask the interrupt again.
> > + */
> > + port->lines[line].irq_mask = 1;
> > +}
> > +
> > +static void gpio_rpmsg_irq_shutdown(struct irq_data *d) {
> > + struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> > + u32 line = d->hwirq;
> > +
> > + port->lines[line].irq_shutdown = 1; }
> > +
> > +static void gpio_rpmsg_irq_bus_lock(struct irq_data *d) {
> > + struct rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> > +
> > + mutex_lock(&port->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 rpmsg_gpio_packet *msg;
> > + u32 line = d->hwirq;
> > +
> > + /*
> > + * For mask irq, do nothing here.
> > + * The remote system will mask interrupt after an interrupt occurs,
> > + * and then send a notification to Linux system. After Linux system
> > + * handles the notification, it sends an rpmsg back to the remote
> > + * system to unmask this interrupt again.
> > + */
> > + if (port->lines[line].irq_mask && !port->lines[line].irq_unmask) {
> > + port->lines[line].irq_mask = 0;
> > + mutex_unlock(&port->info.lock);
> > + return;
> > + }
> > +
> > + msg = rpmsg_gpio_msg_init_common(port, line,
> > + VIRTIO_GPIO_MSG_IRQ_TYPE);
> > +
> > + if (port->lines[line].irq_shutdown) {
> > + port->lines[line].irq_shutdown = 0;
> > + msg->val1 = VIRTIO_GPIO_IRQ_TYPE_NONE;
> > + msg->val2 = 0;
> > + } else {
> > + /* if irq type is not set, use low level trigger as default.
> > */
> > + msg->val1 = port->lines[line].irq_type;
> > + if (!msg->val1)
> > + msg->val1 = VIRTIO_GPIO_IRQ_TYPE_LEVEL_LOW;
> > + if (port->lines[line].irq_unmask) {
> > + msg->val2 = 0;
> > + port->lines[line].irq_unmask = 0;
> > + } else /* irq set wake */
> > + msg->val2 = port->lines[line].irq_wake_enable;
> > + }
> > +
> > + rpmsg_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 void rpmsg_gpio_remove_action(void *data) {
> > + struct rpmsg_gpio_port *port = data;
> > +
> > + port->info.port_store[port->idx] = NULL; }
> > +
> > +static int rpmsg_gpiochip_register(struct rpmsg_device *rpdev, struct
> > +device_node *np) {
> > + struct rpdev_drvdata *drvdata = dev_get_drvdata(&rpdev->dev);
> > + struct rpmsg_gpio_port *port;
> > + struct gpio_irq_chip *girq;
> > + struct gpio_chip *gc;
> > + int ret;
> > +
> > + port = devm_kzalloc(&rpdev->dev, sizeof(*port), GFP_KERNEL);
> > + if (!port)
> > + return -ENOMEM;
> > +
> > + ret = of_property_read_u32(np, "reg", &port->idx);
> > + if (ret)
> > + return ret;
> > +
> > + if (port->idx >= MAX_PORT_PER_CHANNEL)
> > + return -EINVAL;
> > +
> > + ret = devm_mutex_init(&rpdev->dev, &port->info.lock);
> > + if (ret)
> > + return ret;
> > +
> > + ret = of_property_read_u32(np, "ngpios", &port->ngpios);
>
> The number of GPIOs should be obtained from the remote side, as done in
> virtio_gpio. In virtio_gpio, this is retrieved via a get_config operation.
> Here, you
> could implement a specific RPMsg to retrieve the remote topology.
>
> > + if (ret || port->ngpios > GPIOS_PER_PORT_DEFAULT)
> > + port->ngpios = GPIOS_PER_PORT_DEFAULT;
> > +
> > + port->info.reply_msg = devm_kzalloc(&rpdev->dev,
> > + sizeof(struct rpmsg_gpio_packet),
> > + GFP_KERNEL);
> > + if (!port->info.reply_msg)
> > + return -ENOMEM;
> > +
> > + init_completion(&port->info.cmd_complete);
> > + port->info.port_store = drvdata->channel_devices;
> > + port->info.port_store[port->idx] = port;
> > + port->info.rpdev = rpdev;
> > +
> > + gc = &port->gc;
> > + gc->owner = THIS_MODULE;
> > + gc->parent = &rpdev->dev;
> > + gc->fwnode = of_fwnode_handle(np);
> > + gc->ngpio = port->ngpios;
> > + gc->base = -1;
> > + gc->label = devm_kasprintf(&rpdev->dev, GFP_KERNEL, "%s-gpio%d",
> > + drvdata->rproc_name, port->idx);
> > +
> > + gc->direction_input = rpmsg_gpio_direction_input;
> > + gc->direction_output = rpmsg_gpio_direction_output;
> > + gc->get_direction = rpmsg_gpio_get_direction;
> > + gc->get = rpmsg_gpio_get;
> > + gc->set = rpmsg_gpio_set;
> > +
> > + girq = &gc->irq;
> > + gpio_irq_chip_set_chip(girq, &gpio_rpmsg_irq_chip);
> > + girq->parent_handler = NULL;
> > + girq->num_parents = 0;
> > + girq->parents = NULL;
> > + girq->chip->name = devm_kasprintf(&rpdev->dev, GFP_KERNEL, "%s-
> gpio%d",
> > + drvdata->rproc_name,
> > + port->idx);
> > +
> > + ret = devm_add_action_or_reset(&rpdev->dev,
> rpmsg_gpio_remove_action, port);
> > + if (ret)
> > + return ret;
> > +
> > + return devm_gpiochip_add_data(&rpdev->dev, gc, port); }
> > +
> > +static const char *rpmsg_get_rproc_node_name(struct rpmsg_device
> > +*rpdev) {
> > + const char *name = NULL;
> > + struct device_node *np;
> > + struct rproc *rproc;
> > +
> > + rproc = rproc_get_by_child(&rpdev->dev);
> > + if (!rproc)
> > + return NULL;
> > +
> > + np = of_node_get(rproc->dev.of_node);
> > + if (!np && rproc->dev.parent)
> > + np = of_node_get(rproc->dev.parent->of_node);
> > +
> > + if (np) {
> > + name = devm_kstrdup(&rpdev->dev, np->name, GFP_KERNEL);
> > + of_node_put(np);
> > + }
> > +
> > + return name;
> > +}
> > +
> > +static struct device_node *
> > +rpmsg_get_channel_ofnode(struct rpmsg_device *rpdev, char *chan_name)
> > +{
> > + struct device_node *np_chan = NULL, *np;
> > + struct rproc *rproc;
> > +
> > + rproc = rproc_get_by_child(&rpdev->dev);
> > + if (!rproc)
> > + return NULL;
> > +
> > + np = of_node_get(rproc->dev.of_node);
> > + if (!np && rproc->dev.parent)
> > + np = of_node_get(rproc->dev.parent->of_node);
> > +
> > + /* The of_node_put() is performed by of_find_node_by_name(). */
> > + if (np)
> > + np_chan = of_find_node_by_name(np, chan_name);
> > +
> > + return np_chan;
> > +}
> > +
> > +static int rpmsg_gpio_channel_callback(struct rpmsg_device *rpdev, void
> *data,
> > + int len, void *priv, u32 src) {
> > + struct rpmsg_gpio_packet *msg = data;
> > + struct rpmsg_gpio_port *port = NULL;
> > + struct rpdev_drvdata *drvdata;
> > +
> > + drvdata = dev_get_drvdata(&rpdev->dev);
> > + if (drvdata && drvdata->protocol_fixed_up)
> > + msg = drvdata->protocol_fixed_up->recv_fixed_up(rpdev,
> > + data);
> > +
> > + if (!msg || !drvdata)
> > + return -EINVAL;
> > +
> > + if (msg->port_idx < MAX_PORT_PER_CHANNEL)
> > + port = drvdata->channel_devices[msg->port_idx];
> > +
> > + if (!port || msg->line >= port->ngpios) {
> > + dev_err(&rpdev->dev, "wrong port index or line number. port:%d
> line:%d\n",
> > + msg->port_idx, msg->line);
> > + return -EINVAL;
> > + }
> > +
> > + if (msg->type == GPIO_RPMSG_REPLY) {
> > + *port->info.reply_msg = *msg;
> > + complete(&port->info.cmd_complete);
> > + } else if (msg->type == GPIO_RPMSG_NOTIFY) {
> > + generic_handle_domain_irq_safe(port->gc.irq.domain,
> > msg->line);
> > + } else {
> > + dev_err(&rpdev->dev, "wrong command type (0x%x)\n",
> > msg->type);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int rpmsg_gpio_channel_probe(struct rpmsg_device *rpdev) {
> > + struct device *dev = &rpdev->dev;
> > + struct rpdev_drvdata *drvdata;
> > + struct device_node *np;
> > + int ret = -ENODEV;
> > +
> > + if (!dev->of_node) {
> > + np = rpmsg_get_channel_ofnode(rpdev, rpdev->id.name);
> > + if (np) {
> > + dev->of_node = np;
> > + set_primary_fwnode(dev, of_fwnode_handle(np));
> > + }
> > + return -EPROBE_DEFER;
>
> Here is it a bug ? else you should explain in a comment why you perform some
> actions when np != 0 but return -EPROBE_DEFER
>
-EPROBE_DEFER is to let the system to probe the driver again.
Shenwei
> Regards,
> Arnaud
>
> > + }
> > +
> > + drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
> > + if (!drvdata)
> > + return -ENOMEM;
> > +
> > + drvdata->rproc_name = rpmsg_get_rproc_node_name(rpdev);
> > + drvdata->protocol_fixed_up = (struct rpmsg_gpio_fixed_up *)rpdev-
> >id.driver_data;
> > + dev_set_drvdata(dev, drvdata);
> > +
> > + for_each_child_of_node_scoped(dev->of_node, child) {
> > + if (!of_device_is_available(child))
> > + continue;
> > +
> > + if (!of_match_node(dev->driver->of_match_table, child))
> > + continue;
> > +
> > + ret = rpmsg_gpiochip_register(rpdev, child);
> > + if (ret < 0)
> > + break;
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static const struct of_device_id rpmsg_gpio_dt_ids[] = {
> > + { .compatible = "rpmsg-gpio" },
> > + { /* sentinel */ }
> > +};
> > +
> > +static struct rpmsg_device_id rpmsg_gpio_channel_id_table[] = {
> > + { .name = "rpmsg-io" },
> > + { },
> > +};
> > +MODULE_DEVICE_TABLE(rpmsg, rpmsg_gpio_channel_id_table);
> > +
> > +static struct rpmsg_driver rpmsg_gpio_channel_client = {
> > + .callback = rpmsg_gpio_channel_callback,
> > + .id_table = rpmsg_gpio_channel_id_table,
> > + .probe = rpmsg_gpio_channel_probe,
> > + .drv = {
> > + .name = KBUILD_MODNAME,
> > + .of_match_table = rpmsg_gpio_dt_ids,
> > + },
> > +};
> > +module_rpmsg_driver(rpmsg_gpio_channel_client);
> > +
> > +MODULE_AUTHOR("Shenwei Wang <[email protected]>");
> > +MODULE_DESCRIPTION("generic rpmsg gpio driver");
> > +MODULE_LICENSE("GPL");
