[En-Nut-Discussion] Attempted at91_adc rework

[Ulrich Prinz]

Hi Marcus,

Marcus Jansson wrote:
> Hi list,
> 
> This is my first try at NutOS.
> Related to Ref:
> http://lists.egnite.de/pipermail/en-nut-discussion/2009-July/010942.html
> I played around with /arch/arm/dev/at91_adc.c for sam7x256.
> 
> I'd be glad to email my code to anyone interested in supervise/mentoring me.
> 
Anybody giving good ideas and solutions in, is welcomed!
> 
> Highlights:
> * ADCInit(int, int, int) - Init can be done with individual channels, 
> allocating just the buffers needed and not all 8 as the previous 
> ADCInit() did. Switching between new and old ADCInit can be done with 
> #define ADC_USE_INDIVIDUAL_CHANNELS, without adding weight to the OS.
> 
> * Some new status values are added: ADC_NO_BUFFER, ADC_BUFFER_OVERWRITE, 
> ADC_NO_NEW_VALUE.
> 
> * ADCBufRead/Write() - Circular buffers of any lenght != 0 (previously 
> needed to be a power of 2). The buffer overwrites itself nicely with 
> status ADC_BUFFER_OVERWRITE returned if values are not read before the 
> buffer wrap head and tail. This might break existing code. Fishy use of 
> _adc_buf_head/tail and constants (+1, +2) removed. Multiple reads from 
> the buffer without new values comming in from the ADC gives last ADC 
> value, without disturbing the buffer.

The idea behind buffers of size as power of 2 have a nice feature. You 
can simply add data by buf[(prt++)&(BUF_SZ-1)]=adcval. So a simple AND 
is used to keep buffer pointers in shape. On most CPUs this is a lot 
faster than if( ptr>baseprt+BUF_SZ) ptr = baseptr. Additon and 
substraction need more time than a simple and. There is additional 
overhead in conversion of counters to pointers and back.

But I didn't look in your code actually to see how you've done it.

> 
> * ADCBufReadCurrent() - Read current ADC-value (the last sample), 
> without disturbing the buffer. Drawback: Adds some extra weight to the OS.

All drivers are put into a library. The final code only inherits 
everything the application code calls.
If one only needs a single value at a certain time, he only calls this 
function. So only this function is included in the final code. This 
might use less overhead than to include the buffered adc functions 
without any need. One then also doesn't have to use RAM for buffers 
never needed.
> 
> * ADCSetFrequency() instead of ADCSetPrescale(). Sets PRESCAL, STARTUP 
> and SHTIM in ADC_MR according to the desired ADC clock frequency given 
> (previous code used fixed values for STARTUP and SHTIM). I dont know if 
> ADCSetPrescale() is necessary, other than not to break code.
> 
I have no idea how many people are using the ADC. So I don't know how 
critical an absolute backwards compatibility is. On the other hand, 
there where some posts on this list argueing about the ADC code but no 
solution was posted. So I think it is not critical to exchange that part.
Harald, Ole, correct me if I am wrong!

> * ADCSetMode() was cleaned up a little, now always uses ADC SLEEP mode.
> 
> Example of use:
> 
> //Activate ADC channel 5-7
> //in single conversion mode,
> //with ADC clock frequency 950 kHz
> 
> ADCInit(ADC_CH5 | ADC_CH6 | ADC_CH7, SINGLE_CONVERSION, 950000UL);
> ADCStartConversion();
> for(i = 0; i < ADC_MAX_CHANNEL; ++i)
> {
>     if(ADCBufRead(i, &adcvalue) != ADC_NO_BUFFER)
>         printf("ch#%d: %d  ", i, adcvalue);
> }
> 
> 
> Outputs:
> ch#5: 514  ch#6: 203  ch#7: 506
> 
Did you add that simple code into your sources? If I add the new adc 
code to the nutos trunk, I'd like to add this example into the 
nut/app/adc tree.

> I dont know if you like this at all.
> 
As I told before, any work is appreciated. Sometimes it's only a good 
idea, sometimes it is a complete work like yours.

> Anyway, I've had problems with ADC frequency higher than 950 kHz. 
> Probably due to the interrupts occuring "too fast" (maybe it is due to 
> the ADC SLEEP mode?), but I have not yet figured out how to speed up the 
> code, use nutEnterCritical()/nutExitCritical() or semaphores in the adc 
> irq to avoid problems. Advice?

I'll have to check your code first and wil start it on my AT81SAM7X-EK. 
Then I can do some measurements. Please be patient, if this may need a 
few days.
But I am pretty shure that at 950kHz and above you canÄt do anything 
without DMA. Interrupt is to slow.
> 
> I have not tested my code throughly for correctness or analysed how much 
> extra weight it adds to the OS. Hopefully no code is broken, but I dont 
> know. As said, any comments is appreciated.

Be shure that I check that :)
You'll get a feedback and possibly some questions.

Best regards,
Ulrich