Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sat, 08 Apr 2023 03:58:54 +0000 (UTC)
From:   James Seo <james@equiv.tech>
Subject: Re: [PATCH v2] hwmon: add HP WMI Sensors driver
Message-ID: <ZDDmewAgPi/4jpcX@equiv.tech>
References: <20230406152321.42010-1-james@equiv.tech>
 <2257deba-187b-82d2-181c-f1fed08a2ff7@gmx.de>
 <ZC+sgnuy5bssD1DN@vb-22lts>
 <e256d281-49a5-2d9a-7def-fd68e177e926@roeck-us.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <e256d281-49a5-2d9a-7def-fd68e177e926@roeck-us.net>
To:     Guenter Roeck <linux@roeck-us.net>
Cc:     Jean Delvare <jdelvare@suse.com>, linux-hwmon@vger.kernel.org,
        platform-driver-x86@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Transfer-Encoding: 7bit
Precedence: bulk

Greetings,

On Fri, Apr 07, 2023 at 05:54:48AM -0700, Guenter Roeck wrote:
> On 4/6/23 22:39, James Seo wrote:
>> Hi,
>> 
>>> is it guaranteed that faulty sensors wont become operational later?
>>> Also filtering out such sensors would make the support for the hwmon_temp_fault and
>>> hwmon_fan_fault attributes meaningless.
>> 
>> Good point. I can't be certain, but the MOF does seem to imply that
>> sensors can indeed be faulty on just a temporary basis.
>> 
> 
> Your current code would explicitly exclude faulty fans from being listed,
> which does not exactly sound like a good idea.

True enough. I recall my reasoning being that faulty sensors would
still be visible in debugfs. I should have seen the problem then.

>> I'll filter out only the sensors that are "Not Connected" at probe
>> time. My thinking is, even if these might turn into connected sensors
>> later, that would mean the user is e.g. hot-plugging a fan (!), and
>> keeping them could result in a large number (~10 on my Z420) of
>> pointless extra channels. And this would also match the behavior of
>> HP's official utility.
>> 
> Ultimately that is an implementation decision. Are the sensors hot-pluggable ?

HP's WMI object specification allows sensors to be hot-pluggable in
principle. I can't definitively say more than that due to a lack of
test hardware (that whitepaper I referenced is from 2005, after all).

So I think the answer is that it depends on the board and the WMI
implementation. That's also what I meant in my reply to Armin when I
said that I couldn't be certain whether faulty sensors can recover.

But I take your point that the driver should be able to handle it if
the board can.

> If so, how does HP's utility handle the insertion or removal of a sensor (fan) ?

HP's utility just pretty-prints a snapshot of what is in WMI at the
moment when the user clicks a button, and then only for the sensors
that were connected when the utility was first started. It doesn't do
anything special to handle insertion or removal beyond that.

> Either case, it is ok with me if disconnected sensors are not listed.
> Not listing faulty sensors seems like a bad idea, though.
> 
> Guenter

Acknowledged. Faulty sensors will be listed in the next version.

Thanks for reviewing. Further suggestions or concerns from you or
anyone else reading this are both welcome and appreciated.

James