[PATCH v2] cbus-retu-wdt: Remove unused nonstandard interfaces

[PATCH v2] cbus-retu-wdt: Remove unused nonstandard interfaces

[Michael Buesch]

This removes nonstandard interfaces from the WDT driver.
/dev/watchdog should be used to access the watchdog, only.

It removes the sysfs interface.
It also removes the "counter_param" module parameter, which
currently is broken anyway (doesn't work, because the value
is overridden right away).

Signed-off-by: Michael Buesch <mb@bu3sch.de>

---

v2: Split out warning cleanup

Index: linux-2.6.38-rc6/drivers/cbus/retu-wdt.c
===================================================================
--- linux-2.6.38-rc6.orig/drivers/cbus/retu-wdt.c	2011-03-04 15:45:37.533948567 +0100
+++ linux-2.6.38-rc6/drivers/cbus/retu-wdt.c	2011-03-04 15:46:21.312816961 +0100
@@ -52,7 +52,6 @@ static DEFINE_MUTEX(retu_wdt_mutex);
 
 /* Current period of watchdog */
 static unsigned int period_val = RETU_WDT_DEFAULT_TIMER;
-static int counter_param = RETU_WDT_MAX_TIMER;
 
 struct retu_wdt_dev {
 	struct device		*dev;
@@ -109,56 +108,6 @@ static void retu_wdt_ping_work(struct wo
 	retu_wdt_ping_enable(wdev);
 }
 
-static ssize_t retu_wdt_period_show(struct device *dev,
-				struct device_attribute *attr, char *buf)
-{
-	/* Show current max counter */
-	return sprintf(buf, "%u\n", (u16)period_val);
-}
-
-/*
- * Note: This inteface is non-standard and likely to disappear!
- * Use /dev/watchdog instead, that's the standard.
- */
-static ssize_t retu_wdt_period_store(struct device *dev,
-				struct device_attribute *attr,
-				const char *buf, size_t count)
-{
-	unsigned int new_period;
-	int ret;
-
-#ifdef CONFIG_WATCHDOG_NOWAYOUT
-	retu_wdt_ping_disable(retu_wdt);
-#endif
-
-	if (sscanf(buf, "%u", &new_period) != 1) {
-		printk(KERN_ALERT "retu_wdt_period_store: Invalid input\n");
-		return -EINVAL;
-	}
-
-	ret = retu_modify_counter(new_period);
-	if (ret < 0)
-		return ret;
-
-	return strnlen(buf, count);
-}
-
-static ssize_t retu_wdt_counter_show(struct device *dev,
-				struct device_attribute *attr, char *buf)
-{
-	u16 counter;
-
-	/* Show current value in watchdog counter */
-	counter = retu_read_reg(dev, RETU_REG_WATCHDOG);
-
-	/* Only the 5 LSB are important */
-	return snprintf(buf, PAGE_SIZE, "%u\n", (counter & 0x3F));
-}
-
-static DEVICE_ATTR(period, S_IRUGO | S_IWUSR, retu_wdt_period_show, \
-			retu_wdt_period_store);
-static DEVICE_ATTR(counter, S_IRUGO, retu_wdt_counter_show, NULL);
-
 static int retu_wdt_open(struct inode *inode, struct file *file)
 {
 	if (test_and_set_bit(0, &retu_wdt->users))
@@ -232,18 +181,6 @@ static long retu_wdt_ioctl(struct file *
 	return 0;
 }
 
-/* Start kicking retu watchdog until user space starts doing the kicking */
-static int __devinit retu_wdt_ping(void)
-{
-#ifdef CONFIG_WATCHDOG_NOWAYOUT
-	retu_modify_counter(RETU_WDT_MAX_TIMER);
-#else
-	retu_wdt_ping_enable(retu_wdt);
-#endif
-
-	return 0;
-}
-
 static const struct file_operations retu_wdt_fops = {
 	.owner		= THIS_MODULE,
 	.write		= retu_wdt_write,
@@ -252,8 +189,6 @@ static const struct file_operations retu
 	.release	= retu_wdt_release,
 };
 
-/*----------------------------------------------------------------------------*/
-
 static int __init retu_wdt_probe(struct platform_device *pdev)
 {
 	struct retu_wdt_dev *wdev;
@@ -265,18 +200,6 @@ static int __init retu_wdt_probe(struct
 
 	wdev->dev = &pdev->dev;
 
-	ret = device_create_file(&pdev->dev, &dev_attr_period);
-	if (ret) {
-		dev_err(&pdev->dev, "Error creating sysfs period\n");
-		goto free1;
-	}
-
-	ret = device_create_file(&pdev->dev, &dev_attr_counter);
-	if (ret) {
-		dev_err(&pdev->dev, "Error creating sysfs counter\n");
-		goto free2;
-	}
-
 	platform_set_drvdata(pdev, wdev);
 	retu_wdt = wdev;
 	wdev->retu_wdt_miscdev.parent = &pdev->dev;
@@ -286,38 +209,21 @@ static int __init retu_wdt_probe(struct
 
 	ret = misc_register(&(wdev->retu_wdt_miscdev));
 	if (ret)
-		goto free3;
+		goto err_free_wdev;
 
 	INIT_DELAYED_WORK(&wdev->ping_work, retu_wdt_ping_work);
 
-	/* passed as module parameter? */
-	ret = retu_modify_counter(counter_param);
-	if (ret == -EINVAL) {
-		ret = retu_modify_counter(RETU_WDT_DEFAULT_TIMER);
-		dev_dbg(&pdev->dev, "Initializing to default value\n");
-	}
-
-	/* Kick the watchdog for kernel booting to finish */
+	/* Kick the watchdog for kernel booting to finish.
+	 * If nowayout is not set, we start the ping work. */
+#ifdef CONFIG_WATCHDOG_NOWAYOUT
 	retu_modify_counter(RETU_WDT_MAX_TIMER);
-
-	ret = retu_wdt_ping();
-	if (ret < 0) {
-		dev_err(&pdev->dev, "Failed to ping\n");
-		goto free4;
-	}
+#else
+	retu_wdt_ping_enable(retu_wdt);
+#endif
 
 	return 0;
 
-free4:
-	misc_deregister(&wdev->retu_wdt_miscdev);
-
-free3:
-	device_remove_file(&pdev->dev, &dev_attr_counter);
-
-free2:
-	device_remove_file(&pdev->dev, &dev_attr_period);
-
-free1:
+err_free_wdev:
 	kfree(wdev);
 
 	return ret;
@@ -329,8 +235,6 @@ static int __devexit retu_wdt_remove(str
 
 	wdev = platform_get_drvdata(pdev);
 	misc_deregister(&wdev->retu_wdt_miscdev);
-	device_remove_file(&pdev->dev, &dev_attr_period);
-	device_remove_file(&pdev->dev, &dev_attr_counter);
 	cancel_delayed_work_sync(&wdev->ping_work);
 	kfree(wdev);
 
@@ -356,9 +260,7 @@ static void __exit retu_wdt_exit(void)
 
 module_init(retu_wdt_init);
 module_exit(retu_wdt_exit);
-module_param(counter_param, int, 0);
 
 MODULE_DESCRIPTION("Retu WatchDog");
 MODULE_AUTHOR("Amit Kucheria");
 MODULE_LICENSE("GPL");
-


-- 
Greetings, Michael.

Re: [PATCH v2] cbus-retu-wdt: Remove unused nonstandard interfaces

[Felipe Balbi]

On Fri, Mar 04, 2011 at 03:52:29PM +0100, Michael Buesch wrote:
> Index: linux-2.6.38-rc6/drivers/cbus/retu-wdt.c
> ===================================================================
> --- linux-2.6.38-rc6.orig/drivers/cbus/retu-wdt.c	2011-03-04 15:45:37.533948567 +0100
> +++ linux-2.6.38-rc6/drivers/cbus/retu-wdt.c	2011-03-04 15:46:21.312816961 +0100
> @@ -52,7 +52,6 @@ static DEFINE_MUTEX(retu_wdt_mutex);
>  
>  /* Current period of watchdog */
>  static unsigned int period_val = RETU_WDT_DEFAULT_TIMER;
> -static int counter_param = RETU_WDT_MAX_TIMER;
>  
>  struct retu_wdt_dev {
>  	struct device		*dev;
> @@ -109,56 +108,6 @@ static void retu_wdt_ping_work(struct wo
>  	retu_wdt_ping_enable(wdev);
>  }
>  
> -static ssize_t retu_wdt_period_show(struct device *dev,
> -				struct device_attribute *attr, char *buf)
> -{
> -	/* Show current max counter */
> -	return sprintf(buf, "%u\n", (u16)period_val);
> -}
> -
> -/*
> - * Note: This inteface is non-standard and likely to disappear!
> - * Use /dev/watchdog instead, that's the standard.
> - */
> -static ssize_t retu_wdt_period_store(struct device *dev,
> -				struct device_attribute *attr,
> -				const char *buf, size_t count)
> -{
> -	unsigned int new_period;
> -	int ret;
> -
> -#ifdef CONFIG_WATCHDOG_NOWAYOUT
> -	retu_wdt_ping_disable(retu_wdt);
> -#endif
> -
> -	if (sscanf(buf, "%u", &new_period) != 1) {
> -		printk(KERN_ALERT "retu_wdt_period_store: Invalid input\n");
> -		return -EINVAL;
> -	}
> -
> -	ret = retu_modify_counter(new_period);
> -	if (ret < 0)
> -		return ret;
> -
> -	return strnlen(buf, count);
> -}
> -
> -static ssize_t retu_wdt_counter_show(struct device *dev,
> -				struct device_attribute *attr, char *buf)
> -{
> -	u16 counter;
> -
> -	/* Show current value in watchdog counter */
> -	counter = retu_read_reg(dev, RETU_REG_WATCHDOG);
> -
> -	/* Only the 5 LSB are important */
> -	return snprintf(buf, PAGE_SIZE, "%u\n", (counter & 0x3F));
> -}
> -
> -static DEVICE_ATTR(period, S_IRUGO | S_IWUSR, retu_wdt_period_show, \
> -			retu_wdt_period_store);
> -static DEVICE_ATTR(counter, S_IRUGO, retu_wdt_counter_show, NULL);
> -
>  static int retu_wdt_open(struct inode *inode, struct file *file)
>  {
>  	if (test_and_set_bit(0, &retu_wdt->users))
> @@ -232,18 +181,6 @@ static long retu_wdt_ioctl(struct file *
>  	return 0;
>  }
>  
> -/* Start kicking retu watchdog until user space starts doing the kicking */
> -static int __devinit retu_wdt_ping(void)
> -{
> -#ifdef CONFIG_WATCHDOG_NOWAYOUT
> -	retu_modify_counter(RETU_WDT_MAX_TIMER);
> -#else
> -	retu_wdt_ping_enable(retu_wdt);
> -#endif
> -
> -	return 0;
> -}

is this part of this patch ? Also, I guess other watchdog drivers have
similar tricks to kick the watchdog.

> @@ -252,8 +189,6 @@ static const struct file_operations retu
>  	.release	= retu_wdt_release,
>  };
>  
> -/*----------------------------------------------------------------------------*/
> -

trailing change.

> @@ -286,38 +209,21 @@ static int __init retu_wdt_probe(struct
>  
>  	ret = misc_register(&(wdev->retu_wdt_miscdev));
>  	if (ret)
> -		goto free3;
> +		goto err_free_wdev;
>  
>  	INIT_DELAYED_WORK(&wdev->ping_work, retu_wdt_ping_work);
>  
> -	/* passed as module parameter? */
> -	ret = retu_modify_counter(counter_param);
> -	if (ret == -EINVAL) {
> -		ret = retu_modify_counter(RETU_WDT_DEFAULT_TIMER);
> -		dev_dbg(&pdev->dev, "Initializing to default value\n");
> -	}
> -
> -	/* Kick the watchdog for kernel booting to finish */
> +	/* Kick the watchdog for kernel booting to finish.
> +	 * If nowayout is not set, we start the ping work. */

multiline comment style is wrong.

> +#ifdef CONFIG_WATCHDOG_NOWAYOUT
>  	retu_modify_counter(RETU_WDT_MAX_TIMER);
> -
> -	ret = retu_wdt_ping();
> -	if (ret < 0) {
> -		dev_err(&pdev->dev, "Failed to ping\n");
> -		goto free4;
> -	}
> +#else
> +	retu_wdt_ping_enable(retu_wdt);
> +#endif

Rather, keep that function and call it here.

-- 
balbi