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 <felix-coreboot@felixheld.de>
Change-Id: I0bda2349e3ae84cba50b187cc773fd8a5b17f4e2
Reviewed-on: https://review.coreboot.org/c/coreboot/+/52301
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Raul Rangel <rrangel@chromium.org>
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);
}