Let's have an AVR with ADC, say ATMega128. Let's have written an ADC ISR which re-triggers the ADC, and let's initialise the ADC for single-conversion and enable the interrupt. That should yield us a continuously running AD conversion, isn't it.
Now if we want to exchange some variables between main and the ISR, a well-behaving main won't temporarily disable the interrupts globally (through sei/cli), but only the ADC interrupt.
How to do that properly?
Jan Waclawek
Surely just clearing ADIE temporarily will do what you want. You could alse clear ADEN.
But presumably this is a trick question.
David.
Surely just clearing ADIE temporarily will do what you want.
You could also clear ADEN.
But presumably this is a trick question.
Jan: I believe the datasheet says that the ADIF flag is set regardless of whether ADIE is set or not. Thus, if you have disable ADIE but the ADC completes, the ADIF flag is set and when you enable ADIE later, the interrupt will be serviced.
I actually use this concept in controlling access to my SPI controller. I have several SPI DACs tied to my main processor along with a slave AVR. The protocol to the DACs is simple, while the slave protocol is very complex and can take some time (many SPI transactions). I allow the DAC code to "hold off" the slave by temporarily turning off the slave's interrupt line, doing the DAC transaction, then releasing the slave interrupt. It works remarkable well -- I never "lose" a slave interrupt while the DAC controls work just fine.
Stu
Stu San,
In fact, my only purpose is atomicity of access to variables. Particularly, in the interrupt, I read out the ADC value and store it in an uint16_t variable (volatile, of course, and an array of them, as I switch the MUX in the interrupt), and then I would like to read it out in the main - as it is a 2-byte value, atomicity is an issue.
However, you are of course right, there might be other valid reasons to selectively disable the ADC interrupt, eg. to delay its execution for some reason - I did not think about that previously.
The "trickiness" in the question is not related to interrupt-related flags as a concept, nor ADC as such, but to the particular implementation of the ADC "machine" in AVRs.
David,
Surely just clearing ADIE temporarily will do what you want.
You could alse clear ADEN.
But presumably this is a trick question.
So, how to clear ADIE temporarily *properly*?
Jan
I can see where apps that want to reduce jitter in (say) bit-banged PWM out of timer interrupts would want detailed precautions, but I've never found the need over the years and just do CLI/SEI pairs around each atomic read/write.
I assume the trick part has to do with ADIF being in the same I/O register as ADIE? Thus, this old bit-pusher doesn't strain the brain but just does the CLI/SEI.
Lee
Now if we want to exchange some variables between main and the ISR, a well-behaving main won't temporarily disable the interrupts globally (through sei/cli), but only the ADC interrupt.
I disagree :!:
The right programming style was always using only cli/sei for every atomic access. :idea:
The reason was, that disabling only one interrupt may cause priority inversion.
With using cli/sei the disable time was always limited to about 4 cycle. :idea:
But with disabling only one interrupt, all other interrupts may stretch the disabling time many times more, maybe 10000 cyle or more. :twisted: :twisted:
Peter
wek wrote:Now if we want to exchange some variables between main and the ISR, a well-behaving main won't temporarily disable the interrupts globally (through sei/cli), but only the ADC interrupt.
I disagree :!:
The right programming style was always using only cli/sei for every atomic access. :idea:
The reason was, that disabling only one interrupt may cause priority inversion.
With using cli/sei the disable time was always limited to about 4 cycle. :idea:
But with disabling only one interrupt, all other interrupts may stretch the disabling time many times more, maybe 10000 cyle or more. :twisted: :twisted:
And, I reserve to myself the right not to conform to any "right" programming style, wherever I find it appropriate. It's my gun, my bullets, and my foot...
Anyway, this is not what the quiz is about.
Jan
It's my gun, my bullets, and my foot...
And what? What if my hand-made timer-interrupt-based PWM (courtesy of Lee) can't stand 4 cycles of jitter
And, I reserve to myself the right not to conform to any "right" programming style, wherever I find it appropriate. It's my gun, my bullets, and my foot...
wek wrote:Then you can't enable the ADC interrupt anyway. ;-)And what? What if my hand-made timer-interrupt-based PWM (courtesy of Lee) can't stand 4 cycles of jitter
That's fully correct. :!:
Already an empty interrupt stall all other interrupts for at least 10 cycle.
If a 4 cycle jitter can not be tolerated, there exist only one solution: let no other interrupts enabled at all.
But also then e.g. every RET instructon cause a jitter of 4 cycle.
The best approach was always, make all interrupts and all atomic access as short as possible.
Also inside a timer interrupt it's easy to compensate a small jitter, since the timer register contain the exact delay value until the interrupt was handled.
Peter
Quote:But then I would not call it "well-behaving".And, I reserve to myself the right not to conform to any "right" programming style, wherever I find it appropriate. It's my gun, my bullets, and my foot...
I agree.
"well-behaving" was an approach, which covers about 99% of tasks.
If a special solution was needed for the remaining 1%, it should never be named as "well-behaving".
Peter
OK. Once again: this is not what the quiz is about.
And, I don't really care what you call "well behaving" - that's your arbitrary definition against my arbitrary definition.
But, if you want a reason for selectively disabling interrupts, again, assume, you need to make a more substantial operation, say, copy an array of values collected in previous interrupts.
However, if you want to discuss the merits of using selective or global interrupt disables, please, start a new thread.
JW
Once again: this is not what the quiz is about.
Your question was already solved on the second post (ADIE-bit).
Thus I wanted to add, that it may be used only, if you know exactly, what you are doing.
Since typically the "stupid" cli/sei approach was more safe (less side effects), faster and less code. :idea:
On every write to ADIE, you must watch, that ADIF was set to 0, otherwise a pending interrupt was lost.:!:
So "CBI ADCSRA, ADIE", "SBI ADCSRA, ADIE" can not be used.
Peter
Your question was already solved on the second post (ADIE-bit).
JW
No, that's not enough. The question is, *how* to disable ADIE temporarily.
The ADIE can never be disabled, it can be accessed always.
The ADIE disable/enable only the ADC-interrupt.
The function of the ADC itself was unaffected by ADIE.
Peter
ADCSRA &= ~(1<<ADIE);will surely generate an atomic CBI won't it?
Yes, that's the pitfall.
Peter
What "pitfall"? What's wrong with a single opcode that only affects one bit? Or are you talking about the older models of AVR where this was a read-modify-write?
What "pitfall"? What's wrong with a single opcode that only affects one bit? Or are you talking about the older models of AVR where this was a read-modify-write?
I assume, it's related to all AVRs.
E.g. ATmega1280:
"Beware that if doing a Read-Modify-
Write on ADCSRA, a pending interrupt can be disabled. This also applies if the SBI and CBI
instructions are used."
Peter
What "pitfall"? What's wrong with a single opcode that only affects one bit?
Or are you talking about the older models of AVR where this was a read-modify-write?
And both Peter and Lee hinted towards the answer already (although I believe there might be one more similar issue) - I just would like to see it written down as a routine.
JW
(PS. Yes, Peter, it's not temporarily ADIE *disable* but temporarily ADIE *clear*, I deserve to be slapped for not being precise enough :-) ).
Ah I see - a conundrum indeed - presumably this was the whole point of Jan's post? I guess there isn't a solution then without potentially losing one pending conversion then? (apart from the already suggested full CLI/SEI thing)
Ah I see - a conundrum indeed - presumably this was the whole point of Jan's post?
I guess there isn't a solution then without potentially losing one pending conversion then? (apart from the already suggested full CLI/SEI thing)
JW
clawson wrote:What "pitfall"? What's wrong with a single opcode that only affects one bit? Or are you talking about the older models of AVR where this was a read-modify-write?I assume, it's related to all AVRs.
E.g. ATmega1280:
"Beware that if doing a Read-Modify-
Write on ADCSRA, a pending interrupt can be disabled. This also applies if the SBI and CBI
instructions are used."Peter
It appears he got us - although it's all Atmel's fault ;-) .
A quick look into the newest Tiny datasheets ('T4-5-9-10 and 'T261-461-861 Automotive), under the SFR table at the end, footnote 3:
Note that, unlike most other AVRs, the CBI and SBI
instructions will only operation the specified bit, and can therefore be used on registers containing such Status Flags.
The similarly dated ATmega164PA/324PA/644PA datasheet still warns about the not-only-one-bit problem.
Jan
Well to be honest, from previous discussion here I thought the change of CBI/SBI to true bit ops actually dated much earlier and affected more devices. I was surprised when Peter said the 1280 still had this warning for example. But I just looked at the 48PA datasheet which must surely be one of the very newest devices and it still has the SBI/CBI warning.
OTOH, knowing the cut/paste nature of Atmel datasheets I wonder if this is an old warning that's just a hangover from the past in some of the newer models?
If simulator 2 really is as accurate as they say for new models then presumably it could be used to test the premise?
EDIT Tiny48/88 has the "unlike most other" comment.
EDIT2: Oh just wait a moment. The mega48/88/168PA datasheet contains both:
This bit is set when an ADC conversion completes and the Data Registers are updated. The
ADC Conversion Complete Interrupt is executed if the ADIE bit and the I-bit in SREG are set.
ADIF is cleared by hardware when executing the corresponding interrupt handling vector. Alternatively,
ADIF is cleared by writing a logical one to the flag. Beware that if doing a Read-Modify-
Write on ADCSRA, a pending interrupt can be disabled. This also applies if the SBI and CBI
instructions are used.
3. Some of the Status Flags are cleared by writing a logical one to them. Note that, unlike most other AVRs, the CBI and SBI
instructions will only operate on the specified bit, and can therefore be used on registers containing such Status Flags. The
CBI and SBI instructions work with registers 0x00 to 0x1F only.
2. Some of the status flags are cleared by writing a logical one to them. Note that the CBI and SBI instructions will operate on
all bits in the I/O register, writing a one back into any flag read as set, thus clearing the flag. The CBI and SBI instructions
work with registers $00 to $1F only.
I would suggest a solution for all AVRs:
#define ADIntDisable() (ADCSR = ADCSR & ~(1<<ADIF | 1<<ADIE)) #define ADIntEnable() (ADCSR = (ADCSR & ~(1<<ADIF)) | 1<<ADIE)
Peter
A second solution:
The main set a flag to signal to the interrupt, that it was reading the data.
Then after finishing the flag was cleared again.
And the interrupt check the flag, and if it was set, the conversion result was stored into a temporary variable.
Peter
OTOH, knowing the cut/paste nature of Atmel datasheets I wonder if this is an old warning that's just a hangover from the past in some of the newer models?
The ATMega128A is supposedly a remake of the ATMega128, and still, all the cbi/sbi warnings are on.
I'd say, this means, for library-like stuff the caution mode should be on.
If simulator 2 really is as accurate as they say for new models then presumably it could be used to test the premise?
Jan
I would suggest a solution for all AVRs:#define ADIntDisable() (ADCSR = ADCSR & ~(1<<ADIF | 1<<ADIE)) #define ADIntEnable() (ADCSR = (ADCSR & ~(1<<ADIF)) | 1<<ADIE)Peter
Jan
