Generic AVR DU board fixes: fuses, usable defaults, DU-correct core paths - #651
Open
ws-asahi wants to merge 12 commits into
Open
Generic AVR DU board fixes: fuses, usable defaults, DU-correct core paths#651ws-asahi wants to merge 12 commits into
ws-asahi wants to merge 12 commits into
Conversation
`!ADC0.CTRLA & 0x01` evaluates as `(!ADC0.CTRLA) & 0x01`, so the early return for a disabled ADC never fired as intended. Parenthesize the mask. The same expression appears twice (analogRead paths for the two ADC generations); the other checks in this file already had the parentheses right.
`(chan + 1) < 3` promotes chan to int, so the documented auto-select value chan = 255 evaluated as 256 < 3 and could never take this path. Compare explicitly instead.
…ost data pointer Three bugs in the unaligned/odd-length paths: - the unaligned leading byte was written without advancing the data pointer, so the rest of the buffer was written shifted by one - writeWords was called without afterwards advancing tAddress/data/ length, so the odd-byte epilogue used stale values - the trailing byte was written to tAddress + length - 2 with a recomputed data offset, landing on the wrong address and corrupting the neighbouring byte while dropping the intended one Rewrite the function to advance state explicitly at each stage. Also return early on a zero-length request (FLASHWRITE_0LENGTH).
avrdu.build.highestcb was B2, and avr64du32 repeated that override. The
32/28-pin DU chip rows have no override of their own, so they inherited
B2 as well. No DU part has a TCB2.
Per DS40002548A (AVR64DU28/32):
- Features: "Two 16-bit Timer/Counter type B (TCB)".
- Product family overview table: the TCB row reads 2 for all four pin
counts (14/20/28/32). Unlike the DD-series, pin count does not add a
third TCB on the DU.
- Table 9-1 peripheral address map: TCB0 (0x0B00) and TCB1 (0x0B10)
only, marked present at every pin count; no TCB2 row.
- Interrupt vector table: TCB0_INT (15) and TCB1_INT (29) only.
- PORTMUX.TCBROUTEA defines bit 0 (TCB0) and bit 1 (TCB1) only.
The ATPACK headers agree - ioavr64du32.h defines TCB0/TCB1 and no
TCB2_INT_vect. Ref_Timers.md already states it in prose: "Some parts
(the smaller pincount DD and all of the future DU and EB parts) do not
have a TCB2." The B2 value looks like a leftover from the avrdd block
this section was modelled on.
No behavioural change today: the DU millis menu sets
avrdu.menu.millis.tcbhighest.build.millistimer=B1 directly rather than
expanding {build.highestcb} (only the avrdd/avrddopti sections expand
it), so nothing currently reads these keys. They are still worth
correcting - if the DU menu is ever made consistent with the DD one,
B2 would emit -DMILLIS_USE_TIMERB2 and every 32/28-pin DU build would
stop at timers.h with "TCB2, selected for millis, does not exist on
this part".
The avr64du32 override and the four existing B1 chip overrides are all
redundant with the family default now; left in place to keep the diff
to the incorrect values.
…SCFG0
The generic avrdu SYSCFG0 template expands {bootloader.resetpinbits}
and {bootloader.eesavebit}, but neither key was defined for the DU
section, and no avrdudefuse5 template existed so fuse 5 (SYSCFG0) was
never written at all. Add the missing resetpinbits indirection, the
eesave menu (parity with avrda/avrdb/avrdd), and the fuse-5 write
templates.
SYSCFG.VUSBCTRL resets to regulator-off, and on the DU the PC3 input buffer sits in the VUSB power domain - so a plain no-bootloader board cannot even read PC3 until VUSB is powered, USB or not. Add a 'VUSB Power Source' menu to the generic avrdu board (internal regulator from VDD, the common case, or external 3.3 V on VUSB) and have init() enable the regulator when USB_VREG_INTERNAL is defined.
The 10-bit compatibility path always did temp >>= 2, assuming a 12-bit native ADC. The DU's ADC is natively 10-bit (SINGLE_8BIT/SINGLE_10BIT only, DS40002548A section 32), so the extra shift squashed readings into an 8-bit range. Shift by (ADC_NATIVE_RESOLUTION - 10) instead, which is 2 on 12-bit parts (unchanged) and 0 on the DU. The same pattern appears in both analogRead paths; both are fixed.
On the DU, the EVGENnSEL fields of PORTx.EVGENCTRLA are 3 bits wide and bits 3/7 read as zero (DS40002548A 18.5.10), so the trick of using them as allocation flags does not work - automatic channel selection (chan = 255) is not possible there. Return 255 (failure) on the DU and document the limitation; explicit channel numbers and the Event library's software tracking work as before.
On the DU, USART1's only pin position is ALT2 (PD6/PD7) - its DEFAULT mux value means 'no pin connection' (DS40002548A, USARTROUTEA). And USART0's DEFAULT (PA0/PA1) collides with the main-clock crystal pins, while PD4/PD5 (ALT3) is the one position present on every DU pincount (PORTA options thin out on the 14-pin part). Define HWSERIAL0_MUX_DEFAULT = ALT3 and HWSERIAL1_MUX_DEFAULT = ALT2 in the DU variants, so Serial0 is PD4/PD5 and Serial1 is PD6/PD7 out of the box with no swap() call - the core's UART_swap.h already honours these overrides. Add NOT_A_PIN placeholder rows for the absent ALT1 positions so the pinswap tables keep their row indexing, and correct the 14-pin LED_BUILTIN comment (Serial1's TxD on PD6 is what collides, not PD4).
The example assumed peripherals the DU does not have (TCD0 was consumed by USB, and the pin/timer lists did not cover the DU mux options). Adjust the timer/pin handling so the example builds and runs on DU parts while behaving as before elsewhere; verified on an AVR64DU32.
Hardware testing showed PC3's digital I/O is powered from VDD and is independent of VUSB and the regulator (the pinout legend is easy to misread). Drop that claim from the menu and init() comments; the menu's purpose stands - anything using the USB peripheral needs VUSB powered. No functional change.
Three defects in the DU branch of the ADC code (the ADC_LOWLAT_bm / no-TEMP2 branch), found while characterising the ADC on real hardware. All fail silently - no error is returned, the sketch just gets wrong numbers. 1. analogRead() and _analogReadEnh() masked the channel with 0x3F. The DU has a 7-bit MUXPOS and places its internal channels at 0x40 GND, 0x42 TEMPSENSE and 0x44 VDDDIV10 (DS40002548B 32.4.12), not at 0x30-0x33 like the neighbouring parts, so ADC_GROUND, ADC_TEMPERATURE and ADC_VDDDIV10 became AIN0/AIN2/AIN4 and read ordinary I/O pins. Observed as a VDD/10 reading averaging 143 with a 1..1023 spread (a floating header pin) and an on-chip temperature of 415 degC. All three mask sites are now 0x7F (_analogReadEnh masks twice - before and after its channel check). 2. Both channel checks used the 0x30-0x33 range, so the DU's real internal channels would be rejected once the mask was right. They now accept exactly 0x40, 0x42 and 0x44 and reject the reserved values in between rather than handing them to the mux. 3. analogClockSpeed() read its int16_t PROGMEM table with pgm_read_byte_near() (low byte only) in the search loop, and with plain array indexing in the comparison and the return - a code-space address dereferenced in the data space. It returned garbage (-9227 kHz on a 24 MHz part where the answer is 2000 kHz) and picked a wrong prescaler whenever a frequency was requested. All accesses now use pgm_read_word_near(), and the return masks CTRLB to its prescaler bits. The EA/EB branch of the same function already did all of this correctly. Verified on an AVR64DU32: GND reads 0, VDDDIV10 and TEMPSENSE read stable, plausible values, and analogClockSpeed() reports 2000 kHz at 24 MHz.
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
1 participant
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.
Fixes for the generic
avrdu(no-bootloader) board so a DU part works out of the box over UPDI.This is based on #650 (first three commits) and, like that PR, isolates the changes from #637.
boards.txt
build.highestcb: B2 → B1. No DU part has a TCB2 (DS40002548A).{bootloader.resetpinbits}/{bootloader.eesavebit}were undefinedand no
avrdudefuse5template existed. Added, plus the eesave menu (parity with avrda/db/dd).init() enables the internal regulator when selected (the common case); boards feeding external 3.3 V into VUSB pick the other option.
Core / example
Now shifts by (ADC_NATIVE_RESOLUTION - 10); both ADC paths fixed, 12-bit parts unchanged.
Serial1 = PD6/PD7 (USART1's only position on the DU).
No core change needed - UART_swap.h already honours variant overrides.
not the 0x30-0x33 of the neighbouring parts,
so a 0x3F mask and the range check together broke ADC_GROUND/TEMPERATURE/VDDDIV10.
Also, analogClockSpeed() read its int16_t PROGMEM table a byte at a time and returned garbage (-9227 kHz where the answer is 2000).
Both verified on an AVR64DU32.
Testing (AVR64DU32 Curiosity Nano, nEDBG/UPDI)
Note: the full fuse write also needs Forgotten OSCCFG for DU chips #633's OSCCFG / SYSCFG1 fixes confirmed it stops at fuse2 without them,
and completes with them applied locally.
This PR doesn't duplicate Forgotten OSCCFG for DU chips #633.
The
avrduusbboard, USB stack, and CDC bootloader stay in new next PR.