drivers: spi: nrfx_spim: Add support for device runtime PM

[adamkondraciuk]

Enable the device runtime power management on the SPIM shim.

[rda-emcraft]

This code works incorrectly in the case of SPI_HOLD_ON_CS or/and SPI_LOCK_ON.

Here's what happens when SPI transactions are split into two calls of spi_transceive()

[00:24:15.862,182] <wrn> pm_device: Unbalanced suspend
[00:24:15.863,128] <wrn> pm_device: Unbalanced suspend
[00:24:15.966,217] <wrn> pm_device: Unbalanced suspend
[00:24:15.966,857] <wrn> pm_device: Unbalanced suspend
[00:24:16.816,741] <wrn> pm_device: Unbalanced suspend

And I've got lucky because switching to power down does not break the transaction. Probably idle pin states between the bytes match power down states.

It does not make sense to call pm_device_runtime_put(dev) if SPI lock is not released or if CS hold is requested.
Similarly it does not make sense to call pm_device_runtime_get(dev) if SPI lock or CS is held.

By the way, all zephyr SPI drivers with device runtime support I've seen does not handle this flags. Probably generic solution will be to integrate device runtime calls into spi_context.h.

[nordic-piks]

This code works incorrectly in the case of SPI_HOLD_ON_CS or/and SPI_LOCK_ON.

Here's what happens when SPI transactions are split into two calls of spi_transceive()

[00:24:15.862,182] <wrn> pm_device: Unbalanced suspend
[00:24:15.863,128] <wrn> pm_device: Unbalanced suspend
[00:24:15.966,217] <wrn> pm_device: Unbalanced suspend
[00:24:15.966,857] <wrn> pm_device: Unbalanced suspend
[00:24:16.816,741] <wrn> pm_device: Unbalanced suspend

And I've got lucky because switching to power down does not break the transaction. Probably idle pin states between the bytes match power down states.

It does not make sense to call pm_device_runtime_put(dev) if SPI lock is not released or if CS hold is requested. Similarly it does not make sense to call pm_device_runtime_get(dev) if SPI lock or CS is held.

By the way, all zephyr SPI drivers with device runtime support I've seen does not handle this flags. Probably generic solution will be to integrate device runtime calls into spi_context.h.

@rda-emcraft Can you rise an issue with this please? That will make easier to implement this.

[nordic-piks]

Hm, maybe issue is not needed as this is PR only.

[rda-emcraft]

@rda-emcraft Can you rise an issue with this please? That will make easier to implement this.

@nordic-piks, @adamkondraciuk
I may raise issue for spi_ambiq.c, spi_ll_stm32.c and spi_smartbond.c that already accepted into the tree and has the same deficiency.

I have read more of spi_context.h and now I think you should call pm_device_runtime_get(dev) along with locking ctx->lock and pm_device_runtime_put(dev) when unlocking ctx->lock. Implementing this might require passing device pointer to spi_context_{lock,release,complete,release,unlock_unconditionally}(). Probably will have this implemented later today.

What I'm trying to say is that part of runtime device PM (_get() and put()) for SPI drivers can be implemented in generic SPI context handling code. Driver only need to supply initialization and code to do real PM operations.

Anyway, I'm really new in Zephyr, so any guidance is appreciated.

And I'm sitting on Zephyr 3.5.99 and calling pm_device_runtime_put(dev) from IRQ context is a no go here. I'm replacing it with pm_device_runtime_put_async(dev).

[rda-emcraft]

@adamkondraciuk

I've created pull request to your branch implementing proposed changes.