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

From: Timur Tabi
Date: Wed May 04 2016 - 10:21:50 EST

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.