-----Original Message-----
From: Arnaud POULIQUEN <arnaud.pouliquen@foss.st.com>
Sent: Thursday, February 19, 2026 3:21 AM
To: Shenwei Wang <shenwei.wang@nxp.com>; Linus Walleij
[off-list ref]; Bartosz Golaszewski [off-list ref]; Jonathan Corbet
[off-list ref]; Rob Herring [off-list ref]; Krzysztof Kozlowski
[off-list ref]; Conor Dooley [off-list ref]; Bjorn Andersson
[off-list ref]; Mathieu Poirier [off-list ref]; Frank Li
[off-list ref]; Sascha Hauer [off-list ref]
Cc: Shuah Khan <skhan@linuxfoundation.org>; linux-gpio@vger.kernel.org; linux-
doc@vger.kernel.org; linux-kernel@vger.kernel.org; Pengutronix Kernel Team
[off-list ref]; Fabio Estevam [off-list ref]; Peng Fan
[off-list ref]; devicetree@vger.kernel.org; linux-
remoteproc@vger.kernel.org; imx@lists.linux.dev; linux-arm-
kernel@lists.infradead.org; dl-linux-imx [off-list ref]; Bartosz
Golaszewski [off-list ref]; Andrew Lunn [off-list ref]
Subject: [EXT] Re: [PATCH v8 3/4] gpio: rpmsg: add generic rpmsg GPIO driver

quoted

+     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);

Is a topology where they is no rproc->dev node but a parent node exist?

If no rproc->dev, it should return NULL in the above check.

quoted

+
+     if (np) {
+             /* Balance the of_node_put() performed by of_find_node_by_name().

*/

quoted

+             of_node_get(np);
+             np_chan = of_find_node_by_name(np, chan_name);
+             of_node_put(np);
+     }
+
+     return np_chan;
+}
+
+static int
+rpmsg_gpio_channel_callback(struct rpmsg_device *rpdev, void *data,
+                         int len, void *priv, u32 src) {
+     struct gpio_rpmsg_packet *msg = data;
+     struct rpmsg_gpio_port *port = NULL;
+     struct rpdev_drvdata *drvdata;
+
+     drvdata = dev_get_drvdata(&rpdev->dev);
+     if (drvdata && msg && msg->port_idx < MAX_PORT_PER_CHANNEL)
+             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);

What happen if the remoteprocessor answer after the completion timeout?
Could it result in desynchronization between the request and the answer?

If the remote processor responds after the timeout, that late reply will be ignored. The current 
transfer should fail with TIMEOUT, and the state won’t be carried over because cmd_complete 
is reinitialized before each new request, so a stale completion won’t desynchronize the next 
transaction. Each command–reply cycle is isolated, so a delayed reply cannot corrupt or mix with 
a subsequent request.

Having a cmd_counter in gpio_rpmsg_head could help to identify current request
and answer

the use of reinit_completion could be also needed

quoted

+     } else if (msg->header.type == GPIO_RPMSG_NOTIFY) {
+             generic_handle_domain_irq_safe(port->gc.irq.domain, msg->pin_idx);
+     } else
+             dev_err(&rpdev->dev, "wrong command type!\n");

Could you print the msg->header.type value to help for debug?

Sure. Will add it in next version.

quoted

+
+     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;
+
+     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;
+     }
+
+     drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
+     if (!drvdata)
+             return -ENOMEM;
+
+     drvdata->rproc_name = rpmsg_get_rproc_node_name(rpdev);
+     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)
+                     dev_err(dev, "Failed to register: %pOF\n", child);
+     }
+
+     return 0;

return ret
or indicate why the return of rpmsg_gpiochip_register is not taken into account

rpmsg_gpiochip_register() failing only affects whether the GPIO instance gets created. The 
rpmsg channel driver itself can still probe successfully and continue to operate for other features. 

quoted

+}
+
+static void rpmsg_gpio_channel_remove(struct rpmsg_device *rpdev) {
+     dev_info(&rpdev->dev, "rpmsg gpio channel driver is removed\n");
+}
+
+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-channel" },

I would remove the "-channel" suffix to have similar naming than "rpmsg-tty" and
"rpmsg-raw"

The channel name comes from the remote firmware, so we can’t freely rename it on the 
Linux side. On i.MX platforms the firmware follows its own naming conventions, and the *-channel 
suffix is part of that scheme.

Thanks,
Shenwei

Regards,
Arnaud

quoted

+     { },
+};
+MODULE_DEVICE_TABLE(rpmsg, rpmsg_gpio_channel_id_table);
+
+static struct rpmsg_driver rpmsg_gpio_channel_client = {
+     .drv.name       = KBUILD_MODNAME,
+     .drv.of_match_table = rpmsg_gpio_dt_ids,
+     .id_table       = rpmsg_gpio_channel_id_table,
+     .probe          = rpmsg_gpio_channel_probe,
+     .callback       = rpmsg_gpio_channel_callback,
+     .remove         = rpmsg_gpio_channel_remove,
+};
+module_rpmsg_driver(rpmsg_gpio_channel_client);
+
+MODULE_AUTHOR("Shenwei Wang [off-list ref]");
+MODULE_DESCRIPTION("generic rpmsg gpio driver");
+MODULE_LICENSE("GPL");