[U-Boot] [PATCH v1] watchdog: Introduce watchdog driver for Intel Tangier

From: Felipe Balbi felipe.balbi@linux.intel.com
Add watchdog driver for Intel Tangier based platforms.
Signed-off-by: Vincent Tinelli vincent.tinelli@intel.com Signed-off-by: Felipe Balbi felipe.balbi@linux.intel.com Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com --- common/board_f.c | 1 + drivers/watchdog/Kconfig | 8 ++++++ drivers/watchdog/Makefile | 1 + drivers/watchdog/tangier_wdt.c | 63 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 73 insertions(+) create mode 100644 drivers/watchdog/tangier_wdt.c
diff --git a/common/board_f.c b/common/board_f.c index d9431ee79a..ad1eae98a5 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -91,6 +91,7 @@ static int init_func_watchdog_init(void) (defined(CONFIG_M68K) || defined(CONFIG_MICROBLAZE) || \ defined(CONFIG_SH) || defined(CONFIG_AT91SAM9_WATCHDOG) || \ defined(CONFIG_DESIGNWARE_WATCHDOG) || \ + defined(CONFIG_TANGIER_WATCHDOG) || \ defined(CONFIG_IMX_WATCHDOG)) hw_watchdog_init(); puts(" Watchdog enabled\n"); diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index dbdaafc149..66fe70dba1 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -1,5 +1,13 @@ menu "WATCHDOG support"
+config TANGIER_WATCHDOG + bool "Intel Tangier watchdog" + depends on 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. + config ULP_WATCHDOG bool "i.MX7ULP watchdog" help diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile index dea18363ca..7b77d8379f 100644 --- a/drivers/watchdog/Makefile +++ b/drivers/watchdog/Makefile @@ -15,4 +15,5 @@ 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_TANGIER_WATCHDOG) += tangier_wdt.o obj-$(CONFIG_ULP_WATCHDOG) += ulp_wdog.o diff --git a/drivers/watchdog/tangier_wdt.c b/drivers/watchdog/tangier_wdt.c new file mode 100644 index 0000000000..23be71a42f --- /dev/null +++ b/drivers/watchdog/tangier_wdt.c @@ -0,0 +1,63 @@ +/* + * Copyright (c) 2017 Intel Corporation + * + * SPDX-License-Identifier: GPL-2.0+ + */ +#include <common.h> +#include <watchdog.h> +#include <asm/scu.h> + +/* Hardware timeout in seconds */ +#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, + SCU_WATCHDOG_KEEPALIVE = 2, + SCU_WATCHDOG_SET_ACTION_ON_TIMEOUT = 3, +}; + +void hw_watchdog_reset(void) +{ + static unsigned long prev; + unsigned long now; + + if (gd->timer) + now = timer_get_us(); + else + now = rdtsc() / 1000000; + + /* Do not flood SCU */ + if (unlikely((now - prev) > (WATCHDOG_HEARTBEAT * 1000))) { + prev = now; + scu_ipc_simple_command(IPCMSG_WATCHDOG_TIMER, SCU_WATCHDOG_KEEPALIVE); + } +} + +int hw_watchdog_disable(void) +{ + return scu_ipc_simple_command(IPCMSG_WATCHDOG_TIMER, SCU_WATCHDOG_STOP); +} + +void hw_watchdog_init(void) +{ + u32 timeout = WATCHDOG_HEARTBEAT / 1000; + int in_size; + struct ipc_wd_start { + u32 pretimeout; + u32 timeout; + } ipc_wd_start = { timeout, timeout }; + + /* + * SCU expects the input size for watchdog IPC + * to be based on 4 bytes + */ + in_size = DIV_ROUND_UP(sizeof(ipc_wd_start), 4); + + scu_ipc_command(IPCMSG_WATCHDOG_TIMER, SCU_WATCHDOG_START, + (u32 *)&ipc_wd_start, in_size, NULL, 0); +}

Hi Andy,
From: Felipe Balbi felipe.balbi@linux.intel.com
Add watchdog driver for Intel Tangier based platforms.
Signed-off-by: Vincent Tinelli vincent.tinelli@intel.com Signed-off-by: Felipe Balbi felipe.balbi@linux.intel.com Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com
common/board_f.c | 1 + drivers/watchdog/Kconfig | 8 ++++++ drivers/watchdog/Makefile | 1 + drivers/watchdog/tangier_wdt.c | 63 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 73 insertions(+) create mode 100644 drivers/watchdog/tangier_wdt.c
diff --git a/common/board_f.c b/common/board_f.c index d9431ee79a..ad1eae98a5 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -91,6 +91,7 @@ static int init_func_watchdog_init(void) (defined(CONFIG_M68K) || defined(CONFIG_MICROBLAZE) || \ defined(CONFIG_SH) || defined(CONFIG_AT91SAM9_WATCHDOG) || \ defined(CONFIG_DESIGNWARE_WATCHDOG) || \
- defined(CONFIG_TANGIER_WATCHDOG) || \ defined(CONFIG_IMX_WATCHDOG))
I have stumbled upon similar patch... There should be new Kconfig option created and enabled in required SoCs.
hw_watchdog_init(); puts(" Watchdog enabled\n"); diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index dbdaafc149..66fe70dba1 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -1,5 +1,13 @@ menu "WATCHDOG support"
+config TANGIER_WATCHDOG
- bool "Intel Tangier watchdog"
- depends on 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.
config ULP_WATCHDOG bool "i.MX7ULP watchdog" help diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile index dea18363ca..7b77d8379f 100644 --- a/drivers/watchdog/Makefile +++ b/drivers/watchdog/Makefile @@ -15,4 +15,5 @@ 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_TANGIER_WATCHDOG) += tangier_wdt.o obj-$(CONFIG_ULP_WATCHDOG) += ulp_wdog.o diff --git a/drivers/watchdog/tangier_wdt.c b/drivers/watchdog/tangier_wdt.c new file mode 100644 index 0000000000..23be71a42f --- /dev/null +++ b/drivers/watchdog/tangier_wdt.c @@ -0,0 +1,63 @@ +/*
- Copyright (c) 2017 Intel Corporation
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <watchdog.h> +#include <asm/scu.h>
+/* Hardware timeout in seconds */ +#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,
- SCU_WATCHDOG_KEEPALIVE = 2,
- SCU_WATCHDOG_SET_ACTION_ON_TIMEOUT = 3,
+};
+void hw_watchdog_reset(void) +{
- static unsigned long prev;
- unsigned long now;
- if (gd->timer)
now = timer_get_us();
- else
now = rdtsc() / 1000000;
- /* Do not flood SCU */
- if (unlikely((now - prev) > (WATCHDOG_HEARTBEAT * 1000))) {
prev = now;
scu_ipc_simple_command(IPCMSG_WATCHDOG_TIMER,
SCU_WATCHDOG_KEEPALIVE);
- }
+}
+int hw_watchdog_disable(void) +{
- return scu_ipc_simple_command(IPCMSG_WATCHDOG_TIMER,
SCU_WATCHDOG_STOP); +}
+void hw_watchdog_init(void) +{
- u32 timeout = WATCHDOG_HEARTBEAT / 1000;
- int in_size;
- struct ipc_wd_start {
u32 pretimeout;
u32 timeout;
- } ipc_wd_start = { timeout, timeout };
- /*
* SCU expects the input size for watchdog IPC
* to be based on 4 bytes
*/
- in_size = DIV_ROUND_UP(sizeof(ipc_wd_start), 4);
- scu_ipc_command(IPCMSG_WATCHDOG_TIMER, SCU_WATCHDOG_START,
(u32 *)&ipc_wd_start, in_size, NULL, 0);
+}
The code seems OK, but recently patches to add wdt-uclass has been posted:
http://patchwork.ozlabs.org/patch/751448/ http://patchwork.ozlabs.org/patch/751451/
Maybe it would be better to port this driver to the uclass from the very beginning?
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de

On Tue, 2017-04-18 at 16:49 +0200, Lukasz Majewski wrote:
Hi Andy,
From: Felipe Balbi felipe.balbi@linux.intel.com
Add watchdog driver for Intel Tangier based platforms.
Signed-off-by: Vincent Tinelli vincent.tinelli@intel.com Signed-off-by: Felipe Balbi felipe.balbi@linux.intel.com Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com
common/board_f.c | 1 + drivers/watchdog/Kconfig | 8 ++++++ drivers/watchdog/Makefile | 1 + drivers/watchdog/tangier_wdt.c | 63 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 73 insertions(+) create mode 100644 drivers/watchdog/tangier_wdt.c
diff --git a/common/board_f.c b/common/board_f.c index d9431ee79a..ad1eae98a5 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -91,6 +91,7 @@ static int init_func_watchdog_init(void) (defined(CONFIG_M68K) || defined(CONFIG_MICROBLAZE) || \ defined(CONFIG_SH) || defined(CONFIG_AT91SAM9_WATCHDOG) || \ defined(CONFIG_DESIGNWARE_WATCHDOG) || \
- defined(CONFIG_TANGIER_WATCHDOG) || \
defined(CONFIG_IMX_WATCHDOG))
I have stumbled upon similar patch... There should be new Kconfig option created and enabled in required SoCs.
I'm glad to use one if there is one. For now I prefer to have it functional.
The code seems OK, but recently patches to add wdt-uclass has been posted:
http://patchwork.ozlabs.org/patch/751448/ http://patchwork.ozlabs.org/patch/751451/
Maybe it would be better to port this driver to the uclass from the very beginning?
Good point. Will do.

On Tue, Apr 18, 2017 at 05:57:31PM +0300, Andy Shevchenko wrote:
On Tue, 2017-04-18 at 16:49 +0200, Lukasz Majewski wrote:
Hi Andy,
From: Felipe Balbi felipe.balbi@linux.intel.com
Add watchdog driver for Intel Tangier based platforms.
Signed-off-by: Vincent Tinelli vincent.tinelli@intel.com Signed-off-by: Felipe Balbi felipe.balbi@linux.intel.com Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com
common/board_f.c | 1 + drivers/watchdog/Kconfig | 8 ++++++ drivers/watchdog/Makefile | 1 + drivers/watchdog/tangier_wdt.c | 63 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 73 insertions(+) create mode 100644 drivers/watchdog/tangier_wdt.c
diff --git a/common/board_f.c b/common/board_f.c index d9431ee79a..ad1eae98a5 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -91,6 +91,7 @@ static int init_func_watchdog_init(void) (defined(CONFIG_M68K) || defined(CONFIG_MICROBLAZE) || \ defined(CONFIG_SH) || defined(CONFIG_AT91SAM9_WATCHDOG) || \ defined(CONFIG_DESIGNWARE_WATCHDOG) || \
- defined(CONFIG_TANGIER_WATCHDOG) || \
defined(CONFIG_IMX_WATCHDOG))
I have stumbled upon similar patch... There should be new Kconfig option created and enabled in required SoCs.
I'm glad to use one if there is one. For now I prefer to have it functional.
We have time to get this right. Looking at the code context, CONFIG_HW_WATCHDOG_ENABLE, and some help text saying that it enables the hardware watchdog during U-Boot's initialization process would be enough. Then select that in your next patch and you can leave the rest of the conversion for later. Thanks!

On Tue, 2017-04-18 at 16:49 +0200, Lukasz Majewski wrote:
The code seems OK, but recently patches to add wdt-uclass has been posted:
http://patchwork.ozlabs.org/patch/751448/ http://patchwork.ozlabs.org/patch/751451/
Maybe it would be better to port this driver to the uclass from the very beginning?
I started looking into this direction and have a question: what the point to move to that class if it's broken from the beginning?
I really do not understand who on the earth would like to have wdt_stop() at ->probe() (the only two users do exactly that!).
So, it looks to me now as bikeshedding, otherwise where is the documentation which describes how this all stuff should work?
Can we go with the initial patch?

On Tue, Jul 04, 2017 at 10:48:36PM +0300, Andy Shevchenko wrote:
On Tue, 2017-04-18 at 16:49 +0200, Lukasz Majewski wrote:
The code seems OK, but recently patches to add wdt-uclass has been posted:
http://patchwork.ozlabs.org/patch/751448/ http://patchwork.ozlabs.org/patch/751451/
Maybe it would be better to port this driver to the uclass from the very beginning?
I started looking into this direction and have a question: what the point to move to that class if it's broken from the beginning?
I really do not understand who on the earth would like to have wdt_stop() at ->probe() (the only two users do exactly that!).
It's quite possible I'm just missing something, but I don't see it. If the uclass is totally broken, can you make a suggestion on fixing it? I can see a common case being "lets turn off the watchdog now" and not want a wdt until the full OS, but we must also fully support "U-Boot has and pets the Watchdog".
So, it looks to me now as bikeshedding, otherwise where is the documentation which describes how this all stuff should work?
Can we go with the initial patch?
If it looks like some nightmare to fix the DM uclass, I suppose, but, is it really that bad off atm?

