Message-ID:
 <PAXPR04MB9185C4A4B91F863CFD49718E89C2A@PAXPR04MB9185.eurprd04.prod.outlook.com>
Date: Thu, 6 Nov 2025 20:40:05 +0000
From: Shenwei Wang <shenwei.wang@....com>
To: Andrew Lunn <andrew@...n.ch>
CC: Bjorn Andersson <andersson@...nel.org>, Mathieu Poirier
	<mathieu.poirier@...aro.org>, Rob Herring <robh@...nel.org>, Krzysztof
 Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, Shawn Guo
	<shawnguo@...nel.org>, Sascha Hauer <s.hauer@...gutronix.de>, Jonathan Corbet
	<corbet@....net>, Linus Walleij <linus.walleij@...aro.org>, Bartosz
 Golaszewski <brgl@...ev.pl>, Pengutronix Kernel Team <kernel@...gutronix.de>,
	Fabio Estevam <festevam@...il.com>, Peng Fan <peng.fan@....com>,
	"linux-remoteproc@...r.kernel.org" <linux-remoteproc@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"imx@...ts.linux.dev" <imx@...ts.linux.dev>,
	"linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "linux-doc@...r.kernel.org"
	<linux-doc@...r.kernel.org>, dl-linux-imx <linux-imx@....com>
Subject: Re: [PATCH v5 3/5] docs: staging: gpio-rpmsg: gpio over rpmsg bus

Hi Andrew,

> -----Original Message-----
> From: Andrew Lunn <andrew@...n.ch>
> Sent: Thursday, November 6, 2025 1:06 PM
> To: Shenwei Wang <shenwei.wang@....com>
> Cc: Bjorn Andersson <andersson@...nel.org>; Mathieu Poirier
> <mathieu.poirier@...aro.org>; Rob Herring <robh@...nel.org>; Krzysztof
> Kozlowski <krzk+dt@...nel.org>; Conor Dooley <conor+dt@...nel.org>; Shawn
> Guo <shawnguo@...nel.org>; Sascha Hauer <s.hauer@...gutronix.de>;
> Jonathan Corbet <corbet@....net>; Linus Walleij <linus.walleij@...aro.org>;
> Bartosz Golaszewski <brgl@...ev.pl>; Pengutronix Kernel Team
> <kernel@...gutronix.de>; Fabio Estevam <festevam@...il.com>; Peng Fan
> <peng.fan@....com>; linux-remoteproc@...r.kernel.org;
> devicetree@...r.kernel.org; imx@...ts.linux.dev; linux-arm-
> kernel@...ts.infradead.org; linux-kernel@...r.kernel.org; linux-
> doc@...r.kernel.org; dl-linux-imx <linux-imx@....com>
> Subject: [EXT] Re: [PATCH v5 3/5] docs: staging: gpio-rpmsg: gpio over rpmsg bus
> > +   +-----+------+------+-----+-----+------------+-----+-----+-----+----+
> > +   |0x00 |0x01  |0x02  |0x03 |0x04 |0x05..0x09  |0x0A |0x0B |0x0C |0x0D|
> > +   |cate |major |minor |type |cmd  |reserved[5] |line |port |  data    |
> > +
> > + +-----+------+------+-----+-----+------------+-----+-----+-----+----
> > + +
> > +
> > +- **Cate (Category field)**: Indicates the category of the message, such as
> GPIO, I2C, PMIC, AUDIO, etc.
> 
> We know it is a GPIO message, this document is titled "GPIO RPMSG Protocol". So
> i don't see the need for cate. I can however understand that your device does
> support multiple functions, but to make this generic, it would be better if each
> function had its own channel.
> 

These details are defined in the message header to support multiple functions. For 
the GPIO-specific implementation, the header values are fixed and do not require 
modification, provided the transport protocol version remains unchanged.

Then I should remove those unrelated definitions from this document. Is my understanding
correct?

> > +
> > +  Defined categories:
> > +
> > +  - 1: RPMSG_LIFECYCLE
> > +  - 2: RPMSG_PMIC
> > +  - 3: RPMSG_AUDIO
> > +  - 4: RPMSG_KEY
> > +  - 5: RPMSG_GPIO
> > +  - 6: RPMSG_RTC
> > +  - 7: RPMSG_SENSOR
> > +  - 8: RPMSG_AUTO
> > +  - 9: RPMSG_CATEGORY
> > +  - A: RPMSG_PWM
> > +  - B: RPMSG_UART
> > +
> > +- **Major**: Major version number.
> > +
> > +- **Minor**: Minor version number.
> 
> What is the purpose of Major and Minor? What values are valid. What should
> happen if an invalid value is passed?
> 
> What you should think about is, if you gave this specification to somebody else,
> could they implement it?
> 

Okay. Will change those fields to fixed number and remove the above definitions.

> > +
> > +- **Type (Message Type)**: For the GPIO category, can be one of:
> > +
> > +  - 0: GPIO_RPMSG_SETUP
> > +  - 1: GPIO_RPMSG_REPLY
> > +  - 2: GPIO_RPMSG_NOTIFY
> 
> Is _SETUP always from Linux to the firmware? Is a setup always followed by a
> _REPLY? Do you need to wait for the _REPLY before sending the next _SETUP? If
> there is no _REPLY within X seconds, should Linux retry? Can an _NOTIFY arrive
> between a _SETUP and a _REPLY?
> 

