RE: [PATCH v14 1/5] docs: driver-api: gpio: rpmsg gpio driver over rpmsg bus
"Shenwei Wang (OSS)" <[email protected]> Tue, 14 Jul 2026 18:43:30 +0000
| Newsgroups | org.kernel.vger.linux-remoteproc,dev.linux.lists.imx,org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-devicetree,org.kernel.vger.linux-doc,org.kernel.vger.linux-gpio,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <PAXPR04MB9185B1A3F16D82E8C0C76EE689F92@PAXPR04MB9185.eurprd04.prod.outlook.com> |
> -----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. > > > > 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. 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 > > > >