From: linux@roeck-us•net (Guenter Roeck)
To: linux-arm-kernel@lists•infradead.org
Subject: [RFC PATCH 1/5] watchdog: Add API to trigger reboots
Date: Wed, 07 May 2014 06:01:31 -0700 [thread overview]
Message-ID: <536A2EAB.4000605@roeck-us.net> (raw)
In-Reply-To: <1399463549.4536.10.camel@weser.hi.pengutronix.de>
On 05/07/2014 04:52 AM, Lucas Stach wrote:
> Hi Guenter,
>
> Am Freitag, den 02.05.2014, 21:29 -0700 schrieb Guenter Roeck:
>> On Fri, May 02, 2014 at 06:22:43PM -0700, Maxime Ripard wrote:
>>> Hi Guenter,
>>>
>>> On Thu, May 01, 2014 at 08:41:29AM -0700, Guenter Roeck wrote:
>>>> Some hardware implements reboot through its watchdog hardware,
>>>> for example by triggering a watchdog timeout. Platform specific
>>>> code starts to spread into watchdog drivers, typically by setting
>>>> pointers to a callback functions which is then called from the
>>>> platform reset handler.
>>>>
>>>> To simplify code and provide a unified API to trigger reboots by
>>>> watchdog drivers, provide a single API to trigger such reboots
>>>> through the watchdog subsystem.
>>>>
>>>> Signed-off-by: Guenter Roeck <linux@roeck-us•net>
>>>> ---
>>>> drivers/watchdog/watchdog_core.c | 17 +++++++++++++++++
>>>> include/linux/watchdog.h | 11 +++++++++++
>>>> 2 files changed, 28 insertions(+)
>>>>
>>>> diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c
>>>> index cec9b55..4ec6e2f 100644
>>>> --- a/drivers/watchdog/watchdog_core.c
>>>> +++ b/drivers/watchdog/watchdog_core.c
>>>> @@ -43,6 +43,17 @@
>>>> static DEFINE_IDA(watchdog_ida);
>>>> static struct class *watchdog_class;
>>>>
>>>> +static struct watchdog_device *wdd_reboot_dev;
>>>> +
>>>> +void watchdog_do_reboot(enum reboot_mode mode, const char *cmd)
>>>> +{
>>>> + if (wdd_reboot_dev) {
>>>> + if (wdd_reboot_dev->ops->reboot)
>>>> + wdd_reboot_dev->ops->reboot(wdd_reboot_dev, mode, cmd);
>>>> + }
>>>> +}
>>>> +EXPORT_SYMBOL(watchdog_do_reboot);
>>>> +
>>>> static void watchdog_check_min_max_timeout(struct watchdog_device *wdd)
>>>> {
>>>> /*
>>>> @@ -162,6 +173,9 @@ int watchdog_register_device(struct watchdog_device *wdd)
>>>> return ret;
>>>> }
>>>>
>>>> + if (wdd->ops->reboot)
>>>> + wdd_reboot_dev = wdd;
>>>> +
>>>
>>> Overall, it looks really great, but I guess we can make it a
>>> list. Otherwise, we might end up in a situation where we could not
>>> reboot anymore, like this one for example:
>>> - a first watchdog is probed, registers a reboot function
>>> - a second watchdog is probed, registers a reboot function that
>>> overwrites the first one.
>>> - then, the second watchdog disappears for some reason, and the
>>> reboot is set to NULL
>>>
>> I thought about that, but how likely (or unlikely) is that to ever happen ?
>> So I figured it is not worth the effort, and would just add complexity without
>> real gain. We could always add the list later if we ever encounter a situation
>> where two watchdogs in the same system provide a reboot callback.
>>
>
> While this is not directly related to the issue you are fixing with this
> series, I would like to have it considered when talking about a watchdog
> system reboot API.
>
> On i.MX we have the same situation where we have to reboot through the
> SoC watchdog. This works, but may leave the external components of the
> system (those not integrated in the SoC) in an undefined state. So if we
> have a PMIC with integrated watchdog we would rather like to this one to
> reboot the system, as it the reset is then much more closer to a
> power-on-reset.
>
> This means we could have multiple watchdogs in the system, where we
> really want a specific one (maybe designated through a DT property) to
> do the reset. This isn't compatible with the "last watchdog that
> registers a handler wins the system reset" logic in your patch.
>
Wouldn't the order in which watchdogs are configured in dt define that ?
The last one wins.
Thanks,
Guenter
next prev parent reply other threads:[~2014-05-07 13:01 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-05-01 15:41 [RFC PATCH 0/5] watchdog: Add reboot API Guenter Roeck
2014-05-01 15:41 ` [RFC PATCH 1/5] watchdog: Add API to trigger reboots Guenter Roeck
2014-05-02 10:01 ` Will Deacon
2014-05-02 13:22 ` Guenter Roeck
2014-05-03 1:22 ` Maxime Ripard
2014-05-03 4:29 ` Guenter Roeck
2014-05-05 4:27 ` Maxime Ripard
2014-05-07 11:52 ` Lucas Stach
2014-05-07 13:01 ` Guenter Roeck [this message]
2014-05-07 15:49 ` Lucas Stach
2014-05-07 19:15 ` Maxime Ripard
2014-05-05 18:36 ` Felipe Balbi
2014-05-05 19:45 ` Guenter Roeck
2014-05-01 15:41 ` [RFC PATCH 2/5] arm64: Support reboot through watchdog subsystem Guenter Roeck
2014-05-01 15:41 ` [RFC PATCH 3/5] arm: " Guenter Roeck
2014-05-01 15:41 ` [RFC PATCH 4/5] watchdog: moxart: Register reboot handler with " Guenter Roeck
2014-05-01 15:41 ` [RFC PATCH 5/5] watchdog: sunxi: " Guenter Roeck
2014-05-05 18:26 ` [RFC PATCH 0/5] watchdog: Add reboot API Arnd Bergmann
2014-05-06 14:29 ` Jonas Jensen
2014-05-07 11:01 ` Heiko Stübner
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=536A2EAB.4000605@roeck-us.net \
--to=linux@roeck-us$(echo .)net \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox