Cannot read an ADC in a timer ISR

[clivepalmer]

I am trying to read a few ADCs channels on a 4KHZ ish update rate from a timed interrupt.

I can read the ADC using my command line parser. I have also checked my TPM timer ISR and it works fine.

and hits the ISR every 244us (4KHZ = 250us)

The ADC has been set for a conversion time of 30uS. I have checked my calculation with the freescale ADC calculator and it should be correct.

I want to read 4 ADC channels (30us * 4 = 120us)

My ADC init routine is:-

void adc_init(void)
{

SIM_SCGC6 |= SIM_SCGC6_ADC0_MASK; //  enable clock

ADC0_CFG1 = ADC_CFG1_ADICLK(0)   // bus clock 24MHZ
     | ADC_CFG1_ADIV(1)     //ADCK = input clock/2
     | ADC_CFG1_MODE(1);     //12-bit mode

    ADC0_CFG2 &= ~(ADC_CFG2_MUXSEL_MASK  // channel A
             | ADC_CFG2_ADACKEN_MASK);  //asynchronous clock output disabled
    ADC0_CFG2 |= ADC_CFG2_ADHSC_MASK; //HI speed mode

    ADC0_SC2 &= ~ADC_SC2_ADTRG_MASK; // SW trigger
    ADC0_SC2 |= ADC_SC2_REFSEL(0); //voltage reference source VREFH&L

    ADC0_SC3 = ADC_SC3_ADCO_MASK  //continuous conversion
   | ADC_SC3_AVGE_MASK  //hardware average is enabled
   | ADC_SC3_AVGS(2);   //16 averages
    ADC0_SC1A = ADC_SC1_ADCH(31); // disable module
}

At the moment Im just calling a simple ADC read function.

unsigned int adc_read(uint8_t channel)      //for CHANNEL-A
{
   CRITICAL_SECTION_BEGIN;
   ADC0_SC1A = ADC_SC1_ADCH(31);           //disable adc

   ADC0_SC1A = channel;

   while((ADC0_SC1A & ADC_SC1_COCO_MASK) == 0){}; // wait for conversion complete int to clear
   CRITICAL_SECTION_END;
   return ADC0_RA;
}

Am I being dumb. Is there some good reason why I cannot read an ADC channel from my TPM ISR?

[clivepalmer]

oh forgot to add my timer init routine:-

This timer seems to work fine. I am using a Kinetis FRDM-KL02 PCBA

void timer1Init()
{
disable_irq(18); // TPM1 vector is 34
SIM_SCGC6 |= SIM_SCGC6_TPM1_MASK;  // enable clock for timer

SIM_SOPT7 |= SIM_SOPT7_ADC0TRGSEL(0x09); // TPM1 oveflow trigger source
/* ADC update rate */
TPM1_SC = 0;
TPM1_SC = TPM_SC_PS(0);
TPM1_MOD = 0x0008; // for 244us interrupt
TPM1_CNT = 0;
   TPM1_SC |= TPM_SC_TOIE_MASK|TPM_SC_CMOD(1); // NB Default to 'up-counting' mode.
   TPM1_C0SC = TPM_CnSC_MSB_MASK | TPM_CnSC_ELSB_MASK; // MSB:MSA 1:0 Edge Aligned mode (A defaults to 0)
   TPM1_C0SC |= TPM_CnSC_CHIE_MASK; //channel interrupt enable
   TPM1_C0V = 0x00;

   set_irq_priority (18, 3);
enable_irq(18); // TPM1 vector is 34
}

I have measured my ADC conversion time and now verified that it is 30uS.

So why will my timer ISR not perform an ADC conversion? I am trying to use SW trigger.

Help!!!!!!!!!!!!

[ndavies · Accepted solution]

Clive, Have you looked into the possibility that you are suffering from interrupt starvation or interrupt priority blocking.

The way your code is structured you will be spending 13 percent of the CPUs time in an interrupt ISR waiting for the ADC done bit to toggle. This is with just reading 1 ADC channel in the ISR. When reading 4 channels in the interrupt service routine, You will be spending 50% of the CPUs bandwidth in an ISR, waiting for the ADC done bit to toggle 4 times. I'm sure your program has other things it needs to get done in that time. Since you are in the interrupt, anything non-interrupt or at a lower interrupt priority will be blocked. Spending 50% or your time in 1 interrupt is a usually a problem.

You current code is

Every 244 uS read (timer ISR)

     Read 4 ADC channels. Each read takes 30uS Leading to 120uS spent in ISR

A better algorithm would be to also use the ADC done interrupt as well as the timer interrupt

Every 244 uS  (Timer ISR)

     Start Read of ADC channel 1

Every time the ADC done interrupt occurs (ADC done ISR)

     Read Data from previously started ADC read

     if (not last ADC channel)

          Start Next ADC channel read

With This improved algorithm you will not be stuck in an ISR for 50% of your CPUs available time. The interrupt switching time will be lower than the 120uS wasted waiting for the ADC done bit toggles.  A further improvement would be to use a hardware trigger to the ADC. The timer trigger would then use no CPU cycles. The ADC reads could also be DMA transfers also using no CPU cycles to take the reading. I use the K60 parts so I don't know what DMA sources are available on the KL02.

At 48Mhhz, That 30uS ADC wait loop has blocked the program for 1440 CPU clock cycles that  could have been used for other processing.

Norm

[clivepalmer]

Hi Norm,

Thanks for taking time to reply. I have another timer on a 1ms tick running. I was starting to draw the conclusion that it was stack related. I think you are right and appreciate your solution. I don't have any free GPIO resource to use a HW ADC interrupt.

[ndavies]

Hi Clive,

I had assumed you were using the internal ADC. You shouldn't need to use any GPIO to access the ADCs interrupt bit, if you are using the internal ADC. The internal ADCs conversion complete interrupt is routed internally directly to the NVIC. All you need to do is write and register the ISR. Turn on the interrupt enable bit in the ADC and enable the correct interrupt bit in the NVIC. 

[clivepalmer]

Hi Norm,

Thanks. I now have HW trigger using TPM1.

I would like to use DMA. Unfortunately the Kinetis L series KL02 has no DMA.

[jeremyzhou]

Hi Clive,

It's supported to use HW interrupt to trigger ADC operation.

I've attached a KL25 demo about using TPM as the hardware trigger source of ADC and I think it's worth to refer to even not based on KL02.

Hope this help.
Have a great day,
Ping

-----------------------------------------------------------------------------------------------------------------------
Note: If this post answers your question, please click the Correct Answer button. Thank you!
-----------------------------------------------------------------------------------------------------------------------

[clivepalmer]

Hi Ping,

Thanks for that. I must admit I'm a little confused with the HW triggering and I'm not sure I have it correct.

When my timer overflows and creates the HW ADC trigger, how do I tell the ADC ISR to start conversion on the next channel?

I was doing this in my ADC ISR.

ADC0_SC1A = ADC_SC1_AIEN_MASK | (ADC_SC1_ADCH(channel );

but this seems to be what you do when you perform a SW conversion. I don't understand the need for using ADC0_SC1B. Do you have do ping pong for HW trigger?

[jeremyzhou]

Hi Cilve,

I also found that you were a little confuse with the HW trigger and I'd like to suggest you'd better review the Hardware trigger and channel selects paragraph in RM careful.

If you want to use HW trigger to activate ADC sample differ channels, set differ chanels in ADC completion interrupt routine could work it out.

About the ping-pong mode, it's jsut one kind application of HW trigger.

Hope this help.

Have a good day!

Ping

[clivepalmer]

Hi Ping,

I have been through the RM many times. I have only got as far as I have by reading it. Maybe I'm being a bit dumb, but to me it is not obvious or clear from the RM on how I setup the next channel for HW triggering. For SW it is clear. You set with the next channel. E.g.

ADC0_SC1A = ADC_SC1_AIEN_MASK | (ADC_SC1_ADCH(channel );

So I'm stuck on this point. I have everything else working. I just need to know what I should do to setup the next channel for HW triggering.

[jeremyzhou]

Hi Clive,

When you select HW trigger, you're not allowed to configure a different channel until the last conversion complete which is as same as work in SW trigger mode.

So when a ADC conversion complete, execute ADC0_SC1A = ADC_SC1_AIEN_MASK | (ADC_SC1_ADCH(channel );  could set a different ADC channel.

I hope that you're clearly wiih t it now.
Have a great day,
Ping

-----------------------------------------------------------------------------------------------------------------------
Note: If this post answers your question, please click the Correct Answer button. Thank you!
-----------------------------------------------------------------------------------------------------------------------

[jeremyzhou]

Hi Clive,

I've created a demo to read channel 26、channel 29、channel 30 and the read function is set in TPM interrupt routine.

These ADC sample results are transfered to PC(Fig 1).

                                           Fig 1

So please check the attachment and if you have any futher questions about it, just feel free to contact with me.
Have a great day,
(my name)

-----------------------------------------------------------------------------------------------------------------------
Note: If this post answers your question, please click the Correct Answer button. Thank you!
-----------------------------------------------------------------------------------------------------------------------

[clivepalmer]

Hi Ping,

Thanks for taking the time to verify this for me. I think you have proved to me that there is no issue doing this and that my issue is elsewhere. I think Norm has hit the nail on the head.

I will report back the solution when I have it solved.

[jeremyzhou]

Hi Clive,

There's a contradication between your purpose and code design. As you mentioned before, you want read 4 ADC channels per 250us, then you configured the TPM overflow period to be 244us and read ead 4 ADC channels in TPM interrupt routine.

Because read one ADC channel only cost 30us, then read 4 ADC channel will cost about 120 us. It means it's available to read 4 ADC channels in in TPM interrupt routine.

However in timer1Init() function, you set TPM1 overflow as ADC hardware trigger and in fact, you also didn't set ADC0ALTTRGEN to 1 to enable the ADC0 trigger selection.

In my opinion, you needn't to set any hardware trigger sources of ADC, you could execute adc_read four times to read 4 ADC channels is fine.

You should give a shot.
Have a great day,
Ping

-----------------------------------------------------------------------------------------------------------------------
Note: If this post answers your question, please click the Correct Answer button. Thank you!
-----------------------------------------------------------------------------------------------------------------------

[clivepalmer]

Hi,

Thanks for the response. Yes I have designed it such that I can read all 4 in 120us. At the moment I'm keeping it simple and trying to use one. I have tried both HW and SW trigger but neither work when I read the ADC in the timer ISR. I was hoping to use just SW trigger and this was what I tried first but this does not seem to work. The setting for SOPT7 (timer overflow) shown in my timer init function was what I had when I was trying to see if I could get the code to work with HW trigger. When I was trying SW trigger this was not set. As you correctly pointed out my code snippet I pasted in is wrong. I have however tried both HW and SW trigger with what I believe was the correct settings, so I don't believe this is the issue.

As soon as I perform an ADC read the code just locks up and crashes. This is my issue.

If I am not mistaken TPM1 triggers selected ADC0 out of reset and should be set to 0 and I should not need to set ADC0ALTTRGEN to 1?

Any more thoughts? Thanks.

[jeremyzhou]

Hi Clive,

I'd like to suggest that you should achieve the design step by step.

First, you create a demo to read on ADC channel by use software trigger, then add a TPM routine to make interrupe happen per 244us and move the ADC read function in TPM interrupe routine.

There's no ADC demo code in FRDM-KL02 sample code, however the ADC module in Kinetis L series is same, so you can refer to FRDM_KL26ZDemo demo  in FRDM-KL26 sample code which include ADC sample operation.

Hope this help.

The link of FRDM-KL26 page as follow, then you can download the FRDM-KL26 sample code through.

FRDM-KL26Z: Freescale Freedom Development Platform for Kinetis KL16 and KL26 MCUs (up to 128 KB Flas...
Have a great day,
Ping

-----------------------------------------------------------------------------------------------------------------------
Note: If this post answers your question, please click the Correct Answer button. Thank you!
-----------------------------------------------------------------------------------------------------------------------

[clivepalmer]

Hi Ping,

I have already done what you suggested. I first got the TPM1 timer working correctly at 250us. Verified on a CRO by togging a GPIO line.

I then performed a successful ADC conversion on a channel. When I knew they both worked I then placed the ADC read into the timer ISR.

The moment I did this the code stopped working.

[clivepalmer]

To be more specific. Both the ADC and timer works fine independently in the same application. The moment I add just one line of code to the timer ISR to start a conversion the code stops running.

ADC0_SC1A = 0;

I don't see how the looking at the demo code will help me if I already have both the ADC reads and the timer working.

[jeremyzhou]

Hi Clive,

It's so weird.

Could you upload the TPM interrupe routine which inclue ADC read function please?
Have a great day,
Ping

-----------------------------------------------------------------------------------------------------------------------
Note: If this post answers your question, please click the Correct Answer button. Thank you!
-----------------------------------------------------------------------------------------------------------------------

[clivepalmer]

Hi Ping,

Sure. Here it is. I can read the ADC using adc_read in the main loop or from a command parser and it works fine. The moment I add the adc_read to the ISR as shown below it breaks.

void FTM1_IRQHandler (void)
{
      TPM1_SC |= TPM_SC_TOF_MASK;
      TPM1_C0V = 0x000F;
      TPM1_C0SC |= TPM_STATUS_CH0F_MASK;
       val = adc_read(0);     // This line breaks the code!
       //printf_always("\n ADC data: %x.\n", val);
       toggle_pps();
}

[bobpaddock]

It is unlikely printf is reentrant, it should never be used inside an interrupt.

It is usually unwise to call functions from inside IRQs as well.

I did not see your adc_read() code, it may have problems being called when inside an IRQ as printf has.

If your adc_read() code takes longer than to turn that the FTM1 IRQ happens.  This code will always crash due to a stack overflow.