[PATCH v3 0/3] Read MAC Address from SST vendor specific SFDP region

[PATCH v3 0/3] Read MAC Address from SST vendor specific SFDP region

[Manikandan Muralidharan]

This patch series adds support to parse the SFDP SST vendor map, read and
store the EUI-48 and EUI-64 Address (if its programmed) using the
resource-managed devm_kcalloc which will be freed on driver detach.
Register EUI addresses into NVMEM framework for the net drivers to access
them using nvmem properties.
This change ensures consistent and reliable MAC address retrieval
from QSPI benefiting boards like the sama5d27_wlsom1, sama5d29 curiosity
and sam9x75 curiosity.

--------
changes in v3:
- 2/3 - add support to update the QSPI partition into 'fixed-partition'
	binding in sama5d27_wlsom1
- 3/3 - add nvmem-layout in qspi node for EUI48 MAC Address and nvmem cell
	properties for macb node in sama5d27_wlsom1

changes in v2:
- 1/3 - parse the SST vendor table, read and store the addresses
	into a resource - managed space. Register the addresses
	into NVMEM framework
- 2/3 - add support to update the QSPI partition into 'fixed-partition'
	binding
--------

Manikandan Muralidharan (3):
  mtd: spi-nor: sfdp: parse SFDP SST vendor map and register EUI
    addresses into NVMEM framework
  ARM: dts: microchip: sama5d27_wlsom1: update the QSPI partitions using
    "fixed-partition" binding
  ARM: dts: microchip: sama5d27_wlsom1: Add nvmem-layout in QSPI for
    EUI48 MAC Address

 .../dts/microchip/at91-sama5d27_wlsom1.dtsi   |  65 ++++---
 drivers/mtd/spi-nor/sfdp.c                    | 161 ++++++++++++++++++
 include/linux/mtd/spi-nor.h                   |   7 +
 3 files changed, 209 insertions(+), 24 deletions(-)

-- 
2.25.1

[PATCH v3 1/3] mtd: spi-nor: sfdp: parse SFDP SST vendor map and register EUI addresses into NVMEM framework

[Manikandan Muralidharan]

Some SST flash like SST26VF064BEUI serial quad flash memory is programmed
at the factory with a globally unique EUI-48 and EUI-64 identifiers stored
in the SFDP vendor parameter table and it is permanently write-protected.

Add SST Vendor table SFDP parser to read the EUI-48 and EUI-64
Mac Addresses and allocate them using resource-managed devm_kcalloc
which will be freed on driver detach.

Regitser the Addresses into NVMEM framework and parse them 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 sama5d27-wlsom1-ek, sama5d29 curiosity
and sam9x75 curiosity.

Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
---
 drivers/mtd/spi-nor/sfdp.c  | 161 ++++++++++++++++++++++++++++++++++++
 include/linux/mtd/spi-nor.h |   7 ++
 2 files changed, 168 insertions(+)

diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
index 21727f9a4ac6..920708ae928a 100644
--- a/drivers/mtd/spi-nor/sfdp.c
+++ b/drivers/mtd/spi-nor/sfdp.c
@@ -31,6 +31,7 @@
 					 * Register Map Offsets for Multi-Chip
 					 * SPI Memory Devices.
 					 */
+#define SFDP_MCHP_SST_ID	0x01bf
 
 #define SFDP_SIGNATURE		0x50444653U
 
@@ -1344,6 +1345,163 @@ static int spi_nor_parse_sccr_mc(struct spi_nor *nor,
 	return ret;
 }
 
