netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Lunn <andrew@lunn.ch>
To: Vadim Pasternak <vadimp@mellanox.com>,
	Guenter Roeck <linux@roeck-us.net>
Cc: "davem@davemloft.net" <davem@davemloft.net>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"jiri@resnulli.us" <jiri@resnulli.us>
Subject: Re: [PATCH v0 03/12] mlxsw: core: Add core environment module for port temperature reading
Date: Thu, 21 Jun 2018 20:34:40 +0200	[thread overview]
Message-ID: <20180621183440.GA10038@lunn.ch> (raw)
In-Reply-To: <HE1PR0502MB37531F5503D85EB153A6C672A2760@HE1PR0502MB3753.eurprd05.prod.outlook.com>

> Hi Andrew,

Adding Guenter Roeck, the HWMON maintainer.
 
> The temperature of each individual module can be obtained
> through ethtool.

You mean via --module-info?

FYI: I plan to add hwmon support to the kernel SFP code. So if you
ever decide to swap to the kernel SFP code, not your own, the raw
temperatures will be exported.

> The worst temperature is necessary for the system cooling
> control decision.

I would expect the system cooling would understand that.

> Up to 64 SFP/QSFP modules could be connected to the system.
> Some of them could cooper modules, which doesn't provide
> temperature measurement.

SFP modules are hot-plugable. So i would also expect the hwmon devices
to hotplug. If there is no sensor, then there is no hwmon device... If
there is no hwmon device, it plays no part in the thermal control
loop.

> Some of them could be optical modules, providing untrusted
> temperature measurement, which could impact thermal
> control of the system.

Why would you not trust it? Are you saying some modules simply have
broken temperature sensors? Do you have a whitelist/blacklist of
modules?

> Also optical modules could be from the different vendors,  and
> this is real situation, when, f.e. one module has the warning and
> critical thresholds 75C and 85C, while another 70C and 80C.

But hwmon exports both the actual temperature and the alarm
temperatures. I would expect the thermal control code to use all this
information when making its decisions, not just the current
temperature.

> So, nominal temperature is not the case here, we should know the
> "worst" value for the thermal control decision.

What it sounds like to me is you are working around problems in the
thermal control by fudging the raw temperatures. That is the wrong
thing to do. hwmon should export the raw data, and you should fix the
thermal control code to use it correctly.

	Andrew

  reply	other threads:[~2018-06-21 18:34 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-21 15:27 [PATCH v0 00/12] mlxsw thermal monitoring amendments Vadim Pasternak
2018-06-21 15:27 ` [PATCH v0 01/12] mlxsw: spectrum: Move QSFP EEPROM defenitons to common location Vadim Pasternak
2018-06-21 15:27 ` [PATCH v0 02/12] mlxsw: reg: Add MTBR register Vadim Pasternak
2018-06-21 15:27 ` [PATCH v0 03/12] mlxsw: core: Add core environment module for port temperature reading Vadim Pasternak
2018-06-21 17:11   ` Andrew Lunn
2018-06-21 18:14     ` Vadim Pasternak
2018-06-21 18:34       ` Andrew Lunn [this message]
2018-06-21 19:17         ` Vadim Pasternak
2018-06-21 19:49           ` Andrew Lunn
2018-06-21 20:02             ` Vadim Pasternak
2018-06-21 20:18               ` Andrew Lunn
2018-06-21 22:06         ` Guenter Roeck
2018-06-22  9:00           ` Vadim Pasternak
2018-06-21 15:27 ` [PATCH v0 04/12] mlxsw: core: Extend hwmon interface with FAN fault attribute Vadim Pasternak
2018-06-21 15:27 ` [PATCH v0 05/12] mlxsw: core: Extend hwmon interface with port temperature attributes Vadim Pasternak
2018-06-21 15:28 ` [PATCH v0 06/12] mlxsw: core: Add bus frequency capability flag for the bus type Vadim Pasternak
2018-06-21 15:28 ` [PATCH v0 07/12] mlxsw: core: Set different thermal polling time based on " Vadim Pasternak
2018-06-21 15:28 ` [PATCH v0 08/12] mlxsw: core: Modify thermal zone definition Vadim Pasternak
2018-06-21 15:28 ` [PATCH v0 09/12] mlxsw: core: Extend thermal zone operations with get_trend method Vadim Pasternak

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=20180621183440.GA10038@lunn.ch \
    --to=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=jiri@resnulli.us \
    --cc=linux@roeck-us.net \
    --cc=netdev@vger.kernel.org \
    --cc=vadimp@mellanox.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).