diff mbox

[01/11] watchdog/at91sam9_wdt: remove the file_operations struct

Message ID 1352877369-19740-2-git-send-email-wenyou.yang@atmel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Wenyou Yang Nov. 14, 2012, 7:15 a.m. UTC
Remove the file_operations struct and miscdevice struct for future
add to use the watchdog framework.

Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
---
 drivers/watchdog/at91sam9_wdt.c |  131 ---------------------------------------
 1 file changed, 131 deletions(-)

Comments

Jean-Christophe PLAGNIOL-VILLARD Nov. 16, 2012, 1:54 p.m. UTC | #1
On 15:15 Wed 14 Nov     , Wenyou Yang wrote:
> Remove the file_operations struct and miscdevice struct for future
> add to use the watchdog framework.
NACK

you break the watchdog support

you must not break the kernel suport except if it's impossible to do otherwise
here it is no the case

Best Regards,
J.
> 
> Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
> ---
>  drivers/watchdog/at91sam9_wdt.c |  131 ---------------------------------------
>  1 file changed, 131 deletions(-)
> 
> diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c
> index 05e1be8..549c256 100644
> --- a/drivers/watchdog/at91sam9_wdt.c
> +++ b/drivers/watchdog/at91sam9_wdt.c
> @@ -18,11 +18,9 @@
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>  
>  #include <linux/errno.h>
> -#include <linux/fs.h>
>  #include <linux/init.h>
>  #include <linux/io.h>
>  #include <linux/kernel.h>
> -#include <linux/miscdevice.h>
>  #include <linux/module.h>
>  #include <linux/moduleparam.h>
>  #include <linux/platform_device.h>
> @@ -31,7 +29,6 @@
>  #include <linux/jiffies.h>
>  #include <linux/timer.h>
>  #include <linux/bitops.h>
> -#include <linux/uaccess.h>
>  
>  #include "at91sam9_wdt.h"
>  
> @@ -102,35 +99,6 @@ static void at91_ping(unsigned long data)
>  }
>  
>  /*
> - * Watchdog device is opened, and watchdog starts running.
> - */
> -static int at91_wdt_open(struct inode *inode, struct file *file)
> -{
> -	if (test_and_set_bit(0, &at91wdt_private.open))
> -		return -EBUSY;
> -
> -	at91wdt_private.next_heartbeat = jiffies + heartbeat * HZ;
> -	mod_timer(&at91wdt_private.timer, jiffies + WDT_TIMEOUT);
> -
> -	return nonseekable_open(inode, file);
> -}
> -
> -/*
> - * Close the watchdog device.
> - */
> -static int at91_wdt_close(struct inode *inode, struct file *file)
> -{
> -	clear_bit(0, &at91wdt_private.open);
> -
> -	/* stop internal ping */
> -	if (!at91wdt_private.expect_close)
> -		del_timer(&at91wdt_private.timer);
> -
> -	at91wdt_private.expect_close = 0;
> -	return 0;
> -}
> -
> -/*
>   * Set the watchdog time interval in 1/256Hz (write-once)
>   * Counter is 12 bit.
>   */
> @@ -168,101 +136,11 @@ static const struct watchdog_info at91_wdt_info = {
>  						WDIOF_MAGICCLOSE,
>  };
>  
> -/*
> - * Handle commands from user-space.
> - */
> -static long at91_wdt_ioctl(struct file *file,
> -		unsigned int cmd, unsigned long arg)
> -{
> -	void __user *argp = (void __user *)arg;
> -	int __user *p = argp;
> -	int new_value;
> -
> -	switch (cmd) {
> -	case WDIOC_GETSUPPORT:
> -		return copy_to_user(argp, &at91_wdt_info,
> -				    sizeof(at91_wdt_info)) ? -EFAULT : 0;
> -
> -	case WDIOC_GETSTATUS:
> -	case WDIOC_GETBOOTSTATUS:
> -		return put_user(0, p);
> -
> -	case WDIOC_KEEPALIVE:
> -		at91wdt_private.next_heartbeat = jiffies + heartbeat * HZ;
> -		return 0;
> -
> -	case WDIOC_SETTIMEOUT:
> -		if (get_user(new_value, p))
> -			return -EFAULT;
> -
> -		heartbeat = new_value;
> -		at91wdt_private.next_heartbeat = jiffies + heartbeat * HZ;
> -
> -		return put_user(new_value, p);  /* return current value */
> -
> -	case WDIOC_GETTIMEOUT:
> -		return put_user(heartbeat, p);
> -	}
> -	return -ENOTTY;
> -}
> -
> -/*
> - * Pat the watchdog whenever device is written to.
> - */
> -static ssize_t at91_wdt_write(struct file *file, const char *data, size_t len,
> -								loff_t *ppos)
> -{
> -	if (!len)
> -		return 0;
> -
> -	/* Scan for magic character */
> -	if (!nowayout) {
> -		size_t i;
> -
> -		at91wdt_private.expect_close = 0;
> -
> -		for (i = 0; i < len; i++) {
> -			char c;
> -			if (get_user(c, data + i))
> -				return -EFAULT;
> -			if (c == 'V') {
> -				at91wdt_private.expect_close = 42;
> -				break;
> -			}
> -		}
> -	}
> -
> -	at91wdt_private.next_heartbeat = jiffies + heartbeat * HZ;
> -
> -	return len;
> -}
> -
> -/* ......................................................................... */
> -
> -static const struct file_operations at91wdt_fops = {
> -	.owner			= THIS_MODULE,
> -	.llseek			= no_llseek,
> -	.unlocked_ioctl	= at91_wdt_ioctl,
> -	.open			= at91_wdt_open,
> -	.release		= at91_wdt_close,
> -	.write			= at91_wdt_write,
> -};
> -
> -static struct miscdevice at91wdt_miscdev = {
> -	.minor		= WATCHDOG_MINOR,
> -	.name		= "watchdog",
> -	.fops		= &at91wdt_fops,
> -};
> -
>  static int __init at91wdt_probe(struct platform_device *pdev)
>  {
>  	struct resource	*r;
>  	int res;
>  
> -	if (at91wdt_miscdev.parent)
> -		return -EBUSY;
> -	at91wdt_miscdev.parent = &pdev->dev;
> -
>  	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	if (!r)
>  		return -ENODEV;
> @@ -277,10 +155,6 @@ static int __init at91wdt_probe(struct platform_device *pdev)
>  	if (res)
>  		return res;
>  
> -	res = misc_register(&at91wdt_miscdev);
> -	if (res)
> -		return res;
> -
>  	at91wdt_private.next_heartbeat = jiffies + heartbeat * HZ;
>  	setup_timer(&at91wdt_private.timer, at91_ping, 0);
>  	mod_timer(&at91wdt_private.timer, jiffies + WDT_TIMEOUT);
> @@ -295,10 +169,6 @@ static int __exit at91wdt_remove(struct platform_device *pdev)
>  {
>  	int res;
>  
> -	res = misc_deregister(&at91wdt_miscdev);
> -	if (!res)
> -		at91wdt_miscdev.parent = NULL;
> -
>  	return res;
>  }
>  
> @@ -326,4 +196,3 @@ module_exit(at91sam_wdt_exit);
>  MODULE_AUTHOR("Renaud CERRATO <r.cerrato@til-technologies.fr>");
>  MODULE_DESCRIPTION("Watchdog driver for Atmel AT91SAM9x processors");
>  MODULE_LICENSE("GPL");
> -MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
> -- 
> 1.7.9.5
>
Wenyou Yang Dec. 4, 2012, 3:26 a.m. UTC | #2
Hi JC,

> -----Original Message-----

> From: Jean-Christophe PLAGNIOL-VILLARD [mailto:plagnioj@jcrosoft.com]

> Sent: 2012?11?16? 21:54

> To: Yang, Wenyou

> Cc: linux-arm-kernel@lists.infradead.org; wim@iguana.be;

> linux-watchdog@vger.kernel.org; linux-kernel@vger.kernel.org; Ferre, Nicolas; Lin,

> JM

> Subject: Re: [PATCH 01/11] watchdog/at91sam9_wdt: remove the file_operations

> struct

> 

> On 15:15 Wed 14 Nov     , Wenyou Yang wrote:

> > Remove the file_operations struct and miscdevice struct for future

> > add to use the watchdog framework.

> NACK

> 

> you break the watchdog support

> 

> you must not break the kernel suport except if it's impossible to do otherwise

> here it is no the case


Thanks.

But it is said so in the kernel document (Documentation/watchdog/convert_drivers_to_kernel_api.txt).
"the old drivers define their own file_operations for actions like open(), write()etc... These are now handled by the framework"

And the codes in these function is not used under the new framework which is implemented in the framework.
So I only make it in the separated patch to simpler.
> 

> Best Regards,

> J.


Best Regards
Wenyou Yang

> >

> > Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>

> > ---

> >  drivers/watchdog/at91sam9_wdt.c |  131 ---------------------------------------

> >  1 file changed, 131 deletions(-)

> >

> > diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c

> > index 05e1be8..549c256 100644

> > --- a/drivers/watchdog/at91sam9_wdt.c

> > +++ b/drivers/watchdog/at91sam9_wdt.c

> > @@ -18,11 +18,9 @@

> >  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

> >

> >  #include <linux/errno.h>

> > -#include <linux/fs.h>

> >  #include <linux/init.h>

> >  #include <linux/io.h>

> >  #include <linux/kernel.h>

> > -#include <linux/miscdevice.h>

> >  #include <linux/module.h>

> >  #include <linux/moduleparam.h>

> >  #include <linux/platform_device.h>

> > @@ -31,7 +29,6 @@

> >  #include <linux/jiffies.h>

> >  #include <linux/timer.h>

> >  #include <linux/bitops.h>

> > -#include <linux/uaccess.h>

> >

> >  #include "at91sam9_wdt.h"

> >

> > @@ -102,35 +99,6 @@ static void at91_ping(unsigned long data)

> >  }

