All of lore.kernel.org
 help / color / mirror / Atom feed
From: Simon Arlott <simon@fire.lp0.eu>
To: Guenter Roeck <linux@roeck-us.net>, linux-watchdog@vger.kernel.org
Cc: "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	Ralf Baechle <ralf@linux-mips.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Jason Cooper <jason@lakedaemon.net>,
	Marc Zyngier <marc.zyngier@arm.com>,
	Kevin Cernekee <cernekee@gmail.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Wim Van Sebroeck <wim@iguana.be>,
	Maxime Bizon <mbizon@freebox.fr>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-mips@linux-mips.org, Rob Herring <robh+dt@kernel.org>,
	Pawel Moll <pawel.moll@arm.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Ian Campbell <ijc+devicetree@hellion.org.uk>,
	Kumar Gala <galak@codeaurora.org>,
	Jonas Gorski <jogo@openwrt.org>
Subject: Re: [PATCH (v2) 7/10] watchdog: bcm63xx_wdt: Add get_timeleft function
Date: Wed, 25 Nov 2015 08:17:08 +0000	[thread overview]
Message-ID: <56556E84.50202@simon.arlott.org.uk> (raw)
In-Reply-To: <56552214.2050808@roeck-us.net>

On 25/11/15 02:51, Guenter Roeck wrote:
> On 11/24/2015 02:15 PM, Simon Arlott wrote:
>> Return the remaining time from the hardware control register.
>>
>> Warn when the device is registered if the hardware watchdog is currently
>> running and report the remaining time left.
> 
> This is really two logical changes, isn't it ?

If you insist then I'll split it out into yet another patch.

> Nice trick to figure out if the watchdog is running.
> 
> What is the impact ? Will this result in interrupts ?

Yes, if it is running it will receive interrupts and check hw->running
to determine if it should stop the watchdog or not.

> If so, would it make sense to _not_ reset the system after a timeout
> in this case, but to keep pinging the watchdog while the watchdog device
> is not open ?

As the whole point of a hardware watchdog is to reset the system when
there is a problem with the software, it should not be automatically
reset by the driver on startup. If the watchdog is already running then
it needs to be pinged by userspace before the timeout.

The bootloader (CFE) doesn't leave the watchdog running. On my system I
prepend some code before vmlinuz that starts it running at the maximum
timeout.

A module parameter could be added to automatically ping/stop it if it's
running, but this should be in the watchdog core and not an individual
driver.

-- 
Simon Arlott

WARNING: multiple messages have this Message-ID (diff)
From: Simon Arlott <simon-A6De1vDTPLDsq35pWSNszA@public.gmane.org>
To: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>,
	linux-watchdog-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: "devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Ralf Baechle <ralf-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org>,
	Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>,
	Jason Cooper <jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org>,
	Marc Zyngier <marc.zyngier-5wv7dgnIgG8@public.gmane.org>,
	Kevin Cernekee <cernekee-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Florian Fainelli
	<f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Wim Van Sebroeck <wim-IQzOog9fTRqzQB+pC5nmwQ@public.gmane.org>,
	Maxime Bizon <mbizon-MmRyKUhfbQ9GWvitb5QawA@public.gmane.org>,
	Linux Kernel Mailing List
	<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	linux-mips-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org,
	Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>,
	Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>,
	Ian Campbell
	<ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>,
	Kumar Gala <galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>,
	Jonas Gorski <jogo-p3rKhJxN3npAfugRpC6u6w@public.gmane.org>
Subject: Re: [PATCH (v2) 7/10] watchdog: bcm63xx_wdt: Add get_timeleft function
Date: Wed, 25 Nov 2015 08:17:08 +0000	[thread overview]
Message-ID: <56556E84.50202@simon.arlott.org.uk> (raw)
In-Reply-To: <56552214.2050808-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>

On 25/11/15 02:51, Guenter Roeck wrote:
> On 11/24/2015 02:15 PM, Simon Arlott wrote:
>> Return the remaining time from the hardware control register.
>>
>> Warn when the device is registered if the hardware watchdog is currently
>> running and report the remaining time left.
> 
> This is really two logical changes, isn't it ?

If you insist then I'll split it out into yet another patch.

> Nice trick to figure out if the watchdog is running.
> 
> What is the impact ? Will this result in interrupts ?

Yes, if it is running it will receive interrupts and check hw->running
to determine if it should stop the watchdog or not.

