[PATCH 5/5] cyclic: get rid of cyclic_init()

Rasmus Villemoes rasmus.villemoes at prevas.dk
Sat Oct 29 00:38:10 CEST 2022


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 at denx.de>
> Tested-by: Stefan Roese <sr at denx.de>

Thanks,
Rasmus




More information about the U-Boot mailing list