Message-ID:
 <PAXPR04MB91856F8FFE868B753A38326189A8A@PAXPR04MB9185.eurprd04.prod.outlook.com>
Date: Thu, 18 Dec 2025 15:11:08 +0000
From: Shenwei Wang <shenwei.wang@....com>
To: Arnaud POULIQUEN <arnaud.pouliquen@...s.st.com>, Linus Walleij
	<linusw@...nel.org>, Bartosz Golaszewski <brgl@...nel.org>, Rob Herring
	<robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
	<conor+dt@...nel.org>, Bjorn Andersson <andersson@...nel.org>, Mathieu
 Poirier <mathieu.poirier@...aro.org>, Shawn Guo <shawnguo@...nel.org>, Sascha
 Hauer <s.hauer@...gutronix.de>, Jonathan Corbet <corbet@....net>
CC: Pengutronix Kernel Team <kernel@...gutronix.de>, Fabio Estevam
	<festevam@...il.com>, Peng Fan <peng.fan@....com>,
	"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-remoteproc@...r.kernel.org" <linux-remoteproc@...r.kernel.org>,
	"imx@...ts.linux.dev" <imx@...ts.linux.dev>,
	"linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "linux-doc@...r.kernel.org"
	<linux-doc@...r.kernel.org>, dl-linux-imx <linux-imx@....com>
Subject: Re: [PATCH v6 2/5] remoteproc: imx_rproc: Populate devices under
 "rpmsg" subnode



> -----Original Message-----
> From: Arnaud POULIQUEN <arnaud.pouliquen@...s.st.com>
> Sent: Thursday, December 18, 2025 5:04 AM
> To: Shenwei Wang <shenwei.wang@....com>; Linus Walleij
> <linusw@...nel.org>; Bartosz Golaszewski <brgl@...nel.org>; Rob Herring
> <robh@...nel.org>; Krzysztof Kozlowski <krzk+dt@...nel.org>; Conor Dooley
> <conor+dt@...nel.org>; Bjorn Andersson <andersson@...nel.org>; Mathieu
> Poirier <mathieu.poirier@...aro.org>; Shawn Guo <shawnguo@...nel.org>;
> Sascha Hauer <s.hauer@...gutronix.de>; Jonathan Corbet <corbet@....net>
> Cc: Pengutronix Kernel Team <kernel@...gutronix.de>; Fabio Estevam
> <festevam@...il.com>; Peng Fan <peng.fan@....com>; linux-
> gpio@...r.kernel.org; devicetree@...r.kernel.org; linux-
> kernel@...r.kernel.org; linux-remoteproc@...r.kernel.org; imx@...ts.linux.dev;
> linux-arm-kernel@...ts.infradead.org; linux-doc@...r.kernel.org; dl-linux-imx
> <linux-imx@....com>
> 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__ */

Powered by blists - more mailing lists