Linux-RTC Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v4] rtc: rtc-ds1374: wdt: Use watchdog core for watchdog part
@ 2020-07-09  6:34 Johnson CH Chen (陳昭勳)
  2020-07-09 15:28 ` Guenter Roeck
  2020-07-16  9:26 ` Alexandre Belloni
  0 siblings, 2 replies; 4+ messages in thread
From: Johnson CH Chen (陳昭勳) @ 2020-07-09  6:34 UTC (permalink / raw)
  To: linux-kernel, linux-rtc, linux-watchdog
  Cc: Guenter Roeck, Wim Van Sebroeck, a.zummo, alexandre.belloni

Let ds1374 watchdog use watchdog core functions. It also includes
improving watchdog timer setting and nowayout, and just uses ioctl()
of watchdog core.

Signed-off-by: Johnson Chen <johnsonch.chen@moxa.com>
---
v3->v4:
- Fix coding styles 
- Remove dev_info() in ds1374_wdt_settimeout()
- Fix missing error check

v2->v3:
- Fix a problem reported by WATCHDOG_CORE if WATCHDOG
- Remove save_client
- Let wdt_margin be 0 for watchdog_init_timeout()
- Use dev_info() rather than pr_info()
- Avoid more strings in this driver

v1->v2:
- Use ds1374_wdt_settimeout() before registering the watchdog
- Remove watchdog_unregister_device() because devm_watchdog_register_device() is used
- Remove ds1374_wdt_ping()
- TIMER_MARGIN_MAX to 4095 for 24-bit value
- Keep wdt_margin
- Fix coding styles

 drivers/rtc/Kconfig      |   1 +
 drivers/rtc/rtc-ds1374.c | 258 +++++++++------------------------------
 2 files changed, 62 insertions(+), 197 deletions(-)

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index b54d87d45c89..c25d51f35f0c 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -282,6 +282,7 @@ config RTC_DRV_DS1374
 config RTC_DRV_DS1374_WDT
 	bool "Dallas/Maxim DS1374 watchdog timer"
 	depends on RTC_DRV_DS1374
+	select WATCHDOG_CORE if WATCHDOG
 	help
 	  If you say Y here you will get support for the
 	  watchdog timer in the Dallas Semiconductor DS1374
diff --git a/drivers/rtc/rtc-ds1374.c b/drivers/rtc/rtc-ds1374.c
index 9c51a12cf70f..c71065d26cd2 100644
--- a/drivers/rtc/rtc-ds1374.c
+++ b/drivers/rtc/rtc-ds1374.c
@@ -46,6 +46,7 @@
 #define DS1374_REG_WDALM2	0x06
 #define DS1374_REG_CR		0x07 /* Control */
 #define DS1374_REG_CR_AIE	0x01 /* Alarm Int. Enable */
+#define DS1374_REG_CR_WDSTR	0x08 /* 1=INT, 0=RST */
 #define DS1374_REG_CR_WDALM	0x20 /* 1=Watchdog, 0=Alarm */
 #define DS1374_REG_CR_WACE	0x40 /* WD/Alarm counter enable */
 #define DS1374_REG_SR		0x08 /* Status */
@@ -71,7 +72,9 @@ struct ds1374 {
 	struct i2c_client *client;
 	struct rtc_device *rtc;
 	struct work_struct work;
-
+#ifdef CONFIG_RTC_DRV_DS1374_WDT
+	struct watchdog_device wdt;
+#endif
 	/* The mutex protects alarm operations, and prevents a race
 	 * between the enable_irq() in the workqueue and the free_irq()
 	 * in the remove function.
@@ -369,238 +372,98 @@ static const struct rtc_class_ops ds1374_rtc_ops = {
  *
  *****************************************************************************
  */
-static struct i2c_client *save_client;
 /* Default margin */
-#define WD_TIMO 131762
+#define TIMER_MARGIN_DEFAULT	32
+#define TIMER_MARGIN_MIN	1
+#define TIMER_MARGIN_MAX	4095 /* 24-bit value */
 
 #define DRV_NAME "DS1374 Watchdog"
 
-static int wdt_margin = WD_TIMO;
-static unsigned long wdt_is_open;
+static int wdt_margin;
 module_param(wdt_margin, int, 0);
 MODULE_PARM_DESC(wdt_margin, "Watchdog timeout in seconds (default 32s)");
 
+static bool nowayout = WATCHDOG_NOWAYOUT;
+module_param(nowayout, bool, 0);
+MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default ="
+		__MODULE_STRING(WATCHDOG_NOWAYOUT)")");
+
 static const struct watchdog_info ds1374_wdt_info = {
 	.identity       = "DS1374 WTD",
 	.options        = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING |
 						WDIOF_MAGICCLOSE,
 };
 
-static int ds1374_wdt_settimeout(unsigned int timeout)
+static int ds1374_wdt_settimeout(struct watchdog_device *wdt, unsigned int timeout)
 {
-	int ret = -ENOIOCTLCMD;
-	int cr;
+	struct ds1374 *ds1374 = watchdog_get_drvdata(wdt);
+	struct i2c_client *client = ds1374->client;
+	int ret, cr;
 
-	ret = cr = i2c_smbus_read_byte_data(save_client, DS1374_REG_CR);
-	if (ret < 0)
-		goto out;
+	wdt->timeout = timeout;
+
+	cr = i2c_smbus_read_byte_data(client, DS1374_REG_CR);
+	if (cr < 0)
+		return cr;
 
 	/* Disable any existing watchdog/alarm before setting the new one */
 	cr &= ~DS1374_REG_CR_WACE;
 
-	ret = i2c_smbus_write_byte_data(save_client, DS1374_REG_CR, cr);
+	ret = i2c_smbus_write_byte_data(client, DS1374_REG_CR, cr);
 	if (ret < 0)
-		goto out;
+		return ret;
 
 	/* Set new watchdog time */
-	ret = ds1374_write_rtc(save_client, timeout, DS1374_REG_WDALM0, 3);
-	if (ret) {
-		pr_info("couldn't set new watchdog time\n");
-		goto out;
-	}
+	timeout = timeout * 4096;
+	ret = ds1374_write_rtc(client, timeout, DS1374_REG_WDALM0, 3);
+	if (ret)
+		return ret;
 
 	/* Enable watchdog timer */
 	cr |= DS1374_REG_CR_WACE | DS1374_REG_CR_WDALM;
+	cr &= ~DS1374_REG_CR_WDSTR;/* for RST PIN */
 	cr &= ~DS1374_REG_CR_AIE;
 
-	ret = i2c_smbus_write_byte_data(save_client, DS1374_REG_CR, cr);
+	ret = i2c_smbus_write_byte_data(client, DS1374_REG_CR, cr);
 	if (ret < 0)
-		goto out;
+		return ret;
 
 	return 0;
-out:
-	return ret;
 }
 
