MSP432P401R: Initialize ADC MSP432, sanity check

Robert Cachro

Part Number: MSP432P401R

Hello folks,

I am looking for a sanity check on my settings of the ADC for the MSP432. If someone could please confirm everything below looks correct (without any blatant bugs that could cause jumpy ADC readings) I would be very grateful. I call initialize_adcs() function at the beginning of the program once, prioritize ADC14_IRQHandler() ISR as shown below.

    MAP_Interrupt_setPriority(FAULT_SYSTICK, 0x10); 
    MAP_Interrupt_setPriority(INT_PORT1, 0x20); 
    MAP_Interrupt_setPriority(INT_ADC14, 0x20);

I then call the following once, prior to the main while() loop:

		ADC14->CTL0 |= ADC14_CTL0_ENC |  		/*!< ADC14 enable conversion - same as MAP_ADC14_toggleConversionTrigger(); */
				ADC14_CTL0_SC;					/*!< ADC14 start conversion */

I am reading ADC 0-13 and pull the internal temperature sensor info which required some unclear modification... (which is mostly why I am concerned regarding the settings below)

void initialize_adcs(void)
{

    // Initialize the shared reference module. By default, REFMSTR=1 => REFCTL is used to configure the internal reference
    while(REF_A->CTL0 & REF_A_CTL0_GENBUSY);// If ref generator busy, WAIT

    REF_A->CTL0 |= REF_A_CTL0_VSEL_0 |      // Enable internal 1.2V reference
            REF_A_CTL0_ON;                  // Turn reference on
    REF_A->CTL0 |= REF_A_CTL0_ON;           // Turn reference on
    REF_A->CTL0 &= ~REF_A_CTL0_TCOFF;       // Enable temperature sensor

	// Turn on ADC14, extend sampling time to avoid overflow of results
	ADC14->CTL0 = ADC14_CTL0_ON |		/*!< ADC14 on */
			ADC14_CTL0_MSC |			/*!< ADC14 multiple sample and conversion */
			ADC14_CTL0_SHT0__192 |		/*!< 192 */
			ADC14_CTL0_SHP |			/*!< ADC14 sample-and-hold pulse-mode select */
			ADC14_CTL0_CONSEQ_3;		/*!< Repeat-sequence-of-channels */

    ADC14->CTL1 |= ADC14_CTL1_TCMAP;        // Enable internal temperature sensor

	ADC14->MCTL[0] = ADC14_MCTLN_INCH_0;    // ref+=AVcc, channel = A0
	ADC14->MCTL[1] = ADC14_MCTLN_INCH_1;    // ref+=AVcc, channel = A1
	ADC14->MCTL[2] = ADC14_MCTLN_INCH_2;    // ref+=AVcc, channel = A2
	ADC14->MCTL[3] = ADC14_MCTLN_INCH_3;	// ref+=AVcc, channel = A3
	ADC14->MCTL[4] = ADC14_MCTLN_INCH_4;	// ref+=AVcc, channel = A4
	ADC14->MCTL[5] = ADC14_MCTLN_VRSEL_1 | ADC14_MCTLN_INCH_22; // ADC input ch A22 => temp sense
//	ADC14->MCTL[5] = ADC14_MCTLN_INCH_5;	// original code without temperature sensor
	ADC14->MCTL[6] = ADC14_MCTLN_INCH_6;	// ref+=AVcc, channel = A6
	ADC14->MCTL[7] = ADC14_MCTLN_INCH_7;	// ref+=AVcc, channel = A7
	ADC14->MCTL[8] = ADC14_MCTLN_INCH_8;	// ref+=AVcc, channel = A8
	ADC14->MCTL[9] = ADC14_MCTLN_INCH_9;	// ref+=AVcc, channel = A9
	ADC14->MCTL[10] = ADC14_MCTLN_INCH_10;	// ref+=AVcc, channel = A10
	ADC14->MCTL[11] = ADC14_MCTLN_INCH_11;	// ref+=AVcc, channel = A11
	ADC14->MCTL[12] = ADC14_MCTLN_INCH_12;	// ref+=AVcc, channel = A12
	ADC14->MCTL[13] = ADC14_MCTLN_INCH_13|  // ref+=AVcc, channel = A13
			ADC14_MCTLN_EOS;

	ADC14->IER0 = ADC14_IER0_IE13;           // Enable ADC14IFG.7

	while(!(REF_A->CTL0 & REF_A_CTL0_GENRDY));	// Wait for reference generator to settle

	__enable_interrupt();
	NVIC->ISER[0] = 1 << ((ADC14_IRQn) & 31); // Enable ADC interrupt in NVIC module

}