> >

> >  /*

> > - * Watchdog device is opened, and watchdog starts running.

> > - */

> > -static int at91_wdt_open(struct inode *inode, struct file *file)

> > -{

> > -	if (test_and_set_bit(0, &at91wdt_private.open))

> > -		return -EBUSY;

> > -

> > -	at91wdt_private.next_heartbeat = jiffies + heartbeat * HZ;

> > -	mod_timer(&at91wdt_private.timer, jiffies + WDT_TIMEOUT);

> > -

> > -	return nonseekable_open(inode, file);

> > -}

> > -

> > -/*

> > - * Close the watchdog device.

> > - */

> > -static int at91_wdt_close(struct inode *inode, struct file *file)

> > -{

> > -	clear_bit(0, &at91wdt_private.open);

> > -

> > -	/* stop internal ping */

> > -	if (!at91wdt_private.expect_close)

> > -		del_timer(&at91wdt_private.timer);

> > -

> > -	at91wdt_private.expect_close = 0;

> > -	return 0;

> > -}

> > -

> > -/*

> >   * Set the watchdog time interval in 1/256Hz (write-once)

> >   * Counter is 12 bit.

> >   */

> > @@ -168,101 +136,11 @@ static const struct watchdog_info at91_wdt_info = {

> >  						WDIOF_MAGICCLOSE,

> >  };

> >

> > -/*

> > - * Handle commands from user-space.

> > - */

> > -static long at91_wdt_ioctl(struct file *file,

> > -		unsigned int cmd, unsigned long arg)

> > -{

> > -	void __user *argp = (void __user *)arg;

> > -	int __user *p = argp;

> > -	int new_value;

> > -

> > -	switch (cmd) {

> > -	case WDIOC_GETSUPPORT:

> > -		return copy_to_user(argp, &at91_wdt_info,

> > -				    sizeof(at91_wdt_info)) ? -EFAULT : 0;

> > -

> > -	case WDIOC_GETSTATUS:

> > -	case WDIOC_GETBOOTSTATUS:

> > -		return put_user(0, p);

> > -

> > -	case WDIOC_KEEPALIVE:

> > -		at91wdt_private.next_heartbeat = jiffies + heartbeat * HZ;

> > -		return 0;

> > -

> > -	case WDIOC_SETTIMEOUT:

> > -		if (get_user(new_value, p))

> > -			return -EFAULT;

> > -

> > -		heartbeat = new_value;

> > -		at91wdt_private.next_heartbeat = jiffies + heartbeat * HZ;

> > -

> > -		return put_user(new_value, p);  /* return current value */

> > -

> > -	case WDIOC_GETTIMEOUT:

> > -		return put_user(heartbeat, p);

> > -	}

> > -	return -ENOTTY;

> > -}

> > -

> > -/*

> > - * Pat the watchdog whenever device is written to.

> > - */

> > -static ssize_t at91_wdt_write(struct file *file, const char *data, size_t len,

> > -								loff_t *ppos)

> > -{

> > -	if (!len)

> > -		return 0;

> > -

> > -	/* Scan for magic character */

> > -	if (!nowayout) {

> > -		size_t i;

> > -

> > -		at91wdt_private.expect_close = 0;

> > -

> > -		for (i = 0; i < len; i++) {

> > -			char c;

> > -			if (get_user(c, data + i))

> > -				return -EFAULT;

> > -			if (c == 'V') {

> > -				at91wdt_private.expect_close = 42;

> > -				break;

> > -			}

> > -		}

> > -	}

> > -

> > -	at91wdt_private.next_heartbeat = jiffies + heartbeat * HZ;

> > -

> > -	return len;

> > -}

> > -

> > -/* ......................................................................... */

> > -

> > -static const struct file_operations at91wdt_fops = {

> > -	.owner			= THIS_MODULE,

> > -	.llseek			= no_llseek,

> > -	.unlocked_ioctl	= at91_wdt_ioctl,

> > -	.open			= at91_wdt_open,

> > -	.release		= at91_wdt_close,

> > -	.write			= at91_wdt_write,

> > -};

> > -

> > -static struct miscdevice at91wdt_miscdev = {

> > -	.minor		= WATCHDOG_MINOR,

> > -	.name		= "watchdog",

> > -	.fops		= &at91wdt_fops,

> > -};

> > -

> >  static int __init at91wdt_probe(struct platform_device *pdev)

> >  {

> >  	struct resource	*r;

> >  	int res;

> >

> > -	if (at91wdt_miscdev.parent)

> > -		return -EBUSY;

> > -	at91wdt_miscdev.parent = &pdev->dev;

> > -

> >  	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);

> >  	if (!r)

> >  		return -ENODEV;

> > @@ -277,10 +155,6 @@ static int __init at91wdt_probe(struct platform_device

> *pdev)

> >  	if (res)

> >  		return res;

> >

> > -	res = misc_register(&at91wdt_miscdev);

> > -	if (res)

> > -		return res;

> > -

> >  	at91wdt_private.next_heartbeat = jiffies + heartbeat * HZ;

> >  	setup_timer(&at91wdt_private.timer, at91_ping, 0);

> >  	mod_timer(&at91wdt_private.timer, jiffies + WDT_TIMEOUT);

> > @@ -295,10 +169,6 @@ static int __exit at91wdt_remove(struct platform_device

> *pdev)

> >  {

> >  	int res;

> >

> > -	res = misc_deregister(&at91wdt_miscdev);

> > -	if (!res)

> > -		at91wdt_miscdev.parent = NULL;

> > -

> >  	return res;

> >  }

> >

> > @@ -326,4 +196,3 @@ module_exit(at91sam_wdt_exit);

> >  MODULE_AUTHOR("Renaud CERRATO <r.cerrato@til-technologies.fr>");

