diff mbox series

[v3,07/10] watchdog: wdt-uclass.c: handle all DM watchdogs in watchdog_reset()

Message ID 20210702124510.124401-8-rasmus.villemoes@prevas.dk
State Superseded
Delegated to: Stefan Roese
Headers show
Series handling all DM watchdogs in watchdog_reset() | expand

Commit Message

Rasmus Villemoes July 2, 2021, 12:45 p.m. UTC
A board can have and make use of more than one watchdog device, say
one built into the SOC and an external gpio-petted one. Having
wdt-uclass only handle the first is both a little arbitrary and
unexpected.

So change initr_watchdog() so we visit (probe) all DM watchdog
devices. This essentially boils down to making the init_watchdog_dev
function into a .post_probe method.

Similarly let watchdog_reset() loop over the whole uclass - each
having their own ratelimiting metadata, and a separate "is this device
running" flag.

This gets rid of the watchdog_dev member of struct global_data.  We
do, however, still need the GD_FLG_WDT_READY set in
initr_watchdog(). This is because watchdog_reset() can get called
before DM is ready, and I don't think we can call uclass_get() that
early.

The current code just returns 0 if "getting" the first device fails -
that can of course happen because there are no devices, but it could
also happen if its ->probe call failed. In keeping with that, continue
with the handling of the remaining devices even if one fails to
probe. This is also why we cannot use uclass_probe_all().

If desired, it's possible to later add a per-device "u-boot,autostart"
boolean property, so that one can do CONFIG_WATCHDOG_AUTOSTART
per-device.

Reviewed-by: Stefan Roese <sr@denx.de>
Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
 drivers/watchdog/wdt-uclass.c     | 96 ++++++++++++++++++-------------
 include/asm-generic/global_data.h |  6 --
 2 files changed, 56 insertions(+), 46 deletions(-)

Comments

Simon Glass July 5, 2021, 3:30 p.m. UTC | #1
Hi Rasmus,

On Fri, 2 Jul 2021 at 06:45, Rasmus Villemoes
<rasmus.villemoes@prevas.dk> wrote:
>
> A board can have and make use of more than one watchdog device, say
> one built into the SOC and an external gpio-petted one. Having
> wdt-uclass only handle the first is both a little arbitrary and
> unexpected.
>
> So change initr_watchdog() so we visit (probe) all DM watchdog
> devices. This essentially boils down to making the init_watchdog_dev
> function into a .post_probe method.
>
> Similarly let watchdog_reset() loop over the whole uclass - each
> having their own ratelimiting metadata, and a separate "is this device
> running" flag.
>
> This gets rid of the watchdog_dev member of struct global_data.  We
> do, however, still need the GD_FLG_WDT_READY set in
> initr_watchdog(). This is because watchdog_reset() can get called
> before DM is ready, and I don't think we can call uclass_get() that
> early.
>
> The current code just returns 0 if "getting" the first device fails -
> that can of course happen because there are no devices, but it could
> also happen if its ->probe call failed. In keeping with that, continue
> with the handling of the remaining devices even if one fails to
> probe. This is also why we cannot use uclass_probe_all().
>
> If desired, it's possible to later add a per-device "u-boot,autostart"
> boolean property, so that one can do CONFIG_WATCHDOG_AUTOSTART
> per-device.
>
> Reviewed-by: Stefan Roese <sr@denx.de>
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> ---
>  drivers/watchdog/wdt-uclass.c     | 96 ++++++++++++++++++-------------
>  include/asm-generic/global_data.h |  6 --
>  2 files changed, 56 insertions(+), 46 deletions(-)
>

Please see my belated reply on the previous version of this patch.

Regards,
Simon
Stefan Roese July 15, 2021, 8:15 a.m. UTC | #2
Hi Rasmus,

