FRDM K64F sharing global variables with interrupt service routines

cancel
Showing results for 
Show  only  | Search instead for 
Did you mean: 

FRDM K64F sharing global variables with interrupt service routines

2,258 Views
luimarma
Contributor III

I have to develop a bare metal program with FRDM K64F using interrupts that modify some global variables which can also be modified in the main loop program.

The question is simple, yet complicated. How can I warranty that variables are writen consistently both in irq sevice routine and main program?

As an example I copy&paste a modified version of the hw_timer example.

In this example I have used a variable named "pointMark" that is incremented in an interrupt callback and decremented in the main program.

Can this code crash? If so, how to avoid?

Thanks in advance,

Luis

/*
* Copyright (c) 2013 - 2014, Freescale Semiconductor, Inc.
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without modification,
* are permitted provided that the following conditions are met:
*
* o Redistributions of source code must retain the above copyright notice, this list
* of conditions and the following disclaimer.
*
* o Redistributions in binary form must reproduce the above copyright notice, this
* list of conditions and the following disclaimer in the documentation and/or
* other materials provided with the distribution.
*
* o Neither the name of Freescale Semiconductor, Inc. nor the names of its
* contributors may be used to endorse or promote products derived from this
* software without specific prior written permission.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
* ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
* WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
* DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR
* ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
* (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
* LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
* ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
* SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/

///////////////////////////////////////////////////////////////////////////////
// Includes
///////////////////////////////////////////////////////////////////////////////

// Standard C Included Files
#include <stdio.h>

// SDK Included Files
#include "board.h"
#include "fsl_hwtimer.h"
#include "fsl_debug_console.h"

///////////////////////////////////////////////////////////////////////////////
// Definitions
///////////////////////////////////////////////////////////////////////////////

#define HWTIMER_LL_DEVIF kSystickDevif
#define HWTIMER_LL_ID 0

#define HWTIMER_ISR_PRIOR 5
#define HWTIMER_PERIOD 1000
//#define HWTIMER_PERIOD 100000
#define HWTIMER_DOTS_PER_LINE 40
#define HWTIMER_LINES_COUNT 200

///////////////////////////////////////////////////////////////////////////////
// Variables
///////////////////////////////////////////////////////////////////////////////

extern const hwtimer_devif_t kSystickDevif;
extern const hwtimer_devif_t kPitDevif;
hwtimer_t hwtimer;
static volatile uint16_t pointMark=0U;

///////////////////////////////////////////////////////////////////////////////
// Code
///////////////////////////////////////////////////////////////////////////////

/*!
* @brief Hardware timer callback function
*/
void hwtimer_callback(void* data)
{
   PRINTF(".");
   // if ((HWTIMER_SYS_GetTicks(&hwtimer) % HWTIMER_DOTS_PER_LINE) == 0) pointMark++;
   if ((pointMark % HWTIMER_DOTS_PER_LINE) == 0)
   {
      PRINTF("\r\n");
   }
   if ((HWTIMER_SYS_GetTicks(&hwtimer) % (HWTIMER_LINES_COUNT * HWTIMER_DOTS_PER_LINE)) == 0)
   {
      if (kHwtimerSuccess != HWTIMER_SYS_Stop(&hwtimer))
      {
         PRINTF("\r\nError: hwtimer stop.\r\n");
      }
      PRINTF("End\r\n");
   }
}

/*!
* @brief Main function
*/
int main (void)
{
    // Initialize standard SDK demo application pins
   hardware_init();

   // Print the initial banner
   PRINTF("\r\nHwtimer Example \r\n");

   // Hwtimer initialization
    if (kHwtimerSuccess != HWTIMER_SYS_Init(&hwtimer, &HWTIMER_LL_DEVIF, HWTIMER_LL_ID, NULL))
    {
      PRINTF("\r\nError: hwtimer initialization.\r\n");

    }

   /*
    * In case you wish to raise the Systick ISR priority, then use the below command with the
    * appropriate priority setting:
    * NVIC_SetPriority(SysTick_IRQn, isrPrior);
    */
    NVIC_SetPriority(SysTick_IRQn, HWTIMER_ISR_PRIOR);

   if (kHwtimerSuccess != HWTIMER_SYS_SetPeriod(&hwtimer, HWTIMER_PERIOD))
    {
       PRINTF("\r\nError: hwtimer set period.\r\n");
    }
    if (kHwtimerSuccess != HWTIMER_SYS_RegisterCallback(&hwtimer, hwtimer_callback, NULL))
    {
       PRINTF("\r\nError: hwtimer callback registration.\r\n");
    }
    if (kHwtimerSuccess != HWTIMER_SYS_Start(&hwtimer))
    {
       PRINTF("\r\nError: hwtimer start.\r\n");
    }

   // Wait for Hardware Timer interrupts
    while(1)
    {
       if (pointMark==10) {
          uint16_t pts=0;
          pointMark = 0;
          while (pts++<67000){}
          if (pointMark==0) {
             PRINTF("\r\n");
          } else pointMark--;
       } else {
          uint16_t pts=0;
          while (pts++<64000) {}
         if (pointMark>0) pointMark--;
       }
    }
}

