> -----Original Message-----
> From: Bartosz Golaszewski <[email protected]>
> Sent: Wednesday, February 18, 2026 4:20 AM
> To: Shenwei Wang <[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]>;
> [email protected]; Bartosz Golaszewski <[email protected]>; Andrew
> Lunn <[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]>
> Subject: [EXT] Re: [PATCH v8 3/4] gpio: rpmsg: add generic rpmsg GPIO driver
> > Cc: Bartosz Golaszewski <[email protected]>
> > Cc: Andrew Lunn <[email protected]>
> > Signed-off-by: Shenwei Wang <[email protected]>
> > ---
> > drivers/gpio/Kconfig | 17 ++
> > drivers/gpio/Makefile | 1 +
> > drivers/gpio/gpio-rpmsg.c | 588
> > ++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 606 insertions(+)
> > create mode 100644 drivers/gpio/gpio-rpmsg.c
> >
> > + 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) {
> > + /* Balance the of_node_put() performed by
> > of_find_node_by_name().
> */
> > + of_node_get(np);
> > + np_chan = of_find_node_by_name(np, chan_name);
> > + of_node_put(np);
>
> If you put np here, why even bother with "balancing". If you don't do
> of_node_get() before calling of_find_node_by_name(), you'll be in the same
> place, no?
>
Yeah, the end result is the same. In this case, the comment just needs to say
that
of_find_node_by_name() does an internal of_node_put(), so we don’t need to
call of_node_put() again.
> > + }
> > +
> > + 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);
> > + } 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");
> > +
> > + 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) {
>
> Like mentioned above: this could be:
>
> device_for_each_child_node() {
> fwnode_device_is_available();
> ...
> }
>
Since the suggested change still wouldn’t remove the OF‑specific calls used
later, I’d rather
keep the current approach and just add the OF dependents in Kconfig.
Thanks,
Shenwei
> > + 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;
> > +}
> > +
> > +static void rpmsg_gpio_channel_remove(struct rpmsg_device *rpdev) {
> > + dev_info(&rpdev->dev, "rpmsg gpio channel driver is removed\n");
> > +}
>
> Please drop this, no need to log it,
>
> > +
> > +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" },
> > + { },
> > +};
> > +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,
>
> Can you please do:
>
> .drv = {
> .name = "open-coded-name",
> .of_match_table = ...
> };
>
> ?
>
> Bartosz
>
> > + .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 <[email protected]>");
> > +MODULE_DESCRIPTION("generic rpmsg gpio driver");
> > +MODULE_LICENSE("GPL");
> > --
> > 2.43.0
> >
> >
