Hi Rob,
On Wed, 13 Aug 2025 at 23:48, Rob Herring (Arm) <r...@kernel.org> wrote:
> Use the newly added of_reserved_mem_region_to_resource() and
> of_reserved_mem_region_count() functions to handle "memory-region"
> properties.
>
> The error handling is a bit different in some cases. Often
> "memory-region" is optional, so failed lookup is not an error. But then
> an error in of_reserved_mem_lookup() is treated as an error. However,
> that distinction is not really important. Either the region is available
> and usable or it is not. So now, it is just
> of_reserved_mem_region_to_resource() which is checked for an error.
>
> Acked-by: Arnaud Pouliquen <arnaud.pouliq...@foss.st.com>
> Tested-by: Peng Fan <peng....@nxp.com> # i.MX93-11x11-EVK for imx_rproc.c
> Signed-off-by: Rob Herring (Arm) <r...@kernel.org>
Thanks for your patch!
Reviewed-by: Geert Uytterhoeven <geert+rene...@glider.be> # rcar
One nit below (which applies to most drivers).
> --- a/drivers/remoteproc/rcar_rproc.c
> +++ b/drivers/remoteproc/rcar_rproc.c
> @@ -52,41 +52,33 @@ static int rcar_rproc_prepare(struct rproc *rproc)
> {
> struct device *dev = rproc->dev.parent;
> struct device_node *np = dev->of_node;
> - struct of_phandle_iterator it;
> struct rproc_mem_entry *mem;
> - struct reserved_mem *rmem;
> + int i = 0;
> u32 da;
>
> /* Register associated reserved memory regions */
> - of_phandle_iterator_init(&it, np, "memory-region", NULL, 0);
> - while (of_phandle_iterator_next(&it) == 0) {
> -
> - rmem = of_reserved_mem_lookup(it.node);
> - if (!rmem) {
> - of_node_put(it.node);
> - dev_err(&rproc->dev,
> - "unable to acquire memory-region\n");
> - return -EINVAL;
> - }
> + while (1) {
> + struct resource res;
> + int ret;
> +
> + ret = of_reserved_mem_region_to_resource(np, i++, &res);
> + if (ret)
> + return 0;
>
> - if (rmem->base > U32_MAX) {
> - of_node_put(it.node);
> + if (res.start > U32_MAX)
> return -EINVAL;
> - }
>
> /* No need to translate pa to da, R-Car use same map */
> - da = rmem->base;
> + da = res.start;
> mem = rproc_mem_entry_init(dev, NULL,
> - rmem->base,
> - rmem->size, da,
> + res.start,
> + resource_size(&res), da,
> rcar_rproc_mem_alloc,
> rcar_rproc_mem_release,
> - it.node->name);
> + res.name);
>
> - if (!mem) {
> - of_node_put(it.node);
> + if (!mem)
> return -ENOMEM;
> - }
>
> rproc_add_carveout(rproc, mem);
> }
The "return 0;" below (out of context) is now unreachable.
It may be wise to remove it, so the compiler will complain when someone
ever adds a break statement, and people are forced to consider what
is the proper value to return.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
