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

[Linus Walleij]

On Mon, Feb 23, 2026 at 9:33 PM Shenwei Wang  wrote:

> Arnaud, if you agree with the points above, my proposal is the following:
>  - Remove the fields you mentioned in the protocol and update the driver 
> accordingly so
> that we can establish a true baseline for a generic implementation we all 
> agree.
>  - Then prepare a separate patch to add support for existing NXP platforms by 
> introducing
> the necessary fix‑up functions.
>
> Please let me know if this approach works for you. My goal is to find a 
> solution that works for
> everyone — even though I know that pleasing everyone is almost impossible.

This looks good to me, and Arnaud seems happy too so let's go ahead
with this.

Yours,
Linus Walleij

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

[Arnaud POULIQUEN]

Hello Shenwei

On 2/23/26 21:33, Shenwei Wang wrote:




-Original Message-
From: Arnaud POULIQUEN 
Sent: Monday, February 23, 2026 8:25 AM
To: Linus Walleij ; Shenwei Wang

Cc: Andrew Lunn ; Bartosz Golaszewski ;
Jonathan Corbet ; Rob Herring ; Krzysztof
Kozlowski ; Conor Dooley ; Bjorn
Andersson ; Mathieu Poirier
; Frank Li ; Sascha Hauer
; Shuah Khan ; linux-
g...@vger.kernel.org; linux-doc@vger.kernel.org; linux-ker...@vger.kernel.org;
Pengutronix Kernel Team ; Fabio Estevam
; Peng Fan ;
devicet...@vger.kernel.org; linux-remotep...@vger.kernel.org;
i...@lists.linux.dev; linux-arm-ker...@lists.infradead.org; dl-linux-imx ; Bartosz Golaszewski 
Subject: [EXT] Re: [PATCH v8 3/4] gpio: rpmsg: add generic rpmsg GPIO driver
Hello Linus,

On 2/22/26 15:48, Linus Walleij wrote:

On Fri, Feb 20, 2026 at 7:57 PM Shenwei Wang 

wrote:



Given that, I’d like to hear from the GPIO subsystem maintainers —
@Linus Walleij and @Bartosz Golaszewski — on whether a driver that
works with the current hardware/firmware design could still be
acceptable for upstream inclusion. My understanding is that upstream

generally supports existing, real-world hardware as long as the driver meets
subsystem standards.


What a swell party this has become.

In this kind of situations I usually refer to
Documentation/process/management-style.rst



Thank you for pointing out the document, I was not aware of its existence. Very
informative!
That help me to understand you proposal below.



What is the message I as a maintainer is getting from NXP regarding
"gpio: rpmsg: add generic rpmsg GPIO driver"?

Arnaud, who is the only person in this discussion who actually wrote a
standard RPMSG driver (drivers/tty/rpmsg_tty.c), must ACK this patch
if it wants to call itself a "generic" RPMSG GPIO driver, if he does
not, then it isn't.


In Fact there are already 2 "generic" drivers, the second one it the
drivers/rpmsg/rpmsg_char.c, both are used on several platforms.

It is in my plan to test the rpmsg-gpio on ST platform if we go with the generic
approach.



Is it generic? If it is not, let's call it "NXP rpmsg GPIO driver" and
rename files etc accordingly. Maybe it can share code with the actual
generic RPMSG driver once that arrives, that is more of a library question.


I would like to (re)express my concerns regarding the creation of an 
NXP-specific
driver. To clarify my concerns, ST, like probably some other SoC vendors, has
rpmsg-gpio and rpmsg-i2c drivers in downstream with plans to upstream them.



Linus, thank you for jumping in and providing the guidance.

I would like to clarify one point here: what we are discussing now is not 
whether the
driver itself is generic, but rather that the current protocol is not as 
perfect as Arnaud
would expect, since it contains a few fields that may not be required on their 
platforms.

Arnaud, if you agree with the points above, my proposal is the following:
  - Remove the fields you mentioned in the protocol and update the driver 
accordingly so
that we can establish a true baseline for a generic implementation we all agree.
  - Then prepare a separate patch to add support for existing NXP platforms by 
introducing
the necessary fix‑up functions.

Please let me know if this approach works for you. My goal is to find a 
solution that works for
everyone — even though I know that pleasing everyone is almost impossible.
This looks reasonable to me, but I am not a maintainer, so I will let 
maintainers share their opinions on your proposition.


Please note that you have also to answer to Bjorn and Mathieu about the 
rational to use RPMsg instead of the virtio protocol.


For the ST platform, the main advantage of RPMsg is the ability to mix 
buses on one virtio interface, whereas Virtio requires allocating vrings 
and mailbox channels per Virtio type. When data rate is not the 
priority, RPMsg may be preferable.


Another limitation e observed, when prototyping a virtio-i2c between 
Linux and a remote processor is the allocation of specific DMA memory 
region shared between the processors[1].


[1] 
https://github.com/arnopo/linux/commit/ae47a1cbf95125ab10952283622653d626e56e6a


Thanks and regards,
Arnaud



Thanks,
Shenwei


If we proceed in this direction:

-Any vendor wishing to upstream an rpmsg-gpio driver might submit their own
platform-specific version.

- If NXP upstreams other rpmsg drivers, these will likely remain NXP-centric to
maintain compatibility with their legacy firmware and the nxp-rpmsg-gpio driver,
leading to platform-specific versions in several frameworks.

- The implementation will impact not only the Linux side but also the remote 
side.
Indeed, some operating systems like Zephyr or NuttX implement the rpmsg device
side (Zephyr already implements the rpmsg-tty)

Maintaining a generic approach for RPMsg, similar to what is done for Virtio,
seems to me a more reliable solution, even though it may induce 

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

[Shenwei Wang]

> -Original Message-
> From: Arnaud POULIQUEN 
> Sent: Monday, February 23, 2026 8:25 AM
> To: Linus Walleij ; Shenwei Wang
> 
> Cc: Andrew Lunn ; Bartosz Golaszewski ;
> Jonathan Corbet ; Rob Herring ; Krzysztof
> Kozlowski ; Conor Dooley ; Bjorn
> Andersson ; Mathieu Poirier
> ; Frank Li ; Sascha Hauer
> ; Shuah Khan ; linux-
> g...@vger.kernel.org; linux-doc@vger.kernel.org; linux-ker...@vger.kernel.org;
> Pengutronix Kernel Team ; Fabio Estevam
> ; Peng Fan ;
> devicet...@vger.kernel.org; linux-remotep...@vger.kernel.org;
> i...@lists.linux.dev; linux-arm-ker...@lists.infradead.org; dl-linux-imx 
>  i...@nxp.com>; Bartosz Golaszewski 
> Subject: [EXT] Re: [PATCH v8 3/4] gpio: rpmsg: add generic rpmsg GPIO driver
> Hello Linus,
> 
> On 2/22/26 15:48, Linus Walleij wrote:
> > On Fri, Feb 20, 2026 at 7:57 PM Shenwei Wang 
> wrote:
> >
> >> Given that, I’d like to hear from the GPIO subsystem maintainers —
> >> @Linus Walleij and @Bartosz Golaszewski — on whether a driver that
> >> works with the current hardware/firmware design could still be
> >> acceptable for upstream inclusion. My understanding is that upstream
> generally supports existing, real-world hardware as long as the driver meets
> subsystem standards.
> >
> > What a swell party this has become.
> >
> > In this kind of situations I usually refer to
> > Documentation/process/management-style.rst
> >
> 
> Thank you for pointing out the document, I was not aware of its existence. 
> Very
> informative!
> That help me to understand you proposal below.
> 
> 
> > What is the message I as a maintainer is getting from NXP regarding
> > "gpio: rpmsg: add generic rpmsg GPIO driver"?
> >
> > Arnaud, who is the only person in this discussion who actually wrote a
> > standard RPMSG driver (drivers/tty/rpmsg_tty.c), must ACK this patch
> > if it wants to call itself a "generic" RPMSG GPIO driver, if he does
> > not, then it isn't.
> 
> In Fact there are already 2 "generic" drivers, the second one it the
> drivers/rpmsg/rpmsg_char.c, both are used on several platforms.
> 
> It is in my plan to test the rpmsg-gpio on ST platform if we go with the 
> generic
> approach.
> 
> >
> > Is it generic? If it is not, let's call it "NXP rpmsg GPIO driver" and
> > rename files etc accordingly. Maybe it can share code with the actual
> > generic RPMSG driver once that arrives, that is more of a library question.
> 
> I would like to (re)express my concerns regarding the creation of an 
> NXP-specific
> driver. To clarify my concerns, ST, like probably some other SoC vendors, has
> rpmsg-gpio and rpmsg-i2c drivers in downstream with plans to upstream them.
> 

Linus, thank you for jumping in and providing the guidance.

I would like to clarify one point here: what we are discussing now is not 
whether the 
driver itself is generic, but rather that the current protocol is not as 
perfect as Arnaud 
would expect, since it contains a few fields that may not be required on their 
platforms.

Arnaud, if you agree with the points above, my proposal is the following:
 - Remove the fields you mentioned in the protocol and update the driver 
accordingly so 
that we can establish a true baseline for a generic implementation we all agree.
 - Then prepare a separate patch to add support for existing NXP platforms by 
introducing 
the necessary fix‑up functions.

