Re: [PATCH v2 6/6] platform/x86: ayaneo-ec: Add suspend hook
From: Armin Wolf
Date: Tue Oct 28 2025 - 19:14:11 EST
Am 28.10.25 um 18:49 schrieb Antheas Kapenekakis:
On Tue, 28 Oct 2025 at 16:26, Armin Wolf <W_Armin@xxxxxx> wrote:
Am 28.10.25 um 16:20 schrieb Antheas Kapenekakis:Well, unless you can solve the case of hibernation failing and what
On Tue, 28 Oct 2025 at 14:50, Armin Wolf <W_Armin@xxxxxx> wrote:I agree that most hwmon drivers sadly do not restore the fan control settings during
Am 27.10.25 um 00:17 schrieb Antheas Kapenekakis:It is a safe parameter and it works during suspend. It has parity with
On Sun, 26 Oct 2025 at 23:50, Armin Wolf <W_Armin@xxxxxx> wrote:Understandable, how about introducing a module_param_unsafe() for enabling
Am 15.10.25 um 10:44 schrieb Antheas Kapenekakis:This device features two modes of operation: a factory fan curve
The Ayaneo EC resets after hibernation, losing the charge control state.I am still not happy with potentially breaking fancontrol on this device.
Add a small PM hook to restore this state on hibernation resume.
The fan speed is also lost during hibernation, but since hibernation
failures are common with this class of devices, setting a low fan speed
when the userspace program controlling the fan will potentially not
take over could cause the device to overheat, so it is not restored.
Most users expect fancontrol to continue working after hibernation, so not
restoring the fan speed configuration seems risky to me. Would it be enough
to warn users about his inside the documentation?
managed by the EC and a fixed speed set via override of the EC.
The factory curve is tuned by the manufacturer to result in safe
operation in all conditions by monitoring the CPU temperature and is
not adjustable.
The fixed speed, on its own when set manually, is not use-able,
because this device has a fluctuating temperature based on workload.
So to meet the varying conditions, its speed would either have to be
set too high, leading to excess noise, or too low, potentially
overheating. Therefore, users of this interface control it via a
userspace program, e.g., hhd, coolercontrol, which allows creating a
custom fan curve based on measurements of temperature sensors.
When entering hibernation, the userspace program that controls the fan
speed is frozen, so the fan remains at its previous speed regardless
of temperature readings and there are no safety checks.
When resuming from hibernation, the EC takes over and monitors the
temperature, so it is safe until the userspace program is thawed. If
we introduce a resume hook, we take over from the EC before the
program is ready, introducing a gap where the device can potentially
overheat. If anything, the freeze hook should remove the fan speed
override instead, because suspend-then-hibernate is more of a
liability for overheating if hibernation hangs.
write access to the fan settings? The fan settings would be read-only by default,
so no suspend handling would be necessary. Said suspend handling would only be
necessary when the user _explicitly_ requests write access to the fan settings.
What i am trying to say is that we should either expose a fully working feature
(fan control with suspend support) or none at all (fan speed monitoring only).
What do you thing about that?
current hwmon drivers for other manufacturers.
Hibernation hooks for hwmon are unprecedented, in addition to
compromising the safety of the device. They _could_ be justified for
EC managed curves, since a minority of users might opt to set them via
systemd udev rules and the EC manages the temperature. But this is not
the case here.
Where does the need for these hooks stem from?
Antheas
resume from hibernation. This however is an error inside the drivers themself, as device
drivers are normally expected to restore such settings during resume. Without this the
fancontrol software will suddenly stop working after hibernation, something users do no
expect.
Copying the faulty behavior of existing drivers is not a good idea here.
happens to the fan speed I cannot sign off on it. These devices are
very expensive.
Controlling the fan is basic functionality, it cannot be marked or be
made an unsafe feature.
I am aware of the issue post hibernation, but thats easy to solve
through software, and as a maintainer of fan control software, which
also happens to be the main consumer of this module, I already have a
hook there. As it is needed by the other manufacturers as well.
Antheas
Alright, if Guenter is fine with this approach then i wont complain any longer.
However please document this non-standard behavior so people do not accidentally
"fix" it in the future.
Thank,
Armin Wolf
Thanks,
Armin Wolf
Thanks,
Armin Wolf
Other devices feature adjustable EC fan curves (e.g., Lenovo, Asus,
AYN, MSI). Since the EC monitors the temperature there, it is fine to
restore the fan curve. Speaking of, I am having quite a few issues
with MSI Claws, so that series is a bit on the back burner, so I plan
to push these series first.
I will try to tend to this series in the next days. I wanted to push
the Asus stuff first though.
Antheas
Signed-off-by: Antheas Kapenekakis <lkml@xxxxxxxxxxx>Please use pm_sleep_ptr() here.
---
drivers/platform/x86/ayaneo-ec.c | 42 ++++++++++++++++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/drivers/platform/x86/ayaneo-ec.c b/drivers/platform/x86/ayaneo-ec.c
index 73e9dd39c703..8529f6f8dc69 100644
--- a/drivers/platform/x86/ayaneo-ec.c
+++ b/drivers/platform/x86/ayaneo-ec.c
@@ -37,6 +37,8 @@
#define AYANEO_MODULE_LEFT BIT(0)
#define AYANEO_MODULE_RIGHT BIT(1)
+#define AYANEO_CACHE_LEN 1
+
struct ayaneo_ec_quirk {
bool has_fan_control;
bool has_charge_control;
@@ -47,6 +49,8 @@ struct ayaneo_ec_platform_data {
struct platform_device *pdev;
struct ayaneo_ec_quirk *quirks;
struct acpi_battery_hook battery_hook;
+
+ u8 cache[AYANEO_CACHE_LEN];
};
static const struct ayaneo_ec_quirk quirk_fan = {
@@ -464,10 +468,48 @@ static int ayaneo_ec_probe(struct platform_device *pdev)
return 0;
}
+static int ayaneo_freeze(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct ayaneo_ec_platform_data *data = platform_get_drvdata(pdev);
+ int ret, i = 0;
+
+ if (data->quirks->has_charge_control) {
+ ret = ec_read(AYANEO_CHARGE_REG, &data->cache[i]);
+ if (ret)
+ return ret;
+ i++;
+ }
+
+ return 0;
+}
+
+static int ayaneo_thaw(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct ayaneo_ec_platform_data *data = platform_get_drvdata(pdev);
+ int ret, i = 0;
+
+ if (data->quirks->has_charge_control) {
+ ret = ec_write(AYANEO_CHARGE_REG, data->cache[i]);
+ if (ret)
+ return ret;
+ i++;
+ }
+
+ return 0;
+}
+
+static const struct dev_pm_ops ayaneo_pm_ops = {
+ .freeze = ayaneo_freeze,
+ .thaw = ayaneo_thaw,
+};
+
static struct platform_driver ayaneo_platform_driver = {
.driver = {
.name = "ayaneo-ec",
.dev_groups = ayaneo_ec_groups,
+ .pm = &ayaneo_pm_ops,
Thanks,
Armin Wolf
},
.probe = ayaneo_ec_probe,
};