Re: [PATCH v13 3/4] gpio: rpmsg: add generic rpmsg GPIO driver

["Padhi, Beleswar" <b-padhi@ti.com>]

Hi Mathieu,

On 5/7/2026 10:42 PM, Mathieu Poirier wrote:
> On Tue, May 05, 2026 at 10:46:11AM +0200, Arnaud POULIQUEN wrote:
>> Hi Beleswar
>>
>> On 5/5/26 07:25, Beleswar Prasad Padhi wrote:
>>> Hi Arnaud,
>>>
>>> On 04/05/26 22:34, Arnaud POULIQUEN wrote:
>>>> Hi Beleswar,
>>>>
>>>> On 5/4/26 10:17, Beleswar Prasad Padhi wrote:
>>>>
>>> [...]
>>>
>>>>>> I may have misunderstood your solution. Could you please help me
>>>>>> understand your proposal by explaining how you would handle three
>>>>>> GPIO ports defined in the DT, considering that the endpoint
>>>>>> addresses on the Linux side can be random?
>>>>>> If I assume there is a unique endpoint on the remote side,
>>>>>> I do not understand how you can match, on the firmware side,
>>>>>> the Linux endpoint address to the GPIO port.
>>>>>
>>>>> Sure, let me take an example:
>>>>> Assumptions: 3 GPIO ports in DT, 3 endpoints in Linux (one per port),
>>>>> 1 endpoint in remote (0xd) and 1 rpmsg channel (rpmsg-io)
>>>>>
>>>>>           rpmsg {
>>>>>             rpmsg-io {
>>>>>               #address-cells = <1>;
>>>>>               #size-cells = <0>;
>>>>>
>>>>>               gpio@25 {
>>>>>                 compatible = "rpmsg-gpio";
>>>>>                 reg = <25>;
>>>>>                 gpio-controller;
>>>>>                 #gpio-cells = <2>;
>>>>>                 #interrupt-cells = <2>;
>>>>>                 interrupt-controller;
>>>>>               };
>>>>>
>>>>>               gpio@32 {
>>>>>                 compatible = "rpmsg-gpio";
>>>>>                 reg = <32>;
>>>>>                 gpio-controller;
>>>>>                 #gpio-cells = <2>;
>>>>>                 #interrupt-cells = <2>;
>>>>>                 interrupt-controller;
>>>>>               };
>>>>>
>>>>>               gpio@35 {
>>>>>                 compatible = "rpmsg-gpio";
>>>>>                 reg = <35>;
>>>>>                 gpio-controller;
>>>>>                 #gpio-cells = <2>;
>>>>>                 #interrupt-cells = <2>;
>>>>>                 interrupt-controller;
>>>>>               };
>>>>>             };
>>>>>           };
>>>>>
>>>>> Code Flow:
>>>>> 1. "rpmsg-io" channel is announced from remote firmware with unique dst
>>>>>        ept = 0xd.
>>>>>
>>>>> 2. rpmsg_core.c creates the default dynamic local ept for the channel
>>>>>        ept = 0x405.
>>>>>
>>>>> 3. rpmsg_core.c assigns the allocated addr to rpdev device:
>>>>>        rpdev->src = 0x405 and rpdev->dst = 0xd.
>>>>>
>>>>> 4. rpmsg_gpio_channel_probe() is triggered. For *each* of the GPIO ports
>>>>>        in DT, it will trigger rpmsg_gpiochip_register() which will now:
>>>>>           a. Call port->ept = rpmsg_create_ept(rpdev,
>>>>>                                                                       rpmsg_gpio_channel_callback,
>>>>>                                                                       port,
>>>>>                                                                      {rpdev.id.name,
>>>>>                                                                       RPMSG_ADDR_ANY,
>>>>>                                                                       RPMSG_ADDR_ANY});
>>>>>               Ex- port->ept->addr = 0x408
>>>>>
>>>>>           b. Prepare a 8-byte message having 2 fields:
>>>>>               port->ept->addr (0x408) and port->idx (25)
>>>>>
>>>>>           c. Send this message to remote firmware on default channel ept
>>>>>               (0x405 -> 0xd) by:
>>>>>               rpmsg_send(rpdev->ept, &message, sizeof(message));
>>>>>
>>>>>           d. Remote side receives this message and creates a map of the
>>>>>               linux_ept_addr to gpio_port. (0x408 <-> 25)
>>>>>
>>>>> 5. After this point, any gpio messages sent from Linux from gpio port
>>>>>        endpoints (Ex- 0x408) can be decoded at remote side by looking up
>>>>>        its map (Ex- map[0x408] = 25).
>>>>>
>>>>> 6. Any messages sent from remote to Linux for a particular gpio port can
>>>>>        also be decoded at Linux by simply fetching the priv pointer to get
>>>>>        the per-port device:
>>>>>        struct rpmsg_gpio_port *port = priv;
>>>>>
>>>> Thanks for the details!
>>>>
>>>> To sum up:
>>>> - the default endpoint acts as the GPIO controller (0x405),
>>>> - one extra Linux endpoint is created per port defined in DT.
>>>>
>>>> This should work, but my concerns remain the same:
>>>>
>>>>     1) This implementation forces the remote processor to handle a single
>>>>        endpoint instead of one endpoint per port. This may add complexity to
>>>>        the remote firmware if each port is managed in a separate thread.
>>>
>>> A. Not really, I just chose 1 remote endpoint for this example as you
>>>       suggested to. We can scale it for two-way communication via the
>>>       get_config message like you suggested below.
>>>
>>> B. Isn't it a bad design of the firmware if it is handling 10 gpio ports
>>>       in 10 threads? The logic to handle all the ports is the same, only
>>>       the parameters (e.g. line number, msg) is different.
>>>
>>>>     2) Linux, as a consumer, should not expose its capabilities to the remote
>>>>        side (in your proposal it enumerates the ports defined in the DT).     In my view, the remote processor should expose its capabilities as the
>>>>        provider.
>>>
>>> Agreed on this.
>>>
>>>>   From my perspective, based on your proposal:
>>>>    1) Linux should send a get_config message to the remote proc (0x405 -> 0xD). 2) The remote processor would respond with the list of ports, associated
>>>>       with an remote endpoint addresses.
>>>
>>> Agreed, we can scale it for multiple remote endpoints like this.
>>>
>>>>    3) Linux would parse the response, compare it with the DT, enable the GPIO
>>>>       ports accordingly, creating it local endpoint and associating it with
>>>>       the remote endpoint.
>>>> Using name service to identify the ports should avoid step 1 & 2 ...
>>>
>>> Yes, but won't that make a lot of hard-codings in the driver?
>>>
>>> +static struct rpmsg_device_id rpmsg_gpio_channel_id_table[] = {
>>> +    { .name = "rpmsg-io-25" },
>>> +    { .name = "rpmsg-io-32" },
>>> +    { .name = "rpmsg-io-35" },
>>> +    { },
>>> +};
>>>
>>> What if tomorrow another vendor decides to add more remoteproc
>>> controlled GPIO ports to Linux, they would have to update this struct in
>>> the driver everytime. And the port indexes (25/32/35) could also differ
>>> between vendors. We should make the driver dynamic i.e. vendor
>>> agnostic.
>>>
>>> I think querying the remote firmware at runtime (step 1 & 2 above) is a
>>> common design pattern and makes the driver vendor agnostic. But feel
>>> free to correct me.
>>>
>> You are right. My proposal would require a patch in rpmsg-core. The idea of
>> allowing a postfix in the compatible string has been discussed before, but,
>> if I remember correctly, it was not concluded.
>>
> I also remember discussing this.  I even reviewed one of Arnaud's patch
> and submitted one myself.  This must have been in 2020 and the reason why it
> wasn't merged has escaped my memory.
>   
>> /* rpmsg devices and drivers are matched using the service name */
>> static inline int rpmsg_id_match(const struct rpmsg_device *rpdev,
>> 				  const struct rpmsg_device_id *id)
>> {
>> 	size_t len;
>>
>> +	len = strnlen(id->name, RPMSG_NAME_SIZE);
>> +	if (len && id->name[len - 1] == '*')
>> +		return !strncmp(id->name, rpdev->id.name, len - 1);
>>
>> 	return strncmp(id->name, rpdev->id.name, RPMSG_NAME_SIZE) == 0;
>> }
>>
>> Then, in rpmsg-gpio, and possibly in other drivers such as rpmsg-tty and
>> a future rpmsg-i2c, we could use:
>> static struct rpmsg_device_id rpmsg_gpio_channel_id_table[] = {
>>      { .name = "rpmsg-io" },
>>      { .name = "rpmsg-io-*" },
>>      { },
>> };
> That was my initial approach.  We don't even need an additional "rpmsg-io-*" in
> rpmsg_gpio_channel_id_table[].  All we need is:
>
> /* rpmsg devices and drivers are matched using the service name */
> static inline int rpmsg_id_match(const struct rpmsg_device *rpdev,
>                                   const struct rpmsg_device_id *id)
> {
>   +     size_t len = strnlen(id->name, RPMSG_NAME_SIZE);
>
>   -     return strncmp(id->name, rpdev->id.name, RPMSG_NAME_SIZE) == 0;
>   +     return strncmp(id->name, rpdev->id.name, len) == 0;
> }


This wildcard channel matching is interesting. It would be good to know
the reasons/cons why this patch was not concluded.

>
> And let the rpmsg-virtio-gpio driver parse @rpdev->id.name to match with a
> GPIO controller in the DT.
>
>> If exact name matching is strongly required, then this proposal would not be
>> suitablea.
>>
>> A third option would be a combination of both approaches: instantiate the
>> device using the same name service from the remote side, as done in
>> rpmsg-tty. In that case, a get_config message, or a similar mechanism, would
>> also be needed to retrieve the port information from the remote side.
>>
> I'm not overly fond of a get_config message because it is one more thing we
> have to define and maintain.
>
> Arnaud: is there a get_config message already defined for rpmsg_tty?
>
> Beleswar: Can you provide a link to a virtio device that would use a get_config
> message?


VirtIO typically uses the feature bits for negotiation and discovery.
And such a get_config message would not be needed in VirtIO layer, as
there is no multiplexing. It's a 1:1 mapping of device to driver
instance.

Thanks,
Beleswar

[...]