netdev - Re: [PATCH v2 net-next 02/16] net: aquantia: implement hwmon api for chip 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]

Date:   Wed, 24 Apr 2019 14:32:33 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Igor Russkikh <Igor.Russkikh@...antia.com>
Cc:     "David S . Miller" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Nikita Danilov <Nikita.Danilov@...antia.com>,
        Dmitry Bogdanov <Dmitry.Bogdanov@...antia.com>,
        Yana Esina <yana.esina@...antia.com>
Subject: Re: [PATCH v2 net-next 02/16] net: aquantia: implement hwmon api for
 chip temperature

On Wed, Apr 24, 2019 at 08:28:46AM +0000, Igor Russkikh wrote:
> 
> >> +static u32 aq_hwmon_temp_config[] = {
> >> +	HWMON_T_INPUT | HWMON_T_LABEL,
> >> +	0,
> > 
> > It would be nice to also have
> > 
> >         HWMON_T_MAX | HWMON_T_MIN |
> >         HWMON_T_CRIT | HWMON_T_LCRIT |
> > 
> > which the PHY probably has.
> > 
> > At gives some degree of context. A temperature on its own of say 65C
> > is hard to interpret. Is it too hot?  But if we have a critical of 85C
> > and a max of 95C we know we are O.K.
> 
> I can give here only suggested constants, since we don't have now any interface
> on reading/configuring these.

Hi Igor

You mean your firmware is blocking you from accessing these PHY
registers?

> Do you think that will be ok?

I think the HWMON maintainer prefers they come from the hardware. You
are also going to get odd results, when the provisioning data for the
PHY is different to what you hard code in the driver. The PHY shuts
down to protect itself, but your hard coded values indicate everything
should be O.K.

I would suggest you stick with the current patch, until you can add
more features to the firmware to allow access to these registers.

	 Andrew