bootloaders/riotboot: don't change CPU/pin state

[benpicco]

Contribution description

A reoccurring issue is that some board sets way too much pin state in board_init() and this carries over to riotboot where it is then frozen forever.

But riotboot has no business to set any hw state, it just needs to compare two values in flash and jump to an address - we don't need to touch any hardware registers for that.

Testing procedure

An app that makes use of riotboot, e.g. examples/suit_update still boots as before.

This was verified on same54-xpro.

Issues/PRs references

[riot-ci]

Murdock results

✔️ PASSED

581bd8a cpu: add documentation for DISABLE_BOARD_INIT/DISABLE_CPU_INIT

Success	Failures	Total	Runtime
10271	0	10271	19m:37s

Artifacts

• Documentation preview

[maribu]

I have some concerns:

1. The name RIOTBOOT_MINIMAL matches the goal, but not the effect. Something like DISABLE_BOARD_INIT or DISABLE_CPU_INIT would make it a lot more obvious what is happening
2. Wouldn't it be better to be more fine grained? E.g. being able to disable board_init() and cpu_init() separately.
3. Are we sure we can drop cpu_init()? Looking through a bit of code I see things like disabling or feeding a WDT. I think a user knowing a specific board well may be able to conclude that cpu_init() is optional. I guess board_init() is pretty likely to not be needed for riotboot.

[benpicco]

1. The name RIOTBOOT_MINIMAL matches the goal, but not the effect. Something like DISABLE_BOARD_INIT or DISABLE_CPU_INIT would make it a lot more obvious what is happening

One the one hand I didn't want to give application developers ideas, but then again this is more descriptive when writing cpu/ code.

2. Wouldn't it be better to be more fine grained? E.g. being able to disable board_init() and cpu_init() separately.

I don't think there is a use case, but it makes sense from a consistency point of view when going with the descriptive naming.

3. Are we sure we can drop cpu_init()? Looking through a bit of code I see things like disabling or feeding a WDT. I think a user knowing a specific board well may be able to conclude that cpu_init() is optional. I guess board_init() is pretty likely to not be needed for riotboot.

Yes, for cpu_init() the CPU ran just fine up to that point - in fact we don't want to change anything about the CPU state.
All riotboot does is compare two version numbers in ROM, we don't need to feed any watchdog for that. We want to leave everything very much like it is at boot-up and leave the proper init to the real firmware.

[dylad]

Maybe we could just provide some hooks to the user (empty by default) so they can do whatever they want if they have a special use case for that.
Otherwise I like the general idea here.

[maribu]

Looks good to me.

Would you mind adding some documentation with a friendly warning that those two options are experimental for now and users should only use them if they really know what they are doing?

[benpicco]

I thought not documenting them at all and only using them for riotboot would also be an option 😄

[maribu]

Nah, let's rather not add to the pile of magic undocumented flags if that can be avoided.

[benpicco]

Where do we even document something like that?

[maribu]

How about here? https://doc.riot-os.org/group__config.html

[MrKevinWeiss]

ping @maribu anything else or is this now ready to be merged?