On 10.04.19 10:46, Michal Simek wrote:
On 10. 04. 19 10:40, Stefan Roese wrote:
On 10.04.19 09:44, Michal Simek wrote:
On 09. 04. 19 16:34, Stefan Roese wrote:
On 09.04.19 16:27, Michal Simek wrote:
On 09. 04. 19 16:22, Stefan Roese wrote:
On 09.04.19 12:45, Michal Simek wrote:
On 08. 04. 19 11:28, Stefan Roese wrote:
This patch tries to implement a generic watchdog_reset() function
that
can be used by all boards that want to service the watchdog
device in
U-Boot. This watchdog servicing is enabled via CONFIG_WATCHDOG.
Without this approach, new boards or platforms needed to implement a
board specific version of this functionality, mostly copy'ing the
same
code over and over again into their board or platforms code base.
With this new generic function, the scattered other functions are
now
removed to be replaced by the generic one. The new version also
enables
the configuration of the watchdog timeout via the DT "timeout-sec"
property (if enabled via CONFIG_OF_CONTROL).
This patch also adds a new flag to the GD flags, to flag that the
watchdog is ready to use and adds the pointer to the watchdog device
to the GD. This enables us to remove the global "watchdog_dev"
variable, which was prone to cause problems because of its
potentially
very early use in watchdog_reset(), even before the BSS is cleared.
Signed-off-by: Stefan Roese <[email protected]>
Cc: Heiko Schocher <[email protected]>
Cc: Tom Rini <[email protected]>
Cc: Michal Simek <[email protected]>
Cc: "Marek Behún" <[email protected]>
Cc: Daniel Schwierzeck <[email protected]>
---
This patch depends on the following patches:
[1] watchdog: Move watchdog_dev to data section (BSS may not be
cleared)
https://patchwork.ozlabs.org/patch/1075500/
[2] watchdog: Handle SPL build with watchdog disabled
https://patchwork.ozlabs.org/patch/1074098/
I would like to see [1] applied in this release, since its a real
fix on
some of the platforms with minimal chances of breakage.
This patch now is a bigger rework and is intended for the next merge
window.
Please note that I didn't remove the "timeout-sec" handling from the
driver already reading it from the DT (cdns & at91) in this patch.
The reason for this is, that in the cdns case, the removal also
brings
a functional change, which I wanted to do in a separate patch. And
in the at91 case its because there are updates to this driver
already
queued in the at91 next git branch which would most likely cause
merge
conflict. I'll send a cleanup patch for this driver later after
these
patches have been applied.
Thanks,
Stefan
v2:
- Rename watchdog_start() to initr_watchdog() and move it to
board_r.c.
This way its only called once, after the DM subsystem has bound
all
watchdog drivers. This enables the use of the currently
implemented
logic of using the correct watchdog in U-Boot (via alias etc).
arch/mips/mach-mt7620/cpu.c | 36
-----------------
board/CZ.NIC/turris_mox/turris_mox.c | 30
--------------
board/CZ.NIC/turris_omnia/turris_omnia.c | 35
----------------
.../microblaze-generic/microblaze-generic.c | 40
-------------------
board/xilinx/zynq/board.c | 39
------------------
board/xilinx/zynqmp/zynqmp.c | 39
------------------
common/board_r.c | 40
+++++++++++++++++++
drivers/watchdog/wdt-uclass.c | 26 ++++++++++++
include/asm-generic/global_data.h | 4 ++
9 files changed, 70 insertions(+), 219 deletions(-)
<snip>
int board_early_init_r(void)
{
u32 val;
diff --git a/common/board_r.c b/common/board_r.c
index 472987d5d5..d80f16c3ed 100644
--- a/common/board_r.c
+++ b/common/board_r.c
@@ -48,6 +48,7 @@
#include <linux/compiler.h>
#include <linux/err.h>
#include <efi_loader.h>
+#include <wdt.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -621,6 +622,42 @@ static int initr_bedbug(void)
}
#endif
+#if defined(CONFIG_WATCHDOG) && defined(CONFIG_WDT)
This is not correct.
Here should be just CONFIG_WDT.
Because there are cases where you want just start watchdog in u-boot
but
never service it by u-boot.
AFAIK, this would change the current behavior. Currently, only when
CONFIG_WATCHDOG is enabled the watchdog driver is started
automatically
in U-Boot (and serviced). If I make the change you suggest above, all
boards defining CONFIG_WDT (DM watchdog support) will automatically
enable the watchdog. This might make sense, but AFAICT this changes
the current behavior.
The question is what default is. Because when I was adding support for
Zynq/ZynqMP/Microblaze this logic is used there.
I think that WDT should be there and if you think there are boards
which
want to have both we can cover that by Kconfig.
As mentioned above, I agree that it makes sense to start the watchdog
automatically, if its enabled / selected via CONFIG_WDT. I suggest
that I move forward with your suggested change, but it would be fair
to Cc the maintainers of boards that have CONFIG_WDT set but
CONFIG_WATCHDOG unset. Do you (or anyone else) know of a tool / script
to detect such board configurations in U-Boot (one option set and
another unset)?
good.
Tom?
I've run a Travis job on all boards with an error added for this
configuration (CONFIG_WDT set but CONFIG_WATCHDOG unset). Here the
list of these boards with their maintainers:
taurus: Heiko Schocher <[email protected]>
smartweb: Heiko Schocher <[email protected]>
ast2500: Maxim Sloyko <[email protected]>
picosam9g45: Erik van Luijk <[email protected]>
mt7623n_bpir2: Ryder Lee <[email protected]> Weijie Gao
<[email protected]>
mt7629_rfb: Ryder Lee <[email protected]> Weijie Gao
<[email protected]>
bitmain_antminer_s9: Michal Simek <[email protected]>
On this board disabling watchdog servicing was done by purpose.
And it will stay disabled even with the imply, as the defconfig
currently disables CONFIG_WATCHDOG. So no change here.
BTW: bitmain_antminer_s9_defconfig seems to be the only board that
disables CONFIG_WATCHDOG.
Thanks,
Stefan
_______________________________________________
U-Boot mailing list
[email protected]
https://lists.denx.de/listinfo/u-boot
