On Wed, Mar 6, 2019 at 3:52 AM Julius Werner <jwerner@chromium.org> wrote:
Hi folks,

I'm proposing a small policy change for the way we refer to boolean Kconfig variables in code. Right now, the directive is to always use the IS_ENABLED() macro. I would like to replace this with a shorter macro CONFIG() that also removes the need to type the CONFIG_ prefix again. The idea is mostly to save some horizontal space and the occasional extra line break for this boilerplate and make it a little easier to type.

I like it.

It's a very simple change (patch train starts at https://review.coreboot.org/c/coreboot/+/31773), but since it affects pretty much all code in coreboot and payloads I wanted to send out a quick FYI. Please let me know if anyone has any concerns with this.

While doing so, I think we should consider if some of these CONFIG_xx options are ones we should resolve runtime. Taking ACPI S3 feature as sample, we have acpi_s3_resume_is_allowed() to wrap IS_ENABLED(CONFIG_HAVE_ACPI_RESUME). So if one wanted to provide eg. CMOS bit to disable this, there is less source to change.

There is some indirect CONFIG_xxx changes when you switch payload in config. IMO it should be necessary to simply swap the payload binary in CBFS without rebuilding coreboot proper, so these should also resolve runtime, not build-time. (Sure, we would then need to peek early in process what payload we are going to boot.)

Kyösti