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

[Andrew Lunn <andrew@lunn.ch>]

> 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?

> 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.

> > 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.

What is blurry or complicated here?

       Andrew