/* ADC14 interrupt service routine */
void ADC14_IRQHandler(void) {
	if (ADC14->IFGR0 & ADC14_IFGR0_IFG13)
	{
		A0results[0] = ADC14->MEM[0];
		y1_sim = A0results[0];
		A0results[1] = ADC14->MEM[1];
		A0results[2] = ADC14->MEM[2];
		A0results[3] = ADC14->MEM[3];
		y2_sim = A0results[3];
		A0results[4] = ADC14->MEM[4];
		A0results[5] = ADC14->MEM[5];
		temp = A0results[5];
		lift_off1[0] = ADC14->MEM[6];
		lift_off1[1] = ADC14->MEM[7];
		lift_off1[2] = ADC14->MEM[8];
		lift_off2[0] = ADC14->MEM[9];
		lift_off2[1] = ADC14->MEM[10];
		lift_off2[2] = ADC14->MEM[11];
		A0results[12] = ADC14->MEM[12];
		A0results[13] = ADC14->MEM[13];
	}
}

I am most concerned about ADC values jumping for no reason which could trigger unwanted effects in our system.

Thank you,

Bob

over 7 years ago

0 Amit Ashara over 7 years ago

TI__Guru**** 244380 points

Hello Bob,

The SimpleLink SDK is trying to move from the register operation to more of an API operation. As an example, please refer to the SDK examples under

C:\ti\simplelink_msp432p4_sdk_1_60_00_12\examples\nortos\MSP_EXP432P401R\driverlib

Using API's would help us a lot to reproduce issues and at the same time for you to avoid initialization issues.

0 Robert Cachro over 7 years ago in reply to Amit Ashara

Intellectual 845 points

Thanks Amit,

Unfortunately, we are too close too production launch and have done considerable reliability testing to make any major changes. The project started with the MSP432 as an experimental micro and we know all too well the risk associated with updating to API commands from registers on the fly.. don't want any unknown repercussions. Not looking for a super detailed review, just any blatant bugs or issues with regards to ADC ISR prioritization, ISR, etc. I am not saying there is any issue with code posted above. Just a sanity check.

Best,
Bob

0 Amit Ashara over 7 years ago in reply to Robert Cachro

TI__Guru**** 244380 points

Hello Bob,

Can you please share the configuration of the channels IO's as well. Please do note that we may not be able to provide every possible configuration testing due to the fact that we have just the LaunchPad.

0 Robert Cachro over 7 years ago in reply to Amit Ashara

Intellectual 845 points

Hi Amit,

Currently the IO's pertaining to ADC are set as follows:

	/* Set ADC readings as input to read with ADC */ 
	MAP_GPIO_setAsInputPin(GPIO_PORT_P4, GPIO_PIN2); 
	MAP_GPIO_setAsInputPin(GPIO_PORT_P4, GPIO_PIN3);
	MAP_GPIO_setAsInputPin(GPIO_PORT_P4, GPIO_PIN4); 
	MAP_GPIO_setAsInputPin(GPIO_PORT_P4, GPIO_PIN5);
	MAP_GPIO_setAsInputPin(GPIO_PORT_P4, GPIO_PIN6);
	MAP_GPIO_setAsInputPin(GPIO_PORT_P4, GPIO_PIN7);
	MAP_GPIO_setAsInputPin(GPIO_PORT_P5, GPIO_PIN2);
	MAP_GPIO_setAsInputPin(GPIO_PORT_P5, GPIO_PIN5);

	/* SET ADC PINS TO LOW SO RESIDUAL VOLTAGE DOES NOT AFFECT ADC READINGS */
	MAP_GPIO_setAsOutputPin(GPIO_PORT_P4, GPIO_PIN0);
	MAP_GPIO_setAsOutputPin(GPIO_PORT_P4, GPIO_PIN1);
	MAP_GPIO_setAsOutputPin(GPIO_PORT_P5, GPIO_PIN0);
	MAP_GPIO_setAsOutputPin(GPIO_PORT_P5, GPIO_PIN1);
	MAP_GPIO_setAsOutputPin(GPIO_PORT_P5, GPIO_PIN3);
	MAP_GPIO_setAsOutputPin(GPIO_PORT_P5, GPIO_PIN4);
	MAP_GPIO_setOutputLowOnPin(GPIO_PORT_P4, GPIO_PIN0);
	MAP_GPIO_setOutputLowOnPin(GPIO_PORT_P4, GPIO_PIN1);
	MAP_GPIO_setOutputLowOnPin(GPIO_PORT_P5, GPIO_PIN0);
	MAP_GPIO_setOutputLowOnPin(GPIO_PORT_P5, GPIO_PIN1);
	MAP_GPIO_setOutputLowOnPin(GPIO_PORT_P5, GPIO_PIN3);
	MAP_GPIO_setOutputLowOnPin(GPIO_PORT_P5, GPIO_PIN4);

As you can see, not all ADC channels 0-13 are used for actual readings. There was intentional space left between certain ADC pins which we found were causing a sort of "residual capacitance" noise on the ADC pins right next to each other.

Schematic below shows nodes which are reading ADC signals.

Prior to all of these settings, we initialized the IOs as follow:

    // Terminate all remaining pins on the device
//	P1DIR |= 0xFF; P1OUT = 0;
    P2DIR |= 0xFF; P2OUT = 0;
    P3DIR |= 0xFF; P3OUT = 0;
    P4DIR |= 0xFF; P4OUT = 0;
    P5DIR |= 0xFF; P5OUT = 0;
    P6DIR |= 0xFF; P6OUT = 0;
    P7DIR |= 0xFF; P7OUT = 0;
    P8DIR |= 0xFF; P8OUT = 0;
    P9DIR |= 0xFF; P9OUT = 0;
    P10DIR |= 0xFF; P10OUT = 0;

Thank you!

0 Amit Ashara over 7 years ago in reply to Robert Cachro

TI__Guru**** 244380 points

Hello Robert,

First of all sorry for the delayed response. I had to change part of the code to meet up my LaunchPad requirements and had to assume data variables. The code has one problem, i.e. the interrupt source is not being cleared in the ISR causing it to become a re-entrant function and the memory to be overwritten.

0 Robert Cachro over 7 years ago in reply to Amit Ashara

Intellectual 845 points

Hello Amit,

Many thanks for your review. Can you please elaborate on this problem? The design intent was to update the ADC variables (lift_off1, A0results[X], etc) within the ADC ISR as fast as possible for the main loop to process. So as long as it updates the variables with the new ADC value then I am okay. Technically if it did something like:

(uint16_t) old_ADC_value -> (uint16_t) 0 (for a short period of time) -> (uint16_t) new_ADC value

then that would be of concern. But I believe it should just be replacing the all of the value directly and then jumping back to the main loop.

old_ADC_value -> new_ADC value

Am I understanding this correctly or is there something else happening? Perhaps I should only be calling this ADC ISR once per main loop.

Best,
Robert

0 Amit Ashara over 7 years ago in reply to Robert Cachro

