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

[Andrew Davis]

On 6/29/26 1:26 PM, Shenwei Wang (OSS) wrote:

-----Original Message-----
From: Andrew Davis <a...@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.

Not necessarily, there is a userspace API for interacting with GPIOs.
And there are ways to get/attach GPIO lines to other devices without DT.

Second, by requiring a DT node, we can ensure that only explicitly enabled GPIO 
resources are managed and accessible.

Not sure I follow here, you have a firmware that provides GPIOs to Linux,
Linux should register those with the GPIO framework. Not sure why DT
is required to be involved. Some systems don't do DT, but they have
firmware and GPIOs.

I'm not saying if the system does use DT and has a node specifically
for this firmware/gpio then we shouldn't bind to that and use it,
just questioning making that "required".

Andrew

+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.w...@nxp.com>");
+MODULE_DESCRIPTION("generic rpmsg gpio driver");
+MODULE_LICENSE("GPL");