Labels (1)
12 Replies

1,707 Views
pavel_chromy
NXP Employee
NXP Employee

Dear Louis,

I see that you have already received some answers to your question how to protect global variable from interrupt.

It is true that you can do this by disabling interrupts, but this is for sure not ideal solution. Technically speaking, these answers are correct, but they do not consider whether the question asked was the right one.

The question is: Do you really need to modify single variable in both main and interrupt?

According to my experience the answer is most likely no, as there are better ways to do it. There are only very few reasons when disabling of interrupts is inevitable. In 99% cases, this would not be a necessity with proper design.

It seems to me that the code is unnecessarily "recycling" single variable.

The flow of the program where a single variable is modified in multiple places including interrupt routine it not easy to read.

You should rather have two variables: one incremented in the interrupt and another one in the main loop, the following way (simplified approach, just use the idea and modify it for your needs):

volatile unsigned int ticks;

void hwtimer_callback(void* data)
{

  ticks++;

}

main()

{

  unsigned int start;

  start = ticks; /* capture the value of ticks */

  while (ticks - start < 1000) { }; /* subtract/compare technique: 1000 is the number of ticks to wait */

  PRINTF ("1000 ticks elapsed");

}

You don't need to disable interrupts for this to work properly.

Please note that both ticks and start variables need to be unsigned as this technique relies on arithmetic overflow. Although the behavior of arithmetic overflow is not strictly defined by C standard it can be relied on - it works for most architectures and tools. Still if this is a concern it is possible to use signed subtraction with a larger data type and adjust negative values.

