> -----Original Message-----
> From: Arnaud POULIQUEN <[email protected]>
> Sent: Thursday, December 18, 2025 5:04 AM
> To: Shenwei Wang <[email protected]>; Linus Walleij
> <[email protected]>; Bartosz Golaszewski <[email protected]>; Rob Herring
> <[email protected]>; Krzysztof Kozlowski <[email protected]>; Conor Dooley
> <[email protected]>; Bjorn Andersson <[email protected]>; Mathieu
> Poirier <[email protected]>; Shawn Guo <[email protected]>;
> Sascha Hauer <[email protected]>; Jonathan Corbet <[email protected]>
> Cc: Pengutronix Kernel Team <[email protected]>; Fabio Estevam
> <[email protected]>; Peng Fan <[email protected]>; linux-
> [email protected]; [email protected]; linux-
> [email protected]; [email protected];
> [email protected];
> [email protected]; [email protected]; dl-linux-imx
> <[email protected]>
> Subject: [EXT] Re: [PATCH v6 2/5] remoteproc: imx_rproc: Populate devices
> under "rpmsg" subnode
>
> > + rp_driver->rpdrv.drv.name = name;
> > + rp_driver->rpdrv.id_table = rpdev_id;
> > + rp_driver->rpdrv.probe = imx_rpmsg_endpoint_probe;
> > + rp_driver->rpdrv.remove = imx_rpmsg_endpoint_remove;
> > + rp_driver->rpdrv.callback = imx_rpmsg_endpoint_cb;
> > + rp_driver->driver_data = driver_data;
> > + rp_driver->compat = compat;
> > +
> > + register_rpmsg_driver(&rp_driver->rpdrv);
>
>
> I still believe that creating a dependency between remoteproc and rpmsg is not
> suitable.
>
> Please correct me if I’m wrong, but since you define gpio@0 and gpio@1 in your
> device tree, you call register_rpmsg_driver() twice, creating two instances
> of the
> same driver. To differentiate both, you fill the rpdrv.id_table with the node
> names "gpio@0" and "gpio@1".
>
Nope. The function of register_rpmsg_driver is invoked only once per channel.
> In a topology with two remote processors, each needing rpmsg-gpio, the same
> situation would not work because you would have two "gpio@0" entries.
>
No, it’s working. The gpio-rpmsg driver is designed to support multiple
instances. In fact, that’s the reason
we added the "rpmsg" bus node under the rproc node.
Please note that you cannot use duplicate labels within the same channel.
> What about using the ns-announcement mechanism on the remote side to
> address GPIO port/bank instances?
>
> In the rpmsg-gpio driver, you could define:
>
> static struct rpmsg_device_id rpmsg_gpio_id_table[] = {
> { .name = "rpmsg-gpio" },
> { .name = "rpmsg_gpio-0" },
> { .name = "rpmsg_gpio-1" },
> { .name = "rpmsg_gpio-2" },
> { .name = "rpmsg_gpio-3" },
> { },
> };
>
These definitions are redundant.
The current implementation already supports multiple instances without
requiring this
information to be hardcoded into the driver source.
Regards,
Shenwei
> Then the match between the device tree and the rpmsg bus could be done in the
> rpmsg-gpio driver by matching the rpmsg name with the compatible property plus
> the reg value.
>
> Example device tree snippet:
>
> cm33: remoteproc-cm33 {
> compatible = "fsl,imx8ulp-cm33";
>
> rpmsg {
> rpmsg-io-channel {
> gpio@0 {
> compatible = "rpmsg_gpio";
> reg = <0>;
> };
>
> gpio@1 {
> compatible = "rpmsg_gpio";
> reg = <1>;
> };
>
> ...
> };
>
> ...
> };
> };
>
> With this approach, rpmsg management could be handled entirely within the
> rpmsg-gpio driver, avoiding the need to register multiple instances of the
> rpmsg_gpio driver.
>
> Regards,
> Arnaud
>
> > +
> > + return 0;
> > +}
> > +
> > +static int rproc_of_rpmsg_node_init(struct platform_device *pdev) {
> > + struct device *dev = &pdev->dev;
> > + const char *compat;
> > + int ret;
> > +
> > + struct device_node *np __free(device_node) = of_get_child_by_name(dev-
> >of_node, "rpmsg");
> > + if (!np)
> > + return 0;
> > +
> > + for_each_child_of_node_scoped(np, child) {
> > + compat = imx_of_rpmsg_is_in_map(child->name);
> > + if (!compat)
> > + ret = of_platform_default_populate(child, NULL, dev);
> > + else
> > + ret = imx_of_rpmsg_register_rpdriver(child, dev,
> > + child->name, compat);
> > +
> > + if (ret < 0)
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > static int imx_rproc_probe(struct platform_device *pdev)
> > {
> > struct device *dev = &pdev->dev; @@ -1114,6 +1253,10 @@ static
> > int imx_rproc_probe(struct platform_device *pdev)
> > goto err_put_pm;
> > }
> >
> > + ret = rproc_of_rpmsg_node_init(pdev);
> > + if (ret < 0)
> > + dev_info(dev, "populating 'rpmsg' node failed\n");
> > +
> > return 0;
> >
> > err_put_pm:
> > diff --git a/include/linux/rpmsg/rpdev_info.h
> > b/include/linux/rpmsg/rpdev_info.h
> > new file mode 100644
> > index 000000000000..13e020cd028b
> > --- /dev/null
> > +++ b/include/linux/rpmsg/rpdev_info.h
> > @@ -0,0 +1,33 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/* Copyright 2025 NXP */
> > +
> > +/*
> > + * @file linux/rpdev_info.h
> > + *
> > + * @brief Global header file for RPDEV Info
> > + *
> > + * @ingroup RPMSG
> > + */
> > +#ifndef __LINUX_RPDEV_INFO_H__
> > +#define __LINUX_RPDEV_INFO_H__
> > +
> > +#define MAX_DEV_PER_CHANNEL 10
> > +
> > +/**
> > + * rpdev_platform_info - store the platform information of rpdev
> > + * @rproc_name: the name of the remote proc.
> > + * @rpdev: rpmsg channel device
> > + * @device_node: pointer to the device node of the rpdev.
> > + * @rx_callback: rx callback handler of the rpdev.
> > + * @channel_devices: an array of the devices related to the rpdev.
> > + */
> > +struct rpdev_platform_info {
> > + const char *rproc_name;
> > + struct rpmsg_device *rpdev;
> > + struct device_node *channel_node;
> > + int (*rx_callback)(struct rpmsg_device *rpdev, void *data,
> > + int len, void *priv, u32 src);
> > + void *channel_devices[MAX_DEV_PER_CHANNEL];
> > +};
> > +
> > +#endif /* __LINUX_RPDEV_INFO_H__ */
