linux-watchdog.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Wim Van Sebroeck <wim@linux-watchdog.org>
To: Guenter Roeck <linux@roeck-us.net>
Cc: Paul Gortmaker <paul.gortmaker@windriver.com>,
	Wim Van Sebroeck <wim@linux-watchdog.org>,
	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: Sat, 27 Apr 2019 11:48:34 +0200	[thread overview]
Message-ID: <20190427094833.GA32761@www.linux-watchdog.org> (raw)
In-Reply-To: <20190424210530.GA29993@roeck-us.net>

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.

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.

Kind regards,
Wim.




  reply	other threads:[~2019-04-27 10:32 UTC|newest]

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 [this message]
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 publicly 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=20190427094833.GA32761@www.linux-watchdog.org \
    --to=wim@linux-watchdog.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=paul.gortmaker@windriver.com \
    --cc=wim@iguana.be \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).