Change in coreboot[master]: acpi_table_header: Replace hard-code length via sizeof(acpi_fadt_t)

[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/+/35540 )

Change subject: acpi_table_header: Replace hard-code length via sizeof(acpi_fadt_t)
......................................................................

acpi_table_header: Replace hard-code length via sizeof(acpi_fadt_t)

Minimize use of hard code value for acpi_table_header->length to soft
code.
Replace length of acpi_header_t with sizeof(acpi_fadt_t).

Change-Id: Ibcae72e8f02497719fcd3f180838557e8e9abd38
Signed-off-by: Himanshu Sahdev 
---
M src/mainboard/amd/serengeti_cheetah_fam10/fadt.c
M src/southbridge/amd/sb700/fadt.c
M src/southbridge/amd/sb800/fadt.c
M src/southbridge/intel/i82371eb/fadt.c
4 files changed, 4 insertions(+), 4 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/35540/1

diff --git a/src/mainboard/amd/serengeti_cheetah_fam10/fadt.c b/src/mainboard/amd/serengeti_cheetah_fam10/fadt.c
index 845af71..9f6d48c 100644
--- a/src/mainboard/amd/serengeti_cheetah_fam10/fadt.c
+++ b/src/mainboard/amd/serengeti_cheetah_fam10/fadt.c
@@ -35,7 +35,7 @@
    /* Prepare the header */
    memset((void *)fadt,0,sizeof(acpi_fadt_t));
    memcpy(header->signature,"FACP",4);
-	header->length = 244;
+	header->length = sizeof(acpi_fadt_t);
    header->revision = get_acpi_table_revision(FADT);
    memcpy(header->oem_id,OEM_ID,6);
    memcpy(header->oem_table_id, ACPI_TABLE_CREATOR, 8);
diff --git a/src/southbridge/amd/sb700/fadt.c b/src/southbridge/amd/sb700/fadt.c
index 4a5746c..c81e644 100644
--- a/src/southbridge/amd/sb700/fadt.c
+++ b/src/southbridge/amd/sb700/fadt.c
@@ -36,7 +36,7 @@
    /* Prepare the header */
    memset((void *)fadt, 0, sizeof(acpi_fadt_t));
    memcpy(header->signature, "FACP", 4);
-	header->length = 244;
+	header->length = sizeof(acpi_fadt_t);
    header->revision = get_acpi_table_revision(FADT);
    memcpy(header->oem_id, OEM_ID, 6);
    memcpy(header->oem_table_id, ACPI_TABLE_CREATOR, 8);
diff --git a/src/southbridge/amd/sb800/fadt.c b/src/southbridge/amd/sb800/fadt.c
index 71bdf23..acda6db 100644
--- a/src/southbridge/amd/sb800/fadt.c
+++ b/src/southbridge/amd/sb800/fadt.c
@@ -36,7 +36,7 @@
    /* Prepare the header */
    memset((void *)fadt, 0, sizeof(acpi_fadt_t));
    memcpy(header->signature, "FACP", 4);
-	header->length = 244;
+	header->length = sizeof(acpi_fadt_t);
    header->revision = get_acpi_table_revision(FADT);
    memcpy(header->oem_id, OEM_ID, 6);
    memcpy(header->oem_table_id, ACPI_TABLE_CREATOR, 8);
diff --git a/src/southbridge/intel/i82371eb/fadt.c b/src/southbridge/intel/i82371eb/fadt.c
index 26a1570..9e43e62 100644
--- a/src/southbridge/intel/i82371eb/fadt.c
+++ b/src/southbridge/intel/i82371eb/fadt.c
@@ -38,7 +38,7 @@

memset((void *) fadt, 0, sizeof(acpi_fadt_t));
    memcpy(header->signature, "FACP", 4);
-	header->length = 244;
+	header->length = sizeof(acpi_fadt_t);
    header->revision = ACPI_FADT_REV_ACPI_1_0;
    memcpy(header->oem_id, OEM_ID, 6);
    memcpy(header->oem_table_id, ACPI_TABLE_CREATOR, 8);

Change in coreboot[master]: acpi_table_header: Replace hard-coded length via sizeof(acpi_fadt_t)

[Name of user not set (Code Review)]

