sys/board_common: add generic board_init() function by benpicco · Pull Request #17008 · RIOT-OS/RIOT

benpicco · 2021-10-18T20:19:49Z

Contribution description

Usually board_init() will just initialize the on-board LEDs.
This PR provides a generic, weak board_init() function that can be used as a fall-back if the board does not define a customboard_init()

I also dropped the trivial board_init() functions from a few boards as an example.

Testing procedure

examples/blinky still works as expected.

Issues/PRs references

maribu

lgtm

sys/Makefile

maribu · 2021-10-19T06:54:27Z

sys/board_common/board_common.c

+                    gpio_init(LED##x##_PIN, GPIO_OUT);  \
+                    LED##x##_OFF;                       \
+                    } while (0)
+


I think that drivers/include/led.h can be useful, as it e.g. already has a function like macro LED_OFF(x) and empty fallbacks for non-existing LEDs from 0 to 7.

Sure can do - but that would not help me with the gpio_init()

sys/board_common/board_common.c

dylad · 2021-10-19T07:36:25Z

drivers/include/disp_dev.h

+#ifdef LCD_BACKLIGHT
+    gpio_init(LCD_BACKLIGHT, GPIO_OUT);
+#endif


Do we really need to init this GPIO everytime we want to turn on a backlight ?

And why only a single backlight if we can have multiple displays?

This should be moved to a separate PR, I just wanted to get this past CI.

sys/board_common/board_common.c

boards/esp32-ethernet-kit-v1_0/include/board.h

fjmolinas · 2022-01-27T13:43:55Z

ping

benpicco · 2022-01-27T21:26:52Z

sys/board_common/board_common.c

+#endif
+#ifdef LED7_PIN
+    LED_INIT(7);
+#endif


Thinking about it, maybe this should better be moved to an auto_init_leds module

I like the idea :)

fjmolinas

ACK on my side let's wait and see if anyone has comments and merge this.

leandrolanzieri · 2022-02-22T16:53:22Z

Shouldn't we adapt documentation accordingly? It currently states:

[...] Board initialization functions are defined in board.c. This file must at least define a board_init() function that is called at startup. [...]

boards/esp32-wrover-kit/board.c

new changes

fjmolinas · 2022-02-22T21:46:50Z

Hmm I see you also went for #16013, I think that the last commit was a bit too greedy to do in this one, now there is the question if "kernel_init.h" is the right place for the definition, here it just seems like it was used as a header that is known to be included everywhere.

benpicco · 2022-02-22T22:02:10Z

Oh I just thought that if board.c no longer provides board_init(), there is no justification to declare it in board.h.

I can drop the commit if you prefer that.

fjmolinas · 2022-02-22T22:41:58Z

Oh I just thought that if board.c no longer provides

I definitely agree, that's why in parallel I had nudged @chrysn on #16013, it's just the placing of the function I'm unsure off. As the PR was before I would have merged tomorrow.

benpicco · 2022-02-22T22:47:56Z

What would be a better place for it?

benpicco · 2022-02-22T23:42:10Z

I dropped 48088cb for now.

fjmolinas · 2022-02-23T07:25:29Z

What would be a better place for it?

I don't know... but everything that touches core is a sensible spot, which is why I would rather avoid it from this PR :)

fjmolinas

re-ACK

benpicco requested a review from dylad as a code owner October 18, 2021 20:19

benpicco requested review from fjmolinas and maribu October 18, 2021 20:20

github-actions bot added Area: boards Area: Board ports Area: build system Area: Build system Area: sys Area: System labels Oct 18, 2021

benpicco added Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation and removed Area: build system Area: Build system Area: sys Area: System labels Oct 18, 2021

benpicco force-pushed the board_common branch from 526e81d to 042836f Compare October 18, 2021 20:21

github-actions bot added Area: build system Area: Build system Area: sys Area: System labels Oct 18, 2021

benpicco removed the Area: build system Area: Build system label Oct 18, 2021

benpicco force-pushed the board_common branch from 042836f to 4dd0089 Compare October 18, 2021 20:24

github-actions bot added the Area: build system Area: Build system label Oct 18, 2021

benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Oct 18, 2021

benpicco requested a review from gschorcht as a code owner October 18, 2021 20:52

