[coreboot-gerrit] [L] Change in coreboot[main]: mb/prodrive/atlas: Add initial support for options

[Angel Pons (Code Review)]

Attention is currently required from: Kyösti Mälkki, Lean Sheng Tan, Maximilian Brune, Patrick Rudolph.

Angel Pons has posted comments on this change by Angel Pons. ( https://review.coreboot.org/c/coreboot/+/74122?usp=email )

Change subject: mb/prodrive/atlas: Add initial support for options
......................................................................

Patch Set 28: Code-Review+1

(6 comments)

File src/mainboard/prodrive/atlas/Kconfig:

https://review.coreboot.org/c/coreboot/+/74122/comment/2d53df62_e59b2626?usp... :
PS28, Line 16: 	select DRIVERS_OPTION_CFR_ENABLED
Does this mean "CFR force-enabled" or "CFR enabled by default"?

File src/mainboard/prodrive/atlas/cfr.c:

https://review.coreboot.org/c/coreboot/+/74122/comment/cbba2f82_5f89c1f7?usp... :
PS28, Line 15: const struct sm_object *obj, struct sm_object *new
Is `obj` ever used?

https://review.coreboot.org/c/coreboot/+/74122/comment/94c36948_42819530?usp... :
PS28, Line 27: new->sm_enum.default_value = false;
enum default value is an unsigned integer, IIRC

```suggestion
    	new->sm_enum.default_value = 0;
```

https://review.coreboot.org/c/coreboot/+/74122/comment/27b42e93_8bb439b6?usp... :
PS28, Line 219: (const struct sm_object *[])
I wonder if casting is necessary

https://review.coreboot.org/c/coreboot/+/74122/comment/296ba380_ab95dc49?usp... :
PS28, Line 247: &cpu,
              : 	&main,
Should we reverse these so that Main is the first options page?

https://review.coreboot.org/c/coreboot/+/74122/comment/d89912b2_0b432604?usp... :
PS28, Line 252: /*
              :  * TODO: Writing this by hand is extremely tedious. Introducing a DSL
              :  * (Domain-Specific Language) to describe options which is translated
              :  * into code at build time may be the way to go. Maybe expand SCONFIG
              :  * so that these can be devicetree options?
              :  */
Ah yes, I see this is still quite true