On 29.10.22 00:38, Rasmus Villemoes wrote:
On 28/10/2022 16.10, Stefan Roese wrote:
On 28.10.22 13:50, Rasmus Villemoes wrote:
As for cyclic_uninit(), it was never really the opposite of
cyclic_init() since it didn't free the struct cyclic_drv nor set
gd->cyclic to NULL. Rename it to cyclic_unregister_all() and use that
in test/, and also insert a call at the end of the board_init_f
sequence so that gd->cyclic_list is a fresh empty list before we enter
board_init_r().
While reviewing the code, this was the only thing I wanted to
ask about. Now with this explanation it makes perfect sense.
Perhaps a small comment with this reasoning in the code here in
board_init_r would be helpful.
Yeah, so I went back and forth on whether to put it early in
board_init_r or late in board_init_f, but went with the latter so that
whatever free() gets called goes with the same malloc() - i.e. to avoid
introducing any new ordering dependency against when we can initialize
the full malloc. Perhaps something like this above the
cyclic_unregister_all entry in board_init_f sequence:
/*
* Deregister all cyclic functions before relocation, so that
gd->cyclic_list does not contain any references to pre-relocation
devices. Drivers will register their cyclic functions anew when the
devices are probed again.
This should happen as late as possible so that the window where a
watchdog device is not serviced is as small as possible.
*/
But I don't know if that's too verbose; many other important
initialization functions with implicit ordering dependencies do not have
anything similar. That's not necessarily an argument against starting to
add such comments.
Thanks Rasmus. I've added this comment to the commit.
Thanks,
Stefan
