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 ?