Skip to content

frdm-k64f: Move clock initialization to board_init#6986

Closed
jnohlgard wants to merge 1 commit intoRIOT-OS:masterfrom
jnohlgard:pr/k64f-clock-cleanup
Closed

frdm-k64f: Move clock initialization to board_init#6986
jnohlgard wants to merge 1 commit intoRIOT-OS:masterfrom
jnohlgard:pr/k64f-clock-cleanup

Conversation

@jnohlgard
Copy link
Copy Markdown
Member

The clock setting on this board depends on the existence of an external RMII crystal, which is not mandatory for the CPU itself.

The clock settings in general should be deferred to board_init IMO, there are so many different settings possible that it makes no sense to default the CPU into any particular mode other than the hardware defaults.

@jnohlgard jnohlgard added Platform: ARM Platform: This PR/issue effects ARM-based platforms Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels May 1, 2017
@jnohlgard jnohlgard added this to the Release 2017.07 milestone May 1, 2017
@jnohlgard jnohlgard force-pushed the pr/k64f-clock-cleanup branch from 1ef9a26 to 7b160e9 Compare May 2, 2017 08:53
@jnohlgard
Copy link
Copy Markdown
Member Author

Ping @haukepetersen
This is a straightforward change and Murdock is green.

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.

So if I understand this correctly, the situation here is that we clock the system through a clock signal that is applied to pin PA19 instead of some XTAL or similar source, right?

So IMHO the changes look valid, I have only a simple style request: I would prefer to get rid of the cpu_clock_init (also as the prefix does not fit the module here), and rather paste the containing lines directly into the board_init function to keep things straight forward...

@kYc0o
Copy link
Copy Markdown
Contributor

kYc0o commented Jun 12, 2017

I agree with @haukepetersen here. ACK when the changes are applied.

@jnohlgard
Copy link
Copy Markdown
Member Author

jnohlgard commented Jun 17, 2017

@haukepetersen @kYc0o ping
Renamed the function to board_clock_init, reworded the description text, amended the change to the existing commit.
edit: Added inline to board_clock_init

The clock setting on this board depends on the existence of an external
RMII crystal, which is not mandatory for the CPU itself.
@jnohlgard jnohlgard force-pushed the pr/k64f-clock-cleanup branch from f64f97d to d3d7b1d Compare June 17, 2017 10:12
@kYc0o
Copy link
Copy Markdown
Contributor

kYc0o commented Jun 30, 2017

I think this one only needs an ACK. @haukepetersen

@jnohlgard
Copy link
Copy Markdown
Member Author

Let's postpone this. I want to move cpu.c to kinetis_common and improve the configurability of the clock initialization code to remove the need to configure clocks in board_init.

@jnohlgard jnohlgard closed this Jun 30, 2017
@jnohlgard jnohlgard added the State: archived State: The PR has been archived for possible future re-adaptation label Jun 30, 2017
@jnohlgard jnohlgard removed this from the Release 2017.07 milestone Jun 30, 2017
@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Jun 30, 2017

Are you sure you want to close @gebart ?

@jnohlgard
Copy link
Copy Markdown
Member Author

@aabadie yes, we will save lots of code duplication by going with a shared cpu_init for all Kinetis CPUs. There's only some small improvements needed in the configuration to allow it.

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 State: archived State: The PR has been archived for possible future re-adaptation Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants