[PATCH V2 0/2] ARM: davinci: da850: add wdt DT node

[PATCH V2 0/2] ARM: davinci: da850: add wdt DT node

[Kumar, Anil]

Add wdt DT node and OF_DEV_AUXDATA for wdt driver in
da850 board dt file to use wdt clock.

This is tested on da850 EVM.

Test Procedure:

$watchdog -F /dev/watchdog (Device should not reboot)
 After wdt heartbeat timeout(DEFAULT_HEARTBEAT is 60 sec)
$ctr + c   (Device should reboot after heartbeat timeout)

Changes since V1:

 - Rebase changes on top of v3.8-rc6.
 - Make single patch for wdt DT node and its OF_DEV_AUXDATA entry changes.
 - Fix wdt to machine restart with DT.

Kumar, Anil (2):
  ARM: davinci: da850: add wdt DT node
  ARM: davinci: restart: fix wdt to machine restart with DT

 arch/arm/boot/dts/da850-evm.dts            |    3 +++
 arch/arm/boot/dts/da850.dtsi               |    5 +++++
 arch/arm/mach-davinci/da8xx-dt.c           |    8 +++++++-
 arch/arm/mach-davinci/devices-da8xx.c      |   14 ++++++++++++--
 arch/arm/mach-davinci/include/mach/da8xx.h |    1 -
 5 files changed, 27 insertions(+), 4 deletions(-)

-- 
1.7.4.1

[PATCH V2 1/2] ARM: davinci: da850: add wdt DT node

[Kumar, Anil]

Add da850 wdt DT node.
Add OF_DEV_AUXDATA for wdt driver to use wdt clock.

Signed-off-by: Kumar, Anil <anilkumar.v@ti.com>
---
Changes for V2:

 -Make single patch for wdt DT node and its OF_DEV_AUXDATA entry changes.

:100644 100644 37dc5a3... 4666ce9... M	arch/arm/boot/dts/da850-evm.dts
:100644 100644 640ab75... ffba00f... M	arch/arm/boot/dts/da850.dtsi
:100644 100644 37c27af... 01521ed... M	arch/arm/mach-davinci/da8xx-dt.c
 arch/arm/boot/dts/da850-evm.dts  |    3 +++
 arch/arm/boot/dts/da850.dtsi     |    5 +++++
 arch/arm/mach-davinci/da8xx-dt.c |    8 +++++++-
 3 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts
index 37dc5a3..4666ce9 100644
--- a/arch/arm/boot/dts/da850-evm.dts
+++ b/arch/arm/boot/dts/da850-evm.dts
@@ -24,5 +24,8 @@
 		serial2: serial@1d0d000 {
 			status = "okay";
 		};
+		wdt: wdt@1c21000 {
+			status = "okay";
+		};
 	};
 };
diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index 640ab75..ffba00f 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -56,5 +56,10 @@
 			interrupt-parent = <&intc>;
 			status = "disabled";
 		};
+		wdt: wdt@1c21000 {
+			compatible = "ti,davinci-wdt";
+			reg = <0x21000 0x1000>;
+			status = "disabled";
+		};
 	};
 };
diff --git a/arch/arm/mach-davinci/da8xx-dt.c b/arch/arm/mach-davinci/da8xx-dt.c
index 37c27af..01521ed 100644
--- a/arch/arm/mach-davinci/da8xx-dt.c
+++ b/arch/arm/mach-davinci/da8xx-dt.c
@@ -37,11 +37,17 @@ static void __init da8xx_init_irq(void)
 	of_irq_init(da8xx_irq_match);
 }
 
+struct of_dev_auxdata da850_auxdata_lookup[] __initdata = {
+	OF_DEV_AUXDATA("ti,davinci-wdt", 0x01c21000, "watchdog", NULL),
+	{}
+};
+
 #ifdef CONFIG_ARCH_DAVINCI_DA850
 
 static void __init da850_init_machine(void)
 {
-	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
+	of_platform_populate(NULL, of_default_bus_match_table,
+			da850_auxdata_lookup, NULL);
 
 	da8xx_uart_clk_enable();
 }
