All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] watchdog: simplify definitions of WATCHDOG_NOWAYOUT(_INIT_STATUS)?
@ 2014-09-09 20:18 Uwe Kleine-König
  2014-09-11 15:42 ` Guenter Roeck
  2014-09-16 15:49 ` Uwe Kleine-König
  0 siblings, 2 replies; 4+ messages in thread
From: Uwe Kleine-König @ 2014-09-09 20:18 UTC (permalink / raw)
  To: Wim Van Sebroeck; +Cc: linux-watchdog, kernel

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Hello,

maybe it's arguable if the newly introduced way to define
WATCHDOG_NOWAYOUT and WATCHDOG_NOWAYOUT_INIT_STATUS is really simpler.
But at least it's five lines shorter and makes the questionable naming
of cpp symbols more obvious. (Who can tell the difference between
the semantic of WATCHDOG_NOWAYOUT and WDOG_NO_WAY_OUT without further
checking?)

Best regards
Uwe

 include/linux/watchdog.h | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
index 2a3038ee17a3..395b70e0eccf 100644
--- a/include/linux/watchdog.h
+++ b/include/linux/watchdog.h
@@ -97,13 +97,8 @@ struct watchdog_device {
 #define WDOG_UNREGISTERED	4	/* Has the device been unregistered */
 };
 
-#ifdef CONFIG_WATCHDOG_NOWAYOUT
-#define WATCHDOG_NOWAYOUT		1
-#define WATCHDOG_NOWAYOUT_INIT_STATUS	(1 << WDOG_NO_WAY_OUT)
-#else
-#define WATCHDOG_NOWAYOUT		0
-#define WATCHDOG_NOWAYOUT_INIT_STATUS	0
-#endif
+#define WATCHDOG_NOWAYOUT		IS_BUILTIN(CONFIG_WATCHDOG_NOWAYOUT)
+#define WATCHDOG_NOWAYOUT_INIT_STATUS	(WATCHDOG_NOWAYOUT << WDOG_NO_WAY_OUT)
 
 /* Use the following function to check whether or not the watchdog is active */
 static inline bool watchdog_active(struct watchdog_device *wdd)
-- 
2.1.0


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

* Re: [PATCH] watchdog: simplify definitions of WATCHDOG_NOWAYOUT(_INIT_STATUS)?
  2014-09-09 20:18 [PATCH] watchdog: simplify definitions of WATCHDOG_NOWAYOUT(_INIT_STATUS)? Uwe Kleine-König
@ 2014-09-11 15:42 ` Guenter Roeck
  2014-09-16 15:49 ` Uwe Kleine-König
  1 sibling, 0 replies; 4+ messages in thread
From: Guenter Roeck @ 2014-09-11 15:42 UTC (permalink / raw)
  To: Uwe Kleine-König; +Cc: Wim Van Sebroeck, linux-watchdog, kernel

On Tue, Sep 09, 2014 at 10:18:31PM +0200, Uwe Kleine-König wrote:
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> Hello,
> 
> maybe it's arguable if the newly introduced way to define
> WATCHDOG_NOWAYOUT and WATCHDOG_NOWAYOUT_INIT_STATUS is really simpler.
> But at least it's five lines shorter and makes the questionable naming
> of cpp symbols more obvious. (Who can tell the difference between
> the semantic of WATCHDOG_NOWAYOUT and WDOG_NO_WAY_OUT without further
> checking?)
> 
I like it.

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

Guenter

> Best regards
> Uwe
> 
>  include/linux/watchdog.h | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
> index 2a3038ee17a3..395b70e0eccf 100644
> --- a/include/linux/watchdog.h
> +++ b/include/linux/watchdog.h
> @@ -97,13 +97,8 @@ struct watchdog_device {
>  #define WDOG_UNREGISTERED	4	/* Has the device been unregistered */
>  };
>  
> -#ifdef CONFIG_WATCHDOG_NOWAYOUT
> -#define WATCHDOG_NOWAYOUT		1
> -#define WATCHDOG_NOWAYOUT_INIT_STATUS	(1 << WDOG_NO_WAY_OUT)
> -#else
> -#define WATCHDOG_NOWAYOUT		0
> -#define WATCHDOG_NOWAYOUT_INIT_STATUS	0
> -#endif
> +#define WATCHDOG_NOWAYOUT		IS_BUILTIN(CONFIG_WATCHDOG_NOWAYOUT)
> +#define WATCHDOG_NOWAYOUT_INIT_STATUS	(WATCHDOG_NOWAYOUT << WDOG_NO_WAY_OUT)
>  
>  /* Use the following function to check whether or not the watchdog is active */
>  static inline bool watchdog_active(struct watchdog_device *wdd)
> -- 
> 2.1.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH] watchdog: simplify definitions of WATCHDOG_NOWAYOUT(_INIT_STATUS)?
  2014-09-09 20:18 [PATCH] watchdog: simplify definitions of WATCHDOG_NOWAYOUT(_INIT_STATUS)? Uwe Kleine-König
  2014-09-11 15:42 ` Guenter Roeck
@ 2014-09-16 15:49 ` Uwe Kleine-König
  2014-09-18 15:32   ` Guenter Roeck
  1 sibling, 1 reply; 4+ messages in thread
From: Uwe Kleine-König @ 2014-09-16 15:49 UTC (permalink / raw)
  To: Wim Van Sebroeck; +Cc: linux-watchdog, kernel

Hello Wim,

On Tue, Sep 09, 2014 at 10:18:31PM +0200, Uwe Kleine-König wrote:
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> Hello,
> 
> maybe it's arguable if the newly introduced way to define
> WATCHDOG_NOWAYOUT and WATCHDOG_NOWAYOUT_INIT_STATUS is really simpler.
> But at least it's five lines shorter and makes the questionable naming
> of cpp symbols more obvious. (Who can tell the difference between
> the semantic of WATCHDOG_NOWAYOUT and WDOG_NO_WAY_OUT without further
> checking?)
Do you consider applying this patch?

Uwe

>  include/linux/watchdog.h | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
> index 2a3038ee17a3..395b70e0eccf 100644
> --- a/include/linux/watchdog.h
> +++ b/include/linux/watchdog.h
> @@ -97,13 +97,8 @@ struct watchdog_device {
>  #define WDOG_UNREGISTERED	4	/* Has the device been unregistered */
>  };
>  
> -#ifdef CONFIG_WATCHDOG_NOWAYOUT
> -#define WATCHDOG_NOWAYOUT		1
> -#define WATCHDOG_NOWAYOUT_INIT_STATUS	(1 << WDOG_NO_WAY_OUT)
> -#else
> -#define WATCHDOG_NOWAYOUT		0
> -#define WATCHDOG_NOWAYOUT_INIT_STATUS	0
> -#endif
> +#define WATCHDOG_NOWAYOUT		IS_BUILTIN(CONFIG_WATCHDOG_NOWAYOUT)
> +#define WATCHDOG_NOWAYOUT_INIT_STATUS	(WATCHDOG_NOWAYOUT << WDOG_NO_WAY_OUT)
>  
>  /* Use the following function to check whether or not the watchdog is active */
>  static inline bool watchdog_active(struct watchdog_device *wdd)
> -- 
> 2.1.0
> 
> 

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

* Re: [PATCH] watchdog: simplify definitions of WATCHDOG_NOWAYOUT(_INIT_STATUS)?
  2014-09-16 15:49 ` Uwe Kleine-König
@ 2014-09-18 15:32   ` Guenter Roeck
  0 siblings, 0 replies; 4+ messages in thread
From: Guenter Roeck @ 2014-09-18 15:32 UTC (permalink / raw)
  To: Uwe Kleine-König; +Cc: Wim Van Sebroeck, linux-watchdog, kernel

On Tue, Sep 16, 2014 at 05:49:55PM +0200, Uwe Kleine-König wrote:
> Hello Wim,
> 
> On Tue, Sep 09, 2014 at 10:18:31PM +0200, Uwe Kleine-König wrote:
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > ---
> > Hello,
> > 
> > maybe it's arguable if the newly introduced way to define
> > WATCHDOG_NOWAYOUT and WATCHDOG_NOWAYOUT_INIT_STATUS is really simpler.
> > But at least it's five lines shorter and makes the questionable naming
> > of cpp symbols more obvious. (Who can tell the difference between
> > the semantic of WATCHDOG_NOWAYOUT and WDOG_NO_WAY_OUT without further
> > checking?)
> Do you consider applying this patch?
> 
At least I have it in my watchdog-next branch.

Guenter
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

end of thread, other threads:[~2014-09-18 15:32 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-09 20:18 [PATCH] watchdog: simplify definitions of WATCHDOG_NOWAYOUT(_INIT_STATUS)? Uwe Kleine-König
2014-09-11 15:42 ` Guenter Roeck
2014-09-16 15:49 ` Uwe Kleine-König
2014-09-18 15:32   ` Guenter Roeck

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.