Linux-Watchdog Archive on lore.kernel.org
 help / color / Atom feed
From: Guenter Roeck <linux@roeck-us.net>
To: Wim Van Sebroeck <wim@linux-watchdog.org>
Cc: Paul Gortmaker <paul.gortmaker@windriver.com>,
	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: Mon, 29 Apr 2019 09:28:07 -0700
Message-ID: <20190429162807.GA13388@roeck-us.net> (raw)
In-Reply-To: <20190427094833.GA32761@www.linux-watchdog.org>

On Sat, Apr 27, 2019 at 11:48:34AM +0200, Wim Van Sebroeck wrote:
> Hi All,
> 
> > On Wed, Apr 24, 2019 at 11:37:00AM -0400, Paul Gortmaker wrote:
> > > [Re: [PATCH 2/5] watchdog: watchdog_core: make it explicitly non-modular] On 23/04/2019 (Tue 18:22) Guenter Roeck wrote:
> > > 
> > > > 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
> > > 
> > > I'm confused.  If it is useful, then why not formally make it tristate
> > > so other people can do the same as you do, and nobody is manually making
> > > the change over and over again each time?  I'd support that update.
> > > 
> > No idea. That precedes my involvement in the watchdog subsystem.
> > Let's wait for input from Wim. I have a set of patches ready, but it
> > doesn't make sense to me to submit them if Wim wants to go the non-modular
> > route.
> > 
> > FWIW, I am fine with the other patches except for the npcm patch, because
> > several of the other npcm drivers are buildable as module.
> 
> In general: If systems/devices can't handle modules then we should indeed make sure that we clean it up.
> 
Makes sense.

> For the watchdog core however, I am not in favour of doing that. I also use it as a module when i'm testing.
> I originally wanted to make it tristate (so that it can be loaded as a module), but I didn't had a clean way for the following situation:
> driver built as part of kernel, but watchdog system build as a module. We should imho avoid that.
> So no for this peticular patch and Guenter you can o ahead with another patch to make it tristate.
> 

That should be addressed by "select WATCHDOG_CORE" which is used throughout
the kernel. It would be a problem if we had any "depends on WATCHDOG_CORE".
Fortunately, there are no such dependencies. There are a couple of "depends
on WATCHDOG", but they are all "depends on WATCHDOG" followed by "select
WATCHDOG_CORE" as it should be. So we should be fine; any watchdog driver
built into the kernel forces WATCHDOG_CORE to be built into the kernel as
well.

I'll try to clean up my series and send it out this week. It requires more
than one patch since there are some dependencies on the pretimeout code.

Guenter

  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
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 [this message]
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=20190429162807.GA13388@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