On 05.07.21 17:30, Simon Glass wrote:
> Hi Rasmus,
> 
> On Fri, 2 Jul 2021 at 06:45, Rasmus Villemoes
> <rasmus.villemoes@prevas.dk> wrote:
>>
>> A board can have and make use of more than one watchdog device, say
>> one built into the SOC and an external gpio-petted one. Having
>> wdt-uclass only handle the first is both a little arbitrary and
>> unexpected.
>>
>> So change initr_watchdog() so we visit (probe) all DM watchdog
>> devices. This essentially boils down to making the init_watchdog_dev
>> function into a .post_probe method.
>>
>> Similarly let watchdog_reset() loop over the whole uclass - each
>> having their own ratelimiting metadata, and a separate "is this device
>> running" flag.
>>
>> This gets rid of the watchdog_dev member of struct global_data.  We
>> do, however, still need the GD_FLG_WDT_READY set in
>> initr_watchdog(). This is because watchdog_reset() can get called
>> before DM is ready, and I don't think we can call uclass_get() that
>> early.
>>
>> The current code just returns 0 if "getting" the first device fails -
>> that can of course happen because there are no devices, but it could
>> also happen if its ->probe call failed. In keeping with that, continue
>> with the handling of the remaining devices even if one fails to
>> probe. This is also why we cannot use uclass_probe_all().
>>
>> If desired, it's possible to later add a per-device "u-boot,autostart"
>> boolean property, so that one can do CONFIG_WATCHDOG_AUTOSTART
>> per-device.
>>
>> Reviewed-by: Stefan Roese <sr@denx.de>
>> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
>> ---
>>   drivers/watchdog/wdt-uclass.c     | 96 ++++++++++++++++++-------------
>>   include/asm-generic/global_data.h |  6 --
>>   2 files changed, 56 insertions(+), 46 deletions(-)
>>
> 
> Please see my belated reply on the previous version of this patch.

Rasmus, do you plan to send an updated patchset version, addressing
Simons's comments?

Thanks,
Stefan
Stefan Roese July 31, 2021, 10:06 a.m. UTC | #3
Hi Rasmus,

On 15.07.21 10:15, Stefan Roese wrote:
> Hi Rasmus,
> 
> On 05.07.21 17:30, Simon Glass wrote:
>> Hi Rasmus,
>>
>> On Fri, 2 Jul 2021 at 06:45, Rasmus Villemoes
>> <rasmus.villemoes@prevas.dk> wrote:
>>>
>>> A board can have and make use of more than one watchdog device, say
>>> one built into the SOC and an external gpio-petted one. Having
>>> wdt-uclass only handle the first is both a little arbitrary and
>>> unexpected.
>>>
>>> So change initr_watchdog() so we visit (probe) all DM watchdog
>>> devices. This essentially boils down to making the init_watchdog_dev
>>> function into a .post_probe method.
>>>
>>> Similarly let watchdog_reset() loop over the whole uclass - each
>>> having their own ratelimiting metadata, and a separate "is this device
>>> running" flag.
>>>
>>> This gets rid of the watchdog_dev member of struct global_data.  We
>>> do, however, still need the GD_FLG_WDT_READY set in
>>> initr_watchdog(). This is because watchdog_reset() can get called
>>> before DM is ready, and I don't think we can call uclass_get() that
>>> early.
>>>
>>> The current code just returns 0 if "getting" the first device fails -
>>> that can of course happen because there are no devices, but it could
>>> also happen if its ->probe call failed. In keeping with that, continue
>>> with the handling of the remaining devices even if one fails to
>>> probe. This is also why we cannot use uclass_probe_all().
>>>
>>> If desired, it's possible to later add a per-device "u-boot,autostart"
>>> boolean property, so that one can do CONFIG_WATCHDOG_AUTOSTART
>>> per-device.
>>>
>>> Reviewed-by: Stefan Roese <sr@denx.de>
>>> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
>>> ---
>>>   drivers/watchdog/wdt-uclass.c     | 96 ++++++++++++++++++-------------
>>>   include/asm-generic/global_data.h |  6 --
>>>   2 files changed, 56 insertions(+), 46 deletions(-)
>>>
>>
>> Please see my belated reply on the previous version of this patch.
> 
> Rasmus, do you plan to send an updated patchset version, addressing
> Simons's comments?

Any updates on this?

