[PATCH 2/2] ar7_wdt: convert to become a platform driver

[PATCH 2/2] ar7_wdt: convert to become a platform driver

[Florian Fainelli]

This patch converts the ar7_wdt driver to become a platform
driver. The AR7 SoC specific identification and base register
calculation is performed by the board code, therefore we no
longer need to have access to ar7_chip_id.

Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
diff --git a/drivers/watchdog/ar7_wdt.c b/drivers/watchdog/ar7_wdt.c
index bfce332..9c2c4e7 100644
--- a/drivers/watchdog/ar7_wdt.c
+++ b/drivers/watchdog/ar7_wdt.c
@@ -28,6 +28,7 @@
 #include <linux/errno.h>
 #include <linux/init.h>
 #include <linux/miscdevice.h>
+#include <linux/platform_device.h>
 #include <linux/watchdog.h>
 #include <linux/notifier.h>
 #include <linux/reboot.h>
@@ -76,24 +77,10 @@ static unsigned expect_close;
 /* XXX currently fixed, allows max margin ~68.72 secs */
 #define prescale_value 0xffff
 
-/* Offset of the WDT registers */
-static unsigned long ar7_regs_wdt;
+/* Resource of the WDT registers */
+static struct resource *ar7_regs_wdt;
 /* Pointer to the remapped WDT IO space */
 static struct ar7_wdt *ar7_wdt;