Please let me know if this approach works for you. My goal is to find a 
solution that works for 
everyone — even though I know that pleasing everyone is almost impossible.

Thanks,
Shenwei

> If we proceed in this direction:
> 
> -Any vendor wishing to upstream an rpmsg-gpio driver might submit their own
> platform-specific version.
> 
> - If NXP upstreams other rpmsg drivers, these will likely remain NXP-centric 
> to
> maintain compatibility with their legacy firmware and the nxp-rpmsg-gpio 
> driver,
> leading to platform-specific versions in several frameworks.
> 
> - The implementation will impact not only the Linux side but also the remote 
> side.
> Indeed, some operating systems like Zephyr or NuttX implement the rpmsg device
> side (Zephyr already implements the rpmsg-tty)
> 
> Maintaining a generic approach for RPMsg, similar to what is done for Virtio,
> seems to me a more reliable solution, even though it may induce some
> downstream costs (ST would also need to break compatibility with legacy ST
> remote proc firmware).
> 
> 
> In the end, I am just trying to influence the direction for RPMsg, but based 
> on the
> discussions in this thread, it seems others share similar expectations, which
> should probably be taken into account as well.
> 
> Thanks and Regards,
> Arnaud
> 
> 
> I just want to
> 
> >
> > Yours,
> > Linus Walleij

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

[Bjorn Andersson]

On Mon, Feb 23, 2026 at 03:24:43PM +0100, Arnaud POULIQUEN wrote:
> On 2/22/26 15:48, Linus Walleij wrote:
> > On Fri, Feb 20, 2026 at 7:57 PM Shenwei Wang  wrote:
[..]
> > 
> > Is it generic? If it is not, let's call it "NXP rpmsg GPIO driver" and 
> > rename
> > files etc accordingly. Maybe it can share code with the actual generic
> > RPMSG driver once that arrives, that is more of a library question.
> 
> I would like to (re)express my concerns regarding the creation of an
> NXP-specific driver. To clarify my concerns, ST, like probably some other
> SoC vendors, has rpmsg-gpio and rpmsg-i2c drivers in downstream with plans
> to upstream them.
> 
> If we proceed in this direction:
> 
> -Any vendor wishing to upstream an rpmsg-gpio driver might submit their own
> platform-specific version.
> 
> - If NXP upstreams other rpmsg drivers, these will likely remain NXP-centric
> to maintain compatibility with their legacy firmware and the nxp-rpmsg-gpio
> driver, leading to platform-specific versions in several frameworks.
> 
> - The implementation will impact not only the Linux side but also the remote
> side. Indeed, some operating systems like Zephyr or NuttX implement the
> rpmsg device side (Zephyr already implements the rpmsg-tty)
> 
> Maintaining a generic approach for RPMsg, similar to what is done for
> Virtio, seems to me a more reliable solution, even though it may induce some
> downstream costs (ST would also need to break compatibility with legacy ST
> remote proc firmware).
> 

Could the virtio-based mechanism be used directly (without rpmsg)?


If not, it would be good to derive a generic rpmsg-gpio protocol from
the virtio protocol, and land implementations of this in e.g. Linux and
Zephyr to establish that option.

Regards,
Bjorn

> 
> In the end, I am just trying to influence the direction for RPMsg, but based
> on the discussions in this thread, it seems others share similar
> expectations, which should probably be taken into account as well.
> 
> Thanks and Regards,
> Arnaud
> 
> 
> I just want to
> 
> > 
> > Yours,
> > Linus Walleij
> 

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

[Arnaud POULIQUEN]

Hello Linus,

On 2/22/26 15:48, Linus Walleij wrote:

On Fri, Feb 20, 2026 at 7:57 PM Shenwei Wang  wrote:


Given that, I’d like to hear from the GPIO subsystem maintainers — @Linus 
Walleij and
@Bartosz Golaszewski — on whether a driver that works with the current 
hardware/firmware
design could still be acceptable for upstream inclusion. My understanding is 
that upstream
generally supports existing, real-world hardware as long as the driver meets 
subsystem standards.


What a swell party this has become.

In this kind of situations I usually refer to
Documentation/process/management-style.rst



Thank you for pointing out the document, I was not aware of its 
existence. Very informative!

That help me to understand you proposal below.



What is the message I as a maintainer is getting from NXP regarding
"gpio: rpmsg: add generic rpmsg GPIO driver"?

Arnaud, who is the only person in this discussion who actually wrote
a standard RPMSG driver (drivers/tty/rpmsg_tty.c), must ACK this
patch if it wants to call itself a "generic" RPMSG GPIO driver, if he
does not, then it isn't.


In Fact there are already 2 "generic" drivers, the second one it the 
drivers/rpmsg/rpmsg_char.c, both are used on several platforms.


It is in my plan to test the rpmsg-gpio on ST platform if we go with the 
generic approach.




Is it generic? If it is not, let's call it "NXP rpmsg GPIO driver" and rename
files etc accordingly. Maybe it can share code with the actual generic
RPMSG driver once that arrives, that is more of a library question.


I would like to (re)express my concerns regarding the creation of an 
NXP-specific driver. To clarify my concerns, ST, like probably some 
other SoC vendors, has rpmsg-gpio and rpmsg-i2c drivers in downstream 
with plans to upstream them.


If we proceed in this direction:

-Any vendor wishing to upstream an rpmsg-gpio driver might submit their 
own platform-specific version.


- If NXP upstreams other rpmsg drivers, these will likely remain 
NXP-centric to maintain compatibility with their legacy firmware and the 
nxp-rpmsg-gpio driver, leading to platform-specific versions in several 
frameworks.


- The implementation will impact not only the Linux side but also the 
remote side. Indeed, some operating systems like Zephyr or NuttX 
implement the rpmsg device side (Zephyr already implements the rpmsg-tty)


Maintaining a generic approach for RPMsg, similar to what is done for 
Virtio, seems to me a more reliable solution, even though it may induce 
some downstream costs (ST would also need to break compatibility with 
legacy ST remote proc firmware).



In the end, I am just trying to influence the direction for RPMsg, but 
based on the discussions in this thread, it seems others share similar 
expectations, which should probably be taken into account as well.


Thanks and Regards,
Arnaud


I just want to



Yours,
Linus Walleij

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

[Linus Walleij]

On Fri, Feb 20, 2026 at 7:57 PM Shenwei Wang  wrote:

> Given that, I’d like to hear from the GPIO subsystem maintainers — @Linus 
> Walleij and
> @Bartosz Golaszewski — on whether a driver that works with the current 
> hardware/firmware
> design could still be acceptable for upstream inclusion. My understanding is 
> that upstream
> generally supports existing, real-world hardware as long as the driver meets 
> subsystem standards.

What a swell party this has become.

In this kind of situations I usually refer to
Documentation/process/management-style.rst

What is the message I as a maintainer is getting from NXP regarding
"gpio: rpmsg: add generic rpmsg GPIO driver"?

Arnaud, who is the only person in this discussion who actually wrote
a standard RPMSG driver (drivers/tty/rpmsg_tty.c), must ACK this
patch if it wants to call itself a "generic" RPMSG GPIO driver, if he
does not, then it isn't.

Is it generic? If it is not, let's call it "NXP rpmsg GPIO driver" and rename
files etc accordingly. Maybe it can share code with the actual generic
RPMSG driver once that arrives, that is more of a library question.

Yours,
Linus Walleij

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

[Andrew Lunn]

> To clarify: is Linux moving toward supporting only fully open hardware 
> platforms? I’m 
> not aware of any rule that prevents a company from upstreaming a driver that 
> implements 
> support for an existing hardware/firmware interface.

You are not the first to be requested to implement a clean generic
interface which is vendor neutral for these sorts of peripherals, with
firmware on the other side. Look back in the archives, there was a USB
attached microcontroller offering GPIO, I2C, SPI, etc. In the end the
vendor decided to keep with there out of tree driver. But if it had
progressed, it might of been possible for you to implement that same
protocol over rpmsg, rather than USB. They are similar. And there is
also greybus, which is also a solution in this problem space.

 Andrew

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

[Andrew Lunn]

> Yes, most compilers will lay this out without inserting padding between the 
> fields. 
> However, for a communication packet, we cannot freely reorder or tweak 
> members 
> just to satisfy alignment rules—the field order itself becomes part of the 
> protocol definition.

Notice i've been saying design. If the design is poor, the
implementation has to jump through hoops to make it work, which is
when __packed it useful, it allows you to implement a bad design.

> Even within netdev, where you’re very familiar, the classic ethhdr still 
> carries the packed annotation 
> despite being naturally aligned:
> 
> struct ethhdr {
>   unsigned char   h_dest[ETH_ALEN];   /* destination eth addr */
>   unsigned char   h_source[ETH_ALEN]; /* source ether addr*/
>   __be16  h_proto;/* packet type ID field */
> } __attribute__((packed));

And a lot of engineers will agree that this header is badly
designed. The network stack goes to a lot of trouble to ensure the
Ethernet header is placed into an skbuf with an offset of 2 bytes, so
the IP header is 4 byte aligned. This also makes the DMA engines more
complex, having to do an unaligned transfer at the start. More hoops
to jump though because of bad design.

None of the IP, UDP, TCP headers etc have packed, because they are all
well designed. The IETF did a better design job than IEEE.

   Andrew

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

[Mathieu Poirier]

On Fri, 20 Feb 2026 at 11:57, Shenwei Wang  wrote:
>
>
>
> > -Original Message-
> > From: Andrew Lunn 
> > Sent: Friday, February 20, 2026 11:45 AM
> > To: Shenwei Wang 
> > Cc: Arnaud POULIQUEN ; Linus Walleij
> > ; Bartosz Golaszewski ; Jonathan Corbet
> > ; Rob Herring ; Krzysztof Kozlowski
> > ; Conor Dooley ; Bjorn Andersson
> > ; Mathieu Poirier ; Frank 
> > Li
> > ; Sascha Hauer ; Shuah Khan
> > ; linux-g...@vger.kernel.org; linux-
> > d...@vger.kernel.org; linux-ker...@vger.kernel.org; Pengutronix Kernel Team
> > ; Fabio Estevam ; Peng Fan
> > ; devicet...@vger.kernel.org; linux-
> > remotep...@vger.kernel.org; i...@lists.linux.dev; linux-arm-
> > ker...@lists.infradead.org; dl-linux-imx ; Bartosz
> > Golaszewski 
> > Subject: [EXT] Re: [PATCH v8 3/4] gpio: rpmsg: add generic rpmsg GPIO driver
> > > If there are concerns about specific design elements within the
> > > driver, I’m happy to address those, but redesigning the hardware/firmware
> > interface is not something this series can solve.
> >
> > Then i think you are limited to using the out of tree driver.
> >
>
> Thanks for the feedback.
>
> To clarify: is Linux moving toward supporting only fully open hardware 
> platforms? I’m
> not aware of any rule that prevents a company from upstreaming a driver that 
> implements
> support for an existing hardware/firmware interface.
>
> Given that, I’d like to hear from the GPIO subsystem maintainers — @Linus 
> Walleij and
> @Bartosz Golaszewski — on whether a driver that works with the current 
> hardware/firmware
> design could still be acceptable for upstream inclusion. My understanding is 
> that upstream
> generally supports existing, real-world hardware as long as the driver meets 
> subsystem standards.
>

The HW can't be changed but firmware can.  It is not realistic to
think upstream can accommodate all the quirks happening in downstream
trees - this approach simply doesn't scale.

As if I wasn't clear enough already (along with many others), the
current implementation will not be merged upstream for reasons that
have been amply discussed.  You either comply with the comments we
provided or use the existing out of tree driver.

> Regards,
> Shenwei
>
> > Sorry.
> >
> > Andrew

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

[Shenwei Wang]

> -Original Message-
> From: Andrew Lunn 
> Sent: Friday, February 20, 2026 11:42 AM
> To: Shenwei Wang 
> Cc: Arnaud POULIQUEN ; Linus Walleij
> ; Bartosz Golaszewski ; Jonathan Corbet
> ; Rob Herring ; Krzysztof Kozlowski
> ; Conor Dooley ; Bjorn Andersson
> ; Mathieu Poirier ; Frank Li
> ; Sascha Hauer ; Shuah Khan
> ; linux-g...@vger.kernel.org; linux-
> d...@vger.kernel.org; linux-ker...@vger.kernel.org; Pengutronix Kernel Team
> ; Fabio Estevam ; Peng Fan
> ; devicet...@vger.kernel.org; linux-
> remotep...@vger.kernel.org; i...@lists.linux.dev; linux-arm-
> ker...@lists.infradead.org; dl-linux-imx ; Bartosz
> Golaszewski 
> Subject: [EXT] Re: [PATCH v8 3/4] gpio: rpmsg: add generic rpmsg GPIO driver
> > > > struct msg_hdr {
> > > > u8  id;
> > > > u32 size;
> > > > u8  flags;
> > > > };
> > >
> > > That is just a bad design. The point of not allowing __packed is
> > > that it forces you to design your structures correctly. Maybe AI has
> > > no idea of taste, but Maintainer do and would not allow a u32 to be 
> > > unaligned
> like this.
> > >
> >
> > Let’s keep the discussion technical instead of taste. 😊
> >
> > My point with the earlier example was simply to illustrate how layout
> > differences can happen across architectures or compilers. I’m more
> > interested in understanding how you would prefer this specific
> > structure to be defined so that it avoids unaligned fields while still 
> > maintaining a
> stable on‑wire format.
> 
> struct msg_hdr {
> u32 size;
> u8  id;
> u8  flags;
> };
> 
> The compiler will lay this out as you expect, it won't add any padding between
> the fields.
> 

Yes, most compilers will lay this out without inserting padding between the 
fields. 
However, for a communication packet, we cannot freely reorder or tweak members 
just to satisfy alignment rules—the field order itself becomes part of the 
protocol definition.

Even within netdev, where you’re very familiar, the classic ethhdr still 
carries the packed annotation 
despite being naturally aligned:

struct ethhdr {
unsigned char   h_dest[ETH_ALEN];   /* destination eth addr */
unsigned char   h_source[ETH_ALEN]; /* source ether addr*/
__be16  h_proto;/* packet type ID field */
} __attribute__((packed));

Thanks,
Shenwei

> You need to be careful with sizeof(struct msg_hdr). On 8 and 16 bit machines, 
> it is
> probably 6. On 32, or 64 bit machine it is probably 8.
> 
> I would say having a 6 byte message is probably a bad design, and you should 
> try
> to make the u32 a u16 if you can. Again, not using the __packed is making you
> think about the design, and probably makes the design better as a result.
> 
> Andrew

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

[Shenwei Wang]

> -Original Message-
> From: Andrew Lunn 
> Sent: Friday, February 20, 2026 11:45 AM
> To: Shenwei Wang 
> Cc: Arnaud POULIQUEN ; Linus Walleij
> ; Bartosz Golaszewski ; Jonathan Corbet
> ; Rob Herring ; Krzysztof Kozlowski
> ; Conor Dooley ; Bjorn Andersson
> ; Mathieu Poirier ; Frank Li
> ; Sascha Hauer ; Shuah Khan
> ; linux-g...@vger.kernel.org; linux-
> d...@vger.kernel.org; linux-ker...@vger.kernel.org; Pengutronix Kernel Team
> ; Fabio Estevam ; Peng Fan
> ; devicet...@vger.kernel.org; linux-
> remotep...@vger.kernel.org; i...@lists.linux.dev; linux-arm-
> ker...@lists.infradead.org; dl-linux-imx ; Bartosz
> Golaszewski 
> Subject: [EXT] Re: [PATCH v8 3/4] gpio: rpmsg: add generic rpmsg GPIO driver
> > If there are concerns about specific design elements within the
> > driver, I’m happy to address those, but redesigning the hardware/firmware
> interface is not something this series can solve.
> 
> Then i think you are limited to using the out of tree driver.
> 

Thanks for the feedback.

To clarify: is Linux moving toward supporting only fully open hardware 
platforms? I’m 
not aware of any rule that prevents a company from upstreaming a driver that 
implements 
support for an existing hardware/firmware interface.

Given that, I’d like to hear from the GPIO subsystem maintainers — @Linus 
Walleij and 
@Bartosz Golaszewski — on whether a driver that works with the current 
hardware/firmware 
design could still be acceptable for upstream inclusion. My understanding is 
that upstream 
generally supports existing, real-world hardware as long as the driver meets 
subsystem standards.

Regards,
Shenwei

> Sorry.
> 
> Andrew

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

[Andrew Lunn]

> If there are concerns about specific design elements within the driver, I’m 
> happy to address 
> those, but redesigning the hardware/firmware interface is not something this 
> series can solve.

Then i think you are limited to using the out of tree driver.

Sorry.

Andrew

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

[Andrew Lunn]

> > > struct msg_hdr {
> > > u8  id;
> > > u32 size;
> > > u8  flags;
> > > };
> > 
> > That is just a bad design. The point of not allowing __packed is that it 
> > forces you
> > to design your structures correctly. Maybe AI has no idea of taste, but 
> > Maintainer
> > do and would not allow a u32 to be unaligned like this.
> > 
> 
> Let’s keep the discussion technical instead of taste. 😊
> 
> My point with the earlier example was simply to illustrate how layout 
> differences can 
> happen across architectures or compilers. I’m more interested in 
> understanding how 
> you would prefer this specific structure to be defined so that it avoids 
> unaligned fields 
> while still maintaining a stable on‑wire format.

struct msg_hdr {
u32 size;
u8  id;
u8  flags;
};

The compiler will lay this out as you expect, it won't add any padding
between the fields.

You need to be careful with sizeof(struct msg_hdr). On 8 and 16 bit
machines, it is probably 6. On 32, or 64 bit machine it is probably 8.

I would say having a 6 byte message is probably a bad design, and you
should try to make the u32 a u16 if you can. Again, not using the
__packed is making you think about the design, and probably makes the
design better as a result.

Andrew

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

[Shenwei Wang]