Hello Patrick Rudolph, build bot (Jenkins),

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

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

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

Change subject: acpi_table_header: Replace hard-coded length via sizeof(acpi_fadt_t)
......................................................................

acpi_table_header: Replace hard-coded length via sizeof(acpi_fadt_t)

Minimize use of hard-coded value for acpi_table_header->length to soft
code. Replace length of acpi_header_t with sizeof(acpi_fadt_t).

Change-Id: Ibcae72e8f02497719fcd3f180838557e8e9abd38
Signed-off-by: Himanshu Sahdev 
---
M src/mainboard/amd/serengeti_cheetah_fam10/fadt.c
M src/southbridge/amd/sb700/fadt.c
M src/southbridge/amd/sb800/fadt.c
M src/southbridge/intel/i82371eb/fadt.c
4 files changed, 4 insertions(+), 4 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/35540/2

Change in coreboot[master]: acpi_table_header: Replace hard-coded length via sizeof(acpi_fadt_t)

[Paul Menzel (Code Review)]

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

Change subject: acpi_table_header: Replace hard-coded length via sizeof(acpi_fadt_t)
......................................................................

Patch Set 2: Code-Review+1

Change in coreboot[master]: acpi_table_header: Replace hard-coded length via sizeof(acpi_fadt_t)

[Name of user not set (Code Review)]

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

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

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

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

Change subject: acpi_table_header: Replace hard-coded length via sizeof(acpi_fadt_t)
......................................................................

acpi_table_header: Replace hard-coded length via sizeof(acpi_fadt_t)

Minimize use of hard-coded value for acpi_table_header->length to soft
code. Replace length of acpi_header_t with sizeof(acpi_fadt_t).

Change-Id: Ibcae72e8f02497719fcd3f180838557e8e9abd38
Signed-off-by: Himanshu Sahdev 
---
M src/mainboard/amd/serengeti_cheetah_fam10/fadt.c
M src/mainboard/amd/torpedo/fadt.c
M src/southbridge/amd/sb700/fadt.c
M src/southbridge/amd/sb800/fadt.c
M src/southbridge/intel/i82371eb/fadt.c
5 files changed, 6 insertions(+), 6 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/35540/3

Change in coreboot[master]: acpi_table_header: Replace hard-coded length via sizeof(acpi_fadt_t)

[Name of user not set (Code Review)]

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

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

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

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

Change subject: acpi_table_header: Replace hard-coded length via sizeof(acpi_fadt_t)
......................................................................

acpi_table_header: Replace hard-coded length via sizeof(acpi_fadt_t)

Minimize use of hard-coded value for acpi_table_header->length to soft
code. Replace length of acpi_header_t with sizeof(acpi_fadt_t).

Change-Id: Ibcae72e8f02497719fcd3f180838557e8e9abd38
Signed-off-by: Himanshu Sahdev 
---
M src/mainboard/amd/serengeti_cheetah_fam10/fadt.c
M src/mainboard/amd/torpedo/fadt.c
M src/southbridge/amd/sb700/fadt.c
M src/southbridge/amd/sb800/fadt.c
M src/southbridge/intel/i82371eb/fadt.c
5 files changed, 5 insertions(+), 5 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/35540/5

Change in coreboot[master]: acpi_table_header: Replace hard-coded length via sizeof(acpi_fadt_t)

[HAOUAS Elyes (Code Review)]

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

Change subject: acpi_table_header: Replace hard-coded length via sizeof(acpi_fadt_t)
......................................................................

Patch Set 5:

should I drop this https://review.coreboot.org/c/coreboot/+/33437 ?

Change in coreboot[master]: acpi_table_header: Replace hard-coded length via sizeof(acpi_fadt_t)

[HAOUAS Elyes (Code Review)]

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

Change subject: acpi_table_header: Replace hard-coded length via sizeof(acpi_fadt_t)
......................................................................

Patch Set 5:
> Patch Set 5:
>
> should I drop this https://review.coreboot.org/c/coreboot/+/33437 ?

maybe Yes
Bone!

Change in coreboot[master]: acpi_table_header: Replace hard-coded length via sizeof(acpi_fadt_t)

[HAOUAS Elyes (Code Review)]

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

Change subject: acpi_table_header: Replace hard-coded length via sizeof(acpi_fadt_t)
......................................................................

