Change in coreboot[master]: soc/amd/picasso/acpi: clean up global NVS

[Felix Held (Code Review)]

Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44154 )

Change subject: soc/amd/picasso/acpi: clean up global NVS
......................................................................

soc/amd/picasso/acpi: clean up global NVS

Some fields in GNVS seem to be copied over from Apollolake to
Stoneyridge to Picasso. This patch removes the unused fields.

BUG=b:161165393
TEST=Mandolin still boots and dmesg shows no new ACPI errors.

Change-Id: I8c6b580543089bf0180a7caeb9e6a47dc4ed4a1d
Signed-off-by: Felix Held 
---
M src/soc/amd/picasso/acpi/globalnvs.asl
M src/soc/amd/picasso/include/soc/nvs.h
2 files changed, 20 insertions(+), 40 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/54/44154/1

diff --git a/src/soc/amd/picasso/acpi/globalnvs.asl b/src/soc/amd/picasso/acpi/globalnvs.asl
index 8147619..6e63b8d 100644
--- a/src/soc/amd/picasso/acpi/globalnvs.asl
+++ b/src/soc/amd/picasso/acpi/globalnvs.asl
@@ -17,25 +17,15 @@
    /* Miscellaneous */
    Offset (0x00),
    PCNT,	8,	// 0x00 - Processor Count
-	PPCM,	8,	// 0x01 - Max PPC State
-	LIDS,	8,	// 0x02 - LID State
-	PWRS,	8,	// 0x03 - AC Power State
-	DPTE,	8,	// 0x04 - Enable DPTF
-	CBMC,	32,	// 0x05 - 0x08 - coreboot Memory Console
-	PM1I,	64,	// 0x09 - 0x10 - System Wake Source - PM1 Index
-	GPEI,	64,	// 0x11 - 0x18 - GPE Wake Source
-	NHLA,	64,	// 0x19 - 0x20 - NHLT Address
-	NHLL,	32,	// 0x21 - 0x24 - NHLT Length
-	PRT0,	32,	// 0x25 - 0x28 - PERST_0 Address
-	SCDP,	8,	// 0x29 - SD_CD GPIO portid
-	SCDO,	8,	// 0x2A - GPIO pad offset relative to the community
-	TMPS,	8,	// 0x2B - Temperature Sensor ID
-	TLVL,	8,	// 0x2C - Throttle Level Limit
-	FLVL,	8,	// 0x2D - Current FAN Level
-	TCRT,	8,	// 0x2E - Critical Threshold
-	TPSV,	8,	// 0x2F - Passive Threshold
-	TMAX,	8,	// 0x30 - CPU Tj_max
-	Offset (0x34),  // 0x34 - AOAC Device Enables
+	LIDS,	8,	// 0x01 - LID State
+	PWRS,	8,	// 0x02 - AC Power State
+	CBMC,	32,	// 0x03 - 0x06 - coreboot Memory Console
+	PM1I,	64,	// 0x07 - 0x0e - System Wake Source - PM1 Index
+	GPEI,	64,	// 0x0f - 0x16 - GPE Wake Source
+	TMPS,	8,	// 0x17 - Temperature Sensor ID
+	TCRT,	8,	// 0x18 - Critical Threshold
+	TPSV,	8,	// 0x19 - Passive Threshold
+	Offset (0x20),  // 0x20 - AOAC Device Enables
    , 7,
    IC2E,	1,	//        I2C2, 7
    IC3E,	1,	//        I2C3, 8