> If so, would it make sense to _not_ reset the system after a timeout
> in this case, but to keep pinging the watchdog while the watchdog device
> is not open ?

As the whole point of a hardware watchdog is to reset the system when
there is a problem with the software, it should not be automatically
reset by the driver on startup. If the watchdog is already running then
it needs to be pinged by userspace before the timeout.

The bootloader (CFE) doesn't leave the watchdog running. On my system I
prepend some code before vmlinuz that starts it running at the maximum
timeout.

A module parameter could be added to automatically ping/stop it if it's
running, but this should be in the watchdog core and not an individual
driver.

-- 
Simon Arlott
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  reply	other threads:[~2015-11-25  8:17 UTC|newest]

Thread overview: 95+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-21 19:02 [PATCH 1/4] clocksource: Add brcm,bcm6345-timer device tree binding Simon Arlott
2015-11-21 19:02 ` Simon Arlott
2015-11-21 19:03 ` [PATCH 2/4] MIPS: bmips: Add bcm6345-l2-timer interrupt controller Simon Arlott
2015-11-21 19:04 ` [PATCH 3/4] watchdog: Add brcm,bcm6345-wdt device tree binding Simon Arlott
2015-11-22 22:13   ` Rob Herring
2015-11-22 22:13     ` Rob Herring
2015-11-21 19:05 ` [PATCH 4/4] MIPS: bmips: Convert bcm63xx_wdt to use WATCHDOG_CORE Simon Arlott
2015-11-21 19:05   ` Simon Arlott
2015-11-21 21:32   ` Guenter Roeck
2015-11-21 21:44     ` Simon Arlott
2015-11-21 21:44       ` Simon Arlott
2015-11-22  2:32       ` Guenter Roeck
2015-11-22 14:02         ` [PATCH 4/10] (Was: [PATCH 4/4]) " Simon Arlott
2015-11-22 14:05           ` [PATCH 4/10] watchdog: bcm63xx_wdt: Handle hardware interrupt and remove software timer Simon Arlott
2015-11-24 18:21             ` Guenter Roeck
2015-11-24 18:21               ` Guenter Roeck
2015-11-24 18:21               ` Guenter Roeck
2015-11-25 20:14               ` Jonas Gorski
2015-11-25 20:14                 ` Jonas Gorski
2015-11-25 20:28                 ` Simon Arlott
2015-11-25 20:28                   ` Simon Arlott
2015-11-25 22:33             ` [PATCH (v2) " Simon Arlott
2015-11-25 22:33               ` Simon Arlott
2015-11-22 14:06           ` [PATCH 5/10] watchdog: bcm63xx_wdt: Use WATCHDOG_CORE Simon Arlott
2015-11-22 14:06             ` Simon Arlott
2015-11-25  2:44             ` Guenter Roeck
2015-11-25  2:44               ` Guenter Roeck
2015-11-25 13:02               ` Simon Arlott
2015-11-25 14:10                 ` Guenter Roeck
2015-11-25 14:10                   ` Guenter Roeck
2015-11-25 19:43                   ` Simon Arlott
2015-11-25 19:43                     ` Simon Arlott
2015-11-25 22:40               ` [PATCH (v3) 5/11] " Simon Arlott
2015-11-25 22:40                 ` Simon Arlott
2015-11-22 14:07           ` [PATCH 6/10] watchdog: bcm63xx_wdt: Obtain watchdog clock HZ from "periph" clk Simon Arlott
2015-11-22 14:07             ` Simon Arlott
2015-11-23 15:02             ` Jonas Gorski
2015-11-23 15:02               ` Jonas Gorski
2015-11-23 18:19               ` Florian Fainelli
2015-11-23 18:19                 ` Florian Fainelli
2015-11-23 19:00                 ` Simon Arlott
2015-11-23 19:00                   ` Simon Arlott
2015-11-24 22:12             ` [PATCH (v2) " Simon Arlott
2015-11-24 22:42               ` Florian Fainelli
2015-11-24 22:42                 ` Florian Fainelli
2015-11-25 22:47                 ` [PATCH (v3) 6/11] " Simon Arlott
2015-11-25 22:47                   ` Simon Arlott
2015-11-22 14:09           ` [PATCH 7/10] watchdog: bcm63xx_wdt: Add get_timeleft function Simon Arlott
2015-11-22 14:09             ` Simon Arlott
2015-11-24 22:15             ` [PATCH (v2) " Simon Arlott
2015-11-24 22:15               ` Simon Arlott
2015-11-24 22:43               ` Florian Fainelli
2015-11-24 22:43                 ` Florian Fainelli
2015-11-25  2:51               ` Guenter Roeck
2015-11-25  2:51                 ` Guenter Roeck
2015-11-25  8:17                 ` Simon Arlott [this message]
2015-11-25  8:17                   ` Simon Arlott
2015-11-25 22:50                   ` [PATCH (v3) 7/11] " Simon Arlott
2015-11-25 22:50                     ` Simon Arlott
2015-11-25 22:54                     ` [PATCH (v4) " Simon Arlott
2015-11-25 22:54                       ` Simon Arlott
2015-11-25 22:57                       ` [PATCH (v4) 8/11] watchdog: bcm63xx_wdt: Warn if the watchdog is currently running Simon Arlott
2015-11-25 22:57                         ` Simon Arlott
2015-11-22 14:11           ` [PATCH 8/10] watchdog: bcm63xx_wdt: Remove dependency on mach-bcm63xx functions/defines Simon Arlott
2015-11-22 14:11             ` Simon Arlott
2015-11-22 14:12           ` [PATCH 9/10] watchdog: bcm63xx_wdt: Use bcm63xx_timer interrupt directly Simon Arlott
2015-11-22 14:12             ` Simon Arlott
2015-11-25 23:03             ` [PATCH (v2) 10/11] " Simon Arlott
2015-11-25 23:03               ` Simon Arlott
2015-11-22 14:14           ` [PATCH 10/10] watchdog: bcm63xx_wdt: Use brcm,bcm6345-wdt device tree binding Simon Arlott
2015-11-22 14:14             ` Simon Arlott
2015-11-25 23:09             ` [PATCH (v2) 11/11] " Simon Arlott
2015-11-25 23:09               ` Simon Arlott
2015-11-22 22:12 ` [PATCH 1/4] clocksource: Add brcm,bcm6345-timer " Rob Herring
2015-11-23 15:33 ` Jonas Gorski
2015-11-23 18:55   ` [PATCH (v2) 1/10] " Simon Arlott
2015-11-23 18:55     ` Simon Arlott
2015-11-23 18:57     ` [PATCH (v2) 2/10] MIPS: bmips: Add bcm6345-l2-timer interrupt controller Simon Arlott
2015-11-23 18:57       ` Simon Arlott
2015-11-24 22:10       ` [PATCH (v3) " Simon Arlott
2015-11-24 22:10         ` Simon Arlott
2015-11-24 22:36         ` Florian Fainelli
2015-11-24 22:36           ` Florian Fainelli
2015-11-26 22:32           ` [PATCH (v4) 2/11] " Simon Arlott
2015-11-27  8:37             ` Thomas Gleixner
2015-11-27  8:37               ` Thomas Gleixner
2015-11-28 12:26               ` [PATCH (v5) 3/11] " Simon Arlott
2015-11-28 12:26                 ` Simon Arlott
2015-12-01  0:22                 ` Guenter Roeck
2015-12-01  0:22                   ` Guenter Roeck
2016-05-09 12:01                   ` Álvaro Fernández Rojas
2016-05-09 13:06                     ` Guenter Roeck
2016-05-09 13:06                       ` Guenter Roeck
2016-05-11  6:40                       ` [PATCH 2/4] " Álvaro Fernández Rojas
2015-11-25  3:05     ` [PATCH (v2) 1/10] clocksource: Add brcm,bcm6345-timer device tree binding Rob Herring

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56556E84.50202@simon.arlott.org.uk \
    --to=simon@fire.lp0.eu \
    --cc=cernekee@gmail.com \
    --cc=devicetree@vger.kernel.org \
    --cc=f.fainelli@gmail.com \
    --cc=galak@codeaurora.org \
    --cc=ijc+devicetree@hellion.org.uk \
    --cc=jason@lakedaemon.net \
    --cc=jogo@openwrt.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@linux-mips.org \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=marc.zyngier@arm.com \
    --cc=mark.rutland@arm.com \
    --cc=mbizon@freebox.fr \
    --cc=pawel.moll@arm.com \
    --cc=ralf@linux-mips.org \
    --cc=robh+dt@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=wim@iguana.be \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.