Thanks,
Stefan
Rasmus Villemoes Aug. 2, 2021, 9:18 a.m. UTC | #4
On 31/07/2021 12.06, Stefan Roese wrote:
> Hi Rasmus,
> 
> On 15.07.21 10:15, Stefan Roese wrote:
>> Hi Rasmus,
>>
>> On 05.07.21 17:30, Simon Glass wrote:
>>> Hi Rasmus,
>>>
>>> On Fri, 2 Jul 2021 at 06:45, Rasmus Villemoes
>>> <rasmus.villemoes@prevas.dk> wrote:
>>>>
>>>> A board can have and make use of more than one watchdog device, say
>>>> one built into the SOC and an external gpio-petted one. Having
>>>> wdt-uclass only handle the first is both a little arbitrary and
>>>> unexpected.
>>>>
>>>> So change initr_watchdog() so we visit (probe) all DM watchdog
>>>> devices. This essentially boils down to making the init_watchdog_dev
>>>> function into a .post_probe method.
>>>>
>>>> Similarly let watchdog_reset() loop over the whole uclass - each
>>>> having their own ratelimiting metadata, and a separate "is this device
>>>> running" flag.
>>>>
>>>> This gets rid of the watchdog_dev member of struct global_data.  We
>>>> do, however, still need the GD_FLG_WDT_READY set in
>>>> initr_watchdog(). This is because watchdog_reset() can get called
>>>> before DM is ready, and I don't think we can call uclass_get() that
>>>> early.
>>>>
>>>> The current code just returns 0 if "getting" the first device fails -
>>>> that can of course happen because there are no devices, but it could
>>>> also happen if its ->probe call failed. In keeping with that, continue
>>>> with the handling of the remaining devices even if one fails to
>>>> probe. This is also why we cannot use uclass_probe_all().
>>>>
>>>> If desired, it's possible to later add a per-device "u-boot,autostart"
>>>> boolean property, so that one can do CONFIG_WATCHDOG_AUTOSTART
>>>> per-device.
>>>>
>>>> Reviewed-by: Stefan Roese <sr@denx.de>
>>>> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
>>>> ---
>>>>   drivers/watchdog/wdt-uclass.c     | 96
>>>> ++++++++++++++++++-------------
>>>>   include/asm-generic/global_data.h |  6 --
>>>>   2 files changed, 56 insertions(+), 46 deletions(-)
>>>>
>>>
>>> Please see my belated reply on the previous version of this patch.
>>
>> Rasmus, do you plan to send an updated patchset version, addressing
>> Simons's comments?
> 
> Any updates on this?

Just got back from vacation and trying to catch up.

I'm a little confused. On June 29, you said

  I'm fine with this post_probe() implementation but have no strong
  feelings about this. So if Simon (or someone else) does not object,
  then please continue this way.

Then Simon does reply on July 5

  Imagine you probe
  the device but then go into SDRAM init that hangs the CPU for a few
  seconds. We need a way to signal that we want the watchdog to start,
  so the board owner has a choice as to when this happens.