-
 /*
  * Reload the watchdog timer.  (ie, pat the watchdog)
  */
-static void ds1374_wdt_ping(void)
+static int ds1374_wdt_start(struct watchdog_device *wdt)
 {
+	struct ds1374 *ds1374 = watchdog_get_drvdata(wdt);
 	u32 val;
-	int ret = 0;
 
-	ret = ds1374_read_rtc(save_client, &val, DS1374_REG_WDALM0, 3);
-	if (ret)
-		pr_info("WD TICK FAIL!!!!!!!!!! %i\n", ret);
+	return ds1374_read_rtc(ds1374->client, &val, DS1374_REG_WDALM0, 3);
 }
 
-static void ds1374_wdt_disable(void)
+static int ds1374_wdt_stop(struct watchdog_device *wdt)
 {
+	struct ds1374 *ds1374 = watchdog_get_drvdata(wdt);
+	struct i2c_client *client = ds1374->client;
 	int cr;
 
-	cr = i2c_smbus_read_byte_data(save_client, DS1374_REG_CR);
+	cr = i2c_smbus_read_byte_data(client, DS1374_REG_CR);
+	if (cr < 0)
+		return cr;
+
 	/* Disable watchdog timer */
 	cr &= ~DS1374_REG_CR_WACE;
 
-	i2c_smbus_write_byte_data(save_client, DS1374_REG_CR, cr);
-}
-
-/*
- * Watchdog device is opened, and watchdog starts running.
- */
-static int ds1374_wdt_open(struct inode *inode, struct file *file)
-{
-	struct ds1374 *ds1374 = i2c_get_clientdata(save_client);
-
-	if (MINOR(inode->i_rdev) == WATCHDOG_MINOR) {
-		mutex_lock(&ds1374->mutex);
-		if (test_and_set_bit(0, &wdt_is_open)) {
-			mutex_unlock(&ds1374->mutex);
-			return -EBUSY;
-		}
-		/*
-		 *      Activate
-		 */
-		wdt_is_open = 1;
-		mutex_unlock(&ds1374->mutex);
-		return stream_open(inode, file);
-	}
-	return -ENODEV;
-}
-
-/*
- * Close the watchdog device.
- */
-static int ds1374_wdt_release(struct inode *inode, struct file *file)
-{
-	if (MINOR(inode->i_rdev) == WATCHDOG_MINOR)
-		clear_bit(0, &wdt_is_open);
-
-	return 0;
+	return i2c_smbus_write_byte_data(client, DS1374_REG_CR, cr);
 }
 
