drivers: flash: spi_nor: support device PM #59647
Conversation
Support device power management in spi_nor driver. Only use SUSPEND/RESUME if `CONFIG_SPI_NOR_IDLE_IN_DPD` is not enabled to avoid state conflicts. Signed-off-by: Jordan Yates <jordan.yates@data61.csiro.au>
Exit DPD on init in all cases, even when `CONFIG_SPI_NOR_IDLE_IN_DPD` is not enabled. Signed-off-by: Jordan Yates <jordan.yates@data61.csiro.au>
JordanYates
force-pushed
the
230623_spi_nor
branch
from
b29d3a0
to
aec3995
Compare
|
@JordanYates, this is a very nice addition to spi_nor. I'm wondering if this needs no protection to avoid switching of during ongoing writes/reads. |
| acquire_device(dev); | ||
| rc = enter_dpd(dev); | ||
| release_device(dev); |
There was a problem hiding this comment.
In case when CONFIG_SPI_NOR_IDLE_IN_DPD is enabled, this wakes up device in acquire_device then puts it to sleep twice.
There was a problem hiding this comment.
If that is enabled this code isn't run, see the enclosing #ifdef. Same for the second location
There was a problem hiding this comment.
Ah sorry, somehow missed that.
Shouldn't this check k_sem_count_get before putting device to sleep? Because there may be something scheduled just after PM event, and it will wake up device anyway.
In case this locks on acquire_device, is it possible that more than one PM event get scheduled?
There was a problem hiding this comment.
Checking the semaphore isn't required because it is only taken for the duration of an API call (if it is held longer that is a bug), and therefore our own call to acquire_device ensures that no other action is running.
The higher level "Device PM" API is responsible for ensuring that PM events are only run in the correct state, and "Device Runtime PM" is the correct API to use if you want multiple users for the driver with PM.
Nevermind, I thought that |
Support device power management in spi_nor driver. Only use
SUSPEND/RESUMEifCONFIG_SPI_NOR_IDLE_IN_DPDis not enabled to avoid state conflicts.