* [PATCH 01/12] cmd: tlv_eeprom: remove use of global variable current_dev
2022-05-02 14:18 [PATCH 00/12] split tlv_eeprom command into a separate library Josua Mayer
@ 2022-05-02 14:18 ` Josua Mayer
2022-05-03 6:09 ` Stefan Roese
2022-05-02 14:18 ` [PATCH 02/12] cmd: tlv_eeprom: remove use of global variable has_been_read Josua Mayer
` (10 subsequent siblings)
11 siblings, 1 reply; 22+ messages in thread
From: Josua Mayer @ 2022-05-02 14:18 UTC (permalink / raw)
To: u-boot; +Cc: Josua Mayer, Simon Glass, Stefan Roese, Sven Auhagen
Make tlv_eeprom command device selection an explicit parameter of all
function calls.
Signed-off-by: Josua Mayer <josua@solid-run.com>
---
cmd/tlv_eeprom.c | 50 ++++++++++++++++++++++----------------------
include/tlv_eeprom.h | 3 ++-
2 files changed, 27 insertions(+), 26 deletions(-)
diff --git a/cmd/tlv_eeprom.c b/cmd/tlv_eeprom.c
index bf8d453dc5..f91c11b304 100644
--- a/cmd/tlv_eeprom.c
+++ b/cmd/tlv_eeprom.c
@@ -29,18 +29,18 @@ DECLARE_GLOBAL_DATA_PTR;
/* File scope function prototypes */
static bool is_checksum_valid(u8 *eeprom);
-static int read_eeprom(u8 *eeprom);
-static void show_eeprom(u8 *eeprom);
+static int read_eeprom(int devnum, u8 *eeprom);
+static void show_eeprom(int devnum, u8 *eeprom);
static void decode_tlv(struct tlvinfo_tlv *tlv);
static void update_crc(u8 *eeprom);
-static int prog_eeprom(u8 *eeprom);
+static int prog_eeprom(int devnum, u8 *eeprom);
static bool tlvinfo_find_tlv(u8 *eeprom, u8 tcode, int *eeprom_index);
static bool tlvinfo_delete_tlv(u8 *eeprom, u8 code);
static bool tlvinfo_add_tlv(u8 *eeprom, int tcode, char *strval);
static int set_mac(char *buf, const char *string);
static int set_date(char *buf, const char *string);
static int set_bytes(char *buf, const char *string, int *converted_accum);
-static void show_tlv_devices(void);
+static void show_tlv_devices(int current_dev);
/* Set to 1 if we've read EEPROM into memory */
static int has_been_read;
@@ -48,7 +48,6 @@ static int has_been_read;
static u8 eeprom[TLV_INFO_MAX_LEN];
static struct udevice *tlv_devices[MAX_TLV_DEVICES];
-static unsigned int current_dev;
#define to_header(p) ((struct tlvinfo_header *)p)
#define to_entry(p) ((struct tlvinfo_tlv *)p)
@@ -125,7 +124,7 @@ static bool is_checksum_valid(u8 *eeprom)
*
* Read the EEPROM into memory, if it hasn't already been read.
*/
-static int read_eeprom(u8 *eeprom)
+static int read_eeprom(int devnum, u8 *eeprom)
{
int ret;
struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
@@ -135,12 +134,11 @@ static int read_eeprom(u8 *eeprom)
return 0;
/* Read the header */
- ret = read_tlv_eeprom((void *)eeprom_hdr, 0, HDR_SIZE, current_dev);
+ ret = read_tlv_eeprom((void *)eeprom_hdr, 0, HDR_SIZE, devnum);
/* If the header was successfully read, read the TLVs */
if (ret == 0 && is_valid_tlvinfo_header(eeprom_hdr))
ret = read_tlv_eeprom((void *)eeprom_tlv, HDR_SIZE,
- be16_to_cpu(eeprom_hdr->totallen),
- current_dev);
+ be16_to_cpu(eeprom_hdr->totallen), devnum);
// If the contents are invalid, start over with default contents
if (!is_valid_tlvinfo_header(eeprom_hdr) ||
@@ -165,7 +163,7 @@ static int read_eeprom(u8 *eeprom)
*
* Display the contents of the EEPROM
*/
-static void show_eeprom(u8 *eeprom)
+static void show_eeprom(int devnum, u8 *eeprom)
{
int tlv_end;
int curr_tlv;
@@ -180,7 +178,7 @@ static void show_eeprom(u8 *eeprom)
return;
}
- printf("TLV: %u\n", current_dev);
+ printf("TLV: %u\n", devnum);
printf("TlvInfo Header:\n");
printf(" Id String: %s\n", eeprom_hdr->signature);
printf(" Version: %d\n", eeprom_hdr->version);
@@ -389,7 +387,7 @@ static void update_crc(u8 *eeprom)
*
* Write the EEPROM data from CPU memory to the hardware.
*/
-static int prog_eeprom(u8 *eeprom)
+static int prog_eeprom(int devnum, u8 *eeprom)
{
int ret = 0;
struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
@@ -398,7 +396,7 @@ static int prog_eeprom(u8 *eeprom)
update_crc(eeprom);
eeprom_len = HDR_SIZE + be16_to_cpu(eeprom_hdr->totallen);
- ret = write_tlv_eeprom(eeprom, eeprom_len);
+ ret = write_tlv_eeprom(eeprom, eeprom_len, devnum);
if (ret) {
printf("Programming failed.\n");
return -1;
@@ -433,11 +431,12 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
{
char cmd;
struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
+ static unsigned int current_dev;
// If no arguments, read the EERPOM and display its contents
if (argc == 1) {
- read_eeprom(eeprom);
- show_eeprom(eeprom);
+ read_eeprom(current_dev, eeprom);
+ show_eeprom(current_dev, eeprom);
return 0;
}
@@ -448,7 +447,7 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
// Read the EEPROM contents
if (cmd == 'r') {
has_been_read = 0;
- if (!read_eeprom(eeprom))
+ if (!read_eeprom(current_dev, eeprom))
printf("EEPROM data loaded from device to memory.\n");
return 0;
}
@@ -463,7 +462,7 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
if (argc == 2) {
switch (cmd) {
case 'w': /* write */
- prog_eeprom(eeprom);
+ prog_eeprom(current_dev, eeprom);
break;
case 'e': /* erase */
strcpy(eeprom_hdr->signature, TLV_INFO_ID_STRING);
@@ -476,7 +475,7 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
show_tlv_code_list();
break;
case 'd': /* dev */
- show_tlv_devices();
+ show_tlv_devices(current_dev);
break;
default:
cmd_usage(cmdtp);
@@ -886,7 +885,7 @@ static int set_bytes(char *buf, const char *string, int *converted_accum)
return 0;
}
-static void show_tlv_devices(void)
+static void show_tlv_devices(int current_dev)
{
unsigned int dev;
@@ -956,14 +955,14 @@ int read_tlv_eeprom(void *eeprom, int offset, int len, int dev_num)
/**
* write_tlv_eeprom - write the hwinfo to i2c EEPROM
*/
-int write_tlv_eeprom(void *eeprom, int len)
+int write_tlv_eeprom(void *eeprom, int len, int dev)
{
if (!(gd->flags & GD_FLG_RELOC))
return -ENODEV;
- if (!tlv_devices[current_dev])
+ if (!tlv_devices[dev])
return -ENODEV;
- return i2c_eeprom_write(tlv_devices[current_dev], 0, eeprom, len);
+ return i2c_eeprom_write(tlv_devices[dev], 0, eeprom, len);
}
int read_tlvinfo_tlv_eeprom(void *eeprom, struct tlvinfo_header **hdr,
@@ -1018,10 +1017,11 @@ int mac_read_from_eeprom(void)
int maccount;
u8 macbase[6];
struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
+ int devnum = 0; // TODO: support multiple EEPROMs
puts("EEPROM: ");
- if (read_eeprom(eeprom)) {
+ if (read_eeprom(devnum, eeprom)) {
printf("Read failed.\n");
return -1;
}
@@ -1086,7 +1086,7 @@ int mac_read_from_eeprom(void)
*
* This function must be called after relocation.
*/
-int populate_serial_number(void)
+int populate_serial_number(int devnum)
{
char serialstr[257];
int eeprom_index;
@@ -1095,7 +1095,7 @@ int populate_serial_number(void)
if (env_get("serial#"))
return 0;
- if (read_eeprom(eeprom)) {
+ if (read_eeprom(devnum, eeprom)) {
printf("Read failed.\n");
return -1;
}
diff --git a/include/tlv_eeprom.h b/include/tlv_eeprom.h
index a2c333e744..fd45e5f6eb 100644
--- a/include/tlv_eeprom.h
+++ b/include/tlv_eeprom.h
@@ -84,11 +84,12 @@ int read_tlv_eeprom(void *eeprom, int offset, int len, int dev);
* write_tlv_eeprom - Write the entire EEPROM binary data to the hardware
* @eeprom: Pointer to buffer to hold the binary data
* @len : Maximum size of buffer
+ * @dev : EEPROM device to write
*
* Note: this routine does not validate the EEPROM data.
*
*/
-int write_tlv_eeprom(void *eeprom, int len);
+int write_tlv_eeprom(void *eeprom, int len, int dev);
/**
* read_tlvinfo_tlv_eeprom - Read the TLV from EEPROM, and validate
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 01/12] cmd: tlv_eeprom: remove use of global variable current_dev
2022-05-02 14:18 ` [PATCH 01/12] cmd: tlv_eeprom: remove use of global variable current_dev Josua Mayer
@ 2022-05-03 6:09 ` Stefan Roese
2022-05-03 6:52 ` Josua Mayer
0 siblings, 1 reply; 22+ messages in thread
From: Stefan Roese @ 2022-05-03 6:09 UTC (permalink / raw)
To: Josua Mayer, u-boot; +Cc: Simon Glass, Sven Auhagen
Hi Josua,
a few general comments about this series:
- A cover letter for this patchset would be very helpful, so that
reviewers have a summary of the changes.
- Please Cc the original author of this code Baruch Siach in the
next version as well (if there is next version that is).
On 02.05.22 16:18, Josua Mayer wrote:
> Make tlv_eeprom command device selection an explicit parameter of all
> function calls.
>
> Signed-off-by: Josua Mayer <josua@solid-run.com>
Reviewed-by: Stefan Roese <sr@denx.de>
Thanks,
Stefan
> ---
> cmd/tlv_eeprom.c | 50 ++++++++++++++++++++++----------------------
> include/tlv_eeprom.h | 3 ++-
> 2 files changed, 27 insertions(+), 26 deletions(-)
>
> diff --git a/cmd/tlv_eeprom.c b/cmd/tlv_eeprom.c
> index bf8d453dc5..f91c11b304 100644
> --- a/cmd/tlv_eeprom.c
> +++ b/cmd/tlv_eeprom.c
> @@ -29,18 +29,18 @@ DECLARE_GLOBAL_DATA_PTR;
>
> /* File scope function prototypes */
> static bool is_checksum_valid(u8 *eeprom);
> -static int read_eeprom(u8 *eeprom);
> -static void show_eeprom(u8 *eeprom);
> +static int read_eeprom(int devnum, u8 *eeprom);
> +static void show_eeprom(int devnum, u8 *eeprom);
> static void decode_tlv(struct tlvinfo_tlv *tlv);
> static void update_crc(u8 *eeprom);
> -static int prog_eeprom(u8 *eeprom);
> +static int prog_eeprom(int devnum, u8 *eeprom);
> static bool tlvinfo_find_tlv(u8 *eeprom, u8 tcode, int *eeprom_index);
> static bool tlvinfo_delete_tlv(u8 *eeprom, u8 code);
> static bool tlvinfo_add_tlv(u8 *eeprom, int tcode, char *strval);
> static int set_mac(char *buf, const char *string);
> static int set_date(char *buf, const char *string);
> static int set_bytes(char *buf, const char *string, int *converted_accum);
> -static void show_tlv_devices(void);
> +static void show_tlv_devices(int current_dev);
>
> /* Set to 1 if we've read EEPROM into memory */
> static int has_been_read;
> @@ -48,7 +48,6 @@ static int has_been_read;
> static u8 eeprom[TLV_INFO_MAX_LEN];
>
> static struct udevice *tlv_devices[MAX_TLV_DEVICES];
> -static unsigned int current_dev;
>
> #define to_header(p) ((struct tlvinfo_header *)p)
> #define to_entry(p) ((struct tlvinfo_tlv *)p)
> @@ -125,7 +124,7 @@ static bool is_checksum_valid(u8 *eeprom)
> *
> * Read the EEPROM into memory, if it hasn't already been read.
> */
> -static int read_eeprom(u8 *eeprom)
> +static int read_eeprom(int devnum, u8 *eeprom)
> {
> int ret;
> struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
> @@ -135,12 +134,11 @@ static int read_eeprom(u8 *eeprom)
> return 0;
>
> /* Read the header */
> - ret = read_tlv_eeprom((void *)eeprom_hdr, 0, HDR_SIZE, current_dev);
> + ret = read_tlv_eeprom((void *)eeprom_hdr, 0, HDR_SIZE, devnum);
> /* If the header was successfully read, read the TLVs */
> if (ret == 0 && is_valid_tlvinfo_header(eeprom_hdr))
> ret = read_tlv_eeprom((void *)eeprom_tlv, HDR_SIZE,
> - be16_to_cpu(eeprom_hdr->totallen),
> - current_dev);
> + be16_to_cpu(eeprom_hdr->totallen), devnum);
>
> // If the contents are invalid, start over with default contents
> if (!is_valid_tlvinfo_header(eeprom_hdr) ||
> @@ -165,7 +163,7 @@ static int read_eeprom(u8 *eeprom)
> *
> * Display the contents of the EEPROM
> */
> -static void show_eeprom(u8 *eeprom)
> +static void show_eeprom(int devnum, u8 *eeprom)
> {
> int tlv_end;
> int curr_tlv;
> @@ -180,7 +178,7 @@ static void show_eeprom(u8 *eeprom)
> return;
> }
>
> - printf("TLV: %u\n", current_dev);
> + printf("TLV: %u\n", devnum);
> printf("TlvInfo Header:\n");
> printf(" Id String: %s\n", eeprom_hdr->signature);
> printf(" Version: %d\n", eeprom_hdr->version);
> @@ -389,7 +387,7 @@ static void update_crc(u8 *eeprom)
> *
> * Write the EEPROM data from CPU memory to the hardware.
> */
> -static int prog_eeprom(u8 *eeprom)
> +static int prog_eeprom(int devnum, u8 *eeprom)
> {
> int ret = 0;
> struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
> @@ -398,7 +396,7 @@ static int prog_eeprom(u8 *eeprom)
> update_crc(eeprom);
>
> eeprom_len = HDR_SIZE + be16_to_cpu(eeprom_hdr->totallen);
> - ret = write_tlv_eeprom(eeprom, eeprom_len);
> + ret = write_tlv_eeprom(eeprom, eeprom_len, devnum);
> if (ret) {
> printf("Programming failed.\n");
> return -1;
> @@ -433,11 +431,12 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
> {
> char cmd;
> struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
> + static unsigned int current_dev;
>
> // If no arguments, read the EERPOM and display its contents
> if (argc == 1) {
> - read_eeprom(eeprom);
> - show_eeprom(eeprom);
> + read_eeprom(current_dev, eeprom);
> + show_eeprom(current_dev, eeprom);
> return 0;
> }
>
> @@ -448,7 +447,7 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
> // Read the EEPROM contents
> if (cmd == 'r') {
> has_been_read = 0;
> - if (!read_eeprom(eeprom))
> + if (!read_eeprom(current_dev, eeprom))
> printf("EEPROM data loaded from device to memory.\n");
> return 0;
> }
> @@ -463,7 +462,7 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
> if (argc == 2) {
> switch (cmd) {
> case 'w': /* write */
> - prog_eeprom(eeprom);
> + prog_eeprom(current_dev, eeprom);
> break;
> case 'e': /* erase */
> strcpy(eeprom_hdr->signature, TLV_INFO_ID_STRING);
> @@ -476,7 +475,7 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
> show_tlv_code_list();
> break;
> case 'd': /* dev */
> - show_tlv_devices();
> + show_tlv_devices(current_dev);
> break;
> default:
> cmd_usage(cmdtp);
> @@ -886,7 +885,7 @@ static int set_bytes(char *buf, const char *string, int *converted_accum)
> return 0;
> }
>
> -static void show_tlv_devices(void)
> +static void show_tlv_devices(int current_dev)
> {
> unsigned int dev;
>
> @@ -956,14 +955,14 @@ int read_tlv_eeprom(void *eeprom, int offset, int len, int dev_num)
> /**
> * write_tlv_eeprom - write the hwinfo to i2c EEPROM
> */
> -int write_tlv_eeprom(void *eeprom, int len)
> +int write_tlv_eeprom(void *eeprom, int len, int dev)
> {
> if (!(gd->flags & GD_FLG_RELOC))
> return -ENODEV;
> - if (!tlv_devices[current_dev])
> + if (!tlv_devices[dev])
> return -ENODEV;
>
> - return i2c_eeprom_write(tlv_devices[current_dev], 0, eeprom, len);
> + return i2c_eeprom_write(tlv_devices[dev], 0, eeprom, len);
> }
>
> int read_tlvinfo_tlv_eeprom(void *eeprom, struct tlvinfo_header **hdr,
> @@ -1018,10 +1017,11 @@ int mac_read_from_eeprom(void)
> int maccount;
> u8 macbase[6];
> struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
> + int devnum = 0; // TODO: support multiple EEPROMs
>
> puts("EEPROM: ");
>
> - if (read_eeprom(eeprom)) {
> + if (read_eeprom(devnum, eeprom)) {
> printf("Read failed.\n");
> return -1;
> }
> @@ -1086,7 +1086,7 @@ int mac_read_from_eeprom(void)
> *
> * This function must be called after relocation.
> */
> -int populate_serial_number(void)
> +int populate_serial_number(int devnum)
> {
> char serialstr[257];
> int eeprom_index;
> @@ -1095,7 +1095,7 @@ int populate_serial_number(void)
> if (env_get("serial#"))
> return 0;
>
> - if (read_eeprom(eeprom)) {
> + if (read_eeprom(devnum, eeprom)) {
> printf("Read failed.\n");
> return -1;
> }
> diff --git a/include/tlv_eeprom.h b/include/tlv_eeprom.h
> index a2c333e744..fd45e5f6eb 100644
> --- a/include/tlv_eeprom.h
> +++ b/include/tlv_eeprom.h
> @@ -84,11 +84,12 @@ int read_tlv_eeprom(void *eeprom, int offset, int len, int dev);
> * write_tlv_eeprom - Write the entire EEPROM binary data to the hardware
> * @eeprom: Pointer to buffer to hold the binary data
> * @len : Maximum size of buffer
> + * @dev : EEPROM device to write
> *
> * Note: this routine does not validate the EEPROM data.
> *
> */
> -int write_tlv_eeprom(void *eeprom, int len);
> +int write_tlv_eeprom(void *eeprom, int len, int dev);
>
> /**
> * read_tlvinfo_tlv_eeprom - Read the TLV from EEPROM, and validate
Viele Grüße,
Stefan Roese
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 01/12] cmd: tlv_eeprom: remove use of global variable current_dev
2022-05-03 6:09 ` Stefan Roese
@ 2022-05-03 6:52 ` Josua Mayer
0 siblings, 0 replies; 22+ messages in thread
From: Josua Mayer @ 2022-05-03 6:52 UTC (permalink / raw)
To: Stefan Roese, u-boot; +Cc: Simon Glass, Sven Auhagen
Hi Stefan,
Thank you for starting to review this patchset!
Am 03.05.22 um 09:09 schrieb Stefan Roese:
> Hi Josua,
>
> a few general comments about this series:
>
> - A cover letter for this patchset would be very helpful, so that
> reviewers have a summary of the changes.
I have sent a cover-letter to the list, but you are not in the To field
- perhaps you can find it - as it does give an overview what I am trying
to do (subject [PATCH 00/12] split tlv_eeprom command into a separate
library)
> - Please Cc the original author of this code Baruch Siach in the
> next version as well (if there is next version that is).
Made a mental note, will do.
>
> On 02.05.22 16:18, Josua Mayer wrote:
>> Make tlv_eeprom command device selection an explicit parameter of all
>> function calls.
>>
>> Signed-off-by: Josua Mayer <josua@solid-run.com>
>
> Reviewed-by: Stefan Roese <sr@denx.de>
>
> Thanks,
> Stefan
>
>> ---
>> cmd/tlv_eeprom.c | 50 ++++++++++++++++++++++----------------------
>> include/tlv_eeprom.h | 3 ++-
>> 2 files changed, 27 insertions(+), 26 deletions(-)
>>
>> diff --git a/cmd/tlv_eeprom.c b/cmd/tlv_eeprom.c
>> index bf8d453dc5..f91c11b304 100644
>> --- a/cmd/tlv_eeprom.c
>> +++ b/cmd/tlv_eeprom.c
>> @@ -29,18 +29,18 @@ DECLARE_GLOBAL_DATA_PTR;
>> /* File scope function prototypes */
>> static bool is_checksum_valid(u8 *eeprom);
>> -static int read_eeprom(u8 *eeprom);
>> -static void show_eeprom(u8 *eeprom);
>> +static int read_eeprom(int devnum, u8 *eeprom);
>> +static void show_eeprom(int devnum, u8 *eeprom);
>> static void decode_tlv(struct tlvinfo_tlv *tlv);
>> static void update_crc(u8 *eeprom);
>> -static int prog_eeprom(u8 *eeprom);
>> +static int prog_eeprom(int devnum, u8 *eeprom);
>> static bool tlvinfo_find_tlv(u8 *eeprom, u8 tcode, int *eeprom_index);
>> static bool tlvinfo_delete_tlv(u8 *eeprom, u8 code);
>> static bool tlvinfo_add_tlv(u8 *eeprom, int tcode, char *strval);
>> static int set_mac(char *buf, const char *string);
>> static int set_date(char *buf, const char *string);
>> static int set_bytes(char *buf, const char *string, int
>> *converted_accum);
>> -static void show_tlv_devices(void);
>> +static void show_tlv_devices(int current_dev);
>> /* Set to 1 if we've read EEPROM into memory */
>> static int has_been_read;
>> @@ -48,7 +48,6 @@ static int has_been_read;
>> static u8 eeprom[TLV_INFO_MAX_LEN];
>> static struct udevice *tlv_devices[MAX_TLV_DEVICES];
>> -static unsigned int current_dev;
>> #define to_header(p) ((struct tlvinfo_header *)p)
>> #define to_entry(p) ((struct tlvinfo_tlv *)p)
>> @@ -125,7 +124,7 @@ static bool is_checksum_valid(u8 *eeprom)
>> *
>> * Read the EEPROM into memory, if it hasn't already been read.
>> */
>> -static int read_eeprom(u8 *eeprom)
>> +static int read_eeprom(int devnum, u8 *eeprom)
>> {
>> int ret;
>> struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
>> @@ -135,12 +134,11 @@ static int read_eeprom(u8 *eeprom)
>> return 0;
>> /* Read the header */
>> - ret = read_tlv_eeprom((void *)eeprom_hdr, 0, HDR_SIZE,
>> current_dev);
>> + ret = read_tlv_eeprom((void *)eeprom_hdr, 0, HDR_SIZE, devnum);
>> /* If the header was successfully read, read the TLVs */
>> if (ret == 0 && is_valid_tlvinfo_header(eeprom_hdr))
>> ret = read_tlv_eeprom((void *)eeprom_tlv, HDR_SIZE,
>> - be16_to_cpu(eeprom_hdr->totallen),
>> - current_dev);
>> + be16_to_cpu(eeprom_hdr->totallen), devnum);
>> // If the contents are invalid, start over with default contents
>> if (!is_valid_tlvinfo_header(eeprom_hdr) ||
>> @@ -165,7 +163,7 @@ static int read_eeprom(u8 *eeprom)
>> *
>> * Display the contents of the EEPROM
>> */
>> -static void show_eeprom(u8 *eeprom)
>> +static void show_eeprom(int devnum, u8 *eeprom)
>> {
>> int tlv_end;
>> int curr_tlv;
>> @@ -180,7 +178,7 @@ static void show_eeprom(u8 *eeprom)
>> return;
>> }
>> - printf("TLV: %u\n", current_dev);
>> + printf("TLV: %u\n", devnum);
>> printf("TlvInfo Header:\n");
>> printf(" Id String: %s\n", eeprom_hdr->signature);
>> printf(" Version: %d\n", eeprom_hdr->version);
>> @@ -389,7 +387,7 @@ static void update_crc(u8 *eeprom)
>> *
>> * Write the EEPROM data from CPU memory to the hardware.
>> */
>> -static int prog_eeprom(u8 *eeprom)
>> +static int prog_eeprom(int devnum, u8 *eeprom)
>> {
>> int ret = 0;
>> struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
>> @@ -398,7 +396,7 @@ static int prog_eeprom(u8 *eeprom)
>> update_crc(eeprom);
>> eeprom_len = HDR_SIZE + be16_to_cpu(eeprom_hdr->totallen);
>> - ret = write_tlv_eeprom(eeprom, eeprom_len);
>> + ret = write_tlv_eeprom(eeprom, eeprom_len, devnum);
>> if (ret) {
>> printf("Programming failed.\n");
>> return -1;
>> @@ -433,11 +431,12 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int
>> flag, int argc, char *const argv[])
>> {
>> char cmd;
>> struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
>> + static unsigned int current_dev;
>> // If no arguments, read the EERPOM and display its contents
>> if (argc == 1) {
>> - read_eeprom(eeprom);
>> - show_eeprom(eeprom);
>> + read_eeprom(current_dev, eeprom);
>> + show_eeprom(current_dev, eeprom);
>> return 0;
>> }
>> @@ -448,7 +447,7 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int
>> flag, int argc, char *const argv[])
>> // Read the EEPROM contents
>> if (cmd == 'r') {
>> has_been_read = 0;
>> - if (!read_eeprom(eeprom))
>> + if (!read_eeprom(current_dev, eeprom))
>> printf("EEPROM data loaded from device to memory.\n");
>> return 0;
>> }
>> @@ -463,7 +462,7 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int
>> flag, int argc, char *const argv[])
>> if (argc == 2) {
>> switch (cmd) {
>> case 'w': /* write */
>> - prog_eeprom(eeprom);
>> + prog_eeprom(current_dev, eeprom);
>> break;
>> case 'e': /* erase */
>> strcpy(eeprom_hdr->signature, TLV_INFO_ID_STRING);
>> @@ -476,7 +475,7 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int
>> flag, int argc, char *const argv[])
>> show_tlv_code_list();
>> break;
>> case 'd': /* dev */
>> - show_tlv_devices();
>> + show_tlv_devices(current_dev);
>> break;
>> default:
>> cmd_usage(cmdtp);
>> @@ -886,7 +885,7 @@ static int set_bytes(char *buf, const char
>> *string, int *converted_accum)
>> return 0;
>> }
>> -static void show_tlv_devices(void)
>> +static void show_tlv_devices(int current_dev)
>> {
>> unsigned int dev;
>> @@ -956,14 +955,14 @@ int read_tlv_eeprom(void *eeprom, int offset,
>> int len, int dev_num)
>> /**
>> * write_tlv_eeprom - write the hwinfo to i2c EEPROM
>> */
>> -int write_tlv_eeprom(void *eeprom, int len)
>> +int write_tlv_eeprom(void *eeprom, int len, int dev)
>> {
>> if (!(gd->flags & GD_FLG_RELOC))
>> return -ENODEV;
>> - if (!tlv_devices[current_dev])
>> + if (!tlv_devices[dev])
>> return -ENODEV;
>> - return i2c_eeprom_write(tlv_devices[current_dev], 0, eeprom,
>> len);
>> + return i2c_eeprom_write(tlv_devices[dev], 0, eeprom, len);
>> }
>> int read_tlvinfo_tlv_eeprom(void *eeprom, struct tlvinfo_header
>> **hdr,
>> @@ -1018,10 +1017,11 @@ int mac_read_from_eeprom(void)
>> int maccount;
>> u8 macbase[6];
>> struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
>> + int devnum = 0; // TODO: support multiple EEPROMs
>> puts("EEPROM: ");
>> - if (read_eeprom(eeprom)) {
>> + if (read_eeprom(devnum, eeprom)) {
>> printf("Read failed.\n");
>> return -1;
>> }
>> @@ -1086,7 +1086,7 @@ int mac_read_from_eeprom(void)
>> *
>> * This function must be called after relocation.
>> */
>> -int populate_serial_number(void)
>> +int populate_serial_number(int devnum)
>> {
>> char serialstr[257];
>> int eeprom_index;
>> @@ -1095,7 +1095,7 @@ int populate_serial_number(void)
>> if (env_get("serial#"))
>> return 0;
>> - if (read_eeprom(eeprom)) {
>> + if (read_eeprom(devnum, eeprom)) {
>> printf("Read failed.\n");
>> return -1;
>> }
>> diff --git a/include/tlv_eeprom.h b/include/tlv_eeprom.h
>> index a2c333e744..fd45e5f6eb 100644
>> --- a/include/tlv_eeprom.h
>> +++ b/include/tlv_eeprom.h
>> @@ -84,11 +84,12 @@ int read_tlv_eeprom(void *eeprom, int offset, int
>> len, int dev);
>> * write_tlv_eeprom - Write the entire EEPROM binary data to the
>> hardware
>> * @eeprom: Pointer to buffer to hold the binary data
>> * @len : Maximum size of buffer
>> + * @dev : EEPROM device to write
>> *
>> * Note: this routine does not validate the EEPROM data.
>> *
>> */
>> -int write_tlv_eeprom(void *eeprom, int len);
>> +int write_tlv_eeprom(void *eeprom, int len, int dev);
>> /**
>> * read_tlvinfo_tlv_eeprom - Read the TLV from EEPROM, and validate
>
> Viele Grüße,
> Stefan Roese
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 02/12] cmd: tlv_eeprom: remove use of global variable has_been_read
2022-05-02 14:18 [PATCH 00/12] split tlv_eeprom command into a separate library Josua Mayer
2022-05-02 14:18 ` [PATCH 01/12] cmd: tlv_eeprom: remove use of global variable current_dev Josua Mayer
@ 2022-05-02 14:18 ` Josua Mayer
2022-05-03 6:11 ` Stefan Roese
2022-05-02 14:18 ` [PATCH 03/12] cmd: tlv_eeprom: do_tlv_eeprom: stop using non-api read_eeprom function Josua Mayer
` (9 subsequent siblings)
11 siblings, 1 reply; 22+ messages in thread
From: Josua Mayer @ 2022-05-02 14:18 UTC (permalink / raw)
To: u-boot; +Cc: Josua Mayer, Stefan Roese, Simon Glass, Sven Auhagen
has_been_read is only used as an optimization for do_tlv_eeprom.
Explicitly use and set inside this function, thus making read_eeprom
stateless.
Signed-off-by: Josua Mayer <josua@solid-run.com>
---
cmd/tlv_eeprom.c | 25 ++++++++++++-------------
1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/cmd/tlv_eeprom.c b/cmd/tlv_eeprom.c
index f91c11b304..bfd4882e0d 100644
--- a/cmd/tlv_eeprom.c
+++ b/cmd/tlv_eeprom.c
@@ -42,8 +42,6 @@ static int set_date(char *buf, const char *string);
static int set_bytes(char *buf, const char *string, int *converted_accum);
static void show_tlv_devices(int current_dev);
-/* Set to 1 if we've read EEPROM into memory */
-static int has_been_read;
/* The EERPOM contents after being read into memory */
static u8 eeprom[TLV_INFO_MAX_LEN];
@@ -130,9 +128,6 @@ static int read_eeprom(int devnum, u8 *eeprom)
struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
struct tlvinfo_tlv *eeprom_tlv = to_entry(&eeprom[HDR_SIZE]);
- if (has_been_read)
- return 0;
-
/* Read the header */
ret = read_tlv_eeprom((void *)eeprom_hdr, 0, HDR_SIZE, devnum);
/* If the header was successfully read, read the TLVs */
@@ -149,10 +144,8 @@ static int read_eeprom(int devnum, u8 *eeprom)
update_crc(eeprom);
}
- has_been_read = 1;
-
#ifdef DEBUG
- show_eeprom(eeprom);
+ show_eeprom(devnum, eeprom);
#endif
return ret;
@@ -432,10 +425,15 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
char cmd;
struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
static unsigned int current_dev;
+ /* Set to devnum if we've read EEPROM into memory */
+ static int has_been_read = -1;
// If no arguments, read the EERPOM and display its contents
if (argc == 1) {
- read_eeprom(current_dev, eeprom);
+ if (has_been_read != current_dev) {
+ if (read_eeprom(current_dev, eeprom) == 0)
+ has_been_read = current_dev;
+ }
show_eeprom(current_dev, eeprom);
return 0;
}
@@ -446,14 +444,16 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
// Read the EEPROM contents
if (cmd == 'r') {
- has_been_read = 0;
- if (!read_eeprom(current_dev, eeprom))
+ has_been_read = -1;
+ if (read_eeprom(current_dev, eeprom) == 0) {
printf("EEPROM data loaded from device to memory.\n");
+ has_been_read = current_dev;
+ }
return 0;
}
// Subsequent commands require that the EEPROM has already been read.
- if (!has_been_read) {
+ if (has_been_read != current_dev) {
printf("Please read the EEPROM data first, using the 'tlv_eeprom read' command.\n");
return 0;
}
@@ -509,7 +509,6 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
return 0;
}
current_dev = devnum;
- has_been_read = 0;
} else {
cmd_usage(cmdtp);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 02/12] cmd: tlv_eeprom: remove use of global variable has_been_read
2022-05-02 14:18 ` [PATCH 02/12] cmd: tlv_eeprom: remove use of global variable has_been_read Josua Mayer
@ 2022-05-03 6:11 ` Stefan Roese
0 siblings, 0 replies; 22+ messages in thread
From: Stefan Roese @ 2022-05-03 6:11 UTC (permalink / raw)
To: Josua Mayer, u-boot; +Cc: Simon Glass, Sven Auhagen
On 02.05.22 16:18, Josua Mayer wrote:
> has_been_read is only used as an optimization for do_tlv_eeprom.
> Explicitly use and set inside this function, thus making read_eeprom
> stateless.
>
> Signed-off-by: Josua Mayer <josua@solid-run.com>
> ---
> cmd/tlv_eeprom.c | 25 ++++++++++++-------------
> 1 file changed, 12 insertions(+), 13 deletions(-)
>
> diff --git a/cmd/tlv_eeprom.c b/cmd/tlv_eeprom.c
> index f91c11b304..bfd4882e0d 100644
> --- a/cmd/tlv_eeprom.c
> +++ b/cmd/tlv_eeprom.c
> @@ -42,8 +42,6 @@ static int set_date(char *buf, const char *string);
> static int set_bytes(char *buf, const char *string, int *converted_accum);
> static void show_tlv_devices(int current_dev);
>
> -/* Set to 1 if we've read EEPROM into memory */
> -static int has_been_read;
> /* The EERPOM contents after being read into memory */
> static u8 eeprom[TLV_INFO_MAX_LEN];
>
> @@ -130,9 +128,6 @@ static int read_eeprom(int devnum, u8 *eeprom)
> struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
> struct tlvinfo_tlv *eeprom_tlv = to_entry(&eeprom[HDR_SIZE]);
>
> - if (has_been_read)
> - return 0;
> -
> /* Read the header */
> ret = read_tlv_eeprom((void *)eeprom_hdr, 0, HDR_SIZE, devnum);
> /* If the header was successfully read, read the TLVs */
> @@ -149,10 +144,8 @@ static int read_eeprom(int devnum, u8 *eeprom)
> update_crc(eeprom);
> }
>
> - has_been_read = 1;
> -
> #ifdef DEBUG
> - show_eeprom(eeprom);
> + show_eeprom(devnum, eeprom);
> #endif
>
> return ret;
> @@ -432,10 +425,15 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
> char cmd;
> struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
> static unsigned int current_dev;
> + /* Set to devnum if we've read EEPROM into memory */
> + static int has_been_read = -1;
You are not introducing this variable but still it would be "better" to
change it to bool instead IMHO.
Other than this:
Reviewed-by: Stefan Roese <sr@denx.de>
Thanks,
Stefan
>
> // If no arguments, read the EERPOM and display its contents
> if (argc == 1) {
> - read_eeprom(current_dev, eeprom);
> + if (has_been_read != current_dev) {
> + if (read_eeprom(current_dev, eeprom) == 0)
> + has_been_read = current_dev;
> + }
> show_eeprom(current_dev, eeprom);
> return 0;
> }
> @@ -446,14 +444,16 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
>
> // Read the EEPROM contents
> if (cmd == 'r') {
> - has_been_read = 0;
> - if (!read_eeprom(current_dev, eeprom))
> + has_been_read = -1;
> + if (read_eeprom(current_dev, eeprom) == 0) {
> printf("EEPROM data loaded from device to memory.\n");
> + has_been_read = current_dev;
> + }
> return 0;
> }
>
> // Subsequent commands require that the EEPROM has already been read.
> - if (!has_been_read) {
> + if (has_been_read != current_dev) {
> printf("Please read the EEPROM data first, using the 'tlv_eeprom read' command.\n");
> return 0;
> }
> @@ -509,7 +509,6 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
> return 0;
> }
> current_dev = devnum;
> - has_been_read = 0;
> } else {
> cmd_usage(cmdtp);
> }
Viele Grüße,
Stefan Roese
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 03/12] cmd: tlv_eeprom: do_tlv_eeprom: stop using non-api read_eeprom function
2022-05-02 14:18 [PATCH 00/12] split tlv_eeprom command into a separate library Josua Mayer
2022-05-02 14:18 ` [PATCH 01/12] cmd: tlv_eeprom: remove use of global variable current_dev Josua Mayer
2022-05-02 14:18 ` [PATCH 02/12] cmd: tlv_eeprom: remove use of global variable has_been_read Josua Mayer
@ 2022-05-02 14:18 ` Josua Mayer
2022-05-03 6:12 ` Stefan Roese
2022-05-02 14:18 ` [PATCH 04/12] cmd: tlv_eeprom: convert functions used by command to api functions Josua Mayer
` (8 subsequent siblings)
11 siblings, 1 reply; 22+ messages in thread
From: Josua Mayer @ 2022-05-02 14:18 UTC (permalink / raw)
To: u-boot; +Cc: Josua Mayer, Simon Glass, Sven Auhagen, Stefan Roese
IN the scope of do_tlv_eeprom, the error-checking provided by the
read_eeprom function is not required.
Instead use the API function read_tlv_eeprom.
Signed-off-by: Josua Mayer <josua@solid-run.com>
---
cmd/tlv_eeprom.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/cmd/tlv_eeprom.c b/cmd/tlv_eeprom.c
index bfd4882e0d..00c5b5f840 100644
--- a/cmd/tlv_eeprom.c
+++ b/cmd/tlv_eeprom.c
@@ -431,7 +431,7 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
// If no arguments, read the EERPOM and display its contents
if (argc == 1) {
if (has_been_read != current_dev) {
- if (read_eeprom(current_dev, eeprom) == 0)
+ if (read_tlv_eeprom(eeprom, 0, TLV_INFO_MAX_LEN, current_dev) == 0)
has_been_read = current_dev;
}
show_eeprom(current_dev, eeprom);
@@ -445,7 +445,7 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
// Read the EEPROM contents
if (cmd == 'r') {
has_been_read = -1;
- if (read_eeprom(current_dev, eeprom) == 0) {
+ if (read_tlv_eeprom(eeprom, 0, TLV_INFO_MAX_LEN, current_dev) == 0) {
printf("EEPROM data loaded from device to memory.\n");
has_been_read = current_dev;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 03/12] cmd: tlv_eeprom: do_tlv_eeprom: stop using non-api read_eeprom function
2022-05-02 14:18 ` [PATCH 03/12] cmd: tlv_eeprom: do_tlv_eeprom: stop using non-api read_eeprom function Josua Mayer
@ 2022-05-03 6:12 ` Stefan Roese
0 siblings, 0 replies; 22+ messages in thread
From: Stefan Roese @ 2022-05-03 6:12 UTC (permalink / raw)
To: Josua Mayer, u-boot; +Cc: Simon Glass, Sven Auhagen
On 02.05.22 16:18, Josua Mayer wrote:
> IN the scope of do_tlv_eeprom, the error-checking provided by the
Nitpicking: "In ..."
> read_eeprom function is not required.
> Instead use the API function read_tlv_eeprom.
>
> Signed-off-by: Josua Mayer <josua@solid-run.com>
Reviewed-by: Stefan Roese <sr@denx.de>
Thanks,
Stefan
> ---
> cmd/tlv_eeprom.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/cmd/tlv_eeprom.c b/cmd/tlv_eeprom.c
> index bfd4882e0d..00c5b5f840 100644
> --- a/cmd/tlv_eeprom.c
> +++ b/cmd/tlv_eeprom.c
> @@ -431,7 +431,7 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
> // If no arguments, read the EERPOM and display its contents
> if (argc == 1) {
> if (has_been_read != current_dev) {
> - if (read_eeprom(current_dev, eeprom) == 0)
> + if (read_tlv_eeprom(eeprom, 0, TLV_INFO_MAX_LEN, current_dev) == 0)
> has_been_read = current_dev;
> }
> show_eeprom(current_dev, eeprom);
> @@ -445,7 +445,7 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
> // Read the EEPROM contents
> if (cmd == 'r') {
> has_been_read = -1;
> - if (read_eeprom(current_dev, eeprom) == 0) {
> + if (read_tlv_eeprom(eeprom, 0, TLV_INFO_MAX_LEN, current_dev) == 0) {
> printf("EEPROM data loaded from device to memory.\n");
> has_been_read = current_dev;
> }
Viele Grüße,
Stefan Roese
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 04/12] cmd: tlv_eeprom: convert functions used by command to api functions
2022-05-02 14:18 [PATCH 00/12] split tlv_eeprom command into a separate library Josua Mayer
` (2 preceding siblings ...)
2022-05-02 14:18 ` [PATCH 03/12] cmd: tlv_eeprom: do_tlv_eeprom: stop using non-api read_eeprom function Josua Mayer
@ 2022-05-02 14:18 ` Josua Mayer
2022-05-03 6:16 ` Stefan Roese
2022-05-02 14:18 ` [PATCH 05/12] cmd: tlv_eeprom: remove empty function implementations from header Josua Mayer
` (7 subsequent siblings)
11 siblings, 1 reply; 22+ messages in thread
From: Josua Mayer @ 2022-05-02 14:18 UTC (permalink / raw)
To: u-boot; +Cc: Josua Mayer, Stefan Roese, Sven Auhagen, Simon Glass
- prog_eeprom: write_tlvinfo_tlv_eeprom
- update_crc: tlvinfo_update_crc
- is_valid_tlv: is_valid_tlvinfo_entry
- is_checksum_valid: tlvinfo_check_crc
Signed-off-by: Josua Mayer <josua@solid-run.com>
---
cmd/tlv_eeprom.c | 56 +++++++++++++++----------------------------
include/tlv_eeprom.h | 57 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 76 insertions(+), 37 deletions(-)
diff --git a/cmd/tlv_eeprom.c b/cmd/tlv_eeprom.c
index 00c5b5f840..1b4f2537f6 100644
--- a/cmd/tlv_eeprom.c
+++ b/cmd/tlv_eeprom.c
@@ -28,13 +28,9 @@ DECLARE_GLOBAL_DATA_PTR;
#define MAX_TLV_DEVICES 2
/* File scope function prototypes */
-static bool is_checksum_valid(u8 *eeprom);
static int read_eeprom(int devnum, u8 *eeprom);
static void show_eeprom(int devnum, u8 *eeprom);
static void decode_tlv(struct tlvinfo_tlv *tlv);
-static void update_crc(u8 *eeprom);
-static int prog_eeprom(int devnum, u8 *eeprom);
-static bool tlvinfo_find_tlv(u8 *eeprom, u8 tcode, int *eeprom_index);
static bool tlvinfo_delete_tlv(u8 *eeprom, u8 code);
static bool tlvinfo_add_tlv(u8 *eeprom, int tcode, char *strval);
static int set_mac(char *buf, const char *string);
@@ -58,18 +54,6 @@ static inline bool is_digit(char c)
return (c >= '0' && c <= '9');
}
-/**
- * is_valid_tlv
- *
- * Perform basic sanity checks on a TLV field. The TLV is pointed to
- * by the parameter provided.
- * 1. The type code is not reserved (0x00 or 0xFF)
- */
-static inline bool is_valid_tlv(struct tlvinfo_tlv *tlv)
-{
- return((tlv->type != 0x00) && (tlv->type != 0xFF));
-}
-
/**
* is_hex
*
@@ -83,14 +67,12 @@ static inline u8 is_hex(char p)
}
/**
- * is_checksum_valid
- *
* Validate the checksum in the provided TlvInfo EEPROM data. First,
* verify that the TlvInfo header is valid, then make sure the last
* TLV is a CRC-32 TLV. Then calculate the CRC over the EEPROM data
* and compare it to the value stored in the EEPROM CRC-32 TLV.
*/
-static bool is_checksum_valid(u8 *eeprom)
+bool tlvinfo_check_crc(u8 *eeprom)
{
struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
struct tlvinfo_tlv *eeprom_crc;
@@ -137,11 +119,11 @@ static int read_eeprom(int devnum, u8 *eeprom)
// If the contents are invalid, start over with default contents
if (!is_valid_tlvinfo_header(eeprom_hdr) ||
- !is_checksum_valid(eeprom)) {
+ !tlvinfo_check_crc(eeprom)) {
strcpy(eeprom_hdr->signature, TLV_INFO_ID_STRING);
eeprom_hdr->version = TLV_INFO_VERSION;
eeprom_hdr->totallen = cpu_to_be16(0);
- update_crc(eeprom);
+ tlvinfo_update_crc(eeprom);
}
#ifdef DEBUG
@@ -183,7 +165,7 @@ static void show_eeprom(int devnum, u8 *eeprom)
tlv_end = HDR_SIZE + be16_to_cpu(eeprom_hdr->totallen);
while (curr_tlv < tlv_end) {
eeprom_tlv = to_entry(&eeprom[curr_tlv]);
- if (!is_valid_tlv(eeprom_tlv)) {
+ if (!is_valid_tlvinfo_entry(eeprom_tlv)) {
printf("Invalid TLV field starting at EEPROM offset %d\n",
curr_tlv);
return;
@@ -193,7 +175,7 @@ static void show_eeprom(int devnum, u8 *eeprom)
}
printf("Checksum is %s.\n",
- is_checksum_valid(eeprom) ? "valid" : "invalid");
+ tlvinfo_check_crc(eeprom) ? "valid" : "invalid");
#ifdef DEBUG
printf("EEPROM dump: (0x%x bytes)", TLV_INFO_MAX_LEN);
@@ -340,13 +322,13 @@ static void decode_tlv(struct tlvinfo_tlv *tlv)
}
/**
- * update_crc
+ * tlvinfo_update_crc
*
* This function updates the CRC-32 TLV. If there is no CRC-32 TLV, then
* one is added. This function should be called after each update to the
* EEPROM structure, to make sure the CRC is always correct.
*/
-static void update_crc(u8 *eeprom)
+void tlvinfo_update_crc(u8 *eeprom)
{
struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
struct tlvinfo_tlv *eeprom_crc;
@@ -376,20 +358,20 @@ static void update_crc(u8 *eeprom)
}
/**
- * prog_eeprom
+ * write_tlvinfo_tlv_eeprom
*
- * Write the EEPROM data from CPU memory to the hardware.
+ * Write the TLV data from CPU memory to the hardware.
*/
-static int prog_eeprom(int devnum, u8 *eeprom)
+int write_tlvinfo_tlv_eeprom(void *eeprom, int dev)
{
int ret = 0;
struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
int eeprom_len;
- update_crc(eeprom);
+ tlvinfo_update_crc(eeprom);
eeprom_len = HDR_SIZE + be16_to_cpu(eeprom_hdr->totallen);
- ret = write_tlv_eeprom(eeprom, eeprom_len, devnum);
+ ret = write_tlv_eeprom(eeprom, eeprom_len, dev);
if (ret) {
printf("Programming failed.\n");
return -1;
@@ -462,13 +444,13 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
if (argc == 2) {
switch (cmd) {
case 'w': /* write */
- prog_eeprom(current_dev, eeprom);
+ write_tlvinfo_tlv_eeprom(eeprom, current_dev);
break;
case 'e': /* erase */
strcpy(eeprom_hdr->signature, TLV_INFO_ID_STRING);
eeprom_hdr->version = TLV_INFO_VERSION;
eeprom_hdr->totallen = cpu_to_be16(0);
- update_crc(eeprom);
+ tlvinfo_update_crc(eeprom);
printf("EEPROM data in memory reset.\n");
break;
case 'l': /* list */
@@ -549,7 +531,7 @@ U_BOOT_CMD(tlv_eeprom, 4, 1, do_tlv_eeprom,
* An offset from the beginning of the EEPROM is returned in the
* eeprom_index parameter if the TLV is found.
*/
-static bool tlvinfo_find_tlv(u8 *eeprom, u8 tcode, int *eeprom_index)
+bool tlvinfo_find_tlv(u8 *eeprom, u8 tcode, int *eeprom_index)
{
struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
struct tlvinfo_tlv *eeprom_tlv;
@@ -561,7 +543,7 @@ static bool tlvinfo_find_tlv(u8 *eeprom, u8 tcode, int *eeprom_index)
eeprom_end = HDR_SIZE + be16_to_cpu(eeprom_hdr->totallen);
while (*eeprom_index < eeprom_end) {
eeprom_tlv = to_entry(&eeprom[*eeprom_index]);
- if (!is_valid_tlv(eeprom_tlv))
+ if (!is_valid_tlvinfo_entry(eeprom_tlv))
return false;
if (eeprom_tlv->type == tcode)
return true;
@@ -594,7 +576,7 @@ static bool tlvinfo_delete_tlv(u8 *eeprom, u8 code)
eeprom_hdr->totallen =
cpu_to_be16(be16_to_cpu(eeprom_hdr->totallen) -
tlength);
- update_crc(eeprom);
+ tlvinfo_update_crc(eeprom);
return true;
}
return false;
@@ -695,7 +677,7 @@ static bool tlvinfo_add_tlv(u8 *eeprom, int tcode, char *strval)
// Update the total length and calculate (add) a new CRC-32 TLV
eeprom_hdr->totallen = cpu_to_be16(be16_to_cpu(eeprom_hdr->totallen) +
ENT_SIZE + new_tlv_len);
- update_crc(eeprom);
+ tlvinfo_update_crc(eeprom);
return true;
}
@@ -986,7 +968,7 @@ int read_tlvinfo_tlv_eeprom(void *eeprom, struct tlvinfo_header **hdr,
be16_to_cpu(tlv_hdr->totallen), dev_num);
if (ret < 0)
return ret;
- if (!is_checksum_valid(eeprom))
+ if (!tlvinfo_check_crc(eeprom))
return -EINVAL;
*hdr = tlv_hdr;
diff --git a/include/tlv_eeprom.h b/include/tlv_eeprom.h
index fd45e5f6eb..30626a1067 100644
--- a/include/tlv_eeprom.h
+++ b/include/tlv_eeprom.h
@@ -111,6 +111,51 @@ int write_tlv_eeprom(void *eeprom, int len, int dev);
int read_tlvinfo_tlv_eeprom(void *eeprom, struct tlvinfo_header **hdr,
struct tlvinfo_tlv **first_entry, int dev);
+/**
+ * Write TLV data to the EEPROM.
+ *
+ * - Only writes length of actual tlv data
+ * - updates checksum
+ *
+ * @eeprom: Pointer to buffer to hold the binary data. Must point to a buffer
+ * of size at least TLV_INFO_MAX_LEN.
+ * @dev : EEPROM device to write
+ *
+ */
+int write_tlvinfo_tlv_eeprom(void *eeprom, int dev);
+
+/**
+ * tlvinfo_find_tlv
+ *
+ * This function finds the TLV with the supplied code in the EERPOM.
+ * An offset from the beginning of the EEPROM is returned in the
+ * eeprom_index parameter if the TLV is found.
+ */
+bool tlvinfo_find_tlv(u8 *eeprom, u8 tcode, int *eeprom_index);
+
+/**
+ * tlvinfo_update_crc
+ *
+ * This function updates the CRC-32 TLV. If there is no CRC-32 TLV, then
+ * one is added. This function should be called after each update to the
+ * EEPROM structure, to make sure the CRC is always correct.
+ *
+ * @eeprom: Pointer to buffer to hold the binary data. Must point to a buffer
+ * of size at least TLV_INFO_MAX_LEN.
+ */
+void tlvinfo_update_crc(u8 *eeprom);
+
+/**
+ * Validate the checksum in the provided TlvInfo EEPROM data. First,
+ * verify that the TlvInfo header is valid, then make sure the last
+ * TLV is a CRC-32 TLV. Then calculate the CRC over the EEPROM data
+ * and compare it to the value stored in the EEPROM CRC-32 TLV.
+ *
+ * @eeprom: Pointer to buffer to hold the binary data. Must point to a buffer
+ * of size at least TLV_INFO_MAX_LEN.
+ */
+bool tlvinfo_check_crc(u8 *eeprom);
+
#else /* !CONFIG_IS_ENABLED(CMD_TLV_EEPROM) */
static inline int read_tlv_eeprom(void *eeprom, int offset, int len, int dev)
@@ -150,4 +195,16 @@ static inline bool is_valid_tlvinfo_header(struct tlvinfo_header *hdr)
(be16_to_cpu(hdr->totallen) <= TLV_TOTAL_LEN_MAX));
}
+/**
+ * is_valid_tlv
+ *
+ * Perform basic sanity checks on a TLV field. The TLV is pointed to
+ * by the parameter provided.
+ * 1. The type code is not reserved (0x00 or 0xFF)
+ */
+static inline bool is_valid_tlvinfo_entry(struct tlvinfo_tlv *tlv)
+{
+ return((tlv->type != 0x00) && (tlv->type != 0xFF));
+}
+
#endif /* __TLV_EEPROM_H_ */
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 04/12] cmd: tlv_eeprom: convert functions used by command to api functions
2022-05-02 14:18 ` [PATCH 04/12] cmd: tlv_eeprom: convert functions used by command to api functions Josua Mayer
@ 2022-05-03 6:16 ` Stefan Roese
2022-05-03 7:17 ` Josua Mayer
0 siblings, 1 reply; 22+ messages in thread
From: Stefan Roese @ 2022-05-03 6:16 UTC (permalink / raw)
To: Josua Mayer, u-boot; +Cc: Sven Auhagen, Simon Glass
On 02.05.22 16:18, Josua Mayer wrote:
> - prog_eeprom: write_tlvinfo_tlv_eeprom
> - update_crc: tlvinfo_update_crc
> - is_valid_tlv: is_valid_tlvinfo_entry
> - is_checksum_valid: tlvinfo_check_crc
So while creating a new API it makes sense to prepend the function
names identical IMHO to not "pollute" the namespace. Something like
- tlv_is_valid_entry
- tlv_check_crc
...
Just examples, you get the idea.
Thanks,
Stefan
> Signed-off-by: Josua Mayer <josua@solid-run.com>
> ---
> cmd/tlv_eeprom.c | 56 +++++++++++++++----------------------------
> include/tlv_eeprom.h | 57 ++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 76 insertions(+), 37 deletions(-)
>
> diff --git a/cmd/tlv_eeprom.c b/cmd/tlv_eeprom.c
> index 00c5b5f840..1b4f2537f6 100644
> --- a/cmd/tlv_eeprom.c
> +++ b/cmd/tlv_eeprom.c
> @@ -28,13 +28,9 @@ DECLARE_GLOBAL_DATA_PTR;
> #define MAX_TLV_DEVICES 2
>
> /* File scope function prototypes */
> -static bool is_checksum_valid(u8 *eeprom);
> static int read_eeprom(int devnum, u8 *eeprom);
> static void show_eeprom(int devnum, u8 *eeprom);
> static void decode_tlv(struct tlvinfo_tlv *tlv);
> -static void update_crc(u8 *eeprom);
> -static int prog_eeprom(int devnum, u8 *eeprom);
> -static bool tlvinfo_find_tlv(u8 *eeprom, u8 tcode, int *eeprom_index);
> static bool tlvinfo_delete_tlv(u8 *eeprom, u8 code);
> static bool tlvinfo_add_tlv(u8 *eeprom, int tcode, char *strval);
> static int set_mac(char *buf, const char *string);
> @@ -58,18 +54,6 @@ static inline bool is_digit(char c)
> return (c >= '0' && c <= '9');
> }
>
> -/**
> - * is_valid_tlv
> - *
> - * Perform basic sanity checks on a TLV field. The TLV is pointed to
> - * by the parameter provided.
> - * 1. The type code is not reserved (0x00 or 0xFF)
> - */
> -static inline bool is_valid_tlv(struct tlvinfo_tlv *tlv)
> -{
> - return((tlv->type != 0x00) && (tlv->type != 0xFF));
> -}
> -
> /**
> * is_hex
> *
> @@ -83,14 +67,12 @@ static inline u8 is_hex(char p)
> }
>
> /**
> - * is_checksum_valid
> - *
> * Validate the checksum in the provided TlvInfo EEPROM data. First,
> * verify that the TlvInfo header is valid, then make sure the last
> * TLV is a CRC-32 TLV. Then calculate the CRC over the EEPROM data
> * and compare it to the value stored in the EEPROM CRC-32 TLV.
> */
> -static bool is_checksum_valid(u8 *eeprom)
> +bool tlvinfo_check_crc(u8 *eeprom)
> {
> struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
> struct tlvinfo_tlv *eeprom_crc;
> @@ -137,11 +119,11 @@ static int read_eeprom(int devnum, u8 *eeprom)
>
> // If the contents are invalid, start over with default contents
> if (!is_valid_tlvinfo_header(eeprom_hdr) ||
> - !is_checksum_valid(eeprom)) {
> + !tlvinfo_check_crc(eeprom)) {
> strcpy(eeprom_hdr->signature, TLV_INFO_ID_STRING);
> eeprom_hdr->version = TLV_INFO_VERSION;
> eeprom_hdr->totallen = cpu_to_be16(0);
> - update_crc(eeprom);
> + tlvinfo_update_crc(eeprom);
> }
>
> #ifdef DEBUG
> @@ -183,7 +165,7 @@ static void show_eeprom(int devnum, u8 *eeprom)
> tlv_end = HDR_SIZE + be16_to_cpu(eeprom_hdr->totallen);
> while (curr_tlv < tlv_end) {
> eeprom_tlv = to_entry(&eeprom[curr_tlv]);
> - if (!is_valid_tlv(eeprom_tlv)) {
> + if (!is_valid_tlvinfo_entry(eeprom_tlv)) {
> printf("Invalid TLV field starting at EEPROM offset %d\n",
> curr_tlv);
> return;
> @@ -193,7 +175,7 @@ static void show_eeprom(int devnum, u8 *eeprom)
> }
>
> printf("Checksum is %s.\n",
> - is_checksum_valid(eeprom) ? "valid" : "invalid");
> + tlvinfo_check_crc(eeprom) ? "valid" : "invalid");
>
> #ifdef DEBUG
> printf("EEPROM dump: (0x%x bytes)", TLV_INFO_MAX_LEN);
> @@ -340,13 +322,13 @@ static void decode_tlv(struct tlvinfo_tlv *tlv)
> }
>
> /**
> - * update_crc
> + * tlvinfo_update_crc
> *
> * This function updates the CRC-32 TLV. If there is no CRC-32 TLV, then
> * one is added. This function should be called after each update to the
> * EEPROM structure, to make sure the CRC is always correct.
> */
> -static void update_crc(u8 *eeprom)
> +void tlvinfo_update_crc(u8 *eeprom)
> {
> struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
> struct tlvinfo_tlv *eeprom_crc;
> @@ -376,20 +358,20 @@ static void update_crc(u8 *eeprom)
> }
>
> /**
> - * prog_eeprom
> + * write_tlvinfo_tlv_eeprom
> *
> - * Write the EEPROM data from CPU memory to the hardware.
> + * Write the TLV data from CPU memory to the hardware.
> */
> -static int prog_eeprom(int devnum, u8 *eeprom)
> +int write_tlvinfo_tlv_eeprom(void *eeprom, int dev)
> {
> int ret = 0;
> struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
> int eeprom_len;
>
> - update_crc(eeprom);
> + tlvinfo_update_crc(eeprom);
>
> eeprom_len = HDR_SIZE + be16_to_cpu(eeprom_hdr->totallen);
> - ret = write_tlv_eeprom(eeprom, eeprom_len, devnum);
> + ret = write_tlv_eeprom(eeprom, eeprom_len, dev);
> if (ret) {
> printf("Programming failed.\n");
> return -1;
> @@ -462,13 +444,13 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
> if (argc == 2) {
> switch (cmd) {
> case 'w': /* write */
> - prog_eeprom(current_dev, eeprom);
> + write_tlvinfo_tlv_eeprom(eeprom, current_dev);
> break;
> case 'e': /* erase */
> strcpy(eeprom_hdr->signature, TLV_INFO_ID_STRING);
> eeprom_hdr->version = TLV_INFO_VERSION;
> eeprom_hdr->totallen = cpu_to_be16(0);
> - update_crc(eeprom);
> + tlvinfo_update_crc(eeprom);
> printf("EEPROM data in memory reset.\n");
> break;
> case 'l': /* list */
> @@ -549,7 +531,7 @@ U_BOOT_CMD(tlv_eeprom, 4, 1, do_tlv_eeprom,
> * An offset from the beginning of the EEPROM is returned in the
> * eeprom_index parameter if the TLV is found.
> */
> -static bool tlvinfo_find_tlv(u8 *eeprom, u8 tcode, int *eeprom_index)
> +bool tlvinfo_find_tlv(u8 *eeprom, u8 tcode, int *eeprom_index)
> {
> struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
> struct tlvinfo_tlv *eeprom_tlv;
> @@ -561,7 +543,7 @@ static bool tlvinfo_find_tlv(u8 *eeprom, u8 tcode, int *eeprom_index)
> eeprom_end = HDR_SIZE + be16_to_cpu(eeprom_hdr->totallen);
> while (*eeprom_index < eeprom_end) {
> eeprom_tlv = to_entry(&eeprom[*eeprom_index]);
> - if (!is_valid_tlv(eeprom_tlv))
> + if (!is_valid_tlvinfo_entry(eeprom_tlv))
> return false;
> if (eeprom_tlv->type == tcode)
> return true;
> @@ -594,7 +576,7 @@ static bool tlvinfo_delete_tlv(u8 *eeprom, u8 code)
> eeprom_hdr->totallen =
> cpu_to_be16(be16_to_cpu(eeprom_hdr->totallen) -
> tlength);
> - update_crc(eeprom);
> + tlvinfo_update_crc(eeprom);
> return true;
> }
> return false;
> @@ -695,7 +677,7 @@ static bool tlvinfo_add_tlv(u8 *eeprom, int tcode, char *strval)
> // Update the total length and calculate (add) a new CRC-32 TLV
> eeprom_hdr->totallen = cpu_to_be16(be16_to_cpu(eeprom_hdr->totallen) +
> ENT_SIZE + new_tlv_len);
> - update_crc(eeprom);
> + tlvinfo_update_crc(eeprom);
>
> return true;
> }
> @@ -986,7 +968,7 @@ int read_tlvinfo_tlv_eeprom(void *eeprom, struct tlvinfo_header **hdr,
> be16_to_cpu(tlv_hdr->totallen), dev_num);
> if (ret < 0)
> return ret;
> - if (!is_checksum_valid(eeprom))
> + if (!tlvinfo_check_crc(eeprom))
> return -EINVAL;
>
> *hdr = tlv_hdr;
> diff --git a/include/tlv_eeprom.h b/include/tlv_eeprom.h
> index fd45e5f6eb..30626a1067 100644
> --- a/include/tlv_eeprom.h
> +++ b/include/tlv_eeprom.h
> @@ -111,6 +111,51 @@ int write_tlv_eeprom(void *eeprom, int len, int dev);
> int read_tlvinfo_tlv_eeprom(void *eeprom, struct tlvinfo_header **hdr,
> struct tlvinfo_tlv **first_entry, int dev);
>
> +/**
> + * Write TLV data to the EEPROM.
> + *
> + * - Only writes length of actual tlv data
> + * - updates checksum
> + *
> + * @eeprom: Pointer to buffer to hold the binary data. Must point to a buffer
> + * of size at least TLV_INFO_MAX_LEN.
> + * @dev : EEPROM device to write
> + *
> + */
> +int write_tlvinfo_tlv_eeprom(void *eeprom, int dev);
> +
> +/**
> + * tlvinfo_find_tlv
> + *
> + * This function finds the TLV with the supplied code in the EERPOM.
> + * An offset from the beginning of the EEPROM is returned in the
> + * eeprom_index parameter if the TLV is found.
> + */
> +bool tlvinfo_find_tlv(u8 *eeprom, u8 tcode, int *eeprom_index);
> +
> +/**
> + * tlvinfo_update_crc
> + *
> + * This function updates the CRC-32 TLV. If there is no CRC-32 TLV, then
> + * one is added. This function should be called after each update to the
> + * EEPROM structure, to make sure the CRC is always correct.
> + *
> + * @eeprom: Pointer to buffer to hold the binary data. Must point to a buffer
> + * of size at least TLV_INFO_MAX_LEN.
> + */
> +void tlvinfo_update_crc(u8 *eeprom);
> +
> +/**
> + * Validate the checksum in the provided TlvInfo EEPROM data. First,
> + * verify that the TlvInfo header is valid, then make sure the last
> + * TLV is a CRC-32 TLV. Then calculate the CRC over the EEPROM data
> + * and compare it to the value stored in the EEPROM CRC-32 TLV.
> + *
> + * @eeprom: Pointer to buffer to hold the binary data. Must point to a buffer
> + * of size at least TLV_INFO_MAX_LEN.
> + */
> +bool tlvinfo_check_crc(u8 *eeprom);
> +
> #else /* !CONFIG_IS_ENABLED(CMD_TLV_EEPROM) */
>
> static inline int read_tlv_eeprom(void *eeprom, int offset, int len, int dev)
> @@ -150,4 +195,16 @@ static inline bool is_valid_tlvinfo_header(struct tlvinfo_header *hdr)
> (be16_to_cpu(hdr->totallen) <= TLV_TOTAL_LEN_MAX));
> }
>
> +/**
> + * is_valid_tlv
> + *
> + * Perform basic sanity checks on a TLV field. The TLV is pointed to
> + * by the parameter provided.
> + * 1. The type code is not reserved (0x00 or 0xFF)
> + */
> +static inline bool is_valid_tlvinfo_entry(struct tlvinfo_tlv *tlv)
> +{
> + return((tlv->type != 0x00) && (tlv->type != 0xFF));
> +}
> +
> #endif /* __TLV_EEPROM_H_ */
Viele Grüße,
Stefan Roese
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 04/12] cmd: tlv_eeprom: convert functions used by command to api functions
2022-05-03 6:16 ` Stefan Roese
@ 2022-05-03 7:17 ` Josua Mayer
2022-05-03 10:54 ` Stefan Roese
0 siblings, 1 reply; 22+ messages in thread
From: Josua Mayer @ 2022-05-03 7:17 UTC (permalink / raw)
To: Stefan Roese, u-boot; +Cc: Sven Auhagen, Simon Glass
Am 03.05.22 um 09:16 schrieb Stefan Roese:
> On 02.05.22 16:18, Josua Mayer wrote:
>> - prog_eeprom: write_tlvinfo_tlv_eeprom
>> - update_crc: tlvinfo_update_crc
>> - is_valid_tlv: is_valid_tlvinfo_entry
>> - is_checksum_valid: tlvinfo_check_crc
>
> So while creating a new API it makes sense to prepend the function
> names identical IMHO to not "pollute" the namespace. Something like
>
> - tlv_is_valid_entry
> - tlv_check_crc
> ...
>
> Just examples, you get the idea.
Yes. The hard part in this particular implementation is that the naming
is not consistent.
The most sense I could make is that prefix tlvinfo indicates all tlv
data, i.e. working with the whole structure, while tlvinfo_tlv indicates
working with one data entry. Further write, read and is_ are currently
prefixed in the header, but for previously static functions in the C
file it was put in the middle ...
I found it quite difficult to prepare for splitting off a library in a
way that preserves history, i.e. diffs should still be readable for
spotting mistakes.
I was considering to at the very end do a mass-rename and come up with
better naming, something like
tlv_{set,get}_{blob,string,mac}
tlv_find_entry
tlv_{read,write}_eeprom
But this is pending a refactoring and extension of the tlv parsing code
in board/solidrun/common/tlv_data.*, to figure out what is required or
useful.
>
> Thanks,
> Stefan
>
>> Signed-off-by: Josua Mayer <josua@solid-run.com>
>> ---
>> cmd/tlv_eeprom.c | 56 +++++++++++++++----------------------------
>> include/tlv_eeprom.h | 57 ++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 76 insertions(+), 37 deletions(-)
>>
>> diff --git a/cmd/tlv_eeprom.c b/cmd/tlv_eeprom.c
>> index 00c5b5f840..1b4f2537f6 100644
>> --- a/cmd/tlv_eeprom.c
>> +++ b/cmd/tlv_eeprom.c
>> @@ -28,13 +28,9 @@ DECLARE_GLOBAL_DATA_PTR;
>> #define MAX_TLV_DEVICES 2
>> /* File scope function prototypes */
>> -static bool is_checksum_valid(u8 *eeprom);
>> static int read_eeprom(int devnum, u8 *eeprom);
>> static void show_eeprom(int devnum, u8 *eeprom);
>> static void decode_tlv(struct tlvinfo_tlv *tlv);
>> -static void update_crc(u8 *eeprom);
>> -static int prog_eeprom(int devnum, u8 *eeprom);
>> -static bool tlvinfo_find_tlv(u8 *eeprom, u8 tcode, int *eeprom_index);
>> static bool tlvinfo_delete_tlv(u8 *eeprom, u8 code);
>> static bool tlvinfo_add_tlv(u8 *eeprom, int tcode, char *strval);
>> static int set_mac(char *buf, const char *string);
>> @@ -58,18 +54,6 @@ static inline bool is_digit(char c)
>> return (c >= '0' && c <= '9');
>> }
>> -/**
>> - * is_valid_tlv
>> - *
>> - * Perform basic sanity checks on a TLV field. The TLV is pointed to
>> - * by the parameter provided.
>> - * 1. The type code is not reserved (0x00 or 0xFF)
>> - */
>> -static inline bool is_valid_tlv(struct tlvinfo_tlv *tlv)
>> -{
>> - return((tlv->type != 0x00) && (tlv->type != 0xFF));
>> -}
>> -
>> /**
>> * is_hex
>> *
>> @@ -83,14 +67,12 @@ static inline u8 is_hex(char p)
>> }
>> /**
>> - * is_checksum_valid
>> - *
>> * Validate the checksum in the provided TlvInfo EEPROM data. First,
>> * verify that the TlvInfo header is valid, then make sure the last
>> * TLV is a CRC-32 TLV. Then calculate the CRC over the EEPROM data
>> * and compare it to the value stored in the EEPROM CRC-32 TLV.
>> */
>> -static bool is_checksum_valid(u8 *eeprom)
>> +bool tlvinfo_check_crc(u8 *eeprom)
>> {
>> struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
>> struct tlvinfo_tlv *eeprom_crc;
>> @@ -137,11 +119,11 @@ static int read_eeprom(int devnum, u8 *eeprom)
>> // If the contents are invalid, start over with default contents
>> if (!is_valid_tlvinfo_header(eeprom_hdr) ||
>> - !is_checksum_valid(eeprom)) {
>> + !tlvinfo_check_crc(eeprom)) {
>> strcpy(eeprom_hdr->signature, TLV_INFO_ID_STRING);
>> eeprom_hdr->version = TLV_INFO_VERSION;
>> eeprom_hdr->totallen = cpu_to_be16(0);
>> - update_crc(eeprom);
>> + tlvinfo_update_crc(eeprom);
>> }
>> #ifdef DEBUG
>> @@ -183,7 +165,7 @@ static void show_eeprom(int devnum, u8 *eeprom)
>> tlv_end = HDR_SIZE + be16_to_cpu(eeprom_hdr->totallen);
>> while (curr_tlv < tlv_end) {
>> eeprom_tlv = to_entry(&eeprom[curr_tlv]);
>> - if (!is_valid_tlv(eeprom_tlv)) {
>> + if (!is_valid_tlvinfo_entry(eeprom_tlv)) {
>> printf("Invalid TLV field starting at EEPROM offset %d\n",
>> curr_tlv);
>> return;
>> @@ -193,7 +175,7 @@ static void show_eeprom(int devnum, u8 *eeprom)
>> }
>> printf("Checksum is %s.\n",
>> - is_checksum_valid(eeprom) ? "valid" : "invalid");
>> + tlvinfo_check_crc(eeprom) ? "valid" : "invalid");
>> #ifdef DEBUG
>> printf("EEPROM dump: (0x%x bytes)", TLV_INFO_MAX_LEN);
>> @@ -340,13 +322,13 @@ static void decode_tlv(struct tlvinfo_tlv *tlv)
>> }
>> /**
>> - * update_crc
>> + * tlvinfo_update_crc
>> *
>> * This function updates the CRC-32 TLV. If there is no CRC-32
>> TLV, then
>> * one is added. This function should be called after each update
>> to the
>> * EEPROM structure, to make sure the CRC is always correct.
>> */
>> -static void update_crc(u8 *eeprom)
>> +void tlvinfo_update_crc(u8 *eeprom)
>> {
>> struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
>> struct tlvinfo_tlv *eeprom_crc;
>> @@ -376,20 +358,20 @@ static void update_crc(u8 *eeprom)
>> }
>> /**
>> - * prog_eeprom
>> + * write_tlvinfo_tlv_eeprom
>> *
>> - * Write the EEPROM data from CPU memory to the hardware.
>> + * Write the TLV data from CPU memory to the hardware.
>> */
>> -static int prog_eeprom(int devnum, u8 *eeprom)
>> +int write_tlvinfo_tlv_eeprom(void *eeprom, int dev)
>> {
>> int ret = 0;
>> struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
>> int eeprom_len;
>> - update_crc(eeprom);
>> + tlvinfo_update_crc(eeprom);
>> eeprom_len = HDR_SIZE + be16_to_cpu(eeprom_hdr->totallen);
>> - ret = write_tlv_eeprom(eeprom, eeprom_len, devnum);
>> + ret = write_tlv_eeprom(eeprom, eeprom_len, dev);
>> if (ret) {
>> printf("Programming failed.\n");
>> return -1;
>> @@ -462,13 +444,13 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int
>> flag, int argc, char *const argv[])
>> if (argc == 2) {
>> switch (cmd) {
>> case 'w': /* write */
>> - prog_eeprom(current_dev, eeprom);
>> + write_tlvinfo_tlv_eeprom(eeprom, current_dev);
>> break;
>> case 'e': /* erase */
>> strcpy(eeprom_hdr->signature, TLV_INFO_ID_STRING);
>> eeprom_hdr->version = TLV_INFO_VERSION;
>> eeprom_hdr->totallen = cpu_to_be16(0);
>> - update_crc(eeprom);
>> + tlvinfo_update_crc(eeprom);
>> printf("EEPROM data in memory reset.\n");
>> break;
>> case 'l': /* list */
>> @@ -549,7 +531,7 @@ U_BOOT_CMD(tlv_eeprom, 4, 1, do_tlv_eeprom,
>> * An offset from the beginning of the EEPROM is returned in the
>> * eeprom_index parameter if the TLV is found.
>> */
>> -static bool tlvinfo_find_tlv(u8 *eeprom, u8 tcode, int *eeprom_index)
>> +bool tlvinfo_find_tlv(u8 *eeprom, u8 tcode, int *eeprom_index)
>> {
>> struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
>> struct tlvinfo_tlv *eeprom_tlv;
>> @@ -561,7 +543,7 @@ static bool tlvinfo_find_tlv(u8 *eeprom, u8
>> tcode, int *eeprom_index)
>> eeprom_end = HDR_SIZE + be16_to_cpu(eeprom_hdr->totallen);
>> while (*eeprom_index < eeprom_end) {
>> eeprom_tlv = to_entry(&eeprom[*eeprom_index]);
>> - if (!is_valid_tlv(eeprom_tlv))
>> + if (!is_valid_tlvinfo_entry(eeprom_tlv))
>> return false;
>> if (eeprom_tlv->type == tcode)
>> return true;
>> @@ -594,7 +576,7 @@ static bool tlvinfo_delete_tlv(u8 *eeprom, u8 code)
>> eeprom_hdr->totallen =
>> cpu_to_be16(be16_to_cpu(eeprom_hdr->totallen) -
>> tlength);
>> - update_crc(eeprom);
>> + tlvinfo_update_crc(eeprom);
>> return true;
>> }
>> return false;
>> @@ -695,7 +677,7 @@ static bool tlvinfo_add_tlv(u8 *eeprom, int
>> tcode, char *strval)
>> // Update the total length and calculate (add) a new CRC-32 TLV
>> eeprom_hdr->totallen =
>> cpu_to_be16(be16_to_cpu(eeprom_hdr->totallen) +
>> ENT_SIZE + new_tlv_len);
>> - update_crc(eeprom);
>> + tlvinfo_update_crc(eeprom);
>> return true;
>> }
>> @@ -986,7 +968,7 @@ int read_tlvinfo_tlv_eeprom(void *eeprom, struct
>> tlvinfo_header **hdr,
>> be16_to_cpu(tlv_hdr->totallen), dev_num);
>> if (ret < 0)
>> return ret;
>> - if (!is_checksum_valid(eeprom))
>> + if (!tlvinfo_check_crc(eeprom))
>> return -EINVAL;
>> *hdr = tlv_hdr;
>> diff --git a/include/tlv_eeprom.h b/include/tlv_eeprom.h
>> index fd45e5f6eb..30626a1067 100644
>> --- a/include/tlv_eeprom.h
>> +++ b/include/tlv_eeprom.h
>> @@ -111,6 +111,51 @@ int write_tlv_eeprom(void *eeprom, int len, int
>> dev);
>> int read_tlvinfo_tlv_eeprom(void *eeprom, struct tlvinfo_header **hdr,
>> struct tlvinfo_tlv **first_entry, int dev);
>> +/**
>> + * Write TLV data to the EEPROM.
>> + *
>> + * - Only writes length of actual tlv data
>> + * - updates checksum
>> + *
>> + * @eeprom: Pointer to buffer to hold the binary data. Must point to
>> a buffer
>> + * of size at least TLV_INFO_MAX_LEN.
>> + * @dev : EEPROM device to write
>> + *
>> + */
>> +int write_tlvinfo_tlv_eeprom(void *eeprom, int dev);
>> +
>> +/**
>> + * tlvinfo_find_tlv
>> + *
>> + * This function finds the TLV with the supplied code in the EERPOM.
>> + * An offset from the beginning of the EEPROM is returned in the
>> + * eeprom_index parameter if the TLV is found.
>> + */
>> +bool tlvinfo_find_tlv(u8 *eeprom, u8 tcode, int *eeprom_index);
>> +
>> +/**
>> + * tlvinfo_update_crc
>> + *
>> + * This function updates the CRC-32 TLV. If there is no CRC-32 TLV,
>> then
>> + * one is added. This function should be called after each update
>> to the
>> + * EEPROM structure, to make sure the CRC is always correct.
>> + *
>> + * @eeprom: Pointer to buffer to hold the binary data. Must point to
>> a buffer
>> + * of size at least TLV_INFO_MAX_LEN.
>> + */
>> +void tlvinfo_update_crc(u8 *eeprom);
>> +
>> +/**
>> + * Validate the checksum in the provided TlvInfo EEPROM data. First,
>> + * verify that the TlvInfo header is valid, then make sure the last
>> + * TLV is a CRC-32 TLV. Then calculate the CRC over the EEPROM data
>> + * and compare it to the value stored in the EEPROM CRC-32 TLV.
>> + *
>> + * @eeprom: Pointer to buffer to hold the binary data. Must point to
>> a buffer
>> + * of size at least TLV_INFO_MAX_LEN.
>> + */
>> +bool tlvinfo_check_crc(u8 *eeprom);
>> +
>> #else /* !CONFIG_IS_ENABLED(CMD_TLV_EEPROM) */
>> static inline int read_tlv_eeprom(void *eeprom, int offset, int
>> len, int dev)
>> @@ -150,4 +195,16 @@ static inline bool
>> is_valid_tlvinfo_header(struct tlvinfo_header *hdr)
>> (be16_to_cpu(hdr->totallen) <= TLV_TOTAL_LEN_MAX));
>> }
>> +/**
>> + * is_valid_tlv
>> + *
>> + * Perform basic sanity checks on a TLV field. The TLV is pointed to
>> + * by the parameter provided.
>> + * 1. The type code is not reserved (0x00 or 0xFF)
>> + */
>> +static inline bool is_valid_tlvinfo_entry(struct tlvinfo_tlv *tlv)
>> +{
>> + return((tlv->type != 0x00) && (tlv->type != 0xFF));
>> +}
>> +
>> #endif /* __TLV_EEPROM_H_ */
>
> Viele Grüße,
> Stefan Roese
>
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 04/12] cmd: tlv_eeprom: convert functions used by command to api functions
2022-05-03 7:17 ` Josua Mayer
@ 2022-05-03 10:54 ` Stefan Roese
2022-05-03 19:09 ` Josua Mayer
0 siblings, 1 reply; 22+ messages in thread
From: Stefan Roese @ 2022-05-03 10:54 UTC (permalink / raw)
To: Josua Mayer, u-boot; +Cc: Sven Auhagen, Simon Glass
Hi Josua,
On 03.05.22 09:17, Josua Mayer wrote:
> Am 03.05.22 um 09:16 schrieb Stefan Roese:
>> On 02.05.22 16:18, Josua Mayer wrote:
>>> - prog_eeprom: write_tlvinfo_tlv_eeprom
>>> - update_crc: tlvinfo_update_crc
>>> - is_valid_tlv: is_valid_tlvinfo_entry
>>> - is_checksum_valid: tlvinfo_check_crc
>>
>> So while creating a new API it makes sense to prepend the function
>> names identical IMHO to not "pollute" the namespace. Something like
>>
>> - tlv_is_valid_entry
>> - tlv_check_crc
>> ...
>>
>> Just examples, you get the idea.
> Yes. The hard part in this particular implementation is that the naming
> is not consistent.
>
> The most sense I could make is that prefix tlvinfo indicates all tlv
> data, i.e. working with the whole structure, while tlvinfo_tlv indicates
> working with one data entry. Further write, read and is_ are currently
> prefixed in the header, but for previously static functions in the C
> file it was put in the middle ...
>
> I found it quite difficult to prepare for splitting off a library in a
> way that preserves history, i.e. diffs should still be readable for
> spotting mistakes.
Yes, a decent history would be welcome. But still, when going global
here with a new API this should be consistant.
> I was considering to at the very end do a mass-rename and come up with
> better naming, something like
> tlv_{set,get}_{blob,string,mac}
> tlv_find_entry
> tlv_{read,write}_eeprom
>
> But this is pending a refactoring and extension of the tlv parsing code
> in board/solidrun/common/tlv_data.*, to figure out what is required or
> useful.
So your plan is to this:
a) Get this patchset included
b) Use it in board specific code, e.g. solidrun
c) Do the mass-rename
Is this correct? If yes, why is it better to do the renaming at the end?
Thanks,
Stefan
>>
>> Thanks,
>> Stefan
>>
>>> Signed-off-by: Josua Mayer <josua@solid-run.com>
>>> ---
>>> cmd/tlv_eeprom.c | 56 +++++++++++++++----------------------------
>>> include/tlv_eeprom.h | 57 ++++++++++++++++++++++++++++++++++++++++++++
>>> 2 files changed, 76 insertions(+), 37 deletions(-)
>>>
>>> diff --git a/cmd/tlv_eeprom.c b/cmd/tlv_eeprom.c
>>> index 00c5b5f840..1b4f2537f6 100644
>>> --- a/cmd/tlv_eeprom.c
>>> +++ b/cmd/tlv_eeprom.c
>>> @@ -28,13 +28,9 @@ DECLARE_GLOBAL_DATA_PTR;
>>> #define MAX_TLV_DEVICES 2
>>> /* File scope function prototypes */
>>> -static bool is_checksum_valid(u8 *eeprom);
>>> static int read_eeprom(int devnum, u8 *eeprom);
>>> static void show_eeprom(int devnum, u8 *eeprom);
>>> static void decode_tlv(struct tlvinfo_tlv *tlv);
>>> -static void update_crc(u8 *eeprom);
>>> -static int prog_eeprom(int devnum, u8 *eeprom);
>>> -static bool tlvinfo_find_tlv(u8 *eeprom, u8 tcode, int *eeprom_index);
>>> static bool tlvinfo_delete_tlv(u8 *eeprom, u8 code);
>>> static bool tlvinfo_add_tlv(u8 *eeprom, int tcode, char *strval);
>>> static int set_mac(char *buf, const char *string);
>>> @@ -58,18 +54,6 @@ static inline bool is_digit(char c)
>>> return (c >= '0' && c <= '9');
>>> }
>>> -/**
>>> - * is_valid_tlv
>>> - *
>>> - * Perform basic sanity checks on a TLV field. The TLV is pointed to
>>> - * by the parameter provided.
>>> - * 1. The type code is not reserved (0x00 or 0xFF)
>>> - */
>>> -static inline bool is_valid_tlv(struct tlvinfo_tlv *tlv)
>>> -{
>>> - return((tlv->type != 0x00) && (tlv->type != 0xFF));
>>> -}
>>> -
>>> /**
>>> * is_hex
>>> *
>>> @@ -83,14 +67,12 @@ static inline u8 is_hex(char p)
>>> }
>>> /**
>>> - * is_checksum_valid
>>> - *
>>> * Validate the checksum in the provided TlvInfo EEPROM data. First,
>>> * verify that the TlvInfo header is valid, then make sure the last
>>> * TLV is a CRC-32 TLV. Then calculate the CRC over the EEPROM data
>>> * and compare it to the value stored in the EEPROM CRC-32 TLV.
>>> */
>>> -static bool is_checksum_valid(u8 *eeprom)
>>> +bool tlvinfo_check_crc(u8 *eeprom)
>>> {
>>> struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
>>> struct tlvinfo_tlv *eeprom_crc;
>>> @@ -137,11 +119,11 @@ static int read_eeprom(int devnum, u8 *eeprom)
>>> // If the contents are invalid, start over with default contents
>>> if (!is_valid_tlvinfo_header(eeprom_hdr) ||
>>> - !is_checksum_valid(eeprom)) {
>>> + !tlvinfo_check_crc(eeprom)) {
>>> strcpy(eeprom_hdr->signature, TLV_INFO_ID_STRING);
>>> eeprom_hdr->version = TLV_INFO_VERSION;
>>> eeprom_hdr->totallen = cpu_to_be16(0);
>>> - update_crc(eeprom);
>>> + tlvinfo_update_crc(eeprom);
>>> }
>>> #ifdef DEBUG
>>> @@ -183,7 +165,7 @@ static void show_eeprom(int devnum, u8 *eeprom)
>>> tlv_end = HDR_SIZE + be16_to_cpu(eeprom_hdr->totallen);
>>> while (curr_tlv < tlv_end) {
>>> eeprom_tlv = to_entry(&eeprom[curr_tlv]);
>>> - if (!is_valid_tlv(eeprom_tlv)) {
>>> + if (!is_valid_tlvinfo_entry(eeprom_tlv)) {
>>> printf("Invalid TLV field starting at EEPROM offset %d\n",
>>> curr_tlv);
>>> return;
>>> @@ -193,7 +175,7 @@ static void show_eeprom(int devnum, u8 *eeprom)
>>> }
>>> printf("Checksum is %s.\n",
>>> - is_checksum_valid(eeprom) ? "valid" : "invalid");
>>> + tlvinfo_check_crc(eeprom) ? "valid" : "invalid");
>>> #ifdef DEBUG
>>> printf("EEPROM dump: (0x%x bytes)", TLV_INFO_MAX_LEN);
>>> @@ -340,13 +322,13 @@ static void decode_tlv(struct tlvinfo_tlv *tlv)
>>> }
>>> /**
>>> - * update_crc
>>> + * tlvinfo_update_crc
>>> *
>>> * This function updates the CRC-32 TLV. If there is no CRC-32
>>> TLV, then
>>> * one is added. This function should be called after each update
>>> to the
>>> * EEPROM structure, to make sure the CRC is always correct.
>>> */
>>> -static void update_crc(u8 *eeprom)
>>> +void tlvinfo_update_crc(u8 *eeprom)
>>> {
>>> struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
>>> struct tlvinfo_tlv *eeprom_crc;
>>> @@ -376,20 +358,20 @@ static void update_crc(u8 *eeprom)
>>> }
>>> /**
>>> - * prog_eeprom
>>> + * write_tlvinfo_tlv_eeprom
>>> *
>>> - * Write the EEPROM data from CPU memory to the hardware.
>>> + * Write the TLV data from CPU memory to the hardware.
>>> */
>>> -static int prog_eeprom(int devnum, u8 *eeprom)
>>> +int write_tlvinfo_tlv_eeprom(void *eeprom, int dev)
>>> {
>>> int ret = 0;
>>> struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
>>> int eeprom_len;
>>> - update_crc(eeprom);
>>> + tlvinfo_update_crc(eeprom);
>>> eeprom_len = HDR_SIZE + be16_to_cpu(eeprom_hdr->totallen);
>>> - ret = write_tlv_eeprom(eeprom, eeprom_len, devnum);
>>> + ret = write_tlv_eeprom(eeprom, eeprom_len, dev);
>>> if (ret) {
>>> printf("Programming failed.\n");
>>> return -1;
>>> @@ -462,13 +444,13 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int
>>> flag, int argc, char *const argv[])
>>> if (argc == 2) {
>>> switch (cmd) {
>>> case 'w': /* write */
>>> - prog_eeprom(current_dev, eeprom);
>>> + write_tlvinfo_tlv_eeprom(eeprom, current_dev);
>>> break;
>>> case 'e': /* erase */
>>> strcpy(eeprom_hdr->signature, TLV_INFO_ID_STRING);
>>> eeprom_hdr->version = TLV_INFO_VERSION;
>>> eeprom_hdr->totallen = cpu_to_be16(0);
>>> - update_crc(eeprom);
>>> + tlvinfo_update_crc(eeprom);
>>> printf("EEPROM data in memory reset.\n");
>>> break;
>>> case 'l': /* list */
>>> @@ -549,7 +531,7 @@ U_BOOT_CMD(tlv_eeprom, 4, 1, do_tlv_eeprom,
>>> * An offset from the beginning of the EEPROM is returned in the
>>> * eeprom_index parameter if the TLV is found.
>>> */
>>> -static bool tlvinfo_find_tlv(u8 *eeprom, u8 tcode, int *eeprom_index)
>>> +bool tlvinfo_find_tlv(u8 *eeprom, u8 tcode, int *eeprom_index)
>>> {
>>> struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
>>> struct tlvinfo_tlv *eeprom_tlv;
>>> @@ -561,7 +543,7 @@ static bool tlvinfo_find_tlv(u8 *eeprom, u8
>>> tcode, int *eeprom_index)
>>> eeprom_end = HDR_SIZE + be16_to_cpu(eeprom_hdr->totallen);
>>> while (*eeprom_index < eeprom_end) {
>>> eeprom_tlv = to_entry(&eeprom[*eeprom_index]);
>>> - if (!is_valid_tlv(eeprom_tlv))
>>> + if (!is_valid_tlvinfo_entry(eeprom_tlv))
>>> return false;
>>> if (eeprom_tlv->type == tcode)
>>> return true;
>>> @@ -594,7 +576,7 @@ static bool tlvinfo_delete_tlv(u8 *eeprom, u8 code)
>>> eeprom_hdr->totallen =
>>> cpu_to_be16(be16_to_cpu(eeprom_hdr->totallen) -
>>> tlength);
>>> - update_crc(eeprom);
>>> + tlvinfo_update_crc(eeprom);
>>> return true;
>>> }
>>> return false;
>>> @@ -695,7 +677,7 @@ static bool tlvinfo_add_tlv(u8 *eeprom, int
>>> tcode, char *strval)
>>> // Update the total length and calculate (add) a new CRC-32 TLV
>>> eeprom_hdr->totallen =
>>> cpu_to_be16(be16_to_cpu(eeprom_hdr->totallen) +
>>> ENT_SIZE + new_tlv_len);
>>> - update_crc(eeprom);
>>> + tlvinfo_update_crc(eeprom);
>>> return true;
>>> }
>>> @@ -986,7 +968,7 @@ int read_tlvinfo_tlv_eeprom(void *eeprom, struct
>>> tlvinfo_header **hdr,
>>> be16_to_cpu(tlv_hdr->totallen), dev_num);
>>> if (ret < 0)
>>> return ret;
>>> - if (!is_checksum_valid(eeprom))
>>> + if (!tlvinfo_check_crc(eeprom))
>>> return -EINVAL;
>>> *hdr = tlv_hdr;
>>> diff --git a/include/tlv_eeprom.h b/include/tlv_eeprom.h
>>> index fd45e5f6eb..30626a1067 100644
>>> --- a/include/tlv_eeprom.h
>>> +++ b/include/tlv_eeprom.h
>>> @@ -111,6 +111,51 @@ int write_tlv_eeprom(void *eeprom, int len, int
>>> dev);
>>> int read_tlvinfo_tlv_eeprom(void *eeprom, struct tlvinfo_header **hdr,
>>> struct tlvinfo_tlv **first_entry, int dev);
>>> +/**
>>> + * Write TLV data to the EEPROM.
>>> + *
>>> + * - Only writes length of actual tlv data
>>> + * - updates checksum
>>> + *
>>> + * @eeprom: Pointer to buffer to hold the binary data. Must point to
>>> a buffer
>>> + * of size at least TLV_INFO_MAX_LEN.
>>> + * @dev : EEPROM device to write
>>> + *
>>> + */
>>> +int write_tlvinfo_tlv_eeprom(void *eeprom, int dev);
>>> +
>>> +/**
>>> + * tlvinfo_find_tlv
>>> + *
>>> + * This function finds the TLV with the supplied code in the EERPOM.
>>> + * An offset from the beginning of the EEPROM is returned in the
>>> + * eeprom_index parameter if the TLV is found.
>>> + */
>>> +bool tlvinfo_find_tlv(u8 *eeprom, u8 tcode, int *eeprom_index);
>>> +
>>> +/**
>>> + * tlvinfo_update_crc
>>> + *
>>> + * This function updates the CRC-32 TLV. If there is no CRC-32 TLV,
>>> then
>>> + * one is added. This function should be called after each update
>>> to the
>>> + * EEPROM structure, to make sure the CRC is always correct.
>>> + *
>>> + * @eeprom: Pointer to buffer to hold the binary data. Must point to
>>> a buffer
>>> + * of size at least TLV_INFO_MAX_LEN.
>>> + */
>>> +void tlvinfo_update_crc(u8 *eeprom);
>>> +
>>> +/**
>>> + * Validate the checksum in the provided TlvInfo EEPROM data. First,
>>> + * verify that the TlvInfo header is valid, then make sure the last
>>> + * TLV is a CRC-32 TLV. Then calculate the CRC over the EEPROM data
>>> + * and compare it to the value stored in the EEPROM CRC-32 TLV.
>>> + *
>>> + * @eeprom: Pointer to buffer to hold the binary data. Must point to
>>> a buffer
>>> + * of size at least TLV_INFO_MAX_LEN.
>>> + */
>>> +bool tlvinfo_check_crc(u8 *eeprom);
>>> +
>>> #else /* !CONFIG_IS_ENABLED(CMD_TLV_EEPROM) */
>>> static inline int read_tlv_eeprom(void *eeprom, int offset, int
>>> len, int dev)
>>> @@ -150,4 +195,16 @@ static inline bool
>>> is_valid_tlvinfo_header(struct tlvinfo_header *hdr)
>>> (be16_to_cpu(hdr->totallen) <= TLV_TOTAL_LEN_MAX));
>>> }
>>> +/**
>>> + * is_valid_tlv
>>> + *
>>> + * Perform basic sanity checks on a TLV field. The TLV is pointed to
>>> + * by the parameter provided.
>>> + * 1. The type code is not reserved (0x00 or 0xFF)
>>> + */
>>> +static inline bool is_valid_tlvinfo_entry(struct tlvinfo_tlv *tlv)
>>> +{
>>> + return((tlv->type != 0x00) && (tlv->type != 0xFF));
>>> +}
>>> +
>>> #endif /* __TLV_EEPROM_H_ */
>>
>> Viele Grüße,
>> Stefan Roese
>>
Viele Grüße,
Stefan Roese
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 04/12] cmd: tlv_eeprom: convert functions used by command to api functions
2022-05-03 10:54 ` Stefan Roese
@ 2022-05-03 19:09 ` Josua Mayer
2022-05-05 5:09 ` Stefan Roese
0 siblings, 1 reply; 22+ messages in thread
From: Josua Mayer @ 2022-05-03 19:09 UTC (permalink / raw)
To: Stefan Roese, u-boot; +Cc: Sven Auhagen, Simon Glass
\o/
Am 03.05.22 um 13:54 schrieb Stefan Roese:
> Hi Josua,
>
> On 03.05.22 09:17, Josua Mayer wrote:
>> Am 03.05.22 um 09:16 schrieb Stefan Roese:
>>> On 02.05.22 16:18, Josua Mayer wrote:
>>>> - prog_eeprom: write_tlvinfo_tlv_eeprom
>>>> - update_crc: tlvinfo_update_crc
>>>> - is_valid_tlv: is_valid_tlvinfo_entry
>>>> - is_checksum_valid: tlvinfo_check_crc
>>>
>>> So while creating a new API it makes sense to prepend the function
>>> names identical IMHO to not "pollute" the namespace. Something like
>>>
>>> - tlv_is_valid_entry
>>> - tlv_check_crc
>>> ...
>>>
>>> Just examples, you get the idea.
>> Yes. The hard part in this particular implementation is that the
>> naming is not consistent.
>>
>> The most sense I could make is that prefix tlvinfo indicates all tlv
>> data, i.e. working with the whole structure, while tlvinfo_tlv
>> indicates working with one data entry. Further write, read and is_
>> are currently prefixed in the header, but for previously static
>> functions in the C file it was put in the middle ...
>>
>> I found it quite difficult to prepare for splitting off a library in
>> a way that preserves history, i.e. diffs should still be readable for
>> spotting mistakes.
>
> Yes, a decent history would be welcome. But still, when going global
> here with a new API this should be consistant.
My view more like - patches 1-10 are not really new API, in that I am
only changing what is necessary to allow splitting the code.
>> I was considering to at the very end do a mass-rename and come up
>> with better naming, something like
>> tlv_{set,get}_{blob,string,mac}
>> tlv_find_entry
>> tlv_{read,write}_eeprom
>>
>> But this is pending a refactoring and extension of the tlv parsing
>> code in board/solidrun/common/tlv_data.*, to figure out what is
>> required or useful.
>
> So your plan is to this:
> a) Get this patchset included
> b) Use it in board specific code, e.g. solidrun
> c) Do the mass-rename
>
> Is this correct?
This is close. I would say
1) get the split merged
2) rebase board-specific code
2a) figure out what kinds of set/get/add/remove functions are useful
3) redesign api
- there are inconsistencies with the order of function arguments
- read/write functions imo should use the tlv header to determine
size, not a function argument
- at least one of the tlv data types can appear multiple times,
existing code does not support this
4) submit any renames and extensions to the tlv library
5) submit board-specific use of tlv eeprom data
> If yes, why is it better to do the renaming at the end?
It is very difficult to work on a patch-set that touches the same code
before and after moving it to a different file, which I found myself
doing a lot while prototyping this.
So if now I went ahead and figured out proper names and purposes for all
functions that I think should be the tlv api, then I have to divide that
into those parts that are renames or refactoring of existing
functionality, and those parts that are strictly new - putting the
former before splitting off the library, and the latter to after.
I am not sure I can manage this level of complexity.
- Josua Mayer
>
> Thanks,
> Stefan
>
>>>
>>> Thanks,
>>> Stefan
>>>
>>>> Signed-off-by: Josua Mayer <josua@solid-run.com>
>>>> ---
>>>> cmd/tlv_eeprom.c | 56
>>>> +++++++++++++++----------------------------
>>>> include/tlv_eeprom.h | 57
>>>> ++++++++++++++++++++++++++++++++++++++++++++
>>>> 2 files changed, 76 insertions(+), 37 deletions(-)
>>>>
>>>> diff --git a/cmd/tlv_eeprom.c b/cmd/tlv_eeprom.c
>>>> index 00c5b5f840..1b4f2537f6 100644
>>>> --- a/cmd/tlv_eeprom.c
>>>> +++ b/cmd/tlv_eeprom.c
>>>> @@ -28,13 +28,9 @@ DECLARE_GLOBAL_DATA_PTR;
>>>> #define MAX_TLV_DEVICES 2
>>>> /* File scope function prototypes */
>>>> -static bool is_checksum_valid(u8 *eeprom);
>>>> static int read_eeprom(int devnum, u8 *eeprom);
>>>> static void show_eeprom(int devnum, u8 *eeprom);
>>>> static void decode_tlv(struct tlvinfo_tlv *tlv);
>>>> -static void update_crc(u8 *eeprom);
>>>> -static int prog_eeprom(int devnum, u8 *eeprom);
>>>> -static bool tlvinfo_find_tlv(u8 *eeprom, u8 tcode, int
>>>> *eeprom_index);
>>>> static bool tlvinfo_delete_tlv(u8 *eeprom, u8 code);
>>>> static bool tlvinfo_add_tlv(u8 *eeprom, int tcode, char *strval);
>>>> static int set_mac(char *buf, const char *string);
>>>> @@ -58,18 +54,6 @@ static inline bool is_digit(char c)
>>>> return (c >= '0' && c <= '9');
>>>> }
>>>> -/**
>>>> - * is_valid_tlv
>>>> - *
>>>> - * Perform basic sanity checks on a TLV field. The TLV is pointed to
>>>> - * by the parameter provided.
>>>> - * 1. The type code is not reserved (0x00 or 0xFF)
>>>> - */
>>>> -static inline bool is_valid_tlv(struct tlvinfo_tlv *tlv)
>>>> -{
>>>> - return((tlv->type != 0x00) && (tlv->type != 0xFF));
>>>> -}
>>>> -
>>>> /**
>>>> * is_hex
>>>> *
>>>> @@ -83,14 +67,12 @@ static inline u8 is_hex(char p)
>>>> }
>>>> /**
>>>> - * is_checksum_valid
>>>> - *
>>>> * Validate the checksum in the provided TlvInfo EEPROM data.
>>>> First,
>>>> * verify that the TlvInfo header is valid, then make sure the last
>>>> * TLV is a CRC-32 TLV. Then calculate the CRC over the EEPROM data
>>>> * and compare it to the value stored in the EEPROM CRC-32 TLV.
>>>> */
>>>> -static bool is_checksum_valid(u8 *eeprom)
>>>> +bool tlvinfo_check_crc(u8 *eeprom)
>>>> {
>>>> struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
>>>> struct tlvinfo_tlv *eeprom_crc;
>>>> @@ -137,11 +119,11 @@ static int read_eeprom(int devnum, u8 *eeprom)
>>>> // If the contents are invalid, start over with default
>>>> contents
>>>> if (!is_valid_tlvinfo_header(eeprom_hdr) ||
>>>> - !is_checksum_valid(eeprom)) {
>>>> + !tlvinfo_check_crc(eeprom)) {
>>>> strcpy(eeprom_hdr->signature, TLV_INFO_ID_STRING);
>>>> eeprom_hdr->version = TLV_INFO_VERSION;
>>>> eeprom_hdr->totallen = cpu_to_be16(0);
>>>> - update_crc(eeprom);
>>>> + tlvinfo_update_crc(eeprom);
>>>> }
>>>> #ifdef DEBUG
>>>> @@ -183,7 +165,7 @@ static void show_eeprom(int devnum, u8 *eeprom)
>>>> tlv_end = HDR_SIZE + be16_to_cpu(eeprom_hdr->totallen);
>>>> while (curr_tlv < tlv_end) {
>>>> eeprom_tlv = to_entry(&eeprom[curr_tlv]);
>>>> - if (!is_valid_tlv(eeprom_tlv)) {
>>>> + if (!is_valid_tlvinfo_entry(eeprom_tlv)) {
>>>> printf("Invalid TLV field starting at EEPROM offset
>>>> %d\n",
>>>> curr_tlv);
>>>> return;
>>>> @@ -193,7 +175,7 @@ static void show_eeprom(int devnum, u8 *eeprom)
>>>> }
>>>> printf("Checksum is %s.\n",
>>>> - is_checksum_valid(eeprom) ? "valid" : "invalid");
>>>> + tlvinfo_check_crc(eeprom) ? "valid" : "invalid");
>>>> #ifdef DEBUG
>>>> printf("EEPROM dump: (0x%x bytes)", TLV_INFO_MAX_LEN);
>>>> @@ -340,13 +322,13 @@ static void decode_tlv(struct tlvinfo_tlv *tlv)
>>>> }
>>>> /**
>>>> - * update_crc
>>>> + * tlvinfo_update_crc
>>>> *
>>>> * This function updates the CRC-32 TLV. If there is no CRC-32
>>>> TLV, then
>>>> * one is added. This function should be called after each
>>>> update to the
>>>> * EEPROM structure, to make sure the CRC is always correct.
>>>> */
>>>> -static void update_crc(u8 *eeprom)
>>>> +void tlvinfo_update_crc(u8 *eeprom)
>>>> {
>>>> struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
>>>> struct tlvinfo_tlv *eeprom_crc;
>>>> @@ -376,20 +358,20 @@ static void update_crc(u8 *eeprom)
>>>> }
>>>> /**
>>>> - * prog_eeprom
>>>> + * write_tlvinfo_tlv_eeprom
>>>> *
>>>> - * Write the EEPROM data from CPU memory to the hardware.
>>>> + * Write the TLV data from CPU memory to the hardware.
>>>> */
>>>> -static int prog_eeprom(int devnum, u8 *eeprom)
>>>> +int write_tlvinfo_tlv_eeprom(void *eeprom, int dev)
>>>> {
>>>> int ret = 0;
>>>> struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
>>>> int eeprom_len;
>>>> - update_crc(eeprom);
>>>> + tlvinfo_update_crc(eeprom);
>>>> eeprom_len = HDR_SIZE + be16_to_cpu(eeprom_hdr->totallen);
>>>> - ret = write_tlv_eeprom(eeprom, eeprom_len, devnum);
>>>> + ret = write_tlv_eeprom(eeprom, eeprom_len, dev);
>>>> if (ret) {
>>>> printf("Programming failed.\n");
>>>> return -1;
>>>> @@ -462,13 +444,13 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int
>>>> flag, int argc, char *const argv[])
>>>> if (argc == 2) {
>>>> switch (cmd) {
>>>> case 'w': /* write */
>>>> - prog_eeprom(current_dev, eeprom);
>>>> + write_tlvinfo_tlv_eeprom(eeprom, current_dev);
>>>> break;
>>>> case 'e': /* erase */
>>>> strcpy(eeprom_hdr->signature, TLV_INFO_ID_STRING);
>>>> eeprom_hdr->version = TLV_INFO_VERSION;
>>>> eeprom_hdr->totallen = cpu_to_be16(0);
>>>> - update_crc(eeprom);
>>>> + tlvinfo_update_crc(eeprom);
>>>> printf("EEPROM data in memory reset.\n");
>>>> break;
>>>> case 'l': /* list */
>>>> @@ -549,7 +531,7 @@ U_BOOT_CMD(tlv_eeprom, 4, 1, do_tlv_eeprom,
>>>> * An offset from the beginning of the EEPROM is returned in the
>>>> * eeprom_index parameter if the TLV is found.
>>>> */
>>>> -static bool tlvinfo_find_tlv(u8 *eeprom, u8 tcode, int *eeprom_index)
>>>> +bool tlvinfo_find_tlv(u8 *eeprom, u8 tcode, int *eeprom_index)
>>>> {
>>>> struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
>>>> struct tlvinfo_tlv *eeprom_tlv;
>>>> @@ -561,7 +543,7 @@ static bool tlvinfo_find_tlv(u8 *eeprom, u8
>>>> tcode, int *eeprom_index)
>>>> eeprom_end = HDR_SIZE + be16_to_cpu(eeprom_hdr->totallen);
>>>> while (*eeprom_index < eeprom_end) {
>>>> eeprom_tlv = to_entry(&eeprom[*eeprom_index]);
>>>> - if (!is_valid_tlv(eeprom_tlv))
>>>> + if (!is_valid_tlvinfo_entry(eeprom_tlv))
>>>> return false;
>>>> if (eeprom_tlv->type == tcode)
>>>> return true;
>>>> @@ -594,7 +576,7 @@ static bool tlvinfo_delete_tlv(u8 *eeprom, u8
>>>> code)
>>>> eeprom_hdr->totallen =
>>>> cpu_to_be16(be16_to_cpu(eeprom_hdr->totallen) -
>>>> tlength);
>>>> - update_crc(eeprom);
>>>> + tlvinfo_update_crc(eeprom);
>>>> return true;
>>>> }
>>>> return false;
>>>> @@ -695,7 +677,7 @@ static bool tlvinfo_add_tlv(u8 *eeprom, int
>>>> tcode, char *strval)
>>>> // Update the total length and calculate (add) a new CRC-32 TLV
>>>> eeprom_hdr->totallen =
>>>> cpu_to_be16(be16_to_cpu(eeprom_hdr->totallen) +
>>>> ENT_SIZE + new_tlv_len);
>>>> - update_crc(eeprom);
>>>> + tlvinfo_update_crc(eeprom);
>>>> return true;
>>>> }
>>>> @@ -986,7 +968,7 @@ int read_tlvinfo_tlv_eeprom(void *eeprom,
>>>> struct tlvinfo_header **hdr,
>>>> be16_to_cpu(tlv_hdr->totallen), dev_num);
>>>> if (ret < 0)
>>>> return ret;
>>>> - if (!is_checksum_valid(eeprom))
>>>> + if (!tlvinfo_check_crc(eeprom))
>>>> return -EINVAL;
>>>> *hdr = tlv_hdr;
>>>> diff --git a/include/tlv_eeprom.h b/include/tlv_eeprom.h
>>>> index fd45e5f6eb..30626a1067 100644
>>>> --- a/include/tlv_eeprom.h
>>>> +++ b/include/tlv_eeprom.h
>>>> @@ -111,6 +111,51 @@ int write_tlv_eeprom(void *eeprom, int len,
>>>> int dev);
>>>> int read_tlvinfo_tlv_eeprom(void *eeprom, struct tlvinfo_header
>>>> **hdr,
>>>> struct tlvinfo_tlv **first_entry, int dev);
>>>> +/**
>>>> + * Write TLV data to the EEPROM.
>>>> + *
>>>> + * - Only writes length of actual tlv data
>>>> + * - updates checksum
>>>> + *
>>>> + * @eeprom: Pointer to buffer to hold the binary data. Must point
>>>> to a buffer
>>>> + * of size at least TLV_INFO_MAX_LEN.
>>>> + * @dev : EEPROM device to write
>>>> + *
>>>> + */
>>>> +int write_tlvinfo_tlv_eeprom(void *eeprom, int dev);
>>>> +
>>>> +/**
>>>> + * tlvinfo_find_tlv
>>>> + *
>>>> + * This function finds the TLV with the supplied code in the EERPOM.
>>>> + * An offset from the beginning of the EEPROM is returned in the
>>>> + * eeprom_index parameter if the TLV is found.
>>>> + */
>>>> +bool tlvinfo_find_tlv(u8 *eeprom, u8 tcode, int *eeprom_index);
>>>> +
>>>> +/**
>>>> + * tlvinfo_update_crc
>>>> + *
>>>> + * This function updates the CRC-32 TLV. If there is no CRC-32
>>>> TLV, then
>>>> + * one is added. This function should be called after each update
>>>> to the
>>>> + * EEPROM structure, to make sure the CRC is always correct.
>>>> + *
>>>> + * @eeprom: Pointer to buffer to hold the binary data. Must point
>>>> to a buffer
>>>> + * of size at least TLV_INFO_MAX_LEN.
>>>> + */
>>>> +void tlvinfo_update_crc(u8 *eeprom);
>>>> +
>>>> +/**
>>>> + * Validate the checksum in the provided TlvInfo EEPROM data. First,
>>>> + * verify that the TlvInfo header is valid, then make sure the last
>>>> + * TLV is a CRC-32 TLV. Then calculate the CRC over the EEPROM data
>>>> + * and compare it to the value stored in the EEPROM CRC-32 TLV.
>>>> + *
>>>> + * @eeprom: Pointer to buffer to hold the binary data. Must point
>>>> to a buffer
>>>> + * of size at least TLV_INFO_MAX_LEN.
>>>> + */
>>>> +bool tlvinfo_check_crc(u8 *eeprom);
>>>> +
>>>> #else /* !CONFIG_IS_ENABLED(CMD_TLV_EEPROM) */
>>>> static inline int read_tlv_eeprom(void *eeprom, int offset, int
>>>> len, int dev)
>>>> @@ -150,4 +195,16 @@ static inline bool
>>>> is_valid_tlvinfo_header(struct tlvinfo_header *hdr)
>>>> (be16_to_cpu(hdr->totallen) <= TLV_TOTAL_LEN_MAX));
>>>> }
>>>> +/**
>>>> + * is_valid_tlv
>>>> + *
>>>> + * Perform basic sanity checks on a TLV field. The TLV is pointed to
>>>> + * by the parameter provided.
>>>> + * 1. The type code is not reserved (0x00 or 0xFF)
>>>> + */
>>>> +static inline bool is_valid_tlvinfo_entry(struct tlvinfo_tlv *tlv)
>>>> +{
>>>> + return((tlv->type != 0x00) && (tlv->type != 0xFF));
>>>> +}
>>>> +
>>>> #endif /* __TLV_EEPROM_H_ */
>>>
>>> Viele Grüße,
>>> Stefan Roese
>>>
>
> Viele Grüße,
> Stefan Roese
>
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 04/12] cmd: tlv_eeprom: convert functions used by command to api functions
2022-05-03 19:09 ` Josua Mayer
@ 2022-05-05 5:09 ` Stefan Roese
0 siblings, 0 replies; 22+ messages in thread
From: Stefan Roese @ 2022-05-05 5:09 UTC (permalink / raw)
To: Josua Mayer, u-boot; +Cc: Sven Auhagen, Simon Glass
Hi Josua,
On 03.05.22 21:09, Josua Mayer wrote:
> \o/
>
> Am 03.05.22 um 13:54 schrieb Stefan Roese:
>> Hi Josua,
>>
>> On 03.05.22 09:17, Josua Mayer wrote:
>>> Am 03.05.22 um 09:16 schrieb Stefan Roese:
>>>> On 02.05.22 16:18, Josua Mayer wrote:
>>>>> - prog_eeprom: write_tlvinfo_tlv_eeprom
>>>>> - update_crc: tlvinfo_update_crc
>>>>> - is_valid_tlv: is_valid_tlvinfo_entry
>>>>> - is_checksum_valid: tlvinfo_check_crc
>>>>
>>>> So while creating a new API it makes sense to prepend the function
>>>> names identical IMHO to not "pollute" the namespace. Something like
>>>>
>>>> - tlv_is_valid_entry
>>>> - tlv_check_crc
>>>> ...
>>>>
>>>> Just examples, you get the idea.
>>> Yes. The hard part in this particular implementation is that the
>>> naming is not consistent.
>>>
>>> The most sense I could make is that prefix tlvinfo indicates all tlv
>>> data, i.e. working with the whole structure, while tlvinfo_tlv
>>> indicates working with one data entry. Further write, read and is_
>>> are currently prefixed in the header, but for previously static
>>> functions in the C file it was put in the middle ...
>>>
>>> I found it quite difficult to prepare for splitting off a library in
>>> a way that preserves history, i.e. diffs should still be readable for
>>> spotting mistakes.
>>
>> Yes, a decent history would be welcome. But still, when going global
>> here with a new API this should be consistant.
> My view more like - patches 1-10 are not really new API, in that I am
> only changing what is necessary to allow splitting the code.
>>> I was considering to at the very end do a mass-rename and come up
>>> with better naming, something like
>>> tlv_{set,get}_{blob,string,mac}
>>> tlv_find_entry
>>> tlv_{read,write}_eeprom
>>>
>>> But this is pending a refactoring and extension of the tlv parsing
>>> code in board/solidrun/common/tlv_data.*, to figure out what is
>>> required or useful.
>>
>> So your plan is to this:
>> a) Get this patchset included
>> b) Use it in board specific code, e.g. solidrun
>> c) Do the mass-rename
>>
>> Is this correct?
> This is close. I would say
> 1) get the split merged
> 2) rebase board-specific code
> 2a) figure out what kinds of set/get/add/remove functions are useful
> 3) redesign api
> - there are inconsistencies with the order of function arguments
> - read/write functions imo should use the tlv header to determine
> size, not a function argument
> - at least one of the tlv data types can appear multiple times,
> existing code does not support this
> 4) submit any renames and extensions to the tlv library
> 5) submit board-specific use of tlv eeprom data
>> If yes, why is it better to do the renaming at the end?
> It is very difficult to work on a patch-set that touches the same code
> before and after moving it to a different file, which I found myself
> doing a lot while prototyping this.
> So if now I went ahead and figured out proper names and purposes for all
> functions that I think should be the tlv api, then I have to divide that
> into those parts that are renames or refactoring of existing
> functionality, and those parts that are strictly new - putting the
> former before splitting off the library, and the latter to after.
>
> I am not sure I can manage this level of complexity.
To cut it short, if you plan to rename the API to some consitant naming
in the end, then I am okay with moving forward without a renaming at
this early stage.
Thanks,
Stefan
>
> - Josua Mayer
>
>>
>> Thanks,
>> Stefan
>>
>>>>
>>>> Thanks,
>>>> Stefan
>>>>
>>>>> Signed-off-by: Josua Mayer <josua@solid-run.com>
>>>>> ---
>>>>> cmd/tlv_eeprom.c | 56
>>>>> +++++++++++++++----------------------------
>>>>> include/tlv_eeprom.h | 57
>>>>> ++++++++++++++++++++++++++++++++++++++++++++
>>>>> 2 files changed, 76 insertions(+), 37 deletions(-)
>>>>>
>>>>> diff --git a/cmd/tlv_eeprom.c b/cmd/tlv_eeprom.c
>>>>> index 00c5b5f840..1b4f2537f6 100644
>>>>> --- a/cmd/tlv_eeprom.c
>>>>> +++ b/cmd/tlv_eeprom.c
>>>>> @@ -28,13 +28,9 @@ DECLARE_GLOBAL_DATA_PTR;
>>>>> #define MAX_TLV_DEVICES 2
>>>>> /* File scope function prototypes */
>>>>> -static bool is_checksum_valid(u8 *eeprom);
>>>>> static int read_eeprom(int devnum, u8 *eeprom);
>>>>> static void show_eeprom(int devnum, u8 *eeprom);
>>>>> static void decode_tlv(struct tlvinfo_tlv *tlv);
>>>>> -static void update_crc(u8 *eeprom);
>>>>> -static int prog_eeprom(int devnum, u8 *eeprom);
>>>>> -static bool tlvinfo_find_tlv(u8 *eeprom, u8 tcode, int
>>>>> *eeprom_index);
>>>>> static bool tlvinfo_delete_tlv(u8 *eeprom, u8 code);
>>>>> static bool tlvinfo_add_tlv(u8 *eeprom, int tcode, char *strval);
>>>>> static int set_mac(char *buf, const char *string);
>>>>> @@ -58,18 +54,6 @@ static inline bool is_digit(char c)
>>>>> return (c >= '0' && c <= '9');
>>>>> }
>>>>> -/**
>>>>> - * is_valid_tlv
>>>>> - *
>>>>> - * Perform basic sanity checks on a TLV field. The TLV is pointed to
>>>>> - * by the parameter provided.
>>>>> - * 1. The type code is not reserved (0x00 or 0xFF)
>>>>> - */
>>>>> -static inline bool is_valid_tlv(struct tlvinfo_tlv *tlv)
>>>>> -{
>>>>> - return((tlv->type != 0x00) && (tlv->type != 0xFF));
>>>>> -}
>>>>> -
>>>>> /**
>>>>> * is_hex
>>>>> *
>>>>> @@ -83,14 +67,12 @@ static inline u8 is_hex(char p)
>>>>> }
>>>>> /**
>>>>> - * is_checksum_valid
>>>>> - *
>>>>> * Validate the checksum in the provided TlvInfo EEPROM data.
>>>>> First,
>>>>> * verify that the TlvInfo header is valid, then make sure the last
>>>>> * TLV is a CRC-32 TLV. Then calculate the CRC over the EEPROM data
>>>>> * and compare it to the value stored in the EEPROM CRC-32 TLV.
>>>>> */
>>>>> -static bool is_checksum_valid(u8 *eeprom)
>>>>> +bool tlvinfo_check_crc(u8 *eeprom)
>>>>> {
>>>>> struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
>>>>> struct tlvinfo_tlv *eeprom_crc;
>>>>> @@ -137,11 +119,11 @@ static int read_eeprom(int devnum, u8 *eeprom)
>>>>> // If the contents are invalid, start over with default
>>>>> contents
>>>>> if (!is_valid_tlvinfo_header(eeprom_hdr) ||
>>>>> - !is_checksum_valid(eeprom)) {
>>>>> + !tlvinfo_check_crc(eeprom)) {
>>>>> strcpy(eeprom_hdr->signature, TLV_INFO_ID_STRING);
>>>>> eeprom_hdr->version = TLV_INFO_VERSION;
>>>>> eeprom_hdr->totallen = cpu_to_be16(0);
>>>>> - update_crc(eeprom);
>>>>> + tlvinfo_update_crc(eeprom);
>>>>> }
>>>>> #ifdef DEBUG
>>>>> @@ -183,7 +165,7 @@ static void show_eeprom(int devnum, u8 *eeprom)
>>>>> tlv_end = HDR_SIZE + be16_to_cpu(eeprom_hdr->totallen);
>>>>> while (curr_tlv < tlv_end) {
>>>>> eeprom_tlv = to_entry(&eeprom[curr_tlv]);
>>>>> - if (!is_valid_tlv(eeprom_tlv)) {
>>>>> + if (!is_valid_tlvinfo_entry(eeprom_tlv)) {
>>>>> printf("Invalid TLV field starting at EEPROM offset
>>>>> %d\n",
>>>>> curr_tlv);
>>>>> return;
>>>>> @@ -193,7 +175,7 @@ static void show_eeprom(int devnum, u8 *eeprom)
>>>>> }
>>>>> printf("Checksum is %s.\n",
>>>>> - is_checksum_valid(eeprom) ? "valid" : "invalid");
>>>>> + tlvinfo_check_crc(eeprom) ? "valid" : "invalid");
>>>>> #ifdef DEBUG
>>>>> printf("EEPROM dump: (0x%x bytes)", TLV_INFO_MAX_LEN);
>>>>> @@ -340,13 +322,13 @@ static void decode_tlv(struct tlvinfo_tlv *tlv)
>>>>> }
>>>>> /**
>>>>> - * update_crc
>>>>> + * tlvinfo_update_crc
>>>>> *
>>>>> * This function updates the CRC-32 TLV. If there is no CRC-32
>>>>> TLV, then
>>>>> * one is added. This function should be called after each
>>>>> update to the
>>>>> * EEPROM structure, to make sure the CRC is always correct.
>>>>> */
>>>>> -static void update_crc(u8 *eeprom)
>>>>> +void tlvinfo_update_crc(u8 *eeprom)
>>>>> {
>>>>> struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
>>>>> struct tlvinfo_tlv *eeprom_crc;
>>>>> @@ -376,20 +358,20 @@ static void update_crc(u8 *eeprom)
>>>>> }
>>>>> /**
>>>>> - * prog_eeprom
>>>>> + * write_tlvinfo_tlv_eeprom
>>>>> *
>>>>> - * Write the EEPROM data from CPU memory to the hardware.
>>>>> + * Write the TLV data from CPU memory to the hardware.
>>>>> */
>>>>> -static int prog_eeprom(int devnum, u8 *eeprom)
>>>>> +int write_tlvinfo_tlv_eeprom(void *eeprom, int dev)
>>>>> {
>>>>> int ret = 0;
>>>>> struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
>>>>> int eeprom_len;
>>>>> - update_crc(eeprom);
>>>>> + tlvinfo_update_crc(eeprom);
>>>>> eeprom_len = HDR_SIZE + be16_to_cpu(eeprom_hdr->totallen);
>>>>> - ret = write_tlv_eeprom(eeprom, eeprom_len, devnum);
>>>>> + ret = write_tlv_eeprom(eeprom, eeprom_len, dev);
>>>>> if (ret) {
>>>>> printf("Programming failed.\n");
>>>>> return -1;
>>>>> @@ -462,13 +444,13 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int
>>>>> flag, int argc, char *const argv[])
>>>>> if (argc == 2) {
>>>>> switch (cmd) {
>>>>> case 'w': /* write */
>>>>> - prog_eeprom(current_dev, eeprom);
>>>>> + write_tlvinfo_tlv_eeprom(eeprom, current_dev);
>>>>> break;
>>>>> case 'e': /* erase */
>>>>> strcpy(eeprom_hdr->signature, TLV_INFO_ID_STRING);
>>>>> eeprom_hdr->version = TLV_INFO_VERSION;
>>>>> eeprom_hdr->totallen = cpu_to_be16(0);
>>>>> - update_crc(eeprom);
>>>>> + tlvinfo_update_crc(eeprom);
>>>>> printf("EEPROM data in memory reset.\n");
>>>>> break;
>>>>> case 'l': /* list */
>>>>> @@ -549,7 +531,7 @@ U_BOOT_CMD(tlv_eeprom, 4, 1, do_tlv_eeprom,
>>>>> * An offset from the beginning of the EEPROM is returned in the
>>>>> * eeprom_index parameter if the TLV is found.
>>>>> */
>>>>> -static bool tlvinfo_find_tlv(u8 *eeprom, u8 tcode, int *eeprom_index)
>>>>> +bool tlvinfo_find_tlv(u8 *eeprom, u8 tcode, int *eeprom_index)
>>>>> {
>>>>> struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
>>>>> struct tlvinfo_tlv *eeprom_tlv;
>>>>> @@ -561,7 +543,7 @@ static bool tlvinfo_find_tlv(u8 *eeprom, u8
>>>>> tcode, int *eeprom_index)
>>>>> eeprom_end = HDR_SIZE + be16_to_cpu(eeprom_hdr->totallen);
>>>>> while (*eeprom_index < eeprom_end) {
>>>>> eeprom_tlv = to_entry(&eeprom[*eeprom_index]);
>>>>> - if (!is_valid_tlv(eeprom_tlv))
>>>>> + if (!is_valid_tlvinfo_entry(eeprom_tlv))
>>>>> return false;
>>>>> if (eeprom_tlv->type == tcode)
>>>>> return true;
>>>>> @@ -594,7 +576,7 @@ static bool tlvinfo_delete_tlv(u8 *eeprom, u8
>>>>> code)
>>>>> eeprom_hdr->totallen =
>>>>> cpu_to_be16(be16_to_cpu(eeprom_hdr->totallen) -
>>>>> tlength);
>>>>> - update_crc(eeprom);
>>>>> + tlvinfo_update_crc(eeprom);
>>>>> return true;
>>>>> }
>>>>> return false;
>>>>> @@ -695,7 +677,7 @@ static bool tlvinfo_add_tlv(u8 *eeprom, int
>>>>> tcode, char *strval)
>>>>> // Update the total length and calculate (add) a new CRC-32 TLV
>>>>> eeprom_hdr->totallen =
>>>>> cpu_to_be16(be16_to_cpu(eeprom_hdr->totallen) +
>>>>> ENT_SIZE + new_tlv_len);
>>>>> - update_crc(eeprom);
>>>>> + tlvinfo_update_crc(eeprom);
>>>>> return true;
>>>>> }
>>>>> @@ -986,7 +968,7 @@ int read_tlvinfo_tlv_eeprom(void *eeprom,
>>>>> struct tlvinfo_header **hdr,
>>>>> be16_to_cpu(tlv_hdr->totallen), dev_num);
>>>>> if (ret < 0)
>>>>> return ret;
>>>>> - if (!is_checksum_valid(eeprom))
>>>>> + if (!tlvinfo_check_crc(eeprom))
>>>>> return -EINVAL;
>>>>> *hdr = tlv_hdr;
>>>>> diff --git a/include/tlv_eeprom.h b/include/tlv_eeprom.h
>>>>> index fd45e5f6eb..30626a1067 100644
>>>>> --- a/include/tlv_eeprom.h
>>>>> +++ b/include/tlv_eeprom.h
>>>>> @@ -111,6 +111,51 @@ int write_tlv_eeprom(void *eeprom, int len,
>>>>> int dev);
>>>>> int read_tlvinfo_tlv_eeprom(void *eeprom, struct tlvinfo_header
>>>>> **hdr,
>>>>> struct tlvinfo_tlv **first_entry, int dev);
>>>>> +/**
>>>>> + * Write TLV data to the EEPROM.
>>>>> + *
>>>>> + * - Only writes length of actual tlv data
>>>>> + * - updates checksum
>>>>> + *
>>>>> + * @eeprom: Pointer to buffer to hold the binary data. Must point
>>>>> to a buffer
>>>>> + * of size at least TLV_INFO_MAX_LEN.
>>>>> + * @dev : EEPROM device to write
>>>>> + *
>>>>> + */
>>>>> +int write_tlvinfo_tlv_eeprom(void *eeprom, int dev);
>>>>> +
>>>>> +/**
>>>>> + * tlvinfo_find_tlv
>>>>> + *
>>>>> + * This function finds the TLV with the supplied code in the EERPOM.
>>>>> + * An offset from the beginning of the EEPROM is returned in the
>>>>> + * eeprom_index parameter if the TLV is found.
>>>>> + */
>>>>> +bool tlvinfo_find_tlv(u8 *eeprom, u8 tcode, int *eeprom_index);
>>>>> +
>>>>> +/**
>>>>> + * tlvinfo_update_crc
>>>>> + *
>>>>> + * This function updates the CRC-32 TLV. If there is no CRC-32
>>>>> TLV, then
>>>>> + * one is added. This function should be called after each update
>>>>> to the
>>>>> + * EEPROM structure, to make sure the CRC is always correct.
>>>>> + *
>>>>> + * @eeprom: Pointer to buffer to hold the binary data. Must point
>>>>> to a buffer
>>>>> + * of size at least TLV_INFO_MAX_LEN.
>>>>> + */
>>>>> +void tlvinfo_update_crc(u8 *eeprom);
>>>>> +
>>>>> +/**
>>>>> + * Validate the checksum in the provided TlvInfo EEPROM data. First,
>>>>> + * verify that the TlvInfo header is valid, then make sure the last
>>>>> + * TLV is a CRC-32 TLV. Then calculate the CRC over the EEPROM data
>>>>> + * and compare it to the value stored in the EEPROM CRC-32 TLV.
>>>>> + *
>>>>> + * @eeprom: Pointer to buffer to hold the binary data. Must point
>>>>> to a buffer
>>>>> + * of size at least TLV_INFO_MAX_LEN.
>>>>> + */
>>>>> +bool tlvinfo_check_crc(u8 *eeprom);
>>>>> +
>>>>> #else /* !CONFIG_IS_ENABLED(CMD_TLV_EEPROM) */
>>>>> static inline int read_tlv_eeprom(void *eeprom, int offset, int
>>>>> len, int dev)
>>>>> @@ -150,4 +195,16 @@ static inline bool
>>>>> is_valid_tlvinfo_header(struct tlvinfo_header *hdr)
>>>>> (be16_to_cpu(hdr->totallen) <= TLV_TOTAL_LEN_MAX));
>>>>> }
>>>>> +/**
>>>>> + * is_valid_tlv
>>>>> + *
>>>>> + * Perform basic sanity checks on a TLV field. The TLV is pointed to
>>>>> + * by the parameter provided.
>>>>> + * 1. The type code is not reserved (0x00 or 0xFF)
>>>>> + */
>>>>> +static inline bool is_valid_tlvinfo_entry(struct tlvinfo_tlv *tlv)
>>>>> +{
>>>>> + return((tlv->type != 0x00) && (tlv->type != 0xFF));
>>>>> +}
>>>>> +
>>>>> #endif /* __TLV_EEPROM_H_ */
>>>>
>>>> Viele Grüße,
>>>> Stefan Roese
>>>>
>>
>> Viele Grüße,
>> Stefan Roese
>>
Viele Grüße,
Stefan Roese
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 05/12] cmd: tlv_eeprom: remove empty function implementations from header
2022-05-02 14:18 [PATCH 00/12] split tlv_eeprom command into a separate library Josua Mayer
` (3 preceding siblings ...)
2022-05-02 14:18 ` [PATCH 04/12] cmd: tlv_eeprom: convert functions used by command to api functions Josua Mayer
@ 2022-05-02 14:18 ` Josua Mayer
2022-05-02 14:18 ` [PATCH 06/12] cmd: tlv_eeprom: move missing declarations and defines to header Josua Mayer
` (6 subsequent siblings)
11 siblings, 0 replies; 22+ messages in thread
From: Josua Mayer @ 2022-05-02 14:18 UTC (permalink / raw)
To: u-boot; +Cc: Josua Mayer
tlv_eeprom exposed functions are independent from platforms, hence no
stubs are required.
Signed-off-by: Josua Mayer <josua@solid-run.com>
---
include/tlv_eeprom.h | 24 ++----------------------
1 file changed, 2 insertions(+), 22 deletions(-)
diff --git a/include/tlv_eeprom.h b/include/tlv_eeprom.h
index 30626a1067..55fd72d6d2 100644
--- a/include/tlv_eeprom.h
+++ b/include/tlv_eeprom.h
@@ -65,7 +65,8 @@ struct __attribute__ ((__packed__)) tlvinfo_tlv {
#define TLV_CODE_VENDOR_EXT 0xFD
#define TLV_CODE_CRC_32 0xFE
-#if CONFIG_IS_ENABLED(CMD_TLV_EEPROM)
+/* how many EEPROMs can be used */
+#define TLV_MAX_DEVICES 2
/**
* read_tlv_eeprom - Read the EEPROM binary data from the hardware
@@ -156,27 +157,6 @@ void tlvinfo_update_crc(u8 *eeprom);
*/
bool tlvinfo_check_crc(u8 *eeprom);
-#else /* !CONFIG_IS_ENABLED(CMD_TLV_EEPROM) */
-
-static inline int read_tlv_eeprom(void *eeprom, int offset, int len, int dev)
-{
- return -ENOSYS;
-}
-
-static inline int write_tlv_eeprom(void *eeprom, int len)
-{
- return -ENOSYS;
-}
-
-static inline int
-read_tlvinfo_tlv_eeprom(void *eeprom, struct tlvinfo_header **hdr,
- struct tlvinfo_tlv **first_entry, int dev)
-{
- return -ENOSYS;
-}
-
-#endif /* CONFIG_IS_ENABLED(CMD_TLV_EEPROM) */
-
/**
* is_valid_tlvinfo_header
*
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 06/12] cmd: tlv_eeprom: move missing declarations and defines to header
2022-05-02 14:18 [PATCH 00/12] split tlv_eeprom command into a separate library Josua Mayer
` (4 preceding siblings ...)
2022-05-02 14:18 ` [PATCH 05/12] cmd: tlv_eeprom: remove empty function implementations from header Josua Mayer
@ 2022-05-02 14:18 ` Josua Mayer
2022-05-02 14:18 ` [PATCH 07/12] cmd: tlv_eeprom: hide access to static tlv_devices array behind accessor Josua Mayer
` (5 subsequent siblings)
11 siblings, 0 replies; 22+ messages in thread
From: Josua Mayer @ 2022-05-02 14:18 UTC (permalink / raw)
To: u-boot; +Cc: Josua Mayer, Stefan Roese, Sven Auhagen, Simon Glass
In preparation of splitting the tlv_eeprom command into a separate
library, add function declarations and defines used by the command logic
to the tlv_eeprom header file.
Signed-off-by: Josua Mayer <josua@solid-run.com>
---
cmd/tlv_eeprom.c | 59 ++++++++++++++++++++------------------------
include/tlv_eeprom.h | 29 ++++++++++++++++++++--
2 files changed, 54 insertions(+), 34 deletions(-)
diff --git a/cmd/tlv_eeprom.c b/cmd/tlv_eeprom.c
index 1b4f2537f6..57468edb1c 100644
--- a/cmd/tlv_eeprom.c
+++ b/cmd/tlv_eeprom.c
@@ -31,8 +31,6 @@ DECLARE_GLOBAL_DATA_PTR;
static int read_eeprom(int devnum, u8 *eeprom);
static void show_eeprom(int devnum, u8 *eeprom);
static void decode_tlv(struct tlvinfo_tlv *tlv);
-static bool tlvinfo_delete_tlv(u8 *eeprom, u8 code);
-static bool tlvinfo_add_tlv(u8 *eeprom, int tcode, char *strval);
static int set_mac(char *buf, const char *string);
static int set_date(char *buf, const char *string);
static int set_bytes(char *buf, const char *string, int *converted_accum);
@@ -46,9 +44,6 @@ static struct udevice *tlv_devices[MAX_TLV_DEVICES];
#define to_header(p) ((struct tlvinfo_header *)p)
#define to_entry(p) ((struct tlvinfo_tlv *)p)
-#define HDR_SIZE sizeof(struct tlvinfo_header)
-#define ENT_SIZE sizeof(struct tlvinfo_tlv)
-
static inline bool is_digit(char c)
{
return (c >= '0' && c <= '9');
@@ -84,14 +79,14 @@ bool tlvinfo_check_crc(u8 *eeprom)
return false;
// Is the last TLV a CRC?
- eeprom_crc = to_entry(&eeprom[HDR_SIZE +
- be16_to_cpu(eeprom_hdr->totallen) - (ENT_SIZE + 4)]);
+ eeprom_crc = to_entry(&eeprom[TLV_INFO_HEADER_SIZE +
+ be16_to_cpu(eeprom_hdr->totallen) - (TLV_INFO_ENTRY_SIZE + 4)]);
if (eeprom_crc->type != TLV_CODE_CRC_32 || eeprom_crc->length != 4)
return false;
// Calculate the checksum
calc_crc = crc32(0, (void *)eeprom,
- HDR_SIZE + be16_to_cpu(eeprom_hdr->totallen) - 4);
+ TLV_INFO_HEADER_SIZE + be16_to_cpu(eeprom_hdr->totallen) - 4);
stored_crc = (eeprom_crc->value[0] << 24) |
(eeprom_crc->value[1] << 16) |
(eeprom_crc->value[2] << 8) |
@@ -108,13 +103,13 @@ static int read_eeprom(int devnum, u8 *eeprom)
{
int ret;
struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
- struct tlvinfo_tlv *eeprom_tlv = to_entry(&eeprom[HDR_SIZE]);
+ struct tlvinfo_tlv *eeprom_tlv = to_entry(&eeprom[TLV_INFO_HEADER_SIZE]);
/* Read the header */
- ret = read_tlv_eeprom((void *)eeprom_hdr, 0, HDR_SIZE, devnum);
+ ret = read_tlv_eeprom((void *)eeprom_hdr, 0, TLV_INFO_HEADER_SIZE, devnum);
/* If the header was successfully read, read the TLVs */
if (ret == 0 && is_valid_tlvinfo_header(eeprom_hdr))
- ret = read_tlv_eeprom((void *)eeprom_tlv, HDR_SIZE,
+ ret = read_tlv_eeprom((void *)eeprom_tlv, TLV_INFO_HEADER_SIZE,
be16_to_cpu(eeprom_hdr->totallen), devnum);
// If the contents are invalid, start over with default contents
@@ -161,8 +156,8 @@ static void show_eeprom(int devnum, u8 *eeprom)
printf("TLV Name Code Len Value\n");
printf("-------------------- ---- --- -----\n");
- curr_tlv = HDR_SIZE;
- tlv_end = HDR_SIZE + be16_to_cpu(eeprom_hdr->totallen);
+ curr_tlv = TLV_INFO_HEADER_SIZE;
+ tlv_end = TLV_INFO_HEADER_SIZE + be16_to_cpu(eeprom_hdr->totallen);
while (curr_tlv < tlv_end) {
eeprom_tlv = to_entry(&eeprom[curr_tlv]);
if (!is_valid_tlvinfo_entry(eeprom_tlv)) {
@@ -171,7 +166,7 @@ static void show_eeprom(int devnum, u8 *eeprom)
return;
}
decode_tlv(eeprom_tlv);
- curr_tlv += ENT_SIZE + eeprom_tlv->length;
+ curr_tlv += TLV_INFO_ENTRY_SIZE + eeprom_tlv->length;
}
printf("Checksum is %s.\n",
@@ -339,10 +334,10 @@ void tlvinfo_update_crc(u8 *eeprom)
if (!tlvinfo_find_tlv(eeprom, TLV_CODE_CRC_32, &eeprom_index)) {
unsigned int totallen = be16_to_cpu(eeprom_hdr->totallen);
- if ((totallen + ENT_SIZE + 4) > TLV_TOTAL_LEN_MAX)
+ if ((totallen + TLV_INFO_ENTRY_SIZE + 4) > TLV_TOTAL_LEN_MAX)
return;
- eeprom_index = HDR_SIZE + totallen;
- eeprom_hdr->totallen = cpu_to_be16(totallen + ENT_SIZE + 4);
+ eeprom_index = TLV_INFO_HEADER_SIZE + totallen;
+ eeprom_hdr->totallen = cpu_to_be16(totallen + TLV_INFO_ENTRY_SIZE + 4);
}
eeprom_crc = to_entry(&eeprom[eeprom_index]);
eeprom_crc->type = TLV_CODE_CRC_32;
@@ -350,7 +345,7 @@ void tlvinfo_update_crc(u8 *eeprom)
// Calculate the checksum
calc_crc = crc32(0, (void *)eeprom,
- HDR_SIZE + be16_to_cpu(eeprom_hdr->totallen) - 4);
+ TLV_INFO_HEADER_SIZE + be16_to_cpu(eeprom_hdr->totallen) - 4);
eeprom_crc->value[0] = (calc_crc >> 24) & 0xFF;
eeprom_crc->value[1] = (calc_crc >> 16) & 0xFF;
eeprom_crc->value[2] = (calc_crc >> 8) & 0xFF;
@@ -370,7 +365,7 @@ int write_tlvinfo_tlv_eeprom(void *eeprom, int dev)
tlvinfo_update_crc(eeprom);
- eeprom_len = HDR_SIZE + be16_to_cpu(eeprom_hdr->totallen);
+ eeprom_len = TLV_INFO_HEADER_SIZE + be16_to_cpu(eeprom_hdr->totallen);
ret = write_tlv_eeprom(eeprom, eeprom_len, dev);
if (ret) {
printf("Programming failed.\n");
@@ -539,15 +534,15 @@ bool tlvinfo_find_tlv(u8 *eeprom, u8 tcode, int *eeprom_index)
// Search through the TLVs, looking for the first one which matches the
// supplied type code.
- *eeprom_index = HDR_SIZE;
- eeprom_end = HDR_SIZE + be16_to_cpu(eeprom_hdr->totallen);
+ *eeprom_index = TLV_INFO_HEADER_SIZE;
+ eeprom_end = TLV_INFO_HEADER_SIZE + be16_to_cpu(eeprom_hdr->totallen);
while (*eeprom_index < eeprom_end) {
eeprom_tlv = to_entry(&eeprom[*eeprom_index]);
if (!is_valid_tlvinfo_entry(eeprom_tlv))
return false;
if (eeprom_tlv->type == tcode)
return true;
- *eeprom_index += ENT_SIZE + eeprom_tlv->length;
+ *eeprom_index += TLV_INFO_ENTRY_SIZE + eeprom_tlv->length;
}
return(false);
}
@@ -558,7 +553,7 @@ bool tlvinfo_find_tlv(u8 *eeprom, u8 tcode, int *eeprom_index)
* This function deletes the TLV with the specified type code from the
* EEPROM.
*/
-static bool tlvinfo_delete_tlv(u8 *eeprom, u8 code)
+bool tlvinfo_delete_tlv(u8 *eeprom, u8 code)
{
int eeprom_index;
int tlength;
@@ -568,9 +563,9 @@ static bool tlvinfo_delete_tlv(u8 *eeprom, u8 code)
// Find the TLV and then move all following TLVs "forward"
if (tlvinfo_find_tlv(eeprom, code, &eeprom_index)) {
eeprom_tlv = to_entry(&eeprom[eeprom_index]);
- tlength = ENT_SIZE + eeprom_tlv->length;
+ tlength = TLV_INFO_ENTRY_SIZE + eeprom_tlv->length;
memcpy(&eeprom[eeprom_index], &eeprom[eeprom_index + tlength],
- HDR_SIZE +
+ TLV_INFO_HEADER_SIZE +
be16_to_cpu(eeprom_hdr->totallen) - eeprom_index -
tlength);
eeprom_hdr->totallen =
@@ -589,7 +584,7 @@ static bool tlvinfo_delete_tlv(u8 *eeprom, u8 code)
* the format in which it will be stored in the EEPROM.
*/
#define MAX_TLV_VALUE_LEN 256
-static bool tlvinfo_add_tlv(u8 *eeprom, int tcode, char *strval)
+bool tlvinfo_add_tlv(u8 *eeprom, int tcode, char *strval)
{
struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
struct tlvinfo_tlv *eeprom_tlv;
@@ -656,7 +651,7 @@ static bool tlvinfo_add_tlv(u8 *eeprom, int tcode, char *strval)
}
// Is there room for this TLV?
- if ((be16_to_cpu(eeprom_hdr->totallen) + ENT_SIZE + new_tlv_len) >
+ if ((be16_to_cpu(eeprom_hdr->totallen) + TLV_INFO_ENTRY_SIZE + new_tlv_len) >
TLV_TOTAL_LEN_MAX) {
printf("ERROR: There is not enough room in the EERPOM to save data.\n");
return false;
@@ -666,9 +661,9 @@ static bool tlvinfo_add_tlv(u8 *eeprom, int tcode, char *strval)
if (tlvinfo_find_tlv(eeprom, TLV_CODE_CRC_32, &eeprom_index))
eeprom_hdr->totallen =
cpu_to_be16(be16_to_cpu(eeprom_hdr->totallen) -
- ENT_SIZE - 4);
+ TLV_INFO_ENTRY_SIZE - 4);
else
- eeprom_index = HDR_SIZE + be16_to_cpu(eeprom_hdr->totallen);
+ eeprom_index = TLV_INFO_HEADER_SIZE + be16_to_cpu(eeprom_hdr->totallen);
eeprom_tlv = to_entry(&eeprom[eeprom_index]);
eeprom_tlv->type = tcode;
eeprom_tlv->length = new_tlv_len;
@@ -676,7 +671,7 @@ static bool tlvinfo_add_tlv(u8 *eeprom, int tcode, char *strval)
// Update the total length and calculate (add) a new CRC-32 TLV
eeprom_hdr->totallen = cpu_to_be16(be16_to_cpu(eeprom_hdr->totallen) +
- ENT_SIZE + new_tlv_len);
+ TLV_INFO_ENTRY_SIZE + new_tlv_len);
tlvinfo_update_crc(eeprom);
return true;
@@ -954,7 +949,7 @@ int read_tlvinfo_tlv_eeprom(void *eeprom, struct tlvinfo_header **hdr,
struct tlvinfo_tlv *tlv_ent;
/* Read TLV header */
- ret = read_tlv_eeprom(eeprom, 0, HDR_SIZE, dev_num);
+ ret = read_tlv_eeprom(eeprom, 0, TLV_INFO_HEADER_SIZE, dev_num);
if (ret < 0)
return ret;
@@ -964,7 +959,7 @@ int read_tlvinfo_tlv_eeprom(void *eeprom, struct tlvinfo_header **hdr,
/* Read TLV entries */
tlv_ent = to_entry(&tlv_hdr[1]);
- ret = read_tlv_eeprom(tlv_ent, HDR_SIZE,
+ ret = read_tlv_eeprom(tlv_ent, TLV_INFO_HEADER_SIZE,
be16_to_cpu(tlv_hdr->totallen), dev_num);
if (ret < 0)
return ret;
diff --git a/include/tlv_eeprom.h b/include/tlv_eeprom.h
index 55fd72d6d2..dc7952da6b 100644
--- a/include/tlv_eeprom.h
+++ b/include/tlv_eeprom.h
@@ -24,11 +24,11 @@ struct __attribute__ ((__packed__)) tlvinfo_header {
};
// Header Field Constants
+#define TLV_INFO_HEADER_SIZE sizeof(struct tlvinfo_header)
#define TLV_INFO_ID_STRING "TlvInfo"
#define TLV_INFO_VERSION 0x01
#define TLV_INFO_MAX_LEN 2048
-#define TLV_TOTAL_LEN_MAX (TLV_INFO_MAX_LEN - \
- sizeof(struct tlvinfo_header))
+#define TLV_TOTAL_LEN_MAX (TLV_INFO_MAX_LEN - TLV_INFO_HEADER_SIZE)
/*
* TlvInfo TLV: Layout of a TLV field
@@ -39,6 +39,7 @@ struct __attribute__ ((__packed__)) tlvinfo_tlv {
u8 value[0];
};
+#define TLV_INFO_ENTRY_SIZE sizeof(struct tlvinfo_tlv)
/* Maximum length of a TLV value in bytes */
#define TLV_VALUE_MAX_LEN 255
@@ -134,6 +135,30 @@ int write_tlvinfo_tlv_eeprom(void *eeprom, int dev);
*/
bool tlvinfo_find_tlv(u8 *eeprom, u8 tcode, int *eeprom_index);
+/**
+ * tlvinfo_add_tlv
+ *
+ * This function adds a TLV to the EEPROM, converting the value (a string) to
+ * the format in which it will be stored in the EEPROM.
+ * @eeprom: Pointer to buffer to hold the binary data. Must point to a buffer
+ * of size at least TLV_INFO_MAX_LEN.
+ * @code The TLV Code for the new entry.
+ * @eeprom_index success offset into EEPROM where the new entry has been stored
+ *
+ */
+bool tlvinfo_add_tlv(u8 *eeprom, int code, char *strval);
+
+/**
+ * tlvinfo_delete_tlv
+ *
+ * This function deletes the TLV with the specified type code from the
+ * EEPROM.
+ * @eeprom: Pointer to buffer to hold the binary data. Must point to a buffer
+ * of size at least TLV_INFO_MAX_LEN.
+ * @code The TLV Code of the entry to delete.
+ */
+bool tlvinfo_delete_tlv(u8 *eeprom, u8 code);
+
/**
* tlvinfo_update_crc
*
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 07/12] cmd: tlv_eeprom: hide access to static tlv_devices array behind accessor
2022-05-02 14:18 [PATCH 00/12] split tlv_eeprom command into a separate library Josua Mayer
` (5 preceding siblings ...)
2022-05-02 14:18 ` [PATCH 06/12] cmd: tlv_eeprom: move missing declarations and defines to header Josua Mayer
@ 2022-05-02 14:18 ` Josua Mayer
2022-05-02 14:18 ` [PATCH 08/12] cmd: tlv_eeprom: clean up two defines for one thing Josua Mayer
` (4 subsequent siblings)
11 siblings, 0 replies; 22+ messages in thread
From: Josua Mayer @ 2022-05-02 14:18 UTC (permalink / raw)
To: u-boot; +Cc: Josua Mayer, Sven Auhagen, Simon Glass, Stefan Roese
The tlv_eeprom command logic checks the static tlv_devices array to
validate the eeprom number. This array will be move to a separate tlv
library.
Hide this access behind a new function exists_tlv_eeprom.
Signed-off-by: Josua Mayer <josua@solid-run.com>
---
cmd/tlv_eeprom.c | 12 ++++++++++--
include/tlv_eeprom.h | 7 +++++++
2 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/cmd/tlv_eeprom.c b/cmd/tlv_eeprom.c
index 57468edb1c..f51a9666bf 100644
--- a/cmd/tlv_eeprom.c
+++ b/cmd/tlv_eeprom.c
@@ -44,6 +44,14 @@ static struct udevice *tlv_devices[MAX_TLV_DEVICES];
#define to_header(p) ((struct tlvinfo_header *)p)
#define to_entry(p) ((struct tlvinfo_tlv *)p)
+/**
+ * Check whether eeprom device exists.
+ */
+bool exists_tlv_eeprom(int dev)
+{
+ return dev < TLV_MAX_DEVICES && tlv_devices[dev] != 0;
+}
+
static inline bool is_digit(char c)
{
return (c >= '0' && c <= '9');
@@ -481,7 +489,7 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
unsigned int devnum;
devnum = simple_strtoul(argv[2], NULL, 0);
- if (devnum > MAX_TLV_DEVICES || !tlv_devices[devnum]) {
+ if (!exists_tlv_eeprom(devnum)) {
printf("Invalid device number\n");
return 0;
}
@@ -866,7 +874,7 @@ static void show_tlv_devices(int current_dev)
unsigned int dev;
for (dev = 0; dev < MAX_TLV_DEVICES; dev++)
- if (tlv_devices[dev])
+ if (exists_tlv_eeprom(dev))
printf("TLV: %u%s\n", dev,
(dev == current_dev) ? " (*)" : "");
}
diff --git a/include/tlv_eeprom.h b/include/tlv_eeprom.h
index dc7952da6b..c81c58837d 100644
--- a/include/tlv_eeprom.h
+++ b/include/tlv_eeprom.h
@@ -69,6 +69,13 @@ struct __attribute__ ((__packed__)) tlvinfo_tlv {
/* how many EEPROMs can be used */
#define TLV_MAX_DEVICES 2
+/**
+ * Check whether eeprom device exists.
+ *
+ * @dev: EEPROM device to check.
+ */
+bool exists_tlv_eeprom(int dev);
+
/**
* read_tlv_eeprom - Read the EEPROM binary data from the hardware
* @eeprom: Pointer to buffer to hold the binary data
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 08/12] cmd: tlv_eeprom: clean up two defines for one thing
2022-05-02 14:18 [PATCH 00/12] split tlv_eeprom command into a separate library Josua Mayer
` (6 preceding siblings ...)
2022-05-02 14:18 ` [PATCH 07/12] cmd: tlv_eeprom: hide access to static tlv_devices array behind accessor Josua Mayer
@ 2022-05-02 14:18 ` Josua Mayer
2022-05-02 14:18 ` [PATCH 09/12] cmd: tlv_eeprom: add my copyright Josua Mayer
` (3 subsequent siblings)
11 siblings, 0 replies; 22+ messages in thread
From: Josua Mayer @ 2022-05-02 14:18 UTC (permalink / raw)
To: u-boot; +Cc: Josua Mayer, Simon Glass, Stefan Roese, Sven Auhagen
MAX_TLV_DEVICES defined in C, and TLV_MAX_DEVICES defined in the header
serve the same purpose. Replace all occurences of the former by the
latter.
Signed-off-by: Josua Mayer <josua@solid-run.com>
---
cmd/tlv_eeprom.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/cmd/tlv_eeprom.c b/cmd/tlv_eeprom.c
index f51a9666bf..c110927cb5 100644
--- a/cmd/tlv_eeprom.c
+++ b/cmd/tlv_eeprom.c
@@ -25,8 +25,6 @@
DECLARE_GLOBAL_DATA_PTR;
-#define MAX_TLV_DEVICES 2
-
/* File scope function prototypes */
static int read_eeprom(int devnum, u8 *eeprom);
static void show_eeprom(int devnum, u8 *eeprom);
@@ -39,7 +37,7 @@ static void show_tlv_devices(int current_dev);
/* The EERPOM contents after being read into memory */
static u8 eeprom[TLV_INFO_MAX_LEN];
-static struct udevice *tlv_devices[MAX_TLV_DEVICES];
+static struct udevice *tlv_devices[TLV_MAX_DEVICES];
#define to_header(p) ((struct tlvinfo_header *)p)
#define to_entry(p) ((struct tlvinfo_tlv *)p)
@@ -873,7 +871,7 @@ static void show_tlv_devices(int current_dev)
{
unsigned int dev;
- for (dev = 0; dev < MAX_TLV_DEVICES; dev++)
+ for (dev = 0; dev < TLV_MAX_DEVICES; dev++)
if (exists_tlv_eeprom(dev))
printf("TLV: %u%s\n", dev,
(dev == current_dev) ? " (*)" : "");
@@ -890,7 +888,7 @@ static int find_tlv_devices(struct udevice **tlv_devices_p)
ret = uclass_next_device_check(&dev)) {
if (ret == 0)
tlv_devices_p[count_dev++] = dev;
- if (count_dev >= MAX_TLV_DEVICES)
+ if (count_dev >= TLV_MAX_DEVICES)
break;
}
@@ -899,7 +897,7 @@ static int find_tlv_devices(struct udevice **tlv_devices_p)
static struct udevice *find_tlv_device_by_index(int dev_num)
{
- struct udevice *local_tlv_devices[MAX_TLV_DEVICES] = {};
+ struct udevice *local_tlv_devices[TLV_MAX_DEVICES] = {};
struct udevice **tlv_devices_p;
int ret;
@@ -926,7 +924,7 @@ int read_tlv_eeprom(void *eeprom, int offset, int len, int dev_num)
{
struct udevice *dev;
- if (dev_num >= MAX_TLV_DEVICES)
+ if (dev_num >= TLV_MAX_DEVICES)
return -EINVAL;
dev = find_tlv_device_by_index(dev_num);
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 09/12] cmd: tlv_eeprom: add my copyright
2022-05-02 14:18 [PATCH 00/12] split tlv_eeprom command into a separate library Josua Mayer
` (7 preceding siblings ...)
2022-05-02 14:18 ` [PATCH 08/12] cmd: tlv_eeprom: clean up two defines for one thing Josua Mayer
@ 2022-05-02 14:18 ` Josua Mayer
2022-05-02 14:18 ` [PATCH 10/12] cmd: tlv_eeprom: split off tlv library from command Josua Mayer
` (2 subsequent siblings)
11 siblings, 0 replies; 22+ messages in thread
From: Josua Mayer @ 2022-05-02 14:18 UTC (permalink / raw)
To: u-boot; +Cc: Josua Mayer, Simon Glass, Sven Auhagen, Stefan Roese
While each of the previous individual changes is small, accumulated they
amount to a substantial effort. Explicitly add a Copyright declaration.
Signed-off-by: Josua Mayer <josua@solid-run.com>
---
cmd/tlv_eeprom.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/cmd/tlv_eeprom.c b/cmd/tlv_eeprom.c
index c110927cb5..c66116b2c4 100644
--- a/cmd/tlv_eeprom.c
+++ b/cmd/tlv_eeprom.c
@@ -7,6 +7,7 @@
* Copyright (C) 2014 Srideep <srideep_devireddy@dell.com>
* Copyright (C) 2013 Miles Tseng <miles_tseng@accton.com>
* Copyright (C) 2014,2016 david_yang <david_yang@accton.com>
+ * Copyright (C) 2022 Josua Mayer <josua@solid-run.com>
*/
#include <common.h>
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 10/12] cmd: tlv_eeprom: split off tlv library from command
2022-05-02 14:18 [PATCH 00/12] split tlv_eeprom command into a separate library Josua Mayer
` (8 preceding siblings ...)
2022-05-02 14:18 ` [PATCH 09/12] cmd: tlv_eeprom: add my copyright Josua Mayer
@ 2022-05-02 14:18 ` Josua Mayer
2022-05-02 14:18 ` [PATCH 11/12] arm: mvebu: clearfog: enable tlv library for spl in favour of eeprom cmd Josua Mayer
2022-05-02 14:18 ` [PATCH 12/12] lib: tlv_eeprom: add function for reading one entry into a C string Josua Mayer
11 siblings, 0 replies; 22+ messages in thread
From: Josua Mayer @ 2022-05-02 14:18 UTC (permalink / raw)
To: u-boot
Cc: Josua Mayer, Simon Glass, Michal Simek, Kory Maincent,
Ovidiu Panait, Ashok Reddy Soma, Stefan Roese, Sven Auhagen,
Heinrich Schuchardt, Alexandru Gagniuc, Philippe Reynes, Bin Meng,
Eugen Hristev, Masahisa Kojima, Pali Rohár, Loic Poulain,
Patrick Delaunay
The eeprom command includes functions for reading and writing
tlv-formatted data from an eeprom, as well as an implementation of the
cli command tlv_eeprom.
Split off the parsing, read and write into a standalone tlv library.
Signed-off-by: Josua Mayer <josua@solid-run.com>
---
cmd/Kconfig | 2 +
cmd/tlv_eeprom.c | 742 +-----------------------------------------
lib/Kconfig | 2 +
lib/Makefile | 2 +
lib/tlv/Kconfig | 15 +
lib/tlv/Makefile | 5 +
lib/tlv/tlv_eeprom.c | 750 +++++++++++++++++++++++++++++++++++++++++++
7 files changed, 786 insertions(+), 732 deletions(-)
create mode 100644 lib/tlv/Kconfig
create mode 100644 lib/tlv/Makefile
create mode 100644 lib/tlv/tlv_eeprom.c
diff --git a/cmd/Kconfig b/cmd/Kconfig
index 2b575a2b42..821b5e9d6b 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -166,6 +166,7 @@ config CMD_REGINFO
config CMD_TLV_EEPROM
bool "tlv_eeprom"
+ select EEPROM_TLV_LIB
depends on I2C_EEPROM
help
Display and program the system EEPROM data block in ONIE Tlvinfo
@@ -173,6 +174,7 @@ config CMD_TLV_EEPROM
config SPL_CMD_TLV_EEPROM
bool "tlv_eeprom for SPL"
+ select SPL_EEPROM_TLV_LIB
depends on SPL_I2C_EEPROM
select SPL_DRIVERS_MISC
help
diff --git a/cmd/tlv_eeprom.c b/cmd/tlv_eeprom.c
index c66116b2c4..99b79cad8b 100644
--- a/cmd/tlv_eeprom.c
+++ b/cmd/tlv_eeprom.c
@@ -10,131 +10,26 @@
* Copyright (C) 2022 Josua Mayer <josua@solid-run.com>
*/
-#include <common.h>
#include <command.h>
-#include <dm.h>
-#include <i2c.h>
-#include <i2c_eeprom.h>
-#include <env.h>
-#include <init.h>
-#include <net.h>
-#include <asm/global_data.h>
-#include <linux/ctype.h>
-#include <u-boot/crc.h>
-
-#include "tlv_eeprom.h"
-
-DECLARE_GLOBAL_DATA_PTR;
+#include <linux/kernel.h>
+#include <stdio.h>
+#include <tlv_eeprom.h>
+#include <vsprintf.h>
/* File scope function prototypes */
-static int read_eeprom(int devnum, u8 *eeprom);
static void show_eeprom(int devnum, u8 *eeprom);
-static void decode_tlv(struct tlvinfo_tlv *tlv);
-static int set_mac(char *buf, const char *string);
-static int set_date(char *buf, const char *string);
-static int set_bytes(char *buf, const char *string, int *converted_accum);
static void show_tlv_devices(int current_dev);
+static inline const char *tlv_type2name(u8 type);
+static void decode_tlv(struct tlvinfo_tlv *tlv);
+static int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
+static void show_tlv_code_list(void);
/* The EERPOM contents after being read into memory */
static u8 eeprom[TLV_INFO_MAX_LEN];
-static struct udevice *tlv_devices[TLV_MAX_DEVICES];
-
#define to_header(p) ((struct tlvinfo_header *)p)
#define to_entry(p) ((struct tlvinfo_tlv *)p)
-/**
- * Check whether eeprom device exists.
- */
-bool exists_tlv_eeprom(int dev)
-{
- return dev < TLV_MAX_DEVICES && tlv_devices[dev] != 0;
-}
-
-static inline bool is_digit(char c)
-{
- return (c >= '0' && c <= '9');
-}
-
-/**
- * is_hex
- *
- * Tests if character is an ASCII hex digit
- */
-static inline u8 is_hex(char p)
-{
- return (((p >= '0') && (p <= '9')) ||
- ((p >= 'A') && (p <= 'F')) ||
- ((p >= 'a') && (p <= 'f')));
-}
-
-/**
- * Validate the checksum in the provided TlvInfo EEPROM data. First,
- * verify that the TlvInfo header is valid, then make sure the last
- * TLV is a CRC-32 TLV. Then calculate the CRC over the EEPROM data
- * and compare it to the value stored in the EEPROM CRC-32 TLV.
- */
-bool tlvinfo_check_crc(u8 *eeprom)
-{
- struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
- struct tlvinfo_tlv *eeprom_crc;
- unsigned int calc_crc;
- unsigned int stored_crc;
-
- // Is the eeprom header valid?
- if (!is_valid_tlvinfo_header(eeprom_hdr))
- return false;
-
- // Is the last TLV a CRC?
- eeprom_crc = to_entry(&eeprom[TLV_INFO_HEADER_SIZE +
- be16_to_cpu(eeprom_hdr->totallen) - (TLV_INFO_ENTRY_SIZE + 4)]);
- if (eeprom_crc->type != TLV_CODE_CRC_32 || eeprom_crc->length != 4)
- return false;
-
- // Calculate the checksum
- calc_crc = crc32(0, (void *)eeprom,
- TLV_INFO_HEADER_SIZE + be16_to_cpu(eeprom_hdr->totallen) - 4);
- stored_crc = (eeprom_crc->value[0] << 24) |
- (eeprom_crc->value[1] << 16) |
- (eeprom_crc->value[2] << 8) |
- eeprom_crc->value[3];
- return calc_crc == stored_crc;
-}
-
-/**
- * read_eeprom
- *
- * Read the EEPROM into memory, if it hasn't already been read.
- */
-static int read_eeprom(int devnum, u8 *eeprom)
-{
- int ret;
- struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
- struct tlvinfo_tlv *eeprom_tlv = to_entry(&eeprom[TLV_INFO_HEADER_SIZE]);
-
- /* Read the header */
- ret = read_tlv_eeprom((void *)eeprom_hdr, 0, TLV_INFO_HEADER_SIZE, devnum);
- /* If the header was successfully read, read the TLVs */
- if (ret == 0 && is_valid_tlvinfo_header(eeprom_hdr))
- ret = read_tlv_eeprom((void *)eeprom_tlv, TLV_INFO_HEADER_SIZE,
- be16_to_cpu(eeprom_hdr->totallen), devnum);
-
- // If the contents are invalid, start over with default contents
- if (!is_valid_tlvinfo_header(eeprom_hdr) ||
- !tlvinfo_check_crc(eeprom)) {
- strcpy(eeprom_hdr->signature, TLV_INFO_ID_STRING);
- eeprom_hdr->version = TLV_INFO_VERSION;
- eeprom_hdr->totallen = cpu_to_be16(0);
- tlvinfo_update_crc(eeprom);
- }
-
-#ifdef DEBUG
- show_eeprom(devnum, eeprom);
-#endif
-
- return ret;
-}
-
/**
* show_eeprom
*
@@ -323,70 +218,10 @@ static void decode_tlv(struct tlvinfo_tlv *tlv)
printf("%-20s 0x%02X %3d %s\n", name, tlv->type, tlv->length, value);
}
-/**
- * tlvinfo_update_crc
- *
- * This function updates the CRC-32 TLV. If there is no CRC-32 TLV, then
- * one is added. This function should be called after each update to the
- * EEPROM structure, to make sure the CRC is always correct.
- */
-void tlvinfo_update_crc(u8 *eeprom)
-{
- struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
- struct tlvinfo_tlv *eeprom_crc;
- unsigned int calc_crc;
- int eeprom_index;
-
- // Discover the CRC TLV
- if (!tlvinfo_find_tlv(eeprom, TLV_CODE_CRC_32, &eeprom_index)) {
- unsigned int totallen = be16_to_cpu(eeprom_hdr->totallen);
-
- if ((totallen + TLV_INFO_ENTRY_SIZE + 4) > TLV_TOTAL_LEN_MAX)
- return;
- eeprom_index = TLV_INFO_HEADER_SIZE + totallen;
- eeprom_hdr->totallen = cpu_to_be16(totallen + TLV_INFO_ENTRY_SIZE + 4);
- }
- eeprom_crc = to_entry(&eeprom[eeprom_index]);
- eeprom_crc->type = TLV_CODE_CRC_32;
- eeprom_crc->length = 4;
-
- // Calculate the checksum
- calc_crc = crc32(0, (void *)eeprom,
- TLV_INFO_HEADER_SIZE + be16_to_cpu(eeprom_hdr->totallen) - 4);
- eeprom_crc->value[0] = (calc_crc >> 24) & 0xFF;
- eeprom_crc->value[1] = (calc_crc >> 16) & 0xFF;
- eeprom_crc->value[2] = (calc_crc >> 8) & 0xFF;
- eeprom_crc->value[3] = (calc_crc >> 0) & 0xFF;
-}
-
-/**
- * write_tlvinfo_tlv_eeprom
- *
- * Write the TLV data from CPU memory to the hardware.
- */
-int write_tlvinfo_tlv_eeprom(void *eeprom, int dev)
-{
- int ret = 0;
- struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
- int eeprom_len;
-
- tlvinfo_update_crc(eeprom);
-
- eeprom_len = TLV_INFO_HEADER_SIZE + be16_to_cpu(eeprom_hdr->totallen);
- ret = write_tlv_eeprom(eeprom, eeprom_len, dev);
- if (ret) {
- printf("Programming failed.\n");
- return -1;
- }
-
- printf("Programming passed.\n");
- return 0;
-}
-
/**
* show_tlv_code_list - Display the list of TLV codes and names
*/
-void show_tlv_code_list(void)
+static void show_tlv_code_list(void)
{
int i;
@@ -404,7 +239,7 @@ void show_tlv_code_list(void)
*
* This function implements the tlv_eeprom command.
*/
-int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
+static int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
{
char cmd;
struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
@@ -526,348 +361,6 @@ U_BOOT_CMD(tlv_eeprom, 4, 1, do_tlv_eeprom,
" - List the understood TLV codes and names.\n"
);
-/**
- * tlvinfo_find_tlv
- *
- * This function finds the TLV with the supplied code in the EERPOM.
- * An offset from the beginning of the EEPROM is returned in the
- * eeprom_index parameter if the TLV is found.
- */
-bool tlvinfo_find_tlv(u8 *eeprom, u8 tcode, int *eeprom_index)
-{
- struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
- struct tlvinfo_tlv *eeprom_tlv;
- int eeprom_end;
-
- // Search through the TLVs, looking for the first one which matches the
- // supplied type code.
- *eeprom_index = TLV_INFO_HEADER_SIZE;
- eeprom_end = TLV_INFO_HEADER_SIZE + be16_to_cpu(eeprom_hdr->totallen);
- while (*eeprom_index < eeprom_end) {
- eeprom_tlv = to_entry(&eeprom[*eeprom_index]);
- if (!is_valid_tlvinfo_entry(eeprom_tlv))
- return false;
- if (eeprom_tlv->type == tcode)
- return true;
- *eeprom_index += TLV_INFO_ENTRY_SIZE + eeprom_tlv->length;
- }
- return(false);
-}
-
-/**
- * tlvinfo_delete_tlv
- *
- * This function deletes the TLV with the specified type code from the
- * EEPROM.
- */
-bool tlvinfo_delete_tlv(u8 *eeprom, u8 code)
-{
- int eeprom_index;
- int tlength;
- struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
- struct tlvinfo_tlv *eeprom_tlv;
-
- // Find the TLV and then move all following TLVs "forward"
- if (tlvinfo_find_tlv(eeprom, code, &eeprom_index)) {
- eeprom_tlv = to_entry(&eeprom[eeprom_index]);
- tlength = TLV_INFO_ENTRY_SIZE + eeprom_tlv->length;
- memcpy(&eeprom[eeprom_index], &eeprom[eeprom_index + tlength],
- TLV_INFO_HEADER_SIZE +
- be16_to_cpu(eeprom_hdr->totallen) - eeprom_index -
- tlength);
- eeprom_hdr->totallen =
- cpu_to_be16(be16_to_cpu(eeprom_hdr->totallen) -
- tlength);
- tlvinfo_update_crc(eeprom);
- return true;
- }
- return false;
-}
-
-/**
- * tlvinfo_add_tlv
- *
- * This function adds a TLV to the EEPROM, converting the value (a string) to
- * the format in which it will be stored in the EEPROM.
- */
-#define MAX_TLV_VALUE_LEN 256
-bool tlvinfo_add_tlv(u8 *eeprom, int tcode, char *strval)
-{
- struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
- struct tlvinfo_tlv *eeprom_tlv;
- int new_tlv_len = 0;
- u32 value;
- char data[MAX_TLV_VALUE_LEN];
- int eeprom_index;
-
- // Encode each TLV type into the format to be stored in the EERPOM
- switch (tcode) {
- case TLV_CODE_PRODUCT_NAME:
- case TLV_CODE_PART_NUMBER:
- case TLV_CODE_SERIAL_NUMBER:
- case TLV_CODE_LABEL_REVISION:
- case TLV_CODE_PLATFORM_NAME:
- case TLV_CODE_ONIE_VERSION:
- case TLV_CODE_MANUF_NAME:
- case TLV_CODE_MANUF_COUNTRY:
- case TLV_CODE_VENDOR_NAME:
- case TLV_CODE_DIAG_VERSION:
- case TLV_CODE_SERVICE_TAG:
- strncpy(data, strval, MAX_TLV_VALUE_LEN);
- new_tlv_len = min_t(size_t, MAX_TLV_VALUE_LEN, strlen(strval));
- break;
- case TLV_CODE_DEVICE_VERSION:
- value = simple_strtoul(strval, NULL, 0);
- if (value >= 256) {
- printf("ERROR: Device version must be 255 or less. Value supplied: %u",
- value);
- return false;
- }
- data[0] = value & 0xFF;
- new_tlv_len = 1;
- break;
- case TLV_CODE_MAC_SIZE:
- value = simple_strtoul(strval, NULL, 0);
- if (value >= 65536) {
- printf("ERROR: MAC Size must be 65535 or less. Value supplied: %u",
- value);
- return false;
- }
- data[0] = (value >> 8) & 0xFF;
- data[1] = value & 0xFF;
- new_tlv_len = 2;
- break;
- case TLV_CODE_MANUF_DATE:
- if (set_date(data, strval) != 0)
- return false;
- new_tlv_len = 19;
- break;
- case TLV_CODE_MAC_BASE:
- if (set_mac(data, strval) != 0)
- return false;
- new_tlv_len = 6;
- break;
- case TLV_CODE_CRC_32:
- printf("WARNING: The CRC TLV is set automatically and cannot be set manually.\n");
- return false;
- case TLV_CODE_VENDOR_EXT:
- default:
- if (set_bytes(data, strval, &new_tlv_len) != 0)
- return false;
- break;
- }
-
- // Is there room for this TLV?
- if ((be16_to_cpu(eeprom_hdr->totallen) + TLV_INFO_ENTRY_SIZE + new_tlv_len) >
- TLV_TOTAL_LEN_MAX) {
- printf("ERROR: There is not enough room in the EERPOM to save data.\n");
- return false;
- }
-
- // Add TLV at the end, overwriting CRC TLV if it exists
- if (tlvinfo_find_tlv(eeprom, TLV_CODE_CRC_32, &eeprom_index))
- eeprom_hdr->totallen =
- cpu_to_be16(be16_to_cpu(eeprom_hdr->totallen) -
- TLV_INFO_ENTRY_SIZE - 4);
- else
- eeprom_index = TLV_INFO_HEADER_SIZE + be16_to_cpu(eeprom_hdr->totallen);
- eeprom_tlv = to_entry(&eeprom[eeprom_index]);
- eeprom_tlv->type = tcode;
- eeprom_tlv->length = new_tlv_len;
- memcpy(eeprom_tlv->value, data, new_tlv_len);
-
- // Update the total length and calculate (add) a new CRC-32 TLV
- eeprom_hdr->totallen = cpu_to_be16(be16_to_cpu(eeprom_hdr->totallen) +
- TLV_INFO_ENTRY_SIZE + new_tlv_len);
- tlvinfo_update_crc(eeprom);
-
- return true;
-}
-
-/**
- * set_mac
- *
- * Converts a string MAC address into a binary buffer.
- *
- * This function takes a pointer to a MAC address string
- * (i.e."XX:XX:XX:XX:XX:XX", where "XX" is a two-digit hex number).
- * The string format is verified and then converted to binary and
- * stored in a buffer.
- */
-static int set_mac(char *buf, const char *string)
-{
- char *p = (char *)string;
- int i;
- int err = 0;
- char *end;
-
- if (!p) {
- printf("ERROR: NULL mac addr string passed in.\n");
- return -1;
- }
-
- if (strlen(p) != 17) {
- printf("ERROR: MAC address strlen() != 17 -- %zu\n", strlen(p));
- printf("ERROR: Bad MAC address format: %s\n", string);
- return -1;
- }
-
- for (i = 0; i < 17; i++) {
- if ((i % 3) == 2) {
- if (p[i] != ':') {
- err++;
- printf("ERROR: mac: p[%i] != :, found: `%c'\n",
- i, p[i]);
- break;
- }
- continue;
- } else if (!is_hex(p[i])) {
- err++;
- printf("ERROR: mac: p[%i] != hex digit, found: `%c'\n",
- i, p[i]);
- break;
- }
- }
-
- if (err != 0) {
- printf("ERROR: Bad MAC address format: %s\n", string);
- return -1;
- }
-
- /* Convert string to binary */
- for (i = 0, p = (char *)string; i < 6; i++) {
- buf[i] = p ? hextoul(p, &end) : 0;
- if (p)
- p = (*end) ? end + 1 : end;
- }
-
- if (!is_valid_ethaddr((u8 *)buf)) {
- printf("ERROR: MAC address must not be 00:00:00:00:00:00, a multicast address or FF:FF:FF:FF:FF:FF.\n");
- printf("ERROR: Bad MAC address format: %s\n", string);
- return -1;
- }
-
- return 0;
-}
-
-/**
- * set_date
- *
- * Validates the format of the data string
- *
- * This function takes a pointer to a date string (i.e. MM/DD/YYYY hh:mm:ss)
- * and validates that the format is correct. If so the string is copied
- * to the supplied buffer.
- */
-static int set_date(char *buf, const char *string)
-{
- int i;
-
- if (!string) {
- printf("ERROR: NULL date string passed in.\n");
- return -1;
- }
-
- if (strlen(string) != 19) {
- printf("ERROR: Date strlen() != 19 -- %zu\n", strlen(string));
- printf("ERROR: Bad date format (MM/DD/YYYY hh:mm:ss): %s\n",
- string);
- return -1;
- }
-
- for (i = 0; string[i] != 0; i++) {
- switch (i) {
- case 2:
- case 5:
- if (string[i] != '/') {
- printf("ERROR: Bad date format (MM/DD/YYYY hh:mm:ss): %s\n",
- string);
- return -1;
- }
- break;
- case 10:
- if (string[i] != ' ') {
- printf("ERROR: Bad date format (MM/DD/YYYY hh:mm:ss): %s\n",
- string);
- return -1;
- }
- break;
- case 13:
- case 16:
- if (string[i] != ':') {
- printf("ERROR: Bad date format (MM/DD/YYYY hh:mm:ss): %s\n",
- string);
- return -1;
- }
- break;
- default:
- if (!is_digit(string[i])) {
- printf("ERROR: Bad date format (MM/DD/YYYY hh:mm:ss): %s\n",
- string);
- return -1;
- }
- break;
- }
- }
-
- strcpy(buf, string);
- return 0;
-}
-
-/**
- * set_bytes
- *
- * Converts a space-separated string of decimal numbers into a
- * buffer of bytes.
- *
- * This function takes a pointer to a space-separated string of decimal
- * numbers (i.e. "128 0x55 0321") with "C" standard radix specifiers
- * and converts them to an array of bytes.
- */
-static int set_bytes(char *buf, const char *string, int *converted_accum)
-{
- char *p = (char *)string;
- int i;
- uint byte;
-
- if (!p) {
- printf("ERROR: NULL string passed in.\n");
- return -1;
- }
-
- /* Convert string to bytes */
- for (i = 0, p = (char *)string; (i < TLV_VALUE_MAX_LEN) && (*p != 0);
- i++) {
- while ((*p == ' ') || (*p == '\t') || (*p == ',') ||
- (*p == ';')) {
- p++;
- }
- if (*p != 0) {
- if (!is_digit(*p)) {
- printf("ERROR: Non-digit found in byte string: (%s)\n",
- string);
- return -1;
- }
- byte = simple_strtoul(p, &p, 0);
- if (byte >= 256) {
- printf("ERROR: The value specified is greater than 255: (%u) in string: %s\n",
- byte, string);
- return -1;
- }
- buf[i] = byte & 0xFF;
- }
- }
-
- if (i == TLV_VALUE_MAX_LEN && (*p != 0)) {
- printf("ERROR: Trying to assign too many bytes (max: %d) in string: %s\n",
- TLV_VALUE_MAX_LEN, string);
- return -1;
- }
-
- *converted_accum = i;
- return 0;
-}
-
static void show_tlv_devices(int current_dev)
{
unsigned int dev;
@@ -877,218 +370,3 @@ static void show_tlv_devices(int current_dev)
printf("TLV: %u%s\n", dev,
(dev == current_dev) ? " (*)" : "");
}
-
-static int find_tlv_devices(struct udevice **tlv_devices_p)
-{
- int ret;
- int count_dev = 0;
- struct udevice *dev;
-
- for (ret = uclass_first_device_check(UCLASS_I2C_EEPROM, &dev);
- dev;
- ret = uclass_next_device_check(&dev)) {
- if (ret == 0)
- tlv_devices_p[count_dev++] = dev;
- if (count_dev >= TLV_MAX_DEVICES)
- break;
- }
-
- return (count_dev == 0) ? -ENODEV : 0;
-}
-
-static struct udevice *find_tlv_device_by_index(int dev_num)
-{
- struct udevice *local_tlv_devices[TLV_MAX_DEVICES] = {};
- struct udevice **tlv_devices_p;
- int ret;
-
- if (gd->flags & (GD_FLG_RELOC | GD_FLG_SPL_INIT)) {
- /* Assume BSS is initialized; use static data */
- if (tlv_devices[dev_num])
- return tlv_devices[dev_num];
- tlv_devices_p = tlv_devices;
- } else {
- tlv_devices_p = local_tlv_devices;
- }
-
- ret = find_tlv_devices(tlv_devices_p);
- if (ret == 0 && tlv_devices_p[dev_num])
- return tlv_devices_p[dev_num];
-
- return NULL;
-}
-
-/**
- * read_tlv_eeprom - read the hwinfo from i2c EEPROM
- */
-int read_tlv_eeprom(void *eeprom, int offset, int len, int dev_num)
-{
- struct udevice *dev;
-
- if (dev_num >= TLV_MAX_DEVICES)
- return -EINVAL;
-
- dev = find_tlv_device_by_index(dev_num);
- if (!dev)
- return -ENODEV;
-
- return i2c_eeprom_read(dev, offset, eeprom, len);
-}
-
-/**
- * write_tlv_eeprom - write the hwinfo to i2c EEPROM
- */
-int write_tlv_eeprom(void *eeprom, int len, int dev)
-{
- if (!(gd->flags & GD_FLG_RELOC))
- return -ENODEV;
- if (!tlv_devices[dev])
- return -ENODEV;
-
- return i2c_eeprom_write(tlv_devices[dev], 0, eeprom, len);
-}
-
-int read_tlvinfo_tlv_eeprom(void *eeprom, struct tlvinfo_header **hdr,
- struct tlvinfo_tlv **first_entry, int dev_num)
-{
- int ret;
- struct tlvinfo_header *tlv_hdr;
- struct tlvinfo_tlv *tlv_ent;
-
- /* Read TLV header */
- ret = read_tlv_eeprom(eeprom, 0, TLV_INFO_HEADER_SIZE, dev_num);
- if (ret < 0)
- return ret;
-
- tlv_hdr = eeprom;
- if (!is_valid_tlvinfo_header(tlv_hdr))
- return -EINVAL;
-
- /* Read TLV entries */
- tlv_ent = to_entry(&tlv_hdr[1]);
- ret = read_tlv_eeprom(tlv_ent, TLV_INFO_HEADER_SIZE,
- be16_to_cpu(tlv_hdr->totallen), dev_num);
- if (ret < 0)
- return ret;
- if (!tlvinfo_check_crc(eeprom))
- return -EINVAL;
-
- *hdr = tlv_hdr;
- *first_entry = tlv_ent;
-
- return 0;
-}
-
-/**
- * mac_read_from_eeprom
- *
- * Read the MAC addresses from EEPROM
- *
- * This function reads the MAC addresses from EEPROM and sets the
- * appropriate environment variables for each one read.
- *
- * The environment variables are only set if they haven't been set already.
- * This ensures that any user-saved variables are never overwritten.
- *
- * This function must be called after relocation.
- */
-int mac_read_from_eeprom(void)
-{
- unsigned int i;
- int eeprom_index;
- struct tlvinfo_tlv *eeprom_tlv;
- int maccount;
- u8 macbase[6];
- struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
- int devnum = 0; // TODO: support multiple EEPROMs
-
- puts("EEPROM: ");
-
- if (read_eeprom(devnum, eeprom)) {
- printf("Read failed.\n");
- return -1;
- }
-
- maccount = 1;
- if (tlvinfo_find_tlv(eeprom, TLV_CODE_MAC_SIZE, &eeprom_index)) {
- eeprom_tlv = to_entry(&eeprom[eeprom_index]);
- maccount = (eeprom_tlv->value[0] << 8) | eeprom_tlv->value[1];
- }
-
- memcpy(macbase, "\0\0\0\0\0\0", 6);
- if (tlvinfo_find_tlv(eeprom, TLV_CODE_MAC_BASE, &eeprom_index)) {
- eeprom_tlv = to_entry(&eeprom[eeprom_index]);
- memcpy(macbase, eeprom_tlv->value, 6);
- }
-
- for (i = 0; i < maccount; i++) {
- if (is_valid_ethaddr(macbase)) {
- char ethaddr[18];
- char enetvar[11];
-
- sprintf(ethaddr, "%02X:%02X:%02X:%02X:%02X:%02X",
- macbase[0], macbase[1], macbase[2],
- macbase[3], macbase[4], macbase[5]);
- sprintf(enetvar, i ? "eth%daddr" : "ethaddr", i);
- /* Only initialize environment variables that are blank
- * (i.e. have not yet been set)
- */
- if (!env_get(enetvar))
- env_set(enetvar, ethaddr);
-
- macbase[5]++;
- if (macbase[5] == 0) {
- macbase[4]++;
- if (macbase[4] == 0) {
- macbase[3]++;
- if (macbase[3] == 0) {
- macbase[0] = 0;
- macbase[1] = 0;
- macbase[2] = 0;
- }
- }
- }
- }
- }
-
- printf("%s v%u len=%u\n", eeprom_hdr->signature, eeprom_hdr->version,
- be16_to_cpu(eeprom_hdr->totallen));
-
- return 0;
-}
-
-/**
- * populate_serial_number - read the serial number from EEPROM
- *
- * This function reads the serial number from the EEPROM and sets the
- * appropriate environment variable.
- *
- * The environment variable is only set if it has not been set
- * already. This ensures that any user-saved variables are never
- * overwritten.
- *
- * This function must be called after relocation.
- */
-int populate_serial_number(int devnum)
-{
- char serialstr[257];
- int eeprom_index;
- struct tlvinfo_tlv *eeprom_tlv;
-
- if (env_get("serial#"))
- return 0;
-
- if (read_eeprom(devnum, eeprom)) {
- printf("Read failed.\n");
- return -1;
- }
-
- if (tlvinfo_find_tlv(eeprom, TLV_CODE_SERIAL_NUMBER, &eeprom_index)) {
- eeprom_tlv = to_entry(&eeprom[eeprom_index]);
- memcpy(serialstr, eeprom_tlv->value, eeprom_tlv->length);
- serialstr[eeprom_tlv->length] = 0;
- env_set("serial#", serialstr);
- }
-
- return 0;
-}
diff --git a/lib/Kconfig b/lib/Kconfig
index 858be14f09..4497efbbf4 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -909,3 +909,5 @@ config PHANDLE_CHECK_SEQ
phandles in fdtdec_get_alias_seq() function.
endmenu
+
+source lib/tlv/Kconfig
diff --git a/lib/Makefile b/lib/Makefile
index d9b1811f75..1bc6dad8ac 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -91,6 +91,8 @@ obj-$(CONFIG_LIBAVB) += libavb/
obj-$(CONFIG_$(SPL_TPL_)OF_LIBFDT) += libfdt/
obj-$(CONFIG_$(SPL_TPL_)OF_REAL) += fdtdec_common.o fdtdec.o
+obj-$(CONFIG_$(SPL_)EEPROM_TLV_LIB) += tlv/
+
ifdef CONFIG_SPL_BUILD
obj-$(CONFIG_SPL_YMODEM_SUPPORT) += crc16-ccitt.o
obj-$(CONFIG_$(SPL_TPL_)HASH) += crc16-ccitt.o
diff --git a/lib/tlv/Kconfig b/lib/tlv/Kconfig
new file mode 100644
index 0000000000..b3912ada78
--- /dev/null
+++ b/lib/tlv/Kconfig
@@ -0,0 +1,15 @@
+config EEPROM_TLV_LIB
+ bool "Enable EEPROM TLV library"
+ depends on I2C_EEPROM
+ help
+ Selecting this option will enable the shared EEPROM TLV library code.
+ It provides functions for reading, writing and parsing of
+ TLV-encoded data from EEPROMs.
+
+config SPL_EEPROM_TLV_LIB
+ bool "Enable EEPROM TLV library for SPL"
+ depends on SPL_I2C_EEPROM
+ help
+ Selecting this option will enable the shared EEPROM TLV library code.
+ It provides functions for reading, writing and parsing of
+ TLV-encoded data from EEPROMs.
diff --git a/lib/tlv/Makefile b/lib/tlv/Makefile
new file mode 100644
index 0000000000..8e96752e75
--- /dev/null
+++ b/lib/tlv/Makefile
@@ -0,0 +1,5 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# (C) Copyright 2017 Linaro
+
+obj-$(CONFIG_EEPROM_TLV_LIB) += tlv_eeprom.o
diff --git a/lib/tlv/tlv_eeprom.c b/lib/tlv/tlv_eeprom.c
new file mode 100644
index 0000000000..464f0aa1fa
--- /dev/null
+++ b/lib/tlv/tlv_eeprom.c
@@ -0,0 +1,750 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * Copyright (C) 2013 Curt Brune <curt@cumulusnetworks.com>
+ * Copyright (C) 2014 Srideep <srideep_devireddy@dell.com>
+ * Copyright (C) 2013 Miles Tseng <miles_tseng@accton.com>
+ * Copyright (C) 2014,2016 david_yang <david_yang@accton.com>
+ * Copyright (C) 2022 Josua Mayer <josua@solid-run.com>
+ */
+
+#include <common.h>
+#include <command.h>
+#include <dm.h>
+#include <i2c.h>
+#include <i2c_eeprom.h>
+#include <env.h>
+#include <init.h>
+#include <net.h>
+#include <asm/global_data.h>
+#include <linux/ctype.h>
+#include <u-boot/crc.h>
+
+#include "tlv_eeprom.h"
+
+DECLARE_GLOBAL_DATA_PTR;
+
+/* File scope function prototypes */
+static int read_eeprom(int devnum, u8 *eeprom);
+static int set_mac(char *buf, const char *string);
+static int set_date(char *buf, const char *string);
+static int set_bytes(char *buf, const char *string, int *converted_accum);
+
+/* The EERPOM contents after being read into memory */
+static u8 eeprom[TLV_INFO_MAX_LEN];
+
+static struct udevice *tlv_devices[TLV_MAX_DEVICES];
+
+#define to_header(p) ((struct tlvinfo_header *)p)
+#define to_entry(p) ((struct tlvinfo_tlv *)p)
+
+/**
+ * Check whether eeprom device exists.
+ */
+bool exists_tlv_eeprom(int dev)
+{
+ return dev < TLV_MAX_DEVICES && tlv_devices[dev] != 0;
+}
+
+static inline bool is_digit(char c)
+{
+ return (c >= '0' && c <= '9');
+}
+
+/**
+ * is_hex
+ *
+ * Tests if character is an ASCII hex digit
+ */
+static inline u8 is_hex(char p)
+{
+ return (((p >= '0') && (p <= '9')) ||
+ ((p >= 'A') && (p <= 'F')) ||
+ ((p >= 'a') && (p <= 'f')));
+}
+
+/**
+ * Validate the checksum in the provided TlvInfo EEPROM data. First,
+ * verify that the TlvInfo header is valid, then make sure the last
+ * TLV is a CRC-32 TLV. Then calculate the CRC over the EEPROM data
+ * and compare it to the value stored in the EEPROM CRC-32 TLV.
+ */
+bool tlvinfo_check_crc(u8 *eeprom)
+{
+ struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
+ struct tlvinfo_tlv *eeprom_crc;
+ unsigned int calc_crc;
+ unsigned int stored_crc;
+
+ // Is the eeprom header valid?
+ if (!is_valid_tlvinfo_header(eeprom_hdr))
+ return false;
+
+ // Is the last TLV a CRC?
+ eeprom_crc = to_entry(&eeprom[TLV_INFO_HEADER_SIZE +
+ be16_to_cpu(eeprom_hdr->totallen) - (TLV_INFO_ENTRY_SIZE + 4)]);
+ if (eeprom_crc->type != TLV_CODE_CRC_32 || eeprom_crc->length != 4)
+ return false;
+
+ // Calculate the checksum
+ calc_crc = crc32(0, (void *)eeprom,
+ TLV_INFO_HEADER_SIZE + be16_to_cpu(eeprom_hdr->totallen) - 4);
+ stored_crc = (eeprom_crc->value[0] << 24) |
+ (eeprom_crc->value[1] << 16) |
+ (eeprom_crc->value[2] << 8) |
+ eeprom_crc->value[3];
+ return calc_crc == stored_crc;
+}
+
+/**
+ * read_eeprom
+ *
+ * Read the EEPROM into memory, if it hasn't already been read.
+ */
+static int read_eeprom(int devnum, u8 *eeprom)
+{
+ int ret;
+ struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
+ struct tlvinfo_tlv *eeprom_tlv = to_entry(&eeprom[TLV_INFO_HEADER_SIZE]);
+
+ /* Read the header */
+ ret = read_tlv_eeprom((void *)eeprom_hdr, 0, TLV_INFO_HEADER_SIZE, devnum);
+ /* If the header was successfully read, read the TLVs */
+ if (ret == 0 && is_valid_tlvinfo_header(eeprom_hdr))
+ ret = read_tlv_eeprom((void *)eeprom_tlv, TLV_INFO_HEADER_SIZE,
+ be16_to_cpu(eeprom_hdr->totallen), devnum);
+
+ // If the contents are invalid, start over with default contents
+ if (!is_valid_tlvinfo_header(eeprom_hdr) ||
+ !tlvinfo_check_crc(eeprom)) {
+ strcpy(eeprom_hdr->signature, TLV_INFO_ID_STRING);
+ eeprom_hdr->version = TLV_INFO_VERSION;
+ eeprom_hdr->totallen = cpu_to_be16(0);
+ tlvinfo_update_crc(eeprom);
+ }
+
+#ifdef DEBUG
+ show_eeprom(devnum, eeprom);
+#endif
+
+ return ret;
+}
+
+/**
+ * tlvinfo_update_crc
+ *
+ * This function updates the CRC-32 TLV. If there is no CRC-32 TLV, then
+ * one is added. This function should be called after each update to the
+ * EEPROM structure, to make sure the CRC is always correct.
+ */
+void tlvinfo_update_crc(u8 *eeprom)
+{
+ struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
+ struct tlvinfo_tlv *eeprom_crc;
+ unsigned int calc_crc;
+ int eeprom_index;
+
+ // Discover the CRC TLV
+ if (!tlvinfo_find_tlv(eeprom, TLV_CODE_CRC_32, &eeprom_index)) {
+ unsigned int totallen = be16_to_cpu(eeprom_hdr->totallen);
+
+ if ((totallen + TLV_INFO_ENTRY_SIZE + 4) > TLV_TOTAL_LEN_MAX)
+ return;
+ eeprom_index = TLV_INFO_HEADER_SIZE + totallen;
+ eeprom_hdr->totallen = cpu_to_be16(totallen + TLV_INFO_ENTRY_SIZE + 4);
+ }
+ eeprom_crc = to_entry(&eeprom[eeprom_index]);
+ eeprom_crc->type = TLV_CODE_CRC_32;
+ eeprom_crc->length = 4;
+
+ // Calculate the checksum
+ calc_crc = crc32(0, (void *)eeprom,
+ TLV_INFO_HEADER_SIZE + be16_to_cpu(eeprom_hdr->totallen) - 4);
+ eeprom_crc->value[0] = (calc_crc >> 24) & 0xFF;
+ eeprom_crc->value[1] = (calc_crc >> 16) & 0xFF;
+ eeprom_crc->value[2] = (calc_crc >> 8) & 0xFF;
+ eeprom_crc->value[3] = (calc_crc >> 0) & 0xFF;
+}
+
+/**
+ * write_tlvinfo_tlv_eeprom
+ *
+ * Write the TLV data from CPU memory to the hardware.
+ */
+int write_tlvinfo_tlv_eeprom(void *eeprom, int dev)
+{
+ int ret = 0;
+ struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
+ int eeprom_len;
+
+ tlvinfo_update_crc(eeprom);
+
+ eeprom_len = TLV_INFO_HEADER_SIZE + be16_to_cpu(eeprom_hdr->totallen);
+ ret = write_tlv_eeprom(eeprom, eeprom_len, dev);
+ if (ret) {
+ printf("Programming failed.\n");
+ return -1;
+ }
+
+ printf("Programming passed.\n");
+ return 0;
+}
+
+/**
+ * tlvinfo_find_tlv
+ *
+ * This function finds the TLV with the supplied code in the EERPOM.
+ * An offset from the beginning of the EEPROM is returned in the
+ * eeprom_index parameter if the TLV is found.
+ */
+bool tlvinfo_find_tlv(u8 *eeprom, u8 tcode, int *eeprom_index)
+{
+ struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
+ struct tlvinfo_tlv *eeprom_tlv;
+ int eeprom_end;
+
+ // Search through the TLVs, looking for the first one which matches the
+ // supplied type code.
+ *eeprom_index = TLV_INFO_HEADER_SIZE;
+ eeprom_end = TLV_INFO_HEADER_SIZE + be16_to_cpu(eeprom_hdr->totallen);
+ while (*eeprom_index < eeprom_end) {
+ eeprom_tlv = to_entry(&eeprom[*eeprom_index]);
+ if (!is_valid_tlvinfo_entry(eeprom_tlv))
+ return false;
+ if (eeprom_tlv->type == tcode)
+ return true;
+ *eeprom_index += TLV_INFO_ENTRY_SIZE + eeprom_tlv->length;
+ }
+ return(false);
+}
+
+/**
+ * tlvinfo_delete_tlv
+ *
+ * This function deletes the TLV with the specified type code from the
+ * EEPROM.
+ */
+bool tlvinfo_delete_tlv(u8 *eeprom, u8 code)
+{
+ int eeprom_index;
+ int tlength;
+ struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
+ struct tlvinfo_tlv *eeprom_tlv;
+
+ // Find the TLV and then move all following TLVs "forward"
+ if (tlvinfo_find_tlv(eeprom, code, &eeprom_index)) {
+ eeprom_tlv = to_entry(&eeprom[eeprom_index]);
+ tlength = TLV_INFO_ENTRY_SIZE + eeprom_tlv->length;
+ memcpy(&eeprom[eeprom_index], &eeprom[eeprom_index + tlength],
+ TLV_INFO_HEADER_SIZE +
+ be16_to_cpu(eeprom_hdr->totallen) - eeprom_index -
+ tlength);
+ eeprom_hdr->totallen =
+ cpu_to_be16(be16_to_cpu(eeprom_hdr->totallen) -
+ tlength);
+ tlvinfo_update_crc(eeprom);
+ return true;
+ }
+ return false;
+}
+
+/**
+ * tlvinfo_add_tlv
+ *
+ * This function adds a TLV to the EEPROM, converting the value (a string) to
+ * the format in which it will be stored in the EEPROM.
+ */
+#define MAX_TLV_VALUE_LEN 256
+bool tlvinfo_add_tlv(u8 *eeprom, int tcode, char *strval)
+{
+ struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
+ struct tlvinfo_tlv *eeprom_tlv;
+ int new_tlv_len = 0;
+ u32 value;
+ char data[MAX_TLV_VALUE_LEN];
+ int eeprom_index;
+
+ // Encode each TLV type into the format to be stored in the EERPOM
+ switch (tcode) {
+ case TLV_CODE_PRODUCT_NAME:
+ case TLV_CODE_PART_NUMBER:
+ case TLV_CODE_SERIAL_NUMBER:
+ case TLV_CODE_LABEL_REVISION:
+ case TLV_CODE_PLATFORM_NAME:
+ case TLV_CODE_ONIE_VERSION:
+ case TLV_CODE_MANUF_NAME:
+ case TLV_CODE_MANUF_COUNTRY:
+ case TLV_CODE_VENDOR_NAME:
+ case TLV_CODE_DIAG_VERSION:
+ case TLV_CODE_SERVICE_TAG:
+ strncpy(data, strval, MAX_TLV_VALUE_LEN);
+ new_tlv_len = min_t(size_t, MAX_TLV_VALUE_LEN, strlen(strval));
+ break;
+ case TLV_CODE_DEVICE_VERSION:
+ value = simple_strtoul(strval, NULL, 0);
+ if (value >= 256) {
+ printf("ERROR: Device version must be 255 or less. Value supplied: %u",
+ value);
+ return false;
+ }
+ data[0] = value & 0xFF;
+ new_tlv_len = 1;
+ break;
+ case TLV_CODE_MAC_SIZE:
+ value = simple_strtoul(strval, NULL, 0);
+ if (value >= 65536) {
+ printf("ERROR: MAC Size must be 65535 or less. Value supplied: %u",
+ value);
+ return false;
+ }
+ data[0] = (value >> 8) & 0xFF;
+ data[1] = value & 0xFF;
+ new_tlv_len = 2;
+ break;
+ case TLV_CODE_MANUF_DATE:
+ if (set_date(data, strval) != 0)
+ return false;
+ new_tlv_len = 19;
+ break;
+ case TLV_CODE_MAC_BASE:
+ if (set_mac(data, strval) != 0)
+ return false;
+ new_tlv_len = 6;
+ break;
+ case TLV_CODE_CRC_32:
+ printf("WARNING: The CRC TLV is set automatically and cannot be set manually.\n");
+ return false;
+ case TLV_CODE_VENDOR_EXT:
+ default:
+ if (set_bytes(data, strval, &new_tlv_len) != 0)
+ return false;
+ break;
+ }
+
+ // Is there room for this TLV?
+ if ((be16_to_cpu(eeprom_hdr->totallen) + TLV_INFO_ENTRY_SIZE + new_tlv_len) >
+ TLV_TOTAL_LEN_MAX) {
+ printf("ERROR: There is not enough room in the EERPOM to save data.\n");
+ return false;
+ }
+
+ // Add TLV at the end, overwriting CRC TLV if it exists
+ if (tlvinfo_find_tlv(eeprom, TLV_CODE_CRC_32, &eeprom_index))
+ eeprom_hdr->totallen =
+ cpu_to_be16(be16_to_cpu(eeprom_hdr->totallen) -
+ TLV_INFO_ENTRY_SIZE - 4);
+ else
+ eeprom_index = TLV_INFO_HEADER_SIZE + be16_to_cpu(eeprom_hdr->totallen);
+ eeprom_tlv = to_entry(&eeprom[eeprom_index]);
+ eeprom_tlv->type = tcode;
+ eeprom_tlv->length = new_tlv_len;
+ memcpy(eeprom_tlv->value, data, new_tlv_len);
+
+ // Update the total length and calculate (add) a new CRC-32 TLV
+ eeprom_hdr->totallen = cpu_to_be16(be16_to_cpu(eeprom_hdr->totallen) +
+ TLV_INFO_ENTRY_SIZE + new_tlv_len);
+ tlvinfo_update_crc(eeprom);
+
+ return true;
+}
+
+/**
+ * set_mac
+ *
+ * Converts a string MAC address into a binary buffer.
+ *
+ * This function takes a pointer to a MAC address string
+ * (i.e."XX:XX:XX:XX:XX:XX", where "XX" is a two-digit hex number).
+ * The string format is verified and then converted to binary and
+ * stored in a buffer.
+ */
+static int set_mac(char *buf, const char *string)
+{
+ char *p = (char *)string;
+ int i;
+ int err = 0;
+ char *end;
+
+ if (!p) {
+ printf("ERROR: NULL mac addr string passed in.\n");
+ return -1;
+ }
+
+ if (strlen(p) != 17) {
+ printf("ERROR: MAC address strlen() != 17 -- %zu\n", strlen(p));
+ printf("ERROR: Bad MAC address format: %s\n", string);
+ return -1;
+ }
+
+ for (i = 0; i < 17; i++) {
+ if ((i % 3) == 2) {
+ if (p[i] != ':') {
+ err++;
+ printf("ERROR: mac: p[%i] != :, found: `%c'\n",
+ i, p[i]);
+ break;
+ }
+ continue;
+ } else if (!is_hex(p[i])) {
+ err++;
+ printf("ERROR: mac: p[%i] != hex digit, found: `%c'\n",
+ i, p[i]);
+ break;
+ }
+ }
+
+ if (err != 0) {
+ printf("ERROR: Bad MAC address format: %s\n", string);
+ return -1;
+ }
+
+ /* Convert string to binary */
+ for (i = 0, p = (char *)string; i < 6; i++) {
+ buf[i] = p ? hextoul(p, &end) : 0;
+ if (p)
+ p = (*end) ? end + 1 : end;
+ }
+
+ if (!is_valid_ethaddr((u8 *)buf)) {
+ printf("ERROR: MAC address must not be 00:00:00:00:00:00, a multicast address or FF:FF:FF:FF:FF:FF.\n");
+ printf("ERROR: Bad MAC address format: %s\n", string);
+ return -1;
+ }
+
+ return 0;
+}
+
+/**
+ * set_date
+ *
+ * Validates the format of the data string
+ *
+ * This function takes a pointer to a date string (i.e. MM/DD/YYYY hh:mm:ss)
+ * and validates that the format is correct. If so the string is copied
+ * to the supplied buffer.
+ */
+static int set_date(char *buf, const char *string)
+{
+ int i;
+
+ if (!string) {
+ printf("ERROR: NULL date string passed in.\n");
+ return -1;
+ }
+
+ if (strlen(string) != 19) {
+ printf("ERROR: Date strlen() != 19 -- %zu\n", strlen(string));
+ printf("ERROR: Bad date format (MM/DD/YYYY hh:mm:ss): %s\n",
+ string);
+ return -1;
+ }
+
+ for (i = 0; string[i] != 0; i++) {
+ switch (i) {
+ case 2:
+ case 5:
+ if (string[i] != '/') {
+ printf("ERROR: Bad date format (MM/DD/YYYY hh:mm:ss): %s\n",
+ string);
+ return -1;
+ }
+ break;
+ case 10:
+ if (string[i] != ' ') {
+ printf("ERROR: Bad date format (MM/DD/YYYY hh:mm:ss): %s\n",
+ string);
+ return -1;
+ }
+ break;
+ case 13:
+ case 16:
+ if (string[i] != ':') {
+ printf("ERROR: Bad date format (MM/DD/YYYY hh:mm:ss): %s\n",
+ string);
+ return -1;
+ }
+ break;
+ default:
+ if (!is_digit(string[i])) {
+ printf("ERROR: Bad date format (MM/DD/YYYY hh:mm:ss): %s\n",
+ string);
+ return -1;
+ }
+ break;
+ }
+ }
+
+ strcpy(buf, string);
+ return 0;
+}
+
+/**
+ * set_bytes
+ *
+ * Converts a space-separated string of decimal numbers into a
+ * buffer of bytes.
+ *
+ * This function takes a pointer to a space-separated string of decimal
+ * numbers (i.e. "128 0x55 0321") with "C" standard radix specifiers
+ * and converts them to an array of bytes.
+ */
+static int set_bytes(char *buf, const char *string, int *converted_accum)
+{
+ char *p = (char *)string;
+ int i;
+ uint byte;
+
+ if (!p) {
+ printf("ERROR: NULL string passed in.\n");
+ return -1;
+ }
+
+ /* Convert string to bytes */
+ for (i = 0, p = (char *)string; (i < TLV_VALUE_MAX_LEN) && (*p != 0);
+ i++) {
+ while ((*p == ' ') || (*p == '\t') || (*p == ',') ||
+ (*p == ';')) {
+ p++;
+ }
+ if (*p != 0) {
+ if (!is_digit(*p)) {
+ printf("ERROR: Non-digit found in byte string: (%s)\n",
+ string);
+ return -1;
+ }
+ byte = simple_strtoul(p, &p, 0);
+ if (byte >= 256) {
+ printf("ERROR: The value specified is greater than 255: (%u) in string: %s\n",
+ byte, string);
+ return -1;
+ }
+ buf[i] = byte & 0xFF;
+ }
+ }
+
+ if (i == TLV_VALUE_MAX_LEN && (*p != 0)) {
+ printf("ERROR: Trying to assign too many bytes (max: %d) in string: %s\n",
+ TLV_VALUE_MAX_LEN, string);
+ return -1;
+ }
+
+ *converted_accum = i;
+ return 0;
+}
+
+static int find_tlv_devices(struct udevice **tlv_devices_p)
+{
+ int ret;
+ int count_dev = 0;
+ struct udevice *dev;
+
+ for (ret = uclass_first_device_check(UCLASS_I2C_EEPROM, &dev);
+ dev;
+ ret = uclass_next_device_check(&dev)) {
+ if (ret == 0)
+ tlv_devices_p[count_dev++] = dev;
+ if (count_dev >= TLV_MAX_DEVICES)
+ break;
+ }
+
+ return (count_dev == 0) ? -ENODEV : 0;
+}
+
+static struct udevice *find_tlv_device_by_index(int dev_num)
+{
+ struct udevice *local_tlv_devices[TLV_MAX_DEVICES] = {};
+ struct udevice **tlv_devices_p;
+ int ret;
+
+ if (gd->flags & (GD_FLG_RELOC | GD_FLG_SPL_INIT)) {
+ /* Assume BSS is initialized; use static data */
+ if (tlv_devices[dev_num])
+ return tlv_devices[dev_num];
+ tlv_devices_p = tlv_devices;
+ } else {
+ tlv_devices_p = local_tlv_devices;
+ }
+
+ ret = find_tlv_devices(tlv_devices_p);
+ if (ret == 0 && tlv_devices_p[dev_num])
+ return tlv_devices_p[dev_num];
+
+ return NULL;
+}
+
+/**
+ * read_tlv_eeprom - read the hwinfo from i2c EEPROM
+ */
+int read_tlv_eeprom(void *eeprom, int offset, int len, int dev_num)
+{
+ struct udevice *dev;
+
+ if (dev_num >= TLV_MAX_DEVICES)
+ return -EINVAL;
+
+ dev = find_tlv_device_by_index(dev_num);
+ if (!dev)
+ return -ENODEV;
+
+ return i2c_eeprom_read(dev, offset, eeprom, len);
+}
+
+/**
+ * write_tlv_eeprom - write the hwinfo to i2c EEPROM
+ */
+int write_tlv_eeprom(void *eeprom, int len, int dev)
+{
+ if (!(gd->flags & GD_FLG_RELOC))
+ return -ENODEV;
+ if (!tlv_devices[dev])
+ return -ENODEV;
+
+ return i2c_eeprom_write(tlv_devices[dev], 0, eeprom, len);
+}
+
+int read_tlvinfo_tlv_eeprom(void *eeprom, struct tlvinfo_header **hdr,
+ struct tlvinfo_tlv **first_entry, int dev_num)
+{
+ int ret;
+ struct tlvinfo_header *tlv_hdr;
+ struct tlvinfo_tlv *tlv_ent;
+
+ /* Read TLV header */
+ ret = read_tlv_eeprom(eeprom, 0, TLV_INFO_HEADER_SIZE, dev_num);
+ if (ret < 0)
+ return ret;
+
+ tlv_hdr = eeprom;
+ if (!is_valid_tlvinfo_header(tlv_hdr))
+ return -EINVAL;
+
+ /* Read TLV entries */
+ tlv_ent = to_entry(&tlv_hdr[1]);
+ ret = read_tlv_eeprom(tlv_ent, TLV_INFO_HEADER_SIZE,
+ be16_to_cpu(tlv_hdr->totallen), dev_num);
+ if (ret < 0)
+ return ret;
+ if (!tlvinfo_check_crc(eeprom))
+ return -EINVAL;
+
+ *hdr = tlv_hdr;
+ *first_entry = tlv_ent;
+
+ return 0;
+}
+
+/**
+ * mac_read_from_eeprom
+ *
+ * Read the MAC addresses from EEPROM
+ *
+ * This function reads the MAC addresses from EEPROM and sets the
+ * appropriate environment variables for each one read.
+ *
+ * The environment variables are only set if they haven't been set already.
+ * This ensures that any user-saved variables are never overwritten.
+ *
+ * This function must be called after relocation.
+ */
+int mac_read_from_eeprom(void)
+{
+ unsigned int i;
+ int eeprom_index;
+ struct tlvinfo_tlv *eeprom_tlv;
+ int maccount;
+ u8 macbase[6];
+ struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
+ int devnum = 0; // TODO: support multiple EEPROMs
+
+ puts("EEPROM: ");
+
+ if (read_eeprom(devnum, eeprom)) {
+ printf("Read failed.\n");
+ return -1;
+ }
+
+ maccount = 1;
+ if (tlvinfo_find_tlv(eeprom, TLV_CODE_MAC_SIZE, &eeprom_index)) {
+ eeprom_tlv = to_entry(&eeprom[eeprom_index]);
+ maccount = (eeprom_tlv->value[0] << 8) | eeprom_tlv->value[1];
+ }
+
+ memcpy(macbase, "\0\0\0\0\0\0", 6);
+ if (tlvinfo_find_tlv(eeprom, TLV_CODE_MAC_BASE, &eeprom_index)) {
+ eeprom_tlv = to_entry(&eeprom[eeprom_index]);
+ memcpy(macbase, eeprom_tlv->value, 6);
+ }
+
+ for (i = 0; i < maccount; i++) {
+ if (is_valid_ethaddr(macbase)) {
+ char ethaddr[18];
+ char enetvar[11];
+
+ sprintf(ethaddr, "%02X:%02X:%02X:%02X:%02X:%02X",
+ macbase[0], macbase[1], macbase[2],
+ macbase[3], macbase[4], macbase[5]);
+ sprintf(enetvar, i ? "eth%daddr" : "ethaddr", i);
+ /* Only initialize environment variables that are blank
+ * (i.e. have not yet been set)
+ */
+ if (!env_get(enetvar))
+ env_set(enetvar, ethaddr);
+
+ macbase[5]++;
+ if (macbase[5] == 0) {
+ macbase[4]++;
+ if (macbase[4] == 0) {
+ macbase[3]++;
+ if (macbase[3] == 0) {
+ macbase[0] = 0;
+ macbase[1] = 0;
+ macbase[2] = 0;
+ }
+ }
+ }
+ }
+ }
+
+ printf("%s v%u len=%u\n", eeprom_hdr->signature, eeprom_hdr->version,
+ be16_to_cpu(eeprom_hdr->totallen));
+
+ return 0;
+}
+
+/**
+ * populate_serial_number - read the serial number from EEPROM
+ *
+ * This function reads the serial number from the EEPROM and sets the
+ * appropriate environment variable.
+ *
+ * The environment variable is only set if it has not been set
+ * already. This ensures that any user-saved variables are never
+ * overwritten.
+ *
+ * This function must be called after relocation.
+ */
+int populate_serial_number(int devnum)
+{
+ char serialstr[257];
+ int eeprom_index;
+ struct tlvinfo_tlv *eeprom_tlv;
+
+ if (env_get("serial#"))
+ return 0;
+
+ if (read_eeprom(devnum, eeprom)) {
+ printf("Read failed.\n");
+ return -1;
+ }
+
+ if (tlvinfo_find_tlv(eeprom, TLV_CODE_SERIAL_NUMBER, &eeprom_index)) {
+ eeprom_tlv = to_entry(&eeprom[eeprom_index]);
+ memcpy(serialstr, eeprom_tlv->value, eeprom_tlv->length);
+ serialstr[eeprom_tlv->length] = 0;
+ env_set("serial#", serialstr);
+ }
+
+ return 0;
+}
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 11/12] arm: mvebu: clearfog: enable tlv library for spl in favour of eeprom cmd
2022-05-02 14:18 [PATCH 00/12] split tlv_eeprom command into a separate library Josua Mayer
` (9 preceding siblings ...)
2022-05-02 14:18 ` [PATCH 10/12] cmd: tlv_eeprom: split off tlv library from command Josua Mayer
@ 2022-05-02 14:18 ` Josua Mayer
2022-05-02 14:18 ` [PATCH 12/12] lib: tlv_eeprom: add function for reading one entry into a C string Josua Mayer
11 siblings, 0 replies; 22+ messages in thread
From: Josua Mayer @ 2022-05-02 14:18 UTC (permalink / raw)
To: u-boot; +Cc: Josua Mayer, Stefan Roese
The board file used CONFIG_SPL_CMD_TLV_EEPROM as a library to facilitate
reading tlv data with the memory size from eeprom.
Since the tlv library has been split off, only CONFIG_SPL_EEPROM_TLV_LIB
is required now.
Signed-off-by: Josua Mayer <josua@solid-run.com>
---
configs/clearfog_defconfig | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/configs/clearfog_defconfig b/configs/clearfog_defconfig
index 880f16a6e0..87ba6f29a7 100644
--- a/configs/clearfog_defconfig
+++ b/configs/clearfog_defconfig
@@ -12,6 +12,7 @@ CONFIG_DM_GPIO=y
CONFIG_DEFAULT_DEVICE_TREE="armada-388-clearfog"
CONFIG_SPL_TEXT_BASE=0x40000030
CONFIG_SPL_SERIAL=y
+CONFIG_SPL_DRIVERS_MISC=y
CONFIG_SPL=y
CONFIG_DEBUG_UART_BASE=0xd0012000
CONFIG_DEBUG_UART_CLOCK=250000000
@@ -26,7 +27,6 @@ CONFIG_SYS_CONSOLE_INFO_QUIET=y
CONFIG_DISPLAY_BOARDINFO_LATE=y
CONFIG_SPL_I2C=y
CONFIG_CMD_TLV_EEPROM=y
-CONFIG_SPL_CMD_TLV_EEPROM=y
# CONFIG_CMD_FLASH is not set
CONFIG_CMD_GPIO=y
CONFIG_CMD_I2C=y
@@ -71,3 +71,4 @@ CONFIG_SYS_NS16550=y
CONFIG_KIRKWOOD_SPI=y
CONFIG_USB=y
CONFIG_USB_XHCI_HCD=y
+CONFIG_SPL_EEPROM_TLV_LIB=y
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 12/12] lib: tlv_eeprom: add function for reading one entry into a C string
2022-05-02 14:18 [PATCH 00/12] split tlv_eeprom command into a separate library Josua Mayer
` (10 preceding siblings ...)
2022-05-02 14:18 ` [PATCH 11/12] arm: mvebu: clearfog: enable tlv library for spl in favour of eeprom cmd Josua Mayer
@ 2022-05-02 14:18 ` Josua Mayer
11 siblings, 0 replies; 22+ messages in thread
From: Josua Mayer @ 2022-05-02 14:18 UTC (permalink / raw)
To: u-boot; +Cc: Josua Mayer, Simon Glass, Stefan Roese, Sven Auhagen
This solves the potentially common problem of getting a specific tlv
entry from an eeprom in board-files, without having to introduce several
variables, error handling, memcpy and 0-terminating the string.
Signed-off-by: Josua Mayer <josua@solid-run.com>
---
include/tlv_eeprom.h | 12 ++++++++++++
lib/tlv/tlv_eeprom.c | 25 +++++++++++++++++++++++++
2 files changed, 37 insertions(+)
diff --git a/include/tlv_eeprom.h b/include/tlv_eeprom.h
index c81c58837d..5989c611f5 100644
--- a/include/tlv_eeprom.h
+++ b/include/tlv_eeprom.h
@@ -166,6 +166,18 @@ bool tlvinfo_add_tlv(u8 *eeprom, int code, char *strval);
*/
bool tlvinfo_delete_tlv(u8 *eeprom, u8 code);
+/**
+ * Read the TLV entry with specified code to a buffer as terminated C string.
+ * @eeprom: Pointer to buffer holding the TLV EEPROM binary data.
+ * @code: The TLV Code of the entry to read.
+ * @buffer: Pointer to buffer where the value will be stored. Must have capacity
+ * for the string representation of the data including null terminator.
+ * @length: size of the buffer where the value will be stored.
+ *
+ * Return length of string on success, -1 on error.
+ */
+ssize_t tlvinfo_read_tlv(u8 *eeprom, u8 code, u8 *buffer, size_t length);
+
/**
* tlvinfo_update_crc
*
diff --git a/lib/tlv/tlv_eeprom.c b/lib/tlv/tlv_eeprom.c
index 464f0aa1fa..205960e8f2 100644
--- a/lib/tlv/tlv_eeprom.c
+++ b/lib/tlv/tlv_eeprom.c
@@ -350,6 +350,31 @@ bool tlvinfo_add_tlv(u8 *eeprom, int tcode, char *strval)
return true;
}
+/**
+ * Read the TLV entry with specified code to a buffer as terminated C string.
+ */
+ssize_t tlvinfo_read_tlv(u8 *eeprom, u8 code, u8 *buffer, size_t length)
+{
+ int index;
+ struct tlvinfo_tlv *tlv;
+
+ // read sku from part-number field
+ if (tlvinfo_find_tlv(eeprom, code, &index)) {
+ tlv = (struct tlvinfo_tlv *)&eeprom[index];
+ if (tlv->length > length) {
+ pr_err("%s: tlv value (%d) larger than buffer (%zu)!\n",
+ __func__, tlv->length + 1, length);
+ return -1;
+ }
+ memcpy(buffer, tlv->value, tlv->length);
+ buffer[tlv->length] = 0;
+
+ return tlv->length;
+ }
+
+ return -1;
+}
+
/**
* set_mac
*
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread