watchdog: Introduce watchdog driver for Intel Tangier

PATCH New Andy Shevchenko 4 participants 10 messages Apr 18, 2017 – Jul 5, 2017 Watchdog device drivers

lore

Share & tools

lore

Andy Shevchenko

author

2017-04-18 13:51 UTC #0 lore raw

PATCH 1/1 watchdog: Introduce watchdog driver for Intel Tangier

From: Felipe Balbi <[email protected]>

Add watchdog driver for Intel Tangier based platforms.

Signed-off-by Vincent Tinelli <[email protected]>

Signed-off-by Felipe Balbi <[email protected]>

Signed-off-by Andy Shevchenko <[email protected]>

---
 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

common/board_f.c +1 -0

index d9431ee79a..ad1eae98a5 100644
@@ -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");

drivers/watchdog/Kconfig +8 -0

index dbdaafc149..66fe70dba1 100644
@@ -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

drivers/watchdog/Makefile +1 -0

index dea18363ca..7b77d8379f 100644
@@ -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

drivers/watchdog/tangier_wdt.c new file +63 -0

63 lines not on this page · raw message

Signature · 1 line

2.11.0

Lukasz Majewski

2017-04-18 14:49 UTC #1 lore raw

PATCH 1/1 watchdog: Introduce watchdog driver for Intel Tangier

Hi Andy,

Quoted 20 lines

From: Felipe Balbi <[email protected]>
Add watchdog driver for Intel Tangier based platforms.
Signed-off-by Vincent Tinelli <[email protected]>
Signed-off-by Felipe Balbi <[email protected]>
Signed-off-by Andy Shevchenko <[email protected]>
---
 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
common/board_f.c +0 -0
index d9431ee79a..ad1eae98a5 100644

common/board_f.c static int init_func_watchdog_init(void) · line 95

Quoted 5 lines

common/board_f.c +1 -0

@@ -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.

Quoted 91 lines

common/board_f.c +0 -0

 	hw_watchdog_init();
 	puts("       Watchdog enabled\n");

drivers/watchdog/Kconfig +6 -0

index dbdaafc149..66fe70dba1 100644
@@ -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

drivers/watchdog/Kconfig +2 -0

+	  SoC, say Y here.
+
 config ULP_WATCHDOG
 	bool "i.MX7ULP watchdog"
 	help

drivers/watchdog/Makefile +1 -0

index dea18363ca..7b77d8379f 100644
@@ -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

(quoted diff) +0 -0

b/drivers/watchdog/tangier_wdt.c new file mode 100644 index 0000000000..23be71a42f

drivers/watchdog/tangier_wdt.c new file +37 -0

@@ -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);

drivers/watchdog/tangier_wdt.c new file +6 -0

+	}
+}
+
+int hw_watchdog_disable(void)
+{
+	return scu_ipc_simple_command(IPCMSG_WATCHDOG_TIMER,

SCU_WATCHDOG_STOP); +}

drivers/watchdog/tangier_wdt.c new file +9 -0

+
+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 };

drivers/watchdog/tangier_wdt.c line 63

Quoted 10 lines

drivers/watchdog/tangier_wdt.c new file +10 -0

