[coreboot-gerrit] [M] Change in coreboot[main]: sb/intel/bd82x6x: Add CFR objects for existing options

[Matt DeVillier (Code Review)]

Attention is currently required from: Angel Pons, Intel coreboot Reviewers.

Matt DeVillier has posted comments on this change by Matt DeVillier. ( https://review.coreboot.org/c/coreboot/+/87389?usp=email )

Change subject: sb/intel/bd82x6x: Add CFR objects for existing options
......................................................................

Patch Set 1:

(2 comments)

File src/southbridge/intel/bd82x6x/cfr.h:

https://review.coreboot.org/c/coreboot/+/87389/comment/b064f9e0_6d4e26be?usp... :
PS1, Line 19: enum {
            : 	MAINBOARD_POWER_STATE_OFF = 0,
            : 	MAINBOARD_POWER_STATE_ON,
            : 	MAINBOARD_POWER_STATE_PREVIOUS,
            : };
> I would've expected this enum to be defined elsewhere already

me too, but it's not, and I couldn't figure out a good place to add it. There's also some differing usage across the tree (eg, MAINBOARD_POWER_STATE_OFF vs MAINBOARD_POWER_OFF). Happy to unify if you have an idea how.

https://review.coreboot.org/c/coreboot/+/87389/comment/7b0e43a8_e44573d3?usp... :
PS1, Line 43: 0
> I would add an enum for this option as the mapping is reversed (0 = enabled)

I'd think the correct place for the enum would be in me.h, no?