Cleanup: flash module

No functional changes; just clean up comments and remove dead code

BUG=chrome-os-partner:15579
BRANCH=none
TEST=code compiles

Change-Id: Id006ae18f2b26cea1720196f696f937811b6ba5b
Signed-off-by: Randall Spangler <rspangler@chromium.org>
Reviewed-on: https://gerrit.chromium.org/gerrit/36448
Reviewed-by: Simon Glass <sjg@chromium.org>

5 files changed, 73 insertions, 54 deletions

diff --git a/chip/lm4/flash.c b/chip/lm4/flash.c
index 9309fc7c35..8a9c3dc946 100644
--- a/chip/lm4/flash.c
+++ b/chip/lm4/flash.c
@@ -54,8 +54,10 @@ struct persist_state {
 
 /**
  * Read persistent state into pstate.
+ *
+ * @param pstate	Destination for persistent state
  */
-static int read_pstate(struct persist_state *pstate)
+static void read_pstate(struct persist_state *pstate)
 {
 	memcpy(pstate, flash_physical_dataptr(PSTATE_OFFSET), sizeof(*pstate));
 
@@ -64,12 +66,13 @@ static int read_pstate(struct persist_state *pstate)
 		memset(pstate, 0, sizeof(*pstate));
 		pstate->version = PERSIST_STATE_VERSION;
 	}
-
-	return EC_SUCCESS;
 }
 
 /**
  * Write persistent state from pstate, erasing if necessary.
+ *
+ * @param pstate	Source persistent state
+ * @return EC_SUCCESS, or nonzero if error.
  */
 static int write_pstate(const struct persist_state *pstate)
 {
@@ -77,8 +80,8 @@ static int write_pstate(const struct persist_state *pstate)
 	int rv;
 
 	/* Check if pstate has actually changed */
-	if (!read_pstate(&current_pstate) &&
-	    !memcmp(&current_pstate, pstate, sizeof(*pstate)))
+	read_pstate(&current_pstate);
+	if (!memcmp(&current_pstate, pstate, sizeof(*pstate)))
 		return EC_SUCCESS;
 
 	/* Erase pstate */
@@ -106,6 +109,7 @@ static int write_pstate(const struct persist_state *pstate)
  * flash will be writable.
  *
  * @param enable        Enable write protection
+ * @return EC_SUCCESS, or nonzero if error.
  */
 static int protect_ro_at_boot(int enable)
 {
@@ -114,9 +118,7 @@ static int protect_ro_at_boot(int enable)
 	int rv;
 
 	/* Read the current persist state from flash */
-	rv = read_pstate(&pstate);
-	if (rv)
-		return rv;
+	read_pstate(&pstate);
 
 	/* Update state if necessary */
 	if (pstate.flags != new_flags) {
@@ -153,6 +155,8 @@ static void protect_banks(int start_bank, int bank_count)
 /**
  * Perform a write-buffer operation.  Buffer (FWB) and address (FMA) must be
  * pre-loaded.
+ *
+ * @return EC_SUCCESS, or nonzero if error.
  */
 static int write_buffer(void)
 {
@@ -216,11 +220,9 @@ int flash_physical_write(int offset, int size, const char *data)
 	}
 
 	/* Handle final partial page, if any */
-	if (i > 0) {
-		rv = write_buffer();
-		if (rv != EC_SUCCESS)
-			return rv;
-	}
+	if (i > 0)
+		return write_buffer();
+
 	return EC_SUCCESS;
 }
 
@@ -327,7 +329,6 @@ int flash_set_protect(uint32_t mask, uint32_t flags)
 	 * Process flags we can set.  Track the most recent error, but process
 	 * all flags before returning.
 	 */
-
 	if (mask & EC_FLASH_PROTECT_RO_AT_BOOT) {
 		rv = protect_ro_at_boot(flags & EC_FLASH_PROTECT_RO_AT_BOOT);
 		if (rv)
diff --git a/chip/stm32/flash-stm32f100.c b/chip/stm32/flash-stm32f100.c
index 88284d247d..0801066b9d 100644
--- a/chip/stm32/flash-stm32f100.c
+++ b/chip/stm32/flash-stm32f100.c
@@ -19,7 +19,8 @@
 
 #define US_PER_SECOND 1000000
 
-/* the approximate number of CPU cycles per iteration of the loop when polling
+/*
+ * Approximate number of CPU cycles per iteration of the loop when polling
  * the flash status
  */
 #define CYCLE_PER_FLASH_LOOP 10
@@ -96,7 +97,6 @@ static int wait_busy(void)
 	return (timeout > 0) ? EC_SUCCESS : EC_ERROR_TIMEOUT;
 }
 
-
 static int unlock(int locks)
 {
 	/*
@@ -247,6 +247,8 @@ static int write_optb(int byte, uint8_t value)
 
 /**
  * Read persistent state into pstate.
+ *
+ * @param pstate	Destination for persistent state
  */
 static int read_pstate(struct persist_state *pstate)
 {
@@ -263,6 +265,9 @@ static int read_pstate(struct persist_state *pstate)
 
 /**
  * Write persistent state from pstate, erasing if necessary.
+ *
+ * @param pstate	Source persistent state
+ * @return EC_SUCCESS, or nonzero if error.
  */
 static int write_pstate(const struct persist_state *pstate)
 {
@@ -310,7 +315,6 @@ int flash_physical_write(int offset, int size, const char *data)
 	/* set PG bit */
 	STM32_FLASH_CR |= PG;
 
-
 	for ( ; size > 0; size -= sizeof(uint16_t)) {
 #ifdef CONFIG_TASK_WATCHDOG
 		/* Reload the watchdog timer to avoid watchdog reset when doing
@@ -354,7 +358,6 @@ exit_wr:
 	return res;
 }
 
-
 int flash_physical_erase(int offset, int size)
 {
 	int res = EC_SUCCESS;
@@ -381,9 +384,11 @@ int flash_physical_erase(int offset, int size)
 
 		/* set STRT bit : start erase */
 		STM32_FLASH_CR |= STRT;
+
 #ifdef CONFIG_TASK_WATCHDOG
-		/* Reload the watchdog timer in case the erase takes long time
-		 * so that erasing many flash pages
+		/*
+		 * Reload the watchdog timer to avoid watchdog reset during a
+		 * long erase operation.
 		 */
 		watchdog_reload();
 #endif
@@ -399,8 +404,10 @@ int flash_physical_erase(int offset, int size)
 			goto exit_er;
 		}
 
-		/* Check for error conditions - erase failed, voltage error,
-		 * protection error */
+		/*
+		 * Check for error conditions - erase failed, voltage error,
+		 * protection error
+		 */
 		if (STM32_FLASH_SR & 0x14) {
 			res = EC_ERROR_UNKNOWN;
 			goto exit_er;
@@ -503,10 +510,12 @@ static int protect_entire_flash_until_reboot(void)
 }
 
 /**
- * Determine if write protect register is inconsistent with RO_AT_BOOT and
+ * Check if write protect register state is inconsistent with RO_AT_BOOT and
  * ALL_AT_BOOT state.
+ *
+ * @return zero if consistent, non-zero if inconsistent.
  */
-static int register_need_reset(void)
+static int registers_need_reset(void)
 {
 	uint32_t flags = flash_get_protect();
 	int i;
@@ -563,9 +572,9 @@ int flash_pre_init(void)
 			need_reset = 1;
 		}
 
-		if (register_need_reset()) {
+		if (registers_need_reset()) {
 			/*
-			 * Reset RO protect register to make sure this doesn't
+			 * Reset RO protect registers to make sure this doesn't
 			 * happen again due to RO protect state inconsistency.
 			 */
 			protect_ro_at_boot(
@@ -642,7 +651,6 @@ int flash_set_protect(uint32_t mask, uint32_t flags)
 	 * Process flags we can set.  Track the most recent error, but process
 	 * all flags before returning.
 	 */
-
 	if (mask & EC_FLASH_PROTECT_RO_AT_BOOT) {
 		rv = protect_ro_at_boot(flags & EC_FLASH_PROTECT_RO_AT_BOOT, 0);
 		if (rv)
@@ -670,7 +678,9 @@ int flash_set_protect(uint32_t mask, uint32_t flags)
 	return retval;
 }
 
-/* TODO: crosbug.com/p/12036 */
+/*****************************************************************************/
+/* Console commands */
+
 static int command_set_fake_wp(int argc, char **argv)
 {
 	int val;
diff --git a/chip/stm32/flash-stm32l15x.c b/chip/stm32/flash-stm32l15x.c
index 885047a032..98d1d7b2d6 100644
--- a/chip/stm32/flash-stm32l15x.c
+++ b/chip/stm32/flash-stm32l15x.c
@@ -15,8 +15,9 @@
 
 #define US_PER_SECOND 1000000
 
-/* the approximate number of CPU cycles per iteration of the loop when polling
- * the flash status
+/*
+ * Approximate number of CPU cycles per iteration of the loop when polling
+ * the flash status.
  */
 #define CYCLE_PER_FLASH_LOOP 10
 
@@ -57,6 +58,9 @@ static uint32_t write_buffer[CONFIG_FLASH_WRITE_SIZE / sizeof(uint32_t)];
 static int buffered_off = -1;
 #endif
 
+/* TODO: (crosbug.com/p/15613) Verify write protect on stm32l */
+#undef STM32L_WP_VERIFIED
+
 static int unlock(int locks)
 {
 	/* unlock PECR if needed */
@@ -90,6 +94,7 @@ static uint8_t read_optb(int byte)
 
 }
 
+#ifdef STM32L_WP_VERIFIED
 static void write_optb(int byte, uint8_t value)
 {
 	volatile int32_t *word = (uint32_t *)(STM32_OPTB_BASE + (byte & ~0x3));
@@ -107,12 +112,11 @@ static void write_optb(int byte, uint8_t value)
 
 	lock();
 }
+#endif
 
 /**
- * This function lives in internal RAM,
- * as we cannot read flash during writing.
- * You should neither call other function from this one,
- * nor declare it static.
+ * This function lives in internal RAM, as we cannot read flash during writing.
+ * You must not call other functions from this one or declare it static.
  */
 void  __attribute__((section(".iram.text")))
 	iram_flash_write(uint32_t *addr, uint32_t *data)
@@ -146,8 +150,9 @@ void  __attribute__((section(".iram.text")))
 
 int flash_physical_write(int offset, int size, const char *data)
 {
-	/* this is pretty nasty, we need to enforce alignment instead of this
-	 * wild cast : TODO crosbug.com/p/9526
+	/*
+	 * TODO: (crosbug.com/p/9526) Enforce alignment instead of blindly
+	 * casting data to uint32_t *.
 	 */
 	uint32_t *data32 = (uint32_t *)data;
 	uint32_t *address;
@@ -187,7 +192,8 @@ int flash_physical_write(int offset, int size, const char *data)
 	for (address = (uint32_t *)(CONFIG_FLASH_BASE + offset) ;
 	     size > 0; size -= CONFIG_FLASH_WRITE_SIZE) {
 #ifdef CONFIG_TASK_WATCHDOG
-		/* Reload the watchdog timer to avoid watchdog reset when doing
+		/*
+		 * Reload the watchdog timer to avoid watchdog reset when doing
 		 * long writing with interrupt disabled.
 		 */
 		watchdog_reload();
@@ -201,8 +207,10 @@ int flash_physical_write(int offset, int size, const char *data)
 			goto exit_wr;
 		}
 
-		/* Check for error conditions - erase failed, voltage error,
-		 * protection error */
+		/*
+		 * Check for error conditions: erase failed, voltage error,
+		 * protection error
+		 */
 		if (STM32_FLASH_SR & 0xF00) {
 			res = EC_ERROR_UNKNOWN;
 			goto exit_wr;
@@ -215,7 +223,6 @@ exit_wr:
 	return res;
 }
 
-
 int flash_physical_erase(int offset, int size)
 {
 	uint32_t *address;
@@ -238,15 +245,16 @@ int flash_physical_erase(int offset, int size)
 		/*
 		 * crosbug.com/p/13066
 		 * We can't do the flash_is_erased() trick on stm32l since
-		 * bits erase to 0, not 1. Will address later if needed.
+		 * bits erase to 0, not 1.  Will address later if needed.
 		 */
 
 		/* Start erase */
 		*address = 0x00000000;
 
 #ifdef CONFIG_TASK_WATCHDOG
-		/* Reload the watchdog timer in case the erase takes long time
-		 * so that erasing many flash pages
+		/*
+		 * Reload the watchdog timer to avoid watchdog reset during
+		 * multi-page erase operations.
 		 */
 		watchdog_reload();
 #endif
@@ -262,8 +270,10 @@ int flash_physical_erase(int offset, int size)
 			goto exit_er;
 		}
 
-		/* Check for error conditions - erase failed, voltage error,
-		 * protection error */
+		/*
+		 * Check for error conditions: erase failed, voltage error,
+		 * protection error
+		 */
 		if (STM32_FLASH_SR & 0xF00) {
 			res = EC_ERROR_UNKNOWN;
 			goto exit_er;
@@ -276,7 +286,6 @@ exit_er:
 	return res;
 }
 
-
 int flash_physical_get_protect(int block)
 {
 	uint8_t val = read_optb(STM32_OPTB_WRP_OFF(block/8));
@@ -285,7 +294,7 @@ int flash_physical_get_protect(int block)
 
 void flash_physical_set_protect(int start_bank, int bank_count)
 {
-	if (0) { /* TODO: crosbug.com/p/9849 verify WP */
+#ifdef STM32L_WP_VERIFIED
 	int block;
 
 	for (block = start_bank; block < start_bank + bank_count; block++) {
@@ -293,7 +302,7 @@ void flash_physical_set_protect(int start_bank, int bank_count)
 		uint8_t val = read_optb(byte_off) | (1 << (block % 8));
 		write_optb(byte_off, val);
 	}
-	}
+#endif
 }
 
 uint32_t flash_get_protect(void)
@@ -301,8 +310,9 @@ uint32_t flash_get_protect(void)
 	uint32_t flags = 0;
 	int i;
 
-	/* TODO (vpalatin) : write protect scheme for stm32 */
 	/*
+	 * TODO: (crosbug.com/p/15613) write protect scheme
+	 *
 	 * Always enable write protect until we have WP pin.  For developer to
 	 * unlock WP, please use stm32mon -u and immediately re-program the
 	 * pstate sector.
@@ -331,7 +341,7 @@ uint32_t flash_get_protect(void)
 
 int flash_set_protect(uint32_t mask, uint32_t flags)
 {
-	/* TODO: implement! */
+	/* TODO: (crosbug.com/p/15613) implement! */
 	return EC_SUCCESS;
 }
 
diff --git a/common/flash_common.c b/common/flash_common.c
index f13a978ebf..6afa4adc6e 100644
--- a/common/flash_common.c
+++ b/common/flash_common.c
@@ -5,7 +5,7 @@
 
 /* Flash memory module for Chrome EC - common functions */
 
-#include "config.h"
+#include "common.h"
 #include "console.h"
 #include "flash.h"
 #include "host_command.h"
@@ -62,7 +62,7 @@ int flash_erase(int offset, int size)
 /*****************************************************************************/
 /* Console commands */
 
-/*
+/**
  * Parse offset and size from command line argv[shift] and argv[shift+1]
  *
  * Default values: If argc<=shift, leaves offset unchanged, returning error if
@@ -166,7 +166,6 @@ static int command_flash_write(int argc, char **argv)
 	char *data;
 	int i;
 
-
 	rv = parse_offset_size(argc, argv, 1, &offset, &size);
 	if (rv)
 		return rv;
diff --git a/include/flash.h b/include/flash.h
index 8bf0264c14..cef27065dd 100644
--- a/include/flash.h
+++ b/include/flash.h
@@ -9,7 +9,6 @@
 #define __CROS_EC_FLASH_H
 
 #include "common.h"
-#include "config.h"
 #include "ec_commands.h"  /* For EC_FLASH_PROTECT_* flags */
 
 /*****************************************************************************/