Re: [U-Boot] [PATCH 11/11] tools: Add tool to add crc8 to a mac address

Hi Olliver, (is it one l or two?)

On 8 November 2016 at 08:54, Olliver Schinagl oliver@schinagl.nl wrote:

This patch adds a little tool that takes a generic MAC address and generates a CRC byte for it. The output is the full MAC address without any separators, ready written into an EEPROM.

Signed-off-by: Olliver Schinagl o.schinagl@ultimaker.com

tools/.gitignore | 1 + tools/Makefile | 4 ++++ tools/gen_ethaddr_crc.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+) create mode 100644 tools/gen_ethaddr_crc.c

diff --git a/tools/.gitignore b/tools/.gitignore index cb1e722..0d1f076 100644 --- a/tools/.gitignore +++ b/tools/.gitignore @@ -6,6 +6,7 @@ /fit_check_sign /fit_info /gen_eth_addr +/gen_ethaddr_crc /ifdtool /img2srec /kwboot diff --git a/tools/Makefile b/tools/Makefile index 06afdb0..4879ded 100644 --- a/tools/Makefile +++ b/tools/Makefile @@ -43,6 +43,10 @@ envcrc-objs := envcrc.o lib/crc32.o common/env_embedded.o lib/sha1.o hostprogs-$(CONFIG_CMD_NET) += gen_eth_addr HOSTCFLAGS_gen_eth_addr.o := -pedantic

+hostprogs-$(CONFIG_CMD_NET) += gen_ethaddr_crc +gen_ethaddr_crc-objs := gen_ethaddr_crc.o lib/crc8.o +HOSTCFLAGS_gen_ethaddr_crc.o := -pedantic

hostprogs-$(CONFIG_CMD_LOADS) += img2srec HOSTCFLAGS_img2srec.o := -pedantic

diff --git a/tools/gen_ethaddr_crc.c b/tools/gen_ethaddr_crc.c new file mode 100644 index 0000000..9b5bdb0 --- /dev/null +++ b/tools/gen_ethaddr_crc.c @@ -0,0 +1,52 @@ +/*

• • (C) Copyright 2016
• • Olliver Schinagl oliver@schinagl.nl
• • SPDX-License-Identifier: GPL-2.0+
• */

+#include <ctype.h> +#include <stdint.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <u-boot/crc.h>

+#define ARP_HLEN 6 /* Length of hardware address */

Is there no #define for this in standard headers?

+int main(int argc, char *argv[]) +{

•   uint_fast8_t i;
  

•   uint8_t mac_addr[ARP_HLEN + 1] = { 0x00 };
  

Why do you need to init it?

•   if (argc < 2) {
  

•           puts("Please supply a MAC address.");
  

How about a usage() function which gives generic help as well?

•           return -1;
  

•   }
  

•   if (!((strlen(argv[1]) == 12) || (strlen(argv[1]) == 17))) {
  

•           puts("Please supply a valid MAC address with optionaly\n"
  

optionally. But do you mean 'Please supply a valid MAC address with optional - or : separators?'

•                "dashes (-) or colons (:) as seperators.");
  

separators.

•           return -1;
  

•   }
  

•   i = 0;
  

•   while (*argv[1] != '\0') {
  

How about putting this code in a separate function:

int process_mac(const char *max)

so you don't have to access argv[1] all the time. Also you can return 1 instead of -1 from main() on error.

•           char nibble[2] = { 0x00, '\n' }; /* for strtol */
  

•           nibble[0] = *argv[1]++;
  

•           if (isxdigit(nibble[0])) {
  

•                   if (isupper(nibble[0]))
  

•                           nibble[0] = tolower(nibble[0]);
  

•                   mac_addr[i >> 1] |= strtol(nibble, NULL, 16) << ((i % 2) ? 0 : 4) & ((i % 2) ? 0x0f : 0xf0);
  

How about a nibble_to_hex() function here? This is strange-looking code. I'm wondering what you are trying to do.

•                   i++;
  

•           }
  

•   }
  

•   mac_addr[ARP_HLEN] = crc8(0, mac_addr, ARP_HLEN);
  

I suggest putting this in a separate variable...

•   for (i = 0; i < ARP_HLEN + 1; i++)
  

•           printf("%.2x", mac_addr[i]);
  

•   putchar('\n');
  

and here: printf("%.2x\n", crc)

•   return 0;
  

+}

2.10.2

Regards, Simon