zephyr: flash: clean up the shim layer

Investigate if there is a possibility to call Zephyr flash drivers
directly in the shim layer, not via CrosEC drivers. Unfortunately, it
is possible only for the read operation. Other operations require
additional actions depending on the chip type e.g. splitting into
smaller parts or refreshing watchdog.

Also, move locking of physical flash operations to the CrosEC drivers
from the shim layer.

BUG=b:205175314
TEST=zmake testall && make sure SoftwareSync works to verify flash
operations
BRANCH=main
Signed-off-by: Dawid Niedzwiecki <dn@semihalf.com>

Change-Id: I9c947d46244a255573ebde9a5cd7432a3ee9389c
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/3389268
Tested-by: Dawid Niedźwiecki <dn@semihalf.com>
Reviewed-by: Keith Short <keithshort@chromium.org>
Commit-Queue: Keith Short <keithshort@chromium.org>

10 files changed, 62 insertions, 68 deletions

diff --git a/zephyr/boards/arm/brya/brya.dts b/zephyr/boards/arm/brya/brya.dts
index a37b8f6650..de72be7ab4 100644
--- a/zephyr/boards/arm/brya/brya.dts
+++ b/zephyr/boards/arm/brya/brya.dts
@@ -18,6 +18,7 @@
 		zephyr,console = &uart1;
 		zephyr,shell-uart = &uart1;
 		zephyr,flash = &flash0;
+		zephyr,flash-controller = &int_flash;
 		cros,rtc = &mtc;
 	};
 
diff --git a/zephyr/boards/arm/herobrine_npcx9/herobrine_npcx9.dts b/zephyr/boards/arm/herobrine_npcx9/herobrine_npcx9.dts
index 6fb5fd8384..62064f52f0 100644
--- a/zephyr/boards/arm/herobrine_npcx9/herobrine_npcx9.dts
+++ b/zephyr/boards/arm/herobrine_npcx9/herobrine_npcx9.dts
@@ -19,6 +19,7 @@
 		zephyr,console = &uart1;
 		zephyr,shell-uart = &uart1;
 		zephyr,flash = &flash0;
+		zephyr,flash-controller = &int_flash;
 		cros,rtc = &pcf85063a;
 	};
 
diff --git a/zephyr/boards/arm/npcx9/npcx9.dts b/zephyr/boards/arm/npcx9/npcx9.dts
index 143cdf8926..171e0f12e0 100644
--- a/zephyr/boards/arm/npcx9/npcx9.dts
+++ b/zephyr/boards/arm/npcx9/npcx9.dts
@@ -26,6 +26,7 @@
 		zephyr,console = &uart1;
 		zephyr,shell-uart = &uart1;
 		zephyr,flash = &flash0;
+		zephyr,flash-controller = &int_flash;
 	};
 
 	named-i2c-ports {
diff --git a/zephyr/boards/arm/npcx_evb/npcx_evb.dtsi b/zephyr/boards/arm/npcx_evb/npcx_evb.dtsi
index af5d6218bb..23f0e06345 100644
--- a/zephyr/boards/arm/npcx_evb/npcx_evb.dtsi
+++ b/zephyr/boards/arm/npcx_evb/npcx_evb.dtsi
@@ -13,6 +13,7 @@
 		zephyr,console = &uart1;
 		zephyr,shell-uart = &uart1;
 		zephyr,flash = &flash0;
+		zephyr,flash-controller = &int_flash;
 		cros,rtc = &mtc;
 	};
 
diff --git a/zephyr/boards/arm/trogdor/trogdor.dts b/zephyr/boards/arm/trogdor/trogdor.dts
index d930907cf8..b6e3bb3b7b 100644
--- a/zephyr/boards/arm/trogdor/trogdor.dts
+++ b/zephyr/boards/arm/trogdor/trogdor.dts
@@ -28,6 +28,7 @@
 		zephyr,console = &uart1;
 		zephyr,shell-uart = &uart1;
 		zephyr,flash = &flash0;
+		zephyr,flash-controller = &int_flash;
 		cros,rtc = &mtc;
 	};
 
diff --git a/zephyr/boards/arm/volteer/volteer.dts b/zephyr/boards/arm/volteer/volteer.dts
index cbc8227760..227e940721 100644
--- a/zephyr/boards/arm/volteer/volteer.dts
+++ b/zephyr/boards/arm/volteer/volteer.dts
@@ -30,6 +30,7 @@
 		zephyr,console = &uart1;
 		zephyr,shell-uart = &uart1;
 		zephyr,flash = &flash0;
+		zephyr,flash-controller = &int_flash;
 		cros,rtc = &mtc;
 	};
 
diff --git a/zephyr/drivers/cros_flash/cros_flash_it8xxx2.c b/zephyr/drivers/cros_flash/cros_flash_it8xxx2.c
index 0b1899feda..01e5c1f721 100644
--- a/zephyr/drivers/cros_flash/cros_flash_it8xxx2.c
+++ b/zephyr/drivers/cros_flash/cros_flash_it8xxx2.c
@@ -161,14 +161,6 @@ static int cros_flash_it8xxx2_init(const struct device *dev)
 	return EC_ERROR_UNKNOWN;
 }
 
-static int cros_flash_it8xxx2_read(const struct device *dev, int offset,
-				   int size, char *dst_data)
-{
-	ARG_UNUSED(dev);
-
-	return flash_read(flash_controller, offset, dst_data, size);
-}
-
 static int cros_flash_it8xxx2_write(const struct device *dev, int offset,
 				    int size, const char *src_data)
 {
@@ -297,7 +289,6 @@ static int cros_flash_it8xxx2_protect_now(const struct device *dev, int all)
 /* cros ec flash driver registration */
 static const struct cros_flash_driver_api cros_flash_it8xxx2_driver_api = {
 	.init = cros_flash_it8xxx2_init,
-	.physical_read = cros_flash_it8xxx2_read,
 	.physical_write = cros_flash_it8xxx2_write,
 	.physical_erase = cros_flash_it8xxx2_erase,
 	.physical_get_protect = cros_flash_it8xxx2_get_protect,
diff --git a/zephyr/drivers/cros_flash/cros_flash_npcx.c b/zephyr/drivers/cros_flash/cros_flash_npcx.c
index 4c6a6e5e3c..d6ecd950e1 100644
--- a/zephyr/drivers/cros_flash/cros_flash_npcx.c
+++ b/zephyr/drivers/cros_flash/cros_flash_npcx.c
@@ -34,7 +34,7 @@ struct cros_flash_npcx_data {
 
 static struct spi_config spi_cfg;
 
-#define FLASH_DEV DT_NODELABEL(int_flash)
+#define FLASH_DEV DT_CHOSEN(zephyr_flash_controller)
 #define SPI_CONTROLLER_DEV DT_NODELABEL(spi_fiu0)
 
 #define DRV_DATA(dev) ((struct cros_flash_npcx_data *)(dev)->data)
@@ -435,15 +435,6 @@ static int cros_flash_npcx_init(const struct device *dev)
 	return 0;
 }
 
-/* TODO(b/205175314): Migrate cros-flash driver to Zephyr flash driver) */
-static int cros_flash_npcx_read(const struct device *dev, int offset, int size,
-				char *dst_data)
-{
-	struct cros_flash_npcx_data *data = DRV_DATA(dev);
-
-	return flash_read(data->flash_dev, offset, dst_data, size);
-}
-
 static int cros_flash_npcx_write(const struct device *dev, int offset, int size,
 				 const char *src_data)
 {
@@ -463,8 +454,14 @@ static int cros_flash_npcx_write(const struct device *dev, int offset, int size,
 		return -EINVAL;
 	}
 
+	/* Lock physical flash operations */
+	crec_flash_lock_mapped_storage(1);
+
 	ret = flash_write(data->flash_dev, offset, src_data, size);
 
+	/* Unlock physical flash operations */
+	crec_flash_lock_mapped_storage(0);
+
 	return ret;
 }
 
@@ -491,8 +488,14 @@ static int cros_flash_npcx_erase(const struct device *dev, int offset, int size)
 		return -EINVAL;
 	}
 
+	/* Lock physical flash operations */
+	crec_flash_lock_mapped_storage(1);
+
 	ret = flash_erase(data->flash_dev, offset, size);
 
+	/* Unlock physical flash operations */
+	crec_flash_lock_mapped_storage(0);
+
 	return ret;
 }
 
