Re: [PATCH v2 05/10] boards: enclustra-sa2: read MAC address from EEPROM

[David Picard <david.picard@clermont.in2p3.fr>]

The SoM manual states, p46, bottom:
"The Ethernet MAC address is stored using big-endian byte order (MSB on 
the lowest address). Each module
is assigned two sequential MAC addresses; only the lower one is stored 
in the EEPROM."
https://filesender.renater.fr/?s=download&token=c553d226-7c90-41e3-8c74-96393f1f1edd 


What's the problem with fallback addresses? At least, the board will 
look like an Enclustra product.


Le 26/09/2025 à 14:40, Sascha Hauer a écrit :
> On Thu, Sep 25, 2025 at 01:59:12PM +0200, David Picard wrote:
>> Signed-off-by: David Picard <david.picard@clermont.in2p3.fr>
>>
>> ---
>> v1 --> v2:
>>    - Use the atsha204a driver to read the MAC address instead of
>>      board-specific code.
>> ---
>> ---
>>   arch/arm/boards/enclustra-sa2/board.c          | 74 +++++++++++++++++++++++++-
>>   arch/arm/dts/socfpga_cyclone5_mercury_sa2.dtsi | 16 +++++-
>>   2 files changed, 87 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm/boards/enclustra-sa2/board.c b/arch/arm/boards/enclustra-sa2/board.c
>> index d3117e9a1058738ea541f45b28c6a95184331554..4c2b44252d84c78ed8de5754051d0bf194ce8d02 100644
>> --- a/arch/arm/boards/enclustra-sa2/board.c
>> +++ b/arch/arm/boards/enclustra-sa2/board.c
>> @@ -12,6 +12,12 @@
>>   #include <fcntl.h>
>>   #include <fs.h>
>>   #include <mach/socfpga/cyclone5-regs.h>
>> +#include <net.h>
>> +#include <linux/nvmem-consumer.h>
>> +
>> +/** Enclustra's MAC address vendor prefix is 20:B0:F7 */
>> +#define ENCLUSTRA_PREFIX            (0x20b0f7)
>> +#define MAC_ADDR_NUM_BYTES          (6)
>>   
>>   /*
>>    * Ethernet PHY: Microchip/Micrel KSZ9031RNX
>> @@ -21,14 +27,78 @@ static int phy_fixup(struct phy_device *dev)
>>   	return 0;
>>   }
>>   
>> +/*
>> + * Read the MAC address via the atsha204a driver.
>> + *
>> + * Set two consecutive MAC addresses, as specified by the manufacturer.
>> + */
>> +static void set_mac_addr(void)
>> +{
>> +	uint8_t hwaddr[MAC_ADDR_NUM_BYTES] = { 0, 0, 0, 0, 0, 0 };
>> +	uint32_t hwaddr_prefix;
>> +	u8 *data = NULL;
>> +	static const char * const aliases[] = { "ethernet0" };
>> +	struct device_node *np, *root;
>> +	/* Fallback MAC addresses, used if we can't read from EEPROM: */
>> +	const uint8_t enclustra_ethaddr_fallback1[] = { 0x20, 0xB0, 0xF7, 0x01,
>> +													0x02, 0x03 };
>> +	const uint8_t enclustra_ethaddr_fallback2[] = { 0x20, 0xB0, 0xF7, 0x01,
>> +													0x02, 0x04 };
> Please no fallback addresses. Just skip setting these and barebox will
> use a random MAC.
>
>> +
>> +	root = of_get_root_node();
>> +
>> +	for (int i = 0; i < ARRAY_SIZE(aliases); i++) {
>> +		const char *alias = aliases[i];
>> +		np = of_find_node_by_alias(root, alias);
>> +		if (!np) {
>> +			pr_warn("%s() >> ERROR: can't find alias %s\n", __func__, alias);
>> +			continue;
>> +		}
>> +		data = nvmem_cell_get_and_read(np, "mac-address", MAC_ADDR_NUM_BYTES);
>> +		if (IS_ERR(data)) {
>> +			pr_warn("%s() >> ERROR: can't read NVMEM cell\n", __func__);
>> +			data = NULL;
>> +		}
>> +	}
> You shouldn't need this. of_get_mac_addr_nvmem() called by the net layer
> during startup should handle this for you.
>
>> +	if (!data)
>> +		goto fallback_addr;
>> +
>> +	memcpy(hwaddr, data, MAC_ADDR_NUM_BYTES);
>> +
>> +	debug("MAC address: %02x:%02x:%02x:%02x:%02x:%02x\n",
>> +		  hwaddr[0], hwaddr[1], hwaddr[2],
>> +		  hwaddr[3], hwaddr[4], hwaddr[5]);
>> +
>> +	/* check vendor prefix and set the environment variable */
>> +	hwaddr_prefix = (hwaddr[0] << 16) | (hwaddr[1] << 8) | (hwaddr[2]);
>> +	if (hwaddr_prefix == ENCLUSTRA_PREFIX) {
>> +		eth_register_ethaddr(0, hwaddr);
>> +		hwaddr[5]++;    /* calculate 2nd, consecutive MAC address */
>> +		eth_register_ethaddr(1, hwaddr);
>> +	} else {
>> +		printf("%s() >> ERROR: invalid MAC address vendor prefix,"
>> +			   "using fallback addresses\n", __func__);
>> +		goto fallback_addr;
>> +	}
> Not sure what to do about the consecutive MAC address. There seems to be
> beginning support for this in fixed-cell.yaml, but I can't find an
> implementation for it.
>
>> diff --git a/arch/arm/dts/socfpga_cyclone5_mercury_sa2.dtsi b/arch/arm/dts/socfpga_cyclone5_mercury_sa2.dtsi
>> index fa80b7c63d28d17a24d63ac3ee87531ad320ddb1..55d54d289c81fa4a6d46123bcb93c5fc483485e4 100644
>> --- a/arch/arm/dts/socfpga_cyclone5_mercury_sa2.dtsi
>> +++ b/arch/arm/dts/socfpga_cyclone5_mercury_sa2.dtsi
>> @@ -76,6 +76,19 @@ atsha204a: atsha204a@64 {
>>   		status = "okay";
>>   		compatible = "atmel,atsha204a";
>>   		reg = <0x64>;
>> +
>> +		nvmem-layout {
>> +		compatible = "fixed-layout";
>> +		#address-cells = <1>;
>> +		#size-cells = <1>;
> Indentation wrong here.
>
>> +
>> +			mac_address_0: mac@10 {
>> +				compatible = "mac-base";
>> +				reg = <0x10 0x6>;
>> +				nvmem-cell-cells = <1>;
> must be #nvmem-cell-cells#
>
> Sascha
>