I have spotted several more possible issues in the code:

  1. Please note that if you read the variable incremented in the interrupt at multiple places of the main loop, you might get multiple different values. It seems to me that at one place this is even part of the design:
    pointMark = 0;
      while (pts++<67000){}
      if (pointMark==0) { /* <- does this expect the pointMark to be incremented during the previous delay loop? */
    This is very error prone and might result in unpredictable behavior, please avoid this.
  2. The same applies to HWTIMER_SYS_GetTicks being called multiple times in the interrupt. To be on the safe side call it just once and capture the result in a variable. Please note that the execution of the interrupt may be delayed (namely if you disable interrupts in the main program) and thus the values obtained from multiple calls to HWTIMER_SYS_GetTicks may differ.
  3. Please consider that you may also miss interrupts! Never rely on not missing a timer tick. This always shows as problematic - sooner or later - and these kind of bugs are very hard to trace down. This is the piece of code which, I suppose, relies on not missing an a tick:
    if ((HWTIMER_SYS_GetTicks(&hwtimer) % HWTIMER_DOTS_PER_LINE) == 0) pointMark++;
    Please note the ==0 comparison. If you miss just this very tick when modulo would be zero then the execution would be delayed for whole next period. Even worse, if some other code functionally relies on this it may stop working completely. The solution might be to keep value of hwtimer upon last pointMark increment and use the same subtract/compare technique as described above.

Best regards,

  Pavel

1,707 Views
luimarma
Contributor III

Dear Pavel,

The given example is just this, an example.

The fact is that I have a variable that is changed both in an interrupt service routine (due to a network request) and also in the main loop (due to calculations in the program flow).

This both can happen at the same time and I don't want to make the program crash in case this happens. So I think that I really need something that makes sure that the operation is atomic.

Regards,

Luis

0 Kudos

1,707 Views
mjbcswitzerland
Specialist V

Hi Luis

I confirm that you DO NEED to protect the variable and I think that Pavel didn't understand your case when making the remarks.

The only exception is if there is a method in the processor that supports atomic (read/modify/write) operations on variables.

In the case of flags in memory the bit-banding capability of the Cortex can be used to set and clear bits atomically without needing to protect the variable in question.

In the case of increments of counters or of pointers there is no atomic method (that I am aware of) that could be used to guaranty that an interrupt also performing a modification of the same variable will not cause corruption. Such corruption often being the cause of total system failure....

Regards

Mark

0 Kudos

1,707 Views
pavel_chromy
NXP Employee
NXP Employee

Dear Mark,

my point was that there is no need to protect any variable if you follow "single writer" approach. It can be used in many cases, the timer ticks being just one case. There was nothing about network requests in the original question, still it would be wise to avoid servicing complex stuff directly in the interrupt, but rather set a variable as request and do subsequent operation sequentially in the main thread (and this could be done using single writer approach!)

I assume that the original question was about bare metal application.

Surely it is not possible to do atomic operation when single variable is being written from main and interrupt without hw support (or preventing it by disabling interrupts). The key is to avoid write access to single variable by design and I think this is well doable in bare metal applications.

Best regards, Pavel

0 Kudos

1,707 Views
mjbcswitzerland
Specialist V

Pavel

- If it is possible to not need protection it is indeed the way to go.

- In some cases it won't be possible (also in bare-metal applications) so needs to be done.

Regards

Mark

0 Kudos

1,707 Views
pavel_chromy
NXP Employee
NXP Employee

BTW, have a look at the following link.

Producer–consumer problem - Wikipedia 

For sure it can be done also on bare metal, the only thing the technique relies on is the fact that write to single memory location (32-bit integer on today's common architecture) is atomic.

Please note that many common synchronization problems can be transformed to simple producer-consumer - including the one in the original question.

0 Kudos

1,707 Views
mjbcswitzerland
Specialist V

Hi

You always need to protect variables from interrupts - either by globally disabling interrupts during their modification, or by setting the interrupt mask level so that those which could corrupt them are disabled.

Eg.

_disableInts();

    pointMark--;
_enableInts();

This is now safe.
If not done, the system WILL fail at some point.

Regards

Mark

1,707 Views
luimarma
Contributor III

Thanks for your answer, Mark.

Is there any application note or literature for K64 about how to implement the interrupt dissable depending on an interrupt mask level?

Is there no mutex aproach like for c pthreads to solve the problem?

Regards,

Luis

0 Kudos

1,707 Views
pavel_chromy
NXP Employee
NXP Employee

No mutex approach can be used in interrupt. This is by principle - it is not possible to use block execution in interrupt (wait for a mutex) as this would block forever.

Pavel

0 Kudos

1,707 Views
mjbcswitzerland
Specialist V

Luis

This is a Cortex M4 subject and not K64, therefore you an find details at the ARM Cortex web site.

I would just use disable/enable global interrupts since it is fool-proof and will only take a few ns to complete when protecting variables.

There may be mutex approaches for certain environments but that will depend on the environment being used, whereby I can't imagine that they are more efficient in any way.

Regards

Mark

1,707 Views
egoodii
Senior Contributor III

The 'brute force' complete disable/enable is 'serviceable enough' for most applications, but if you want to disable 'only to a level' (for M3/4 cores) look into BASE PRIORITY (__set_BASEPRI).

FWIW, my usage:

///////////////////////////////////////
/////////  SPI port 1 is shared between the EtherCAT ESC, the isolated Digital I/O, and the output DAC.
/////////   Sequences of bytes to/from CANNOT logically be interrupted, so anywhere there is (or could be)
/////////   an access sequence over SPI1 we must insure that the processor is at the 'same interrupt level'
/////////   as the ESC could provide, and this thus applies to PortC (ESC/IO), PortA (analog) and LPTMR ints,
/////////   (which will set that level implicitly in NVIC) as well as 'background' accesses.
/////////  This still allows 'level 0' interrupts to happen, most notably quadrature-Encoder Overflow!

PUBLIC uint32_t Port_IntsDisableCount; // count of performed disables

#define DISABLE_PORT_INT()    Port_IntsDisableCount++;\
      __set_BASEPRI( 1 << (8-ARM_INTERRUPT_LEVEL_BITS) );  //All SPI-device interrupts on Level 1, disable them (and lower)
#define ENABLE_PORT_INT()      if(--Port_IntsDisableCount==0) \
      __set_BASEPRI( 0 );      //If we get back to 'base level', enable all interrupts

where the header-flag 'PUBLIC' is set to empty ('') for the 'main' inclusion, set to 'extern' for all others.  This counter allows me to put-in pairs of disable/enable without having to worry whether somewhere in the 'preceding call tree' they were already disabled -- the re-enable macro won't effect an enable until we reach the 'outer level'.

1,707 Views
luimarma
Contributor III

Thank you for all the answers.

I have folowed the __set_BASEPRI approach. For anyone also interested there is a good document for the NVIC controler here. Just some piece of information is not correct, since the priority base level should be shifted depending on the priority bits used by the K64 (4 bits). A good reference on this can be found here.

0 Kudos