[PATCH] watchdog: mpcore: Add DT bindings for ARM mpcore watchdog

[PATCH] watchdog: mpcore: Add DT bindings for ARM mpcore watchdog

[Viresh Kumar]

This patch adds Device tree bindings for ARM Mpcore watchdog. It also updates
documentation for bindings.

Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
---
 .../bindings/watchdog/arm-mpcore-wdt.txt           |   19 +++++++++++++++++++
 drivers/watchdog/mpcore_wdt.c                      |   10 ++++++++++
 2 files changed, 29 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/watchdog/arm-mpcore-wdt.txt

diff --git a/Documentation/devicetree/bindings/watchdog/arm-mpcore-wdt.txt b/Documentation/devicetree/bindings/watchdog/arm-mpcore-wdt.txt
new file mode 100644
index 0000000..27a71aa
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/arm-mpcore-wdt.txt
@@ -0,0 +1,19 @@
+* ARM MPCORE WATCHDOG
+
+Required properties:
+- compatible : "arm,mpcore-wdt"
+- reg : Address range of the wdt registers
+
+Optional Properties:
+- interrupt-parent: Should be the phandle for the interrupt controller
+  that services interrupts for this device
+- interrupt: Should contain the wdt interrupt number
+
+Example:
+
+	wdt@ec800620 {
+		compatible = "arm,mpcore-wdt";
+		reg = <0xec800620 0x1000>;
+		interrupt-parent = <&gic>;
+		interrupts = <1 14 0x301>;
+	};
diff --git a/drivers/watchdog/mpcore_wdt.c b/drivers/watchdog/mpcore_wdt.c
index 7c00455..f82625f 100644
--- a/drivers/watchdog/mpcore_wdt.c
+++ b/drivers/watchdog/mpcore_wdt.c
@@ -27,6 +27,7 @@
 #include <linux/io.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
+#include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/pm.h>
 #include <linux/reboot.h>
@@ -402,6 +403,14 @@ static SIMPLE_DEV_PM_OPS(mpcore_wdt_dev_pm_ops, mpcore_wdt_suspend,
 /* work with hotplug and coldplug */
 MODULE_ALIAS("platform:mpcore_wdt");
 
+#ifdef CONFIG_OF
+static const struct of_device_id mpcore_wdt_id_table[] = {
+	{ .compatible = "arm,mpcore-wdt" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, mpcore_wdt_id_table);
+#endif
+
 static struct platform_driver mpcore_wdt_driver = {
 	.probe		= mpcore_wdt_probe,
 	.remove		= __devexit_p(mpcore_wdt_remove),
@@ -410,6 +419,7 @@ static struct platform_driver mpcore_wdt_driver = {
 		.owner	= THIS_MODULE,
 		.name	= "mpcore_wdt",
 		.pm	= &mpcore_wdt_dev_pm_ops,
+		.of_match_table = of_match_ptr(mpcore_wdt_id_table),
 	},
 };
 
-- 
1.7.9

[PATCH] watchdog: mpcore: Add DT bindings for ARM mpcore watchdog

[Viresh Kumar]

This patch adds Device tree bindings for ARM Mpcore watchdog. It also updates
documentation for bindings.

Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
---
 .../bindings/watchdog/arm-mpcore-wdt.txt           |   19 +++++++++++++++++++
 drivers/watchdog/mpcore_wdt.c                      |   10 ++++++++++
 2 files changed, 29 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/watchdog/arm-mpcore-wdt.txt

diff --git a/Documentation/devicetree/bindings/watchdog/arm-mpcore-wdt.txt b/Documentation/devicetree/bindings/watchdog/arm-mpcore-wdt.txt
new file mode 100644
index 0000000..27a71aa
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/arm-mpcore-wdt.txt
@@ -0,0 +1,19 @@
+* ARM MPCORE WATCHDOG
+
+Required properties:
+- compatible : "arm,mpcore-wdt"
+- reg : Address range of the wdt registers
+
+Optional Properties:
+- interrupt-parent: Should be the phandle for the interrupt controller
+  that services interrupts for this device
+- interrupt: Should contain the wdt interrupt number
+
+Example:
+
+	wdt at ec800620 {
+		compatible = "arm,mpcore-wdt";
+		reg = <0xec800620 0x1000>;
+		interrupt-parent = <&gic>;
+		interrupts = <1 14 0x301>;
+	};
diff --git a/drivers/watchdog/mpcore_wdt.c b/drivers/watchdog/mpcore_wdt.c
index 7c00455..f82625f 100644
--- a/drivers/watchdog/mpcore_wdt.c
+++ b/drivers/watchdog/mpcore_wdt.c
@@ -27,6 +27,7 @@
 #include <linux/io.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
+#include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/pm.h>
 #include <linux/reboot.h>
@@ -402,6 +403,14 @@ static SIMPLE_DEV_PM_OPS(mpcore_wdt_dev_pm_ops, mpcore_wdt_suspend,
 /* work with hotplug and coldplug */
 MODULE_ALIAS("platform:mpcore_wdt");
 
+#ifdef CONFIG_OF
+static const struct of_device_id mpcore_wdt_id_table[] = {
+	{ .compatible = "arm,mpcore-wdt" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, mpcore_wdt_id_table);
+#endif
+
 static struct platform_driver mpcore_wdt_driver = {
 	.probe		= mpcore_wdt_probe,
 	.remove		= __devexit_p(mpcore_wdt_remove),
@@ -410,6 +419,7 @@ static struct platform_driver mpcore_wdt_driver = {
 		.owner	= THIS_MODULE,
 		.name	= "mpcore_wdt",
 		.pm	= &mpcore_wdt_dev_pm_ops,
+		.of_match_table = of_match_ptr(mpcore_wdt_id_table),
 	},
 };
 
-- 
1.7.9

Re: [PATCH] watchdog: mpcore: Add DT bindings for ARM mpcore watchdog

[Pawel Moll]

Morning,

Dnia 2012-04-20, pią o godzinie 20:12 +0530, Viresh Kumar pisze:
> This patch adds Device tree bindings for ARM Mpcore watchdog. It also updates
> documentation for bindings.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
> ---
>  .../bindings/watchdog/arm-mpcore-wdt.txt           |   19 +++++++++++++++++++
>  drivers/watchdog/mpcore_wdt.c                      |   10 ++++++++++

Bindings for the MPCore TWD already exist in the tree:

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=d8e0364364d333feb4564bb7d7d983182b34427e

I've Cc-ed Marc Zyngier, who is the author of the patch.

Regards

Pawel

-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium.  Thank you.

[PATCH] watchdog: mpcore: Add DT bindings for ARM mpcore watchdog

[Pawel Moll]

Morning,

Dnia 2012-04-20, pi? o godzinie 20:12 +0530, Viresh Kumar pisze:
> This patch adds Device tree bindings for ARM Mpcore watchdog. It also updates
> documentation for bindings.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
> ---
>  .../bindings/watchdog/arm-mpcore-wdt.txt           |   19 +++++++++++++++++++
>  drivers/watchdog/mpcore_wdt.c                      |   10 ++++++++++

Bindings for the MPCore TWD already exist in the tree:

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=d8e0364364d333feb4564bb7d7d983182b34427e

I've Cc-ed Marc Zyngier, who is the author of the patch.

Regards

Pawel

-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium.  Thank you.

[PATCH V2] watchdog: mpcore: Add DT probing support for ARM mpcore watchdog

[Viresh Kumar]

This patch adds Device tree probing support for ARM Mpcore watchdog. Its binding
were already documented in Documentation/devicetree/bindings/arm/twd.txt.

Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
---
V1->V2:
- Reuse earlier bindings defined in twd.txt

 drivers/watchdog/mpcore_wdt.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/drivers/watchdog/mpcore_wdt.c b/drivers/watchdog/mpcore_wdt.c
index 7c00455..98df379 100644
--- a/drivers/watchdog/mpcore_wdt.c
+++ b/drivers/watchdog/mpcore_wdt.c
@@ -27,6 +27,7 @@
 #include <linux/io.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
+#include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/pm.h>
 #include <linux/reboot.h>
@@ -402,6 +403,16 @@ static SIMPLE_DEV_PM_OPS(mpcore_wdt_dev_pm_ops, mpcore_wdt_suspend,
 /* work with hotplug and coldplug */
 MODULE_ALIAS("platform:mpcore_wdt");
 
+#ifdef CONFIG_OF
+static const struct of_device_id mpcore_wdt_id_table[] = {
+	{ .compatible = "arm,cortex-a9-twd-wdt" },
+	{ .compatible = "arm,cortex-a5-twd-wdt" },
+	{ .compatible = "arm,arm11mp-twd-wdt" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, mpcore_wdt_id_table);
+#endif
+
 static struct platform_driver mpcore_wdt_driver = {
 	.probe		= mpcore_wdt_probe,
 	.remove		= __devexit_p(mpcore_wdt_remove),
@@ -410,6 +421,7 @@ static struct platform_driver mpcore_wdt_driver = {
 		.owner	= THIS_MODULE,
 		.name	= "mpcore_wdt",
 		.pm	= &mpcore_wdt_dev_pm_ops,
+		.of_match_table = of_match_ptr(mpcore_wdt_id_table),
 	},
 };
 
-- 
1.7.9

[PATCH V2] watchdog: mpcore: Add DT probing support for ARM mpcore watchdog

[Viresh Kumar]

This patch adds Device tree probing support for ARM Mpcore watchdog. Its binding
were already documented in Documentation/devicetree/bindings/arm/twd.txt.

Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
---
V1->V2:
- Reuse earlier bindings defined in twd.txt

 drivers/watchdog/mpcore_wdt.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/drivers/watchdog/mpcore_wdt.c b/drivers/watchdog/mpcore_wdt.c
index 7c00455..98df379 100644
--- a/drivers/watchdog/mpcore_wdt.c
+++ b/drivers/watchdog/mpcore_wdt.c
@@ -27,6 +27,7 @@
 #include <linux/io.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
+#include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/pm.h>
 #include <linux/reboot.h>
@@ -402,6 +403,16 @@ static SIMPLE_DEV_PM_OPS(mpcore_wdt_dev_pm_ops, mpcore_wdt_suspend,
 /* work with hotplug and coldplug */
 MODULE_ALIAS("platform:mpcore_wdt");
 
+#ifdef CONFIG_OF
+static const struct of_device_id mpcore_wdt_id_table[] = {
+	{ .compatible = "arm,cortex-a9-twd-wdt" },
+	{ .compatible = "arm,cortex-a5-twd-wdt" },
+	{ .compatible = "arm,arm11mp-twd-wdt" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, mpcore_wdt_id_table);
+#endif
+
 static struct platform_driver mpcore_wdt_driver = {
 	.probe		= mpcore_wdt_probe,
 	.remove		= __devexit_p(mpcore_wdt_remove),
@@ -410,6 +421,7 @@ static struct platform_driver mpcore_wdt_driver = {
 		.owner	= THIS_MODULE,
 		.name	= "mpcore_wdt",
 		.pm	= &mpcore_wdt_dev_pm_ops,
+		.of_match_table = of_match_ptr(mpcore_wdt_id_table),
 	},
 };
 
-- 
1.7.9

Re: [PATCH V2] watchdog: mpcore: Add DT probing support for ARM mpcore watchdog

[Marc Zyngier]

On 21/04/12 12:41, Viresh Kumar wrote:
> This patch adds Device tree probing support for ARM Mpcore watchdog. Its binding
> were already documented in Documentation/devicetree/bindings/arm/twd.txt.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
> ---
> V1->V2:
> - Reuse earlier bindings defined in twd.txt
> 
>  drivers/watchdog/mpcore_wdt.c |   12 ++++++++++++
>  1 files changed, 12 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/watchdog/mpcore_wdt.c b/drivers/watchdog/mpcore_wdt.c
> index 7c00455..98df379 100644
> --- a/drivers/watchdog/mpcore_wdt.c
> +++ b/drivers/watchdog/mpcore_wdt.c
> @@ -27,6 +27,7 @@
>  #include <linux/io.h>
>  #include <linux/module.h>
>  #include <linux/moduleparam.h>
> +#include <linux/of.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm.h>
>  #include <linux/reboot.h>
> @@ -402,6 +403,16 @@ static SIMPLE_DEV_PM_OPS(mpcore_wdt_dev_pm_ops, mpcore_wdt_suspend,
>  /* work with hotplug and coldplug */
>  MODULE_ALIAS("platform:mpcore_wdt");
>  
> +#ifdef CONFIG_OF
> +static const struct of_device_id mpcore_wdt_id_table[] = {
> +	{ .compatible = "arm,cortex-a9-twd-wdt" },
> +	{ .compatible = "arm,cortex-a5-twd-wdt" },
> +	{ .compatible = "arm,arm11mp-twd-wdt" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, mpcore_wdt_id_table);
> +#endif
> +
>  static struct platform_driver mpcore_wdt_driver = {
>  	.probe		= mpcore_wdt_probe,
>  	.remove		= __devexit_p(mpcore_wdt_remove),
> @@ -410,6 +421,7 @@ static struct platform_driver mpcore_wdt_driver = {
>  		.owner	= THIS_MODULE,
>  		.name	= "mpcore_wdt",
>  		.pm	= &mpcore_wdt_dev_pm_ops,
> +		.of_match_table = of_match_ptr(mpcore_wdt_id_table),
>  	},
>  };
>  

Irk! Have you actually tested this?

The DT binding indicates:
- reg : Specify the base address and the size of the TWD watchdog
        register window.

while all the offsets in smp_twd.h are expressed in bytes from the TWD
*timer* base. So you have to either fix these offsets (which breaks
potential users of the non-DT version of the driver), or correct the
base when using DT.

	M.
-- 
Jazz is not dead. It just smells funny...

--
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 V2] watchdog: mpcore: Add DT probing support for ARM mpcore watchdog

[Marc Zyngier]

On 21/04/12 12:41, Viresh Kumar wrote:
> This patch adds Device tree probing support for ARM Mpcore watchdog. Its binding
> were already documented in Documentation/devicetree/bindings/arm/twd.txt.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
> ---
> V1->V2:
> - Reuse earlier bindings defined in twd.txt
> 
>  drivers/watchdog/mpcore_wdt.c |   12 ++++++++++++
>  1 files changed, 12 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/watchdog/mpcore_wdt.c b/drivers/watchdog/mpcore_wdt.c
> index 7c00455..98df379 100644
> --- a/drivers/watchdog/mpcore_wdt.c
> +++ b/drivers/watchdog/mpcore_wdt.c
> @@ -27,6 +27,7 @@
>  #include <linux/io.h>
>  #include <linux/module.h>
>  #include <linux/moduleparam.h>
> +#include <linux/of.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm.h>
>  #include <linux/reboot.h>
> @@ -402,6 +403,16 @@ static SIMPLE_DEV_PM_OPS(mpcore_wdt_dev_pm_ops, mpcore_wdt_suspend,
>  /* work with hotplug and coldplug */
>  MODULE_ALIAS("platform:mpcore_wdt");
>  
> +#ifdef CONFIG_OF
> +static const struct of_device_id mpcore_wdt_id_table[] = {
> +	{ .compatible = "arm,cortex-a9-twd-wdt" },
> +	{ .compatible = "arm,cortex-a5-twd-wdt" },
> +	{ .compatible = "arm,arm11mp-twd-wdt" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, mpcore_wdt_id_table);
> +#endif
> +
>  static struct platform_driver mpcore_wdt_driver = {
>  	.probe		= mpcore_wdt_probe,
>  	.remove		= __devexit_p(mpcore_wdt_remove),
> @@ -410,6 +421,7 @@ static struct platform_driver mpcore_wdt_driver = {
>  		.owner	= THIS_MODULE,
>  		.name	= "mpcore_wdt",
>  		.pm	= &mpcore_wdt_dev_pm_ops,
> +		.of_match_table = of_match_ptr(mpcore_wdt_id_table),
>  	},
>  };
>  

Irk! Have you actually tested this?

The DT binding indicates:
- reg : Specify the base address and the size of the TWD watchdog
        register window.

while all the offsets in smp_twd.h are expressed in bytes from the TWD
*timer* base. So you have to either fix these offsets (which breaks
potential users of the non-DT version of the driver), or correct the
base when using DT.

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH V2] watchdog: mpcore: Add DT probing support for ARM mpcore watchdog

[viresh kumar]

On Mon, Apr 23, 2012 at 2:06 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> Irk! Have you actually tested this?

Yes. But with incorrect base address. I passed timers address by mistake.

> The DT binding indicates:
> - reg : Specify the base address and the size of the TWD watchdog
>        register window.
>
> while all the offsets in smp_twd.h are expressed in bytes from the TWD
> *timer* base. So you have to either fix these offsets (which breaks
> potential users of the non-DT version of the driver), or correct the
> base when using DT.

You are correct. Which one do you prefer:
- Change bindings to pass timers base address
- subtract 0x20 from base address for DT case
- something else.

--
viresh
--
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 V2] watchdog: mpcore: Add DT probing support for ARM mpcore watchdog

[viresh kumar]

On Mon, Apr 23, 2012 at 2:06 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> Irk! Have you actually tested this?

Yes. But with incorrect base address. I passed timers address by mistake.

> The DT binding indicates:
> - reg : Specify the base address and the size of the TWD watchdog
> ? ? ? ?register window.
>
> while all the offsets in smp_twd.h are expressed in bytes from the TWD
> *timer* base. So you have to either fix these offsets (which breaks
> potential users of the non-DT version of the driver), or correct the
> base when using DT.

You are correct. Which one do you prefer:
- Change bindings to pass timers base address
- subtract 0x20 from base address for DT case
- something else.

--
viresh

Re: [PATCH V2] watchdog: mpcore: Add DT probing support for ARM mpcore watchdog

[Marc Zyngier]

On 23/04/12 12:35, viresh kumar wrote:
> On Mon, Apr 23, 2012 at 2:06 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> Irk! Have you actually tested this?
> 
> Yes. But with incorrect base address. I passed timers address by mistake.

Ah, that explains why it worked. I suppose you do have additional
patches that fix the IRQ request bit?

>> The DT binding indicates:
>> - reg : Specify the base address and the size of the TWD watchdog
>>        register window.
>>
>> while all the offsets in smp_twd.h are expressed in bytes from the TWD
>> *timer* base. So you have to either fix these offsets (which breaks
>> potential users of the non-DT version of the driver), or correct the
>> base when using DT.
> 
> You are correct. Which one do you prefer:
> - Change bindings to pass timers base address

No. We already discussed this on LAK, and the outcome was the current
binding, so let's not change that again.

> - subtract 0x20 from base address for DT case

That's a possibility.

> - something else.

Given that no in-tree platform seem to be using this watchdog (at least
a quick grep didn't reveal anything), I'd be inclined to simply change
the offset in smp_twd.h and let them break.

	M.
-- 
Jazz is not dead. It just smells funny...

--
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 V2] watchdog: mpcore: Add DT probing support for ARM mpcore watchdog

[Marc Zyngier]

On 23/04/12 12:35, viresh kumar wrote:
> On Mon, Apr 23, 2012 at 2:06 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> Irk! Have you actually tested this?
> 
> Yes. But with incorrect base address. I passed timers address by mistake.

Ah, that explains why it worked. I suppose you do have additional
patches that fix the IRQ request bit?

>> The DT binding indicates:
>> - reg : Specify the base address and the size of the TWD watchdog
>>        register window.
>>
>> while all the offsets in smp_twd.h are expressed in bytes from the TWD
>> *timer* base. So you have to either fix these offsets (which breaks
>> potential users of the non-DT version of the driver), or correct the
>> base when using DT.
> 
> You are correct. Which one do you prefer:
> - Change bindings to pass timers base address

No. We already discussed this on LAK, and the outcome was the current
binding, so let's not change that again.

> - subtract 0x20 from base address for DT case

That's a possibility.

> - something else.

Given that no in-tree platform seem to be using this watchdog (at least
a quick grep didn't reveal anything), I'd be inclined to simply change
the offset in smp_twd.h and let them break.

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH V2] watchdog: mpcore: Add DT probing support for ARM mpcore watchdog

[viresh kumar]

On 4/23/12, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On 23/04/12 12:35, viresh kumar wrote:
>> - subtract 0x20 from base address for DT case
>
> That's a possibility.
>
>> - something else.
>
> Given that no in-tree platform seem to be using this watchdog (at least
> a quick grep didn't reveal anything), I'd be inclined to simply change
> the offset in smp_twd.h and let them break.

I believe atlest ST Ericsson's Ux500 series is using it, Linus??

Obviously the best solution is to fix these offsets and pass watchdog
base address from DT or machine/board files.

I will see what i can do best.

--
viresh

[PATCH V2] watchdog: mpcore: Add DT probing support for ARM mpcore watchdog

[viresh kumar]

On 4/23/12, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On 23/04/12 12:35, viresh kumar wrote:
>> - subtract 0x20 from base address for DT case
>
> That's a possibility.
>
>> - something else.
>
> Given that no in-tree platform seem to be using this watchdog (at least
> a quick grep didn't reveal anything), I'd be inclined to simply change
> the offset in smp_twd.h and let them break.

I believe atlest ST Ericsson's Ux500 series is using it, Linus??

Obviously the best solution is to fix these offsets and pass watchdog
base address from DT or machine/board files.

I will see what i can do best.

--
viresh

Re: [PATCH V2] watchdog: mpcore: Add DT probing support for ARM mpcore watchdog

[viresh kumar]

On 4/23/12, Marc Zyngier <marc.zyngier@arm.com> wrote:
> Ah, that explains why it worked. I suppose you do have additional
> patches that fix the IRQ request bit?

Missed this earlier. That patchset is in review currently and have already
fixed that. :)

> Given that no in-tree platform seem to be using this watchdog (at least
> a quick grep didn't reveal anything), I'd be inclined to simply change
> the offset in smp_twd.h and let them break.

Even i can't find any users of it. :)

@Wim: Please apply following patch before this one.
Here we go:

From: Viresh Kumar <viresh.kumar@st.com>
Date: Mon, 23 Apr 2012 19:39:47 +0530
Subject: [PATCH] ARM: SMP_TWD: WDOG: Start registers from 0x00 instead of
 0x20

TWD_WDOG is at offset 0x20 from TWD base address. Current register offsets
contain this extra 0x20 offset, i.e. users were required to pass base address of
TWD instead of WDOG to WDOG driver.

Change this, so that users can pass base address of WDOG to WDOG driver instead
of TWD module. For this, subtract 0x20 from offsets of WDOG registers.

This could break any current users of TWD_WDOG, but i couldn't find any users of
this driver in current Linux tree. So, haven't fixed any platform code. If some
platforms are broken please report to me, so that we can get them fixed in this
patch only.

Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
---
 arch/arm/include/asm/smp_twd.h |   17 +++++++++++------
 1 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/arch/arm/include/asm/smp_twd.h b/arch/arm/include/asm/smp_twd.h
index 57857d1..ff3f67e 100644
--- a/arch/arm/include/asm/smp_twd.h
+++ b/arch/arm/include/asm/smp_twd.h
@@ -6,12 +6,17 @@
 #define TWD_TIMER_CONTROL		0x08
 #define TWD_TIMER_INTSTAT		0x0C

-#define TWD_WDOG_LOAD			0x20
-#define TWD_WDOG_COUNTER		0x24
-#define TWD_WDOG_CONTROL		0x28
-#define TWD_WDOG_INTSTAT		0x2C
-#define TWD_WDOG_RESETSTAT		0x30
-#define TWD_WDOG_DISABLE		0x34
+/*
+ * TWD_WDOG is at offset 0x20 from TWD base address. Following register offsets
+ * doesn't contain this extra 0x20 offset, i.e. users of TWD_WDOG
must pass base
+ * address of WDOG to WDOG driver instead of TWD module.
+ */
+#define TWD_WDOG_LOAD			0x00
+#define TWD_WDOG_COUNTER		0x04
+#define TWD_WDOG_CONTROL		0x08
+#define TWD_WDOG_INTSTAT		0x0C
+#define TWD_WDOG_RESETSTAT		0x10
+#define TWD_WDOG_DISABLE		0x14

 #define TWD_WDOG_LOAD_MIN		0x00000000
 #define TWD_WDOG_LOAD_MAX		0xFFFFFFFF

[PATCH V2] watchdog: mpcore: Add DT probing support for ARM mpcore watchdog

[viresh kumar]

On 4/23/12, Marc Zyngier <marc.zyngier@arm.com> wrote:
> Ah, that explains why it worked. I suppose you do have additional
> patches that fix the IRQ request bit?

Missed this earlier. That patchset is in review currently and have already
fixed that. :)

> Given that no in-tree platform seem to be using this watchdog (at least
> a quick grep didn't reveal anything), I'd be inclined to simply change
> the offset in smp_twd.h and let them break.

Even i can't find any users of it. :)

@Wim: Please apply following patch before this one.
Here we go:

From: Viresh Kumar <viresh.kumar@st.com>
Date: Mon, 23 Apr 2012 19:39:47 +0530
Subject: [PATCH] ARM: SMP_TWD: WDOG: Start registers from 0x00 instead of
 0x20

TWD_WDOG is at offset 0x20 from TWD base address. Current register offsets
contain this extra 0x20 offset, i.e. users were required to pass base address of
TWD instead of WDOG to WDOG driver.

Change this, so that users can pass base address of WDOG to WDOG driver instead
of TWD module. For this, subtract 0x20 from offsets of WDOG registers.

This could break any current users of TWD_WDOG, but i couldn't find any users of
this driver in current Linux tree. So, haven't fixed any platform code. If some
platforms are broken please report to me, so that we can get them fixed in this
patch only.

Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
---
 arch/arm/include/asm/smp_twd.h |   17 +++++++++++------
 1 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/arch/arm/include/asm/smp_twd.h b/arch/arm/include/asm/smp_twd.h
index 57857d1..ff3f67e 100644
--- a/arch/arm/include/asm/smp_twd.h
+++ b/arch/arm/include/asm/smp_twd.h
@@ -6,12 +6,17 @@
 #define TWD_TIMER_CONTROL		0x08
 #define TWD_TIMER_INTSTAT		0x0C

-#define TWD_WDOG_LOAD			0x20
-#define TWD_WDOG_COUNTER		0x24
-#define TWD_WDOG_CONTROL		0x28
-#define TWD_WDOG_INTSTAT		0x2C
-#define TWD_WDOG_RESETSTAT		0x30
-#define TWD_WDOG_DISABLE		0x34
+/*
+ * TWD_WDOG is at offset 0x20 from TWD base address. Following register offsets
+ * doesn't contain this extra 0x20 offset, i.e. users of TWD_WDOG
must pass base
+ * address of WDOG to WDOG driver instead of TWD module.
+ */
+#define TWD_WDOG_LOAD			0x00
+#define TWD_WDOG_COUNTER		0x04
+#define TWD_WDOG_CONTROL		0x08
+#define TWD_WDOG_INTSTAT		0x0C
+#define TWD_WDOG_RESETSTAT		0x10
+#define TWD_WDOG_DISABLE		0x14

 #define TWD_WDOG_LOAD_MIN		0x00000000
 #define TWD_WDOG_LOAD_MAX		0xFFFFFFFF

Re: [PATCH V2] watchdog: mpcore: Add DT probing support for ARM mpcore watchdog

[Marc Zyngier]

On 23/04/12 15:21, viresh kumar wrote:
> On 4/23/12, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> Ah, that explains why it worked. I suppose you do have additional
>> patches that fix the IRQ request bit?
> 
> Missed this earlier. That patchset is in review currently and have already
> fixed that. :)

Right. Please post that patch as a prerequisite for any other change.

>> Given that no in-tree platform seem to be using this watchdog (at least
>> a quick grep didn't reveal anything), I'd be inclined to simply change
>> the offset in smp_twd.h and let them break.
> 
> Even i can't find any users of it. :)
> 
> @Wim: Please apply following patch before this one.

Probably it would be better to keep everything in a single series,
including the above IRQ fixes. It would surely make things easier for
the maintainer and other people who are reviewing the changes you're
making to the driver.

> Here we go:
> 
> From: Viresh Kumar <viresh.kumar@st.com>
> Date: Mon, 23 Apr 2012 19:39:47 +0530
> Subject: [PATCH] ARM: SMP_TWD: WDOG: Start registers from 0x00 instead of
>  0x20
> 
> TWD_WDOG is at offset 0x20 from TWD base address. Current register offsets
> contain this extra 0x20 offset, i.e. users were required to pass base address of
> TWD instead of WDOG to WDOG driver.
> 
> Change this, so that users can pass base address of WDOG to WDOG driver instead
> of TWD module. For this, subtract 0x20 from offsets of WDOG registers.
> 
> This could break any current users of TWD_WDOG, but i couldn't find any users of
> this driver in current Linux tree. So, haven't fixed any platform code. If some
> platforms are broken please report to me, so that we can get them fixed in this
> patch only.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
> ---
>  arch/arm/include/asm/smp_twd.h |   17 +++++++++++------
>  1 files changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/include/asm/smp_twd.h b/arch/arm/include/asm/smp_twd.h
> index 57857d1..ff3f67e 100644
> --- a/arch/arm/include/asm/smp_twd.h
> +++ b/arch/arm/include/asm/smp_twd.h
> @@ -6,12 +6,17 @@
>  #define TWD_TIMER_CONTROL		0x08
>  #define TWD_TIMER_INTSTAT		0x0C
> 
> -#define TWD_WDOG_LOAD			0x20
> -#define TWD_WDOG_COUNTER		0x24
> -#define TWD_WDOG_CONTROL		0x28
> -#define TWD_WDOG_INTSTAT		0x2C
> -#define TWD_WDOG_RESETSTAT		0x30
> -#define TWD_WDOG_DISABLE		0x34
> +/*
> + * TWD_WDOG is at offset 0x20 from TWD base address. Following register offsets
> + * doesn't contain this extra 0x20 offset, i.e. users of TWD_WDOG
> must pass base
> + * address of WDOG to WDOG driver instead of TWD module.
> + */
> +#define TWD_WDOG_LOAD			0x00
> +#define TWD_WDOG_COUNTER		0x04
> +#define TWD_WDOG_CONTROL		0x08
> +#define TWD_WDOG_INTSTAT		0x0C
> +#define TWD_WDOG_RESETSTAT		0x10
> +#define TWD_WDOG_DISABLE		0x14
> 
>  #define TWD_WDOG_LOAD_MIN		0x00000000
>  #define TWD_WDOG_LOAD_MAX		0xFFFFFFFF
> 

Now that we made it that far, why not moving the TWD_WDOG_* #defines to
the driver itself? Nobody else should care about it anyway...

	M.
-- 
Jazz is not dead. It just smells funny...

--
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 V2] watchdog: mpcore: Add DT probing support for ARM mpcore watchdog

[Marc Zyngier]

On 23/04/12 15:21, viresh kumar wrote:
> On 4/23/12, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> Ah, that explains why it worked. I suppose you do have additional
>> patches that fix the IRQ request bit?
> 
> Missed this earlier. That patchset is in review currently and have already
> fixed that. :)

Right. Please post that patch as a prerequisite for any other change.

>> Given that no in-tree platform seem to be using this watchdog (at least
>> a quick grep didn't reveal anything), I'd be inclined to simply change
>> the offset in smp_twd.h and let them break.
> 
> Even i can't find any users of it. :)
> 
> @Wim: Please apply following patch before this one.

Probably it would be better to keep everything in a single series,
including the above IRQ fixes. It would surely make things easier for
the maintainer and other people who are reviewing the changes you're
making to the driver.

> Here we go:
> 
> From: Viresh Kumar <viresh.kumar@st.com>
> Date: Mon, 23 Apr 2012 19:39:47 +0530
> Subject: [PATCH] ARM: SMP_TWD: WDOG: Start registers from 0x00 instead of
>  0x20
> 
> TWD_WDOG is at offset 0x20 from TWD base address. Current register offsets
> contain this extra 0x20 offset, i.e. users were required to pass base address of
> TWD instead of WDOG to WDOG driver.
> 
> Change this, so that users can pass base address of WDOG to WDOG driver instead
> of TWD module. For this, subtract 0x20 from offsets of WDOG registers.
> 
> This could break any current users of TWD_WDOG, but i couldn't find any users of
> this driver in current Linux tree. So, haven't fixed any platform code. If some
> platforms are broken please report to me, so that we can get them fixed in this
> patch only.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
> ---
>  arch/arm/include/asm/smp_twd.h |   17 +++++++++++------
>  1 files changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/include/asm/smp_twd.h b/arch/arm/include/asm/smp_twd.h
> index 57857d1..ff3f67e 100644
> --- a/arch/arm/include/asm/smp_twd.h
> +++ b/arch/arm/include/asm/smp_twd.h
> @@ -6,12 +6,17 @@
>  #define TWD_TIMER_CONTROL		0x08
>  #define TWD_TIMER_INTSTAT		0x0C
> 
> -#define TWD_WDOG_LOAD			0x20
> -#define TWD_WDOG_COUNTER		0x24
> -#define TWD_WDOG_CONTROL		0x28
> -#define TWD_WDOG_INTSTAT		0x2C
> -#define TWD_WDOG_RESETSTAT		0x30
> -#define TWD_WDOG_DISABLE		0x34
> +/*
> + * TWD_WDOG is at offset 0x20 from TWD base address. Following register offsets
> + * doesn't contain this extra 0x20 offset, i.e. users of TWD_WDOG
> must pass base
> + * address of WDOG to WDOG driver instead of TWD module.
> + */
> +#define TWD_WDOG_LOAD			0x00
> +#define TWD_WDOG_COUNTER		0x04
> +#define TWD_WDOG_CONTROL		0x08
> +#define TWD_WDOG_INTSTAT		0x0C
> +#define TWD_WDOG_RESETSTAT		0x10
> +#define TWD_WDOG_DISABLE		0x14
> 
>  #define TWD_WDOG_LOAD_MIN		0x00000000
>  #define TWD_WDOG_LOAD_MAX		0xFFFFFFFF
> 

Now that we made it that far, why not moving the TWD_WDOG_* #defines to
the driver itself? Nobody else should care about it anyway...

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH V2] watchdog: mpcore: Add DT probing support for ARM mpcore watchdog

[viresh kumar]

On 4/23/12, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On 23/04/12 15:21, viresh kumar wrote:
>> On 4/23/12, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>> Ah, that explains why it worked. I suppose you do have additional
>>> patches that fix the IRQ request bit?
>>
>> Missed this earlier. That patchset is in review currently and have
>> already fixed that. :)
>
> Right. Please post that patch as a prerequisite for any other change.

Haah!! I misread your question. I don't know how, but i thought you are
asking me to fix this address for SPEAr (My SoC). And that patch set
is currently in review.

I didn't get this IRQ request bit thing? Can you please explain.

>>> Given that no in-tree platform seem to be using this watchdog (at least
>>> a quick grep didn't reveal anything), I'd be inclined to simply change
>>> the offset in smp_twd.h and let them break.
>>
>> Even i can't find any users of it. :)
>>
>> @Wim: Please apply following patch before this one.
>
> Probably it would be better to keep everything in a single series,
> including the above IRQ fixes. It would surely make things easier for
> the maintainer and other people who are reviewing the changes you're
> making to the driver.

Even i would like to keep related patches in a single set. But i am doubtful
about this IRQ thing.

>> Here we go:
>>
>> From: Viresh Kumar <viresh.kumar@st.com>
>> Date: Mon, 23 Apr 2012 19:39:47 +0530
>> Subject: [PATCH] ARM: SMP_TWD: WDOG: Start registers from 0x00 instead of
>>  0x20
>>
>> TWD_WDOG is at offset 0x20 from TWD base address. Current register
>> offsets
>> contain this extra 0x20 offset, i.e. users were required to pass base
>> address of
>> TWD instead of WDOG to WDOG driver.
>>
>> Change this, so that users can pass base address of WDOG to WDOG driver
>> instead
>> of TWD module. For this, subtract 0x20 from offsets of WDOG registers.
>>
>> This could break any current users of TWD_WDOG, but i couldn't find any
>> users of
>> this driver in current Linux tree. So, haven't fixed any platform code. If
>> some
>> platforms are broken please report to me, so that we can get them fixed in
>> this
>> patch only.
>>
>> Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
>> ---
>>  arch/arm/include/asm/smp_twd.h |   17 +++++++++++------
>>  1 files changed, 11 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/smp_twd.h
>> b/arch/arm/include/asm/smp_twd.h
>> index 57857d1..ff3f67e 100644
>> --- a/arch/arm/include/asm/smp_twd.h
>> +++ b/arch/arm/include/asm/smp_twd.h
>> @@ -6,12 +6,17 @@
>>  #define TWD_TIMER_CONTROL		0x08
>>  #define TWD_TIMER_INTSTAT		0x0C
>>
>> -#define TWD_WDOG_LOAD			0x20
>> -#define TWD_WDOG_COUNTER		0x24
>> -#define TWD_WDOG_CONTROL		0x28
>> -#define TWD_WDOG_INTSTAT		0x2C
>> -#define TWD_WDOG_RESETSTAT		0x30
>> -#define TWD_WDOG_DISABLE		0x34
>> +/*
>> + * TWD_WDOG is at offset 0x20 from TWD base address. Following register
>> offsets
>> + * doesn't contain this extra 0x20 offset, i.e. users of TWD_WDOG
>> must pass base
>> + * address of WDOG to WDOG driver instead of TWD module.
>> + */
>> +#define TWD_WDOG_LOAD			0x00
>> +#define TWD_WDOG_COUNTER		0x04
>> +#define TWD_WDOG_CONTROL		0x08
>> +#define TWD_WDOG_INTSTAT		0x0C
>> +#define TWD_WDOG_RESETSTAT		0x10
>> +#define TWD_WDOG_DISABLE		0x14
>>
>>  #define TWD_WDOG_LOAD_MIN		0x00000000
>>  #define TWD_WDOG_LOAD_MAX		0xFFFFFFFF
>>
>
> Now that we made it that far, why not moving the TWD_WDOG_* #defines to
> the driver itself? Nobody else should care about it anyway...

I thought of it too, but then left it. I thought this is done purposefully.
Will move it once i get your concern on IRQ stuff.

--
viresh

[PATCH V2] watchdog: mpcore: Add DT probing support for ARM mpcore watchdog

[viresh kumar]

On 4/23/12, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On 23/04/12 15:21, viresh kumar wrote:
>> On 4/23/12, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>> Ah, that explains why it worked. I suppose you do have additional
>>> patches that fix the IRQ request bit?
>>
>> Missed this earlier. That patchset is in review currently and have
>> already fixed that. :)
>
> Right. Please post that patch as a prerequisite for any other change.

Haah!! I misread your question. I don't know how, but i thought you are
asking me to fix this address for SPEAr (My SoC). And that patch set
is currently in review.

I didn't get this IRQ request bit thing? Can you please explain.

>>> Given that no in-tree platform seem to be using this watchdog (at least
>>> a quick grep didn't reveal anything), I'd be inclined to simply change
>>> the offset in smp_twd.h and let them break.
>>
>> Even i can't find any users of it. :)
>>
>> @Wim: Please apply following patch before this one.
>
> Probably it would be better to keep everything in a single series,
> including the above IRQ fixes. It would surely make things easier for
> the maintainer and other people who are reviewing the changes you're
> making to the driver.

Even i would like to keep related patches in a single set. But i am doubtful
about this IRQ thing.

>> Here we go:
>>
>> From: Viresh Kumar <viresh.kumar@st.com>
>> Date: Mon, 23 Apr 2012 19:39:47 +0530
>> Subject: [PATCH] ARM: SMP_TWD: WDOG: Start registers from 0x00 instead of
>>  0x20
>>
>> TWD_WDOG is at offset 0x20 from TWD base address. Current register
>> offsets
>> contain this extra 0x20 offset, i.e. users were required to pass base
>> address of
>> TWD instead of WDOG to WDOG driver.
>>
>> Change this, so that users can pass base address of WDOG to WDOG driver
>> instead
>> of TWD module. For this, subtract 0x20 from offsets of WDOG registers.
>>
>> This could break any current users of TWD_WDOG, but i couldn't find any
>> users of
>> this driver in current Linux tree. So, haven't fixed any platform code. If
>> some
>> platforms are broken please report to me, so that we can get them fixed in
>> this
>> patch only.
>>
>> Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
>> ---
>>  arch/arm/include/asm/smp_twd.h |   17 +++++++++++------
>>  1 files changed, 11 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/smp_twd.h
>> b/arch/arm/include/asm/smp_twd.h
>> index 57857d1..ff3f67e 100644
>> --- a/arch/arm/include/asm/smp_twd.h
>> +++ b/arch/arm/include/asm/smp_twd.h
>> @@ -6,12 +6,17 @@
>>  #define TWD_TIMER_CONTROL		0x08
>>  #define TWD_TIMER_INTSTAT		0x0C
>>
>> -#define TWD_WDOG_LOAD			0x20
>> -#define TWD_WDOG_COUNTER		0x24
>> -#define TWD_WDOG_CONTROL		0x28
>> -#define TWD_WDOG_INTSTAT		0x2C
>> -#define TWD_WDOG_RESETSTAT		0x30
>> -#define TWD_WDOG_DISABLE		0x34
>> +/*
>> + * TWD_WDOG is at offset 0x20 from TWD base address. Following register
>> offsets
>> + * doesn't contain this extra 0x20 offset, i.e. users of TWD_WDOG
>> must pass base
>> + * address of WDOG to WDOG driver instead of TWD module.
>> + */
>> +#define TWD_WDOG_LOAD			0x00
>> +#define TWD_WDOG_COUNTER		0x04
>> +#define TWD_WDOG_CONTROL		0x08
>> +#define TWD_WDOG_INTSTAT		0x0C
>> +#define TWD_WDOG_RESETSTAT		0x10
>> +#define TWD_WDOG_DISABLE		0x14
>>
>>  #define TWD_WDOG_LOAD_MIN		0x00000000
>>  #define TWD_WDOG_LOAD_MAX		0xFFFFFFFF
>>
>
> Now that we made it that far, why not moving the TWD_WDOG_* #defines to
> the driver itself? Nobody else should care about it anyway...

I thought of it too, but then left it. I thought this is done purposefully.
Will move it once i get your concern on IRQ stuff.

--
viresh

Re: [PATCH V2] watchdog: mpcore: Add DT probing support for ARM mpcore watchdog

[Marc Zyngier]

On 23/04/12 16:33, viresh kumar wrote:
> On 4/23/12, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> On 23/04/12 15:21, viresh kumar wrote:
>>> On 4/23/12, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>>> Ah, that explains why it worked. I suppose you do have additional
>>>> patches that fix the IRQ request bit?
>>>
>>> Missed this earlier. That patchset is in review currently and have
>>> already fixed that. :)
>>
>> Right. Please post that patch as a prerequisite for any other change.
> 
> Haah!! I misread your question. I don't know how, but i thought you are
> asking me to fix this address for SPEAr (My SoC). And that patch set
> is currently in review.
> 
> I didn't get this IRQ request bit thing? Can you please explain.

The TWD watchdog uses a per-cpu interrupt (usually interrupt #30), and
the GIC configuration should flag it as such. With this setup,
request_irq() should fail, and the right API is request_percpu_irq(),
together with enable_percpu_irq()/disable_percpu_irq().

So how it works in your environment is a bit puzzling. Unless you're not
using GIC?

	M.
-- 
Jazz is not dead. It just smells funny...

--
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 V2] watchdog: mpcore: Add DT probing support for ARM mpcore watchdog

[Marc Zyngier]

On 23/04/12 16:33, viresh kumar wrote:
> On 4/23/12, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> On 23/04/12 15:21, viresh kumar wrote:
>>> On 4/23/12, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>>> Ah, that explains why it worked. I suppose you do have additional
>>>> patches that fix the IRQ request bit?
>>>
>>> Missed this earlier. That patchset is in review currently and have
>>> already fixed that. :)
>>
>> Right. Please post that patch as a prerequisite for any other change.
> 
> Haah!! I misread your question. I don't know how, but i thought you are
> asking me to fix this address for SPEAr (My SoC). And that patch set
> is currently in review.
> 
> I didn't get this IRQ request bit thing? Can you please explain.

The TWD watchdog uses a per-cpu interrupt (usually interrupt #30), and
the GIC configuration should flag it as such. With this setup,
request_irq() should fail, and the right API is request_percpu_irq(),
together with enable_percpu_irq()/disable_percpu_irq().

So how it works in your environment is a bit puzzling. Unless you're not
using GIC?

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH V2] watchdog: mpcore: Add DT probing support for ARM mpcore watchdog

[viresh kumar]

On 4/23/12, Marc Zyngier <marc.zyngier@arm.com> wrote:
> The TWD watchdog uses a per-cpu interrupt (usually interrupt #30), and
> the GIC configuration should flag it as such. With this setup,
> request_irq() should fail, and the right API is request_percpu_irq(),
> together with enable_percpu_irq()/disable_percpu_irq().

request_irq() doesn't fail for me even if i pass it.

> So how it works in your environment is a bit puzzling. Unless you're not
> using GIC?

We use GIC and are running SMP. But we don't use wdt interrupt :)
We only test it for system reset/reboot.

I don't have a patch for this. Can you please add that?

--
Viresh

[PATCH V2] watchdog: mpcore: Add DT probing support for ARM mpcore watchdog

[viresh kumar]

On 4/23/12, Marc Zyngier <marc.zyngier@arm.com> wrote:
> The TWD watchdog uses a per-cpu interrupt (usually interrupt #30), and
> the GIC configuration should flag it as such. With this setup,
> request_irq() should fail, and the right API is request_percpu_irq(),
> together with enable_percpu_irq()/disable_percpu_irq().

request_irq() doesn't fail for me even if i pass it.

> So how it works in your environment is a bit puzzling. Unless you're not
> using GIC?

We use GIC and are running SMP. But we don't use wdt interrupt :)
We only test it for system reset/reboot.

I don't have a patch for this. Can you please add that?

--
Viresh

Re: [PATCH V2] watchdog: mpcore: Add DT probing support for ARM mpcore watchdog

[Marc Zyngier]

On 23/04/12 17:01, viresh kumar wrote:
> On 4/23/12, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> The TWD watchdog uses a per-cpu interrupt (usually interrupt #30), and
>> the GIC configuration should flag it as such. With this setup,
>> request_irq() should fail, and the right API is request_percpu_irq(),
>> together with enable_percpu_irq()/disable_percpu_irq().
> 
> request_irq() doesn't fail for me even if i pass it.

Well, it definitely should. Can you share the relevant part of your DT?

	M.
-- 
Jazz is not dead. It just smells funny...

--
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 V2] watchdog: mpcore: Add DT probing support for ARM mpcore watchdog

[Marc Zyngier]

On 23/04/12 17:01, viresh kumar wrote:
> On 4/23/12, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> The TWD watchdog uses a per-cpu interrupt (usually interrupt #30), and
>> the GIC configuration should flag it as such. With this setup,
>> request_irq() should fail, and the right API is request_percpu_irq(),
>> together with enable_percpu_irq()/disable_percpu_irq().
> 
> request_irq() doesn't fail for me even if i pass it.

Well, it definitely should. Can you share the relevant part of your DT?

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH V2] watchdog: mpcore: Add DT probing support for ARM mpcore watchdog

[viresh kumar]

On 4/23/12, Marc Zyngier <marc.zyngier@arm.com> wrote:
> Well, it definitely should. Can you share the relevant part of your DT?

I Just tested it once, and will do it tomorrow again.

			wdt@ec800620 {
				compatible = "arm,cortex-a9-twd-wdt";
				reg = <0xec800620 0x20>;
				interrupts = <1 14 0x301>;
				status = "disabled";
			};

Looking at your confidence, i am starting to feel, i am wrong here ;)

--
viresh

[PATCH V2] watchdog: mpcore: Add DT probing support for ARM mpcore watchdog

[viresh kumar]

On 4/23/12, Marc Zyngier <marc.zyngier@arm.com> wrote:
> Well, it definitely should. Can you share the relevant part of your DT?

I Just tested it once, and will do it tomorrow again.

			wdt at ec800620 {
				compatible = "arm,cortex-a9-twd-wdt";
				reg = <0xec800620 0x20>;
				interrupts = <1 14 0x301>;
				status = "disabled";
			};

Looking at your confidence, i am starting to feel, i am wrong here ;)

--
viresh

Re: [PATCH V2] watchdog: mpcore: Add DT probing support for ARM mpcore watchdog

[Marc Zyngier]

On 23/04/12 17:16, viresh kumar wrote:
> On 4/23/12, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> Well, it definitely should. Can you share the relevant part of your DT?
> 
> I Just tested it once, and will do it tomorrow again.
> 
> 			wdt@ec800620 {
> 				compatible = "arm,cortex-a9-twd-wdt";
> 				reg = <0xec800620 0x20>;
> 				interrupts = <1 14 0x301>;
> 				status = "disabled";
> 			};
> 
> Looking at your confidence, i am starting to feel, i am wrong here ;)

Well, having written the code that ensures request_irq() fails when
passed a PPI, I'd be very worried if the above started to be working... ;-)

	M.
-- 
Jazz is not dead. It just smells funny...

--
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 V2] watchdog: mpcore: Add DT probing support for ARM mpcore watchdog

[Marc Zyngier]

On 23/04/12 17:16, viresh kumar wrote:
> On 4/23/12, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> Well, it definitely should. Can you share the relevant part of your DT?
> 
> I Just tested it once, and will do it tomorrow again.
> 
> 			wdt at ec800620 {
> 				compatible = "arm,cortex-a9-twd-wdt";
> 				reg = <0xec800620 0x20>;
> 				interrupts = <1 14 0x301>;
> 				status = "disabled";
> 			};
> 
> Looking at your confidence, i am starting to feel, i am wrong here ;)

Well, having written the code that ensures request_irq() fails when
passed a PPI, I'd be very worried if the above started to be working... ;-)

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH V2] watchdog: mpcore: Add DT probing support for ARM mpcore watchdog

[Viresh Kumar]

On 4/23/2012 9:56 PM, Marc Zyngier wrote:
>> > Looking at your confidence, i am starting to feel, i am wrong here ;)
> Well, having written the code that ensures request_irq() fails when
> passed a PPI, I'd be very worried if the above started to be working... 

And yes. I was wrong. 

WARNING: at /home/vireshk/work/spear/kernel/linux-2.6/kernel/irq/manage.c:1368 request_threaded_irq+0x128/0x148()
Modules linked in:
Backtrace: 
[<c000c200>] (dump_backtrace+0x0/0x10c) from [<c0249814>] (dump_stack+0x18/0x1c)
 r6:00000558 r5:c005cb34 r4:00000000
[<c02497fc>] (dump_stack+0x0/0x1c) from [<c00157d4>] (warn_slowpath_common+0x54/0x6c)
[<c0015780>] (warn_slowpath_common+0x0/0x6c) from [<c0015810>] (warn_slowpath_null+0x24/0x28)
 r8:0000001e r7:c01f69c4 r6:00000000 r5:c033b840 r4:ee21770c
[<c00157ec>] (warn_slowpath_null+0x0/0x28) from [<c005cb34>] (request_threaded_irq+0x128/0x148)
[<c005ca0c>] (request_threaded_irq+0x0/0x148) from [<c005e95c>] (devm_request_threaded_irq+0x5c/0x94)
[<c005e900>] (devm_request_threaded_irq+0x0/0x94) from [<c02460b8>] (mpcore_wdt_probe+0x88/0x2d4)
[<c0246030>] (mpcore_wdt_probe+0x0/0x2d4) from [<c0180548>] (platform_drv_probe+0x20/0x24)
 r7:00000000 r6:ee101c08 r5:c03601e4 r4:c038abf4

-- 
viresh

[PATCH V2] watchdog: mpcore: Add DT probing support for ARM mpcore watchdog

[Viresh Kumar]

On 4/23/2012 9:56 PM, Marc Zyngier wrote:
>> > Looking at your confidence, i am starting to feel, i am wrong here ;)
> Well, having written the code that ensures request_irq() fails when
> passed a PPI, I'd be very worried if the above started to be working... 

And yes. I was wrong. 

WARNING: at /home/vireshk/work/spear/kernel/linux-2.6/kernel/irq/manage.c:1368 request_threaded_irq+0x128/0x148()
Modules linked in:
Backtrace: 
[<c000c200>] (dump_backtrace+0x0/0x10c) from [<c0249814>] (dump_stack+0x18/0x1c)
 r6:00000558 r5:c005cb34 r4:00000000
[<c02497fc>] (dump_stack+0x0/0x1c) from [<c00157d4>] (warn_slowpath_common+0x54/0x6c)
[<c0015780>] (warn_slowpath_common+0x0/0x6c) from [<c0015810>] (warn_slowpath_null+0x24/0x28)
 r8:0000001e r7:c01f69c4 r6:00000000 r5:c033b840 r4:ee21770c
[<c00157ec>] (warn_slowpath_null+0x0/0x28) from [<c005cb34>] (request_threaded_irq+0x128/0x148)
[<c005ca0c>] (request_threaded_irq+0x0/0x148) from [<c005e95c>] (devm_request_threaded_irq+0x5c/0x94)
[<c005e900>] (devm_request_threaded_irq+0x0/0x94) from [<c02460b8>] (mpcore_wdt_probe+0x88/0x2d4)
[<c0246030>] (mpcore_wdt_probe+0x0/0x2d4) from [<c0180548>] (platform_drv_probe+0x20/0x24)
 r7:00000000 r6:ee101c08 r5:c03601e4 r4:c038abf4

-- 
viresh