RE: [PATCH v14 4/5] gpio: rpmsg: add generic rpmsg GPIO driver

["Shenwei Wang (OSS)" <shenwei.wang@oss.nxp.com>]

> -----Original Message-----
> From: Andrew Davis <afd@ti.com>
> Sent: Thursday, June 25, 2026 3:32 PM

...
> Subject: Re: [PATCH v14 4/5] gpio: rpmsg: add generic rpmsg GPIO driver
> > +       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.
> 
> The support would depend on if the right firmware is loaded/running on the given
> remote core. Also if you want to make this generic, then any vendor should be
> able to make a firmware that implements this protocol and make use of this
> driver.
> Suggest dropping this NXP specific device list.
> 

Agree.

> > +
> > +       If unsure, say N.
> > +
> > +endmenu
> > +
> >   menu "SPI GPIO expanders"
> >       depends on SPI_MASTER
> >
> > diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile index
> > b267598b517d..ee75c0e65b8b 100644
> > --- a/drivers/gpio/Makefile
> > +++ b/drivers/gpio/Makefile
> > @@ -157,6 +157,7 @@ obj-$(CONFIG_GPIO_RDC321X)                += gpio-

...
> > +
> > +static int rpmsg_gpio_channel_probe(struct rpmsg_device *rpdev) {
> > +     struct device *dev = &rpdev->dev;
> > +     struct device_node *np;
> > +     const char *rproc_name;
> > +     int idx;
> > +
> > +     idx = rpmsg_get_gpio_index(rpdev->id.name, CHAN_NAME_PREFIX);
> > +     if (idx < 0)
> > +             return -EINVAL;
> > +
> > +     if (!dev->of_node) {
> > +             np = rpmsg_get_channel_ofnode(rpdev, GPIO_COMPAT_STR, idx);
> > +             if (!np)
> > +                     return -ENODEV;
> 
> This seems to imply that DT nodes are required. RPMSG is a discoverable bus
> with a nameservice that can bind/probe new devices. While then optionally
> binding to a DT node when available so sub-devices can be described in DT is fine,
> I don't see why it should be required.
> 

First, a GPIO node typically acts as a provider for other devices.
Second, by requiring a DT node, we can ensure that only explicitly enabled GPIO resources are managed and accessible.

> > +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,
> 
> Does this line actually do anything anymore? Maybe it did when this was a
> platform_driver, but this is a rpmsg_driver and will probe though .id_table
> matches.
> 

Yes, it can be removed because the driver will find the dt node on its own.

Thanks,
Shenwei

> Andrew
> 
> > +     },
> > +};
> > +module_rpmsg_driver(rpmsg_gpio_channel_client);
> > +
> > +MODULE_AUTHOR("Shenwei Wang <shenwei.wang@nxp.com>");
> > +MODULE_DESCRIPTION("generic rpmsg gpio driver");
> > +MODULE_LICENSE("GPL");
>