diff --git a/src/soc/amd/picasso/include/soc/nvs.h b/src/soc/amd/picasso/include/soc/nvs.h
index 214ab1d..b6708c8 100644
--- a/src/soc/amd/picasso/include/soc/nvs.h
+++ b/src/soc/amd/picasso/include/soc/nvs.h
@@ -17,27 +17,17 @@
 struct __packed global_nvs {
    /* Miscellaneous */
    uint8_t		pcnt; /* 0x00 - Processor Count */
-	uint8_t		ppcm; /* 0x01 - Max PPC State */
-	uint8_t		lids; /* 0x02 - LID State */
-	uint8_t		pwrs; /* 0x03 - AC Power State */
-	uint8_t		dpte; /* 0x04 - Enable DPTF */
-	uint32_t	cbmc; /* 0x05 - 0x08 - coreboot Memory Console */
-	uint64_t	pm1i; /* 0x09 - 0x10 - System Wake Source - PM1 Index */
-	uint64_t	gpei; /* 0x11 - 0x18 - GPE Wake Source */
-	uint64_t	nhla; /* 0x19 - 0x20 - NHLT Address */
-	uint32_t	nhll; /* 0x21 - 0x24 - NHLT Length */
-	uint32_t	prt0; /* 0x25 - 0x28 - PERST_0 Address */
-	uint8_t		scdp; /* 0x29 - SD_CD GPIO portid */
-	uint8_t		scdo; /* 0x2A - GPIO pad relative offset */
-	uint8_t		tmps; /* 0x2B - Temperature Sensor ID */
-	uint8_t		tlvl; /* 0x2C - Throttle Level Limit */
-	uint8_t		flvl; /* 0x2D - Current FAN Level */
-	uint8_t		tcrt; /* 0x2E - Critical Threshold */
-	uint8_t		tpsv; /* 0x2F - Passive Threshold */
-	uint8_t		tmax; /* 0x30 - CPU Tj_max */
-	uint8_t		pad1[3];
-	aoac_devs_t	aoac; /* 0x34 - AOAC device enables */
-	uint8_t		unused[200];
+	uint8_t		lids; /* 0x01 - LID State */
+	uint8_t		pwrs; /* 0x02 - AC Power State */
+	uint32_t	cbmc; /* 0x03 - 0x06 - coreboot Memory Console */
+	uint64_t	pm1i; /* 0x07 - 0x0e - System Wake Source - PM1 Index */
+	uint64_t	gpei; /* 0x0f - 0x16 - GPE Wake Source */
+	uint8_t		tmps; /* 0x17 - Temperature Sensor ID */
+	uint8_t		tcrt; /* 0x18 - Critical Threshold */
+	uint8_t		tpsv; /* 0x19 - Passive Threshold */
+	uint8_t		pad1[6];
+	aoac_devs_t	aoac; /* 0x20 - AOAC device enables */
+	uint8_t		unused[220];

/* ChromeOS specific (0x100 - 0xfff) */
    chromeos_acpi_t chromeos;

[Felix Held (Code Review)]

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44154 )

Change subject: soc/amd/picasso/acpi: clean up global NVS
......................................................................

Patch Set 1:

would be good to have this tested on a zork device before it gets merged

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44154 )

Change subject: soc/amd/picasso/acpi: clean up global NVS
......................................................................

Patch Set 1: Code-Review+2

[Aaron Durbin (Code Review)]

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44154 )

Change subject: soc/amd/picasso/acpi: clean up global NVS
......................................................................

Patch Set 1: Code-Review+2

[Raul Rangel (Code Review)]

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

Change subject: soc/amd/picasso/acpi: clean up global NVS
......................................................................

Patch Set 1: Code-Review+2

(1 comment)

Can suspend stress test for 12 iterations on Ezkinil.

https://review.coreboot.org/c/coreboot/+/44154/1/src/soc/amd/picasso/include... 
File src/soc/amd/picasso/include/soc/nvs.h:

https://review.coreboot.org/c/coreboot/+/44154/1/src/soc/amd/picasso/include... 
PS1, Line 29: aoac
Looks like we just set this and never read from it again. Now that we have power resources, I think we could remove these. Can clean that up in a followup though.

[Felix Held (Code Review)]

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44154 )

Change subject: soc/amd/picasso/acpi: clean up global NVS
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/44154/1/src/soc/amd/picasso/include... 
File src/soc/amd/picasso/include/soc/nvs.h:

https://review.coreboot.org/c/coreboot/+/44154/1/src/soc/amd/picasso/include... 
PS1, Line 29: aoac
> Looks like we just set this and never read from it again. […]

might be worth looking into this later; not sure how much will change or be added in that area. only removed the obviously wrong or unused struct fields in this patch

[Felix Held (Code Review)]

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

Change subject: soc/amd/picasso/acpi: clean up global NVS
......................................................................

soc/amd/picasso/acpi: clean up global NVS

Some fields in GNVS seem to be copied over from Apollolake to
Stoneyridge to Picasso. This patch removes the unused fields.

BUG=b:161165393
TEST=Mandolin still boots and dmesg shows no new ACPI errors.

Change-Id: I8c6b580543089bf0180a7caeb9e6a47dc4ed4a1d
Signed-off-by: Felix Held 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/44154
Tested-by: build bot (Jenkins) 
Reviewed-by: Angel Pons 
Reviewed-by: Aaron Durbin 
Reviewed-by: Raul Rangel 
---
M src/soc/amd/picasso/acpi/globalnvs.asl
M src/soc/amd/picasso/include/soc/nvs.h
2 files changed, 20 insertions(+), 40 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Aaron Durbin: Looks good to me, approved
  Raul Rangel: Looks good to me, approved
  Angel Pons: Looks good to me, approved

diff --git a/src/soc/amd/picasso/acpi/globalnvs.asl b/src/soc/amd/picasso/acpi/globalnvs.asl
index 8147619..6e63b8d 100644
--- a/src/soc/amd/picasso/acpi/globalnvs.asl
+++ b/src/soc/amd/picasso/acpi/globalnvs.asl
@@ -17,25 +17,15 @@
    /* Miscellaneous */
    Offset (0x00),
    PCNT,	8,	// 0x00 - Processor Count
