On Tue, Nov 24, 2020 at 09:26:32AM +0100, Ahmad Fatoum wrote:
> Hi,
>
> On 24.11.20 09:23, Sascha Hauer wrote:
> >> +static struct watchdog *__watchdog;
> >
> > The name looks like you have something to hide. Perhaps boot_enabled_watchdog?
>
> Can do.
>
> >> +
> >> +static int watchdog_of_fixup(struct device_node *root, void *arg)
> >> +{
> >> + int alias_id;
> >> + char *buf;
> >> +
> >> + if (!__watchdog)
> >> + return 0;
> >> +
> >> + alias_id = watchdog_get_alias_id_from(__watchdog, root);
> >> + if (alias_id < 0)
> >> + return 0;
> >> +
> >> + buf = basprintf("systemd.watchdog-device=/dev/watchdog%d", alias_id);
> >> + if (!buf)
> >> + return 0;
> >> +
> >> + globalvar_add_simple("linux.bootargs.dyn.watchdog", buf);
> >> + free(buf);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static int __maybe_unused of_register_watchdog_fixup(void)
> >> +{
> >> + return of_register_fixup(watchdog_of_fixup, NULL);
> >> +}
> >> +#ifdef CONFIG_SYSTEMD_OF_WATCHDOG
> >> +/* _must_ not be run after late_initcall(of_register_bootargs_fixup) */
> >
> > I smell problems here. How about calling this directly from
> > of_fixup_bootargs()?
>
> Feels wrong to have watchdog specific stuff over there.
Also feels wrong to put code dependencies into comments.
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
barebox mailing list
barebox@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox
