Re: [PATCH v2 6/6] platform/x86: ayaneo-ec: Add suspend hook

[Armin Wolf <W_Armin@gmx.de>]

Am 27.10.25 um 00:17 schrieb Antheas Kapenekakis:

> On Sun, 26 Oct 2025 at 23:50, Armin Wolf <W_Armin@gmx.de> wrote:
>> Am 15.10.25 um 10:44 schrieb Antheas Kapenekakis:
>>
>>> The Ayaneo EC resets after hibernation, losing the charge control state.
>>> 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.
>> I am still not happy with potentially breaking fancontrol on this device.
>> 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?
> This device features two modes of operation: a factory fan curve
> 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.

Understandable, how about introducing a module_param_unsafe() for enabling
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?

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@antheas.dev>
>>> ---
>>>    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,
>> Please use pm_sleep_ptr() here.
>>
>> Thanks,
>> Armin Wolf
>>
>>>        },
>>>        .probe = ayaneo_ec_probe,
>>>    };
>