Re: [PATCH] common: avoid bootchooser double watchdog configuration

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

On 6/18/20 12:25 PM, Ahmad Fatoum wrote:
> Hello Gilles,
> 
> On 6/18/20 12:48 AM, Gilles DOFFE wrote:
>> In case bootchooser is used, boot_entry() will be called twice:
>> 1) boot bootchooser
>> 2) boot <chosen_entry>
>> Thus it will lead to twice watchdog configuration if watchdog_timeout
>> is set.
>> Except that it should be activated only once in any way, it leads
>> to unwanted reset when issuing too much "near" calls of the watchdog
>> configuration for da9062.
> 
> Imagine I have boot.default="bootchooser recovery". Both bootchooser
> slots fail, so I boot the recovery slot, but with your patch
> applied, the watchdog wouldn't be enabled, no?

Brainfart; It would have been enabled earlier. I still don't like
clobbering the global boot_watchdog_timeout. Bette example: On interactive
use, booting fails. User enters manual boot command, but system is reset
prematurely, because the watchdog is not pinged anew.

Resetting boot_watchdog_timeout to the original value when the boot
command returns could work. Fixing it at the watchdog level instead as
described below would also fix manual wd 30; wd 30 in succession.

> The da9063 suffers from the same problem and this was instead addressed
> by dropping requests that come in too fast. See
> cbfdbf38c190 ("mfd: da9063: fix watchdog ping execution")
> 
> Cheers
> Ahmad
> 
>> Can be reproduced with this command:
>>  barebox/ wd 30;wd 30
>> Even if this commands are intentional, 'boot bootchooser' is not.
>> Resetting watchdog_timeout to 0 after first watchdog
>> configuration solves this problem ensuring watchdog will not be
>> configured a second time.
>>
>> Signed-off-by: Gilles DOFFE <gilles.doffe@savoirfairelinux.com>
>> ---
>>  common/boot.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/common/boot.c b/common/boot.c
>> index f546fce62..8d09f96b6 100644
>> --- a/common/boot.c
>> +++ b/common/boot.c
>> @@ -150,6 +150,8 @@ int boot_entry(struct bootentry *be, int verbose, int dryrun)
>>  					   boot_watchdog_timeout);
>>  		if (ret)
>>  			pr_warn("Failed to enable watchdog: %s\n", strerror(-ret));
>> +
>> +		boot_watchdog_timeout = 0;
>>  	}
>>  
>>  	ret = be->boot(be, verbose, dryrun);
>>
> 

-- 
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