+
+	/*
+	 * 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 at denx.de

Andy Shevchenko

author

2017-04-18 14:57 UTC #2 lore raw

PATCH 1/1 watchdog: Introduce watchdog driver for Intel Tangier

Quoted 17 lines · Lukasz Majewski

Hi Andy,
Quoted 15 lines
From: Felipe Balbi <[email protected]>
Add watchdog driver for Intel Tangier based platforms.
Signed-off-by Vincent Tinelli <[email protected]>
Signed-off-by Felipe Balbi <[email protected]>
Signed-off-by Andy Shevchenko <[email protected]>
---
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

Quoted 14 lines

Quoted 11 lines
common/board_f.c +0 -0
index d9431ee79a..ad1eae98a5 100644
@@ -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) || \
common/board_f.c +1 -0
+	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.

Quoted 8 lines

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.

Signature · 2 lines

Andy Shevchenko <[email protected]>
Intel Finland Oy

Tom Rini

2017-04-18 16:30 UTC #3 lore raw

PATCH 1/1 watchdog: Introduce watchdog driver for Intel Tangier

Quoted 33 lines · Andy Shevchenko

Quoted 17 lines · Lukasz Majewski
Hi Andy,
Quoted 15 lines
From: Felipe Balbi <[email protected]>
Add watchdog driver for Intel Tangier based platforms.
Signed-off-by Vincent Tinelli <[email protected]>
Signed-off-by Felipe Balbi <[email protected]>
Signed-off-by Andy Shevchenko <[email protected]>
---
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
Quoted 14 lines
Quoted 11 lines
common/board_f.c +0 -0
index d9431ee79a..ad1eae98a5 100644
@@ -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) || \
common/board_f.c +1 -0
+	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.

Quoted 2 lines

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!

Signature · 8 lines

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170418/9fe36ea0/attachment.sig>

Andy Shevchenko

author

2017-07-04 19:48 UTC #4 lore raw

PATCH 1/1 watchdog: Introduce watchdog driver for Intel Tangier

In reply to Lukasz Majewski

Quoted 9 lines · Lukasz Majewski

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?

Signature · 2 lines

Andy Shevchenko <[email protected]>
Intel Finland Oy

Tom Rini

2017-07-04 20:31 UTC #5 lore raw

PATCH 1/1 watchdog: Introduce watchdog driver for Intel Tangier

Quoted 8 lines · Andy Shevchenko

Quoted 7 lines · Lukasz Majewski
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/

Quoted 8 lines

Quoted 2 lines
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".

Quoted 4 lines

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?

Signature · 8 lines

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170704/b839097c/attachment.sig>

Andy Shevchenko

author

2017-07-04 20:41 UTC #6 lore raw

PATCH 1/1 watchdog: Introduce watchdog driver for Intel Tangier

Quoted 17 lines · Tom Rini

Quoted 16 lines · Andy Shevchenko
Quoted 10 lines · Lukasz Majewski
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?

Quoted 6 lines

Quoted 2 lines
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).

Quoted 5 lines

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).

Quoted 8 lines

Quoted 4 lines
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.

Signature · 2 lines

Andy Shevchenko <[email protected]>
Intel Finland Oy

lukma

2017-07-04 21:23 UTC #7 lore raw

PATCH 1/1 watchdog: Introduce watchdog driver for Intel Tangier

Hi Andy,

Quoted 14 lines · Tom Rini

Quoted 13 lines · Andy Shevchenko
Quoted 12 lines · Lukasz Majewski
Quoted 10 lines
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?

Quoted 6 lines

Quoted 6 lines
Quoted 6 lines
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/

Quoted 33 lines

Quoted 4 lines
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).
Quoted 5 lines
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).
Quoted 8 lines
Quoted 4 lines
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.

Signature · 9 lines

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 at denx.de

Andy Shevchenko

2017-07-04 22:36 UTC #8 lore raw

PATCH 1/1 watchdog: Introduce watchdog driver for Intel Tangier

Quoted 8 lines · lukma

Quoted 8 lines · Tom Rini
Quoted 7 lines · Andy Shevchenko
Quoted 6 lines · Lukasz Majewski
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?

Quoted 8 lines

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
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/

No, it doesn't clarify for users of WDT class. It's another story AFAICS.

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Andy Shevchenko

author

2017-07-05 15:59 UTC #9 lore raw

PATCH 1/1 watchdog: Introduce watchdog driver for Intel Tangier

In reply to Tom Rini

Quoted 11 lines · Tom Rini

Quoted 6 lines · Andy Shevchenko
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.

Signature · 2 lines

Andy Shevchenko <[email protected]>
Intel Finland Oy