Hi Andrew, > -----Original Message----- > From: Andrew Lunn <[email protected]> > Sent: Thursday, November 6, 2025 1:06 PM > To: Shenwei Wang <[email protected]> > Cc: Bjorn Andersson <[email protected]>; Mathieu Poirier > <[email protected]>; Rob Herring <[email protected]>; Krzysztof > Kozlowski <[email protected]>; Conor Dooley <[email protected]>; Shawn > Guo <[email protected]>; Sascha Hauer <[email protected]>; > Jonathan Corbet <[email protected]>; Linus Walleij <[email protected]>; > Bartosz Golaszewski <[email protected]>; Pengutronix Kernel Team > <[email protected]>; Fabio Estevam <[email protected]>; Peng Fan > <[email protected]>; [email protected]; > [email protected]; [email protected]; linux-arm- > [email protected]; [email protected]; linux- > [email protected]; dl-linux-imx <[email protected]> > 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