github-actions bot added Area: drivers Area: Device drivers Area: cpu Area: CPU/MCU ports Platform: ESP Platform: This PR/issue effects ESP-based platforms labels Oct 18, 2021

maribu reviewed Oct 19, 2021

View reviewed changes

kaspar030 reviewed Oct 19, 2021

View reviewed changes

sys/board_common/board_common.c Show resolved Hide resolved

dylad reviewed Oct 19, 2021

View reviewed changes

benpicco added the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Oct 19, 2021

gschorcht reviewed Dec 13, 2021

View reviewed changes

sys/board_common/board_common.c Outdated Show resolved Hide resolved

gschorcht reviewed Dec 13, 2021

View reviewed changes

boards/esp32-ethernet-kit-v1_0/include/board.h Show resolved Hide resolved

github-actions bot removed the Area: sys Area: System label Jan 27, 2022

benpicco commented Jan 27, 2022

View reviewed changes

benpicco mentioned this pull request Jan 28, 2022

sys/auto_init: add auto_init_leds, drop LED init code from boards #17584

Merged

fjmolinas previously approved these changes Feb 22, 2022

View reviewed changes

benpicco removed the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Feb 22, 2022

aabadie reviewed Feb 22, 2022

View reviewed changes

boards/esp32-wrover-kit/board.c Outdated Show resolved Hide resolved

benpicco mentioned this pull request Feb 22, 2022

board: drop custom board_init() aabadie/riot-generator#15

Merged

benpicco and others added 3 commits February 22, 2022 18:27

boards/esp32-*: drop custom board_init()

3977023

boards: drop empty custom board_init & empty board.c

464d941

doc/porting-boards.md: update documentation of board_init()

35ecfad

benpicco force-pushed the board_common branch from 4e07418 to 35ecfad Compare February 22, 2022 17:27

github-actions bot added the Area: doc Area: Documentation label Feb 22, 2022

benpicco requested a review from basilfx as a code owner February 22, 2022 18:08

github-actions bot added the Platform: native Platform: This PR/issue effects the native platform label Feb 22, 2022

benpicco force-pushed the board_common branch from bf30671 to 859d386 Compare February 22, 2022 18:11

github-actions bot added the Area: core Area: RIOT kernel. Handle PRs marked with this with care! label Feb 22, 2022

benpicco force-pushed the board_common branch from 859d386 to 48088cb Compare February 22, 2022 18:19

benpicco force-pushed the board_common branch from 48088cb to 35ecfad Compare February 22, 2022 23:42

github-actions bot removed Platform: native Platform: This PR/issue effects the native platform Area: core Area: RIOT kernel. Handle PRs marked with this with care! labels Feb 22, 2022

fjmolinas approved these changes Feb 23, 2022

View reviewed changes

fjmolinas merged commit 5920872 into RIOT-OS:master Feb 23, 2022

benpicco deleted the board_common branch February 23, 2022 08:46

benpicco mentioned this pull request Feb 25, 2022

boards: drop board_init() from board.h #17707

Merged

OlegHahm added this to the Release 2022.04 milestone Apr 25, 2022

Conversation

benpicco commented Oct 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution description

Testing procedure

Issues/PRs references

Uh oh!

maribu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

maribu Oct 19, 2021

Choose a reason for hiding this comment

Uh oh!

benpicco Jan 27, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dylad Oct 19, 2021

Choose a reason for hiding this comment

Uh oh!

benpicco Oct 19, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

fjmolinas commented Jan 27, 2022

Uh oh!

benpicco Jan 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maribu Jan 28, 2022

Choose a reason for hiding this comment

Uh oh!

fjmolinas left a comment

Choose a reason for hiding this comment

Uh oh!

leandrolanzieri commented Feb 22, 2022

Uh oh!

Uh oh!

fjmolinas commented Feb 22, 2022

Uh oh!

benpicco commented Feb 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fjmolinas commented Feb 22, 2022

Uh oh!

benpicco commented Feb 22, 2022

Uh oh!

benpicco commented Feb 22, 2022

Uh oh!

fjmolinas commented Feb 23, 2022

Uh oh!

fjmolinas left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

benpicco commented Oct 18, 2021 •

edited

Loading

benpicco Jan 27, 2022 •

edited

Loading

benpicco commented Feb 22, 2022 •

edited

Loading