Migrating OPTIGA TRUST I2C PAL from nrf_drv_* to nrfx_twim causes nrf_pwr_mgmt_run to not return

[martinspedro]

I am currently testing Optiga Trust X using an NRF52 DK. I am using armgcc as my toolchain.

The example provided by the nRF5 SDK v17.0.0 is a great starting point and I was able to fully test the Optiga functionalities.

Now, I am integrating the Optiga Trust X libs with my product codebase. So far, I have already included the required macros in the sdk_config.h, added the files to the makefile, got a successful compilation and run the code. However, to do so, since Optiga Trust X I2C Platform Abstraction Layer (PAL) provided by Nordic uses the legacy TWI drivers and the twi manager, I needed to remove my code that uses the nrfx_twim driver. Otherwise the application of the nrfx_glue and legacy porting layer messes up compilation by defining the legacy drivers and undefining the nrfx so much that I cannot get an executable.

As far as I am aware, nrfx_twim cannot coexist with nrf_drv or nrf_twi_mngr, so I started porting the I2C NRF PAL to use only the nrfx_twim. The port was apparently successful, since the code compiles, but when running the code, it gets stuck __SEV(), on nrf_pwr_mgmt_run call, on line 361 of nrf_pwr_mgmt.c, which contains:

// Wait for an event.
__WFE();
// Clear the internal event register.
__SEV();
__WFE();

My port of the to the nrfx_twim is attached below. Please note that I am using a blocking call, but have also tested using a non-blocking call with a twim_handler and it causes the same result. The call stack is in the following image.

Fullscreen nrfx_twim_pal_i2c.c Download

/**
* MIT License
*
* Copyright (c) 2018 Infineon Technologies AG
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in all
* copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
* SOFTWARE
*
*
* \file
*
* \brief This file implements the platform abstraction layer(pal) APIs for I2C.
*
* \addtogroup  grPAL
* @{
*/

/**********************************************************************************************************************
 * HEADER FILES
 *********************************************************************************************************************/
#include "optiga/pal/pal_i2c.h"
#include "optiga/ifx_i2c/ifx_i2c.h"
#include "pal_pin_config.h"
#include "nrfx_twim.h"
#include <stdbool.h>

/// @cond hidden

/**********************************************************************************************************************
 * MACROS
 *********************************************************************************************************************/
#define PAL_I2C_MASTER_MAX_BITRATE  (400)

/** @brief I2C driver instance */
#define TWI_INSTANCE_ID             0

/** @brief Maximal number of pending I2C transactions */
//#define MAX_PENDING_TRANSACTIONS    5

/*********************************************************************************************************************
 * LOCAL DATA
 *********************************************************************************************************************/

/* Pointer to the current pal i2c context */
static pal_i2c_t * gp_pal_i2c_current_ctx;

/** @brief Definition of TWI manager instance */
#ifndef IFX_2GO_SUPPORT
//NRF_TWI_MNGR_DEF(m_app_twi, MAX_PENDING_TRANSACTIONS, TWI_INSTANCE_ID);
const nrfx_twim_t m_app_twi = NRFX_TWIM_INSTANCE(TWI_INSTANCE_ID);
#else
const nrfx_twim_t m_app_twi = NRFX_TWIM_INSTANCE(TWI_INSTANCE_ID);

//nrf_twi_mngr_t m_app_twi;
#endif

/** @brief Definition of TWI manager transfer instance */
//static nrf_twi_mngr_transfer_t    m_transfer;

/** @brief Definition of TWI manager transaction instance */
//static nrf_twi_mngr_transaction_t m_transaction;

static bool initialized = false;

/**********************************************************************************************************************
 * LOCAL ROUTINES
 *********************************************************************************************************************/

/**
 * Pal I2C event handler function to invoke the registered upper layer callback<br>
 *
 *<b>API Details:</b>
 *  - This function implements the platform specific i2c event handling mechanism<br>
 *  - It calls the registered upper layer function after completion of the I2C read/write operations<br>
 *  - The respective event status are explained below.
 *   - #PAL_I2C_EVENT_ERROR when I2C fails due to low level failures(NACK/I2C protocol errors)
 *   - #PAL_I2C_EVENT_SUCCESS when operation is successfully completed
 *
 * \param[in] p_pal_i2c_ctx   Pointer to the pal i2c context #pal_i2c_t
 * \param[in] event           Status of the event reported after read/write completion or due to I2C errors
 *
 */
static void app_twi_callback(ret_code_t result, void * p_user_data)
{
    app_event_handler_t upper_layer_handler;
    //lint --e{611} suppress "void* function pointer is type casted to app_event_handler_t type"
    upper_layer_handler = (app_event_handler_t)gp_pal_i2c_current_ctx->upper_layer_event_handler;

    if (result == NRF_SUCCESS)
    {
        upper_layer_handler(gp_pal_i2c_current_ctx->upper_layer_ctx, PAL_I2C_EVENT_SUCCESS);
    }
    else
    {
        upper_layer_handler(gp_pal_i2c_current_ctx->upper_layer_ctx, PAL_I2C_EVENT_ERROR);
    }
}

/// @endcond

/**********************************************************************************************************************
 * API IMPLEMENTATION
 *********************************************************************************************************************/

/**
 * API to initialize the i2c master with the given context.
 * <br>
 *
 *<b>API Details:</b>
 * - The platform specific initialization of I2C master has to be implemented as part of this API, if required.<br>
 * - If the target platform does not demand explicit initialization of i2c master
 *   (Example: If the platform driver takes care of init after the reset), it would not be required to implement.<br>
 * - The implementation must take care the following scenarios depending upon the target platform selected.
 *   - The implementation must handle the acquiring and releasing of the I2C bus before initializing the I2C master to
 *     avoid interrupting the ongoing slave I2C transactions using the same I2C master.
 *   - If the I2C bus is in busy state, the API must not initialize and return #PAL_STATUS_I2C_BUSY status.
 *   - Repeated initialization must be taken care with respect to the platform requirements. (Example: Multiple users/applications
 *     sharing the same I2C master resource)
 *
 *<b>User Input:</b><br>
 * - The input #pal_i2c_t p_i2c_context must not be NULL.<br>
 *
 * \param[in] p_i2c_context   Pal i2c context to be initialized
 *
 * \retval  #PAL_STATUS_SUCCESS  Returns when the I2C master init it successfull
 * \retval  #PAL_STATUS_FAILURE  Returns when the I2C init fails.
 */
pal_status_t pal_i2c_init(const pal_i2c_t* p_i2c_context)
{
#ifndef IFX_2GO_SUPPORT
    const nrfx_twim_config_t config = {
       .scl                 = OPTIGA_PIN_I2C_SCL,
       .sda                 = OPTIGA_PIN_I2C_SDA,
       .frequency           = NRF_TWIM_FREQ_400K,
       .interrupt_priority  = NRFX_TWIM_DEFAULT_CONFIG_IRQ_PRIORITY,
       .hold_bus_uninit     = NRFX_TWIM_DEFAULT_CONFIG_HOLD_BUS_UNINIT
    };
#else
    #include "ifx_2go_common.h"
    nrfx_twim_config_t const config = {
       .scl                = ifx_2go_pin_config()->scl,
       .sda                = ifx_2go_pin_config()->sda,
       .frequency          = NRF_TWI_FREQ_400K,
       .interrupt_priority = APP_IRQ_PRIORITY_LOWEST,
       .hold_bus_uninit     = false
    };
#endif

    if(initialized)
    {
        nrfx_twim_uninit(&m_app_twi);
    }

    // Initialize I2C driver
    if (nrfx_twim_init(&m_app_twi, &config, NULL, NULL) != NRF_SUCCESS)
    {
            return PAL_STATUS_FAILURE;
    }
    nrfx_twim_enable(&m_app_twi);

    initialized = true;
    return PAL_STATUS_SUCCESS;
}

/**
 * API to de-initialize the I2C master with the specified context.
 * <br>
 *
 *<b>API Details:</b>
 * - The platform specific de-initialization of I2C master has to be implemented as part of this API, if required.<br>
 * - If the target platform does not demand explicit de-initialization of i2c master
 *   (Example: If the platform driver takes care of init after the reset), it would not be required to implement.<br>
 * - The implementation must take care the following scenarios depending upon the target platform selected.
 *   - The implementation must handle the acquiring and releasing of the I2C bus before de-initializing the I2C master to
 *     avoid interrupting the ongoing slave I2C transactions using the same I2C master.
 *   - If the I2C bus is in busy state, the API must not de-initialize and return #PAL_STATUS_I2C_BUSY status.
 *   - This API must ensure that multiple users/applications sharing the same I2C master resource is not impacted.
 *
 *<b>User Input:</b><br>
 * - The input #pal_i2c_t p_i2c_context must not be NULL.<br>
 *
 * \param[in] p_i2c_context   I2C context to be de-initialized
 *
 * \retval  #PAL_STATUS_SUCCESS  Returns when the I2C master de-init it successfull
 * \retval  #PAL_STATUS_FAILURE  Returns when the I2C de-init fails.
 */
pal_status_t pal_i2c_deinit(const pal_i2c_t* p_i2c_context)
{
    if(initialized) {
        nrfx_twim_uninit(&m_app_twi);
    }
    initialized = false;
    return PAL_STATUS_SUCCESS;
}

/**
 * Platform abstraction layer API to write the data to I2C slave.
 * <br>
 * <br>
 * \image html pal_i2c_write.png "pal_i2c_write()" width=20cm
 *
 *
 *<b>API Details:</b>
 * - The API attempts to write if the I2C bus is free, else it returns busy status #PAL_STATUS_I2C_BUSY<br>
 * - The bus is released only after the completion of transmission or after completion of error handling.<br>
 * - The API invokes the upper layer handler with the respective event status as explained below.
 *   - #PAL_I2C_EVENT_BUSY when I2C bus in busy state
 *   - #PAL_I2C_EVENT_ERROR when API fails
 *   - #PAL_I2C_EVENT_SUCCESS when operation is successfully completed asynchronously
 *<br>
 *
 *<b>User Input:</b><br>
 * - The input #pal_i2c_t p_i2c_context must not be NULL.<br>
 * - The upper_layer_event_handler must be initialized in the p_i2c_context before invoking the API.<br>
 *
 *<b>Notes:</b><br>
 *  - Otherwise the below implementation has to be updated to handle different bitrates based on the input context.<br>
 *  - The caller of this API must take care of the guard time based on the slave's requirement.<br>
 *
 * \param[in] p_i2c_context  Pointer to the pal I2C context #pal_i2c_t
 * \param[in] p_data         Pointer to the data to be written
 * \param[in] length         Length of the data to be written
 *
 * \retval  #PAL_STATUS_SUCCESS  Returns when the I2C write is invoked successfully
 * \retval  #PAL_STATUS_FAILURE  Returns when the I2C write fails.
 * \retval  #PAL_STATUS_I2C_BUSY Returns when the I2C bus is busy.
 */
pal_status_t pal_i2c_write(pal_i2c_t* p_i2c_context,uint8_t* p_data , uint16_t length)
{
    gp_pal_i2c_current_ctx = p_i2c_context;
/*
    m_transfer.p_data    = p_data;
    m_transfer.length    = length;
    m_transfer.operation = NRF_TWI_MNGR_WRITE_OP(IFX_I2C_BASE_ADDR);
    m_transfer.flags     = 0;

    m_transaction.callback            = app_twi_callback;
    m_transaction.number_of_transfers = 1;
    m_transaction.p_required_twi_cfg  = NULL;
    m_transaction.p_transfers         = &m_transfer;
    m_transaction.p_user_data         = (void*) PAL_STATUS_SUCCESS;
*/

    nrfx_twim_xfer_desc_t tx_xfer = NRFX_TWIM_XFER_DESC_TX(IFX_I2C_BASE_ADDR, p_data, length);


    if (nrfx_twim_xfer(&m_app_twi, &tx_xfer, 0) != NRF_SUCCESS)
    {
        app_twi_callback(NRF_ERROR_BUSY, 0);
    }

    return PAL_STATUS_SUCCESS;
}

/**
 * Platform abstraction layer API to read the data from I2C slave.
 * <br>
 * <br>
 * \image html pal_i2c_read.png "pal_i2c_read()" width=20cm
 *
 *<b>API Details:</b>
 * - The API attempts to read if the I2C bus is free, else it returns busy status #PAL_STATUS_I2C_BUSY<br>
 * - The bus is released only after the completion of reception or after completion of error handling.<br>
 * - The API invokes the upper layer handler with the respective event status as explained below.
 *   - #PAL_I2C_EVENT_BUSY when I2C bus in busy state
 *   - #PAL_I2C_EVENT_ERROR when API fails
 *   - #PAL_I2C_EVENT_SUCCESS when operation is successfully completed asynchronously
 *<br>
 *
 *<b>User Input:</b><br>
 * - The input #pal_i2c_t p_i2c_context must not be NULL.<br>
 * - The upper_layer_event_handler must be initialized in the p_i2c_context before invoking the API.<br>
 *
 *<b>Notes:</b><br>
 *  - Otherwise the below implementation has to be updated to handle different bitrates based on the input context.<br>
 *  - The caller of this API must take care of the guard time based on the slave's requirement.<br>
 *
 * \param[in]  p_i2c_context  pointer to the PAL i2c context #pal_i2c_t
 * \param[in]  p_data         Pointer to the data buffer to store the read data
 * \param[in]  length         Length of the data to be read
 *
 * \retval  #PAL_STATUS_SUCCESS  Returns when the I2C read is invoked successfully
 * \retval  #PAL_STATUS_FAILURE  Returns when the I2C read fails.
 * \retval  #PAL_STATUS_I2C_BUSY Returns when the I2C bus is busy.
 */
pal_status_t pal_i2c_read(pal_i2c_t* p_i2c_context , uint8_t* p_data , uint16_t length)
{
    gp_pal_i2c_current_ctx = p_i2c_context;
/*
    m_transfer.p_data    = p_data;
    m_transfer.length    = length;
    m_transfer.operation = NRF_TWI_MNGR_READ_OP(IFX_I2C_BASE_ADDR);
    m_transfer.flags     = 0;

    m_transaction.callback            = app_twi_callback;
    m_transaction.number_of_transfers = 1;
    m_transaction.p_required_twi_cfg  = 0;
    m_transaction.p_transfers         = &m_transfer;
    m_transaction.p_user_data         = (void*) PAL_STATUS_SUCCESS;
*/
    nrfx_twim_xfer_desc_t rx_xfer = NRFX_TWIM_XFER_DESC_RX(IFX_I2C_BASE_ADDR, p_data, length);


    if (nrfx_twim_xfer(&m_app_twi, &rx_xfer, 0) != NRF_SUCCESS)
    {
        app_twi_callback(NRF_ERROR_BUSY, 0);
    }

    return PAL_STATUS_SUCCESS;
}

/**
 * Platform abstraction layer API to set the bitrate/speed(KHz) of I2C master.
 * <br>
 *
 *<b>API Details:</b>
 * - Sets the bitrate of I2C master if the I2C bus is free, else it returns busy status #PAL_STATUS_I2C_BUSY<br>
 * - The bus is released after the setting the bitrate.<br>
 * - This API must take care of setting the bitrate to I2C master's maximum supported value.
 * - Eg. In XMC4500, the maximum supported bitrate is 400 KHz. If the supplied bitrate is greater than 400KHz, the API will
 *   set the I2C master's bitrate to 400KHz.
 * - Use the #PAL_I2C_MASTER_MAX_BITRATE macro to specify the maximum supported bitrate value for the target platform.
 * - If upper_layer_event_handler is initialized, the upper layer handler is invoked with the respective event
 *   status listed below.
 *   - #PAL_I2C_EVENT_BUSY when I2C bus in busy state
 *   - #PAL_I2C_EVENT_ERROR when API fails to set the bit rate
 *   - #PAL_I2C_EVENT_SUCCESS when operation is successful
 *<br>
 *
 *<b>User Input:</b><br>
 * - The input #pal_i2c_t  p_i2c_context must not be NULL.<br>
 *
 * \param[in] p_i2c_context  Pointer to the pal i2c context
 * \param[in] bitrate        Bitrate to be used by i2c master in KHz
 *
 * \retval  #PAL_STATUS_SUCCESS  Returns when the setting of bitrate is successfully completed
 * \retval  #PAL_STATUS_FAILURE  Returns when the setting of bitrate fails.
 * \retval  #PAL_STATUS_I2C_BUSY Returns when the I2C bus is busy.
 */
pal_status_t pal_i2c_set_bitrate(const pal_i2c_t* p_i2c_context , uint16_t bitrate)
{
    // Bitrate is fixed to the maximum frequency on this platform (400K)
    return PAL_STATUS_SUCCESS;
}

#ifdef IFX_2GO_SUPPORT
pal_status_t pal_i2c_set_instance(nrfx_twim_t* twi_inst)
{
        m_app_twi = *twi_inst;
}
#endif/*IFX_2GO_SUPPORT*/

/**
* @}
*/

I have two questions:

1. Is not possible to use, on the same application, nrf_drv_ + nrf_twi_mgnr and a nrfx_twim drivers, even if the former controls the TWI0 and the latter the TWIM1, correct?

2. Why am I getting stuck on the pwr_mgmt_run routine and what can be done to fix it?

Best regards,

Pedro

[Susheel Nuguru]

Hi Martin,

Thanks for waiting so patiently, unfortunately, the expert of crypto is still unable to look at this at the moment, i will come back to you as soon as i get new info. sorry for the delays.

[martinspedro]

Dear  Susheel Nuguru,

Thank you for getting back to this. 

To clarify, I solved myTWI module issues using the solution I proposed above. 

I was just hoping that someone from your side could clarify if my approach is enough to uninit the nrf_crypto module. Also, I would like to understand why the SDK had a comment saying Function to uninitialize OPTIGA backend - currently no implementation is required.

[Susheel Nuguru]

Martin,

I understand that you have solved the issue with the solution you proposed. It seems a sensible solution, however, i would like to get the expert to give his blessings to this.

[martinspedro]

Of course, Susheel Nuguru!
I am also waiting for the crypto expert answer on the topic.

[Einar Thorsrud]

Hi,

martinspedro said:

Either way, to release the hardware peripherals being used to communicate with Optiga, this problem can be solved by rewriting this function as:

I do not see any problems with your approach. By adding a call to optiga_comms_close() in optiga_backend_uninit() you uninit the TWI manager and driver when calling nrf_crypto_uninit(), and that will be initialized again if you call nrf_crypto_init() at a later stage. I have added an internal feature request for this.

martinspedro said:

Since my knowledge of the nrf_crypto is limited, I am not sure if anything needs to be unitialized. At least, Optiga's backend should be removed from the list of available backends, no? Can you please advice?

This is not how nrf_crypto is designed. You select backends in sdk_config.h, and the preprocessor verifies that there is maximum one backend for each specific algorithm. And when you call nrf_crypto_init() all backend are enabled and stay enabled until you call nrf_crypto_uninit().

[martinspedro]

Hi Einar,

Thank you for looking into this and confirming that the approach to uninit all the Optiga related resources is indeed ok.

Also appreciate the explanation on the nrf_crypto. It confirms that I've build a good understand of it after the initial post.

All is clear now, thank you for the time.

Best regards, 
Pedro