Re: [PATCH 2/5] remoteproc: imx_rpoc: fix carveout name parsing

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Incomplete error handling of `of_property_read_string_index()` causes driver probe failure and silent truncation of memory regions if `memory-region-names` is malformed.
- [Low] The header `imx_rproc.h` is missing necessary `#include` directives for the APIs and types used in the newly added `imx_rproc_rmem_to_resource` inline function.
--

commit 892c9b40a1305beaf42a4c79b061576551f828eb
Author: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>

remoteproc: imx_rpoc: fix carveout name parsing
    
This commit modifies carveout name parsing to use the names provided by the
"memory-region-names" device tree property if available. It introduces a
common helper function imx_rproc_rmem_to_resource() to fall back to the old
behavior if the property is not present.

> diff --git a/drivers/remoteproc/imx_rproc.h b/drivers/remoteproc/imx_rproc.h
> --- a/drivers/remoteproc/imx_rproc.h
> +++ b/drivers/remoteproc/imx_rproc.h

[ ... ]

> @@ -45,4 +45,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)
> +{

[Severity: Low]
This isn't a bug, but this header file appears to be missing includes for
<linux/of.h>, <linux/of_reserved_mem.h>, and <linux/ioport.h> which define
the structures and APIs used in this new inline function.

Could this cause build failures in the future if another file includes this
header without first including the prerequisite headers?

> +	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;

[Severity: High]
If the "memory-region-names" property exists but has fewer entries than the
"memory-region" array, of_property_read_string_index() can return -ENODATA.

Will returning -ENODATA here instead of 0 cause imx_rproc_prepare() to break
prematurely out of its while loop, silently ignoring the remaining valid
memory regions?

Also, could this cause imx_rproc_addr_init() to completely fail the driver
probe if it encounters an incomplete property? Should this treat any negative
return value from of_property_read_string_index() as non-fatal to properly
maintain the old fallback behavior?

> +
> +	return ret;
> +}
> +
>  #endif /* _IMX_RPROC_H */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260522111849.783-1-laurentiumihalcea111@gmail.com?part=2