nrf_dfu_flash_store change to QSPI function -> callback issue in nrfx_qspi_write

[Dominik Eugster]

Hi

I'm chnaging the Secure Bootloader to an external falsh for bank1, connected with QSPI.

In the function on_data_obj_write_request is the call for nrf_dfu_flash_store (file nrf_dfu_req_handler.c

static void on_data_obj_write_request(nrf_dfu_request_t * p_req, nrf_dfu_response_t * p_res)
{
    NRF_LOG_DEBUG("Handle NRF_DFU_OP_OBJECT_WRITE (data)");

    if (!nrf_dfu_validation_init_cmd_present())
    {
        /* Can't accept data because DFU isn't initialized by init command. */
        p_res->result = NRF_DFU_RES_CODE_OPERATION_NOT_PERMITTED;
        return;
    }

    uint32_t const data_object_offset = s_dfu_settings.progress.firmware_image_offset -
                                        s_dfu_settings.progress.firmware_image_offset_last;

    if ((p_req->write.len + data_object_offset) > s_dfu_settings.progress.data_object_size)
    {
        /* Can't accept data because too much data has been received. */
        NRF_LOG_ERROR("Write request too long");
        p_res->result = NRF_DFU_RES_CODE_INVALID_PARAMETER;
        return;
    }

    uint32_t const write_addr = m_firmware_start_addr + s_dfu_settings.write_offset;
    /* CRC must be calculated before handing off the data to fstorage because the data is
     * freed on write completion.
     */
    uint32_t const next_crc =
        crc32_compute(p_req->write.p_data, p_req->write.len, &s_dfu_settings.progress.firmware_image_crc);

    ASSERT(p_req->callback.write);

    ret_code_t ret =
        nrf_dfu_flash_store(write_addr, p_req->write.p_data, p_req->write.len, p_req->callback.write);

    if (ret != NRF_SUCCESS)
    {
        /* When nrf_dfu_flash_store() fails because there is no space in the queue,
         * stop processing the request so that the peer can detect a CRC error
         * and retransmit this object. Remember to manually free the buffer !
         */
        p_req->callback.write((void*)p_req->write.p_data);
        return;
    }

    /* Update the CRC of the firmware image. */
    s_dfu_settings.write_offset                   += p_req->write.len;
    s_dfu_settings.progress.firmware_image_offset += p_req->write.len;
    s_dfu_settings.progress.firmware_image_crc     = next_crc;

    /* This is only used when the PRN is triggered and the 'write' message
     * is answered with a CRC message and these field are copied into the response.
     */
    p_res->write.crc    = s_dfu_settings.progress.firmware_image_crc;
    p_res->write.offset = s_dfu_settings.progress.firmware_image_offset;
}

There is a callback which is needed:

ret_code_t ret =
        nrf_dfu_flash_store(write_addr, p_req->write.p_data, p_req->write.len, p_req->callback.write);

inside this function I use the QSPI function:

hal_qspi_write(&p_src, len, (dest & EXTERNAL_FLASH_ADDRESS_LOWER_BYTES), (void*)callback))

which at the end is calling the nrfx function nrfx_qspi_write. 

Here I should have a callback, but I don't know what I should implement.

The original callback in the nrf_fstorage_write is implemented like this:
return (p_fs->p_api)->write(p_fs, dest, p_src, len, p_context);

ret_code_t nrf_fstorage_write(nrf_fstorage_t const * p_fs,
                              uint32_t               dest,
                              void           const * p_src,
                              uint32_t               len,
                              void                 * p_context)
{
    NRF_FSTORAGE_PARAM_CHECK(p_fs,        NRF_ERROR_NULL);
    NRF_FSTORAGE_PARAM_CHECK(p_src,       NRF_ERROR_NULL);
    NRF_FSTORAGE_PARAM_CHECK(p_fs->p_api, NRF_ERROR_INVALID_STATE);
    NRF_FSTORAGE_PARAM_CHECK(len,         NRF_ERROR_INVALID_LENGTH);

    /* Length must be a multiple of the program unit. */
    NRF_FSTORAGE_PARAM_CHECK(!(len % p_fs->p_flash_info->program_unit), NRF_ERROR_INVALID_LENGTH);

    /* Source and destination addresses must be word-aligned. */
    NRF_FSTORAGE_PARAM_CHECK(addr_is_aligned32(dest),                NRF_ERROR_INVALID_ADDR);
    NRF_FSTORAGE_PARAM_CHECK(addr_is_aligned32((uint32_t)p_src),     NRF_ERROR_INVALID_ADDR);
    NRF_FSTORAGE_PARAM_CHECK(addr_is_within_bounds(p_fs, dest, len), NRF_ERROR_INVALID_ADDR);

    return (p_fs->p_api)->write(p_fs, dest, p_src, len, p_context);
}

Now the DFU is strating on bank1 but the App is generating an error because data is missing because of this callack I guess?

I was following this topic:

https://devzone.nordicsemi.com/f/nordic-q-a/48961/dfu-with-external-qspi-memory

[Edvin]

Dominik Eugster said:

Like I understand call this in a seperate function? (without the if)

That was my idea, at least. Because you don't have an external device telling you what to do, this would be the first step if the bootloader has decided to enter DFU mode. 

After validating the init packet, the next step is to start swapping out the actual application with the new image. We now assume that the application is legit, so the bootloader should then delete the old application from flash, and start receiving the new application. Since you don't have anyone pushing the new application to the bootloader, you need to make the bootloader read it from the external flash, and place it in it's internal flash. After the entire new application is in flash, in bank 0, I believe you can just reset the device, and the bootloader will discover that there is a new application present, and it will validate this one. 

So in all, the steps you are bypassing is the "wait and receive init packet" followed by "validate the init packet command", in addition to "wait and receive full application" and "application transfer complete -> reset" command. 

The bootloader has the possibility to use dual bank. In which case, it can receive the new application before deleting the old one. However, if the new application is larger than half of the old application, it will delete the bank0, and store it directly there. However, when the bootloader uses BLE to receive the image, it doesn't risk bricking the device if the transfer fails, or if the image following the valid init packet is actually invalid. So it may be a good idea to try to validate the actual image while it is still stored in the external flash before erasing the old application (because I assume that it is the application that put it in the QSPI in the first place). 

I am not 100% sure how this is done by default, but you can look for a place that validates the image in bank 1, and try to use this on the image in the QSPI flash.

Best regards,

Edvin

[Dominik Eugster]

Hi Edvin

Since the QSPI flash support is integrated in the original bootloader I use again the 2 image bootloader. I use bank1 for QSPI flash, bank0 is internal. I was debugging all and it is working like this (triggered by BLE over the Nordic App)

1. Bootloader starts. BLE DFU with new App -> init packet and new App will be checked and placed in bank 0, application starts and is running.

2. During running application do again BLE DFU. Booloader is starting, checking init package and Application, recognize that bank0 is used, download the new application over QSP tothe external flash, validate it and if all is valid copy the image from external flash to internal and start the application.

Everything fine and running.

Now: init package and application (dat and bin file) will be placed by custom made app in the external flash. The application will check own CRC, if all is valid, GPREGRET  will be triggered and chip restart will be done.

Then bootloader check GPREGRET, now there is a new init package in the external flash, call nrf_dfu_validation_init_cmd_execute. I can use the original function because first if is true and m_valid_init_cmd_present will be false and so it will call the if with stored_init_cmd_decode

nrf_dfu_result_t nrf_dfu_validation_init_cmd_execute(uint32_t * p_dst_data_addr,
                                                     uint32_t * p_data_len)
{
    nrf_dfu_result_t ret_val = NRF_DFU_RES_CODE_SUCCESS;

    if (s_dfu_settings.progress.command_offset != s_dfu_settings.progress.command_size)
    {
        // The object wasn't the right (requested) size.
        NRF_LOG_ERROR("Execute with faulty offset");
        ret_val = NRF_DFU_RES_CODE_OPERATION_NOT_PERMITTED;
    }
    else if (m_valid_init_cmd_present)
    {
        *p_dst_data_addr = nrf_dfu_bank1_start_addr();
        ret_val          = update_data_size_get(mp_init, p_data_len);
    }
    else if (stored_init_cmd_decode())

After this in the nrf_bootloader_init there is the call for nrf_bootloader_fw_activate which should do the rest. Or am I missing something?

I will try tomorrow

[Edvin]

That sounds about right to me. You may want to test this with the debug bootloader, so that you can check the logs from the bootloader. Or you can enable the logs in your bootloader, but that probably requires you to change the bootloader's start address as well.

Best regards,

Edvin

[Dominik Eugster]

Hi Edvin

I did the debugging, here the esults:

in the dfu_enter_check I enter with an additional flag of QSPI data is ready, working.

Then I call this function:

if (ret_val == NRF_SUCCESS)
    {
        init_pkg_size = (uint32_t) (init_command_buffer[0] | (init_command_buffer[1] << 8) | 
                                    (init_command_buffer[2] << 16) | (init_command_buffer[3] << 24));
        ret_val = nrf_dfu_validation_init_cmd_create(init_pkg_size);
        if (ret_val == NRF_DFU_RES_CODE_SUCCESS)
        {
            //copy the data in the init package target
            //cut the size (first 4 bytes)
            ret_val = nrf_dfu_validation_init_cmd_append(&init_command_buffer[FW_UPDATE_EXTERNAL_FLASH_INIT_PACKAGE_SIZE_VARIABLE], init_pkg_size);
            if (ret_val == NRF_DFU_RES_CODE_SUCCESS)
            {
                // Execute a previously received init packed. Subsequent executes will have no effect.
                ret_val = nrf_dfu_validation_init_cmd_execute(&firmware_start_addr, &firmware_size);

               // Execute a previously received init packed. Subsequent executes will have no effect.
        		if (ret_val == NRF_DFU_RES_CODE_SUCCESS)
        		{
                    if (nrf_dfu_validation_activation_prepare(firmware_start_addr, firmware_size) == NRF_DFU_RES_CODE_SUCCESS)
                    {
                        NRF_LOG_INFO("Postvalidation successful.");
                    }
        		}
            }
        }
    }

ret_val is success, looks good.

Then should I reset the chip or what should I do? Reinit?

Because nothing is happening, maybe a function is missing which normaly will be triggered over the BLE DFU which is now missing? Maybe app_activate is missing because call is before dfu_enter check?

Thank you

[Edvin]

So this means that the init packet is good, right?

After that, you decide whether you want to check whether the image itself is good or not (you mentioned that it was checked at an earlier stage, right?)

If you skip that check, or it looks good, then starts the process of deleting the old image in bank0, and replace it with the image in the QSPI flash. Usually, this is triggered by the DFU master sending the image in chunks. I guess you can look for that, and re-use parts of it. But basically, the work that needs to be done is to erase all the pages needed for the new image, and write the data from the QSPI flash to bank0. After this, you can restart, and the bootloader should detect that there is a new image present (I think). When it does, it will check the CRC once more, and store the findings into the bootloader settings. Then it will do a final reset, and boot up the appplication.

At least, from memory, this is the flow. I guess you can start by copying the image, resetting, and see how it behaves.

Best regards,

Edvin