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;
Sender: Guenter Roeck <groeck7@gmail.com>
Date:   Sat, 31 Dec 2022 06:52:11 -0800
From:   Guenter Roeck <linux@roeck-us.net>
To:     Armin Wolf <W_Armin@gmx.de>
Cc:     jdelvare@suse.com, linux-hwmon@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/3] hwmon: (ftsteutates) Convert to
 devm_hwmon_device_register_with_info()
Message-ID: <20221231145211.GA2280076@roeck-us.net>
References: <20221224041855.83981-1-W_Armin@gmx.de>
 <20221224041855.83981-3-W_Armin@gmx.de>
 <20221224151835.GA569106@roeck-us.net>
 <b0bfb455-f702-7ac8-230f-01c8e4aa2be3@gmx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <b0bfb455-f702-7ac8-230f-01c8e4aa2be3@gmx.de>
Precedence: bulk

On Sat, Dec 31, 2022 at 12:40:40PM +0100, Armin Wolf wrote:
> Am 24.12.22 um 16:18 schrieb Guenter Roeck:
> 
> > On Sat, Dec 24, 2022 at 05:18:54AM +0100, Armin Wolf wrote:
> > > Convert driver to use devm_hwmon_device_register_with_info()
> > > to reduce module size by 40%. The non-standard fan_source
> > > attribute is replaced with the standard pwmX_auto_channels_temp
> > > attribute and its special behaviour being documented.
> > > Also export fan present status as fanX_fault.
> > There are multiple changes in this patch which are not related
> > to the actual conversion. Please split into separate patches for
> > each functional change.
> > 
> > While I agree with the fan_source -> pwmX_auto_channels_temp
> > change, it needs to be done separately after the conversion,
> > because it is an ABI change which we may have to undo if someone
> > complains.
> > 
> > "export fan present status as fanX_fault" also needs to be a
> > separate patch.
> > 
> > In this context, normally alarms are supposed to auto-clear
> > after reading an attribute, if the condition no longer exists.
> > Is it really necessary to explicitly clear the condition, or
> > would it be possible to auto-clear it after reading the attribute ?
> > 
> > Thanks,
> > Guenter
> 
> According to the datasheet, the alarm flags are "sticky" and need to
> be reset explicitly. Unfortunately, the BMC does not immediately reassign
> those flags in case of the alarm condition being present, but does this at
> the next "poll cycle". In order to emulate auto-clearable alarms, the driver
> would have to constantly poll the BMC, which would still glitch since we do
> not know the duration of a "poll cycle".

Many chips have that problem. The solution is to either accept it
(which many drivers do) or to work around it by cashing reported alarms
and only clearing the alarm bit form the cache after a poll cycle.
The lm90 driver is probably the most sophisticated of those.
There is no need to do anything special, much less anything that prevents
standard applications such as "sensors" from working.

Guenter