Re: [PATCH v10 04/10] nvmem: layouts: Support fixed-layout as the nvmem device node itself

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] A device tree updated to explicitly mark a legacy NVMEM provider with `compatible = "fixed-layout"` will cause duplicate cell registration and fail the driver probe.
--

--- Patch [4]: [PATCH v10 04/10] nvmem: layouts: Support fixed-layout as the nvmem device node itself ---
commit d0f87955eb48c0cfb8be42e6d6cfadf192c1f792
Author: Loic Poulain <[email protected]>

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

The commit modifies of_nvmem_layout_get_container() to fall back to returning
the nvmem device node itself if it's compatible with "fixed-layout". This
allows cell parsing for providers whose DT node is the fixed-layout container
directly.

> 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]
Does this new logic allow a legacy provider's device node to be returned as a
fixed-layout container and parsed a second time?

If a device tree author updates a legacy NVMEM provider node (where the driver
sets config->add_legacy_fixed_of_cells = true) to explicitly include the
"fixed-layout" compatible property, nvmem_register() will parse it 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 nvmem_add_one_cell() does not check for duplicates, identical cells
will be appended, causing nvmem_populate_sysfs_cells() to fail with -EEXIST
and cleanly abort the driver probe.

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4