lkml.org 
[lkml]   [2020]   [Mar]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: 答复: [v2,1/1] hwmon: (nct7904) Add watchdog function
From
Date
On 3/26/20 7:39 PM, yuechao.zhao(赵越超) wrote:
> Hi Guenter
> Thank you very much for the suggestion
>
> But, I wish to consult you on a few questions.
> The NCT7904 is very special in watchdog function. Its minimum unit is minutes.
> So, what unit do we use for setting the timeout in user space. Minutes or seconds?
> Does the kernel document specify it?
>
Documentation/watchdog/watchdog-api.rst very clearly specifies that
the timeout is set in seconds.

> Also, about this suggestion:
> > + data->wdt.timeout = timeout * 60; /* in seconds */
> > + data->wdt.min_timeout = 1;
> > + data->wdt.max_timeout = 15300;
> Please make it 60 * 255 (or use a respective define) to clarify where the number comes from.
>
> The reason for not use 60 * 255(or use a respective define ) is that we will set the default timeout with
> "timeout" parameter when "insmod nct7904.ko".

This is not an argument for
data->wdt.max_timeout = 15300;
instead of
data->wdt.max_timeout = 60 * 255;
or

#define MAX_TIMEOUT (60 * 255)
...
data->wdt.max_timeout = MAX_TIMEOUT;

> The relevant code as follows:
> > +static int timeout = WATCHDOG_TIMEOUT; module_param(timeout, int, 0);
> > +MODULE_PARM_DESC(timeout, "Watchdog timeout in minutes. 1 <= timeout <= 255, default="
> > + __MODULE_STRING(WATCHODOG_TIMEOUT) ".");
>

I accepted that you want to have a module parameter which specifies
the timeout in minutes instead of seconds (as would be customary).
However, I completely fail to understand what that has to do with
using an unexplained constant of "15300" when setting the maximum
timeout variable instead of "60 * 255" or, as I had suggested, a
define.

Thanks,
Guenter

\
 
 \ /
  Last update: 2020-03-27 21:54    [W:0.069 / U:0.724 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site