Remove flash_read()

Everything now uses flash_dataptr() to get at flash memory rather than
calling the read function, since the read function adds a needless memcpy().

BUG=chrome-os-partner:11150
TEST=manual

flashwp enable
reboot
flashinfo -> should show ro_at_boot
flashwp disable
reboot
flashinfo -> should no longer show ro_at_boot

Change-Id: I1830e2f036cf6777115c782c1737335ff2eb4ce0
Signed-off-by: Randall Spangler <rspangler@chromium.org>
Reviewed-on: https://gerrit.chromium.org/gerrit/27796

6 files changed, 58 insertions, 115 deletions

diff --git a/chip/lm4/flash.c b/chip/lm4/flash.c
index c69538d5bc..0df24ed838 100644
--- a/chip/lm4/flash.c
+++ b/chip/lm4/flash.c
@@ -32,39 +32,26 @@ int flash_get_write_block_size(void)
 	return FLASH_WRITE_BYTES;
 }
 
-
 int flash_get_erase_block_size(void)
 {
 	return FLASH_ERASE_BYTES;
 }
 
-
 int flash_get_protect_block_size(void)
 {
 	BUILD_ASSERT(FLASH_PROTECT_BYTES == CONFIG_FLASH_BANK_SIZE);
 	return FLASH_PROTECT_BYTES;
 }
 
-
 int flash_physical_size(void)
 {
 	return (LM4_FLASH_FSIZE + 1) * FLASH_PROTECT_BYTES;
 }
 
-
-int flash_physical_read(int offset, int size, char *data)
-{
-	/* Just read the flash from its memory window. */
-	/* TODO: (crosbug.com/p/7473) is this affected by data cache?
-	 * That is, if we read a block, then alter it, then read it
-	 * again, do we get the old data? */
-	memcpy(data, (char *)offset, size);
-	return EC_SUCCESS;
-}
-
-
-/* Perform a write-buffer operation.  Buffer (FWB) and address (FMA) must be
- * pre-loaded. */
+/**
+ * Perform a write-buffer operation.  Buffer (FWB) and address (FMA) must be
+ * pre-loaded.
+ */
 static int write_buffer(void)
 {
 	int t;
diff --git a/chip/lm4/mock_flash.c b/chip/lm4/mock_flash.c
index 7f79cb2c9c..e516439355 100644
--- a/chip/lm4/mock_flash.c
+++ b/chip/lm4/mock_flash.c
@@ -45,19 +45,6 @@ int flash_physical_size(void)
 }
 
 
-int flash_physical_read(int offset, int size, char *data)
-{
-	int i;
-	if (offset >= FLASH_MOCK_BEGIN)
-		memcpy(data, pstate_space + offset - FLASH_MOCK_BEGIN, size);
-	else {
-		for (i = offset; i < offset + size; ++i)
-			*(data++) = (char)(i & 0xff);
-	}
-	return EC_SUCCESS;
-}
-
-
 int flash_physical_write(int offset, int size, const char* data)
 {
 	int i;
diff --git a/chip/stm32/flash-stm32f100.c b/chip/stm32/flash-stm32f100.c
index 2e88f3b511..a52e1ec2d2 100644
--- a/chip/stm32/flash-stm32f100.c
+++ b/chip/stm32/flash-stm32f100.c
@@ -51,35 +51,22 @@ int flash_get_write_block_size(void)
 	return FLASH_WRITE_BYTES;
 }
 
-
 int flash_get_erase_block_size(void)
 {
 	return FLASH_ERASE_BYTES;
 }
 
-
 int flash_get_protect_block_size(void)
 {
 	BUILD_ASSERT(FLASH_PROTECT_BYTES == CONFIG_FLASH_BANK_SIZE);
 	return FLASH_PROTECT_BYTES;
 }
 
-
 int flash_physical_size(void)
 {
 	return CONFIG_FLASH_SIZE;
 }
 
-
-int flash_physical_read(int offset, int size, char *data)
-{
-	/* Just read the flash from its memory window. */
-	/* TODO: (crosbug.com/p/7473) is this affected by data cache? */
-	memcpy(data, (char *)offset, size);
-	return EC_SUCCESS;
-}
-
-
 static int wait_busy(void)
 {
 	int timeout = FLASH_TIMEOUT_LOOP;
diff --git a/chip/stm32/flash-stm32l15x.c b/chip/stm32/flash-stm32l15x.c
index 2ac9227c2e..e7f048b5d7 100644
--- a/chip/stm32/flash-stm32l15x.c
+++ b/chip/stm32/flash-stm32l15x.c
@@ -79,15 +79,6 @@ int flash_physical_size(void)
 	return CONFIG_FLASH_SIZE;
 }
 
-
-int flash_physical_read(int offset, int size, char *data)
-{
-	/* Just read the flash from its memory window. */
-	/* TODO: (crosbug.com/p/7473) is this affected by data cache? */
-	memcpy(data, (char *)offset, size);
-	return EC_SUCCESS;
-}
-
 static int unlock(int locks)
 {
 	/* unlock PECR if needed */
diff --git a/common/flash_common.c b/common/flash_common.c
index c011e0ab25..26d017db87 100644
--- a/common/flash_common.c
+++ b/common/flash_common.c
@@ -68,21 +68,23 @@ static int wp_pin_asserted(void)
 static int read_pstate(void)
 {
 #ifdef CHIP_stm32
-		memset(&pstate, 0, sizeof(pstate));
-		pstate.version = PERSIST_STATE_VERSION;
+	memset(&pstate, 0, sizeof(pstate));
+	pstate.version = PERSIST_STATE_VERSION;
+	return EC_SUCCESS;
 #else
-	int rv = flash_physical_read(PSTATE_OFFSET, sizeof(pstate),
-				     (char *)&pstate);
-	if (rv)
-		return rv;
+	const char *flash_pstate = flash_physical_dataptr(PSTATE_OFFSET);
+
+	if (flash_pstate)
+		memcpy(&pstate, flash_pstate, sizeof(pstate));
 
 	/* Sanity-check data and initialize if necessary */
-	if (pstate.version != PERSIST_STATE_VERSION) {
+	if (pstate.version != PERSIST_STATE_VERSION || !flash_pstate) {
 		memset(&pstate, 0, sizeof(pstate));
 		pstate.version = PERSIST_STATE_VERSION;
 	}
+
+	return flash_pstate ? EC_SUCCESS : EC_ERROR_UNKNOWN;
 #endif /* CHIP_stm32 */
-	return EC_SUCCESS;
 }
 
 /* Write persistent state from pstate, erasing if necessary. */
@@ -153,14 +155,6 @@ int flash_dataptr(int offset, int size_req, int align, char **ptrp)
 	return CONFIG_FLASH_SIZE - offset;
 }
 
-int flash_read(int offset, int size, char *data)
-{
-	if (flash_dataptr(offset, size, 1, NULL) < 0)
-		return EC_ERROR_INVAL;  /* Invalid range */
-
-	return flash_physical_read(offset, size, data);
-}
-
 int flash_write(int offset, int size, const char *data)
 {
 	if (flash_dataptr(offset, size, flash_get_write_block_size(),
@@ -193,6 +187,8 @@ int flash_enable_protect(int enable)
 	int rv;
 
 	/* Fail if write protect block is already locked */
+	/* TODO: and in the wrong state... if it's asking to enable and we're
+	 * already enabled, we can just succeed... */
 	if (flash_physical_get_protect(PSTATE_BANK))
 		return EC_ERROR_UNKNOWN;
 
diff --git a/include/flash.h b/include/flash.h
index 95cafa374d..7cb1c4fd14 100644
--- a/include/flash.h
+++ b/include/flash.h
@@ -19,14 +19,22 @@
  */
 int flash_physical_pre_init(void);
 
-/* Return the write / erase / protect block size, in bytes.  Operations must be
+/**
+ * Return the write / erase / protect block size, in bytes.  Operations must be
  * aligned to and multiples of the granularity.  For example, erase operations
- * must have offset and size which are multiples of the erase block size. */
+ * must have offset and size which are multiples of the erase block size.
+ */
 int flash_get_write_block_size(void);
 int flash_get_erase_block_size(void);
 int flash_get_protect_block_size(void);
 
-/* Return the physical size of flash in bytes */
+/**
+ * Return the physical size of flash in bytes as read from the flash chip
+ * itself.
+ *
+ * Used during testing to confirm CONFIG_FLASH_PHYSICAL_SIZE is correct.
+ * Most other code should use CONFIG_FLASH_PHYSICAL_SIZE.
+ */
 int flash_physical_size(void);
 
 /**
@@ -45,9 +53,6 @@ static inline char *flash_physical_dataptr(int offset)
 	return (char *)offset;
 }
 
-/* Read <size> bytes of data from offset <offset> into <data>. */
-int flash_physical_read(int offset, int size, char *data);
-
 /* Write <size> bytes of data to flash at byte offset <offset>.
  * <data> must be 32-bit aligned. */
 int flash_physical_write(int offset, int size, const char *data);
@@ -69,11 +74,17 @@ void flash_physical_set_protect(int start_bank, int bank_count);
 /*****************************************************************************/
 /* High-level interface for use by other modules. */
 
-/* Initializes the module. */
+/**
+ * Initialize the module.
+ *
+ * Applies at-boot protection settings if necessary.
+ */
 int flash_pre_init(void);
 
-/* Returns the usable size of flash in bytes.  Note that this is
- * smaller than the actual flash size, */
+/**
+ * Return the usable size of flash in bytes.  Note that this may be smaller
+ * than the physical flash size.
+ */
 int flash_get_size(void);
 
 /**
@@ -97,52 +108,34 @@ int flash_get_size(void);
  */
 int flash_dataptr(int offset, int size_req, int align, char **ptrp);
 
-/* Reads <size> bytes of data from offset <offset> into <data>. */
-int flash_read(int offset, int size, char *data);
-
-/* Writes <size> bytes of data to flash at byte offset <offset>.
- * <data> must be 32-bit aligned. */
-int flash_write(int offset, int size, const char *data);
-
-/* Erases <size> bytes of flash at byte offset <offset>. */
-int flash_erase(int offset, int size);
-
-/* Flash protection APIs
- *
- * Flash can be protected on a per-block basis at any point by calling
- * flash_protect_until_reboot().  Once a block is protected, it will stay
- * protected until reboot.  This function may be called at any time, regardless
- * of the persistent flash protection state, and protection will be applied
- * immediately.
- *
- * Flash may also be protected in a persistent fashion by calling
- * flash_set_protect().  This sets a persistent flag for each block which is
- * checked at boot time and applied if the hardware write protect pin is
- * enabled.
+/**
+ * Write to flash.
  *
- * The flash persistent protection settings are themselves protected by a lock,
- * which can be set via flash_lock_protect().  Once the protection settings are
- * locked:
+ * Offset and size must be a multiple of get_flash_write_block_size().
  *
- *   (1) They will be immediately applied (as if flash_protect_until_reboot()
- *   had been called).
+ * @param offset	Flash offset to write.
+ * @param size	        Number of bytes to write.
+ * @param data          Data to write to flash.
+ */
+int flash_write(int offset, int size, const char *data);
+
+/**
+ * Erase flash.
  *
- *   (2) The persistent settings cannot be changed.  That is, subsequent calls
- *   to flash_set_protect() and flash_lock_protect() will fail.
+ * Offset and size must be a multiple of get_flash_erase_block_size().
  *
- * The lock can be bypassed by cold-booting the system with the hardware write
- * protect pin deasserted.  In this case, the persistent settings and lock
- * state may be changed until flash_lock_protect(non-zero) is called, or until
- * the system is rebooted with the write protect pin asserted - at which point,
- * protection is re-applied. */
+ * @param offset	Flash offset to erase.
+ * @param size	        Number of bytes to erase.
+ */
+int flash_erase(int offset, int size);
 
 /**
  * Protect the entire flash until reboot.
+ *
+ * If the write protect pin is deasserted, this request is ignored.
  */
 int flash_protect_until_reboot(void);
 
-/* Higher-level APIs to emulate SPI write protect */
-
 /**
  * Enable write protect for the read-only code.
  *
@@ -155,7 +148,7 @@ int flash_protect_until_reboot(void);
  */
 int flash_enable_protect(int enable);
 
-/* Flags for flash_get_protect_lock() */
+/* Flags for flash_get_protect() */
 /*
  * Flash protection lock has been set.  Note that if the write protect pin was
  * deasserted at boot time, this simply indicates the state of the lock
@@ -176,7 +169,9 @@ int flash_enable_protect(int enable);
 /* At least one bank of flash which should be protected is not protected */
 #define FLASH_PROTECT_PARTIAL      (1 << 5)
 
-/* Return the flash protect lock status. */
+/**
+ * Return the flash protect lock status.
+ */
 int flash_get_protect(void);
 
 #endif  /* __CROS_EC_FLASH_H */