Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix ADC prescaler for LGT #32

Merged
merged 1 commit into from
Jul 28, 2020
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 8 additions & 16 deletions lgt8f/cores/lgt8f/wiring.c
Original file line number Diff line number Diff line change
Expand Up @@ -351,28 +351,20 @@ void init()
#endif

#if defined(ADCSRA)
// set a2d prescaler so we are inside the desired 50-200 KHz range.
#if F_CPU >= 16000000 // 16 MHz / 128 = 125 KHz
sbi(ADCSRA, ADPS2);
sbi(ADCSRA, ADPS1);
sbi(ADCSRA, ADPS0);
#elif F_CPU >= 8000000 // 8 MHz / 64 = 125 KHz
sbi(ADCSRA, ADPS2);
sbi(ADCSRA, ADPS1);
cbi(ADCSRA, ADPS0);
#elif F_CPU >= 4000000 // 4 MHz / 32 = 125 KHz
sbi(ADCSRA, ADPS2);
cbi(ADCSRA, ADPS1);
sbi(ADCSRA, ADPS0);
#elif F_CPU >= 2000000 // 2 MHz / 16 = 125 KHz
// set a2d prescaler so we are inside the desired 300-3000 kHz range.
#if F_CPU >= 32000000 // 32 MHz / 16 = 2000 kHz : prescaler = 4
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea!, I'd rather have a speed similar to that of the uno by default (when running at 16Mhz) though, as most people expect it to behave similarly.
I'll gladly add a section to the readme about speeding it up further :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do I get that right, you want to make it slower intentionally? ^^

From my point of view, the sample speed only matters if doing free running / ISR. Someone who is coding such, will also set the prescaler - and that one will be wrong anyways, if using the Atmega specs.
Due to the different numbers of needed cycles for a full conversion (Atmega:25 first, 13 successive; LGT8: 50 first, 15 successive), we'll never match a sample rate the initial code was written and timed for. The developer has to modify his code in any case if running on LGT, the same way it has to be modified for any other type of controller.

I'd rather put some informational block somewhere, describing the different ADC timing for those special cases, but not slow down for everyone.

Till now, years after, the prescaler always was /128, and nobody cared about "it's not the same speed as Atmega".

Copy link
Owner

@dbuezas dbuezas Jul 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well... good point actually.
Having something time critical depending on the timing of an analogRead is ridiculous anyway...
But isn't the stability of the measurements compromised at such high rates?

Regarding prescaler = 2, I was doing something quite similar to you, but I didn't do set the analog reference with analogReference(DEFAULT);. I wasn't reading from the DAC but from a mic and a signal generator, and noted that everything above ~3v got the maximum reading.
I'm also using the differential amplifier so it may be related

I'll test on my side soon and if it looks good I'll merge. Let me know if you make more experiments also!

thanks for the pr again

Copy link
Owner

@dbuezas dbuezas Jul 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure you did set the vref correctly?

Yes, and this only happened with prescaler = 2, it went away with bigger ones

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, the problem only arises with prescaler 2 (= /4) at 8MHz?

My patch would set:
32MHz -> prescaler 4 -> 2000kHz
16MHz -> prescaler 3 -> 2000kHz
8 MHz -> prescaler 2 -> 2000kHz
4+2+1 MHz -> prescaler 1 -> 2000kHz/1000kHz/500kHz

Or did you manually try prescaler 2 at 32MHz? This won't work.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I ment prescaler of /2 (ADPS=0 or 1).
And yes, it was 32MHz, that was probably the problem!

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I got your point: at lower clock speeds, lower prescalers still give the ADC enough time to make a good measurement.

Do you know why the Uno core sets it so high then?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Atmega ADC clock is specified 50-200kHz. The core sets prescaler 7 (/128), which results in 125kHz at 16MHz. Prescaler 6 would result in 250kHz when running at 16MHz, which is out of spec.

LGT's clock is 300-3000kHz, and approximates it's sample+hold with 12 bits (needs 2 cycles more than 10 bits).

You may run it a lot faster, but with lower precision - that's what the data sheet says. For this, I think you must set ADMUX[ADLAR] (left aligned result) and only read the high 8 bits as one-byte-read from ADCH.
Faster ADC also needs a way faster sample-and-hold, guess would also need to set ADCSRC[SPD], enabling highspeed conversion mode on "low-impendance analog inputs" (data sheet). This bit may help you in getting over your experienced limit, if your source is low impendance and delivering enough drive.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha!
Good. I'm convinced.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Side notes:

Running ADC clock 16 MHz has problems at highest 2 bits change to 1 (the first ones the approximation tests). the internal comparators seem to slow to switch from 0 to 1/2vcc and +-1/4vcc.
With 8MHz it is better, but still can see a small hysteresis at 1/2vcc.

If you maybe saw the problem not at 3.3v, but 2.5v, that could have been the reason. It must be very silicon-specific and based on the day of week the device was built...

4 MHz is "good" at analogRead(), but not with free running; sample-and-hold does not look to fully charge the capacitator; even not with SPD set. The DAC0 port I am using to drive the A0 port may be not strong enough.

May be good on smaller signals (differential ADC with high multiplier, as in PWM codes), which also has lower impendance.

2 MHz is perfect for both; free running is sampling/interrupting with 90913Hz (which I wonder of, theoretically should be 2000kHz/15cycles = 133kHz. Something is either wrong in the data sheet (it's rather 22 cycles/conversion, not 15), or I am unable to count loops ^^

sbi(ADCSRA, ADPS2);
cbi(ADCSRA, ADPS1);
cbi(ADCSRA, ADPS0);
#elif F_CPU >= 1000000 // 1 MHz / 8 = 125 KHz
#elif F_CPU >= 16000000 // 16 MHz / 8 = 2000 kHz : prescaler = 3
cbi(ADCSRA, ADPS2);
sbi(ADCSRA, ADPS1);
sbi(ADCSRA, ADPS0);
#else // 128 kHz / 2 = 64 KHz -> This is the closest you can get, the prescaler is 2
#elif F_CPU >= 8000000 // 8 MHz / 4 = 2000 kHz : prescaler = 2
cbi(ADCSRA, ADPS2);
sbi(ADCSRA, ADPS1);
cbi(ADCSRA, ADPS0);
#else // Smallest prescaler is 1 (/2) : 4MHz=2000kHz, 2MHz=1000kHz, 1MHz=500kHz
cbi(ADCSRA, ADPS2);
cbi(ADCSRA, ADPS1);
sbi(ADCSRA, ADPS0);
Comment on lines +363 to 370
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't test it in normal mode, but in free running mode, the readings are not good with prescaler = 2. It works ok between 0 and 3.3v, but between 3.3 and 5v it reads the maximum.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uhh... To be honest, i only checked full range sweep (dac2adc) at 32 and 16M. No free running and no lower sysclk and only at default VCC.
I'll do my homework again tmr :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, homework ;)

Checked both single-read and free-running with sysclk from 1M to 32M, different prescaler.
I never got the 3.3v thing you saw. Are you sure you did set the vref correctly?

For testing, I connected D4 with A0.

Even with 32M and prescaler=/8 (ADC at 4M), the slope is ok so far, but with quite some low-bit-noise. Prescaler=/16 has consistent results.

What I actually saw on my oscilloscope, that the ADC is not really linear at the lower 5% of values. But that doesnt really matter.

Regards,
Oliver

void setup() {
  Serial.begin(9600);
  test();
}

void loop() {
}

int result[256];
int numsamples;

int prescaler[]={2,2,4,8,16,32,64,128};

char hex[]="0123456789ABCDEF";
void printHex(unsigned int data) {
         Serial.print(hex[(data>>12)&0xF]);
         Serial.print(hex[(data>>8)&0xF]);
         Serial.print(hex[(data>>4)&0xF]);
         Serial.print(hex[data&0xF]);
}

void test() {

  Serial.println("Start");

  // set prescaler for testing
  ADCSRA=ADCSRA&0xf8|4;

  // init/preset DAC
  pinMode(DAC0, ANALOG);
  analogWrite(DAC0, 0);

  Serial.print(F("sysclk = "));
  Serial.print(F_CPU/1000000,DEC);
  Serial.println(F("MHz"));

  int p=prescaler[ADCSRA&7];
  Serial.print(F("adcclk = "));
  Serial.print(F_CPU/1000/p,DEC);
  Serial.println(F("kHz"));

  memset(result,0,sizeof(result));
  testSingle();

  memset(result,0,sizeof(result));
  testFreeRunning();
}

void testSingle() {
  Serial.println(F("Test single"));
  analogReference(DEFAULT);  // 5v ref

    for (int i=0; i<256;i++) {
    analogWrite(DAC0,i);
    result[i]=analogRead(A0);
  }
  printout();
}

void testFreeRunning() {
  Serial.println(F("Test free running"));
  analogWrite(DAC0,0);

  numsamples=0;
  
  ADMUX |= (0 & 0x07);    // a0 in
  ADMUX |= (1 << REFS0);  // 5v ref

  ADCSRA |= (1 << ADATE); // enable auto trigger
  ADCSRA |= (1 << ADIE);  // enable interrupts when measurement complete
  ADCSRA |= (1 << ADEN);  // enable ADC
  ADCSRA |= (1 << ADSC);  // start ADC

  delay(100); // wait
  printout();    
}

ISR(ADC_vect)
{
  analogWrite(DAC0,numsamples);

  byte m = ADCL;
  byte j = ADCH;
  result[numsamples++] = (j << 8) | m;

  if (numsamples==256) {
      ADCSRA = 0; // stop it
  }
}

void printout() {
  for (int i=0; i<256;i++) {
    if (i%0x10==0) { Serial.println(F("")); } else { Serial.print(F(" ")); }
    printHex(result[i]);
  }
  Serial.println(F(""));
  Serial.println(F(""));
}

Expand Down