Linux-Watchdog Archive on lore.kernel.org
 help / color / Atom feed
From: Guenter Roeck <linux@roeck-us.net>
To: Paul Gortmaker <paul.gortmaker@windriver.com>,
	Wim Van Sebroeck <wim@linux-watchdog.org>
Cc: linux-watchdog@vger.kernel.org, Wim Van Sebroeck <wim@iguana.be>,
	Alan Cox <alan@lxorguk.ukuu.org.uk>
Subject: Re: [PATCH 2/5] watchdog: watchdog_core: make it explicitly non-modular
Date: Tue, 23 Apr 2019 18:22:00 -0700
Message-ID: <439d89d7-bae6-c5c8-e9bf-5477304bc065@roeck-us.net> (raw)
In-Reply-To: <1556034515-28792-3-git-send-email-paul.gortmaker@windriver.com>

On 4/23/19 8:48 AM, Paul Gortmaker wrote:
> The Kconfig currently controlling compilation of this code is:
> 
> config WATCHDOG_CORE
>         bool "WatchDog Timer Driver Core"
> 
> ...meaning that it currently is not being built as a module by anyone.
> 
> Lets remove the modular code that is essentially orphaned, so that
> when reading the driver there is no doubt it is builtin-only.
> 
> We replace module.h with export.h since the file does export some
> symbols.  We don't add init.h since the file already has that.
> 
> We also delete the MODULE_LICENSE tag etc. since all that information
> is already contained at the top of the file in the comments.
> 

I must admit that I am not at all happy about this change. While not
configurable by default, I used tristate a lot (after enabling it
manually) to test watchdog core code while changing it. It saves a
lot of time to be able to reload the watchdog core without having
to reboot the entire system after each change. Removing the ability
to do that just because it is not enabled in the field and just
to save a few lines of code (and because having modules seems to
have come out of favor lately) does not make sense to me.

I won't NACK the series outright, but I'll leave it up to Wim as
the senior maintainer to decide what he wants to do with it.

Guenter

> Cc: Wim Van Sebroeck <wim@iguana.be>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
> Cc: linux-watchdog@vger.kernel.org
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> ---
>   drivers/watchdog/watchdog_core.c | 15 +--------------
>   1 file changed, 1 insertion(+), 14 deletions(-)
> 
> diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c
> index eb8fa25f8eb2..f9f88f59d181 100644
> --- a/drivers/watchdog/watchdog_core.c
> +++ b/drivers/watchdog/watchdog_core.c
> @@ -28,7 +28,7 @@
>   
>   #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>   
> -#include <linux/module.h>	/* For EXPORT_SYMBOL/module stuff/... */
> +#include <linux/export.h>	/* For EXPORT_SYMBOL stuff */
>   #include <linux/types.h>	/* For standard types */
>   #include <linux/errno.h>	/* For the -ENODEV/... values */
>   #include <linux/kernel.h>	/* For printk/panic/... */
> @@ -359,17 +359,4 @@ static int __init watchdog_init(void)
>   	watchdog_deferred_registration();
>   	return 0;
>   }
> -
> -static void __exit watchdog_exit(void)
> -{
> -	watchdog_dev_exit();
> -	ida_destroy(&watchdog_ida);
> -}
> -
>   subsys_initcall_sync(watchdog_init);
> -module_exit(watchdog_exit);
> -
> -MODULE_AUTHOR("Alan Cox <alan@lxorguk.ukuu.org.uk>");
> -MODULE_AUTHOR("Wim Van Sebroeck <wim@iguana.be>");
> -MODULE_DESCRIPTION("WatchDog Timer Driver Core");
> -MODULE_LICENSE("GPL");
> 


  reply index

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-23 15:48 [PATCH 0/5] wdt: clean up unused modular infrastructure Paul Gortmaker
2019-04-23 15:48 ` [PATCH 1/5] watchdog: rtd119x: drop unused module.h include Paul Gortmaker
2019-04-29 16:38   ` Guenter Roeck
2019-04-23 15:48 ` [PATCH 2/5] watchdog: watchdog_core: make it explicitly non-modular Paul Gortmaker
2019-04-24  1:22   ` Guenter Roeck [this message]
2019-04-24 15:37     ` Paul Gortmaker
2019-04-24 21:05       ` Guenter Roeck
2019-04-27  9:48         ` Wim Van Sebroeck
2019-04-29 16:28           ` Guenter Roeck
2019-04-23 15:48 ` [PATCH 3/5] watchdog: npcm: " Paul Gortmaker
2019-04-29 16:40   ` Guenter Roeck
2019-04-29 18:21     ` Paul Gortmaker
2019-04-23 15:48 ` [PATCH 4/5] watchdog: intel_scu: " Paul Gortmaker
2019-04-29 16:37   ` Guenter Roeck
2019-04-23 15:48 ` [PATCH 5/5] watchdog: coh901327: " Paul Gortmaker
2019-04-23 21:29   ` Linus Walleij
2019-04-29 16:37   ` Guenter Roeck
2019-05-12 15:43 ` [PATCH 0/5] wdt: clean up unused modular infrastructure Avi Fishman

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=439d89d7-bae6-c5c8-e9bf-5477304bc065@roeck-us.net \
    --to=linux@roeck-us.net \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=paul.gortmaker@windriver.com \
    --cc=wim@iguana.be \
    --cc=wim@linux-watchdog.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Watchdog Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-watchdog/0 linux-watchdog/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-watchdog linux-watchdog/ https://lore.kernel.org/linux-watchdog \
		linux-watchdog@vger.kernel.org linux-watchdog@archiver.kernel.org
	public-inbox-index linux-watchdog


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


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