system: fix system_get_scratchpad API

The current API for system_get_scratchpad mixes the status and the value
being read. Update the signature to allow both.

BRANCH=none
BUG=b:195481980
TEST=make testall && zmake testall

Signed-off-by: Yuval Peress <peress@chromium.org>
Change-Id: I3a5f5ad523d507c53a5d474806f58afafb82e70c
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/3074828
Commit-Queue: Denis Brockus <dbrockus@chromium.org>
Reviewed-by: Denis Brockus <dbrockus@chromium.org>

17 files changed, 86 insertions, 58 deletions

diff --git a/chip/host/system.c b/chip/host/system.c
index b685d86a7d..60d765deab 100644
--- a/chip/host/system.c
+++ b/chip/host/system.c
@@ -225,22 +225,21 @@ int system_set_scratchpad(uint32_t value)
 	return EC_SUCCESS;
 }
 
-uint32_t system_get_scratchpad(void)
+int system_get_scratchpad(uint32_t *value)
 {
 	FILE *f = get_persistent_storage("scratchpad", "r");
-	uint32_t value;
 	int success;
 
 	if (f == NULL)
-		return 0;
+		return EC_ERROR_UNKNOWN;
 
-	success = fscanf(f, "%u", &value);
+	success = fscanf(f, "%u", value);
 	release_persistent_storage(f);
 
 	if (success)
-		return value;
+		return EC_SUCCESS;
 	else
-		return 0;
+		return EC_ERROR_UNKNOWN;
 }
 
 static void __jump_resetvec(void)
diff --git a/chip/ish/system.c b/chip/ish/system.c
index 1e6fe87800..30a2576e5e 100644
--- a/chip/ish/system.c
+++ b/chip/ish/system.c
@@ -147,9 +147,9 @@ int system_set_scratchpad(uint32_t value)
 	return EC_SUCCESS;
 }
 
-uint32_t system_get_scratchpad(void)
+int system_get_scratchpad(uint32_t *unused)
 {
-	return 0;
+	return EC_ERROR_UNIMPLEMENTED;
 }
 
 void system_hibernate(uint32_t seconds, uint32_t microseconds)
diff --git a/chip/it83xx/system.c b/chip/it83xx/system.c
index a85cbe7c0f..16871e5826 100644
--- a/chip/it83xx/system.c
+++ b/chip/it83xx/system.c
@@ -373,16 +373,11 @@ int system_set_scratchpad(uint32_t value)
 	return EC_SUCCESS;
 }
 
-uint32_t system_get_scratchpad(void)
+int system_get_scratchpad(uint32_t *value)
 {
-	uint32_t value = 0;
-
-	value |= BRAM_SCRATCHPAD3 << 24;
-	value |= BRAM_SCRATCHPAD2 << 16;
-	value |= BRAM_SCRATCHPAD1 << 8;
-	value |= BRAM_SCRATCHPAD0;
-
-	return value;
+	*value = (BRAM_SCRATCHPAD3 << 24) | (BRAM_SCRATCHPAD2 << 16) |
+		 (BRAM_SCRATCHPAD1 << 8) | (BRAM_SCRATCHPAD0);
+	return EC_SUCCESS;
 }
 
 static uint32_t system_get_chip_id(void)
diff --git a/chip/lm4/system.c b/chip/lm4/system.c
index bdb15d3861..56bd1a82fd 100644
--- a/chip/lm4/system.c
+++ b/chip/lm4/system.c
@@ -579,9 +579,10 @@ int system_set_scratchpad(uint32_t value)
 	return hibdata_write(HIBDATA_INDEX_SCRATCHPAD, value);
 }
 
-uint32_t system_get_scratchpad(void)
+int system_get_scratchpad(uint32_t *value)
 {
-	return hibdata_read(HIBDATA_INDEX_SCRATCHPAD);
+	*value = hibdata_read(HIBDATA_INDEX_SCRATCHPAD);
+	return EC_SUCCESS;
 }
 
 const char *system_get_chip_vendor(void)
diff --git a/chip/mchp/system.c b/chip/mchp/system.c
index c17383bcad..d67314d716 100644
--- a/chip/mchp/system.c
+++ b/chip/mchp/system.c
@@ -413,9 +413,10 @@ int system_set_scratchpad(uint32_t value)
 	return EC_SUCCESS;
 }
 
-uint32_t system_get_scratchpad(void)
+int system_get_scratchpad(uint32_t *value)
 {
-	return MCHP_VBAT_RAM(HIBDATA_INDEX_SCRATCHPAD);
+	*value = MCHP_VBAT_RAM(HIBDATA_INDEX_SCRATCHPAD);
+	return EC_SUCCESS;
 }
 
 /*
diff --git a/chip/mec1322/system.c b/chip/mec1322/system.c
index bfa4a3c031..6e482d3a78 100644
--- a/chip/mec1322/system.c
+++ b/chip/mec1322/system.c
@@ -218,9 +218,10 @@ int system_set_scratchpad(uint32_t value)
 	return EC_SUCCESS;
 }
 
-uint32_t system_get_scratchpad(void)
+int system_get_scratchpad(uint32_t *value)
 {
-	return MEC1322_VBAT_RAM(HIBDATA_INDEX_SCRATCHPAD);
+	*value = MEC1322_VBAT_RAM(HIBDATA_INDEX_SCRATCHPAD);
+	return EC_SUCCESS;
 }
 
 void system_hibernate(uint32_t seconds, uint32_t microseconds)
diff --git a/chip/mt_scp/mt8183/system.c b/chip/mt_scp/mt8183/system.c
index dc822e47d5..03f39298c9 100644
--- a/chip/mt_scp/mt8183/system.c
+++ b/chip/mt_scp/mt8183/system.c
@@ -35,9 +35,10 @@ int system_set_scratchpad(uint32_t value)
 	return EC_SUCCESS;
 }
 
-uint32_t system_get_scratchpad(void)
+int system_get_scratchpad(uint32_t *value)
 {
-	return SCP_GPR[0] & 0xffff;
+	*value = SCP_GPR[0] & 0xffff;
+	return EC_SUCCESS;
 }
 
 const char *system_get_chip_vendor(void)
diff --git a/chip/npcx/system.c b/chip/npcx/system.c
index 48727c2686..ac7056330f 100644
--- a/chip/npcx/system.c
+++ b/chip/npcx/system.c
@@ -1141,9 +1141,10 @@ int system_set_scratchpad(uint32_t value)
 	return bbram_data_write(BBRM_DATA_INDEX_SCRATCHPAD, value);
 }
 
-uint32_t system_get_scratchpad(void)
+int system_get_scratchpad(uint32_t *value)
 {
-	return bbram_data_read(BBRM_DATA_INDEX_SCRATCHPAD);
+	*value = bbram_data_read(BBRM_DATA_INDEX_SCRATCHPAD);
+	return EC_SUCCESS;
 }
 
 int system_is_reboot_warm(void)
diff --git a/chip/stm32/system.c b/chip/stm32/system.c
index 060b4fa227..66158991d2 100644
--- a/chip/stm32/system.c
+++ b/chip/stm32/system.c
@@ -526,9 +526,10 @@ int system_set_scratchpad(uint32_t value)
 	return bkpdata_write(BKPDATA_INDEX_SCRATCHPAD, (uint16_t)value);
 }
 
-uint32_t system_get_scratchpad(void)
+int system_get_scratchpad(uint32_t *value)
 {
-	return (uint32_t)bkpdata_read(BKPDATA_INDEX_SCRATCHPAD);
+	*value = (uint32_t)bkpdata_read(BKPDATA_INDEX_SCRATCHPAD);
+	return EC_SUCCESS;
 }
 
 const char *system_get_chip_vendor(void)
diff --git a/common/system.c b/common/system.c
index 236b886f1d..6c1d0277e2 100644
--- a/common/system.c
+++ b/common/system.c
@@ -1174,6 +1174,7 @@ DECLARE_HOST_COMMAND(EC_CMD_SYSINFO, host_command_sysinfo,
 static int command_scratchpad(int argc, char **argv)
 {
 	int rv = EC_SUCCESS;
+	uint32_t scratchpad_value;
 
 	if (argc == 2) {
 		char *e;
@@ -1181,9 +1182,19 @@ static int command_scratchpad(int argc, char **argv)
 		if (*e)
 			return EC_ERROR_PARAM1;
 		rv = system_set_scratchpad(s);
+
+		if (rv) {
+			ccprintf("Error setting scratchpad register (%d)\b",
+				 rv);
+			return rv;
+		}
 	}
 
-	ccprintf("Scratchpad: 0x%08x\n", system_get_scratchpad());
+	rv = system_get_scratchpad(&scratchpad_value);
+	if (rv)
+		ccprintf("Error reading scratchpad register (%d)\n", rv);
+	else
+		ccprintf("Scratchpad: 0x%08x\n", scratchpad_value);
 	return rv;
 }
 DECLARE_CONSOLE_COMMAND(scratchpad, command_scratchpad,
diff --git a/common/test_util.c b/common/test_util.c
index 8b3cbaad03..b23f85509b 100644
--- a/common/test_util.c
+++ b/common/test_util.c
@@ -102,7 +102,10 @@ int test_get_error_count(void)
 
 uint32_t test_get_state(void)
 {
-	return system_get_scratchpad();
+	uint32_t state;
+
+	system_get_scratchpad(&state);
+	return state;
 }
 
 test_mockable void test_clean_up(void)
diff --git a/include/system.h b/include/system.h
index 559e97befe..1b1e45c2a1 100644
--- a/include/system.h
+++ b/include/system.h
@@ -361,9 +361,15 @@ void system_reset(int flags);
 int system_set_scratchpad(uint32_t value);
 
 /**
- * Return the current scratchpad register value.
+ * Get the scratchpad register value.
+ *
+ * The scratchpad register maintains its contents across a
+ * software-requested warm reset.
+ *
+ * @param value Where to store the content of the register.
+ * @return      EC_SUCCESS, or non-zero if error.
  */
-uint32_t system_get_scratchpad(void);
+int system_get_scratchpad(uint32_t *value);
 
 /**
  * Return the chip vendor/name/revision string.
diff --git a/test/kb_scan.c b/test/kb_scan.c
index b3b42813e4..a43808c0c1 100644
--- a/test/kb_scan.c
+++ b/test/kb_scan.c
@@ -557,7 +557,9 @@ static int test_check_boot_down(void)
 
 void test_init(void)
 {
-	uint32_t state = system_get_scratchpad();
+	uint32_t state;
+
+	system_get_scratchpad(&state);
 
 	if (state & TEST_STATE_MASK(TEST_STATE_STEP_2)) {
 		/* Power-F3-ESC */
diff --git a/test/scratchpad.c b/test/scratchpad.c
index eeae7fcb93..1bea76f7a1 100644
--- a/test/scratchpad.c
+++ b/test/scratchpad.c
@@ -15,13 +15,13 @@ test_static int test_scratchpad(void)
 	int rv;
 	uint32_t scratch;
 
-	scratch = system_get_scratchpad();
+	TEST_EQ(system_get_scratchpad(&scratch), EC_SUCCESS, "%d");
 	TEST_EQ(scratch, 0, "%d");
 
 	rv = system_set_scratchpad(1);
 	TEST_EQ(rv, EC_SUCCESS, "%d");
 
-	scratch = system_get_scratchpad();
+	TEST_EQ(system_get_scratchpad(&scratch), EC_SUCCESS, "%d");
 	TEST_EQ(scratch, 1, "%d");
 
 	return EC_SUCCESS;
diff --git a/test/system.c b/test/system.c
index 6874708cc7..79383b82d9 100644
--- a/test/system.c
+++ b/test/system.c
@@ -29,9 +29,9 @@ static int test_reboot_on_shutdown(void)
 	params.cmd = EC_REBOOT_COLD;
 	params.flags = EC_REBOOT_FLAG_ON_AP_SHUTDOWN;
 
