This post is older than 2 years and might not be relevant anymore
More Info: Consider searching for newer posts

QSPI high current improvements for anomaly 122 in nrf52840

Hi,

I am trying to improve the nrf QSPI driver on Zephyr to handle anomaly 122 on nrf52840. Our application needs access to external flash as also as other modules that we use, like openthread, so putting uninit and init on flash calls everywhere is not desired.

I'm basing myself on this first post from Simon indicating to uninitialize the QSPI in order to get low currents and on this post which gets to the same conclusion I've had of the requirement of setting the CS high after uninit the QSPI.

As the access to the flash QSPI is done from different places, I think it would be appropriate to handle the improvements at a lower level, like at the QSPI driver. I thought of creating a Kconfig flag related to the change and encapsulate the APIs functions with calls to nrfx_qspi_init() and nrfx_qspi_uninit() before and after the APIs, respectively, besides setting the CS pin as output high. It would be something like that:

static int qspi_nor_read_pan_122(const struct device *dev, off_t addr, void *dest,
			 size_t size)
{
    ... = nrfx_qspi_init();

    qspi_nor_read(dev, addr, dest, size);
    
    nrfx_qspi_uninit();

    nrf_gpio_cfg_output(QSPI_PROP_AT(csn_pins, 0));
    nrf_gpio_pin_set(QSPI_PROP_AT(csn_pins, 0));
}

#if defined(CONFIG_NORDIC_QSPI_NOR_PAN_122)
static const struct flash_driver_api qspi_nor_api = {
	.read = qspi_nor_read_pan_122,
	.write = ...
};
#else
static const struct flash_driver_api qspi_nor_api = {
	.read = qspi_nor_read,
	.write = ...
};

The API functions seems to be stateless. The write_protection() needs to preserve a state, but that stays on the flash (nothing on the nrf core or on variables), and an unit and init after it would not change it from what I could see.

Another thing is that the QSPI init is not protected against race conditions as it is used only by the kernel at POST_KERNEL. Would it be required to protect init and uninit with qspi_lock() in the encapsulations above?

Is there anything else dangerous with that approach? Is there another way that wouldn't require to initialize and uninitialize the QSPI in every access to the external flash?

I would appreciate any feedback.

Best,

Parents
  • Hi,

    I think this looks like a good idea, and as far as I can this also looks good. I would likely recommend that the uninit sequence is:

    1. set cs high using nrf_gpio // when qspi uninit the pins will fall back to this configuration, so you might as well do it here to avoid an extra cs toggle after qspi uninit
    2. write the magic lines of code presented in errata 122
    3. qspi uninit

    Kenneth

  • Thank you Kenneth! The first tip was nice! The uninit function provided by the Nordic HAL module already implements the lines of code of errata 122.

    I've implemented the draft and I'm able to use the external flash with QSPI, with low currents (bellow 15uA, instead of previous 640uA).

    Now, as I've struggled to find the solution and I imagine it can help others, I'll see if Zephyr maintainers could take a look at it in an issue or PR.

    I'll be back here if I'm successful in the PR.

    Thanks again Kenneth!

Reply
  • Thank you Kenneth! The first tip was nice! The uninit function provided by the Nordic HAL module already implements the lines of code of errata 122.

    I've implemented the draft and I'm able to use the external flash with QSPI, with low currents (bellow 15uA, instead of previous 640uA).

    Now, as I've struggled to find the solution and I imagine it can help others, I'll see if Zephyr maintainers could take a look at it in an issue or PR.

    I'll be back here if I'm successful in the PR.

    Thanks again Kenneth!

Children
Related