| 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
| ||
|
Message-ID: <acc13faa-998e-60de-8636-a1c08b9a274d@linux.intel.com>
Date: Wed, 12 Nov 2025 14:48:58 +0200 (EET)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Antheas Kapenekakis <lkml@...heas.dev>
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>,
Armin Wolf <W_Armin@....de>
Subject: Re: [PATCH v4 4/6] platform/x86: ayaneo-ec: Add controller power
and modules attributes
On Wed, 12 Nov 2025, Antheas Kapenekakis wrote:
> On Wed, 12 Nov 2025 at 13:34, Ilpo Järvinen
> <ilpo.jarvinen@...ux.intel.com> wrote:
> >
> > On Tue, 11 Nov 2025, Antheas Kapenekakis wrote:
> >
> > > On Tue, 11 Nov 2025 at 14:41, Ilpo Järvinen
> > > <ilpo.jarvinen@...ux.intel.com> wrote:
> > > >
> > > > On Mon, 10 Nov 2025, Antheas Kapenekakis wrote:
> > > >
> > > > > The Ayaneo 3 features hot-swappable controller modules. The ejection
> > > > > and management is done through HID. However, after ejecting the modules,
> > > > > the controller needs to be power cycled via the EC to re-initialize.
> > > > >
> > > > > For this, the EC provides a variable that holds whether the left or
> > > > > right modules are connected, and a power control register to turn
> > > > > the controller on or off. After ejecting the modules, the controller
> > > > > should be turned off. Then, after both modules are reinserted,
> > > > > the controller may be powered on again to re-initialize.
> > > > >
> > > > > This patch introduces two new sysfs attributes:
> > > > > - `controller_modules`: a read-only attribute that indicates whether
> > > > > the left and right modules are connected (none, left, right, both).
> > > > > - `controller_power`: a read-write attribute that allows the user
> > > > > to turn the controller on or off (with '1'/'0').
> > > > >
> > > > > Therefore, after ejection is complete, userspace can power off the
> > > > > controller, then wait until both modules have been reinserted
> > > > > (`controller_modules` will return 'both') to turn on the controller.
> > > > >
> > > > > Reviewed-by: Armin Wolf <W_Armin@....de>
> > > > > Signed-off-by: Antheas Kapenekakis <lkml@...heas.dev>
> > > > > ---
> > > > > .../ABI/testing/sysfs-platform-ayaneo-ec | 19 ++++
> > > > > MAINTAINERS | 1 +
> > > > > drivers/platform/x86/ayaneo-ec.c | 106 ++++++++++++++++++
> > > > > 3 files changed, 126 insertions(+)
> > > > > create mode 100644 Documentation/ABI/testing/sysfs-platform-ayaneo-ec
> > > > >
> > > > > diff --git a/Documentation/ABI/testing/sysfs-platform-ayaneo-ec b/Documentation/ABI/testing/sysfs-platform-ayaneo-ec
> > > > > new file mode 100644
> > > > > index 000000000000..4cffbf5fc7ca
> > > > > --- /dev/null
> > > > > +++ b/Documentation/ABI/testing/sysfs-platform-ayaneo-ec
> > > > > @@ -0,0 +1,19 @@
> > > > > +What: /sys/devices/platform/ayaneo-ec/controller_power
> > > > > +Date: Nov 2025
> > > > > +KernelVersion: 6.19
> > > > > +Contact: "Antheas Kapenekakis" <lkml@...heas.dev>
> > > > > +Description:
> > > > > + Current controller power state. Allows turning on and off
> > > > > + the controller power (e.g. for power savings). Write 1 to
> > > > > + turn on, 0 to turn off. File is readable and writable.
> > > > > +
> > > > > +What: /sys/devices/platform/ayaneo-ec/controller_modules
> > > > > +Date: Nov 2025
> > > > > +KernelVersion: 6.19
> > > > > +Contact: "Antheas Kapenekakis" <lkml@...heas.dev>
> > > > > +Description:
> > > > > + Shows which controller modules are currently connected to
> > > > > + the device. Possible values are "left", "right" and "both".
> > > > > + File is read-only. The Windows software for this device
> > > > > + will only set controller power to 1 if both module sides
> > > > > + are connected (i.e. this file returns "both").
> > > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > > index c5bf7207c45f..f8ab009b6224 100644
> > > > > --- a/MAINTAINERS
> > > > > +++ b/MAINTAINERS
> > > > > @@ -4196,6 +4196,7 @@ AYANEO PLATFORM EC DRIVER
> > > > > M: Antheas Kapenekakis <lkml@...heas.dev>
> > > > > L: platform-driver-x86@...r.kernel.org
> > > > > S: Maintained
> > > > > +F: Documentation/ABI/testing/sysfs-platform-ayaneo
> > > > > F: drivers/platform/x86/ayaneo-ec.c
> > > > >
> > > > > AZ6007 DVB DRIVER
> > > > > diff --git a/drivers/platform/x86/ayaneo-ec.c b/drivers/platform/x86/ayaneo-ec.c
> > > > > index 697bb053a7d6..0652c044ad76 100644
> > > > > --- a/drivers/platform/x86/ayaneo-ec.c
> > > > > +++ b/drivers/platform/x86/ayaneo-ec.c
> > > > > @@ -8,6 +8,7 @@
> > > > > */
> > > > >
> > > > > #include <linux/acpi.h>
> > > > > +#include <linux/bits.h>
> > > > > #include <linux/dmi.h>
> > > > > #include <linux/err.h>
> > > > > #include <linux/hwmon.h>
> > > > > @@ -16,6 +17,7 @@
> > > > > #include <linux/module.h>
> > > > > #include <linux/platform_device.h>
> > > > > #include <linux/power_supply.h>
> > > > > +#include <linux/sysfs.h>
> > > > > #include <acpi/battery.h>
> > > > >
> > > > > #define AYANEO_PWM_ENABLE_REG 0x4A
> > > > > @@ -32,9 +34,17 @@
> > > > > #define AYANEO_CHARGE_VAL_AUTO 0xaa
> > > > > #define AYANEO_CHARGE_VAL_INHIBIT 0x55
> > > > >
> > > > > +#define AYANEO_POWER_REG 0x2d
> > > > > +#define AYANEO_POWER_OFF 0xfe
> > > > > +#define AYANEO_POWER_ON 0xff
> > > > > +#define AYANEO_MODULE_REG 0x2f
> > > > > +#define AYANEO_MODULE_LEFT BIT(0)
> > > > > +#define AYANEO_MODULE_RIGHT BIT(1)
> > > > > +
> > > > > struct ayaneo_ec_quirk {
> > > > > bool has_fan_control;
> > > > > bool has_charge_control;
> > > > > + bool has_magic_modules;
> > > > > };
> > > > >
> > > > > struct ayaneo_ec_platform_data {
> > > > > @@ -46,6 +56,7 @@ struct ayaneo_ec_platform_data {
> > > > > static const struct ayaneo_ec_quirk quirk_ayaneo3 = {
> > > > > .has_fan_control = true,
> > > > > .has_charge_control = true,
> > > > > + .has_magic_modules = true,
> > > > > };
> > > > >
> > > > > static const struct dmi_system_id dmi_table[] = {
> > > > > @@ -266,6 +277,100 @@ static int ayaneo_remove_battery(struct power_supply *battery,
> > > > > return 0;
> > > > > }
> > > > >
> > > > > +static ssize_t controller_power_store(struct device *dev,
> > > > > + struct device_attribute *attr,
> > > > > + const char *buf,
> > > > > + size_t count)
> > > > > +{
> > > > > + bool value;
> > > > > + int ret;
> > > > > +
> > > > > + ret = kstrtobool(buf, &value);
> > > > > + if (ret)
> > > > > + return ret;
> > > > > +
> > > > > + ret = ec_write(AYANEO_POWER_REG, value ? AYANEO_POWER_ON : AYANEO_POWER_OFF);
> > > > > + if (ret)
> > > > > + return ret;
> > > > > +
> > > > > + return count;
> > > > > +}
> > > > > +
> > > > > +static ssize_t controller_power_show(struct device *dev,
> > > > > + struct device_attribute *attr,
> > > > > + char *buf)
> > > > > +{
> > > > > + int ret;
> > > > > + u8 val;
> > > > > +
> > > > > + ret = ec_read(AYANEO_POWER_REG, &val);
> > > > > + if (ret)
> > > > > + return ret;
> > > > > +
> > > > > + return sysfs_emit(buf, "%d\n", val == AYANEO_POWER_ON);
> > > > > +}
> > > > > +
> > > > > +static DEVICE_ATTR_RW(controller_power);
> > > > > +
> > > > > +static ssize_t controller_modules_show(struct device *dev,
> > > > > + struct device_attribute *attr, char *buf)
> > > > > +{
> > > > > + char *out;
> > > > > + int ret;
> > > > > + u8 val;
> > > > > +
> > > > > + ret = ec_read(AYANEO_MODULE_REG, &val);
> > > > > + if (ret)
> > > > > + return ret;
> > > > > +
> > > > > + switch (~val & (AYANEO_MODULE_LEFT | AYANEO_MODULE_RIGHT)) {
> > > >
> > > > This too is constructing mask still here which is ugly.
> > >
> > > I can bring back the bools :-)
> > >
> > > I agree but that's what I came up with to remove them
> >
> > Just Add a define for the mask instead as was requested. There's no need
> > to make this any harder than that.
>
> Should the mask be:
>
> (AYANEO_MODULE_LEFT | AYANEO_MODULE_RIGHT)
>
> or
>
> GENMASK(2,0)ish
This has a wrong bit index.
> I find the latter a bit more error prone since it redefines. I can
> move forward with the former one later today if there arent any other
> comments
Either way is fine with me, as long as it's not in the switch itself.
I think I lean a bit more to the former one.
--
i.
Powered by blists - more mailing lists