Patch Set 5:
> Patch Set 5:
> > Patch Set 5:
> >
> > should I drop this https://review.coreboot.org/c/coreboot/+/33437 ?
>
> maybe Yes
> Done!

Change in coreboot[master]: acpi_table_header: Replace hard-coded length via sizeof(acpi_fadt_t)

[HAOUAS Elyes (Code Review)]

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

Change subject: acpi_table_header: Replace hard-coded length via sizeof(acpi_fadt_t)
......................................................................

Patch Set 5:

(1 comment)

missing 
src/southbridge/nvidia/mcp55/fadt.c
src/mainboard/amd/serengeti_cheetah_fam10/fadt.c

(see https://review.coreboot.org/c/coreboot/+/33437 )

https://review.coreboot.org/c/coreboot/+/35540/5/src/mainboard/amd/torpedo/f... 
File src/mainboard/amd/torpedo/fadt.c:

https://review.coreboot.org/c/coreboot/+/35540/5/src/mainboard/amd/torpedo/f... 
PS5, Line 53: ACPI_FADT_REV_ACPI_1_0
get_acpi_table_revision(FADT); ?

Change in coreboot[master]: acpi_table_header: Replace hard-coded length via sizeof(acpi_fadt_t)

[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/+/35540 )

Change subject: acpi_table_header: Replace hard-coded length via sizeof(acpi_fadt_t)
......................................................................

Patch Set 5:

(1 comment)

Refer CB:35539 for revision.

https://review.coreboot.org/c/coreboot/+/35540/5/src/mainboard/amd/torpedo/f... 
File src/mainboard/amd/torpedo/fadt.c:

https://review.coreboot.org/c/coreboot/+/35540/5/src/mainboard/amd/torpedo/f... 
PS5, Line 53: ACPI_FADT_REV_ACPI_1_0
> get_acpi_table_revision(FADT); ?

arch/acpi.h contains multiple macros for FADT Major Revisions as per the ACPI specification. I just changed according to the previous hard-coded values in the files via the specific macros in reference. 
arch/x86/acpi.c defines the function get_acpi_table_revision which returns ACPI_FADT_REV_ACPI_3_0 for FADT. Should I modify the FADT return case value as per latest ACPI spec(6). What do you suggest?

Change in coreboot[master]: acpi_table_header: Replace hard-coded length via sizeof(acpi_fadt_t)

[Richard Spiegel (Code Review)]

Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35540 )

Change subject: acpi_table_header: Replace hard-coded length via sizeof(acpi_fadt_t)
......................................................................

Patch Set 5:

(1 comment)

https://review.coreboot.org/c/coreboot/+/35540/5/src/mainboard/amd/torpedo/f... 
File src/mainboard/amd/torpedo/fadt.c:

https://review.coreboot.org/c/coreboot/+/35540/5/src/mainboard/amd/torpedo/f... 
PS5, Line 53: ACPI_FADT_REV_ACPI_1_0
> arch/acpi.h contains multiple macros for FADT Major Revisions as per the ACPI specification. […]

ACPI 1.0 has incompatibilities with later versions. Anything that's originally 2.0 or more recent can be updated to 6, so update get_acpi_table_revision to reflect that, but only use it when original hardcoded value is not 1.0. It should not be used here, as it's originally 1.0.
Also, I do believe that updating get_acpi_table_revision should be a separate patch, does not belong to the current commit message.

Change in coreboot[master]: acpi_table_header: Replace hard-coded length via sizeof(acpi_fadt_t)

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/35540 )

Change subject: acpi_table_header: Replace hard-coded length via sizeof(acpi_fadt_t)
......................................................................

acpi_table_header: Replace hard-coded length via sizeof(acpi_fadt_t)

Minimize use of hard-coded value for acpi_table_header->length to soft
code. Replace length of acpi_header_t with sizeof(acpi_fadt_t).

Change-Id: Ibcae72e8f02497719fcd3f180838557e8e9abd38
Signed-off-by: Himanshu Sahdev 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/35540
Tested-by: build bot (Jenkins) 
Reviewed-by: Richard Spiegel 
---
M src/mainboard/amd/serengeti_cheetah_fam10/fadt.c
M src/mainboard/amd/torpedo/fadt.c
M src/southbridge/amd/sb700/fadt.c
M src/southbridge/amd/sb800/fadt.c
M src/southbridge/intel/i82371eb/fadt.c
5 files changed, 5 insertions(+), 5 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Richard Spiegel: Looks good to me, approved