-/*
- * Pat the watchdog whenever device is written to.
- */
-static ssize_t ds1374_wdt_write(struct file *file, const char __user *data,
-				size_t len, loff_t *ppos)
-{
-	if (len) {
-		ds1374_wdt_ping();
-		return 1;
-	}
-	return 0;
-}
-
-static ssize_t ds1374_wdt_read(struct file *file, char __user *data,
-				size_t len, loff_t *ppos)
-{
-	return 0;
-}
-
-/*
- * Handle commands from user-space.
- */
-static long ds1374_wdt_ioctl(struct file *file, unsigned int cmd,
-							unsigned long arg)
-{
-	int new_margin, options;
-
-	switch (cmd) {
-	case WDIOC_GETSUPPORT:
-		return copy_to_user((struct watchdog_info __user *)arg,
-		&ds1374_wdt_info, sizeof(ds1374_wdt_info)) ? -EFAULT : 0;
-
-	case WDIOC_GETSTATUS:
-	case WDIOC_GETBOOTSTATUS:
-		return put_user(0, (int __user *)arg);
-	case WDIOC_KEEPALIVE:
-		ds1374_wdt_ping();
-		return 0;
-	case WDIOC_SETTIMEOUT:
-		if (get_user(new_margin, (int __user *)arg))
-			return -EFAULT;
-
-		/* the hardware's tick rate is 4096 Hz, so
-		 * the counter value needs to be scaled accordingly
-		 */
-		new_margin <<= 12;
-		if (new_margin < 1 || new_margin > 16777216)
-			return -EINVAL;
-
-		wdt_margin = new_margin;
-		ds1374_wdt_settimeout(new_margin);
-		ds1374_wdt_ping();
-		/* fallthrough */
-	case WDIOC_GETTIMEOUT:
-		/* when returning ... inverse is true */
-		return put_user((wdt_margin >> 12), (int __user *)arg);
-	case WDIOC_SETOPTIONS:
-		if (copy_from_user(&options, (int __user *)arg, sizeof(int)))
-			return -EFAULT;
-
-		if (options & WDIOS_DISABLECARD) {
-			pr_info("disable watchdog\n");
-			ds1374_wdt_disable();
-			return 0;
-		}
-
-		if (options & WDIOS_ENABLECARD) {
-			pr_info("enable watchdog\n");
-			ds1374_wdt_settimeout(wdt_margin);
-			ds1374_wdt_ping();
-			return 0;
-		}
-		return -EINVAL;
-	}
-	return -ENOTTY;
-}
-
-static long ds1374_wdt_unlocked_ioctl(struct file *file, unsigned int cmd,
-			unsigned long arg)
-{
-	int ret;
-	struct ds1374 *ds1374 = i2c_get_clientdata(save_client);
-
-	mutex_lock(&ds1374->mutex);
-	ret = ds1374_wdt_ioctl(file, cmd, arg);
-	mutex_unlock(&ds1374->mutex);
-
-	return ret;
-}
-
-static int ds1374_wdt_notify_sys(struct notifier_block *this,
-			unsigned long code, void *unused)
-{
-	if (code == SYS_DOWN || code == SYS_HALT)
-		/* Disable Watchdog */
-		ds1374_wdt_disable();
-	return NOTIFY_DONE;
-}
-
-static const struct file_operations ds1374_wdt_fops = {
-	.owner			= THIS_MODULE,
-	.read			= ds1374_wdt_read,
-	.unlocked_ioctl		= ds1374_wdt_unlocked_ioctl,
-	.compat_ioctl		= compat_ptr_ioctl,
-	.write			= ds1374_wdt_write,
-	.open                   = ds1374_wdt_open,
-	.release                = ds1374_wdt_release,
-	.llseek			= no_llseek,
-};
-
-static struct miscdevice ds1374_miscdev = {
-	.minor          = WATCHDOG_MINOR,
-	.name           = "watchdog",
-	.fops           = &ds1374_wdt_fops,
-};
-
-static struct notifier_block ds1374_wdt_notifier = {
-	.notifier_call = ds1374_wdt_notify_sys,
+static const struct watchdog_ops ds1374_wdt_ops = {
+	.owner          = THIS_MODULE,
+	.start          = ds1374_wdt_start,
+	.stop           = ds1374_wdt_stop,
+	.set_timeout    = ds1374_wdt_settimeout,
 };
-
 #endif /*CONFIG_RTC_DRV_DS1374_WDT*/
 /*
  *****************************************************************************
@@ -652,16 +515,22 @@ static int ds1374_probe(struct i2c_client *client,
 		return ret;
 
 #ifdef CONFIG_RTC_DRV_DS1374_WDT
-	save_client = client;
-	ret = misc_register(&ds1374_miscdev);
+	ds1374->wdt.info = &ds1374_wdt_info;
+	ds1374->wdt.ops = &ds1374_wdt_ops;
+	ds1374->wdt.timeout = TIMER_MARGIN_DEFAULT;
+	ds1374->wdt.min_timeout = TIMER_MARGIN_MIN;
+	ds1374->wdt.max_timeout = TIMER_MARGIN_MAX;
+
+	watchdog_init_timeout(&ds1374->wdt, wdt_margin, &client->dev);
+	watchdog_set_nowayout(&ds1374->wdt, nowayout);
+	watchdog_stop_on_reboot(&ds1374->wdt);
+	watchdog_stop_on_unregister(&ds1374->wdt);
+	watchdog_set_drvdata(&ds1374->wdt, ds1374);
+	ds1374_wdt_settimeout(&ds1374->wdt, ds1374->wdt.timeout);
+
+	ret = devm_watchdog_register_device(&client->dev, &ds1374->wdt);
 	if (ret)
 		return ret;
-	ret = register_reboot_notifier(&ds1374_wdt_notifier);
-	if (ret) {
-		misc_deregister(&ds1374_miscdev);
-		return ret;
-	}
-	ds1374_wdt_settimeout(131072);
 #endif
 
 	return 0;
@@ -670,11 +539,6 @@ static int ds1374_probe(struct i2c_client *client,
 static int ds1374_remove(struct i2c_client *client)
 {
 	struct ds1374 *ds1374 = i2c_get_clientdata(client);
-#ifdef CONFIG_RTC_DRV_DS1374_WDT
-	misc_deregister(&ds1374_miscdev);
-	ds1374_miscdev.parent = NULL;
-	unregister_reboot_notifier(&ds1374_wdt_notifier);
-#endif
 
 	if (client->irq > 0) {
 		mutex_lock(&ds1374->mutex);
-- 
2.20.1

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v4] rtc: rtc-ds1374: wdt: Use watchdog core for watchdog part
  2020-07-09  6:34 [PATCH v4] rtc: rtc-ds1374: wdt: Use watchdog core for watchdog part Johnson CH Chen (陳昭勳)
@ 2020-07-09 15:28 ` Guenter Roeck
  2020-07-13  2:38   ` Johnson CH Chen (陳昭勳)
  2020-07-16  9:26 ` Alexandre Belloni
  1 sibling, 1 reply; 4+ messages in thread
From: Guenter Roeck @ 2020-07-09 15:28 UTC (permalink / raw)
  To: Johnson CH Chen (陳昭勳),
	linux-kernel, linux-rtc, linux-watchdog
  Cc: Wim Van Sebroeck, a.zummo, alexandre.belloni

On 7/8/20 11:34 PM, Johnson CH Chen (陳昭勳) wrote:
> Let ds1374 watchdog use watchdog core functions. It also includes
> improving watchdog timer setting and nowayout, and just uses ioctl()
> of watchdog core.
> 
> Signed-off-by: Johnson Chen <johnsonch.chen@moxa.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> v3->v4:
> - Fix coding styles 
> - Remove dev_info() in ds1374_wdt_settimeout()
> - Fix missing error check
> 
> v2->v3:
> - Fix a problem reported by WATCHDOG_CORE if WATCHDOG
> - Remove save_client
> - Let wdt_margin be 0 for watchdog_init_timeout()
> - Use dev_info() rather than pr_info()
> - Avoid more strings in this driver
> 
> v1->v2:
> - Use ds1374_wdt_settimeout() before registering the watchdog
> - Remove watchdog_unregister_device() because devm_watchdog_register_device() is used
> - Remove ds1374_wdt_ping()
> - TIMER_MARGIN_MAX to 4095 for 24-bit value
> - Keep wdt_margin
> - Fix coding styles
> 
>  drivers/rtc/Kconfig      |   1 +
>  drivers/rtc/rtc-ds1374.c | 258 +++++++++------------------------------
>  2 files changed, 62 insertions(+), 197 deletions(-)
> 
> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> index b54d87d45c89..c25d51f35f0c 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -282,6 +282,7 @@ config RTC_DRV_DS1374
>  config RTC_DRV_DS1374_WDT
>  	bool "Dallas/Maxim DS1374 watchdog timer"
>  	depends on RTC_DRV_DS1374
> +	select WATCHDOG_CORE if WATCHDOG
>  	help
>  	  If you say Y here you will get support for the
>  	  watchdog timer in the Dallas Semiconductor DS1374
> diff --git a/drivers/rtc/rtc-ds1374.c b/drivers/rtc/rtc-ds1374.c
> index 9c51a12cf70f..c71065d26cd2 100644
> --- a/drivers/rtc/rtc-ds1374.c
> +++ b/drivers/rtc/rtc-ds1374.c
> @@ -46,6 +46,7 @@
>  #define DS1374_REG_WDALM2	0x06
>  #define DS1374_REG_CR		0x07 /* Control */
>  #define DS1374_REG_CR_AIE	0x01 /* Alarm Int. Enable */
> +#define DS1374_REG_CR_WDSTR	0x08 /* 1=INT, 0=RST */
>  #define DS1374_REG_CR_WDALM	0x20 /* 1=Watchdog, 0=Alarm */
>  #define DS1374_REG_CR_WACE	0x40 /* WD/Alarm counter enable */
>  #define DS1374_REG_SR		0x08 /* Status */
> @@ -71,7 +72,9 @@ struct ds1374 {
>  	struct i2c_client *client;
>  	struct rtc_device *rtc;
>  	struct work_struct work;
> -
> +#ifdef CONFIG_RTC_DRV_DS1374_WDT
> +	struct watchdog_device wdt;
> +#endif
>  	/* The mutex protects alarm operations, and prevents a race
>  	 * between the enable_irq() in the workqueue and the free_irq()
>  	 * in the remove function.
> @@ -369,238 +372,98 @@ static const struct rtc_class_ops ds1374_rtc_ops = {
>   *
>   *****************************************************************************
>   */
> -static struct i2c_client *save_client;
>  /* Default margin */
> -#define WD_TIMO 131762
> +#define TIMER_MARGIN_DEFAULT	32
> +#define TIMER_MARGIN_MIN	1
> +#define TIMER_MARGIN_MAX	4095 /* 24-bit value */
>  
>  #define DRV_NAME "DS1374 Watchdog"
>  
> -static int wdt_margin = WD_TIMO;
> -static unsigned long wdt_is_open;
> +static int wdt_margin;
>  module_param(wdt_margin, int, 0);
>  MODULE_PARM_DESC(wdt_margin, "Watchdog timeout in seconds (default 32s)");
>  
> +static bool nowayout = WATCHDOG_NOWAYOUT;
> +module_param(nowayout, bool, 0);
> +MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default ="
> +		__MODULE_STRING(WATCHDOG_NOWAYOUT)")");
> +
>  static const struct watchdog_info ds1374_wdt_info = {
>  	.identity       = "DS1374 WTD",
>  	.options        = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING |
>  						WDIOF_MAGICCLOSE,
>  };
>  
> -static int ds1374_wdt_settimeout(unsigned int timeout)
> +static int ds1374_wdt_settimeout(struct watchdog_device *wdt, unsigned int timeout)
>  {
> -	int ret = -ENOIOCTLCMD;
> -	int cr;
> +	struct ds1374 *ds1374 = watchdog_get_drvdata(wdt);
> +	struct i2c_client *client = ds1374->client;
> +	int ret, cr;
>  
> -	ret = cr = i2c_smbus_read_byte_data(save_client, DS1374_REG_CR);
> -	if (ret < 0)
> -		goto out;
> +	wdt->timeout = timeout;
> +
> +	cr = i2c_smbus_read_byte_data(client, DS1374_REG_CR);
> +	if (cr < 0)
> +		return cr;
>  
>  	/* Disable any existing watchdog/alarm before setting the new one */
>  	cr &= ~DS1374_REG_CR_WACE;
>  
> -	ret = i2c_smbus_write_byte_data(save_client, DS1374_REG_CR, cr);
> +	ret = i2c_smbus_write_byte_data(client, DS1374_REG_CR, cr);
>  	if (ret < 0)
> -		goto out;
> +		return ret;
>  
>  	/* Set new watchdog time */
> -	ret = ds1374_write_rtc(save_client, timeout, DS1374_REG_WDALM0, 3);
> -	if (ret) {
> -		pr_info("couldn't set new watchdog time\n");
> -		goto out;
> -	}
> +	timeout = timeout * 4096;
> +	ret = ds1374_write_rtc(client, timeout, DS1374_REG_WDALM0, 3);
> +	if (ret)
> +		return ret;
>  
>  	/* Enable watchdog timer */
>  	cr |= DS1374_REG_CR_WACE | DS1374_REG_CR_WDALM;
> +	cr &= ~DS1374_REG_CR_WDSTR;/* for RST PIN */
>  	cr &= ~DS1374_REG_CR_AIE;
>  
> -	ret = i2c_smbus_write_byte_data(save_client, DS1374_REG_CR, cr);
> +	ret = i2c_smbus_write_byte_data(client, DS1374_REG_CR, cr);
>  	if (ret < 0)
> -		goto out;
> +		return ret;
>  
>  	return 0;
> -out:
> -	return ret;
>  }
>  
> -
>  /*
>   * Reload the watchdog timer.  (ie, pat the watchdog)
>   */
> -static void ds1374_wdt_ping(void)
> +static int ds1374_wdt_start(struct watchdog_device *wdt)
>  {
> +	struct ds1374 *ds1374 = watchdog_get_drvdata(wdt);
>  	u32 val;
> -	int ret = 0;
>  
> -	ret = ds1374_read_rtc(save_client, &val, DS1374_REG_WDALM0, 3);
> -	if (ret)
> -		pr_info("WD TICK FAIL!!!!!!!!!! %i\n", ret);
> +	return ds1374_read_rtc(ds1374->client, &val, DS1374_REG_WDALM0, 3);
>  }
>  
> -static void ds1374_wdt_disable(void)
> +static int ds1374_wdt_stop(struct watchdog_device *wdt)
>  {
> +	struct ds1374 *ds1374 = watchdog_get_drvdata(wdt);
> +	struct i2c_client *client = ds1374->client;
>  	int cr;
>  
> -	cr = i2c_smbus_read_byte_data(save_client, DS1374_REG_CR);
> +	cr = i2c_smbus_read_byte_data(client, DS1374_REG_CR);
> +	if (cr < 0)
> +		return cr;
> +
>  	/* Disable watchdog timer */
>  	cr &= ~DS1374_REG_CR_WACE;
>  
> -	i2c_smbus_write_byte_data(save_client, DS1374_REG_CR, cr);
> -}
> -
> -/*
> - * Watchdog device is opened, and watchdog starts running.
> - */
> -static int ds1374_wdt_open(struct inode *inode, struct file *file)
> -{
> -	struct ds1374 *ds1374 = i2c_get_clientdata(save_client);
> -
> -	if (MINOR(inode->i_rdev) == WATCHDOG_MINOR) {
> -		mutex_lock(&ds1374->mutex);
> -		if (test_and_set_bit(0, &wdt_is_open)) {
> -			mutex_unlock(&ds1374->mutex);
> -			return -EBUSY;
> -		}
> -		/*
> -		 *      Activate
> -		 */
> -		wdt_is_open = 1;
> -		mutex_unlock(&ds1374->mutex);
> -		return stream_open(inode, file);
> -	}
> -	return -ENODEV;
> -}
> -
> -/*
> - * Close the watchdog device.
> - */
> -static int ds1374_wdt_release(struct inode *inode, struct file *file)
> -{
> -	if (MINOR(inode->i_rdev) == WATCHDOG_MINOR)
> -		clear_bit(0, &wdt_is_open);
> -
> -	return 0;
> +	return i2c_smbus_write_byte_data(client, DS1374_REG_CR, cr);
>  }
>  
> -/*
> - * Pat the watchdog whenever device is written to.
> - */
> -static ssize_t ds1374_wdt_write(struct file *file, const char __user *data,
> -				size_t len, loff_t *ppos)
> -{
> -	if (len) {
> -		ds1374_wdt_ping();
> -		return 1;
> -	}
> -	return 0;
> -}
> -
> -static ssize_t ds1374_wdt_read(struct file *file, char __user *data,
> -				size_t len, loff_t *ppos)
> -{
> -	return 0;
> -}
> -
> -/*
> - * Handle commands from user-space.
> - */
> -static long ds1374_wdt_ioctl(struct file *file, unsigned int cmd,
> -							unsigned long arg)
> -{
> -	int new_margin, options;
> -
> -	switch (cmd) {
> -	case WDIOC_GETSUPPORT:
> -		return copy_to_user((struct watchdog_info __user *)arg,
> -		&ds1374_wdt_info, sizeof(ds1374_wdt_info)) ? -EFAULT : 0;
> -
> -	case WDIOC_GETSTATUS:
> -	case WDIOC_GETBOOTSTATUS:
> -		return put_user(0, (int __user *)arg);
> -	case WDIOC_KEEPALIVE:
> -		ds1374_wdt_ping();
> -		return 0;
> -	case WDIOC_SETTIMEOUT:
> -		if (get_user(new_margin, (int __user *)arg))
> -			return -EFAULT;
> -
> -		/* the hardware's tick rate is 4096 Hz, so
> -		 * the counter value needs to be scaled accordingly
> -		 */
> -		new_margin <<= 12;
> -		if (new_margin < 1 || new_margin > 16777216)
> -			return -EINVAL;
> -
> -		wdt_margin = new_margin;
> -		ds1374_wdt_settimeout(new_margin);
> -		ds1374_wdt_ping();
> -		/* fallthrough */
> -	case WDIOC_GETTIMEOUT:
> -		/* when returning ... inverse is true */
> -		return put_user((wdt_margin >> 12), (int __user *)arg);
> -	case WDIOC_SETOPTIONS:
> -		if (copy_from_user(&options, (int __user *)arg, sizeof(int)))
> -			return -EFAULT;
> -
> -		if (options & WDIOS_DISABLECARD) {
> -			pr_info("disable watchdog\n");
> -			ds1374_wdt_disable();
> -			return 0;
> -		}
> -
> -		if (options & WDIOS_ENABLECARD) {
> -			pr_info("enable watchdog\n");
> -			ds1374_wdt_settimeout(wdt_margin);
> -			ds1374_wdt_ping();
> -			return 0;
> -		}
> -		return -EINVAL;
> -	}
> -	return -ENOTTY;
> -}
> -
> -static long ds1374_wdt_unlocked_ioctl(struct file *file, unsigned int cmd,
> -			unsigned long arg)
> -{
> -	int ret;
> -	struct ds1374 *ds1374 = i2c_get_clientdata(save_client);
> -
> -	mutex_lock(&ds1374->mutex);
> -	ret = ds1374_wdt_ioctl(file, cmd, arg);
> -	mutex_unlock(&ds1374->mutex);
> -
> -	return ret;
> -}
> -
> -static int ds1374_wdt_notify_sys(struct notifier_block *this,
> -			unsigned long code, void *unused)
> -{
> -	if (code == SYS_DOWN || code == SYS_HALT)
> -		/* Disable Watchdog */
> -		ds1374_wdt_disable();
> -	return NOTIFY_DONE;
> -}
> -
> -static const struct file_operations ds1374_wdt_fops = {
> -	.owner			= THIS_MODULE,
> -	.read			= ds1374_wdt_read,
> -	.unlocked_ioctl		= ds1374_wdt_unlocked_ioctl,
> -	.compat_ioctl		= compat_ptr_ioctl,
> -	.write			= ds1374_wdt_write,
> -	.open                   = ds1374_wdt_open,
> -	.release                = ds1374_wdt_release,
> -	.llseek			= no_llseek,
> -};
> -
> -static struct miscdevice ds1374_miscdev = {
> -	.minor          = WATCHDOG_MINOR,
> -	.name           = "watchdog",
> -	.fops           = &ds1374_wdt_fops,
> -};
> -
> -static struct notifier_block ds1374_wdt_notifier = {
> -	.notifier_call = ds1374_wdt_notify_sys,
> +static const struct watchdog_ops ds1374_wdt_ops = {
> +	.owner          = THIS_MODULE,
> +	.start          = ds1374_wdt_start,
> +	.stop           = ds1374_wdt_stop,
> +	.set_timeout    = ds1374_wdt_settimeout,
>  };
> -
>  #endif /*CONFIG_RTC_DRV_DS1374_WDT*/
>  /*
>   *****************************************************************************
> @@ -652,16 +515,22 @@ static int ds1374_probe(struct i2c_client *client,
>  		return ret;
>  
>  #ifdef CONFIG_RTC_DRV_DS1374_WDT
> -	save_client = client;
> -	ret = misc_register(&ds1374_miscdev);
> +	ds1374->wdt.info = &ds1374_wdt_info;
> +	ds1374->wdt.ops = &ds1374_wdt_ops;
> +	ds1374->wdt.timeout = TIMER_MARGIN_DEFAULT;
> +	ds1374->wdt.min_timeout = TIMER_MARGIN_MIN;
> +	ds1374->wdt.max_timeout = TIMER_MARGIN_MAX;
> +
> +	watchdog_init_timeout(&ds1374->wdt, wdt_margin, &client->dev);
> +	watchdog_set_nowayout(&ds1374->wdt, nowayout);
> +	watchdog_stop_on_reboot(&ds1374->wdt);
> +	watchdog_stop_on_unregister(&ds1374->wdt);
> +	watchdog_set_drvdata(&ds1374->wdt, ds1374);
> +	ds1374_wdt_settimeout(&ds1374->wdt, ds1374->wdt.timeout);
> +
> +	ret = devm_watchdog_register_device(&client->dev, &ds1374->wdt);
>  	if (ret)
>  		return ret;
> -	ret = register_reboot_notifier(&ds1374_wdt_notifier);
> -	if (ret) {
> -		misc_deregister(&ds1374_miscdev);
> -		return ret;
> -	}
> -	ds1374_wdt_settimeout(131072);
>  #endif
>  
>  	return 0;
> @@ -670,11 +539,6 @@ static int ds1374_probe(struct i2c_client *client,
>  static int ds1374_remove(struct i2c_client *client)
>  {
>  	struct ds1374 *ds1374 = i2c_get_clientdata(client);
> -#ifdef CONFIG_RTC_DRV_DS1374_WDT
> -	misc_deregister(&ds1374_miscdev);
> -	ds1374_miscdev.parent = NULL;
> -	unregister_reboot_notifier(&ds1374_wdt_notifier);
> -#endif
>  
>  	if (client->irq > 0) {
>  		mutex_lock(&ds1374->mutex);
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: [PATCH v4] rtc: rtc-ds1374: wdt: Use watchdog core for watchdog part
  2020-07-09 15:28 ` Guenter Roeck
@ 2020-07-13  2:38   ` Johnson CH Chen (陳昭勳)
  0 siblings, 0 replies; 4+ messages in thread
From: Johnson CH Chen (陳昭勳) @ 2020-07-13  2:38 UTC (permalink / raw)
  To: alexandre.belloni
  Cc: Wim Van Sebroeck, a.zummo, Guenter Roeck, linux-kernel,
	linux-rtc, linux-watchdog

Dear Alexandre Belloni,

> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> Sent: Thursday, July 9, 2020 11:28 PM
> To: Johnson CH Chen (陳昭勳) <JohnsonCH.Chen@moxa.com>;
> linux-kernel@vger.kernel.org; linux-rtc@vger.kernel.org;
> linux-watchdog@vger.kernel.org
> Cc: Wim Van Sebroeck <wim@linux-watchdog.org>; a.zummo@towertech.it;
> alexandre.belloni@bootlin.com
> Subject: Re: [PATCH v4] rtc: rtc-ds1374: wdt: Use watchdog core for watchdog
> part
> 
> On 7/8/20 11:34 PM, Johnson CH Chen (陳昭勳) wrote:
> > Let ds1374 watchdog use watchdog core functions. It also includes
> > improving watchdog timer setting and nowayout, and just uses ioctl()
> > of watchdog core.
> >
> > Signed-off-by: Johnson Chen <johnsonch.chen@moxa.com>
> 
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> 

If there is no other big issue, could you please apply this patch? So many thanks!

Best regards,
Johnson

> > ---
> > v3->v4:
> > - Fix coding styles
> > - Remove dev_info() in ds1374_wdt_settimeout()
> > - Fix missing error check
> >
> > v2->v3:
> > - Fix a problem reported by WATCHDOG_CORE if WATCHDOG
> > - Remove save_client
> > - Let wdt_margin be 0 for watchdog_init_timeout()
> > - Use dev_info() rather than pr_info()
> > - Avoid more strings in this driver
> >
> > v1->v2:
> > - Use ds1374_wdt_settimeout() before registering the watchdog
> > - Remove watchdog_unregister_device() because
> > devm_watchdog_register_device() is used
> > - Remove ds1374_wdt_ping()
> > - TIMER_MARGIN_MAX to 4095 for 24-bit value
> > - Keep wdt_margin
> > - Fix coding styles
> >
> >  drivers/rtc/Kconfig      |   1 +
> >  drivers/rtc/rtc-ds1374.c | 258
> > +++++++++------------------------------
> >  2 files changed, 62 insertions(+), 197 deletions(-)
> >
> > diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig index
> > b54d87d45c89..c25d51f35f0c 100644
> > --- a/drivers/rtc/Kconfig
> > +++ b/drivers/rtc/Kconfig
> > @@ -282,6 +282,7 @@ config RTC_DRV_DS1374  config
> RTC_DRV_DS1374_WDT
> >  	bool "Dallas/Maxim DS1374 watchdog timer"
> >  	depends on RTC_DRV_DS1374
> > +	select WATCHDOG_CORE if WATCHDOG
> >  	help
> >  	  If you say Y here you will get support for the
> >  	  watchdog timer in the Dallas Semiconductor DS1374 diff --git
> > a/drivers/rtc/rtc-ds1374.c b/drivers/rtc/rtc-ds1374.c index
> > 9c51a12cf70f..c71065d26cd2 100644
> > --- a/drivers/rtc/rtc-ds1374.c
> > +++ b/drivers/rtc/rtc-ds1374.c
> > @@ -46,6 +46,7 @@
> >  #define DS1374_REG_WDALM2	0x06
> >  #define DS1374_REG_CR		0x07 /* Control */
> >  #define DS1374_REG_CR_AIE	0x01 /* Alarm Int. Enable */
> > +#define DS1374_REG_CR_WDSTR	0x08 /* 1=INT, 0=RST */
> >  #define DS1374_REG_CR_WDALM	0x20 /* 1=Watchdog, 0=Alarm */
> >  #define DS1374_REG_CR_WACE	0x40 /* WD/Alarm counter enable */
> >  #define DS1374_REG_SR		0x08 /* Status */
> > @@ -71,7 +72,9 @@ struct ds1374 {
> >  	struct i2c_client *client;
> >  	struct rtc_device *rtc;
> >  	struct work_struct work;
> > -
> > +#ifdef CONFIG_RTC_DRV_DS1374_WDT
> > +	struct watchdog_device wdt;
> > +#endif
> >  	/* The mutex protects alarm operations, and prevents a race
> >  	 * between the enable_irq() in the workqueue and the free_irq()
> >  	 * in the remove function.
> > @@ -369,238 +372,98 @@ static const struct rtc_class_ops ds1374_rtc_ops
> = {
> >   *
> >
> ****************************************************************
> *************
> >   */
> > -static struct i2c_client *save_client;
> >  /* Default margin */
> > -#define WD_TIMO 131762
> > +#define TIMER_MARGIN_DEFAULT	32
> > +#define TIMER_MARGIN_MIN	1
> > +#define TIMER_MARGIN_MAX	4095 /* 24-bit value */
> >
> >  #define DRV_NAME "DS1374 Watchdog"
> >
> > -static int wdt_margin = WD_TIMO;
> > -static unsigned long wdt_is_open;
> > +static int wdt_margin;
> >  module_param(wdt_margin, int, 0);
> >  MODULE_PARM_DESC(wdt_margin, "Watchdog timeout in seconds
> (default
> > 32s)");
> >
> > +static bool nowayout = WATCHDOG_NOWAYOUT; module_param(nowayout,
> > +bool, 0); MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped
> once
> > +started (default ="
> > +		__MODULE_STRING(WATCHDOG_NOWAYOUT)")");
> > +
> >  static const struct watchdog_info ds1374_wdt_info = {
> >  	.identity       = "DS1374 WTD",
> >  	.options        = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING |
> >  						WDIOF_MAGICCLOSE,
> >  };
> >
> > -static int ds1374_wdt_settimeout(unsigned int timeout)
> > +static int ds1374_wdt_settimeout(struct watchdog_device *wdt,
> > +unsigned int timeout)
> >  {
> > -	int ret = -ENOIOCTLCMD;
> > -	int cr;
> > +	struct ds1374 *ds1374 = watchdog_get_drvdata(wdt);
> > +	struct i2c_client *client = ds1374->client;
> > +	int ret, cr;
> >
> > -	ret = cr = i2c_smbus_read_byte_data(save_client, DS1374_REG_CR);
> > -	if (ret < 0)
> > -		goto out;
> > +	wdt->timeout = timeout;
> > +
> > +	cr = i2c_smbus_read_byte_data(client, DS1374_REG_CR);
> > +	if (cr < 0)
> > +		return cr;
> >
> >  	/* Disable any existing watchdog/alarm before setting the new one */
> >  	cr &= ~DS1374_REG_CR_WACE;
> >
> > -	ret = i2c_smbus_write_byte_data(save_client, DS1374_REG_CR, cr);
> > +	ret = i2c_smbus_write_byte_data(client, DS1374_REG_CR, cr);
> >  	if (ret < 0)
> > -		goto out;
> > +		return ret;
> >
> >  	/* Set new watchdog time */
> > -	ret = ds1374_write_rtc(save_client, timeout, DS1374_REG_WDALM0, 3);
> > -	if (ret) {
> > -		pr_info("couldn't set new watchdog time\n");
> > -		goto out;
> > -	}
> > +	timeout = timeout * 4096;
> > +	ret = ds1374_write_rtc(client, timeout, DS1374_REG_WDALM0, 3);
> > +	if (ret)
> > +		return ret;
> >
> >  	/* Enable watchdog timer */
> >  	cr |= DS1374_REG_CR_WACE | DS1374_REG_CR_WDALM;
> > +	cr &= ~DS1374_REG_CR_WDSTR;/* for RST PIN */
> >  	cr &= ~DS1374_REG_CR_AIE;
> >
> > -	ret = i2c_smbus_write_byte_data(save_client, DS1374_REG_CR, cr);
> > +	ret = i2c_smbus_write_byte_data(client, DS1374_REG_CR, cr);
> >  	if (ret < 0)
> > -		goto out;
> > +		return ret;
> >
> >  	return 0;
> > -out:
> > -	return ret;
> >  }
> >
> > -
> >  /*
> >   * Reload the watchdog timer.  (ie, pat the watchdog)
> >   */
> > -static void ds1374_wdt_ping(void)
> > +static int ds1374_wdt_start(struct watchdog_device *wdt)
> >  {
> > +	struct ds1374 *ds1374 = watchdog_get_drvdata(wdt);
> >  	u32 val;
> > -	int ret = 0;
> >
> > -	ret = ds1374_read_rtc(save_client, &val, DS1374_REG_WDALM0, 3);
> > -	if (ret)
> > -		pr_info("WD TICK FAIL!!!!!!!!!! %i\n", ret);
> > +	return ds1374_read_rtc(ds1374->client, &val, DS1374_REG_WDALM0, 3);
> >  }
> >
> > -static void ds1374_wdt_disable(void)
> > +static int ds1374_wdt_stop(struct watchdog_device *wdt)
> >  {
> > +	struct ds1374 *ds1374 = watchdog_get_drvdata(wdt);
> > +	struct i2c_client *client = ds1374->client;
> >  	int cr;
> >
> > -	cr = i2c_smbus_read_byte_data(save_client, DS1374_REG_CR);
> > +	cr = i2c_smbus_read_byte_data(client, DS1374_REG_CR);
> > +	if (cr < 0)
> > +		return cr;
> > +
> >  	/* Disable watchdog timer */
> >  	cr &= ~DS1374_REG_CR_WACE;
> >
> > -	i2c_smbus_write_byte_data(save_client, DS1374_REG_CR, cr);
> > -}
> > -
> > -/*
> > - * Watchdog device is opened, and watchdog starts running.
> > - */
> > -static int ds1374_wdt_open(struct inode *inode, struct file *file) -{
> > -	struct ds1374 *ds1374 = i2c_get_clientdata(save_client);
> > -
> > -	if (MINOR(inode->i_rdev) == WATCHDOG_MINOR) {
> > -		mutex_lock(&ds1374->mutex);
> > -		if (test_and_set_bit(0, &wdt_is_open)) {
> > -			mutex_unlock(&ds1374->mutex);
> > -			return -EBUSY;
> > -		}
> > -		/*
> > -		 *      Activate
> > -		 */
> > -		wdt_is_open = 1;
> > -		mutex_unlock(&ds1374->mutex);
> > -		return stream_open(inode, file);
> > -	}
> > -	return -ENODEV;
> > -}
> > -
> > -/*
> > - * Close the watchdog device.
> > - */
> > -static int ds1374_wdt_release(struct inode *inode, struct file *file)
> > -{
> > -	if (MINOR(inode->i_rdev) == WATCHDOG_MINOR)
> > -		clear_bit(0, &wdt_is_open);
> > -
> > -	return 0;
> > +	return i2c_smbus_write_byte_data(client, DS1374_REG_CR, cr);
> >  }
> >
> > -/*
> > - * Pat the watchdog whenever device is written to.
> > - */
> > -static ssize_t ds1374_wdt_write(struct file *file, const char __user *data,
> > -				size_t len, loff_t *ppos)
> > -{
> > -	if (len) {
> > -		ds1374_wdt_ping();
> > -		return 1;
> > -	}
> > -	return 0;
> > -}
> > -
> > -static ssize_t ds1374_wdt_read(struct file *file, char __user *data,
> > -				size_t len, loff_t *ppos)
> > -{
> > -	return 0;
> > -}
> > -
> > -/*
> > - * Handle commands from user-space.
> > - */
> > -static long ds1374_wdt_ioctl(struct file *file, unsigned int cmd,
> > -							unsigned long arg)
> > -{
> > -	int new_margin, options;
> > -
> > -	switch (cmd) {
> > -	case WDIOC_GETSUPPORT:
> > -		return copy_to_user((struct watchdog_info __user *)arg,
> > -		&ds1374_wdt_info, sizeof(ds1374_wdt_info)) ? -EFAULT : 0;
> > -
> > -	case WDIOC_GETSTATUS:
> > -	case WDIOC_GETBOOTSTATUS:
> > -		return put_user(0, (int __user *)arg);
> > -	case WDIOC_KEEPALIVE:
> > -		ds1374_wdt_ping();
> > -		return 0;
> > -	case WDIOC_SETTIMEOUT:
> > -		if (get_user(new_margin, (int __user *)arg))
> > -			return -EFAULT;
> > -
> > -		/* the hardware's tick rate is 4096 Hz, so
> > -		 * the counter value needs to be scaled accordingly
> > -		 */
> > -		new_margin <<= 12;
> > -		if (new_margin < 1 || new_margin > 16777216)
> > -			return -EINVAL;
> > -
> > -		wdt_margin = new_margin;
> > -		ds1374_wdt_settimeout(new_margin);
> > -		ds1374_wdt_ping();
> > -		/* fallthrough */
> > -	case WDIOC_GETTIMEOUT:
> > -		/* when returning ... inverse is true */
> > -		return put_user((wdt_margin >> 12), (int __user *)arg);
> > -	case WDIOC_SETOPTIONS:
> > -		if (copy_from_user(&options, (int __user *)arg, sizeof(int)))
> > -			return -EFAULT;
> > -
> > -		if (options & WDIOS_DISABLECARD) {
> > -			pr_info("disable watchdog\n");
> > -			ds1374_wdt_disable();
> > -			return 0;
> > -		}
> > -
> > -		if (options & WDIOS_ENABLECARD) {
> > -			pr_info("enable watchdog\n");
> > -			ds1374_wdt_settimeout(wdt_margin);
> > -			ds1374_wdt_ping();
> > -			return 0;
> > -		}
> > -		return -EINVAL;
> > -	}
> > -	return -ENOTTY;
> > -}
> > -
> > -static long ds1374_wdt_unlocked_ioctl(struct file *file, unsigned int cmd,
> > -			unsigned long arg)
> > -{
> > -	int ret;
> > -	struct ds1374 *ds1374 = i2c_get_clientdata(save_client);
> > -
> > -	mutex_lock(&ds1374->mutex);
> > -	ret = ds1374_wdt_ioctl(file, cmd, arg);
> > -	mutex_unlock(&ds1374->mutex);
> > -
> > -	return ret;
> > -}
> > -
> > -static int ds1374_wdt_notify_sys(struct notifier_block *this,
> > -			unsigned long code, void *unused)
> > -{
> > -	if (code == SYS_DOWN || code == SYS_HALT)
> > -		/* Disable Watchdog */
> > -		ds1374_wdt_disable();
> > -	return NOTIFY_DONE;
> > -}
> > -
> > -static const struct file_operations ds1374_wdt_fops = {
> > -	.owner			= THIS_MODULE,
> > -	.read			= ds1374_wdt_read,
> > -	.unlocked_ioctl		= ds1374_wdt_unlocked_ioctl,
> > -	.compat_ioctl		= compat_ptr_ioctl,
> > -	.write			= ds1374_wdt_write,
> > -	.open                   = ds1374_wdt_open,
> > -	.release                = ds1374_wdt_release,
> > -	.llseek			= no_llseek,
> > -};
> > -
> > -static struct miscdevice ds1374_miscdev = {
> > -	.minor          = WATCHDOG_MINOR,
> > -	.name           = "watchdog",
> > -	.fops           = &ds1374_wdt_fops,
> > -};
> > -
> > -static struct notifier_block ds1374_wdt_notifier = {
> > -	.notifier_call = ds1374_wdt_notify_sys,
> > +static const struct watchdog_ops ds1374_wdt_ops = {
> > +	.owner          = THIS_MODULE,
> > +	.start          = ds1374_wdt_start,
> > +	.stop           = ds1374_wdt_stop,
> > +	.set_timeout    = ds1374_wdt_settimeout,
> >  };
> > -
> >  #endif /*CONFIG_RTC_DRV_DS1374_WDT*/
> >  /*
> >
> >
> ****************************************************************
> ******
> > ******* @@ -652,16 +515,22 @@ static int ds1374_probe(struct
> > i2c_client *client,
> >  		return ret;
> >
> >  #ifdef CONFIG_RTC_DRV_DS1374_WDT
> > -	save_client = client;
> > -	ret = misc_register(&ds1374_miscdev);
> > +	ds1374->wdt.info = &ds1374_wdt_info;
> > +	ds1374->wdt.ops = &ds1374_wdt_ops;
> > +	ds1374->wdt.timeout = TIMER_MARGIN_DEFAULT;
> > +	ds1374->wdt.min_timeout = TIMER_MARGIN_MIN;
> > +	ds1374->wdt.max_timeout = TIMER_MARGIN_MAX;
> > +
> > +	watchdog_init_timeout(&ds1374->wdt, wdt_margin, &client->dev);
> > +	watchdog_set_nowayout(&ds1374->wdt, nowayout);
> > +	watchdog_stop_on_reboot(&ds1374->wdt);
> > +	watchdog_stop_on_unregister(&ds1374->wdt);
> > +	watchdog_set_drvdata(&ds1374->wdt, ds1374);
> > +	ds1374_wdt_settimeout(&ds1374->wdt, ds1374->wdt.timeout);
> > +
> > +	ret = devm_watchdog_register_device(&client->dev, &ds1374->wdt);
> >  	if (ret)
> >  		return ret;
> > -	ret = register_reboot_notifier(&ds1374_wdt_notifier);
> > -	if (ret) {
> > -		misc_deregister(&ds1374_miscdev);
> > -		return ret;
> > -	}
> > -	ds1374_wdt_settimeout(131072);
> >  #endif
> >
> >  	return 0;
> > @@ -670,11 +539,6 @@ static int ds1374_probe(struct i2c_client
> > *client,  static int ds1374_remove(struct i2c_client *client)  {
> >  	struct ds1374 *ds1374 = i2c_get_clientdata(client); -#ifdef
> > CONFIG_RTC_DRV_DS1374_WDT
> > -	misc_deregister(&ds1374_miscdev);
> > -	ds1374_miscdev.parent = NULL;
> > -	unregister_reboot_notifier(&ds1374_wdt_notifier);
> > -#endif
> >
> >  	if (client->irq > 0) {
> >  		mutex_lock(&ds1374->mutex);
> >


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v4] rtc: rtc-ds1374: wdt: Use watchdog core for watchdog part
  2020-07-09  6:34 [PATCH v4] rtc: rtc-ds1374: wdt: Use watchdog core for watchdog part Johnson CH Chen (陳昭勳)
  2020-07-09 15:28 ` Guenter Roeck
@ 2020-07-16  9:26 ` Alexandre Belloni
  1 sibling, 0 replies; 4+ messages in thread
From: Alexandre Belloni @ 2020-07-16  9:26 UTC (permalink / raw)
  To: linux-rtc, Johnson CH Chen, linux-watchdog, linux-kernel
  Cc: Alexandre Belloni, Wim Van Sebroeck, Guenter Roeck, a.zummo

On Thu, 9 Jul 2020 06:34:06 +0000, Johnson CH Chen (陳昭勳) wrote:
> Let ds1374 watchdog use watchdog core functions. It also includes
> improving watchdog timer setting and nowayout, and just uses ioctl()
> of watchdog core.

Applied, thanks!

[1/1] rtc: ds1374: wdt: Use watchdog core for watchdog part
      commit: d3de4beb14a887754b83843bf23b6dbe8d022764

Best regards,
-- 
Alexandre Belloni <alexandre.belloni@bootlin.com>

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-09  6:34 [PATCH v4] rtc: rtc-ds1374: wdt: Use watchdog core for watchdog part Johnson CH Chen (陳昭勳)
2020-07-09 15:28 ` Guenter Roeck
2020-07-13  2:38   ` Johnson CH Chen (陳昭勳)
2020-07-16  9:26 ` Alexandre Belloni

Linux-RTC Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-rtc/0 linux-rtc/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-rtc linux-rtc/ https://lore.kernel.org/linux-rtc \
		linux-rtc@vger.kernel.org
	public-inbox-index linux-rtc

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-rtc


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git