Re: [PATCH v8 4/9] nvmem: layouts: Support fixed-layout as the nvmem device node itself

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Double parsing of NVMEM cells leading to duplicate sysfs entries and device probe failure when a legacy provider adds the `fixed-layout` compatible string.
--

commit 5ae6267ddd6db9a2a2626cb61243805bccbca0b9
Author: Loic Poulain <loic.poulain@oss.qualcomm.com>

nvmem: layouts: Support fixed-layout as the nvmem device node itself

This commit modifies of_nvmem_layout_get_container() to return the nvmem
device node itself if it lacks an "nvmem-layout" child but has the
"fixed-layout" compatible string.

> diff --git a/drivers/nvmem/layouts.c b/drivers/nvmem/layouts.c
> index b90584e1b99ea..efa631ce7283b 100644
> --- a/drivers/nvmem/layouts.c
> +++ b/drivers/nvmem/layouts.c
> @@ -167,7 +167,18 @@ static int nvmem_layout_bus_populate(struct nvmem_device *nvmem,
>  
>  struct device_node *of_nvmem_layout_get_container(struct nvmem_device *nvmem)
>  {
> -	return of_get_child_by_name(nvmem->dev.of_node, "nvmem-layout");
> +	struct device_node *np;
> +
> +	/* Search for nvmem-layout child */
> +	np = of_get_child_by_name(nvmem->dev.of_node, "nvmem-layout");
> +	if (np)
> +		return np;
> +
> +	/* The nvmem of_node is itself a fixed-layout node */
> +	if (of_device_is_compatible(nvmem->dev.of_node, "fixed-layout"))
> +		return of_node_get(nvmem->dev.of_node);

[Severity: Medium]
Could this lead to double parsing of NVMEM cells and device probe failures
for legacy providers?

If a legacy provider (which sets config->add_legacy_fixed_of_cells = true
in its driver) updates its device tree node to include the "fixed-layout"
compatible string, it looks like nvmem_register() might parse the cells
twice:

drivers/nvmem/core.c:nvmem_register() {
        ...
        if (config->add_legacy_fixed_of_cells) {
                rval = nvmem_add_cells_from_legacy_of(nvmem);
                if (rval)
                        goto err_remove_cells;
        }

        rval = nvmem_add_cells_from_fixed_layout(nvmem);
        if (rval)
                goto err_remove_cells;
        ...
}

Because of_nvmem_layout_get_container() now returns the nvmem device node
itself, both functions would process the same child nodes.

Would nvmem_populate_sysfs_cells() then fail with -EEXIST due to duplicate
sysfs entries and abort the device registration?

> +
> +	return NULL;
>  }
>  EXPORT_SYMBOL_GPL(of_nvmem_layout_get_container);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703-block-as-nvmem-v8-0-98ae32bfc49a@oss.qualcomm.com?part=4