Hi Arnaud,
> -----Original Message-----
> From: Arnaud POULIQUEN <arnaud.pouliquen(a)foss.st.com>
> Sent: Monday, October 6, 2025 4:53 AM
> To: Shenwei Wang <shenwei.wang(a)nxp.com>; Bjorn Andersson
> <andersson(a)kernel.org>; Mathieu Poirier <mathieu.poirier(a)linaro.org>; Rob
> Herring <robh(a)kernel.org>; Krzysztof Kozlowski <krzk+dt(a)kernel.org>; Conor
> Dooley <conor+dt(a)kernel.org>; Shawn Guo <shawnguo(a)kernel.org>; Sascha
> Hauer <s.hauer(a)pengutronix.de>; Linus Walleij <linus.walleij(a)linaro.org>;
> Bartosz Golaszewski <brgl(a)bgdev.pl>
> Cc: Pengutronix Kernel Team <kernel(a)pengutronix.de>; Fabio Estevam
> <festevam(a)gmail.com>; Peng Fan <peng.fan(a)nxp.com>; linux-
> remoteproc(a)vger.kernel.org; devicetree(a)vger.kernel.org; imx(a)lists.linux.dev;
> linux-arm-kernel(a)lists.infradead.org; linux-kernel(a)vger.kernel.org; dl-linux-imx
> <linux-imx(a)nxp.com>; openamp-rp(a)lists.openampproject.org
> Subject: [EXT] Re: [PATCH v2 3/4] gpio: imx-rpmsg: add imx-rpmsg GPIO driver
> >> Then, the RPMsg device should be probed either by the remote
> >> processor using the name service announcement mechanism or if not
> >> possible by your remoteproc driver.
> >>
> > The idea is to probe the GPIO driver successfully only after the remote
> processor is online and has sent the name service announcement.
> > Until then, the GPIO driver will remain in a deferred state, ensuring that all
> consumers of the associated GPIOs are also deferred.
> > The implementation you provided below does not guarantee that the
> > related consumers will be properly deferred. This is the most important
> behavior for a GPIO/I2C controllers.
>
>
> As long as you keep the GPIO/I2C device as a child of the remote processor node,
> you should not have deferred probe issues.
> The|of_platform_populate()|function ensures
> that the I2C/GPIO devices are probed when the remote processor is started.
> Calling|devm_gpiochip_add_data|in the RPMsg driver probe should also
> prevent such issues.
>
Here, deferred probing is not an issue -it's an intentional feature. We need to ensure that all consumers of the GPIO/I2C controllers remain
in the deferred state until the remote processor is fully online.
For instance, consider a regulator node that references a GPIO line from the RPMSG GPIO controller. The regulator will stay in the deferred state
until the remote processor comes online and its services are announced and received.
Thanks,
Shenwei
> Regards,
> Arnaud
>
> >
> > Thanks,
> > Shenwei
> >
> >> To better understand my proposal you can have a look to [1]and [2].
> >> Here is another example for an rpmsg_i2c( ST downstream implementation):
> >> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit
Hi Arnaud,
> -----Original Message-----
> From: Arnaud POULIQUEN <arnaud.pouliquen(a)foss.st.com>
> Sent: Friday, October 3, 2025 2:40 AM
> To: Shenwei Wang <shenwei.wang(a)nxp.com>; Bjorn Andersson
> <andersson(a)kernel.org>; Mathieu Poirier <mathieu.poirier(a)linaro.org>; Rob
> Herring <robh(a)kernel.org>; Krzysztof Kozlowski <krzk+dt(a)kernel.org>; Conor
> Dooley <conor+dt(a)kernel.org>; Shawn Guo <shawnguo(a)kernel.org>; Sascha
> Hauer <s.hauer(a)pengutronix.de>; Linus Walleij <linus.walleij(a)linaro.org>;
> Bartosz Golaszewski <brgl(a)bgdev.pl>
> Cc: Pengutronix Kernel Team <kernel(a)pengutronix.de>; Fabio Estevam
> <festevam(a)gmail.com>; Peng Fan <peng.fan(a)nxp.com>; linux-
> remoteproc(a)vger.kernel.org; devicetree(a)vger.kernel.org; imx(a)lists.linux.dev;
> > These processors communicate via the RPMSG protocol.
> > The driver implements the standard GPIO interface, allowing the Linux
> > side to control GPIO controllers which reside in the remote processor
> > via RPMSG protocol.
>
> What about my request in previous version to make this driver generic?
>
The only platform-dependent part of this driver is the layout of the message packet, which defines the
communication protocol between the host and the remote processor. It would be challenging to require
other vendors to follow our protocol and conform to the expected behavior.
> In ST we have similar driver in downstream, we would be interested in reusing it if
> generic. Indeed we need some rpmsg mechanism for gpio-interrupt. Today we
> have a downstream rpmsg driver [1][2] that could migrate on a generic rpmsg-
> gpio driver.
>
> > +
> > +#include <linux/err.h>
> > +#include <linux/gpio/driver.h>
> > +#include <linux/rpmsg/imx_rpmsg.h>
> > +#include <linux/init.h>
> > +#include <linux/irqdomain.h>
> > +#include <linux/of.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/rpmsg.h>
> > +
> > +#define IMX_RPMSG_GPIO_PER_PORT 32
> > +#define RPMSG_TIMEOUT 1000
> > +
> > +enum gpio_input_trigger_type {
> > + GPIO_RPMSG_TRI_IGNORE,
> > + GPIO_RPMSG_TRI_RISING,
> > + GPIO_RPMSG_TRI_FALLING,
> > + GPIO_RPMSG_TRI_BOTH_EDGE,
> > + GPIO_RPMSG_TRI_LOW_LEVEL,
> > + GPIO_RPMSG_TRI_HIGH_LEVEL,
> > +};
> What about taking inspiration from the|IRQ_TYPE|bitfield defined in|irq.h|?
> For instance:
> GPIO_RPMSG_TRI_BOTH_EDGE = GPIO_RPMSG_TRI_FALLING |
> GPIO_RPMSG_TRI_RISING,
Yes, the suggestion is better.
> > +
> > +enum gpio_rpmsg_header_type {
> > + GPIO_RPMSG_SETUP,
> > + GPIO_RPMSG_REPLY,
> > + GPIO_RPMSG_NOTIFY,
> > +};
> > +
> > +enum gpio_rpmsg_header_cmd {
> > + GPIO_RPMSG_INPUT_INIT,
> > + GPIO_RPMSG_OUTPUT_INIT,
> > + GPIO_RPMSG_INPUT_GET,
> > +};
> > +
> > +struct gpio_rpmsg_data {
> > + struct imx_rpmsg_head header;
> > + u8 pin_idx;
> > + u8 port_idx;
> > + union {
> > + u8 event;
> > + u8 retcode;
> > + u8 value;
> > + } out;
> > + union {
> > + u8 wakeup;
> > + u8 value;
> nitpicking put "value" field out of union as common
I'm afraid we can't make it common, as the two 'value' fields serve different purposes-one is used for outgoing messages and
the other for incoming messages-and they are located in different parts of the packet.
> > + } in;
> > +} __packed __aligned(8);
>
> Any reason to pack it an align it?
> This structure will be copied in the RPMSg payload, right?
>
Yes. The message will then be transmitted via the MU hardware to the remote processor, so proper alignment is required.
> I also wonder if that definition should not be in a header file with double licensing
> that the DT. Indeed this need to be common with the remote side
> implementation that can be non GPL implementation.
>
> > +
> > +struct imx_rpmsg_gpio_pin {
> > + u8 irq_shutdown;
> > + u8 irq_unmask;
> > + u8 irq_mask;
> > + u32 irq_wake_enable;
> > + u32 irq_type;
> > + struct gpio_rpmsg_data msg;
> > +};
> > +
> > +struct imx_gpio_rpmsg_info {
> > + struct rpmsg_device *rpdev;
> > + struct gpio_rpmsg_data *notify_msg;
> > + struct gpio_rpmsg_data *reply_msg;
> > + struct completion cmd_complete;
> > + struct mutex lock;
> > + msg->pin_idx = gpio;
> > + msg->port_idx = port->idx;
> > +
> > + ret = gpio_send_message(port, msg, true);
> > + if (!ret)
> > + ret = !!port->gpio_pins[gpio].msg.in.value;
> Does this code is save? !! return a boolean right?
> why not force to 1 if greater that 1?
>
This approach is intended to simplify the implementation. Forcing to 1 would introduce an additional condition check.
I'm open to changing it if that's considered standard practice.
> > +
> > + return ret;
> > +}
> > +
> > +static int imx_rpmsg_gpio_direction_input(struct gpio_chip *gc,
> > + unsigned int gpio) {
> > + struct imx_rpmsg_gpio_port *port = gpiochip_get_data(gc);
> > + struct gpio_rpmsg_data *msg = NULL;
> > +
> > + guard(mutex)(&port->info.lock);
> > +
> > + msg = gpio_get_pin_msg(port, gpio);
> > + msg->header.cate = IMX_RPMSG_GPIO;
> Do you use a single rpmsg channel for several feature?
> Any reason to not use one rpmsg channel per feature category?
>
The current implementation on the remote side uses a single channel to handle all GPIO controllers.
However, this driver itself does not have that limitation.
> > + msg->header.major = IMX_RMPSG_MAJOR;
> > + msg->header.minor = IMX_RMPSG_MINOR;
> > + msg->header.type = GPIO_RPMSG_SETUP;
> > + msg->header.cmd = GPIO_RPMSG_INPUT_INIT;
> > + msg->pin_idx = gpio;
> > + msg->port_idx = port->idx;
> > +
> > + msg->out.event = GPIO_RPMSG_TRI_IGNORE;
> > + msg->in.wakeup = 0;
> > +
> > + return gpio_send_message(port, msg, true); }
> > +
> > +static inline void imx_rpmsg_gpio_direction_output_init(struct gpio_chip *gc,
> > + unsigned int gpio, int val, struct gpio_rpmsg_data *msg)
> > +
> > +static struct platform_driver imx_rpmsg_gpio_driver = {
> > + .driver = {
> > + .name = "gpio-imx-rpmsg",
> > + .of_match_table = imx_rpmsg_gpio_dt_ids,
> > + },
> > + .probe = imx_rpmsg_gpio_probe,
> > +};
> > +
> > +module_platform_driver(imx_rpmsg_gpio_driver);
> This implementation seems strange to me.
> You have a platform driver, but your RPMsg driver appears split between this
> driver and the remoteproc driver, especially regarding the
> imx_rpmsg_endpoint_probe() function.
>
See my reply below.
> From my point of view, this driver should declare both a platform_driver and an
> rpmsg_driver structures.
>
> Your imx_rpmsg_gpio_driver platform driver should be probed by your
> remoteproc platform.
> This platform driver would be responsible for:
> - Parsing the device tree node
> - Registering the RPMsg driver
>
> Then, the RPMsg device should be probed either by the remote processor using
> the name service announcement mechanism or if not possible by your
> remoteproc driver.
>
The idea is to probe the GPIO driver successfully only after the remote processor is online and has sent the name service announcement.
Until then, the GPIO driver will remain in a deferred state, ensuring that all consumers of the associated GPIOs are also deferred.
The implementation you provided below does not guarantee that the related consumers will be properly deferred. This is the most
important behavior for a GPIO/I2C controllers.
Thanks,
Shenwei
> To better understand my proposal you can have a look to [1]and [2].
> Here is another example for an rpmsg_i2c( ST downstream implementation):
> https://github.co/
> m%2FSTMicroelectronics%2Flinux%2Fblob%2Fv6.6-
> stm32mp%2Fdrivers%2Fi2c%2Fbusses%2Fi2c-
> rpmsg.c&data=05%7C02%7Cshenwei.wang%40nxp.com%7C22a9c88be60b474e
> 391008de02502ec7%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63
> 8950740622597592%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRyd
> WUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%
> 3D%7C0%7C%7C%7C&sdata=6lCk20Qhb%2F0MTw0NFtto7tj7EFYwZ%2BlOR1F3
> Qk7kQn8%3D&reserved=0
> https://github.co/
> m%2FSTMicroelectronics%2Flinux%2Fblob%2Fv6.6-
> stm32mp%2FDocumentation%2Fdevicetree%2Fbindings%2Fi2c%2Fi2c-
> rpmsg.yaml&data=05%7C02%7Cshenwei.wang%40nxp.com%7C22a9c88be60b4
> 74e391008de02502ec7%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7
> C638950740622612512%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnR
> ydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D
> %3D%7C0%7C%7C%7C&sdata=4Gva%2FpqP2u8T57XDxSDaoHhvDeJ%2Fo5HtAB
> L9TY5gbDI%3D&reserved=0
>
> Thanks and Regards,
> Arnaud
>
> > +
> > +MODULE_AUTHOR("NXP Semiconductor");
> > +MODULE_DESCRIPTION("NXP i.MX SoC rpmsg gpio driver");
> > +MODULE_LICENSE("GPL");
[Public]
Hi members of openamp-rp list,
Relaying a message from Linus W. that was submitted as a GitHub issue to the OpenAMP website. (for future reference, https://github.com/OpenAMP/open-amp is where the folks interested in RPMSG are)
===
I don't see any other way to communicate with the OpenAMP project
than to file an issue here.
There is an RPMSG-based GPIO driver submitted to the Linux kernel:
https://lore.kernel.org/linux-remoteproc/20250922200413.309707-1-shenwei.wa…
Please participate in the discussion if you are interested.
BR
Linus Walleij
===
With the message relayed, I will close the GitHub issue that is filed against the website.
Best regards,
Nathalie Chan King Choy
Open Source Program Manager
[Public]
Hi members of openamp-rp list,
Relaying a message from Linus W. that was submitted as a GitHub issue
to the OpenAMP website. (for future reference,
[1]https://github.com/OpenAMP/open-amp is where the folks interested in
RPMSG are)
===
I don't see any other way to communicate with the OpenAMP project
than to file an issue here.
There is an RPMSG-based GPIO driver submitted to the Linux kernel:
[2]https://lore.kernel.org/linux-remoteproc/20250922200413.309707-1-she
nwei.wang(a)nxp.com/
Please participate in the discussion if you are interested.
BR
Linus Walleij
===
With the message relayed, I will close the GitHub issue that is filed
against the website.
Best regards,
Nathalie Chan King Choy
Open Source Program Manager
References
1. https://github.com/OpenAMP/open-amp
2. https://lore.kernel.org/linux-remoteproc/20250922200413.309707-1-shenwei.wa…
