On Thu, Oct 15, 2026 at 10:26:51PM +0200, Francesco Valla wrote: > Hello Frank, > > > The driver currently derives the memory region type from the referenced > > memory node name, creating a hidden DT ABI. Since memory node names are > > not constrained by the binding, incorrect names can silently pass DT > > validation, as seen in examples using "vdevbuffer" instead of > > "vdev0buffer" ([1] and [2]). > > > > Parse memory-region-names to identify the expected memory regions and avoid > > relying on memory node names as ABI. > > > > Link: > > https://lore.kernel.org/imx/[email protected]/ > > [1] > > Link: > > https://lore.kernel.org/imx/alexXTYQHUs76J7v@SMW015318/T/#mb9dc709ff3adc24d9db6d30c973f37b580f6f1f9 > > [2] > > > > Signed-off-by: Laurentiu Mihalcea <[email protected]> > > Reviewed-by: Peng Fan <[email protected]> > > Signed-off-by: Frank Li <[email protected]> > > > > diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c > > index 745ce52cd822..ce9bdfc1fa98 100644 > > --- a/drivers/remoteproc/imx_rproc.c > > +++ b/drivers/remoteproc/imx_rproc.c > > @@ -643,7 +643,7 @@ static int imx_rproc_prepare(struct rproc *rproc) > > int err; > > struct resource res; > > > > - err = of_reserved_mem_region_to_resource(np, i++, &res); > > + err = imx_rproc_rmem_to_resource(np, i++, &res); > > if (err) > > break; > > > > @@ -818,11 +818,20 @@ static int imx_rproc_addr_init(struct imx_rproc *priv, > > if (nph <= 0) > > return 0; > > > > + if (!of_property_present(np, "memory-region-names")) { > > + dev_warn(dev, "using node names for carveouts should be > > avoided\n"); > > + } else { > > + if (nph != of_property_count_strings(np, > > "memory-region-names")) { > > + dev_err(dev, "invalid reserved memory name count\n"); > > + return -EINVAL; > > + } > > + } > > + > > /* remap optional addresses */ > > for (a = 0; a < nph; a++) { > > struct resource res; > > > > - err = of_reserved_mem_region_to_resource(np, a, &res); > > + err = imx_rproc_rmem_to_resource(np, a, &res); > > if (err) { > > dev_err(dev, "unable to resolve memory region\n"); > > return err; > > diff --git a/drivers/remoteproc/imx_rproc.h b/drivers/remoteproc/imx_rproc.h > > index 0d7d48352a10..3632bc375c71 100644 > > --- a/drivers/remoteproc/imx_rproc.h > > +++ b/drivers/remoteproc/imx_rproc.h > > @@ -7,6 +7,8 @@ > > #ifndef _IMX_RPROC_H > > #define _IMX_RPROC_H > > > > +#include <linux/of_reserved_mem.h> > > + > > /* address translation table */ > > struct imx_rproc_att { > > u32 da; /* device address (From Cortex M4 view)*/ > > @@ -45,4 +47,23 @@ struct imx_rproc_dcfg { > > u32 reset_vector_mask; > > }; > > > > +static inline int imx_rproc_rmem_to_resource(struct device_node *np, > > + int index, > > + struct resource *res) > > +{ > > + int ret; > > + > > + ret = of_reserved_mem_region_to_resource(np, index, res); > > + if (ret) > > + return ret; > > + > > + /* "memory-region-names" is optional */ > > + ret = of_property_read_string_index(np, "memory-region-names", > > + index, &res->name); > > + if (ret == -EINVAL) > > + return 0; > > + > > + return ret; > > +} > > + > > Shouldn't this be a regular function in imx_rproc.c instead of being > defined as inline here?
Yes, will fix at next version. Frank > Keeping it in the driver source file would > probably make the logic easier to follow, also considering that the > presence of memory-region-names is evaluated there in > imx_rproc_addr_init(). > > > #endif /* _IMX_RPROC_H */ > > Regards, > Francesco > > -- > Francesco Valla <[email protected]>
