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

[ludovic.desroches@atmel.com (ludovic.desroches)]

Hi Wenyou,

On 12/04/2012 04:26 AM, Yang, Wenyou wrote:
> Hi JC,
> 
>> -----Original Message-----
>> From: Jean-Christophe PLAGNIOL-VILLARD [mailto:plagnioj at jcrosoft.com]
>> Sent: 2012?11?16? 21:54
>> To: Yang, Wenyou
>> Cc: linux-arm-kernel at lists.infradead.org; wim at iguana.be;
>> linux-watchdog at vger.kernel.org; linux-kernel at 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 at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>