diff --git a/src/mainboard/amd/serengeti_cheetah_fam10/fadt.c b/src/mainboard/amd/serengeti_cheetah_fam10/fadt.c
index 845af71..9f6d48c 100644
--- a/src/mainboard/amd/serengeti_cheetah_fam10/fadt.c
+++ b/src/mainboard/amd/serengeti_cheetah_fam10/fadt.c
@@ -35,7 +35,7 @@
    /* Prepare the header */
    memset((void *)fadt,0,sizeof(acpi_fadt_t));
    memcpy(header->signature,"FACP",4);
-	header->length = 244;
+	header->length = sizeof(acpi_fadt_t);
    header->revision = get_acpi_table_revision(FADT);
    memcpy(header->oem_id,OEM_ID,6);
    memcpy(header->oem_table_id, ACPI_TABLE_CREATOR, 8);
diff --git a/src/mainboard/amd/torpedo/fadt.c b/src/mainboard/amd/torpedo/fadt.c
index 25fb448..08763bd 100644
--- a/src/mainboard/amd/torpedo/fadt.c
+++ b/src/mainboard/amd/torpedo/fadt.c
@@ -49,7 +49,7 @@
    /* Prepare the header */
    memset((void *)fadt, 0, sizeof(acpi_fadt_t));
    memcpy(header->signature, "FACP", 4);
-	header->length = 244;
+	header->length = sizeof(acpi_fadt_t);
    header->revision = ACPI_FADT_REV_ACPI_1_0;
    memcpy(header->oem_id, OEM_ID, 6);
    memcpy(header->oem_table_id, ACPI_TABLE_CREATOR, 8);
diff --git a/src/southbridge/amd/sb700/fadt.c b/src/southbridge/amd/sb700/fadt.c
index 4a5746c..c81e644 100644
--- a/src/southbridge/amd/sb700/fadt.c
+++ b/src/southbridge/amd/sb700/fadt.c
@@ -36,7 +36,7 @@
    /* Prepare the header */
    memset((void *)fadt, 0, sizeof(acpi_fadt_t));
    memcpy(header->signature, "FACP", 4);
-	header->length = 244;
+	header->length = sizeof(acpi_fadt_t);
    header->revision = get_acpi_table_revision(FADT);
    memcpy(header->oem_id, OEM_ID, 6);
    memcpy(header->oem_table_id, ACPI_TABLE_CREATOR, 8);
diff --git a/src/southbridge/amd/sb800/fadt.c b/src/southbridge/amd/sb800/fadt.c
index 71bdf23..acda6db 100644
--- a/src/southbridge/amd/sb800/fadt.c
+++ b/src/southbridge/amd/sb800/fadt.c
@@ -36,7 +36,7 @@
    /* Prepare the header */
    memset((void *)fadt, 0, sizeof(acpi_fadt_t));
    memcpy(header->signature, "FACP", 4);
-	header->length = 244;
+	header->length = sizeof(acpi_fadt_t);
    header->revision = get_acpi_table_revision(FADT);
    memcpy(header->oem_id, OEM_ID, 6);
    memcpy(header->oem_table_id, ACPI_TABLE_CREATOR, 8);
diff --git a/src/southbridge/intel/i82371eb/fadt.c b/src/southbridge/intel/i82371eb/fadt.c
index 26a1570..9e43e62 100644
--- a/src/southbridge/intel/i82371eb/fadt.c
+++ b/src/southbridge/intel/i82371eb/fadt.c
@@ -38,7 +38,7 @@

memset((void *) fadt, 0, sizeof(acpi_fadt_t));
    memcpy(header->signature, "FACP", 4);
-	header->length = 244;
+	header->length = sizeof(acpi_fadt_t);
    header->revision = ACPI_FADT_REV_ACPI_1_0;
    memcpy(header->oem_id, OEM_ID, 6);
    memcpy(header->oem_table_id, ACPI_TABLE_CREATOR, 8);