Skip to content

boards: Remove per-board definition of board_init#16013

Closed
chrysn wants to merge 4 commits intoRIOT-OS:masterfrom
chrysn-pull-requests:no-board_init-definition-in-headers
Closed

boards: Remove per-board definition of board_init#16013
chrysn wants to merge 4 commits intoRIOT-OS:masterfrom
chrysn-pull-requests:no-board_init-definition-in-headers

Conversation

@chrysn
Copy link
Member

@chrysn chrysn commented Feb 15, 2021

This removes the occurrences that can easily be removed automatically.

Command used:

sed -i -z 's@\n/[\n* ]*.brief[ a-zA-Z,().-]*[\n*/ ]*\nvoid board_init(void);\n@@' boards/**/*.h

Closes: #16007

Summary of #16007: Not all boards do it, so it's clearly not necessary, and the caller of board_init does an extern on it anyway; moreover, it clutters documentation, which should be "every board defines this to do X", not "we do X" for every board. (Notes about the concrete implementation should go to the implementation anyway).

This is not complete yet in that it only does the "easy" ones that are caught by a single-line regexp. A follow-up in here will go through the rest manually, but first I'd like to see how badly things fail.

Testing procedure

Builds still complete.

This removes the occurrences that can easily be removed automatically.

Command used:

    sed -i -z 's@\n/[\n* ]*.brief[ a-zA-Z,().-]*[\n*/ ]*\nvoid board_init(void);\n@@' boards/**/*.h

Closes: RIOT-OS#16007
@chrysn chrysn marked this pull request as draft February 15, 2021 15:45
@chrysn chrysn added Area: boards Area: Board ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Feb 15, 2021
@chrysn chrysn changed the title boards: Remove per-board definition of board.h boards: Remove per-board definition of board_init Feb 23, 2021
#include "periph_cpu.h"
#include "kernel_init.h"
#include "board.h"
#include "board_generic.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be included in board.h instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't know -- at some point we get to choose.

A nice aspect of separating "that where the functions any board needs to provide" from "that where board-specific macros like LED0_ON are defined" is that it reduces rebuilding, and separates otherwise unrelated concerns.

@fjmolinas
Copy link
Contributor

This is not complete yet in that it only does the "easy" ones that are caught by a single-line regexp. A follow-up in here will go through the rest manually, but first I'd like to see how badly things fail.

Seems not that bad.

@MrKevinWeiss MrKevinWeiss added this to the Release 2021.07 milestone Jun 21, 2021
@MrKevinWeiss MrKevinWeiss removed this from the Release 2021.07 milestone Jul 15, 2021
@fjmolinas
Copy link
Contributor

@chrysn do you think you could give this one a rebase and make it not a draft, I think we can get it in!

@chrysn
Copy link
Member Author

chrysn commented Feb 23, 2022

I'll retry when my RIOT-unrelated workload is back to a level where I can; thanks for the nudge.

@benpicco
Copy link
Contributor

48088cb has the removal of board_init() from board.h for the current master

@chrysn
Copy link
Member Author

chrysn commented Feb 26, 2022

Thank you @benpicco for taking this over with a better PR, closing this in favor of #17707,

@chrysn chrysn closed this Feb 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: boards Area: Board ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

board_init: Inconsistent declaration

4 participants