coreboot-gerrit February 2025

coreboot-gerrit@coreboot.org

1 participants
2886 discussions

[S] Change in coreboot[main]: CFR: Add min/max/step values and hex display flag for number options
by Matt DeVillier (Code Review) 07 Feb '25

07 Feb '25

Matt DeVillier has submitted this change. ( https://review.coreboot.org/c/coreboot/+/86039?usp=email ) Change subject: CFR: Add min/max/step values and hex display flag for number options ...................................................................... CFR: Add min/max/step values and hex display flag for number options This commit adds support for minimum/maximum limit values as well as step sizes for CFR number options. Additionally, add a new flag that specifies the option should be displayed in hexadecimal notation instead of decimal. Change-Id: I2e70f1430fb1911f1ad974832f8abfe76f928ac3 Signed-off-by: Filip Brozovic <fbrozovic(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/86039 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> --- M src/commonlib/include/commonlib/cfr.h M src/drivers/option/cfr.c M src/drivers/option/cfr_frontend.h 3 files changed, 30 insertions(+), 6 deletions(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved diff --git a/src/commonlib/include/commonlib/cfr.h b/src/commonlib/include/commonlib/cfr.h index 570ce3c..af68269 100644 --- a/src/commonlib/include/commonlib/cfr.h +++ b/src/commonlib/include/commonlib/cfr.h @@ -121,6 +121,15 @@ */ }; +/* + * The optional flags describe how a numeric option is to be displayed. + * CFR_NUM_OPT_DISPFLAG_HEX: + * Displays a NUMBER option in hexadecimal instead of decimal notation. + */ +enum cfr_numeric_option_display_flags { + CFR_NUM_OPT_DISPFLAG_HEX = 1 << 0, +}; + /* Supports multiple option types: ENUM, NUMBER, BOOL */ struct __packed lb_cfr_numeric_option { uint32_t tag; /* @@ -134,6 +143,11 @@ */ uint32_t flags; /* enum cfr_option_flags */ uint32_t default_value; + uint32_t min; + uint32_t max; + uint32_t step; + uint32_t display_flags; /* enum cfr_numeric_option_display_flags */ + /* * struct lb_cfr_varbinary opt_name * struct lb_cfr_varbinary ui_name diff --git a/src/drivers/option/cfr.c b/src/drivers/option/cfr.c index c439ed9..26bdd55 100644 --- a/src/drivers/option/cfr.c +++ b/src/drivers/option/cfr.c @@ -104,8 +104,9 @@ static uint32_t write_numeric_option(char *current, uint32_t tag, const uint64_t object_id, const char *opt_name, const char *ui_name, const char *ui_helptext, - uint32_t flags, uint32_t default_value, const struct sm_enum_value *values, - const uint64_t dep_id, const uint32_t *dep_values, const uint32_t num_dep_values) + uint32_t flags, uint32_t default_value, uint32_t min, uint32_t max, uint32_t step, + uint32_t display_flags, const struct sm_enum_value *values, const uint64_t dep_id, + const uint32_t *dep_values, const uint32_t num_dep_values) { struct lb_cfr_numeric_option *option = (struct lb_cfr_numeric_option *)current; size_t len; @@ -117,6 +118,10 @@ if (option->flags & (CFR_OPTFLAG_INACTIVE | CFR_OPTFLAG_VOLATILE)) option->flags |= CFR_OPTFLAG_READONLY; option->default_value = default_value; + option->min = (min <= max) ? min : 0; + option->max = (min == 0 && max == 0) ? UINT32_MAX : max; + option->step = step; + option->display_flags = display_flags; option->size = sizeof(*option); current += option->size; @@ -148,7 +153,7 @@ { return write_numeric_option(current, CFR_TAG_OPTION_ENUM, object_id, sm_enum->opt_name, sm_enum->ui_name, sm_enum->ui_helptext, - sm_enum->flags, sm_enum->default_value, sm_enum->values, + sm_enum->flags, sm_enum->default_value, 0, 0, 0, 0, sm_enum->values, dep_id, dep_values, num_dep_values); } @@ -159,8 +164,9 @@ { return write_numeric_option(current, CFR_TAG_OPTION_NUMBER, object_id, sm_number->opt_name, sm_number->ui_name, sm_number->ui_helptext, - sm_number->flags, sm_number->default_value, NULL, dep_id, - dep_values, num_dep_values); + sm_number->flags, sm_number->default_value, sm_number->min, sm_number->max, + sm_number->step, sm_number->display_flags, NULL, dep_id, dep_values, + num_dep_values); } static uint32_t sm_write_opt_bool(char *current, const struct sm_obj_bool *sm_bool, @@ -170,7 +176,7 @@ { return write_numeric_option(current, CFR_TAG_OPTION_BOOL, object_id, sm_bool->opt_name, sm_bool->ui_name, sm_bool->ui_helptext, - sm_bool->flags, sm_bool->default_value, NULL, dep_id, + sm_bool->flags, sm_bool->default_value, 0, 0, 0, 0, NULL, dep_id, dep_values, num_dep_values); } diff --git a/src/drivers/option/cfr_frontend.h b/src/drivers/option/cfr_frontend.h index 21c18e7..17332bd 100644 --- a/src/drivers/option/cfr_frontend.h +++ b/src/drivers/option/cfr_frontend.h @@ -30,6 +30,10 @@ const char *ui_name; const char *ui_helptext; uint32_t default_value; + uint32_t min; + uint32_t max; + uint32_t step; + uint32_t display_flags; /* enum cfr_numeric_option_display_flags */ }; struct sm_obj_bool { -- To view, visit https://review.coreboot.org/c/coreboot/+/86039?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I2e70f1430fb1911f1ad974832f8abfe76f928ac3 Gerrit-Change-Number: 86039 Gerrit-PatchSet: 6 Gerrit-Owner: Filip Brozovic <fbrozovic(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

[M] Change in coreboot[main]: CFR: add dependencies based on specific option values
by Matt DeVillier (Code Review) 07 Feb '25

07 Feb '25

Matt DeVillier has submitted this change. ( https://review.coreboot.org/c/coreboot/+/85987?usp=email ) Change subject: CFR: add dependencies based on specific option values ...................................................................... CFR: add dependencies based on specific option values Implements a way for CFR options to depend on another option being set to one or more specific values. This is achieved by writing a list of values as a varbinary struct. Change-Id: Iaf7965551490969052eb27c207fa524470d4dd6a Signed-off-by: Filip Brozovic <fbrozovic(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/85987 Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-by: Matt DeVillier <matt.devillier(a)gmail.com> Reviewed-by: Sean Rhodes <sean(a)starlabs.systems> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M Documentation/drivers/cfr_internal.md M src/commonlib/include/commonlib/cfr.h M src/drivers/option/cfr.c M src/drivers/option/cfr_frontend.h 4 files changed, 117 insertions(+), 29 deletions(-) Approvals: Sean Rhodes: Looks good to me, but someone else must approve build bot (Jenkins): Verified Matt DeVillier: Looks good to me, but someone else must approve Angel Pons: Looks good to me, approved diff --git a/Documentation/drivers/cfr_internal.md b/Documentation/drivers/cfr_internal.md index 95fef66..b2355d7 100644 --- a/Documentation/drivers/cfr_internal.md +++ b/Documentation/drivers/cfr_internal.md @@ -50,8 +50,9 @@ The CFR options can have a dependency that must be evaluated at runtime by the OS/payload that parses the CFR record and displays the UI. -By using the `WITH_DEP()` macro you can specify another numberic option that -is checked to hide the current option. +By using the `WITH_DEP()` macro you can specify another numeric option that +is checked to hide the current option. The `WITH_DEP_VALUES()` macro allows +specifying one or more values that cause the dependent option to be displayed. **Example:** Declares a dependency from `sata_disable_port0` to `sata_enable`. The option `sata_disable_port0` will be hidden as long as "sata_enable" is 0. @@ -76,6 +77,37 @@ }, WITH_DEP(&sata_enable)); ``` +**Example:** Declares a dependency from `com1_termination` to `com1_mode`. +The option `com1_termination` will only be shown if `com1_mode` is set to RS-485. + +``` +#define COM_MODE_DISABLED 3 +#define COM_MODE_RS232 0 +#define COM_MODE_RS485 1 + +static struct sm_object com1_mode = SM_DECLARE_ENUM({ + .flags = CFR_OPTFLAG_RUNTIME, + .opt_name = "com1_mode", + .ui_name = "COM1 Mode", + .ui_helptext = NULL, + .default_value = 1, + .values = (const struct sm_enum_value[]) { + { "Disabled", COM_MODE_DISABLED }, + { "RS-232", COM_MODE_RS232 }, + { "RS-485", COM_MODE_RS485 }, + SM_ENUM_VALUE_END }, +}); + +static struct sm_object com1_termination = SM_DECLARE_BOOL({ + .flags = CFR_OPTFLAG_RUNTIME, + .opt_name = "com1_termination", + .ui_name = "Enable COM1 termination resistors", + .ui_helptext = NULL, + .default_value = false, +}, WITH_DEP_VALUES(&com1_mode, COM_MODE_RS485)); + +``` + ### Providing mainboard custom options A mainboard that uses CFR can provide a list of custom options diff --git a/src/commonlib/include/commonlib/cfr.h b/src/commonlib/include/commonlib/cfr.h index 5d8b37b..570ce3c 100644 --- a/src/commonlib/include/commonlib/cfr.h +++ b/src/commonlib/include/commonlib/cfr.h @@ -63,6 +63,7 @@ CFR_TAG_VARCHAR_UI_HELPTEXT = 9, CFR_TAG_VARCHAR_DEF_VALUE = 10, CFR_TAG_OPTION_COMMENT = 11, + CFR_TAG_DEP_VALUES = 12, }; /* @@ -104,7 +105,8 @@ struct __packed lb_cfr_varbinary { uint32_t tag; /* * CFR_TAG_VARCHAR_OPT_NAME, CFR_TAG_VARCHAR_UI_NAME, - * CFR_TAG_VARCHAR_UI_HELPTEXT or CFR_TAG_VARCHAR_DEF_VALUE + * CFR_TAG_VARCHAR_UI_HELPTEXT, CFR_TAG_VARCHAR_DEF_VALUE + * or CFR_TAG_DEP_VALUES */ uint32_t size; /* Length of the entire structure */ uint32_t data_length; /* Length of data, including NULL terminator for strings */ @@ -136,6 +138,7 @@ * struct lb_cfr_varbinary opt_name * struct lb_cfr_varbinary ui_name * struct lb_cfr_varbinary ui_helptext (Optional) + * struct lb_cfr_varbinary dependency_values (Optional) * struct lb_cfr_enum_value enum_values[] */ }; @@ -153,6 +156,7 @@ * struct lb_cfr_varbinary opt_name * struct lb_cfr_varbinary ui_name * struct lb_cfr_varbinary ui_helptext (Optional) + * struct lb_cfr_varbinary dependency_values (Optional) */ }; @@ -172,6 +176,7 @@ /* * struct lb_cfr_varbinary ui_name * struct lb_cfr_varbinary ui_helptext (Optional) + * struct lb_cfr_varbinary dependency_values (Optional) */ }; @@ -186,6 +191,7 @@ uint32_t flags; /* enum cfr_option_flags */ /* * struct lb_cfr_varbinary ui_name + * struct lb_cfr_varbinary dependency_values (Optional) * struct lb_cfr_varchar_option options[] */ }; diff --git a/src/drivers/option/cfr.c b/src/drivers/option/cfr.c index 15c4fd9..c439ed9 100644 --- a/src/drivers/option/cfr.c +++ b/src/drivers/option/cfr.c @@ -69,6 +69,25 @@ return write_cfr_varchar(current, string, CFR_TAG_VARCHAR_UI_HELPTEXT); } +static uint32_t sm_write_dep_values(char *current, + const uint32_t *dep_values, const uint32_t num_dep_values) +{ + /* Dependency values are optional */ + if (!dep_values || !num_dep_values) + return 0; + + struct lb_cfr_varbinary *cfr_values = (struct lb_cfr_varbinary *)current; + cfr_values->tag = CFR_TAG_DEP_VALUES; + cfr_values->data_length = sizeof(*dep_values) * num_dep_values; + char *data = current + sizeof(*cfr_values); + memcpy(data, dep_values, cfr_values->data_length); + + /* Make sure that every TAG/SIZE field is always aligned to LB_ENTRY_ALIGN */ + cfr_values->size = ALIGN_UP(sizeof(*cfr_values) + cfr_values->data_length, LB_ENTRY_ALIGN); + + return cfr_values->size; +} + static uint32_t sm_write_enum_value(char *current, const struct sm_enum_value *e) { struct lb_cfr_enum_value *enum_val = (struct lb_cfr_enum_value *)current; @@ -86,7 +105,7 @@ static uint32_t write_numeric_option(char *current, uint32_t tag, const uint64_t object_id, const char *opt_name, const char *ui_name, const char *ui_helptext, uint32_t flags, uint32_t default_value, const struct sm_enum_value *values, - const uint64_t dep_id) + const uint64_t dep_id, const uint32_t *dep_values, const uint32_t num_dep_values) { struct lb_cfr_numeric_option *option = (struct lb_cfr_numeric_option *)current; size_t len; @@ -110,6 +129,7 @@ return 0; current += len; current += sm_write_ui_helptext(current, ui_helptext); + current += sm_write_dep_values(current, dep_values, num_dep_values); if (option->tag == CFR_TAG_OPTION_ENUM && values) { for (const struct sm_enum_value *e = values; e->ui_name; e++) { @@ -122,35 +142,41 @@ } static uint32_t sm_write_opt_enum(char *current, const struct sm_obj_enum *sm_enum, - const uint64_t object_id, const uint64_t dep_id) + const uint64_t object_id, const uint64_t dep_id, + const uint32_t *dep_values, const uint32_t num_dep_values) { return write_numeric_option(current, CFR_TAG_OPTION_ENUM, object_id, sm_enum->opt_name, sm_enum->ui_name, sm_enum->ui_helptext, sm_enum->flags, sm_enum->default_value, sm_enum->values, - dep_id); + dep_id, dep_values, num_dep_values); } static uint32_t sm_write_opt_number(char *current, const struct sm_obj_number *sm_number, - const uint64_t object_id, const uint64_t dep_id) + const uint64_t object_id, const uint64_t dep_id, + const uint32_t *dep_values, const uint32_t num_dep_values) { return write_numeric_option(current, CFR_TAG_OPTION_NUMBER, object_id, sm_number->opt_name, sm_number->ui_name, sm_number->ui_helptext, - sm_number->flags, sm_number->default_value, NULL, dep_id); + sm_number->flags, sm_number->default_value, NULL, dep_id, + dep_values, num_dep_values); } static uint32_t sm_write_opt_bool(char *current, const struct sm_obj_bool *sm_bool, - const uint64_t object_id, const uint64_t dep_id) + const uint64_t object_id, const uint64_t dep_id, + const uint32_t *dep_values, const uint32_t num_dep_values) { return write_numeric_option(current, CFR_TAG_OPTION_BOOL, object_id, sm_bool->opt_name, sm_bool->ui_name, sm_bool->ui_helptext, - sm_bool->flags, sm_bool->default_value, NULL, dep_id); + sm_bool->flags, sm_bool->default_value, NULL, dep_id, + dep_values, num_dep_values); } static uint32_t sm_write_opt_varchar(char *current, const struct sm_obj_varchar *sm_varchar, - const uint64_t object_id, const uint64_t dep_id) + const uint64_t object_id, const uint64_t dep_id, + const uint32_t *dep_values, const uint32_t num_dep_values) { struct lb_cfr_varchar_option *option = (struct lb_cfr_varchar_option *)current; @@ -175,13 +201,15 @@ return 0; current += len; current += sm_write_ui_helptext(current, sm_varchar->ui_helptext); + current += sm_write_dep_values(current, dep_values, num_dep_values); option->size = cfr_record_size((char *)option, current); return option->size; } static uint32_t sm_write_opt_comment(char *current, const struct sm_obj_comment *sm_comment, - const uint32_t object_id, const uint32_t dep_id) + const uint32_t object_id, const uint32_t dep_id, + const uint32_t *dep_values, const uint32_t num_dep_values) { struct lb_cfr_option_comment *comment = (struct lb_cfr_option_comment *)current; size_t len; @@ -200,6 +228,7 @@ return 0; current += len; current += sm_write_ui_helptext(current, sm_comment->ui_helptext); + current += sm_write_dep_values(current, dep_values, num_dep_values); comment->size = cfr_record_size((char *)comment, current); return comment->size; @@ -215,7 +244,8 @@ static uint32_t sm_write_object(char *current, const struct sm_object *sm_obj); static uint32_t sm_write_form(char *current, struct sm_obj_form *sm_form, - const uint64_t object_id, const uint64_t dep_id) + const uint64_t object_id, const uint64_t dep_id, + const uint32_t *dep_values, const uint32_t num_dep_values) { struct lb_cfr_option_form *form = (struct lb_cfr_option_form *)current; size_t len; @@ -234,6 +264,7 @@ if (!len) return 0; current += len; + current += sm_write_dep_values(current, dep_values, num_dep_values); while (sm_form->obj_list[i]) current += sm_write_object(current, sm_form->obj_list[i++]); @@ -245,6 +276,8 @@ static uint32_t sm_write_object(char *current, const struct sm_object *sm_obj) { uint64_t dep_id, obj_id; + const uint32_t *dep_values; + uint32_t num_dep_values; struct sm_object sm_obj_copy; assert(sm_obj); @@ -253,10 +286,14 @@ /* Set dependency ID */ dep_id = 0; + dep_values = NULL; + num_dep_values = 0; if (sm_obj->dep) { - assert(sm_obj->dep->kind == SM_OBJ_BOOL); - if (sm_obj->dep->kind == SM_OBJ_BOOL) + if (sm_obj->dep->kind == SM_OBJ_BOOL || sm_obj->dep->kind == SM_OBJ_ENUM) { dep_id = sm_gen_obj_id((void *)sm_obj->dep); + dep_values = sm_obj->dep_values; + num_dep_values = sm_obj->num_dep_values; + } } /* Invoke callback to update fields */ @@ -273,21 +310,22 @@ return 0; case SM_OBJ_ENUM: return sm_write_opt_enum(current, &sm_obj->sm_enum, obj_id, - dep_id); + dep_id, dep_values, num_dep_values); case SM_OBJ_NUMBER: return sm_write_opt_number(current, &sm_obj->sm_number, obj_id, - dep_id); + dep_id, dep_values, num_dep_values); case SM_OBJ_BOOL: return sm_write_opt_bool(current, &sm_obj->sm_bool, obj_id, - dep_id); + dep_id, dep_values, num_dep_values); case SM_OBJ_VARCHAR: return sm_write_opt_varchar(current, &sm_obj->sm_varchar, obj_id, - dep_id); + dep_id, dep_values, num_dep_values); case SM_OBJ_COMMENT: return sm_write_opt_comment(current, &sm_obj->sm_comment, obj_id, - dep_id); + dep_id, dep_values, num_dep_values); case SM_OBJ_FORM: - return sm_write_form(current, (struct sm_obj_form *)&sm_obj->sm_form, obj_id, dep_id); + return sm_write_form(current, (struct sm_obj_form *)&sm_obj->sm_form, obj_id, + dep_id, dep_values, num_dep_values); default: BUG(); printk(BIOS_ERR, "Unknown setup menu object kind %u, ignoring\n", sm_obj->kind); @@ -311,13 +349,13 @@ current += cfr_root->size; while (sm_root && sm_root[i]) - current += sm_write_form(current, sm_root[i++], 0, 0); + current += sm_write_form(current, sm_root[i++], 0, 0, NULL, 0); /* * Add generic forms. */ for (obj = &_cfr_forms[0]; obj != &_ecfr_forms[0]; obj++) - current += sm_write_form(current, obj, 0, 0); + current += sm_write_form(current, obj, 0, 0, NULL, 0); cfr_root->size = cfr_record_size((char *)cfr_root, current); diff --git a/src/drivers/option/cfr_frontend.h b/src/drivers/option/cfr_frontend.h index 2161853..21c18e7 100644 --- a/src/drivers/option/cfr_frontend.h +++ b/src/drivers/option/cfr_frontend.h @@ -75,6 +75,8 @@ struct sm_object { enum sm_object_kind kind; const struct sm_object *dep; + const uint32_t *dep_values; + const uint32_t num_dep_values; void (*ctor)(const struct sm_object *obj, struct sm_object *new); /* Called on object creation */ union { struct sm_obj_enum sm_enum; @@ -87,21 +89,31 @@ }; /* sm_object helpers with type checking */ -#define SM_DECLARE_ENUM(...) { .kind = SM_OBJ_ENUM, .dep = NULL, \ +#define SM_DECLARE_ENUM(...) { .kind = SM_OBJ_ENUM, .dep = NULL, \ + .dep_values = NULL, .num_dep_values = 0, \ .ctor = NULL, .sm_enum = __VA_ARGS__ } -#define SM_DECLARE_NUMBER(...) { .kind = SM_OBJ_NUMBER, .dep = NULL, \ +#define SM_DECLARE_NUMBER(...) { .kind = SM_OBJ_NUMBER, .dep = NULL, \ + .dep_values = NULL, .num_dep_values = 0, \ .ctor = NULL, .sm_number = __VA_ARGS__ } -#define SM_DECLARE_BOOL(...) { .kind = SM_OBJ_BOOL, .dep = NULL, \ +#define SM_DECLARE_BOOL(...) { .kind = SM_OBJ_BOOL, .dep = NULL, \ + .dep_values = NULL, .num_dep_values = 0, \ .ctor = NULL, .sm_bool = __VA_ARGS__ } -#define SM_DECLARE_VARCHAR(...) { .kind = SM_OBJ_VARCHAR, .dep = NULL, \ +#define SM_DECLARE_VARCHAR(...) { .kind = SM_OBJ_VARCHAR, .dep = NULL, \ + .dep_values = NULL, .num_dep_values = 0, \ .ctor = NULL, .sm_varchar = __VA_ARGS__ } -#define SM_DECLARE_COMMENT(...) { .kind = SM_OBJ_COMMENT, .dep = NULL, \ +#define SM_DECLARE_COMMENT(...) { .kind = SM_OBJ_COMMENT, .dep = NULL, \ + .dep_values = NULL, .num_dep_values = 0, \ .ctor = NULL, .sm_comment = __VA_ARGS__ } -#define SM_DECLARE_FORM(...) { .kind = SM_OBJ_FORM, .dep = NULL, \ +#define SM_DECLARE_FORM(...) { .kind = SM_OBJ_FORM, .dep = NULL, \ + .dep_values = NULL, .num_dep_values = 0, \ .ctor = NULL, .sm_form = __VA_ARGS__ } #define WITH_CALLBACK(c) .ctor = (c) #define WITH_DEP(d) .dep = (d) +#define WITH_DEP_VALUES(d, ...) \ + .dep = (d), \ + .dep_values = ((const uint32_t[]) { __VA_ARGS__ }), \ + .num_dep_values = sizeof((uint32_t[]) { __VA_ARGS__ }) / sizeof(uint32_t) void cfr_write_setup_menu(struct lb_cfr *cfr_root, struct sm_obj_form *sm_root[]); -- To view, visit https://review.coreboot.org/c/coreboot/+/85987?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Iaf7965551490969052eb27c207fa524470d4dd6a Gerrit-Change-Number: 85987 Gerrit-PatchSet: 5 Gerrit-Owner: Filip Brozovic <fbrozovic(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

[XS] Change in coreboot[main]: CFR: Add version field to root struct
by Matt DeVillier (Code Review) 07 Feb '25

07 Feb '25

Matt DeVillier has submitted this change. ( https://review.coreboot.org/c/coreboot/+/86080?usp=email ) Change subject: CFR: Add version field to root struct ...................................................................... CFR: Add version field to root struct Add a version field to the CFR root struct so parsers can check compatibility when parsing structs. Change-Id: Ifcb950f1bdedc0ab925f3841befb7e7001c0f7f4 Signed-off-by: Filip Brozovic <fbrozovic(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/86080 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Matt DeVillier <matt.devillier(a)gmail.com> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> --- M src/commonlib/include/commonlib/cfr.h M src/commonlib/include/commonlib/coreboot_tables.h M src/drivers/option/cfr.c 3 files changed, 4 insertions(+), 0 deletions(-) Approvals: Angel Pons: Looks good to me, approved Matt DeVillier: Looks good to me, but someone else must approve build bot (Jenkins): Verified diff --git a/src/commonlib/include/commonlib/cfr.h b/src/commonlib/include/commonlib/cfr.h index cc8431a..5d8b37b 100644 --- a/src/commonlib/include/commonlib/cfr.h +++ b/src/commonlib/include/commonlib/cfr.h @@ -49,6 +49,8 @@ * */ +#define CFR_VERSION 0x00000000 + enum cfr_tags { CFR_TAG_OPTION_FORM = 1, CFR_TAG_ENUM_VALUE = 2, diff --git a/src/commonlib/include/commonlib/coreboot_tables.h b/src/commonlib/include/commonlib/coreboot_tables.h index def48b7..78dbe8e 100644 --- a/src/commonlib/include/commonlib/coreboot_tables.h +++ b/src/commonlib/include/commonlib/coreboot_tables.h @@ -602,6 +602,7 @@ struct lb_cfr { uint32_t tag; uint32_t size; + uint32_t version; uint32_t checksum; /* Checksum of the variable payload. */ /* struct lb_cfr_option_form forms[] */ }; diff --git a/src/drivers/option/cfr.c b/src/drivers/option/cfr.c index e833d7b..15c4fd9 100644 --- a/src/drivers/option/cfr.c +++ b/src/drivers/option/cfr.c @@ -307,6 +307,7 @@ cfr_root->tag = LB_TAG_CFR_ROOT; cfr_root->size = sizeof(*cfr_root); + cfr_root->version = CFR_VERSION; current += cfr_root->size; while (sm_root && sm_root[i]) -- To view, visit https://review.coreboot.org/c/coreboot/+/86080?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ifcb950f1bdedc0ab925f3841befb7e7001c0f7f4 Gerrit-Change-Number: 86080 Gerrit-PatchSet: 3 Gerrit-Owner: Filip Brozovic <fbrozovic(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

[S] Change in coreboot[main]: mb/google/trulo/var/uldrenite: Add fw_config probe for Cellular
by Matt DeVillier (Code Review) 07 Feb '25

07 Feb '25

Matt DeVillier has submitted this change. ( https://review.coreboot.org/c/coreboot/+/86255?usp=email ) ( 3 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: mb/google/trulo/var/uldrenite: Add fw_config probe for Cellular ...................................................................... mb/google/trulo/var/uldrenite: Add fw_config probe for Cellular Use fw_config to probe Cellular. BUG=b:392040004 BRANCH=firmware-trulo-15217.771.B TEST=emerge-nissa coreboot chromeos-bootimage Change-Id: Ib664f543c6012b44a0a604d0943416519d92a057 Signed-off-by: John Su <john_su(a)compal.corp-partner.google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/86255 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Subrata Banik <subratabanik(a)google.com> Reviewed-by: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com> --- M src/mainboard/google/brya/variants/uldrenite/overridetree.cb M src/mainboard/google/brya/variants/uldrenite/variant.c 2 files changed, 32 insertions(+), 3 deletions(-) Approvals: build bot (Jenkins): Verified Dtrain Hsu: Looks good to me, approved Subrata Banik: Looks good to me, approved diff --git a/src/mainboard/google/brya/variants/uldrenite/overridetree.cb b/src/mainboard/google/brya/variants/uldrenite/overridetree.cb index 3c06750..862968e 100644 --- a/src/mainboard/google/brya/variants/uldrenite/overridetree.cb +++ b/src/mainboard/google/brya/variants/uldrenite/overridetree.cb @@ -1,4 +1,8 @@ fw_config + field DB_CELLULAR 1 2 + option CELLULAR_ABSENT 0 + option CELLULAR_RW350R 1 + end field TOUCHSCREEN 4 4 option TOUCHSCREEN_UNKNOWN 0 option TOUCHSCREEN_NONE 1 @@ -278,7 +282,9 @@ chip drivers/usb/acpi register "desc" = ""USB2 WWAN"" register "type" = "UPC_TYPE_INTERNAL" - device ref usb2_port5 on end + device ref usb2_port5 on + probe DB_CELLULAR CELLULAR_RW350R + end end chip drivers/usb/acpi register "desc" = ""USB2 User Facing Camera"" @@ -459,7 +465,9 @@ register "srcclk_pin" = "2" register "ext_pm_support" = "ACPI_PCIE_RP_EMIT_ALL" register "skip_on_off_support" = "true" - device generic 0 alias rp2_rtd3 on end + device generic 0 alias rp2_rtd3 on + probe DB_CELLULAR CELLULAR_RW350R + end end chip drivers/wwan/fm register "fcpo_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_H23)" @@ -468,8 +476,11 @@ register "wake_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_E16)" register "add_acpi_dma_property" = "true" use rp2_rtd3 as rtd3dev - device generic 0 alias rp2_wwan on end + device generic 0 alias rp2_wwan on + probe DB_CELLULAR CELLULAR_RW350R + end end + probe DB_CELLULAR CELLULAR_RW350R end # PCIE2 WWAN card device ref shared_sram on end device ref heci1 on end diff --git a/src/mainboard/google/brya/variants/uldrenite/variant.c b/src/mainboard/google/brya/variants/uldrenite/variant.c index 96b2291..f5bbaf6 100644 --- a/src/mainboard/google/brya/variants/uldrenite/variant.c +++ b/src/mainboard/google/brya/variants/uldrenite/variant.c @@ -37,6 +37,17 @@ PAD_NC(GPP_E17, NONE), }; +static const struct pad_config lte_disable_pads[] = { + /* A8 : WWAN_RF_DISABLE_ODL */ + PAD_NC(GPP_A8, NONE), + /* F12 : WWAN_RST_L */ + PAD_NC_LOCK(GPP_F12, NONE, LOCK_CONFIG), + /* F13 : PLTRST_WWAN# */ + PAD_NC(GPP_F13, NONE), + /* H23 : WWAN_EN */ + PAD_NC(GPP_H23, NONE), +}; + void fw_config_gpio_padbased_override(struct pad_config *padbased_table) { if (fw_config_probe(FW_CONFIG(TOUCHSCREEN, TOUCHSCREEN_NONE))) { @@ -44,10 +55,17 @@ gpio_padbased_override(padbased_table, touchscreen_disable_pads, ARRAY_SIZE(touchscreen_disable_pads)); } + if (fw_config_probe(FW_CONFIG(DB_CELLULAR, CELLULAR_ABSENT))) { + printk(BIOS_INFO, "Disable Cellular GPIO pins.\n"); + gpio_padbased_override(padbased_table, lte_disable_pads, + ARRAY_SIZE(lte_disable_pads)); + } } void variant_init(void) { + if (fw_config_probe(FW_CONFIG(DB_CELLULAR, CELLULAR_ABSENT))) + return; /* * RW350R power on seuqence: * De-assert WWAN_EN -> 20ms -> de-assert WWAN_RST -> 30ms -> -- To view, visit https://review.coreboot.org/c/coreboot/+/86255?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ib664f543c6012b44a0a604d0943416519d92a057 Gerrit-Change-Number: 86255 Gerrit-PatchSet: 5 Gerrit-Owner: John Su <john_su(a)compal.corp-partner.google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Frank Wu <frank_wu(a)compal.corp-partner.google.com> Gerrit-CC: Jamie Chen <jamie_chen(a)compal.corp-partner.google.com>

1 0

[S] Change in coreboot[main]: intel/acpi: Put BSP as the first entry
by Matt DeVillier (Code Review) 07 Feb '25

07 Feb '25

Matt DeVillier has submitted this change. ( https://review.coreboot.org/c/coreboot/+/86247?usp=email ) Change subject: intel/acpi: Put BSP as the first entry ...................................................................... intel/acpi: Put BSP as the first entry Linux complains in dmesg as a firmware bug that BSP is not the first entry. NetBSD hangs and OpenBSD panics early on boot. With this patch I was able to boot NetBSD and OpenBSD on darp10-b when loaded in GRUB. Note: vanilla bootloaders for NetBSD and OpenBSD still result in an apparent hang for an unknown reason. Change-Id: I520a2e080c9f07a5866729ae2283990d20c0d691 Signed-off-by: Vladimir Serbinenko <phcoder(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/86247 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Sean Rhodes <sean(a)starlabs.systems> Reviewed-by: Jérémy Compostella <jeremy.compostella(a)intel.com> --- M src/soc/intel/common/block/acpi/cpu_hybrid.c 1 file changed, 29 insertions(+), 12 deletions(-) Approvals: build bot (Jenkins): Verified Jérémy Compostella: Looks good to me, approved Sean Rhodes: Looks good to me, but someone else must approve diff --git a/src/soc/intel/common/block/acpi/cpu_hybrid.c b/src/soc/intel/common/block/acpi/cpu_hybrid.c index ee4777a..0e7ec53 100644 --- a/src/soc/intel/common/block/acpi/cpu_hybrid.c +++ b/src/soc/intel/common/block/acpi/cpu_hybrid.c @@ -18,8 +18,8 @@ struct cpu_apic_info_type { /* * Ordered APIC IDs based on core type. - * Array begins with Performance Cores' APIC IDs, - * then followed by Efficeint Cores's APIC IDs. + * Array begins with BSP, then come all Performance Cores' APIC IDs, + * then followed by Efficient Cores's APIC IDs. */ int32_t apic_ids[CONFIG_MAX_CPUS]; @@ -27,11 +27,15 @@ uint16_t total_cpu_cnt; /* - * Total Performance core count. This will be used - * to identify the start of Efficient Cores's + * This will be used to identify the start of Efficient Cores's * APIC ID list */ - uint16_t perf_cpu_cnt; + uint16_t eff_cores_start; + + /* + * Set to true if bsp is a performance core + */ + bool is_bsp_perf; }; static struct cpu_apic_info_type cpu_apic_info; @@ -48,12 +52,25 @@ int32_t eff_apic_ids[CONFIG_MAX_CPUS] = {0}; extern struct cpu_info cpu_infos[]; uint32_t i, j = 0; + u32 bsp_lapicid = lapicid(); + + /* As per spec first comes BSP. + See 5.2.12.1 MADT Processor Local APIC / SAPIC Structure Entry Order + " platform firmware should list the boot processor as the first + processor entry in the MADT. " + */ + cpu_apic_info.apic_ids[0] = bsp_lapicid; for (i = 0; i < ARRAY_SIZE(cpu_apic_info.apic_ids); i++) { if (!cpu_infos[i].cpu) continue; + if (cpu_infos[i].cpu->path.apic.apic_id == bsp_lapicid) { + cpu_apic_info.is_bsp_perf = + cpu_infos[i].cpu->path.apic.core_type == CPU_TYPE_PERF; + continue; + } if (cpu_infos[i].cpu->path.apic.core_type == CPU_TYPE_PERF) - cpu_apic_info.apic_ids[perf_core_cnt++] = + cpu_apic_info.apic_ids[1 + perf_core_cnt++] = cpu_infos[i].cpu->path.apic.apic_id; else eff_apic_ids[eff_core_cnt++] = @@ -61,18 +78,18 @@ } if (perf_core_cnt > 1) - bubblesort(cpu_apic_info.apic_ids, perf_core_cnt, NUM_ASCENDING); + bubblesort(cpu_apic_info.apic_ids + 1, perf_core_cnt, NUM_ASCENDING); - for (i = perf_core_cnt; j < eff_core_cnt; i++, j++) + for (i = perf_core_cnt + 1; j < eff_core_cnt; i++, j++) cpu_apic_info.apic_ids[i] = eff_apic_ids[j]; if (eff_core_cnt > 1) - bubblesort(&cpu_apic_info.apic_ids[perf_core_cnt], eff_core_cnt, NUM_ASCENDING); + bubblesort(&cpu_apic_info.apic_ids[perf_core_cnt + 1], eff_core_cnt, NUM_ASCENDING); /* Populate total core count */ - cpu_apic_info.total_cpu_cnt = perf_core_cnt + eff_core_cnt; + cpu_apic_info.total_cpu_cnt = perf_core_cnt + eff_core_cnt + 1; - cpu_apic_info.perf_cpu_cnt = perf_core_cnt; + cpu_apic_info.eff_cores_start = perf_core_cnt + 1; } static unsigned long acpi_create_madt_lapics_hybrid(unsigned long current) @@ -132,7 +149,7 @@ acpi_get_cpu_nomi_perf(&eff_core_nom_perf, &perf_core_nom_perf); - if (core_id < cpu_apic_info.perf_cpu_cnt) + if (core_id == 0 ? cpu_apic_info.is_bsp_perf : (core_id < cpu_apic_info.eff_cores_start)) acpigen_set_package_element_int(pkg_path, CPPC_NOM_PERF_IDX, perf_core_nom_perf); else acpigen_set_package_element_int(pkg_path, CPPC_NOM_PERF_IDX, -- To view, visit https://review.coreboot.org/c/coreboot/+/86247?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I520a2e080c9f07a5866729ae2283990d20c0d691 Gerrit-Change-Number: 86247 Gerrit-PatchSet: 7 Gerrit-Owner: Vladimir Serbinenko <phcoder(a)gmail.com> Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jeremy Soller <jeremy(a)system76.com> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Tim Crawford <tcrawford(a)system76.com>

1 0

[XS] Change in coreboot[main]: soc/intel/meteorlake: Add missing USB port definitions
by Matt DeVillier (Code Review) 07 Feb '25

07 Feb '25

Attention is currently required from: Dinesh Gehlot, Eran Mitrani, Intel coreboot Reviewers, Jakub "Kuba" Czapiga, Jérémy Compostella, Kapil Porwal, Subrata Banik, Tarun. Matt DeVillier has posted comments on this change by Matt DeVillier. ( https://review.coreboot.org/c/coreboot/+/86319?usp=email ) Change subject: soc/intel/meteorlake: Add missing USB port definitions ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/86319/comment/a45f6adb_60e06344?us… : PS1, Line 15: TSS_HSCI > TCSS_XHCI ? Done -- To view, visit https://review.coreboot.org/c/coreboot/+/86319?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I5dc97f150ff064d55e7969f10c1cea8ba72d6bfb Gerrit-Change-Number: 86319 Gerrit-PatchSet: 2 Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Reviewer: Jakub "Kuba" Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Jakub "Kuba" Czapiga <czapiga(a)google.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Comment-Date: Fri, 07 Feb 2025 20:59:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Jérémy Compostella <jeremy.compostella(a)intel.com>

1 0

[XS] Change in coreboot[main]: soc/intel/meteorlake: Add missing USB port definitions
by Matt DeVillier (Code Review) 07 Feb '25

07 Feb '25

Attention is currently required from: Dinesh Gehlot, Eran Mitrani, Intel coreboot Reviewers, Jakub "Kuba" Czapiga, Kapil Porwal, Matt DeVillier, Subrata Banik, Tarun. Hello Dinesh Gehlot, Eran Mitrani, Intel coreboot Reviewers, Jakub "Kuba" Czapiga, Kapil Porwal, Sean Rhodes, Subrata Banik, Tarun, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/86319?usp=email to look at the new patch set (#2). Change subject: soc/intel/meteorlake: Add missing USB port definitions ...................................................................... soc/intel/meteorlake: Add missing USB port definitions The TCSS_XHCI controller has a single USB2 port followed by 4 USB3 ports; the XHCI controller has 12 USB2 ports followed by 2 USB3 ports. The topology was queried from the root hub on each controller and returned via the descriptor. Add the 2 missing USB2 ports to the XHCI controller and the one to the TSS_XHCI controller. TEST=build/boot Win11, Linux 6.x on starlabs/starbook_mtl. Change-Id: I5dc97f150ff064d55e7969f10c1cea8ba72d6bfb Signed-off-by: Matt DeVillier <matt.devillier(a)gmail.com> --- M src/soc/intel/meteorlake/chipset.cb 1 file changed, 9 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/19/86319/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/86319?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I5dc97f150ff064d55e7969f10c1cea8ba72d6bfb Gerrit-Change-Number: 86319 Gerrit-PatchSet: 2 Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Reviewer: Jakub "Kuba" Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Jakub "Kuba" Czapiga <czapiga(a)google.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com>

1 0

[S] Change in coreboot[main]: soc/intel/cmn/blk: Remove boot partition check for forced cse sync
by Dinesh Gehlot (Code Review) 07 Feb '25

07 Feb '25

Attention is currently required from: Karthik Ramasubramanian, Martin L Roth. Dinesh Gehlot has posted comments on this change by Dinesh Gehlot. ( https://review.coreboot.org/c/coreboot/+/86153?usp=email ) Change subject: soc/intel/cmn/blk: Remove boot partition check for forced cse sync ...................................................................... Patch Set 10: (1 comment) File src/soc/intel/common/block/cse/cse_lite.c: https://review.coreboot.org/c/coreboot/+/86153/comment/01af882c_dbba8ab7?us… : PS4, Line 185: if (vboot_is_gbb_flag_set(VB2_GBB_FLAG_FORCE_CSE_SYNC)) { > Please update this check to: […] Thankyou, the linker issue is now resolved now with this. -- To view, visit https://review.coreboot.org/c/coreboot/+/86153?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: If1e4180cb5fec3990fdee2b0e412173b1c8c6ded Gerrit-Change-Number: 86153 Gerrit-PatchSet: 10 Gerrit-Owner: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-CC: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Fri, 07 Feb 2025 20:53:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Karthik Ramasubramanian <kramasub(a)google.com>

1 0

[XS] Change in coreboot[main]: soc/intel/meteorlake: Add missing USB port definitions
by Jérémy Compostella (Code Review) 07 Feb '25

07 Feb '25

Attention is currently required from: Dinesh Gehlot, Eran Mitrani, Intel coreboot Reviewers, Jakub "Kuba" Czapiga, Kapil Porwal, Matt DeVillier, Subrata Banik, Tarun. Jérémy Compostella has posted comments on this change by Matt DeVillier. ( https://review.coreboot.org/c/coreboot/+/86319?usp=email ) The change is no longer submittable: All-Comments-Resolved is unsatisfied now. Change subject: soc/intel/meteorlake: Add missing USB port definitions ...................................................................... Patch Set 1: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/86319/comment/ca96b211_46e66358?us… : PS1, Line 15: TSS_HSCI TCSS_XHCI ? -- To view, visit https://review.coreboot.org/c/coreboot/+/86319?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I5dc97f150ff064d55e7969f10c1cea8ba72d6bfb Gerrit-Change-Number: 86319 Gerrit-PatchSet: 1 Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Reviewer: Jakub "Kuba" Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Jakub "Kuba" Czapiga <czapiga(a)google.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Comment-Date: Fri, 07 Feb 2025 20:53:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[S] Change in coreboot[main]: soc/intel/cmn/blk: Remove boot partition check for forced cse sync
by Dinesh Gehlot (Code Review) 07 Feb '25

07 Feb '25

Attention is currently required from: Dinesh Gehlot, Intel coreboot Reviewers, Martin L Roth. Hello Intel coreboot Reviewers, Martin L Roth, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/86153?usp=email to look at the new patch set (#10). The following approvals got outdated and were removed: Verified-1 by build bot (Jenkins) Change subject: soc/intel/cmn/blk: Remove boot partition check for forced cse sync ...................................................................... soc/intel/cmn/blk: Remove boot partition check for forced cse sync This patch enhances the forced CSE sync mechanism by eliminating the boot partition check for RO. It utilizes the current CSE mechanism to determine if the system has undergone a cold boot. Please note, this is a proof of concept to address forced CSE sync constraint in the presence of a pre-CPU reset. The change is currently WIP and the final implementation might vary. BUG=b:380220737 TEST=Verified forced CSE sync on google/rex. Change-Id: If1e4180cb5fec3990fdee2b0e412173b1c8c6ded Signed-off-by: Dinesh Gehlot <digehlot(a)google.com> --- M src/soc/intel/common/block/cse/cse_lite.c M src/soc/intel/common/block/include/intelblocks/cse.h 2 files changed, 36 insertions(+), 8 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/53/86153/10 -- To view, visit https://review.coreboot.org/c/coreboot/+/86153?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: If1e4180cb5fec3990fdee2b0e412173b1c8c6ded Gerrit-Change-Number: 86153 Gerrit-PatchSet: 10 Gerrit-Owner: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-CC: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit February 2025