linux-kernel - Re: 答复: 答复: [v2,1/1] hwmon: (nct7904) Add watchdog function

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>] [day] [month] [year] [list]

Date:   Sun, 29 Mar 2020 23:19:41 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     yuechao.zhao(赵越超) 
        <yuechao.zhao@...antech.com.cn>
Cc:     "345351830@...com" <345351830@...com>,
        Jean Delvare <jdelvare@...e.com>,
        "linux-hwmon@...r.kernel.org" <linux-hwmon@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "Amy.Shih" <Amy.Shih@...antech.com.tw>,
        "Oakley.Ding" <Oakley.Ding@...antech.com.tw>,
        Jia.Sui(贾睢) <Jia.Sui@...antech.com.cn>,
        shengkui.leng(冷胜魁) 
        <shengkui.leng@...antech.com.cn>,
        Rainbow.Zhang(張玉) 
        <Rainbow.Zhang@...antech.com.cn>
Subject: Re: 答复: 答复: [v2,1/1] hwmon: (nct7904) Add watchdog function

On 3/29/20 10:42 PM, yuechao.zhao(赵越超) wrote:
> Hi Guenter
> 
> Thank you very much.
> I misunderstood your suggestion. (Please make it 60 * 255 (or use a respective define) to clarify where the number comes from)
> So, I will modify it.
> Also, I will define a module parameter which specifies the timeout in minutes instead of seconds.
> 
The module parameter already specifies the timeout in minutes.

Guenter

> Thanks
> Yuechao
> 
> -----邮件原件-----
> 发件人: Guenter Roeck [mailto:groeck7@...il.com] 代表 Guenter Roeck
> 发送时间: 2020年3月28日 4:54
> 收件人: yuechao.zhao(赵越超)
> 抄送: 345351830@...com; Jean Delvare; linux-hwmon@...r.kernel.org; linux-kernel@...r.kernel.org; Amy.Shih; Oakley.Ding; Jia.Sui(贾睢); shengkui.leng(冷胜魁); Rainbow.Zhang(張玉)
> 主题: Re: 答复: [v2,1/1] hwmon: (nct7904) Add watchdog function
> 
> 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
>