On Thu, 12 Feb 2026 22:36:55 +0100, Shenwei Wang <[email protected]> said:
> On an AMP platform, 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.
>
> 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
>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index b45fb799e36c..3179a54f0634 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -1892,6 +1892,23 @@ config GPIO_SODAVILLE
>
>  endmenu
>
> +menu "RPMSG GPIO drivers"
> +     depends on RPMSG
> +
> +config GPIO_RPMSG
> +     tristate "Generic RPMSG GPIO support"
> +     depends on REMOTEPROC
> +     select GPIOLIB_IRQCHIP
> +     default REMOTEPROC

You're using a lot of OF-centric APIs here, don't you need to depend on OF?
Alternatively, it seems that only rpmsg_get_channel_ofnode() really requires
OF-nodes and everything else could just use firmware node API.

> +     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 c05f7d795c43..501aba56ad68 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -158,6 +158,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..163f51fd45b5
> --- /dev/null
> +++ b/drivers/gpio/gpio-rpmsg.c
> @@ -0,0 +1,588 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright 2026 NXP
> + *
> + * The driver exports a standard gpiochip interface to control
> + * the GPIO controllers via RPMSG on a remote processor.
> + */

Add newline here.

[snip]

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

> +     }
> +
> +     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();
        ...
}

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

Reply via email to