On 3/8/21 3:21 AM, Flavio Suligoi wrote:
> This patch series add a new way to consider the module parameters for the
> watchdog module.
>
> Instead of adding this kind of module parameters independently to each
> driver, the best solution is declaring each feature only once,
> in the watchdog core.
>
I agree to and like the idea, but I don't see the point of letting drivers
opt in or opt out. This adds a lot of complexity for little if any gain.
Guenter
> Additionally, I added a implementation example of this "global" parameters
> using the module "wdat_wdt"
>
> In details:
>
> ===============================
> Watchdog Core Global Parameters
> ===============================
>
> Information for watchdog kernel modules developers.
>
> Introduction
> ============
>
> Different watchdog modules frequently require the same type of parameters
> (for example: *timeout*, *nowayout* feature, *start_enabled* to start the
> watchdog on module insertion, etc.).
> Instead of adding this kind of module parameters independently to each
> driver, the best solution is declaring each feature only once,
> in the watchdog core.
>
> In this way, each driver can read these "global" parameters and then,
> if needed, can implement them, according to the particular hw watchdog
> characteristic.
>
> Using this approach, it is possible reduce some duplicate code in the *new*
> watchdog drivers and simplify the code maintenance. Moreover, the code
> will be clearer, since the same kind of parameters are often called
> with different names (see Documentation/watchdog/watchdog-parameters.rst).
> Obviously, for compatibility reasons, we cannot remove the already existing
> parameters from the code of the various watchdog modules, but we can use
> this "global" approach for the new watchdog drivers.
>
>
> Global parameters declaration
> ==============================
>
> The global parameters data structure is declared in
> include/linux/watchdog.h, as::
>
> struct watchdog_global_parameters_struct {
> int timeout;
> int ioport;
> int irq;
> unsigned long features;
> /* Bit numbers for features flags */
> #define WDOG_GLOBAL_PARAM_VERBOSE 0
> #define WDOG_GLOBAL_PARAM_TEST_MODE 1
> #define WDOG_GLOBAL_PARAM_START_ENABLED 2
> #define WDOG_GLOBAL_PARAM_NOWAYOUT 3
> };
>
> The variable "feature" is a bitwise flags container, to store boolean
> features, such as:
>
> * nowayout
> * start_enable
> * etc...
>
> Other variables can be added, to store some numerical values and other data
> required.
>
> The global parameters are declared (as usual for the module parameters)
> in the first part of drivers/watchdog/watchdog_core.c file.
> The above global data structure is then managed by the function
> *void global_parameters_init()*, in the same file.
>
> Global parameters use
> =====================
>
> Each watchdog driver, to check if one of the global parameters is enabled,
> can use the corresponding in-line function declared in
> include/linux/watchdog.h.
> At the moment the following functions are ready to use:
>
> * watchdog_global_param_verbose_enabled()
> * watchdog_global_param_test_mode_enabled()
> * watchdog_global_param_start_enabled()
> * watchdog_global_param_nowayout_enabled()
>
>
>
> Flavio Suligoi (2):
> watchdog: add global watchdog kernel module parameters structure
> watchdog: wdat: add start_enable global parameter
>
> Documentation/watchdog/index.rst | 1 +
> .../watchdog-core-global-parameters.rst | 74 +++++++++++++++++++
> drivers/watchdog/watchdog_core.c | 74 +++++++++++++++++++
> drivers/watchdog/wdat_wdt.c | 2 +
> include/linux/watchdog.h | 42 +++++++++++
> 5 files changed, 193 insertions(+)
> create mode 100644 Documentation/watchdog/watchdog-core-global-parameters.rst
>
