Re: [PATCH v2 1/2] nvmem: layouts: Add fixed-layout driver

[Srinivas Kandagatla <srini@kernel.org>]

On 5/15/26 12:56 PM, Mathieu Dubois-Briand wrote:
> Current implementation isn't working well when device tree nodes have a
> phandle on a fixed-layout nvmem node. As the fixed layout is handled in
> nvmem core, no driver is ever associated with the layout, and the device
> consumer driver probe is deferred indefinitely.
> 
> Remove the specific handling of fixed-layout and add a layout driver.
> This makes the fixed-layout similar to all other layouts, fixing the
> whole issue.
> 
> Fixes: fc29fd821d9a ("nvmem: core: Rework layouts to become regular devices")
> Cc: stable@vger.kernel.org
> Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
> ---
>  MAINTAINERS                          |  5 ++++
>  drivers/nvmem/core.c                 | 23 +---------------
>  drivers/nvmem/layouts.c              | 11 --------
>  drivers/nvmem/layouts/Makefile       |  1 +
>  drivers/nvmem/layouts/fixed-layout.c | 52 ++++++++++++++++++++++++++++++++++++
>  include/linux/nvmem-provider.h       |  7 +++++
>  6 files changed, 66 insertions(+), 33 deletions(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 882214b0e7db..c48c4e129736 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10018,6 +10018,11 @@ F:	drivers/base/firmware_loader/
>  F:	rust/kernel/firmware.rs
>  F:	include/linux/firmware.h
>  
> +FIXED-LAYOUT NVMEM LAYOUT DRIVER
> +M:	Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
> +S:	Maintained
> +F:	drivers/nvmem/layouts/fixed-layout.c
> +
>  FLEXTIMER FTM-QUADDEC DRIVER
>  M:	Patrick Havelange <patrick.havelange@essensium.com>
>  L:	linux-iio@vger.kernel.org
> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> index 311cb2e5a5c0..0ec4924c4bda 100644
> --- a/drivers/nvmem/core.c
> +++ b/drivers/nvmem/core.c
> @@ -786,7 +786,7 @@ static int nvmem_validate_keepouts(struct nvmem_device *nvmem)
>  	return 0;
>  }
>  
> -static int nvmem_add_cells_from_dt(struct nvmem_device *nvmem, struct device_node *np)
> +int nvmem_add_cells_from_dt(struct nvmem_device *nvmem, struct device_node *np)

Export this in this patch itself.

>  {
>  	struct device *dev = &nvmem->dev;
>  	const __be32 *addr;
> @@ -840,23 +840,6 @@ static int nvmem_add_cells_from_legacy_of(struct nvmem_device *nvmem)
>  	return nvmem_add_cells_from_dt(nvmem, nvmem->dev.of_node);
>  }
>  
> -static int nvmem_add_cells_from_fixed_layout(struct nvmem_device *nvmem)
> -{
> -	struct device_node *layout_np;
> -	int err = 0;
> -
> -	layout_np = of_nvmem_layout_get_container(nvmem);
> -	if (!layout_np)
> -		return 0;
> -
> -	if (of_device_is_compatible(layout_np, "fixed-layout"))
> -		err = nvmem_add_cells_from_dt(nvmem, layout_np);
> -
> -	of_node_put(layout_np);
> -
> -	return err;
> -}
> -
>  int nvmem_layout_register(struct nvmem_layout *layout)
>  {
>  	int ret;
> @@ -1005,10 +988,6 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
>  			goto err_remove_cells;
>  	}
>  
> -	rval = nvmem_add_cells_from_fixed_layout(nvmem);
> -	if (rval)
> -		goto err_remove_cells;
> -
>  	dev_dbg(&nvmem->dev, "Registering nvmem device %s\n", config->name);
>  
>  	rval = device_add(&nvmem->dev);
> diff --git a/drivers/nvmem/layouts.c b/drivers/nvmem/layouts.c
> index b90584e1b99e..07a34be9669c 100644
> --- a/drivers/nvmem/layouts.c
> +++ b/drivers/nvmem/layouts.c
> @@ -125,11 +125,6 @@ static int nvmem_layout_create_device(struct nvmem_device *nvmem,
>  	return 0;
>  }
>  
> -static const struct of_device_id of_nvmem_layout_skip_table[] = {
> -	{ .compatible = "fixed-layout", },
> -	{}
> -};
> -
>  static int nvmem_layout_bus_populate(struct nvmem_device *nvmem,
>  				     struct device_node *layout_dn)
>  {
> @@ -142,12 +137,6 @@ static int nvmem_layout_bus_populate(struct nvmem_device *nvmem,
>  		return 0;
>  	}
>  
> -	/* Fixed layouts are parsed manually somewhere else for now */
> -	if (of_match_node(of_nvmem_layout_skip_table, layout_dn)) {
> -		pr_debug("%s() - skipping %pOF node\n", __func__, layout_dn);
> -		return 0;
> -	}
> -
>  	if (of_node_check_flag(layout_dn, OF_POPULATED_BUS)) {
>  		pr_debug("%s() - skipping %pOF, already populated\n",
>  			 __func__, layout_dn);
> diff --git a/drivers/nvmem/layouts/Makefile b/drivers/nvmem/layouts/Makefile
> index 4940c9db0665..dd6c6c70b1a9 100644
> --- a/drivers/nvmem/layouts/Makefile
> +++ b/drivers/nvmem/layouts/Makefile
> @@ -3,6 +3,7 @@
>  # Makefile for nvmem layouts.
>  #
>  
> +obj-$(CONFIG_NVMEM_LAYOUTS) += fixed-layout.o
>  obj-$(CONFIG_NVMEM_LAYOUT_SL28_VPD) += sl28vpd.o
>  obj-$(CONFIG_NVMEM_LAYOUT_ONIE_TLV) += onie-tlv.o
>  obj-$(CONFIG_NVMEM_LAYOUT_U_BOOT_ENV) += u-boot-env.o
> diff --git a/drivers/nvmem/layouts/fixed-layout.c b/drivers/nvmem/layouts/fixed-layout.c
> new file mode 100644
> index 000000000000..bc7da9a904d4
> --- /dev/null
> +++ b/drivers/nvmem/layouts/fixed-layout.c
> @@ -0,0 +1,52 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright 2026 Bootlin
> + *
> + * Authors: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
> + */
> +
> +#include <linux/nvmem-provider.h>
> +#include <linux/of.h>
> +
> +static int fixed_layout_add_cells(struct nvmem_layout *layout)
> +{
> +	struct device_node *np;
> +
> +	np = of_nvmem_layout_get_container(layout->nvmem);
> +	if (!np)
> +		return -ENOENT;
> +
> +	return nvmem_add_cells_from_dt(layout->nvmem, np);

np is leaking here.

> +}
> +
> +static int fixed_layout_probe(struct nvmem_layout *layout)
> +{
> +	layout->add_cells = fixed_layout_add_cells;
> +
> +	return nvmem_layout_register(layout);
> +}
> +
> +static void fixed_layout_remove(struct nvmem_layout *layout)
> +{
> +	nvmem_layout_unregister(layout);
> +}
> +
> +static const struct of_device_id fixed_layout_of_match_table[] = {
> +	{ .compatible = "fixed-layout", },
> +	{},
> +};
> +
> +static struct nvmem_layout_driver fixed_layout_layout = {
> +	.driver = {
> +		.name = "fixed-layout",
> +		.of_match_table = fixed_layout_of_match_table,
> +	},
> +	.probe = fixed_layout_probe,
> +	.remove = fixed_layout_remove,
> +};
> +module_nvmem_layout_driver(fixed_layout_layout);
> +
> +MODULE_AUTHOR("Mathieu Dubois-Briand");
> +MODULE_LICENSE("GPL");
> +MODULE_DEVICE_TABLE(of, fixed_layout_of_match_table);
> +MODULE_DESCRIPTION("NVMEM fixed-layout driver");
> diff --git a/include/linux/nvmem-provider.h b/include/linux/nvmem-provider.h
> index f3b13da78aac..e7eaa9a89b8b 100644
> --- a/include/linux/nvmem-provider.h
> +++ b/include/linux/nvmem-provider.h
> @@ -176,6 +176,7 @@ int nvmem_add_one_cell(struct nvmem_device *nvmem,
>  
>  int nvmem_layout_register(struct nvmem_layout *layout);
>  void nvmem_layout_unregister(struct nvmem_layout *layout);
> +int nvmem_add_cells_from_dt(struct nvmem_device *nvmem, struct device_node *np);
>  
>  #define nvmem_layout_driver_register(drv) \
>  	__nvmem_layout_driver_register(drv, THIS_MODULE)
> @@ -214,6 +215,12 @@ static inline int nvmem_layout_register(struct nvmem_layout *layout)
>  
>  static inline void nvmem_layout_unregister(struct nvmem_layout *layout) {}
>  
> +static inline int nvmem_add_cells_from_dt(struct nvmem_device *nvmem,
> +					  struct device_node *np)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
>  #endif /* CONFIG_NVMEM */
>  
>  #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OF)
>