linux-kernel - Re: [PATCH v2 2/2] watchdog: imx2_wdt: add set

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <563A2849.5040707@mentor.com>
Date:	Wed, 4 Nov 2015 17:46:17 +0200
From:	Vladimir Zapolskiy <vladimir_zapolskiy@...tor.com>
To:	Robin Gong <b38343@...escale.com>, <wim@...ana.be>,
	<linux@...ck-us.net>
CC:	<linux-watchdog@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v2 2/2] watchdog: imx2_wdt: add set_pretimeout interface

Hi Robin,

On 03.11.2015 08:11, Robin Gong wrote:
> Enable set_pretimeout interface and trigger the pretimeout interrupt before
> watchdog timeout event happen.
> 
> Signed-off-by: Robin Gong <b38343@...escale.com>
> ---

[snip]

> +
> +static irqreturn_t imx2_wdt_isr(int irq, void *dev_id)
> +{
> +	struct platform_device *pdev = dev_id;
> +	struct watchdog_device *wdog = platform_get_drvdata(pdev);
> +	struct imx2_wdt_device *wdev = watchdog_get_drvdata(wdog);
> +	u32 val;
> +
> +	regmap_read(wdev->regmap, IMX2_WDT_WICR, &val);
> +	if (val & IMX2_WDT_WICR_WTIS) {
> +		/*clear interrupt status bit*/
> +		regmap_write(wdev->regmap, IMX2_WDT_WICR, val);
> +		panic("pre-timeout:%d, %d Seconds remained\n", wdog->pretimeout,
> +		      wdog->timeout - wdog->pretimeout);

I don't think it is a good idea to panic on pretimeout interrupt, for
instance pretimeout interrupt may be used for any other purposes - ping
watchdog, dump some system information before reboot and so on.

In general I am even not completely convinced that the pretimeout interrupt
handler should be placed in the driver, it may happen that there are some
users outside, who wants to get this interrupt and act according to the event.

> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +

--
With best wishes,
Vladimir

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/