Strange behaviour of Attiny's ADC

Question

#include <avr/io.h>
#include <util/delay.h>

void init(){
    // LED output.
    DDRB |= 1<<4;
    // Turn on ADC on third pin, continuous mode, prescaler 128.
    ADMUX |= (1<<MUX0) | (1<<MUX1);
    ADCSRA |= (1<<ADEN) | (1<<ADSC) | (1<<ADATE) | (1<<ADPS2) | (1<<ADPS1) | (1<<ADPS0);
}

int main(){
    init();

    uint16_t adc = 0;
    while(1){
        adc = (ADCH << 8) | ADCL;
        // adc = (adc + 1) & 1023;
        // _delay_ms(1);

        if(adc > 512){
            PORTB |= 1<<4;
        }
        else{
            // PORTB = 0;
            PORTB &= ~(1<<4);
        }
    }
}

This is the code I have running on my ATtiny13A. There is an LED connected to pin B4, and potentiometer to B3. The code is supposed to turn on the LED if the analog value is greater than 512 (half of maximum 1024). It doesn't work though, and LED seems to be disabled all the time.

Normally I would assume there is something wrong in configuration of peripherals, but there are some very strange things happening. For example, if I change PORTB &= ~(1<<4); to PORTB = 0 (which in this case should do exactly the same thing), the code magically starts working just as expected. Another thing I tried is changing LED status using simple counter by uncommenting the two lines near _delay_ms - and again, LED starts blinking as expected.

The only time when this code does not work is when it's in exactly this form - so there must be something wrong with combination of ADC reading and output port bit writing.

Why does this happen?

Answer 1

1. Incorrect 16 bit register access pattern

   When ADCL is read, the ADC Data Register is not updated until ADCH is read.

In C, the evaluation order of the | operator is undefined, so this code is could be reading ADCH first, and then reading ADCL. Once you read ADCL then you have locked the data register until the next pass though the loop, which is certainly not what you want.

Try changing this line to...

adc = ADC;

(the C compiler should generate the correct access order coding).

2. Reading ADC while it is being updated.

This diagram strongly suggests that the ADC register is updating up until ADIF goes high...

I'd expect that new bits are shifted into place with each successive approximation, and the extra delay of the nop could be enough so that the ADC is ready by the time you hit it just by chance.

Try replacing...

adc = (ADCH << 8) | ADCL;

...with...

while (! ADCSRA & _BV(ADIF) );
adc = ADC;
ADCSRA |= _BV( ADIF );

...and see if that resolves the timing issue.

Report back if this fixes the problem. If not, we will move on to next try!