> >  MODULE_DESCRIPTION("Watchdog driver for Atmel AT91SAM9x processors");

> >  MODULE_LICENSE("GPL");

> > -MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);

> > --

> > 1.7.9.5

> >
Ludovic Desroches Dec. 4, 2012, 7:59 a.m. UTC | #3
Hi Wenyou,

On 12/04/2012 04:26 AM, Yang, Wenyou wrote:
> Hi JC,
> 
>> -----Original Message-----
>> From: Jean-Christophe PLAGNIOL-VILLARD [mailto:plagnioj@jcrosoft.com]
>> Sent: 2012?11?16? 21:54
>> To: Yang, Wenyou
>> Cc: linux-arm-kernel@lists.infradead.org; wim@iguana.be;
>> linux-watchdog@vger.kernel.org; linux-kernel@vger.kernel.org; Ferre, Nicolas; Lin,
>> JM
>> Subject: Re: [PATCH 01/11] watchdog/at91sam9_wdt: remove the file_operations
>> struct
>>
>> On 15:15 Wed 14 Nov     , Wenyou Yang wrote:
>>> Remove the file_operations struct and miscdevice struct for future
>>> add to use the watchdog framework.
>> NACK
>>
>> you break the watchdog support
>>
>> you must not break the kernel suport except if it's impossible to do otherwise
>> here it is no the case
> 
> Thanks.
> 
> But it is said so in the kernel document (Documentation/watchdog/convert_drivers_to_kernel_api.txt).
> "the old drivers define their own file_operations for actions like open(), write()etc... These are now handled by the framework"
> 
> And the codes in these function is not used under the new framework which is implemented in the framework.

The problem is not about remove this stuff.

> So I only make it in the separated patch to simpler.

That's the point, if this patch is applied alone, the driver will be
broken. In this case don't split your patch.


Regards

Ludovic

>>
>> Best Regards,
>> J.
> 
> Best Regards
> Wenyou Yang
> 
>>>
>>> Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
>>> ---
>>>   drivers/watchdog/at91sam9_wdt.c |  131 ---------------------------------------
>>>   1 file changed, 131 deletions(-)
>>>
>>> diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c
>>> index 05e1be8..549c256 100644
>>> --- a/drivers/watchdog/at91sam9_wdt.c
>>> +++ b/drivers/watchdog/at91sam9_wdt.c
>>> @@ -18,11 +18,9 @@
>>>   #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>>>
>>>   #include <linux/errno.h>
>>> -#include <linux/fs.h>
>>>   #include <linux/init.h>
>>>   #include <linux/io.h>
>>>   #include <linux/kernel.h>
>>> -#include <linux/miscdevice.h>
>>>   #include <linux/module.h>
>>>   #include <linux/moduleparam.h>
>>>   #include <linux/platform_device.h>
>>> @@ -31,7 +29,6 @@
>>>   #include <linux/jiffies.h>
>>>   #include <linux/timer.h>
>>>   #include <linux/bitops.h>
>>> -#include <linux/uaccess.h>
>>>
>>>   #include "at91sam9_wdt.h"
>>>
>>> @@ -102,35 +99,6 @@ static void at91_ping(unsigned long data)
>>>   }
>>>
>>>   /*
>>> - * Watchdog device is opened, and watchdog starts running.
>>> - */
>>> -static int at91_wdt_open(struct inode *inode, struct file *file)
>>> -{
>>> -	if (test_and_set_bit(0, &at91wdt_private.open))
>>> -		return -EBUSY;
>>> -
>>> -	at91wdt_private.next_heartbeat = jiffies + heartbeat * HZ;
>>> -	mod_timer(&at91wdt_private.timer, jiffies + WDT_TIMEOUT);
>>> -
>>> -	return nonseekable_open(inode, file);
>>> -}
>>> -
>>> -/*
>>> - * Close the watchdog device.
>>> - */
>>> -static int at91_wdt_close(struct inode *inode, struct file *file)
>>> -{
>>> -	clear_bit(0, &at91wdt_private.open);
>>> -
>>> -	/* stop internal ping */
>>> -	if (!at91wdt_private.expect_close)
>>> -		del_timer(&at91wdt_private.timer);
>>> -
>>> -	at91wdt_private.expect_close = 0;
>>> -	return 0;
>>> -}
>>> -
>>> -/*
>>>    * Set the watchdog time interval in 1/256Hz (write-once)
>>>    * Counter is 12 bit.
>>>    */
>>> @@ -168,101 +136,11 @@ static const struct watchdog_info at91_wdt_info = {
>>>   						WDIOF_MAGICCLOSE,
>>>   };
>>>
>>> -/*
>>> - * Handle commands from user-space.
>>> - */
>>> -static long at91_wdt_ioctl(struct file *file,
>>> -		unsigned int cmd, unsigned long arg)
>>> -{
>>> -	void __user *argp = (void __user *)arg;
>>> -	int __user *p = argp;
>>> -	int new_value;
>>> -
>>> -	switch (cmd) {
>>> -	case WDIOC_GETSUPPORT:
>>> -		return copy_to_user(argp, &at91_wdt_info,
>>> -				    sizeof(at91_wdt_info)) ? -EFAULT : 0;
>>> -
>>> -	case WDIOC_GETSTATUS:
>>> -	case WDIOC_GETBOOTSTATUS:
>>> -		return put_user(0, p);
>>> -
>>> -	case WDIOC_KEEPALIVE:
>>> -		at91wdt_private.next_heartbeat = jiffies + heartbeat * HZ;
>>> -		return 0;
>>> -
>>> -	case WDIOC_SETTIMEOUT:
>>> -		if (get_user(new_value, p))
>>> -			return -EFAULT;
>>> -
>>> -		heartbeat = new_value;
>>> -		at91wdt_private.next_heartbeat = jiffies + heartbeat * HZ;
>>> -
>>> -		return put_user(new_value, p);  /* return current value */
>>> -
>>> -	case WDIOC_GETTIMEOUT:
>>> -		return put_user(heartbeat, p);
>>> -	}
>>> -	return -ENOTTY;
>>> -}
>>> -
>>> -/*
>>> - * Pat the watchdog whenever device is written to.
>>> - */
>>> -static ssize_t at91_wdt_write(struct file *file, const char *data, size_t len,
>>> -								loff_t *ppos)
>>> -{
>>> -	if (!len)
>>> -		return 0;
>>> -
>>> -	/* Scan for magic character */
>>> -	if (!nowayout) {
>>> -		size_t i;
>>> -
>>> -		at91wdt_private.expect_close = 0;
>>> -
>>> -		for (i = 0; i < len; i++) {
>>> -			char c;
>>> -			if (get_user(c, data + i))
>>> -				return -EFAULT;
>>> -			if (c == 'V') {
>>> -				at91wdt_private.expect_close = 42;
>>> -				break;
>>> -			}
>>> -		}
>>> -	}
>>> -
>>> -	at91wdt_private.next_heartbeat = jiffies + heartbeat * HZ;
>>> -
>>> -	return len;
>>> -}
>>> -
>>> -/* ......................................................................... */
>>> -
>>> -static const struct file_operations at91wdt_fops = {
>>> -	.owner			= THIS_MODULE,
>>> -	.llseek			= no_llseek,
>>> -	.unlocked_ioctl	= at91_wdt_ioctl,
>>> -	.open			= at91_wdt_open,
>>> -	.release		= at91_wdt_close,
>>> -	.write			= at91_wdt_write,
>>> -};
>>> -
>>> -static struct miscdevice at91wdt_miscdev = {
>>> -	.minor		= WATCHDOG_MINOR,
>>> -	.name		= "watchdog",
>>> -	.fops		= &at91wdt_fops,
>>> -};
>>> -
>>>   static int __init at91wdt_probe(struct platform_device *pdev)
>>>   {
>>>   	struct resource	*r;
>>>   	int res;
>>>
>>> -	if (at91wdt_miscdev.parent)
>>> -		return -EBUSY;
>>> -	at91wdt_miscdev.parent = &pdev->dev;
>>> -
>>>   	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>   	if (!r)
>>>   		return -ENODEV;
>>> @@ -277,10 +155,6 @@ static int __init at91wdt_probe(struct platform_device
>> *pdev)
>>>   	if (res)
>>>   		return res;
>>>
>>> -	res = misc_register(&at91wdt_miscdev);
>>> -	if (res)
>>> -		return res;
>>> -
>>>   	at91wdt_private.next_heartbeat = jiffies + heartbeat * HZ;
>>>   	setup_timer(&at91wdt_private.timer, at91_ping, 0);
>>>   	mod_timer(&at91wdt_private.timer, jiffies + WDT_TIMEOUT);
>>> @@ -295,10 +169,6 @@ static int __exit at91wdt_remove(struct platform_device
>> *pdev)
>>>   {
>>>   	int res;
>>>
>>> -	res = misc_deregister(&at91wdt_miscdev);
>>> -	if (!res)
>>> -		at91wdt_miscdev.parent = NULL;
>>> -
>>>   	return res;
>>>   }
>>>
>>> @@ -326,4 +196,3 @@ module_exit(at91sam_wdt_exit);
>>>   MODULE_AUTHOR("Renaud CERRATO <r.cerrato@til-technologies.fr>");
>>>   MODULE_DESCRIPTION("Watchdog driver for Atmel AT91SAM9x processors");
>>>   MODULE_LICENSE("GPL");
>>> -MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
>>> --
>>> 1.7.9.5
>>>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
Wenyou Yang Dec. 4, 2012, 8:07 a.m. UTC | #4
Hi Ludovic,

> -----Original Message-----

> From: Desroches, Ludovic

> Sent: 2012?12?4? 15:59

> To: Yang, Wenyou

> Cc: Jean-Christophe PLAGNIOL-VILLARD; linux-watchdog@vger.kernel.org; Lin, JM;

> Ferre, Nicolas; linux-kernel@vger.kernel.org; wim@iguana.be;

> linux-arm-kernel@lists.infradead.org; Desroches, Ludovic

> Subject: Re: [PATCH 01/11] watchdog/at91sam9_wdt: remove the file_operations

> struct

> 

> Hi Wenyou,

> 

> On 12/04/2012 04:26 AM, Yang, Wenyou wrote:

> > Hi JC,

> >

> >> -----Original Message-----

> >> From: Jean-Christophe PLAGNIOL-VILLARD [mailto:plagnioj@jcrosoft.com]

> >> Sent: 2012?11?16? 21:54

> >> To: Yang, Wenyou

> >> Cc: linux-arm-kernel@lists.infradead.org; wim@iguana.be;

> >> linux-watchdog@vger.kernel.org; linux-kernel@vger.kernel.org; Ferre, Nicolas;

> Lin,

> >> JM

> >> Subject: Re: [PATCH 01/11] watchdog/at91sam9_wdt: remove the file_operations

> >> struct

> >>

> >> On 15:15 Wed 14 Nov     , Wenyou Yang wrote:

> >>> Remove the file_operations struct and miscdevice struct for future

> >>> add to use the watchdog framework.

> >> NACK

> >>

> >> you break the watchdog support

> >>

> >> you must not break the kernel suport except if it's impossible to do otherwise

> >> here it is no the case

> >

> > Thanks.

> >

> > But it is said so in the kernel document

> (Documentation/watchdog/convert_drivers_to_kernel_api.txt).

> > "the old drivers define their own file_operations for actions like open(), write()etc...

> These are now handled by the framework"

> >

> > And the codes in these function is not used under the new framework which is

> implemented in the framework.

> 

> The problem is not about remove this stuff.

> 

> > So I only make it in the separated patch to simpler.

> 

> That's the point, if this patch is applied alone, the driver will be

> broken. In this case don't split your patch.

> 

It makes sense. Thanks
> 

> Regards

> 

> Ludovic

> 

Best Regards
Wenyou Yang


> >>

> >> Best Regards,

> >> J.

> >

> > Best Regards

> > Wenyou Yang

> >

> >>>

> >>> Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>

> >>> ---

> >>>   drivers/watchdog/at91sam9_wdt.c |  131 ---------------------------------------

> >>>   1 file changed, 131 deletions(-)

> >>>

> >>> diff --git a/drivers/watchdog/at91sam9_wdt.c

> b/drivers/watchdog/at91sam9_wdt.c

> >>> index 05e1be8..549c256 100644

> >>> --- a/drivers/watchdog/at91sam9_wdt.c

> >>> +++ b/drivers/watchdog/at91sam9_wdt.c

> >>> @@ -18,11 +18,9 @@

> >>>   #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

> >>>

> >>>   #include <linux/errno.h>

> >>> -#include <linux/fs.h>

> >>>   #include <linux/init.h>

> >>>   #include <linux/io.h>

> >>>   #include <linux/kernel.h>

> >>> -#include <linux/miscdevice.h>

> >>>   #include <linux/module.h>

> >>>   #include <linux/moduleparam.h>

> >>>   #include <linux/platform_device.h>

> >>> @@ -31,7 +29,6 @@

> >>>   #include <linux/jiffies.h>

> >>>   #include <linux/timer.h>

> >>>   #include <linux/bitops.h>

> >>> -#include <linux/uaccess.h>

> >>>

> >>>   #include "at91sam9_wdt.h"

> >>>

> >>> @@ -102,35 +99,6 @@ static void at91_ping(unsigned long data)

> >>>   }

