[U-Boot] [PATCH] mxs: add watchdog

[Stefano Babic <sbabic@denx.de>]

Hi Alexey,

sorry for delay - I missed your patch.

On 08/10/2014 05:14, Alexey Ignatov wrote:
> Use RTC watchdog feature as hardware watchdog.
> 
> Signed-off-by: Alexey Ignatov <lexszero@gmail.com>
> ---
>  drivers/watchdog/Makefile     |  1 +
>  drivers/watchdog/mxsrtc_wdt.c | 23 +++++++++++++++++++++++
>  include/configs/mxs.h         |  4 ++++
>  3 files changed, 28 insertions(+)
>  create mode 100644 drivers/watchdog/mxsrtc_wdt.c
> 
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index 0276a10..ffe42d1 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -16,3 +16,4 @@ obj-$(CONFIG_XILINX_TB_WATCHDOG) += xilinx_tb_wdt.o
>  obj-$(CONFIG_BFIN_WATCHDOG)  += bfin_wdt.o
>  obj-$(CONFIG_OMAP_WATCHDOG) += omap_wdt.o
>  obj-$(CONFIG_DESIGNWARE_WATCHDOG) += designware_wdt.o
> +obj-$(CONFIG_MXS_RTC_WATCHDOG) += mxsrtc_wdt.o
> diff --git a/drivers/watchdog/mxsrtc_wdt.c b/drivers/watchdog/mxsrtc_wdt.c
> new file mode 100644
> index 0000000..33e7663
> --- /dev/null
> +++ b/drivers/watchdog/mxsrtc_wdt.c
> @@ -0,0 +1,23 @@
> +#include <common.h>
> +#include <asm/io.h>
> +#include <watchdog.h>
> +#include <asm/arch/imx-regs.h>
> +
> +#ifdef CONFIG_MXS_RTC_WATCHDOG

You do not need this #ifdef. The file is compiled only if
CONFIG_MXS_RTC_WATCHDOG, as you set in Makefile. You can drop it.


> +#ifndef CONFIG_WATCHDOG_TIMEOUT_MSECS
> +#define CONFIG_WATCHDOG_TIMEOUT_MSECS 60000
> +#endif


> +void hw_watchdog_reset(void)
> +{
> +	struct mxs_rtc_regs *rtc_regs = (struct mxs_rtc_regs *)MXS_RTC_BASE;
> +	writel(CONFIG_WATCHDOG_TIMEOUT_MSECS, &rtc_regs->hw_rtc_watchdog);
> +}
> +
> +void hw_watchdog_init(void)
> +{
> +	struct mxs_rtc_regs *rtc_regs = (struct mxs_rtc_regs *)MXS_RTC_BASE;
> +
> +	hw_watchdog_reset();

I think it does not matter, but why do we need to trigger the timer if
we have not yet initialized it ?

> +	writel(RTC_CTRL_WATCHDOGEN, &rtc_regs->hw_rtc_watchdog_set);
> +}
> +#endif
> diff --git a/include/configs/mxs.h b/include/configs/mxs.h
> index 8bce28f..1f1b87a 100644
> --- a/include/configs/mxs.h
> +++ b/include/configs/mxs.h
> @@ -202,4 +202,8 @@
>  #define CONFIG_EHCI_IS_TDI
>  #endif
>  
> +#ifdef CONFIG_HW_WATCHDOG
> +#define CONFIG_MXS_RTC_WATCHDOG
> +#endif

You have to split in two patch. The first one for the driver, the second
one to add support for the board(s).

However, if you add it here, all boards will start the watchdog and
maybe some of them do not want to have it. Trigger must be done later in
kernel, too. I think each board maintainer should decide if he want to
have watchdog enable or not.

Please move CONFIG_HW_WATCHDOG to the board where you want that it must
be on, and please put the board's maintainer in CC in your next version
- thanks !

Best regards,
Stefano Babic
> +
>  #endif	/* __CONFIGS_MXS_H__ */
> 


-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================