[PATCH 1/2] watchdog: add gpio watchdog driver
Stefan Roese
sr at denx.de
Sat May 22 09:18:02 CEST 2021
On 11.05.21 09:20, Stefan Roese wrote:
> On 11.05.21 08:40, Rasmus Villemoes wrote:
>> On 11/05/2021 07.55, Stefan Roese wrote:
>>> On 10.05.21 17:47, Rasmus Villemoes wrote:
>>>> A rather common kind of external watchdog circuit is one that is kept
>>>> alive by toggling a gpio. Add a driver for handling such a watchdog.
>>>>
>>>> The compatible string is probably a little odd as it has nothing to do
>>>> with linux per se - however, I chose that to make .dts snippets
>>>> reusable between device trees used with U-Boot and linux, and this is
>>>> the (only) compatible string that linux' corresponding driver and DT
>>>> binding accepts. I have asked whether one should/could add "wdt-gpio"
>>>> to that binding, but the answer was no:
>>>>
>>>> https://lore.kernel.org/lkml/CAL_JsqKEGaFpiFV_oAtE+S_bnHkg4qry+bhx2EDs=NSbVf_giA@mail.gmail.com/
>>>>
>>>>
>>>>
>>>> If someone feels strongly about this, I can certainly remove the
>>>> "linux," part from the string - it probably wouldn't the only place
>>>> where
>>>> one can't reuse a DT snippet as-is between linux and U-Boot.
>>>>
>>>> Signed-off-by: Rasmus Villemoes <rasmus.villemoes at prevas.dk>
>>>> ---
>>>> .../watchdog/gpio-wdt.txt | 15 ++++++
>>>> drivers/watchdog/Kconfig | 7 +++
>>>> drivers/watchdog/Makefile | 1 +
>>>> drivers/watchdog/gpio_wdt.c | 51
>>>> +++++++++++++++++++
>>>> 4 files changed, 74 insertions(+)
>>>> create mode 100644 doc/device-tree-bindings/watchdog/gpio-wdt.txt
>>>> create mode 100644 drivers/watchdog/gpio_wdt.c
>>>>
>>>> diff --git a/doc/device-tree-bindings/watchdog/gpio-wdt.txt
>>>> b/doc/device-tree-bindings/watchdog/gpio-wdt.txt
>>>> new file mode 100644
>>>> index 0000000000..2283b7ba6e
>>>> --- /dev/null
>>>> +++ b/doc/device-tree-bindings/watchdog/gpio-wdt.txt
>>>> @@ -0,0 +1,15 @@
>>>> +GPIO watchdog timer
>>>> +
>>>> +Describes a simple watchdog timer which is reset by toggling a gpio.
>>>> +
>>>> +Required properties:
>>>> +
>>>> +- compatible: must be "linux,wdt-gpio"
>>>> +- gpios: gpio to toggle when wdt driver reset method is called
>>>> +
>>>> +Example:
>>>> +
>>>> + gpio-wdt {
>>>> + gpios = <&gpio0 1 0>;
>>>> + compatible = "linux,wdt-gpio";
>>>> + };
>>>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>>>> index f0ff2612a6..2cf378db29 100644
>>>> --- a/drivers/watchdog/Kconfig
>>>> +++ b/drivers/watchdog/Kconfig
>>>> @@ -147,6 +147,13 @@ config WDT_CORTINA
>>>> This driver support all CPU ISAs supported by Cortina
>>>> Access CAxxxx SoCs.
>>>> +config WDT_GPIO
>>>> + bool "External gpio watchdog support"
>>>> + depends on WDT
>>>> + depends on DM_GPIO
>>>> + help
>>>> + Support for external watchdog fed by toggling a gpio.
>>>> +
>>>> config WDT_MPC8xx
>>>> bool "MPC8xx watchdog timer support"
>>>> depends on WDT && MPC8xx
>>>> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
>>>> index 5c7ef593fe..f14415bb8e 100644
>>>> --- a/drivers/watchdog/Makefile
>>>> +++ b/drivers/watchdog/Makefile
>>>> @@ -25,6 +25,7 @@ obj-$(CONFIG_WDT_BOOKE) += booke_wdt.o
>>>> obj-$(CONFIG_WDT_CORTINA) += cortina_wdt.o
>>>> obj-$(CONFIG_WDT_ORION) += orion_wdt.o
>>>> obj-$(CONFIG_WDT_CDNS) += cdns_wdt.o
>>>> +obj-$(CONFIG_WDT_GPIO) += gpio_wdt.o
>>>> obj-$(CONFIG_WDT_MPC8xx) += mpc8xx_wdt.o
>>>> obj-$(CONFIG_WDT_MT7620) += mt7620_wdt.o
>>>> obj-$(CONFIG_WDT_MT7621) += mt7621_wdt.o
>>>> diff --git a/drivers/watchdog/gpio_wdt.c b/drivers/watchdog/gpio_wdt.c
>>>> new file mode 100644
>>>> index 0000000000..9dba9c254e
>>>> --- /dev/null
>>>> +++ b/drivers/watchdog/gpio_wdt.c
>>>> @@ -0,0 +1,51 @@
>>>> +// SPDX-License-Identifier: GPL-2.0+
>>>> +
>>>> +#include <common.h>
>>>> +#include <dm.h>
>>>> +#include <dm/device_compat.h>
>>>> +#include <wdt.h>
>>>> +#include <asm/gpio.h>
>>>> +
>>>> +struct gpio_wdt_priv {
>>>> + struct gpio_desc gpio;
>>>> + int state;
>>>> +};
>>>> +
>>>> +static int gpio_wdt_reset(struct udevice *dev)
>>>> +{
>>>> + struct gpio_wdt_priv *priv = dev_get_priv(dev);
>>>> +
>>>> + priv->state = !priv->state;
>>>> + return dm_gpio_set_value(&priv->gpio, priv->state);
>>>> +}
>>>> +
>>>> +static int dm_probe(struct udevice *dev)
>>>> +{
>>>> + struct gpio_wdt_priv *priv = dev_get_priv(dev);
>>>> + int ret;
>>>> +
>>>> + ret = gpio_request_by_name(dev, "gpios", 0, &priv->gpio,
>>>> GPIOD_IS_OUT);
>>>> + if (ret < 0) {
>>>> + dev_err(dev, "Request for wdt gpio failed: %d\n", ret);
>>>> + return ret;
>>>> + }
>>>> + return gpio_wdt_reset(dev);
>>>> +}
>>>> +
>>>> +static const struct wdt_ops gpio_wdt_ops = {
>>>> + .reset = gpio_wdt_reset,
>>>> +};
>>>
>>> I'm noticing that you don't have a "start" function. How is this GPIO
>>> watchdog enabled? Is it "always on"? Or does it start with the first
>>> triggering?
>>>
>>
>> I have yet to encounter one of these that isn't always-running - it's
>> more or less the only reason one would add an external watchdog circuit
>> instead of relying on whatever the SOC has.
>
> Okay.
>
>>> Perhaps it makes sense to add a note about this into the driver to make
>>> this clear.
>>
>> Will do. But I now see a much bigger problem due to some refactoring in
>> wdt-uclass that basically requires a ->start method or GD_FLG_WDT_READY
>> won't be set. Sigh.
I'm looking through the pending watchdog patches right now. Do you plan
to send an updated version of this patchset soon (mostly updates to
documentation)?
Thanks,
Stefan
More information about the U-Boot
mailing list