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

[Shenwei Wang]

> -----Original Message-----
> From: Andrew Lunn <andrew at lunn.ch>
> Sent: Tuesday, February 24, 2026 12:18 PM
> To: Shenwei Wang <shenwei.wang at nxp.com>
> Cc: Arnaud POULIQUEN <arnaud.pouliquen at foss.st.com>; Linus Walleij
> <linusw at kernel.org>; Bartosz Golaszewski <brgl at kernel.org>; Jonathan Corbet
> <corbet at lwn.net>; Rob Herring <robh at kernel.org>; Krzysztof Kozlowski
> <krzk+dt at kernel.org>; Conor Dooley <conor+dt at kernel.org>; Bjorn Andersson
> <andersson at kernel.org>; Mathieu Poirier <mathieu.poirier at linaro.org>; Frank Li
> <frank.li at nxp.com>; Sascha Hauer <s.hauer at pengutronix.de>; Shuah Khan
> <skhan at linuxfoundation.org>; linux-gpio at vger.kernel.org; linux-
> doc at vger.kernel.org; linux-kernel at vger.kernel.org; Pengutronix Kernel Team
> <kernel at pengutronix.de>; Fabio Estevam <festevam at gmail.com>; Peng Fan
> <peng.fan at nxp.com>; devicetree at vger.kernel.org; linux-
> remoteproc at vger.kernel.org; imx at lists.linux.dev; linux-arm-
> kernel at lists.infradead.org; dl-linux-imx <linux-imx at nxp.com>; Bartosz
> Golaszewski <brgl at bgdev.pl>
> Subject: [EXT] Re: [PATCH v8 3/4] gpio: rpmsg: add generic rpmsg GPIO driver
> 
> Caution: This is an external email. Please take care when clicking links or opening
> attachments. When in doubt, report the message using the 'Report this email'
> button
> 
> 
> > I don’t think we can remove port_idx if the protocol is expected to
> > support multiple instances. If you only ever support a single
> > instance, then sure, it becomes redundant—but imposing a single‑instance
> limitation on a generic protocol doesn’t seem appropriate.
> 
> The DT binding example is:
> 
> +      rpmsg {
> +        rpmsg-io-channel {
> +          #address-cells = <1>;
> +          #size-cells = <0>;
> +
> +          gpio at 0 {
> +            compatible = "rpmsg-gpio";
> +            reg = <0>;
> +            gpio-controller;
> +            #gpio-cells = <2>;
> +            #interrupt-cells = <2>;
> +            interrupt-controller;
> +          };
> +
> +          gpio at 1 {
> +            compatible = "rpmsg-gpio";
> +            reg = <1>;
> +            gpio-controller;
> +            #gpio-cells = <2>;
> +            #interrupt-cells = <2>;
> +            interrupt-controller;
> +          };
> +        };
> +      };
> 
> Doesn't this instantiates the driver twice, once on channel 0 and once on channel
> 1?
> 
> How does port_idx fit into this?

I think you were assuming there is only one remoteproc in the system?
In practice, the setup can look more like this:

+ remote_cm33{
+     rpmsg {
+        rpmsg-io-channel {
+          #address-cells = <1>;
+          #size-cells = <0>;
+
+          gpio at 0 {
+            compatible = "rpmsg-gpio";
+            reg = <0>;
+            gpio-controller;
+            #gpio-cells = <2>;
+            #interrupt-cells = <2>;
+            interrupt-controller;
+          };
+          gpio at 1 {
+            compatible = "rpmsg-gpio";
+            reg = <1>;
+            gpio-controller;
+            #gpio-cells = <2>;
+            #interrupt-cells = <2>;
+            interrupt-controller;
+          };
+  ...
+        };
+      };
+};
+
+ remote_dsp {
+     rpmsg {
+        rpmsg-io-channel {
+          #address-cells = <1>;
+          #size-cells = <0>;
+
+          gpio at 0 {
+            compatible = "rpmsg-gpio";
+            reg = <0>;
+            gpio-controller;
+            #gpio-cells = <2>;
+            #interrupt-cells = <2>;
+            interrupt-controller;
+          };
+  ...
+        };
+      };
+};

> 
> > Regarding type, it’s needed, especially for the in packets. There are
> > two distinct kinds of incoming
> > packets: notification‑in and reply‑in. Because of that differences,
> > Combining cmd and type would blur that distinction and complicate the
> implementation.
> 
> > > #define GPIO_RPMSG_CMD_DIR_INPUT        1
> > > #define GPIO_RPMSG_CMD_DIR_OUTPUT       2
> > > #define GPIO_RPMSG_CMD_GET_DIR          3
> > > #define GPIO_RPMSG_CMD_GET              4
> > > #define GPIO_RPMSG_CMD_SET              5
> 
> These are all simple RPCs. You make a request, your get a reply, using the same
> CMD. I don't see how you can make any mixup here.
> 

Message type and cmd are not the same thing.
Keeping them separate allows the same packet format to be used for both IN and OUT 
messages. The type field identifies whether a packet is a request, a reply, or a notification, 
while cmd identifies the actual operation. No benefits to combine them.

> > > These map onto the gpio_chip ops. And i leave space for the
> > > _multiple ops if they are needed in the future.
> > >
> > > #define GPIO_PRMSG_CMD_INTR_CONFIG      32
> > > #define GPIO_PRMSG_CMD_INTR_EVENT       33
> 
> GPIO_PRMSG_CMD_INTR_CONFIG is again just a plain RPC, request of "please
> configure the interrupt handling like this", replay of "yes, done".
> 
> GPIO_PRMSG_CMD_INTR_EVENT is more interesting. The other end can
> spontaneously send this, indicating an interrupt. Once Linux has handled the
> interrupt, especially level interrupts, it needs to be acknowledged. So it sends
> back an GPIO_PRMSG_CMD_INTR_EVENT. It could be considered an RPC in the
> opposite direction, but i think that would be wrong. I expect there are multiple
> overlapping GPIO_PRMSG_CMD_INTR_EVENT flying around, so you cannot
> enforce a strict RPC style communication.
> 

In v8 gpio-rpmsg.rst, we already added the following cmd:
+GPIO_RPMSG_NOTIFY_REPLY (Cmd=4)
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+The reply message for the notification is optional. The remote firmware can
+implement it to simulate the interrupt acknowledgment behavior.
+
+**Request:**
+
+.. code-block:: none
+
+   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
+   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05..0x09 |0x0A |0x0B |0x0C |0x0D|
+   | 5   | 1   | 0   | 0   | 4   |  0        |line |port |level| 0  |
+   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
+
+- **line**: The GPIO line(pin) index of the port.
+- **port**: The GPIO port(bank) index.
+

At this point, I don’t see a need to introduce the two commands you proposed. If they 
become useful later, we can add them then.

Thanks,
Shenwei

> What is blurry or complicated here?
> 
>        Andrew