TI__Guru**** 244380 points

Hello Robert

I am still unclear why the fastest loop is required, If the intent of the test is to check the ADC inputs

0 Robert Cachro over 7 years ago in reply to Amit Ashara

Intellectual 845 points

After reviewing the implementation, I guess it's not necessarily required to run it as fast as I can. So I tested a modification to initialize_adc()

Changed from

ADC14_CTL0_CONSEQ_3 ((uint32_t)0x00060000) /*!< Repeat-sequence-of-channels */

ADC14_CTL0_CONSEQ_1 ((uint32_t)0x00020000) /*!< Sequence-of-channels */

and then I called:

		ADC14->CTL0 |= ADC14_CTL0_ENC |  		/*!< ADC14 enable conversion - same as MAP_ADC14_toggleConversionTrigger(); */
				ADC14_CTL0_SC;					/*!< ADC14 start conversion */

at the beginning of my main loop to take the measurement prior to each calculation.

Is this what you recommend? The other implementation isn't necessarily wrong though is it? The reason it was implemented like that was because I did not want to call the enable ADC conversion command above every time an updated set of ADC value was needed within the main loop and within a timer ISR routine.

I guess I don't understand what you mean by: i.e. the interrupt source is not being cleared in the ISR causing it to become a re-entrant function and the memory to be overwritten. How do I clear the ADC ISR?

Best,

Robert

0 Amit Ashara over 7 years ago in reply to Robert Cachro

TI__Guru**** 244380 points

Hello Robert

Yes, that is what I recommended since the intent was to test the Analog Pins.

As for the second part of the last post, the interrupt source is not being cleared in the Interrupt Handler (ISR) for the ADC. You must write to the ADC14CLRIFGR0 or ADC14CLRIFGR1 registers based on what the interrupt conditions are.

0 Robert Cachro over 7 years ago in reply to Amit Ashara

Intellectual 845 points

Hi Amit,

Referencing MSP432 example "msp432p401x_adc14_06.c" under the MSP432P4xx Code Examples.

The ADC ISR routine below:

// ADC14 interrupt service routine
void ADC14_IRQHandler(void)
{
    if (ADC14->IFGR0 & ADC14_IFGR0_IFG3)
    {
        A0results[index] = ADC14->MEM[0];   // Move A0 results, IFG is cleared
        A1results[index] = ADC14->MEM[1];   // Move A1 results, IFG is cleared
        A2results[index] = ADC14->MEM[2];   // Move A2 results, IFG is cleared
        A3results[index] = ADC14->MEM[3];   // Move A3 results, IFG is cleared
        index = (index + 1) & 0x7;          // Increment results index, modulo
        __no_operation();                   // Set Breakpoint1 here
    }
}

does not clear the ISR as you mentioned which is what I based my code off of.

Is this incorrect? I always assumed you had to clear the ISR..

Is it sufficient to do the following:

/* ADC14 interrupt service routine */
void ADC14_IRQHandler(void) {

	uint64_t status;
    status = MAP_ADC14_getEnabledInterruptStatus();
    MAP_ADC14_clearInterruptFlag(status);

	if (status & ADC14->IFGR0 & ADC14_IFGR0_IFG13)
	{
		A0results[0] = ADC14->MEM[0];
		y1_sim = A0results[0];
		A0results[1] = ADC14->MEM[1];
		A0results[2] = ADC14->MEM[2];
		A0results[3] = ADC14->MEM[3];
		y2_sim = A0results[3];
		A0results[4] = ADC14->MEM[4];
		A0results[5] = ADC14->MEM[5];
		temp = A0results[5];
		lift_off1[0] = ADC14->MEM[6];
		lift_off1[1] = ADC14->MEM[7];
		lift_off1[2] = ADC14->MEM[8];
		lift_off2[0] = ADC14->MEM[9];
		lift_off2[1] = ADC14->MEM[10];
		lift_off2[2] = ADC14->MEM[11];
		A0results[12] = ADC14->MEM[12];
		A0results[13] = ADC14->MEM[13];
	}
}

Best,

Robert

0 Amit Ashara over 7 years ago in reply to Robert Cachro

TI__Guru**** 244380 points

Hello Robert,

The first code does not write to the ADC14IFGCLRx register while the second routine does. The example you are referring to does not seem to be from the SimpleLink MSP432P4 SDK.

0 Robert Cachro over 7 years ago in reply to Amit Ashara

Intellectual 845 points

No the first example is from MSP432Ware. Does that mean that it is correct since we are using MSP432Ware? I will go ahead and clear it via the simple second example.

Amit thank you for all of your help throughout the process. Our team here very much appreciates it.

Wishing you the best in 2018!

Regards,

Robert

0 Amit Ashara over 7 years ago in reply to Robert Cachro

TI__Guru**** 244380 points

Hello Robert,

I have not used the MSP432Ware but only the SimpleLink SDK. I do expect that the MSP432Ware to be correct, but I cannot say it with certainty since time and again we see issues with our example code when put in an actual implementation. However when reported, we do correct it. Since the software platform going forwards is the SDK, any update to MSP432Ware may not be possible.

0 Robert Cachro over 7 years ago in reply to Amit Ashara

Intellectual 845 points

Hi Amit,

I have attempted to clear the ADC ISR and came across an issue and now I am unsure as to what is correct.

The following code works (notice if loop)

/* ADC14 interrupt service routine */
void ADC14_IRQHandler(void) {

	uint64_t status;
    status = MAP_ADC14_getEnabledInterruptStatus();
    MAP_ADC14_clearInterruptFlag(status);

	if (status & ADC14_IFGR0_IFG13)
	{
		A0results[0] = ADC14->MEM[0];
		y1_sim = A0results[0];
		A0results[1] = ADC14->MEM[1];
		A0results[2] = ADC14->MEM[2];
		A0results[3] = ADC14->MEM[3];
		y2_sim = A0results[3];
		A0results[4] = ADC14->MEM[4];
		A0results[5] = ADC14->MEM[5];
		temp = A0results[5];
		lift_off1[0] = ADC14->MEM[6];
		lift_off1[1] = ADC14->MEM[7];
		lift_off1[2] = ADC14->MEM[8];
		lift_off2[0] = ADC14->MEM[9];
		lift_off2[1] = ADC14->MEM[10];
		lift_off2[2] = ADC14->MEM[11];
		A0results[12] = ADC14->MEM[12];
		A0results[13] = ADC14->MEM[13];
	}
}

However the following does not:

/* ADC14 interrupt service routine */
void ADC14_IRQHandler(void) {

	uint64_t status;
    status = MAP_ADC14_getEnabledInterruptStatus();
    MAP_ADC14_clearInterruptFlag(status);

	if (status & ADC14->IFGR0 & ADC14_IFGR0_IFG13)
	{
		A0results[0] = ADC14->MEM[0];
		y1_sim = A0results[0];
		A0results[1] = ADC14->MEM[1];
		A0results[2] = ADC14->MEM[2];
		A0results[3] = ADC14->MEM[3];
		y2_sim = A0results[3];
		A0results[4] = ADC14->MEM[4];
		A0results[5] = ADC14->MEM[5];
		temp = A0results[5];
		lift_off1[0] = ADC14->MEM[6];
		lift_off1[1] = ADC14->MEM[7];
		lift_off1[2] = ADC14->MEM[8];
		lift_off2[0] = ADC14->MEM[9];
		lift_off2[1] = ADC14->MEM[10];
		lift_off2[2] = ADC14->MEM[11];
		A0results[12] = ADC14->MEM[12];
		A0results[13] = ADC14->MEM[13];
	}
}

The difference in the if statement within the ISR seems to be causing the issue. It only works if I drop:

ADC14->IFGR0

Regarding logic, is it okay to remove this ? Apparently the above IFGR0 represents /*!< Interrupt Flag 0 Register */

I tried to mix the MSP432Ware sample code shown previously with your advice to clear the ADC ISR flag without success unless I do this.

Please advise.

Regards,

Robert

0 Amit Ashara over 7 years ago in reply to Robert Cachro

TI__Guru**** 244380 points

Hello Robert,

When executing the statement:

if (status & ADC14->IFGR0 & ADC14_IFGR0_IFG13)

The MAP_ADC14_clearInterruptFlag has already cleared the interrupt flag, hence ADC14->IFGR0 will return a 0 causing the "&" to be evaluated as a 0 and hence the if loop shall not be executed.

0 Robert Cachro over 7 years ago in reply to Amit Ashara

Intellectual 845 points

Thanks Amit,

That is quite confusing, why would the example from MSP432Ware be written like that in the first place?

if (ADC14->IFGR0 & ADC14_IFGR0_IFG13)

It seems like the interrupt flag was not supposed to be cleared... Could it have something to do with the fact that the ADC was set to repeat a sequence-of-channels?

ADC14_CTL0_CONSEQ_3                      ((uint32_t)0x00060000)          /*!< Repeat-sequence-of-channels */

It seems like you believe this is the correct way to implement?

/* ADC14 interrupt service routine */
void ADC14_IRQHandler(void) {

	uint64_t status;
	status = MAP_ADC14_getEnabledInterruptStatus();
    MAP_ADC14_clearInterruptFlag(status);

	if (status & ADC14_IFGR0_IFG13)
	{
		A0results[0] = ADC14->MEM[0];
		y1_sim = A0results[0];

...
...

Best,

Robert

0 Amit Ashara over 7 years ago in reply to Robert Cachro

TI__Guru**** 244380 points

Hello Robert,

Absolutely. The last code that you have shown is the correct method to implement the ISR.

0 Robert Cachro over 7 years ago in reply to Amit Ashara

Intellectual 845 points

Okay understood, thank you.

One final question regarding the SDK:

In an example in MSP432 SDK. "adc14_single_conversion_repeat.c", the following is implemented as the ISR.

 //![Single Sample Result]
/* ADC Interrupt Handler. This handler is called whenever there is a conversion
 * that is finished for ADC_MEM0.
 */
void ADC14_IRQHandler(void)
{
    uint64_t status = MAP_ADC14_getEnabledInterruptStatus();
    MAP_ADC14_clearInterruptFlag(status);

    if (ADC_INT0 & status)
    {
        curADCResult = MAP_ADC14_getResult(ADC_MEM0);
        normalizedADCRes = (curADCResult * 3.3) / 16384;

        MAP_ADC14_toggleConversionTrigger();
    }
}

Within this ISR, the toggleConverstionTrigger() function is called near the end of the routine. This is obviously done because they are setting the sample timer to ADC_MANUAL_ITERATION, (single sample mode configuration) and a call to ADC14_toggleConversionTrigger() is required at the end of the if loop for the repea to occur. However this seems to me a very roundabout way of performing a repeat of a single channel. Why not just set it to ADC_AUTOMATIC_ITERATION method? Seems odd that the manual method would be used...

Best,

Robert

0 Amit Ashara over 7 years ago in reply to Robert Cachro

TI__Guru**** 244380 points

Hello Robert,

I am not sure what the intent was, however having examples as a framework may have been the driving factor for using the call for MAP_ADC14_toggleConversionTrigger.

0 Robert Cachro over 7 years ago in reply to Amit Ashara

Intellectual 845 points

Understood, thank you Amit.
Your help is much appreciated.

Regards,
Robert

**Attention** This is a public forum

MSP low-power microcontrollers

MSP low-power microcontroller forum

MSP432P401R: Initialize ADC MSP432, sanity check