On Fri, 5 Mar 2021, 5:09 PM Simon Glass, sjg@chromium.org wrote:
Hi Chris,
On Tue, 2 Mar 2021 at 16:00, Chris Packham judge.packham@gmail.com wrote:
In commit 75918afa649b ("powerpc: Drop old non-generic-board code") we lost the call to init_85xx_watchdog() which had the effect of disabling support for the watchdog on 85xx and similar SoCs (i.e. the QorIQ P Series and T Series).
Signed-off-by: Chris Packham judge.packham@gmail.com
Admittedly this is a bit ugly but it's the most literal reinstatement of the code that was lost. At the very least I should probably rename init_85xx_watchdog() to hw_watchdog_init() so it fits with the rest of the code.
The other question is how has this gone unnoticed for ~5 years. I think the answer is because only the keymile boards were using it.
Finally I am wondering if this should be converted to a DM driver. But given the fact that the watchdog is part of the core and not a peripheral on the SoC I don't know how that would look (e.g. what would I put in the DTS?).
So I thought I'd run this up the flag pole as-is and see what feedback I get.
common/board_f.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/common/board_f.c b/common/board_f.c index 0cddf0359dca..3778571a7196 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -111,6 +111,11 @@ static int init_func_watchdog_init(void) hw_watchdog_init(); puts(" Watchdog enabled\n"); # endif +# ifdef CONFIG_MPC85xx
init_85xx_watchdog();That should be in a header file.
puts(" Watchdog enabled\n");+# endif
WATCHDOG_RESET(); return 0;-- 2.30.1
I don't think you have made the existing code worse...so I suppose it is OK.
But really this should be converted to DM. I just noticed there is no watchdog uclass.
It's called UCLASS_WDT. In the end I went with implementing a DM driver. Assuming that lands there will be some old code that can be removed. I'd still like to here from the keymile maintainers as they're the only in tree user of this.
- Simon