ADC only working once on ATMEGA324PA

Question

I have some code which should read the values of a couple of ADC pins, each time around the commutator loop.

static uint16_t adc0;
static uint16_t adc1;

void init(void) {
    ...
    hw_configure_adcs();
    ...
}

void loop(void) {
    ...
    adc0 = hw_read_adc(0);
    adc1 = hw_read_adc(1);
    ...
}

void hw_configure_adcs(void) {
    ADCSRA = (1<<ADEN) | (1<<ADPS2) | (1<<ADPS0);
}

uint16_t hw_read_adc(uint8_t n) {
    ADMUX = (1<<REFS0) | (n & 0x07);
    ADCSRA |= (1<<ADSC); // start conversion
    uint16_t count;
    for (count = 0; !(ADCSRA & (1<<ADIF)); count++); // wait for conversion to complete
    // ADCSRA |= (1<<ADIF); // tried with and without this
    return (ADCH << 8) | ADCL; // return ADC value
}

What I see is odd: The values of adc0 and adc1 are set to the same value and never change, until the AVR chip is restarted/reflashed.

(The value is 0x00d1 at 0.71V and 0x0128 at 1.00V, which seems reasonable.)

I have tried:

• Turning the voltage down: adc0 and adc1 stay constant and only go down when the AVR code is reflashed (and hence the chip restarted).
• Turning the voltage up: adc0 and adc1 stay constant and only go up when the AVR code is reflashed (and hence the chip restarted).
• Returning count from hw_read_adc() instead of the ADC value: This returns varying numbers between 0x34 and 0x38 which are different for the two ADCs and continuously vary over time.

From these tests, I infer that the ADCs are being read, but that I am missing some "clear ADCH and ADCL and get them ready to accept the new reading" step.

I have re-read section 23 of http://www.atmel.com/images/Atmel-8272-8-bit-AVR-microcontroller-ATmega164A_PA-324A_PA-644A_PA-1284_P_datasheet.pdf many times, but have obviously overlooked something vital.

Answer 1

After much googling, I found: http://www.avrfreaks.net/forum/adc-only-happens-once-reset

The problem was that return (ADCH << 8) | ADCL; was compiled so that it read the high register first (as you might expect).

Page 252 of the datasheet says: "Otherwise, ADCL must be read first, then ADCH".

Changing my code to return ADC fixed the problem.

My guess as to what was happening is:

• The read from ADCH occurred.
• The read from ADCL had the effect of locking the ADC-result, to prevent tearing.
• The next ADC read had nowhere to write its result, as the ADC-result was locked.
• Repeat...

Answer 2

The Problem with your code is you read ADCH first.

ADCL must be read first, then ADCH, to ensure that the content of the Data Registers belongs to the same conversion. Once ADCL is read, ADC access to Data Registers is blocked. This means that if ADCL has been read, and a conversion completes before ADCH is read, neither register is updated and the result from the conversion is lost. When ADCH is read, ADC access to the ADCH and ADCL Registers is re-enabled.

So the correct code should be-

return ADCL | (ADCH << 8) ;