Re: [PATCH v5 3/7] mtd: spi-nor: sfdp: expose the SFDP as a read-only NVMEM device

[Manikandan.M]

Hi Michael,

On 7/21/26 12:32 PM, Michael Walle wrote:
> On Tue Jul 21, 2026 at 7:28 AM CEST, Manikandan Muralidharan wrote:
>> The SPI NOR core already reads the SFDP tables during enumeration and
>> caches them in nor->sfdp->dwords (see spi_nor_parse_sfdp()). Re-expose
>> that cached data as a read-only NVMEM device, in on-flash byte order,
>> rooted at the flash's SFDP child node (compatible "jedec,sfdp").
>>
>> This lets NVMEM cells reference any SFDP data: a fixed-layout for
>> parameters at a known offset, or an nvmem-layout parser for vendor data
>> whose location must be discovered at runtime.The device is only registered
>> when an "sfdp" node is present in the device tree.
>>
>> Signed-off-by: Manikandan Muralidharan <manikandan.m@xxxxxxxxxxxxx>
>> ---
>>   drivers/mtd/spi-nor/core.c |  8 ++++
>>   drivers/mtd/spi-nor/core.h |  1 +
>>   drivers/mtd/spi-nor/sfdp.c | 86 ++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 95 insertions(+)
>>
>> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
>> index ccf4396cdcd0..b833d8ec2d65 100644
>> --- a/drivers/mtd/spi-nor/core.c
>> +++ b/drivers/mtd/spi-nor/core.c
>> @@ -3204,6 +3204,14 @@ static int spi_nor_init_params(struct spi_nor *nor)
>>   		spi_nor_init_params_deprecated(nor);
>>   	}
>>   
>> +	/*
>> +	 * Expose the SFDP table as an NVMEM device only when
>> +	 * the flash actually provides one
>> +	 */
>> +	ret = spi_nor_register_sfdp_nvmem(nor);
>> +	if (ret)
>> +		return ret;
>> +
>>   	ret = spi_nor_late_init_params(nor);
>>   	if (ret)
>>   		return ret;
>> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
>> index ba2d1a862c9d..0a6484298c5c 100644
>> --- a/drivers/mtd/spi-nor/core.h
>> +++ b/drivers/mtd/spi-nor/core.h
>> @@ -698,6 +698,7 @@ int spi_nor_controller_ops_write_reg(struct spi_nor *nor, u8 opcode,
>>   
>>   int spi_nor_check_sfdp_signature(struct spi_nor *nor);
>>   int spi_nor_parse_sfdp(struct spi_nor *nor);
>> +int spi_nor_register_sfdp_nvmem(struct spi_nor *nor);
> 
> That's probably not needed if..
> 
>>   static inline struct spi_nor *mtd_to_spi_nor(struct mtd_info *mtd)
>>   {
>> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
>> index 4600983cb579..704799fe92ae 100644
>> --- a/drivers/mtd/spi-nor/sfdp.c
>> +++ b/drivers/mtd/spi-nor/sfdp.c
>> @@ -6,6 +6,8 @@
>>   
>>   #include <linux/bitfield.h>
>>   #include <linux/mtd/spi-nor.h>
>> +#include <linux/nvmem-provider.h>
>> +#include <linux/of.h>
>>   #include <linux/slab.h>
>>   #include <linux/sort.h>
>>   
>> @@ -1612,3 +1614,87 @@ int spi_nor_parse_sfdp(struct spi_nor *nor)
>>   	kfree(param_headers);
>>   	return err;
>>   }
>> +
>> +static int spi_nor_sfdp_reg_read(void *priv, unsigned int offset,
>> +				 void *val, size_t bytes)
>> +{
>> +	struct spi_nor *nor = priv;
>> +	struct sfdp *sfdp = nor->sfdp;
>> +	size_t sfdp_size = sfdp->num_dwords * sizeof(*sfdp->dwords);
>> +
>> +	if (offset >= sfdp_size || bytes > sfdp_size - offset)
>> +		return -EINVAL;
>> +
>> +	/* The cached SFDP is kept in on-flash (little-endian) byte order. */
>> +	memcpy(val, (u8 *)sfdp->dwords + offset, bytes);
>> +
>> +	return 0;
>> +}
>> +
>> +static void spi_nor_sfdp_nvmem_put_np(void *data)
>> +{
>> +	of_node_put(data);
>> +}
>> +
>> +/**
>> + * spi_nor_register_sfdp_nvmem() - expose the SFDP as a read-only NVMEM device
>> + * @nor:	pointer to a 'struct spi_nor'
>> + *
>> + * Expose the whole SFDP, in on-flash byte order, as a read-only NVMEM device
>> + * rooted at the flash's SFDP child node (compatible "jedec,sfdp"). This lets
>> + * generic (fixed-layout) or vendor (nvmem-layout) cells reference any SFDP
>> + * data. The device is only registered when a child node with the "jedec,sfdp"
>> + * compatible is described in the device tree.
>> + *
>> + * Return: 0 on success or if there is nothing to do, -errno otherwise.
>> + */
>> +int spi_nor_register_sfdp_nvmem(struct spi_nor *nor)
> 
> .. you move all this into the core, as the sfdp.c is just for
> parsing the tables.
> 
>> +{
>> +	struct device *dev = nor->dev;
>> +	struct nvmem_config config = { };
>> +	struct nvmem_device *nvmem;
>> +	struct device_node *np;
>> +	int ret;
>> +
>> +	if (!nor->sfdp)
>> +		return 0;
>> +
>> +	for_each_available_child_of_node(dev_of_node(dev), np)
>> +		if (of_device_is_compatible(np, "jedec,sfdp"))
>> +			break;
> 
> There is already of_get_compatible_child() doing exactly this.
> 
>> +	if (!np)
>> +		return 0;
>> +
>> +	/*
>> +	 * Register the put before devm_nvmem_register() so it runs last on
>> +	 * detach, after the NVMEM device that uses the node is gone.
>> +	 */
>> +	ret = devm_add_action_or_reset(dev, spi_nor_sfdp_nvmem_put_np, np);
>> +	if (ret)
>> +		return ret;
>> +
>> +	config.dev = dev;
>> +	config.of_node = np;
>> +	config.name = "sfdp";
>> +	config.id = NVMEM_DEVID_AUTO;
> 
> Or rather NVEMEM_DEVID_NONE? There will ever be just one SFDP nvmem
> device.
> 
> How does the sysfs path looks like?
Currently the sysfs looks like:

/sys/bus/nvmem/devices/sfdp0

I kept AUTO so that the bare "sfdp" name would clash on a board(if any) 
with a second flash exposing a "jedec,sfdp" node.

> 
> -michael
> 
>> +	config.owner = THIS_MODULE;
>> +	config.read_only = true;
>> +	config.word_size = 1;
>> +	config.stride = 1;
>> +	config.size = (int)(nor->sfdp->num_dwords * sizeof(*nor->sfdp->dwords));
>> +	config.reg_read = spi_nor_sfdp_reg_read;
>> +	config.priv = nor;
>> +
>> +	nvmem = devm_nvmem_register(dev, &config);
>> +	if (IS_ERR(nvmem)) {
>> +		/* NVMEM support is optional. */
>> +		if (PTR_ERR(nvmem) == -EOPNOTSUPP)
>> +			return 0;
>> +		return dev_err_probe(dev, PTR_ERR(nvmem),
>> +				     "failed to register SFDP NVMEM device\n");
>> +	}
>> +
>> +	dev_dbg(dev, "exposed %d-byte SFDP as an NVMEM device\n", config.size);
>> +
>> +	return 0;
>> +}


-- 
Thanks and Regards,
Manikandan M.