nrf_blk_dev_read_req is called from critical region in the SDK

[joel-sc]

Hello,

I'm using SDK nrf5_17.0.2 on nrf52840

I'm implementing a custom nrf block device for a spi nand flash, and I'm getting trouble with the calling site of the nrf_blk_dev_read_req call from the usb msc (app_usbd_msc).

nrf_blk_dev_read_req could be called either by app_usbd_msc.c::cmd_read_start or by app_usbd_msc.c:msc::msc_blockdev_ev_handler as shown in the following screenshot:

The call stack from the cmd_read_start is "the normal path" and this path works. The callstack from the msc_blockdev_ev_handler comes from my block device where I have to notify the usb msc stack the status of the block device request (for the example at the end of the read request):

const nrf_block_dev_event_t ev = {
        .ev_type = NRF_BLOCK_DEV_EVT_BLK_READ_DONE,
        .result = NRF_BLOCK_DEV_RESULT_SUCCESS,
        .p_context = p_work->p_context,
        .p_blk_req = p_blk,
};

p_work->ev_handler(p_blk_dev, &ev);

The problem is in the ev_handler (pointing to msc_blockdev_ev_handler) implementation where the _BLK_READ_DONE command is handled like the following:

 case NRF_BLOCK_DEV_EVT_BLK_READ_DONE:
    CRITICAL_REGION_ENTER();
    msc_blockdev_read_done_handler(p_blk_dev, p_event);
    CRITICAL_REGION_EXIT();
    break;

msc_blockdev_read_done_handler is called in a critical region. So following the call stack from this point leads to app_usbd_msc.c::read_block_mem_processor:L950 where nrf_blk_dev_read_req is called if there is still some bytes to process.

The problem is that I currently reading page size (4096 bytes) in the external memory by SPI in a asynchronous way so I'm using SPIM with EasyDMA. And the SPI irq is never called because NVIC->ICER has been clear in CRITICAL_REGION_ENTER.

Why msc_blockdev_read_done_handler has to be called in a critical region ? How can I fix this issue ?

Best

Joël

[Simonr]

Hi Joël

The USB MSC implementation in the nRF5 SDK uses the nrf_blk (block device) library underneath to manage Flash Memory. Therefore it won't work with custom implementations of the block device without a heavy rework of the MSC class, that would basically end up as another implementation of the Block device. When USB MSC is used, FatFS (or other file system) is managed by the OS that communicates with the device via USB. The device will just receive commands on block operations to perform. nrf_blk was created to ease this process. 

Our suggestion would be to focus on making your Flash memory work with the existing implementation of nrf_blk, so it can work as USB mass storage seamlessly.

Best regards,

Simon

[joel-sc]

Hello Simon,

Thanks for your reply.

Simonr said:

Our suggestion would be to focus on making your Flash memory work with the existing implementation of nrf_blk, so it can work as USB mass storage seamlessly.

That's more or less what I'm doing. However I need to implement my block device (which implements the nrf_block_dev interface). I need to do that because the current nrf_block_dev_qspi is not compatible with nand flash device. So I've created a block_device_ftl which is a concrete implementation of the nrf_block_dev interface. For bad block management the block device use dhara [1] as Flash Translation Layer (FTL). The FTL will use my spi nand flash driver. I think this design fit well for my use case, but I still have trouble with interrupt masking as explain in my previous post.

In fact the initial problem I've encountered concern the usage of the nrfx_spim driver, and lead to perform the transfer asynchronously. Let me explain this.

My first attempts what to do the simple thing that should work; synchronous data transfer using nrfx spim:
my_err spi_write_read(spi_client *client, const uint8_t *tx, size_t tx_len, uint8_t *rx, size_t rx_len) {

    ASSERT(client);
    ASSERT(client->device);
    ASSERT(client->device->spi);

    nrfx_spim_xfer_desc_t xfer = {
            .p_rx_buffer = rx,
            .rx_length   = rx_len,
            .p_tx_buffer = tx,
            .tx_length   = tx_len,
    };

    ret_code_t ret = nrfx_spim_xfer(client->device->spi, &xfer, 0);
    return nrf_to_err(ret);
}

The spi instance used is the SPIM3 and it is initialized like this:

static const nrfx_spim_config_t m_spi_config1 = {
        .ss_pin       = NRFX_SPIM_PIN_NOT_USED,
        .miso_pin     = FLASH_MISO,
        .mosi_pin     = FLASH_MOSI,
        .sck_pin      = FLASH_CLK,
        .orc          = 0xFF,
        .mode         = NRF_SPIM_MODE_0,
        .bit_order    = NRF_SPIM_BIT_ORDER_MSB_FIRST,
        .frequency    = NRF_SPIM_FREQ_32M,
        .irq_priority = SPI_DEFAULT_CONFIG_IRQ_PRIORITY,
};

nrfx_spim_init(dev->spi, &m_spi_config1, NULL, NULL);

But the issue with this was the nrfx_spim_xfer was interrupted by some irq and my SPI transfers was corrupted/invalid. My assumption is the flash doesn't seem support these bus interruptions.

So I've tried to surround the nrfx_spim_xfer call by __disable_ird() / __enable_irq(). With this patch the previous issue was corrected. However the softdevice raised some assert, I've discovered the softdevice is asserting probably because of timing requirements. So I'm trying to do these transfers asynchronously with the DMA.

Best,

Joël

[Simonr]

joel-sc said:

But the issue with this was the nrfx_spim_xfer was interrupted by some irq and my SPI transfers was corrupted/invalid. My assumption is the flash doesn't seem support these bus interruptions.

 Are you able to find out what IRQ this is? Is it the external NAND chip you're suspecting that does not support bus interruptions? The asserts you mention does indeed seem to be timing related, so that's a good call I think.

Can you check whether you have this SOFTDEVICE_PRESENT macro for entering a critical region enabled. See snippet below. The event shouldn't be erased when you enter/exit this CRITICAL_REGION, and should be trigged once you enable interrupts again after the CRITICAL tasks has been completed.

Best regards,

Simon

[joel-sc]

Hello,

Simonr said:

 Are you able to find out what IRQ this is?

Unfortunately, no...

Simonr said:

Is it the external NAND chip you're suspecting that does not support bus interruptions?

That is not explicitly indicated in the datasheet... The flash is a NAND MT29F8G01, but you won't be able to find the datasheet on the internet.

Simonr said:

Can you check whether you have this SOFTDEVICE_PRESENT macro for entering a critical region enabled.

Yes , it's defined.

Simonr said:

See snippet below

I think you forgot to paste this snippet ?

The implementation of the sd_nvic_critical_region_enter write ~0 to the NVIC->ICER which disable all external irq. So I don't get how pending interrupt will be raised once sd_nvic_critical_region_exit is called.

Best,

[Simonr]

Hi

Seems like the snippet wasn't included indeed. Very sorry about that, but here it is:

#ifdef SOFTDEVICE_PRESENT
#define CRITICAL_REGION_ENTER() \
 { \
 uint8_t __CR_NESTED = 0; \
 app_util_critical_region_enter(&__CR_NESTED);
#else
#define CRITICAL_REGION_ENTER() app_util_critical_region_enter(NULL)
#endif

I think you're misunderstanding the interrupts section. Interrupts are disabled during the critical region, but not the events that raise interrupts. This means that if an event raises an interrupt while they're disabled, it will be pending until the critical region is exited, and interrupts are enabled again.

Best regards,

Simon