-static void ar7_wdt_get_regs(void)
-{
-	u16 chip_id = ar7_chip_id();
-	switch (chip_id) {
-	case AR7_CHIP_7100:
-	case AR7_CHIP_7200:
-		ar7_regs_wdt = AR7_REGS_WDT;
-		break;
-	default:
-		ar7_regs_wdt = UR8_REGS_WDT;
-		break;
-	}
-}
-
 
 static void ar7_wdt_kick(u32 value)
 {
@@ -298,22 +285,33 @@ static struct miscdevice ar7_wdt_miscdev = {
 	.fops		= &ar7_wdt_fops,
 };
 
-static int __init ar7_wdt_init(void)
+static int __init ar7_wdt_probe(struct platform_device *pdev)
 {
 	int rc;
 
 	spin_lock_init(&wdt_lock);
 
-	ar7_wdt_get_regs();
+	ar7_regs_wdt =
+		platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
+	if (!ar7_regs_wdt) {
+		printk(KERN_ERR DRVNAME ": could not get registers resource\n");
+		rc = -ENODEV;
+		goto out;
+	}
 
-	if (!request_mem_region(ar7_regs_wdt, sizeof(struct ar7_wdt),
-							LONGNAME)) {
+	if (!request_mem_region(ar7_regs_wdt->start,
+				resource_size(ar7_regs_wdt), LONGNAME)) {
 		printk(KERN_WARNING DRVNAME ": watchdog I/O region busy\n");
-		return -EBUSY;
+		rc = -EBUSY;
+		goto out;
 	}
 
-	ar7_wdt = (struct ar7_wdt *)
-			ioremap(ar7_regs_wdt, sizeof(struct ar7_wdt));
+	ar7_wdt = ioremap(ar7_regs_wdt->start, resource_size(ar7_regs_wdt));
+	if (!ar7_wdt) {
+		printk(KERN_ERR DRVNAME ": could not ioremap registers\n");
+		rc = -ENXIO;
+		goto out;
+	}
 
 	ar7_wdt_disable_wdt();
 	ar7_wdt_prescale(prescale_value);
@@ -337,17 +335,35 @@ out_register:
 	unregister_reboot_notifier(&ar7_wdt_notifier);
 out_alloc:
 	iounmap(ar7_wdt);
-	release_mem_region(ar7_regs_wdt, sizeof(struct ar7_wdt));
+	release_mem_region(ar7_regs_wdt->start, resource_size(ar7_regs_wdt));
 out:
 	return rc;
 }
 
-static void __exit ar7_wdt_cleanup(void)
+static int __devexit ar7_wdt_remove(struct platform_device *pdev)
 {
 	misc_deregister(&ar7_wdt_miscdev);
 	unregister_reboot_notifier(&ar7_wdt_notifier);
 	iounmap(ar7_wdt);
-	release_mem_region(ar7_regs_wdt, sizeof(struct ar7_wdt));
+	release_mem_region(ar7_regs_wdt->start, resource_size(ar7_regs_wdt));
+
+	return 0;
+}
+
+static struct platform_driver ar7_wdt_driver = {
+	.driver.name = "ar7_wdt",
+	.probe = ar7_wdt_probe,
+	.remove = __devexit_p(ar7_wdt_remove),
+};
+
+static int __init ar7_wdt_init(void)
+{
+	return platform_driver_register(&ar7_wdt_driver);
+}
+
+static void __exit ar7_wdt_cleanup(void)
+{
+	platform_driver_unregister(&ar7_wdt_driver);
 }
 
 module_init(ar7_wdt_init);

Re: [PATCH 2/2] ar7_wdt: convert to become a platform driver

[Wim Van Sebroeck]

Hi Florian,

> This patch converts the ar7_wdt driver to become a platform
> driver. The AR7 SoC specific identification and base register
> calculation is performed by the board code, therefore we no
> longer need to have access to ar7_chip_id.

> @@ -298,22 +285,33 @@ static struct miscdevice ar7_wdt_miscdev = {
>  	.fops		= &ar7_wdt_fops,
>  };
>  
> -static int __init ar7_wdt_init(void)
> +static int __init ar7_wdt_probe(struct platform_device *pdev)

Should be __devinit .

> +static struct platform_driver ar7_wdt_driver = {
> +	.driver.name = "ar7_wdt",
> +	.probe = ar7_wdt_probe,
> +	.remove = __devexit_p(ar7_wdt_remove),
> +};

I prefer to have it as follows (so that the driver.owner field is also set):
static struct platform_driver ar7_wdt_driver = {
	.probe = ar7_wdt_probe,
	.remove = __devexit_p(ar7_wdt_remove),
	.driver = {
		.owner = THIS_MODULE,
		.name = "ar7_wdt",
	},
};

I suggest to also change the reboot notifier code into a platform shutdown method.
You then get something like the attached patch (untested, uncompiled and I included above 2 remarks).
For the rest: code is OK for me. After the __init to __devinit fix you can add my signed-off-by.

Kind regards,
Wim.

Re: [PATCH 2/2] ar7_wdt: convert to become a platform driver

[Wim Van Sebroeck]

[-- Attachment #1: Type: text/plain, Size: 1340 bytes --]

Hi Florian,

Forgot the attachment...

Kind regards,
Wim.

> Hi Florian,
> 
> > This patch converts the ar7_wdt driver to become a platform
> > driver. The AR7 SoC specific identification and base register
> > calculation is performed by the board code, therefore we no
> > longer need to have access to ar7_chip_id.
> 
> > @@ -298,22 +285,33 @@ static struct miscdevice ar7_wdt_miscdev = {
> >  	.fops		= &ar7_wdt_fops,
> >  };
> >  
> > -static int __init ar7_wdt_init(void)
> > +static int __init ar7_wdt_probe(struct platform_device *pdev)
> 
> Should be __devinit .
> 
> > +static struct platform_driver ar7_wdt_driver = {
> > +	.driver.name = "ar7_wdt",
> > +	.probe = ar7_wdt_probe,
> > +	.remove = __devexit_p(ar7_wdt_remove),
> > +};
> 
> I prefer to have it as follows (so that the driver.owner field is also set):
> static struct platform_driver ar7_wdt_driver = {
> 	.probe = ar7_wdt_probe,
> 	.remove = __devexit_p(ar7_wdt_remove),
> 	.driver = {
> 		.owner = THIS_MODULE,
> 		.name = "ar7_wdt",
> 	},
> };
> 
> I suggest to also change the reboot notifier code into a platform shutdown method.
> You then get something like the attached patch (untested, uncompiled and I included above 2 remarks).
> For the rest: code is OK for me. After the __init to __devinit fix you can add my signed-off-by.
> 
> Kind regards,
> Wim.
> 

[-- Attachment #2: ar7_wdt.diff --]
[-- Type: text/x-diff, Size: 2301 bytes --]

--- ar7_wdt.c.orig	2009-07-18 20:54:33.000000000 +0200
+++ ar7_wdt.c	2009-07-18 21:06:00.000000000 +0200
@@ -30,8 +30,6 @@
 #include <linux/miscdevice.h>
 #include <linux/platform_device.h>
 #include <linux/watchdog.h>
-#include <linux/notifier.h>
-#include <linux/reboot.h>
 #include <linux/fs.h>
 #include <linux/ioport.h>
 #include <linux/io.h>
@@ -189,20 +187,6 @@
 	return 0;
 }
 
-static int ar7_wdt_notify_sys(struct notifier_block *this,
-			      unsigned long code, void *unused)
-{
-	if (code == SYS_HALT || code == SYS_POWER_OFF)
-		if (!nowayout)
-			ar7_wdt_disable_wdt();
-
-	return NOTIFY_DONE;
-}
-
-static struct notifier_block ar7_wdt_notifier = {
-	.notifier_call = ar7_wdt_notify_sys,
-};
-
 static ssize_t ar7_wdt_write(struct file *file, const char *data,
 			     size_t len, loff_t *ppos)
 {
@@ -286,7 +270,7 @@
 	.fops		= &ar7_wdt_fops,
 };
 
-static int __init ar7_wdt_probe(struct platform_device *pdev)
+static int __devinit ar7_wdt_probe(struct platform_device *pdev)
 {
 	int rc;
 
@@ -318,22 +302,13 @@
 	ar7_wdt_prescale(prescale_value);
 	ar7_wdt_update_margin(margin);
 
-	rc = register_reboot_notifier(&ar7_wdt_notifier);
-	if (rc) {
-		printk(KERN_ERR DRVNAME
-			": unable to register reboot notifier\n");
-		goto out_alloc;
-	}
-
 	rc = misc_register(&ar7_wdt_miscdev);
 	if (rc) {
 		printk(KERN_ERR DRVNAME ": unable to register misc device\n");
-		goto out_register;
+		goto out_alloc;
 	}
 	goto out;
 
-out_register:
-	unregister_reboot_notifier(&ar7_wdt_notifier);
 out_alloc:
 	iounmap(ar7_wdt);
 	release_mem_region(ar7_regs_wdt->start, resource_size(ar7_regs_wdt));
@@ -344,17 +319,26 @@
 static int __devexit ar7_wdt_remove(struct platform_device *pdev)
 {
 	misc_deregister(&ar7_wdt_miscdev);
-	unregister_reboot_notifier(&ar7_wdt_notifier);
 	iounmap(ar7_wdt);
 	release_mem_region(ar7_regs_wdt->start, resource_size(ar7_regs_wdt));
 
 	return 0;
 }
 
+static void ar7_wdt_shutdown(struct platform_device *pdev)
+{
+	if (!nowayout)
+		ar7_wdt_disable_wdt();
+}
+
 static struct platform_driver ar7_wdt_driver = {
-	.driver.name = "ar7_wdt",
 	.probe = ar7_wdt_probe,
 	.remove = __devexit_p(ar7_wdt_remove),
+	.shutdown = ar7_wdt_shutdown,
+	.driver = {
+		.owner = THIS_MODULE,
+		.name = "ar7_wdt",
+	},
 };
 
 static int __init ar7_wdt_init(void)

Re: [PATCH 2/2] ar7_wdt: convert to become a platform driver

[Florian Fainelli]

Hi Wim,

Le Saturday 18 July 2009 23:49:58 Wim Van Sebroeck, vous avez écrit :
> Hi Florian,
>
> Forgot the attachment...
>
> Kind regards,
> Wim.
>
> > Hi Florian,
> >
> > > This patch converts the ar7_wdt driver to become a platform
> > > driver. The AR7 SoC specific identification and base register
> > > calculation is performed by the board code, therefore we no
> > > longer need to have access to ar7_chip_id.
> > >
> > > @@ -298,22 +285,33 @@ static struct miscdevice ar7_wdt_miscdev = {
> > >  	.fops		= &ar7_wdt_fops,
> > >  };
> > >
> > > -static int __init ar7_wdt_init(void)
> > > +static int __init ar7_wdt_probe(struct platform_device *pdev)
> >
> > Should be __devinit .
> >
> > > +static struct platform_driver ar7_wdt_driver = {
> > > +	.driver.name = "ar7_wdt",
> > > +	.probe = ar7_wdt_probe,
> > > +	.remove = __devexit_p(ar7_wdt_remove),
> > > +};
> >
> > I prefer to have it as follows (so that the driver.owner field is also
> > set): static struct platform_driver ar7_wdt_driver = {
> > 	.probe = ar7_wdt_probe,
> > 	.remove = __devexit_p(ar7_wdt_remove),
> > 	.driver = {
> > 		.owner = THIS_MODULE,
> > 		.name = "ar7_wdt",
> > 	},
> > };
> >
> > I suggest to also change the reboot notifier code into a platform
> > shutdown method. You then get something like the attached patch
> > (untested, uncompiled and I included above 2 remarks). For the rest: code
> > is OK for me. After the __init to __devinit fix you can add my
> > signed-off-by.

Thanks, patch updated and attached.

> >
> > Kind regards,
> > Wim.
--
From: Florian Fainelli <florian@openwrt.org>
Subject: [PATCH 2/2 v2] ar7_wdt: convert to become a platform driver

This patch converts the ar7_wdt driver to become
a platform driver. The AR7 SoC specific identification
and base register calculation is performed by the board
code, therefore we no longer need to have access to
ar7_chip_id. We also remove the reboot notifier code to
use the platform shutdown method as Wim suggested.

Signed-off-by: Florian Fainelli <florian@openwrt.org>
Signed-off-by: Wim Van Sebroeck <wim@iguana.be>
--
diff --git a/drivers/watchdog/ar7_wdt.c b/drivers/watchdog/ar7_wdt.c
index 2f8643e..82855b0 100644
--- a/drivers/watchdog/ar7_wdt.c
+++ b/drivers/watchdog/ar7_wdt.c
@@ -28,9 +28,8 @@
 #include <linux/errno.h>
 #include <linux/init.h>
 #include <linux/miscdevice.h>
+#include <linux/platform_device.h>
 #include <linux/watchdog.h>
-#include <linux/notifier.h>
-#include <linux/reboot.h>
 #include <linux/fs.h>
 #include <linux/ioport.h>
 #include <linux/io.h>
@@ -76,24 +75,10 @@ static unsigned expect_close;
 /* XXX currently fixed, allows max margin ~68.72 secs */
 #define prescale_value 0xffff
 
-/* Offset of the WDT registers */
-static unsigned long ar7_regs_wdt;
+/* Resource of the WDT registers */
+static struct resource *ar7_regs_wdt;
 /* Pointer to the remapped WDT IO space */
 static struct ar7_wdt *ar7_wdt;
-static void ar7_wdt_get_regs(void)
-{
-	u16 chip_id = ar7_chip_id();
-	switch (chip_id) {
-	case AR7_CHIP_7100:
-	case AR7_CHIP_7200:
-		ar7_regs_wdt = AR7_REGS_WDT;
-		break;
-	default:
-		ar7_regs_wdt = UR8_REGS_WDT;
-		break;
-	}
-}
-
 
 static void ar7_wdt_kick(u32 value)
 {
@@ -202,20 +187,6 @@ static int ar7_wdt_release(struct inode *inode, struct file *file)
 	return 0;
 }
 
-static int ar7_wdt_notify_sys(struct notifier_block *this,
-			      unsigned long code, void *unused)
-{
-	if (code == SYS_HALT || code == SYS_POWER_OFF)
-		if (!nowayout)
-			ar7_wdt_disable_wdt();
-
-	return NOTIFY_DONE;
-}
-
-static struct notifier_block ar7_wdt_notifier = {
-	.notifier_call = ar7_wdt_notify_sys,
-};
-
 static ssize_t ar7_wdt_write(struct file *file, const char *data,
 			     size_t len, loff_t *ppos)
 {
@@ -299,56 +270,85 @@ static struct miscdevice ar7_wdt_miscdev = {
 	.fops		= &ar7_wdt_fops,
 };
 
-static int __init ar7_wdt_init(void)
+static int __devinit ar7_wdt_probe(struct platform_device *pdev)
 {
 	int rc;
 
 	spin_lock_init(&wdt_lock);
 
-	ar7_wdt_get_regs();
+	ar7_regs_wdt =
+		platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
+	if (!ar7_regs_wdt) {
+		printk(KERN_ERR DRVNAME ": could not get registers resource\n");
+		rc = -ENODEV;
+		goto out;
+	}
 
-	if (!request_mem_region(ar7_regs_wdt, sizeof(struct ar7_wdt),
-							LONGNAME)) {
+	if (!request_mem_region(ar7_regs_wdt->start,
+				resource_size(ar7_regs_wdt), LONGNAME)) {
 		printk(KERN_WARNING DRVNAME ": watchdog I/O region busy\n");
-		return -EBUSY;
+		rc = -EBUSY;
+		goto out;
 	}
 
-	ar7_wdt = (struct ar7_wdt *)
-			ioremap(ar7_regs_wdt, sizeof(struct ar7_wdt));
+	ar7_wdt = ioremap(ar7_regs_wdt->start, resource_size(ar7_regs_wdt));
+	if (!ar7_wdt) {
+		printk(KERN_ERR DRVNAME ": could not ioremap registers\n");
+		rc = -ENXIO;
+		goto out;
+	}
 
 	ar7_wdt_disable_wdt();
 	ar7_wdt_prescale(prescale_value);
 	ar7_wdt_update_margin(margin);
 
-	rc = register_reboot_notifier(&ar7_wdt_notifier);
-	if (rc) {
-		printk(KERN_ERR DRVNAME
-			": unable to register reboot notifier\n");
-		goto out_alloc;
-	}
-
 	rc = misc_register(&ar7_wdt_miscdev);
 	if (rc) {
 		printk(KERN_ERR DRVNAME ": unable to register misc device\n");
-		goto out_register;
+		goto out_alloc;
 	}
 	goto out;
 
-out_register:
-	unregister_reboot_notifier(&ar7_wdt_notifier);
 out_alloc:
 	iounmap(ar7_wdt);
-	release_mem_region(ar7_regs_wdt, sizeof(struct ar7_wdt));
+	release_mem_region(ar7_regs_wdt->start, resource_size(ar7_regs_wdt));
 out:
 	return rc;
 }
 
-static void __exit ar7_wdt_cleanup(void)
+static int __devexit ar7_wdt_remove(struct platform_device *pdev)
 {
 	misc_deregister(&ar7_wdt_miscdev);
-	unregister_reboot_notifier(&ar7_wdt_notifier);
 	iounmap(ar7_wdt);
-	release_mem_region(ar7_regs_wdt, sizeof(struct ar7_wdt));
+	release_mem_region(ar7_regs_wdt->start, resource_size(ar7_regs_wdt));
+
+	return 0;
+}
+
+static void ar7_wdt_shutdown(struct platform_device *pdev)
+{
+	if (!nowayout)
+		ar7_wdt_disable_wdt();
+}
+
+static struct platform_driver ar7_wdt_driver = {
+	.probe = ar7_wdt_probe,
+	.remove = __devexit_p(ar7_wdt_remove),
+	.shutdown = ar7_wdt_shutdown,
+	.driver = {
+		.owner = THIS_MODULE,
+		.name = "ar7_wdt",
+	},
+};
+
+static int __init ar7_wdt_init(void)
+{
+	return platform_driver_register(&ar7_wdt_driver);
+}
+
+static void __exit ar7_wdt_cleanup(void)
+{
+	platform_driver_unregister(&ar7_wdt_driver);
 }
 
 module_init(ar7_wdt_init);

Re: [PATCH 2/2] ar7_wdt: convert to become a platform driver

[Florian Fainelli]

Hi Wim,

Le Tuesday 21 July 2009 12:11:32 Florian Fainelli, vous avez écrit :
> Hi Wim,
>
> Le Saturday 18 July 2009 23:49:58 Wim Van Sebroeck, vous avez écrit :
> > Hi Florian,
> >
> > Forgot the attachment...
> >
> > Kind regards,
> > Wim.
> >
> > > Hi Florian,
> > >
> > > > This patch converts the ar7_wdt driver to become a platform
> > > > driver. The AR7 SoC specific identification and base register
> > > > calculation is performed by the board code, therefore we no
> > > > longer need to have access to ar7_chip_id.
> > > >
> > > > @@ -298,22 +285,33 @@ static struct miscdevice ar7_wdt_miscdev = {
> > > >  	.fops		= &ar7_wdt_fops,
> > > >  };
> > > >
> > > > -static int __init ar7_wdt_init(void)
> > > > +static int __init ar7_wdt_probe(struct platform_device *pdev)
> > >
> > > Should be __devinit .
> > >
> > > > +static struct platform_driver ar7_wdt_driver = {
> > > > +	.driver.name = "ar7_wdt",
> > > > +	.probe = ar7_wdt_probe,
> > > > +	.remove = __devexit_p(ar7_wdt_remove),
> > > > +};
> > >
> > > I prefer to have it as follows (so that the driver.owner field is also
> > > set): static struct platform_driver ar7_wdt_driver = {
> > > 	.probe = ar7_wdt_probe,
> > > 	.remove = __devexit_p(ar7_wdt_remove),
> > > 	.driver = {
> > > 		.owner = THIS_MODULE,
> > > 		.name = "ar7_wdt",
> > > 	},
> > > };
> > >
> > > I suggest to also change the reboot notifier code into a platform
> > > shutdown method. You then get something like the attached patch
> > > (untested, uncompiled and I included above 2 remarks). For the rest:
> > > code is OK for me. After the __init to __devinit fix you can add my
> > > signed-off-by.
>
> Thanks, patch updated and attached.
>
> > > Kind regards,
> > > Wim.
>
> --
> From: Florian Fainelli <florian@openwrt.org>
> Subject: [PATCH 2/2 v2] ar7_wdt: convert to become a platform driver
>
> This patch converts the ar7_wdt driver to become
> a platform driver. The AR7 SoC specific identification
> and base register calculation is performed by the board
> code, therefore we no longer need to have access to
> ar7_chip_id. We also remove the reboot notifier code to
> use the platform shutdown method as Wim suggested.
>
> Signed-off-by: Florian Fainelli <florian@openwrt.org>
> Signed-off-by: Wim Van Sebroeck <wim@iguana.be>

Any news on this patch ?

> --
> diff --git a/drivers/watchdog/ar7_wdt.c b/drivers/watchdog/ar7_wdt.c
> index 2f8643e..82855b0 100644
> --- a/drivers/watchdog/ar7_wdt.c
> +++ b/drivers/watchdog/ar7_wdt.c
> @@ -28,9 +28,8 @@
>  #include <linux/errno.h>
>  #include <linux/init.h>
>  #include <linux/miscdevice.h>
> +#include <linux/platform_device.h>
>  #include <linux/watchdog.h>
> -#include <linux/notifier.h>
> -#include <linux/reboot.h>
>  #include <linux/fs.h>
>  #include <linux/ioport.h>
>  #include <linux/io.h>
> @@ -76,24 +75,10 @@ static unsigned expect_close;
>  /* XXX currently fixed, allows max margin ~68.72 secs */
>  #define prescale_value 0xffff
>
> -/* Offset of the WDT registers */
> -static unsigned long ar7_regs_wdt;
> +/* Resource of the WDT registers */
> +static struct resource *ar7_regs_wdt;
>  /* Pointer to the remapped WDT IO space */
>  static struct ar7_wdt *ar7_wdt;
> -static void ar7_wdt_get_regs(void)
> -{
> -	u16 chip_id = ar7_chip_id();
> -	switch (chip_id) {
> -	case AR7_CHIP_7100:
> -	case AR7_CHIP_7200:
> -		ar7_regs_wdt = AR7_REGS_WDT;
> -		break;
> -	default:
> -		ar7_regs_wdt = UR8_REGS_WDT;
> -		break;
> -	}
> -}
> -
>
>  static void ar7_wdt_kick(u32 value)
>  {
> @@ -202,20 +187,6 @@ static int ar7_wdt_release(struct inode *inode, struct
> file *file) return 0;
>  }
>
> -static int ar7_wdt_notify_sys(struct notifier_block *this,
> -			      unsigned long code, void *unused)
> -{
> -	if (code == SYS_HALT || code == SYS_POWER_OFF)
> -		if (!nowayout)
> -			ar7_wdt_disable_wdt();
> -
> -	return NOTIFY_DONE;
> -}
> -
> -static struct notifier_block ar7_wdt_notifier = {
> -	.notifier_call = ar7_wdt_notify_sys,
> -};
> -
>  static ssize_t ar7_wdt_write(struct file *file, const char *data,
>  			     size_t len, loff_t *ppos)
>  {
> @@ -299,56 +270,85 @@ static struct miscdevice ar7_wdt_miscdev = {
>  	.fops		= &ar7_wdt_fops,
>  };
>
> -static int __init ar7_wdt_init(void)
> +static int __devinit ar7_wdt_probe(struct platform_device *pdev)
>  {
>  	int rc;
>
>  	spin_lock_init(&wdt_lock);
>
> -	ar7_wdt_get_regs();
> +	ar7_regs_wdt =
> +		platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
> +	if (!ar7_regs_wdt) {
> +		printk(KERN_ERR DRVNAME ": could not get registers resource\n");
> +		rc = -ENODEV;
> +		goto out;
> +	}
>
> -	if (!request_mem_region(ar7_regs_wdt, sizeof(struct ar7_wdt),
> -							LONGNAME)) {
> +	if (!request_mem_region(ar7_regs_wdt->start,
> +				resource_size(ar7_regs_wdt), LONGNAME)) {
>  		printk(KERN_WARNING DRVNAME ": watchdog I/O region busy\n");
> -		return -EBUSY;
> +		rc = -EBUSY;
> +		goto out;
>  	}
>
> -	ar7_wdt = (struct ar7_wdt *)
> -			ioremap(ar7_regs_wdt, sizeof(struct ar7_wdt));
> +	ar7_wdt = ioremap(ar7_regs_wdt->start, resource_size(ar7_regs_wdt));
> +	if (!ar7_wdt) {
> +		printk(KERN_ERR DRVNAME ": could not ioremap registers\n");
> +		rc = -ENXIO;
> +		goto out;
> +	}
>
>  	ar7_wdt_disable_wdt();
>  	ar7_wdt_prescale(prescale_value);
>  	ar7_wdt_update_margin(margin);
>
> -	rc = register_reboot_notifier(&ar7_wdt_notifier);
> -	if (rc) {
> -		printk(KERN_ERR DRVNAME
> -			": unable to register reboot notifier\n");
> -		goto out_alloc;
> -	}
> -
>  	rc = misc_register(&ar7_wdt_miscdev);
>  	if (rc) {
>  		printk(KERN_ERR DRVNAME ": unable to register misc device\n");
> -		goto out_register;
> +		goto out_alloc;
>  	}
>  	goto out;
>
> -out_register:
> -	unregister_reboot_notifier(&ar7_wdt_notifier);
>  out_alloc:
>  	iounmap(ar7_wdt);
> -	release_mem_region(ar7_regs_wdt, sizeof(struct ar7_wdt));
> +	release_mem_region(ar7_regs_wdt->start, resource_size(ar7_regs_wdt));
>  out:
>  	return rc;
>  }
>
> -static void __exit ar7_wdt_cleanup(void)
> +static int __devexit ar7_wdt_remove(struct platform_device *pdev)
>  {
>  	misc_deregister(&ar7_wdt_miscdev);
> -	unregister_reboot_notifier(&ar7_wdt_notifier);
>  	iounmap(ar7_wdt);
> -	release_mem_region(ar7_regs_wdt, sizeof(struct ar7_wdt));
> +	release_mem_region(ar7_regs_wdt->start, resource_size(ar7_regs_wdt));
> +
> +	return 0;
> +}
> +
> +static void ar7_wdt_shutdown(struct platform_device *pdev)
> +{
> +	if (!nowayout)
> +		ar7_wdt_disable_wdt();
> +}
> +
> +static struct platform_driver ar7_wdt_driver = {
> +	.probe = ar7_wdt_probe,
> +	.remove = __devexit_p(ar7_wdt_remove),
> +	.shutdown = ar7_wdt_shutdown,
> +	.driver = {
> +		.owner = THIS_MODULE,
> +		.name = "ar7_wdt",
> +	},
> +};
> +
> +static int __init ar7_wdt_init(void)
> +{
> +	return platform_driver_register(&ar7_wdt_driver);
> +}
> +
> +static void __exit ar7_wdt_cleanup(void)
> +{
> +	platform_driver_unregister(&ar7_wdt_driver);
>  }
>
>  module_init(ar7_wdt_init);



-- 
Best regards, Florian Fainelli
Email: florian@openwrt.org
Web: http://openwrt.org
IRC: [florian] on irc.freenode.net
-------------------------------

Re: [PATCH 2/2] ar7_wdt: convert to become a platform driver

[Wim Van Sebroeck]

Hi Florian,

> > From: Florian Fainelli <florian@openwrt.org>
> > Subject: [PATCH 2/2 v2] ar7_wdt: convert to become a platform driver
> >
> > This patch converts the ar7_wdt driver to become
> > a platform driver. The AR7 SoC specific identification
> > and base register calculation is performed by the board
> > code, therefore we no longer need to have access to
> > ar7_chip_id. We also remove the reboot notifier code to
> > use the platform shutdown method as Wim suggested.
> >
> > Signed-off-by: Florian Fainelli <florian@openwrt.org>
> > Signed-off-by: Wim Van Sebroeck <wim@iguana.be>
> 
> Any news on this patch ?

This one was ok for me. I think we agreed that Ralf would take it up in his tree.
I can also take it up in my next tree still.

Ralf, did you add the patch allready or will I do it?

Thanks in advance,
Kind regards,
Wim.

Re: [PATCH 2/2] ar7_wdt: convert to become a platform driver

[Florian Fainelli]

Le Tuesday 11 August 2009 15:01:33 Wim Van Sebroeck, vous avez écrit :
> Hi Florian,
>
> > > From: Florian Fainelli <florian@openwrt.org>
> > > Subject: [PATCH 2/2 v2] ar7_wdt: convert to become a platform driver
> > >
> > > This patch converts the ar7_wdt driver to become
> > > a platform driver. The AR7 SoC specific identification
> > > and base register calculation is performed by the board
> > > code, therefore we no longer need to have access to
> > > ar7_chip_id. We also remove the reboot notifier code to
> > > use the platform shutdown method as Wim suggested.
> > >
> > > Signed-off-by: Florian Fainelli <florian@openwrt.org>
> > > Signed-off-by: Wim Van Sebroeck <wim@iguana.be>
> >
> > Any news on this patch ?
>
> This one was ok for me. I think we agreed that Ralf would take it up in his
> tree. I can also take it up in my next tree still.

Oh, I did not understand that sorry, I thought Ralf would take the first one 
which is MIPS-specific.
-- 
Best regards, Florian Fainelli
Email: florian@openwrt.org
Web: http://openwrt.org
IRC: [florian] on irc.freenode.net
-------------------------------

Re: [PATCH 2/2] ar7_wdt: convert to become a platform driver

[Wim Van Sebroeck]

[-- Attachment #1: Type: text/plain, Size: 1082 bytes --]

Hi Florian,

> > > > From: Florian Fainelli <florian@openwrt.org>
> > > > Subject: [PATCH 2/2 v2] ar7_wdt: convert to become a platform driver
> > > >
> > > > This patch converts the ar7_wdt driver to become
> > > > a platform driver. The AR7 SoC specific identification
> > > > and base register calculation is performed by the board
> > > > code, therefore we no longer need to have access to
> > > > ar7_chip_id. We also remove the reboot notifier code to
> > > > use the platform shutdown method as Wim suggested.
> > > >
> > > > Signed-off-by: Florian Fainelli <florian@openwrt.org>
> > > > Signed-off-by: Wim Van Sebroeck <wim@iguana.be>
> > >
> > > Any news on this patch ?
> >
> > This one was ok for me. I think we agreed that Ralf would take it up in his
> > tree. I can also take it up in my next tree still.
> 
> Oh, I did not understand that sorry, I thought Ralf would take the first one 
> which is MIPS-specific.

I added the second patch to my tree, but saw that the error handling on probe could be improved.
Can you test attached patch?

Thanks in advance,
Wim.


[-- Attachment #2: ar7_wdt-fix-probe-errors.diff --]
[-- Type: text/x-diff, Size: 642 bytes --]

diff --git a/drivers/watchdog/ar7_wdt.c b/drivers/watchdog/ar7_wdt.c
index 82855b0..2e94b71 100644
--- a/drivers/watchdog/ar7_wdt.c
+++ b/drivers/watchdog/ar7_wdt.c
@@ -295,7 +295,7 @@ static int __devinit ar7_wdt_probe(struct platform_device *pdev)
 	if (!ar7_wdt) {
 		printk(KERN_ERR DRVNAME ": could not ioremap registers\n");
 		rc = -ENXIO;
-		goto out;
+		goto out_mem_region;
 	}
 
 	ar7_wdt_disable_wdt();
@@ -311,6 +311,7 @@ static int __devinit ar7_wdt_probe(struct platform_device *pdev)
 
 out_alloc:
 	iounmap(ar7_wdt);
+out_mem_region:
 	release_mem_region(ar7_regs_wdt->start, resource_size(ar7_regs_wdt));
 out:
 	return rc;

Re: [PATCH 2/2] ar7_wdt: convert to become a platform driver

[Florian Fainelli]

Le jeudi 27 août 2009 22:47:19, Wim Van Sebroeck a écrit :
> Hi Florian,
>
> > > > > From: Florian Fainelli <florian@openwrt.org>
> > > > > Subject: [PATCH 2/2 v2] ar7_wdt: convert to become a platform
> > > > > driver
> > > > >
> > > > > This patch converts the ar7_wdt driver to become
> > > > > a platform driver. The AR7 SoC specific identification
> > > > > and base register calculation is performed by the board
> > > > > code, therefore we no longer need to have access to
> > > > > ar7_chip_id. We also remove the reboot notifier code to
> > > > > use the platform shutdown method as Wim suggested.
> > > > >
> > > > > Signed-off-by: Florian Fainelli <florian@openwrt.org>
> > > > > Signed-off-by: Wim Van Sebroeck <wim@iguana.be>
> > > >
> > > > Any news on this patch ?
> > >
> > > This one was ok for me. I think we agreed that Ralf would take it up in
> > > his tree. I can also take it up in my next tree still.
> >
> > Oh, I did not understand that sorry, I thought Ralf would take the first
> > one which is MIPS-specific.
>
> I added the second patch to my tree, but saw that the error handling on
> probe could be improved. Can you test attached patch?

Works fine. Feel free to add my Tested-by: Florian Fainelli <florian@openwrt.org>. Thanks !
-- 
Best regards, Florian Fainelli
Email: florian@openwrt.org
Web: http://openwrt.org
IRC: [florian] on irc.freenode.net
-------------------------------