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

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

[Manikandan Muralidharan]

From: Varshini Rajendran <varshini.rajendran@microchip.com>

EUI identifier and the MAC Address of the Ethernet Interface is stored
after the SFDP table of contents starting at address 0x260 in the
QSPI memory.
Register the entire SFDP region read by the spi-nor (nor->sfdp) into the
NVMEM framework and read the MAC Address when requested using the nvmem
properties in the DT by the net drivers.

In kernel the Ethernet MAC address relied on U-Boot env variables or
generated a random address, which posed challenges for boards without
on-board EEPROMs or with multiple Ethernet ports.
This change ensures consistent and reliable MAC address retrieval from QSPI,
benefiting boards like the sama5d29 curiosity and sam9x75 curiosity.

Signed-off-by: Varshini Rajendran <varshini.rajendran@microchip.com>
[manikandan.m@microchip.com: Integrate the nvmem->read callback framework]
Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
---
 drivers/mtd/spi-nor/sst.c | 62 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/drivers/mtd/spi-nor/sst.c b/drivers/mtd/spi-nor/sst.c
index 175211fe6a5e..a0abf201ad41 100644
--- a/drivers/mtd/spi-nor/sst.c
+++ b/drivers/mtd/spi-nor/sst.c
@@ -5,6 +5,7 @@
  */
 
 #include <linux/mtd/spi-nor.h>
+#include <linux/nvmem-provider.h>
 
 #include "core.h"
 
@@ -13,6 +14,8 @@
 
 #define SST26VF_CR_BPNV		BIT(3)
 
+#define SST26VF_SFDP_EUI48	0x30
+
 static int sst26vf_nor_lock(struct spi_nor *nor, loff_t ofs, u64 len)
 {
 	return -EOPNOTSUPP;
@@ -56,8 +59,67 @@ static int sst26vf_nor_late_init(struct spi_nor *nor)
 	return 0;
 }
 
