[coreboot] Re: RFC: Replacing IS_ENABLED(CONFIG_XXX) with the shorter CONFIG(XXX)

2019-03-05 Thread Patrick Rudolph
On Tue, 2019-03-05 at 17:52 -0800, Julius Werner 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.
> 
Sounds like a good plan. Please keep Documentation in sync:
Documentation/core/Kconfig.md seems to cover the implementation
details.

> 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.
> ___
> coreboot mailing list -- coreboot@coreboot.org
> To unsubscribe send an email to coreboot-le...@coreboot.org
-- 
Patrick Rudolph

9elements Agency GmbH, Kortumstraße 19-21, 44787 Bochum, Germany
Email:  patrick.rudo...@9elements.com
Phone:  +49 234 68 94 188

Sitz der Gesellschaft: Bochum
Handelsregister: Amtsgericht Bochum, HRB 17519
Geschäftsführung: Sebastian Deutsch, Daniel Hoelzgen
___
coreboot mailing list -- coreboot@coreboot.org
To unsubscribe send an email to coreboot-le...@coreboot.org


[coreboot] Re: RFC: Replacing IS_ENABLED(CONFIG_XXX) with the shorter CONFIG(XXX)

2019-03-05 Thread Hung-Te Lin
Will it be more explicit if we call it HAS_CONFIG(XXX) ? Or HAS_KCONFIG(XXX)
CONFIG(XXX) seems too generic to me that some drivers may wan to use it for
wrapping a reference to config tables, like GPIO(XXX).

On Wed, Mar 6, 2019 at 9:52 AM Julius Werner  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.
>
> 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.
> ___
> coreboot mailing list -- coreboot@coreboot.org
> To unsubscribe send an email to coreboot-le...@coreboot.org
>
___
coreboot mailing list -- coreboot@coreboot.org
To unsubscribe send an email to coreboot-le...@coreboot.org


[coreboot] Re: RFC: Replacing IS_ENABLED(CONFIG_XXX) with the shorter CONFIG(XXX)

2019-03-05 Thread ron minnich
I like it too

On Tue, Mar 5, 2019 at 8:21 PM Kyösti Mälkki  wrote:
>
>
>
> On Wed, Mar 6, 2019 at 3:52 AM Julius Werner  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
> ___
> coreboot mailing list -- coreboot@coreboot.org
> To unsubscribe send an email to coreboot-le...@coreboot.org
___
coreboot mailing list -- coreboot@coreboot.org
To unsubscribe send an email to coreboot-le...@coreboot.org


[coreboot] Re: RFC: Replacing IS_ENABLED(CONFIG_XXX) with the shorter CONFIG(XXX)

2019-03-05 Thread Kyösti Mälkki
On Wed, Mar 6, 2019 at 3:52 AM Julius Werner  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
___
coreboot mailing list -- coreboot@coreboot.org
To unsubscribe send an email to coreboot-le...@coreboot.org


[coreboot] RFC: Replacing IS_ENABLED(CONFIG_XXX) with the shorter CONFIG(XXX)

2019-03-05 Thread Julius Werner
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.

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.
___
coreboot mailing list -- coreboot@coreboot.org
To unsubscribe send an email to coreboot-le...@coreboot.org