HAOUAS Elyes has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31689
Change subject: arch/x86/include/arch/acpi.h: Clean-up ......................................................................
arch/x86/include/arch/acpi.h: Clean-up
As we are running ACPI v3.0, references to older than v3.0 are removed. Also clean comments.
Change-Id: I0cce0035ed2b952d59cc1a4a9e6017dae67ef6db Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/arch/x86/include/arch/acpi.h 1 file changed, 13 insertions(+), 22 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/31689/1
diff --git a/src/arch/x86/include/arch/acpi.h b/src/arch/x86/include/arch/acpi.h index aced58a..225b8e2 100644 --- a/src/arch/x86/include/arch/acpi.h +++ b/src/arch/x86/include/arch/acpi.h @@ -90,7 +90,7 @@ char signature[8]; /* RSDP signature */ u8 checksum; /* Checksum of the first 20 bytes */ char oem_id[6]; /* OEM ID */ - u8 revision; /* 0 for ACPI 1.0, 2 for ACPI 2.0/3.0/4.0/6.2a */ + u8 revision; /* RSDP revision */ u32 rsdt_address; /* Physical address of RSDT (32 bits) */ u32 length; /* Total RSDP length (incl. extended part) */ u64 xsdt_address; /* Physical address of XSDT (64 bits) */ @@ -104,12 +104,7 @@ u8 space_id; /* Address space ID */ u8 bit_width; /* Register size in bits */ u8 bit_offset; /* Register bit offset */ - union { - u8 resv; /* Reserved in ACPI 2.0 - 2.0b */ - u8 access_size; /* Access size in - * ACPI 2.0c/3.0/4.0/5.0/6.2a - */ - }; + u8 access_size; /* Access size */ u32 addrl; /* Register address, low 32 bits */ u32 addrh; /* Register address, high 32 bits */ } __packed acpi_addr_t; @@ -484,11 +479,7 @@ struct acpi_table_header header; u32 firmware_ctrl; u32 dsdt; - u8 reserved; /* Eliminated in ACPI 2.0. Platforms should set - * this field to zero but field values of one - * are also allowed to maintain compatibility - * with ACPI 1.0. - */ + u8 reserved; /* This value is 0 */ u8 preferred_pm_profile; u16 sci_int; u32 smi_cmd; @@ -578,7 +569,7 @@ /* Bits 20-31: reserved ACPI 3.0 & 4.0 */ #define ACPI_FADT_HW_REDUCED_ACPI (1 << 20) #define ACPI_FADT_LOW_PWR_IDLE_S0 (1 << 21) -/* bits 22-31: reserved ACPI 5.0/6.2a */ +/* bits 22-31: reserved ACPI 5.0/6.3 */
/* FADT Boot Architecture Flags */ #define ACPI_FADT_LEGACY_DEVICES (1 << 0) @@ -592,7 +583,7 @@ /* FADT ARM Boot Architecture Flags */ #define ACPI_FADT_ARM_PSCI_COMPLIANT (1 << 0) #define ACPI_FADT_ARM_PSCI_USE_HVC (1 << 1) -/* bits 2-16: reserved ACPI 6.2a */ +/* bits 2-16: reserved ACPI 6.3 */
/* FADT Preferred Power Management Profile */ enum acpi_preferred_pm_profiles { @@ -604,7 +595,7 @@ PM_SOHO_SERVER = 5, PM_APPLIANCE_PC = 6, PM_PERFORMANCE_SERVER = 7, - PM_TABLET = 8, /* ACPI 5.0/6.2a */ + PM_TABLET = 8, /* ACPI 5.0 & greater */ };
/* FACS (Firmware ACPI Control Structure) */ @@ -617,10 +608,10 @@ u32 flags; /* FACS flags */ u32 x_firmware_waking_vector_l; /* X FW waking vector, low */ u32 x_firmware_waking_vector_h; /* X FW waking vector, high */ - u8 version; /* ACPI 6.2-A: 2 */ - u8 resv1[3]; /* ACPI 6.2-A: 0 */ + u8 version; /* FACS version */ + u8 resv1[3]; /* This value is 0 */ u32 ospm_flags; /* 64BIT_WAKE_F */ - u8 resv2[24]; /* ACPI 6.2-A: 0 */ + u8 resv2[24]; /* This value is 0 */ } __packed acpi_facs_t;
/* FACS flags */ @@ -678,7 +669,7 @@ u64 error_region; } __packed acpi_bert_t;
-/* Generic Error Data Entry (ACPI spec v6.2-A, table 382) */ +/* Generic Error Data Entry (ACPI spec v6.3, Table 18-392) */ typedef struct acpi_hest_generic_data { guid_t section_type; u32 error_severity; @@ -691,7 +682,7 @@ /* error data */ } __packed acpi_hest_generic_data_t;
-/* Generic Error Data Entry (ACPI spec v6.2-A, table 382) */ +/* Generic Error Data Entry (ACPI spec v6.3, Table 18-392) */ typedef struct acpi_hest_generic_data_v300 { guid_t section_type; u32 error_severity; @@ -717,7 +708,7 @@ #define ACPI_GENERROR_VALID_FRUID_TEXT BIT(1) #define ACPI_GENERROR_VALID_TIMESTAMP BIT(2)
-/* Generic Error Status Block (ACPI spec v6.2-A, table 381) */ +/* Generic Error Status Block (ACPI spec v6.3, Table 18-391) */ typedef struct acpi_generic_error_status { u32 block_status; u32 raw_data_offset; /* must follow any generic entries */ @@ -755,7 +746,7 @@
/* Port types for ACPI _UPC object */ enum acpi_upc_type { - /* These types are defined in ACPI 6.2 section 9.14 */ + /* These types are defined in ACPI 6.3 section 9.14 */ UPC_TYPE_A, UPC_TYPE_MINI_AB, UPC_TYPE_EXPRESSCARD,
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31689
to look at the new patch set (#2).
Change subject: arch/x86/include/arch/acpi.h: Clean-up ......................................................................
arch/x86/include/arch/acpi.h: Clean-up
As we are running ACPI v3.0, references to older than v3.0 are removed. Also clean comments.
Change-Id: I0cce0035ed2b952d59cc1a4a9e6017dae67ef6db Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/arch/x86/include/arch/acpi.h 1 file changed, 14 insertions(+), 20 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/31689/2
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31689
to look at the new patch set (#3).
Change subject: arch/x86/include/arch/acpi.h: Clean-up ......................................................................
arch/x86/include/arch/acpi.h: Clean-up
As we are running ACPI v3.0, references to older than v3.0 are removed. Also clean comments.
Change-Id: I0cce0035ed2b952d59cc1a4a9e6017dae67ef6db Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/arch/x86/include/arch/acpi.h 1 file changed, 14 insertions(+), 21 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/31689/3
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31689 )
Change subject: arch/x86/include/arch/acpi.h: Clean-up ......................................................................
Patch Set 3:
(14 comments)
https://review.coreboot.org/#/c/31689/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/31689/3//COMMIT_MSG@7 PS3, Line 7: arch/x86/include/arch/acpi.h Too much information, `arch/x86/acpi:` should be enough
https://review.coreboot.org/#/c/31689/3//COMMIT_MSG@7 PS3, Line 7: arch/x86/include/arch/acpi.h: Clean-up This could be more specific. If I didn't miss anything, you are only changing comments? So `Clean up comments` maybe?
https://review.coreboot.org/#/c/31689/3/src/arch/x86/include/arch/acpi.h File src/arch/x86/include/arch/acpi.h:
https://review.coreboot.org/#/c/31689/3/src/arch/x86/include/arch/acpi.h@106 PS3, Line 106: union { /* TODO: Remove obsolete 'u8 resv' */ Why comment and not just do it (in a separate commit)?
https://review.coreboot.org/#/c/31689/3/src/arch/x86/include/arch/acpi.h@108 PS3, Line 108: in upper versions please be more specific, e.g. `since ACPI 2.0c`
https://review.coreboot.org/#/c/31689/3/src/arch/x86/include/arch/acpi.h@484 PS3, Line 484: u8 reserved; /* This value is 0 */ Sounds odd, maybe "Should be 0"? But actually, we wouldn't set anything else anyway, would we? so why a comment at all?
https://review.coreboot.org/#/c/31689/3/src/arch/x86/include/arch/acpi.h@574 PS3, Line 574: /* bits 22-31: reserved ACPI 5.0/6.3 */ Please don't. if anything, makes this something like "reserved since ACPI 5.0" or "reserved in ACPI 5.0+". Keeping this up to date only encourages future noise.
https://review.coreboot.org/#/c/31689/3/src/arch/x86/include/arch/acpi.h@588 PS3, Line 588: /* bits 2-16: reserved ACPI 6.3 */ same, but actually since when? Actually, how can this comment be useful?
https://review.coreboot.org/#/c/31689/3/src/arch/x86/include/arch/acpi.h@600 PS3, Line 600: PM_TABLET = 8, /* ACPI 5.0 & greater */ ah, that's better
https://review.coreboot.org/#/c/31689/3/src/arch/x86/include/arch/acpi.h@614 PS3, Line 614: his value is 0 who cares?
https://review.coreboot.org/#/c/31689/3/src/arch/x86/include/arch/acpi.h@613 PS3, Line 613: u8 version; /* FACS version */ : u8 resv1[3]; /* This value is 0 */ : u32 ospm_flags; /* 64BIT_WAKE_F */ : u8 resv2[24]; /* This value is 0 */ I was surprised to see that you added the earlier comments... please make sure that you are content with the result this time to avoid future noise.
https://review.coreboot.org/#/c/31689/3/src/arch/x86/include/arch/acpi.h@674 PS3, Line 674: (ACPI spec v6.3, Table 18-392) Please just drop the reference, it's not worth to keep this up to date. We have full-text search features in PDF viewers, don't we?
https://review.coreboot.org/#/c/31689/3/src/arch/x86/include/arch/acpi.h@687 PS3, Line 687: /* Generic Error Data Entry (ACPI spec v6.3, Table 18-392) */ same here
https://review.coreboot.org/#/c/31689/3/src/arch/x86/include/arch/acpi.h@713 PS3, Line 713: /* Generic Error Status Block (ACPI spec v6.3, Table 18-391) */ and here
https://review.coreboot.org/#/c/31689/3/src/arch/x86/include/arch/acpi.h@751 PS3, Line 751: /* These types are defined in ACPI 6.3 section 9.14 */ um, please just drop it
Hello Patrick Rudolph, build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31689
to look at the new patch set (#4).
Change subject: arch/x86/acpi:: Clean up comments ......................................................................
arch/x86/acpi:: Clean up comments
As we are running ACPI v3.0, references to older than v3.0 are removed.
Change-Id: I0cce0035ed2b952d59cc1a4a9e6017dae67ef6db Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/arch/x86/include/arch/acpi.h 1 file changed, 12 insertions(+), 20 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/31689/4
Hello Patrick Rudolph, build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31689
to look at the new patch set (#5).
Change subject: arch/x86/acpi: Clean up comments ......................................................................
arch/x86/acpi: Clean up comments
As we are running ACPI v3.0, references to older than v3.0 are removed.
Change-Id: I0cce0035ed2b952d59cc1a4a9e6017dae67ef6db Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/arch/x86/include/arch/acpi.h 1 file changed, 12 insertions(+), 20 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/31689/5
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31689 )
Change subject: arch/x86/acpi: Clean up comments ......................................................................
Patch Set 5: Code-Review+2
I still don't see a reason for most of the comments. But at least there should be less urge to clean them up again :)
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31689 )
Change subject: arch/x86/acpi: Clean up comments ......................................................................
Patch Set 5:
I still don't see a reason for most of the comments. But at least there should be less urge to clean them up again :)
sure!. (Out of topic): We still stucked to version 3.0, I think it is time to upgrade to upper version. maybe V6.3 :p
(same remark for SMBIOS)
-
HAOUAS Elyes (Code Review) -
Nico Huber (Code Review)