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.
Reviewed-by: Stefan Roese sr@denx.de Tested-by: Stefan Roese sr@denx.de
Thanks, Rasmus