npcx: Decide erase size based on ro/rw region size

This change updates the erase operation in npcx chip to use 64k/32k/4k
block erase depending upon the alignment of CONFIG_RO_SIZE. This helps
reduce the EC SW sync time from ~9.5 seconds to ~5.4 seconds on NPCX7.

Ideally, we would want to check the offset and size of region to be
erased dynamically and decide which erase operation to use. However,
common flash code checks against CONFIG_FLASH_ERASE_SIZE to ensure
that the area being erased is aligned to that size. Thus, even if we
add dynamic erase at chip level, it isn't going to help.

This change also updates CONFIG_FLASH_BANK_SIZE to be the same as
CONFIG_FLASH_ERASE_SIZE since it is checked by common code. I am
honestly not sure why the CONFIG_FLASH_BANK_SIZE is tightly coupled
with CONFIG_FLASH_ERASE_SIZE. But, based on the usage, it seems to be
a safe change.

On the other hand, changing CONFIG_FLASH_BANK_SIZE helps reduce the
write time as well, thus overall helping with the EC SW Sync time.

Please see go/cros-npcx7-ec-sw-sync for more details.

BUG=b:113530328
BRANCH=nocturne
TEST=Verified that EC SW sync time goes down from 9.5 seconds to 5.4
seconds.

Change-Id: I5908eeeb3e4207a27abe804db8eb9d39ef9d73c4
Signed-off-by: Furquan Shaikh <furquan@google.com>
Reviewed-on: https://chromium-review.googlesource.com/1195598
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Furquan Shaikh <furquan@chromium.org>
Reviewed-by: Aaron Durbin <adurbin@chromium.org>
Reviewed-by: Jett Rink <jettrink@chromium.org>

5 files changed, 41 insertions, 10 deletions

diff --git a/chip/npcx/config_chip.h b/chip/npcx/config_chip.h
index e956e4cf6a..494f76c352 100644
--- a/chip/npcx/config_chip.h
+++ b/chip/npcx/config_chip.h
@@ -53,13 +53,6 @@
 /* Address of RAM log used by Booter */
 #define ADDR_BOOT_RAMLOG        0x100C7FC0
 
-/* SPI Flash Spec of W25Q20CV */
-#define CONFIG_FLASH_BANK_SIZE	0x00001000  /* protect bank size 4K bytes */
-#define CONFIG_FLASH_ERASE_SIZE	0x00001000  /* sector erase size 4K bytes */
-#define CONFIG_FLASH_WRITE_SIZE	0x00000001  /* minimum write size */
-
-#define CONFIG_FLASH_WRITE_IDEAL_SIZE 256   /* one page size for write */
-
 #include "config_flash_layout.h"
 
 /* Optional features present on this chip */
diff --git a/chip/npcx/config_flash_layout.h b/chip/npcx/config_flash_layout.h
index e517ce4398..8413453167 100644
--- a/chip/npcx/config_flash_layout.h
+++ b/chip/npcx/config_flash_layout.h
@@ -64,6 +64,42 @@
 #define CONFIG_RW_MEM_OFF	CONFIG_RO_MEM_OFF
 #define CONFIG_RW_SIZE		CONFIG_RO_SIZE
 
+#if (CONFIG_RO_SIZE != CONFIG_RW_SIZE)
+#error "Unsupported.. FLASH_ERASE_SIZE assumes RO and RW size is same!"
+#endif
+
+#if (CONFIG_RO_MEM_OFF != 0)
+#error "Unsupported.. CONFIG_RO_MEM_OFF is assumed to be 0!"
+#endif
+
+/*
+ * CONFIG_FLASH_ERASE_SIZE is set to maximum possible out of 64k, 32k and 4k
+ * depending upon alignment of CONFIG_RO_SIZE. There are two assumptions here:
+ * 1. CONFIG_RO_MEM_OFF is always 0 i.e. RO starts at 0.
+ * 2. CONFIG_RO_SIZE and CONFIG_RW_SIZE are the same.
+ *
+ * If above assumptions are not true, then additional checks would be required
+ * to ensure that erase block size is selected based on the alignment of both
+ * CONFIG_RO_SIZE and CONFIG_RW_SIZE and the offset of RO.
+ */
+#if ((CONFIG_RO_SIZE & (0x10000 - 1)) == 0)
+#define CONFIG_FLASH_ERASE_SIZE	0x10000
+#define NPCX_ERASE_COMMAND		CMD_BLOCK_64K_ERASE
+#elif ((CONFIG_RO_SIZE & (0x8000 - 1)) == 0)
+#define CONFIG_FLASH_ERASE_SIZE	0x8000
+#define NPCX_ERASE_COMMAND		CMD_BLOCK_32K_ERASE
+#else
+#define CONFIG_FLASH_ERASE_SIZE	0x1000
+#define NPCX_ERASE_COMMAND		CMD_SECTOR_ERASE
+#endif
+
+#define CONFIG_FLASH_BANK_SIZE		CONFIG_FLASH_ERASE_SIZE
+#define CONFIG_FLASH_WRITE_SIZE		0x1  /* minimum write size */
+#define CONFIG_FLASH_WRITE_IDEAL_SIZE	256   /* one page size for write */
+
+/* Use 4k sector erase for NPCX monitor flash erase operations. */
+#define NPCX_MONITOR_FLASH_ERASE_SIZE	0x1000
+
 /* RO image resides at start of protected region, right after header */
 #define CONFIG_RO_STORAGE_OFF	CONFIG_RO_HDR_SIZE
 /* RW image resides at start of writable region */
diff --git a/chip/npcx/flash.c b/chip/npcx/flash.c
index 2106e68f75..f1c4ce630e 100644
--- a/chip/npcx/flash.c
+++ b/chip/npcx/flash.c
@@ -559,7 +559,7 @@ int flash_physical_erase(int offset, int size)
 		/* Set erase address */
 		flash_set_address(offset);
 		/* Start erase */
-		flash_execute_cmd(CMD_SECTOR_ERASE, MASK_CMD_ADR);
+		flash_execute_cmd(NPCX_ERASE_COMMAND, MASK_CMD_ADR);
 
 		/* Wait erase completed */
 		rv = flash_wait_ready(FLASH_ABORT_TIMEOUT);
diff --git a/chip/npcx/registers.h b/chip/npcx/registers.h
index 01da6f658f..48b6b21c9b 100644
--- a/chip/npcx/registers.h
+++ b/chip/npcx/registers.h
@@ -1842,6 +1842,8 @@ enum {
 #define CMD_WRITE_STATUS_REG             0x01
 #define CMD_FLASH_PROGRAM                0x02
 #define CMD_SECTOR_ERASE                 0x20
+#define CMD_BLOCK_32K_ERASE              0x52
+#define CMD_BLOCK_64K_ERASE              0xd8
 #define CMD_PROGRAM_UINT_SIZE            0x08
 #define CMD_PAGE_SIZE                    0x00
 #define CMD_READ_ID_TYPE                 0x47
diff --git a/chip/npcx/spiflashfw/npcx_monitor.c b/chip/npcx/spiflashfw/npcx_monitor.c
index ed2d2bbd5b..a823abdfc7 100644
--- a/chip/npcx/spiflashfw/npcx_monitor.c
+++ b/chip/npcx/spiflashfw/npcx_monitor.c
@@ -196,8 +196,8 @@ void sspi_flash_physical_erase(int offset, int size)
 	sspi_flash_tristate(0);
 
 	/* Alignment has been checked in upper layer */
-	for (; size > 0; size -= CONFIG_FLASH_ERASE_SIZE,
-		offset += CONFIG_FLASH_ERASE_SIZE) {
+	for (; size > 0; size -= NPCX_MONITOR_FLASH_ERASE_SIZE,
+		offset += NPCX_MONITOR_FLASH_ERASE_SIZE) {
 		/* Enable write */
 		sspi_flash_write_enable();
 		/* Set erase address */