On 9/17/24 8:21 AM, Lothar Rubusch wrote: [...]
+int enclustra_get_mac_from_eeprom(unsigned char *enetaddr, const char *alias) +{ + struct udevice *dev; + u32 hwaddr_h; + u8 data[4]; + int i, j, eeprom_addr, mac_len, ret; + + ret = uclass_get_device_by_name(UCLASS_MISC, alias, &dev); + if (ret) { + printf("%s: Failed, cannot find EEPROM! ret = %d\n", __func__, ret); + return ret; + } + + /* Make sure atsha204a is in a defined state (part of protocol) */ + if (atsha204a_sleep(dev)) {
Please propagate return values.
+ printf("%s(): Failed to bring EEPROM in defined state\n", __func__); + return -ENODEV; + } + + if (atsha204a_wakeup(dev)) { + printf("%s(): Failed to wakeup EEPROM\n", __func__); + return -ENODEV; + } + + /* Read twice portions of 4 bytes (atsha204 protocol). One from address 4
Use Linux comment style, not netdev: /* * Read some * stuff * and so on */
+ * the other from address 5 of the OTP zone. Then convert the data to + * the 6 elements of the MAC address. + */ + eeprom_addr = 4; + mac_len = 6;
These can be const variables assigned at the beginning of this function.
+ for (i = 0; i < 2; i++) { + eeprom_addr += i; + if (atsha204a_read(dev, ATSHA204A_ZONE_OTP, false, eeprom_addr, data)) { + printf("%s(): Failed to parse ATSHA204A_ZONE_OTP of EEPROM\n", + __func__); + return -EFAULT; + } + + for (j = 0; j < 4 && j + i * 4 < mac_len; j++) + enetaddr[j + i * 4] = data[j]; + } + + /* Check if the value is a valid mac registered for + * Enclustra GmbH + */ + hwaddr_h = enetaddr[0] | enetaddr[1] << 8 | enetaddr[2] << 16; + if ((hwaddr_h & 0xFFFFFF) != ENCLUSTRA_MAC) { + printf("%s(): Failed, parsed MAC is no Enclustra MAC\n", __func__); + return -ENOENT; + } + + if (!is_valid_ethaddr(enetaddr)) { + printf("%s(): Failed, address read from EEPROM is invalid!\n", + __func__); + return -EINVAL; + } + + printf("ethaddr set to %02X:%02X:%02X:%02X:%02X:%02X\n", + enetaddr[0], enetaddr[1], enetaddr[2], + enetaddr[3], enetaddr[4], enetaddr[5]);
This should be: debug("ethaddr set to %pM\n", ethaddr); see lib/vsprintf.c for the formatting strings.
+ return 0; +} + +__weak int enclustra_setup_mac_address(void) +{ + unsigned char enetaddr[6]; + + if (enclustra_mac_is_in_env("ethaddr")) + return 0; + + if (enclustra_get_mac_is_enabled("ethernet0")) + return 0; + + if (enclustra_get_mac_from_eeprom(enetaddr, "atsha204a@64"))
Please propagate return values, fix globally.
+ return -ENXIO; + + if (eth_env_set_enetaddr("ethaddr", enetaddr)) + return -ENXIO; + + if (!enclustra_get_mac1_from_mac(enetaddr)) + return eth_env_set_enetaddr("eth1addr", enetaddr); + + printf("%s(): Failed, unable to set mac address!\n", __func__); + return -ENXIO; +}
[...]
+int enclustra_get_mac1_from_mac(unsigned char *enetaddr) +{ + u32 hwaddr_h; + + /* Increment MAC addr */ + hwaddr_h = (enetaddr[3] << 16) | (enetaddr[4] << 8) | enetaddr[5]; + hwaddr_h = (hwaddr_h + 1) & 0xFFFFFF; + enetaddr[3] = (hwaddr_h >> 16) & 0xFF; + enetaddr[4] = (hwaddr_h >> 8) & 0xFF; + enetaddr[5] = hwaddr_h & 0xFF; + + if (!is_valid_ethaddr(enetaddr)) { + printf("%s(): Failed, address computed from enetaddr is invalid!\n", + __func__); + return -EINVAL; + } + + printf("eth1addr set to %02X:%02X:%02X:%02X:%02X:%02X\n", + enetaddr[0], enetaddr[1], enetaddr[2], + enetaddr[3], enetaddr[4], enetaddr[5]);
debug("... %pM\n", enetaddr);
+ return 0; +} diff --git a/board/enclustra/common/mac_ds28.c b/board/enclustra/common/mac_ds28.c new file mode 100644 index 0000000000..9aed4f1de1 --- /dev/null +++ b/board/enclustra/common/mac_ds28.c @@ -0,0 +1,88 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (c) 2024 Enclustra GmbH + */ + +#include <linux/compat.h> +#include <dm.h> +#include <i2c.h> +#include <net.h> + +#include "enclustra_mac.h" + +#define DS28_I2C_ADDR 0x5C +#define DS28_SYS_I2C_EEPROM_BUS 0 + +int enclustra_get_mac_from_eeprom(unsigned char *enetaddr, const char *alias) +{ + struct udevice *dev; + u32 hwaddr_h; + struct dm_i2c_chip *chip; + uint chip_addr = DS28_I2C_ADDR; + int alen = 1; + int ret; + + if (i2c_get_chip_for_busnum(DS28_SYS_I2C_EEPROM_BUS, chip_addr, + alen, &dev)) + return -ENODEV; + + chip = dev_get_parent_plat(dev); + if (chip->offset_len != alen) { + debug("I2C chip %x: alen %d does not match offset_len %d\n", + chip_addr, alen, chip->offset_len); + return -EADDRNOTAVAIL; + } + + ret = dm_i2c_read(dev, 0x10, enetaddr, 6); + if (ret) { + printf("%s(): Failed reading EEPROM! ret = %d\n", __func__, ret); + return ret; + } + + /* Check if the value is a valid mac registered for + * Enclustra GmbH + */ + hwaddr_h = enetaddr[0] | enetaddr[1] << 8 | enetaddr[2] << 16;
Better use () around the shifts, i.e. ... | (enetaddr[i] << 8) | ...
+ if ((hwaddr_h & 0xFFFFFF) != ENCLUSTRA_MAC) { + printf("%s(): Failed, parsed MAC is no Enclustra MAC\n", __func__); + return -ENOENT; + } + + if (!is_valid_ethaddr(enetaddr)) { + printf("%s(): Failed, address read from EEPROM is invalid!\n", + __func__); + return -EINVAL; + } + + printf("ethaddr set to %02X:%02X:%02X:%02X:%02X:%02X\n", + enetaddr[0], enetaddr[1], enetaddr[2], + enetaddr[3], enetaddr[4], enetaddr[5]); + + return 0; +} + +__weak int enclustra_setup_mac_address(void)
Is the __weak really necessary ?
+{ + unsigned char enetaddr[6]; + + if (enclustra_mac_is_in_env("ethaddr")) + return 0; + + if (enclustra_get_mac_is_enabled("ethernet0")) + return 0; + + // NB: DS28 is still not available in official DT, so referencing + // here by i2c busnumber and address directly + // preparation for DT access here, though
Use C comment style /* */
+ if (enclustra_get_mac_from_eeprom(enetaddr, "")) + return -ENXIO; + + if (eth_env_set_enetaddr("ethaddr", enetaddr)) + return -ENXIO; + + if (!enclustra_get_mac1_from_mac(enetaddr)) + return eth_env_set_enetaddr("eth1addr", enetaddr); + + printf("%s(): Failed, unable to set mac address!\n", __func__);
MAC address, MAC in capitals. [...]
--- a/drivers/misc/Kconfig +++ b/drivers/misc/Kconfig @@ -73,7 +73,7 @@ config ATSHA204A help Enable support for I2C connected Atmel's ATSHA204A CryptoAuthentication module found for example on the Turris Omnia - board. + board and Enclustra SoC FPGA boards.
This change should be elsewhere in this series ?