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

[Vladimir Panteleev]

On Mon, 19 Sept 2022 at 04:17, Guenter Roeck <linux@xxxxxxxxxxxx> 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