-- 
1.7.4.1

[PATCH V2 2/2] ARM: davinci: restart: fix wdt to machine restart with DT

[Kumar, Anil]

In non DT case da8xx_register_watchdog() is called to register platform device
"da8xx_wdt_device" by board file. But in DT case it is not called and wdt
device get registered via wdt DT node.

Currently code is passing platform device "da8xx_wdt_device" in
"davinci_watchdog_reset" function in both DT and non DT case and that
leads to crash in DT boot.

Update restart function to make it generic DaVinci machine restart
in case of DT and non DT boot.

Signed-off-by: Kumar, Anil <anilkumar.v@ti.com>
---
:100644 100644 2d5502d... 1df68fd... M	arch/arm/mach-davinci/devices-da8xx.c
:100644 100644 700d311... ef9f70e... M	arch/arm/mach-davinci/include/mach/da8xx.h
 arch/arm/mach-davinci/devices-da8xx.c      |   14 ++++++++++++--
 arch/arm/mach-davinci/include/mach/da8xx.h |    1 -
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
index 2d5502d..1df68fd 100644
--- a/arch/arm/mach-davinci/devices-da8xx.c
+++ b/arch/arm/mach-davinci/devices-da8xx.c
@@ -359,7 +359,7 @@ static struct resource da8xx_watchdog_resources[] = {
 	},
 };
 