-	TEST_ASSERT(test_send_host_command(
-			EC_CMD_REBOOT_EC, 0, &params,
-			sizeof(params), NULL, 0) == EC_SUCCESS);
+	TEST_EQ(test_send_host_command(EC_CMD_REBOOT_EC, 0, &params,
+				       sizeof(params), NULL, 0),
+		EC_SUCCESS, "%d");
 
 	system_set_scratchpad(TEST_STATE_STEP_2);
 	test_chipset_off();
@@ -54,16 +54,16 @@ static int test_cancel_reboot(void)
 	params.cmd = EC_REBOOT_COLD;
 	params.flags = EC_REBOOT_FLAG_ON_AP_SHUTDOWN;
 
-	TEST_ASSERT(test_send_host_command(
-			EC_CMD_REBOOT_EC, 0, &params,
-			sizeof(params), NULL, 0) == EC_SUCCESS);
+	TEST_EQ(test_send_host_command(EC_CMD_REBOOT_EC, 0, &params,
+				       sizeof(params), NULL, 0),
+		EC_SUCCESS, "%d");
 
 	params.cmd = EC_REBOOT_CANCEL;
 	params.flags = 0;
 
-	TEST_ASSERT(test_send_host_command(
-			EC_CMD_REBOOT_EC, 0, &params,
-			sizeof(params), NULL, 0) == EC_SUCCESS);
+	TEST_EQ(test_send_host_command(EC_CMD_REBOOT_EC, 0, &params,
+				       sizeof(params), NULL, 0),
+		EC_SUCCESS, "%d");
 
 	test_chipset_off();
 	msleep(30);
@@ -94,7 +94,14 @@ static void fail_and_clean_up(void)
 
 void run_test(int argc, char **argv)
 {
-	uint32_t state = system_get_scratchpad();
+	uint32_t state = 0;
+
+	/* The return value isn't checked here on purpose. The scratchpad file
+	 * may exist from a previous run or it may be in a clean state. A
+	 * previous run of this test should reset the scratchpad value to 0
+	 * regardless of the final result.
+	 */
+	system_get_scratchpad(&state);
 
 	test_reset();
 
diff --git a/test/utils.c b/test/utils.c
index 9402e92740..ede0c807ed 100644
--- a/test/utils.c
+++ b/test/utils.c
@@ -270,8 +270,11 @@ static int test_shared_mem(void)
 
 static int test_scratchpad(void)
 {
+	uint32_t scratchpad_value;
+
 	system_set_scratchpad(0xfeed);
-	TEST_ASSERT(system_get_scratchpad() == 0xfeed);
+	TEST_EQ(system_get_scratchpad(&scratchpad_value), EC_SUCCESS, "%d");
+	TEST_EQ(scratchpad_value, 0xfeed, "%d");
 
 	return EC_SUCCESS;
 }
diff --git a/zephyr/shim/src/system.c b/zephyr/shim/src/system.c
index 5a9612fa26..d6af88a9ff 100644
--- a/zephyr/shim/src/system.c
+++ b/zephyr/shim/src/system.c
@@ -109,23 +109,19 @@ int system_set_scratchpad(uint32_t value)
 			 GET_BBRAM_SIZE(scratchpad), (uint8_t *)&value);
 }
 
-uint32_t system_get_scratchpad(void)
+int system_get_scratchpad(uint32_t *value)
 {
-	uint32_t value;
-
 	if (bbram_dev == NULL) {
 		LOG_ERR("bbram_dev doesn't binding");
-		/*
-		 * TODO(b/195481980): Seperate the scratchpad value & API
-		 * status.
-		 */
-		return 0;
+		return -EC_ERROR_INVAL;
 	}
 
-	cros_bbram_read(bbram_dev, GET_BBRAM_OFFSET(scratchpad),
-			GET_BBRAM_SIZE(scratchpad), (uint8_t *)&value);
+	if (cros_bbram_read(bbram_dev, GET_BBRAM_OFFSET(scratchpad),
+			    GET_BBRAM_SIZE(scratchpad), (uint8_t *)value)) {
+		return -EC_ERROR_INVAL;
+	}
 
-	return value;
+	return 0;
 }
 
 void system_hibernate(uint32_t seconds, uint32_t microseconds)