boards/mulle: move nvram init into auto_init #13052

kaspar030 · 2020-01-08T20:41:43Z

Contribution description

Currently, mulle has an extra call to xtimer_init() in its board_init(), as the following nvram init used by a boot counter needs it.
This does not work as is when xtimer is not needed / wanted. E.g., #11874 breaks due to this.

This PR moves the nvram initialization and boot counter stuff into its own function, which is then called in auto_init right after xtimer initialization. The mulle specific xtimer_init() call is removed.

I'm not happy to see board specific stuff in auto init, but more unhappy to have xtimer_init() called in board_init()...

Could we maybe just remove the boot counter and nvram init?

Testing procedure

flash on mulle, maybe ensure boot counter is still updated

Issues/PRs references

#11874

Was needed by nvram init before, whose initialization moved to auto_init.

fjmolinas · 2020-01-08T21:45:54Z

sys/auto_init/auto_init.c

@@ -115,6 +115,10 @@ void auto_init(void)
    DEBUG("Auto init xtimer module.\n");
    xtimer_init();


Why not instead add a pseudomodulextimer_autonit and have:

DEFAULT_MODULE+=xtimer_autoinit

DISABLE_MODULE+=xtimer_autoinit

The init in the boardfile could be kept.

And an ifdef in sys/auto_init/auto_init.c around ximter_init

Well, I'd have to do the same thing with ztimer, and also guard that, and also add another pseudomodule. "ztimer" is a meta module pulling in ztimer_core and the ztimer auto init...

Maybe we should just replace the use of xtimer in nvram_spi. It is only used for "xtimer_spin(_ticks_to_usec(1))", which could be replaced with a simple spin loop.

Or just remove the boot counter. Or does mulle need that?

Maybe we should just replace the use of xtimer in nvram_spi. It is only used for "xtimer_spin(_ticks_to_usec(1))", which could be replaced with a simple spin loop.

I think that would be the best fix, it would remove xtimer/ztimer as a dependency.

Or just remove the boot counter. Or does mulle need that?

I don't have a mulle, but there is one in Berlin.

Well, I'd have to do the same thing with ztimer, and also guard that, and also add another pseudomodule. "ztimer" is a meta module pulling in ztimer_core and the ztimer auto init...

Lets do it with all auto-init functions then? I would find it very useful to be able to disable auto_init only for specific modules. No extra guard would be needed, only adding AUTO_INIT to the existing one.

diff --git a/makefiles/defaultmodules.inc.mk b/makefiles/defaultmodules.inc.mk index 9d8331407c..973983ca8b 100644 --- a/makefiles/defaultmodules.inc.mk +++ b/makefiles/defaultmodules.inc.mk @@ -1,3 +1,5 @@ DEFAULT_MODULE += board cpu core core_msg sys DEFAULT_MODULE += auto_init + +DEFAULT_MODULE += xtimer_auto_init diff --git a/makefiles/pseudomodules.inc.mk b/makefiles/pseudomodules.inc.mk index e7f747a3ef..7f3efa5be3 100644 --- a/makefiles/pseudomodules.inc.mk +++ b/makefiles/pseudomodules.inc.mk @@ -197,4 +197,7 @@ PSEUDOMODULES += crypto_aes_precalculated # This pseudomodule causes a loop in AES to be unrolled (more flash, less CPU) PSEUDOMODULES += crypto_aes_unroll +# All auto_init modules are pseudomodules +PSEUDOMODULES += %_auto_init + # Packages may also add modules to PSEUDOMODULES in their `Makefile.include`. diff --git a/sys/auto_init/auto_init.c b/sys/auto_init/auto_init.c index b72b856875..6a69f2a357 100644 --- a/sys/auto_init/auto_init.c +++ b/sys/auto_init/auto_init.c @@ -24,7 +24,8 @@ #include "diskio.h" #endif -#ifdef MODULE_XTIMER +#ifdef MODULE_XTIMER_AUTO_INIT +#error #include "xtimer.h" #endif @@ -111,7 +112,7 @@ void auto_init(void) void auto_init_random(void); auto_init_random(); #endif -#ifdef MODULE_XTIMER +#ifdef MODULE_XTIMER_AUTO_INIT DEBUG("Auto init xtimer module.\n"); xtimer_init(); #endif

That being said removing the xtimer dependency is still the better fix IMO. This wold just be a nice feature :)

basilfx · 2020-01-08T23:16:15Z

Can't we have something like a board_auto_init(), and move it over there?

The Silicon Labs SLTB001a does something similar (with a tuned while-loop), and I currently am writing a CPU-specific driver that I want to auto-init for SAUL (maybe cpu_auto_init() as well :-P), but not in the global auto-init.

kaspar030 · 2020-01-08T23:19:28Z

Can't we have something like a board_auto_init()?

