Repository navigation
Bring the xmega's timer driver over to TCA0 - #79
Merged
Merged
Conversation
The TC4/TC5 driver from the xmega E5 project, ported to the type A timer of the Dx. The shape survives the move - one class, a traits struct picking the instance, dual-slope with the overflow at BOTTOM, 10-bit PWM duty - because what differs between the two parts is the register layer and not the design. Three things could not come across unchanged. TCA0 is routed to PORTB in init(). It comes out of reset on PORTA, where PA0 and PA2 carry the strip's USART into the CCL; a timer enabled on its default route would drive both. PORTD belongs to PowerSwitch, PORTC and PORTF to the console and the two-wire bus, so PORTB is what is left. The overflow runs at interrupt level 0. The xmega put it on the PMIC's low level so millis() could preempt the callback; the Dx has a single LVL1VEC and WS2812::init() has it for USART0_DRE, which is what keeps bytes inside the strip's reset gap. Taking it would corrupt a frame, so the callback is simply not interruptible and the header says so. setPeriod() writes PERBUF while the timer runs, where the xmega wrote PER. A TOP that lands below the counter mid-period is not picked up until the counter has gone round 0xFFFF - one period of several hundred milliseconds exactly where a shorter one was asked for. PERBUF is taken over at BOTTOM. The prescaler also gained the Dx's 16 between the 8 and the 64, so a period that used to round up to the 64 keeps four times the resolution. Measured: 12 bytes of flash in the image, because nothing calls it yet and LTO folds the callback to nullptr and keeps only the empty ISR. Linked against a program that drives the whole API - init, start, read, setPeriod, all three PWM channels - it is 1761 bytes. avr-dx and simulator both build clean with -Wall -Wextra, the simulator's tests pass, documented-sizes.py is green. Not on hardware. The register writes were read out of the disassembly instead: PORTMUX.TCAROUTEA masked to 0x07 and set to PORTB, CTRLB's WGMODE to DSBOTTOM, CMD_RESET and CMD_RESTART to CTRLESET, the ISR on vector 7 clearing OVF in INTFLAGS, and both the PER and the PERBUF store inside the same atomic block.
The first cut compiled for the AVR128DA48 and nothing else. It was the only file in the backend that did: the other twelve sources all build for an avr128da32 unchanged, so the standard here was already the family and the timer fell short of it. Two separate mistakes. TimerA1 named TCA1, which is fitted to the 48- and 64-pin parts alone - and the TIMER_USE_TCA1 switch did not protect it, because that only guarded the explicit instantiation in the .cpp while the traits' member bodies are compiled wherever the header is read. Both traits now sit behind their own switch. And the route was PORTB, which the 28- and 32-pin parts do not have at all; of the four they do have, A, C, D and F, every one is already taken by the strip, the console, the supply switch and the bus. So the port is named by its letter in TIMER_TCA0_ROUTE_PORT, and the PORT_t and the PORTMUX group config are pasted from it - one decision instead of two macros that can disagree, which is what the traits carried before. Where no default exists the header stops with a sentence naming the macro, rather than thirty lines further down in four errors about undeclared names. The timer name could not be a macro argument the same way: TCA0 is itself a macro, an argument is expanded on the way through, and PORTMUX_ pasted onto the opening bracket of "(*(TCA_t *) 0x0A00)" is where it ended. One route macro per timer, with the name as plain text in the body, has no such step. Built for seven derivatives: avr128da48, avr128da64 and avr128db48 clean; avr128da32, avr128da28, avr64dd14 and avr32da28 stop at the #error, and -DTIMER_TCA0_ROUTE_PORT=D builds the last three of those. Turning TIMER_USE_TCA1 on reaches the second #error on a part without TCA1, and builds on a DA48 with TCA1 routed to PORTC. The image is byte for byte what it was - 48090 flash, 1700 RAM, 1761 bytes linked against the API test - since none of this reaches run time.
Four faults came across from the xmega driver with the port, and one more was found on the way. read() divided before it shifted, so the truncation was scaled up by the prescaler: at 1024 it moved in steps of 1024 us where a tick is 43 us. Shifted first, it is good to one tick, and 2 * 0xFFFF << 10 still fits. A duty is a fraction of the period, but it was turned into an absolute compare value once, against the PER of the moment. A later setPeriod() left the compare values where they were, so init(1000), 50 % and then setPeriod(2000) ran at 25 %. The duty is now kept per channel and the compare buffers are recomputed alongside PERBUF, so both are taken over at the same BOTTOM. Top is kept as well instead of being read back from PER, because a value still on its way through PERBUF is not in PER yet. PERBUF is now used for a stopped timer too, not only a running one: resume() continues mid-period, which is the same hazard. A timer not yet started gets PER and CMPn loaded directly by start() while it is disabled - otherwise the first period ran on CMPn = 0 from init()'s RESET, since the buffers only arrive at the first BOTTOM. Not a forced UPDATE before the RESTART instead: both go to CTRLESET, and an UPDATE still pending there ORed with RESTART is RESET. The bounds check made setPeriod()'s "set to maximum" branch unreachable; an out of range period is now refused outright, and 0 us with it. attachInterrupt() clears an overflow flag left from while the interrupt was off, which otherwise called the new callback at once. stop() leaves a timer alone before init(). F_CPU has to be a multiple of 2 MHz, asserted. Measured: the image is unchanged at 48090 bytes, nothing calls the driver. A program driving the whole API grows by 322 bytes of flash (1290 -> 1612 with Timer.cpp linked standalone at -Os -flto), plus 8 bytes of RAM for Top and the three duties. avr-dx builds clean with -Wall -Wextra, documented-sizes.py is green. In the disassembly start() reads: ENABLE off, PER, CMP0..CMP2, CLKSEL, RESTART, ENABLE on. Not on hardware.
The README named the one open question as the overflow coming at BOTTOM rather than at TOP, "the difference between the period asked for and twice it". It is not: both come once per pass up and down, so the period is the same either way and only its phase moves by half. A wrong period would come from a mode overflowing at both ends, or counting single-slope, and it would be half the period asked for, not twice it. What BOTTOM against TOP does decide is when the first overflow after start() comes, and whether read(), which counts from BOTTOM, is half a period off. Those two are what the paragraph now asks a scope for. The configuration itself is checked against Microchip's own TCA examples (TB3217 and the MCC Melody ones for the DA48 and DD32): DSBOTTOM with its overflow documented at BOTTOM, PER = 500 giving 1 kHz at a 1 MHz timer clock, which is the factor 2 setPeriod() divides by, and the overflow flag cleared by hand in the ISR. Documentation only.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ports the timer driver from the retired xmega E5 project onto the Dx's type A timer. The
source is theAndreas/XmegaTimer — the finished
version of the driver that lived in
Wordclock_xmegaForArduino/as a torso, whereinit(),setPeriod()andread()were implemented andstart(), the interrupt and all of the PWMwere declared and never written.
The shape comes across intact: one class, a traits struct choosing the instance, dual-slope
counting with the overflow at BOTTOM, 10-bit duty, and a compare channel checked at compile
time rather than at run time. What differs between an xmega E5 and an AVR128DA48 is the
register layer, not the design.
What could not come across unchanged
The route. TCA0 comes out of reset on PORTA, and PA0 and PA2 are USART0 TXD and XCK going
into the CCL — the strip's pulse widths. A timer enabled on its default route drives both, so
init()setsPORTMUX.TCAROUTEAto PORTB before any channel can reach a pin. PORTD is leftto
PowerSwitch.h, PORTC and PORTF are the console and the two-wire bus.The interrupt level. On the xmega the overflow sat on the PMIC's low level so the core's
millis()could preempt the callback. The Dx has oneCPUINT.LVL1VECwhere the xmega had twolevels, and
WS2812::init()has already given it toUSART0_DRE, the handler that keeps bytesflowing inside the strip's reset gap. Taking it for the timer would corrupt a frame. The
overflow therefore runs at level 0 and the callback is not interruptible — the header says so
rather than leaving it to be discovered.
The period update.
setPeriod()writesPERBUF, where the xmega wrotePERoutright. In dual-slope mode a TOP that lands below the counter mid-period is notpicked up until the counter has run to
0xFFFFand wrapped — one period of several hundredmilliseconds exactly where a shorter one was asked for.
PERBUFis taken over at BOTTOM. Thisis the one behavioural fix rather than a translation; the xmega had the same hazard.
The prescaler also gained a step, since the Dx has a 16 between the 8 and the 64. A period that
used to round up to the 64 now keeps four times the resolution.
Measured
The first number is small because nothing calls the driver yet: with no caller, LTO folds the
callback to
nullptrand keeps nothing but the empty ISR. The clock's tick is still polled offmillis()inmain.cpp, and this does not change that — it is a driver sitting ready, not anew time base.
Both
avr-dxandsimulatorbuild clean under-Wall -Wextra, the simulator's test suitepasses, and
tools/documented-sizes.pyis green (the README's size sample moved 48078 → 48090with
--fix).esp32andrp2350are untouched: every backend globs its ownplatform/directory, and both new files are under
platform/avr-dx/.Derivative portability
The first commit compiled for the AVR128DA48 and for nothing else, which made it the only file in
this backend that does — the other twelve sources build for an
avr128da32unchanged. The secondcommit fixes that.
Two independent mistakes.
TimerA1namesTCA1, which is fitted to the 48- and 64-pin partsalone, and
TIMER_USE_TCA1did not protect it: that switch guarded only the explicitinstantiation in the
.cpp, while a traits member body is compiled wherever the header is read.Both traits now sit behind their own switch. And the route was PORTB, which the 28- and 32-pin
parts do not have — of the four they do have, A, C, D and F, every one is already taken by the
strip, the console, the supply switch and the bus.
The port is therefore named by its letter in
TIMER_TCA0_ROUTE_PORT, with thePORT_tand thePORTMUX group config pasted from it, so the pair cannot disagree. Where no default exists the
header stops at an
#errornaming the macro.avr128da48,avr128da64,avr128db48avr128da32,avr128da28,avr64dd14,avr32da28#error-DTIMER_TCA0_ROUTE_PORT=DTIMER_USE_TCA1on, part without TCA1#errorTIMER_USE_TCA1on, DA48, TCA1 on PORTCOne thing worth recording, because it cost a round: the timer name cannot be a macro argument.
TCA0is itself a macro, an argument is expanded on the way through the paste helper, andPORTMUX_pasted onto the opening bracket of(*(TCA_t *) 0x0A00)is where it stops. One routemacro per timer, with the name as plain text in the body, has no such step.
None of this reaches run time: the image is byte for byte what it was.
Review fixes
The third commit fixes what a review of XmegaTimer turned up. All of it came across with the port,
and the same fixes go back to the xmega repository.
read()divided before it shifted, so the prescaler scaled the truncation up: at 1024 itmoved in steps of 1024 us where a tick is 43 us. Shifted first, it is good to one tick.
init(1000), 50 % and thensetPeriod(2000)ran at 25 %. The duty is now kept per channel, andsetPeriod()recomputes the compare buffers next toPERBUF, so both arrive at the same BOTTOM.PERBUFfor a stopped timer too, becauseresume()continues mid-period.start()loadsPERandCMPndirectly while the timer is disabled, since otherwise the first period ran onCMPn = 0. A forcedUPDATEwould not do: anUPDATEstill pending inCTRLESET, ORed withRESTART, reads asRESET.setPeriod()'s "set to maximum" branch was unreachable, so an out of range period(and 0 us) is now refused outright.
attachInterrupt()clears a stale overflow flag.stop()does nothing beforeinit().F_CPUhas to be a multiple of 2 MHz, which is now asserted.The image is unchanged at 48090 bytes. The whole API costs 322 bytes of flash more (1290 -> 1612
with
Timer.cpplinked standalone) and 8 bytes of RAM.What is not verified
It has not run on hardware, which puts it exactly where the rest of this backend is — see
the README's Gaps section. The register writes were checked by reading the disassembly instead:
PORTMUX.TCAROUTEAmasked with0x07and set to0x01(PORTB)CTRLB←WGMODE_DSBOTTOM(0x07)CTRLESET←0x0C(CMD_RESET) ininit(),0x08(CMD_RESTART) instart()TCA0_OVF— clearingOVFinINTFLAGSat0x0A0Bstart()in the order ENABLE off,PER(0x0A26),CMP0..CMP2,CLKSEL,RESTART,ENABLE on;
setPeriod()storingPERBUF(0x0A36) andCMP0BUF..CMP2BUFtogetherWhat a disassembly cannot show is the one thing a scope would: that the overflow really arrives
at BOTTOM and not at TOP, which is the difference between the period asked for and twice it.
The compile-time channel guard was tested by asking for a fourth channel on a three-channel
timer; it fails with
static assertion failed: this timer has no such compare channel.