> -Original Message-
> From: Andrew Lunn 
> Sent: Friday, February 20, 2026 10:19 AM
> To: Shenwei Wang 
> Cc: Arnaud POULIQUEN ; Linus Walleij
> ; Bartosz Golaszewski ; Jonathan Corbet
> ; Rob Herring ; Krzysztof Kozlowski
> ; Conor Dooley ; Bjorn Andersson
> ; Mathieu Poirier ; Frank Li
> ; Sascha Hauer ; Shuah Khan
> ; linux-g...@vger.kernel.org; linux-
> d...@vger.kernel.org; linux-ker...@vger.kernel.org; Pengutronix Kernel Team
> ; Fabio Estevam ; Peng Fan
> ; devicet...@vger.kernel.org; linux-
> remotep...@vger.kernel.org; i...@lists.linux.dev; linux-arm-
> ker...@lists.infradead.org; dl-linux-imx ; Bartosz
> Golaszewski 
> Subject: [EXT] Re: [PATCH v8 3/4] gpio: rpmsg: add generic rpmsg GPIO driver
> > Example: 32‑bit ARM vs 64‑bit ARM (AArch32 vs AArch64) Consider this
> > message header:
> > struct msg_hdr {
> > u8  id;
> > u32 size;
> > u8  flags;
> > };
> 
> That is just a bad design. The point of not allowing __packed is that it 
> forces you
> to design your structures correctly. Maybe AI has no idea of taste, but 
> Maintainer
> do and would not allow a u32 to be unaligned like this.
> 

Let’s keep the discussion technical instead of taste. 😊

My point with the earlier example was simply to illustrate how layout 
differences can 
happen across architectures or compilers. I’m more interested in understanding 
how 
you would prefer this specific structure to be defined so that it avoids 
unaligned fields 
while still maintaining a stable on‑wire format.

For the rpmsg‑gpio driver specifically, I can drop the __packed annotation if 
that is the 
consensus, since the known configurations work without requiring it. 

Thanks,
Shenwei

>Andrew

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

[Shenwei Wang]

> -Original Message-
> From: Andrew Lunn 
> Sent: Friday, February 20, 2026 7:49 AM
> To: Arnaud POULIQUEN 
> Cc: Shenwei Wang ; Linus Walleij
> ; Bartosz Golaszewski ; Jonathan Corbet
> ; Rob Herring ; Krzysztof Kozlowski
> ; Conor Dooley ; Bjorn Andersson
> ; Mathieu Poirier ; Frank Li
> ; Sascha Hauer ; Shuah Khan
> ; linux-g...@vger.kernel.org; linux-
> d...@vger.kernel.org; linux-ker...@vger.kernel.org; Pengutronix Kernel Team
> ; Fabio Estevam ; Peng Fan
> ; devicet...@vger.kernel.org; linux-
> remotep...@vger.kernel.org; i...@lists.linux.dev; linux-arm-
> ker...@lists.infradead.org; dl-linux-imx ; Bartosz
> Golaszewski 
> Subject: [EXT] Re: [PATCH v8 3/4] gpio: rpmsg: add generic rpmsg GPIO driver
> >
> > On 2/19/26 14:42, Andrew Lunn wrote:
> > > > > +   u8 id;  /* Message ID Code */
> > > > > +   u8 vendor;  /* Vendor ID number */
> > > >
> > > > Does this fields above are mandatory, seems that it is just some
> > > > constant values that are useless.
> > > >
> > > > > +   u8 version; /* Vendor-specific version number */
> > > >
> > > > Why it is vendor specific? the version should represent the
> > > > rpmsg-tty protocol version.
> > > >
> > > > > +   u8 type;/* Message type */
> > > > > +   u8 cmd; /* Command code */
> > > > > +   u8 reserved[5];
> > > >
> > > > What is the purpose of this reserved field?
> > >
> > > They have an implementation of the other end running on there
> > > systems, and it sounds like it is widely deployed, and they are
> > > trying to keep backwards compatibility. The protocol also implements
> > > more than GPIO. There is also I2C, maybe watchdog, i don't remember,
> > > but early versions of this patchset had a list. Some of these fields
> > > are used for some of these other devices.
> > >
> > > I've been arguing it should be a clean design, with the protocol
> > > focusing on GPIO. And that the rpmsg channel makes it clear this is
> > > a GPIO device, the protocol itself does not need to include fields
> > > to differentiate between GPIO, I2C etc.
> > >
> > > When they start submitting I2C over rpmsg, i expect the same sort of
> > > discussion will start again, so the likelihood of keeping backwards
> > > compatible with there firmware seems low to me.
> >
> > Agree with you.
> 
> Hi Shenwei
> 
> We now have two Maintainers who think you should make a clean design...
> 
> You should go talk with your Management and decide what they want to do.
> Drop this patch series and only support the out of tree driver? Or rework this
> driver and the firmware to the liking of the mainline Maintainers.
> 

The driver is meant to support a specific piece of hardware, and its design 
follows the 
company’s existing IP, which is already widely deployed. While we're open to 
sharing 
the IP details with anyone who wants to adopt the technology, changing the IP 
itself 
isn’t the goal of this patch series.

From the driver perspective, the implementation is clean and generic, and it 
can support 
multiple hardware variants—even those based on other vendors’ IP—without 
requiring 
architectural changes. I think it would be more productive if the review 
focused on the 
driver’s implementation and integration into the subsystem, rather than asking 
to redesign 
or replace an existing, widely‑released IP block. The intention here is to 
upstream support 
for hardware/firmware that already exists, not to redefine it.

If there are concerns about specific design elements within the driver, I’m 
happy to address 
those, but redesigning the hardware/firmware interface is not something this 
series can solve.

Thanks,
Shenwei


> Andrew

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

[Andrew Lunn]

> Example: 32‑bit ARM vs 64‑bit ARM (AArch32 vs AArch64)
> Consider this message header:
> struct msg_hdr {
> u8  id;
> u32 size;
> u8  flags;
> };

That is just a bad design. The point of not allowing __packed is that
it forces you to design your structures correctly. Maybe AI has no
idea of taste, but Maintainer do and would not allow a u32 to be
unaligned like this.

   Andrew

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

[Shenwei Wang]

> -Original Message-
> From: Arnaud POULIQUEN 
> Sent: Friday, February 20, 2026 3:13 AM
> To: Shenwei Wang ; Linus Walleij
> ; Bartosz Golaszewski ; Jonathan Corbet
> ; Rob Herring ; Krzysztof Kozlowski
> ; Conor Dooley ; Bjorn Andersson
> ; Mathieu Poirier ; Frank Li
> ; Sascha Hauer 
> Cc: Shuah Khan ; linux-g...@vger.kernel.org; linux-
> d...@vger.kernel.org; linux-ker...@vger.kernel.org; Pengutronix Kernel Team
> ; Fabio Estevam ; Peng Fan
> ; devicet...@vger.kernel.org; linux-
> remotep...@vger.kernel.org; i...@lists.linux.dev; linux-arm-
> ker...@lists.infradead.org; dl-linux-imx ; Bartosz
> Golaszewski ; Andrew Lunn 
> Subject: [EXT] Re: [PATCH v8 3/4] gpio: rpmsg: add generic rpmsg GPIO driver
> >>> @@ -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 ..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.
> >>> + */
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +
> >>> +#define RPMSG_GPIO_ID5
> >>> +#define RPMSG_VENDOR 1
> >>> +#define RPMSG_VERSION0
> >>> +
> >>> +#define GPIOS_PER_PORT_DEFAULT   32
> >>> +#define RPMSG_TIMEOUT1000
> >>> +
> >>> +/* GPIO RPMSG header type */
> >>> +#define GPIO_RPMSG_SETUP 0
> >>> +#define GPIO_RPMSG_REPLY 1
> >>> +#define GPIO_RPMSG_NOTIFY2
> >>> +
> >>> +/* GPIO Interrupt trigger type */
> >>> +#define GPIO_RPMSG_TRI_IGNORE0
> >>> +#define GPIO_RPMSG_TRI_RISING1
> >>> +#define GPIO_RPMSG_TRI_FALLING   2
> >>> +#define GPIO_RPMSG_TRI_BOTH_EDGE 3
> >>> +#define GPIO_RPMSG_TRI_LOW_LEVEL 4
> >>> +#define GPIO_RPMSG_TRI_HIGH_LEVEL5
> >>> +
> >>> +/* GPIO RPMSG commands */
> >>> +#define GPIO_RPMSG_INPUT_INIT0
> >>> +#define GPIO_RPMSG_OUTPUT_INIT   1
> >>> +#define GPIO_RPMSG_INPUT_GET 2
> >>> +#define GPIO_RPMSG_DIRECTION_GET 3
> >>> +
> >>> +#define MAX_PORT_PER_CHANNEL10
> >>> +
> >>> +/*
> >>> + * @rproc_name: the name of the remote proc.
> >>> + * @channel_devices: an array of the devices related to the rpdev.
> >>> + */
> >>> +struct rpdev_drvdata {
> >>> + const char *rproc_name;
> >>> + void *channel_devices[MAX_PORT_PER_CHANNEL];
> >>> +};
> >>> +
> >>> +struct gpio_rpmsg_head {
> >>
> >> Sometime the prefix is gpio_rpmsg, sometime rpmsg_gpio or just gpio,
> >> could you use "rpmsg_gpio" prefix in the whole driver?
> >>
> >
> > All the types use prefix gpio_rpmsg. All the functions use rpmsg_gpio.
> >
> >>> + u8 id;  /* Message ID Code */
> >>> + u8 vendor;  /* Vendor ID number */
> >>
> >> Does this fields above are mandatory, seems that it is just some
> >> constant values that are useless.
> >>
> >>> + u8 version; /* Vendor-specific version number */
> >>
> >> Why it is ven

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

[Shenwei Wang]

> -Original Message-
> From: Arnaud POULIQUEN 
> Sent: Friday, February 20, 2026 3:33 AM
> To: Shenwei Wang ; Linus Walleij
> ; Bartosz Golaszewski ; Jonathan Corbet
> ; Rob Herring ; Krzysztof Kozlowski
> ; Conor Dooley ; Bjorn Andersson
> ; Mathieu Poirier ; Frank Li
> ; Sascha Hauer 
> Cc: Shuah Khan ; linux-g...@vger.kernel.org; linux-
> d...@vger.kernel.org; linux-ker...@vger.kernel.org; Pengutronix Kernel Team
> ; Fabio Estevam ; Peng Fan
> ; devicet...@vger.kernel.org; linux-
> remotep...@vger.kernel.org; i...@lists.linux.dev; linux-arm-
> ker...@lists.infradead.org; dl-linux-imx ; Bartosz
> Golaszewski ; Andrew Lunn 
> Subject: [EXT] Re: [PATCH v8 3/4] gpio: rpmsg: add generic rpmsg GPIO driver
> >> Subject: [EXT] Re: [PATCH v8 3/4] gpio: rpmsg: add generic rpmsg GPIO
> >> driver
> >>> + 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);
> >>
> >> Is a topology where they is no rproc->dev node but a parent node exist?
> >>
> >
> > If no rproc->dev, it should return NULL in the above check.
> 
> Regarding rproc_alloc, seems that rproc->dev.of_node is always NULL.
> so probably test on it is useless.
> 

Even if rproc->dev.of_node happens to be NULL in current rproc_alloc() usage, 
the logic here is
written to handle both cases correctly. It’s safer not to rely on that 
assumption, since making 
decisions based on an expected NULL value could break if the allocation path 
changes in the future. 
Keeping the check avoids this unnecessary constraint.

> >
> >>> +
> >>> + 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);
> >>> + }
> >>> +
> >>> + 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);
> >>
> >> What happen if the remoteprocessor answer after the completion timeout?
> >> Could it result in desynchronization between the request and the answer?
> >
> > If the remote processor responds after the timeout, that late reply
> > will be ignored. The current transfer should fail with TIMEOUT, and
> > the state won’t be carried over because cmd_complete is reinitialized
> > before each new request, so a stale completion won’t desynchronize the
> > next transaction. Each command–reply cycle is isolated, so a delayed reply
> cannot corrupt or mix with a subsequent request.
> 
> I missed the reinit_completion. Indeed, that prevents issue if reply arrive 
> after
> the time out.
> 
> That said a second request can be sent before the remote processor responds to
> the first one:
> - resquest 1 sent to remoteprocessor.
> - timeout occurs
> - request 2 sent to remote processor
> - reply of request 1 received
> 
> Wouldn't this lead to a desynchronization between requests and replies?
> I do not see a mechanism that would prevent this
> 

There’s a lock in place that sequences the requests, so we won’t end up with 
two requests 
running in parallel. If request 1 hasn’t completed yet, request 2 won’t start.

> >
> >>
> >> Having a cmd_counter in gpio_rpmsg_head could help to identify
> >> current request and answer
> >>
> >> the use of reinit_completion could be also needed
> >

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

[Andrew Lunn]

On Fri, Feb 20, 2026 at 10:16:42AM +0100, Arnaud POULIQUEN wrote:
> 
> 
> On 2/19/26 14:42, Andrew Lunn wrote:
> > > > +   u8 id;  /* Message ID Code */
> > > > +   u8 vendor;  /* Vendor ID number */
> > > 
> > > Does this fields above are mandatory, seems that it is just some constant
> > > values that are useless.
> > > 
> > > > +   u8 version; /* Vendor-specific version number */
> > > 
> > > Why it is vendor specific? the version should represent the rpmsg-tty
> > > protocol version.
> > > 
> > > > +   u8 type;/* Message type */
> > > > +   u8 cmd; /* Command code */
> > > > +   u8 reserved[5];
> > > 
> > > What is the purpose of this reserved field?
> > 
> > They have an implementation of the other end running on there systems,
> > and it sounds like it is widely deployed, and they are trying to keep
> > backwards compatibility. The protocol also implements more than
> > GPIO. There is also I2C, maybe watchdog, i don't remember, but early
> > versions of this patchset had a list. Some of these fields are used
> > for some of these other devices.
> > 
> > I've been arguing it should be a clean design, with the protocol
> > focusing on GPIO. And that the rpmsg channel makes it clear this is a
> > GPIO device, the protocol itself does not need to include fields to
> > differentiate between GPIO, I2C etc.
> > 
> > When they start submitting I2C over rpmsg, i expect the same sort of
> > discussion will start again, so the likelihood of keeping backwards
> > compatible with there firmware seems low to me.
> 
> Agree with you.

Hi Shenwei

We now have two Maintainers who think you should make a clean
design...

You should go talk with your Management and decide what they want to
do. Drop this patch series and only support the out of tree driver? Or
rework this driver and the firmware to the liking of the mainline
Maintainers.

Andrew

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

[Arnaud POULIQUEN]

On 2/19/26 22:13, Shenwei Wang wrote:




-Original Message-
From: Arnaud POULIQUEN 
Sent: Thursday, February 19, 2026 3:21 AM
To: Shenwei Wang ; Linus Walleij
; Bartosz Golaszewski ; Jonathan Corbet
; Rob Herring ; Krzysztof Kozlowski
; Conor Dooley ; Bjorn Andersson
; Mathieu Poirier ; Frank Li
; Sascha Hauer 
Cc: Shuah Khan ; linux-g...@vger.kernel.org; linux-
d...@vger.kernel.org; linux-ker...@vger.kernel.org; Pengutronix Kernel Team
; Fabio Estevam ; Peng Fan
; devicet...@vger.kernel.org; linux-
remotep...@vger.kernel.org; i...@lists.linux.dev; linux-arm-
ker...@lists.infradead.org; dl-linux-imx ; Bartosz
Golaszewski ; Andrew Lunn 
Subject: [EXT] Re: [PATCH v8 3/4] gpio: rpmsg: add generic rpmsg GPIO driver

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


Is a topology where they is no rproc->dev node but a parent node exist?



If no rproc->dev, it should return NULL in the above check.


Regarding rproc_alloc, seems that rproc->dev.of_node is always NULL.
so probably test on it is useless.




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


What happen if the remoteprocessor answer after the completion timeout?
Could it result in desynchronization between the request and the answer?


If the remote processor responds after the timeout, that late reply will be 
ignored. The current
transfer should fail with TIMEOUT, and the state won’t be carried over because 
cmd_complete
is reinitialized before each new request, so a stale completion won’t 
desynchronize the next
transaction. Each command–reply cycle is isolated, so a delayed reply cannot 
corrupt or mix with
a subsequent request.


I missed the reinit_completion. Indeed, that prevents issue if reply 
arrive after the time out.


That said a second request can be sent before the remote processor 
responds to the first one:

- resquest 1 sent to remoteprocessor.
- timeout occurs
- request 2 sent to remote processor
- reply of request 1 received

Wouldn't this lead to a desynchronization between requests and replies? 
I do not see a mechanism that would prevent this






Having a cmd_counter in gpio_rpmsg_head could help to identify current request
and answer

the use of reinit_completion could be also needed


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


Could you print the msg->header.type value to help for debug?



Sure. Will add it in next version.


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


return ret
or indicate why the return of rpmsg_gpiochip_register is not taken into account



rpmsg

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

[Arnaud POULIQUEN]

On 2/19/26 14:42, Andrew Lunn wrote:

+   u8 id;  /* Message ID Code */
+   u8 vendor;  /* Vendor ID number */


Does this fields above are mandatory, seems that it is just some constant
values that are useless.


+   u8 version; /* Vendor-specific version number */


Why it is vendor specific? the version should represent the rpmsg-tty
protocol version.


+   u8 type;/* Message type */
+   u8 cmd; /* Command code */
+   u8 reserved[5];


What is the purpose of this reserved field?


They have an implementation of the other end running on there systems,
and it sounds like it is widely deployed, and they are trying to keep
backwards compatibility. The protocol also implements more than
GPIO. There is also I2C, maybe watchdog, i don't remember, but early
versions of this patchset had a list. Some of these fields are used
for some of these other devices.

I've been arguing it should be a clean design, with the protocol
focusing on GPIO. And that the rpmsg channel makes it clear this is a
GPIO device, the protocol itself does not need to include fields to
differentiate between GPIO, I2C etc.

When they start submitting I2C over rpmsg, i expect the same sort of
discussion will start again, so the likelihood of keeping backwards
compatible with there firmware seems low to me.


Agree with you.

Thanks,
Arnaud



   Andrew

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

[Arnaud POULIQUEN]

On 2/19/26 21:04, Shenwei Wang wrote:




-Original Message-
From: Arnaud POULIQUEN 
Sent: Thursday, February 19, 2026 3:21 AM
To: Shenwei Wang ; Linus Walleij
; Bartosz Golaszewski ; Jonathan Corbet
; Rob Herring ; Krzysztof Kozlowski
; Conor Dooley ; Bjorn Andersson
; Mathieu Poirier ; Frank Li
; Sascha Hauer 
Cc: Shuah Khan ; linux-g...@vger.kernel.org; linux-
d...@vger.kernel.org; linux-ker...@vger.kernel.org; Pengutronix Kernel Team
; Fabio Estevam ; Peng Fan
; devicet...@vger.kernel.org; linux-
remotep...@vger.kernel.org; i...@lists.linux.dev; linux-arm-
ker...@lists.infradead.org; dl-linux-imx ; Bartosz
Golaszewski ; Andrew Lunn 
Subject: [EXT] Re: [PATCH v8 3/4] gpio: rpmsg: add generic rpmsg GPIO driver

Hello Shenwei,

This versiob seems much more generic, thanks!

On 2/12/26 22:36, Shenwei Wang wrote:

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 
Cc: Andrew Lunn 
Signed-off-by: Shenwei Wang 
---
   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
+ 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 ..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.
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define RPMSG_GPIO_ID5
+#define RPMSG_VENDOR 1
+#define RPMSG_VERSION0
+
+#define GPIOS_PER_PORT_DEFAULT   32
+#define RPMSG_TIMEOUT1000
+
+/* GPIO RPMSG header type */
+#define GPIO_RPMSG_SETUP 0
+#define GPIO_RPMSG_REPLY 1
+#define GPIO_RPMSG_NOTIFY2
+
+/* GPIO Interrupt trigger type */
+#define GPIO_RPMSG_TRI_IGNORE0
+#define GPIO_RPMSG_TRI_RISING1
+#define GPIO_RPMSG_TRI_FALLING   2
+#define GPIO_RPMSG_TRI_BOTH_EDGE 3
+#define GPIO_RPMSG_TRI_LOW_LEVEL 4
+#define GPIO_RPMSG_TRI_HIGH_LEVEL5
+
+/* GPIO RPMSG commands */
+#define GPIO_RPMSG_INPUT_INIT0
+#define GPIO_RPMSG_OUTPUT_INIT   1
+#define GPIO_RPMSG_INPUT_GET 2
+#define GPIO_RPMSG_DIRECTION_GET 3
+
+#define MAX_PORT_PER_CHANNEL10
+
+/*
+ * @rproc_name: the name of the remote proc.
+ * @channel_devices: an array of the devices related to the rpdev.
+ */
+struct rpdev_drvdata {
+ const char *rproc_name;
+ void *channel_devices[MAX_PORT_PER_CHANNEL];
+};
+
+struct gpio_rpmsg_head {


Sometime the prefix is gpio_rpmsg, sometime rpmsg_gpio or just gpio, could you
use "rpmsg_gpio" prefix in the whole driver?



All the types use prefix gpio_rpmsg. All the functions use rpmsg_gpio.


+ u8 id;  /* Message ID Code */
+ u8 vendor;  /* Vendor ID number */


Does this fields above are mandatory, seems that it is just some constant values
that are useless.


+ u8 version; /* Vendor-specific version number */

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

[Shenwei Wang]

> -Original Message-
> From: Arnaud POULIQUEN 
> Sent: Thursday, February 19, 2026 3:21 AM
> To: Shenwei Wang ; Linus Walleij
> ; Bartosz Golaszewski ; Jonathan Corbet
> ; Rob Herring ; Krzysztof Kozlowski
> ; Conor Dooley ; Bjorn Andersson
> ; Mathieu Poirier ; Frank Li
> ; Sascha Hauer 
> Cc: Shuah Khan ; linux-g...@vger.kernel.org; linux-
> d...@vger.kernel.org; linux-ker...@vger.kernel.org; Pengutronix Kernel Team
> ; Fabio Estevam ; Peng Fan
> ; devicet...@vger.kernel.org; linux-
> remotep...@vger.kernel.org; i...@lists.linux.dev; linux-arm-
> ker...@lists.infradead.org; dl-linux-imx ; Bartosz
> Golaszewski ; Andrew Lunn 
> Subject: [EXT] Re: [PATCH v8 3/4] gpio: rpmsg: add generic rpmsg GPIO driver
> > + 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);
> 
> Is a topology where they is no rproc->dev node but a parent node exist?
> 

If no rproc->dev, it should return NULL in the above check.

> > +
> > + 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);
> > + }
> > +
> > + 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);
> 
> What happen if the remoteprocessor answer after the completion timeout?
> Could it result in desynchronization between the request and the answer?

If the remote processor responds after the timeout, that late reply will be 
ignored. The current 
transfer should fail with TIMEOUT, and the state won’t be carried over because 
cmd_complete 
is reinitialized before each new request, so a stale completion won’t 
desynchronize the next 
transaction. Each command–reply cycle is isolated, so a delayed reply cannot 
corrupt or mix with 
a subsequent request.

> 
> Having a cmd_counter in gpio_rpmsg_head could help to identify current request
> and answer
> 
> the use of reinit_completion could be also needed
> 
> > + } 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");
> 
> Could you print the msg->header.type value to help for debug?
> 

Sure. Will add it in next version.

