Hi Michal,
On 05.04.19 14:07, Michal Simek wrote:
<snip>
+++ b/drivers/watchdog/wdt-uclass.c
@@ -10,6 +10,12 @@
#include <dm/device-internal.h>
#include <dm/lists.h>
+#ifndef WDT_DEFAULT_TIMEOUT
+#define WDT_DEFAULT_TIMEOUT 60
+#endif
+
+DECLARE_GLOBAL_DATA_PTR;
+
int wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags)
{
const struct wdt_ops *ops = device_get_ops(dev);
@@ -63,6 +69,67 @@ int wdt_expire_now(struct udevice *dev, ulong flags)
return ret;
}
+#if defined(CONFIG_WATCHDOG)
This is not enough because you can enable it just for full u-boot but
not for SPL.
Why not? CONFIG_WATCHDOG can be enabled with or without
CONFIG_SPL_WATCHDOG_SUPPORT being enabled.
It means you should also handle CONFIG_SPL_WATCHDOG_SUPPORT
macros.
Just try to enable CADENCE_WDT, WDT and Watchdog for zcu100 and build
I just tried this for xilinx_zynqmp_zcu100_revC_defconfig. Per default
it has enabled:
$ grep 'WATCHDOG\|WDT' .config
# CONFIG_SPL_WATCHDOG_SUPPORT is not set
# CONFIG_SYSRESET_WATCHDOG is not set
CONFIG_WATCHDOG=y
# CONFIG_WATCHDOG_RESET_DISABLE is not set
# CONFIG_BCM2835_WDT is not set
# CONFIG_ULP_WATCHDOG is not set
CONFIG_WDT=y
# CONFIG_WDT_ASPEED is not set
# CONFIG_WDT_ORION is not set
CONFIG_WDT_CDNS=y
# CONFIG_XILINX_TB_WATCHDOG is not set
# CONFIG_IMX_WATCHDOG is not set
# CONFIG_WDT_AT91 is not set
I can also enable SPL_WATCHDOG_SUPPORT and it compiles without issues:
$ grep 'WATCHDOG\|WDT' .config
CONFIG_SPL_WATCHDOG_SUPPORT=y
# CONFIG_SYSRESET_WATCHDOG is not set
CONFIG_WATCHDOG=y
# CONFIG_WATCHDOG_RESET_DISABLE is not set
# CONFIG_BCM2835_WDT is not set
# CONFIG_ULP_WATCHDOG is not set
CONFIG_WDT=y
# CONFIG_WDT_ASPEED is not set
# CONFIG_WDT_ORION is not set
CONFIG_WDT_CDNS=y
# CONFIG_XILINX_TB_WATCHDOG is not set
# CONFIG_IMX_WATCHDOG is not set
# CONFIG_WDT_AT91 is not set
What am I missing?
+/*
+ * Called by macro WATCHDOG_RESET. This function be called *very* early,
+ * so we need to make sure, that the watchdog driver is ready before using
+ * it in this function.
+ */
+void watchdog_reset(void)
+{
+ static ulong next_reset;
+ ulong now;
+
+ /* Exit if GD is not ready or watchdog is not initialized yet */
+ if (!gd || !(gd->flags & GD_FLG_WDT_READY))
+ return;
+
+ /* Do not reset the watchdog too often */
+ now = get_timer(0);
+ if (now > next_reset) {
+ next_reset = now + 1000; /* reset every 1000ms */
+ wdt_reset(gd->watchdog_dev);
+ }
+}
+
+static void watchdog_start(void)
+{
+ u32 timeout = WDT_DEFAULT_TIMEOUT;
+
+ /*
+ * Use only the first watchdog device in U-Boot to trigger the
+ * watchdog reset
+ */
+ if (gd->watchdog_dev) {
I hope that gd structure is cleared somewhere.
I had this also in mind with respect to BSS not being cleared very
early. Its cleared in board_init_f_init_reserve(), so quite early
before board_init_f() is called.
+ debug("Only one watchdog device used!\n");
+ return;
+ }
+
+ /*
+ * Init watchdog: This will call the probe function of the
+ * watchdog driver, enabling the use of the device
+ */
+ if (uclass_get_device(UCLASS_WDT, 0,
+ (struct udevice **)&gd->watchdog_dev)) {
+ debug("Watchdog: Not found!\n");
+ return;
+ }
I think you should c&p what I have done in zynq/zynqmp. It means check
watchdog0 alias first because this is supposed to be the first primary
watchdog.
zynq/zynqmp/mb code
if (uclass_get_device_by_seq(UCLASS_WDT, 0, &watchdog_dev)) {
debug("Watchdog: Not found by seq!\n");
if (uclass_get_device(UCLASS_WDT, 0, &watchdog_dev)) {
puts("Watchdog: Not found!\n");
return 0;
}
}
In my (internal) first version, I had exactly this code included. I
removed the first part, as it makes no real sense to call it at this
stage (wdt_post_bind() for the first WDT found in the system). My
understanding is, that wdt_post_bind() will get called for all WDT
found in the system. My current logic here is, to only register the
first WDT found to the global watchdog_dev in GD. To prevent that
multiple WDT are used to service the U-Boot watchdog via WATCHDOG_RESET.
Or does the DM subsystem automatically bind the watchdog0 alias first
in its binding sequence?
I do not have a system with multiple watchdogs here, so its not that
easy to test this. I could simulate this though somehow...
+
+ if (CONFIG_IS_ENABLED(OF_CONTROL)) {
+ timeout = dev_read_u32_default(gd->watchdog_dev, "timeout-sec",
+ WDT_DEFAULT_TIMEOUT);
+ }
You should remove them from drivers.
drivers/watchdog/at91sam9_wdt.c:119: priv->timeout =
dev_read_u32_default(dev, "timeout-sec",
drivers/watchdog/cdns_wdt.c:238: priv->timeout =
dev_read_u32_default(dev, "timeout-sec",
Yes, its on my plan. I've also some additions for the AT91 driver on
the list since a few days, which will most likely get applied before
this patch. I'll send a cleanup patch to remove those now superfluous
DT handling later.
Thanks,
Stefan
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
