On Wed, 15 Jul 2026 at 09:55, Shenwei Wang (OSS) <[email protected]> wrote: > > > > > -----Original Message----- > > From: Mathieu Poirier <[email protected]> > > Sent: Wednesday, July 15, 2026 10:26 AM > > To: Shenwei Wang (OSS) <[email protected]> > > Cc: 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]>; Frank Li <[email protected]>; Sascha Hauer > > <[email protected]>; Shuah Khan <[email protected]>; linux- > > [email protected]; [email protected]; > > [email protected]; > > Pengutronix Kernel Team <[email protected]>; Fabio Estevam > > <[email protected]>; Shenwei Wang <[email protected]>; Peng Fan > > <[email protected]>; [email protected]; linux- > > [email protected]; [email protected]; linux-arm- > > [email protected]; dl-linux-imx <[email protected]>; Arnaud > > POULIQUEN <[email protected]>; [email protected]; Andrew Lunn > > <[email protected]> > > Subject: Re: [PATCH v14 1/5] docs: driver-api: gpio: rpmsg gpio driver over > > rpmsg > > bus > > > > On Tue, Jul 14, 2026 at 06:43:30PM +0000, Shenwei Wang (OSS) wrote: > > > > > > > > > > -----Original Message----- > > > > From: Mathieu Poirier <[email protected]> > > > > Sent: Tuesday, July 14, 2026 11:36 AM > > > > To: Shenwei Wang (OSS) <[email protected]> > > > > Cc: 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]>; Frank Li <[email protected]>; Sascha Hauer > > > > <[email protected]>; Shuah Khan <[email protected]>; > > > > linux- [email protected]; [email protected]; > > > > [email protected]; Pengutronix Kernel Team > > > > <[email protected]>; Fabio Estevam <[email protected]>; Shenwei > > > > Wang <[email protected]>; Peng Fan <[email protected]>; > > > > [email protected]; linux- [email protected]; > > > > [email protected]; linux-arm- [email protected]; > > > > dl-linux-imx <[email protected]>; Arnaud POULIQUEN > > > > <[email protected]>; [email protected]; Andrew Lunn > > > > <[email protected]> > > > > Subject: Re: [PATCH v14 1/5] docs: driver-api: gpio: rpmsg gpio > > > > driver over rpmsg bus > > > > > > > > On Tue, Jul 14, 2026 at 02:29:12PM +0000, Shenwei Wang (OSS) wrote: > > > d > > > > > > > + - 3: Both edge trigger > > > > > > > + - 4: High level trigger > > > > > > > + - 8: Low level trigger > > > > > > > > > > > > I'm in agreement with cmd 3, 4, 5 and 6. > > > > > > > > > > > > > > > > How about cmd 2? > > > > > > > > Yes, I forgot cmd 2. > > > > > > > > > > > > > > > > + > > > > > > > +**Reply:** > > > > > > > + > > > > > > > +.. code-block:: none > > > > > > > + > > > > > > > + +------+--------+--------+ > > > > > > > + | 0x00 | 0x01 | 0x02 | > > > > > > > + | 1 | status | 0 | > > > > > > > + +------+--------+--------+ > > > > > > > + > > > > > > > +- **status**: > > > > > > > + > > > > > > > + - 0: Ok > > > > > > > + - 1: Error > > > > > > > + > > > > > > > +SET_WAKEUP (Cmd=16) > > > > > > > +~~~~~~~~~~~~~~~~~~~ > > > > > > > > > > > > I already commented on messages that are not part of the > > > > > > virtio-gpio specification. > > > > > > > > > > > > > > > > I don't believe virtio-gpio needs a SET_WAKEUP command. > > > > > > > > > > In the RPMSG case, SET_WAKEUP is required because it performs an > > > > > operation on the remote processor, so the remote side must be > > > > > explicitly > > > > notified. > > > > > > > > What operations would that be? > > > > > > > > > > The operation is enabling the line as a wakeup source. In practice, > > > this involves configuring hardware-specific wakeup settings (e.g., > > > register bits) on the remote processor, so the remote side must be > > > explicitly > > notified through SET_WAKEUP. > > > > > > > As I said before, we are not enhancing the virtio-gpio specification. If > > the > > functionality is not part of the specification, it is not part of this > > protocol. > > > > It's about the functionality, not the specification. >
No, it's about the specification. > Without SET_WAKEUP, a virtio-GPIO line can still be used as a wakeup source, > but > an RPMSG implementation cannot because the remote processor is never notified > to configure its wakeup settings. > > So both implementations may follow the same virtio-gpio specification, but > they do > not provide the same wakeup functionality. > No. The kernel treats rpmsg-gpio the same way as virtio-gpio - they need to provide exactly the same functionality. > Anyway, if you want to drop the function, the command can be removed. > > Shenwei > > > > > > > > > > > > > > For virtio-gpio, however, everything is handled locally. The other > > > > > side does not need to perform any action, so sending a SET_WAKEUP > > > > > command > > > > appears unnecessary. > > > > > > > > What is "everything"? > > > > > > > > > > I mean masking, unmasking, and marking the interrupt as a wakeup > > > source. All of these operations are handled on local processor. > > > > There may be enough capability in your remote processor to do all that, but > > we > > can't assume the same applies to all implementations. rpmsg-gpio is not > > about > > inventing a new protocol or delegating actions to the remote processor. It > > is > > about carrying virtio-gpio packets on top of RPMSG. It needs to operate > > _exactly_ the same way as virtio-gpio. > > > > > > > > Shenwei > > > > > > > > > > > > > > > > > > > > + > > > > > > > +**Request:** > > > > > > > + > > > > > > > +.. code-block:: none > > > > > > > + > > > > > > > + +------+------+------+------+------+------+------+------+ > > > > > > > + | 0x00 | 0x01 | 0x02 | 0x03 | 0x04 | 0x05 | 0x06 | 0x07 | > > > > > > > + | 1 | line | value | > > > > > > > + +------+------+------+------+------+------+------+------+ > > > > > > > + > > > > > > > +- **value**: Wakeup enable. > > > > > > > + > > > > > > > + The remote system should always aim to stay in a > > > > > > > + power-efficient state by shutting down or clock-gating the > > > > > > > + GPIO blocks that aren't in use. Since the remoteproc driver > > > > > > > + is responsible for managing the power states of the remote > > > > > > > + firmware, the GPIO driver does not require to know the > > > > > > > + firmware's > > > > running states. > > > > > > > + > > > > > > > + When the wakeup bit is set, the remote firmware should > > > > > > > + configure the line as a wakeup source. The firmware should > > > > > > > + send the notification message to Linux after it is woken from > > > > > > > the GPIO > > line. > > > > > > > + > > > > > > > + - 0: Disable wakeup from GPIO > > > > > > > + - 1: Enable wakeup from GPIO > > > > > > > + > > > > > > > +**Reply:** > > > > > > > + > > > > > > > +.. code-block:: none > > > > > > > + > > > > > > > + +------+--------+--------+ > > > > > > > + | 0x00 | 0x01 | 0x02 | > > > > > > > + | 1 | status | 0 | > > > > > > > + +------+--------+--------+ > > > > > > > + > > > > > > > +- **status**: > > > > > > > + > > > > > > > + - 0: Ok > > > > > > > + - 1: Error > > > > > > > + > > > > > > > +Notification Message > > > > > > > +-------------------- > > > > > > > > > > > > "Interrupt Messages" > > > > > > > > > > > > > + > > > > > > > +Notifications are sent by the remote core and they have > > > > > > > +**Type=2 (GPIO_RPMSG_NOTIFY)**: > > > > > > > + > > > > > > > +When a GPIO line asserts an interrupt on the remote > > > > > > > +processor, the firmware should immediately mask the > > > > > > > +corresponding interrupt source and send a notification > > > > > > > +message to the Linux. Upon completion of the interrupt > > > > > > > +handling on the Linux side, the driver should issue a command > > > > > > > +**SET_IRQ_TYPE** to the firmware to > > > > unmask the interrupt. > > > > > > > + > > > > > > > +A Notification message can arrive between a SEND and its > > > > > > > +REPLY message, and the driver is expected to handle this > > > > > > > scenario. > > > > > > > + > > > > > > > +.. code-block:: none > > > > > > > + > > > > > > > + +------+------+--------+ > > > > > > > + | 0x00 | 0x01 | 0x02 | > > > > > > > + | 2 | line | trigger| > > > > > > > + +------+------+--------+ > > > > > > > > > > > > 2 things here: > > > > > > > > > > > > 1) You did not include messages that mask and unmask interrupts > > > > > > at the driver side. > > > > > > > > > > Interrupt masking and unmasking are handled entirely on the local > > processor. > > > > > > > > > > When an interrupt occurs, the remote system masks the interrupt > > > > > and then sends a notification to Linux. After Linux processes the > > > > > notification, it sends a SET_IRQ_TYPE message back to the remote > > > > > system, > > > > which then unmasks the interrupt. > > > > > > > > This is the kind of information that should be part of this > > > > documentation. > > > > > > > > > > > > > > Thanks, > > > > > Shenwei > > > > > > > > > > > > > > > > > 2) We are carrying virtio-gpio messages on top of RPMSG and as > > > > > > such, this whole protocol should be about thar: > > > > > > > > > > > > +------+------+--------+-------- > > > > > > | 0x00 | payload | > > > > > > | Q | | > > > > > > +------+------+--------+-------- > > > > > > > > > > > > Q = 0 requestq > > > > > > Q = 1 eventq > > > > > > > > > > > > The "payload" part is simply the format of the messages as found > > > > > > in the virtio- gpio specification. From there, the only thing > > > > > > left to mention is which messages are not supported, i.e get line > > > > > > names. > > > > > > > > > > > > > + > > > > > > > +- **line**: The GPIO line (pin) index of the port. > > > > > > > + > > > > > > > +- **trigger**: Optional parameter to indicate the trigger event > > > > > > > type. > > > > > > > > > > > > Not part of the spec - remove. > > > > > > > > > > > > Given the refactoring work that is still needed, I will not look > > > > > > at the implementation. > > > > > > > > > > > > Thanks, > > > > > > Mathieu > > > > > > > > > > > > > + > > > > > > > diff --git a/Documentation/driver-api/gpio/index.rst > > > > > > > b/Documentation/driver-api/gpio/index.rst > > > > > > > index bee58f709b9a..e5eb1f82f01f 100644 > > > > > > > --- a/Documentation/driver-api/gpio/index.rst > > > > > > > +++ b/Documentation/driver-api/gpio/index.rst > > > > > > > @@ -16,6 +16,7 @@ Contents: > > > > > > > drivers-on-gpio > > > > > > > bt8xxgpio > > > > > > > pca953x > > > > > > > + gpio-rpmsg > > > > > > > > > > > > > > Core > > > > > > > ==== > > > > > > > -- > > > > > > > 2.43.0 > > > > > > >