@@ -609,7 +612,6 @@ static int cros_flash_npcx_get_status(const struct device *dev, uint8_t *sr1,
 /* cros ec flash driver registration */
 static const struct cros_flash_driver_api cros_flash_npcx_driver_api = {
 	.init = cros_flash_npcx_init,
-	.physical_read = cros_flash_npcx_read,
 	.physical_write = cros_flash_npcx_write,
 	.physical_erase = cros_flash_npcx_erase,
 	.physical_get_protect = cros_flash_npcx_get_protect,
diff --git a/zephyr/include/drivers/cros_flash.h b/zephyr/include/drivers/cros_flash.h
index cb519a0069..d78f5de2de 100644
--- a/zephyr/include/drivers/cros_flash.h
+++ b/zephyr/include/drivers/cros_flash.h
@@ -35,9 +35,6 @@
  */
 typedef int (*cros_flash_api_init)(const struct device *dev);
 
-typedef int (*cros_flash_api_physical_read)(const struct device *dev,
-					    int offset, int size, char *data);
-
 typedef int (*cros_flash_api_physical_write)(const struct device *dev,
 					     int offset, int size,
 					     const char *data);
@@ -67,7 +64,6 @@ typedef int (*cros_flash_api_physical_get_status)(const struct device *dev,
 
 __subsystem struct cros_flash_driver_api {
 	cros_flash_api_init init;
-	cros_flash_api_physical_read physical_read;
 	cros_flash_api_physical_write physical_write;
 	cros_flash_api_physical_erase physical_erase;
 	cros_flash_api_physical_get_protect physical_get_protect;
@@ -105,34 +101,6 @@ static inline int z_impl_cros_flash_init(const struct device *dev)
 }
 
 /**
- * @brief Read from physical flash.
- *
- * @param dev Pointer to the device structure for the flash driver instance.
- * @param offset Flash offset to read.
- * @param size Number of bytes to read.
- * @param data Destination buffer for data.  Must be 32-bit aligned.
- *
- * @return 0 If successful.
- * @retval -ENOTSUP Not supported api function.
- */
-__syscall int cros_flash_physical_read(const struct device *dev, int offset,
-				       int size, char *data);
-
-static inline int z_impl_cros_flash_physical_read(const struct device *dev,
-						  int offset, int size,
-						  char *data)
-{
-	const struct cros_flash_driver_api *api =
-		(const struct cros_flash_driver_api *)dev->api;
-
-	if (!api->physical_read) {
-		return -ENOTSUP;
-	}
-
-	return api->physical_read(dev, offset, size, data);
-}
-
-/**
  * @brief Write to physical flash.
  *
  * Offset and size must be a multiple of CONFIG_FLASH_WRITE_SIZE.
diff --git a/zephyr/shim/src/flash.c b/zephyr/shim/src/flash.c
index d82e860e17..ddd1b0c8ef 100644
--- a/zephyr/shim/src/flash.c
+++ b/zephyr/shim/src/flash.c
@@ -6,6 +6,7 @@
 #include <flash.h>
 #include <kernel.h>
 #include <logging/log.h>
+#include <drivers/flash.h>
 
 #include "console.h"
 #include "drivers/cros_flash.h"
@@ -20,10 +21,14 @@ LOG_MODULE_REGISTER(shim_flash, LOG_LEVEL_ERR);
 #else
 #define cros_flash_dev DEVICE_DT_GET(DT_CHOSEN(cros_ec_flash))
 #endif
+#if !DT_HAS_CHOSEN(zephyr_flash_controller)
+#error "zephyr,flash-controller device must be chosen"
+#else
+#define flash_ctrl_dev DEVICE_DT_GET(DT_CHOSEN(zephyr_flash_controller))
+#endif
 
 K_MUTEX_DEFINE(flash_lock);
 
-/* TODO(b/174873770): Add calls to Zephyr code here */
 #ifdef CONFIG_EXTERNAL_STORAGE
 void crec_flash_lock_mapped_storage(int lock)
 {
@@ -43,14 +48,13 @@ int crec_flash_physical_write(int offset, int size, const char *data)
 	    (CONFIG_FLASH_WRITE_SIZE - 1))
 		return EC_ERROR_INVAL;
 
-	/* Lock physical flash operations */
-	crec_flash_lock_mapped_storage(1);
-
+	/*
+	 * We need to call cros_flash driver because the procedure
+	 * may differ depending on the chip type e.g. ite chips need to
+	 * call watchdog_reload before calling the Zephyr flash driver.
+	 */
 	rv = cros_flash_physical_write(cros_flash_dev, offset, size, data);
 
-	/* Unlock physical flash operations */
-	crec_flash_lock_mapped_storage(0);
-
 	return rv;
 }
 
@@ -58,34 +62,52 @@ int crec_flash_physical_erase(int offset, int size)
 {
 	int rv;
 
-	/* Lock physical flash operations */
-	crec_flash_lock_mapped_storage(1);
-
+	/*
+	 * We need to call cros_flash driver because the procedure
+	 * may differ depending on the chip type e.g. ite chips need to
+	 * split a large erase operation and reload watchdog, otherwise
+	 * EC reboot happens
+	 */
 	rv = cros_flash_physical_erase(cros_flash_dev, offset, size);
 
-	/* Unlock physical flash operations */
-	crec_flash_lock_mapped_storage(0);
-
 	return rv;
 }
 
 int crec_flash_physical_get_protect(int bank)
 {
+	/*
+	 * We need to call cros_flash driver because Zephyr flash API
+	 * doesn't support reading protected areas and the procedure is
+	 * different for each flash type.
+	 */
 	return cros_flash_physical_get_protect(cros_flash_dev, bank);
 }
 
 uint32_t crec_flash_physical_get_protect_flags(void)
 {
+	/*
+	 * We need to call cros_flash driver because Zephyr flash API
+	 * doesn't support reading protected areas and the procedure is
+	 * different for each flash type.
+	 */
 	return cros_flash_physical_get_protect_flags(cros_flash_dev);
 }
 
 int crec_flash_physical_protect_at_boot(uint32_t new_flags)
 {
+	/*
+	 * It is EC specific, so it needs to be implemented in cros_flash driver
+	 * per chip.
+	 */
 	return cros_flash_physical_protect_at_boot(cros_flash_dev, new_flags);
 }
 
 int crec_flash_physical_protect_now(int all)
 {
+	/*
+	 * It is EC specific, so it needs to be implemented in cros_flash driver
+	 * per chip.
+	 */
 	return cros_flash_physical_protect_now(cros_flash_dev, all);
 }
 
@@ -93,9 +115,13 @@ int crec_flash_physical_read(int offset, int size, char *data)
 {
 	int rv;
 
-	/* Lock physical flash operations */
+	/*
+	 * Lock the physical flash operation here because, we call the Zephyr
+	 * driver directly.
+	 */
 	crec_flash_lock_mapped_storage(1);
-	rv = cros_flash_physical_read(cros_flash_dev, offset, size, data);
+
+	rv = flash_read(flash_ctrl_dev, offset, data, size);
 
 	/* Unlock physical flash operations */
 	crec_flash_lock_mapped_storage(0);
@@ -107,7 +133,8 @@ static int flash_dev_init(const struct device *unused)
 {
 	ARG_UNUSED(unused);
 
-	if (!device_is_ready(cros_flash_dev))
+	if (!device_is_ready(cros_flash_dev) ||
+	    !device_is_ready(flash_ctrl_dev))
 		k_oops();
 	cros_flash_init(cros_flash_dev);