From 819e9d4560dbb8d0c02dbbcef112703732981ecb Mon Sep 17 00:00:00 2001 From: Jordan Yates Date: Fri, 24 May 2024 10:18:44 +1000 Subject: [PATCH 1/3] flash: spi_nor: remove `SPI_NOR_IDLE_IN_DPD` Remove `SPI_NOR_IDLE_IN_DPD` to simplify the possible transition states in the `spi_nor` driver. This option was originally added 5 years ago when device runtime PM was in a much less mature state. I do not believe having a separate power management implementation for this one in-tree driver is in the interests of the project. The behaviour of `SPI_NOR_IDLE_IN_DPD` leads to extremly suboptimal behaviour in use cases where multiple small reads are performed sequentially, see #69588. Removal of this option does not break the behaviour of any existing applications, only increases current consumption in idle by ~10uA until device runtime PM is enabled. Signed-off-by: Jordan Yates --- drivers/flash/Kconfig.nor | 12 --------- drivers/flash/spi_nor.c | 37 +++------------------------ samples/drivers/spi_flash/sample.yaml | 10 -------- 3 files changed, 4 insertions(+), 55 deletions(-) diff --git a/drivers/flash/Kconfig.nor b/drivers/flash/Kconfig.nor index eed33c8e1807f..4a0275b24076a 100644 --- a/drivers/flash/Kconfig.nor +++ b/drivers/flash/Kconfig.nor @@ -78,16 +78,4 @@ config SPI_NOR_FLASH_LAYOUT_PAGE_SIZE (32768), the sector size (4096), or any non-zero multiple of the sector size. -config SPI_NOR_IDLE_IN_DPD - bool "Use Deep Power-Down mode when flash is not being accessed." - help - Where supported deep power-down mode can reduce current draw - to as little as 0.1% of standby current. However it takes - some milliseconds to enter and exit from this mode. - - Select this option for applications where device power - management is not enabled, the flash remains inactive for - long periods, and when used the impact of waiting for mode - enter and exit delays is acceptable. - endif # SPI_NOR diff --git a/drivers/flash/spi_nor.c b/drivers/flash/spi_nor.c index 88d3ad5c30454..aa0d71bec85f9 100644 --- a/drivers/flash/spi_nor.c +++ b/drivers/flash/spi_nor.c @@ -35,15 +35,10 @@ LOG_MODULE_REGISTER(spi_nor, CONFIG_FLASH_LOG_LEVEL); * * Some devices support a Deep Power-Down mode which reduces current * to as little as 0.1% of standby. * - * The power reduction from DPD is sufficient to warrant allowing its - * use even in cases where Zephyr's device power management is not - * available. This is selected through the SPI_NOR_IDLE_IN_DPD - * Kconfig option. - * * When mapped to the Zephyr Device Power Management states: * * PM_DEVICE_STATE_ACTIVE covers both active and standby modes; - * * PM_DEVICE_STATE_SUSPENDED, and PM_DEVICE_STATE_OFF all correspond to - * deep-power-down mode. + * * PM_DEVICE_STATE_SUSPENDED corresponds to deep-power-down mode; + * * PM_DEVICE_STATE_OFF covers the powered off state; */ #define SPI_NOR_MAX_ADDR_WIDTH 4 @@ -552,11 +547,7 @@ static int exit_dpd(const struct device *const dev) return ret; } -/* Everything necessary to acquire owning access to the device. - * - * This means taking the lock and, if necessary, waking the device - * from deep power-down mode. - */ +/* Everything necessary to acquire owning access to the device. */ static void acquire_device(const struct device *dev) { if (IS_ENABLED(CONFIG_MULTITHREADING)) { @@ -564,23 +555,11 @@ static void acquire_device(const struct device *dev) k_sem_take(&driver_data->sem, K_FOREVER); } - - if (IS_ENABLED(CONFIG_SPI_NOR_IDLE_IN_DPD)) { - exit_dpd(dev); - } } -/* Everything necessary to release access to the device. - * - * This means (optionally) putting the device into deep power-down - * mode, and releasing the lock. - */ +/* Everything necessary to release access to the device. */ static void release_device(const struct device *dev) { - if (IS_ENABLED(CONFIG_SPI_NOR_IDLE_IN_DPD)) { - enter_dpd(dev); - } - if (IS_ENABLED(CONFIG_MULTITHREADING)) { struct spi_nor_data *const driver_data = dev->data; @@ -1439,11 +1418,6 @@ static int spi_nor_pm_control(const struct device *dev, enum pm_device_action ac int rc = 0; switch (action) { -#ifdef CONFIG_SPI_NOR_IDLE_IN_DPD - case PM_DEVICE_ACTION_SUSPEND: - case PM_DEVICE_ACTION_RESUME: - break; -#else case PM_DEVICE_ACTION_SUSPEND: acquire_device(dev); rc = enter_dpd(dev); @@ -1454,11 +1428,9 @@ static int spi_nor_pm_control(const struct device *dev, enum pm_device_action ac rc = exit_dpd(dev); release_device(dev); break; -#endif /* CONFIG_SPI_NOR_IDLE_IN_DPD */ case PM_DEVICE_ACTION_TURN_ON: /* Coming out of power off */ rc = spi_nor_configure(dev); -#ifndef CONFIG_SPI_NOR_IDLE_IN_DPD if (rc == 0) { /* Move to DPD, the correct device state * for PM_DEVICE_STATE_SUSPENDED @@ -1467,7 +1439,6 @@ static int spi_nor_pm_control(const struct device *dev, enum pm_device_action ac rc = enter_dpd(dev); release_device(dev); } -#endif /* CONFIG_SPI_NOR_IDLE_IN_DPD */ break; case PM_DEVICE_ACTION_TURN_OFF: break; diff --git a/samples/drivers/spi_flash/sample.yaml b/samples/drivers/spi_flash/sample.yaml index b7c7a06924386..377c5ccec6ef3 100644 --- a/samples/drivers/spi_flash/sample.yaml +++ b/samples/drivers/spi_flash/sample.yaml @@ -20,13 +20,3 @@ tests: - "Attempting to write 4 bytes" - "Data read matches data written. Good!!" depends_on: spi - sample.drivers.spi.flash_dpd: - tags: - - spi - - flash - filter: dt_compat_enabled("jedec,spi-nor") - platform_exclude: hifive_unmatched - build_only: true - extra_configs: - - CONFIG_SPI_NOR_IDLE_IN_DPD=y - depends_on: spi From 9dc23ee584807f420ffd2d630ba17355a0b1a133 Mon Sep 17 00:00:00 2001 From: Jordan Yates Date: Fri, 24 May 2024 10:36:19 +1000 Subject: [PATCH 2/3] flash: spi_nor: automatically run device runtime PM Automatically handle device runtime PM for all flash API calls. Asynchronously release the device after a small delay to minimise power state transitions under multiple sequential API calls (e.g. NVS). Signed-off-by: Jordan Yates --- doc/releases/migration-guide-4.0.rst | 5 +++ drivers/flash/Kconfig.nor | 11 +++++++ drivers/flash/spi_nor.c | 47 ++++++++++++++++++++++++++++ 3 files changed, 63 insertions(+) diff --git a/doc/releases/migration-guide-4.0.rst b/doc/releases/migration-guide-4.0.rst index 4e5ae29a09d08..95959ec840b38 100644 --- a/doc/releases/migration-guide-4.0.rst +++ b/doc/releases/migration-guide-4.0.rst @@ -113,6 +113,11 @@ Other Subsystems Flash map ========= + * ``CONFIG_SPI_NOR_IDLE_IN_DPD`` has been removed from the :kconfig:option:`CONFIG_SPI_NOR` + driver. An enhanced version of this functionality can be obtained by enabling + :ref:`pm-device-runtime` on the device (Tunable with + :kconfig:option:`CONFIG_SPI_NOR_ACTIVE_DWELL_MS`). + hawkBit ======= diff --git a/drivers/flash/Kconfig.nor b/drivers/flash/Kconfig.nor index 4a0275b24076a..46ea130efbfef 100644 --- a/drivers/flash/Kconfig.nor +++ b/drivers/flash/Kconfig.nor @@ -78,4 +78,15 @@ config SPI_NOR_FLASH_LAYOUT_PAGE_SIZE (32768), the sector size (4096), or any non-zero multiple of the sector size. +config SPI_NOR_ACTIVE_DWELL_MS + int "Dwell period (ms) after last use to stay in active mode" + depends on PM_DEVICE_RUNTIME + default 10 + help + Flash accesses commonly occur in bursts, where entering and exiting DPD + mode between each access adds significantly to the total operation time. + This option controls how long to remain in active mode after each API + call, eliminating the active->idle->active transition sequence if another + transaction occurs before the dwell period expires. + endif # SPI_NOR diff --git a/drivers/flash/spi_nor.c b/drivers/flash/spi_nor.c index aa0d71bec85f9..4f423f00112ea 100644 --- a/drivers/flash/spi_nor.c +++ b/drivers/flash/spi_nor.c @@ -19,6 +19,7 @@ #include #include #include +#include #include "spi_nor.h" #include "jesd216.h" @@ -65,6 +66,12 @@ LOG_MODULE_REGISTER(spi_nor, CONFIG_FLASH_LOG_LEVEL); #define ANY_INST_HAS_WP_GPIOS ANY_INST_HAS_PROP(wp_gpios) #define ANY_INST_HAS_HOLD_GPIOS ANY_INST_HAS_PROP(hold_gpios) +#ifdef CONFIG_SPI_NOR_ACTIVE_DWELL_MS +#define ACTIVE_DWELL_MS CONFIG_SPI_NOR_ACTIVE_DWELL_MS +#else +#define ACTIVE_DWELL_MS 0 +#endif + #define DEV_CFG(_dev_) ((const struct spi_nor_config * const) (_dev_)->config) /* MXICY Related defines*/ @@ -762,11 +769,19 @@ static int spi_nor_read(const struct device *dev, off_t addr, void *dest, return -EINVAL; } + /* Ensure flash is powered before read */ + if (pm_device_runtime_get(dev) < 0) { + return -EIO; + } + acquire_device(dev); ret = spi_nor_cmd_addr_read(dev, SPI_NOR_CMD_READ, addr, dest, size); release_device(dev); + + /* Release flash power requirement */ + (void)pm_device_runtime_put_async(dev, K_MSEC(ACTIVE_DWELL_MS)); return ret; } @@ -779,6 +794,10 @@ static int flash_spi_nor_ex_op(const struct device *dev, uint16_t code, ARG_UNUSED(in); ARG_UNUSED(out); + if (pm_device_runtime_get(dev) < 0) { + return -EIO; + } + acquire_device(dev); switch (code) { @@ -794,6 +813,7 @@ static int flash_spi_nor_ex_op(const struct device *dev, uint16_t code, } release_device(dev); + (void)pm_device_runtime_put_async(dev, K_MSEC(ACTIVE_DWELL_MS)); return ret; } #endif @@ -811,6 +831,11 @@ static int spi_nor_write(const struct device *dev, off_t addr, return -EINVAL; } + /* Ensure flash is powered before write */ + if (pm_device_runtime_get(dev) < 0) { + return -EIO; + } + acquire_device(dev); ret = spi_nor_write_protection_set(dev, false); if (ret == 0) { @@ -858,6 +883,9 @@ static int spi_nor_write(const struct device *dev, off_t addr, } release_device(dev); + + /* Release flash power requirement */ + (void)pm_device_runtime_put_async(dev, K_MSEC(ACTIVE_DWELL_MS)); return ret; } @@ -881,6 +909,11 @@ static int spi_nor_erase(const struct device *dev, off_t addr, size_t size) return -EINVAL; } + /* Ensure flash is powered before erase */ + if (pm_device_runtime_get(dev) < 0) { + return -EIO; + } + acquire_device(dev); ret = spi_nor_write_protection_set(dev, false); @@ -936,6 +969,8 @@ static int spi_nor_erase(const struct device *dev, off_t addr, size_t size) release_device(dev); + /* Release flash power requirement */ + (void)pm_device_runtime_put_async(dev, K_MSEC(ACTIVE_DWELL_MS)); return ret; } @@ -977,12 +1012,18 @@ static int spi_nor_write_protection_set(const struct device *dev, static int spi_nor_sfdp_read(const struct device *dev, off_t addr, void *dest, size_t size) { + if (pm_device_runtime_get(dev) < 0) { + return -EIO; + } + acquire_device(dev); int ret = read_sfdp(dev, addr, dest, size); release_device(dev); + (void)pm_device_runtime_put_async(dev, K_MSEC(ACTIVE_DWELL_MS)); + return ret; } @@ -995,12 +1036,18 @@ static int spi_nor_read_jedec_id(const struct device *dev, return -EINVAL; } + if (pm_device_runtime_get(dev) < 0) { + return -EIO; + } + acquire_device(dev); int ret = spi_nor_cmd_read(dev, SPI_NOR_CMD_RDID, id, SPI_NOR_MAX_ID_LEN); release_device(dev); + (void)pm_device_runtime_put_async(dev, K_MSEC(ACTIVE_DWELL_MS)); + return ret; } From 6b7a85252a1e7fa446231a7ba058b30b5a47e8ae Mon Sep 17 00:00:00 2001 From: Jordan Yates Date: Sat, 25 May 2024 21:00:16 +1000 Subject: [PATCH 3/3] flash: spi_nor: handle SPI bus power Automatically request and release the SPI bus before talking to the flash chip. Signed-off-by: Jordan Yates --- drivers/flash/spi_nor.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/flash/spi_nor.c b/drivers/flash/spi_nor.c index 4f423f00112ea..236955887ab1f 100644 --- a/drivers/flash/spi_nor.c +++ b/drivers/flash/spi_nor.c @@ -557,16 +557,24 @@ static int exit_dpd(const struct device *const dev) /* Everything necessary to acquire owning access to the device. */ static void acquire_device(const struct device *dev) { + const struct spi_nor_config *cfg = dev->config; + if (IS_ENABLED(CONFIG_MULTITHREADING)) { struct spi_nor_data *const driver_data = dev->data; k_sem_take(&driver_data->sem, K_FOREVER); } + + (void)pm_device_runtime_get(cfg->spi.bus); } /* Everything necessary to release access to the device. */ static void release_device(const struct device *dev) { + const struct spi_nor_config *cfg = dev->config; + + (void)pm_device_runtime_put(cfg->spi.bus); + if (IS_ENABLED(CONFIG_MULTITHREADING)) { struct spi_nor_data *const driver_data = dev->data;