From 1a1614679587918741a3240ebfe5075281a61927 Mon Sep 17 00:00:00 2001 From: Lean Sheng Tan Date: Thu, 26 Dec 2024 21:46:08 +0100 Subject: [PATCH] Fix up CFR's open issues Fix some typos and also update the naming convention of `CFR_OPTFLAG_GRAYOUT` to `CFR_OPTFLAG_INACTIVE` as per reviews. Signed-off-by: Lean Sheng Tan Change-Id: Id66808382b93e32c58024462c18b20c2a89d6d23 Reviewed-on: https://review.coreboot.org/c/coreboot/+/85780 Tested-by: build bot (Jenkins) Reviewed-by: Angel Pons Reviewed-by: Christian Walter --- Documentation/drivers/cfr.md | 6 +++--- src/commonlib/include/commonlib/cfr.h | 4 ++-- src/drivers/option/cfr.c | 11 ++++------- 3 files changed, 9 insertions(+), 12 deletions(-) diff --git a/Documentation/drivers/cfr.md b/Documentation/drivers/cfr.md index ade6d2b14a..0405f5c58c 100644 --- a/Documentation/drivers/cfr.md +++ b/Documentation/drivers/cfr.md @@ -26,7 +26,7 @@ visible name, a help text, a default value and status flags. All strings are in ASCII. The boot options are grouped into forms, where each form hold -one ore more options. Boot options that are not used in the current +one or more options. Boot options that are not used in the current boot flow, and are never reachable should be marked as hidden. Dependecies between options can be defined in the code and should be evaluated by the CFR parser/UI. @@ -66,7 +66,7 @@ The public API can be found in ## Implementation design ### Tags Tags identify the structure defined in `src/commonlib/include/commonlib/cfr.h`. -Every struct might be immideatly followed by additional structs (so called +Every struct might be immediately followed by additional structs (so called sub nodes), having their own tag and size field. The sum of all sub nodes size fields plus the size of the struct itself equals the size field. @@ -208,7 +208,7 @@ effect on the non-volatile variable. Prevents writes to the variable. -* `CFR_OPTFLAG_GRAYOUT` +* `CFR_OPTFLAG_INACTIVE` Implies `READONLY`. The option is visible, but cannot be modified because one of the dependencies are not given. However there's a diff --git a/src/commonlib/include/commonlib/cfr.h b/src/commonlib/include/commonlib/cfr.h index 22058e96f9..cc8431a2db 100644 --- a/src/commonlib/include/commonlib/cfr.h +++ b/src/commonlib/include/commonlib/cfr.h @@ -68,7 +68,7 @@ enum cfr_tags { * effect on the non-volatile variable. * CFR_OPTFLAG_READONLY: * Prevents writes to the variable. - * CFR_OPTFLAG_GRAYOUT: + * CFR_OPTFLAG_INACTIVE: * Implies READONLY. The option is visible, but cannot be modified * because one of the dependencies are not given. However there's a * possibility to enable the option by changing runtime configuration. @@ -93,7 +93,7 @@ enum cfr_tags { */ enum cfr_option_flags { CFR_OPTFLAG_READONLY = 1 << 0, - CFR_OPTFLAG_GRAYOUT = 1 << 1, + CFR_OPTFLAG_INACTIVE = 1 << 1, CFR_OPTFLAG_SUPPRESS = 1 << 2, CFR_OPTFLAG_VOLATILE = 1 << 3, CFR_OPTFLAG_RUNTIME = 1 << 4, diff --git a/src/drivers/option/cfr.c b/src/drivers/option/cfr.c index b323b4318a..e833d7bdba 100644 --- a/src/drivers/option/cfr.c +++ b/src/drivers/option/cfr.c @@ -29,9 +29,6 @@ static uint32_t cfr_record_size(const char *startp, const char *endp) static uint32_t write_cfr_varchar(char *current, const char *string, uint32_t tag) { - uint8_t *data; - size_t padding; - ASSERT(string); if (!string) return 0; @@ -98,7 +95,7 @@ static uint32_t write_numeric_option(char *current, uint32_t tag, const uint64_t option->object_id = object_id; option->dependency_id = dep_id; option->flags = flags; - if (option->flags & (CFR_OPTFLAG_GRAYOUT | CFR_OPTFLAG_VOLATILE)) + if (option->flags & (CFR_OPTFLAG_INACTIVE | CFR_OPTFLAG_VOLATILE)) option->flags |= CFR_OPTFLAG_READONLY; option->default_value = default_value; option->size = sizeof(*option); @@ -163,7 +160,7 @@ static uint32_t sm_write_opt_varchar(char *current, const struct sm_obj_varchar option->object_id = object_id; option->dependency_id = dep_id; option->flags = sm_varchar->flags; - if (option->flags & (CFR_OPTFLAG_GRAYOUT | CFR_OPTFLAG_VOLATILE)) + if (option->flags & (CFR_OPTFLAG_INACTIVE | CFR_OPTFLAG_VOLATILE)) option->flags |= CFR_OPTFLAG_READONLY; option->size = sizeof(*option); @@ -193,7 +190,7 @@ static uint32_t sm_write_opt_comment(char *current, const struct sm_obj_comment comment->object_id = object_id; comment->dependency_id = dep_id; comment->flags = sm_comment->flags; - if (comment->flags & (CFR_OPTFLAG_GRAYOUT | CFR_OPTFLAG_VOLATILE)) + if (comment->flags & (CFR_OPTFLAG_INACTIVE | CFR_OPTFLAG_VOLATILE)) comment->flags |= CFR_OPTFLAG_READONLY; comment->size = sizeof(*comment); @@ -228,7 +225,7 @@ static uint32_t sm_write_form(char *current, struct sm_obj_form *sm_form, form->object_id = object_id; form->dependency_id = dep_id; form->flags = sm_form->flags; - if (form->flags & (CFR_OPTFLAG_GRAYOUT | CFR_OPTFLAG_VOLATILE)) + if (form->flags & (CFR_OPTFLAG_INACTIVE | CFR_OPTFLAG_VOLATILE)) form->flags |= CFR_OPTFLAG_READONLY; form->size = sizeof(*form);