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

[Andrew Davis <afd@ti.com>]

On 6/29/26 1:26 PM, Shenwei Wang (OSS) wrote:
> 
> 
>> -----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.

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