Change in coreboot[master]: arch/acpi.h: Avoid explicitly assign values to enum names

[Name of user not set (Code Review)]

Name of user not set #1002476 has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36430 )

Change subject: arch/acpi.h: Avoid explicitly assign values to enum names
......................................................................

arch/acpi.h: Avoid explicitly assign values to enum names

Compiler by default assign values starting from the initial integral constant.

Change-Id: I1a5aa9762051780b67013f67f101112fc7a0bf6e
Signed-off-by: Himanshu Sahdev 
---
M src/arch/x86/include/arch/acpi.h
1 file changed, 19 insertions(+), 19 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/30/36430/1

diff --git a/src/arch/x86/include/arch/acpi.h b/src/arch/x86/include/arch/acpi.h
index 479067f..a9bd9f6 100644
--- a/src/arch/x86/include/arch/acpi.h
+++ b/src/arch/x86/include/arch/acpi.h
@@ -302,10 +302,10 @@

enum dev_scope_type {
    SCOPE_PCI_ENDPOINT = 1,
-	SCOPE_PCI_SUB = 2,
-	SCOPE_IOAPIC = 3,
-	SCOPE_MSI_HPET = 4,
-	SCOPE_ACPI_NAMESPACE_DEVICE = 5
+	SCOPE_PCI_SUB,
+	SCOPE_IOAPIC,
+	SCOPE_MSI_HPET,
+	SCOPE_ACPI_NAMESPACE_DEVICE
 };

typedef struct dev_scope {
@@ -321,11 +321,11 @@
 } __packed dev_scope_t;

enum dmar_type {
-	DMAR_DRHD = 0,
-	DMAR_RMRR = 1,
-	DMAR_ATSR = 2,
-	DMAR_RHSA = 3,
-	DMAR_ANDD = 4
+	DMAR_DRHD,
+	DMAR_RMRR,
+	DMAR_ATSR,
+	DMAR_RHSA,
+	DMAR_ANDD
 };

enum {
@@ -599,15 +599,15 @@

/* FADT Preferred Power Management Profile */
 enum acpi_preferred_pm_profiles {
-	PM_UNSPECIFIED		= 0,
-	PM_DESKTOP		= 1,
-	PM_MOBILE		= 2,
-	PM_WORKSTATION		= 3,
-	PM_ENTERPRISE_SERVER	= 4,
-	PM_SOHO_SERVER		= 5,
-	PM_APPLIANCE_PC		= 6,
-	PM_PERFORMANCE_SERVER	= 7,
-	PM_TABLET		= 8,	/* ACPI 5.0 & greater */
+	PM_UNSPECIFIED,
+	PM_DESKTOP,
+	PM_MOBILE,
+	PM_WORKSTATION3,
+	PM_ENTERPRISE_SERVER4,
+	PM_SOHO_SERVER,
+	PM_APPLIANCE_PC,
+	PM_PERFORMANCE_SERVER,
+	PM_TABLET,	/* ACPI 5.0 & greater */
 };

/* FACS (Firmware ACPI Control Structure) */
@@ -780,7 +780,7 @@
 };

enum acpi_ipmi_interface_type {
-	IPMI_INTERFACE_RESERVED = 0,
+	IPMI_INTERFACE_RESERVED,
    IPMI_INTERFACE_KCS,
    IPMI_INTERFACE_SMIC,
    IPMI_INTERFACE_BT,

[Name of user not set (Code Review)]

Name of user not set #1002476 has uploaded a new patch set (#2). ( https://review.coreboot.org/c/coreboot/+/36430 )

Change subject: arch/acpi.h: Avoid explicitly assign values to enum names
......................................................................

arch/acpi.h: Avoid explicitly assign values to enum names

Compiler by default assign values starting from the initial integral constant.

Change-Id: I1a5aa9762051780b67013f67f101112fc7a0bf6e
Signed-off-by: Himanshu Sahdev 
---
M src/arch/x86/include/arch/acpi.h
1 file changed, 19 insertions(+), 19 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/30/36430/2

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36430 )

Change subject: arch/acpi.h: Avoid explicitly assign values to enum names
......................................................................

Patch Set 2: Code-Review+2

[Patrick Georgi (Code Review)]

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36430 )

Change subject: arch/acpi.h: Avoid explicitly assign values to enum names
......................................................................

Patch Set 2:

Hm, I'm not too sure about this: these values are exported to outside consumers and so I think there's value making them explicit. Otherwise some future developer might decide to reorder the entries alphabetically, or add an entry somewhere in between.

[Name of user not set (Code Review)]

Name of user not set #1002476 has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36430 )

Change subject: arch/acpi.h: Avoid explicitly assign values to enum names
......................................................................

Patch Set 2:
> Patch Set 2:
>
> Hm, I'm not too sure about this: these values are exported to outside consumers and so I think there's value making them explicit. Otherwise some future developer might decide to reorder the entries alphabetically, or add an entry somewhere in between.

Yes, I understand your point but in my opinion,the concept of enum is very basic. Though considering the case, there is still no particular convention we are following in coreboot for this as here on line:783, only initial value is assigned, which again, being 0, is not a necessity. And we can see a lot of instances where different approaches are applied.
I am just trying to provide a better readability for only consecutive entries.

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has removed a vote from this change. ( https://review.coreboot.org/c/coreboot/+/36430 )

Change subject: arch/acpi.h: Avoid explicitly assign values to enum names
......................................................................

Removed Code-Review+2 by HAOUAS Elyes

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36430 )

Change subject: arch/acpi.h: Avoid explicitly assign values to enum names
......................................................................

Patch Set 2:

+2 removed to avoid any merge bu mistake.
this said, we indeed need a "convention" for enums

[S] Change in coreboot[master]: arch/acpi.h: Avoid explicitly assign values to enum names

[Martin L Roth (Code Review)]

Martin L Roth has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/36430?usp=email )

Change subject: arch/acpi.h: Avoid explicitly assign values to enum names
......................................................................

Abandoned

This patch has not been touched in over 12 months. Anyone who wants to take over work on this patch, please feel free to restore it and do any work needed to get it merged. If you create a new patch based on this work, please credit the original author.