[coreboot-gerrit] Change in coreboot[master]: soc/amd/common/block/pm: rework pm_set_power_failure_state

[Felix Held (Code Review)]

Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson.
Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/52301 )

Change subject: soc/amd/common/block/pm: rework pm_set_power_failure_state
......................................................................

soc/amd/common/block/pm: rework pm_set_power_failure_state

Picasso and Stoneyridge didn't do a read-modify-write operation on the
lower nibble of PM_RTC_SHADOW_REG, but just wrote the upper nibble as
all zeros. Since the upper nibble might be uninitialized before the
lower nibble gets written, do what Picasso and Stoneyridge did here
instead of what the reference code does. Also add a comment why and how
this register behaves a bit weird.

Signed-off-by: Felix Held 
Change-Id: I0bda2349e3ae84cba50b187cc773fd8a5b17f4e2
---
M src/soc/amd/common/block/pm/pmlib.c
1 file changed, 5 insertions(+), 7 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/52301/1

diff --git a/src/soc/amd/common/block/pm/pmlib.c b/src/soc/amd/common/block/pm/pmlib.c
index 9cb9d52..f1b0b27 100644
--- a/src/soc/amd/common/block/pm/pmlib.c
+++ b/src/soc/amd/common/block/pm/pmlib.c
@@ -5,17 +5,17 @@
 #include <console/console.h>
 #include <types.h>

+/* This register is a bit of an odd one. The configuration gets written into the lower nibble,
+   but ends up being copied to the upper nibble which gets initialized by this. */
 #define PM_RTC_SHADOW_REG	0x5b
-/* Init bit to be set by BIOS while configuring the PWR_FAIL_* shadow bits. */
-#define   PWR_FAIL_INIT		BIT(2)
-#define   PWR_FAIL_MASK		(BIT(0) | BIT(1) | BIT(2) | BIT(3))
+#define   PWR_PWRSTATE		BIT(2) /* power state bit; needs to be written as 1 */
 #define   PWR_FAIL_OFF		0x0 /* Always power off after power resumes */
 #define   PWR_FAIL_ON		0x1 /* Always power on after power resumes */
 #define   PWR_FAIL_PREV		0x3 /* Use previous setting after power resumes */

void pm_set_power_failure_state(void)
 {
-	uint8_t val, pwr_fail = PWR_FAIL_INIT;
+	uint8_t pwr_fail = PWR_PWRSTATE;

switch (CONFIG_MAINBOARD_POWER_FAILURE_STATE) {
    case MAINBOARD_POWER_STATE_OFF:
@@ -37,7 +37,5 @@
    	break;
    }

-	val = pm_io_read8(PM_RTC_SHADOW_REG) & ~PWR_FAIL_MASK;
-	val |= pwr_fail;
-	pm_io_write8(PM_RTC_SHADOW_REG, val);
+	pm_io_write8(PM_RTC_SHADOW_REG, pwr_fail);
 }