Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220918140829.443722-1-git@vladimir.panteleev.md> <aabb7c21-9a80-696e-6a38-29de57e025ba@roeck-us.net>
In-Reply-To: <aabb7c21-9a80-696e-6a38-29de57e025ba@roeck-us.net>
From:   Vladimir Panteleev <git@vladimir.panteleev.md>
Date:   Mon, 19 Sep 2022 05:58:26 +0000
Message-ID: <CAHhfkvw_U_uF1UFcLTBUsw=_YoM_7pi3tw3KCovTT6PZTnH0ig@mail.gmail.com>
Subject: Re: [PATCH] watchdog: sp5100_tco: Add "action" module parameter
To:     Guenter Roeck <linux@roeck-us.net>
Cc:     Wim Van Sebroeck <wim@linux-watchdog.org>,
        linux-watchdog@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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