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

[Shenwei Wang]

> -----Original Message-----
> From: Andrew Lunn <andrew at lunn.ch>
> Sent: Monday, April 27, 2026 3:28 PM
> To: Shenwei Wang <shenwei.wang at nxp.com>
> Cc: Padhi, Beleswar <b-padhi at ti.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 v13 3/4] gpio: rpmsg: add generic rpmsg GPIO driver
> > > > +struct rpmsg_gpio_packet {
> > > > +     u8 type;        /* Message type */
> > > > +     u8 cmd;         /* Command code */
> > > > +     u8 port_idx;
> > > > +     u8 line;
> > > > +     u8 val1;
> > > > +     u8 val2;
> > > > +};
> > >
> > >
> > > Could you please document the fields in these structs (and the below ones
> too)?
> > > From the code, it looks like while sending a message from Linux to
> > > Firmware; val1 and val2 are used to describe the values to set.
> > > Whereas while receiving a response, val1 represents a possible error
> > > code, and val2 represents the actual message of get type queries. If
> > > that is so, you might want to change the variable names to be more
> descriptive and also use a union.
> > >
> >
> > The fields in the two structs are fairly self-explanatory. Do we really need the
> additional comments?
> > The previous version of the patch used a union, which was updated to support
> the fixed_up hooks.
> > Now that the fixed_up hooks have been removed, I can revert this back to the
> union-based implementation.
> 
> I thought you had already adopted the virtio message format?
> 
> 
> /* Possible values of the status field */
> #define VIRTIO_GPIO_STATUS_OK                   0x0
> #define VIRTIO_GPIO_STATUS_ERR                  0x1
> 
> struct virtio_gpio_response {
>         __u8 status;
>         __u8 value;
> };
> 
> Seems pretty obvious what status means. value depends on the request,
> get_direction actually uses it, and it can be one of
> 
> #define VIRTIO_GPIO_DIRECTION_NONE              0x00
> #define VIRTIO_GPIO_DIRECTION_OUT               0x01
> #define VIRTIO_GPIO_DIRECTION_IN                0x02
> 
> and gpio_get uses it as a bool for the state of the GPIO.
> 
> Why do we need all the complexity for val1, val2, etc?
> 

It is the same message format. Please see the message definition (GET_DIRECTION) below:

+GET_DIRECTION (Cmd=2)
+~~~~~~~~~~~~~~~~~~~~~
+
+**Request:**
+
+.. code-block:: none
+
+   +-----+-----+-----+-----+-----+----+
+   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05|
+   | 0   | 2   |port |line | 0   | 0  |
+   +-----+-----+-----+-----+-----+----+
+
+**Reply:**
+
+.. code-block:: none
+
+   +-----+-----+-----+-----+-----+----+
+   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05|
+   | 1   | 2   |port |line | err | dir|
+   +-----+-----+-----+-----+-----+----+
+
+- **err**: See above for definitions.
+
+- **dir**: Direction.
+
+  - 0: None
+  - 1: Output
+  - 2: Input
+

Shenwei

>   Andrew