Re: [PATCH 2/2] boot: introduce option to pass barebox-enabled watchdog to systemd

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

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.

> 
> 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@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox