Change in ...coreboot[master]: soc/intel/skylake: Fix the warning "type 'hex' are always defined"

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31050

Change subject: soc/intel/skylake: Fix the warning "type 'hex' are always defined"
......................................................................

soc/intel/skylake: Fix the warning "type 'hex' are always defined"

This is spoted using "./util/lint/kconfig_lint"

Change-Id: Icfda267936a23d9d14832116d67571f42f685906
Signed-off-by: Elyes HAOUAS 
---
M src/soc/intel/skylake/chip_fsp20.c
1 file changed, 10 insertions(+), 8 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/50/31050/1

diff --git a/src/soc/intel/skylake/chip_fsp20.c b/src/soc/intel/skylake/chip_fsp20.c
index 18c2aef..3ade8d7 100644
--- a/src/soc/intel/skylake/chip_fsp20.c
+++ b/src/soc/intel/skylake/chip_fsp20.c
@@ -391,14 +391,16 @@
    	params->SpiFlashCfgLockDown = 0;
    }
    /* only replacing preexisting subsys ID defaults when non-zero */
-#if defined(CONFIG_SUBSYSTEM_VENDOR_ID) && CONFIG_SUBSYSTEM_VENDOR_ID
-	params->DefaultSvid = CONFIG_SUBSYSTEM_VENDOR_ID;
-	params->PchSubSystemVendorId = CONFIG_SUBSYSTEM_VENDOR_ID;
-#endif
-#if defined(CONFIG_SUBSYSTEM_DEVICE_ID) && CONFIG_SUBSYSTEM_DEVICE_ID
-	params->DefaultSid = CONFIG_SUBSYSTEM_DEVICE_ID;
-	params->PchSubSystemId = CONFIG_SUBSYSTEM_DEVICE_ID;
-#endif
+	if (CONFIG_SUBSYSTEM_VENDOR_ID != 0) {
+		params->DefaultSvid = CONFIG_SUBSYSTEM_VENDOR_ID;
+		params->PchSubSystemVendorId = CONFIG_SUBSYSTEM_VENDOR_ID;
+	}
+
+	if (CONFIG_SUBSYSTEM_DEVICE_ID != 0) {
+		params->DefaultSid = CONFIG_SUBSYSTEM_DEVICE_ID;
+		params->PchSubSystemId = CONFIG_SUBSYSTEM_DEVICE_ID;
+	}
+
    params->PchPmWolEnableOverride = config->WakeConfigWolEnableOverride;
    params->PchPmPcieWakeFromDeepSx = config->WakeConfigPcieWakeFromDeepSx;
    params->PchPmDeepSxPol = config->PmConfigDeepSxPol;

[HAOUAS Elyes (Code Review)]

Hello Patrick Rudolph, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/31050

to look at the new patch set (#2).

Change subject: soc/intel/skylake: Fix the warning "type 'hex' are always defined"
......................................................................

soc/intel/skylake: Fix the warning "type 'hex' are always defined"

This is spoted using "./util/lint/kconfig_lint"

Change-Id: Icfda267936a23d9d14832116d67571f42f685906
Signed-off-by: Elyes HAOUAS 
---
M src/soc/intel/skylake/chip.c
M src/soc/intel/skylake/chip_fsp20.c
2 files changed, 16 insertions(+), 14 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/50/31050/2

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31050 )

Change subject: soc/intel/skylake: Fix the warning "type 'hex' are always defined"
......................................................................

Patch Set 2: Code-Review+1

(2 comments)

https://review.coreboot.org/#/c/31050/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/31050/2//COMMIT_MSG@9 
PS2, Line 9: spoted
spotted

https://review.coreboot.org/#/c/31050/2//COMMIT_MSG@10 
PS2, Line 10: 
Please add something like:
> While at it, do the check in C and not the preprocessor.

[HAOUAS Elyes (Code Review)]

Hello Patrick Rudolph, Paul Menzel, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/31050

