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

[PATCH v2 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 sama5d29 curiosity and
sam9x75 curiosity.

--------
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: sama5d29_curiosity: update the QSPI partitions
    using "fixed-partition" binding
  ARM: dts: microchip: sama5d29_curiosity: Add nvmem-layout in QSPI for
    EUI48 MAC Address

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

-- 
2.25.1

[PATCH v2 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 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 v2 2/3] ARM: dts: microchip: sama5d29_curiosity: 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-sama5d29_curiosity.dts | 54 ++++++++++---------
 1 file changed, 29 insertions(+), 25 deletions(-)

diff --git a/arch/arm/boot/dts/microchip/at91-sama5d29_curiosity.dts b/arch/arm/boot/dts/microchip/at91-sama5d29_curiosity.dts
index 7be215781549..35756cc01e68 100644
--- a/arch/arm/boot/dts/microchip/at91-sama5d29_curiosity.dts
+++ b/arch/arm/boot/dts/microchip/at91-sama5d29_curiosity.dts
@@ -469,8 +469,6 @@ &qspi1 {
 	status = "okay";
 
 	flash@0 {
-		#address-cells = <1>;
-		#size-cells = <1>;
 		compatible = "jedec,spi-nor";
 		reg = <0>;
 		spi-max-frequency = <80000000>;
@@ -480,34 +478,40 @@ flash@0 {
 		label = "atmel_qspi1";
 		status = "okay";
 
-		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 v2 3/3] ARM: dts: microchip: sama5d29_curiosity: 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>
---
 .../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 35756cc01e68..6c5ff08f0b3f 100644
--- a/arch/arm/boot/dts/microchip/at91-sama5d29_curiosity.dts
+++ b/arch/arm/boot/dts/microchip/at91-sama5d29_curiosity.dts
@@ -478,6 +478,16 @@ flash@0 {
 		label = "atmel_qspi1";
 		status = "okay";
 
+		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 v2 3/3] ARM: dts: microchip: sama5d29_curiosity: Add nvmem-layout in QSPI for EUI48 MAC Address

[Andrew Lunn]

On Wed, Mar 26, 2025 at 12:51:40PM +0530, 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>
> ---
>  .../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 35756cc01e68..6c5ff08f0b3f 100644
> --- a/arch/arm/boot/dts/microchip/at91-sama5d29_curiosity.dts
> +++ b/arch/arm/boot/dts/microchip/at91-sama5d29_curiosity.dts
> @@ -478,6 +478,16 @@ flash@0 {
>  		label = "atmel_qspi1";
>  		status = "okay";
>  
> +		nvmem-layout {
> +			compatible = "fixed-layout";
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +
> +			mac_address_eui48: mac-address@0 {
> +				reg = <0x0 0x6>;
> +			};
> +		};
> +

I've not looked too deeply how this all works. Don't you need a
reference in the ethernet node pointing to this?

And are there ordering issues? Boards used to use the MAC address from
somewhere else now start using this address, causing a change in
behaviour. I would expect somewhere a comment that this MAC address
will be used last, after all other options have been tried, in order
to avoid regressions.

   Andrew

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

[Rob Herring (Arm)]

On Wed, 26 Mar 2025 12:51:37 +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 sama5d29 curiosity and
> sam9x75 curiosity.
> 
> --------
> 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: sama5d29_curiosity: update the QSPI partitions
>     using "fixed-partition" binding
>   ARM: dts: microchip: sama5d29_curiosity: Add nvmem-layout in QSPI for
>     EUI48 MAC Address
> 
>  .../dts/microchip/at91-sama5d29_curiosity.dts |  62 ++++---
>  drivers/mtd/spi-nor/sfdp.c                    | 161 ++++++++++++++++++
>  include/linux/mtd/spi-nor.h                   |   7 +
>  3 files changed, 206 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-20250326 (exact match)

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 20250326072140.172244-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 v2 3/3] ARM: dts: microchip: sama5d29_curiosity: Add nvmem-layout in QSPI for EUI48 MAC Address

[Manikandan.M]

Hi Andrew Lunn,

On 26/03/25 6:48 pm, Andrew Lunn wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Wed, Mar 26, 2025 at 12:51:40PM +0530, 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>
>> ---
>>   .../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 35756cc01e68..6c5ff08f0b3f 100644
>> --- a/arch/arm/boot/dts/microchip/at91-sama5d29_curiosity.dts
>> +++ b/arch/arm/boot/dts/microchip/at91-sama5d29_curiosity.dts
>> @@ -478,6 +478,16 @@ flash@0 {
>>                label = "atmel_qspi1";
>>                status = "okay";
>>
>> +             nvmem-layout {
>> +                     compatible = "fixed-layout";
>> +                     #address-cells = <1>;
>> +                     #size-cells = <1>;
>> +
>> +                     mac_address_eui48: mac-address@0 {
>> +                             reg = <0x0 0x6>;
>> +                     };
>> +             };
>> +
> 
> I've not looked too deeply how this all works. Don't you need a
> reference in the ethernet node pointing to this?
Yes we need a reference to 'mac_address_eui48' using nvmem-cells in the 
Ethernet node, since the sama5d29_curiosity uses a daughter card for PHY 
[1], the DTS properties are defined in overlay files. Here is the quick 
usage of the nvmem ref in the ethernet node:
	macb0 {
		nvmem-cells = <&mac_address_eui48>;
		nvmem-cell-names = "mac-address";
		
		phy {

		};
	};

[1] --> https://www.microchip.com/en-us/development-tool/ev90j04a
> 
> And are there ordering issues? Boards used to use the MAC address from
> somewhere else now start using this address, causing a change in
> behaviour. I would expect somewhere a comment that this MAC address
> will be used last, after all other options have been tried, in order
> to avoid regressions.
> 
The order of search is documented in of_get_mac_address() in 
net/core/of_net.c file

The driver attempts to retrieve the MAC address through a hierarchical 
approach: first checking device tree properties, then exploring NVMEM 
cells, followed by the U-Boot 'ethaddr' environment variable. If no 
valid MAC address is found through these methods, the driver will 
generate a random but valid MAC address as a final fallback mechanism.
>     Andrew

-- 
Thanks and Regards,
Manikandan M.

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

[Andrew Lunn]

On Thu, Mar 27, 2025 at 06:03:05AM +0000, Manikandan.M@microchip.com wrote:
> Hi Andrew Lunn,
> 
> On 26/03/25 6:48 pm, Andrew Lunn wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > 
> > On Wed, Mar 26, 2025 at 12:51:40PM +0530, 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>
> >> ---
> >>   .../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 35756cc01e68..6c5ff08f0b3f 100644
> >> --- a/arch/arm/boot/dts/microchip/at91-sama5d29_curiosity.dts
> >> +++ b/arch/arm/boot/dts/microchip/at91-sama5d29_curiosity.dts
> >> @@ -478,6 +478,16 @@ flash@0 {
> >>                label = "atmel_qspi1";
> >>                status = "okay";
> >>
> >> +             nvmem-layout {
> >> +                     compatible = "fixed-layout";
> >> +                     #address-cells = <1>;
> >> +                     #size-cells = <1>;
> >> +
> >> +                     mac_address_eui48: mac-address@0 {
> >> +                             reg = <0x0 0x6>;
> >> +                     };
> >> +             };
> >> +
> > 
> > I've not looked too deeply how this all works. Don't you need a
> > reference in the ethernet node pointing to this?
> Yes we need a reference to 'mac_address_eui48' using nvmem-cells in the 
> Ethernet node, since the sama5d29_curiosity uses a daughter card for PHY 
> [1], the DTS properties are defined in overlay files. Here is the quick 
> usage of the nvmem ref in the ethernet node:
> 	macb0 {
> 		nvmem-cells = <&mac_address_eui48>;
> 		nvmem-cell-names = "mac-address";
> 		
> 		phy {
> 
> 		};
> 	};

So why are you not adding this as part of this patch?

> > And are there ordering issues? Boards used to use the MAC address from
> > somewhere else now start using this address, causing a change in
> > behaviour. I would expect somewhere a comment that this MAC address
> > will be used last, after all other options have been tried, in order
> > to avoid regressions.
> > 
> The order of search is documented in of_get_mac_address() in 
> net/core/of_net.c file
> 
> The driver attempts to retrieve the MAC address through a hierarchical 
> approach: first checking device tree properties, then exploring NVMEM 
> cells, followed by the U-Boot 'ethaddr' environment variable. If no 
> valid MAC address is found through these methods, the driver will 
> generate a random but valid MAC address as a final fallback mechanism.

This is not quite correct. macb first uses
of_get_ethdev_address()->of_get_mac_address() which looks for DT
properties:

	ret = of_get_mac_addr(np, "mac-address", addr);
	if (!ret)
		return 0;

	ret = of_get_mac_addr(np, "local-mac-address", addr);
	if (!ret)
		return 0;

	ret = of_get_mac_addr(np, "address", addr);
	if (!ret)
		return 0;

And then it looks in nvram.

	return of_get_mac_address_nvmem(np, addr);

If they all fail, it uses macb_get_hwaddr() which looks in 4 different
locations within the macb register set.

Then lastly it uses a random MAC address.

So with your proposed change, anybody using the curiosity board and
this last mechanism to set the MAC address sees a change in behaviour,
it will start using nvram instead. You should at least document this,
and if possible, argue that nobody is using this last mechanism
because ....

	Andrew

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

[Manikandan.M]

Hi Andrew,

On 27/03/25 6:44 pm, Andrew Lunn wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Thu, Mar 27, 2025 at 06:03:05AM +0000, Manikandan.M@microchip.com wrote:
>> Hi Andrew Lunn,
>>
>> On 26/03/25 6:48 pm, Andrew Lunn wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> On Wed, Mar 26, 2025 at 12:51:40PM +0530, 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>
>>>> ---
>>>>    .../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 35756cc01e68..6c5ff08f0b3f 100644
>>>> --- a/arch/arm/boot/dts/microchip/at91-sama5d29_curiosity.dts
>>>> +++ b/arch/arm/boot/dts/microchip/at91-sama5d29_curiosity.dts
>>>> @@ -478,6 +478,16 @@ flash@0 {
>>>>                 label = "atmel_qspi1";
>>>>                 status = "okay";
>>>>
>>>> +             nvmem-layout {
>>>> +                     compatible = "fixed-layout";
>>>> +                     #address-cells = <1>;
>>>> +                     #size-cells = <1>;
>>>> +
>>>> +                     mac_address_eui48: mac-address@0 {
>>>> +                             reg = <0x0 0x6>;
>>>> +                     };
>>>> +             };
>>>> +
>>>
>>> I've not looked too deeply how this all works. Don't you need a
>>> reference in the ethernet node pointing to this?
>> Yes we need a reference to 'mac_address_eui48' using nvmem-cells in the
>> Ethernet node, since the sama5d29_curiosity uses a daughter card for PHY
>> [1], the DTS properties are defined in overlay files. Here is the quick
>> usage of the nvmem ref in the ethernet node:
>>        macb0 {
>>                nvmem-cells = <&mac_address_eui48>;
>>                nvmem-cell-names = "mac-address";
>>
>>                phy {
>>
>>                };
>>        };
> 
> So why are you not adding this as part of this patch?
As mentioned, we maintain the DT nodes and properties of all daughter 
cards and modules which are not part of the on-board peripherals in a 
separate repo's as overlay [1].In the next version I will also include 
the DT changes of the board which actually has an on-board PHY with 
mac-address programmed in QSPI SFDP vendor area to convey the changes 
better- SAMA5D27 WLSOM1

[1] --> 
https://github.com/linux4microchip/dt-overlay-mchp/blob/master/sama5d29_curiosity/sama5d29_curiosity_ksz8091.dtso

> 
>>> And are there ordering issues? Boards used to use the MAC address from
>>> somewhere else now start using this address, causing a change in
>>> behaviour. I would expect somewhere a comment that this MAC address
>>> will be used last, after all other options have been tried, in order
>>> to avoid regressions.
>>>
>> The order of search is documented in of_get_mac_address() in
>> net/core/of_net.c file
>>
>> The driver attempts to retrieve the MAC address through a hierarchical
>> approach: first checking device tree properties, then exploring NVMEM
>> cells, followed by the U-Boot 'ethaddr' environment variable. If no
>> valid MAC address is found through these methods, the driver will
>> generate a random but valid MAC address as a final fallback mechanism.
> 
> This is not quite correct. macb first uses
> of_get_ethdev_address()->of_get_mac_address() which looks for DT
> properties:
> 
>          ret = of_get_mac_addr(np, "mac-address", addr);
>          if (!ret)
>                  return 0;
> 
>          ret = of_get_mac_addr(np, "local-mac-address", addr);
>          if (!ret)
>                  return 0;
> 
>          ret = of_get_mac_addr(np, "address", addr);
>          if (!ret)
>                  return 0;
> 
> And then it looks in nvram.
> 
>          return of_get_mac_address_nvmem(np, addr);
> 
> If they all fail, it uses macb_get_hwaddr() which looks in 4 different
> locations within the macb register set.
> 
> Then lastly it uses a random MAC address.
> 
> So with your proposed change, anybody using the curiosity board and
> this last mechanism to set the MAC address sees a change in behaviour,
> it will start using nvram instead. You should at least document this,
> and if possible, argue that nobody is using this last mechanism
> because ....

Retrieving MAC Address from NVRAM is practiced in sama7g5ek but with 
EEPROM memory
For the next version, I will add a note in the commit message explaining 
the hierarchical order and the possibilities of hitting 
of_get_mac_address_nvmem()

> 
>          Andrew

-- 
Thanks and Regards,
Manikandan M.