From: Guenter Roeck <linux@roeck-us.net>
To: "yuechao.zhao(赵越超)" <yuechao.zhao@advantech.com.cn>
Cc: "345351830@qq.com" <345351830@qq.com>,
"Jean Delvare" <jdelvare@suse.com>,
"linux-hwmon@vger.kernel.org" <linux-hwmon@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"Amy.Shih" <Amy.Shih@advantech.com.tw>,
"Oakley.Ding" <Oakley.Ding@advantech.com.tw>,
"Jia.Sui(贾睢)" <Jia.Sui@advantech.com.cn>,
"shengkui.leng(冷胜魁)" <shengkui.leng@advantech.com.cn>,
"Rainbow.Zhang(張玉)" <Rainbow.Zhang@advantech.com.cn>
Subject: Re: 答复: [v2,1/1] hwmon: (nct7904) Add watchdog function
Date: Fri, 27 Mar 2020 13:54:16 -0700 [thread overview]
Message-ID: <c8dd611e-3ff4-a041-6800-8396dd517e7b@roeck-us.net> (raw)
In-Reply-To: <37833c60d7df45109897dcf35fb1b102@ACNMB2.ACN.ADVANTECH.CORP>
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
next prev parent reply other threads:[~2020-03-27 20:54 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-25 8:22 [v2,1/1] hwmon: (nct7904) Add watchdog function yuechao.zhao
2020-03-25 22:47 ` Guenter Roeck
[not found] ` <37833c60d7df45109897dcf35fb1b102@ACNMB2.ACN.ADVANTECH.CORP>
2020-03-27 20:54 ` Guenter Roeck [this message]
[not found] ` <ded25cd289e24701a3f9008ee4a78158@ACNMB2.ACN.ADVANTECH.CORP>
2020-03-30 6:19 ` 答复: 答复: " Guenter Roeck
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c8dd611e-3ff4-a041-6800-8396dd517e7b@roeck-us.net \
--to=linux@roeck-us.net \
--cc=345351830@qq.com \
--cc=Amy.Shih@advantech.com.tw \
--cc=Jia.Sui@advantech.com.cn \
--cc=Oakley.Ding@advantech.com.tw \
--cc=Rainbow.Zhang@advantech.com.cn \
--cc=jdelvare@suse.com \
--cc=linux-hwmon@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=shengkui.leng@advantech.com.cn \
--cc=yuechao.zhao@advantech.com.cn \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).