Re: [PATCH RFC] Watchdog: sbsa_gwdt: Enhance timeout range

[Timur Tabi]

Pratyush Anand wrote:
> static irqreturn_t sbsa_gwdt_interrupt(int irq, void *dev_id)
> {
> +    struct sbsa_gwdt *gwdt = (struct sbsa_gwdt *)dev_id;
> +    struct watchdog_device *wdd = &gwdt->wdd;
> +    u64 timeout = (u64)gwdt->clk * wdd->timeout;
> +
> +    writeq(timeout + arch_counter_get_cntvct(),
> +                    gwdt->control_base + SBSA_GWDT_WCV);
> +
>       panic(WATCHDOG_NAME " timeout");

I'm on the fence about this.

On one hand, I have always opposed the idea that the interrupt handler 
needs to function properly in order for the timeout to be correct.  Fu's 
original patch required this for every timeout.

The current code, however, only uses the interrupt when action=1.  In 
this case, WCV is only reprogrammed in order to prevent the system from 
resetting during the kexec.  Technically, the watchdog timeout has 
already been handled.

However, this should be unnecessary, because it can't be a problem 
that's unique to the SBSA watchdog.  Every system that kexecs another 
kernel needs to be able to handle a watchdog timeout.  Shouldn't the 
kexec code already ping or disable the watchdog?  We need a 
cross-platform solution.  Drivers should not need to do this.

--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, hosted by The Linux Foundation.