to look at the new patch set (#3).

Change subject: soc/intel/skylake: Fix the warning "type 'hex' are always defined"
......................................................................

soc/intel/skylake: Fix the warning "type 'hex' are always defined"

This is sppoted using "./util/lint/kconfig_lint"
While at it, do the check in C and not the preprocessor.

Change-Id: Icfda267936a23d9d14832116d67571f42f685906
Signed-off-by: Elyes HAOUAS 
---
M src/soc/intel/skylake/chip.c
M src/soc/intel/skylake/chip_fsp20.c
2 files changed, 16 insertions(+), 14 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/50/31050/3

Change in ...coreboot[master]: src: Fix the warning "type 'hex' are always defined"

[HAOUAS Elyes (Code Review)]

Hello Patrick Rudolph, Paul Menzel, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/31050

to look at the new patch set (#4).

Change subject: src: Fix the warning "type 'hex' are always defined"
......................................................................

src: Fix the warning "type 'hex' are always defined"

This is sppoted using "./util/lint/kconfig_lint"
While at it, do the check in C and not the preprocessor.

Change-Id: Icfda267936a23d9d14832116d67571f42f685906
Signed-off-by: Elyes HAOUAS 
---
M src/arch/x86/include/arch/memlayout.h
M src/device/pci_ops_mmconf.c
M src/drivers/elog/boot_count.c
M src/include/pc80/mc146818rtc.h
M src/soc/intel/common/block/pcr/pcr.c
M src/soc/intel/skylake/chip.c
M src/soc/intel/skylake/chip_fsp20.c
7 files changed, 21 insertions(+), 19 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/50/31050/4

Change in ...coreboot[master]: src: Fix the warning "type 'hex' are always defined"

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31050 )

Change subject: src: Fix the warning "type 'hex' are always defined"
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/#/c/31050/4/src/arch/x86/include/arch/memlayout.... 
File src/arch/x86/include/arch/memlayout.h:

https://review.coreboot.org/#/c/31050/4/src/arch/x86/include/arch/memlayout.... 
PS4, Line 26: CONFIG_RAMTOP
Isn't this sort of thing supposed to be using IS_ENABLED()? I thought that's what it was for.

Change in ...coreboot[master]: src: Fix the warning "type 'hex' are always defined"

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31050 )

Change subject: src: Fix the warning "type 'hex' are always defined"
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/#/c/31050/4/src/arch/x86/include/arch/memlayout.... 
File src/arch/x86/include/arch/memlayout.h:

https://review.coreboot.org/#/c/31050/4/src/arch/x86/include/arch/memlayout.... 
PS4, Line 26: CONFIG_RAMTOP
> Isn't this sort of thing supposed to be using IS_ENABLED()? I thought that's what it was for.

AFAIR IS_ENABLED() only works with booleans.

Change in ...coreboot[master]: src: Fix the warning "type 'hex' are always defined"

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31050 )

Change subject: src: Fix the warning "type 'hex' are always defined"
......................................................................

Patch Set 4: Code-Review+2

(1 comment)

https://review.coreboot.org/#/c/31050/4//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/31050/4//COMMIT_MSG@9 
PS4, Line 9: This is sppoted using "./util/lint/kconfig_lint"
spotted

Change in ...coreboot[master]: src: Fix the warning "type 'hex' are always defined"

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31050 )

Change subject: src: Fix the warning "type 'hex' are always defined"
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/#/c/31050/4/src/arch/x86/include/arch/memlayout.... 
File src/arch/x86/include/arch/memlayout.h:

https://review.coreboot.org/#/c/31050/4/src/arch/x86/include/arch/memlayout.... 
PS4, Line 26: CONFIG_RAMTOP
> AFAIR IS_ENABLED() only works with booleans.

Ohh, this is a hex... okay, carry on then. (Maybe writing (CONFIG_RAMTOP == 0x0) would make that clearer?)

Change in ...coreboot[master]: src: Fix the warning "type 'hex' are always defined"

[HAOUAS Elyes (Code Review)]

Hello Patrick Rudolph, Julius Werner, Paul Menzel, build bot (Jenkins), Nico Huber,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/31050

to look at the new patch set (#5).

Change subject: src: Fix the warning "type 'hex' are always defined"
......................................................................

src: Fix the warning "type 'hex' are always defined"

This is spotted using "./util/lint/kconfig_lint"
While at it, do the check in C and not the preprocessor.

Change-Id: Icfda267936a23d9d14832116d67571f42f685906
Signed-off-by: Elyes HAOUAS 
---
M src/arch/x86/include/arch/memlayout.h
M src/device/pci_ops_mmconf.c
M src/drivers/elog/boot_count.c
M src/include/pc80/mc146818rtc.h
M src/soc/intel/common/block/pcr/pcr.c
M src/soc/intel/skylake/chip.c
M src/soc/intel/skylake/chip_fsp20.c
7 files changed, 21 insertions(+), 19 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/50/31050/5

Change in ...coreboot[master]: src: Fix the warning "type 'hex' are always defined"

[HAOUAS Elyes (Code Review)]

Hello Patrick Rudolph, Julius Werner, Paul Menzel, build bot (Jenkins), Nico Huber,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/31050

to look at the new patch set (#6).

Change subject: src: Fix the warning "type 'hex' are always defined"
......................................................................

src: Fix the warning "type 'hex' are always defined"

This is spotted using "./util/lint/kconfig_lint"
While at it, do the check in C and not the preprocessor.

Change-Id: Icfda267936a23d9d14832116d67571f42f685906
Signed-off-by: Elyes HAOUAS 
---
M src/arch/x86/include/arch/memlayout.h
M src/device/pci_ops_mmconf.c
M src/drivers/elog/boot_count.c
M src/include/pc80/mc146818rtc.h
M src/soc/intel/common/block/pcr/pcr.c
M src/soc/intel/skylake/chip.c
M src/soc/intel/skylake/chip_fsp20.c
7 files changed, 21 insertions(+), 19 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/50/31050/6

Change in ...coreboot[master]: src: Fix the warning "type 'hex' are always defined"

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31050 )

Change subject: src: Fix the warning "type 'hex' are always defined"
......................................................................

Patch Set 6: Code-Review+2

Change in ...coreboot[master]: src: Fix the warning "type 'hex' are always defined"

[Nico Huber (Code Review)]

Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31050 )

Change subject: src: Fix the warning "type 'hex' are always defined"
......................................................................

src: Fix the warning "type 'hex' are always defined"

This is spotted using "./util/lint/kconfig_lint"
While at it, do the check in C and not the preprocessor.

Change-Id: Icfda267936a23d9d14832116d67571f42f685906
Signed-off-by: Elyes HAOUAS 
Reviewed-on: https://review.coreboot.org/c/31050
Tested-by: build bot (Jenkins) 
Reviewed-by: Nico Huber 
---
M src/arch/x86/include/arch/memlayout.h
M src/device/pci_ops_mmconf.c
M src/drivers/elog/boot_count.c
M src/include/pc80/mc146818rtc.h
M src/soc/intel/common/block/pcr/pcr.c
M src/soc/intel/skylake/chip.c
M src/soc/intel/skylake/chip_fsp20.c
7 files changed, 21 insertions(+), 19 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Nico Huber: Looks good to me, approved

diff --git a/src/arch/x86/include/arch/memlayout.h b/src/arch/x86/include/arch/memlayout.h
index f93dece..de80f42 100644
--- a/src/arch/x86/include/arch/memlayout.h
+++ b/src/arch/x86/include/arch/memlayout.h
@@ -23,7 +23,7 @@
 #define ARCH_STAGE_HAS_BSS_SECTION 0
 #endif

-#if !defined(CONFIG_RAMTOP) || !CONFIG_RAMTOP
+#if (CONFIG_RAMTOP == 0)
 # error "CONFIG_RAMTOP not configured"
 #endif

diff --git a/src/device/pci_ops_mmconf.c b/src/device/pci_ops_mmconf.c
index 04f3d23..f321347 100644
--- a/src/device/pci_ops_mmconf.c
+++ b/src/device/pci_ops_mmconf.c
@@ -15,7 +15,7 @@
 #include <device/pci.h>
 #include <device/pci_ops.h>

-#if !defined(CONFIG_MMCONF_BASE_ADDRESS) || !CONFIG_MMCONF_BASE_ADDRESS
+#if (CONFIG_MMCONF_BASE_ADDRESS == 0)
 #error "CONFIG_MMCONF_BASE_ADDRESS needs to be non-zero!"
 #endif

diff --git a/src/drivers/elog/boot_count.c b/src/drivers/elog/boot_count.c
index c4373ee..fd86f39 100644
--- a/src/drivers/elog/boot_count.c
+++ b/src/drivers/elog/boot_count.c
@@ -30,7 +30,7 @@
 # include "option_table.h"
 # define BOOT_COUNT_CMOS_OFFSET (CMOS_VSTART_boot_count_offset >> 3)
 #else
-# if defined(CONFIG_ELOG_BOOT_COUNT_CMOS_OFFSET) && CONFIG_ELOG_BOOT_COUNT_CMOS_OFFSET
+# if (CONFIG_ELOG_BOOT_COUNT_CMOS_OFFSET != 0)
 #  define BOOT_COUNT_CMOS_OFFSET CONFIG_ELOG_BOOT_COUNT_CMOS_OFFSET
 # else
 #  error "Must configure CONFIG_ELOG_BOOT_COUNT_CMOS_OFFSET"
diff --git a/src/include/pc80/mc146818rtc.h b/src/include/pc80/mc146818rtc.h
index 5d7497d..5b71c3d 100644
--- a/src/include/pc80/mc146818rtc.h
+++ b/src/include/pc80/mc146818rtc.h
@@ -198,7 +198,7 @@
 # include "option_table.h"
 # define CMOS_POST_OFFSET (CMOS_VSTART_cmos_post_offset >> 3)
 #else
-# if defined(CONFIG_CMOS_POST_OFFSET) && CONFIG_CMOS_POST_OFFSET
+# if (CONFIG_CMOS_POST_OFFSET != 0)
 #  define CMOS_POST_OFFSET CONFIG_CMOS_POST_OFFSET
 # else
 #  error "Must configure CONFIG_CMOS_POST_OFFSET"
diff --git a/src/soc/intel/common/block/pcr/pcr.c b/src/soc/intel/common/block/pcr/pcr.c
index 804ccff..e354c03 100644
--- a/src/soc/intel/common/block/pcr/pcr.c
+++ b/src/soc/intel/common/block/pcr/pcr.c
@@ -21,7 +21,7 @@
 #include <soc/pci_devs.h>
 #include <timer.h>

-#if !defined(CONFIG_PCR_BASE_ADDRESS) || (CONFIG_PCR_BASE_ADDRESS == 0)
+#if (CONFIG_PCR_BASE_ADDRESS == 0)
 #error "PCR_BASE_ADDRESS need to be non-zero!"
 #endif

diff --git a/src/soc/intel/skylake/chip.c b/src/soc/intel/skylake/chip.c
index 07ac4e8..cbbfaaa 100644
--- a/src/soc/intel/skylake/chip.c
+++ b/src/soc/intel/skylake/chip.c
@@ -170,12 +170,12 @@
    	params->LockDownConfigSpiEiss = 0;
    }
    /* only replacing preexisting subsys ID defaults when non-zero */
-#if defined(CONFIG_SUBSYSTEM_VENDOR_ID) && CONFIG_SUBSYSTEM_VENDOR_ID
-	params->PchConfigSubSystemVendorId = CONFIG_SUBSYSTEM_VENDOR_ID;
-#endif
-#if defined(CONFIG_SUBSYSTEM_DEVICE_ID) && CONFIG_SUBSYSTEM_DEVICE_ID
-	params->PchConfigSubSystemId = CONFIG_SUBSYSTEM_DEVICE_ID;
-#endif
+	if (CONFIG_SUBSYSTEM_VENDOR_ID != 0)
+		params->PchConfigSubSystemVendorId = CONFIG_SUBSYSTEM_VENDOR_ID;
+
+	if (CONFIG_SUBSYSTEM_DEVICE_ID != 0)
+		params->PchConfigSubSystemId = CONFIG_SUBSYSTEM_DEVICE_ID;
+
    params->WakeConfigWolEnableOverride =
    	config->WakeConfigWolEnableOverride;
    params->WakeConfigPcieWakeFromDeepSx =
diff --git a/src/soc/intel/skylake/chip_fsp20.c b/src/soc/intel/skylake/chip_fsp20.c
index 18c2aef..3ade8d7 100644
--- a/src/soc/intel/skylake/chip_fsp20.c
+++ b/src/soc/intel/skylake/chip_fsp20.c
@@ -391,14 +391,16 @@
    	params->SpiFlashCfgLockDown = 0;
    }
    /* only replacing preexisting subsys ID defaults when non-zero */
-#if defined(CONFIG_SUBSYSTEM_VENDOR_ID) && CONFIG_SUBSYSTEM_VENDOR_ID
-	params->DefaultSvid = CONFIG_SUBSYSTEM_VENDOR_ID;
-	params->PchSubSystemVendorId = CONFIG_SUBSYSTEM_VENDOR_ID;
-#endif
-#if defined(CONFIG_SUBSYSTEM_DEVICE_ID) && CONFIG_SUBSYSTEM_DEVICE_ID
-	params->DefaultSid = CONFIG_SUBSYSTEM_DEVICE_ID;
-	params->PchSubSystemId = CONFIG_SUBSYSTEM_DEVICE_ID;
-#endif
+	if (CONFIG_SUBSYSTEM_VENDOR_ID != 0) {
+		params->DefaultSvid = CONFIG_SUBSYSTEM_VENDOR_ID;
+		params->PchSubSystemVendorId = CONFIG_SUBSYSTEM_VENDOR_ID;
+	}
+
+	if (CONFIG_SUBSYSTEM_DEVICE_ID != 0) {
+		params->DefaultSid = CONFIG_SUBSYSTEM_DEVICE_ID;
+		params->PchSubSystemId = CONFIG_SUBSYSTEM_DEVICE_ID;
+	}
+
    params->PchPmWolEnableOverride = config->WakeConfigWolEnableOverride;
    params->PchPmPcieWakeFromDeepSx = config->WakeConfigPcieWakeFromDeepSx;
    params->PchPmDeepSxPol = config->PmConfigDeepSxPol;