[PATCH] ar7: register watchdog driver only if enabled in hardware configuration

[PATCH] ar7: register watchdog driver only if enabled in hardware configuration

[Florian Fainelli]

This patch checks if the watchdog enable bit is set in the DCL
register meaning that the hardware watchdog actually works and
if so, register the ar7_wdt platform_device.

Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
diff --git a/arch/mips/ar7/platform.c b/arch/mips/ar7/platform.c
index e2278c0..835f3f0 100644
--- a/arch/mips/ar7/platform.c
+++ b/arch/mips/ar7/platform.c
@@ -503,6 +503,7 @@ static int __init ar7_register_devices(void)
 {
 	u16 chip_id;
 	int res;
+	u32 *bootcr, val;
 #ifdef CONFIG_SERIAL_8250
 	static struct uart_port uart_port[2];
 
@@ -595,7 +596,13 @@ static int __init ar7_register_devices(void)
 
 	ar7_wdt_res.end = ar7_wdt_res.start + 0x20;
 
-	res = platform_device_register(&ar7_wdt);
+	bootcr = (u32 *)ioremap_nocache(AR7_REGS_DCL, 4);
+	val = *bootcr;
+	iounmap(bootcr);
+
+	/* Register watchdog only if enabled in hardware */
+	if (val & AR7_WDT_HW_ENA)
+		res = platform_device_register(&ar7_wdt);
 
 	return res;
 }
diff --git a/arch/mips/include/asm/mach-ar7/ar7.h b/arch/mips/include/asm/mach-ar7/ar7.h
index de71694..21cbbc7 100644
--- a/arch/mips/include/asm/mach-ar7/ar7.h
+++ b/arch/mips/include/asm/mach-ar7/ar7.h
@@ -78,6 +78,9 @@
 #define AR7_REF_CLOCK	25000000
 #define AR7_XTAL_CLOCK	24000000
 