+#define SFDP_MCHP_PARAM_TABLE_LEN	28
+#define SFDP_SST26VF064BEUI_ID		0xFF4326BFU
+
+#define SFDP_MCHP_EUI48			0x30
+#define SFDP_MCHP_EUI48_MASK		GENMASK(7, 0)
+#define SFDP_MCHP_EUI48_MAC_LEN		6
+
+#define SFDP_MCHP_EUI64			0x40
+#define SFDP_MCHP_EUI64_MASK		GENMASK(31, 24)
+#define SFDP_MCHP_EUI64_MAC_LEN		8
+
+/**
+ * spi_nor_mchp_sfdp_read_addr()- read callback to copy the EUI-48 or EUI-68
+ *				  Addresses for device that request via NVMEM
+ *
+ * @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 spi_nor_mchp_sfdp_read_addr(void *priv, unsigned int off,
+				       void *val, size_t bytes)
+{
+	struct spi_nor *nor = priv;
+
+	if (SFDP_MCHP_PARAM_TABLE_LEN == nor->mchp_eui->vendor_param_length) {
+		switch (bytes) {
+		case SFDP_MCHP_EUI48_MAC_LEN:
+			memcpy(val, nor->mchp_eui->ethaddr_eui48, SFDP_MCHP_EUI48_MAC_LEN);
+			break;
+		case SFDP_MCHP_EUI64_MAC_LEN:
+			memcpy(val, nor->mchp_eui->ethaddr_eui64, SFDP_MCHP_EUI64_MAC_LEN);
+			break;
+		default:
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
+/**
+ * spi_nor_parse_mchp_sfdp() - Parse the Microchip vendor specific parameter table
+ *			       Read and store the EUI-48 and EUI-64 address to
+ *			       struct spi_nor_sst_mchp_eui_info if the addresses are
+ *			       programmed in the SST26VF064BEUI sst flag
+ *
+ * @nor:		pointer to a 'struct spi_nor'
+ * @sccr_header:	pointer to the 'struct sfdp_parameter_header' describing
+ *			the Microchip vendor parameter header length and version.
+ *
+ * Return: 0 on success of if addresses are not programmed, -errno otherwise.
+ */
+static int spi_nor_parse_mchp_sfdp(struct spi_nor *nor,
+				   const struct sfdp_parameter_header *mchp_header)
+{
+	struct nvmem_device *nvmem;
+	struct nvmem_config nvmem_config = { };
+	struct spi_nor_sst_mchp_eui_info *mchp_eui;
+	u32 *dwords, addr, sst_flash_id;
+	size_t len;
+	int ret = 0, size = 0;
+
+	if (SFDP_MCHP_PARAM_TABLE_LEN != mchp_header->length)
+		return -EINVAL;
+
+	addr = SFDP_PARAM_HEADER_PTP(mchp_header);
+	/* Get the SST SPI NOR FLASH ID */
+	ret = spi_nor_read_sfdp_dma_unsafe(nor, addr, sizeof(sst_flash_id),
+					   &sst_flash_id);
+	if (ret < 0)
+		return ret;
+
+	/* Check the SPI NOR FLASH ID */
+	if (le32_to_cpu(sst_flash_id) != SFDP_SST26VF064BEUI_ID)
+		return -EINVAL;
+
+	len = mchp_header->length * sizeof(*dwords);
+	dwords = kmalloc(len, GFP_KERNEL);
+	if (!dwords)
+		return -ENOMEM;
+
+	ret = spi_nor_read_sfdp(nor, addr, len, dwords);
+	if (ret)
+		goto out;
+
+	le32_to_cpu_array(dwords, mchp_header->length);
+
+	mchp_eui = devm_kzalloc(nor->dev, sizeof(*mchp_eui), GFP_KERNEL);
+	if (!mchp_eui) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	if (SFDP_MCHP_EUI48 == FIELD_GET(SFDP_MCHP_EUI48_MASK,
+					 dwords[SFDP_DWORD(25)])) {
+		mchp_eui->ethaddr_eui48 = devm_kcalloc(nor->dev,
+						       SFDP_MCHP_EUI48_MAC_LEN,
+						       sizeof(u8), GFP_KERNEL);
+		if (!mchp_eui->ethaddr_eui48) {
+			ret = -ENOMEM;
+			devm_kfree(nor->dev, mchp_eui);
+			goto out;
+		}
+		memcpy(mchp_eui->ethaddr_eui48, (u8 *)&dwords[SFDP_DWORD(25)] + 1,
+		       SFDP_MCHP_EUI48_MAC_LEN);
+		size = SFDP_MCHP_EUI48_MAC_LEN;
+	}
+
+	if (SFDP_MCHP_EUI64 == FIELD_GET(SFDP_MCHP_EUI64_MASK,
+					 dwords[SFDP_DWORD(26)])) {
+		mchp_eui->ethaddr_eui64 = devm_kcalloc(nor->dev,
+						       SFDP_MCHP_EUI64_MAC_LEN,
+						       sizeof(u8), GFP_KERNEL);
+		if (!mchp_eui->ethaddr_eui64) {
+			ret = -ENOMEM;
+			devm_kfree(nor->dev, mchp_eui->ethaddr_eui48);
+			devm_kfree(nor->dev, mchp_eui);
+			goto out;
+		}
+		memcpy(mchp_eui->ethaddr_eui64, (u8 *)&dwords[SFDP_DWORD(27)],
+		       SFDP_MCHP_EUI64_MAC_LEN);
+		size += SFDP_MCHP_EUI64_MAC_LEN;
+	}
+
+	/*
+	 * Return if SST26VF064BEUI sst flash is not programmed
+	 * with EUI-48 or EUI-64 information
+	 */
+	if (!size) {
+		devm_kfree(nor->dev, mchp_eui);
+		goto out;
+	}
+
+	mchp_eui->vendor_param_length = mchp_header->length;
+	nor->mchp_eui = mchp_eui;
+	nvmem_config.word_size = 1;
+	nvmem_config.stride = 1;
+	nvmem_config.dev = nor->dev;
+	nvmem_config.size = size;
+	nvmem_config.priv = nor;
+	nvmem_config.reg_read = spi_nor_mchp_sfdp_read_addr;
+
+	nvmem = devm_nvmem_register(nor->dev, &nvmem_config);
+	if (IS_ERR(nvmem)) {
+		dev_err(nor->dev, "failed to register NVMEM device: %ld\n",
+			PTR_ERR(nvmem));
+		ret = PTR_ERR(nvmem);
+	}
+
+out:
+	kfree(dwords);
+	return ret;
+}
+
 /**
  * spi_nor_post_sfdp_fixups() - Updates the flash's parameters and settings
  * after SFDP has been parsed. Called only for flashes that define JESD216 SFDP
@@ -1564,6 +1722,9 @@ int spi_nor_parse_sfdp(struct spi_nor *nor)
 			err = spi_nor_parse_sccr_mc(nor, param_header);
 			break;
 
+		case SFDP_MCHP_SST_ID:
+			err = spi_nor_parse_mchp_sfdp(nor, param_header);
+			break;
 		default:
 			break;
 		}
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index cdcfe0fd2e7d..051078d23ea1 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -339,6 +339,12 @@ struct flash_info;
 struct spi_nor_manufacturer;
 struct spi_nor_flash_parameter;
 
+struct spi_nor_sst_mchp_eui_info {
+	u8 vendor_param_length;
+	u8 *ethaddr_eui48;
+	u8 *ethaddr_eui64;
+};
+
 /**
  * struct spi_nor - Structure for defining the SPI NOR layer
  * @mtd:		an mtd_info structure
@@ -408,6 +414,7 @@ struct spi_nor {
 	u32			flags;
 	enum spi_nor_cmd_ext	cmd_ext_type;
 	struct sfdp		*sfdp;
+	struct spi_nor_sst_mchp_eui_info *mchp_eui;
 	struct dentry		*debugfs_root;
 
 	const struct spi_nor_controller_ops *controller_ops;
-- 
2.25.1

[PATCH v3 2/3] ARM: dts: microchip: sama5d27_wlsom1: update the QSPI partitions using "fixed-partition" binding

[Manikandan Muralidharan]

update the QSPI partitions using "fixed-partition" binding

Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
---
 .../dts/microchip/at91-sama5d27_wlsom1.dtsi   | 54 ++++++++++---------
 1 file changed, 29 insertions(+), 25 deletions(-)

diff --git a/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi b/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi
index 9543214adc9f..b34c5072425a 100644
--- a/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi
+++ b/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi
@@ -229,8 +229,6 @@ &qspi1 {
 	status = "disabled";
 
 	qspi1_flash: flash@0 {
-		#address-cells = <1>;
-		#size-cells = <1>;
 		compatible = "jedec,spi-nor";
 		reg = <0>;
 		spi-max-frequency = <104000000>;
@@ -240,34 +238,40 @@ qspi1_flash: flash@0 {
 		m25p,fast-read;
 		status = "disabled";
 
-		at91bootstrap@0 {
-			label = "at91bootstrap";
-			reg = <0x0 0x40000>;
-		};
+		partitions {
+			compatible = "fixed-partitions";
+			#address-cells = <1>;
+			#size-cells = <1>;
 
-		bootloader@40000 {
-			label = "bootloader";
-			reg = <0x40000 0xc0000>;
-		};
+			at91bootstrap@0 {
+				label = "at91bootstrap";
+				reg = <0x0 0x40000>;
+			};
 
-		bootloaderenvred@100000 {
-			label = "bootloader env redundant";
-			reg = <0x100000 0x40000>;
-		};
+			bootloader@40000 {
+				label = "bootloader";
+				reg = <0x40000 0xc0000>;
+			};
 
-		bootloaderenv@140000 {
-			label = "bootloader env";
-			reg = <0x140000 0x40000>;
-		};
+			bootloaderenvred@100000 {
+				label = "bootloader env redundant";
+				reg = <0x100000 0x40000>;
+			};
 
-		dtb@180000 {
-			label = "device tree";
-			reg = <0x180000 0x80000>;
-		};
+			bootloaderenv@140000 {
+				label = "bootloader env";
+				reg = <0x140000 0x40000>;
+			};
 
-		kernel@200000 {
-			label = "kernel";
-			reg = <0x200000 0x600000>;
+			dtb@180000 {
+				label = "device tree";
+				reg = <0x180000 0x80000>;
+			};
+
+			kernel@200000 {
+				label = "kernel";
+				reg = <0x200000 0x600000>;
+			};
 		};
 	};
 };
-- 
2.25.1

[PATCH v3 3/3] ARM: dts: microchip: sama5d27_wlsom1: Add nvmem-layout in QSPI for EUI48 MAC Address

[Manikandan Muralidharan]

Add nvmem-layout in QSPI to read the EUI48 Mac address by the
net drivers using the nvmem property.The offset is set to 0x0
since the factory programmed address is available in the
resource managed space and the size determine if the requested
address is of EUI48 (0x6) or EUI-64 (0x8) type.
This is useful for cases where U-Boot is skipped and the Ethernet
MAC address is needed to be configured by the kernel

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

diff --git a/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi b/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi
index b34c5072425a..be06df1b7d66 100644
--- a/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi
+++ b/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi
@@ -210,6 +210,9 @@ &macb0 {
 	#size-cells = <0>;
 	phy-mode = "rmii";
 
+	nvmem-cells = <&mac_address_eui48>;
+	nvmem-cell-names = "mac-address";
+
 	ethernet-phy@0 {
 		reg = <0x0>;
 		interrupt-parent = <&pioA>;
@@ -238,6 +241,16 @@ qspi1_flash: flash@0 {
 		m25p,fast-read;
 		status = "disabled";
 
+		nvmem-layout {
+			compatible = "fixed-layout";
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			mac_address_eui48: mac-address@0 {
+				reg = <0x0 0x6>;
+			};
+		};
+
 		partitions {
 			compatible = "fixed-partitions";
 			#address-cells = <1>;
-- 
2.25.1

Re: [PATCH v3 0/3] Read MAC Address from SST vendor specific SFDP region

[Rob Herring (Arm)]

On Wed, 21 May 2025 12:33:33 +0530, Manikandan Muralidharan wrote:
> This patch series adds support to parse the SFDP SST vendor map, read and
> store the EUI-48 and EUI-64 Address (if its programmed) using the
> resource-managed devm_kcalloc which will be freed on driver detach.
> Register EUI addresses into NVMEM framework for the net drivers to access
> them using nvmem properties.
> This change ensures consistent and reliable MAC address retrieval
> from QSPI benefiting boards like the sama5d27_wlsom1, sama5d29 curiosity
> and sam9x75 curiosity.
> 
> --------
> changes in v3:
> - 2/3 - add support to update the QSPI partition into 'fixed-partition'
> 	binding in sama5d27_wlsom1
> - 3/3 - add nvmem-layout in qspi node for EUI48 MAC Address and nvmem cell
> 	properties for macb node in sama5d27_wlsom1
> 
> changes in v2:
> - 1/3 - parse the SST vendor table, read and store the addresses
> 	into a resource - managed space. Register the addresses
> 	into NVMEM framework
> - 2/3 - add support to update the QSPI partition into 'fixed-partition'
> 	binding
> --------
> 
> Manikandan Muralidharan (3):
>   mtd: spi-nor: sfdp: parse SFDP SST vendor map and register EUI
>     addresses into NVMEM framework
>   ARM: dts: microchip: sama5d27_wlsom1: update the QSPI partitions using
>     "fixed-partition" binding
>   ARM: dts: microchip: sama5d27_wlsom1: Add nvmem-layout in QSPI for
>     EUI48 MAC Address
> 
>  .../dts/microchip/at91-sama5d27_wlsom1.dtsi   |  65 ++++---
>  drivers/mtd/spi-nor/sfdp.c                    | 161 ++++++++++++++++++
>  include/linux/mtd/spi-nor.h                   |   7 +
>  3 files changed, 209 insertions(+), 24 deletions(-)
> 
> --
> 2.25.1
> 
> 
> 


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


This patch series was applied (using b4) to base:
 Base: attempting to guess base-commit...
 Base: tags/next-20250516 (best guess, 2/3 blobs matched)

If this is not the correct base, please add 'base-commit' tag
(or use b4 which does this automatically)

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

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

Re: [PATCH v3 1/3] mtd: spi-nor: sfdp: parse SFDP SST vendor map and register EUI addresses into NVMEM framework

[Claudiu Beznea]

Hi, Manikandan,

On 21.05.2025 10:03, Manikandan Muralidharan wrote:
> Some SST flash like SST26VF064BEUI serial quad flash memory is programmed
> at the factory with a globally unique EUI-48 and EUI-64 identifiers stored
> in the SFDP vendor parameter table and it is permanently write-protected.
> 
> Add SST Vendor table SFDP parser to read the EUI-48 and EUI-64
> Mac Addresses and allocate them using resource-managed devm_kcalloc
> which will be freed on driver detach.
> 
> Regitser the Addresses into NVMEM framework and parse them 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 sama5d27-wlsom1-ek, sama5d29 curiosity
> and sam9x75 curiosity.
> 
> Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
> ---
>  drivers/mtd/spi-nor/sfdp.c  | 161 ++++++++++++++++++++++++++++++++++++
>  include/linux/mtd/spi-nor.h |   7 ++
>  2 files changed, 168 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
> index 21727f9a4ac6..920708ae928a 100644
> --- a/drivers/mtd/spi-nor/sfdp.c
> +++ b/drivers/mtd/spi-nor/sfdp.c
> @@ -31,6 +31,7 @@
>  					 * Register Map Offsets for Multi-Chip
>  					 * SPI Memory Devices.
>  					 */
> +#define SFDP_MCHP_SST_ID	0x01bf
>  
>  #define SFDP_SIGNATURE		0x50444653U
>  
> @@ -1344,6 +1345,163 @@ static int spi_nor_parse_sccr_mc(struct spi_nor *nor,
>  	return ret;
>  }
>  
> +#define SFDP_MCHP_PARAM_TABLE_LEN	28
> +#define SFDP_SST26VF064BEUI_ID		0xFF4326BFU
> +
> +#define SFDP_MCHP_EUI48			0x30
> +#define SFDP_MCHP_EUI48_MASK		GENMASK(7, 0)
> +#define SFDP_MCHP_EUI48_MAC_LEN		6
> +
> +#define SFDP_MCHP_EUI64			0x40
> +#define SFDP_MCHP_EUI64_MASK		GENMASK(31, 24)
> +#define SFDP_MCHP_EUI64_MAC_LEN		8
> +
> +/**
> + * spi_nor_mchp_sfdp_read_addr()- read callback to copy the EUI-48 or EUI-68
> + *				  Addresses for device that request via NVMEM
> + *
> + * @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 spi_nor_mchp_sfdp_read_addr(void *priv, unsigned int off,
> +				       void *val, size_t bytes)
> +{
> +	struct spi_nor *nor = priv;
> +
> +	if (SFDP_MCHP_PARAM_TABLE_LEN == nor->mchp_eui->vendor_param_length) {

From checkpatch.pl:

[Checkpatch]      WARNING: Comparisons should place the constant on the
right side of the test
#71: FILE: drivers/mtd/spi-nor/sfdp.c:1375:
+       if (SFDP_MCHP_PARAM_TABLE_LEN == nor->mchp_eui->vendor_param_length) {

Also, to avoid indenting the above block you can reverse the check here,
and return, e.g.:

if (nor->mchp_eui->vendor_param_length != SFDP_MCHP_PARAM_TABLE_LEN)
        return 0;

> +		switch (bytes) {
> +		case SFDP_MCHP_EUI48_MAC_LEN:
> +			memcpy(val, nor->mchp_eui->ethaddr_eui48, SFDP_MCHP_EUI48_MAC_LEN);
> +			break;
> +		case SFDP_MCHP_EUI64_MAC_LEN:
> +			memcpy(val, nor->mchp_eui->ethaddr_eui64, SFDP_MCHP_EUI64_MAC_LEN);
> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * spi_nor_parse_mchp_sfdp() - Parse the Microchip vendor specific parameter table
> + *			       Read and store the EUI-48 and EUI-64 address to
> + *			       struct spi_nor_sst_mchp_eui_info if the addresses are
> + *			       programmed in the SST26VF064BEUI sst flag
> + *
> + * @nor:		pointer to a 'struct spi_nor'
> + * @sccr_header:	pointer to the 'struct sfdp_parameter_header' describing
> + *			the Microchip vendor parameter header length and version.
> + *
> + * Return: 0 on success of if addresses are not programmed, -errno otherwise.
> + */
> +static int spi_nor_parse_mchp_sfdp(struct spi_nor *nor,
> +				   const struct sfdp_parameter_header *mchp_header)
> +{
> +	struct nvmem_device *nvmem;
> +	struct nvmem_config nvmem_config = { };
> +	struct spi_nor_sst_mchp_eui_info *mchp_eui;
> +	u32 *dwords, addr, sst_flash_id;
> +	size_t len;
> +	int ret = 0, size = 0;
> +
> +	if (SFDP_MCHP_PARAM_TABLE_LEN != mchp_header->length)

From checkpatch.pl:

WARNING: Comparisons should place the constant on the right side of the test
#109: FILE: drivers/mtd/spi-nor/sfdp.c:1413:
+       if (SFDP_MCHP_PARAM_TABLE_LEN != mchp_header->length)

> +		return -EINVAL;
> +
> +	addr = SFDP_PARAM_HEADER_PTP(mchp_header);
> +	/* Get the SST SPI NOR FLASH ID */
> +	ret = spi_nor_read_sfdp_dma_unsafe(nor, addr, sizeof(sst_flash_id),
> +					   &sst_flash_id);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Check the SPI NOR FLASH ID */
> +	if (le32_to_cpu(sst_flash_id) != SFDP_SST26VF064BEUI_ID)
> +		return -EINVAL;
> +
> +	len = mchp_header->length * sizeof(*dwords);
> +	dwords = kmalloc(len, GFP_KERNEL);

I think this can be replaced by:

u32 *dwords __free(kfree) = kmalloc(...);

> +	if (!dwords)
> +		return -ENOMEM;
> +
> +	ret = spi_nor_read_sfdp(nor, addr, len, dwords);
> +	if (ret)
> +		goto out;
> +
> +	le32_to_cpu_array(dwords, mchp_header->length);
> +
> +	mchp_eui = devm_kzalloc(nor->dev, sizeof(*mchp_eui), GFP_KERNEL);
> +	if (!mchp_eui) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +
> +	if (SFDP_MCHP_EUI48 == FIELD_GET(SFDP_MCHP_EUI48_MASK,
> +					 dwords[SFDP_DWORD(25)])) {
> +		mchp_eui->ethaddr_eui48 = devm_kcalloc(nor->dev,
> +						       SFDP_MCHP_EUI48_MAC_LEN,
> +						       sizeof(u8), GFP_KERNEL);
> +		if (!mchp_eui->ethaddr_eui48) {
> +			ret = -ENOMEM;
> +			devm_kfree(nor->dev, mchp_eui);
> +			goto out;
> +		}
> +		memcpy(mchp_eui->ethaddr_eui48, (u8 *)&dwords[SFDP_DWORD(25)] + 1,
> +		       SFDP_MCHP_EUI48_MAC_LEN);
> +		size = SFDP_MCHP_EUI48_MAC_LEN;
> +	}
> +
> +	if (SFDP_MCHP_EUI64 == FIELD_GET(SFDP_MCHP_EUI64_MASK,
> +					 dwords[SFDP_DWORD(26)])) {
> +		mchp_eui->ethaddr_eui64 = devm_kcalloc(nor->dev,
> +						       SFDP_MCHP_EUI64_MAC_LEN,
> +						       sizeof(u8), GFP_KERNEL);
> +		if (!mchp_eui->ethaddr_eui64) {
> +			ret = -ENOMEM;
> +			devm_kfree(nor->dev, mchp_eui->ethaddr_eui48);
> +			devm_kfree(nor->dev, mchp_eui);
> +			goto out;
> +		}
> +		memcpy(mchp_eui->ethaddr_eui64, (u8 *)&dwords[SFDP_DWORD(27)],
> +		       SFDP_MCHP_EUI64_MAC_LEN);
> +		size += SFDP_MCHP_EUI64_MAC_LEN;
> +	}
> +
> +	/*
> +	 * Return if SST26VF064BEUI sst flash is not programmed
> +	 * with EUI-48 or EUI-64 information
> +	 */
> +	if (!size) {
> +		devm_kfree(nor->dev, mchp_eui);
> +		goto out;
> +	}
> +
> +	mchp_eui->vendor_param_length = mchp_header->length;
> +	nor->mchp_eui = mchp_eui;
> +	nvmem_config.word_size = 1;
> +	nvmem_config.stride = 1;
> +	nvmem_config.dev = nor->dev;
> +	nvmem_config.size = size;
> +	nvmem_config.priv = nor;
> +	nvmem_config.reg_read = spi_nor_mchp_sfdp_read_addr;
> +
> +	nvmem = devm_nvmem_register(nor->dev, &nvmem_config);
> +	if (IS_ERR(nvmem)) {
> +		dev_err(nor->dev, "failed to register NVMEM device: %ld\n",
> +			PTR_ERR(nvmem));
> +		ret = PTR_ERR(nvmem);
> +	}
> +
> +out:
> +	kfree(dwords);
> +	return ret;
> +}
> +
>  /**
>   * spi_nor_post_sfdp_fixups() - Updates the flash's parameters and settings
>   * after SFDP has been parsed. Called only for flashes that define JESD216 SFDP
> @@ -1564,6 +1722,9 @@ int spi_nor_parse_sfdp(struct spi_nor *nor)
>  			err = spi_nor_parse_sccr_mc(nor, param_header);
>  			break;
>  
> +		case SFDP_MCHP_SST_ID:
> +			err = spi_nor_parse_mchp_sfdp(nor, param_header);
> +			break;
>  		default:
>  			break;
>  		}
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index cdcfe0fd2e7d..051078d23ea1 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -339,6 +339,12 @@ struct flash_info;
>  struct spi_nor_manufacturer;
>  struct spi_nor_flash_parameter;
>  
> +struct spi_nor_sst_mchp_eui_info {
> +	u8 vendor_param_length;
> +	u8 *ethaddr_eui48;
> +	u8 *ethaddr_eui64;

you may want to keep pointer first to avoid padding, if any.

Thank you,
Claudiu

> +};
> +
>  /**
>   * struct spi_nor - Structure for defining the SPI NOR layer
>   * @mtd:		an mtd_info structure
> @@ -408,6 +414,7 @@ struct spi_nor {
>  	u32			flags;
>  	enum spi_nor_cmd_ext	cmd_ext_type;
>  	struct sfdp		*sfdp;
> +	struct spi_nor_sst_mchp_eui_info *mchp_eui;
>  	struct dentry		*debugfs_root;
>  
>  	const struct spi_nor_controller_ops *controller_ops;

Re: [PATCH v3 2/3] ARM: dts: microchip: sama5d27_wlsom1: update the QSPI partitions using "fixed-partition" binding

[Claudiu Beznea]

Hi, Manikandan,

On 21.05.2025 10:03, Manikandan Muralidharan wrote:
> update the QSPI partitions using "fixed-partition" binding

Please use capital letter at the beginning of the sentence  and dot at the
end of it.

Can you please also explain here why you did this update?

Thank you,
Claudiu

> 
> Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
> ---
>  .../dts/microchip/at91-sama5d27_wlsom1.dtsi   | 54 ++++++++++---------
>  1 file changed, 29 insertions(+), 25 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi b/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi
> index 9543214adc9f..b34c5072425a 100644
> --- a/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi
> +++ b/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi
> @@ -229,8 +229,6 @@ &qspi1 {
>  	status = "disabled";
>  
>  	qspi1_flash: flash@0 {
> -		#address-cells = <1>;
> -		#size-cells = <1>;
>  		compatible = "jedec,spi-nor";
>  		reg = <0>;
>  		spi-max-frequency = <104000000>;
> @@ -240,34 +238,40 @@ qspi1_flash: flash@0 {
>  		m25p,fast-read;
>  		status = "disabled";
>  
> -		at91bootstrap@0 {
> -			label = "at91bootstrap";
> -			reg = <0x0 0x40000>;
> -		};
> +		partitions {
> +			compatible = "fixed-partitions";
> +			#address-cells = <1>;
> +			#size-cells = <1>;
>  
> -		bootloader@40000 {
> -			label = "bootloader";
> -			reg = <0x40000 0xc0000>;
> -		};
> +			at91bootstrap@0 {
> +				label = "at91bootstrap";
> +				reg = <0x0 0x40000>;
> +			};
>  
> -		bootloaderenvred@100000 {
> -			label = "bootloader env redundant";
> -			reg = <0x100000 0x40000>;
> -		};
> +			bootloader@40000 {
> +				label = "bootloader";
> +				reg = <0x40000 0xc0000>;
> +			};
>  
> -		bootloaderenv@140000 {
> -			label = "bootloader env";
> -			reg = <0x140000 0x40000>;
> -		};
> +			bootloaderenvred@100000 {
> +				label = "bootloader env redundant";
> +				reg = <0x100000 0x40000>;
> +			};
>  
> -		dtb@180000 {
> -			label = "device tree";
> -			reg = <0x180000 0x80000>;
> -		};
> +			bootloaderenv@140000 {
> +				label = "bootloader env";
> +				reg = <0x140000 0x40000>;
> +			};
>  
> -		kernel@200000 {
> -			label = "kernel";
> -			reg = <0x200000 0x600000>;
> +			dtb@180000 {
> +				label = "device tree";
> +				reg = <0x180000 0x80000>;
> +			};
> +
> +			kernel@200000 {
> +				label = "kernel";
> +				reg = <0x200000 0x600000>;
> +			};
>  		};
>  	};
>  };

Re: [PATCH v3 1/3] mtd: spi-nor: sfdp: parse SFDP SST vendor map and register EUI addresses into NVMEM framework

[Tudor Ambarus]

Hi,

On 5/21/25 8:03 AM, Manikandan Muralidharan wrote:
>  drivers/mtd/spi-nor/sfdp.c  | 161 ++++++++++++++++++++++++++++++++++++
>  include/linux/mtd/spi-nor.h |   7 ++
>  2 files changed, 168 insertions(+)

Please find a way to move the vendor specific handling to a vendor file,
don't pollute the core drivers. In what concerns the nvmem idea, I find
it fine. Michael may comment more on it as I think he dealt with a
similar use case in the past.

Cheers,
ta

Re: [PATCH v3 3/3] ARM: dts: microchip: sama5d27_wlsom1: Add nvmem-layout in QSPI for EUI48 MAC Address

[Tudor Ambarus]

On 5/21/25 8:03 AM, Manikandan Muralidharan wrote:
> Add nvmem-layout in QSPI to read the EUI48 Mac address by the
> net drivers using the nvmem property.The offset is set to 0x0
> since the factory programmed address is available in the
> resource managed space and the size determine if the requested
> address is of EUI48 (0x6) or EUI-64 (0x8) type.
> This is useful for cases where U-Boot is skipped and the Ethernet
> MAC address is needed to be configured by the kernel
> 
> Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
> ---
>  .../boot/dts/microchip/at91-sama5d27_wlsom1.dtsi    | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi b/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi
> index b34c5072425a..be06df1b7d66 100644
> --- a/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi
> +++ b/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi
> @@ -210,6 +210,9 @@ &macb0 {
>  	#size-cells = <0>;
>  	phy-mode = "rmii";
>  
> +	nvmem-cells = <&mac_address_eui48>;
> +	nvmem-cell-names = "mac-address";
> +
>  	ethernet-phy@0 {
>  		reg = <0x0>;
>  		interrupt-parent = <&pioA>;
> @@ -238,6 +241,16 @@ qspi1_flash: flash@0 {
>  		m25p,fast-read;
>  		status = "disabled";
>  
> +		nvmem-layout {
> +			compatible = "fixed-layout";
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +
> +			mac_address_eui48: mac-address@0 {
> +				reg = <0x0 0x6>;
> +			};

How would this work if in the future the mchp vendor table adds some
other info that needs to be referenced as nvmem? How do you distinguish
the info from the table?
Would it be possible to have some kind of address and size to reference
the SFDP?

> +		};
> +
>  		partitions {
>  			compatible = "fixed-partitions";
>  			#address-cells = <1>;

Re: [PATCH v3 3/3] ARM: dts: microchip: sama5d27_wlsom1: Add nvmem-layout in QSPI for EUI48 MAC Address

[Manikandan.M]

Hi Tudor,

On 09/06/25 1:47 pm, Tudor Ambarus wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 5/21/25 8:03 AM, Manikandan Muralidharan wrote:
>> Add nvmem-layout in QSPI to read the EUI48 Mac address by the
>> net drivers using the nvmem property.The offset is set to 0x0
>> since the factory programmed address is available in the
>> resource managed space and the size determine if the requested
>> address is of EUI48 (0x6) or EUI-64 (0x8) type.
>> This is useful for cases where U-Boot is skipped and the Ethernet
>> MAC address is needed to be configured by the kernel
>>
>> Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
>> ---
>>   .../boot/dts/microchip/at91-sama5d27_wlsom1.dtsi    | 13 +++++++++++++
>>   1 file changed, 13 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi b/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi
>> index b34c5072425a..be06df1b7d66 100644
>> --- a/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi
>> +++ b/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi
>> @@ -210,6 +210,9 @@ &macb0 {
>>        #size-cells = <0>;
>>        phy-mode = "rmii";
>>
>> +     nvmem-cells = <&mac_address_eui48>;
>> +     nvmem-cell-names = "mac-address";
>> +
>>        ethernet-phy@0 {
>>                reg = <0x0>;
>>                interrupt-parent = <&pioA>;
>> @@ -238,6 +241,16 @@ qspi1_flash: flash@0 {
>>                m25p,fast-read;
>>                status = "disabled";
>>
>> +             nvmem-layout {
>> +                     compatible = "fixed-layout";
>> +                     #address-cells = <1>;
>> +                     #size-cells = <1>;
>> +
>> +                     mac_address_eui48: mac-address@0 {
>> +                             reg = <0x0 0x6>;
>> +                     };
> 
> How would this work if in the future the mchp vendor table adds some
> other info that needs to be referenced as nvmem? How do you distinguish
> the info from the table?
> Would it be possible to have some kind of address and size to reference
> the SFDP?

I was previously advised not to hardcode the offset in the Device Tree 
[1]. In the current implementation (patch 1/3), the read callback for 
the MCHP vendor table distinguishes between EUI-48 and EUI-64 MAC 
addresses based on the nvmem size, which corresponds to the size of the 
respective MAC address.

[1] --> https://lore.kernel.org/lkml/D889HZF97H8U.1UUX54BAVLAC3@kernel.org/

> 
>> +             };
>> +
>>                partitions {
>>                        compatible = "fixed-partitions";
>>                        #address-cells = <1>;
> 

-- 
Thanks and Regards,
Manikandan M.

Re: [PATCH v3 1/3] mtd: spi-nor: sfdp: parse SFDP SST vendor map and register EUI addresses into NVMEM framework

[Michael Walle]

[-- Attachment #1: Type: text/plain, Size: 797 bytes --]

On Mon Jun 9, 2025 at 10:04 AM CEST, Tudor Ambarus wrote:
> Hi,
>
> On 5/21/25 8:03 AM, Manikandan Muralidharan wrote:
> >  drivers/mtd/spi-nor/sfdp.c  | 161 ++++++++++++++++++++++++++++++++++++
> >  include/linux/mtd/spi-nor.h |   7 ++
> >  2 files changed, 168 insertions(+)
>
> Please find a way to move the vendor specific handling to a vendor file,
> don't pollute the core drivers.

IIRC I've suggested to move the handling into the core, but it
should be more generic, not handling any MAC address related things,
but more like a framework/helper functions to expose anything
related to SFDP.

-michael

> In what concerns the nvmem idea, I find
> it fine. Michael may comment more on it as I think he dealt with a
> similar use case in the past.
>
> Cheers,
> ta


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]

Re: [PATCH v3 3/3] ARM: dts: microchip: sama5d27_wlsom1: Add nvmem-layout in QSPI for EUI48 MAC Address

[Michael Walle]

[-- Attachment #1: Type: text/plain, Size: 3690 bytes --]

Hi,

On Mon Jun 9, 2025 at 11:27 AM CEST, Manikandan.M wrote:
> >> Add nvmem-layout in QSPI to read the EUI48 Mac address by the
> >> net drivers using the nvmem property.The offset is set to 0x0
> >> since the factory programmed address is available in the
> >> resource managed space and the size determine if the requested
> >> address is of EUI48 (0x6) or EUI-64 (0x8) type.
> >> This is useful for cases where U-Boot is skipped and the Ethernet
> >> MAC address is needed to be configured by the kernel
> >>
> >> Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
> >> ---
> >>   .../boot/dts/microchip/at91-sama5d27_wlsom1.dtsi    | 13 +++++++++++++
> >>   1 file changed, 13 insertions(+)
> >>
> >> diff --git a/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi b/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi
> >> index b34c5072425a..be06df1b7d66 100644
> >> --- a/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi
> >> +++ b/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi
> >> @@ -210,6 +210,9 @@ &macb0 {
> >>        #size-cells = <0>;
> >>        phy-mode = "rmii";
> >>
> >> +     nvmem-cells = <&mac_address_eui48>;
> >> +     nvmem-cell-names = "mac-address";
> >> +
> >>        ethernet-phy@0 {
> >>                reg = <0x0>;
> >>                interrupt-parent = <&pioA>;
> >> @@ -238,6 +241,16 @@ qspi1_flash: flash@0 {
> >>                m25p,fast-read;
> >>                status = "disabled";
> >>
> >> +             nvmem-layout {

IMHO this should be "sfdp {".

> >> +                     compatible = "fixed-layout";

Please read my feedback on the first version again..

For the DT maintainers. The SFDP is a small table based content that
provide basic information about the flash. There are standard tables
which are specified by the JEDEC standard and there are vendor
tables, most of the time without proper documentation (or none at
all).

Somehow we need to specify at what table we want to look at. I'd
like to see a binding which can potentially expose anything inside
the SFDP.

So I've suggested to use "compatible = jedec,sfdp-vendor-table-NNNN"
where NNNN is the table parameter id. Additionally, the standard ids
could have names like "jedec,sfdp-bfpt" (basic flash parameter table).

So in your case that would be:

flash {
	sfdp {
		mac_address: table-1 {
			compatible = "jedec,sfdp-idNNNN";
		};
	};
};

I don't know what NNNN is. Could you please provide a dump of the
sfdp of your flash.

-michael

> >> +                     #address-cells = <1>;
> >> +                     #size-cells = <1>;
> >> +
> >> +                     mac_address_eui48: mac-address@0 {
> >> +                             reg = <0x0 0x6>;
> >> +                     };
> > 
> > How would this work if in the future the mchp vendor table adds some
> > other info that needs to be referenced as nvmem? How do you distinguish
> > the info from the table?
> > Would it be possible to have some kind of address and size to reference
> > the SFDP?
>
> I was previously advised not to hardcode the offset in the Device Tree 
> [1]. In the current implementation (patch 1/3), the read callback for 
> the MCHP vendor table distinguishes between EUI-48 and EUI-64 MAC 
> addresses based on the nvmem size, which corresponds to the size of the 
> respective MAC address.
>
> [1] --> https://lore.kernel.org/lkml/D889HZF97H8U.1UUX54BAVLAC3@kernel.org/
>
> > 
> >> +             };
> >> +
> >>                partitions {
> >>                        compatible = "fixed-partitions";
> >>                        #address-cells = <1>;
> > 


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]

Re: [PATCH v3 3/3] ARM: dts: microchip: sama5d27_wlsom1: Add nvmem-layout in QSPI for EUI48 MAC Address

[Manikandan.M]

Hi Michael,

On 11/06/25 1:01 pm, Michael Walle wrote:
> Hi,
> 
> On Mon Jun 9, 2025 at 11:27 AM CEST, Manikandan.M wrote:
>>>> Add nvmem-layout in QSPI to read the EUI48 Mac address by the
>>>> net drivers using the nvmem property.The offset is set to 0x0
>>>> since the factory programmed address is available in the
>>>> resource managed space and the size determine if the requested
>>>> address is of EUI48 (0x6) or EUI-64 (0x8) type.
>>>> This is useful for cases where U-Boot is skipped and the Ethernet
>>>> MAC address is needed to be configured by the kernel
>>>>
>>>> Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
>>>> ---
>>>>    .../boot/dts/microchip/at91-sama5d27_wlsom1.dtsi    | 13 +++++++++++++
>>>>    1 file changed, 13 insertions(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi b/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi
>>>> index b34c5072425a..be06df1b7d66 100644
>>>> --- a/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi
>>>> +++ b/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi
>>>> @@ -210,6 +210,9 @@ &macb0 {
>>>>         #size-cells = <0>;
>>>>         phy-mode = "rmii";
>>>>
>>>> +     nvmem-cells = <&mac_address_eui48>;
>>>> +     nvmem-cell-names = "mac-address";
>>>> +
>>>>         ethernet-phy@0 {
>>>>                 reg = <0x0>;
>>>>                 interrupt-parent = <&pioA>;
>>>> @@ -238,6 +241,16 @@ qspi1_flash: flash@0 {
>>>>                 m25p,fast-read;
>>>>                 status = "disabled";
>>>>
>>>> +             nvmem-layout {
> 
> IMHO this should be "sfdp {".
> 
>>>> +                     compatible = "fixed-layout";
> 
> Please read my feedback on the first version again..
> 
> For the DT maintainers. The SFDP is a small table based content that
> provide basic information about the flash. There are standard tables
> which are specified by the JEDEC standard and there are vendor
> tables, most of the time without proper documentation (or none at
> all).
> 
> Somehow we need to specify at what table we want to look at. I'd
> like to see a binding which can potentially expose anything inside
> the SFDP.
> 
> So I've suggested to use "compatible = jedec,sfdp-vendor-table-NNNN"
> where NNNN is the table parameter id. Additionally, the standard ids
> could have names like "jedec,sfdp-bfpt" (basic flash parameter table).
> 
> So in your case that would be:
> 
> flash {
> 	sfdp {
> 		mac_address: table-1 {
> 			compatible = "jedec,sfdp-idNNNN";
> 		};
> 	};

Should the nvmem-layout be included as a child node under sfdp {}, or 
should it be implemented as a separate vendor-specific driver to handle 
the changes introduced in patch 1/3 ?

> };
> 
> I don't know what NNNN is. Could you please provide a dump of the
> sfdp of your flash.

Please find the entire SFDP data of SST26VF064BEUI flash in Table 11.1 
of 11.0 APPENDIX

https://ww1.microchip.com/downloads/aemDocuments/documents/MPD/ProductDocuments/DataSheets/SST26VF064BEUI-Data-Sheet-DS20006138.pdf


The vendor parameter ID is 'BF' if I am not wrong.
> 
> -michael
> 
>>>> +                     #address-cells = <1>;
>>>> +                     #size-cells = <1>;
>>>> +
>>>> +                     mac_address_eui48: mac-address@0 {
>>>> +                             reg = <0x0 0x6>;
>>>> +                     };
>>>
>>> How would this work if in the future the mchp vendor table adds some
>>> other info that needs to be referenced as nvmem? How do you distinguish
>>> the info from the table?
>>> Would it be possible to have some kind of address and size to reference
>>> the SFDP?
>>
>> I was previously advised not to hardcode the offset in the Device Tree
>> [1]. In the current implementation (patch 1/3), the read callback for
>> the MCHP vendor table distinguishes between EUI-48 and EUI-64 MAC
>> addresses based on the nvmem size, which corresponds to the size of the
>> respective MAC address.
>>
>> [1] --> https://lore.kernel.org/lkml/D889HZF97H8U.1UUX54BAVLAC3@kernel.org/
>>
>>>
>>>> +             };
>>>> +
>>>>                 partitions {
>>>>                         compatible = "fixed-partitions";
>>>>                         #address-cells = <1>;
>>>
> 

-- 
Thanks and Regards,
Manikandan M.

Re: [PATCH v3 3/3] ARM: dts: microchip: sama5d27_wlsom1: Add nvmem-layout in QSPI for EUI48 MAC Address

[Michael Walle]

[-- Attachment #1: Type: text/plain, Size: 5043 bytes --]

Hi,

> >>>> Add nvmem-layout in QSPI to read the EUI48 Mac address by the
> >>>> net drivers using the nvmem property.The offset is set to 0x0
> >>>> since the factory programmed address is available in the
> >>>> resource managed space and the size determine if the requested
> >>>> address is of EUI48 (0x6) or EUI-64 (0x8) type.
> >>>> This is useful for cases where U-Boot is skipped and the Ethernet
> >>>> MAC address is needed to be configured by the kernel
> >>>>
> >>>> Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
> >>>> ---
> >>>>    .../boot/dts/microchip/at91-sama5d27_wlsom1.dtsi    | 13 +++++++++++++
> >>>>    1 file changed, 13 insertions(+)
> >>>>
> >>>> diff --git a/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi b/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi
> >>>> index b34c5072425a..be06df1b7d66 100644
> >>>> --- a/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi
> >>>> +++ b/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi
> >>>> @@ -210,6 +210,9 @@ &macb0 {
> >>>>         #size-cells = <0>;
> >>>>         phy-mode = "rmii";
> >>>>
> >>>> +     nvmem-cells = <&mac_address_eui48>;
> >>>> +     nvmem-cell-names = "mac-address";
> >>>> +
> >>>>         ethernet-phy@0 {
> >>>>                 reg = <0x0>;
> >>>>                 interrupt-parent = <&pioA>;
> >>>> @@ -238,6 +241,16 @@ qspi1_flash: flash@0 {
> >>>>                 m25p,fast-read;
> >>>>                 status = "disabled";
> >>>>
> >>>> +             nvmem-layout {
> > 
> > IMHO this should be "sfdp {".
> > 
> >>>> +                     compatible = "fixed-layout";
> > 
> > Please read my feedback on the first version again..
> > 
> > For the DT maintainers. The SFDP is a small table based content that
> > provide basic information about the flash. There are standard tables
> > which are specified by the JEDEC standard and there are vendor
> > tables, most of the time without proper documentation (or none at
> > all).
> > 
> > Somehow we need to specify at what table we want to look at. I'd
> > like to see a binding which can potentially expose anything inside
> > the SFDP.
> > 
> > So I've suggested to use "compatible = jedec,sfdp-vendor-table-NNNN"
> > where NNNN is the table parameter id. Additionally, the standard ids
> > could have names like "jedec,sfdp-bfpt" (basic flash parameter table).
> > 
> > So in your case that would be:
> > 
> > flash {
> > 	sfdp {
> > 		mac_address: table-1 {
> > 			compatible = "jedec,sfdp-idNNNN";
> > 		};
> > 	};
>
> Should the nvmem-layout be included as a child node under sfdp {}, or 
> should it be implemented as a separate vendor-specific driver to handle 
> the changes introduced in patch 1/3 ?

There is no nvmem-layout involved here.

But another possibility is to make it one. Then you have to
 (1) expose the *entire* sfpd as a nvmem device
 (2a) write an nvmem-layouts driver (in drivers/nvmem/layouts/)
 (2b) come up with a DT binding that is generic enough to expose
      various parameters of the SFDP, not just a one-off for the
      MAC address use case.

Maybe that is even a better fit.

> > };
> > 
> > I don't know what NNNN is. Could you please provide a dump of the
> > sfdp of your flash.
>
> Please find the entire SFDP data of SST26VF064BEUI flash in Table 11.1 
> of 11.0 APPENDIX

Please dump it according to [1], so we have it in a machine readable
format.

-michael

[1] https://docs.kernel.org/driver-api/mtd/spi-nor.html


> > On Mon Jun 9, 2025 at 11:27 AM CEST, Manikandan.M wrote:

>
> https://ww1.microchip.com/downloads/aemDocuments/documents/MPD/ProductDocuments/DataSheets/SST26VF064BEUI-Data-Sheet-DS20006138.pdf
>
>
> The vendor parameter ID is 'BF' if I am not wrong.
> > 
> > -michael
> > 
> >>>> +                     #address-cells = <1>;
> >>>> +                     #size-cells = <1>;
> >>>> +
> >>>> +                     mac_address_eui48: mac-address@0 {
> >>>> +                             reg = <0x0 0x6>;
> >>>> +                     };
> >>>
> >>> How would this work if in the future the mchp vendor table adds some
> >>> other info that needs to be referenced as nvmem? How do you distinguish
> >>> the info from the table?
> >>> Would it be possible to have some kind of address and size to reference
> >>> the SFDP?
> >>
> >> I was previously advised not to hardcode the offset in the Device Tree
> >> [1]. In the current implementation (patch 1/3), the read callback for
> >> the MCHP vendor table distinguishes between EUI-48 and EUI-64 MAC
> >> addresses based on the nvmem size, which corresponds to the size of the
> >> respective MAC address.
> >>
> >> [1] --> https://lore.kernel.org/lkml/D889HZF97H8U.1UUX54BAVLAC3@kernel.org/
> >>
> >>>
> >>>> +             };
> >>>> +
> >>>>                 partitions {
> >>>>                         compatible = "fixed-partitions";
> >>>>                         #address-cells = <1>;
> >>>
> > 


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]

Re: [PATCH v3 3/3] ARM: dts: microchip: sama5d27_wlsom1: Add nvmem-layout in QSPI for EUI48 MAC Address

[Manikandan.M]

Hi Michael,

On 12/06/25 11:47 am, Michael Walle wrote:
> Hi,
> 
>>>>>> Add nvmem-layout in QSPI to read the EUI48 Mac address by the
>>>>>> net drivers using the nvmem property.The offset is set to 0x0
>>>>>> since the factory programmed address is available in the
>>>>>> resource managed space and the size determine if the requested
>>>>>> address is of EUI48 (0x6) or EUI-64 (0x8) type.
>>>>>> This is useful for cases where U-Boot is skipped and the Ethernet
>>>>>> MAC address is needed to be configured by the kernel
>>>>>>
>>>>>> Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
>>>>>> ---
>>>>>>     .../boot/dts/microchip/at91-sama5d27_wlsom1.dtsi    | 13 +++++++++++++
>>>>>>     1 file changed, 13 insertions(+)
>>>>>>
>>>>>> diff --git a/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi b/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi
>>>>>> index b34c5072425a..be06df1b7d66 100644
>>>>>> --- a/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi
>>>>>> +++ b/arch/arm/boot/dts/microchip/at91-sama5d27_wlsom1.dtsi
>>>>>> @@ -210,6 +210,9 @@ &macb0 {
>>>>>>          #size-cells = <0>;
>>>>>>          phy-mode = "rmii";
>>>>>>
>>>>>> +     nvmem-cells = <&mac_address_eui48>;
>>>>>> +     nvmem-cell-names = "mac-address";
>>>>>> +
>>>>>>          ethernet-phy@0 {
>>>>>>                  reg = <0x0>;
>>>>>>                  interrupt-parent = <&pioA>;
>>>>>> @@ -238,6 +241,16 @@ qspi1_flash: flash@0 {
>>>>>>                  m25p,fast-read;
>>>>>>                  status = "disabled";
>>>>>>
>>>>>> +             nvmem-layout {
>>>
>>> IMHO this should be "sfdp {".
>>>
>>>>>> +                     compatible = "fixed-layout";
>>>
>>> Please read my feedback on the first version again..
>>>
>>> For the DT maintainers. The SFDP is a small table based content that
>>> provide basic information about the flash. There are standard tables
>>> which are specified by the JEDEC standard and there are vendor
>>> tables, most of the time without proper documentation (or none at
>>> all).
>>>
>>> Somehow we need to specify at what table we want to look at. I'd
>>> like to see a binding which can potentially expose anything inside
>>> the SFDP.
>>>
>>> So I've suggested to use "compatible = jedec,sfdp-vendor-table-NNNN"
>>> where NNNN is the table parameter id. Additionally, the standard ids
>>> could have names like "jedec,sfdp-bfpt" (basic flash parameter table).
>>>
>>> So in your case that would be:
>>>
>>> flash {
>>> 	sfdp {
>>> 		mac_address: table-1 {
>>> 			compatible = "jedec,sfdp-idNNNN";
>>> 		};
>>> 	};
>>
>> Should the nvmem-layout be included as a child node under sfdp {}, or
>> should it be implemented as a separate vendor-specific driver to handle
>> the changes introduced in patch 1/3 ?
> 
> There is no nvmem-layout involved here.
> 
> But another possibility is to make it one. Then you have to
>   (1) expose the *entire* sfpd as a nvmem device
>   (2a) write an nvmem-layouts driver (in drivers/nvmem/layouts/)
>   (2b) come up with a DT binding that is generic enough to expose
>        various parameters of the SFDP, not just a one-off for the
>        MAC address use case.
> 
> Maybe that is even a better fit.
> 
>>> };
>>>
>>> I don't know what NNNN is. Could you please provide a dump of the
>>> sfdp of your flash.
>>
>> Please find the entire SFDP data of SST26VF064BEUI flash in Table 11.1
>> of 11.0 APPENDIX
> 
> Please dump it according to [1], so we have it in a machine readable
> format.

Here is the dump of sfdp as per [1],

[root@sama5 ~]$ xxd -p /sys/bus/spi/devices/spi2.0/spi-nor/sfdp
53464450060102ff00060110300000ff81000106000100ffbf00021c0002
0001fffffffffffffffffffffffffffffffffd20f1ffffffff0344eb086b
083b80bbfeffffffffff00ffffff440b0c200dd80fd810d820914824806f
1d81ed0f773830b030b0f7ffffff29c25cfff030c080ffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffff0004fff37f0000f57f0000f9ff
7d00f57f0000f37f0000ffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffbf2643ffb95ffdff30f260f332ff0a122346ff0f19320f1919ffffff
ffffffff00669938ff05013506040232b03072428de89888a585c09faf5a
ffff06ec060c0003080bffffffffff07ffff0202ff060300fdfd040700fc
0300fefe0202070e3004916245a09240001ec0000005a092


> 
> -michael
> 
> [1] https://docs.kernel.org/driver-api/mtd/spi-nor.html
> 
> 
>>> On Mon Jun 9, 2025 at 11:27 AM CEST, Manikandan.M wrote:
> 
>>
>> https://ww1.microchip.com/downloads/aemDocuments/documents/MPD/ProductDocuments/DataSheets/SST26VF064BEUI-Data-Sheet-DS20006138.pdf
>>
>>
>> The vendor parameter ID is 'BF' if I am not wrong.
>>>
>>> -michael
>>>
>>>>>> +                     #address-cells = <1>;
>>>>>> +                     #size-cells = <1>;
>>>>>> +
>>>>>> +                     mac_address_eui48: mac-address@0 {
>>>>>> +                             reg = <0x0 0x6>;
>>>>>> +                     };
>>>>>
>>>>> How would this work if in the future the mchp vendor table adds some
>>>>> other info that needs to be referenced as nvmem? How do you distinguish
>>>>> the info from the table?
>>>>> Would it be possible to have some kind of address and size to reference
>>>>> the SFDP?
>>>>
>>>> I was previously advised not to hardcode the offset in the Device Tree
>>>> [1]. In the current implementation (patch 1/3), the read callback for
>>>> the MCHP vendor table distinguishes between EUI-48 and EUI-64 MAC
>>>> addresses based on the nvmem size, which corresponds to the size of the
>>>> respective MAC address.
>>>>
>>>> [1] --> https://lore.kernel.org/lkml/D889HZF97H8U.1UUX54BAVLAC3@kernel.org/
>>>>
>>>>>
>>>>>> +             };
>>>>>> +
>>>>>>                  partitions {
>>>>>>                          compatible = "fixed-partitions";
>>>>>>                          #address-cells = <1>;
>>>>>
>>>
> 

-- 
Thanks and Regards,
Manikandan M.