boards/common/atmega: gracefully handle CKDIV8 fuse #8952
Conversation
aabadie
added
Type: enhancement
|
@ZetaR60 Do I understand correctly that this effectively overrides the CLKDIV8 fuse setting? |
boards/common/atmega/board.c
Outdated
| static inline void set_prescaler(void) | ||
| { | ||
| /* Enable clock change */ | ||
| CLKPR = 0x80; |
There was a problem hiding this comment.
Is there a define available for this?
|
@bergzand Yes. When the CLKDIV8 fuse bit is set, it initializes the clock prescaler as set to divide the clock by 8. This just overwrites that change with the chosen clock prescaler setting. |
boards/common/atmega/board.c
Outdated
| void led_init(void); | ||
|
|
||
| static inline void set_prescaler(void) |
There was a problem hiding this comment.
This function looks CPU-related to me. Would it be problematic to move it to cpu/atmega_common/include/cpu.h together with the #define you added above? Maybe rename it to atmega_set_prescaler then.
There was a problem hiding this comment.
Note to others: bergzand and I discussed this in IRC. There are some issues with moving it due to wanting to have the ability to override CPU_ATMEGA_CLK_SCALE_INIT in board.h, which would require adding an include for board.h to cpu.h if it is moved. We haven't decided yet the best way to handle this.
|
Tested it on an arduino mega2560 with the timer_periodic_wake example, works perfectly, booting is a lot slower though, but that's to be expected :) |
There was a problem hiding this comment.
I've tested a bit with moving the code around to a more cpu based approach.
I suggest moving the CPU_ATMEGA_CLK_SCALE_INIT define to the common atmega periph_conf.h file as this is the place where clock configuration and other CPU initialization parameters are defined.
The function can be moved to the cpu.h file as inline with atmega_ prefixed to the name.
This way only the include for periph_conf.h has to be added to the cpu.h file and everything should work.
I've compiled it in a local branch here and everything seems to work
| @@ -23,10 +23,25 @@ | |||
| #include "irq.h" | |||
| #include "periph/gpio.h" | |||
|
|
|||
| #ifndef CPU_ATMEGA_CLK_SCALE_INIT | |||
| #define CPU_ATMEGA_CLK_SCALE_INIT CPU_ATMEGA_CLK_SCALE_DIV1 | |||
There was a problem hiding this comment.
This should go into ./boards/common/arduino-atmega/include/periph_conf.h preferably under the CLOCK_CORECLOCK definition and with a small bit of doxygen.
Can be done without the #ifndef construction but I don't mind keeping it.
There was a problem hiding this comment.
Problem though: not all boards using boards/common/atmega are Arduinos.
There was a problem hiding this comment.
I think we can solve that problem in a follow up PR. Discussed a bit offline with @bergzand and maybe found a solution.
boards/common/atmega/board.c
Outdated
| void led_init(void); | ||
|
|
||
| static inline void set_prescaler(void) |
There was a problem hiding this comment.
This one into cpu/atmega_common/include/cpu.h as static inline void atmega_set_prescaler(void).
There was a problem hiding this comment.
Fixed, but also takes the clock scale as an argument, which means that cpu.h doesn't need another include.
|
Did another quick test just to make sure that I made no mistakes when moving things around. Everything is working fine. Shall we now push the remaining movement of code to a refactor PR, squash, and merge? |
ZetaR60
force-pushed
the
RIOT_atmega_graceful_clock
branch
from
10f2a9d to
2979626
Compare
|
Squashed. Thanks for helping with this! |
|
@bergzand I think you forgot to approve the changes from your review. |
bergzand
added
the
CI: ready for build
So far as I can tell, all ATmega chips ship with the CLKDIV8 fuse bit set. Some boards ship with the fuse unprogrammed, others do not (such as the mega-xplained board). If this fuse is programmed, the system clock starts with the wrong frequency. This PR gracefully handles initialization on ATmega boards that start with a divided clock. Incidentally, this also allows setting a custom division setting in board.h.
Tested on ATmega1284p / mega-xplained.