> >>>

> >>>   /*

> >>> - * Watchdog device is opened, and watchdog starts running.

> >>> - */

> >>> -static int at91_wdt_open(struct inode *inode, struct file *file)

> >>> -{

> >>> -	if (test_and_set_bit(0, &at91wdt_private.open))

> >>> -		return -EBUSY;

> >>> -

> >>> -	at91wdt_private.next_heartbeat = jiffies + heartbeat * HZ;

> >>> -	mod_timer(&at91wdt_private.timer, jiffies + WDT_TIMEOUT);

> >>> -

> >>> -	return nonseekable_open(inode, file);

> >>> -}

> >>> -

> >>> -/*

> >>> - * Close the watchdog device.

> >>> - */

> >>> -static int at91_wdt_close(struct inode *inode, struct file *file)

> >>> -{

> >>> -	clear_bit(0, &at91wdt_private.open);

> >>> -

> >>> -	/* stop internal ping */

> >>> -	if (!at91wdt_private.expect_close)

> >>> -		del_timer(&at91wdt_private.timer);

> >>> -

> >>> -	at91wdt_private.expect_close = 0;

> >>> -	return 0;

> >>> -}

> >>> -

> >>> -/*

> >>>    * Set the watchdog time interval in 1/256Hz (write-once)

> >>>    * Counter is 12 bit.

> >>>    */

> >>> @@ -168,101 +136,11 @@ static const struct watchdog_info at91_wdt_info = {

> >>>   						WDIOF_MAGICCLOSE,

> >>>   };

> >>>

> >>> -/*

> >>> - * Handle commands from user-space.

> >>> - */

> >>> -static long at91_wdt_ioctl(struct file *file,

> >>> -		unsigned int cmd, unsigned long arg)