+/* DCL */
+#define AR7_WDT_HW_ENA	0x10
+
 struct plat_cpmac_data {
 	int reset_bit;
 	int power_bit;

Re: [PATCH] ar7: register watchdog driver only if enabled in hardware configuration

[Alexander Clouter]

Florian Fainelli <florian@openwrt.org> wrote:
>
> This patch checks if the watchdog enable bit is set in the DCL
> register meaning that the hardware watchdog actually works and
> if so, register the ar7_wdt platform_device.
> 
> Signed-off-by: Florian Fainelli <florian@openwrt.org>
> ---
> diff --git a/arch/mips/ar7/platform.c b/arch/mips/ar7/platform.c
> index e2278c0..835f3f0 100644
> --- a/arch/mips/ar7/platform.c
> +++ b/arch/mips/ar7/platform.c
> @@ -503,6 +503,7 @@ static int __init ar7_register_devices(void)
> {
>        u16 chip_id;
>        int res;
> +       u32 *bootcr, val;
> #ifdef CONFIG_SERIAL_8250
>        static struct uart_port uart_port[2];
> 
> @@ -595,7 +596,13 @@ static int __init ar7_register_devices(void)
> 
>        ar7_wdt_res.end = ar7_wdt_res.start + 0x20;
> 
> -       res = platform_device_register(&ar7_wdt);
> +       bootcr = (u32 *)ioremap_nocache(AR7_REGS_DCL, 4);
> +       val = *bootcr;
> +       iounmap(bootcr);
> +
> +       /* Register watchdog only if enabled in hardware */
> +       if (val & AR7_WDT_HW_ENA)
> +               res = platform_device_register(&ar7_wdt);
>
I think the 'correct' way to do this is:
---
void __iomem *bootcr;
u32 val;

...

bootcr = ioremap_nocache(AR7_REGS_DCL, 4);
val = *bootcr;
iounmap(bootcr);
---

I'm betting this could be reduced to:
---
if (ioread32(AR7_REGS_DCL) & AR7_WDT_HW_ENA)
	res = platform_device_register(&ar7_wdt);
---

However without the hardware...I cannot test this.

Cheers

-- 
Alexander Clouter
.sigmonster says: Accuracy, n.:
                  	The vice of being right

Re: [PATCH] ar7: register watchdog driver only if enabled in hardware configuration

[Alexander Clouter]

Florian Fainelli <florian@openwrt.org> wrote:
>
> This patch checks if the watchdog enable bit is set in the DCL
> register meaning that the hardware watchdog actually works and
> if so, register the ar7_wdt platform_device.
> 
> Signed-off-by: Florian Fainelli <florian@openwrt.org>
> ---
> diff --git a/arch/mips/ar7/platform.c b/arch/mips/ar7/platform.c
> index e2278c0..835f3f0 100644
> --- a/arch/mips/ar7/platform.c
> +++ b/arch/mips/ar7/platform.c
> @@ -503,6 +503,7 @@ static int __init ar7_register_devices(void)
> {
>        u16 chip_id;
>        int res;
> +       u32 *bootcr, val;
> #ifdef CONFIG_SERIAL_8250
>        static struct uart_port uart_port[2];
> 
> @@ -595,7 +596,13 @@ static int __init ar7_register_devices(void)
> 
>        ar7_wdt_res.end = ar7_wdt_res.start + 0x20;
> 
> -       res = platform_device_register(&ar7_wdt);
> +       bootcr = (u32 *)ioremap_nocache(AR7_REGS_DCL, 4);
> +       val = *bootcr;
> +       iounmap(bootcr);
> +
> +       /* Register watchdog only if enabled in hardware */
> +       if (val & AR7_WDT_HW_ENA)
> +               res = platform_device_register(&ar7_wdt);
> 
>        return res;
> }
>
'res' can now return NULL[1].  Solved if you do:
----
int res = -ENODEV;
----

I'm guessing this is the most apprioate?

Cheers

[1] I cannot see the full file annoyingly, it's not in my linux-mips 
	git tree.

-- 
Alexander Clouter
.sigmonster says: It doesn't matter whether you win or lose -- until you lose.

Re: [PATCH] ar7: register watchdog driver only if enabled in hardware configuration

[Sergei Shtylyov]

Hello.

Alexander Clouter wrote:

>> This patch checks if the watchdog enable bit is set in the DCL
>> register meaning that the hardware watchdog actually works and
>> if so, register the ar7_wdt platform_device.
>>
>> Signed-off-by: Florian Fainelli <florian@openwrt.org>
>> ---
>> diff --git a/arch/mips/ar7/platform.c b/arch/mips/ar7/platform.c
>> index e2278c0..835f3f0 100644
>> --- a/arch/mips/ar7/platform.c
>> +++ b/arch/mips/ar7/platform.c
>> @@ -503,6 +503,7 @@ static int __init ar7_register_devices(void)
>> {
>>        u16 chip_id;
>>        int res;
>> +       u32 *bootcr, val;
>> #ifdef CONFIG_SERIAL_8250
>>        static struct uart_port uart_port[2];
>>
>> @@ -595,7 +596,13 @@ static int __init ar7_register_devices(void)
>>
>>        ar7_wdt_res.end = ar7_wdt_res.start + 0x20;
>>
>> -       res = platform_device_register(&ar7_wdt);
>> +       bootcr = (u32 *)ioremap_nocache(AR7_REGS_DCL, 4);
>> +       val = *bootcr;
>> +       iounmap(bootcr);
>> +
>> +       /* Register watchdog only if enabled in hardware */
>> +       if (val & AR7_WDT_HW_ENA)
>> +               res = platform_device_register(&ar7_wdt);
>>
>>     
> I think the 'correct' way to do this is:
> ---
> void __iomem *bootcr;
> u32 val;
>
> ...
>
> bootcr = ioremap_nocache(AR7_REGS_DCL, 4);
> val = *bootcr;
>   

  Wait, you can't dereference a pointer to void...

WBR, Sergei

Re: [PATCH] ar7: register watchdog driver only if enabled in hardware configuration

[Alexander Clouter]

Hi,

* Sergei Shtylyov <sshtylyov@ru.mvista.com> [2009-08-05 12:19:07+0400]:
> 
> Hello.
> 
> Alexander Clouter wrote:
> 
> > I think the 'correct' way to do this is:
> > ---
> > void __iomem *bootcr;
> > u32 val;
> > 
> > ...
> > 
> > bootcr = ioremap_nocache(AR7_REGS_DCL, 4);
> > val = *bootcr;
> >   
> 
> Wait, you can't dereference a pointer to void...
> 
bah,
----
val = __raw_readl(bootcr);
----

Cheers

-- 
Alexander Clouter
.sigmonster says: If you keep anything long enough, you can throw it away.

Re: [PATCH] ar7: register watchdog driver only if enabled in hardware configuration

[Florian Fainelli]

Hey Alexander,

Le Wednesday 05 August 2009 10:00:35 Alexander Clouter, vous avez écrit :
> Florian Fainelli <florian@openwrt.org> wrote:
> > This patch checks if the watchdog enable bit is set in the DCL
> > register meaning that the hardware watchdog actually works and
> > if so, register the ar7_wdt platform_device.
> >
> > Signed-off-by: Florian Fainelli <florian@openwrt.org>
> > ---
> > diff --git a/arch/mips/ar7/platform.c b/arch/mips/ar7/platform.c
> > index e2278c0..835f3f0 100644
> > --- a/arch/mips/ar7/platform.c
> > +++ b/arch/mips/ar7/platform.c
> > @@ -503,6 +503,7 @@ static int __init ar7_register_devices(void)
> > {
> >        u16 chip_id;
> >        int res;
> > +       u32 *bootcr, val;
> > #ifdef CONFIG_SERIAL_8250
> >        static struct uart_port uart_port[2];
> >
> > @@ -595,7 +596,13 @@ static int __init ar7_register_devices(void)
> >
> >        ar7_wdt_res.end = ar7_wdt_res.start + 0x20;
> >
> > -       res = platform_device_register(&ar7_wdt);
> > +       bootcr = (u32 *)ioremap_nocache(AR7_REGS_DCL, 4);
> > +       val = *bootcr;
> > +       iounmap(bootcr);
> > +
> > +       /* Register watchdog only if enabled in hardware */
> > +       if (val & AR7_WDT_HW_ENA)
> > +               res = platform_device_register(&ar7_wdt);
> >
> >        return res;
> > }
>
> 'res' can now return NULL[1].  Solved if you do:
> ----
> int res = -ENODEV;
> ----
>
> I'm guessing this is the most apprioate?

I prefer letting this as-is, since if the watchdog was not enabled in 
hardware, we will not register the watchdog driver, and return the last 
platform_device_register call.

>
> Cheers
>
> [1] I cannot see the full file annoyingly, it's not in my linux-mips
> 	git tree.

Not sure which tree you checked out, but it is in linux-queue: 
http://www.linux-mips.org/git?p=linux-queue.git;a=tree;f=arch/mips/ar7;h=40ee7382dfabf05ee6c967016e42f94992655c20;hb=HEAD
-- 
Best regards, Florian Fainelli
Email: florian@openwrt.org
Web: http://openwrt.org
IRC: [florian] on irc.freenode.net
-------------------------------

Re: [PATCH] ar7: register watchdog driver only if enabled in hardware configuration

[Florian Fainelli]

Ralf,

Any comments on that patch ?

Thanks !

On Tuesday 04 August 2009 23:09:36 Florian Fainelli wrote:
> This patch checks if the watchdog enable bit is set in the DCL
> register meaning that the hardware watchdog actually works and
> if so, register the ar7_wdt platform_device.
>
> Signed-off-by: Florian Fainelli <florian@openwrt.org>
> ---
> diff --git a/arch/mips/ar7/platform.c b/arch/mips/ar7/platform.c
> index e2278c0..835f3f0 100644
> --- a/arch/mips/ar7/platform.c
> +++ b/arch/mips/ar7/platform.c
> @@ -503,6 +503,7 @@ static int __init ar7_register_devices(void)
>  {
>  	u16 chip_id;
>  	int res;
> +	u32 *bootcr, val;
>  #ifdef CONFIG_SERIAL_8250
>  	static struct uart_port uart_port[2];
>
> @@ -595,7 +596,13 @@ static int __init ar7_register_devices(void)
>
>  	ar7_wdt_res.end = ar7_wdt_res.start + 0x20;
>
> -	res = platform_device_register(&ar7_wdt);
> +	bootcr = (u32 *)ioremap_nocache(AR7_REGS_DCL, 4);
> +	val = *bootcr;
> +	iounmap(bootcr);
> +
> +	/* Register watchdog only if enabled in hardware */
> +	if (val & AR7_WDT_HW_ENA)
> +		res = platform_device_register(&ar7_wdt);
>
>  	return res;
>  }
> diff --git a/arch/mips/include/asm/mach-ar7/ar7.h
> b/arch/mips/include/asm/mach-ar7/ar7.h index de71694..21cbbc7 100644
> --- a/arch/mips/include/asm/mach-ar7/ar7.h
> +++ b/arch/mips/include/asm/mach-ar7/ar7.h
> @@ -78,6 +78,9 @@
>  #define AR7_REF_CLOCK	25000000
>  #define AR7_XTAL_CLOCK	24000000
>
> +/* DCL */
> +#define AR7_WDT_HW_ENA	0x10
> +
>  struct plat_cpmac_data {
>  	int reset_bit;
>  	int power_bit;