+/**
+ * sst26vf_sfdp_mac_addr_read() - check if the EUI-48 MAC Address is programmed
+ * and read the data from the prestored SFDP data
+ *
+ * @priv: User context passed to read callbacks.
+ * @offset: Offset within the NVMEM device.
+ * @val: pointer where to fill the ethernet address
+ * @bytes: Length of the NVMEM cell
+ *
+ * Return: 0 on success, -EINVAL  otherwise.
+ */
+static int sst26vf_sfdp_mac_addr_read(void *priv, unsigned int off,
+				      void *val, size_t bytes)
+{
+	struct spi_nor *nor = priv;
+	struct sfdp *sfdp = nor->sfdp;
+	loff_t offset = off;
+	size_t sfdp_size;
+
+	/*
+	 * Check if the EUI-48 MAC address is programmed in the next six address
+	 * locations.
+	 * @off is programmed in the DT and stores the start of MAC Address
+	 * byte, (off - 1) stores the bit length of the Extended Unique
+	 * Identifier
+	 */
+	if (SST26VF_SFDP_EUI48 != *((u8 *)sfdp->dwords + (offset - 1)))
+		return -EINVAL;
+
+	sfdp_size = sfdp->num_dwords * sizeof(*sfdp->dwords);
+	memory_read_from_buffer(val, bytes, &offset, sfdp->dwords,
+				sfdp_size);
+	return 0;
+}
+
+static struct nvmem_config sst26vf_sfdp_nvmem_config = {
+	.word_size = 1,
+	.stride = 1,
+};
+
+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);
+	}
+
+	return 0;
+}
+
 static const struct spi_nor_fixups sst26vf_nor_fixups = {
 	.late_init = sst26vf_nor_late_init,
+	.post_sfdp = sst26vf_nor_post_sfdp,
 };
 
 static const struct flash_info sst_nor_parts[] = {
-- 
2.25.1

[PATCH 2/2] ARM: dts: microchip: sama5d29_curiosity: Add nvmem-layout in QSPI to describe EUI48 MAC address region

[Manikandan Muralidharan]

Add nvmem-layout in QSPI to describe EUI48 MAC address region.
This is useful for cases where U-Boot is skipped and the Ethernet
MAC address is needed to be configured in Linux.

Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
---
 .../arm/boot/dts/microchip/at91-sama5d29_curiosity.dts | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/microchip/at91-sama5d29_curiosity.dts b/arch/arm/boot/dts/microchip/at91-sama5d29_curiosity.dts
index 7be215781549..81aca8502195 100644
--- a/arch/arm/boot/dts/microchip/at91-sama5d29_curiosity.dts
+++ b/arch/arm/boot/dts/microchip/at91-sama5d29_curiosity.dts
@@ -480,6 +480,16 @@ flash@0 {
 		label = "atmel_qspi1";
 		status = "okay";
 
+		nvmem-layout {
+			compatible = "fixed-layout";
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			mac_address_eui48: mac-address@261 {
+				reg = <0x261 0x6>;
+			};
+		};
+
 		at91bootstrap@0 {
 			label = "at91bootstrap";
 			reg = <0x0 0x40000>;
-- 
2.25.1

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

[Miquel Raynal]

On 05/03/2025 at 15:31:33 +0530, Manikandan Muralidharan <manikandan.m@microchip.com> wrote:

> From: Varshini Rajendran <varshini.rajendran@microchip.com>
>
> EUI identifier and the MAC Address of the Ethernet Interface is stored
> after the SFDP table of contents starting at address 0x260 in the
> QSPI memory.
> Register the entire SFDP region read by the spi-nor (nor->sfdp) into the
> NVMEM framework and read the MAC Address when requested using the nvmem
> properties in the DT by the net drivers.
>
> In kernel the Ethernet MAC address relied on U-Boot env variables or
> generated a random address, which posed challenges for boards without
> on-board EEPROMs or with multiple Ethernet ports.
> This change ensures consistent and reliable MAC address retrieval from QSPI,
> benefiting boards like the sama5d29 curiosity and sam9x75 curiosity.

Do you mean spi-nor have a programmable area in their SFDP table? Isn't
this supposed to be a read-only area written once in factory?

I am not a big fan of exposing the whole SFDP area. I would suggest to
expose just the MAC address. You can make use of nvmem layout drivers if
that is needed.

Thanks,
Miquèl

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

[Michael Walle]

On Wed Mar 5, 2025 at 11:01 AM CET, Manikandan Muralidharan wrote:
> From: Varshini Rajendran <varshini.rajendran@microchip.com>
>
> EUI identifier and the MAC Address of the Ethernet Interface is stored
> after the SFDP table of contents starting at address 0x260 in the
> QSPI memory.
> Register the entire SFDP region read by the spi-nor (nor->sfdp) into the
> NVMEM framework and read the MAC Address when requested using the nvmem
> properties in the DT by the net drivers.
>
> In kernel the Ethernet MAC address relied on U-Boot env variables or
> generated a random address, which posed challenges for boards without
> on-board EEPROMs or with multiple Ethernet ports.
> This change ensures consistent and reliable MAC address retrieval from QSPI,
> benefiting boards like the sama5d29 curiosity and sam9x75 curiosity.
>
> Signed-off-by: Varshini Rajendran <varshini.rajendran@microchip.com>
> [manikandan.m@microchip.com: Integrate the nvmem->read callback framework]
> Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
> ---
>  drivers/mtd/spi-nor/sst.c | 62 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 62 insertions(+)
>
> diff --git a/drivers/mtd/spi-nor/sst.c b/drivers/mtd/spi-nor/sst.c
> index 175211fe6a5e..a0abf201ad41 100644
> --- a/drivers/mtd/spi-nor/sst.c
> +++ b/drivers/mtd/spi-nor/sst.c
> @@ -5,6 +5,7 @@
>   */
>  
>  #include <linux/mtd/spi-nor.h>
> +#include <linux/nvmem-provider.h>
>  
>  #include "core.h"
>  
> @@ -13,6 +14,8 @@
>  
>  #define SST26VF_CR_BPNV		BIT(3)
>  
> +#define SST26VF_SFDP_EUI48	0x30
> +
>  static int sst26vf_nor_lock(struct spi_nor *nor, loff_t ofs, u64 len)
>  {
>  	return -EOPNOTSUPP;
> @@ -56,8 +59,67 @@ static int sst26vf_nor_late_init(struct spi_nor *nor)
>  	return 0;
>  }
>  
> +/**
> + * sst26vf_sfdp_mac_addr_read() - check if the EUI-48 MAC Address is programmed
> + * and read the data from the prestored SFDP data
> + *
> + * @priv: User context passed to read callbacks.
> + * @offset: Offset within the NVMEM device.
> + * @val: pointer where to fill the ethernet address
> + * @bytes: Length of the NVMEM cell
> + *
> + * Return: 0 on success, -EINVAL  otherwise.
> + */
> +static int sst26vf_sfdp_mac_addr_read(void *priv, unsigned int off,
> +				      void *val, size_t bytes)
> +{
> +	struct spi_nor *nor = priv;
> +	struct sfdp *sfdp = nor->sfdp;
> +	loff_t offset = off;
> +	size_t sfdp_size;
> +
> +	/*
> +	 * Check if the EUI-48 MAC address is programmed in the next six address
> +	 * locations.
> +	 * @off is programmed in the DT and stores the start of MAC Address
> +	 * byte, (off - 1) stores the bit length of the Extended Unique
> +	 * Identifier
> +	 */
> +	if (SST26VF_SFDP_EUI48 != *((u8 *)sfdp->dwords + (offset - 1)))
> +		return -EINVAL;

What happens if you read at a different offset? You're exposing
the entire SFDP region. What happens if there is a 0x30 at a
different location?

> +
> +	sfdp_size = sfdp->num_dwords * sizeof(*sfdp->dwords);
> +	memory_read_from_buffer(val, bytes, &offset, sfdp->dwords,
> +				sfdp_size);
> +	return 0;
> +}
> +
> +static struct nvmem_config sst26vf_sfdp_nvmem_config = {
> +	.word_size = 1,
> +	.stride = 1,
> +};
> +
> +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.

-michael

> +	}
> +
> +	return 0;
> +}
> +
>  static const struct spi_nor_fixups sst26vf_nor_fixups = {
>  	.late_init = sst26vf_nor_late_init,
> +	.post_sfdp = sst26vf_nor_post_sfdp,
>  };
>  
>  static const struct flash_info sst_nor_parts[] = {

Re: [PATCH 2/2] ARM: dts: microchip: sama5d29_curiosity: Add nvmem-layout in QSPI to describe EUI48 MAC address region

[Michael Walle]

On Wed Mar 5, 2025 at 11:01 AM CET, Manikandan Muralidharan wrote:
> Add nvmem-layout in QSPI to describe EUI48 MAC address region.
> This is useful for cases where U-Boot is skipped and the Ethernet
> MAC address is needed to be configured in Linux.
>
> Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
> ---
>  .../arm/boot/dts/microchip/at91-sama5d29_curiosity.dts | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/arch/arm/boot/dts/microchip/at91-sama5d29_curiosity.dts b/arch/arm/boot/dts/microchip/at91-sama5d29_curiosity.dts
> index 7be215781549..81aca8502195 100644
> --- a/arch/arm/boot/dts/microchip/at91-sama5d29_curiosity.dts
> +++ b/arch/arm/boot/dts/microchip/at91-sama5d29_curiosity.dts
> @@ -480,6 +480,16 @@ flash@0 {
>  		label = "atmel_qspi1";
>  		status = "okay";
>  
> +		nvmem-layout {
> +			compatible = "fixed-layout";
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +
> +			mac_address_eui48: mac-address@261 {

I don't think the offset should be hardcoded in the device tree.
Apparently it is a property of this very chip (and not backed up by
a JEDEC standard). What happens if the SFDP are changed for this
flash? I.e. The length of the SFDP region changes.

Also, this looks like it is a layout for the SPI flash contents and
not the SFDP, how do you differentiate between these two? Also
please update your device tree to use 'compatible =
"fixed-partitions"'.

-michael

> +				reg = <0x261 0x6>;
> +			};
> +		};



> +
>  		at91bootstrap@0 {
>  			label = "at91bootstrap";
>  			reg = <0x0 0x40000>;

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

[Rob Herring (Arm)]

On Wed, 05 Mar 2025 15:31:33 +0530, Manikandan Muralidharan wrote:
> From: Varshini Rajendran <varshini.rajendran@microchip.com>
> 
> EUI identifier and the MAC Address of the Ethernet Interface is stored
> after the SFDP table of contents starting at address 0x260 in the
> QSPI memory.
> Register the entire SFDP region read by the spi-nor (nor->sfdp) into the
> NVMEM framework and read the MAC Address when requested using the nvmem
> properties in the DT by the net drivers.
> 
> In kernel the Ethernet MAC address relied on U-Boot env variables or
> generated a random address, which posed challenges for boards without
> on-board EEPROMs or with multiple Ethernet ports.
> This change ensures consistent and reliable MAC address retrieval from QSPI,
> benefiting boards like the sama5d29 curiosity and sam9x75 curiosity.
> 
> Signed-off-by: Varshini Rajendran <varshini.rajendran@microchip.com>
> [manikandan.m@microchip.com: Integrate the nvmem->read callback framework]
> Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
> ---
>  drivers/mtd/spi-nor/sst.c | 62 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 62 insertions(+)
> 


My bot found new DTB warnings on the .dts files added or changed in this
series.

Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
are fixed by another series. Ultimately, it is up to the platform
maintainer whether these warnings are acceptable or not. No need to reply
unless the platform maintainer has comments.

If you already ran DT checks and didn't see these error(s), then
make sure dt-schema is up to date:

  pip3 install dtschema --upgrade


New warnings running 'make CHECK_DTBS=y for arch/arm/boot/dts/microchip/' for 20250305100134.1171124-1-manikandan.m@microchip.com:

arch/arm/boot/dts/microchip/at91-sama5d29_curiosity.dtb: flash@0: Unevaluated properties are not allowed ('nvmem-layout' was unexpected)
	from schema $id: http://devicetree.org/schemas/mtd/jedec,spi-nor.yaml#

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

[kernel test robot]

Hi Manikandan,

kernel test robot noticed the following build warnings:

[auto build test WARNING on mtd/spi-nor/next]
[also build test WARNING on robh/for-next abelloni/rtc-next linus/master v6.14-rc5 next-20250305]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Manikandan-Muralidharan/ARM-dts-microchip-sama5d29_curiosity-Add-nvmem-layout-in-QSPI-to-describe-EUI48-MAC-address-region/20250305-180433
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git spi-nor/next
patch link:    https://lore.kernel.org/r/20250305100134.1171124-1-manikandan.m%40microchip.com
patch subject: [PATCH 1/2] mtd: spi-nor: sst: register SFDP region into NVMEM framework to read MAC Address
config: m68k-stmark2_defconfig (https://download.01.org/0day-ci/archive/20250306/202503061244.wH2sylN8-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250306/202503061244.wH2sylN8-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202503061244.wH2sylN8-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/mtd/spi-nor/sst.c:75: warning: Function parameter or struct member 'off' not described in 'sst26vf_sfdp_mac_addr_read'
>> drivers/mtd/spi-nor/sst.c:75: warning: Excess function parameter 'offset' description in 'sst26vf_sfdp_mac_addr_read'


vim +75 drivers/mtd/spi-nor/sst.c

    61	
    62	/**
    63	 * sst26vf_sfdp_mac_addr_read() - check if the EUI-48 MAC Address is programmed
    64	 * and read the data from the prestored SFDP data
    65	 *
    66	 * @priv: User context passed to read callbacks.
    67	 * @offset: Offset within the NVMEM device.
    68	 * @val: pointer where to fill the ethernet address
    69	 * @bytes: Length of the NVMEM cell
    70	 *
    71	 * Return: 0 on success, -EINVAL  otherwise.
    72	 */
    73	static int sst26vf_sfdp_mac_addr_read(void *priv, unsigned int off,
    74					      void *val, size_t bytes)
  > 75	{
    76		struct spi_nor *nor = priv;
    77		struct sfdp *sfdp = nor->sfdp;
    78		loff_t offset = off;
    79		size_t sfdp_size;
    80	
    81		/*
    82		 * Check if the EUI-48 MAC address is programmed in the next six address
    83		 * locations.
    84		 * @off is programmed in the DT and stores the start of MAC Address
    85		 * byte, (off - 1) stores the bit length of the Extended Unique
    86		 * Identifier
    87		 */
    88		if (SST26VF_SFDP_EUI48 != *((u8 *)sfdp->dwords + (offset - 1)))
    89			return -EINVAL;
    90	
    91		sfdp_size = sfdp->num_dwords * sizeof(*sfdp->dwords);
    92		memory_read_from_buffer(val, bytes, &offset, sfdp->dwords,
    93					sfdp_size);
    94		return 0;
    95	}
    96	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

[Michael Walle]

Hi,

> >> From: Varshini Rajendran <varshini.rajendran@microchip.com>
> >>
> >> EUI identifier and the MAC Address of the Ethernet Interface is stored
> >> after the SFDP table of contents starting at address 0x260 in the
> >> QSPI memory.
> >> Register the entire SFDP region read by the spi-nor (nor->sfdp) into the
> >> NVMEM framework and read the MAC Address when requested using the nvmem
> >> properties in the DT by the net drivers.
> >>
> >> In kernel the Ethernet MAC address relied on U-Boot env variables or
> >> generated a random address, which posed challenges for boards without
> >> on-board EEPROMs or with multiple Ethernet ports.
> >> This change ensures consistent and reliable MAC address retrieval from QSPI,
> >> benefiting boards like the sama5d29 curiosity and sam9x75 curiosity.
> > 
> > Do you mean spi-nor have a programmable area in their SFDP table? Isn't
> > this supposed to be a read-only area written once in factory?
> > 
> The SST26VF064BEUI serial quad flash memory is programmed at the factory 
> with a globally unique address stored in the SFDP vendor
> parameter table and it is permanently write-protected.

Why didn't you mention that this is a vendor table in the commit
message?

Anyway, please write proper support in the core for parsing vendor
tables and exposing them as a nvmem device if needed (and asked to
do so by the driver of course).

-michael

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

[Miquel Raynal]

>>> +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

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

[Michael Walle]

> >>> +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.

You don't do it, because there is not even a relative offset that is
fixed. There should be a pointer to the vendor table inside the SFDP
structure. Thus, you need to properly parse it.

Regarding how to reference it within the device tree, I'd assume
something along 'compatible = "jedec,sfdp-vendor-table-NNN";' or
similar. But no static/relative offsets.

-michael

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

[Miquel Raynal]

>> >> 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.
>
> You don't do it, because there is not even a relative offset that is
> fixed. There should be a pointer to the vendor table inside the SFDP
> structure. Thus, you need to properly parse it.

If there is nothing static and the location can be derived by reading
the chip tables, I'd suggest to offload this out of the spi-nor core and
instead use some kind of an nvmem layout driver to retrieve the
corresponding region?

> Regarding how to reference it within the device tree, I'd assume
> something along 'compatible = "jedec,sfdp-vendor-table-NNN";' or
> similar. But no static/relative offsets.

I see. We need to invoke our DT binding gurus.

Cheers,
Miquèl