linux-kernel - Re: [PATCH] hwmon: (nct7904) Add extra sysfs support for fan, voltage and temperature.

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

Message-ID: <20190610201952.GA13191@roeck-us.net>
Date:   Mon, 10 Jun 2019 13:19:52 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     Amy.Shih@...antech.com.tw
Cc:     she90122@...il.com, oakley.ding@...antech.com.tw,
        jia.sui@...antech.com.cn, Jean Delvare <jdelvare@...e.com>,
        linux-hwmon@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] hwmon: (nct7904) Add extra sysfs support for fan,
 voltage and temperature.

On Mon, Jun 10, 2019 at 08:07:35AM +0000, Amy.Shih@...antech.com.tw wrote:
> From: "amy.shih" <amy.shih@...antech.com.tw>
> 
> This patch do 2 jobs as described in below:
> 
> 1. NCT-7904D also supports reading of channel limitation registers
> and SMI status registers for fan, voltage and temperature monitoring,
> add below sysfs nodes:
> 
> -fan[1-*]_min
> -fan[1-*]_alarm
> -in[0-*]_min
> -in[0-*]_max
> -in[0-*]_alarm

Not really. Voltage attributes still start with 1.

> -temp[1-*]_max
> -temp[1-*]_max_hyst
> -temp[1-*]_emergency
> -temp[1-*]_emergency_hyst

Why emergency and not crit ? The registers are labeled "critical",
and there is no mention of "emergency" in the datasheet.

> -temp[1-*]_alarm
> 
> 2. Add the temp[1-*]_type sysfs to show type of temperature sensor is
> thermal diode, thermistor, AMD SB-TSI or Intel PECI.
> 
> Signed-off-by: amy.shih <amy.shih@...antech.com.tw>
> ---

This is where the changelog should go. Also, patches should be versioned.

More detailed review will follow.

Thanks,
Guenter