> >>> -{

> >>> -	void __user *argp = (void __user *)arg;

> >>> -	int __user *p = argp;

> >>> -	int new_value;

> >>> -

> >>> -	switch (cmd) {

> >>> -	case WDIOC_GETSUPPORT:

> >>> -		return copy_to_user(argp, &at91_wdt_info,

> >>> -				    sizeof(at91_wdt_info)) ? -EFAULT : 0;

> >>> -

> >>> -	case WDIOC_GETSTATUS:

> >>> -	case WDIOC_GETBOOTSTATUS:

> >>> -		return put_user(0, p);

> >>> -

> >>> -	case WDIOC_KEEPALIVE:

> >>> -		at91wdt_private.next_heartbeat = jiffies + heartbeat * HZ;

> >>> -		return 0;

> >>> -

> >>> -	case WDIOC_SETTIMEOUT:

> >>> -		if (get_user(new_value, p))

> >>> -			return -EFAULT;

> >>> -

> >>> -		heartbeat = new_value;

> >>> -		at91wdt_private.next_heartbeat = jiffies + heartbeat * HZ;

> >>> -

> >>> -		return put_user(new_value, p);  /* return current value */

> >>> -

> >>> -	case WDIOC_GETTIMEOUT:

> >>> -		return put_user(heartbeat, p);

> >>> -	}

> >>> -	return -ENOTTY;

> >>> -}

> >>> -

> >>> -/*

> >>> - * Pat the watchdog whenever device is written to.

> >>> - */

> >>> -static ssize_t at91_wdt_write(struct file *file, const char *data, size_t len,

> >>> -								loff_t *ppos)

> >>> -{

> >>> -	if (!len)

> >>> -		return 0;

> >>> -

> >>> -	/* Scan for magic character */

> >>> -	if (!nowayout) {

> >>> -		size_t i;

> >>> -

> >>> -		at91wdt_private.expect_close = 0;

> >>> -

> >>> -		for (i = 0; i < len; i++) {

> >>> -			char c;

> >>> -			if (get_user(c, data + i))

> >>> -				return -EFAULT;

> >>> -			if (c == 'V') {

> >>> -				at91wdt_private.expect_close = 42;

> >>> -				break;

> >>> -			}

> >>> -		}

> >>> -	}

> >>> -

> >>> -	at91wdt_private.next_heartbeat = jiffies + heartbeat * HZ;

> >>> -

> >>> -	return len;

> >>> -}

> >>> -

> >>> -/* ......................................................................... */

> >>> -

> >>> -static const struct file_operations at91wdt_fops = {

> >>> -	.owner			= THIS_MODULE,

> >>> -	.llseek			= no_llseek,

> >>> -	.unlocked_ioctl	= at91_wdt_ioctl,

> >>> -	.open			= at91_wdt_open,

> >>> -	.release		= at91_wdt_close,

> >>> -	.write			= at91_wdt_write,

> >>> -};

> >>> -

> >>> -static struct miscdevice at91wdt_miscdev = {

> >>> -	.minor		= WATCHDOG_MINOR,

> >>> -	.name		= "watchdog",

> >>> -	.fops		= &at91wdt_fops,

> >>> -};

> >>> -

> >>>   static int __init at91wdt_probe(struct platform_device *pdev)

> >>>   {

> >>>   	struct resource	*r;

> >>>   	int res;

> >>>

> >>> -	if (at91wdt_miscdev.parent)

> >>> -		return -EBUSY;

> >>> -	at91wdt_miscdev.parent = &pdev->dev;

> >>> -

> >>>   	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);

> >>>   	if (!r)

> >>>   		return -ENODEV;

> >>> @@ -277,10 +155,6 @@ static int __init at91wdt_probe(struct platform_device

> >> *pdev)

> >>>   	if (res)

> >>>   		return res;

> >>>

> >>> -	res = misc_register(&at91wdt_miscdev);

> >>> -	if (res)

> >>> -		return res;

> >>> -

> >>>   	at91wdt_private.next_heartbeat = jiffies + heartbeat * HZ;

> >>>   	setup_timer(&at91wdt_private.timer, at91_ping, 0);

> >>>   	mod_timer(&at91wdt_private.timer, jiffies + WDT_TIMEOUT);

> >>> @@ -295,10 +169,6 @@ static int __exit at91wdt_remove(struct

> platform_device

> >> *pdev)

> >>>   {

> >>>   	int res;

> >>>

> >>> -	res = misc_deregister(&at91wdt_miscdev);

> >>> -	if (!res)

> >>> -		at91wdt_miscdev.parent = NULL;

> >>> -

> >>>   	return res;

> >>>   }

> >>>

> >>> @@ -326,4 +196,3 @@ module_exit(at91sam_wdt_exit);

> >>>   MODULE_AUTHOR("Renaud CERRATO <r.cerrato@til-technologies.fr>");

> >>>   MODULE_DESCRIPTION("Watchdog driver for Atmel AT91SAM9x

> processors");

> >>>   MODULE_LICENSE("GPL");

> >>> -MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);

> >>> --

> >>> 1.7.9.5

> >>>

> > _______________________________________________

> > linux-arm-kernel mailing list

> > linux-arm-kernel@lists.infradead.org

> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

> >
diff mbox

Patch

diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c
index 05e1be8..549c256 100644
--- a/drivers/watchdog/at91sam9_wdt.c
+++ b/drivers/watchdog/at91sam9_wdt.c
@@ -18,11 +18,9 @@ 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include <linux/errno.h>
-#include <linux/fs.h>
 #include <linux/init.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
-#include <linux/miscdevice.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
 #include <linux/platform_device.h>
@@ -31,7 +29,6 @@ 
 #include <linux/jiffies.h>
 #include <linux/timer.h>
 #include <linux/bitops.h>
-#include <linux/uaccess.h>
 
 #include "at91sam9_wdt.h"
 
@@ -102,35 +99,6 @@  static void at91_ping(unsigned long data)
 }
 
 /*
- * Watchdog device is opened, and watchdog starts running.
- */
-static int at91_wdt_open(struct inode *inode, struct file *file)
-{
-	if (test_and_set_bit(0, &at91wdt_private.open))
-		return -EBUSY;
-
-	at91wdt_private.next_heartbeat = jiffies + heartbeat * HZ;
-	mod_timer(&at91wdt_private.timer, jiffies + WDT_TIMEOUT);
-
-	return nonseekable_open(inode, file);
-}
-
-/*
- * Close the watchdog device.
- */
-static int at91_wdt_close(struct inode *inode, struct file *file)
-{
-	clear_bit(0, &at91wdt_private.open);
-
-	/* stop internal ping */
-	if (!at91wdt_private.expect_close)
-		del_timer(&at91wdt_private.timer);
-
-	at91wdt_private.expect_close = 0;
-	return 0;
-}
-
-/*
  * Set the watchdog time interval in 1/256Hz (write-once)
  * Counter is 12 bit.
  */
@@ -168,101 +136,11 @@  static const struct watchdog_info at91_wdt_info = {
 						WDIOF_MAGICCLOSE,
 };
 
-/*
- * Handle commands from user-space.
- */
-static long at91_wdt_ioctl(struct file *file,
-		unsigned int cmd, unsigned long arg)
-{
-	void __user *argp = (void __user *)arg;
-	int __user *p = argp;
-	int new_value;
-
-	switch (cmd) {
-	case WDIOC_GETSUPPORT:
-		return copy_to_user(argp, &at91_wdt_info,
-				    sizeof(at91_wdt_info)) ? -EFAULT : 0;
-
-	case WDIOC_GETSTATUS:
-	case WDIOC_GETBOOTSTATUS:
-		return put_user(0, p);
-
-	case WDIOC_KEEPALIVE:
-		at91wdt_private.next_heartbeat = jiffies + heartbeat * HZ;
-		return 0;
-
-	case WDIOC_SETTIMEOUT:
-		if (get_user(new_value, p))
-			return -EFAULT;
-
-		heartbeat = new_value;
-		at91wdt_private.next_heartbeat = jiffies + heartbeat * HZ;
-
-		return put_user(new_value, p);  /* return current value */
-
-	case WDIOC_GETTIMEOUT:
-		return put_user(heartbeat, p);
-	}
-	return -ENOTTY;
-}
-
-/*
- * Pat the watchdog whenever device is written to.
- */
-static ssize_t at91_wdt_write(struct file *file, const char *data, size_t len,
-								loff_t *ppos)
-{
-	if (!len)
-		return 0;
-
-	/* Scan for magic character */
-	if (!nowayout) {
-		size_t i;
-
-		at91wdt_private.expect_close = 0;
-
-		for (i = 0; i < len; i++) {
-			char c;
-			if (get_user(c, data + i))
-				return -EFAULT;
-			if (c == 'V') {
-				at91wdt_private.expect_close = 42;
-				break;
-			}
-		}
-	}
-
-	at91wdt_private.next_heartbeat = jiffies + heartbeat * HZ;
-
-	return len;
-}
-
-/* ......................................................................... */
-
-static const struct file_operations at91wdt_fops = {
-	.owner			= THIS_MODULE,
-	.llseek			= no_llseek,
-	.unlocked_ioctl	= at91_wdt_ioctl,
-	.open			= at91_wdt_open,
-	.release		= at91_wdt_close,
-	.write			= at91_wdt_write,
-};
-
-static struct miscdevice at91wdt_miscdev = {
-	.minor		= WATCHDOG_MINOR,
-	.name		= "watchdog",
-	.fops		= &at91wdt_fops,
-};
-
 static int __init at91wdt_probe(struct platform_device *pdev)
 {
 	struct resource	*r;
 	int res;
 
-	if (at91wdt_miscdev.parent)
-		return -EBUSY;
-	at91wdt_miscdev.parent = &pdev->dev;
-
 	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!r)
 		return -ENODEV;
@@ -277,10 +155,6 @@  static int __init at91wdt_probe(struct platform_device *pdev)
 	if (res)
 		return res;
 
-	res = misc_register(&at91wdt_miscdev);
-	if (res)
-		return res;
-
 	at91wdt_private.next_heartbeat = jiffies + heartbeat * HZ;
 	setup_timer(&at91wdt_private.timer, at91_ping, 0);
 	mod_timer(&at91wdt_private.timer, jiffies + WDT_TIMEOUT);
@@ -295,10 +169,6 @@  static int __exit at91wdt_remove(struct platform_device *pdev)
 {
 	int res;
 
-	res = misc_deregister(&at91wdt_miscdev);
-	if (!res)
-		at91wdt_miscdev.parent = NULL;
-
 	return res;
 }
 
@@ -326,4 +196,3 @@  module_exit(at91sam_wdt_exit);
 MODULE_AUTHOR("Renaud CERRATO <r.cerrato@til-technologies.fr>");
 MODULE_DESCRIPTION("Watchdog driver for Atmel AT91SAM9x processors");
 MODULE_LICENSE("GPL");
-MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);