From: Guenter Roeck <linux@roeck-us.net>
To: Amy.Shih@advantech.com.tw
Cc: she90122@gmail.com, oakley.ding@advantech.com.tw,
jia.sui@advantech.com.cn, Jean Delvare <jdelvare@suse.com>,
linux-hwmon@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] hwmon: (nct7904) Add extra sysfs support for fan, voltage and temperature.
Date: Mon, 10 Jun 2019 13:19:52 -0700 [thread overview]
Message-ID: <20190610201952.GA13191@roeck-us.net> (raw)
In-Reply-To: <20190610080736.6593-1-Amy.Shih@advantech.com.tw>
On Mon, Jun 10, 2019 at 08:07:35AM +0000, Amy.Shih@advantech.com.tw wrote:
> From: "amy.shih" <amy.shih@advantech.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@advantech.com.tw>
> ---
This is where the changelog should go. Also, patches should be versioned.
More detailed review will follow.
Thanks,
Guenter
next prev parent reply other threads:[~2019-06-10 20:19 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-10 8:07 [PATCH] hwmon: (nct7904) Add extra sysfs support for fan, voltage and temperature Amy.Shih
2019-06-10 20:19 ` Guenter Roeck [this message]
-- strict thread matches above, loose matches on Subject: below --
2019-05-31 10:26 Amy.Shih
2019-06-06 3:58 ` Guenter Roeck
2019-05-31 10:22 Amy.Shih
2019-06-05 20:53 ` 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=20190610201952.GA13191@roeck-us.net \
--to=linux@roeck-us.net \
--cc=Amy.Shih@advantech.com.tw \
--cc=jdelvare@suse.com \
--cc=jia.sui@advantech.com.cn \
--cc=linux-hwmon@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=oakley.ding@advantech.com.tw \
--cc=she90122@gmail.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.