Change in coreboot[master]: /src/acpi: Correct sizes for ACPI data fields

[Jason Glenesk (Code Review)]

Hello Jason Glenesk,

I'd like you to do a code review. Please visit

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

to review the following change.

Change subject: /src/acpi: Correct sizes for ACPI data fields
......................................................................

/src/acpi: Correct sizes for ACPI data fields

Correct sizes of Count, Type, and Latency data field in _CST object to
integer, byte, word, respectively. Correct size of NumEntries data field
in _CSD object to integer.

BUG=b:155307433
TEST=Boot Morphius and dump SSDT _CST and _CSD objects. Confirm that
sizes written conform to ACPI_6_3_May16.pdf ACPI specification.
BRANCH=Zork

Change-Id: I356b46f2fa787e18442a66280b6545a3b525a08b
Signed-off-by: Jason Glenesk 
---
M src/acpi/acpigen.c
1 file changed, 4 insertions(+), 4 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/45339/1

diff --git a/src/acpi/acpigen.c b/src/acpi/acpigen.c
index 44f9106..7910cbc 100644
--- a/src/acpi/acpigen.c
+++ b/src/acpi/acpigen.c
@@ -850,8 +850,8 @@
 {
    acpigen_write_package(4);
    acpigen_write_register_resource(&cstate->resource);
-	acpigen_write_dword(cstate->ctype);
-	acpigen_write_dword(cstate->latency);
+	acpigen_write_byte(cstate->ctype);
+	acpigen_write_word(cstate->latency);
    acpigen_write_dword(cstate->power);
    acpigen_pop_len();
 }
@@ -861,7 +861,7 @@
    int i;
    acpigen_write_name("_CST");
    acpigen_write_package(nentries+1);
-	acpigen_write_dword(nentries);
+	acpigen_write_integer(nentries);

for (i = 0; i < nentries; i++)
    	acpigen_write_CST_package_entry(cstate + i);
@@ -875,7 +875,7 @@
    acpigen_write_name("_CSD");
    acpigen_write_package(1);
    acpigen_write_package(6);
-	acpigen_write_byte(6);	// 6 values
+	acpigen_write_integer(6);	// 6 values
    acpigen_write_byte(0);	// revision 0
    acpigen_write_dword(domain);
    acpigen_write_dword(coordtype);

[Jason Glenesk (Code Review)]

Jason Glenesk has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45339 )

Change subject: /src/acpi: Correct sizes for ACPI data fields
......................................................................

Patch Set 1:

Please help to review this change.

[Raul Rangel (Code Review)]

Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45339 )

Change subject: /src/acpi: Correct sizes for ACPI data fields
......................................................................

Patch Set 1: Code-Review+2

[Tim Wawrzynczak (Code Review)]

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45339 )

Change subject: /src/acpi: Correct sizes for ACPI data fields
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/45339/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/45339/1//COMMIT_MSG@7 
PS1, Line 7: /src/acpi
acpi:

[Jason Glenesk (Code Review)]

Jason Glenesk has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45339 )

Change subject: /src/acpi: Correct sizes for ACPI data fields
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/45339/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/45339/1//COMMIT_MSG@7 
PS1, Line 7: /src/acpi
> acpi:

Ack

Change in coreboot[master]: acpi: Correct sizes for ACPI data fields

[Jason Glenesk (Code Review)]

Jason Glenesk has uploaded a new patch set (#2) to the change originally created by Jason Glenesk. ( https://review.coreboot.org/c/coreboot/+/45339 )

Change subject: acpi: Correct sizes for ACPI data fields
......................................................................

acpi: Correct sizes for ACPI data fields

Correct sizes of Count, Type, and Latency data field in _CST object to
integer, byte, word, respectively. Correct size of NumEntries data field
in _CSD object to integer.

BUG=b:155307433
TEST=Boot Morphius and dump SSDT _CST and _CSD objects. Confirm that
sizes written conform to ACPI_6_3_May16.pdf ACPI specification.
BRANCH=Zork

Change-Id: I356b46f2fa787e18442a66280b6545a3b525a08b
Signed-off-by: Jason Glenesk 
---
M src/acpi/acpigen.c
1 file changed, 4 insertions(+), 4 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/45339/2

Change in coreboot[master]: acpi: Correct sizes for ACPI data fields

[Marshall Dawson (Code Review)]

Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45339 )

Change subject: acpi: Correct sizes for ACPI data fields
......................................................................

Patch Set 2: Code-Review+2

Change in coreboot[master]: acpi: Correct sizes for ACPI data fields

[Jason Glenesk (Code Review)]

Hello build bot (Jenkins), Raul Rangel, Furquan Shaikh, Marshall Dawson, Jason Glenesk, Aaron Durbin, Felix Held,

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

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

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

Change subject: acpi: Correct sizes for ACPI data fields
......................................................................

acpi: Correct sizes for ACPI data fields

Correct sizes of Count, Type, and Latency data field in _CST object to
integer, byte, word, respectively. Correct size of NumEntries data field
in _CSD object to integer.

BUG=b:155307433
TEST=Boot Morphius and dump SSDT _CST and _CSD objects. Confirm that
sizes written conform to ACPI_6_3_May16.pdf ACPI specification.
BRANCH=Zork

Change-Id: I356b46f2fa787e18442a66280b6545a3b525a08b
Signed-off-by: Jason Glenesk 
---
M src/acpi/acpigen.c
1 file changed, 4 insertions(+), 4 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/45339/3

Change in coreboot[master]: acpi: Correct sizes for ACPI data fields

[Tim Wawrzynczak (Code Review)]

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45339 )

Change subject: acpi: Correct sizes for ACPI data fields
......................................................................

Patch Set 3:

Good find, thanks!

Change in coreboot[master]: acpi: Correct sizes for ACPI data fields

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45339 )

Change subject: acpi: Correct sizes for ACPI data fields
......................................................................

Patch Set 3: Code-Review+2

Change in coreboot[master]: acpi: Correct sizes for ACPI data fields

[Felix Held (Code Review)]

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/45339 )

Change subject: acpi: Correct sizes for ACPI data fields
......................................................................

acpi: Correct sizes for ACPI data fields

Correct sizes of Count, Type, and Latency data field in _CST object to
integer, byte, word, respectively. Correct size of NumEntries data field
in _CSD object to integer.

BUG=b:155307433
TEST=Boot Morphius and dump SSDT _CST and _CSD objects. Confirm that
sizes written conform to ACPI_6_3_May16.pdf ACPI specification.
BRANCH=Zork

Change-Id: I356b46f2fa787e18442a66280b6545a3b525a08b
Signed-off-by: Jason Glenesk 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/45339
Reviewed-by: Furquan Shaikh 
Reviewed-by: Marshall Dawson 
Reviewed-by: Raul Rangel 
Tested-by: build bot (Jenkins) 
---
M src/acpi/acpigen.c
1 file changed, 4 insertions(+), 4 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Furquan Shaikh: Looks good to me, approved
  Marshall Dawson: Looks good to me, approved
  Raul Rangel: Looks good to me, approved

diff --git a/src/acpi/acpigen.c b/src/acpi/acpigen.c
index 44f9106..7910cbc 100644
--- a/src/acpi/acpigen.c
+++ b/src/acpi/acpigen.c
@@ -850,8 +850,8 @@
 {
    acpigen_write_package(4);
    acpigen_write_register_resource(&cstate->resource);
-	acpigen_write_dword(cstate->ctype);
-	acpigen_write_dword(cstate->latency);
+	acpigen_write_byte(cstate->ctype);
+	acpigen_write_word(cstate->latency);
    acpigen_write_dword(cstate->power);
    acpigen_pop_len();
 }
@@ -861,7 +861,7 @@
    int i;
    acpigen_write_name("_CST");
    acpigen_write_package(nentries+1);
-	acpigen_write_dword(nentries);
+	acpigen_write_integer(nentries);

for (i = 0; i < nentries; i++)
    	acpigen_write_CST_package_entry(cstate + i);
@@ -875,7 +875,7 @@
    acpigen_write_name("_CSD");
    acpigen_write_package(1);
    acpigen_write_package(6);
-	acpigen_write_byte(6);	// 6 values
+	acpigen_write_integer(6);	// 6 values
    acpigen_write_byte(0);	// revision 0
    acpigen_write_dword(domain);
    acpigen_write_dword(coordtype);