[PATCH 0/2] Initial implementation of Intel MID watchdog driver

[PATCH 0/2] Initial implementation of Intel MID watchdog driver

[David Cohen]

Hi,

This patch set is an initial implementation of a generic Intel MID watchdog
driver.
It currently supports Intel Merrifield only, but will extend to other SoCs of
MID family.

Br, David

--
David Cohen (2):
  watchdog: add Intel MID watchdog driver support
  x86: intel-mid: add watchdog platform code for Merrifield

 arch/x86/platform/intel-mid/device_libs/Makefile   |   1 +
 .../platform/intel-mid/device_libs/platform_wdt.c  |  43 +++++
 drivers/watchdog/Kconfig                           |  12 ++
 drivers/watchdog/Makefile                          |   1 +
 drivers/watchdog/intel-mid_wdt.c                   | 209 +++++++++++++++++++++
 5 files changed, 266 insertions(+)
 create mode 100644 arch/x86/platform/intel-mid/device_libs/platform_wdt.c
 create mode 100644 drivers/watchdog/intel-mid_wdt.c

-- 
1.9.0

[PATCH 1/2] watchdog: add Intel MID watchdog driver support

[David Cohen]

Add initial Intel MID watchdog driver support.

This driver is an initial implementation of generic Intel MID watchdog
driver. Currently it supports Intel Merrifield platform.

Signed-off-by: Eric Ernst <eric.ernst@intel.com>
Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
---
 drivers/watchdog/Kconfig         |  12 +++
 drivers/watchdog/Makefile        |   1 +
 drivers/watchdog/intel-mid_wdt.c | 209 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 222 insertions(+)
 create mode 100644 drivers/watchdog/intel-mid_wdt.c

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 79d25894343a..4da09b8b2f11 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -643,6 +643,18 @@ config INTEL_SCU_WATCHDOG
 
 	  To compile this driver as a module, choose M here.
 
+config INTEL_MID_WATCHDOG
+	bool "Intel MID SCU Watchdog Mobile Platforms"
+	depends on X86_INTEL_MID && WATCHDOG_CORE
+	---help---
+	  Watchdog timer driver built into the Intel SCU for Intel MID
+	  Platforms.
+
+	  This driver currently supports only the watchdog evolution
+	  implementation in SCU, available for Merrifield generation.
+
+	  To compile this driver as a module, choose M here.
+
 config ITCO_WDT
 	tristate "Intel TCO Timer/Watchdog"
 	depends on (X86 || IA64) && PCI
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 985a66cda76f..b927e7496eb4 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -113,6 +113,7 @@ obj-$(CONFIG_W83977F_WDT) += w83977f_wdt.o
 obj-$(CONFIG_MACHZ_WDT) += machzwd.o
 obj-$(CONFIG_SBC_EPX_C3_WATCHDOG) += sbc_epx_c3.o
 obj-$(CONFIG_INTEL_SCU_WATCHDOG) += intel_scu_watchdog.o
+obj-$(CONFIG_INTEL_MID_WATCHDOG) += intel-mid_wdt.o
 
 # M32R Architecture
 
diff --git a/drivers/watchdog/intel-mid_wdt.c b/drivers/watchdog/intel-mid_wdt.c
new file mode 100644
index 000000000000..6339c6351454
--- /dev/null
+++ b/drivers/watchdog/intel-mid_wdt.c
@@ -0,0 +1,209 @@
+/*
+ *      intel-mid_wdt: generic Intel MID SCU watchdog driver
+ *
+ *      Platforms supported so far:
+ *      - Merrifield only
+ *
+ *      Copyright (C) 2014 Intel Corporation. All rights reserved.
+ *      Contact: David Cohen <david.a.cohen@linux.intel.com>
+ *
+ *      This program is free software; you can redistribute it and/or
+ *      modify it under the terms of version 2 of the GNU General
+ *      Public License as published by the Free Software Foundation.
+ */
+
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/nmi.h>
+#include <linux/platform_device.h>
+#include <linux/watchdog.h>
+
+#include <asm/intel_scu_ipc.h>
+#include <asm/intel-mid.h>
+
+#define IPC_WATCHDOG 0xf8
+
+#define MID_WDT_PRETIMEOUT		15
+#define MID_WDT_TIMEOUT_MIN		(1 + MID_WDT_PRETIMEOUT)
+#define MID_WDT_TIMEOUT_MAX		170
+#define MID_WDT_DEFAULT_TIMEOUT		90
+
+/* SCU watchdog messages */
+enum {
+	SCU_WATCHDOG_START = 0,
+	SCU_WATCHDOG_STOP,
+	SCU_WATCHDOG_KEEPALIVE,
+};
+
+static inline int wdt_command(int sub, u32 *in, int inlen)
+{
+	return intel_scu_ipc_command(IPC_WATCHDOG, sub, in, inlen, NULL, 0);
+}
+
+static int wdt_start(struct watchdog_device *wd)
+{
+	int ret, in_size;
+	int timeout = wd->timeout;
+	struct ipc_wd_start {
+		u32 pretimeout;
+		u32 timeout;
+	} ipc_wd_start = { timeout - MID_WDT_PRETIMEOUT, timeout };
+
+	/*
+	 * SCU expects the input size for watchdog IPC to
+	 * be based on double-word
+	 */
+	in_size = (sizeof(ipc_wd_start) + 3) / 4;
+
+	ret = wdt_command(SCU_WATCHDOG_START, (u32 *)&ipc_wd_start, in_size);
+	if (ret) {
+		struct device *dev = watchdog_get_drvdata(wd);
+		dev_crit(dev, "error starting watchdog: %d\n", ret);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int wdt_ping(struct watchdog_device *wd)
+{
+	int ret;
+
+	ret = wdt_command(SCU_WATCHDOG_KEEPALIVE, NULL, 0);
+	if (ret) {
+		struct device *dev = watchdog_get_drvdata(wd);
+		dev_crit(dev, "Error executing keepalive: 0x%x\n", ret);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int wdt_stop(struct watchdog_device *wd)
+{
+	int ret;
+
+	ret = wdt_command(SCU_WATCHDOG_STOP, NULL, 0);
+	if (ret) {
+		struct device *dev = watchdog_get_drvdata(wd);
+		dev_crit(dev, "Error stopping watchdog: 0x%x\n", ret);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static irqreturn_t mid_wdt_warning_irq(int irq, void *dev_id)
+{
+	/* Let's dump some data before panic */
+	trigger_all_cpu_backtrace();
+	panic("Kernel Watchdog");
+
+	/* This code should not be reached */
+	return IRQ_HANDLED;
+}
+
+/*
+ * Current driver supports Merrifield only.
+ * This function will get more generic when necessary.
+ */
+static int handle_dev_ioapic(struct device *dev, int irq)
+{
+	int ioapic;
+	struct io_apic_irq_attr irq_attr;
+
+	ioapic = mp_find_ioapic(irq);
+	if (ioapic >= 0) {
+		irq_attr.ioapic = ioapic;
+		irq_attr.ioapic_pin = irq;
+		irq_attr.trigger = 1;
+		irq_attr.polarity = 0; /* Active high */
+		io_apic_set_pci_routing(NULL, irq, &irq_attr);
+	} else {
+		dev_warn(dev, "can not find interrupt %d in ioapic\n", irq);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static const struct watchdog_info mid_wdt_info = {
+	.identity = "Intel MID SCU watchdog",
+	.options = WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT,
+};
+
+static const struct watchdog_ops mid_wdt_ops = {
+	.owner = THIS_MODULE,
+	.start = wdt_start,
+	.stop = wdt_stop,
+	.ping = wdt_ping,
+};
+
+static int mid_wdt_probe(struct platform_device *pdev)
+{
+	struct watchdog_device *wdt_dev;
+	struct resource *res;
+	int ret;
+	int irq;
+
+	wdt_dev = devm_kzalloc(&pdev->dev, sizeof(*wdt_dev), GFP_KERNEL);
+	if (!wdt_dev)
+		return -ENOMEM;
+
+	wdt_dev->info = &mid_wdt_info;
+	wdt_dev->ops = &mid_wdt_ops;
+	wdt_dev->min_timeout = MID_WDT_TIMEOUT_MIN;
+	wdt_dev->max_timeout = MID_WDT_TIMEOUT_MAX;
+	wdt_dev->timeout = MID_WDT_DEFAULT_TIMEOUT;
+
+	watchdog_set_drvdata(wdt_dev, &pdev->dev);
+	platform_set_drvdata(pdev, wdt_dev);
+
+	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "missing IRQ resource\n");
+		return -EINVAL;
+	}
+	irq = res->start;
+
+	handle_dev_ioapic(&pdev->dev, irq);
+	ret = devm_request_irq(&pdev->dev, irq, mid_wdt_warning_irq,
+			       IRQF_SHARED | IRQF_NO_SUSPEND, "watchdog",
+			       wdt_dev);
+	if (ret) {
+		dev_err(&pdev->dev, "error requesting warning irq %d\n", irq);
+		return ret;
+	}
+
+	ret = watchdog_register_device(wdt_dev);
+	if (ret) {
+		dev_err(&pdev->dev, "error registering watchdog device\n");
+		return ret;
+	}
+
+	dev_info(&pdev->dev, "Intel MID watchdog device probed\n");
+
+	return 0;
+}
+
+static int mid_wdt_remove(struct platform_device *pdev)
+{
+	struct watchdog_device *wd = platform_get_drvdata(pdev);
+	watchdog_unregister_device(wd);
+	return 0;
+}
+
+static struct platform_driver mid_wdt_driver = {
+	.probe		= mid_wdt_probe,
+	.remove		= mid_wdt_remove,
+	.driver		= {
+		.owner	= THIS_MODULE,
+		.name	= "intel_mid_wdt",
+	},
+};
+
+module_platform_driver(mid_wdt_driver);
+
+MODULE_AUTHOR("David Cohen <david.a.cohen@linux.intel.com>");
+MODULE_DESCRIPTION("Watchdog Driver for Intel MID platform");
+MODULE_LICENSE("GPL");
-- 
1.9.0

[PATCH 2/2] x86: intel-mid: add watchdog platform code for Merrifield

[David Cohen]

This patch adds platform code for Intel Merrifield.
Since the watchdog is not part of SFI table, we have no other option but
to manually register watchdog's platform device (argh!).

Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
---
 arch/x86/platform/intel-mid/device_libs/Makefile   |  1 +
 .../platform/intel-mid/device_libs/platform_wdt.c  | 43 ++++++++++++++++++++++
 2 files changed, 44 insertions(+)
 create mode 100644 arch/x86/platform/intel-mid/device_libs/platform_wdt.c

diff --git a/arch/x86/platform/intel-mid/device_libs/Makefile b/arch/x86/platform/intel-mid/device_libs/Makefile
index 097e7a7940d8..af9307f2cc28 100644
--- a/arch/x86/platform/intel-mid/device_libs/Makefile
+++ b/arch/x86/platform/intel-mid/device_libs/Makefile
@@ -20,3 +20,4 @@ obj-$(subst m,y,$(CONFIG_DRM_MEDFIELD)) += platform_tc35876x.o
 obj-$(subst m,y,$(CONFIG_SERIAL_MRST_MAX3110)) += platform_max3111.o
 # MISC Devices
 obj-$(subst m,y,$(CONFIG_KEYBOARD_GPIO)) += platform_gpio_keys.o
+obj-$(subst m,y,$(CONFIG_INTEL_MID_WATCHDOG)) += platform_wdt.o
diff --git a/arch/x86/platform/intel-mid/device_libs/platform_wdt.c b/arch/x86/platform/intel-mid/device_libs/platform_wdt.c
new file mode 100644
index 000000000000..be4fc66fbe1b
--- /dev/null
+++ b/arch/x86/platform/intel-mid/device_libs/platform_wdt.c
@@ -0,0 +1,43 @@
+/*
+ * platform_wdt.c: Watchdog platform library file
+ *
+ * (C) Copyright 2014 Intel Corporation
+ * Author: David Cohen <david.a.cohen@linux.intel.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; version 2
+ * of the License.
+ */
+
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <asm/intel-mid.h>
+
+#define EXT_TIMER0_MSI 15
+
+static const struct resource tangier_wdt_res[] = {
+	{
+		.start = EXT_TIMER0_MSI,
+		.flags = IORESOURCE_IRQ,
+	},
+};
+
+static struct platform_device mid_wdt_dev = {
+	.name = "intel_mid_wdt",
+	.id = -1,
+};
+
+static int __init register_mid_wdt(void)
+{
+	if (intel_mid_identify_cpu() == INTEL_MID_CPU_CHIP_TANGIER) {
+		platform_device_add_resources(&mid_wdt_dev,
+					      tangier_wdt_res,
+					      ARRAY_SIZE(tangier_wdt_res));
+		return platform_device_register(&mid_wdt_dev);
+	}
+
+	return -ENODEV;
+}
+
+rootfs_initcall(register_mid_wdt);
-- 
1.9.0

Re: [PATCH 1/2] watchdog: add Intel MID watchdog driver support

[Randy Dunlap]

On 04/08/2014 01:59 PM, David Cohen wrote:
> Add initial Intel MID watchdog driver support.
> 
> This driver is an initial implementation of generic Intel MID watchdog
> driver. Currently it supports Intel Merrifield platform.
> 
> Signed-off-by: Eric Ernst <eric.ernst@intel.com>
> Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
> ---
>  drivers/watchdog/Kconfig         |  12 +++
>  drivers/watchdog/Makefile        |   1 +
>  drivers/watchdog/intel-mid_wdt.c | 209 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 222 insertions(+)
>  create mode 100644 drivers/watchdog/intel-mid_wdt.c
> 
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 79d25894343a..4da09b8b2f11 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -643,6 +643,18 @@ config INTEL_SCU_WATCHDOG
>  
>  	  To compile this driver as a module, choose M here.
>  
> +config INTEL_MID_WATCHDOG
> +	bool "Intel MID SCU Watchdog Mobile Platforms"
> +	depends on X86_INTEL_MID && WATCHDOG_CORE
> +	---help---
> +	  Watchdog timer driver built into the Intel SCU for Intel MID
> +	  Platforms.
> +
> +	  This driver currently supports only the watchdog evolution
> +	  implementation in SCU, available for Merrifield generation.
> +
> +	  To compile this driver as a module, choose M here.

Does choosing M work when INTEL_MID_WATCHDOG is a bool?
and why is INTEL_MID_WATCHDOG a bool?

> +
>  config ITCO_WDT
>  	tristate "Intel TCO Timer/Watchdog"
>  	depends on (X86 || IA64) && PCI


-- 
~Randy

Re: [PATCH 1/2] watchdog: add Intel MID watchdog driver support

[Guenter Roeck]

On 04/08/2014 01:59 PM, David Cohen wrote:
> Add initial Intel MID watchdog driver support.
>
> This driver is an initial implementation of generic Intel MID watchdog
> driver. Currently it supports Intel Merrifield platform.

How does this compare with the existing SCU watchdog driver
(intel_scu_watchdog) ? The watchdog IPC message (0xf8) seems
to be the same, so there must be at least some overlap.
Would it be possible to have just one watchdog driver
serving the different SCU based devices ?

>
> Signed-off-by: Eric Ernst <eric.ernst@intel.com>
> Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
> ---
>   drivers/watchdog/Kconfig         |  12 +++
>   drivers/watchdog/Makefile        |   1 +
>   drivers/watchdog/intel-mid_wdt.c | 209 +++++++++++++++++++++++++++++++++++++++
>   3 files changed, 222 insertions(+)
>   create mode 100644 drivers/watchdog/intel-mid_wdt.c
>
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 79d25894343a..4da09b8b2f11 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -643,6 +643,18 @@ config INTEL_SCU_WATCHDOG
>
>   	  To compile this driver as a module, choose M here.
>
> +config INTEL_MID_WATCHDOG
> +	bool "Intel MID SCU Watchdog Mobile Platforms"
> +	depends on X86_INTEL_MID && WATCHDOG_CORE

All other watchdog drivers using the watchdog core use
	select WATCHDOG_CORE

so this one should do the same.

Guenter

Re: [PATCH 1/2] watchdog: add Intel MID watchdog driver support

[One Thousand Gnomes]

On Tue, 08 Apr 2014 17:17:34 -0700
Guenter Roeck <linux@roeck-us.net> wrote:

> On 04/08/2014 01:59 PM, David Cohen wrote:
> > Add initial Intel MID watchdog driver support.
> >
> > This driver is an initial implementation of generic Intel MID watchdog
> > driver. Currently it supports Intel Merrifield platform.
> 
> How does this compare with the existing SCU watchdog driver
> (intel_scu_watchdog) ? The watchdog IPC message (0xf8) seems
> to be the same, so there must be at least some overlap.
> Would it be possible to have just one watchdog driver
> serving the different SCU based devices ?

There are enough differences that we'd end up with one spaghetti driver
full of conditionals rather than two simple clean drivers.

Alan

Re: [PATCH 2/2] x86: intel-mid: add watchdog platform code for Merrifield

[One Thousand Gnomes]

On Tue,  8 Apr 2014 13:59:04 -0700
David Cohen <david.a.cohen@linux.intel.com> wrote:

> This patch adds platform code for Intel Merrifield.
> Since the watchdog is not part of SFI table, we have no other option but
> to manually register watchdog's platform device (argh!).

Before going too far down that path have you considered compiling in a
devicetree instead ?

Alan

Re: [PATCH 2/2] x86: intel-mid: add watchdog platform code for Merrifield

[Alexander Stein]

On Wednesday 09 April 2014 13:42:23, One Thousand Gnomes wrote:
> On Tue,  8 Apr 2014 13:59:04 -0700
> David Cohen <david.a.cohen@linux.intel.com> wrote:
> 
> > This patch adds platform code for Intel Merrifield.
> > Since the watchdog is not part of SFI table, we have no other option but
> > to manually register watchdog's platform device (argh!).
> 
> Before going too far down that path have you considered compiling in a
> devicetree instead ?

I'm getting curious: How can I use device-tree on x86(_64)?
Reading the dependencies from CONFIG_OF it can only be used on 32bit systems with some special hardware bases.
So, how to use otherwise?

Best regards,
Alexander
-- 
Dipl.-Inf. Alexander Stein

SYS TEC electronic GmbH
Am Windrad 2
08468 Heinsdorfergrund
Tel.: 03765 38600-1156
Fax: 03765 38600-4100
Email: alexander.stein@systec-electronic.com
Website: www.systec-electronic.com
 
Managing Director: Dipl.-Phys. Siegmar Schmidt
Commercial registry: Amtsgericht Chemnitz, HRB 28082

Re: [PATCH 2/2] x86: intel-mid: add watchdog platform code for Merrifield

[One Thousand Gnomes]

> I'm getting curious: How can I use device-tree on x86(_64)?
> Reading the dependencies from CONFIG_OF it can only be used on 32bit systems with some special hardware bases.
> So, how to use otherwise?

There isn't any fundamental thing tying device tree to a given
architecture or 32bitness, it's just that sane PC architectures use ACPI
to enumerate devices, and/or have a discoverable bus architecture.

Some of the phones don't so this now becoems a point of consideration. In
fact it's already also used on CE4100 (which is an embedded media SoC
found in some TV devices and set-top boxes) and on the OLPC (One laptop
per child). There is no intrinsic reason it couldn't be used in other x86
special cases.

If its PC shaped however it probably has ACPI and ACPI and DT are not a
1:1 mapping. ACPI has method invocations, and various firmware provided
interfaces such as the EC, Device tree is better at some other bits.

Converting the phones to embedded device tree rather than adding a
billion little platform files on the other hand seems to me like a
no-brainer.

Alan

Re: [PATCH 2/2] x86: intel-mid: add watchdog platform code for Merrifield

[Alexander Stein]

On Wednesday 09 April 2014 14:58:48, One Thousand Gnomes wrote:
> > I'm getting curious: How can I use device-tree on x86(_64)?
> > Reading the dependencies from CONFIG_OF it can only be used on 32bit systems with some special hardware bases.
> > So, how to use otherwise?
> 
> There isn't any fundamental thing tying device tree to a given
> architecture or 32bitness, it's just that sane PC architectures use ACPI
> to enumerate devices, and/or have a discoverable bus architecture.
> 
> Some of the phones don't so this now becoems a point of consideration. In
> fact it's already also used on CE4100 (which is an embedded media SoC
> found in some TV devices and set-top boxes) and on the OLPC (One laptop
> per child). There is no intrinsic reason it couldn't be used in other x86
> special cases.
> 
> If its PC shaped however it probably has ACPI and ACPI and DT are not a
> 1:1 mapping. ACPI has method invocations, and various firmware provided
> interfaces such as the EC, Device tree is better at some other bits.

Yep, like SPI devices on embedded x86 hardware.

> Converting the phones to embedded device tree rather than adding a
> billion little platform files on the other hand seems to me like a
> no-brainer.

I found http://thread.gmane.org/gmane.linux.drivers.devicetree/4475. Why didn't that get into mainline?

Best regards,
Alexander
-- 
Dipl.-Inf. Alexander Stein

SYS TEC electronic GmbH
Am Windrad 2
08468 Heinsdorfergrund
Tel.: 03765 38600-1156
Fax: 03765 38600-4100
Email: alexander.stein@systec-electronic.com
Website: www.systec-electronic.com
 
Managing Director: Dipl.-Phys. Siegmar Schmidt
Commercial registry: Amtsgericht Chemnitz, HRB 28082

Re: [PATCH 2/2] x86: intel-mid: add watchdog platform code for Merrifield

[Guenter Roeck]

On Wed, Apr 09, 2014 at 04:03:14PM +0200, Alexander Stein wrote:
> On Wednesday 09 April 2014 14:58:48, One Thousand Gnomes wrote:
> > > I'm getting curious: How can I use device-tree on x86(_64)?
> > > Reading the dependencies from CONFIG_OF it can only be used on 32bit systems with some special hardware bases.
> > > So, how to use otherwise?
> > 
> > There isn't any fundamental thing tying device tree to a given
> > architecture or 32bitness, it's just that sane PC architectures use ACPI
> > to enumerate devices, and/or have a discoverable bus architecture.
> > 
> > Some of the phones don't so this now becoems a point of consideration. In
> > fact it's already also used on CE4100 (which is an embedded media SoC
> > found in some TV devices and set-top boxes) and on the OLPC (One laptop
> > per child). There is no intrinsic reason it couldn't be used in other x86
> > special cases.
> > 
> > If its PC shaped however it probably has ACPI and ACPI and DT are not a
> > 1:1 mapping. ACPI has method invocations, and various firmware provided
> > interfaces such as the EC, Device tree is better at some other bits.
> 
> Yep, like SPI devices on embedded x86 hardware.
> 
> > Converting the phones to embedded device tree rather than adding a
> > billion little platform files on the other hand seems to me like a
> > no-brainer.
> 
> I found http://thread.gmane.org/gmane.linux.drivers.devicetree/4475. Why didn't that get into mainline?
> 

Isn't that patch set in mainline ?

We are working on a patch set to make DT support on x86 more
widely available. Thierry Reding did some work on it a while ago.
Some of it is upstream, other parts are available in 
	https://github.com/avionic-design/linux/commits/medatom/master

We'll see if and what will be accepted upstream.

Guenter

Re: [PATCH 2/2] x86: intel-mid: add watchdog platform code for Merrifield

[Alexander Stein]

On Wednesday 09 April 2014 08:18:22, Guenter Roeck wrote:
> On Wed, Apr 09, 2014 at 04:03:14PM +0200, Alexander Stein wrote:
> > On Wednesday 09 April 2014 14:58:48, One Thousand Gnomes wrote:
> > > > I'm getting curious: How can I use device-tree on x86(_64)?
> > > > Reading the dependencies from CONFIG_OF it can only be used on 32bit systems with some special hardware bases.
> > > > So, how to use otherwise?
> > > 
> > > There isn't any fundamental thing tying device tree to a given
> > > architecture or 32bitness, it's just that sane PC architectures use ACPI
> > > to enumerate devices, and/or have a discoverable bus architecture.
> > > 
> > > Some of the phones don't so this now becoems a point of consideration. In
> > > fact it's already also used on CE4100 (which is an embedded media SoC
> > > found in some TV devices and set-top boxes) and on the OLPC (One laptop
> > > per child). There is no intrinsic reason it couldn't be used in other x86
> > > special cases.
> > > 
> > > If its PC shaped however it probably has ACPI and ACPI and DT are not a
> > > 1:1 mapping. ACPI has method invocations, and various firmware provided
> > > interfaces such as the EC, Device tree is better at some other bits.
> > 
> > Yep, like SPI devices on embedded x86 hardware.
> > 
> > > Converting the phones to embedded device tree rather than adding a
> > > billion little platform files on the other hand seems to me like a
> > > no-brainer.
> > 
> > I found http://thread.gmane.org/gmane.linux.drivers.devicetree/4475. Why didn't that get into mainline?
> > 
> 
> Isn't that patch set in mainline ?

Oh, I see that this patch didn't get included 1:1 as shown in that link. I only checked USE_OF in arch/x86/Kconfig which didn't get included that way :(

> We are working on a patch set to make DT support on x86 more
> widely available. Thierry Reding did some work on it a while ago.
> Some of it is upstream, other parts are available in 
> 	https://github.com/avionic-design/linux/commits/medatom/master
> 
> We'll see if and what will be accepted upstream.

As far as I can see, CONFIG_OF is only selected if a specific hardware is enabled.

Regards,
Alexander
-- 
Dipl.-Inf. Alexander Stein

SYS TEC electronic GmbH
Am Windrad 2
08468 Heinsdorfergrund
Tel.: 03765 38600-1156
Fax: 03765 38600-4100
Email: alexander.stein@systec-electronic.com
Website: www.systec-electronic.com
 
Managing Director: Dipl.-Phys. Siegmar Schmidt
Commercial registry: Amtsgericht Chemnitz, HRB 28082

Re: [PATCH 2/2] x86: intel-mid: add watchdog platform code for Merrifield

[Guenter Roeck]

On Wed, Apr 09, 2014 at 06:10:11PM +0200, Alexander Stein wrote:
> On Wednesday 09 April 2014 08:18:22, Guenter Roeck wrote:
> > On Wed, Apr 09, 2014 at 04:03:14PM +0200, Alexander Stein wrote:
> > > On Wednesday 09 April 2014 14:58:48, One Thousand Gnomes wrote:
> > > > > I'm getting curious: How can I use device-tree on x86(_64)?
> > > > > Reading the dependencies from CONFIG_OF it can only be used on 32bit systems with some special hardware bases.
> > > > > So, how to use otherwise?
> > > > 
> > > > There isn't any fundamental thing tying device tree to a given
> > > > architecture or 32bitness, it's just that sane PC architectures use ACPI
> > > > to enumerate devices, and/or have a discoverable bus architecture.
> > > > 
> > > > Some of the phones don't so this now becoems a point of consideration. In
> > > > fact it's already also used on CE4100 (which is an embedded media SoC
> > > > found in some TV devices and set-top boxes) and on the OLPC (One laptop
> > > > per child). There is no intrinsic reason it couldn't be used in other x86
> > > > special cases.
> > > > 
> > > > If its PC shaped however it probably has ACPI and ACPI and DT are not a
> > > > 1:1 mapping. ACPI has method invocations, and various firmware provided
> > > > interfaces such as the EC, Device tree is better at some other bits.
> > > 
> > > Yep, like SPI devices on embedded x86 hardware.
> > > 
> > > > Converting the phones to embedded device tree rather than adding a
> > > > billion little platform files on the other hand seems to me like a
> > > > no-brainer.
> > > 
> > > I found http://thread.gmane.org/gmane.linux.drivers.devicetree/4475. Why didn't that get into mainline?
> > > 
> > 
> > Isn't that patch set in mainline ?
> 
> Oh, I see that this patch didn't get included 1:1 as shown in that link. I only checked USE_OF in arch/x86/Kconfig which didn't get included that way :(
> 
> > We are working on a patch set to make DT support on x86 more
> > widely available. Thierry Reding did some work on it a while ago.
> > Some of it is upstream, other parts are available in 
> > 	https://github.com/avionic-design/linux/commits/medatom/master
> > 
> > We'll see if and what will be accepted upstream.
> 
> As far as I can see, CONFIG_OF is only selected if a specific hardware is enabled.
> 
Correct, and I expect that won't change. Default on x86 platform is ACPI.

Guenter

Re: [PATCH 1/2] watchdog: add Intel MID watchdog driver support

[David Cohen]

On Tue, Apr 08, 2014 at 04:56:36PM -0700, Randy Dunlap wrote:
> On 04/08/2014 01:59 PM, David Cohen wrote:
> > Add initial Intel MID watchdog driver support.
> > 
> > This driver is an initial implementation of generic Intel MID watchdog
> > driver. Currently it supports Intel Merrifield platform.
> > 
> > Signed-off-by: Eric Ernst <eric.ernst@intel.com>
> > Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
> > ---
> >  drivers/watchdog/Kconfig         |  12 +++
> >  drivers/watchdog/Makefile        |   1 +
> >  drivers/watchdog/intel-mid_wdt.c | 209 +++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 222 insertions(+)
> >  create mode 100644 drivers/watchdog/intel-mid_wdt.c
> > 
> > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> > index 79d25894343a..4da09b8b2f11 100644
> > --- a/drivers/watchdog/Kconfig
> > +++ b/drivers/watchdog/Kconfig
> > @@ -643,6 +643,18 @@ config INTEL_SCU_WATCHDOG
> >  
> >  	  To compile this driver as a module, choose M here.
> >  
> > +config INTEL_MID_WATCHDOG
> > +	bool "Intel MID SCU Watchdog Mobile Platforms"
> > +	depends on X86_INTEL_MID && WATCHDOG_CORE
> > +	---help---
> > +	  Watchdog timer driver built into the Intel SCU for Intel MID
> > +	  Platforms.
> > +
> > +	  This driver currently supports only the watchdog evolution
> > +	  implementation in SCU, available for Merrifield generation.
> > +
> > +	  To compile this driver as a module, choose M here.
> 
> Does choosing M work when INTEL_MID_WATCHDOG is a bool?
> and why is INTEL_MID_WATCHDOG a bool?

The error here is the left over line from the template I used. There
should be no mention about choosing M.
It's bool because the watchdog is started by default on fw. We need this
driver probed during boot to pet/kick/ping asap. IMHO It makes not much
sense to compile it as module.

Br, David

> 
> > +
> >  config ITCO_WDT
> >  	tristate "Intel TCO Timer/Watchdog"
> >  	depends on (X86 || IA64) && PCI
> 
> 
> -- 
> ~Randy

Re: [PATCH 2/2] x86: intel-mid: add watchdog platform code for Merrifield

[David Cohen]

On Wed, Apr 09, 2014 at 01:42:23PM +0100, One Thousand Gnomes wrote:
> On Tue,  8 Apr 2014 13:59:04 -0700
> David Cohen <david.a.cohen@linux.intel.com> wrote:
> 
> > This patch adds platform code for Intel Merrifield.
> > Since the watchdog is not part of SFI table, we have no other option but
> > to manually register watchdog's platform device (argh!).
> 
> Before going too far down that path have you considered compiling in a
> devicetree instead ?

This is the only device missing from SFI table I am aware of.
The original implementation is to register the watchdog without a
device/driver match. i.e. directly from the init call. Then the init
call has a conditional check if it's Merrifield with
intel_mid_identify_cpu() to avoid it being registered in !Merrifield
platforms. But I want to lock this intel_mid_identify_cpu() inside
intel_mid platform code, since it's too ugly to stay inside drivers.

If we figure out there's more devices missing from SFI table, then we
could revisit here and consider another approach.

Br, David

> 
> Alan

Re: [PATCH 2/2] x86: intel-mid: add watchdog platform code for Merrifield

[One Thousand Gnomes]

> > We'll see if and what will be accepted upstream.
> 
> As far as I can see, CONFIG_OF is only selected if a specific hardware is enabled.

For conventional PC it makes no sense. For non conventional devices -
well Kconfig is also convered by the GPL ;-)

Alan

Re: [PATCH 1/2] watchdog: add Intel MID watchdog driver support

[Guenter Roeck]

On 04/09/2014 10:48 AM, David Cohen wrote:
> On Tue, Apr 08, 2014 at 04:56:36PM -0700, Randy Dunlap wrote:
>> On 04/08/2014 01:59 PM, David Cohen wrote:
>>> Add initial Intel MID watchdog driver support.
>>>
>>> This driver is an initial implementation of generic Intel MID watchdog
>>> driver. Currently it supports Intel Merrifield platform.
>>>
>>> Signed-off-by: Eric Ernst <eric.ernst@intel.com>
>>> Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
>>> ---
>>>   drivers/watchdog/Kconfig         |  12 +++
>>>   drivers/watchdog/Makefile        |   1 +
>>>   drivers/watchdog/intel-mid_wdt.c | 209 +++++++++++++++++++++++++++++++++++++++
>>>   3 files changed, 222 insertions(+)
>>>   create mode 100644 drivers/watchdog/intel-mid_wdt.c
>>>
>>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>>> index 79d25894343a..4da09b8b2f11 100644
>>> --- a/drivers/watchdog/Kconfig
>>> +++ b/drivers/watchdog/Kconfig
>>> @@ -643,6 +643,18 @@ config INTEL_SCU_WATCHDOG
>>>
>>>   	  To compile this driver as a module, choose M here.
>>>
>>> +config INTEL_MID_WATCHDOG
>>> +	bool "Intel MID SCU Watchdog Mobile Platforms"
>>> +	depends on X86_INTEL_MID && WATCHDOG_CORE
>>> +	---help---
>>> +	  Watchdog timer driver built into the Intel SCU for Intel MID
>>> +	  Platforms.
>>> +
>>> +	  This driver currently supports only the watchdog evolution
>>> +	  implementation in SCU, available for Merrifield generation.
>>> +
>>> +	  To compile this driver as a module, choose M here.
>>
>> Does choosing M work when INTEL_MID_WATCHDOG is a bool?
>> and why is INTEL_MID_WATCHDOG a bool?
>
> The error here is the left over line from the template I used. There
> should be no mention about choosing M.
> It's bool because the watchdog is started by default on fw. We need this
> driver probed during boot to pet/kick/ping asap. IMHO It makes not much
> sense to compile it as module.
>

Is that true for your firmware, but for all users of the supported set
of devices ?

The reason for having modules is to be able to load the driver only
when needed, while using the same distribution for multiple platforms.
So either you are forcing the driver to be loaded on every hardware
which has it enabled, or you are limiting the scope of a distribution
to one specific CPU. Neither seems desirable.

If _you_ want to build the driver into the kernel, you always can.
But you have to have a better reason than that to enforce it on
everyone else.

Guenter

Re: [PATCH 1/2] watchdog: add Intel MID watchdog driver support

[David Cohen]

On Thu, Apr 10, 2014 at 06:51:03AM -0700, Guenter Roeck wrote:
> On 04/09/2014 10:48 AM, David Cohen wrote:
> >On Tue, Apr 08, 2014 at 04:56:36PM -0700, Randy Dunlap wrote:
> >>On 04/08/2014 01:59 PM, David Cohen wrote:
> >>>Add initial Intel MID watchdog driver support.
> >>>
> >>>This driver is an initial implementation of generic Intel MID watchdog
> >>>driver. Currently it supports Intel Merrifield platform.
> >>>
> >>>Signed-off-by: Eric Ernst <eric.ernst@intel.com>
> >>>Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
> >>>---
> >>>  drivers/watchdog/Kconfig         |  12 +++
> >>>  drivers/watchdog/Makefile        |   1 +
> >>>  drivers/watchdog/intel-mid_wdt.c | 209 +++++++++++++++++++++++++++++++++++++++
> >>>  3 files changed, 222 insertions(+)
> >>>  create mode 100644 drivers/watchdog/intel-mid_wdt.c
> >>>
> >>>diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> >>>index 79d25894343a..4da09b8b2f11 100644
> >>>--- a/drivers/watchdog/Kconfig
> >>>+++ b/drivers/watchdog/Kconfig
> >>>@@ -643,6 +643,18 @@ config INTEL_SCU_WATCHDOG
> >>>
> >>>  	  To compile this driver as a module, choose M here.
> >>>
> >>>+config INTEL_MID_WATCHDOG
> >>>+	bool "Intel MID SCU Watchdog Mobile Platforms"
> >>>+	depends on X86_INTEL_MID && WATCHDOG_CORE
> >>>+	---help---
> >>>+	  Watchdog timer driver built into the Intel SCU for Intel MID
> >>>+	  Platforms.
> >>>+
> >>>+	  This driver currently supports only the watchdog evolution
> >>>+	  implementation in SCU, available for Merrifield generation.
> >>>+
> >>>+	  To compile this driver as a module, choose M here.
> >>
> >>Does choosing M work when INTEL_MID_WATCHDOG is a bool?
> >>and why is INTEL_MID_WATCHDOG a bool?
> >
> >The error here is the left over line from the template I used. There
> >should be no mention about choosing M.
> >It's bool because the watchdog is started by default on fw. We need this
> >driver probed during boot to pet/kick/ping asap. IMHO It makes not much
> >sense to compile it as module.
> >
> 
> Is that true for your firmware, but for all users of the supported set
> of devices ?

I don't think there is another fw to support same device. But...

> 
> The reason for having modules is to be able to load the driver only
> when needed, while using the same distribution for multiple platforms.
> So either you are forcing the driver to be loaded on every hardware
> which has it enabled, or you are limiting the scope of a distribution
> to one specific CPU. Neither seems desirable.
> 
> If _you_ want to build the driver into the kernel, you always can.
> But you have to have a better reason than that to enforce it on
> everyone else.

... when we're stuck so much time working with Android we forget there
is life out there too. It really makes no sense "bool".
I'll fix it in next version.

Br, David

> 
> Guenter
> 

Re: [PATCH 2/2] x86: intel-mid: add watchdog platform code for Merrifield

[Guenter Roeck]

On Tue, Apr 08, 2014 at 01:59:04PM -0700, David Cohen wrote:
> This patch adds platform code for Intel Merrifield.
> Since the watchdog is not part of SFI table, we have no other option but
> to manually register watchdog's platform device (argh!).
> 
> Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
> ---

Does it really make sense to have this as separate patch ? 

It is quite common for watchdog (and many other) drivers to
register the driver and instantiate the device. I think it
would be better and more consistent to have both patches
merged into one.

Thanks,
Guenter

Re: [PATCH 2/2] x86: intel-mid: add watchdog platform code for Merrifield

[David Cohen]

On Thu, Apr 10, 2014 at 12:15:23PM -0700, Guenter Roeck wrote:
> On Tue, Apr 08, 2014 at 01:59:04PM -0700, David Cohen wrote:
> > This patch adds platform code for Intel Merrifield.
> > Since the watchdog is not part of SFI table, we have no other option but
> > to manually register watchdog's platform device (argh!).
> > 
> > Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
> > ---
> 
> Does it really make sense to have this as separate patch ? 
> 
> It is quite common for watchdog (and many other) drivers to
> register the driver and instantiate the device. I think it
> would be better and more consistent to have both patches
> merged into one.

Are you talking about to merge them without code changes or make the
driver responsible for the device enumeration (by make the driver to
allocate the device)?

If it's a simple merge, I'd say I don't like to mix drivers and arch
patches.

If we're talking about moving the device registration to driver, I
strongly disagree it would be better and more consistent. The way I sent
the driver makes it less dependent of how the enumeration happens.
If this device is added to SFI table, the driver would need no change.

Br, David

> 
> Thanks,
> Guenter

Re: [PATCH 2/2] x86: intel-mid: add watchdog platform code for Merrifield

[Guenter Roeck]

On Thu, Apr 10, 2014 at 12:30:10PM -0700, David Cohen wrote:
> On Thu, Apr 10, 2014 at 12:15:23PM -0700, Guenter Roeck wrote:
> > On Tue, Apr 08, 2014 at 01:59:04PM -0700, David Cohen wrote:
> > > This patch adds platform code for Intel Merrifield.
> > > Since the watchdog is not part of SFI table, we have no other option but
> > > to manually register watchdog's platform device (argh!).
> > > 
> > > Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
> > > ---
> > 
> > Does it really make sense to have this as separate patch ? 
> > 
> > It is quite common for watchdog (and many other) drivers to
> > register the driver and instantiate the device. I think it
> > would be better and more consistent to have both patches
> > merged into one.
> 
> Are you talking about to merge them without code changes or make the
> driver responsible for the device enumeration (by make the driver to
> allocate the device)?
> 
> If it's a simple merge, I'd say I don't like to mix drivers and arch
> patches.
> 
> If we're talking about moving the device registration to driver, I
> strongly disagree it would be better and more consistent. The way I sent
> the driver makes it less dependent of how the enumeration happens.
> If this device is added to SFI table, the driver would need no change.
> 
I don't see why that would be a problem. Guess we'll have to agree
to disagree.

Guenter

Re: [PATCH 2/2] x86: intel-mid: add watchdog platform code for Merrifield

[David Cohen]

On Thu, Apr 10, 2014 at 01:35:36PM -0700, Guenter Roeck wrote:
> On Thu, Apr 10, 2014 at 12:30:10PM -0700, David Cohen wrote:
> > On Thu, Apr 10, 2014 at 12:15:23PM -0700, Guenter Roeck wrote:
> > > On Tue, Apr 08, 2014 at 01:59:04PM -0700, David Cohen wrote:
> > > > This patch adds platform code for Intel Merrifield.
> > > > Since the watchdog is not part of SFI table, we have no other option but
> > > > to manually register watchdog's platform device (argh!).
> > > > 
> > > > Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
> > > > ---
> > > 
> > > Does it really make sense to have this as separate patch ? 
> > > 
> > > It is quite common for watchdog (and many other) drivers to
> > > register the driver and instantiate the device. I think it
> > > would be better and more consistent to have both patches
> > > merged into one.
> > 
> > Are you talking about to merge them without code changes or make the
> > driver responsible for the device enumeration (by make the driver to
> > allocate the device)?
> > 
> > If it's a simple merge, I'd say I don't like to mix drivers and arch
> > patches.
> > 
> > If we're talking about moving the device registration to driver, I
> > strongly disagree it would be better and more consistent. The way I sent
> > the driver makes it less dependent of how the enumeration happens.
> > If this device is added to SFI table, the driver would need no change.
> > 
> I don't see why that would be a problem. Guess we'll have to agree
> to disagree.

Sounds fine :)
If you're not too much against keeping the way it is, I'd like to send
the v2 with 2 patches again.

Br, David

> 
> Guenter

Re: [PATCH 2/2] x86: intel-mid: add watchdog platform code for Merrifield

[Guenter Roeck]

On 04/10/2014 02:23 PM, David Cohen wrote:
> On Thu, Apr 10, 2014 at 01:35:36PM -0700, Guenter Roeck wrote:
>> On Thu, Apr 10, 2014 at 12:30:10PM -0700, David Cohen wrote:
>>> On Thu, Apr 10, 2014 at 12:15:23PM -0700, Guenter Roeck wrote:
>>>> On Tue, Apr 08, 2014 at 01:59:04PM -0700, David Cohen wrote:
>>>>> This patch adds platform code for Intel Merrifield.
>>>>> Since the watchdog is not part of SFI table, we have no other option but
>>>>> to manually register watchdog's platform device (argh!).
>>>>>
>>>>> Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
>>>>> ---
>>>>
>>>> Does it really make sense to have this as separate patch ?
>>>>
>>>> It is quite common for watchdog (and many other) drivers to
>>>> register the driver and instantiate the device. I think it
>>>> would be better and more consistent to have both patches
>>>> merged into one.
>>>
>>> Are you talking about to merge them without code changes or make the
>>> driver responsible for the device enumeration (by make the driver to
>>> allocate the device)?
>>>
>>> If it's a simple merge, I'd say I don't like to mix drivers and arch
>>> patches.
>>>
>>> If we're talking about moving the device registration to driver, I
>>> strongly disagree it would be better and more consistent. The way I sent
>>> the driver makes it less dependent of how the enumeration happens.
>>> If this device is added to SFI table, the driver would need no change.
>>>
>> I don't see why that would be a problem. Guess we'll have to agree
>> to disagree.
>
> Sounds fine :)
> If you're not too much against keeping the way it is, I'd like to send
> the v2 with 2 patches again.
>

Not my decision to make. I am not the watchdog maintainer.

Guenter

[PATCH v2 0/2] Initial implementation of Intel MID watchdog driver

[David Cohen]

Hi,

This patch set is an initial implementation of a generic Intel MID watchdog
driver.
It currently supports Intel Merrifield only, but will extend to other SoCs of
MID family.

changes from v1 to v2:
 - Fixed Kconfig entry as per Guenter Roeck suggestion

Br, David

--
David Cohen (2):
  watchdog: add Intel MID watchdog driver support
  x86: intel-mid: add watchdog platform code for Merrifield

 arch/x86/platform/intel-mid/device_libs/Makefile   |   1 +
 .../platform/intel-mid/device_libs/platform_wdt.c  |  43 +++++
 drivers/watchdog/Kconfig                           |  13 ++
 drivers/watchdog/Makefile                          |   1 +
 drivers/watchdog/intel-mid_wdt.c                   | 209 +++++++++++++++++++++
 5 files changed, 267 insertions(+)
 create mode 100644 arch/x86/platform/intel-mid/device_libs/platform_wdt.c
 create mode 100644 drivers/watchdog/intel-mid_wdt.c

-- 
1.9.1

[PATCH v2 1/2] watchdog: add Intel MID watchdog driver support

[David Cohen]

Add initial Intel MID watchdog driver support.

This driver is an initial implementation of generic Intel MID watchdog
driver. Currently it supports Intel Merrifield platform.

Signed-off-by: Eric Ernst <eric.ernst@intel.com>
Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
---
 drivers/watchdog/Kconfig         |  13 +++
 drivers/watchdog/Makefile        |   1 +
 drivers/watchdog/intel-mid_wdt.c | 209 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 223 insertions(+)
 create mode 100644 drivers/watchdog/intel-mid_wdt.c

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 79d25894343a..0e110c37aa5c 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -643,6 +643,19 @@ config INTEL_SCU_WATCHDOG
 
 	  To compile this driver as a module, choose M here.
 
+config INTEL_MID_WATCHDOG
+	tristate "Intel MID Watchdog Timer"
+	depends on X86_INTEL_MID
+	select WATCHDOG_CORE
+	---help---
+	  Watchdog timer driver built into the Intel SCU for Intel MID
+	  Platforms.
+
+	  This driver currently supports only the watchdog evolution
+	  implementation in SCU, available for Merrifield generation.
+
+	  To compile this driver as a module, choose M here.
+
 config ITCO_WDT
 	tristate "Intel TCO Timer/Watchdog"
 	depends on (X86 || IA64) && PCI
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 985a66cda76f..b927e7496eb4 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -113,6 +113,7 @@ obj-$(CONFIG_W83977F_WDT) += w83977f_wdt.o
 obj-$(CONFIG_MACHZ_WDT) += machzwd.o
 obj-$(CONFIG_SBC_EPX_C3_WATCHDOG) += sbc_epx_c3.o
 obj-$(CONFIG_INTEL_SCU_WATCHDOG) += intel_scu_watchdog.o
+obj-$(CONFIG_INTEL_MID_WATCHDOG) += intel-mid_wdt.o
 
 # M32R Architecture
 
diff --git a/drivers/watchdog/intel-mid_wdt.c b/drivers/watchdog/intel-mid_wdt.c
new file mode 100644
index 000000000000..6339c6351454
--- /dev/null
+++ b/drivers/watchdog/intel-mid_wdt.c
@@ -0,0 +1,209 @@
+/*
+ *      intel-mid_wdt: generic Intel MID SCU watchdog driver
+ *
+ *      Platforms supported so far:
+ *      - Merrifield only
+ *
+ *      Copyright (C) 2014 Intel Corporation. All rights reserved.
+ *      Contact: David Cohen <david.a.cohen@linux.intel.com>
+ *
+ *      This program is free software; you can redistribute it and/or
+ *      modify it under the terms of version 2 of the GNU General
+ *      Public License as published by the Free Software Foundation.
+ */
+
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/nmi.h>
+#include <linux/platform_device.h>
+#include <linux/watchdog.h>
+
+#include <asm/intel_scu_ipc.h>
+#include <asm/intel-mid.h>
+
+#define IPC_WATCHDOG 0xf8
+
+#define MID_WDT_PRETIMEOUT		15
+#define MID_WDT_TIMEOUT_MIN		(1 + MID_WDT_PRETIMEOUT)
+#define MID_WDT_TIMEOUT_MAX		170
+#define MID_WDT_DEFAULT_TIMEOUT		90
+
+/* SCU watchdog messages */
+enum {
+	SCU_WATCHDOG_START = 0,
+	SCU_WATCHDOG_STOP,
+	SCU_WATCHDOG_KEEPALIVE,
+};
+
+static inline int wdt_command(int sub, u32 *in, int inlen)
+{
+	return intel_scu_ipc_command(IPC_WATCHDOG, sub, in, inlen, NULL, 0);
+}
+
+static int wdt_start(struct watchdog_device *wd)
+{
+	int ret, in_size;
+	int timeout = wd->timeout;
+	struct ipc_wd_start {
+		u32 pretimeout;
+		u32 timeout;
+	} ipc_wd_start = { timeout - MID_WDT_PRETIMEOUT, timeout };
+
+	/*
+	 * SCU expects the input size for watchdog IPC to
+	 * be based on double-word
+	 */
+	in_size = (sizeof(ipc_wd_start) + 3) / 4;
+
+	ret = wdt_command(SCU_WATCHDOG_START, (u32 *)&ipc_wd_start, in_size);
+	if (ret) {
+		struct device *dev = watchdog_get_drvdata(wd);
+		dev_crit(dev, "error starting watchdog: %d\n", ret);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int wdt_ping(struct watchdog_device *wd)
+{
+	int ret;
+
+	ret = wdt_command(SCU_WATCHDOG_KEEPALIVE, NULL, 0);
+	if (ret) {
+		struct device *dev = watchdog_get_drvdata(wd);
+		dev_crit(dev, "Error executing keepalive: 0x%x\n", ret);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int wdt_stop(struct watchdog_device *wd)
+{
+	int ret;
+
+	ret = wdt_command(SCU_WATCHDOG_STOP, NULL, 0);
+	if (ret) {
+		struct device *dev = watchdog_get_drvdata(wd);
+		dev_crit(dev, "Error stopping watchdog: 0x%x\n", ret);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static irqreturn_t mid_wdt_warning_irq(int irq, void *dev_id)
+{
+	/* Let's dump some data before panic */
+	trigger_all_cpu_backtrace();
+	panic("Kernel Watchdog");
+
+	/* This code should not be reached */
+	return IRQ_HANDLED;
+}
+
+/*
+ * Current driver supports Merrifield only.
+ * This function will get more generic when necessary.
+ */
+static int handle_dev_ioapic(struct device *dev, int irq)
+{
+	int ioapic;
+	struct io_apic_irq_attr irq_attr;
+
+	ioapic = mp_find_ioapic(irq);
+	if (ioapic >= 0) {
+		irq_attr.ioapic = ioapic;
+		irq_attr.ioapic_pin = irq;
+		irq_attr.trigger = 1;
+		irq_attr.polarity = 0; /* Active high */
+		io_apic_set_pci_routing(NULL, irq, &irq_attr);
+	} else {
+		dev_warn(dev, "can not find interrupt %d in ioapic\n", irq);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static const struct watchdog_info mid_wdt_info = {
+	.identity = "Intel MID SCU watchdog",
+	.options = WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT,
+};
+
+static const struct watchdog_ops mid_wdt_ops = {
+	.owner = THIS_MODULE,
+	.start = wdt_start,
+	.stop = wdt_stop,
+	.ping = wdt_ping,
+};
+
+static int mid_wdt_probe(struct platform_device *pdev)
+{
+	struct watchdog_device *wdt_dev;
+	struct resource *res;
+	int ret;
+	int irq;
+
+	wdt_dev = devm_kzalloc(&pdev->dev, sizeof(*wdt_dev), GFP_KERNEL);
+	if (!wdt_dev)
+		return -ENOMEM;
+
+	wdt_dev->info = &mid_wdt_info;
+	wdt_dev->ops = &mid_wdt_ops;
+	wdt_dev->min_timeout = MID_WDT_TIMEOUT_MIN;
+	wdt_dev->max_timeout = MID_WDT_TIMEOUT_MAX;
+	wdt_dev->timeout = MID_WDT_DEFAULT_TIMEOUT;
+
+	watchdog_set_drvdata(wdt_dev, &pdev->dev);
+	platform_set_drvdata(pdev, wdt_dev);
+
+	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "missing IRQ resource\n");
+		return -EINVAL;
+	}
+	irq = res->start;
+
+	handle_dev_ioapic(&pdev->dev, irq);
+	ret = devm_request_irq(&pdev->dev, irq, mid_wdt_warning_irq,
+			       IRQF_SHARED | IRQF_NO_SUSPEND, "watchdog",
+			       wdt_dev);
+	if (ret) {
+		dev_err(&pdev->dev, "error requesting warning irq %d\n", irq);
+		return ret;
+	}
+
+	ret = watchdog_register_device(wdt_dev);
+	if (ret) {
+		dev_err(&pdev->dev, "error registering watchdog device\n");
+		return ret;
+	}
+
+	dev_info(&pdev->dev, "Intel MID watchdog device probed\n");
+
+	return 0;
+}
+
+static int mid_wdt_remove(struct platform_device *pdev)
+{
+	struct watchdog_device *wd = platform_get_drvdata(pdev);
+	watchdog_unregister_device(wd);
+	return 0;
+}
+
+static struct platform_driver mid_wdt_driver = {
+	.probe		= mid_wdt_probe,
+	.remove		= mid_wdt_remove,
+	.driver		= {
+		.owner	= THIS_MODULE,
+		.name	= "intel_mid_wdt",
+	},
+};
+
+module_platform_driver(mid_wdt_driver);
+
+MODULE_AUTHOR("David Cohen <david.a.cohen@linux.intel.com>");
+MODULE_DESCRIPTION("Watchdog Driver for Intel MID platform");
+MODULE_LICENSE("GPL");
-- 
1.9.1

[PATCH v2 2/2] x86: intel-mid: add watchdog platform code for Merrifield

[David Cohen]

This patch adds platform code for Intel Merrifield.
Since the watchdog is not part of SFI table, we have no other option but
to manually register watchdog's platform device (argh!).

Change-Id: I920968649847577a0aa1ae0b26f2a3551f77d576
Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
---
 arch/x86/platform/intel-mid/device_libs/Makefile   |  1 +
 .../platform/intel-mid/device_libs/platform_wdt.c  | 43 ++++++++++++++++++++++
 2 files changed, 44 insertions(+)
 create mode 100644 arch/x86/platform/intel-mid/device_libs/platform_wdt.c

diff --git a/arch/x86/platform/intel-mid/device_libs/Makefile b/arch/x86/platform/intel-mid/device_libs/Makefile
index 097e7a7940d8..af9307f2cc28 100644
--- a/arch/x86/platform/intel-mid/device_libs/Makefile
+++ b/arch/x86/platform/intel-mid/device_libs/Makefile
@@ -20,3 +20,4 @@ obj-$(subst m,y,$(CONFIG_DRM_MEDFIELD)) += platform_tc35876x.o
 obj-$(subst m,y,$(CONFIG_SERIAL_MRST_MAX3110)) += platform_max3111.o
 # MISC Devices
 obj-$(subst m,y,$(CONFIG_KEYBOARD_GPIO)) += platform_gpio_keys.o
+obj-$(subst m,y,$(CONFIG_INTEL_MID_WATCHDOG)) += platform_wdt.o
diff --git a/arch/x86/platform/intel-mid/device_libs/platform_wdt.c b/arch/x86/platform/intel-mid/device_libs/platform_wdt.c
new file mode 100644
index 000000000000..be4fc66fbe1b
--- /dev/null
+++ b/arch/x86/platform/intel-mid/device_libs/platform_wdt.c
@@ -0,0 +1,43 @@
+/*
+ * platform_wdt.c: Watchdog platform library file
+ *
+ * (C) Copyright 2014 Intel Corporation
+ * Author: David Cohen <david.a.cohen@linux.intel.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; version 2
+ * of the License.
+ */
+
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <asm/intel-mid.h>
+
+#define EXT_TIMER0_MSI 15
+
+static const struct resource tangier_wdt_res[] = {
+	{
+		.start = EXT_TIMER0_MSI,
+		.flags = IORESOURCE_IRQ,
+	},
+};
+
+static struct platform_device mid_wdt_dev = {
+	.name = "intel_mid_wdt",
+	.id = -1,
+};
+
+static int __init register_mid_wdt(void)
+{
+	if (intel_mid_identify_cpu() == INTEL_MID_CPU_CHIP_TANGIER) {
+		platform_device_add_resources(&mid_wdt_dev,
+					      tangier_wdt_res,
+					      ARRAY_SIZE(tangier_wdt_res));
+		return platform_device_register(&mid_wdt_dev);
+	}
+
+	return -ENODEV;
+}
+
+rootfs_initcall(register_mid_wdt);
-- 
1.9.1

[PATCH v3 0/2] Initial implementation of Intel MID watchdog driver

[David Cohen]

Hi,

This patch set is an initial implementation of a generic Intel MID watchdog
driver.
It currently supports Intel Merrifield only, but will extend to other SoCs of
MID family.

changes from v2 to v3:
 - Moved call of x86 platform symbols to wdt platform code to allow build the
   driver as module

changes from v1 to v2:
 - Fixed Kconfig entry as per Guenter Roeck suggestion

Br, David

--
David Cohen (2):
  watchdog: add Intel MID watchdog driver support
  x86: intel-mid: add watchdog platform code for Merrifield

 arch/x86/platform/intel-mid/device_libs/Makefile   |   1 +
 .../platform/intel-mid/device_libs/platform_wdt.c  |  63 +++++++
 drivers/watchdog/Kconfig                           |  13 ++
 drivers/watchdog/Makefile                          |   1 +
 drivers/watchdog/intel-mid_wdt.c                   | 187 +++++++++++++++++++++
 include/linux/intel-mid_wdt.h                      |  20 +++
 6 files changed, 285 insertions(+)
 create mode 100644 arch/x86/platform/intel-mid/device_libs/platform_wdt.c
 create mode 100644 drivers/watchdog/intel-mid_wdt.c
 create mode 100644 include/linux/intel-mid_wdt.h

-- 
1.9.1

[PATCH v3 1/2] watchdog: add Intel MID watchdog driver support

[David Cohen]

Add initial Intel MID watchdog driver support.

This driver is an initial implementation of generic Intel MID watchdog
driver. Currently it supports Intel Merrifield platform.

Signed-off-by: Eric Ernst <eric.ernst@intel.com>
Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
---
 drivers/watchdog/Kconfig         |  13 +++
 drivers/watchdog/Makefile        |   1 +
 drivers/watchdog/intel-mid_wdt.c | 187 +++++++++++++++++++++++++++++++++++++++
 include/linux/intel-mid_wdt.h    |  20 +++++
 4 files changed, 221 insertions(+)
 create mode 100644 drivers/watchdog/intel-mid_wdt.c
 create mode 100644 include/linux/intel-mid_wdt.h

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 79d25894343a..0e110c37aa5c 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -643,6 +643,19 @@ config INTEL_SCU_WATCHDOG
 
 	  To compile this driver as a module, choose M here.
 
+config INTEL_MID_WATCHDOG
+	tristate "Intel MID Watchdog Timer"
+	depends on X86_INTEL_MID
+	select WATCHDOG_CORE
+	---help---
+	  Watchdog timer driver built into the Intel SCU for Intel MID
+	  Platforms.
+
+	  This driver currently supports only the watchdog evolution
+	  implementation in SCU, available for Merrifield generation.
+
+	  To compile this driver as a module, choose M here.
+
 config ITCO_WDT
 	tristate "Intel TCO Timer/Watchdog"
 	depends on (X86 || IA64) && PCI
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 985a66cda76f..b927e7496eb4 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -113,6 +113,7 @@ obj-$(CONFIG_W83977F_WDT) += w83977f_wdt.o
 obj-$(CONFIG_MACHZ_WDT) += machzwd.o
 obj-$(CONFIG_SBC_EPX_C3_WATCHDOG) += sbc_epx_c3.o
 obj-$(CONFIG_INTEL_SCU_WATCHDOG) += intel_scu_watchdog.o
+obj-$(CONFIG_INTEL_MID_WATCHDOG) += intel-mid_wdt.o
 
 # M32R Architecture
 
diff --git a/drivers/watchdog/intel-mid_wdt.c b/drivers/watchdog/intel-mid_wdt.c
new file mode 100644
index 000000000000..d8bf3c3cd2b7
--- /dev/null
+++ b/drivers/watchdog/intel-mid_wdt.c
@@ -0,0 +1,187 @@
+/*
+ *      intel-mid_wdt: generic Intel MID SCU watchdog driver
+ *
+ *      Platforms supported so far:
+ *      - Merrifield only
+ *
+ *      Copyright (C) 2014 Intel Corporation. All rights reserved.
+ *      Contact: David Cohen <david.a.cohen@linux.intel.com>
+ *
+ *      This program is free software; you can redistribute it and/or
+ *      modify it under the terms of version 2 of the GNU General
+ *      Public License as published by the Free Software Foundation.
+ */
+
+#include <linux/intel-mid_wdt.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/nmi.h>
+#include <linux/platform_device.h>
+#include <linux/watchdog.h>
+
+#include <asm/intel_scu_ipc.h>
+#include <asm/intel-mid.h>
+
+#define IPC_WATCHDOG 0xf8
+
+#define MID_WDT_PRETIMEOUT		15
+#define MID_WDT_TIMEOUT_MIN		(1 + MID_WDT_PRETIMEOUT)
+#define MID_WDT_TIMEOUT_MAX		170
+#define MID_WDT_DEFAULT_TIMEOUT		90
+
+/* SCU watchdog messages */
+enum {
+	SCU_WATCHDOG_START = 0,
+	SCU_WATCHDOG_STOP,
+	SCU_WATCHDOG_KEEPALIVE,
+};
+
+static inline int wdt_command(int sub, u32 *in, int inlen)
+{
+	return intel_scu_ipc_command(IPC_WATCHDOG, sub, in, inlen, NULL, 0);
+}
+
+static int wdt_start(struct watchdog_device *wd)
+{
+	int ret, in_size;
+	int timeout = wd->timeout;
+	struct ipc_wd_start {
+		u32 pretimeout;
+		u32 timeout;
+	} ipc_wd_start = { timeout - MID_WDT_PRETIMEOUT, timeout };
+
+	/*
+	 * SCU expects the input size for watchdog IPC to
+	 * be based on double-word
+	 */
+	in_size = (sizeof(ipc_wd_start) + 3) / 4;
+
+	ret = wdt_command(SCU_WATCHDOG_START, (u32 *)&ipc_wd_start, in_size);
+	if (ret) {
+		struct device *dev = watchdog_get_drvdata(wd);
+		dev_crit(dev, "error starting watchdog: %d\n", ret);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int wdt_ping(struct watchdog_device *wd)
+{
+	int ret;
+
+	ret = wdt_command(SCU_WATCHDOG_KEEPALIVE, NULL, 0);
+	if (ret) {
+		struct device *dev = watchdog_get_drvdata(wd);
+		dev_crit(dev, "Error executing keepalive: 0x%x\n", ret);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int wdt_stop(struct watchdog_device *wd)
+{
+	int ret;
+
+	ret = wdt_command(SCU_WATCHDOG_STOP, NULL, 0);
+	if (ret) {
+		struct device *dev = watchdog_get_drvdata(wd);
+		dev_crit(dev, "Error stopping watchdog: 0x%x\n", ret);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static irqreturn_t mid_wdt_warning_irq(int irq, void *dev_id)
+{
+	panic("Kernel Watchdog");
+
+	/* This code should not be reached */
+	return IRQ_HANDLED;
+}
+
+static const struct watchdog_info mid_wdt_info = {
+	.identity = "Intel MID SCU watchdog",
+	.options = WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT,
+};
+
+static const struct watchdog_ops mid_wdt_ops = {
+	.owner = THIS_MODULE,
+	.start = wdt_start,
+	.stop = wdt_stop,
+	.ping = wdt_ping,
+};
+
+static int mid_wdt_probe(struct platform_device *pdev)
+{
+	struct watchdog_device *wdt_dev;
+	struct intel_mid_wdt_pdata *pdata = pdev->dev.platform_data;
+	int ret;
+
+	if (!pdata) {
+		dev_err(&pdev->dev, "missing platform data\n");
+		return -EINVAL;
+	}
+
+	if (pdata->probe) {
+		ret = pdata->probe();
+		if (ret)
+			return ret;
+	}
+
+	wdt_dev = devm_kzalloc(&pdev->dev, sizeof(*wdt_dev), GFP_KERNEL);
+	if (!wdt_dev)
+		return -ENOMEM;
+
+	wdt_dev->info = &mid_wdt_info;
+	wdt_dev->ops = &mid_wdt_ops;
+	wdt_dev->min_timeout = MID_WDT_TIMEOUT_MIN;
+	wdt_dev->max_timeout = MID_WDT_TIMEOUT_MAX;
+	wdt_dev->timeout = MID_WDT_DEFAULT_TIMEOUT;
+
+	watchdog_set_drvdata(wdt_dev, &pdev->dev);
+	platform_set_drvdata(pdev, wdt_dev);
+
+	ret = devm_request_irq(&pdev->dev, pdata->irq, mid_wdt_warning_irq,
+			       IRQF_SHARED | IRQF_NO_SUSPEND, "watchdog",
+			       wdt_dev);
+	if (ret) {
+		dev_err(&pdev->dev, "error requesting warning irq %d\n",
+			pdata->irq);
+		return ret;
+	}
+
+	ret = watchdog_register_device(wdt_dev);
+	if (ret) {
+		dev_err(&pdev->dev, "error registering watchdog device\n");
+		return ret;
+	}
+
+	dev_info(&pdev->dev, "Intel MID watchdog device probed\n");
+
+	return 0;
+}
+
+static int mid_wdt_remove(struct platform_device *pdev)
+{
+	struct watchdog_device *wd = platform_get_drvdata(pdev);
+	watchdog_unregister_device(wd);
+	return 0;
+}
+
+static struct platform_driver mid_wdt_driver = {
+	.probe		= mid_wdt_probe,
+	.remove		= mid_wdt_remove,
+	.driver		= {
+		.owner	= THIS_MODULE,
+		.name	= "intel_mid_wdt",
+	},
+};
+
+module_platform_driver(mid_wdt_driver);
+
+MODULE_AUTHOR("David Cohen <david.a.cohen@linux.intel.com>");
+MODULE_DESCRIPTION("Watchdog Driver for Intel MID platform");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/intel-mid_wdt.h b/include/linux/intel-mid_wdt.h
new file mode 100644
index 000000000000..705dff1c62f8
--- /dev/null
+++ b/include/linux/intel-mid_wdt.h
@@ -0,0 +1,20 @@
+/*
+ *      intel-mid_wdt: generic Intel MID SCU watchdog driver
+ *
+ *      Copyright (C) 2014 Intel Corporation. All rights reserved.
+ *      Contact: David Cohen <david.a.cohen@linux.intel.com>
+ *
+ *      This program is free software; you can redistribute it and/or
+ *      modify it under the terms of version 2 of the GNU General
+ *      Public License as published by the Free Software Foundation.
+ */
+
+#ifndef __INTEL_MID_WDT_H__
+#define __INTEL_MID_WDT_H__
+
+struct intel_mid_wdt_pdata {
+	int irq;
+	int (*probe)(void);
+};
+
+#endif /*__INTEL_MID_WDT_H__*/
-- 
1.9.1

[PATCH v3 2/2] x86: intel-mid: add watchdog platform code for Merrifield

[David Cohen]

This patch adds platform code for Intel Merrifield.
Since the watchdog is not part of SFI table, we have no other option but
to manually register watchdog's platform device (argh!).

Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
---
 arch/x86/platform/intel-mid/device_libs/Makefile   |  1 +
 .../platform/intel-mid/device_libs/platform_wdt.c  | 63 ++++++++++++++++++++++
 2 files changed, 64 insertions(+)
 create mode 100644 arch/x86/platform/intel-mid/device_libs/platform_wdt.c

diff --git a/arch/x86/platform/intel-mid/device_libs/Makefile b/arch/x86/platform/intel-mid/device_libs/Makefile
index 097e7a7940d8..af9307f2cc28 100644
--- a/arch/x86/platform/intel-mid/device_libs/Makefile
+++ b/arch/x86/platform/intel-mid/device_libs/Makefile
@@ -20,3 +20,4 @@ obj-$(subst m,y,$(CONFIG_DRM_MEDFIELD)) += platform_tc35876x.o
 obj-$(subst m,y,$(CONFIG_SERIAL_MRST_MAX3110)) += platform_max3111.o
 # MISC Devices
 obj-$(subst m,y,$(CONFIG_KEYBOARD_GPIO)) += platform_gpio_keys.o
+obj-$(subst m,y,$(CONFIG_INTEL_MID_WATCHDOG)) += platform_wdt.o
diff --git a/arch/x86/platform/intel-mid/device_libs/platform_wdt.c b/arch/x86/platform/intel-mid/device_libs/platform_wdt.c
new file mode 100644
index 000000000000..653242110d57
--- /dev/null
+++ b/arch/x86/platform/intel-mid/device_libs/platform_wdt.c
@@ -0,0 +1,63 @@
+/*
+ * platform_wdt.c: Watchdog platform library file
+ *
+ * (C) Copyright 2014 Intel Corporation
+ * Author: David Cohen <david.a.cohen@linux.intel.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; version 2
+ * of the License.
+ */
+
+#include <linux/init.h>
+#include <linux/intel-mid_wdt.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <asm/intel-mid.h>
+
+#define TANGIER_EXT_TIMER0_MSI 15
+
+static struct platform_device wdt_dev = {
+	.name = "intel_mid_wdt",
+	.id = -1,
+};
+
+static int tangier_probe(void)
+{
+	int ioapic;
+	struct io_apic_irq_attr irq_attr;
+
+	ioapic = mp_find_ioapic(TANGIER_EXT_TIMER0_MSI);
+	if (ioapic >= 0) {
+		irq_attr.ioapic = ioapic;
+		irq_attr.ioapic_pin = TANGIER_EXT_TIMER0_MSI;
+		irq_attr.trigger = 1;
+		irq_attr.polarity = 0; /* Active high */
+		io_apic_set_pci_routing(NULL, TANGIER_EXT_TIMER0_MSI,
+					&irq_attr);
+	} else {
+		pr_warn("intel_mid_wdt: can not find interrupt %d in ioapic\n",
+			TANGIER_EXT_TIMER0_MSI);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static struct intel_mid_wdt_pdata tangier_pdata = {
+	.irq = TANGIER_EXT_TIMER0_MSI,
+	.probe = tangier_probe,
+};
+
+static int __init register_mid_wdt(void)
+{
+	if (intel_mid_identify_cpu() == INTEL_MID_CPU_CHIP_TANGIER) {
+		wdt_dev.dev.platform_data = &tangier_pdata;
+		return platform_device_register(&wdt_dev);
+	}
+
+	return -ENODEV;
+}
+
+rootfs_initcall(register_mid_wdt);
-- 
1.9.1

Re: [PATCH v3 1/2] watchdog: add Intel MID watchdog driver support

[Guenter Roeck]

On Tue, Apr 15, 2014 at 11:41:11AM -0700, David Cohen wrote:
> Add initial Intel MID watchdog driver support.
> 
> This driver is an initial implementation of generic Intel MID watchdog
> driver. Currently it supports Intel Merrifield platform.
> 
> Signed-off-by: Eric Ernst <eric.ernst@intel.com>
> Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
> ---
>  drivers/watchdog/Kconfig         |  13 +++
>  drivers/watchdog/Makefile        |   1 +
>  drivers/watchdog/intel-mid_wdt.c | 187 +++++++++++++++++++++++++++++++++++++++
>  include/linux/intel-mid_wdt.h    |  20 +++++

include/linux/platform_data/intel-mid_wdt.h ?

>  4 files changed, 221 insertions(+)
>  create mode 100644 drivers/watchdog/intel-mid_wdt.c
>  create mode 100644 include/linux/intel-mid_wdt.h
> 
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 79d25894343a..0e110c37aa5c 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -643,6 +643,19 @@ config INTEL_SCU_WATCHDOG
>  
>  	  To compile this driver as a module, choose M here.
>  
> +config INTEL_MID_WATCHDOG
> +	tristate "Intel MID Watchdog Timer"
> +	depends on X86_INTEL_MID
> +	select WATCHDOG_CORE
> +	---help---
> +	  Watchdog timer driver built into the Intel SCU for Intel MID
> +	  Platforms.
> +
> +	  This driver currently supports only the watchdog evolution
> +	  implementation in SCU, available for Merrifield generation.
> +
> +	  To compile this driver as a module, choose M here.
> +
>  config ITCO_WDT
>  	tristate "Intel TCO Timer/Watchdog"
>  	depends on (X86 || IA64) && PCI
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index 985a66cda76f..b927e7496eb4 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -113,6 +113,7 @@ obj-$(CONFIG_W83977F_WDT) += w83977f_wdt.o
>  obj-$(CONFIG_MACHZ_WDT) += machzwd.o
>  obj-$(CONFIG_SBC_EPX_C3_WATCHDOG) += sbc_epx_c3.o
>  obj-$(CONFIG_INTEL_SCU_WATCHDOG) += intel_scu_watchdog.o
> +obj-$(CONFIG_INTEL_MID_WATCHDOG) += intel-mid_wdt.o
>  
>  # M32R Architecture
>  
> diff --git a/drivers/watchdog/intel-mid_wdt.c b/drivers/watchdog/intel-mid_wdt.c
> new file mode 100644
> index 000000000000..d8bf3c3cd2b7
> --- /dev/null
> +++ b/drivers/watchdog/intel-mid_wdt.c
> @@ -0,0 +1,187 @@
> +/*
> + *      intel-mid_wdt: generic Intel MID SCU watchdog driver
> + *
> + *      Platforms supported so far:
> + *      - Merrifield only
> + *
> + *      Copyright (C) 2014 Intel Corporation. All rights reserved.
> + *      Contact: David Cohen <david.a.cohen@linux.intel.com>
> + *
> + *      This program is free software; you can redistribute it and/or
> + *      modify it under the terms of version 2 of the GNU General
> + *      Public License as published by the Free Software Foundation.
> + */
> +
> +#include <linux/intel-mid_wdt.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/nmi.h>
> +#include <linux/platform_device.h>
> +#include <linux/watchdog.h>
> +
> +#include <asm/intel_scu_ipc.h>
> +#include <asm/intel-mid.h>
> +
> +#define IPC_WATCHDOG 0xf8
> +
> +#define MID_WDT_PRETIMEOUT		15
> +#define MID_WDT_TIMEOUT_MIN		(1 + MID_WDT_PRETIMEOUT)
> +#define MID_WDT_TIMEOUT_MAX		170
> +#define MID_WDT_DEFAULT_TIMEOUT		90
> +
> +/* SCU watchdog messages */
> +enum {
> +	SCU_WATCHDOG_START = 0,
> +	SCU_WATCHDOG_STOP,
> +	SCU_WATCHDOG_KEEPALIVE,
> +};
> +
> +static inline int wdt_command(int sub, u32 *in, int inlen)
> +{
> +	return intel_scu_ipc_command(IPC_WATCHDOG, sub, in, inlen, NULL, 0);
> +}
> +
> +static int wdt_start(struct watchdog_device *wd)
> +{
> +	int ret, in_size;
> +	int timeout = wd->timeout;
> +	struct ipc_wd_start {
> +		u32 pretimeout;
> +		u32 timeout;
> +	} ipc_wd_start = { timeout - MID_WDT_PRETIMEOUT, timeout };
> +
> +	/*
> +	 * SCU expects the input size for watchdog IPC to
> +	 * be based on double-word
> +	 */
> +	in_size = (sizeof(ipc_wd_start) + 3) / 4;
> +
DIV_ROUND_UP ?

Not sure how "double-word" matches the calculation. What is a "double-word" ?
4 bytes ?

> +	ret = wdt_command(SCU_WATCHDOG_START, (u32 *)&ipc_wd_start, in_size);
> +	if (ret) {
> +		struct device *dev = watchdog_get_drvdata(wd);
> +		dev_crit(dev, "error starting watchdog: %d\n", ret);
> +		return -EIO;

I am quite sure sparse is going to complain about this ...
Why not just return ret ? This would result in a more detailed
error code/reason.

> +	}
> +
> +	return 0;
> +}
> +
> +static int wdt_ping(struct watchdog_device *wd)
> +{
> +	int ret;
> +
> +	ret = wdt_command(SCU_WATCHDOG_KEEPALIVE, NULL, 0);
> +	if (ret) {
> +		struct device *dev = watchdog_get_drvdata(wd);
> +		dev_crit(dev, "Error executing keepalive: 0x%x\n", ret);
> +		return -EIO;

Same here.

> +	}
> +
> +	return 0;
> +}
> +
> +static int wdt_stop(struct watchdog_device *wd)
> +{
> +	int ret;
> +
> +	ret = wdt_command(SCU_WATCHDOG_STOP, NULL, 0);
> +	if (ret) {
> +		struct device *dev = watchdog_get_drvdata(wd);
> +		dev_crit(dev, "Error stopping watchdog: 0x%x\n", ret);
> +		return -EIO;

Same here.

> +	}
> +
> +	return 0;
> +}
> +
> +static irqreturn_t mid_wdt_warning_irq(int irq, void *dev_id)

This isn't really a warning, so the function name is a bit misleading.

> +{
> +	panic("Kernel Watchdog");
> +
> +	/* This code should not be reached */
> +	return IRQ_HANDLED;
> +}
> +
> +static const struct watchdog_info mid_wdt_info = {
> +	.identity = "Intel MID SCU watchdog",
> +	.options = WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT,
> +};
> +
> +static const struct watchdog_ops mid_wdt_ops = {
> +	.owner = THIS_MODULE,
> +	.start = wdt_start,
> +	.stop = wdt_stop,
> +	.ping = wdt_ping,
> +};
> +
> +static int mid_wdt_probe(struct platform_device *pdev)
> +{
> +	struct watchdog_device *wdt_dev;
> +	struct intel_mid_wdt_pdata *pdata = pdev->dev.platform_data;
> +	int ret;
> +
> +	if (!pdata) {
> +		dev_err(&pdev->dev, "missing platform data\n");
> +		return -EINVAL;
> +	}
> +
> +	if (pdata->probe) {
> +		ret = pdata->probe();

Sure you don't want to pass the platform device as parameter ?
May not be needed today, but it might be better than to rely on the assumption
that the called code gets it from a static variable or structure (if needed).

This would enable you, for example, to use dev_warn() instead of pr_warn()
in the platform code.

> +		if (ret)
> +			return ret;
> +	}
> +
> +	wdt_dev = devm_kzalloc(&pdev->dev, sizeof(*wdt_dev), GFP_KERNEL);
> +	if (!wdt_dev)
> +		return -ENOMEM;
> +
> +	wdt_dev->info = &mid_wdt_info;
> +	wdt_dev->ops = &mid_wdt_ops;
> +	wdt_dev->min_timeout = MID_WDT_TIMEOUT_MIN;
> +	wdt_dev->max_timeout = MID_WDT_TIMEOUT_MAX;
> +	wdt_dev->timeout = MID_WDT_DEFAULT_TIMEOUT;
> +
> +	watchdog_set_drvdata(wdt_dev, &pdev->dev);
> +	platform_set_drvdata(pdev, wdt_dev);
> +
> +	ret = devm_request_irq(&pdev->dev, pdata->irq, mid_wdt_warning_irq,
> +			       IRQF_SHARED | IRQF_NO_SUSPEND, "watchdog",
> +			       wdt_dev);

If irq is mandatory, it might make sense to check if it is valid.

> +	if (ret) {
> +		dev_err(&pdev->dev, "error requesting warning irq %d\n",
> +			pdata->irq);
> +		return ret;
> +	}
> +
> +	ret = watchdog_register_device(wdt_dev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "error registering watchdog device\n");
> +		return ret;
> +	}
> +
> +	dev_info(&pdev->dev, "Intel MID watchdog device probed\n");
> +
> +	return 0;
> +}
> +
> +static int mid_wdt_remove(struct platform_device *pdev)
> +{
> +	struct watchdog_device *wd = platform_get_drvdata(pdev);
> +	watchdog_unregister_device(wd);
> +	return 0;
> +}
> +
> +static struct platform_driver mid_wdt_driver = {
> +	.probe		= mid_wdt_probe,
> +	.remove		= mid_wdt_remove,
> +	.driver		= {
> +		.owner	= THIS_MODULE,
> +		.name	= "intel_mid_wdt",
> +	},
> +};
> +
> +module_platform_driver(mid_wdt_driver);
> +
> +MODULE_AUTHOR("David Cohen <david.a.cohen@linux.intel.com>");
> +MODULE_DESCRIPTION("Watchdog Driver for Intel MID platform");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/intel-mid_wdt.h b/include/linux/intel-mid_wdt.h
> new file mode 100644
> index 000000000000..705dff1c62f8
> --- /dev/null
> +++ b/include/linux/intel-mid_wdt.h
> @@ -0,0 +1,20 @@
> +/*
> + *      intel-mid_wdt: generic Intel MID SCU watchdog driver
> + *
> + *      Copyright (C) 2014 Intel Corporation. All rights reserved.
> + *      Contact: David Cohen <david.a.cohen@linux.intel.com>
> + *
> + *      This program is free software; you can redistribute it and/or
> + *      modify it under the terms of version 2 of the GNU General
> + *      Public License as published by the Free Software Foundation.
> + */
> +
> +#ifndef __INTEL_MID_WDT_H__
> +#define __INTEL_MID_WDT_H__
> +
> +struct intel_mid_wdt_pdata {
> +	int irq;
> +	int (*probe)(void);
> +};
> +
> +#endif /*__INTEL_MID_WDT_H__*/
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH v3 2/2] x86: intel-mid: add watchdog platform code for Merrifield

[Guenter Roeck]

On Tue, Apr 15, 2014 at 11:41:12AM -0700, David Cohen wrote:
> This patch adds platform code for Intel Merrifield.
> Since the watchdog is not part of SFI table, we have no other option but
> to manually register watchdog's platform device (argh!).
> 
argh ... does this really belong into the commit log ?

> Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
> ---
>  arch/x86/platform/intel-mid/device_libs/Makefile   |  1 +
>  .../platform/intel-mid/device_libs/platform_wdt.c  | 63 ++++++++++++++++++++++
>  2 files changed, 64 insertions(+)
>  create mode 100644 arch/x86/platform/intel-mid/device_libs/platform_wdt.c
> 
> diff --git a/arch/x86/platform/intel-mid/device_libs/Makefile b/arch/x86/platform/intel-mid/device_libs/Makefile
> index 097e7a7940d8..af9307f2cc28 100644
> --- a/arch/x86/platform/intel-mid/device_libs/Makefile
> +++ b/arch/x86/platform/intel-mid/device_libs/Makefile
> @@ -20,3 +20,4 @@ obj-$(subst m,y,$(CONFIG_DRM_MEDFIELD)) += platform_tc35876x.o
>  obj-$(subst m,y,$(CONFIG_SERIAL_MRST_MAX3110)) += platform_max3111.o
>  # MISC Devices
>  obj-$(subst m,y,$(CONFIG_KEYBOARD_GPIO)) += platform_gpio_keys.o
> +obj-$(subst m,y,$(CONFIG_INTEL_MID_WATCHDOG)) += platform_wdt.o
> diff --git a/arch/x86/platform/intel-mid/device_libs/platform_wdt.c b/arch/x86/platform/intel-mid/device_libs/platform_wdt.c
> new file mode 100644
> index 000000000000..653242110d57
> --- /dev/null
> +++ b/arch/x86/platform/intel-mid/device_libs/platform_wdt.c
> @@ -0,0 +1,63 @@
> +/*
> + * platform_wdt.c: Watchdog platform library file
> + *
> + * (C) Copyright 2014 Intel Corporation
> + * Author: David Cohen <david.a.cohen@linux.intel.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; version 2
> + * of the License.
> + */
> +
> +#include <linux/init.h>
> +#include <linux/intel-mid_wdt.h>

	/platform_data/...

> +#include <linux/interrupt.h>
> +#include <linux/platform_device.h>
> +#include <asm/intel-mid.h>
> +
> +#define TANGIER_EXT_TIMER0_MSI 15
> +
> +static struct platform_device wdt_dev = {
> +	.name = "intel_mid_wdt",
> +	.id = -1,
> +};
> +
> +static int tangier_probe(void)
> +{
> +	int ioapic;
> +	struct io_apic_irq_attr irq_attr;
> +
				= { };
would ensure that there are no un-initialized variables
if the structure definition ever changes.

Or just pre-initialize the entire structure as much as possible.

> +	ioapic = mp_find_ioapic(TANGIER_EXT_TIMER0_MSI);
> +	if (ioapic >= 0) {
> +		irq_attr.ioapic = ioapic;
> +		irq_attr.ioapic_pin = TANGIER_EXT_TIMER0_MSI;
> +		irq_attr.trigger = 1;
> +		irq_attr.polarity = 0; /* Active high */
> +		io_apic_set_pci_routing(NULL, TANGIER_EXT_TIMER0_MSI,
> +					&irq_attr);

No need for an error check ? I understand no one else does,
but why does the function return an error if no one cares about it ?

> +	} else {
> +		pr_warn("intel_mid_wdt: can not find interrupt %d in ioapic\n",
> +			TANGIER_EXT_TIMER0_MSI);

As mentioned earlier, I think it would pake sense to pass
struct platform_device * as argument and use dev_warn.

It would also enable you to use the irq passed from the parameter
instead of checking for the hard-coded define.

> +		return -EINVAL;

Why not return the error returned by mp_find_ioapic() ?

> +	}
> +
> +	return 0;
> +}
> +
> +static struct intel_mid_wdt_pdata tangier_pdata = {
> +	.irq = TANGIER_EXT_TIMER0_MSI,
> +	.probe = tangier_probe,
> +};
> +
> +static int __init register_mid_wdt(void)
> +{
> +	if (intel_mid_identify_cpu() == INTEL_MID_CPU_CHIP_TANGIER) {
> +		wdt_dev.dev.platform_data = &tangier_pdata;
> +		return platform_device_register(&wdt_dev);
> +	}
> +
> +	return -ENODEV;
> +}
> +
> +rootfs_initcall(register_mid_wdt);
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH v3 1/2] watchdog: add Intel MID watchdog driver support

[David Cohen]

On Tue, Apr 15, 2014 at 12:01:49PM -0700, Guenter Roeck wrote:
> On Tue, Apr 15, 2014 at 11:41:11AM -0700, David Cohen wrote:
> > Add initial Intel MID watchdog driver support.
> > 
> > This driver is an initial implementation of generic Intel MID watchdog
> > driver. Currently it supports Intel Merrifield platform.
> > 
> > Signed-off-by: Eric Ernst <eric.ernst@intel.com>
> > Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
> > ---
> >  drivers/watchdog/Kconfig         |  13 +++
> >  drivers/watchdog/Makefile        |   1 +
> >  drivers/watchdog/intel-mid_wdt.c | 187 +++++++++++++++++++++++++++++++++++++++
> >  include/linux/intel-mid_wdt.h    |  20 +++++
> 
> include/linux/platform_data/intel-mid_wdt.h ?

Ack

> 
> >  4 files changed, 221 insertions(+)
> >  create mode 100644 drivers/watchdog/intel-mid_wdt.c
> >  create mode 100644 include/linux/intel-mid_wdt.h
> > 
> > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> > index 79d25894343a..0e110c37aa5c 100644
> > --- a/drivers/watchdog/Kconfig
> > +++ b/drivers/watchdog/Kconfig
> > @@ -643,6 +643,19 @@ config INTEL_SCU_WATCHDOG
> >  
> >  	  To compile this driver as a module, choose M here.
> >  
> > +config INTEL_MID_WATCHDOG
> > +	tristate "Intel MID Watchdog Timer"
> > +	depends on X86_INTEL_MID
> > +	select WATCHDOG_CORE
> > +	---help---
> > +	  Watchdog timer driver built into the Intel SCU for Intel MID
> > +	  Platforms.
> > +
> > +	  This driver currently supports only the watchdog evolution
> > +	  implementation in SCU, available for Merrifield generation.
> > +
> > +	  To compile this driver as a module, choose M here.
> > +
> >  config ITCO_WDT
> >  	tristate "Intel TCO Timer/Watchdog"
> >  	depends on (X86 || IA64) && PCI
> > diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> > index 985a66cda76f..b927e7496eb4 100644
> > --- a/drivers/watchdog/Makefile
> > +++ b/drivers/watchdog/Makefile
> > @@ -113,6 +113,7 @@ obj-$(CONFIG_W83977F_WDT) += w83977f_wdt.o
> >  obj-$(CONFIG_MACHZ_WDT) += machzwd.o
> >  obj-$(CONFIG_SBC_EPX_C3_WATCHDOG) += sbc_epx_c3.o
> >  obj-$(CONFIG_INTEL_SCU_WATCHDOG) += intel_scu_watchdog.o
> > +obj-$(CONFIG_INTEL_MID_WATCHDOG) += intel-mid_wdt.o
> >  
> >  # M32R Architecture
> >  
> > diff --git a/drivers/watchdog/intel-mid_wdt.c b/drivers/watchdog/intel-mid_wdt.c
> > new file mode 100644
> > index 000000000000..d8bf3c3cd2b7
> > --- /dev/null
> > +++ b/drivers/watchdog/intel-mid_wdt.c
> > @@ -0,0 +1,187 @@
> > +/*
> > + *      intel-mid_wdt: generic Intel MID SCU watchdog driver
> > + *
> > + *      Platforms supported so far:
> > + *      - Merrifield only
> > + *
> > + *      Copyright (C) 2014 Intel Corporation. All rights reserved.
> > + *      Contact: David Cohen <david.a.cohen@linux.intel.com>
> > + *
> > + *      This program is free software; you can redistribute it and/or
> > + *      modify it under the terms of version 2 of the GNU General
> > + *      Public License as published by the Free Software Foundation.
> > + */
> > +
> > +#include <linux/intel-mid_wdt.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/module.h>
> > +#include <linux/nmi.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/watchdog.h>
> > +
> > +#include <asm/intel_scu_ipc.h>
> > +#include <asm/intel-mid.h>
> > +
> > +#define IPC_WATCHDOG 0xf8
> > +
> > +#define MID_WDT_PRETIMEOUT		15
> > +#define MID_WDT_TIMEOUT_MIN		(1 + MID_WDT_PRETIMEOUT)
> > +#define MID_WDT_TIMEOUT_MAX		170
> > +#define MID_WDT_DEFAULT_TIMEOUT		90
> > +
> > +/* SCU watchdog messages */
> > +enum {
> > +	SCU_WATCHDOG_START = 0,
> > +	SCU_WATCHDOG_STOP,
> > +	SCU_WATCHDOG_KEEPALIVE,
> > +};
> > +
> > +static inline int wdt_command(int sub, u32 *in, int inlen)
> > +{
> > +	return intel_scu_ipc_command(IPC_WATCHDOG, sub, in, inlen, NULL, 0);
> > +}
> > +
> > +static int wdt_start(struct watchdog_device *wd)
> > +{
> > +	int ret, in_size;
> > +	int timeout = wd->timeout;
> > +	struct ipc_wd_start {
> > +		u32 pretimeout;
> > +		u32 timeout;
> > +	} ipc_wd_start = { timeout - MID_WDT_PRETIMEOUT, timeout };
> > +
> > +	/*
> > +	 * SCU expects the input size for watchdog IPC to
> > +	 * be based on double-word
> > +	 */
> > +	in_size = (sizeof(ipc_wd_start) + 3) / 4;
> > +
> DIV_ROUND_UP ?

Ack

> 
> Not sure how "double-word" matches the calculation. What is a "double-word" ?
> 4 bytes ?
> 
> > +	ret = wdt_command(SCU_WATCHDOG_START, (u32 *)&ipc_wd_start, in_size);
> > +	if (ret) {
> > +		struct device *dev = watchdog_get_drvdata(wd);
> > +		dev_crit(dev, "error starting watchdog: %d\n", ret);
> > +		return -EIO;
> 
> I am quite sure sparse is going to complain about this ...
> Why not just return ret ? This would result in a more detailed
> error code/reason.

wdt_command() may return errors != -EIO. But IMHO -EIO makes more sense
whatever is the issue regarding to IPC in this scenario.
But I can return ret as per request. Same to cases below.

> 
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int wdt_ping(struct watchdog_device *wd)
> > +{
> > +	int ret;
> > +
> > +	ret = wdt_command(SCU_WATCHDOG_KEEPALIVE, NULL, 0);
> > +	if (ret) {
> > +		struct device *dev = watchdog_get_drvdata(wd);
> > +		dev_crit(dev, "Error executing keepalive: 0x%x\n", ret);
> > +		return -EIO;
> 
> Same here.
> 
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int wdt_stop(struct watchdog_device *wd)
> > +{
> > +	int ret;
> > +
> > +	ret = wdt_command(SCU_WATCHDOG_STOP, NULL, 0);
> > +	if (ret) {
> > +		struct device *dev = watchdog_get_drvdata(wd);
> > +		dev_crit(dev, "Error stopping watchdog: 0x%x\n", ret);
> > +		return -EIO;
> 
> Same here.
> 
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static irqreturn_t mid_wdt_warning_irq(int irq, void *dev_id)
> 
> This isn't really a warning, so the function name is a bit misleading.

Ack

> 
> > +{
> > +	panic("Kernel Watchdog");
> > +
> > +	/* This code should not be reached */
> > +	return IRQ_HANDLED;
> > +}
> > +
> > +static const struct watchdog_info mid_wdt_info = {
> > +	.identity = "Intel MID SCU watchdog",
> > +	.options = WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT,
> > +};
> > +
> > +static const struct watchdog_ops mid_wdt_ops = {
> > +	.owner = THIS_MODULE,
> > +	.start = wdt_start,
> > +	.stop = wdt_stop,
> > +	.ping = wdt_ping,
> > +};
> > +
> > +static int mid_wdt_probe(struct platform_device *pdev)
> > +{
> > +	struct watchdog_device *wdt_dev;
> > +	struct intel_mid_wdt_pdata *pdata = pdev->dev.platform_data;
> > +	int ret;
> > +
> > +	if (!pdata) {
> > +		dev_err(&pdev->dev, "missing platform data\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (pdata->probe) {
> > +		ret = pdata->probe();
> 
> Sure you don't want to pass the platform device as parameter ?
> May not be needed today, but it might be better than to rely on the assumption
> that the called code gets it from a static variable or structure (if needed).
> 
> This would enable you, for example, to use dev_warn() instead of pr_warn()
> in the platform code.

Ack.

> 
> > +		if (ret)
> > +			return ret;
> > +	}
> > +
> > +	wdt_dev = devm_kzalloc(&pdev->dev, sizeof(*wdt_dev), GFP_KERNEL);
> > +	if (!wdt_dev)
> > +		return -ENOMEM;
> > +
> > +	wdt_dev->info = &mid_wdt_info;
> > +	wdt_dev->ops = &mid_wdt_ops;
> > +	wdt_dev->min_timeout = MID_WDT_TIMEOUT_MIN;
> > +	wdt_dev->max_timeout = MID_WDT_TIMEOUT_MAX;
> > +	wdt_dev->timeout = MID_WDT_DEFAULT_TIMEOUT;
> > +
> > +	watchdog_set_drvdata(wdt_dev, &pdev->dev);
> > +	platform_set_drvdata(pdev, wdt_dev);
> > +
> > +	ret = devm_request_irq(&pdev->dev, pdata->irq, mid_wdt_warning_irq,
> > +			       IRQF_SHARED | IRQF_NO_SUSPEND, "watchdog",
> > +			       wdt_dev);
> 
> If irq is mandatory, it might make sense to check if it is valid.

I can add this extra sanity check.

Thanks for your review.

BR, David

> 
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "error requesting warning irq %d\n",
> > +			pdata->irq);
> > +		return ret;
> > +	}
> > +
> > +	ret = watchdog_register_device(wdt_dev);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "error registering watchdog device\n");
> > +		return ret;
> > +	}
> > +
> > +	dev_info(&pdev->dev, "Intel MID watchdog device probed\n");
> > +
> > +	return 0;
> > +}
> > +
> > +static int mid_wdt_remove(struct platform_device *pdev)
> > +{
> > +	struct watchdog_device *wd = platform_get_drvdata(pdev);
> > +	watchdog_unregister_device(wd);
> > +	return 0;
> > +}
> > +
> > +static struct platform_driver mid_wdt_driver = {
> > +	.probe		= mid_wdt_probe,
> > +	.remove		= mid_wdt_remove,
> > +	.driver		= {
> > +		.owner	= THIS_MODULE,
> > +		.name	= "intel_mid_wdt",
> > +	},
> > +};
> > +
> > +module_platform_driver(mid_wdt_driver);
> > +
> > +MODULE_AUTHOR("David Cohen <david.a.cohen@linux.intel.com>");
> > +MODULE_DESCRIPTION("Watchdog Driver for Intel MID platform");
> > +MODULE_LICENSE("GPL");
> > diff --git a/include/linux/intel-mid_wdt.h b/include/linux/intel-mid_wdt.h
> > new file mode 100644
> > index 000000000000..705dff1c62f8
> > --- /dev/null
> > +++ b/include/linux/intel-mid_wdt.h
> > @@ -0,0 +1,20 @@
> > +/*
> > + *      intel-mid_wdt: generic Intel MID SCU watchdog driver
> > + *
> > + *      Copyright (C) 2014 Intel Corporation. All rights reserved.
> > + *      Contact: David Cohen <david.a.cohen@linux.intel.com>
> > + *
> > + *      This program is free software; you can redistribute it and/or
> > + *      modify it under the terms of version 2 of the GNU General
> > + *      Public License as published by the Free Software Foundation.
> > + */
> > +
> > +#ifndef __INTEL_MID_WDT_H__
> > +#define __INTEL_MID_WDT_H__
> > +
> > +struct intel_mid_wdt_pdata {
> > +	int irq;
> > +	int (*probe)(void);
> > +};
> > +
> > +#endif /*__INTEL_MID_WDT_H__*/
> > -- 
> > 1.9.1
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 

Re: [PATCH v3 2/2] x86: intel-mid: add watchdog platform code for Merrifield

[David Cohen]

On Tue, Apr 15, 2014 at 12:09:02PM -0700, Guenter Roeck wrote:
> On Tue, Apr 15, 2014 at 11:41:12AM -0700, David Cohen wrote:
> > This patch adds platform code for Intel Merrifield.
> > Since the watchdog is not part of SFI table, we have no other option but
> > to manually register watchdog's platform device (argh!).
> > 
> argh ... does this really belong into the commit log ?

Yes. It means: "this developer was forced to do this ugly thing due to
fw-related sloppiness".

> 
> > Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
> > ---
> >  arch/x86/platform/intel-mid/device_libs/Makefile   |  1 +
> >  .../platform/intel-mid/device_libs/platform_wdt.c  | 63 ++++++++++++++++++++++
> >  2 files changed, 64 insertions(+)
> >  create mode 100644 arch/x86/platform/intel-mid/device_libs/platform_wdt.c
> > 
> > diff --git a/arch/x86/platform/intel-mid/device_libs/Makefile b/arch/x86/platform/intel-mid/device_libs/Makefile
> > index 097e7a7940d8..af9307f2cc28 100644
> > --- a/arch/x86/platform/intel-mid/device_libs/Makefile
> > +++ b/arch/x86/platform/intel-mid/device_libs/Makefile
> > @@ -20,3 +20,4 @@ obj-$(subst m,y,$(CONFIG_DRM_MEDFIELD)) += platform_tc35876x.o
> >  obj-$(subst m,y,$(CONFIG_SERIAL_MRST_MAX3110)) += platform_max3111.o
> >  # MISC Devices
> >  obj-$(subst m,y,$(CONFIG_KEYBOARD_GPIO)) += platform_gpio_keys.o
> > +obj-$(subst m,y,$(CONFIG_INTEL_MID_WATCHDOG)) += platform_wdt.o
> > diff --git a/arch/x86/platform/intel-mid/device_libs/platform_wdt.c b/arch/x86/platform/intel-mid/device_libs/platform_wdt.c
> > new file mode 100644
> > index 000000000000..653242110d57
> > --- /dev/null
> > +++ b/arch/x86/platform/intel-mid/device_libs/platform_wdt.c
> > @@ -0,0 +1,63 @@
> > +/*
> > + * platform_wdt.c: Watchdog platform library file
> > + *
> > + * (C) Copyright 2014 Intel Corporation
> > + * Author: David Cohen <david.a.cohen@linux.intel.com>
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License
> > + * as published by the Free Software Foundation; version 2
> > + * of the License.
> > + */
> > +
> > +#include <linux/init.h>
> > +#include <linux/intel-mid_wdt.h>
> 
> 	/platform_data/...

Ack

> 
> > +#include <linux/interrupt.h>
> > +#include <linux/platform_device.h>
> > +#include <asm/intel-mid.h>
> > +
> > +#define TANGIER_EXT_TIMER0_MSI 15
> > +
> > +static struct platform_device wdt_dev = {
> > +	.name = "intel_mid_wdt",
> > +	.id = -1,
> > +};
> > +
> > +static int tangier_probe(void)
> > +{
> > +	int ioapic;
> > +	struct io_apic_irq_attr irq_attr;
> > +
> 				= { };
> would ensure that there are no un-initialized variables
> if the structure definition ever changes.
> 
> Or just pre-initialize the entire structure as much as possible.

The first suggestion seems better.

> 
> > +	ioapic = mp_find_ioapic(TANGIER_EXT_TIMER0_MSI);
> > +	if (ioapic >= 0) {
> > +		irq_attr.ioapic = ioapic;
> > +		irq_attr.ioapic_pin = TANGIER_EXT_TIMER0_MSI;
> > +		irq_attr.trigger = 1;
> > +		irq_attr.polarity = 0; /* Active high */
> > +		io_apic_set_pci_routing(NULL, TANGIER_EXT_TIMER0_MSI,
> > +					&irq_attr);
> 
> No need for an error check ? I understand no one else does,
> but why does the function return an error if no one cares about it ?

That's an interesting question. But whatever is the answer, I believe
checking the error makes more sense. I'll change it.

> 
> > +	} else {
> > +		pr_warn("intel_mid_wdt: can not find interrupt %d in ioapic\n",
> > +			TANGIER_EXT_TIMER0_MSI);
> 
> As mentioned earlier, I think it would pake sense to pass
> struct platform_device * as argument and use dev_warn.
> 
> It would also enable you to use the irq passed from the parameter
> instead of checking for the hard-coded define.

Ack.

> 
> > +		return -EINVAL;
> 
> Why not return the error returned by mp_find_ioapic() ?

My intention was to ensure in this scenario we got the wrong IRQ. But it
perhaps makes more sense to keep the original error.

Thanks again for the review.

Br, David

> 
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static struct intel_mid_wdt_pdata tangier_pdata = {
> > +	.irq = TANGIER_EXT_TIMER0_MSI,
> > +	.probe = tangier_probe,
> > +};
> > +
> > +static int __init register_mid_wdt(void)
> > +{
> > +	if (intel_mid_identify_cpu() == INTEL_MID_CPU_CHIP_TANGIER) {
> > +		wdt_dev.dev.platform_data = &tangier_pdata;
> > +		return platform_device_register(&wdt_dev);
> > +	}
> > +
> > +	return -ENODEV;
> > +}
> > +
> > +rootfs_initcall(register_mid_wdt);
> > -- 
> > 1.9.1
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 

[PATCH v4 0/2] Initial implementation of Intel MID watchdog driver

[David Cohen]

Hi,

This patch set is an initial implementation of a generic Intel MID watchdog
driver.
It currently supports Intel Merrifield only, but will extend to other SoCs of
MID family.

changes from v3 to v4:
 - Addressed comments from Guenter Roeck

Br, David

--
David Cohen (2):
  watchdog: add Intel MID watchdog driver support
  x86: intel-mid: add watchdog platform code for Merrifield

 arch/x86/platform/intel-mid/device_libs/Makefile   |   1 +
 .../platform/intel-mid/device_libs/platform_wdt.c  |  66 ++++++++
 drivers/watchdog/Kconfig                           |  13 ++
 drivers/watchdog/Makefile                          |   1 +
 drivers/watchdog/intel-mid_wdt.c                   | 184 +++++++++++++++++++++
 include/linux/platform_data/intel-mid_wdt.h        |  22 +++
 6 files changed, 287 insertions(+)
 create mode 100644 arch/x86/platform/intel-mid/device_libs/platform_wdt.c
 create mode 100644 drivers/watchdog/intel-mid_wdt.c
 create mode 100644 include/linux/platform_data/intel-mid_wdt.h

-- 
1.9.1

[PATCH v4 1/2] watchdog: add Intel MID watchdog driver support

[David Cohen]

Add initial Intel MID watchdog driver support.

This driver is an initial implementation of generic Intel MID watchdog
driver. Currently it supports Intel Merrifield platform.

Signed-off-by: Eric Ernst <eric.ernst@intel.com>
Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
---
 drivers/watchdog/Kconfig                    |  13 ++
 drivers/watchdog/Makefile                   |   1 +
 drivers/watchdog/intel-mid_wdt.c            | 184 ++++++++++++++++++++++++++++
 include/linux/platform_data/intel-mid_wdt.h |  22 ++++
 4 files changed, 220 insertions(+)
 create mode 100644 drivers/watchdog/intel-mid_wdt.c
 create mode 100644 include/linux/platform_data/intel-mid_wdt.h

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 79d25894343a..0e110c37aa5c 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -643,6 +643,19 @@ config INTEL_SCU_WATCHDOG
 
 	  To compile this driver as a module, choose M here.
 
+config INTEL_MID_WATCHDOG
+	tristate "Intel MID Watchdog Timer"
+	depends on X86_INTEL_MID
+	select WATCHDOG_CORE
+	---help---
+	  Watchdog timer driver built into the Intel SCU for Intel MID
+	  Platforms.
+
+	  This driver currently supports only the watchdog evolution
+	  implementation in SCU, available for Merrifield generation.
+
+	  To compile this driver as a module, choose M here.
+
 config ITCO_WDT
 	tristate "Intel TCO Timer/Watchdog"
 	depends on (X86 || IA64) && PCI
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 985a66cda76f..b927e7496eb4 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -113,6 +113,7 @@ obj-$(CONFIG_W83977F_WDT) += w83977f_wdt.o
 obj-$(CONFIG_MACHZ_WDT) += machzwd.o
 obj-$(CONFIG_SBC_EPX_C3_WATCHDOG) += sbc_epx_c3.o
 obj-$(CONFIG_INTEL_SCU_WATCHDOG) += intel_scu_watchdog.o
+obj-$(CONFIG_INTEL_MID_WATCHDOG) += intel-mid_wdt.o
 
 # M32R Architecture
 
diff --git a/drivers/watchdog/intel-mid_wdt.c b/drivers/watchdog/intel-mid_wdt.c
new file mode 100644
index 000000000000..ca66e8e74635
--- /dev/null
+++ b/drivers/watchdog/intel-mid_wdt.c
@@ -0,0 +1,184 @@
+/*
+ *      intel-mid_wdt: generic Intel MID SCU watchdog driver
+ *
+ *      Platforms supported so far:
+ *      - Merrifield only
+ *
+ *      Copyright (C) 2014 Intel Corporation. All rights reserved.
+ *      Contact: David Cohen <david.a.cohen@linux.intel.com>
+ *
+ *      This program is free software; you can redistribute it and/or
+ *      modify it under the terms of version 2 of the GNU General
+ *      Public License as published by the Free Software Foundation.
+ */
+
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/nmi.h>
+#include <linux/platform_device.h>
+#include <linux/watchdog.h>
+#include <linux/platform_data/intel-mid_wdt.h>
+
+#include <asm/intel_scu_ipc.h>
+#include <asm/intel-mid.h>
+
+#define IPC_WATCHDOG 0xf8
+
+#define MID_WDT_PRETIMEOUT		15
+#define MID_WDT_TIMEOUT_MIN		(1 + MID_WDT_PRETIMEOUT)
+#define MID_WDT_TIMEOUT_MAX		170
+#define MID_WDT_DEFAULT_TIMEOUT		90
+
+/* SCU watchdog messages */
+enum {
+	SCU_WATCHDOG_START = 0,
+	SCU_WATCHDOG_STOP,
+	SCU_WATCHDOG_KEEPALIVE,
+};
+
+static inline int wdt_command(int sub, u32 *in, int inlen)
+{
+	return intel_scu_ipc_command(IPC_WATCHDOG, sub, in, inlen, NULL, 0);
+}
+
+static int wdt_start(struct watchdog_device *wd)
+{
+	int ret, in_size;
+	int timeout = wd->timeout;
+	struct ipc_wd_start {
+		u32 pretimeout;
+		u32 timeout;
+	} ipc_wd_start = { timeout - MID_WDT_PRETIMEOUT, 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);
+
+	ret = wdt_command(SCU_WATCHDOG_START, (u32 *)&ipc_wd_start, in_size);
+	if (ret) {
+		struct device *dev = watchdog_get_drvdata(wd);
+		dev_crit(dev, "error starting watchdog: %d\n", ret);
+	}
+
+	return ret;
+}
+
+static int wdt_ping(struct watchdog_device *wd)
+{
+	int ret;
+
+	ret = wdt_command(SCU_WATCHDOG_KEEPALIVE, NULL, 0);
+	if (ret) {
+		struct device *dev = watchdog_get_drvdata(wd);
+		dev_crit(dev, "Error executing keepalive: 0x%x\n", ret);
+	}
+
+	return ret;
+}
+
+static int wdt_stop(struct watchdog_device *wd)
+{
+	int ret;
+
+	ret = wdt_command(SCU_WATCHDOG_STOP, NULL, 0);
+	if (ret) {
+		struct device *dev = watchdog_get_drvdata(wd);
+		dev_crit(dev, "Error stopping watchdog: 0x%x\n", ret);
+	}
+
+	return ret;
+}
+
+static irqreturn_t mid_wdt_irq(int irq, void *dev_id)
+{
+	panic("Kernel Watchdog");
+
+	/* This code should not be reached */
+	return IRQ_HANDLED;
+}
+
+static const struct watchdog_info mid_wdt_info = {
+	.identity = "Intel MID SCU watchdog",
+	.options = WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT,
+};
+
+static const struct watchdog_ops mid_wdt_ops = {
+	.owner = THIS_MODULE,
+	.start = wdt_start,
+	.stop = wdt_stop,
+	.ping = wdt_ping,
+};
+
+static int mid_wdt_probe(struct platform_device *pdev)
+{
+	struct watchdog_device *wdt_dev;
+	struct intel_mid_wdt_pdata *pdata = pdev->dev.platform_data;
+	int ret;
+
+	if (!pdata) {
+		dev_err(&pdev->dev, "missing platform data\n");
+		return -EINVAL;
+	}
+
+	if (pdata->probe) {
+		ret = pdata->probe(pdev);
+		if (ret)
+			return ret;
+	}
+
+	wdt_dev = devm_kzalloc(&pdev->dev, sizeof(*wdt_dev), GFP_KERNEL);
+	if (!wdt_dev)
+		return -ENOMEM;
+
+	wdt_dev->info = &mid_wdt_info;
+	wdt_dev->ops = &mid_wdt_ops;
+	wdt_dev->min_timeout = MID_WDT_TIMEOUT_MIN;
+	wdt_dev->max_timeout = MID_WDT_TIMEOUT_MAX;
+	wdt_dev->timeout = MID_WDT_DEFAULT_TIMEOUT;
+
+	watchdog_set_drvdata(wdt_dev, &pdev->dev);
+	platform_set_drvdata(pdev, wdt_dev);
+
+	ret = devm_request_irq(&pdev->dev, pdata->irq, mid_wdt_irq,
+			       IRQF_SHARED | IRQF_NO_SUSPEND, "watchdog",
+			       wdt_dev);
+	if (ret) {
+		dev_err(&pdev->dev, "error requesting warning irq %d\n",
+			pdata->irq);
+		return ret;
+	}
+
+	ret = watchdog_register_device(wdt_dev);
+	if (ret) {
+		dev_err(&pdev->dev, "error registering watchdog device\n");
+		return ret;
+	}
+
+	dev_info(&pdev->dev, "Intel MID watchdog device probed\n");
+
+	return 0;
+}
+
+static int mid_wdt_remove(struct platform_device *pdev)
+{
+	struct watchdog_device *wd = platform_get_drvdata(pdev);
+	watchdog_unregister_device(wd);
+	return 0;
+}
+
+static struct platform_driver mid_wdt_driver = {
+	.probe		= mid_wdt_probe,
+	.remove		= mid_wdt_remove,
+	.driver		= {
+		.owner	= THIS_MODULE,
+		.name	= "intel_mid_wdt",
+	},
+};
+
+module_platform_driver(mid_wdt_driver);
+
+MODULE_AUTHOR("David Cohen <david.a.cohen@linux.intel.com>");
+MODULE_DESCRIPTION("Watchdog Driver for Intel MID platform");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/platform_data/intel-mid_wdt.h b/include/linux/platform_data/intel-mid_wdt.h
new file mode 100644
index 000000000000..b98253466ace
--- /dev/null
+++ b/include/linux/platform_data/intel-mid_wdt.h
@@ -0,0 +1,22 @@
+/*
+ *      intel-mid_wdt: generic Intel MID SCU watchdog driver
+ *
+ *      Copyright (C) 2014 Intel Corporation. All rights reserved.
+ *      Contact: David Cohen <david.a.cohen@linux.intel.com>
+ *
+ *      This program is free software; you can redistribute it and/or
+ *      modify it under the terms of version 2 of the GNU General
+ *      Public License as published by the Free Software Foundation.
+ */
+
+#ifndef __INTEL_MID_WDT_H__
+#define __INTEL_MID_WDT_H__
+
+#include <linux/platform_device.h>
+
+struct intel_mid_wdt_pdata {
+	int irq;
+	int (*probe)(struct platform_device *pdev);
+};
+
+#endif /*__INTEL_MID_WDT_H__*/
-- 
1.9.1

[PATCH v4 2/2] x86: intel-mid: add watchdog platform code for Merrifield

[David Cohen]

This patch adds platform code for Intel Merrifield.
Since the watchdog is not part of SFI table, we have no other option but
to manually register watchdog's platform device (argh!).

Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
---
 arch/x86/platform/intel-mid/device_libs/Makefile   |  1 +
 .../platform/intel-mid/device_libs/platform_wdt.c  | 66 ++++++++++++++++++++++
 2 files changed, 67 insertions(+)
 create mode 100644 arch/x86/platform/intel-mid/device_libs/platform_wdt.c

diff --git a/arch/x86/platform/intel-mid/device_libs/Makefile b/arch/x86/platform/intel-mid/device_libs/Makefile
index 097e7a7940d8..af9307f2cc28 100644
--- a/arch/x86/platform/intel-mid/device_libs/Makefile
+++ b/arch/x86/platform/intel-mid/device_libs/Makefile
@@ -20,3 +20,4 @@ obj-$(subst m,y,$(CONFIG_DRM_MEDFIELD)) += platform_tc35876x.o
 obj-$(subst m,y,$(CONFIG_SERIAL_MRST_MAX3110)) += platform_max3111.o
 # MISC Devices
 obj-$(subst m,y,$(CONFIG_KEYBOARD_GPIO)) += platform_gpio_keys.o
+obj-$(subst m,y,$(CONFIG_INTEL_MID_WATCHDOG)) += platform_wdt.o
diff --git a/arch/x86/platform/intel-mid/device_libs/platform_wdt.c b/arch/x86/platform/intel-mid/device_libs/platform_wdt.c
new file mode 100644
index 000000000000..4b0434b54aff
--- /dev/null
+++ b/arch/x86/platform/intel-mid/device_libs/platform_wdt.c
@@ -0,0 +1,66 @@
+/*
+ * platform_wdt.c: Watchdog platform library file
+ *
+ * (C) Copyright 2014 Intel Corporation
+ * Author: David Cohen <david.a.cohen@linux.intel.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; version 2
+ * of the License.
+ */
+
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/platform_data/intel-mid_wdt.h>
+#include <asm/intel-mid.h>
+
+#define TANGIER_EXT_TIMER0_MSI 15
+
+static struct platform_device wdt_dev = {
+	.name = "intel_mid_wdt",
+	.id = -1,
+};
+
+static int tangier_probe(struct platform_device *pdev)
+{
+	int ioapic;
+	struct io_apic_irq_attr irq_attr = { 0 };
+
+	ioapic = mp_find_ioapic(TANGIER_EXT_TIMER0_MSI);
+	if (ioapic >= 0) {
+		int ret;
+		irq_attr.ioapic = ioapic;
+		irq_attr.ioapic_pin = TANGIER_EXT_TIMER0_MSI;
+		irq_attr.trigger = 1;
+		irq_attr.polarity = 0; /* Active high */
+		ret = io_apic_set_pci_routing(NULL, TANGIER_EXT_TIMER0_MSI,
+					      &irq_attr);
+		if (ret)
+			return ret;
+	} else {
+		dev_warn(&pdev->dev, "cannot find interrupt %d in ioapic\n",
+			 TANGIER_EXT_TIMER0_MSI);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static struct intel_mid_wdt_pdata tangier_pdata = {
+	.irq = TANGIER_EXT_TIMER0_MSI,
+	.probe = tangier_probe,
+};
+
+static int __init register_mid_wdt(void)
+{
+	if (intel_mid_identify_cpu() == INTEL_MID_CPU_CHIP_TANGIER) {
+		wdt_dev.dev.platform_data = &tangier_pdata;
+		return platform_device_register(&wdt_dev);
+	}
+
+	return -ENODEV;
+}
+
+rootfs_initcall(register_mid_wdt);
-- 
1.9.1

Re: [PATCH v4 2/2] x86: intel-mid: add watchdog platform code for Merrifield

[David Cohen]

On Tue, Apr 15, 2014 at 01:06:06PM -0700, David Cohen wrote:
> This patch adds platform code for Intel Merrifield.
> Since the watchdog is not part of SFI table, we have no other option but
> to manually register watchdog's platform device (argh!).
> 
> Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
> ---
>  arch/x86/platform/intel-mid/device_libs/Makefile   |  1 +
>  .../platform/intel-mid/device_libs/platform_wdt.c  | 66 ++++++++++++++++++++++
>  2 files changed, 67 insertions(+)
>  create mode 100644 arch/x86/platform/intel-mid/device_libs/platform_wdt.c
> 
> diff --git a/arch/x86/platform/intel-mid/device_libs/Makefile b/arch/x86/platform/intel-mid/device_libs/Makefile
> index 097e7a7940d8..af9307f2cc28 100644
> --- a/arch/x86/platform/intel-mid/device_libs/Makefile
> +++ b/arch/x86/platform/intel-mid/device_libs/Makefile
> @@ -20,3 +20,4 @@ obj-$(subst m,y,$(CONFIG_DRM_MEDFIELD)) += platform_tc35876x.o
>  obj-$(subst m,y,$(CONFIG_SERIAL_MRST_MAX3110)) += platform_max3111.o
>  # MISC Devices
>  obj-$(subst m,y,$(CONFIG_KEYBOARD_GPIO)) += platform_gpio_keys.o
> +obj-$(subst m,y,$(CONFIG_INTEL_MID_WATCHDOG)) += platform_wdt.o
> diff --git a/arch/x86/platform/intel-mid/device_libs/platform_wdt.c b/arch/x86/platform/intel-mid/device_libs/platform_wdt.c
> new file mode 100644
> index 000000000000..4b0434b54aff
> --- /dev/null
> +++ b/arch/x86/platform/intel-mid/device_libs/platform_wdt.c
> @@ -0,0 +1,66 @@
> +/*
> + * platform_wdt.c: Watchdog platform library file
> + *
> + * (C) Copyright 2014 Intel Corporation
> + * Author: David Cohen <david.a.cohen@linux.intel.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; version 2
> + * of the License.
> + */
> +
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/platform_device.h>
> +#include <linux/platform_data/intel-mid_wdt.h>
> +#include <asm/intel-mid.h>
> +
> +#define TANGIER_EXT_TIMER0_MSI 15
> +
> +static struct platform_device wdt_dev = {
> +	.name = "intel_mid_wdt",
> +	.id = -1,
> +};
> +
> +static int tangier_probe(struct platform_device *pdev)
> +{
> +	int ioapic;
> +	struct io_apic_irq_attr irq_attr = { 0 };
> +
> +	ioapic = mp_find_ioapic(TANGIER_EXT_TIMER0_MSI);

Forgot remove this hardcoded irq number. I'll send v4.1 of this patch.

BR, David

> +	if (ioapic >= 0) {
> +		int ret;
> +		irq_attr.ioapic = ioapic;
> +		irq_attr.ioapic_pin = TANGIER_EXT_TIMER0_MSI;
> +		irq_attr.trigger = 1;
> +		irq_attr.polarity = 0; /* Active high */
> +		ret = io_apic_set_pci_routing(NULL, TANGIER_EXT_TIMER0_MSI,
> +					      &irq_attr);
> +		if (ret)
> +			return ret;
> +	} else {
> +		dev_warn(&pdev->dev, "cannot find interrupt %d in ioapic\n",
> +			 TANGIER_EXT_TIMER0_MSI);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static struct intel_mid_wdt_pdata tangier_pdata = {
> +	.irq = TANGIER_EXT_TIMER0_MSI,
> +	.probe = tangier_probe,
> +};
> +
> +static int __init register_mid_wdt(void)
> +{
> +	if (intel_mid_identify_cpu() == INTEL_MID_CPU_CHIP_TANGIER) {
> +		wdt_dev.dev.platform_data = &tangier_pdata;
> +		return platform_device_register(&wdt_dev);
> +	}
> +
> +	return -ENODEV;
> +}
> +
> +rootfs_initcall(register_mid_wdt);
> -- 
> 1.9.1
> 

Re: [PATCH v4 2/2] x86: intel-mid: add watchdog platform code for Merrifield

[Guenter Roeck]

On Tue, Apr 15, 2014 at 01:09:00PM -0700, David Cohen wrote:
> On Tue, Apr 15, 2014 at 01:06:06PM -0700, David Cohen wrote:
> > This patch adds platform code for Intel Merrifield.
> > Since the watchdog is not part of SFI table, we have no other option but
> > to manually register watchdog's platform device (argh!).
> > 
> > Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
> > ---
> >  arch/x86/platform/intel-mid/device_libs/Makefile   |  1 +
> >  .../platform/intel-mid/device_libs/platform_wdt.c  | 66 ++++++++++++++++++++++
> >  2 files changed, 67 insertions(+)
> >  create mode 100644 arch/x86/platform/intel-mid/device_libs/platform_wdt.c
> > 
> > diff --git a/arch/x86/platform/intel-mid/device_libs/Makefile b/arch/x86/platform/intel-mid/device_libs/Makefile
> > index 097e7a7940d8..af9307f2cc28 100644
> > --- a/arch/x86/platform/intel-mid/device_libs/Makefile
> > +++ b/arch/x86/platform/intel-mid/device_libs/Makefile
> > @@ -20,3 +20,4 @@ obj-$(subst m,y,$(CONFIG_DRM_MEDFIELD)) += platform_tc35876x.o
> >  obj-$(subst m,y,$(CONFIG_SERIAL_MRST_MAX3110)) += platform_max3111.o
> >  # MISC Devices
> >  obj-$(subst m,y,$(CONFIG_KEYBOARD_GPIO)) += platform_gpio_keys.o
> > +obj-$(subst m,y,$(CONFIG_INTEL_MID_WATCHDOG)) += platform_wdt.o
> > diff --git a/arch/x86/platform/intel-mid/device_libs/platform_wdt.c b/arch/x86/platform/intel-mid/device_libs/platform_wdt.c
> > new file mode 100644
> > index 000000000000..4b0434b54aff
> > --- /dev/null
> > +++ b/arch/x86/platform/intel-mid/device_libs/platform_wdt.c
> > @@ -0,0 +1,66 @@
> > +/*
> > + * platform_wdt.c: Watchdog platform library file
> > + *
> > + * (C) Copyright 2014 Intel Corporation
> > + * Author: David Cohen <david.a.cohen@linux.intel.com>
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License
> > + * as published by the Free Software Foundation; version 2
> > + * of the License.
> > + */
> > +
> > +#include <linux/init.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/platform_data/intel-mid_wdt.h>
> > +#include <asm/intel-mid.h>
> > +
> > +#define TANGIER_EXT_TIMER0_MSI 15
> > +
> > +static struct platform_device wdt_dev = {
> > +	.name = "intel_mid_wdt",
> > +	.id = -1,
> > +};
> > +
> > +static int tangier_probe(struct platform_device *pdev)
> > +{
> > +	int ioapic;
> > +	struct io_apic_irq_attr irq_attr = { 0 };
> > +
> > +	ioapic = mp_find_ioapic(TANGIER_EXT_TIMER0_MSI);
> 
> Forgot remove this hardcoded irq number. I'll send v4.1 of this patch.
> 
> BR, David
> 
> > +	if (ioapic >= 0) {
> > +		int ret;
> > +		irq_attr.ioapic = ioapic;
> > +		irq_attr.ioapic_pin = TANGIER_EXT_TIMER0_MSI;
> > +		irq_attr.trigger = 1;
> > +		irq_attr.polarity = 0; /* Active high */
> > +		ret = io_apic_set_pci_routing(NULL, TANGIER_EXT_TIMER0_MSI,
> > +					      &irq_attr);

Here is another one, just in case you missed it.

Guenter

Re: [PATCH v4 2/2] x86: intel-mid: add watchdog platform code for Merrifield

[David Cohen]

On Tue, Apr 15, 2014 at 02:13:49PM -0700, Guenter Roeck wrote:
> On Tue, Apr 15, 2014 at 01:09:00PM -0700, David Cohen wrote:
> > On Tue, Apr 15, 2014 at 01:06:06PM -0700, David Cohen wrote:
> > > This patch adds platform code for Intel Merrifield.
> > > Since the watchdog is not part of SFI table, we have no other option but
> > > to manually register watchdog's platform device (argh!).
> > > 
> > > Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
> > > ---
> > >  arch/x86/platform/intel-mid/device_libs/Makefile   |  1 +
> > >  .../platform/intel-mid/device_libs/platform_wdt.c  | 66 ++++++++++++++++++++++
> > >  2 files changed, 67 insertions(+)
> > >  create mode 100644 arch/x86/platform/intel-mid/device_libs/platform_wdt.c
> > > 
> > > diff --git a/arch/x86/platform/intel-mid/device_libs/Makefile b/arch/x86/platform/intel-mid/device_libs/Makefile
> > > index 097e7a7940d8..af9307f2cc28 100644
> > > --- a/arch/x86/platform/intel-mid/device_libs/Makefile
> > > +++ b/arch/x86/platform/intel-mid/device_libs/Makefile
> > > @@ -20,3 +20,4 @@ obj-$(subst m,y,$(CONFIG_DRM_MEDFIELD)) += platform_tc35876x.o
> > >  obj-$(subst m,y,$(CONFIG_SERIAL_MRST_MAX3110)) += platform_max3111.o
> > >  # MISC Devices
> > >  obj-$(subst m,y,$(CONFIG_KEYBOARD_GPIO)) += platform_gpio_keys.o
> > > +obj-$(subst m,y,$(CONFIG_INTEL_MID_WATCHDOG)) += platform_wdt.o
> > > diff --git a/arch/x86/platform/intel-mid/device_libs/platform_wdt.c b/arch/x86/platform/intel-mid/device_libs/platform_wdt.c
> > > new file mode 100644
> > > index 000000000000..4b0434b54aff
> > > --- /dev/null
> > > +++ b/arch/x86/platform/intel-mid/device_libs/platform_wdt.c
> > > @@ -0,0 +1,66 @@
> > > +/*
> > > + * platform_wdt.c: Watchdog platform library file
> > > + *
> > > + * (C) Copyright 2014 Intel Corporation
> > > + * Author: David Cohen <david.a.cohen@linux.intel.com>
> > > + *
> > > + * This program is free software; you can redistribute it and/or
> > > + * modify it under the terms of the GNU General Public License
> > > + * as published by the Free Software Foundation; version 2
> > > + * of the License.
> > > + */
> > > +
> > > +#include <linux/init.h>
> > > +#include <linux/interrupt.h>
> > > +#include <linux/platform_device.h>
> > > +#include <linux/platform_data/intel-mid_wdt.h>
> > > +#include <asm/intel-mid.h>
> > > +
> > > +#define TANGIER_EXT_TIMER0_MSI 15
> > > +
> > > +static struct platform_device wdt_dev = {
> > > +	.name = "intel_mid_wdt",
> > > +	.id = -1,
> > > +};
> > > +
> > > +static int tangier_probe(struct platform_device *pdev)
> > > +{
> > > +	int ioapic;
> > > +	struct io_apic_irq_attr irq_attr = { 0 };
> > > +
> > > +	ioapic = mp_find_ioapic(TANGIER_EXT_TIMER0_MSI);
> > 
> > Forgot remove this hardcoded irq number. I'll send v4.1 of this patch.
> > 
> > BR, David
> > 
> > > +	if (ioapic >= 0) {
> > > +		int ret;
> > > +		irq_attr.ioapic = ioapic;
> > > +		irq_attr.ioapic_pin = TANGIER_EXT_TIMER0_MSI;
> > > +		irq_attr.trigger = 1;
> > > +		irq_attr.polarity = 0; /* Active high */
> > > +		ret = io_apic_set_pci_routing(NULL, TANGIER_EXT_TIMER0_MSI,
> > > +					      &irq_attr);
> 
> Here is another one, just in case you missed it.

This one I didn't miss. I checked and it returns dummy error code (-1).
Better to keep the hardcoded value.

Br, David

> 
> Guenter

Re: [PATCH v4 2/2] x86: intel-mid: add watchdog platform code for Merrifield

[Guenter Roeck]

On Tue, Apr 15, 2014 at 02:17:22PM -0700, David Cohen wrote:
> On Tue, Apr 15, 2014 at 02:13:49PM -0700, Guenter Roeck wrote:
> > On Tue, Apr 15, 2014 at 01:09:00PM -0700, David Cohen wrote:
> > > On Tue, Apr 15, 2014 at 01:06:06PM -0700, David Cohen wrote:
> > > > This patch adds platform code for Intel Merrifield.
> > > > Since the watchdog is not part of SFI table, we have no other option but
> > > > to manually register watchdog's platform device (argh!).
> > > > 
> > > > Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
> > > > ---
> > > >  arch/x86/platform/intel-mid/device_libs/Makefile   |  1 +
> > > >  .../platform/intel-mid/device_libs/platform_wdt.c  | 66 ++++++++++++++++++++++
> > > >  2 files changed, 67 insertions(+)
> > > >  create mode 100644 arch/x86/platform/intel-mid/device_libs/platform_wdt.c
> > > > 
> > > > diff --git a/arch/x86/platform/intel-mid/device_libs/Makefile b/arch/x86/platform/intel-mid/device_libs/Makefile
> > > > index 097e7a7940d8..af9307f2cc28 100644
> > > > --- a/arch/x86/platform/intel-mid/device_libs/Makefile
> > > > +++ b/arch/x86/platform/intel-mid/device_libs/Makefile
> > > > @@ -20,3 +20,4 @@ obj-$(subst m,y,$(CONFIG_DRM_MEDFIELD)) += platform_tc35876x.o
> > > >  obj-$(subst m,y,$(CONFIG_SERIAL_MRST_MAX3110)) += platform_max3111.o
> > > >  # MISC Devices
> > > >  obj-$(subst m,y,$(CONFIG_KEYBOARD_GPIO)) += platform_gpio_keys.o
> > > > +obj-$(subst m,y,$(CONFIG_INTEL_MID_WATCHDOG)) += platform_wdt.o
> > > > diff --git a/arch/x86/platform/intel-mid/device_libs/platform_wdt.c b/arch/x86/platform/intel-mid/device_libs/platform_wdt.c
> > > > new file mode 100644
> > > > index 000000000000..4b0434b54aff
> > > > --- /dev/null
> > > > +++ b/arch/x86/platform/intel-mid/device_libs/platform_wdt.c
> > > > @@ -0,0 +1,66 @@
> > > > +/*
> > > > + * platform_wdt.c: Watchdog platform library file
> > > > + *
> > > > + * (C) Copyright 2014 Intel Corporation
> > > > + * Author: David Cohen <david.a.cohen@linux.intel.com>
> > > > + *
> > > > + * This program is free software; you can redistribute it and/or
> > > > + * modify it under the terms of the GNU General Public License
> > > > + * as published by the Free Software Foundation; version 2
> > > > + * of the License.
> > > > + */
> > > > +
> > > > +#include <linux/init.h>
> > > > +#include <linux/interrupt.h>
> > > > +#include <linux/platform_device.h>
> > > > +#include <linux/platform_data/intel-mid_wdt.h>
> > > > +#include <asm/intel-mid.h>
> > > > +
> > > > +#define TANGIER_EXT_TIMER0_MSI 15
> > > > +
> > > > +static struct platform_device wdt_dev = {
> > > > +	.name = "intel_mid_wdt",
> > > > +	.id = -1,
> > > > +};
> > > > +
> > > > +static int tangier_probe(struct platform_device *pdev)
> > > > +{
> > > > +	int ioapic;
> > > > +	struct io_apic_irq_attr irq_attr = { 0 };
> > > > +
> > > > +	ioapic = mp_find_ioapic(TANGIER_EXT_TIMER0_MSI);
> > > 
> > > Forgot remove this hardcoded irq number. I'll send v4.1 of this patch.
> > > 
> > > BR, David
> > > 
> > > > +	if (ioapic >= 0) {
> > > > +		int ret;
> > > > +		irq_attr.ioapic = ioapic;
> > > > +		irq_attr.ioapic_pin = TANGIER_EXT_TIMER0_MSI;
> > > > +		irq_attr.trigger = 1;
> > > > +		irq_attr.polarity = 0; /* Active high */
> > > > +		ret = io_apic_set_pci_routing(NULL, TANGIER_EXT_TIMER0_MSI,
> > > > +					      &irq_attr);
> > 
> > Here is another one, just in case you missed it.
> 
> This one I didn't miss. I checked and it returns dummy error code (-1).
> Better to keep the hardcoded value.
> 
Thanks for letting me know (guess the idea is for mp_find_ioapic to return
"invalid irq"), but I meant the second hard-coded irq....

Guenter

Re: [PATCH v4 2/2] x86: intel-mid: add watchdog platform code for Merrifield

[David Cohen]

On Tue, Apr 15, 2014 at 02:35:51PM -0700, Guenter Roeck wrote:
> On Tue, Apr 15, 2014 at 02:17:22PM -0700, David Cohen wrote:
> > On Tue, Apr 15, 2014 at 02:13:49PM -0700, Guenter Roeck wrote:
> > > On Tue, Apr 15, 2014 at 01:09:00PM -0700, David Cohen wrote:
> > > > On Tue, Apr 15, 2014 at 01:06:06PM -0700, David Cohen wrote:
> > > > > This patch adds platform code for Intel Merrifield.
> > > > > Since the watchdog is not part of SFI table, we have no other option but
> > > > > to manually register watchdog's platform device (argh!).
> > > > > 
> > > > > Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
> > > > > ---
> > > > >  arch/x86/platform/intel-mid/device_libs/Makefile   |  1 +
> > > > >  .../platform/intel-mid/device_libs/platform_wdt.c  | 66 ++++++++++++++++++++++
> > > > >  2 files changed, 67 insertions(+)
> > > > >  create mode 100644 arch/x86/platform/intel-mid/device_libs/platform_wdt.c
> > > > > 
> > > > > diff --git a/arch/x86/platform/intel-mid/device_libs/Makefile b/arch/x86/platform/intel-mid/device_libs/Makefile
> > > > > index 097e7a7940d8..af9307f2cc28 100644
> > > > > --- a/arch/x86/platform/intel-mid/device_libs/Makefile
> > > > > +++ b/arch/x86/platform/intel-mid/device_libs/Makefile
> > > > > @@ -20,3 +20,4 @@ obj-$(subst m,y,$(CONFIG_DRM_MEDFIELD)) += platform_tc35876x.o
> > > > >  obj-$(subst m,y,$(CONFIG_SERIAL_MRST_MAX3110)) += platform_max3111.o
> > > > >  # MISC Devices
> > > > >  obj-$(subst m,y,$(CONFIG_KEYBOARD_GPIO)) += platform_gpio_keys.o
> > > > > +obj-$(subst m,y,$(CONFIG_INTEL_MID_WATCHDOG)) += platform_wdt.o
> > > > > diff --git a/arch/x86/platform/intel-mid/device_libs/platform_wdt.c b/arch/x86/platform/intel-mid/device_libs/platform_wdt.c
> > > > > new file mode 100644
> > > > > index 000000000000..4b0434b54aff
> > > > > --- /dev/null
> > > > > +++ b/arch/x86/platform/intel-mid/device_libs/platform_wdt.c
> > > > > @@ -0,0 +1,66 @@
> > > > > +/*
> > > > > + * platform_wdt.c: Watchdog platform library file
> > > > > + *
> > > > > + * (C) Copyright 2014 Intel Corporation
> > > > > + * Author: David Cohen <david.a.cohen@linux.intel.com>
> > > > > + *
> > > > > + * This program is free software; you can redistribute it and/or
> > > > > + * modify it under the terms of the GNU General Public License
> > > > > + * as published by the Free Software Foundation; version 2
> > > > > + * of the License.
> > > > > + */
> > > > > +
> > > > > +#include <linux/init.h>
> > > > > +#include <linux/interrupt.h>
> > > > > +#include <linux/platform_device.h>
> > > > > +#include <linux/platform_data/intel-mid_wdt.h>
> > > > > +#include <asm/intel-mid.h>
> > > > > +
> > > > > +#define TANGIER_EXT_TIMER0_MSI 15
> > > > > +
> > > > > +static struct platform_device wdt_dev = {
> > > > > +	.name = "intel_mid_wdt",
> > > > > +	.id = -1,
> > > > > +};
> > > > > +
> > > > > +static int tangier_probe(struct platform_device *pdev)
> > > > > +{
> > > > > +	int ioapic;
> > > > > +	struct io_apic_irq_attr irq_attr = { 0 };
> > > > > +
> > > > > +	ioapic = mp_find_ioapic(TANGIER_EXT_TIMER0_MSI);
> > > > 
> > > > Forgot remove this hardcoded irq number. I'll send v4.1 of this patch.
> > > > 
> > > > BR, David
> > > > 
> > > > > +	if (ioapic >= 0) {
> > > > > +		int ret;
> > > > > +		irq_attr.ioapic = ioapic;
> > > > > +		irq_attr.ioapic_pin = TANGIER_EXT_TIMER0_MSI;
> > > > > +		irq_attr.trigger = 1;
> > > > > +		irq_attr.polarity = 0; /* Active high */
> > > > > +		ret = io_apic_set_pci_routing(NULL, TANGIER_EXT_TIMER0_MSI,
> > > > > +					      &irq_attr);
> > > 
> > > Here is another one, just in case you missed it.
> > 
> > This one I didn't miss. I checked and it returns dummy error code (-1).
> > Better to keep the hardcoded value.
> > 
> Thanks for letting me know (guess the idea is for mp_find_ioapic to return
> "invalid irq"), but I meant the second hard-coded irq....

Hm :)
I am aware about that one. I just tried to be a bit lazy when replying.

Thanks,

David

> 
> Guenter

[PATCH v4.1 2/2] x86: intel-mid: add watchdog platform code for Merrifield

[David Cohen]

This patch adds platform code for Intel Merrifield.
Since the watchdog is not part of SFI table, we have no other option but
to manually register watchdog's platform device (argh!).

Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
---

changes from v4 to v4.1:
 - dropped hardcoded irq value on mrfl_probe() function in favor or pdata's irq
   value.
--
 arch/x86/platform/intel-mid/device_libs/Makefile   |  1 +
 .../platform/intel-mid/device_libs/platform_wdt.c  | 71 ++++++++++++++++++++++
 2 files changed, 72 insertions(+)
 create mode 100644 arch/x86/platform/intel-mid/device_libs/platform_wdt.c

diff --git a/arch/x86/platform/intel-mid/device_libs/Makefile b/arch/x86/platform/intel-mid/device_libs/Makefile
index 097e7a7940d8..af9307f2cc28 100644
--- a/arch/x86/platform/intel-mid/device_libs/Makefile
+++ b/arch/x86/platform/intel-mid/device_libs/Makefile
@@ -20,3 +20,4 @@ obj-$(subst m,y,$(CONFIG_DRM_MEDFIELD)) += platform_tc35876x.o
 obj-$(subst m,y,$(CONFIG_SERIAL_MRST_MAX3110)) += platform_max3111.o
 # MISC Devices
 obj-$(subst m,y,$(CONFIG_KEYBOARD_GPIO)) += platform_gpio_keys.o
+obj-$(subst m,y,$(CONFIG_INTEL_MID_WATCHDOG)) += platform_wdt.o
diff --git a/arch/x86/platform/intel-mid/device_libs/platform_wdt.c b/arch/x86/platform/intel-mid/device_libs/platform_wdt.c
new file mode 100644
index 000000000000..a05315eb052d
--- /dev/null
+++ b/arch/x86/platform/intel-mid/device_libs/platform_wdt.c
@@ -0,0 +1,71 @@
+/*
+ * platform_wdt.c: Watchdog platform library file
+ *
+ * (C) Copyright 2014 Intel Corporation
+ * Author: David Cohen <david.a.cohen@linux.intel.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; version 2
+ * of the License.
+ */
+
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/platform_data/intel-mid_wdt.h>
+#include <asm/intel-mid.h>
+
+#define TANGIER_EXT_TIMER0_MSI 15
+
+static struct platform_device wdt_dev = {
+	.name = "intel_mid_wdt",
+	.id = -1,
+};
+
+static int tangier_probe(struct platform_device *pdev)
+{
+	int ioapic;
+	int irq;
+	struct intel_mid_wdt_pdata *pdata = pdev->dev.platform_data;
+	struct io_apic_irq_attr irq_attr = { 0 };
+
+	if (!pdata)
+		return -EINVAL;
+
+	irq = pdata->irq;
+	ioapic = mp_find_ioapic(irq);
+	if (ioapic >= 0) {
+		int ret;
+		irq_attr.ioapic = ioapic;
+		irq_attr.ioapic_pin = irq;
+		irq_attr.trigger = 1;
+		/* irq_attr.polarity = 0; -> Active high */
+		ret = io_apic_set_pci_routing(NULL, irq, &irq_attr);
+		if (ret)
+			return ret;
+	} else {
+		dev_warn(&pdev->dev, "cannot find interrupt %d in ioapic\n",
+			 irq);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static struct intel_mid_wdt_pdata tangier_pdata = {
+	.irq = TANGIER_EXT_TIMER0_MSI,
+	.probe = tangier_probe,
+};
+
+static int __init register_mid_wdt(void)
+{
+	if (intel_mid_identify_cpu() == INTEL_MID_CPU_CHIP_TANGIER) {
+		wdt_dev.dev.platform_data = &tangier_pdata;
+		return platform_device_register(&wdt_dev);
+	}
+
+	return -ENODEV;
+}
+
+rootfs_initcall(register_mid_wdt);
-- 
1.9.1

Re: [PATCH v4 1/2] watchdog: add Intel MID watchdog driver support

[Guenter Roeck]

On 04/15/2014 01:06 PM, David Cohen wrote:
> Add initial Intel MID watchdog driver support.
>
> This driver is an initial implementation of generic Intel MID watchdog
> driver. Currently it supports Intel Merrifield platform.
>
> Signed-off-by: Eric Ernst <eric.ernst@intel.com>
> Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

Re: [PATCH v4.1 2/2] x86: intel-mid: add watchdog platform code for Merrifield

[Guenter Roeck]

On 04/15/2014 03:20 PM, David Cohen wrote:
> This patch adds platform code for Intel Merrifield.
> Since the watchdog is not part of SFI table, we have no other option but
> to manually register watchdog's platform device (argh!).
>
> Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>