-	PPCM,	8,	// 0x01 - Max PPC State
-	LIDS,	8,	// 0x02 - LID State
-	PWRS,	8,	// 0x03 - AC Power State
-	DPTE,	8,	// 0x04 - Enable DPTF
-	CBMC,	32,	// 0x05 - 0x08 - coreboot Memory Console
-	PM1I,	64,	// 0x09 - 0x10 - System Wake Source - PM1 Index
-	GPEI,	64,	// 0x11 - 0x18 - GPE Wake Source
-	NHLA,	64,	// 0x19 - 0x20 - NHLT Address
-	NHLL,	32,	// 0x21 - 0x24 - NHLT Length
-	PRT0,	32,	// 0x25 - 0x28 - PERST_0 Address
-	SCDP,	8,	// 0x29 - SD_CD GPIO portid
-	SCDO,	8,	// 0x2A - GPIO pad offset relative to the community
-	TMPS,	8,	// 0x2B - Temperature Sensor ID
-	TLVL,	8,	// 0x2C - Throttle Level Limit
-	FLVL,	8,	// 0x2D - Current FAN Level
-	TCRT,	8,	// 0x2E - Critical Threshold
-	TPSV,	8,	// 0x2F - Passive Threshold
-	TMAX,	8,	// 0x30 - CPU Tj_max
-	Offset (0x34),  // 0x34 - AOAC Device Enables
+	LIDS,	8,	// 0x01 - LID State
+	PWRS,	8,	// 0x02 - AC Power State
+	CBMC,	32,	// 0x03 - 0x06 - coreboot Memory Console
+	PM1I,	64,	// 0x07 - 0x0e - System Wake Source - PM1 Index
+	GPEI,	64,	// 0x0f - 0x16 - GPE Wake Source
+	TMPS,	8,	// 0x17 - Temperature Sensor ID
+	TCRT,	8,	// 0x18 - Critical Threshold
+	TPSV,	8,	// 0x19 - Passive Threshold
+	Offset (0x20),  // 0x20 - AOAC Device Enables
    , 7,
    IC2E,	1,	//        I2C2, 7
    IC3E,	1,	//        I2C3, 8
diff --git a/src/soc/amd/picasso/include/soc/nvs.h b/src/soc/amd/picasso/include/soc/nvs.h
index 214ab1d..b6708c8 100644
--- a/src/soc/amd/picasso/include/soc/nvs.h
+++ b/src/soc/amd/picasso/include/soc/nvs.h
@@ -17,27 +17,17 @@
 struct __packed global_nvs {
    /* Miscellaneous */
    uint8_t		pcnt; /* 0x00 - Processor Count */
-	uint8_t		ppcm; /* 0x01 - Max PPC State */
-	uint8_t		lids; /* 0x02 - LID State */
-	uint8_t		pwrs; /* 0x03 - AC Power State */
-	uint8_t		dpte; /* 0x04 - Enable DPTF */
-	uint32_t	cbmc; /* 0x05 - 0x08 - coreboot Memory Console */
-	uint64_t	pm1i; /* 0x09 - 0x10 - System Wake Source - PM1 Index */
-	uint64_t	gpei; /* 0x11 - 0x18 - GPE Wake Source */
-	uint64_t	nhla; /* 0x19 - 0x20 - NHLT Address */
-	uint32_t	nhll; /* 0x21 - 0x24 - NHLT Length */
-	uint32_t	prt0; /* 0x25 - 0x28 - PERST_0 Address */
-	uint8_t		scdp; /* 0x29 - SD_CD GPIO portid */
-	uint8_t		scdo; /* 0x2A - GPIO pad relative offset */
-	uint8_t		tmps; /* 0x2B - Temperature Sensor ID */
-	uint8_t		tlvl; /* 0x2C - Throttle Level Limit */
-	uint8_t		flvl; /* 0x2D - Current FAN Level */
-	uint8_t		tcrt; /* 0x2E - Critical Threshold */
-	uint8_t		tpsv; /* 0x2F - Passive Threshold */
-	uint8_t		tmax; /* 0x30 - CPU Tj_max */
-	uint8_t		pad1[3];
-	aoac_devs_t	aoac; /* 0x34 - AOAC device enables */
-	uint8_t		unused[200];
+	uint8_t		lids; /* 0x01 - LID State */
+	uint8_t		pwrs; /* 0x02 - AC Power State */
+	uint32_t	cbmc; /* 0x03 - 0x06 - coreboot Memory Console */
+	uint64_t	pm1i; /* 0x07 - 0x0e - System Wake Source - PM1 Index */
+	uint64_t	gpei; /* 0x0f - 0x16 - GPE Wake Source */
+	uint8_t		tmps; /* 0x17 - Temperature Sensor ID */
+	uint8_t		tcrt; /* 0x18 - Critical Threshold */
+	uint8_t		tpsv; /* 0x19 - Passive Threshold */
+	uint8_t		pad1[6];
+	aoac_devs_t	aoac; /* 0x20 - AOAC device enables */
+	uint8_t		unused[220];

/* ChromeOS specific (0x100 - 0xfff) */
    chromeos_acpi_t chromeos;