> -----Original Message-----
> From: Andrew Lunn <[email protected]>
> Sent: Tuesday, February 24, 2026 12:18 PM
> To: Shenwei Wang <[email protected]>
> Cc: Arnaud POULIQUEN <[email protected]>; Linus Walleij
> <[email protected]>; Bartosz Golaszewski <[email protected]>; Jonathan Corbet
> <[email protected]>; Rob Herring <[email protected]>; Krzysztof Kozlowski
> <[email protected]>; Conor Dooley <[email protected]>; Bjorn Andersson
> <[email protected]>; Mathieu Poirier <[email protected]>; Frank Li
> <[email protected]>; Sascha Hauer <[email protected]>; Shuah Khan
> <[email protected]>; [email protected]; linux-
> [email protected]; [email protected]; Pengutronix Kernel Team
> <[email protected]>; Fabio Estevam <[email protected]>; Peng Fan
> <[email protected]>; [email protected]; linux-
> [email protected]; [email protected]; linux-arm-
> [email protected]; dl-linux-imx <[email protected]>; Bartosz
> Golaszewski <[email protected]>
> 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@0 {
> + compatible = "rpmsg-gpio";
> + reg = <0>;
> + gpio-controller;
> + #gpio-cells = <2>;
> + #interrupt-cells = <2>;
> + interrupt-controller;
> + };
> +
> + gpio@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@0 {
+ compatible = "rpmsg-gpio";
+ reg = <0>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ #interrupt-cells = <2>;
+ interrupt-controller;
+ };
+ gpio@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@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