> > +
> > + 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) {
> > + if (!of_device_is_available(child))
> > + continue;
> > +
> > + if (!of_match_node(dev->driver->of_match_table, child))
> > + continue;
&g

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

[Shenwei Wang]

> -Original Message-
> From: Andrew Lunn 
> Sent: Thursday, February 19, 2026 2:51 PM
> To: Shenwei Wang 
> Cc: Arnaud POULIQUEN ; Linus Walleij
> ; Bartosz Golaszewski ; Jonathan Corbet
> ; Rob Herring ; Krzysztof Kozlowski
> ; Conor Dooley ; Bjorn Andersson
> ; Mathieu Poirier ; Frank Li
> ; Sascha Hauer ; Shuah Khan
> ; linux-g...@vger.kernel.org; linux-
> d...@vger.kernel.org; linux-ker...@vger.kernel.org; Pengutronix Kernel Team
> ; Fabio Estevam ; Peng Fan
> ; devicet...@vger.kernel.org; linux-
> remotep...@vger.kernel.org; i...@lists.linux.dev; linux-arm-
> ker...@lists.infradead.org; dl-linux-imx ; Bartosz
> Golaszewski 
> Subject: [EXT] Re: [PATCH v8 3/4] gpio: rpmsg: add generic rpmsg GPIO driver
> > > Any reason to use __packed and alignement here?
> > > This structure will be copied in a vring buffer right?
> > >
> >
> > Using __packed together with an explicit alignment is a common pattern
> > for defining communication packets. The goal is to ensure a stable and
> > predictable layout across different architectures and compilers.
> 
> Being mostly a netdev person, i can say that the network Maintainers actually
> refuses patches with __packed. If you have designed your protocol correctly,
> defined your structure correctly, you should not need them.
> 

The fact is that there are many __packed usages just under the Ethernet 
directory.
$ grep -rn __packed drivers/net/ethernet | wc -l
553

Regards,
Shenwei

> We do however accept things like
> 
> BUILD_BUG_ON(sizeof(strcut foo) != 8);
> 
> just to make sure the compile is doing what you expect.
> 
> Andrew

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

[Andrew Lunn]

> > Any reason to use __packed and alignement here?
> > This structure will be copied in a vring buffer right?
> > 
> 
> Using __packed together with an explicit alignment is a common pattern for 
> defining 
> communication packets. The goal is to ensure a stable and predictable layout 
> across 
> different architectures and compilers.

Being mostly a netdev person, i can say that the network Maintainers
actually refuses patches with __packed. If you have designed your
protocol correctly, defined your structure correctly, you should not
need them.

We do however accept things like

BUILD_BUG_ON(sizeof(strcut foo) != 8);

just to make sure the compile is doing what you expect.

Andrew

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

[Shenwei Wang]

> -Original Message-
> From: Arnaud POULIQUEN 
> Sent: Thursday, February 19, 2026 3:21 AM
> To: Shenwei Wang ; Linus Walleij
> ; Bartosz Golaszewski ; Jonathan Corbet
> ; Rob Herring ; Krzysztof Kozlowski
> ; Conor Dooley ; Bjorn Andersson
> ; Mathieu Poirier ; Frank Li
> ; Sascha Hauer 
> Cc: Shuah Khan ; linux-g...@vger.kernel.org; linux-
> d...@vger.kernel.org; linux-ker...@vger.kernel.org; Pengutronix Kernel Team
> ; Fabio Estevam ; Peng Fan
> ; devicet...@vger.kernel.org; linux-
> remotep...@vger.kernel.org; i...@lists.linux.dev; linux-arm-
> ker...@lists.infradead.org; dl-linux-imx ; Bartosz
> Golaszewski ; Andrew Lunn 
> Subject: [EXT] Re: [PATCH v8 3/4] gpio: rpmsg: add generic rpmsg GPIO driver
> 
> Hello Shenwei,
> 
> This versiob seems much more generic, thanks!
> 
> On 2/12/26 22:36, Shenwei Wang wrote:
> > 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 
> > Cc: Andrew Lunn 
> > Signed-off-by: Shenwei Wang 
> > ---
> >   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
> > + 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 ..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.
> > + */
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#define RPMSG_GPIO_ID5
> > +#define RPMSG_VENDOR 1
> > +#define RPMSG_VERSION0
> > +
> > +#define GPIOS_PER_PORT_DEFAULT   32
> > +#define RPMSG_TIMEOUT1000
> > +
> > +/* GPIO RPMSG header type */
> > +#define GPIO_RPMSG_SETUP 0
> > +#define GPIO_RPMSG_REPLY 1
> > +#define GPIO_RPMSG_NOTIFY2
> > +
> > +/* GPIO Interrupt trigger type */
> > +#define GPIO_RPMSG_TRI_IGNORE0
> > +#define GPIO_RPMSG_TRI_RISING1
> > +#define GPIO_RPMSG_TRI_FALLING   2
>

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

[Shenwei Wang]

> -Original Message-
> From: Andrew Lunn 
> Sent: Thursday, February 19, 2026 9:25 AM
> To: Shenwei Wang 
> Cc: Arnaud POULIQUEN ; Linus Walleij
> ; Bartosz Golaszewski ; Jonathan Corbet
> ; Rob Herring ; Krzysztof Kozlowski
> ; Conor Dooley ; Bjorn Andersson
> ; Mathieu Poirier ; Frank Li
> ; Sascha Hauer ; Shuah Khan
> ; linux-g...@vger.kernel.org; linux-
> d...@vger.kernel.org; linux-ker...@vger.kernel.org; Pengutronix Kernel Team
> ; Fabio Estevam ; Peng Fan
> ; devicet...@vger.kernel.org; linux-
> remotep...@vger.kernel.org; i...@lists.linux.dev; linux-arm-
> ker...@lists.infradead.org; dl-linux-imx ; Bartosz
> Golaszewski 
> Subject: [EXT] Re: [PATCH v8 3/4] gpio: rpmsg: add generic rpmsg GPIO driver
> On Thu, Feb 19, 2026 at 02:17:26PM +, Shenwei Wang wrote:
> >
> >
> > > -Original Message-
> > > From: Andrew Lunn 
> > > Sent: Thursday, February 19, 2026 7:27 AM
> > > To: Arnaud POULIQUEN 
> > > Cc: Shenwei Wang ; Linus Walleij
> > > ; Bartosz Golaszewski ; Jonathan
> > > Corbet ; Rob Herring ; Krzysztof
> > > Kozlowski ; Conor Dooley ;
> > > Bjorn Andersson ; Mathieu Poirier
> > > ; Frank Li ; Sascha
> > > Hauer ; Shuah Khan
> > > ; linux-g...@vger.kernel.org; linux-
> > > d...@vger.kernel.org; linux-ker...@vger.kernel.org; Pengutronix
> > > Kernel Team ; Fabio Estevam
> > > ; Peng Fan ;
> > > devicet...@vger.kernel.org; linux- remotep...@vger.kernel.org;
> > > i...@lists.linux.dev; linux-arm- ker...@lists.infradead.org;
> > > dl-linux-imx ; Bartosz Golaszewski
> > > 
> > > Subject: [EXT] Re: [PATCH v8 3/4] gpio: rpmsg: add generic rpmsg
> > > GPIO driver
> > >
> > > > > +   if (sync) {
> > > > > +   err = wait_for_completion_timeout(&info->cmd_complete,
> > > > > + 
> > > > > msecs_to_jiffies(RPMSG_TIMEOUT));
> > > > > +   if (err == 0) {
> > > > > +   dev_err(&info->rpdev->dev, "rpmsg_send 
> > > > > timeout!\n");
> > > > > +   return -ETIMEDOUT;
> > > >
> > > > strange condition you return an error if err == 0, for redability use 
> > > > 'ret'
> > > > variable or simply:
> > > >
> > > >   if(!wait_for_completion_timeout(&info->cmd_complete,
> > > > msecs_to_jiffies(RPMSG_TIMEOUT)) {
> > > >   dev_err(&info->rpdev->dev, "rpmsg_send 
> > > > timeout!\n");
> > > >   return -ETIMEDOUT;
> > > >   }
> > >
> > > This will be from a comment i made. It appears that
> > > do_wait_for_common() can return -ERESTARTSYS. I assume that should
> > > be returned to user space?
> > >
> >
> > It looks like there might be a bit of confusion around what
> > wait_for_completion_timeout() actually returns. That function never
> > returns -ERESTARTSYS. Instead, its behavior is pretty
> > simple:
> >
> > - 0 means the wait timed out
> > - A positive value means the completion happened (the value is just
> > the remaining jiffies)
> >
> > So the driver returns the timeout error, and the upper application can
> > decide how it wants to handle that situation, for example restart or ignore.
> 
> wait_for_completion_timeout():
> return wait_for_common(x, timeout, TASK_UNINTERRUPTIBLE);
> 
> wait_for_common():
> return __wait_for_common(x, io_schedule_timeout, timeout, state);
> 
> __wait_for_common():
> timeout = do_wait_for_common(x, action, timeout, state);
> ...
> return timeout;
> 
> do_wait_for_common():
> 
> do {
> if (signal_pending_state(state, current)) {
> timeout = -ERESTARTSYS;

Once wait_for_common() is called with state = TASK_UNINTERRUPTIBLE, 
signal_pending_state() will 
always return false. As a result, -ERESTARTSYS is not a possible return value 
in this scenario.

Thanks,
Shenwei 

> break;
> }
> ...
> } while (!x->done && timeout); ...
> if (!x->done)
> return timeout;
> ...
> return timeout ?: 1;
> Andrew

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

[Andrew Lunn]

On Thu, Feb 19, 2026 at 02:17:26PM +, Shenwei Wang wrote:
> 
> 
> > -Original Message-
> > From: Andrew Lunn 
> > Sent: Thursday, February 19, 2026 7:27 AM
> > To: Arnaud POULIQUEN 
> > Cc: Shenwei Wang ; Linus Walleij
> > ; Bartosz Golaszewski ; Jonathan Corbet
> > ; Rob Herring ; Krzysztof Kozlowski
> > ; Conor Dooley ; Bjorn Andersson
> > ; Mathieu Poirier ; Frank 
> > Li
> > ; Sascha Hauer ; Shuah Khan
> > ; linux-g...@vger.kernel.org; linux-
> > d...@vger.kernel.org; linux-ker...@vger.kernel.org; Pengutronix Kernel Team
> > ; Fabio Estevam ; Peng Fan
> > ; devicet...@vger.kernel.org; linux-
> > remotep...@vger.kernel.org; i...@lists.linux.dev; linux-arm-
> > ker...@lists.infradead.org; dl-linux-imx ; Bartosz
> > Golaszewski 
> > Subject: [EXT] Re: [PATCH v8 3/4] gpio: rpmsg: add generic rpmsg GPIO driver
> > 
> > > > +   if (sync) {
> > > > +   err = wait_for_completion_timeout(&info->cmd_complete,
> > > > + 
> > > > msecs_to_jiffies(RPMSG_TIMEOUT));
> > > > +   if (err == 0) {
> > > > +   dev_err(&info->rpdev->dev, "rpmsg_send timeout!\n");
> > > > +   return -ETIMEDOUT;
> > >
> > > strange condition you return an error if err == 0, for redability use 
> > > 'ret'
> > > variable or simply:
> > >
> > >   if(!wait_for_completion_timeout(&info->cmd_complete,
> > > msecs_to_jiffies(RPMSG_TIMEOUT)) {
> > >   dev_err(&info->rpdev->dev, "rpmsg_send timeout!\n");
> > >   return -ETIMEDOUT;
> > >   }
> > 
> > This will be from a comment i made. It appears that
> > do_wait_for_common() can return -ERESTARTSYS. I assume that should be
> > returned to user space?
> > 
> 
> It looks like there might be a bit of confusion around what 
> wait_for_completion_timeout() 
> actually returns. That function never returns -ERESTARTSYS. Instead, its 
> behavior is pretty 
> simple:
> 
> - 0 means the wait timed out
> - A positive value means the completion happened (the value is just the 
> remaining jiffies)
> 
> So the driver returns the timeout error, and the upper application can decide 
> how it wants
> to handle that situation, for example restart or ignore.

wait_for_completion_timeout():
return wait_for_common(x, timeout, TASK_UNINTERRUPTIBLE);

wait_for_common():
return __wait_for_common(x, io_schedule_timeout, timeout, state);

__wait_for_common():
timeout = do_wait_for_common(x, action, timeout, state);
...
return timeout;

do_wait_for_common():

do {
if (signal_pending_state(state, current)) {
timeout = -ERESTARTSYS;
break;
}
... 
} while (!x->done && timeout);
...
if (!x->done)
return timeout;
...
return timeout ?: 1;

This last line is interesting, and i had to go look at the
documentation:

https://gcc.gnu.org/onlinedocs/gcc/Conditionals.html

So this is equivalent to

return timeout ? timeout : 1;

Hence, it does appear wait_for_completion_timeout() can return
-ERESTARTSYS

There is however a comment in include/linux/errno.h

/*
 * These should never be seen by user programs.  To return one of ERESTART*
 * codes, signal_pending() MUST be set.  Note that ptrace can observe these
 * at syscall exit tracing, but they will never be left for the debugged user
 * process to see.
 */
#define ERESTARTSYS 512

So we do seem to be talking about a corner case, allowing gdb(1) to
know about it, but not user space.

Andrew

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

[Andrew Lunn]

> > +   u8 id;  /* Message ID Code */
> > +   u8 vendor;  /* Vendor ID number */
> 
> Does this fields above are mandatory, seems that it is just some constant
> values that are useless.
> 
> > +   u8 version; /* Vendor-specific version number */
> 
> Why it is vendor specific? the version should represent the rpmsg-tty
> protocol version.
> 
> > +   u8 type;/* Message type */
> > +   u8 cmd; /* Command code */
> > +   u8 reserved[5];
> 
> What is the purpose of this reserved field?

They have an implementation of the other end running on there systems,
and it sounds like it is widely deployed, and they are trying to keep
backwards compatibility. The protocol also implements more than
GPIO. There is also I2C, maybe watchdog, i don't remember, but early
versions of this patchset had a list. Some of these fields are used
for some of these other devices.

I've been arguing it should be a clean design, with the protocol
focusing on GPIO. And that the rpmsg channel makes it clear this is a
GPIO device, the protocol itself does not need to include fields to
differentiate between GPIO, I2C etc.

When they start submitting I2C over rpmsg, i expect the same sort of
discussion will start again, so the likelihood of keeping backwards
compatible with there firmware seems low to me.

   Andrew

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

[Shenwei Wang]

> -Original Message-
> From: Andrew Lunn 
> Sent: Thursday, February 19, 2026 7:27 AM
> To: Arnaud POULIQUEN 
> Cc: Shenwei Wang ; Linus Walleij
> ; Bartosz Golaszewski ; Jonathan Corbet
> ; Rob Herring ; Krzysztof Kozlowski
> ; Conor Dooley ; Bjorn Andersson
> ; Mathieu Poirier ; Frank Li
> ; Sascha Hauer ; Shuah Khan
> ; linux-g...@vger.kernel.org; linux-
> d...@vger.kernel.org; linux-ker...@vger.kernel.org; Pengutronix Kernel Team
> ; Fabio Estevam ; Peng Fan
> ; devicet...@vger.kernel.org; linux-
> remotep...@vger.kernel.org; i...@lists.linux.dev; linux-arm-
> ker...@lists.infradead.org; dl-linux-imx ; Bartosz
> Golaszewski 
> Subject: [EXT] Re: [PATCH v8 3/4] gpio: rpmsg: add generic rpmsg GPIO driver
> 
> > > +   if (sync) {
> > > +   err = wait_for_completion_timeout(&info->cmd_complete,
> > > + 
> > > msecs_to_jiffies(RPMSG_TIMEOUT));
> > > +   if (err == 0) {
> > > +   dev_err(&info->rpdev->dev, "rpmsg_send timeout!\n");
> > > +   return -ETIMEDOUT;
> >
> > strange condition you return an error if err == 0, for redability use 'ret'
> > variable or simply:
> >
> >   if(!wait_for_completion_timeout(&info->cmd_complete,
> > msecs_to_jiffies(RPMSG_TIMEOUT)) {
> >   dev_err(&info->rpdev->dev, "rpmsg_send timeout!\n");
> >   return -ETIMEDOUT;
> >   }
> 
> This will be from a comment i made. It appears that
> do_wait_for_common() can return -ERESTARTSYS. I assume that should be
> returned to user space?
> 

It looks like there might be a bit of confusion around what 
wait_for_completion_timeout() 
actually returns. That function never returns -ERESTARTSYS. Instead, its 
behavior is pretty 
simple:

- 0 means the wait timed out
- A positive value means the completion happened (the value is just the 
remaining jiffies)

So the driver returns the timeout error, and the upper application can decide 
how it wants
to handle that situation, for example restart or ignore.

Thanks,
Shenwei

> Andrew

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

[Andrew Lunn]

> > +   if (sync) {
> > +   err = wait_for_completion_timeout(&info->cmd_complete,
> > + 
> > msecs_to_jiffies(RPMSG_TIMEOUT));
> > +   if (err == 0) {
> > +   dev_err(&info->rpdev->dev, "rpmsg_send timeout!\n");
> > +   return -ETIMEDOUT;
> 
> strange condition you return an error if err == 0, for redability use 'ret'
> variable or simply:
> 
>   if(!wait_for_completion_timeout(&info->cmd_complete,
> msecs_to_jiffies(RPMSG_TIMEOUT)) {
>   dev_err(&info->rpdev->dev, "rpmsg_send timeout!\n");
>   return -ETIMEDOUT;
>   }

This will be from a comment i made. It appears that
do_wait_for_common() can return -ERESTARTSYS. I assume that should be
returned to user space?

Andrew

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

[Arnaud POULIQUEN]

Hello Shenwei,

This versiob seems much more generic, thanks!

On 2/12/26 22:36, Shenwei Wang wrote:

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 
Cc: Andrew Lunn 
Signed-off-by: Shenwei Wang 
---
  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
+   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 ..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.
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define RPMSG_GPIO_ID  5
+#define RPMSG_VENDOR   1
+#define RPMSG_VERSION  0
+
+#define GPIOS_PER_PORT_DEFAULT 32
+#define RPMSG_TIMEOUT  1000
+
+/* GPIO RPMSG header type */
+#define GPIO_RPMSG_SETUP   0
+#define GPIO_RPMSG_REPLY   1
+#define GPIO_RPMSG_NOTIFY  2
+
+/* GPIO Interrupt trigger type */
+#define GPIO_RPMSG_TRI_IGNORE  0
+#define GPIO_RPMSG_TRI_RISING  1
+#define GPIO_RPMSG_TRI_FALLING 2
+#define GPIO_RPMSG_TRI_BOTH_EDGE   3
+#define GPIO_RPMSG_TRI_LOW_LEVEL   4
+#define GPIO_RPMSG_TRI_HIGH_LEVEL  5
+
+/* GPIO RPMSG commands */
+#define GPIO_RPMSG_INPUT_INIT  0
+#define GPIO_RPMSG_OUTPUT_INIT 1
+#define GPIO_RPMSG_INPUT_GET   2
+#define GPIO_RPMSG_DIRECTION_GET   3
+
+#define MAX_PORT_PER_CHANNEL10
+
+/*
+ * @rproc_name: the name of the remote proc.
+ * @channel_devices: an array of the devices related to the rpdev.
+ */
+struct rpdev_drvdata {
+   const char *rproc_name;
+   void *channel_devices[MAX_PORT_PER_CHANNEL];
+};
+
+struct gpio_rpmsg_head {


Sometime the prefix is gpio_rpmsg, sometime rpmsg_gpio or just gpio,
could you use "rpmsg_gpio" prefix in the whole driver?


+   u8 id;  /* Message ID Code */
+   u8 vendor;  /* Vendor ID number */


Does this fields above are mandatory, seems that it is just some 
constant values that are useless.



+   u8 version; /* Vendor-specific version number */


Why it is vendor specific? the version should represent the rpmsg-tty 
protocol version.



+   u8 type;/* Message type */
+   u8 cmd; /* Command code */
+   u8 reserved[5];


What is the purpose of this reserved field?


+} __packed;
+
+struct gpio_rpmsg_packet {
+   struct gpio_rpmsg_head header;
+   u8 pin_idx;
+   u8 port_idx;
+   union {
+   u8 event;
+   u8 retcode;
+   u8 value;
+   } out;
+   union {
+   u8 wakeup;
+   u8 value;
+   } in;
+} __packed __aligned(8);


Any reason to use __packed and alignement here?
This structure will be copied in a vring buffer right?


+
+struct gpio_rpmsg_pin {
+   u8 irq_shutdown;
+   u8 irq_unmask;
+   u8 irq_mask;
+   u32 irq_wake_enable;
+   u32 irq_type;
+   

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

[Shenwei Wang]

> -Original Message-
> From: Bartosz Golaszewski 
> Sent: Wednesday, February 18, 2026 4:20 AM
> To: Shenwei Wang 
> Cc: Shuah Khan ; linux-g...@vger.kernel.org; linux-
> d...@vger.kernel.org; linux-ker...@vger.kernel.org; Pengutronix Kernel Team
> ; Fabio Estevam ; Peng Fan
> ; devicet...@vger.kernel.org; linux-
> remotep...@vger.kernel.org; i...@lists.linux.dev; linux-arm-
> ker...@lists.infradead.org; dl-linux-imx ;
> arnaud.pouliq...@foss.st.com; Bartosz Golaszewski ; Andrew
> Lunn ; Linus Walleij ; Bartosz
> Golaszewski ; Jonathan Corbet ; Rob Herring
> ; Krzysztof Kozlowski ; Conor Dooley
> ; Bjorn Andersson ; Mathieu
> Poirier ; Frank Li ; Sascha 
> Hauer
> 
> Subject: [EXT] Re: [PATCH v8 3/4] gpio: rpmsg: add generic rpmsg GPIO driver
> > Cc: Bartosz Golaszewski 
> > Cc: Andrew Lunn 
> > Signed-off-by: Shenwei Wang 
> > ---
> >  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
> >
> > + 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?
> 

Yeah, the end result is the same. In this case, the comment just needs to say 
that 
of_find_node_by_name() does an internal of_node_put(), so we don’t need to 
call of_node_put() again.

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

Since the suggested change still wouldn’t remove the OF‑specific calls used 
later, I’d rather 
keep the current approach and just add the OF dependents in Kconfig.

Thanks,
Shenwei

> > + if (!of_device_is_available(child))
> > +  

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

[Bartosz Golaszewski]

On Thu, 12 Feb 2026 22:36:55 +0100, Shenwei Wang  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 
> Cc: Andrew Lunn 
> Signed-off-by: Shenwei Wang 
> ---
>  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 ..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;
> +