On 3/28/24 12:17, Marek Behún wrote:
On Thu, 28 Mar 2024 10:56:01 +0100 Stefan Roese sr@denx.de wrote:
On 3/27/24 17:23, Marek Behún wrote:
Implement reading board serial number, first MAC address and board version from MCU. MCU supports board information if the FEAT_BOARD_INFO feature bit is set in MCU features.
Prefer getting board information from MCU if supported, fallback to Atmel SHA chip.
Signed-off-by: Marek Behún kabel@kernel.org
Minor comment below. Other than this:
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
board/CZ.NIC/turris_atsha_otp.c | 27 +------ board/CZ.NIC/turris_omnia/Makefile | 2 +- board/CZ.NIC/turris_omnia/turris_omnia.c | 94 +++++++++++++++++++++++- 3 files changed, 93 insertions(+), 30 deletions(-)
diff --git a/board/CZ.NIC/turris_atsha_otp.c b/board/CZ.NIC/turris_atsha_otp.c index a29fe36231..85eebcdf18 100644 --- a/board/CZ.NIC/turris_atsha_otp.c +++ b/board/CZ.NIC/turris_atsha_otp.c @@ -11,6 +11,7 @@ #include <atsha204a-i2c.h>
#include "turris_atsha_otp.h" +#include "turris_common.h"
#define TURRIS_ATSHA_OTP_VERSION 0 #define TURRIS_ATSHA_OTP_SERIAL 1 @@ -32,26 +33,6 @@ static struct udevice *get_atsha204a_dev(void) return dev; }
-static void increment_mac(u8 *mac) -{
- int i;
- for (i = 5; i >= 3; i--) {
mac[i] += 1;if (mac[i])break;- }
-}
-static void set_mac_if_invalid(int i, u8 *mac) -{
- u8 oldmac[6];
- if (is_valid_ethaddr(mac) &&
!eth_env_get_enetaddr_by_index("eth", i, oldmac))eth_env_set_enetaddr_by_index("eth", i, mac);-}
- int turris_atsha_otp_init_mac_addresses(int first_idx) { struct udevice *dev = get_atsha204a_dev();
@@ -84,11 +65,7 @@ int turris_atsha_otp_init_mac_addresses(int first_idx) mac[4] = mac1[2]; mac[5] = mac1[3];
- set_mac_if_invalid((first_idx + 0) % 3, mac);
- increment_mac(mac);
- set_mac_if_invalid((first_idx + 1) % 3, mac);
- increment_mac(mac);
- set_mac_if_invalid((first_idx + 2) % 3, mac);
turris_init_mac_addresses(first_idx, mac);
return 0; }
diff --git a/board/CZ.NIC/turris_omnia/Makefile b/board/CZ.NIC/turris_omnia/Makefile index dc39b44ae1..341378b4e5 100644 --- a/board/CZ.NIC/turris_omnia/Makefile +++ b/board/CZ.NIC/turris_omnia/Makefile @@ -2,4 +2,4 @@ # # Copyright (C) 2017 Marek Behún kabel@kernel.org
-obj-y := turris_omnia.o ../turris_atsha_otp.o +obj-y := turris_omnia.o ../turris_atsha_otp.o ../turris_common.o diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c b/board/CZ.NIC/turris_omnia/turris_omnia.c index 6dfde5ee7a..f63640ad64 100644 --- a/board/CZ.NIC/turris_omnia/turris_omnia.c +++ b/board/CZ.NIC/turris_omnia/turris_omnia.c @@ -18,6 +18,7 @@ #include <asm/io.h> #include <asm/arch/cpu.h> #include <asm/arch/soc.h> +#include <asm/unaligned.h> #include <dm/uclass.h> #include <dt-bindings/gpio/gpio.h> #include <fdt_support.h> @@ -25,12 +26,14 @@ #include <time.h> #include <turris-omnia-mcu-interface.h> #include <linux/bitops.h> +#include <linux/bitrev.h> #include <linux/delay.h> #include <u-boot/crc.h>
#include "../drivers/ddr/marvell/a38x/ddr3_init.h" #include <../serdes/a38x/high_speed_env_spec.h> #include "../turris_atsha_otp.h" +#include "../turris_common.h"
DECLARE_GLOBAL_DATA_PTR;
@@ -186,6 +189,70 @@ static bool omnia_mcu_has_feature(u32 feature) return feature & features; }
+static u32 omnia_mcu_crc32(const void *p, size_t len) +{
- u32 val, crc = 0;
- compiletime_assert(!(len % 4), "length has to be a multiple of 4");
- while (len) {
val = bitrev32(get_unaligned_le32(p));crc = crc32(crc, (void *)&val, 4);p += 4;len -= 4;- }
- return ~bitrev32(crc);
+}
+/* Can only be called after relocation, since it needs cleared BSS */ +static int omnia_mcu_board_info(char *serial, u8 *mac, char *version) +{
- static u8 reply[17];
- static bool cached;
- if (!cached) {
u8 csum;int ret;ret = omnia_mcu_read(CMD_BOARD_INFO_GET, reply, sizeof(reply));if (ret)return ret;if (reply[0] != 16)return -EBADMSG;csum = reply[16];reply[16] = 0;if ((omnia_mcu_crc32(&reply[1], 16) & 0xff) != csum)return -EBADMSG;cached = true;- }
- if (serial) {
const char *serial_env;serial_env = env_get("serial#");if (serial_env && strlen(serial_env) == 16) {strcpy(serial, serial_env);Usage of strcpy() is discouraged AFAIK.
Yeah, it would make sense to use strncpy, but the potential leak is prevented by strlen() check.
strncpy is also not optimal. Please see here if you are interested:
https://www.geeksforgeeks.org/why-strcpy-and-strncpy-are-not-safe-to-use/
AFAIU, there is no pressing need to change this implementation here though right now. I just wanted to make this comment.
Thanks, Stefan