From a804c8dc5f5e3760948e5ef22f5c605af9cfc695 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Beh=C3=BAn?= Date: Tue, 21 May 2024 09:13:25 +0200 Subject: [PATCH 01/11] common: eeprom_layout: Assign default layout methods and parameters before specific ones MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Assign the default eeprom layout parameter .data_size and methods .print() and .update() before calling eeprom_layout_assign() in eeprom_layout_setup(). This allows eeprom_layout_assign() to overwrite these if needed. Signed-off-by: Marek Behún --- common/eeprom/eeprom_layout.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/common/eeprom/eeprom_layout.c b/common/eeprom/eeprom_layout.c index 5a9be1da061..406db3f7d15 100644 --- a/common/eeprom/eeprom_layout.c +++ b/common/eeprom/eeprom_layout.c @@ -111,14 +111,14 @@ void eeprom_layout_setup(struct eeprom_layout *layout, unsigned char *buf, else layout->layout_version = layout_version; + layout->data_size = buf_size; + layout->print = eeprom_layout_print; + layout->update = eeprom_layout_update_field; + eeprom_layout_assign(layout, layout_version); layout->data = buf; for (i = 0; i < layout->num_of_fields; i++) { layout->fields[i].buf = buf; buf += layout->fields[i].size; } - - layout->data_size = buf_size; - layout->print = eeprom_layout_print; - layout->update = eeprom_layout_update_field; } From 15378a3fe1838a3c5abf2330a6eb4e92462783c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Beh=C3=BAn?= Date: Tue, 21 May 2024 09:13:26 +0200 Subject: [PATCH 02/11] common: eeprom_layout: Split field finding code from the field update function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Split the eeprom layout field finding code from the eeprom_layout_update_field() function in order to make it usable in alternative implementations of update method. Signed-off-by: Marek Behún --- common/eeprom/eeprom_layout.c | 46 +++++++++++++++++++++++------------ include/eeprom_layout.h | 4 +++ 2 files changed, 35 insertions(+), 15 deletions(-) diff --git a/common/eeprom/eeprom_layout.c b/common/eeprom/eeprom_layout.c index 406db3f7d15..801e90d38d0 100644 --- a/common/eeprom/eeprom_layout.c +++ b/common/eeprom/eeprom_layout.c @@ -56,6 +56,28 @@ static void eeprom_layout_print(const struct eeprom_layout *layout) fields[i].print(&fields[i]); } +/* + * eeprom_layout_find_field() - finds a layout field by name + * @layout: A pointer to an existing struct layout. + * @field_name: The name of the field to update. + * @warn: Whether to print a warning if the field is not found. + * + * Returns: a pointer to the found field or NULL on failure. + */ +struct eeprom_field *eeprom_layout_find_field(struct eeprom_layout *layout, + char *field_name, bool warn) +{ + for (int i = 0; i < layout->num_of_fields; i++) + if (layout->fields[i].name != RESERVED_FIELDS && + !strcmp(layout->fields[i].name, field_name)) + return &layout->fields[i]; + + if (warn) + printf("No such field '%s'\n", field_name); + + return NULL; +} + /* * eeprom_layout_update_field() - update a single field in the layout data. * @layout: A pointer to an existing struct layout. @@ -67,8 +89,8 @@ static void eeprom_layout_print(const struct eeprom_layout *layout) static int eeprom_layout_update_field(struct eeprom_layout *layout, char *field_name, char *new_data) { - int i, err; - struct eeprom_field *fields = layout->fields; + struct eeprom_field *field; + int err; if (new_data == NULL) return 0; @@ -76,21 +98,15 @@ static int eeprom_layout_update_field(struct eeprom_layout *layout, if (field_name == NULL) return -1; - for (i = 0; i < layout->num_of_fields; i++) { - if (fields[i].name == RESERVED_FIELDS || - strcmp(fields[i].name, field_name)) - continue; + field = eeprom_layout_find_field(layout, field_name, true); + if (field == NULL) + return -1; - err = fields[i].update(&fields[i], new_data); - if (err) - printf("Invalid data for field %s\n", field_name); + err = field->update(field, new_data); + if (err) + printf("Invalid data for field %s\n", field_name); - return err; - } - - printf("No such field '%s'\n", field_name); - - return -1; + return err; } /* diff --git a/include/eeprom_layout.h b/include/eeprom_layout.h index 730d963ab96..b1d62205958 100644 --- a/include/eeprom_layout.h +++ b/include/eeprom_layout.h @@ -9,6 +9,8 @@ #ifndef _LAYOUT_ #define _LAYOUT_ +#include + #define RESERVED_FIELDS NULL #define LAYOUT_VERSION_UNRECOGNIZED -1 #define LAYOUT_VERSION_AUTODETECT -2 @@ -24,6 +26,8 @@ struct eeprom_layout { char *new_data); }; +struct eeprom_field *eeprom_layout_find_field(struct eeprom_layout *layout, + char *field_name, bool warn); void eeprom_layout_setup(struct eeprom_layout *layout, unsigned char *buf, unsigned int buf_size, int layout_version); __weak void __eeprom_layout_assign(struct eeprom_layout *layout, From 96dfa5869da586eb1c8d84c22164a2bf399968a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Beh=C3=BAn?= Date: Tue, 21 May 2024 09:13:27 +0200 Subject: [PATCH 03/11] common: eeprom_field: Fix updating binary field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The __eeprom_field_update_bin() function is expected to parse a hex string into bytes (potentially in reverse order), but the simple_strtoul() function is given 0 as base. This does not work since the string does not contain '0x' prefix. Add explicit base 16. Signed-off-by: Marek Behún --- common/eeprom/eeprom_field.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/eeprom/eeprom_field.c b/common/eeprom/eeprom_field.c index f56eebe679f..9b831414a4b 100644 --- a/common/eeprom/eeprom_field.c +++ b/common/eeprom/eeprom_field.c @@ -55,7 +55,7 @@ static int __eeprom_field_update_bin(struct eeprom_field *field, tmp[k] = value[reverse ? i - 1 + k : i + k]; } - byte = simple_strtoul(tmp, &endptr, 0); + byte = simple_strtoul(tmp, &endptr, 16); if (*endptr != '\0' || byte < 0) return -1; From 951dc4e077b2ba5eadaba3beb9b61f131ffa6fdc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Beh=C3=BAn?= Date: Tue, 21 May 2024 09:13:28 +0200 Subject: [PATCH 04/11] common: eeprom_field: Drop unnecessary comparison MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The byte variable is of type unsigned char, it is never less than zero. The error case is handled by *endptr, so drop the comparison altogether. Signed-off-by: Marek Behún --- common/eeprom/eeprom_field.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/eeprom/eeprom_field.c b/common/eeprom/eeprom_field.c index 9b831414a4b..26f6041e548 100644 --- a/common/eeprom/eeprom_field.c +++ b/common/eeprom/eeprom_field.c @@ -56,7 +56,7 @@ static int __eeprom_field_update_bin(struct eeprom_field *field, } byte = simple_strtoul(tmp, &endptr, 16); - if (*endptr != '\0' || byte < 0) + if (*endptr != '\0') return -1; field->buf[j] = byte; From 642ec48c76e6469c9a14f5c0359363af25bdaafb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Beh=C3=BAn?= Date: Tue, 21 May 2024 09:13:29 +0200 Subject: [PATCH 05/11] cmd: eeprom: Fix usage help for the eeprom command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bus and devaddr arguments of the eeprom command are optional, and if only one is given, it is assumed to be devaddr. Change the usage help from to [[bus] [devaddr] Signed-off-by: Marek Behún --- cmd/eeprom.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/eeprom.c b/cmd/eeprom.c index 322765ad02a..0d604832e44 100644 --- a/cmd/eeprom.c +++ b/cmd/eeprom.c @@ -418,14 +418,14 @@ int do_eeprom(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) U_BOOT_CMD( eeprom, 8, 1, do_eeprom, "EEPROM sub-system", - "read addr off cnt\n" - "eeprom write addr off cnt\n" + "read [[bus] devaddr] addr off cnt\n" + "eeprom write [[bus] devaddr] addr off cnt\n" " - read/write `cnt' bytes from `devaddr` EEPROM at offset `off'" #ifdef CONFIG_CMD_EEPROM_LAYOUT "\n" - "eeprom print [-l ] \n" + "eeprom print [-l ] [[bus] devaddr]\n" " - Print layout fields and their data in human readable format\n" - "eeprom update [-l ] field_name field_value\n" + "eeprom update [-l ] [[bus] devaddr] field_name field_value\n" " - Update a specific eeprom field with new data.\n" " The new data must be written in the same human readable format as shown by the print command.\n" "\n" From 2d49dafe4ac572228fe4309cf0445b5f82ff2fd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Beh=C3=BAn?= Date: Tue, 21 May 2024 09:13:30 +0200 Subject: [PATCH 06/11] cmd: eeprom: Hide eeprom layout versioning behind a Kconfig option MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a new Kconfig option EEPROM_LAYOUT_VERSIONS, and hide eeprom layout versionsing code behind it. Only print the relevant help in 'eeprom' command usage if this option is enabled. Enable this new option for cm_fx6_defconfig and cm_t43_defconfig. These are the only boards using EEPROM layout versioning. Signed-off-by: Marek Behún --- cmd/Kconfig | 9 ++++++++- cmd/eeprom.c | 20 +++++++++++++++----- configs/cm_fx6_defconfig | 1 + configs/cm_t43_defconfig | 1 + 4 files changed, 25 insertions(+), 6 deletions(-) diff --git a/cmd/Kconfig b/cmd/Kconfig index b026439c773..8c370993f67 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -803,9 +803,16 @@ config CMD_EEPROM_LAYOUT types of eeprom fields. Can be used for defining custom layouts. +config EEPROM_LAYOUT_VERSIONS + bool "Support specifying eeprom layout version" + depends on CMD_EEPROM_LAYOUT + help + Support specifying eeprom layout version in the 'eeprom' command + via the -l option. + config EEPROM_LAYOUT_HELP_STRING string "Tells user what layout names are supported" - depends on CMD_EEPROM_LAYOUT + depends on EEPROM_LAYOUT_VERSIONS default "" help Help printed with the LAYOUT VERSIONS part of the 'eeprom' diff --git a/cmd/eeprom.c b/cmd/eeprom.c index 0d604832e44..d610dc99317 100644 --- a/cmd/eeprom.c +++ b/cmd/eeprom.c @@ -252,10 +252,12 @@ static int parse_i2c_bus_addr(int *i2c_bus, ulong *i2c_addr, int argc, #ifdef CONFIG_CMD_EEPROM_LAYOUT +#ifdef CONFIG_EEPROM_LAYOUT_VERSIONS __weak int eeprom_parse_layout_version(char *str) { return LAYOUT_VERSION_UNRECOGNIZED; } +#endif static unsigned char eeprom_buf[CONFIG_SYS_EEPROM_SIZE]; @@ -359,7 +361,7 @@ int do_eeprom(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) if (action == EEPROM_ACTION_INVALID) return CMD_RET_USAGE; -#ifdef CONFIG_CMD_EEPROM_LAYOUT +#ifdef CONFIG_EEPROM_LAYOUT_VERSIONS if (action == EEPROM_PRINT || action == EEPROM_UPDATE) { if (!strcmp(argv[index], "-l")) { NEXT_PARAM(argc, index); @@ -415,6 +417,12 @@ int do_eeprom(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) field_name, field_value, addr, off, cnt); } +#ifdef CONFIG_EEPROM_LAYOUT_VERSIONS +#define EEPROM_LAYOUT_SPEC "[-l ] " +#else +#define EEPROM_LAYOUT_SPEC "" +#endif + U_BOOT_CMD( eeprom, 8, 1, do_eeprom, "EEPROM sub-system", @@ -423,16 +431,18 @@ U_BOOT_CMD( " - read/write `cnt' bytes from `devaddr` EEPROM at offset `off'" #ifdef CONFIG_CMD_EEPROM_LAYOUT "\n" - "eeprom print [-l ] [[bus] devaddr]\n" + "eeprom print " EEPROM_LAYOUT_SPEC "[[bus] devaddr]\n" " - Print layout fields and their data in human readable format\n" - "eeprom update [-l ] [[bus] devaddr] field_name field_value\n" + "eeprom update " EEPROM_LAYOUT_SPEC "[[bus] devaddr] field_name field_value\n" " - Update a specific eeprom field with new data.\n" - " The new data must be written in the same human readable format as shown by the print command.\n" - "\n" + " The new data must be written in the same human readable format as shown by the print command." +#ifdef CONFIG_EEPROM_LAYOUT_VERSIONS + "\n\n" "LAYOUT VERSIONS\n" "The -l option can be used to force the command to interpret the EEPROM data using the chosen layout.\n" "If the -l option is omitted, the command will auto detect the layout based on the data in the EEPROM.\n" "The values which can be provided with the -l option are:\n" CONFIG_EEPROM_LAYOUT_HELP_STRING"\n" #endif +#endif ); diff --git a/configs/cm_fx6_defconfig b/configs/cm_fx6_defconfig index a4d5f91b35f..386616cc420 100644 --- a/configs/cm_fx6_defconfig +++ b/configs/cm_fx6_defconfig @@ -46,6 +46,7 @@ CONFIG_SYS_MAXARGS=32 CONFIG_CMD_GREPENV=y CONFIG_CMD_EEPROM=y CONFIG_CMD_EEPROM_LAYOUT=y +CONFIG_EEPROM_LAYOUT_VERSIONS=y CONFIG_EEPROM_LAYOUT_HELP_STRING="v2, v3" CONFIG_SYS_I2C_EEPROM_BUS=2 CONFIG_SYS_EEPROM_PAGE_WRITE_BITS=4 diff --git a/configs/cm_t43_defconfig b/configs/cm_t43_defconfig index 93e667292c8..32f126a5174 100644 --- a/configs/cm_t43_defconfig +++ b/configs/cm_t43_defconfig @@ -50,6 +50,7 @@ CONFIG_SYS_PROMPT="CM-T43 # " CONFIG_CMD_ASKENV=y CONFIG_CMD_EEPROM=y CONFIG_CMD_EEPROM_LAYOUT=y +CONFIG_EEPROM_LAYOUT_VERSIONS=y CONFIG_EEPROM_LAYOUT_HELP_STRING="v2, v3" CONFIG_SYS_EEPROM_PAGE_WRITE_BITS=4 CONFIG_SYS_EEPROM_PAGE_WRITE_DELAY_MS=5 From fda747c8f1f287607ffac7c6c3b2d99c7f08540e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Beh=C3=BAn?= Date: Tue, 21 May 2024 09:13:31 +0200 Subject: [PATCH 07/11] cmd: eeprom: Deduplicate parse_i2c_bus_addr() calls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deduplicate the calls to parse_i2c_bus_addr(). Signed-off-by: Marek Behún --- cmd/eeprom.c | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/cmd/eeprom.c b/cmd/eeprom.c index d610dc99317..12902e812e4 100644 --- a/cmd/eeprom.c +++ b/cmd/eeprom.c @@ -339,6 +339,21 @@ static int eeprom_execute_command(enum eeprom_action action, int i2c_bus, return rcode; } +static int eeprom_action_expected_argc(enum eeprom_action action) +{ + switch (action) { + case EEPROM_READ: + case EEPROM_WRITE: + return 3; + case EEPROM_PRINT: + return 0; + case EEPROM_UPDATE: + return 2; + default: + return CMD_RET_USAGE; + } +} + #define NEXT_PARAM(argc, index) { (argc)--; (index)++; } int do_eeprom(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { @@ -371,25 +386,8 @@ int do_eeprom(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) } #endif - switch (action) { - case EEPROM_READ: - case EEPROM_WRITE: - ret = parse_i2c_bus_addr(&i2c_bus, &i2c_addr, argc, - argv + index, 3); - break; - case EEPROM_PRINT: - ret = parse_i2c_bus_addr(&i2c_bus, &i2c_addr, argc, - argv + index, 0); - break; - case EEPROM_UPDATE: - ret = parse_i2c_bus_addr(&i2c_bus, &i2c_addr, argc, - argv + index, 2); - break; - default: - /* Get compiler to stop whining */ - return CMD_RET_USAGE; - } - + ret = parse_i2c_bus_addr(&i2c_bus, &i2c_addr, argc, argv + index, + eeprom_action_expected_argc(action)); if (ret == CMD_RET_USAGE) return ret; From 15f0536f293e9c75ee5f5e70201593f83654710e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Beh=C3=BAn?= Date: Tue, 21 May 2024 09:13:32 +0200 Subject: [PATCH 08/11] cmd: eeprom: Refactor eeprom device specifier parsing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In preparation for allowing to access eeprom by driver-model device name, refactor the eeprom device specifier parsing. Instead of filling two parameters (i2c_bus, i2c_addr), the parsing function now fills one parameter of type struct eeprom_dev_spec. Signed-off-by: Marek Behún --- cmd/eeprom.c | 77 ++++++++++++++++++++++++++++------------------------ 1 file changed, 41 insertions(+), 36 deletions(-) diff --git a/cmd/eeprom.c b/cmd/eeprom.c index 12902e812e4..e29639780de 100644 --- a/cmd/eeprom.c +++ b/cmd/eeprom.c @@ -208,41 +208,42 @@ static long parse_numeric_param(char *str) return (*endptr != '\0') ? -1 : value; } +struct eeprom_dev_spec { + int i2c_bus; + ulong i2c_addr; +}; + /** - * parse_i2c_bus_addr - parse the i2c bus and i2c devaddr parameters + * parse_eeprom_dev_spec - parse the eeprom device specifier * - * @i2c_bus: address to store the i2c bus - * @i2c_addr: address to store the device i2c address - * @argc: count of command line arguments left to parse + * @dev: pointer to eeprom device specifier + * @argc: count of command line arguments that can be used to parse + * the device specifier * @argv: command line arguments left to parse - * @argc_no_bus_addr: argc value we expect to see when bus & addr aren't given * * @returns: number of arguments parsed or CMD_RET_USAGE if error */ -static int parse_i2c_bus_addr(int *i2c_bus, ulong *i2c_addr, int argc, - char *const argv[], int argc_no_bus_addr) +static int parse_eeprom_dev_spec(struct eeprom_dev_spec *dev, int argc, + char *const argv[]) { - int argc_no_bus = argc_no_bus_addr + 1; - int argc_bus_addr = argc_no_bus_addr + 2; - #ifdef CONFIG_SYS_I2C_EEPROM_ADDR - if (argc == argc_no_bus_addr) { - *i2c_bus = -1; - *i2c_addr = CONFIG_SYS_I2C_EEPROM_ADDR; + if (argc == 0) { + dev->i2c_bus = -1; + dev->i2c_addr = CONFIG_SYS_I2C_EEPROM_ADDR; return 0; } #endif - if (argc == argc_no_bus) { - *i2c_bus = -1; - *i2c_addr = parse_numeric_param(argv[0]); + if (argc == 1) { + dev->i2c_bus = -1; + dev->i2c_addr = parse_numeric_param(argv[0]); return 1; } - if (argc == argc_bus_addr) { - *i2c_bus = parse_numeric_param(argv[0]); - *i2c_addr = parse_numeric_param(argv[1]); + if (argc == 2) { + dev->i2c_bus = parse_numeric_param(argv[0]); + dev->i2c_addr = parse_numeric_param(argv[1]); return 2; } @@ -287,9 +288,10 @@ static enum eeprom_action parse_action(char *cmd) return EEPROM_ACTION_INVALID; } -static int eeprom_execute_command(enum eeprom_action action, int i2c_bus, - ulong i2c_addr, int layout_ver, char *key, - char *value, ulong addr, ulong off, ulong cnt) +static int eeprom_execute_command(enum eeprom_action action, + struct eeprom_dev_spec *dev, + int layout_ver, char *key, char *value, + ulong addr, ulong off, ulong cnt) { int rcode = 0; const char *const fmt = @@ -301,25 +303,26 @@ static int eeprom_execute_command(enum eeprom_action action, int i2c_bus, if (action == EEPROM_ACTION_INVALID) return CMD_RET_USAGE; - eeprom_init(i2c_bus); + eeprom_init(dev->i2c_bus); if (action == EEPROM_READ) { - printf(fmt, i2c_addr, "read", addr, off, cnt); + printf(fmt, dev->i2c_addr, "read", addr, off, cnt); - rcode = eeprom_read(i2c_addr, off, (uchar *)addr, cnt); + rcode = eeprom_read(dev->i2c_addr, off, (uchar *)addr, cnt); puts("done\n"); return rcode; } else if (action == EEPROM_WRITE) { - printf(fmt, i2c_addr, "write", addr, off, cnt); + printf(fmt, dev->i2c_addr, "write", addr, off, cnt); - rcode = eeprom_write(i2c_addr, off, (uchar *)addr, cnt); + rcode = eeprom_write(dev->i2c_addr, off, (uchar *)addr, cnt); puts("done\n"); return rcode; } #ifdef CONFIG_CMD_EEPROM_LAYOUT - rcode = eeprom_read(i2c_addr, 0, eeprom_buf, CONFIG_SYS_EEPROM_SIZE); + rcode = eeprom_read(dev->i2c_addr, 0, eeprom_buf, + CONFIG_SYS_EEPROM_SIZE); if (rcode < 0) return rcode; @@ -333,7 +336,8 @@ static int eeprom_execute_command(enum eeprom_action action, int i2c_bus, layout.update(&layout, key, value); - rcode = eeprom_write(i2c_addr, 0, layout.data, CONFIG_SYS_EEPROM_SIZE); + rcode = eeprom_write(dev->i2c_addr, 0, layout.data, + CONFIG_SYS_EEPROM_SIZE); #endif return rcode; @@ -359,9 +363,9 @@ int do_eeprom(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { int layout_ver = LAYOUT_VERSION_AUTODETECT; enum eeprom_action action = EEPROM_ACTION_INVALID; - int i2c_bus = -1, index = 0; - ulong i2c_addr = -1, addr = 0, cnt = 0, off = 0; - int ret; + struct eeprom_dev_spec dev; + ulong addr = 0, cnt = 0, off = 0; + int ret, index = 0; char *field_name = ""; char *field_value = ""; @@ -386,8 +390,9 @@ int do_eeprom(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) } #endif - ret = parse_i2c_bus_addr(&i2c_bus, &i2c_addr, argc, argv + index, - eeprom_action_expected_argc(action)); + ret = parse_eeprom_dev_spec(&dev, + argc - eeprom_action_expected_argc(action), + argv + index); if (ret == CMD_RET_USAGE) return ret; @@ -411,8 +416,8 @@ int do_eeprom(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) } #endif - return eeprom_execute_command(action, i2c_bus, i2c_addr, layout_ver, - field_name, field_value, addr, off, cnt); + return eeprom_execute_command(action, &dev, layout_ver, field_name, + field_value, addr, off, cnt); } #ifdef CONFIG_EEPROM_LAYOUT_VERSIONS From e9774afedd20bfe8a18b20a63010c1d18cd16cd7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Beh=C3=BAn?= Date: Tue, 21 May 2024 09:13:33 +0200 Subject: [PATCH 09/11] cmd: eeprom: Refactor command execution into function by action MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refactor the eeprom_execute_command() function into separate functions do_eeprom_rw(), do_eeprom_print() and do_eeprom_update(). Signed-off-by: Marek Behún --- cmd/eeprom.c | 123 ++++++++++++++++++++++++++++++++------------------- 1 file changed, 77 insertions(+), 46 deletions(-) diff --git a/cmd/eeprom.c b/cmd/eeprom.c index e29639780de..c76cf431578 100644 --- a/cmd/eeprom.c +++ b/cmd/eeprom.c @@ -288,61 +288,75 @@ static enum eeprom_action parse_action(char *cmd) return EEPROM_ACTION_INVALID; } -static int eeprom_execute_command(enum eeprom_action action, - struct eeprom_dev_spec *dev, - int layout_ver, char *key, char *value, - ulong addr, ulong off, ulong cnt) +static int do_eeprom_rw(struct eeprom_dev_spec *dev, bool read, + ulong addr, ulong off, ulong cnt) { - int rcode = 0; const char *const fmt = "\nEEPROM @0x%lX %s: addr 0x%08lx off 0x%04lx count %ld ... "; -#ifdef CONFIG_CMD_EEPROM_LAYOUT - struct eeprom_layout layout; -#endif + uchar *memloc = (uchar *)addr; + int ret; - if (action == EEPROM_ACTION_INVALID) - return CMD_RET_USAGE; + printf(fmt, dev->i2c_addr, read ? "read" : "write", addr, off, cnt); + if (read) + ret = eeprom_read(dev->i2c_addr, off, memloc, cnt); + else + ret = eeprom_write(dev->i2c_addr, off, memloc, cnt); + puts("done\n"); - eeprom_init(dev->i2c_bus); - if (action == EEPROM_READ) { - printf(fmt, dev->i2c_addr, "read", addr, off, cnt); - - rcode = eeprom_read(dev->i2c_addr, off, (uchar *)addr, cnt); - - puts("done\n"); - return rcode; - } else if (action == EEPROM_WRITE) { - printf(fmt, dev->i2c_addr, "write", addr, off, cnt); - - rcode = eeprom_write(dev->i2c_addr, off, (uchar *)addr, cnt); - - puts("done\n"); - return rcode; - } + return ret; +} #ifdef CONFIG_CMD_EEPROM_LAYOUT - rcode = eeprom_read(dev->i2c_addr, 0, eeprom_buf, - CONFIG_SYS_EEPROM_SIZE); - if (rcode < 0) - return rcode; - eeprom_layout_setup(&layout, eeprom_buf, CONFIG_SYS_EEPROM_SIZE, +static int do_eeprom_layout(struct eeprom_dev_spec *dev, int layout_ver, + struct eeprom_layout *layout) +{ + int ret; + + ret = eeprom_read(dev->i2c_addr, 0, eeprom_buf, CONFIG_SYS_EEPROM_SIZE); + if (ret) + return ret; + + eeprom_layout_setup(layout, eeprom_buf, CONFIG_SYS_EEPROM_SIZE, layout_ver); - if (action == EEPROM_PRINT) { - layout.print(&layout); - return 0; - } - - layout.update(&layout, key, value); - - rcode = eeprom_write(dev->i2c_addr, 0, layout.data, - CONFIG_SYS_EEPROM_SIZE); -#endif - - return rcode; + return 0; } +static int do_eeprom_print(struct eeprom_dev_spec *dev, int layout_ver) +{ + struct eeprom_layout layout; + int ret; + + ret = do_eeprom_layout(dev, layout_ver, &layout); + if (ret) + return ret; + + layout.print(&layout); + + return 0; +} + +static int do_eeprom_update(struct eeprom_dev_spec *dev, int layout_ver, + char *key, char *value) +{ + struct eeprom_layout layout; + int ret; + + ret = do_eeprom_layout(dev, layout_ver, &layout); + if (ret) + return ret; + + ret = layout.update(&layout, key, value); + if (ret) + return CMD_RET_FAILURE; + + return eeprom_write(dev->i2c_addr, 0, layout.data, + CONFIG_SYS_EEPROM_SIZE); +} + +#endif + static int eeprom_action_expected_argc(enum eeprom_action action) { switch (action) { @@ -361,13 +375,15 @@ static int eeprom_action_expected_argc(enum eeprom_action action) #define NEXT_PARAM(argc, index) { (argc)--; (index)++; } int do_eeprom(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { - int layout_ver = LAYOUT_VERSION_AUTODETECT; enum eeprom_action action = EEPROM_ACTION_INVALID; struct eeprom_dev_spec dev; ulong addr = 0, cnt = 0, off = 0; int ret, index = 0; +#ifdef CONFIG_CMD_EEPROM_LAYOUT char *field_name = ""; char *field_value = ""; + int layout_ver = LAYOUT_VERSION_AUTODETECT; +#endif if (argc <= 1) return CMD_RET_USAGE; @@ -416,8 +432,23 @@ int do_eeprom(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) } #endif - return eeprom_execute_command(action, &dev, layout_ver, field_name, - field_value, addr, off, cnt); + eeprom_init(dev.i2c_bus); + + switch (action) { + case EEPROM_READ: + case EEPROM_WRITE: + return do_eeprom_rw(&dev, action == EEPROM_READ, + addr, off, cnt); +#ifdef CONFIG_CMD_EEPROM_LAYOUT + case EEPROM_PRINT: + return do_eeprom_print(&dev, layout_ver); + case EEPROM_UPDATE: + return do_eeprom_update(&dev, layout_ver, + field_name, field_value); +#endif + default: + return CMD_RET_USAGE; + } } #ifdef CONFIG_EEPROM_LAYOUT_VERSIONS From e5dbc80fe0e87d8a6e5226c4b3329d648fd89386 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Beh=C3=BAn?= Date: Tue, 21 May 2024 09:13:34 +0200 Subject: [PATCH 10/11] cmd: eeprom: Don't read/write whole EEPROM if not necessary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Don't read/write whole EEPROM if not necessary when printing / updating EEPROM layout fields. Only read/write layout.data_size bytes. Signed-off-by: Marek Behún --- cmd/eeprom.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/cmd/eeprom.c b/cmd/eeprom.c index c76cf431578..9c4af88738e 100644 --- a/cmd/eeprom.c +++ b/cmd/eeprom.c @@ -311,16 +311,10 @@ static int do_eeprom_rw(struct eeprom_dev_spec *dev, bool read, static int do_eeprom_layout(struct eeprom_dev_spec *dev, int layout_ver, struct eeprom_layout *layout) { - int ret; - - ret = eeprom_read(dev->i2c_addr, 0, eeprom_buf, CONFIG_SYS_EEPROM_SIZE); - if (ret) - return ret; - eeprom_layout_setup(layout, eeprom_buf, CONFIG_SYS_EEPROM_SIZE, layout_ver); - return 0; + return eeprom_read(dev->i2c_addr, 0, eeprom_buf, layout->data_size); } static int do_eeprom_print(struct eeprom_dev_spec *dev, int layout_ver) @@ -351,8 +345,7 @@ static int do_eeprom_update(struct eeprom_dev_spec *dev, int layout_ver, if (ret) return CMD_RET_FAILURE; - return eeprom_write(dev->i2c_addr, 0, layout.data, - CONFIG_SYS_EEPROM_SIZE); + return eeprom_write(dev->i2c_addr, 0, layout.data, layout.data_size); } #endif From 57a9e8d86fde5732f77577adf52dcb62fcdc068a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Beh=C3=BAn?= Date: Tue, 21 May 2024 09:13:35 +0200 Subject: [PATCH 11/11] cmd: eeprom: Extend to EEPROMs probed via driver model MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extend the 'eeprom' command to allow accessing EEPROMs probed via driver model, uclass UCLASS_I2C_EEPROM. When the CONFIG_I2C_EEPROM config option is enabled (and so the i2c-eeprom driver is built), the 'eeprom' command now accepts driver model device name as EEPROM specifier for the 'eeprom' command, in addition to the legacy [[bus] devaddr] specifier. Moreover if no device specifier is given, then the first UCLASS_I2C_EEPROM device is used, if found. Signed-off-by: Marek Behún --- cmd/eeprom.c | 122 ++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 112 insertions(+), 10 deletions(-) diff --git a/cmd/eeprom.c b/cmd/eeprom.c index 9c4af88738e..a39fc5ffdcb 100644 --- a/cmd/eeprom.c +++ b/cmd/eeprom.c @@ -22,8 +22,10 @@ #include #include #include +#include #include #include +#include #include #include @@ -209,10 +211,41 @@ static long parse_numeric_param(char *str) } struct eeprom_dev_spec { +#if CONFIG_IS_ENABLED(I2C_EEPROM) + struct udevice *dev; +#endif int i2c_bus; ulong i2c_addr; }; +static void eeprom_dev_spec_init(struct eeprom_dev_spec *dev) +{ +#if CONFIG_IS_ENABLED(I2C_EEPROM) + if (!dev->dev) +#endif + eeprom_init(dev->i2c_bus); +} + +static int eeprom_dev_spec_read(struct eeprom_dev_spec *dev, + unsigned offset, uchar *buffer, unsigned cnt) +{ +#if CONFIG_IS_ENABLED(I2C_EEPROM) + if (dev->dev) + return i2c_eeprom_read(dev->dev, offset, buffer, cnt); +#endif + return eeprom_read(dev->i2c_addr, offset, buffer, cnt); +} + +static int eeprom_dev_spec_write(struct eeprom_dev_spec *dev, + unsigned offset, uchar *buffer, unsigned cnt) +{ +#if CONFIG_IS_ENABLED(I2C_EEPROM) + if (dev->dev) + return i2c_eeprom_write(dev->dev, offset, buffer, cnt); +#endif + return eeprom_write(dev->i2c_addr, offset, buffer, cnt); +} + /** * parse_eeprom_dev_spec - parse the eeprom device specifier * @@ -226,6 +259,28 @@ struct eeprom_dev_spec { static int parse_eeprom_dev_spec(struct eeprom_dev_spec *dev, int argc, char *const argv[]) { +#if CONFIG_IS_ENABLED(I2C_EEPROM) + if (argc == 0) { + if (!uclass_first_device_err(UCLASS_I2C_EEPROM, &dev->dev)) + return 0; + } + + if (argc == 1) { + if (!uclass_get_device_by_name(UCLASS_I2C_EEPROM, argv[0], + &dev->dev)) + return 1; + + /* + * If we could not find the device by name and the parameter is + * not numeric (and so won't be handled later), fail. + */ + if (parse_numeric_param(argv[0]) == -1) { + printf("Can't get eeprom device: %s\n", argv[0]); + return CMD_RET_USAGE; + } + } +#endif + #ifdef CONFIG_SYS_I2C_EEPROM_ADDR if (argc == 0) { dev->i2c_bus = -1; @@ -265,6 +320,7 @@ static unsigned char eeprom_buf[CONFIG_SYS_EEPROM_SIZE]; #endif enum eeprom_action { + EEPROM_LIST, EEPROM_READ, EEPROM_WRITE, EEPROM_PRINT, @@ -274,6 +330,10 @@ enum eeprom_action { static enum eeprom_action parse_action(char *cmd) { +#if CONFIG_IS_ENABLED(I2C_EEPROM) + if (!strncmp(cmd, "list", 4)) + return EEPROM_LIST; +#endif if (!strncmp(cmd, "read", 4)) return EEPROM_READ; if (!strncmp(cmd, "write", 5)) @@ -288,6 +348,24 @@ static enum eeprom_action parse_action(char *cmd) return EEPROM_ACTION_INVALID; } +#if CONFIG_IS_ENABLED(I2C_EEPROM) +static int do_eeprom_list(void) +{ + struct udevice *dev; + struct uclass *uc; + int err; + + err = uclass_get(UCLASS_I2C_EEPROM, &uc); + if (err) + return CMD_RET_FAILURE; + + uclass_foreach_dev(dev, uc) + printf("%s (%s)\n", dev->name, dev->driver->name); + + return CMD_RET_SUCCESS; +} +#endif + static int do_eeprom_rw(struct eeprom_dev_spec *dev, bool read, ulong addr, ulong off, ulong cnt) { @@ -298,9 +376,9 @@ static int do_eeprom_rw(struct eeprom_dev_spec *dev, bool read, printf(fmt, dev->i2c_addr, read ? "read" : "write", addr, off, cnt); if (read) - ret = eeprom_read(dev->i2c_addr, off, memloc, cnt); + ret = eeprom_dev_spec_read(dev, off, memloc, cnt); else - ret = eeprom_write(dev->i2c_addr, off, memloc, cnt); + ret = eeprom_dev_spec_write(dev, off, memloc, cnt); puts("done\n"); return ret; @@ -314,7 +392,7 @@ static int do_eeprom_layout(struct eeprom_dev_spec *dev, int layout_ver, eeprom_layout_setup(layout, eeprom_buf, CONFIG_SYS_EEPROM_SIZE, layout_ver); - return eeprom_read(dev->i2c_addr, 0, eeprom_buf, layout->data_size); + return eeprom_dev_spec_read(dev, 0, eeprom_buf, layout->data_size); } static int do_eeprom_print(struct eeprom_dev_spec *dev, int layout_ver) @@ -345,7 +423,7 @@ static int do_eeprom_update(struct eeprom_dev_spec *dev, int layout_ver, if (ret) return CMD_RET_FAILURE; - return eeprom_write(dev->i2c_addr, 0, layout.data, layout.data_size); + return eeprom_dev_spec_write(dev, 0, layout.data, layout.data_size); } #endif @@ -353,6 +431,8 @@ static int do_eeprom_update(struct eeprom_dev_spec *dev, int layout_ver, static int eeprom_action_expected_argc(enum eeprom_action action) { switch (action) { + case EEPROM_LIST: + return 0; case EEPROM_READ: case EEPROM_WRITE: return 3; @@ -389,6 +469,11 @@ int do_eeprom(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) if (action == EEPROM_ACTION_INVALID) return CMD_RET_USAGE; +#if CONFIG_IS_ENABLED(I2C_EEPROM) + if (action == EEPROM_LIST) + return do_eeprom_list(); +#endif + #ifdef CONFIG_EEPROM_LAYOUT_VERSIONS if (action == EEPROM_PRINT || action == EEPROM_UPDATE) { if (!strcmp(argv[index], "-l")) { @@ -425,7 +510,7 @@ int do_eeprom(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) } #endif - eeprom_init(dev.i2c_bus); + eeprom_dev_spec_init(&dev); switch (action) { case EEPROM_READ: @@ -450,19 +535,37 @@ int do_eeprom(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) #define EEPROM_LAYOUT_SPEC "" #endif +#if CONFIG_IS_ENABLED(I2C_EEPROM) +# define EEPROM_DEV_SPEC "[device_specifier]" +#else +# define EEPROM_DEV_SPEC "[[bus] devaddr]" +#endif + U_BOOT_CMD( eeprom, 8, 1, do_eeprom, "EEPROM sub-system", - "read [[bus] devaddr] addr off cnt\n" - "eeprom write [[bus] devaddr] addr off cnt\n" +#if CONFIG_IS_ENABLED(I2C_EEPROM) + "list\n" + "eeprom " +#endif + "read " EEPROM_DEV_SPEC " addr off cnt\n" + "eeprom write " EEPROM_DEV_SPEC " addr off cnt\n" " - read/write `cnt' bytes from `devaddr` EEPROM at offset `off'" #ifdef CONFIG_CMD_EEPROM_LAYOUT "\n" - "eeprom print " EEPROM_LAYOUT_SPEC "[[bus] devaddr]\n" + "eeprom print " EEPROM_LAYOUT_SPEC EEPROM_DEV_SPEC "\n" " - Print layout fields and their data in human readable format\n" - "eeprom update " EEPROM_LAYOUT_SPEC "[[bus] devaddr] field_name field_value\n" + "eeprom update " EEPROM_LAYOUT_SPEC EEPROM_DEV_SPEC " field_name field_value\n" " - Update a specific eeprom field with new data.\n" " The new data must be written in the same human readable format as shown by the print command." +#endif +#if CONFIG_IS_ENABLED(I2C_EEPROM) + "\n\n" + "DEVICE SPECIFIER - the eeprom device can be specified\n" + " [dev_name] - by device name (devices can listed with the eeprom list command)\n" + " [[bus] devaddr] - or by I2C bus and I2C device address\n" + "If no device specifier is given, the first driver-model found device is used." +#endif #ifdef CONFIG_EEPROM_LAYOUT_VERSIONS "\n\n" "LAYOUT VERSIONS\n" @@ -471,5 +574,4 @@ U_BOOT_CMD( "The values which can be provided with the -l option are:\n" CONFIG_EEPROM_LAYOUT_HELP_STRING"\n" #endif -#endif );