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

Stefan Roese sr at denx.de
Wed Nov 2 09:45:10 CET 2022


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


More information about the U-Boot mailing list