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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: 
 <CAGwozwHCp-NCSx+B2mD_nRxtLktSNgiZgDGwCr4O8prXxNJ2AA@mail.gmail.com>
Date: Wed, 15 Oct 2025 11:36:52 +0200
From: Antheas Kapenekakis <lkml@...heas.dev>
To: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Cc: platform-driver-x86@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>,
	linux-hwmon@...r.kernel.org, Hans de Goede <hansg@...nel.org>,
	Derek John Clark <derekjohn.clark@...il.com>,
	Joaquín Ignacio Aramendía <samsagax@...il.com>,
	Jean Delvare <jdelvare@...e.com>, Guenter Roeck <linux@...ck-us.net>
Subject: Re: [PATCH v2 6/6] platform/x86: ayaneo-ec: Add suspend hook

On Wed, 15 Oct 2025 at 11:27, Ilpo Järvinen
<ilpo.jarvinen@...ux.intel.com> wrote:
>
> On Wed, 15 Oct 2025, Antheas Kapenekakis wrote:
>
> > On Wed, 15 Oct 2025 at 11:11, Ilpo Järvinen
> > <ilpo.jarvinen@...ux.intel.com> wrote:
> > >
> > > On Wed, 15 Oct 2025, Antheas Kapenekakis wrote:
> > >
> > > > 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.
> > > >
> > > > Signed-off-by: Antheas Kapenekakis <lkml@...heas.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++;
> > >
> > > What is this for?
> >
> > Adding additional registers to restore. Currently, there is only one
> > so it looks out of place.
>
> When are patches for those additional registers going to be submitted?
> If it's not like right around the corner, I'd prefer i be added only
> at that time.

I can move the increment to be inside the access to remove the extra
line. The idea was to minimize the diff when adding other registers.

As we test the driver more registers might get added, I cannot answer
that currently. I was also testing the controller power register, but
results were mixed.

> In any case, please mention in the changelog there's going to be more to
> registers cache so it becomes justified why you're using an array for the
> cache.

Ok.

Antheas

> --
>  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++;
> > >
> > > Same question here.
> > >
> > > > +     }
> > > > +
> > > > +     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,
> > > >       },
> > > >       .probe = ayaneo_ec_probe,
> > > >  };
> > > >
> > >
> > > --
> > >  i.
> > >
> > >
> >