[The board owner does have a choice, it's called CONFIG_WATCHDOG_AUTOSTART.]

  I also understand this is not quite how things work at present and I'm
  fine with copying the existing behaviour.

which I didn't read as an objection, as I am precisely not changing any
existing behaviour. As I've said previously, keeping what is done in my
current post_probe function in a separate helper, called from inside the
loop in initr_watchdog which probes all watchdog devices, won't change
anything at all.

But precisely because it won't change anything, in the interest of
getting the gpio driver in, I'll send a v4 with that change alone, then
I'll let Stefan pick whichever version he and Simon can agree to.

But let me one last time repeat why I think the post_probe approach is
the cleanest and a natural fit for DM: post_probe is (AIUI) a place
where a uclass can do some action it wants done for every device
belonging to that uclass. When CONFIG_WATCHDOG_AUTOSTART is set,
wdt_uclass does have such an action. When it's not set, the post_probe
method is a no-op.

Rasmus
Stefan Roese Aug. 2, 2021, 10:39 a.m. UTC | #5
On 02.08.21 11:18, Rasmus Villemoes wrote:
> On 31/07/2021 12.06, Stefan Roese wrote:
>> Hi Rasmus,
>>
>> On 15.07.21 10:15, Stefan Roese wrote:
>>> Hi Rasmus,
>>>
>>> On 05.07.21 17:30, Simon Glass wrote:
>>>> Hi Rasmus,
>>>>
>>>> On Fri, 2 Jul 2021 at 06:45, Rasmus Villemoes
>>>> <rasmus.villemoes@prevas.dk> wrote:
>>>>>
>>>>> A board can have and make use of more than one watchdog device, say
>>>>> one built into the SOC and an external gpio-petted one. Having
>>>>> wdt-uclass only handle the first is both a little arbitrary and
>>>>> unexpected.
>>>>>
>>>>> So change initr_watchdog() so we visit (probe) all DM watchdog
>>>>> devices. This essentially boils down to making the init_watchdog_dev
>>>>> function into a .post_probe method.
>>>>>
>>>>> Similarly let watchdog_reset() loop over the whole uclass - each
>>>>> having their own ratelimiting metadata, and a separate "is this device
>>>>> running" flag.
>>>>>
>>>>> This gets rid of the watchdog_dev member of struct global_data.  We
>>>>> do, however, still need the GD_FLG_WDT_READY set in
>>>>> initr_watchdog(). This is because watchdog_reset() can get called
>>>>> before DM is ready, and I don't think we can call uclass_get() that
>>>>> early.
>>>>>
>>>>> The current code just returns 0 if "getting" the first device fails -
>>>>> that can of course happen because there are no devices, but it could
>>>>> also happen if its ->probe call failed. In keeping with that, continue
>>>>> with the handling of the remaining devices even if one fails to
>>>>> probe. This is also why we cannot use uclass_probe_all().
>>>>>
>>>>> If desired, it's possible to later add a per-device "u-boot,autostart"
>>>>> boolean property, so that one can do CONFIG_WATCHDOG_AUTOSTART
>>>>> per-device.
>>>>>
>>>>> Reviewed-by: Stefan Roese <sr@denx.de>
>>>>> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
>>>>> ---
>>>>>    drivers/watchdog/wdt-uclass.c     | 96
>>>>> ++++++++++++++++++-------------
>>>>>    include/asm-generic/global_data.h |  6 --
>>>>>    2 files changed, 56 insertions(+), 46 deletions(-)
>>>>>
>>>>
>>>> Please see my belated reply on the previous version of this patch.
>>>
>>> Rasmus, do you plan to send an updated patchset version, addressing
>>> Simons's comments?
>>
>> Any updates on this?
> 
> Just got back from vacation and trying to catch up.
> 
> I'm a little confused. On June 29, you said
> 
>    I'm fine with this post_probe() implementation but have no strong
>    feelings about this. So if Simon (or someone else) does not object,
>    then please continue this way.
> 
> Then Simon does reply on July 5
> 
>    Imagine you probe
>    the device but then go into SDRAM init that hangs the CPU for a few
>    seconds. We need a way to signal that we want the watchdog to start,
>    so the board owner has a choice as to when this happens.
> 
> [The board owner does have a choice, it's called CONFIG_WATCHDOG_AUTOSTART.]
> 
>    I also understand this is not quite how things work at present and I'm
>    fine with copying the existing behaviour.
> 
> which I didn't read as an objection, as I am precisely not changing any
> existing behaviour. As I've said previously, keeping what is done in my
> current post_probe function in a separate helper, called from inside the
> loop in initr_watchdog which probes all watchdog devices, won't change
> anything at all.
> 
> But precisely because it won't change anything, in the interest of
> getting the gpio driver in, I'll send a v4 with that change alone, then
> I'll let Stefan pick whichever version he and Simon can agree to.
> 
> But let me one last time repeat why I think the post_probe approach is
> the cleanest and a natural fit for DM: post_probe is (AIUI) a place
> where a uclass can do some action it wants done for every device
> belonging to that uclass. When CONFIG_WATCHDOG_AUTOSTART is set,
> wdt_uclass does have such an action. When it's not set, the post_probe
> method is a no-op.

I might have misunderstood Simon. So if Simon is "okay" with v3, then
I can push this version soon - no need to send a v4 then.

Simon?

Thanks,
Stefan
Simon Glass Aug. 2, 2021, 7:20 p.m. UTC | #6
Hi Rasmus,

On Mon, 2 Aug 2021 at 03:18, Rasmus Villemoes
<rasmus.villemoes@prevas.dk> wrote:
>
> On 31/07/2021 12.06, Stefan Roese wrote:
> > Hi Rasmus,
> >
> > On 15.07.21 10:15, Stefan Roese wrote:
> >> Hi Rasmus,
> >>
> >> On 05.07.21 17:30, Simon Glass wrote:
> >>> Hi Rasmus,
> >>>
> >>> On Fri, 2 Jul 2021 at 06:45, Rasmus Villemoes
> >>> <rasmus.villemoes@prevas.dk> wrote:
> >>>>
> >>>> A board can have and make use of more than one watchdog device, say
> >>>> one built into the SOC and an external gpio-petted one. Having
> >>>> wdt-uclass only handle the first is both a little arbitrary and
> >>>> unexpected.
> >>>>
> >>>> So change initr_watchdog() so we visit (probe) all DM watchdog
> >>>> devices. This essentially boils down to making the init_watchdog_dev
> >>>> function into a .post_probe method.
> >>>>
> >>>> Similarly let watchdog_reset() loop over the whole uclass - each
> >>>> having their own ratelimiting metadata, and a separate "is this device
> >>>> running" flag.
> >>>>
> >>>> This gets rid of the watchdog_dev member of struct global_data.  We
> >>>> do, however, still need the GD_FLG_WDT_READY set in
> >>>> initr_watchdog(). This is because watchdog_reset() can get called
> >>>> before DM is ready, and I don't think we can call uclass_get() that
> >>>> early.
> >>>>
> >>>> The current code just returns 0 if "getting" the first device fails -
> >>>> that can of course happen because there are no devices, but it could
> >>>> also happen if its ->probe call failed. In keeping with that, continue
> >>>> with the handling of the remaining devices even if one fails to
> >>>> probe. This is also why we cannot use uclass_probe_all().
> >>>>
> >>>> If desired, it's possible to later add a per-device "u-boot,autostart"
> >>>> boolean property, so that one can do CONFIG_WATCHDOG_AUTOSTART
> >>>> per-device.
> >>>>
> >>>> Reviewed-by: Stefan Roese <sr@denx.de>
> >>>> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> >>>> ---
> >>>>   drivers/watchdog/wdt-uclass.c     | 96
> >>>> ++++++++++++++++++-------------
> >>>>   include/asm-generic/global_data.h |  6 --
> >>>>   2 files changed, 56 insertions(+), 46 deletions(-)
> >>>>
> >>>
> >>> Please see my belated reply on the previous version of this patch.
> >>
> >> Rasmus, do you plan to send an updated patchset version, addressing
> >> Simons's comments?
> >
> > Any updates on this?
>
> Just got back from vacation and trying to catch up.
>
> I'm a little confused. On June 29, you said
>
>   I'm fine with this post_probe() implementation but have no strong
>   feelings about this. So if Simon (or someone else) does not object,
>   then please continue this way.
>
> Then Simon does reply on July 5
>
>   Imagine you probe
>   the device but then go into SDRAM init that hangs the CPU for a few
>   seconds. We need a way to signal that we want the watchdog to start,
>   so the board owner has a choice as to when this happens.
>
> [The board owner does have a choice, it's called CONFIG_WATCHDOG_AUTOSTART.]
>
>   I also understand this is not quite how things work at present and I'm
>   fine with copying the existing behaviour.
>
> which I didn't read as an objection, as I am precisely not changing any
> existing behaviour. As I've said previously, keeping what is done in my
> current post_probe function in a separate helper, called from inside the
> loop in initr_watchdog which probes all watchdog devices, won't change
> anything at all.
>
> But precisely because it won't change anything, in the interest of
> getting the gpio driver in, I'll send a v4 with that change alone, then
> I'll let Stefan pick whichever version he and Simon can agree to.
>
> But let me one last time repeat why I think the post_probe approach is
> the cleanest and a natural fit for DM: post_probe is (AIUI) a place
> where a uclass can do some action it wants done for every device
> belonging to that uclass. When CONFIG_WATCHDOG_AUTOSTART is set,
> wdt_uclass does have such an action. When it's not set, the post_probe
> method is a no-op.

I can certainly see you point. It definitely looks very natural for DM.

But I still think the problem of a device possibly resetting the board
when probed (despite the various watchdog-reset things sprinkled
throughout the code) many means it is worth having an explicit 'start'
operation in this case. In fact this fits with DM even better, since
probe() is supposed to just fire up the hardware, not start any
operations.

Regards,
Simon
Rasmus Villemoes Aug. 3, 2021, 6:48 a.m. UTC | #7
On 02/08/2021 21.20, Simon Glass wrote:
> Hi Rasmus,
> 
>> But let me one last time repeat why I think the post_probe approach is
>> the cleanest and a natural fit for DM: post_probe is (AIUI) a place
>> where a uclass can do some action it wants done for every device
>> belonging to that uclass. When CONFIG_WATCHDOG_AUTOSTART is set,
>> wdt_uclass does have such an action. When it's not set, the post_probe
>> method is a no-op.
> 
> I can certainly see you point. It definitely looks very natural for DM.
> 
> But I still think the problem of a device possibly resetting the board
> when probed (despite the various watchdog-reset things sprinkled
> throughout the code)

But that is the _very point_ of having the developer be able to opt in
[1] to starting the watchdog device ASAP (i.e. as soon as U-Boot is
capable of handling it). If some part of the boot process hangs or
U-Boot goes into an inf-loop without hitting a WATCHDOG_RESET in there,
one wants (or may want) the board to reset and hopefully follow some
other path the next time around. If the developer/project doesn't want
or need that, WATCHDOG_AUTOSTART can be disabled. And if the developer
wants to make use of this, it makes sense to have the watchdog device
monitor as much of the boot process as possible.

[1] ok, for historical reasons it's default-y, so actually an opt-out
knob, but the developer does have a choice regardless.

Rasmus
Simon Glass Aug. 4, 2021, 2:35 p.m. UTC | #8
Hi Rasmus,

On Tue, 3 Aug 2021 at 00:48, Rasmus Villemoes
<rasmus.villemoes@prevas.dk> wrote:
>
> On 02/08/2021 21.20, Simon Glass wrote:
> > Hi Rasmus,
> >
> >> But let me one last time repeat why I think the post_probe approach is
> >> the cleanest and a natural fit for DM: post_probe is (AIUI) a place
> >> where a uclass can do some action it wants done for every device
> >> belonging to that uclass. When CONFIG_WATCHDOG_AUTOSTART is set,
> >> wdt_uclass does have such an action. When it's not set, the post_probe
> >> method is a no-op.
> >
> > I can certainly see you point. It definitely looks very natural for DM.
> >
> > But I still think the problem of a device possibly resetting the board
> > when probed (despite the various watchdog-reset things sprinkled
> > throughout the code)
>
> But that is the _very point_ of having the developer be able to opt in
> [1] to starting the watchdog device ASAP (i.e. as soon as U-Boot is
> capable of handling it). If some part of the boot process hangs or
> U-Boot goes into an inf-loop without hitting a WATCHDOG_RESET in there,
> one wants (or may want) the board to reset and hopefully follow some
> other path the next time around. If the developer/project doesn't want
> or need that, WATCHDOG_AUTOSTART can be disabled. And if the developer

That requires a rebuild though.

This is more of a driver-model thing than anything else and I know it
seems like a minor point. But the way it is designed at present,
probing a device should get it ready for use and not actually start it
doing something. That is the way all(?) devices work and I think it
makes sense to keep watchdogs consistent with that.

> wants to make use of this, it makes sense to have the watchdog device
> monitor as much of the boot process as possible.

This is not an issue, since you enable all watchdogs immediately at present.

>
> [1] ok, for historical reasons it's default-y, so actually an opt-out
> knob, but the developer does have a choice regardless.
>

Regards,
Simon
diff mbox series

Patch

diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
index ca20c124d4..0972541148 100644
--- a/drivers/watchdog/wdt-uclass.c
+++ b/drivers/watchdog/wdt-uclass.c
@@ -35,44 +35,23 @@  struct wdt_priv {
 	bool running;
 };
 
-static void init_watchdog_dev(struct udevice *dev)
+int initr_watchdog(void)
 {
-	struct wdt_priv *priv;
+	struct udevice *dev;
+	struct uclass *uc;
 	int ret;
 
-	priv = dev_get_uclass_priv(dev);
-
-	if (!CONFIG_IS_ENABLED(WATCHDOG_AUTOSTART)) {
-		printf("WDT:   Not starting %s\n", dev->name);
-		return;
-	}
-
-	ret = wdt_start(dev, priv->timeout * 1000, 0);
-	if (ret != 0) {
-		printf("WDT:   Failed to start %s\n", dev->name);
-		return;
+	ret = uclass_get(UCLASS_WDT, &uc);
+	if (ret) {
+		log_debug("Error getting UCLASS_WDT: %d\n", ret);
+		return 0;
 	}
 
-	printf("WDT:   Started %s with%s servicing (%ds timeout)\n", dev->name,
-	       IS_ENABLED(CONFIG_WATCHDOG) ? "" : "out", priv->timeout);
-}
-
-int initr_watchdog(void)
-{
-	/*
-	 * Init watchdog: This will call the probe function of the
-	 * watchdog driver, enabling the use of the device
-	 */
-	if (uclass_get_device_by_seq(UCLASS_WDT, 0,
-				     (struct udevice **)&gd->watchdog_dev)) {
-		debug("WDT:   Not found by seq!\n");
-		if (uclass_get_device(UCLASS_WDT, 0,
-				      (struct udevice **)&gd->watchdog_dev)) {
-			printf("WDT:   Not found!\n");
-			return 0;
-		}
+	uclass_foreach_dev(dev, uc) {
+		ret = device_probe(dev);
+		if (ret)
+			log_debug("Error probing %s: %d\n", dev->name, ret);
 	}
-	init_watchdog_dev(gd->watchdog_dev);
 
 	gd->flags |= GD_FLG_WDT_READY;
 	return 0;
@@ -155,22 +134,34 @@  void watchdog_reset(void)
 {
 	struct wdt_priv *priv;
 	struct udevice *dev;
+	struct uclass *uc;
 	ulong now;
 
 	/* Exit if GD is not ready or watchdog is not initialized yet */
 	if (!gd || !(gd->flags & GD_FLG_WDT_READY))
 		return;
 
-	dev = gd->watchdog_dev;
-	priv = dev_get_uclass_priv(dev);
-	if (!priv->running)
+	if (uclass_get(UCLASS_WDT, &uc))
 		return;
 
-	/* Do not reset the watchdog too often */
-	now = get_timer(0);
-	if (time_after_eq(now, priv->next_reset)) {
-		priv->next_reset = now + priv->reset_period;
-		wdt_reset(dev);
+	/*
+	 * All devices bound to the wdt uclass should have been probed
+	 * in initr_watchdog(). But just in case something went wrong,
+	 * check device_active() before accessing the uclass private
+	 * data.
+	 */
+	uclass_foreach_dev(dev, uc) {
+		if (!device_active(dev))
+			continue;
+		priv = dev_get_uclass_priv(dev);
+		if (!priv->running)
+			continue;
+		/* Do not reset the watchdog too often */
+		now = get_timer(0);
+		if (time_after_eq(now, priv->next_reset)) {
+			priv->next_reset = now + priv->reset_period;
+			wdt_reset(dev);
+		}
 	}
 }
 #endif
@@ -224,11 +215,36 @@  static int wdt_pre_probe(struct udevice *dev)
 	return 0;
 }
 
+static int wdt_post_probe(struct udevice *dev)
+{
+	struct wdt_priv *priv;
+	int ret;
+
+	priv = dev_get_uclass_priv(dev);
+
+	if (!CONFIG_IS_ENABLED(WATCHDOG_AUTOSTART)) {
+		printf("WDT:   Not starting %s\n", dev->name);
+		return 0;
+	}
+
+	ret = wdt_start(dev, priv->timeout * 1000, 0);
+	if (ret != 0) {
+		printf("WDT:   Failed to start %s\n", dev->name);
+		return 0;
+	}
+
+	printf("WDT:   Started %s with%s servicing (%ds timeout)\n", dev->name,
+	       IS_ENABLED(CONFIG_WATCHDOG) ? "" : "out", priv->timeout);
+
+	return 0;
+}
+
 UCLASS_DRIVER(wdt) = {
 	.id			= UCLASS_WDT,
 	.name			= "watchdog",
 	.flags			= DM_UC_FLAG_SEQ_ALIAS,
 	.post_bind		= wdt_post_bind,
 	.pre_probe		= wdt_pre_probe,
+	.post_probe		= wdt_post_probe,
 	.per_device_auto	= sizeof(struct wdt_priv),
 };
diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h
index 47921d27b1..b554016628 100644
--- a/include/asm-generic/global_data.h
+++ b/include/asm-generic/global_data.h
@@ -445,12 +445,6 @@  struct global_data {
 	 */
 	fdt_addr_t translation_offset;
 #endif
-#if CONFIG_IS_ENABLED(WDT)
-	/**
-	 * @watchdog_dev: watchdog device
-	 */
-	struct udevice *watchdog_dev;
-#endif
 #ifdef CONFIG_GENERATE_ACPI_TABLE
 	/**
 	 * @acpi_ctx: ACPI context pointer