Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41440 )
Change subject: sconfig: Add support for firmware configuration ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/41440/1/util/sconfig/main.c File util/sconfig/main.c:
https://review.coreboot.org/c/coreboot/+/41440/1/util/sconfig/main.c@373 PS1, Line 373: if (!field) {
This is what lets a variant add options to an existing field in overridetree. […]
I'm going to split this and support 3 different types of 'field'
field <start> <end> [option...] end field <bit> [option...] end // single-bit field field [option...] end // when adding options to a field
This way when a field is defined with bits it has to be a new field, and when a variant wants to add options to a field it doesn't need to supply the bits and the field must already exist.
https://review.coreboot.org/c/coreboot/+/41440/1/util/sconfig/main.c@392 PS1, Line 392: option = option->next;
meta question: Do we want to let a variant override an option? (rather than just add options to exi […]
After thinking about it I'm going to say that we don't want to allow this for now and I'll make the code throw an error it it is attempted. There should be sufficient flexibility with adding new fields and options that this should not be required.