I had that first (my name was 'board_init_late()`), but didn't know where to put it. First thing in auto-init doesn't make sense for this use case, it needs to be after xtimer. At the end seemed late. So I just went with a special case for mulle...

fjmolinas · 2020-01-09T08:13:40Z

I had that first (my name was 'board_init_late()`), but didn't know where to put it. First thing in auto-init doesn't make sense for this use case, it needs to be after xtimer. At the end seemed late. So I just went with a special case for mulle...

Yes I think having a board_init that comes after all the auto_init stuff would be cool. But in that case It would be better for it to be generic, allowing to register init functions to be called after auto_init, doesn't need to be BOARD related.

fjmolinas · 2020-01-11T09:14:25Z

This does not work as is when xtimer is not needed / wanted. E.g., #11874 breaks due to this

Thinking back on this, doesn't this mean you do not want xtimer at all? Why does this fix your issue, you still have xtimer? I think I'm missing something.

kaspar030 · 2020-01-13T10:24:32Z

Thinking back on this, doesn't this mean you do not want xtimer at all? Why does this fix your issue, you still have xtimer? I think I'm missing something.

This PR moves the xtimer dependent nvram initialization to after xtimer's initialization in auto_init, dropping the need for xtimer_init() in the board_init().

In the ztimer PR, if the xtimer module is used, ztimer provides the xtimer API using a wrapper, but there is no xtimer_init(), thus mulle fails to compile.

I could wrap ztimer_init() in xtimer_init() and keep mulle's early init, but that requires other hacks, as ztimer's init is not idempotent. (not sure xtimer still is after #9530).

TL;DR mulle's early xtimer_init() is a weird special case that would need more weird special handling with ztimer.

fjmolinas · 2020-02-12T16:24:08Z

Can you use #13089 for this?

stale · 2020-08-15T18:27:07Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

kaspar030 added 2 commits January 8, 2020 21:34

boards/mulle: move nvram initialization into auto_init

e929be0

boards/mulle: drop xtimer initialization from board_init()

75df232

Was needed by nvram init before, whose initialization moved to auto_init.

kaspar030 added 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 Area: boards Area: Board ports labels Jan 8, 2020

kaspar030 requested a review from haukepetersen January 8, 2020 20:41

fjmolinas reviewed Jan 8, 2020

View reviewed changes

fjmolinas mentioned this pull request Jan 11, 2020

Makefile.dep: auto_init_% as DEFAULT_MODULES #13089

Merged

stale bot added the State: stale State: The issue / PR has no activity for >185 days label Aug 15, 2020

stale bot closed this Sep 16, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

boards/mulle: move nvram init into auto_init #13052

boards/mulle: move nvram init into auto_init #13052

Uh oh!

kaspar030 commented Jan 8, 2020

Uh oh!

fjmolinas Jan 8, 2020

Uh oh!

fjmolinas Jan 8, 2020

Uh oh!

fjmolinas Jan 8, 2020

Uh oh!

kaspar030 Jan 8, 2020

Uh oh!

fjmolinas Jan 9, 2020

Uh oh!

fjmolinas Jan 9, 2020

Uh oh!

basilfx commented Jan 8, 2020 •

edited

Loading

Uh oh!

kaspar030 commented Jan 8, 2020

Uh oh!

fjmolinas commented Jan 9, 2020

Uh oh!

fjmolinas commented Jan 11, 2020

Uh oh!

kaspar030 commented Jan 13, 2020

Uh oh!

fjmolinas commented Feb 12, 2020

Uh oh!

stale bot commented Aug 15, 2020

Uh oh!

Uh oh!

		@@ -115,6 +115,10 @@ void auto_init(void)
		DEBUG("Auto init xtimer module.\n");
		xtimer_init();

boards/mulle: move nvram init into auto_init #13052

boards/mulle: move nvram init into auto_init #13052

Uh oh!

Conversation

kaspar030 commented Jan 8, 2020

Contribution description

Testing procedure

Issues/PRs references

Uh oh!

fjmolinas Jan 8, 2020

Choose a reason for hiding this comment

Uh oh!

fjmolinas Jan 8, 2020

Choose a reason for hiding this comment

Uh oh!

fjmolinas Jan 8, 2020

Choose a reason for hiding this comment

Uh oh!

kaspar030 Jan 8, 2020

Choose a reason for hiding this comment

Uh oh!

fjmolinas Jan 9, 2020

Choose a reason for hiding this comment

Uh oh!

fjmolinas Jan 9, 2020

Choose a reason for hiding this comment

Uh oh!

basilfx commented Jan 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kaspar030 commented Jan 8, 2020

Uh oh!

fjmolinas commented Jan 9, 2020

Uh oh!

fjmolinas commented Jan 11, 2020

Uh oh!

kaspar030 commented Jan 13, 2020

Uh oh!

fjmolinas commented Feb 12, 2020

Uh oh!

stale bot commented Aug 15, 2020

Uh oh!

Uh oh!

basilfx commented Jan 8, 2020 •

edited

Loading