embassy-nrf: Timer HAL broken for extended timers (TIMER3/TIMER4)

[peter9477]

The embassy-nrf timer.rs implementation has an impl_timer! macro that allows defining "extended" timers (which is used for TIMER3 and TIMER4 on chips where those are supported). This creates a constant CCS equal to 4 or 6, which is used in the Timer::new() to initialize the CC registers. This will panic at run time when TIMER3 or TIMER4 are used to create a Timer, because the impl_timer! macro involves only pac::timer0, which has only 4 CC registers.

As noted in the discussion below https://matrix.to/#/!YoLPkieCYHGzdjUhOK:matrix.org/$dC6N7X1MOSACQwlS0GI9wZsCKI1HS-yGsHlTPTAAEt8?via=matrix.org&via=tchncs.de&via=grusbv.com this will need to reference pac::timer3, possibly via a regs_ext() implementation in the trait (as suggested by @Dirbaio).

[peter9477]

We looked at this and couldn't see an obvious and simple way to make the impl_timer macro somehow handle both pac::timer0 and pac::timer3. It looked like by using a few traits and trait bounds it would be feasible, but we've tried something much simpler which appears to work.

aceeedd

This is done only for 52840 for the moment, as a proof of concept. Basically since the timer3 register block is a pure superset of the timer0 register block, and since the Timer type is generic on the CCS count so it knows how many compare registers it can access, by always using timer3 when it's defined for a given chip it appears this can resolve the problem. (We tested and it avoids the panic.)

Is this an acceptable solution? If so we can apply the changes for the other chips and PR this.

(That one changeset isn't complete as we messed up the macro scoping but subsequent commits resolved that.)

[Dirbaio]

yes, it is acceptable! it's not elegant but there's no better solutions I can think of. The STM32 HAL does similar things in some places.