This thread has been locked.

If you have a related question, please click the "Ask a related question" button in the top right corner. The newly created question will be automatically linked to this question.

Concerto - uCRC-module usage within nested interrupts

Hi all,

From reading 1.11 of the technical reference manual for f28m35x, and from the implemention file, I was the under the impression that the controlsuite-function UCRCCalculation(unsigned long ulBase, unsigned long ulType,unsigned char * pucBuffer unsigned long ulNumBytes) actually was reentrant. That is, it may be called from nested interrupts that may preempt each other. However, I have ran some tests computing CRC32 for a 256 byte in the idle task, and computing CRC16 in an interrupt, and come to the conclusion that there is a race condition and sometimes the CRC32 is incorrect.

My question is whether this is a bug? Or perhaps I misunderstood, and it was not the intention to cater for such interrupt nesting.

Best regards,

Christian

  • Anyone? Please confirm if it is supposed to work with preemption.The function does save and restore the main uCRC registers.

  • Christian,

    We are looking into this. 

    Please give us a day or so to dig deeper into your issue.


    Thank you,
    Brett

  • Thank you Brett for looking at this. If you confirm it is supposed to work with preemption, then I can probably contribute a minimal project illustrating the problem. But if it is not supposed to work, then I would like to not spend a lot of time trying to set up a minimal case showing it. My current solution is to just temporarily mask of the relevant interrupts when computing CRC.

    Best regards,

    Christian

  • Christian,

    From my understanding, we set up the software so that it would be re-entrant. 

    We had some trouble recreating your test case.  Therefore, it probably would be beneficial if you could send us a minimal test case which has the issue.


    Thank you,
    Brett

  • Hi Brett,

    6013.crcRaceCondition.zip

    Please find attached a modified version of the m3 timers example project. It has the the issue I have seen with the function UCRCCalculation(). CRC16 is calculated in a timer-interrupt and has side-effects on a CRC32 calculated in the while(1)-loop.

    Best regards,

    Christian

  • Hi Christian,

    The context restore part of the calculation function looks to be incorrect. Please replace driverlib/ucrc.c with the attached .c file, recompile driverlib.lib and then try your code.

    //###########################################################################
    // FILE:   ucrc.c
    // TITLE:  Driver for the uCRC module.
    //###########################################################################
    // $TI Release: F28M35x Support Library v150 $
    // $Release Date: Mon Sep 17 09:14:32 CDT 2012 $
    //###########################################################################
    
    #include "inc/hw_ucrc.h"
    #include "inc/hw_memmap.h"
    #include "inc/hw_types.h"
    #include "driverlib/ucrc.h"
    #include "driverlib/debug.h"
    
    //*****************************************************************************
    //! \addtogroup ucrc_api
    //! @{
    //*****************************************************************************
    
    //*****************************************************************************
    //! Clear the uCRC result register.
    //!
    //! \param ulBase is the base address of the uCRC peripheral.
    //!
    //! Resets the result register back to initial state
    //!
    //*****************************************************************************
    void UCRCClear(unsigned long ulBase){
        // Check the arguments.
        ASSERT(ulBase == UCRC_BASE);
    
        HWREG(ulBase + UCRC_O_CONTROL) |= UCRC_CONTROL_CLEAR;
    
    }
    
    //*****************************************************************************
    //! Configure the uCRC module for a particular type of CRC.
    //!
    //! \param ulBase is the base address of the uCRC peripheral.
    //! \param ulType is the type of CRC to perform.
    //!
    //! Configures the UCRC peripheral for a given type of CRC.  The \e ucType
    //! parameter may be any of the following:
    //! - \b UCRC_CONFIG_CRC8 - perform CRC8 (Polynomial = 0x03)
    //! - \b UCRC_CONFIG_CRC16_1 - perform CRC16 (Polynomial = 0x8005)
    //! - \b UCRC_CONFIG_CRC16_2 - perform CRC16 (Polynomial = 0x1021)
    //! - \b UCRC_CONFIG_CRC32 - perform CRC32 (Polynomial = 0x04C11DB7)
    //!
    //*****************************************************************************
    void UCRCConfig(unsigned long ulBase, unsigned long ulType){
        // Check the arguments.
        ASSERT(ulBase == UCRC_BASE);
        ASSERT(ulType &
               (UCRC_CONFIG_CRC8 | UCRC_CONFIG_CRC16_1 | UCRC_CONFIG_CRC16_2 |
        UCRC_CONFIG_CRC32));
    
        HWREG(ulBase + UCRC_O_CONFIG) = ulType;
    
    }
    
    //*****************************************************************************
    //! Perform a CRC calculation
    //!
    //! \param ulBase is the base address of the uCRC peripheral.
    //! \param ulType is the type of CRC to perform.
    //! \param pucBuffer is the buffer to perform CRC calculation on.
    //! \param ulNumBytes is the number of bytes to perform CRC on.
    //!
    //! Configures the uCRC module and performs the requested CRC operation.
    //!
    //! \return Requested CRC value for the memory given.
    //!
    //*****************************************************************************
    unsigned long UCRCCalculation(unsigned long ulBase, unsigned long ulType,
                                  unsigned char * pucBuffer,
                                  unsigned long ulNumBytes){
    
        unsigned long ulTemp;
        unsigned long ulPreviousCRC;
        unsigned long ulPreviousMode;
        unsigned char *pucBufPtr;
    
        // Check the arguments.
        ASSERT(ulBase == UCRC_BASE);
        ASSERT(ulType &
               (UCRC_CONFIG_CRC8 | UCRC_CONFIG_CRC16_1 | UCRC_CONFIG_CRC16_2 |
        UCRC_CONFIG_CRC32));
        
        // Save previous CRC calculation value incase a calculation was already in
        // progress
        ulPreviousCRC = HWREG(ulBase + UCRC_O_RES);
        ulPreviousMode = HWREG(ulBase + UCRC_O_CONFIG);
    
        UCRCClear(ulBase);
        UCRCConfig(ulBase, ulType);
    
        // uCRC only calculates on reads from a re-mapped space of memory,
        // so we need to remap the address before we start calculation.
        pucBufPtr = (unsigned char *)UCRC_REMAP_ADDRESS(pucBuffer);
    
        // Read the buffer while calculating CRC
        while(pucBufPtr <
              (unsigned char *)UCRC_REMAP_ADDRESS(pucBuffer + ulNumBytes)) {
            ulTemp = HWREGB(pucBufPtr++);
        }
    
        // Read calculated CRC
        ulTemp = HWREG(ulBase + UCRC_O_RES);
    
        // Restore previous CRC setup and value
        UCRCClear(ulBase);
        //<<VC131015
        // The order of the context restore was incorrect. If the current mode
        // is CRC 16 or 8, trying to restore a 32 bit partial CRC results in the
        // upper 16 or 24 bits being zeroed out. The solution is to restore the 
        // mode first, then the result.
        HWREG(ulBase + UCRC_O_CONFIG) = ulPreviousMode;
        HWREG(ulBase + UCRC_O_RES) = ulPreviousCRC;
        //VC131015>>
    
        return ulTemp;
    
    }
    
    //*****************************************************************************
    // Close the Doxygen group.
    //! @}
    //*****************************************************************************
    
    
    

    The function tries to write the 32-bit partial result from the CRC32 into the uCRCRES register when the CRC mode is CRC16.... It zeroes out the upper 16-bits so you get a corrupt CRC when you continue your CRC32. The solution is to restore the mode first to CRC32, then the partial result. Please confirm if this fix works.

  • Hi Vishal,

    Great work! That was the fix. I have ran quite a few tests here and have seen no problems. Thank you.

    I would really appreciate if you could also try to answer the following related questions:

    -To prove its reentrancy, do you need to inspect the assembly code. Or is it possible to argue from the source code?

    -Do you think aggressive optimization settings could break its reentrancy?

     

    Best regards,

    Christian

     

  • Christian,

    Christian L said:
    -To prove its reentrancy, do you need to inspect the assembly code. Or is it possible to argue from the source code?

    Usually you can figure out from the source code itself. In this case, the save/restore looked fine at first glance. I only noticed the problem when i stepped through the code while observing the uCRC registers. I would only look at the assembly if the compiler is doing something completely unexpected.

    I did notice that the save/restore was using registers R4 and R5 instead of the stack...so maybe it might be a good idea to also declare ulPreviousCRC and ulPreviousMode as volatile....hopefully that will get the compiler to do a PUSH/POP to the stack instead. Ill have to look into that more closely.

    Christian L said:
    -Do you think aggressive optimization settings could break its reentrancy?

    In my opinion, the compiler is pretty robust and will take care to meet all the constraints or, in the worst case, warn you if its doing something that it thinks you might not like...e.g. deleting unreferenced variables or implicitly defined functions. 

  • Thanks again Vishal.

    I guess using R4-R5 is not a problem since they are also themselves saved and restored according to the calling convention.

    I assume your fix will be part of the next control-suite release for concerto..

    Best regards,

    Christian

  • Christian L said:
    I assume your fix will be part of the next control-suite release for concerto..

    Yep, I have filed a bug against it. It should be fixed on the next revision.