| Message ID | 20190612172821.16907-1-andriy.shevchenko@linux.intel.com |
|---|---|
| State | Superseded |
| Delegated to: | Bin Meng |
| Headers | show |
| Series | [U-Boot,v2] watchdog: tangier: Convert to use WDT class | expand |
On 12.06.19 19:28, Andy Shevchenko wrote: > Convert legacy driver to use watchdog class. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > - dropped redundant assignments (Stefan) > - used clamp_t() instead of max_t()/min_t() pair > > arch/x86/cpu/tangier/Kconfig | 1 - > arch/x86/dts/edison.dts | 4 ++ > configs/edison_defconfig | 2 + > drivers/watchdog/Kconfig | 17 ++++---- > drivers/watchdog/Makefile | 2 +- > drivers/watchdog/tangier_wdt.c | 73 +++++++++++++++++++++------------- > 6 files changed, 60 insertions(+), 39 deletions(-) > > diff --git a/arch/x86/cpu/tangier/Kconfig b/arch/x86/cpu/tangier/Kconfig > index a3bd16799d..d2b7edecd6 100644 > --- a/arch/x86/cpu/tangier/Kconfig > +++ b/arch/x86/cpu/tangier/Kconfig > @@ -10,7 +10,6 @@ config INTEL_TANGIER > imply MMC_SDHCI > imply MMC_SDHCI_SDMA > imply MMC_SDHCI_TANGIER > - imply TANGIER_WATCHDOG > imply USB > imply USB_DWC3 > > diff --git a/arch/x86/dts/edison.dts b/arch/x86/dts/edison.dts > index e8564bbb8a..c0487656d3 100644 > --- a/arch/x86/dts/edison.dts > +++ b/arch/x86/dts/edison.dts > @@ -104,6 +104,10 @@ > reg = <0xff009000 0x1000>; > }; > > + watchdog: wdt@0 { > + compatible = "intel,tangier-wdt"; > + }; > + > reset { > compatible = "intel,reset-tangier"; > u-boot,dm-pre-reloc; > diff --git a/configs/edison_defconfig b/configs/edison_defconfig > index 840c87ac5a..468754493e 100644 > --- a/configs/edison_defconfig > +++ b/configs/edison_defconfig > @@ -39,5 +39,7 @@ CONFIG_USB_GADGET_VENDOR_NUM=0x8087 > CONFIG_USB_GADGET_PRODUCT_NUM=0x0a99 > CONFIG_USB_GADGET_DOWNLOAD=y > # CONFIG_USB_HOST_ETHER is not set > +CONFIG_WDT=y > +CONFIG_WDT_TANGIER=y > CONFIG_FAT_WRITE=y > CONFIG_SHA1=y > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig > index b01dbc446d..2270ed2e39 100644 > --- a/drivers/watchdog/Kconfig > +++ b/drivers/watchdog/Kconfig > @@ -34,15 +34,6 @@ config OMAP_WATCHDOG > help > Say Y here to enable the OMAP3+ watchdog driver. > > -config TANGIER_WATCHDOG > - bool "Intel Tangier watchdog" > - depends on INTEL_MID > - select HW_WATCHDOG > - help > - This enables support for watchdog controller available on > - Intel Tangier SoC. If you're using a board with Intel Tangier > - SoC, say Y here. > - > config ULP_WATCHDOG > bool "i.MX7ULP watchdog" > help > @@ -161,4 +152,12 @@ config WDT_MPC8xx > help > Select this to enable mpc8xx watchdog timer > > +config WDT_TANGIER > + bool "Intel Tangier watchdog timer support" > + depends on WDT && INTEL_MID > + help > + This enables support for watchdog controller available on > + Intel Tangier SoC. If you're using a board with Intel Tangier > + SoC, say Y here. > + > endmenu > diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile > index 6f20e73810..2dfc8a37b0 100644 > --- a/drivers/watchdog/Makefile > +++ b/drivers/watchdog/Makefile > @@ -14,7 +14,6 @@ obj-$(CONFIG_S5P) += s5p_wdt.o > obj-$(CONFIG_XILINX_TB_WATCHDOG) += xilinx_tb_wdt.o > obj-$(CONFIG_OMAP_WATCHDOG) += omap_wdt.o > obj-$(CONFIG_DESIGNWARE_WATCHDOG) += designware_wdt.o > -obj-$(CONFIG_TANGIER_WATCHDOG) += tangier_wdt.o > obj-$(CONFIG_ULP_WATCHDOG) += ulp_wdog.o > obj-$(CONFIG_WDT) += wdt-uclass.o > obj-$(CONFIG_WDT_SANDBOX) += sandbox_wdt.o > @@ -28,3 +27,4 @@ obj-$(CONFIG_WDT_MPC8xx) += mpc8xx_wdt.o > obj-$(CONFIG_WDT_MT7621) += mt7621_wdt.o > obj-$(CONFIG_WDT_MTK) += mtk_wdt.o > obj-$(CONFIG_WDT_SP805) += sp805_wdt.o > +obj-$(CONFIG_WDT_TANGIER) += tangier_wdt.o > diff --git a/drivers/watchdog/tangier_wdt.c b/drivers/watchdog/tangier_wdt.c > index be4a8f467a..21238dffa2 100644 > --- a/drivers/watchdog/tangier_wdt.c > +++ b/drivers/watchdog/tangier_wdt.c > @@ -3,7 +3,9 @@ > * Copyright (c) 2017 Intel Corporation > */ > #include <common.h> > -#include <watchdog.h> > +#include <dm.h> > +#include <wdt.h> > +#include <div64.h> > #include <asm/scu.h> > > /* Hardware timeout in seconds */ > @@ -12,12 +14,6 @@ > #define WDT_TIMEOUT_MAX 170 > #define WDT_DEFAULT_TIMEOUT 90 Nitpicking: If you by any chance need to re-send this patch again, please remove WDT_DEFAULT_TIMEOUT completely from this file. Reviewed-by: Stefan Roese <sr@denx.de> Thanks, Stefan > > -#ifndef CONFIG_WATCHDOG_TIMEOUT_MSECS > -#define WATCHDOG_HEARTBEAT 60000 > -#else > -#define WATCHDOG_HEARTBEAT CONFIG_WATCHDOG_TIMEOUT_MSECS > -#endif > - > enum { > SCU_WATCHDOG_START = 0, > SCU_WATCHDOG_STOP = 1, > @@ -25,39 +21,33 @@ enum { > SCU_WATCHDOG_SET_ACTION_ON_TIMEOUT = 3, > }; > > -void hw_watchdog_reset(void) > +static int tangier_wdt_reset(struct udevice *dev) > { > - static unsigned long last; > - unsigned long now; > - > - if (gd->timer) > - now = timer_get_us(); > - else > - now = rdtsc() / 1000; > - > - /* Do not flood SCU */ > - if (last > now) > - last = 0; > - > - if (unlikely((now - last) > (WDT_PRETIMEOUT / 2) * 1000000)) { > - last = now; > - scu_ipc_simple_command(IPCMSG_WATCHDOG_TIMER, SCU_WATCHDOG_KEEPALIVE); > - } > + scu_ipc_simple_command(IPCMSG_WATCHDOG_TIMER, SCU_WATCHDOG_KEEPALIVE); > + return 0; > } > > -int hw_watchdog_disable(void) > +static int tangier_wdt_stop(struct udevice *dev) > { > return scu_ipc_simple_command(IPCMSG_WATCHDOG_TIMER, SCU_WATCHDOG_STOP); > } > > -void hw_watchdog_init(void) > +static int tangier_wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags) > { > - u32 timeout = WATCHDOG_HEARTBEAT / 1000; > + u32 timeout_sec; > int in_size; > struct ipc_wd_start { > u32 pretimeout; > u32 timeout; > - } ipc_wd_start = { timeout - WDT_PRETIMEOUT, timeout }; > + } ipc_wd_start; > + > + /* Calculate timeout in seconds and restrict to min and max value */ > + do_div(timeout_ms, 1000); > + timeout_sec = clamp_t(u32, timeout_ms, WDT_TIMEOUT_MIN, WDT_TIMEOUT_MAX); > + > + /* Update values in the IPC request */ > + ipc_wd_start.pretimeout = timeout_sec - WDT_PRETIMEOUT; > + ipc_wd_start.timeout = timeout_sec; > > /* > * SCU expects the input size for watchdog IPC > @@ -67,4 +57,31 @@ void hw_watchdog_init(void) > > scu_ipc_command(IPCMSG_WATCHDOG_TIMER, SCU_WATCHDOG_START, > (u32 *)&ipc_wd_start, in_size, NULL, 0); > + > + return 0; > +} > + > +static const struct wdt_ops tangier_wdt_ops = { > + .reset = tangier_wdt_reset, > + .start = tangier_wdt_start, > + .stop = tangier_wdt_stop, > +}; > + > +static const struct udevice_id tangier_wdt_ids[] = { > + { .compatible = "intel,tangier-wdt" }, > + { /* sentinel */ } > +}; > + > +static int tangier_wdt_probe(struct udevice *dev) > +{ > + debug("%s: Probing wdt%u\n", __func__, dev->seq); > + return 0; > } > + > +U_BOOT_DRIVER(wdt_tangier) = { > + .name = "wdt_tangier", > + .id = UCLASS_WDT, > + .of_match = tangier_wdt_ids, > + .ops = &tangier_wdt_ops, > + .probe = tangier_wdt_probe, > +}; >
On Thu, Jun 13, 2019 at 06:51:34AM +0200, Stefan Roese wrote: > On 12.06.19 19:28, Andy Shevchenko wrote: > > #define WDT_DEFAULT_TIMEOUT 90 > > Nitpicking: If you by any chance need to re-send this patch again, > please remove WDT_DEFAULT_TIMEOUT completely from this file. This is left in order to have some traces of what firmware default is in use. I can convert this into comment. > Reviewed-by: Stefan Roese <sr@denx.de> Thanks!
On 13.06.19 13:10, Andy Shevchenko wrote: > On Thu, Jun 13, 2019 at 06:51:34AM +0200, Stefan Roese wrote: >> On 12.06.19 19:28, Andy Shevchenko wrote: > >>> #define WDT_DEFAULT_TIMEOUT 90 >> >> Nitpicking: If you by any chance need to re-send this patch again, >> please remove WDT_DEFAULT_TIMEOUT completely from this file. > > This is left in order to have some traces of what firmware default is in use. > I can convert this into comment. Sure. You also could remove it and use this original default value either via Kconfig (CONFIG_WATCHDOG_TIMEOUT_MSECS) or via DT ("timeout-sec" property). All is fine with me. Thanks, Stefan
Hi Andy, On Thu, Jun 13, 2019 at 7:10 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Thu, Jun 13, 2019 at 06:51:34AM +0200, Stefan Roese wrote: > > On 12.06.19 19:28, Andy Shevchenko wrote: > > > > #define WDT_DEFAULT_TIMEOUT 90 > > > > Nitpicking: If you by any chance need to re-send this patch again, > > please remove WDT_DEFAULT_TIMEOUT completely from this file. > > This is left in order to have some traces of what firmware default is in use. > I can convert this into comment. > Would you do a v3 to address this? > > Reviewed-by: Stefan Roese <sr@denx.de> > Regards, Bin
On Fri, Jun 21, 2019 at 03:19:39PM +0800, Bin Meng wrote: > On Thu, Jun 13, 2019 at 7:10 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > > > On Thu, Jun 13, 2019 at 06:51:34AM +0200, Stefan Roese wrote: > > > On 12.06.19 19:28, Andy Shevchenko wrote: > > > > > > #define WDT_DEFAULT_TIMEOUT 90 > > > > > > Nitpicking: If you by any chance need to re-send this patch again, > > > please remove WDT_DEFAULT_TIMEOUT completely from this file. > > > > This is left in order to have some traces of what firmware default is in use. > > I can convert this into comment. > > > > Would you do a v3 to address this? Yes. Just sent v3. > > > Reviewed-by: Stefan Roese <sr@denx.de>
diff --git a/arch/x86/cpu/tangier/Kconfig b/arch/x86/cpu/tangier/Kconfig index a3bd16799d..d2b7edecd6 100644 --- a/arch/x86/cpu/tangier/Kconfig +++ b/arch/x86/cpu/tangier/Kconfig @@ -10,7 +10,6 @@ config INTEL_TANGIER imply MMC_SDHCI imply MMC_SDHCI_SDMA imply MMC_SDHCI_TANGIER - imply TANGIER_WATCHDOG imply USB imply USB_DWC3 diff --git a/arch/x86/dts/edison.dts b/arch/x86/dts/edison.dts index e8564bbb8a..c0487656d3 100644 --- a/arch/x86/dts/edison.dts +++ b/arch/x86/dts/edison.dts @@ -104,6 +104,10 @@ reg = <0xff009000 0x1000>; }; + watchdog: wdt@0 { + compatible = "intel,tangier-wdt"; + }; + reset { compatible = "intel,reset-tangier"; u-boot,dm-pre-reloc; diff --git a/configs/edison_defconfig b/configs/edison_defconfig index 840c87ac5a..468754493e 100644 --- a/configs/edison_defconfig +++ b/configs/edison_defconfig @@ -39,5 +39,7 @@ CONFIG_USB_GADGET_VENDOR_NUM=0x8087 CONFIG_USB_GADGET_PRODUCT_NUM=0x0a99 CONFIG_USB_GADGET_DOWNLOAD=y # CONFIG_USB_HOST_ETHER is not set +CONFIG_WDT=y +CONFIG_WDT_TANGIER=y CONFIG_FAT_WRITE=y CONFIG_SHA1=y diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index b01dbc446d..2270ed2e39 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -34,15 +34,6 @@ config OMAP_WATCHDOG help Say Y here to enable the OMAP3+ watchdog driver. -config TANGIER_WATCHDOG - bool "Intel Tangier watchdog" - depends on INTEL_MID - select HW_WATCHDOG - help - This enables support for watchdog controller available on - Intel Tangier SoC. If you're using a board with Intel Tangier - SoC, say Y here. - config ULP_WATCHDOG bool "i.MX7ULP watchdog" help @@ -161,4 +152,12 @@ config WDT_MPC8xx help Select this to enable mpc8xx watchdog timer +config WDT_TANGIER + bool "Intel Tangier watchdog timer support" + depends on WDT && INTEL_MID + help + This enables support for watchdog controller available on + Intel Tangier SoC. If you're using a board with Intel Tangier + SoC, say Y here. + endmenu diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile index 6f20e73810..2dfc8a37b0 100644 --- a/drivers/watchdog/Makefile +++ b/drivers/watchdog/Makefile @@ -14,7 +14,6 @@ obj-$(CONFIG_S5P) += s5p_wdt.o obj-$(CONFIG_XILINX_TB_WATCHDOG) += xilinx_tb_wdt.o obj-$(CONFIG_OMAP_WATCHDOG) += omap_wdt.o obj-$(CONFIG_DESIGNWARE_WATCHDOG) += designware_wdt.o -obj-$(CONFIG_TANGIER_WATCHDOG) += tangier_wdt.o obj-$(CONFIG_ULP_WATCHDOG) += ulp_wdog.o obj-$(CONFIG_WDT) += wdt-uclass.o obj-$(CONFIG_WDT_SANDBOX) += sandbox_wdt.o @@ -28,3 +27,4 @@ obj-$(CONFIG_WDT_MPC8xx) += mpc8xx_wdt.o obj-$(CONFIG_WDT_MT7621) += mt7621_wdt.o obj-$(CONFIG_WDT_MTK) += mtk_wdt.o obj-$(CONFIG_WDT_SP805) += sp805_wdt.o +obj-$(CONFIG_WDT_TANGIER) += tangier_wdt.o diff --git a/drivers/watchdog/tangier_wdt.c b/drivers/watchdog/tangier_wdt.c index be4a8f467a..21238dffa2 100644 --- a/drivers/watchdog/tangier_wdt.c +++ b/drivers/watchdog/tangier_wdt.c @@ -3,7 +3,9 @@ * Copyright (c) 2017 Intel Corporation */ #include <common.h> -#include <watchdog.h> +#include <dm.h> +#include <wdt.h> +#include <div64.h> #include <asm/scu.h> /* Hardware timeout in seconds */ @@ -12,12 +14,6 @@ #define WDT_TIMEOUT_MAX 170 #define WDT_DEFAULT_TIMEOUT 90 -#ifndef CONFIG_WATCHDOG_TIMEOUT_MSECS -#define WATCHDOG_HEARTBEAT 60000 -#else -#define WATCHDOG_HEARTBEAT CONFIG_WATCHDOG_TIMEOUT_MSECS -#endif - enum { SCU_WATCHDOG_START = 0, SCU_WATCHDOG_STOP = 1, @@ -25,39 +21,33 @@ enum { SCU_WATCHDOG_SET_ACTION_ON_TIMEOUT = 3, }; -void hw_watchdog_reset(void) +static int tangier_wdt_reset(struct udevice *dev) { - static unsigned long last; - unsigned long now; - - if (gd->timer) - now = timer_get_us(); - else - now = rdtsc() / 1000; - - /* Do not flood SCU */ - if (last > now) - last = 0; - - if (unlikely((now - last) > (WDT_PRETIMEOUT / 2) * 1000000)) { - last = now; - scu_ipc_simple_command(IPCMSG_WATCHDOG_TIMER, SCU_WATCHDOG_KEEPALIVE); - } + scu_ipc_simple_command(IPCMSG_WATCHDOG_TIMER, SCU_WATCHDOG_KEEPALIVE); + return 0; } -int hw_watchdog_disable(void) +static int tangier_wdt_stop(struct udevice *dev) { return scu_ipc_simple_command(IPCMSG_WATCHDOG_TIMER, SCU_WATCHDOG_STOP); } -void hw_watchdog_init(void) +static int tangier_wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags) { - u32 timeout = WATCHDOG_HEARTBEAT / 1000; + u32 timeout_sec; int in_size; struct ipc_wd_start { u32 pretimeout; u32 timeout; - } ipc_wd_start = { timeout - WDT_PRETIMEOUT, timeout }; + } ipc_wd_start; + + /* Calculate timeout in seconds and restrict to min and max value */ + do_div(timeout_ms, 1000); + timeout_sec = clamp_t(u32, timeout_ms, WDT_TIMEOUT_MIN, WDT_TIMEOUT_MAX); + + /* Update values in the IPC request */ + ipc_wd_start.pretimeout = timeout_sec - WDT_PRETIMEOUT; + ipc_wd_start.timeout = timeout_sec; /* * SCU expects the input size for watchdog IPC @@ -67,4 +57,31 @@ void hw_watchdog_init(void) scu_ipc_command(IPCMSG_WATCHDOG_TIMER, SCU_WATCHDOG_START, (u32 *)&ipc_wd_start, in_size, NULL, 0); + + return 0; +} + +static const struct wdt_ops tangier_wdt_ops = { + .reset = tangier_wdt_reset, + .start = tangier_wdt_start, + .stop = tangier_wdt_stop, +}; + +static const struct udevice_id tangier_wdt_ids[] = { + { .compatible = "intel,tangier-wdt" }, + { /* sentinel */ } +}; + +static int tangier_wdt_probe(struct udevice *dev) +{ + debug("%s: Probing wdt%u\n", __func__, dev->seq); + return 0; } + +U_BOOT_DRIVER(wdt_tangier) = { + .name = "wdt_tangier", + .id = UCLASS_WDT, + .of_match = tangier_wdt_ids, + .ops = &tangier_wdt_ops, + .probe = tangier_wdt_probe, +};
Convert legacy driver to use watchdog class. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- - dropped redundant assignments (Stefan) - used clamp_t() instead of max_t()/min_t() pair arch/x86/cpu/tangier/Kconfig | 1 - arch/x86/dts/edison.dts | 4 ++ configs/edison_defconfig | 2 + drivers/watchdog/Kconfig | 17 ++++---- drivers/watchdog/Makefile | 2 +- drivers/watchdog/tangier_wdt.c | 73 +++++++++++++++++++++------------- 6 files changed, 60 insertions(+), 39 deletions(-)