Re: [PATCH v2 net-next 02/16] net: aquantia: implement hwmon api for chip temperature

[Andrew Lunn <andrew@lunn.ch>]

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