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

Use (CLK_PER / 2) instead of CLKTCA for millis / micros clock #72

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

harryjph
Copy link

@harryjph harryjph commented May 6, 2020

This PR switches the TCB clock select from TCA (which is, by default, CPU clock / 64) to the CPU clock / 2. Then it maintains a counter in the ISR to only increment the micros / millis count every 32 interrupts, because by switching the clock source, we get interrupts 32x more frequently.

The whole reason for this is that without this the millis clock depends on TCA0, which is supposed to drive PWM pins. Without this, users cannot increase the frequency of the PWM without increasing the speed at which millis counts.

Say a user wants to drive the PWM with 4x the default frequency. They set TCA0 to be CPU clock / 16, up from CPU clock / 64, and the PWM will work at 4x the frequency. One problem: millis() ticks 4x faster, and delay() delays for 4x shorter time. This PR corrects this behaviour.

This PR has been written to add as little overhead as possible to the timer ISR, which is important as this is now called 32x as often.

@harryjph
Copy link
Author

harryjph commented May 6, 2020

I have tested this using the following code and an oscilloscope:

void setup() {
  // Increase TCA0 speed to F_CPU
  TCA0.SINGLE.CTRLA = TCA_SINGLE_CLKSEL_DIV1_gc | TCA_SINGLE_ENABLE_bm;

  // Timing test - Drive a pin high for one second
  pinMode(4, OUTPUT);
  digitalWrite(4, HIGH);
  delay(1000);
  digitalWrite(4, LOW);
}

void loop() {}

This is the output:

After image

So I am confident that this maintains correct timing. This test, without this patch, produces the following output:

Before image

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@softhack007
Copy link

Just curious - does "CPU clock / 2" mean that the ISR would be called at 8Mhz on a 16Mhz processor? Hope not, otherwise the ISR would only have two asm instructions before the next interrupt is generated...

@softhack007
Copy link

softhack007 commented Dec 1, 2021

another idea - it seems that MegaCoreX has a different implementation, see https://github.com/MCUdude/MegaCoreX/blob/master/megaavr/cores/coreX-corefiles/wiring.c. Their ISR is very small, several clock freqs are supported, and other fancy stuff.

MCUdude/MegaCoreX@298601b

Does MegaCoreX have the problem described by this PR, i.e. changing the PWM freq will make millis() and friends run faster/slower? Would it be an option to adopt wiring.c from MegaCoreX?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants