Tim Wawrzynczak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45939 )
Change subject: fw_config: Convert fw_config to a 64-bit field ......................................................................
fw_config: Convert fw_config to a 64-bit field
We all knew this was coming, 32 bits is never enough. Doing this early so that it doesn't affect too much code yet. Take care of every usage of fw_config throughout the codebase so the conversion is all done at once.
BUG=b:169668368 TEST=added 0x1 to SSFC and 0x201 to FW_CONFIG from ectool on Volteer. Verified via console print that FW_CONFIG is 0x100000201.
Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org Change-Id: I6f2065d347eafa0ef7b346caeabdc3b626402092 --- M src/ec/google/chromeec/ec.c M src/ec/google/chromeec/ec.h M src/include/fw_config.h M src/lib/fw_config.c M src/mainboard/google/dedede/board_info.c M src/mainboard/google/dedede/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/zork/variants/baseboard/helpers.c M util/sconfig/main.c 8 files changed, 50 insertions(+), 36 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/45939/1
diff --git a/src/ec/google/chromeec/ec.c b/src/ec/google/chromeec/ec.c index 40285dc..f9e8c7d 100644 --- a/src/ec/google/chromeec/ec.c +++ b/src/ec/google/chromeec/ec.c @@ -841,9 +841,20 @@ return cbi_get_uint32(id, CBI_TAG_SKU_ID); }
-int google_chromeec_cbi_get_fw_config(uint32_t *fw_config) +/* ChromeOS firmware is treating FW_CONFIG & SSFC as one 64-bit field */ +int google_chromeec_cbi_get_fw_config(uint64_t *fw_config) { - return cbi_get_uint32(fw_config, CBI_TAG_FW_CONFIG); + uint32_t config; + uint32_t ssfc; + + if (cbi_get_uint32(&config, CBI_TAG_FW_CONFIG)) + return -1; + + if (cbi_get_uint32(&ssfc, CBI_TAG_SSFC)) + return -1; + + *fw_config = (uint64_t)((uint64_t)ssfc << 32) | (uint64_t)config; + return 0; }
int google_chromeec_cbi_get_oem_id(uint32_t *id) diff --git a/src/ec/google/chromeec/ec.h b/src/ec/google/chromeec/ec.h index 9d4e588..d074d1a 100644 --- a/src/ec/google/chromeec/ec.h +++ b/src/ec/google/chromeec/ec.h @@ -74,7 +74,7 @@ */ int google_chromeec_cbi_get_oem_id(uint32_t *id); int google_chromeec_cbi_get_sku_id(uint32_t *id); -int google_chromeec_cbi_get_fw_config(uint32_t *fw_config); +int google_chromeec_cbi_get_fw_config(uint64_t *fw_config); int google_chromeec_cbi_get_dram_part_num(char *buf, size_t bufsize); int google_chromeec_cbi_get_oem_name(char *buf, size_t bufsize); /* version may be stored in CBI as a smaller integer width, but the EC code diff --git a/src/include/fw_config.h b/src/include/fw_config.h index 81980b9..494ce7f 100644 --- a/src/include/fw_config.h +++ b/src/include/fw_config.h @@ -18,8 +18,8 @@ struct fw_config { const char *field_name; const char *option_name; - uint32_t mask; - uint32_t value; + uint64_t mask; + uint64_t value; };
/* Generate a pointer to a compound literal of the fw_config structure. */ @@ -53,7 +53,7 @@ * * Return pointer to cached `struct fw_config` if successfully probed, otherwise NULL. */ -const struct fw_config *fw_config_get_found(uint32_t field_mask); +const struct fw_config *fw_config_get_found(uint64_t field_mask);
#else
diff --git a/src/lib/fw_config.c b/src/lib/fw_config.c index fdfab0a..a4aa777 100644 --- a/src/lib/fw_config.c +++ b/src/lib/fw_config.c @@ -7,6 +7,7 @@ #include <device/device.h> #include <ec/google/chromeec/ec.h> #include <fw_config.h> +#include <inttypes.h> #include <lib.h> #include <stdbool.h> #include <stdint.h> @@ -14,11 +15,11 @@ /** * fw_config_get() - Provide firmware configuration value. * - * Return 32bit firmware configuration value determined for the system. + * Return 64bit firmware configuration value determined for the system. */ -static uint32_t fw_config_get(void) +static uint64_t fw_config_get(void) { - static uint32_t fw_config_value; + static uint64_t fw_config_value; static bool fw_config_value_initialized;
/* Nothing to prepare if setup is already done. */ @@ -35,7 +36,7 @@ __func__); fw_config_value = 0; } else { - printk(BIOS_INFO, "FW_CONFIG value from CBFS is 0x%08x\n", + printk(BIOS_INFO, "FW_CONFIG value from CBFS is 0x%" PRIx64 "\n", fw_config_value); return fw_config_value; } @@ -47,7 +48,7 @@ printk(BIOS_WARNING, "%s: Could not get fw_config from EC\n", __func__); }
- printk(BIOS_INFO, "FW_CONFIG value is 0x%08x\n", fw_config_value); + printk(BIOS_INFO, "FW_CONFIG value is 0x%" PRIx64 "\n", fw_config_value); return fw_config_value; }
@@ -59,7 +60,8 @@ printk(BIOS_INFO, "fw_config match found: %s=%s\n", match->field_name, match->option_name); else - printk(BIOS_INFO, "fw_config match found: mask=0x%08x value=0x%08x\n", + printk(BIOS_INFO, "fw_config match found: mask=0x%" PRIx64 " value=0x%" + PRIx64 "\n", match->mask, match->value); return true; } @@ -70,20 +72,20 @@ #if ENV_RAMSTAGE
/* - * The maximum number of fw_config fields is limited by the 32-bit mask that is used to + * The maximum number of fw_config fields is limited by the 64-bit mask that is used to * represent them. */ -#define MAX_CACHE_ELEMENTS (8 * sizeof(uint32_t)) +#define MAX_CACHE_ELEMENTS (8 * sizeof(uint64_t))
static const struct fw_config *cached_configs[MAX_CACHE_ELEMENTS];
-static size_t probe_index(uint32_t mask) +static size_t probe_index(uint64_t mask) { assert(mask); - return __ffs(mask); + return __ffs_64(mask); }
-const struct fw_config *fw_config_get_found(uint32_t field_mask) +const struct fw_config *fw_config_get_found(uint64_t field_mask) { const struct fw_config *config; config = cached_configs[probe_index(field_mask)]; diff --git a/src/mainboard/google/dedede/board_info.c b/src/mainboard/google/dedede/board_info.c index fdb4b5f..22d35d7 100644 --- a/src/mainboard/google/dedede/board_info.c +++ b/src/mainboard/google/dedede/board_info.c @@ -3,7 +3,7 @@ #include <baseboard/variants.h> #include <ec/google/chromeec/ec.h>
-int board_info_get_fw_config(uint32_t *fw_config) +int board_info_get_fw_config(uint64_t *fw_config) { return google_chromeec_cbi_get_fw_config(fw_config); } diff --git a/src/mainboard/google/dedede/variants/baseboard/include/baseboard/variants.h b/src/mainboard/google/dedede/variants/baseboard/include/baseboard/variants.h index bd94ef4..44b2b72 100644 --- a/src/mainboard/google/dedede/variants/baseboard/include/baseboard/variants.h +++ b/src/mainboard/google/dedede/variants/baseboard/include/baseboard/variants.h @@ -21,7 +21,7 @@ * @param fw_config Address where the fw_config is stored. * @return 0 on success or negative integer for errors. */ -int board_info_get_fw_config(uint32_t *fw_config); +int board_info_get_fw_config(uint64_t *fw_config);
/* Return memory configuration structure. */ const struct mb_cfg *variant_memcfg_config(void); diff --git a/src/mainboard/google/zork/variants/baseboard/helpers.c b/src/mainboard/google/zork/variants/baseboard/helpers.c index d95ab82..63ff24a 100644 --- a/src/mainboard/google/zork/variants/baseboard/helpers.c +++ b/src/mainboard/google/zork/variants/baseboard/helpers.c @@ -48,9 +48,9 @@ FW_CONFIG_SHIFT_FAN = 27, };
-static int get_fw_config(uint32_t *val) +static int get_fw_config(uint64_t *val) { - static uint32_t known_value; + static uint64_t known_value;
if (known_value) { *val = known_value; @@ -67,9 +67,9 @@ return 0; }
-static unsigned int extract_field(uint32_t mask, int shift) +static unsigned int extract_field(uint64_t mask, int shift) { - uint32_t fw_config; + uint64_t fw_config;
/* On errors nothing is assumed to be set. */ if (get_fw_config(&fw_config)) diff --git a/util/sconfig/main.c b/util/sconfig/main.c index d5504cd..30e08c3 100644 --- a/util/sconfig/main.c +++ b/util/sconfig/main.c @@ -4,8 +4,9 @@ #include <assert.h> #include <ctype.h> #include <getopt.h> -/* stat.h needs to be included before commonlib/helpers.h to avoid errors.*/ +#include <inttypes.h> #include <libgen.h> +/* stat.h needs to be included before commonlib/helpers.h to avoid errors.*/ #include <sys/stat.h> #include <commonlib/helpers.h> #include <stdint.h> @@ -402,8 +403,8 @@ { struct fw_config_field *field = find_fw_config_field(name);
- /* Check that field is within 32bits. */ - if (start_bit > end_bit || end_bit > 31) { + /* Check that field is within 64 bits. */ + if (start_bit > end_bit || end_bit > 63) { printf("ERROR: fw_config field %s has invalid range %u-%u\n", name, start_bit, end_bit); exit(1); @@ -455,12 +456,12 @@ void add_fw_config_option(struct fw_config_field *field, const char *name, unsigned int value) { struct fw_config_option *option; - uint32_t field_max_value; + uint64_t field_max_value;
/* Check that option value fits within field mask. */ - field_max_value = (1 << (1 + field->end_bit - field->start_bit)) - 1; - if (value > field_max_value) { - printf("ERROR: fw_config option %s:%s value %u larger than field max %u\n", + field_max_value = (1ull << (1ull + field->end_bit - field->start_bit)) - 1ull; + if ((uint64_t)value > field_max_value) { + printf("ERROR: fw_config option %s:%s value %u larger than field max %" PRIx64 "\n", field->name, name, value, field_max_value); exit(1); } @@ -532,23 +533,23 @@
while (field) { struct fw_config_option *option = field->options; - uint32_t mask; + uint64_t mask;
fprintf(fil, "#define FW_CONFIG_FIELD_%s_NAME "%s"\n", field->name, field->name);
/* Compute mask from start and end bit. */ - mask = ((1 << (1 + field->end_bit - field->start_bit)) - 1); + mask = ((1ull << (1ull + field->end_bit - field->start_bit)) - 1ull); mask <<= field->start_bit;
- fprintf(fil, "#define FW_CONFIG_FIELD_%s_MASK 0x%08x\n", + fprintf(fil, "#define FW_CONFIG_FIELD_%s_MASK 0x%" PRIx64 "\n", field->name, mask);
while (option) { fprintf(fil, "#define FW_CONFIG_FIELD_%s_OPTION_%s_NAME "%s"\n", field->name, option->name, option->name); - fprintf(fil, "#define FW_CONFIG_FIELD_%s_OPTION_%s_VALUE 0x%08x\n", - field->name, option->name, option->value << field->start_bit); + fprintf(fil, "#define FW_CONFIG_FIELD_%s_OPTION_%s_VALUE 0x%" PRIx64 "\n", + field->name, option->name, (uint64_t)(option->value << (uint64_t)field->start_bit));
option = option->next; } @@ -569,7 +570,7 @@ /* Find matching field. */ struct fw_config_field *field; struct fw_config_option *option; - uint32_t mask, value; + uint64_t mask, value;
field = find_fw_config_field(probe->field); if (!field) {
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45939 )
Change subject: fw_config: Convert fw_config to a 64-bit field ......................................................................
Patch Set 1:
(3 comments)
https://review.coreboot.org/c/coreboot/+/45939/1/util/sconfig/main.c File util/sconfig/main.c:
https://review.coreboot.org/c/coreboot/+/45939/1/util/sconfig/main.c@464 PS1, Line 464: printf("ERROR: fw_config option %s:%s value %u larger than field max %" PRIx64 "\n", line over 96 characters
https://review.coreboot.org/c/coreboot/+/45939/1/util/sconfig/main.c@551 PS1, Line 551: fprintf(fil, "#define FW_CONFIG_FIELD_%s_OPTION_%s_VALUE 0x%" PRIx64 "\n", line over 96 characters
https://review.coreboot.org/c/coreboot/+/45939/1/util/sconfig/main.c@552 PS1, Line 552: field->name, option->name, (uint64_t)(option->value << (uint64_t)field->start_bit)); line over 96 characters
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45939 )
Change subject: fw_config: Convert fw_config to a 64-bit field ......................................................................
Patch Set 1:
There are also a few "32bit" references in Documentation/lib/fw_config.md
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45939 )
Change subject: fw_config: Convert fw_config to a 64-bit field ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45939/1/src/ec/google/chromeec/ec.c File src/ec/google/chromeec/ec.c:
https://review.coreboot.org/c/coreboot/+/45939/1/src/ec/google/chromeec/ec.c... PS1, Line 854: return -1 Isn't this going to break getting the fw_config on all existing platforms that don't have the SSFC field? Maybe add a Kconfig option to control this?
uint32_t ssfc = 0; if (cbi_get_uint32(&ssfc, CBI_TAG_SSFC) && !CONFIG(FW_CONFIG_32_BITS)) return -1;
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45939 )
Change subject: fw_config: Convert fw_config to a 64-bit field ......................................................................
Patch Set 1:
(1 comment)
Patch Set 1:
There are also a few "32bit" references in Documentation/lib/fw_config.md
Done.
https://review.coreboot.org/c/coreboot/+/45939/1/src/ec/google/chromeec/ec.c File src/ec/google/chromeec/ec.c:
https://review.coreboot.org/c/coreboot/+/45939/1/src/ec/google/chromeec/ec.c... PS1, Line 854: return -1
Isn't this going to break getting the fw_config on all existing platforms that don't have the SSFC f […]
Yeah you're right. How about making it "optional"? As in, if `cbi_get_uint32(&ssfc, CBI_TAG_SSFC` returns an error, then don't return -1 here, but just use 0 for `ssfc` instead?
Hello build bot (Jenkins), Martin Roth, Furquan Shaikh, Patrick Georgi, Caveh Jalali, Duncan Laurie, Julius Werner, Nick Vaccaro, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45939
to look at the new patch set (#2).
Change subject: fw_config: Convert fw_config to a 64-bit field ......................................................................
fw_config: Convert fw_config to a 64-bit field
We all knew this was coming, 32 bits is never enough. Doing this early so that it doesn't affect too much code yet. Take care of every usage of fw_config throughout the codebase so the conversion is all done at once.
BUG=b:169668368 TEST=added 0x1 to SSFC and 0x201 to FW_CONFIG from ectool on Volteer. Verified via console print that FW_CONFIG is 0x100000201.
Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org Change-Id: I6f2065d347eafa0ef7b346caeabdc3b626402092 --- M Documentation/lib/fw_config.md M src/ec/google/chromeec/ec.c M src/ec/google/chromeec/ec.h M src/include/fw_config.h M src/lib/fw_config.c M src/mainboard/google/dedede/board_info.c M src/mainboard/google/dedede/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/zork/variants/baseboard/helpers.c M util/sconfig/main.c 9 files changed, 57 insertions(+), 41 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/45939/2
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45939 )
Change subject: fw_config: Convert fw_config to a 64-bit field ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45939/2/util/sconfig/main.c File util/sconfig/main.c:
https://review.coreboot.org/c/coreboot/+/45939/2/util/sconfig/main.c@406 PS2, Line 406: /* Check that field is within 64 bits. */ do we need to limit field width to 32 bits? what about fields spanning the 32 bit boundary?
https://review.coreboot.org/c/coreboot/+/45939/2/util/sconfig/main.c@554 PS2, Line 554: (uint64_t)(option->value << (uint64_t)field->start_bit) hmm... looks like you're casting to uint64_t after the bits are truncated:
(uint64_t)option->value << field->start_bit
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45939 )
Change subject: fw_config: Convert fw_config to a 64-bit field ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45939/1/src/ec/google/chromeec/ec.c File src/ec/google/chromeec/ec.c:
https://review.coreboot.org/c/coreboot/+/45939/1/src/ec/google/chromeec/ec.c... PS1, Line 854: return -1
Yeah you're right. […]
I think this should be good. It's not *impossible* that both fields are present and we get an error reading only the second field, but the chances are probably exceedingly small.
Hello build bot (Jenkins), Martin Roth, Furquan Shaikh, Patrick Georgi, Caveh Jalali, Duncan Laurie, Julius Werner, Nick Vaccaro, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45939
to look at the new patch set (#3).
Change subject: fw_config: Convert fw_config to a 64-bit field ......................................................................
fw_config: Convert fw_config to a 64-bit field
We all knew this was coming, 32 bits is never enough. Doing this early so that it doesn't affect too much code yet. Take care of every usage of fw_config throughout the codebase so the conversion is all done at once.
BUG=b:169668368 TEST=added 0x1 to SSFC and 0x201 to FW_CONFIG from ectool on Volteer. Verified via console print that FW_CONFIG is 0x100000201.
Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org Change-Id: I6f2065d347eafa0ef7b346caeabdc3b626402092 --- M Documentation/lib/fw_config.md M src/ec/google/chromeec/ec.c M src/ec/google/chromeec/ec.h M src/include/fw_config.h M src/lib/fw_config.c M src/mainboard/google/dedede/board_info.c M src/mainboard/google/dedede/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/zork/variants/baseboard/helpers.c M util/sconfig/main.c 9 files changed, 57 insertions(+), 41 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/45939/3
Hello build bot (Jenkins), Martin Roth, Furquan Shaikh, Patrick Georgi, Caveh Jalali, Duncan Laurie, Julius Werner, Nick Vaccaro, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45939
to look at the new patch set (#4).
Change subject: fw_config: Convert fw_config to a 64-bit field ......................................................................
fw_config: Convert fw_config to a 64-bit field
We all knew this was coming, 32 bits is never enough. Doing this early so that it doesn't affect too much code yet. Take care of every usage of fw_config throughout the codebase so the conversion is all done at once.
BUG=b:169668368 TEST=added 0x1 to SSFC and 0x201 to FW_CONFIG from ectool on Volteer. Verified via console print that FW_CONFIG is 0x100000201.
Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org Change-Id: I6f2065d347eafa0ef7b346caeabdc3b626402092 --- M Documentation/lib/fw_config.md M src/ec/google/chromeec/ec.c M src/ec/google/chromeec/ec.h M src/include/fw_config.h M src/lib/fw_config.c M src/mainboard/google/dedede/board_info.c M src/mainboard/google/dedede/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/zork/variants/baseboard/helpers.c M util/sconfig/main.c M util/sconfig/sconfig.h M util/sconfig/sconfig.y 11 files changed, 64 insertions(+), 46 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/45939/4
Hello build bot (Jenkins), Martin Roth, Furquan Shaikh, Patrick Georgi, Caveh Jalali, Duncan Laurie, Julius Werner, Nick Vaccaro, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45939
to look at the new patch set (#5).
Change subject: fw_config: Convert fw_config to a 64-bit field ......................................................................
fw_config: Convert fw_config to a 64-bit field
We all knew this was coming, 32 bits is never enough. Doing this early so that it doesn't affect too much code yet. Take care of every usage of fw_config throughout the codebase so the conversion is all done at once.
BUG=b:169668368 TEST=added 0x1 to SSFC and 0x201 to FW_CONFIG from ectool on Volteer. Verified via console print that FW_CONFIG is 0x100000201.
Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org Change-Id: I6f2065d347eafa0ef7b346caeabdc3b626402092 --- M Documentation/lib/fw_config.md M src/ec/google/chromeec/ec.c M src/ec/google/chromeec/ec.h M src/include/fw_config.h M src/lib/fw_config.c M src/mainboard/google/dedede/board_info.c M src/mainboard/google/dedede/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/zork/variants/baseboard/helpers.c M util/sconfig/main.c M util/sconfig/sconfig.h M util/sconfig/sconfig.y 11 files changed, 64 insertions(+), 46 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/45939/5
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45939 )
Change subject: fw_config: Convert fw_config to a 64-bit field ......................................................................
Patch Set 4:
(3 comments)
https://review.coreboot.org/c/coreboot/+/45939/1/src/ec/google/chromeec/ec.c File src/ec/google/chromeec/ec.c:
https://review.coreboot.org/c/coreboot/+/45939/1/src/ec/google/chromeec/ec.c... PS1, Line 854: return -1
I think this should be good. […]
and given the current EC API, I'm not sure we can distinguish between the cases.
https://review.coreboot.org/c/coreboot/+/45939/2/util/sconfig/main.c File util/sconfig/main.c:
https://review.coreboot.org/c/coreboot/+/45939/2/util/sconfig/main.c@406 PS2, Line 406: /* Check that field is within 64 bits. */
do we need to limit field width to 32 bits? […]
Why is that a problem?
https://review.coreboot.org/c/coreboot/+/45939/2/util/sconfig/main.c@554 PS2, Line 554: (uint64_t)(option->value << (uint64_t)field->start_bit)
hmm... looks like you're casting to uint64_t after the bits are truncated: […]
You raise a good point, I think actually perhaps option->value should be a 64-bit number, but I'm not sure if that means the parser needs to be changed as well, need to check on that.
Hello build bot (Jenkins), Martin Roth, Furquan Shaikh, Patrick Georgi, Caveh Jalali, Duncan Laurie, Julius Werner, Nick Vaccaro, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45939
to look at the new patch set (#6).
Change subject: fw_config: Convert fw_config to a 64-bit field ......................................................................
fw_config: Convert fw_config to a 64-bit field
We all knew this was coming, 32 bits is never enough. Doing this early so that it doesn't affect too much code yet. Take care of every usage of fw_config throughout the codebase so the conversion is all done at once.
BUG=b:169668368 TEST=added 0x1 to SSFC and 0x201 to FW_CONFIG from ectool on Volteer. Verified via console print that FW_CONFIG is 0x100000201.
Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org Change-Id: I6f2065d347eafa0ef7b346caeabdc3b626402092 --- M Documentation/lib/fw_config.md M src/ec/google/chromeec/ec.c M src/ec/google/chromeec/ec.h M src/include/fw_config.h M src/lib/fw_config.c M src/mainboard/google/dedede/board_info.c M src/mainboard/google/dedede/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/zork/variants/baseboard/helpers.c M util/sconfig/main.c M util/sconfig/sconfig.h M util/sconfig/sconfig.y 11 files changed, 64 insertions(+), 46 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/45939/6
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45939 )
Change subject: fw_config: Convert fw_config to a 64-bit field ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45939/2/util/sconfig/main.c File util/sconfig/main.c:
https://review.coreboot.org/c/coreboot/+/45939/2/util/sconfig/main.c@406 PS2, Line 406: /* Check that field is within 64 bits. */
Why is that a problem?
google_chromeec_cbi_get_fw_config() composes 2 seemingly unrelated 32-bit CBI values into a 64-bit value. it seems a bit fragile to allow fields to span these 32-bit values.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45939 )
Change subject: fw_config: Convert fw_config to a 64-bit field ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45939/2/util/sconfig/main.c File util/sconfig/main.c:
https://review.coreboot.org/c/coreboot/+/45939/2/util/sconfig/main.c@406 PS2, Line 406: /* Check that field is within 64 bits. */
google_chromeec_cbi_get_fw_config() composes 2 seemingly […]
But FW_CONFIG doesn't have to come from the chrome EC, it can also be sourced from a file in CBFS (FW_CONFIG_SOURCE_CBFS). There could be other sources in the future too.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45939 )
Change subject: fw_config: Convert fw_config to a 64-bit field ......................................................................
Patch Set 8:
ping 😊
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45939 )
Change subject: fw_config: Convert fw_config to a 64-bit field ......................................................................
Patch Set 9:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45939/9/src/ec/google/chromeec/ec.c File src/ec/google/chromeec/ec.c:
https://review.coreboot.org/c/coreboot/+/45939/9/src/ec/google/chromeec/ec.c... PS9, Line 853: if (cbi_get_uint32(&ssfc, CBI_TAG_SSFC)) I think the whole SSFC is still under review to evaluate whether it is really required. I am not sure where we will end up on it, but we might have to support a single FW_CONFIG field which is 64-bit wide instead of 32-bit FW_CONFIG and 32-bit SSFC.
I am thinking should we just assume there is no SSFC for now and fill 0s in the upper 32-bit? When we get to supporting SSFC, it should be easy to add this call to get CBI_TAG_SSFC.
https://review.coreboot.org/c/coreboot/+/45939/9/util/sconfig/main.c File util/sconfig/main.c:
https://review.coreboot.org/c/coreboot/+/45939/9/util/sconfig/main.c@456 PS9, Line 456: uint64_t value The way SSFC support is being added in this CL expects that the devicetree representation takes care of transforming the SSFC fields into a 64-bit field when defining the fw_config field and options. We had some discussion (captured in b/169182605) and the conclusion was that we should allow mainboard devicetree to represent fw_config and fw_source separately and have sconfig handle the transformation from 2 32-bit fields into a single 64-bit field. This would allow the devicetree field/options to match what is really configured in CBI without having to handle that manually.
Given that the fate of SSFC is still undetermined, I think we can continue with what you have. But, if we ever decide to support SSFC as a separate field, we might have to support in sconfig to handle fw_source as well.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45939 )
Change subject: fw_config: Convert fw_config to a 64-bit field ......................................................................
Patch Set 9:
(4 comments)
https://review.coreboot.org/c/coreboot/+/45939/9/src/ec/google/chromeec/ec.c File src/ec/google/chromeec/ec.c:
https://review.coreboot.org/c/coreboot/+/45939/9/src/ec/google/chromeec/ec.c... PS9, Line 853: if (cbi_get_uint32(&ssfc, CBI_TAG_SSFC))
I think the whole SSFC is still under review to evaluate whether it is really required. […]
SGTM.
https://review.coreboot.org/c/coreboot/+/45939/2/util/sconfig/main.c File util/sconfig/main.c:
https://review.coreboot.org/c/coreboot/+/45939/2/util/sconfig/main.c@406 PS2, Line 406: /* Check that field is within 64 bits. */
But FW_CONFIG doesn't have to come from the chrome EC, it can also be sourced from a file in CBFS (F […]
Ack
https://review.coreboot.org/c/coreboot/+/45939/2/util/sconfig/main.c@554 PS2, Line 554: (uint64_t)(option->value << (uint64_t)field->start_bit)
You raise a good point, I think actually perhaps option->value should be a 64-bit number, but I'm no […]
Done
https://review.coreboot.org/c/coreboot/+/45939/9/util/sconfig/main.c File util/sconfig/main.c:
https://review.coreboot.org/c/coreboot/+/45939/9/util/sconfig/main.c@456 PS9, Line 456: uint64_t value
The way SSFC support is being added in this CL expects that the devicetree representation takes care of transforming the SSFC fields into a 64-bit field when defining the fw_config field and options. We had some discussion (captured in b/169182605) and the conclusion was that we should allow mainboard devicetree to represent fw_config and fw_source separately and have sconfig handle the transformation from 2 32-bit fields into a single 64-bit field. This would allow the devicetree field/options to match what is really configured in CBI without having to handle that manually.
Upgrading FW_CONFIG in general to 64-bit support means that when FW_CONFIG is supplied from CBFS, devtree won't limit you to dealing with a 32-bit field 😊 so regardless of fw_source vs. SSFC-as-upper-32-bits, I think the 64-bit support here is needed for that reason anyway
Given that the fate of SSFC is still undetermined, I think we can continue with what you have. But, if we ever decide to support SSFC as a separate field, we might have to support in sconfig to handle fw_source as well.
Sure, that more or less becomes a clone of the fw_config infrastructure.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45939 )
Change subject: fw_config: Convert fw_config to a 64-bit field ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45939/9/util/sconfig/main.c File util/sconfig/main.c:
https://review.coreboot.org/c/coreboot/+/45939/9/util/sconfig/main.c@456 PS9, Line 456: uint64_t value
Upgrading FW_CONFIG in general to 64-bit support means that when FW_CONFIG is supplied from CBFS, devtree won't limit you to dealing with a 32-bit field 😊 so regardless of fw_source vs. SSFC-as-upper-32-bits, I think the 64-bit support here is needed for that reason anyway
I agree. Moving to 64 bits for FW_CONFIG is independent of how it is really organized in CBI or CBFS. I was just pointing out that we might have to add some more support in sconfig if we have to support 32-bit SSFC + 32-bit FW_CONFIG in devicetree representation in the future.
Hello build bot (Jenkins), Furquan Shaikh, Martin Roth, Patrick Georgi, Caveh Jalali, Duncan Laurie, Julius Werner, Nick Vaccaro, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45939
to look at the new patch set (#10).
Change subject: fw_config: Convert fw_config to a 64-bit field ......................................................................
fw_config: Convert fw_config to a 64-bit field
We all knew this was coming, 32 bits is never enough. Doing this early so that it doesn't affect too much code yet. Take care of every usage of fw_config throughout the codebase so the conversion is all done at once.
BUG=b:169668368 TEST=Hacked up this code to OR 0x1_000_0000 with CBI-sourced FW_CONFIG and verify the console print contained that bit.
Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org Change-Id: I6f2065d347eafa0ef7b346caeabdc3b626402092 --- M Documentation/lib/fw_config.md M src/ec/google/chromeec/ec.c M src/ec/google/chromeec/ec.h M src/include/fw_config.h M src/lib/fw_config.c M src/mainboard/google/dedede/board_info.c M src/mainboard/google/dedede/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/zork/variants/baseboard/helpers.c M util/sconfig/main.c M util/sconfig/sconfig.h M util/sconfig/sconfig.y 11 files changed, 60 insertions(+), 46 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/45939/10
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45939 )
Change subject: fw_config: Convert fw_config to a 64-bit field ......................................................................
Patch Set 10: Code-Review+2
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45939 )
Change subject: fw_config: Convert fw_config to a 64-bit field ......................................................................
Patch Set 10:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45939/9/src/ec/google/chromeec/ec.c File src/ec/google/chromeec/ec.c:
https://review.coreboot.org/c/coreboot/+/45939/9/src/ec/google/chromeec/ec.c... PS9, Line 853: if (cbi_get_uint32(&ssfc, CBI_TAG_SSFC))
SGTM.
Done
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45939 )
Change subject: fw_config: Convert fw_config to a 64-bit field ......................................................................
Patch Set 10: Code-Review+2
Tim Wawrzynczak has submitted this change. ( https://review.coreboot.org/c/coreboot/+/45939 )
Change subject: fw_config: Convert fw_config to a 64-bit field ......................................................................
fw_config: Convert fw_config to a 64-bit field
We all knew this was coming, 32 bits is never enough. Doing this early so that it doesn't affect too much code yet. Take care of every usage of fw_config throughout the codebase so the conversion is all done at once.
BUG=b:169668368 TEST=Hacked up this code to OR 0x1_000_0000 with CBI-sourced FW_CONFIG and verify the console print contained that bit.
Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org Change-Id: I6f2065d347eafa0ef7b346caeabdc3b626402092 Reviewed-on: https://review.coreboot.org/c/coreboot/+/45939 Reviewed-by: Furquan Shaikh furquan@google.com Reviewed-by: Angel Pons th3fanbus@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M Documentation/lib/fw_config.md M src/ec/google/chromeec/ec.c M src/ec/google/chromeec/ec.h M src/include/fw_config.h M src/lib/fw_config.c M src/mainboard/google/dedede/board_info.c M src/mainboard/google/dedede/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/zork/variants/baseboard/helpers.c M util/sconfig/main.c M util/sconfig/sconfig.h M util/sconfig/sconfig.y 11 files changed, 60 insertions(+), 46 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved Angel Pons: Looks good to me, approved
diff --git a/Documentation/lib/fw_config.md b/Documentation/lib/fw_config.md index 63a56dc..dcf1bb4e 100644 --- a/Documentation/lib/fw_config.md +++ b/Documentation/lib/fw_config.md @@ -73,18 +73,18 @@
## Firmware Configuration Value
-The 32bit value used as the firmware configuration bitmask is meant to be determined at runtime +The 64-bit value used as the firmware configuration bitmask is meant to be determined at runtime but could also be defined at compile time if needed.
There are two supported sources for providing this information to coreboot.
### CBFS
-The value can be provided with a 32bit raw value in CBFS that is read by coreboot. The value +The value can be provided with a 64-bit raw value in CBFS that is read by coreboot. The value can be set at build time but also adjusted in an existing image with `cbfstool`.
To enable this select the `CONFIG_FW_CONFIG_CBFS` option in the build configuration and add a -raw 32bit value to CBFS with the name of the current prefix at `CONFIG_FW_PREFIX/fw_config`. +raw 64-bit value to CBFS with the name of the current prefix at `CONFIG_FW_PREFIX/fw_config`.
When `fw_config_probe_device()` or `fw_config_probe()` is called it will look for the specified file in CBFS use the value it contains when matching fields and options. @@ -291,8 +291,8 @@ struct fw_config { const char *field_name; const char *option_name; - uint32_t mask; - uint32_t value; + uint64_t mask; + uint64_t value; }; ```
diff --git a/src/ec/google/chromeec/ec.c b/src/ec/google/chromeec/ec.c index 39cf895..2ffccbc7 100644 --- a/src/ec/google/chromeec/ec.c +++ b/src/ec/google/chromeec/ec.c @@ -841,9 +841,16 @@ return cbi_get_uint32(id, CBI_TAG_SKU_ID); }
-int google_chromeec_cbi_get_fw_config(uint32_t *fw_config) +int google_chromeec_cbi_get_fw_config(uint64_t *fw_config) { - return cbi_get_uint32(fw_config, CBI_TAG_FW_CONFIG); + uint32_t config; + + if (cbi_get_uint32(&config, CBI_TAG_FW_CONFIG)) + return -1; + + /* FIXME: Yet to determine source of other 32 bits... */ + *fw_config = (uint64_t)config; + return 0; }
int google_chromeec_cbi_get_oem_id(uint32_t *id) diff --git a/src/ec/google/chromeec/ec.h b/src/ec/google/chromeec/ec.h index c2ceff8..bed8594 100644 --- a/src/ec/google/chromeec/ec.h +++ b/src/ec/google/chromeec/ec.h @@ -83,7 +83,7 @@ */ int google_chromeec_cbi_get_oem_id(uint32_t *id); int google_chromeec_cbi_get_sku_id(uint32_t *id); -int google_chromeec_cbi_get_fw_config(uint32_t *fw_config); +int google_chromeec_cbi_get_fw_config(uint64_t *fw_config); int google_chromeec_cbi_get_dram_part_num(char *buf, size_t bufsize); int google_chromeec_cbi_get_oem_name(char *buf, size_t bufsize); /* version may be stored in CBI as a smaller integer width, but the EC code diff --git a/src/include/fw_config.h b/src/include/fw_config.h index 81980b9..494ce7f 100644 --- a/src/include/fw_config.h +++ b/src/include/fw_config.h @@ -18,8 +18,8 @@ struct fw_config { const char *field_name; const char *option_name; - uint32_t mask; - uint32_t value; + uint64_t mask; + uint64_t value; };
/* Generate a pointer to a compound literal of the fw_config structure. */ @@ -53,7 +53,7 @@ * * Return pointer to cached `struct fw_config` if successfully probed, otherwise NULL. */ -const struct fw_config *fw_config_get_found(uint32_t field_mask); +const struct fw_config *fw_config_get_found(uint64_t field_mask);
#else
diff --git a/src/lib/fw_config.c b/src/lib/fw_config.c index ec32059..0973cbe 100644 --- a/src/lib/fw_config.c +++ b/src/lib/fw_config.c @@ -7,6 +7,7 @@ #include <device/device.h> #include <ec/google/chromeec/ec.h> #include <fw_config.h> +#include <inttypes.h> #include <lib.h> #include <stdbool.h> #include <stdint.h> @@ -14,11 +15,11 @@ /** * fw_config_get() - Provide firmware configuration value. * - * Return 32bit firmware configuration value determined for the system. + * Return 64bit firmware configuration value determined for the system. */ -static uint32_t fw_config_get(void) +static uint64_t fw_config_get(void) { - static uint32_t fw_config_value; + static uint64_t fw_config_value; static bool fw_config_value_initialized;
/* Nothing to prepare if setup is already done. */ @@ -35,7 +36,7 @@ __func__); fw_config_value = 0; } else { - printk(BIOS_INFO, "FW_CONFIG value from CBFS is 0x%08x\n", + printk(BIOS_INFO, "FW_CONFIG value from CBFS is 0x%" PRIx64 "\n", fw_config_value); return fw_config_value; } @@ -47,7 +48,7 @@ printk(BIOS_WARNING, "%s: Could not get fw_config from EC\n", __func__); }
- printk(BIOS_INFO, "FW_CONFIG value is 0x%08x\n", fw_config_value); + printk(BIOS_INFO, "FW_CONFIG value is 0x%" PRIx64 "\n", fw_config_value); return fw_config_value; }
@@ -59,7 +60,8 @@ printk(BIOS_INFO, "fw_config match found: %s=%s\n", match->field_name, match->option_name); else - printk(BIOS_INFO, "fw_config match found: mask=0x%08x value=0x%08x\n", + printk(BIOS_INFO, "fw_config match found: mask=0x%" PRIx64 " value=0x%" + PRIx64 "\n", match->mask, match->value); return true; } @@ -70,20 +72,20 @@ #if ENV_RAMSTAGE
/* - * The maximum number of fw_config fields is limited by the 32-bit mask that is used to + * The maximum number of fw_config fields is limited by the 64-bit mask that is used to * represent them. */ -#define MAX_CACHE_ELEMENTS (8 * sizeof(uint32_t)) +#define MAX_CACHE_ELEMENTS (8 * sizeof(uint64_t))
static const struct fw_config *cached_configs[MAX_CACHE_ELEMENTS];
-static size_t probe_index(uint32_t mask) +static size_t probe_index(uint64_t mask) { assert(mask); - return __ffs(mask); + return __ffs64(mask); }
-const struct fw_config *fw_config_get_found(uint32_t field_mask) +const struct fw_config *fw_config_get_found(uint64_t field_mask) { const struct fw_config *config; config = cached_configs[probe_index(field_mask)]; diff --git a/src/mainboard/google/dedede/board_info.c b/src/mainboard/google/dedede/board_info.c index fdb4b5f..22d35d7 100644 --- a/src/mainboard/google/dedede/board_info.c +++ b/src/mainboard/google/dedede/board_info.c @@ -3,7 +3,7 @@ #include <baseboard/variants.h> #include <ec/google/chromeec/ec.h>
-int board_info_get_fw_config(uint32_t *fw_config) +int board_info_get_fw_config(uint64_t *fw_config) { return google_chromeec_cbi_get_fw_config(fw_config); } diff --git a/src/mainboard/google/dedede/variants/baseboard/include/baseboard/variants.h b/src/mainboard/google/dedede/variants/baseboard/include/baseboard/variants.h index dc855c6..bb41e45 100644 --- a/src/mainboard/google/dedede/variants/baseboard/include/baseboard/variants.h +++ b/src/mainboard/google/dedede/variants/baseboard/include/baseboard/variants.h @@ -21,7 +21,7 @@ * @param fw_config Address where the fw_config is stored. * @return 0 on success or negative integer for errors. */ -int board_info_get_fw_config(uint32_t *fw_config); +int board_info_get_fw_config(uint64_t *fw_config);
/* Return memory configuration structure. */ const struct mb_cfg *variant_memcfg_config(void); diff --git a/src/mainboard/google/zork/variants/baseboard/helpers.c b/src/mainboard/google/zork/variants/baseboard/helpers.c index cc07fe1..7071035 100644 --- a/src/mainboard/google/zork/variants/baseboard/helpers.c +++ b/src/mainboard/google/zork/variants/baseboard/helpers.c @@ -48,9 +48,9 @@ FW_CONFIG_SHIFT_FAN = 27, };
-static int get_fw_config(uint32_t *val) +static int get_fw_config(uint64_t *val) { - static uint32_t known_value; + static uint64_t known_value;
if (known_value) { *val = known_value; @@ -67,9 +67,9 @@ return 0; }
-static unsigned int extract_field(uint32_t mask, int shift) +static unsigned int extract_field(uint64_t mask, int shift) { - uint32_t fw_config; + uint64_t fw_config;
/* On errors nothing is assumed to be set. */ if (get_fw_config(&fw_config)) diff --git a/util/sconfig/main.c b/util/sconfig/main.c index 4f13293..a7b2ce6 100644 --- a/util/sconfig/main.c +++ b/util/sconfig/main.c @@ -4,8 +4,9 @@ #include <assert.h> #include <ctype.h> #include <getopt.h> -/* stat.h needs to be included before commonlib/helpers.h to avoid errors.*/ +#include <inttypes.h> #include <libgen.h> +/* stat.h needs to be included before commonlib/helpers.h to avoid errors.*/ #include <sys/stat.h> #include <commonlib/helpers.h> #include <stdint.h> @@ -402,8 +403,8 @@ { struct fw_config_field *field = find_fw_config_field(name);
- /* Check that field is within 32bits. */ - if (start_bit > end_bit || end_bit > 31) { + /* Check that field is within 64 bits. */ + if (start_bit > end_bit || end_bit > 63) { printf("ERROR: fw_config field %s has invalid range %u-%u\n", name, start_bit, end_bit); exit(1); @@ -452,15 +453,16 @@ } }
-void add_fw_config_option(struct fw_config_field *field, const char *name, unsigned int value) +void add_fw_config_option(struct fw_config_field *field, const char *name, uint64_t value) { struct fw_config_option *option; - uint32_t field_max_value; + uint64_t field_max_value;
/* Check that option value fits within field mask. */ - field_max_value = (1 << (1 + field->end_bit - field->start_bit)) - 1; + field_max_value = (1ull << (1ull + field->end_bit - field->start_bit)) - 1ull; if (value > field_max_value) { - printf("ERROR: fw_config option %s:%s value %u larger than field max %u\n", + printf("ERROR: fw_config option %s:%s value %" PRIx64 " larger than field max %" + PRIx64 "\n", field->name, name, value, field_max_value); exit(1); } @@ -475,7 +477,7 @@ } /* Compare values. */ if (value == option->value) { - printf("ERROR: fw_config option %s:%s[%u] redefined as %s\n", + printf("ERROR: fw_config option %s:%s[%" PRIx64 "] redefined as %s\n", field->name, option->name, value, name); exit(1); } @@ -532,23 +534,24 @@
while (field) { struct fw_config_option *option = field->options; - uint32_t mask; + uint64_t mask;
fprintf(fil, "#define FW_CONFIG_FIELD_%s_NAME "%s"\n", field->name, field->name);
/* Compute mask from start and end bit. */ - mask = ((1 << (1 + field->end_bit - field->start_bit)) - 1); + mask = ((1ull << (1ull + field->end_bit - field->start_bit)) - 1ull); mask <<= field->start_bit;
- fprintf(fil, "#define FW_CONFIG_FIELD_%s_MASK 0x%08x\n", + fprintf(fil, "#define FW_CONFIG_FIELD_%s_MASK 0x%" PRIx64 "\n", field->name, mask);
while (option) { fprintf(fil, "#define FW_CONFIG_FIELD_%s_OPTION_%s_NAME "%s"\n", field->name, option->name, option->name); - fprintf(fil, "#define FW_CONFIG_FIELD_%s_OPTION_%s_VALUE 0x%08x\n", - field->name, option->name, option->value << field->start_bit); + fprintf(fil, "#define FW_CONFIG_FIELD_%s_OPTION_%s_VALUE 0x%" + PRIx64 "\n", field->name, option->name, + option->value << field->start_bit);
option = option->next; } @@ -569,7 +572,7 @@ /* Find matching field. */ struct fw_config_field *field; struct fw_config_option *option; - uint32_t mask, value; + uint64_t mask, value;
field = find_fw_config_field(probe->field); if (!field) { diff --git a/util/sconfig/sconfig.h b/util/sconfig/sconfig.h index e2ff4c7..0db1ce5 100644 --- a/util/sconfig/sconfig.h +++ b/util/sconfig/sconfig.h @@ -1,6 +1,7 @@ /* sconfig, coreboot device tree compiler */ /* SPDX-License-Identifier: GPL-2.0-only */
+#include <stdint.h> #include <stdio.h> #include <stdlib.h> #include <string.h> @@ -31,7 +32,7 @@ struct fw_config_option; struct fw_config_option { const char *name; - unsigned int value; + uint64_t value; struct fw_config_option *next; }; struct fw_config_field; @@ -213,6 +214,6 @@ unsigned int start_bit, unsigned int end_bit);
void add_fw_config_option(struct fw_config_field *field, const char *name, - unsigned int value); + uint64_t value);
void add_fw_config_probe(struct bus *bus, const char *field, const char *option); diff --git a/util/sconfig/sconfig.y b/util/sconfig/sconfig.y index cf71b02..84dfe24 100755 --- a/util/sconfig/sconfig.y +++ b/util/sconfig/sconfig.y @@ -2,6 +2,7 @@ /* sconfig, coreboot device tree compiler */ /* SPDX-License-Identifier: GPL-2.0-only */
+#include <stdint.h> #include "sconfig.h"
int yylex(); @@ -16,7 +17,7 @@ struct device *dev; struct chip_instance *chip_instance; char *string; - int number; + uint64_t number; }
%token CHIP DEVICE REGISTER ALIAS REFERENCE ASSOCIATION BOOL STATUS MANDATORY BUS RESOURCE END EQUALS HEX STRING PCI PNP I2C APIC CPU_CLUSTER CPU DOMAIN IRQ DRQ SLOT_DESC IO NUMBER SUBSYSTEMID INHERIT IOAPIC_IRQ IOAPIC PCIINT GENERIC SPI USB MMIO LPC ESPI FW_CONFIG_TABLE FW_CONFIG_FIELD FW_CONFIG_OPTION FW_CONFIG_PROBE @@ -116,7 +117,7 @@
/* option <value> */ fw_config_option: FW_CONFIG_OPTION STRING NUMBER /* == field value */ - { add_fw_config_option(cur_field, $<string>2, strtoul($<string>3, NULL, 0)); }; + { add_fw_config_option(cur_field, $<string>2, strtoull($<string>3, NULL, 0)); };
/* probe <field> <option> */ fw_config_probe: FW_CONFIG_PROBE STRING /* == field */ STRING /* == option */
-
Angel Pons (Code Review) -
build bot (Jenkins) (Code Review)
-
Caveh Jalali (Code Review)
-
Duncan Laurie (Code Review)
-
Furquan Shaikh (Code Review)
-
Martin Roth (Code Review)
-
Tim Wawrzynczak (Code Review)