Skip to content

cpu: efm32_common: use board defined EMU/CMU settings#8165

Merged
haukepetersen merged 1 commit intoRIOT-OS:masterfrom
basilfx:feature/efm32_emu_cmu
Nov 29, 2017
Merged

cpu: efm32_common: use board defined EMU/CMU settings#8165
haukepetersen merged 1 commit intoRIOT-OS:masterfrom
basilfx:feature/efm32_emu_cmu

Conversation

@basilfx
Copy link
Copy Markdown
Member

@basilfx basilfx commented Nov 28, 2017

This is split-off #8139.

This PR allows one to define differen Energy Management Unit (EMU) and Clock Management Unit (CMU) settings on a per-board basis.

The defaults are good enough in most cases, but you need other values for radio support.

@basilfx basilfx added Platform: ARM Platform: This PR/issue effects ARM-based platforms Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation labels Nov 28, 2017
* @{
*/
#ifndef CMU_HFXOINIT
#define CMU_HFXOINIT CMU_HFXOINIT_DEFAULT
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here and below: why define the default values for each board? Wouldn't it be much cleaner (and better scalable) to define the default values in cpu/efm32_common/cpu.c:

#ifndef CMU_xx
#define CMU_xx   CMU_xx_DEFAULT
#endif

This way a board would only need to define these values if the board actually does not use the default values...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can also place them inline, for instance:

#ifdef _SILICON_LABS_32B_SERIES_1
static void dcdc_init(void)
{
#ifdef EMU_DCDCINIT
    EMU_DCDCInit_TypeDef init_dcdc = EMU_DCDCINIT;
#else
    EMU_DCDCInit_TypeDef init_dcdc = EMU_DCDCINIT_DEFAULT;
#endif

    EMU_DCDCInit(&init_dcdc);
}
#endif

Added benefit: cpu.c is more to the point. What would be your preference?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally prefer not to inline these things and have ifdef blocks in the beginning of the file. This makes the code much more readable.

@basilfx basilfx force-pushed the feature/efm32_emu_cmu branch from 1565949 to 5a675a0 Compare November 29, 2017 13:05
@basilfx basilfx added the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Nov 29, 2017
@basilfx
Copy link
Copy Markdown
Member Author

basilfx commented Nov 29, 2017

Fixed comments + rebase.

Copy link
Copy Markdown
Contributor

@haukepetersen haukepetersen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK

@haukepetersen
Copy link
Copy Markdown
Contributor

please squash and trigger Murdock

@basilfx basilfx force-pushed the feature/efm32_emu_cmu branch from 5a675a0 to 2950300 Compare November 29, 2017 14:14
@basilfx basilfx added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable labels Nov 29, 2017
@basilfx basilfx force-pushed the feature/efm32_emu_cmu branch from 2950300 to be24c66 Compare November 29, 2017 15:15
@basilfx
Copy link
Copy Markdown
Member Author

basilfx commented Nov 29, 2017

Rebased to include #8175 (no changes to PR).

@haukepetersen
Copy link
Copy Markdown
Contributor

ACK holds, Murdock is happy -> go

@haukepetersen haukepetersen merged commit 8b7c9b9 into RIOT-OS:master Nov 29, 2017
@aabadie aabadie added this to the Release 2018.01 milestone Jan 18, 2018
@basilfx basilfx deleted the feature/efm32_emu_cmu branch February 24, 2018 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ARM Platform: This PR/issue effects ARM-based platforms Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants