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

[Guenter Roeck]

On Mon, Jun 10, 2019 at 08:07:35AM +0000, Amy.Shih@xxxxxxxxxxxxxxxx wrote:
> From: "amy.shih" <amy.shih@xxxxxxxxxxxxxxxx>
> 
> 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@xxxxxxxxxxxxxxxx>
> ---

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

More detailed review will follow.

Thanks,
Guenter