On Tue, 2017-07-04 at 16:31 -0400, Tom Rini wrote:
On Tue, Jul 04, 2017 at 10:48:36PM +0300, Andy Shevchenko wrote:
On Tue, 2017-04-18 at 16:49 +0200, Lukasz Majewski wrote:
The code seems OK, but recently patches to add wdt-uclass has been posted:
http://patchwork.ozlabs.org/patch/751448/ http://patchwork.ozlabs.org/patch/751451/
Maybe it would be better to port this driver to the uclass from the very beginning?
I started looking into this direction and have a question: what the point to move to that class if it's broken from the beginning?
I really do not understand who on the earth would like to have wdt_stop() at ->probe() (the only two users do exactly that!).
It's quite possible I'm just missing something, but I don't see it. If the uclass is totally broken, can you make a suggestion on fixing it?
I need to think about it (I have an idea, though it requires time to try).
I can see a common case being "lets turn off the watchdog now" and not want a wdt until the full OS, but we must also fully support "U-Boot has and pets the Watchdog".
The users of WDT class now just disable watchdog. One of them, though, is using it for reset (not my case for Tangier).
So, it looks to me now as bikeshedding, otherwise where is the documentation which describes how this all stuff should work?
Can we go with the initial patch?
If it looks like some nightmare to fix the DM uclass, I suppose, but, is it really that bad off atm?
Not nightmare, but I really have not much time. Simon approached me couple of weeks ago asking what is the status with my patches for Tangier. That's why I found a slot to look at it.
So, I just have sent a v2 of this driver with old approach to move things forward, otherwise I have no idea when it could be done.

Hi Andy,
On Tue, 2017-07-04 at 16:31 -0400, Tom Rini wrote:
On Tue, Jul 04, 2017 at 10:48:36PM +0300, Andy Shevchenko wrote:
On Tue, 2017-04-18 at 16:49 +0200, Lukasz Majewski wrote:
The code seems OK, but recently patches to add wdt-uclass has been posted:
http://patchwork.ozlabs.org/patch/751448/ http://patchwork.ozlabs.org/patch/751451/
Maybe it would be better to port this driver to the uclass from the very beginning?
I started looking into this direction and have a question: what the point to move to that class if it's broken from the beginning?
I really do not understand who on the earth would like to have wdt_stop() at ->probe() (the only two users do exactly that!).
At least for TI (omap), please find explanation in the description of this commit:
https://patchwork.ozlabs.org/patch/729819/
It's quite possible I'm just missing something, but I don't see it. If the uclass is totally broken, can you make a suggestion on fixing it?
I need to think about it (I have an idea, though it requires time to try).
I can see a common case being "lets turn off the watchdog now" and not want a wdt until the full OS, but we must also fully support "U-Boot has and pets the Watchdog".
The users of WDT class now just disable watchdog. One of them, though, is using it for reset (not my case for Tangier).
So, it looks to me now as bikeshedding, otherwise where is the documentation which describes how this all stuff should work?
Can we go with the initial patch?
If it looks like some nightmare to fix the DM uclass, I suppose, but, is it really that bad off atm?
Not nightmare, but I really have not much time. Simon approached me couple of weeks ago asking what is the status with my patches for Tangier. That's why I found a slot to look at it.
So, I just have sent a v2 of this driver with old approach to move things forward, otherwise I have no idea when it could be done.

On Wed, Jul 5, 2017 at 12:23 AM, lukma lukma@denx.de wrote:
On Tue, 2017-07-04 at 16:31 -0400, Tom Rini wrote:
On Tue, Jul 04, 2017 at 10:48:36PM +0300, Andy Shevchenko wrote:
On Tue, 2017-04-18 at 16:49 +0200, Lukasz Majewski wrote:
I started looking into this direction and have a question: what the point to move to that class if it's broken from the beginning?
I really do not understand who on the earth would like to have wdt_stop() at ->probe() (the only two users do exactly that!).
At least for TI (omap), please find explanation in the description of this commit:
No, it doesn't clarify for users of WDT class. It's another story AFAICS.

On Tue, 2017-07-04 at 16:31 -0400, Tom Rini wrote:
On Tue, Jul 04, 2017 at 10:48:36PM +0300, Andy Shevchenko wrote:
On Tue, 2017-04-18 at 16:49 +0200, Lukasz Majewski wrote:
So, it looks to me now as bikeshedding, otherwise where is the documentation which describes how this all stuff should work?
Can we go with the initial patch?
If it looks like some nightmare to fix the DM uclass, I suppose, but, is it really that bad off atm?
So, I have looked more, I even start creating patches and it's a deep hole. It looks like that class has nothing to do with the main purpose of watchdog. There is no integration into U-Boot watchdog infrastructure (to actually use it).
I'm about to test what I have, I need to correct a bit still, and will send a v3 using old approach. I will send patches against WDT class as RFC, but I'm not going to support them. My opinion is steady now -- that class is unusable in the current state.
P.S. If I'm missing something obvious I would like to hear it ASAP to make my driver use that class.
participants (5)
-
Andy Shevchenko
-
Andy Shevchenko
-
Lukasz Majewski
-
lukma
-
Tom Rini