Yes, the SETUP message is always sent from Linux to the remote firmware. Each SETUP 
corresponds to a single REPLY message. The GPIO driver serializes messages and determines 
whether a REPLY is required. If a REPLY is expected but not received within the timeout 
period (currently 1 second in the driver), the driver returns -ETIMEOUT.

A NOTIFY message can arrive between a SETUP and the REPLY, and the driver is designed to handle this scenario.

> > +
> > +- **Cmd**: Command code, used for GPIO_RPMSG_SETUP messages.
> > +
> > +- **reserved[5]**: Reserved bytes.
> 
> Why are these in the middle. It is more normal to have reserved bytes at the end.
> 

The reserved[5] bytes may be used for other type of functions. For GPIO, all should be 0.

> You should also specify these bytes should be set to 0. If you don't it will be hard
> to use them in the future, because they will contain 42, or some other random
> values.
> 
> Is there a relationship between major:minor and reserved?
> 

No so far. Maybe it could be related if the protocol is updated in the future.

> > +
> > +- **line**: The GPIO line index.
> > +
> > +- **port**: The GPIO controller index.
> 
> data?
> 

Data is explained in the commands below.

> > +GPIO Commands
> > +-------------
> 
> This is a GPIO specification, so i would only expect GPIO commands...
> 
> > +
> > +Commands are specified in the **Cmd** field for **GPIO_RPMSG_SETUP**
> (Type=0) messages.
> > +
> > +GPIO_RPMSG_INPUT_INIT (Cmd=0)
> > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > +
> > +**Request:**
> > +
> > +.. code-block:: none
> > +
> > +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
> > +   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05..0x09 |0x0A |0x0B |0x0C |0x0D|
> > +   | 5   | 1   | 0   | 0   | 0   |  0        |line |port | val | wk |
> > +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
> > +
> > +- **val**: Interrupt trigger type.
> > +
> > +  - 0: Interrupt disabled
> > +  - 1: Rising edge trigger
> > +  - 2: Falling edge trigger
> > +  - 3: Both edge trigger
> > +  - 4: Low level trigger
> > +  - 5: High level trigger
> > +
> > +- **wk**: Wakeup enable.
> > +
> > +  - 0: Disable wakeup from GPIO
> > +  - 1: Enable wakeup from GPIO
> 
> What do you mean by wakeup?
> 

The gpio line can be enabled as an wakeup source for the system.

> > +
> > +**Reply:**
> > +
> > +.. code-block:: none
> > +
> > +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
> > +   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05..0x09 |0x0A |0x0B |0x0C |0x0D|
> > +   | 5   | 1   | 0   | 1   | 1   |  0        |line |port | err | 0  |
> > +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
> > +
> > +- **err**: Error code from the remote core.
> > +
> > +  - 0: Success
> > +  - 1: General error (early remote software only returns this
> > + unclassified error)
> > +  - 2: Not supported
> > +  - 3: Resource not available
> > +  - 4: Resource busy
> > +  - 5: Parameter error
> 
> It would be good to give some examples of when these should be used.
> 
> Say the hardware does not support both edges. Is that 2? Why would a resource
> be busy? How is busy different to not available?
> 
> > +
> > +GPIO_RPMSG_OUTPUT_INIT (Cmd=1)
> > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > +
> > +**Request:**
> > +
> > +.. code-block:: none
> > +
> > +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
> > +   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05..0x09 |0x0A |0x0B |0x0C |0x0D|
> > +   | 5   | 1   | 0   | 0   | 1   |  0        |line |port | val | 0  |
> > +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
> > +
> > +- **val**: Output level.
> > +
> > +  - 0: Low
> > +  - 1: High
> 
> Maybe make a comment about the order. Some GPIO controllers suffer from
> glitches when you swap them from input to output. While it is an input, you first
> need to set the output value, and then configure the pin for output.
> 

These are platform-dependent details that should be managed by the remote firmware. 
In the Linux driver, we simply request the remote side to configure the GPIO direction.

> > +Notification Message
> > +--------------------
> > +
> > +Notifications are sent with **Type=2 (GPIO_RPMSG_NOTIFY)**:
> > +
> > +.. code-block:: none
> > +
> > +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
> > +   |0x00 |0x01 |0x02 |0x03 |0x04 |0x05..0x09 |0x0A |0x0B |0x0C |0x0D|
> > +   | 5   | 1   | 0   | 2   | 0   |  0        |line |port | 0   | 0  |
> > +   +-----+-----+-----+-----+-----+-----------+-----+-----+-----+----+
> > +
> > +- **line**: The GPIO line index.
> > +- **port**: The GPIO controller index.
> 
> There is no need to acknowledge the notification? How do level interrupts work?
> 

Currently, there is no need to acknowledge the message, as the interrupt is managed entirely 
by the remote firmware. On the Linux side, a single notification message is received when an 
interrupt is triggered.

Thanks,
Shenwei

>         Andrew

Powered by blists - more mailing lists