| Message ID | 20210510154738.3385393-1-rasmus.villemoes@prevas.dk |
|---|---|
| State | Superseded |
| Delegated to: | Stefan Roese |
| Headers | show |
| Series | [1/2] watchdog: add gpio watchdog driver | expand |
Hi Rasmus, On Mon, 10 May 2021 at 09:47, Rasmus Villemoes <rasmus.villemoes@prevas.dk> 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. It seems fine to me. We share DT bindings with Linux anyway. > > Signed-off-by: Rasmus Villemoes <rasmus.villemoes@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. please add a bit more detail. How do you configure it? e.g. point to the binding file (which you should add to U-Boot if not there). > + > 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; bool ? > +}; > + > +static int gpio_wdt_reset(struct udevice *dev) > +{ > + struct gpio_wdt_priv *priv = dev_get_priv(dev); > + > + priv->state = !priv->state; blank line before final return (please fix below also) > + 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, > +}; > + > +static const struct udevice_id gpio_wdt_ids[] = { > + { .compatible = "linux,wdt-gpio" }, > + {} > +}; > + > +U_BOOT_DRIVER(wdt_gpio) = { > + .name = "wdt_gpio", > + .id = UCLASS_WDT, > + .of_match = gpio_wdt_ids, > + .ops = &gpio_wdt_ops, > + .probe = dm_probe, > + .priv_auto = sizeof(struct gpio_wdt_priv), > +}; > -- > 2.29.2 > Regards, Simon
On 10/05/2021 18.28, Simon Glass wrote: > Hi Rasmus, > > On Mon, 10 May 2021 at 09:47, Rasmus Villemoes > <rasmus.villemoes@prevas.dk> 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. > > It seems fine to me. We share DT bindings with Linux anyway. > >> >> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@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. > > please add a bit more detail. How do you configure it? e.g. point to > the binding file (which you should add to U-Boot if not there). You mean add a reference to doc/device-tree-bindings/watchdog/gpio-wdt.txt which is added in the very same patch? I can do that. No other config option in drivers/watchdog/Kconfig has that, but perhaps that's because there's no mention anywhere of wdt under doc/device-tree-bindings/. And more generally, very few Kconfig files have any such reference. >> + >> 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; > > bool ? Perhaps, though I don't tend to think of the two values a gpio can have as true/false; it's more like an uint:1, so any integer type that can represent 0 and 1 (i.e., any) would do. I don't really care either way. >> +}; >> + >> +static int gpio_wdt_reset(struct udevice *dev) >> +{ >> + struct gpio_wdt_priv *priv = dev_get_priv(dev); >> + >> + priv->state = !priv->state; > > blank line before final return (please fix below also) > OK, but I don't think that improves readability in such a tiny function - and checkpatch didn't complain. Thanks, Rasmus
Hi Rasmus, On Mon, 10 May 2021 at 12:45, Rasmus Villemoes <rasmus.villemoes@prevas.dk> wrote: > > On 10/05/2021 18.28, Simon Glass wrote: > > Hi Rasmus, > > > > On Mon, 10 May 2021 at 09:47, Rasmus Villemoes > > <rasmus.villemoes@prevas.dk> 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. > > > > It seems fine to me. We share DT bindings with Linux anyway. > > > >> > >> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@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. > > > > please add a bit more detail. How do you configure it? e.g. point to > > the binding file (which you should add to U-Boot if not there). > > You mean add a reference to > doc/device-tree-bindings/watchdog/gpio-wdt.txt which is added in the > very same patch? I can do that. > > No other config option in drivers/watchdog/Kconfig has that, but perhaps > that's because there's no mention anywhere of wdt under > doc/device-tree-bindings/. And more generally, very few Kconfig files > have any such reference. Well, the thing is....there is a patman warning about adding at least three lines of text to next Kconfig options. I think people should really sit and think for a few minutes about what useful information can be imparted, so users don't need to go to the source code to figure out basic details. If you don't want to add a mention of the binding, perhaps you could mention that the pin needs to be toggled each xx ms otherwise the board will be reset? I'm not sure, but what you have provided gives me very little idea about how it works :-) > > >> + > >> 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; > > > > bool ? > > Perhaps, though I don't tend to think of the two values a gpio can have > as true/false; it's more like an uint:1, so any integer type that can > represent 0 and 1 (i.e., any) would do. I don't really care either way. Well it's up to you. > > >> +}; > >> + > >> +static int gpio_wdt_reset(struct udevice *dev) > >> +{ > >> + struct gpio_wdt_priv *priv = dev_get_priv(dev); > >> + > >> + priv->state = !priv->state; > > > > blank line before final return (please fix below also) > > > > OK, but I don't think that improves readability in such a tiny function > - and checkpatch didn't complain. It's a U-Boot thing. Regards, Simon
On 10/05/2021 21.19, Simon Glass wrote: > Hi Rasmus, > > On Mon, 10 May 2021 at 12:45, Rasmus Villemoes > <rasmus.villemoes@prevas.dk> wrote: >> >>>> 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. >>> >>> please add a bit more detail. How do you configure it? e.g. point to >>> the binding file (which you should add to U-Boot if not there). >> >> You mean add a reference to >> doc/device-tree-bindings/watchdog/gpio-wdt.txt which is added in the >> very same patch? I can do that. >> >> No other config option in drivers/watchdog/Kconfig has that, but perhaps >> that's because there's no mention anywhere of wdt under >> doc/device-tree-bindings/. And more generally, very few Kconfig files >> have any such reference. > > Well, the thing is....there is a patman warning about adding at least > three lines of text to next Kconfig options. I think people should > really sit and think for a few minutes about what useful information > can be imparted, so users don't need to go to the source code to > figure out basic details. > > If you don't want to add a mention of the binding, It's not that I don't want to, I was just pointing out that no other WDT drivers have that, nor a binding to refer to. I'll add a ref. perhaps you could > mention that the pin needs to be toggled each xx ms otherwise the > board will be reset? Well, that part depends on which particular watchdog circuit the hardware guys have decided to put on the board - these are very generic components. I'll add the hw_margin_ms property to the .txt file. Rasmus
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@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? Perhaps it makes sense to add a note about this into the driver to make this clear. Thanks, Stefan > + > +static const struct udevice_id gpio_wdt_ids[] = { > + { .compatible = "linux,wdt-gpio" }, > + {} > +}; > + > +U_BOOT_DRIVER(wdt_gpio) = { > + .name = "wdt_gpio", > + .id = UCLASS_WDT, > + .of_match = gpio_wdt_ids, > + .ops = &gpio_wdt_ops, > + .probe = dm_probe, > + .priv_auto = sizeof(struct gpio_wdt_priv), > +}; > Viele Grüße, Stefan
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@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. > 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 was actually hoping to get this small driver in first, then start doing the promised "handle _all_ registered watchdogs in wdt_uclass' watchdog_reset", because testing that is much easier once the sandbox test has two distinct "devices". I guess I'll just provide a dummy start method to work around that. Then when I equip wdt_uclass with some uclass-priv data that will contain not only the reset_period and last_reset metadata, but also keep track of whether the device is running (and then wdt_start() can be a no-op in that case, while the gpio driver can set that flag in its probe function). Rasmus
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@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 was actually hoping to get this small driver in first, then start > doing the promised "handle _all_ registered watchdogs in wdt_uclass' > watchdog_reset", because testing that is much easier once the sandbox > test has two distinct "devices". I guess I'll just provide a dummy start > method to work around that. Then when I equip wdt_uclass with some > uclass-priv data that will contain not only the reset_period and > last_reset metadata, but also keep track of whether the device is > running (and then wdt_start() can be a no-op in that case, while the > gpio driver can set that flag in its probe function). Sounds very promising. Looking forward to your work on this. Thanks, Stefan
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@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
On 22/05/2021 09.18, Stefan Roese wrote: > On 11.05.21 09:20, Stefan Roese wrote: >> On 11.05.21 08:40, Rasmus Villemoes wrote: >>> 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)? Yes, I have a larger set of patches pending that includes these, but also the "handle all DM watchdogs in watchdog_reset()". Will send in the next few days. Thanks, Rasmus
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, +}; + +static const struct udevice_id gpio_wdt_ids[] = { + { .compatible = "linux,wdt-gpio" }, + {} +}; + +U_BOOT_DRIVER(wdt_gpio) = { + .name = "wdt_gpio", + .id = UCLASS_WDT, + .of_match = gpio_wdt_ids, + .ops = &gpio_wdt_ops, + .probe = dm_probe, + .priv_auto = sizeof(struct gpio_wdt_priv), +};
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@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