Hi Chris,
On Tue, 2 Mar 2021 at 16:00, Chris Packham <judge.pack...@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.pack...@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.
- Simon
