Re: [PATCH 1/2] mtd: spi-nor: sst: register SFDP region into NVMEM framework to read MAC Address

From: Miquel Raynal <miquel.raynal@bootlin.com>
To: <Manikandan.M@microchip.com>
Cc: robh@kernel.org, conor+dt@kernel.org, mwalle@kernel.org,
	vigneshr@ti.com, devicetree@vger.kernel.org, richard@nod.at,
	linux-kernel@vger.kernel.org, alexandre.belloni@bootlin.com,
	tudor.ambarus@linaro.org, linux-mtd@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org, claudiu.beznea@tuxon.dev,
	Varshini.Rajendran@microchip.com, krzk+dt@kernel.org,
	pratyush@kernel.org
Subject: Re: [PATCH 1/2] mtd: spi-nor: sst: register SFDP region into NVMEM framework to read MAC Address
Date: Thu, 06 Mar 2025 09:34:41 +0100	[thread overview]
Message-ID: <87eczawo9q.fsf@bootlin.com> (raw)
In-Reply-To: <6fee6e71-106f-474b-9a0c-5df5fb0caa00@microchip.com> (Manikandan M.'s message of "Thu, 6 Mar 2025 07:09:44 +0000")

>>> +static int sst26vf_nor_post_sfdp(struct spi_nor *nor)
>>> +{
>>> +     struct nvmem_device *nvmem;
>>> +
>>> +     sst26vf_sfdp_nvmem_config.dev = nor->dev;
>>> +     sst26vf_sfdp_nvmem_config.size = nor->sfdp->num_dwords * sizeof(*nor->sfdp->dwords);
>>> +     sst26vf_sfdp_nvmem_config.priv = nor;
>>> +     sst26vf_sfdp_nvmem_config.reg_read = sst26vf_sfdp_mac_addr_read;
>>> +
>>> +     nvmem = devm_nvmem_register(nor->dev, &sst26vf_sfdp_nvmem_config);
>>> +     if (IS_ERR(nvmem)) {
>>> +             dev_err(nor->dev, "failed to register NVMEM device: %ld\n", PTR_ERR(nvmem));
>>> +             return PTR_ERR(nvmem);
>> 
>> I don't think it makes sense to have this one-off in a particular
>> driver. If at all, this should be handled in the core. Sorry, but
>> this really looks like an ugly hack.
>> 
>
> Because the EUI identifier within the SFDP is unique to the 
> SST26VF064BEUI flash, I opted to handle it here rather than in the core.
>
> Also here the MAC address data resides within the 0x260-0x26F range, I 
> will resize the nvmem_config.size to 0x10 instead of registering the 
> full SFDP region as NVMEM.

Open question to all parties in this thread: how do we give an offset in
the device tree that is relative to the sfdp region and not the data
region? I believe we care not to mix these areas while describing.

Thanks,
Miquèl