Clean up system module

No functional changes.  (it might look like
SYSTEM_HIB_MINIMUM_DURATION is a change, but it's not used at present)

BUG=chrome-os-partner:15579
BRANCH=none
TEST=version; chip info should print successfully

Change-Id: Idd7f60a29528e9f6af4f91cd5a556e7336acee9f
Signed-off-by: Randall Spangler <rspangler@chromium.org>
Reviewed-on: https://gerrit.chromium.org/gerrit/36599
Reviewed-by: Bill Richardson <wfrichar@chromium.org>

4 files changed, 145 insertions, 95 deletions

diff --git a/chip/lm4/system.c b/chip/lm4/system.c
index d69942e85c..b998cb5e2d 100644
--- a/chip/lm4/system.c
+++ b/chip/lm4/system.c
@@ -39,15 +39,21 @@ enum hibdata_index {
  */
 #define HIB_RESET_USEC 150000
 
+/**
+ * Wait for a write to commit to a hibernate register.
+ *
+ * @return EC_SUCCESS if successful, non-zero if error.
+ */
 static int wait_for_hibctl_wc(void)
 {
 	int i;
+
 	/* Wait for write-capable */
 	for (i = 0; i < 1000000; i++) {
 		if (LM4_HIBERNATE_HIBCTL & LM4_HIBCTL_WRC)
 			return EC_SUCCESS;
 	}
-	return EC_ERROR_UNKNOWN;
+	return EC_ERROR_TIMEOUT;
 }
 
 /**
@@ -282,7 +288,7 @@ void system_hibernate(uint32_t seconds, uint32_t microseconds)
 	hibernate(seconds, microseconds, 0);
 }
 
-int system_pre_init(void)
+void system_pre_init(void)
 {
 	volatile uint32_t scratch  __attribute__((unused));
 
@@ -341,8 +347,6 @@ int system_pre_init(void)
 
 	/* Brown-outs should trigger a reset */
 	LM4_SYSTEM_PBORCTL |= 0x02;
-
-	return EC_SUCCESS;
 }
 
 void system_reset(int flags)
diff --git a/chip/stm32/system.c b/chip/stm32/system.c
index df10f6810a..a985966875 100644
--- a/chip/stm32/system.c
+++ b/chip/stm32/system.c
@@ -13,8 +13,10 @@
 #include "version.h"
 #include "watchdog.h"
 
-/* TODO: Fake WP is stored at most significant bit of saved reset flags to save
- * space.  Remove it when we have real write protect pin */
+/*
+ * TODO: Fake WP is stored at most significant bit of saved reset flags to save
+ * space.  Remove it when we have real write protect pin
+ */
 
 enum bkpdata_index {
 	BKPDATA_INDEX_SCRATCHPAD,	/* General-purpose scratchpad */
@@ -29,7 +31,6 @@ enum bkpdata_index {
 	BKPDATA_INDEX_VBNV_CONTEXT7,
 };
 
-
 /**
  * Read backup register at specified index.
  *
@@ -74,8 +75,9 @@ static void check_reset_cause(void)
 	bkpdata_write(BKPDATA_INDEX_SAVED_RESET_FLAGS, 0 | fake_wp);
 
 	if (raw_cause & 0x60000000) {
-		/* IWDG or WWDG
-		 * if the watchdog was not used as an hard reset mechanism
+		/*
+		 * IWDG or WWDG, if the watchdog was not used as an hard reset
+		 * mechanism
 		 */
 		if (!(flags & RESET_FLAG_HARD))
 			flags |= RESET_FLAG_WATCHDOG;
@@ -91,7 +93,7 @@ static void check_reset_cause(void)
 		flags |= RESET_FLAG_RESET_PIN;
 
 	if (pwr_status & 0x00000002)
-		/* Hibernation and waked */
+		/* Hibernated and subsequently awakened */
 		flags |= RESET_FLAG_HIBERNATE;
 
 	if (!flags && (raw_cause & 0xfe000000))
@@ -100,15 +102,13 @@ static void check_reset_cause(void)
 	system_set_reset_flags(flags);
 }
 
-
 void system_hibernate(uint32_t seconds, uint32_t microseconds)
 {
 	while (1)
 		/* NOT IMPLEMENTED */;
 }
 
-
-int system_pre_init(void)
+void system_pre_init(void)
 {
 	/* enable clock on Power module */
 	STM32_RCC_APB1ENR |= 1 << 28;
@@ -142,11 +142,8 @@ int system_pre_init(void)
 #endif
 
 	check_reset_cause();
-
-	return EC_SUCCESS;
 }
 
-
 void system_reset(int flags)
 {
 	uint32_t save_flags = 0;
@@ -157,8 +154,10 @@ void system_reset(int flags)
 	/* Disable interrupts to avoid task swaps during reboot */
 	interrupt_disable();
 
-	/* TODO: Check if a collision between reset flags and fake wp occurred.
-	 * Remove this when we have real write protect pin. */
+	/*
+	 * TODO: Check if a collision between reset flags and fake wp occurred.
+	 * Remove this when we have real write protect pin.
+	 */
 	ASSERT(!(system_get_reset_flags() & 0x8000));
 
 	/* Save current reset reasons if necessary */
@@ -191,7 +190,6 @@ void system_reset(int flags)
 		;
 }
 
-
 int system_set_scratchpad(uint32_t value)
 {
 	/* Check if value fits in 16 bits */
@@ -200,31 +198,26 @@ int system_set_scratchpad(uint32_t value)
 	return bkpdata_write(BKPDATA_INDEX_SCRATCHPAD, (uint16_t)value);
 }
 
-
 uint32_t system_get_scratchpad(void)
 {
 	return (uint32_t)bkpdata_read(BKPDATA_INDEX_SCRATCHPAD);
 }
 
-
 const char *system_get_chip_vendor(void)
 {
 	return "stm";
 }
 
-
 const char *system_get_chip_name(void)
 {
 	return STRINGIFY(CHIP_VARIANT);
 }
 
-
 const char *system_get_chip_revision(void)
 {
 	return "";
 }
 
-
 int system_get_vbnvcontext(uint8_t *block)
 {
 	enum bkpdata_index i;
@@ -240,7 +233,6 @@ int system_get_vbnvcontext(uint8_t *block)
 	return EC_SUCCESS;
 }
 
-
 int system_set_vbnvcontext(const uint8_t *block)
 {
 	enum bkpdata_index i;
@@ -259,7 +251,6 @@ int system_set_vbnvcontext(const uint8_t *block)
 	return EC_SUCCESS;
 }
 
-
 /* TODO: crosbug.com/p/12036 */
 int system_set_fake_wp(int val)
 {
@@ -273,7 +264,6 @@ int system_set_fake_wp(int val)
 	return bkpdata_write(BKPDATA_INDEX_SAVED_RESET_FLAGS, flags);
 }
 
-
 /* TODO: crosbug.com/p/12036 */
 int system_get_fake_wp(void)
 {
diff --git a/common/system_common.c b/common/system_common.c
index 7db000758d..5fdb6a23dd 100644
--- a/common/system_common.c
+++ b/common/system_common.c
@@ -8,7 +8,6 @@
 #include "clock.h"
 #include "common.h"
 #include "console.h"
-#include "ec_commands.h"
 #include "flash.h"
 #include "gpio.h"
 #include "hooks.h"
@@ -105,7 +104,6 @@ int system_is_locked(void)
 #endif
 }
 
-
 int system_usable_ram_end(void)
 {
 	/* Leave space at the end of RAM for jump data and tags.
@@ -257,11 +255,8 @@ int system_get_image_used(enum system_image_copy_t copy)
 	return size ? size + 1 : 0;  /* 0xea byte IS part of the image */
 }
 
-/* Returns true if the given range is overlapped with the active image.
- *
- * We only care the runtime code since the EC is running over it.
- * We don't care about the vector table, FMAP, and init code. */
-int system_unsafe_to_overwrite(uint32_t offset, uint32_t size) {
+int system_unsafe_to_overwrite(uint32_t offset, uint32_t size)
+{
 	uint32_t r_offset;
 	uint32_t r_size;
 
@@ -285,16 +280,19 @@ int system_unsafe_to_overwrite(uint32_t offset, uint32_t size) {
 		return 0;
 }
 
-
 const char *system_get_image_copy_string(void)
 {
 	int copy = system_get_image_copy();
 	return copy < ARRAY_SIZE(image_names) ? image_names[copy] : "?";
 }
 
-
-/* Jump to what we hope is the init address of an image.  This function does
- * not return. */
+/**
+ * Jump to what we hope is the init address of an image.
+ *
+ * This function does not return.
+ *
+ * @param init_addr	Init address of target image
+ */
 static void jump_to_image(uint32_t init_addr)
 {
 	void (*resetvec)(void) = (void(*)(void))init_addr;
@@ -328,8 +326,9 @@ static void jump_to_image(uint32_t init_addr)
 	resetvec();
 }
 
-
-/* Return the base pointer for the image copy, or 0xffffffff if error. */
+/**
+ * Return the base pointer for the image copy, or 0xffffffff if error.
+ */
 static uint32_t get_base(enum system_image_copy_t copy)
 {
 	switch (copy) {
@@ -342,7 +341,9 @@ static uint32_t get_base(enum system_image_copy_t copy)
 	}
 }
 
-/* Return the size of the image copy, or 0 if error. */
+/**
+ * Return the size of the image copy, or 0 if error.
+ */
 static uint32_t get_size(enum system_image_copy_t copy)
 {
 	switch (copy) {
@@ -355,7 +356,6 @@ static uint32_t get_size(enum system_image_copy_t copy)
 	}
 }
 
-
 int system_run_image_copy(enum system_image_copy_t copy)
 {
 	uint32_t base;
@@ -404,7 +404,6 @@ int system_run_image_copy(enum system_image_copy_t copy)
 	return EC_ERROR_UNKNOWN;
 }
 
-
 const char *system_get_version(enum system_image_copy_t copy)
 {
 	uint32_t addr;
@@ -432,7 +431,6 @@ const char *system_get_version(enum system_image_copy_t copy)
 	return "";
 }
 
-
 int system_get_board_version(void)
 {
 	int v = 0;
@@ -449,13 +447,12 @@ int system_get_board_version(void)
 	return v;
 }
 
-
 const char *system_get_build_info(void)
 {
 	return build_info;
 }
 
-int system_common_pre_init(void)
+void system_common_pre_init(void)
 {
 	uint32_t addr;
 
@@ -477,8 +474,9 @@ int system_common_pre_init(void)
 	if (jdata->magic == JUMP_DATA_MAGIC &&
 	    jdata->version >= 1 &&
 	    reset_flags == 0) {
-		int delta;       /* Change in jump data struct size between the
-				  * previous image and this one. */
+		/* Change in jump data struct size between the previous image
+		 * and this one. */
+		int delta;
 
 		/* Yes, we jumped to this image */
 		jumped_to_image = 1;
@@ -524,11 +522,11 @@ int system_common_pre_init(void)
 		/* Clear the whole jump_data struct */
 		memset(jdata, 0, sizeof(struct jump_data));
 	}
-
-	return EC_SUCCESS;
 }
 
-/* Handle a pending reboot command */
+/**
+ * Handle a pending reboot command.
+ */
 static int handle_pending_reboot(enum ec_reboot_cmd cmd)
 {
 	switch (cmd) {
@@ -596,7 +594,6 @@ DECLARE_CONSOLE_COMMAND(sysinfo, command_sysinfo,
 			"Print system info",
 			NULL);
 
-
 #define CONFIG_CONSOLE_CMD_SCRATCHPAD
 #ifdef CONFIG_CONSOLE_CMD_SCRATCHPAD
 static int command_scratchpad(int argc, char **argv)
@@ -620,7 +617,6 @@ DECLARE_CONSOLE_COMMAND(scratchpad, command_scratchpad,
 			NULL);
 #endif /* CONFIG_CONSOLE_CMD_SCRATCHPAD */
 
-
 static int command_hibernate(int argc, char **argv)
 {
 	int seconds = 0;
@@ -645,7 +641,6 @@ DECLARE_CONSOLE_COMMAND(hibernate, command_hibernate,
 			"Hibernate the EC",
 			NULL);
 
-
 static int command_version(int argc, char **argv)
 {
 	ccprintf("Chip:    %s %s %s\n", system_get_chip_vendor(),
@@ -661,7 +656,6 @@ DECLARE_CONSOLE_COMMAND(version, command_version,
 			"Print versions",
 			NULL);
 
-
 static int command_sysjump(int argc, char **argv)
 {
 	uint32_t addr;
@@ -700,7 +694,6 @@ DECLARE_CONSOLE_COMMAND(sysjump, command_sysjump,
 			"Jump to a system image or address",
 			NULL);
 
-
 static int command_reboot(int argc, char **argv)
 {
 	int flags = 0;
diff --git a/include/system.h b/include/system.h
index 1385c4d006..480b655974 100644
--- a/include/system.h
+++ b/include/system.h
@@ -34,13 +34,17 @@ enum system_image_copy_t {
 	SYSTEM_IMAGE_RW
 };
 
-/* Pre-initializes the module.  This occurs before clocks or tasks are
- * set up. */
-int system_pre_init(void);
+/**
+ * Pre-initializes the module.  This occurs before clocks or tasks are
+ * set up.
+ */
+void system_pre_init(void);
 
-/* System common pre-initialization; called after chip-specific
- * system_pre_init(). */
-int system_common_pre_init(void);
+/**
+ * System common pre-initialization; called after chip-specific
+ * system_pre_init().
+ */
+void system_common_pre_init(void);
 
 /**
  * Get the reset flags.
@@ -68,46 +72,77 @@ void system_clear_reset_flags(uint32_t flags);
  */
 void system_print_reset_flags(void);
 
-/* Return non-zero if the system is locked down for normal consumer use.
+/**
+ * Check if system is locked down for normal consumer use.
+ *
+ * @return non-zero if the system is locked down for normal consumer use.
  * Potentially-dangerous developer and/or factory commands must be disabled
  * unless this command returns 0.
  *
  * This should be controlled by the same mechanism which write-protects the
  * read-only image (so that the only way to unlock the system is to unprotect
- * the read-only image). */
+ * the read-only image).
+ */
 int system_is_locked(void);
 
-/* Disable jumping between images for the rest of this boot. */
+/**
+ * Disable jumping between images for the rest of this boot.
+ */
 void system_disable_jump(void);
 
-/* Return the image copy which is currently running. */
+/**
+ * Return the image copy which is currently running.
+ */
 enum system_image_copy_t system_get_image_copy(void);
 
-/* Return non-zero if the system has switched between image copies at least
- * once since the last real boot. */
+/**
+ * Return non-zero if the system has switched between image copies at least
+ * once since the last real boot.
+ */
 int system_jumped_to_this_image(void);
 
-/* Preserve data across a jump between images.  <tag> identifies the data
- * type.  <size> must be a multiple of 4 bytes, and less than 255 bytes.
- * <version> is the data version, so that tag data can evolve as firmware
- * is updated.  <data> points to the data to save.
+/**
+ * Preserve data across a jump between images.
+ *
+ * This may ONLY be called from within a HOOK_SYSJUMP handler.
  *
- * This may ONLY be called from within a HOOK_SYSJUMP handler. */
+ * @param tag		Data type
+ * @param size          Size of data; must be a multiple of 4 bytes, and less
+ *			than 255 bytes.
+ * @param version       Data version, so that tag data can evolve as firmware
+ *			is updated.
+ * @param data		Pointer to data to save
+ * @return EC_SUCCESS, or non-zero if error.
+ */
 int system_add_jump_tag(uint16_t tag, int version, int size, const void *data);
 
-/* Retrieve data stored by a previous image's call to
- * system_add_jump_tag().  If a matching tag is found, retrieves
- * <size> and <version>, and returns a pointer to the data.  Returns
- * NULL if no matching tag is found. */
+/**
+ * Retrieve previously stored jump data
+ *
+ * This retrieves data stored by a previous image's call to
+ * system_add_jump_tag().
+ *
+ * @param tag		Data type to retrieve
+ * @param version	Set to data version if successful
+ * @param size		Set to data size if successful
+ * @return		A pointer to the data, or NULL if no matching tag is
+ *			found.
+ */
 const uint8_t *system_get_jump_tag(uint16_t tag, int *version, int *size);
 
-/* Return the address just past the last usable byte in RAM. */
+/**
+ * Return the address just past the last usable byte in RAM.
+ */
 int system_usable_ram_end(void);
 
-/* Return true if the given range is overlapped with the active image. */
+/**
+ * Return non-zero if the given range is overlapped with the active image.
+ */
 int system_unsafe_to_overwrite(uint32_t offset, uint32_t size);
 
-/* Return a text description of the image copy which is currently running. */
+/**
+ * Return a text description of the image copy which is currently running.
+ */
 const char *system_get_image_copy_string(void);
 
 /**
@@ -121,20 +156,31 @@ const char *system_get_image_copy_string(void);
  */
 int system_get_image_used(enum system_image_copy_t copy);
 
-/* Jump to the specified image copy. */
+/**
+ * Jump to the specified image copy.
+ */
 int system_run_image_copy(enum system_image_copy_t copy);
 
-/* Return the version string for an image copy, or an empty string if
- * error.  If copy==SYSTEM_IMAGE_UNKNOWN, returns the version for the
- * currently-running image. */
+/**
+ * Get the version string for an image
+ *
+ * @param copy		Image copy to get version from, or SYSTEM_IMAGE_UNKNOWN
+ *			to get the version for the currently running image.
+ * @return The version string for the image copy, or an empty string if
+ * error.
+ */
 const char *system_get_version(enum system_image_copy_t copy);
 
-/* Return the board version number.  The meaning of this number is
- * board-dependent; see enum board_version in board.h for known versions. */
+/**
+ * Return the board version number.  The meaning of this number is
+ * board-dependent; see enum board_version in board.h for known versions.
+ */
 int system_get_board_version(void);
 
-/* Return information about the build including the version, build date and
- * user/machine which performed the build. */
+/**
+ * Return information about the build including the version, build date and
+ * user/machine which performed the build.
+ */
 const char *system_get_build_info(void);
 
 /* Flags for system_reset() */
@@ -154,20 +200,37 @@ const char *system_get_build_info(void);
  */
 #define SYSTEM_RESET_LEAVE_AP_OFF   (1 << 2)
 
+/**
+ * Reset the system.
+ *
+ * @param flags		Reset flags; see SYSTEM_RESET_* above.
+ */
 void system_reset(int flags);
 
-/* System warm reboot while keeping the RAM alive. */
+/**
+ * System warm reboot while keeping the RAM alive.
+ */
 void system_warm_reboot(void);
 
-/* Set a scratchpad register to the specified value.  The scratchpad
- * register must maintain its contents across a software-requested
- * warm reset. */
+/**
+ * Set a scratchpad register to the specified value.
+ *
+ * The scratchpad register must maintain its contents across a
+ * software-requested warm reset.
+ *
+ * @param value		Value to store.
+ * @return EC_SUCCESS, or non-zero if error.
+ */
 int system_set_scratchpad(uint32_t value);
 
-/* Return the current scratchpad register value. */
+/**
+ * Return the current scratchpad register value.
+ */
 uint32_t system_get_scratchpad(void);
 
-/* Return the chip info */
+/**
+ * Return the chip vendor/name/revision string.
+ */
 const char *system_get_chip_vendor(void);
 const char *system_get_chip_name(void);
 const char *system_get_chip_revision(void);
@@ -200,6 +263,6 @@ int system_set_vbnvcontext(const uint8_t *block);
 void system_hibernate(uint32_t seconds, uint32_t microseconds);
 
 /* Minimum duration to get proper hibernation */
-#define SYSTEM_HIB_MINIMUM_DURATION 0, 1000
+#define SYSTEM_HIB_MINIMUM_DURATION 0, 150000
 
 #endif  /* __CROS_EC_SYSTEM_H */