linux-kernel - Re: [PATCH] watchdog: sp5100_tco: Add "action" module parameter

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHhfkvw_U_uF1UFcLTBUsw=_YoM_7pi3tw3KCovTT6PZTnH0ig@mail.gmail.com>
Date:   Mon, 19 Sep 2022 05:58:26 +0000
From:   Vladimir Panteleev <git@...dimir.panteleev.md>
To:     Guenter Roeck <linux@...ck-us.net>
Cc:     Wim Van Sebroeck <wim@...ux-watchdog.org>,
        linux-watchdog@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] watchdog: sp5100_tco: Add "action" module parameter

On Mon, 19 Sept 2022 at 04:17, Guenter Roeck <linux@...ck-us.net> wrote:
>
> On 9/18/22 07:08, Vladimir Panteleev wrote:
> > +MODULE_PARM_DESC(action, "Action taken when watchdog expires, 0 to reset, 1 to poweroff (default="
> > +              __MODULE_STRING(WATCHDOG_ACTION) ")");
> > +
>
> Other module parameters are not visible. I do not see the benefit of
> having this one visible.

My bad

> > -#define SP5100_WDT_ACTION_RESET              BIT(2)
> > +#define SP5100_WDT_ACTION            BIT(2)
>
> I do not see the point of renaming this define.

The bit is just called "action" ("WatchDogAction") in the technical
documentation. I figure that the original author chose to name the
define "ACTION_RESET" instead of just "ACTION" because the original
implementation only ever cleared this bit, therefore only setting the
action to "reset". Now that this is no longer true, calling it simply
"action" to match the spec seemed more appropriate. What do you think?

Thanks!
- Vladimir