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
