Re: [PATCH v5 2/5] remoteproc: imx_rproc: Populate devices under "rpmsg" subnode

Mathieu Poirier
2025-11-26T18:53:49Z

On Tue, Nov 04, 2025 at 02:33:12PM -0600, Shenwei Wang wrote:
> Register the RPMsg channel driver and populate remote devices defined
> under the "rpmsg" subnode upon receiving their notification messages.
>
> The following illustrates the expected DTS layout structure:
>
> cm33: remoteproc-cm33 {
> compatible = "fsl,imx8ulp-cm33";
>
> rpmsg {
> rpmsg-io-channel {
> gpio@0 {
> compatible = "fsl,imx-rpmsg-gpio";
> reg = <0>;
> };
>
> gpio@1 {
> compatible = "fsl,imx-rpmsg-gpio";
> reg = <1>;
> };
>
> ...
> };
>
> rpmsg-i2c-channel {
> i2c@0 {
> compatible = "fsl,imx-rpmsg-i2c";
> reg = <0>;
> };
> };
>
> ...
> };
> };
>
> Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
> ---
> drivers/remoteproc/imx_rproc.c | 146 ++++++++++++++++++++++++++++++++
> include/linux/rpmsg/imx_rpmsg.h | 48 +++++++++++
> 2 files changed, 194 insertions(+)
> create mode 100644 include/linux/rpmsg/imx_rpmsg.h
>
> diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> index a6eef0080ca9..e21a7980c490 100644
> --- a/drivers/remoteproc/imx_rproc.c
> +++ b/drivers/remoteproc/imx_rproc.c
> @@ -8,6 +8,7 @@
> #include <linux/clk.h>
> #include <linux/err.h>
> #include <linux/firmware/imx/sci.h>
> +#include <linux/rpmsg/imx_rpmsg.h>
> #include <linux/interrupt.h>
> #include <linux/kernel.h>
> #include <linux/mailbox_client.h>
> @@ -15,6 +16,8 @@
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_platform.h>
> #include <linux/of_reserved_mem.h>
> #include <linux/platform_device.h>
> #include <linux/pm_domain.h>
> @@ -22,6 +25,7 @@
> #include <linux/reboot.h>
> #include <linux/regmap.h>
> #include <linux/remoteproc.h>
> +#include <linux/rpmsg.h>
> #include <linux/workqueue.h>
>
> #include "imx_rproc.h"
> @@ -1084,6 +1088,144 @@ static int imx_rproc_sys_off_handler(struct sys_off_data *data)
> return NOTIFY_DONE;
> }
>
> +struct imx_rpmsg_driver {
> + struct rpmsg_driver rpdrv;
> + void *driver_data;
> +};
> +
> +static char *channel_device_map[][2] = {
> + {"rpmsg-io-channel", "fsl,imx-rpmsg-gpio"},
> + {"rpmsg-i2c-channel", "fsl,imx-rpmsg-i2c"},
> +};
> +
> +static int imx_rpmsg_endpoint_cb(struct rpmsg_device *rpdev, void *data,
> + int len, void *priv, u32 src)
> +{
> + struct imx_rpmsg_driver_data *drvdata;
> +
> + drvdata = dev_get_drvdata(&rpdev->dev);
> + if (drvdata && drvdata->rx_callback)
> + return drvdata->rx_callback(rpdev, data, len, priv, src);
> +
> + return 0;
> +}
> +
> +static void imx_rpmsg_endpoint_remove(struct rpmsg_device *rpdev)
> +{
> + of_platform_depopulate(&rpdev->dev);
> +}
> +
> +static int imx_rpmsg_endpoint_probe(struct rpmsg_device *rpdev)
> +{

Where does the rpmsg_device come from? Usually there is a call to
rpmsg_register_device() and I don't see it anywhere in this patchset. I also
don't see a link to the remote processor. I can't continue with this set for as
long as I don't have this information.

> + struct imx_rpmsg_driver_data *drvdata;
> + struct imx_rpmsg_driver *imx_rpdrv;
> + struct device *dev = &rpdev->dev;
> + struct of_dev_auxdata *auxdata;
> + struct rpmsg_driver *rpdrv;
> + int i;
> +
> + rpdrv = container_of(dev->driver, struct rpmsg_driver, drv);
> + imx_rpdrv = container_of(rpdrv, struct imx_rpmsg_driver, rpdrv);
> +
> + if (!imx_rpdrv->driver_data)
> + return -EINVAL;
> +
> + drvdata = devm_kmemdup(dev, imx_rpdrv->driver_data, sizeof(*drvdata), GFP_KERNEL);
> + if (!drvdata)
> + return -ENOMEM;
> +
> + i = drvdata->map_idx;
> + if (i >= ARRAY_SIZE(channel_device_map))
> + return -ENODEV;
> +
> + auxdata = devm_kzalloc(dev, sizeof(*auxdata) * 2, GFP_KERNEL);
> + if (!auxdata)
> + return -ENOMEM;
> +
> + drvdata->rpdev = rpdev;
> + auxdata[0].compatible = channel_device_map[i][1];
> + auxdata[0].platform_data = drvdata;
> + dev_set_drvdata(dev, drvdata);
> +
> + of_platform_populate(drvdata->channel_node, NULL, auxdata, dev);
> + of_node_put(drvdata->channel_node);
> +
> + return 0;
> +}
> +
> +static int imx_of_rpmsg_is_in_map(const char *name)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(channel_device_map); i++) {
> + if (strcmp(name, channel_device_map[i][0]) == 0)
> + return i;
> + }
> +
> + return -1;
> +}
> +
> +static int imx_of_rpmsg_register_rpdriver(struct device_node *channel,
> + struct device *dev, int idx)
> +{
> + struct imx_rpmsg_driver_data *driver_data;
> + struct imx_rpmsg_driver *rp_driver;
> + struct rpmsg_device_id *rpdev_id;
> +
> + rpdev_id = devm_kzalloc(dev, sizeof(*rpdev_id) * 2, GFP_KERNEL);
> + if (!rpdev_id)
> + return -ENOMEM;
> +
> + strscpy(rpdev_id[0].name, channel_device_map[idx][0], RPMSG_NAME_SIZE);
> +
> + rp_driver = devm_kzalloc(dev, sizeof(*rp_driver), GFP_KERNEL);
> + if (!rp_driver)
> + return -ENOMEM;
> +
> + driver_data = devm_kzalloc(dev, sizeof(*driver_data), GFP_KERNEL);
> + if (!driver_data)
> + return -ENOMEM;
> +
> + driver_data->rproc_name = dev->of_node->name;
> + driver_data->channel_node = channel;
> + driver_data->map_idx = idx;
> +
> + rp_driver->rpdrv.drv.name = channel_device_map[idx][0];
> + 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;
> +
> + register_rpmsg_driver(&rp_driver->rpdrv);
> +
> + return 0;
> +}
> +
> +static int imx_of_rpmsg_node_init(struct platform_device *pdev)
> +{
> + struct device_node *np __free(device_node);
> + struct device *dev = &pdev->dev;
> + int idx, ret;
> +
> + np = of_get_child_by_name(dev->of_node, "rpmsg");
> + if (!np)
> + return 0;
> +
> + for_each_child_of_node_scoped(np, child) {
> + idx = imx_of_rpmsg_is_in_map(child->name);
> + if (idx < 0)
> + ret = of_platform_default_populate(child, NULL, dev);
> + else
> + ret = imx_of_rpmsg_register_rpdriver(child, dev, idx);
> +
> + if (ret < 0)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> static int imx_rproc_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> @@ -1177,6 +1319,10 @@ static int imx_rproc_probe(struct platform_device *pdev)
> goto err_put_clk;
> }
>
> + ret = imx_of_rpmsg_node_init(pdev);
> + if (ret < 0)
> + dev_info(dev, "populating 'rpmsg' node failed\n");
> +
> return 0;
>
> err_put_clk:
> diff --git a/include/linux/rpmsg/imx_rpmsg.h b/include/linux/rpmsg/imx_rpmsg.h
> new file mode 100644
> index 000000000000..04a5ad2d4a1d
> --- /dev/null
> +++ b/include/linux/rpmsg/imx_rpmsg.h
> @@ -0,0 +1,48 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright 2025 NXP */
> +
> +/*
> + * @file linux/imx_rpmsg.h
> + *
> + * @brief Global header file for iMX RPMSG
> + *
> + * @ingroup RPMSG
> + */
> +#ifndef __LINUX_IMX_RPMSG_H__
> +#define __LINUX_IMX_RPMSG_H__
> +
> +/* Category define */
> +#define IMX_RMPSG_LIFECYCLE 1
> +#define IMX_RPMSG_PMIC 2
> +#define IMX_RPMSG_AUDIO 3
> +#define IMX_RPMSG_KEY 4
> +#define IMX_RPMSG_GPIO 5
> +#define IMX_RPMSG_RTC 6
> +#define IMX_RPMSG_SENSOR 7
> +
> +/* rpmsg version */
> +#define IMX_RMPSG_MAJOR 1
> +#define IMX_RMPSG_MINOR 0
> +
> +#define MAX_DEV_PER_CHANNEL 10
> +
> +struct imx_rpmsg_head {
> + u8 cate; /* Category */
> + u8 major; /* Major version */
> + u8 minor; /* Minor version */
> + u8 type; /* Message type */
> + u8 cmd; /* Command code */
> + u8 reserved[5];
> +} __packed;
> +
> +struct imx_rpmsg_driver_data {
> + int map_idx;
> + 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_IMX_RPMSG_H__ */
> --
> 2.43.0
>