Re: [PATCH v9 04/10] 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:
- [High] Double-parsing of nvmem cells causes device probe failure when the provider node has `compatible = "fixed-layout"`.
--

--- Patch [4]: [PATCH v9 04/10] nvmem: layouts: Support fixed-layout as the nvmem device node itself ---
commit d3f11032479fe63790d2e2a677a2a7e3ebc199dc
Author: Loic Poulain <loic.poulain@oss.qualcomm.com>

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

This commit allows of_nvmem_layout_get_container() to fall back to the nvmem
device node itself if it is compatible with "fixed-layout". This enables the
cells to be parsed directly by nvmem_add_cells_from_fixed_layout() when no
child layout node is present.

> 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: High]
Will returning the provider's own device node here cause a double-parsing
issue and device probe failure due to sysfs attribute collisions?

In drivers/nvmem/core.c:nvmem_register(), both legacy parsing and
fixed-layout parsing are executed consecutively without checking for
duplication:

    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;

If a provider sets config->add_legacy_fixed_of_cells = true for backwards
compatibility, but the device tree uses the new "fixed-layout" compatible on
the provider node, both functions will parse the same of_node children.

Could this lead to duplicate cells being added to nvmem->cells, causing
nvmem_populate_sysfs_cells() to fail with -EEXIST?

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730-block-as-nvmem-v9-0-f72935817dbf@oss.qualcomm.com?part=4