-struct platform_device da8xx_wdt_device = {
+static struct platform_device da8xx_wdt_device = {
 	.name		= "watchdog",
 	.id		= -1,
 	.num_resources	= ARRAY_SIZE(da8xx_watchdog_resources),
@@ -368,7 +368,17 @@ struct platform_device da8xx_wdt_device = {
 
 void da8xx_restart(char mode, const char *cmd)
 {
-	davinci_watchdog_reset(&da8xx_wdt_device);
+	struct device *dev = NULL;
+	struct platform_device *wdt_device = NULL;
+
+	dev = bus_find_device_by_name(&platform_bus_type, NULL, "watchdog");
+	if (!dev) {
+		pr_err("wdt device not found to machine reboot\n");
+		return;
+	}
+
+	wdt_device = to_platform_device(dev);
+	davinci_watchdog_reset(wdt_device);
 }
 
 int __init da8xx_register_watchdog(void)
diff --git a/arch/arm/mach-davinci/include/mach/da8xx.h b/arch/arm/mach-davinci/include/mach/da8xx.h
index 700d311..ef9f70e 100644
--- a/arch/arm/mach-davinci/include/mach/da8xx.h
+++ b/arch/arm/mach-davinci/include/mach/da8xx.h
@@ -112,7 +112,6 @@ extern struct da8xx_lcdc_platform_data sharp_lcd035q3dg01_pdata;
 extern struct da8xx_lcdc_platform_data sharp_lk043t1dg01_pdata;
 extern struct davinci_spi_platform_data da8xx_spi_pdata[];
 
-extern struct platform_device da8xx_wdt_device;
 
 extern const short da830_emif25_pins[];
 extern const short da830_spi0_pins[];
-- 
1.7.4.1

Re: [PATCH V2 1/2] ARM: davinci: da850: add wdt DT node

[Sekhar Nori]

On 2/6/2013 9:30 AM, Kumar, Anil wrote:
> Add da850 wdt DT node.
> Add OF_DEV_AUXDATA for wdt driver to use wdt clock.
> 
> Signed-off-by: Kumar, Anil <anilkumar.v@ti.com>

> --- a/arch/arm/mach-davinci/da8xx-dt.c
> +++ b/arch/arm/mach-davinci/da8xx-dt.c
> @@ -37,11 +37,17 @@ static void __init da8xx_init_irq(void)
>  	of_irq_init(da8xx_irq_match);
>  }
>  
> +struct of_dev_auxdata da850_auxdata_lookup[] __initdata = {
> +	OF_DEV_AUXDATA("ti,davinci-wdt", 0x01c21000, "watchdog", NULL),
> +	{}
> +};
> +
>  #ifdef CONFIG_ARCH_DAVINCI_DA850
>  
>  static void __init da850_init_machine(void)
>  {
> -	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
> +	of_platform_populate(NULL, of_default_bus_match_table,
> +			da850_auxdata_lookup, NULL);

These hunks clashed with Manish's I2C0 node addition. I resolved it
locally. I pushed the commit to v3.9/dt-2 branch of my gitorious tree.
Please use this as the baseline for further DT patches.

Thanks,
Sekhar

Re: [PATCH V2 2/2] ARM: davinci: restart: fix wdt to machine restart with DT

[Sekhar Nori]

On 2/6/2013 9:30 AM, Kumar, Anil wrote:
> In non DT case da8xx_register_watchdog() is called to register platform device
> "da8xx_wdt_device" by board file. But in DT case it is not called and wdt
> device get registered via wdt DT node.
> 
> Currently code is passing platform device "da8xx_wdt_device" in
> "davinci_watchdog_reset" function in both DT and non DT case and that
> leads to crash in DT boot.
> 
> Update restart function to make it generic DaVinci machine restart

s/DaVinci/da8xx. Also, subject can simply be "ARM: davinci: da850 DT:
add support for machine reboot" since reboot never worked with DT
before, "fix" isn't really appropriate.

> in case of DT and non DT boot.
> 
> Signed-off-by: Kumar, Anil <anilkumar.v@ti.com>
> ---
> :100644 100644 2d5502d... 1df68fd... M	arch/arm/mach-davinci/devices-da8xx.c
> :100644 100644 700d311... ef9f70e... M	arch/arm/mach-davinci/include/mach/da8xx.h
>  arch/arm/mach-davinci/devices-da8xx.c      |   14 ++++++++++++--
>  arch/arm/mach-davinci/include/mach/da8xx.h |    1 -
>  2 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
> index 2d5502d..1df68fd 100644
> --- a/arch/arm/mach-davinci/devices-da8xx.c
> +++ b/arch/arm/mach-davinci/devices-da8xx.c
> @@ -359,7 +359,7 @@ static struct resource da8xx_watchdog_resources[] = {
>  	},
>  };
>  
> -struct platform_device da8xx_wdt_device = {
> +static struct platform_device da8xx_wdt_device = {

Making of da8xx_wdt_device static should find a mention in description.

>  	.name		= "watchdog",
>  	.id		= -1,
>  	.num_resources	= ARRAY_SIZE(da8xx_watchdog_resources),
> @@ -368,7 +368,17 @@ struct platform_device da8xx_wdt_device = {
>  
>  void da8xx_restart(char mode, const char *cmd)
>  {
> -	davinci_watchdog_reset(&da8xx_wdt_device);
> +	struct device *dev = NULL;
> +	struct platform_device *wdt_device = NULL;

No need to initialize these variables here.

> +
> +	dev = bus_find_device_by_name(&platform_bus_type, NULL, "watchdog");
> +	if (!dev) {
> +		pr_err("wdt device not found to machine reboot\n");

Rather: "%s: failed to find watchdog device", __func__

Thanks,
Sekhar

RE: [PATCH V2 2/2] ARM: davinci: restart: fix wdt to machine restart with DT

[Kumar, Anil]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2789 bytes --]

Hi Sekhar,

Thanks for the review.

On Thu, Feb 07, 2013 at 23:45:53, Nori, Sekhar wrote:
> 
> On 2/6/2013 9:30 AM, Kumar, Anil wrote:
> > In non DT case da8xx_register_watchdog() is called to register platform device
> > "da8xx_wdt_device" by board file. But in DT case it is not called and wdt
> > device get registered via wdt DT node.
> > 
> > Currently code is passing platform device "da8xx_wdt_device" in
> > "davinci_watchdog_reset" function in both DT and non DT case and that
> > leads to crash in DT boot.
> > 
> > Update restart function to make it generic DaVinci machine restart
> 
> s/DaVinci/da8xx. Also, subject can simply be "ARM: davinci: da850 DT:
> add support for machine reboot" since reboot never worked with DT
> before, "fix" isn't really appropriate.

ok

> 
> > in case of DT and non DT boot.
> > 
> > Signed-off-by: Kumar, Anil <anilkumar.v@ti.com>
> > ---
> > :100644 100644 2d5502d... 1df68fd... M	arch/arm/mach-davinci/devices-da8xx.c
> > :100644 100644 700d311... ef9f70e... M	arch/arm/mach-davinci/include/mach/da8xx.h
> >  arch/arm/mach-davinci/devices-da8xx.c      |   14 ++++++++++++--
> >  arch/arm/mach-davinci/include/mach/da8xx.h |    1 -
> >  2 files changed, 12 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
> > index 2d5502d..1df68fd 100644
> > --- a/arch/arm/mach-davinci/devices-da8xx.c
> > +++ b/arch/arm/mach-davinci/devices-da8xx.c
> > @@ -359,7 +359,7 @@ static struct resource da8xx_watchdog_resources[] = {
> >  	},
> >  };
> >  
> > -struct platform_device da8xx_wdt_device = {
> > +static struct platform_device da8xx_wdt_device = {
> 
> Making of da8xx_wdt_device static should find a mention in description.

Ok, 
I made da8xx_wdt_device structure static as it is used only
in file. I will update description for this.

> 
> >  	.name		= "watchdog",
> >  	.id		= -1,
> >  	.num_resources	= ARRAY_SIZE(da8xx_watchdog_resources),
> > @@ -368,7 +368,17 @@ struct platform_device da8xx_wdt_device = {
> >  
> >  void da8xx_restart(char mode, const char *cmd)
> >  {
> > -	davinci_watchdog_reset(&da8xx_wdt_device);
> > +	struct device *dev = NULL;
> > +	struct platform_device *wdt_device = NULL;
> 
> No need to initialize these variables here.

Ok,
I think no need to define wdt_device, can be used as

davinci_watchdog_reset(to_platform_device(dev));

> 
> > +
> > +	dev = bus_find_device_by_name(&platform_bus_type, NULL, "watchdog");
> > +	if (!dev) {
> > +		pr_err("wdt device not found to machine reboot\n");
> 
> Rather: "%s: failed to find watchdog device", __func__

ok
Thanks,
Anil
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH V2 2/2] ARM: davinci: restart: fix wdt to machine restart with DT

[Sekhar Nori]

On 2/8/2013 8:34 AM, Kumar, Anil wrote:

> On Thu, Feb 07, 2013 at 23:45:53, Nori, Sekhar wrote:

>> On 2/6/2013 9:30 AM, Kumar, Anil wrote:

>>> diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
>>> index 2d5502d..1df68fd 100644
>>> --- a/arch/arm/mach-davinci/devices-da8xx.c
>>> +++ b/arch/arm/mach-davinci/devices-da8xx.c
>>> @@ -359,7 +359,7 @@ static struct resource da8xx_watchdog_resources[] = {
>>>  	},
>>>  };
>>>  
>>> -struct platform_device da8xx_wdt_device = {
>>> +static struct platform_device da8xx_wdt_device = {
>>
>> Making of da8xx_wdt_device static should find a mention in description.
> 
> Ok, 
> I made da8xx_wdt_device structure static as it is used only
> in file. I will update description for this.
> 
>>
>>>  	.name		= "watchdog",
>>>  	.id		= -1,
>>>  	.num_resources	= ARRAY_SIZE(da8xx_watchdog_resources),
>>> @@ -368,7 +368,17 @@ struct platform_device da8xx_wdt_device = {
>>>  
>>>  void da8xx_restart(char mode, const char *cmd)
>>>  {
>>> -	davinci_watchdog_reset(&da8xx_wdt_device);
>>> +	struct device *dev = NULL;
>>> +	struct platform_device *wdt_device = NULL;
>>
>> No need to initialize these variables here.
> 
> Ok,
> I think no need to define wdt_device, can be used as
> 
> davinci_watchdog_reset(to_platform_device(dev));
> 
>>
>>> +
>>> +	dev = bus_find_device_by_name(&platform_bus_type, NULL, "watchdog");
>>> +	if (!dev) {
>>> +		pr_err("wdt device not found to machine reboot\n");
>>
>> Rather: "%s: failed to find watchdog device", __func__
> 
> ok

Since time is short and I really want to be able to reboot
when using DT in v3.9, I went ahead and fixed these comments.
Updated patch follows. I tested it on DA850 EVM.

Thanks,
Sekhar

----8<----
>From 19c7c0d81a9b1fe7342d0208b629b5e085e4f3ae Mon Sep 17 00:00:00 2001
From: "Kumar, Anil" <anilkumar.v@ti.com>
Date: Wed, 6 Feb 2013 09:30:04 +0530
Subject: [PATCH 3/3] ARM: davinci: da850 DT: add support for machine reboot

Update the da8xx_restart() function to support machine reboot
when booting using DT. Have the function search for the watchdog
platform device instead of always using "da8xx_wdt_device" which
is never registered in DT case.

While at it, also make da8xx_wdt_device static since there is no need
for it to be globally visible.

Tested machine reboot with both DT and non-DT boot on DA850 EVM.

Signed-off-by: Kumar, Anil <anilkumar.v@ti.com>
Signed-off-by: Sekhar Nori <nsekhar@ti.com>
---
 arch/arm/mach-davinci/devices-da8xx.c      |   12 ++++++++++--
 arch/arm/mach-davinci/include/mach/da8xx.h |    1 -
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
index 2d5502d..cc6916d 100644
--- a/arch/arm/mach-davinci/devices-da8xx.c
+++ b/arch/arm/mach-davinci/devices-da8xx.c
@@ -359,7 +359,7 @@ static struct resource da8xx_watchdog_resources[] = {
 	},
 };
 
-struct platform_device da8xx_wdt_device = {
+static struct platform_device da8xx_wdt_device = {
 	.name		= "watchdog",
 	.id		= -1,
 	.num_resources	= ARRAY_SIZE(da8xx_watchdog_resources),
@@ -368,7 +368,15 @@ struct platform_device da8xx_wdt_device = {
 
 void da8xx_restart(char mode, const char *cmd)
 {
-	davinci_watchdog_reset(&da8xx_wdt_device);
+	struct device *dev;
+
+	dev = bus_find_device_by_name(&platform_bus_type, NULL, "watchdog");
+	if (!dev) {
+		pr_err("%s: failed to find watchdog device\n", __func__);
+		return;
+	}
+
+	davinci_watchdog_reset(to_platform_device(dev));
 }
 
 int __init da8xx_register_watchdog(void)
diff --git a/arch/arm/mach-davinci/include/mach/da8xx.h b/arch/arm/mach-davinci/include/mach/da8xx.h
index 700d311..ef9f70e 100644
--- a/arch/arm/mach-davinci/include/mach/da8xx.h
+++ b/arch/arm/mach-davinci/include/mach/da8xx.h
@@ -112,7 +112,6 @@ extern struct da8xx_lcdc_platform_data sharp_lcd035q3dg01_pdata;
 extern struct da8xx_lcdc_platform_data sharp_lk043t1dg01_pdata;
 extern struct davinci_spi_platform_data da8xx_spi_pdata[];
 
-extern struct platform_device da8xx_wdt_device;
 
 extern const short da830_emif25_pins[];
 extern const short da830_spi0_pins[];