| 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: <9c482e01-88be-af7e-8a73-1e07b8781354@linux.intel.com>
Date: Wed, 15 Oct 2025 12:05:24 +0300 (EEST)
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>
Subject: Re: [PATCH v2 4/6] platform/x86: ayaneo-ec: Add controller power
and modules attributes
On Wed, 15 Oct 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.
>
> Signed-off-by: Antheas Kapenekakis <lkml@...heas.dev>
> ---
> .../ABI/testing/sysfs-platform-ayaneo | 19 ++++
> MAINTAINERS | 1 +
> drivers/platform/x86/ayaneo-ec.c | 100 ++++++++++++++++++
> 3 files changed, 120 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-platform-ayaneo
>
> diff --git a/Documentation/ABI/testing/sysfs-platform-ayaneo b/Documentation/ABI/testing/sysfs-platform-ayaneo
> new file mode 100644
> index 000000000000..1fa32ba60fd0
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-platform-ayaneo
> @@ -0,0 +1,19 @@
> +What: /sys/devices/platform/<platform>/controller_power
> +Date: Oct 2025
> +KernelVersion: 6.9
> +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/<platform>/controller_modules
> +Date: Oct 2025
> +KernelVersion: 6.9
> +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 8c4d0c26ca77..3dfa004555dd 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4191,6 +4191,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 23c283f5eb61..363b61fc6e12 100644
> --- a/drivers/platform/x86/ayaneo-ec.c
> +++ b/drivers/platform/x86/ayaneo-ec.c
> @@ -30,9 +30,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 {
> @@ -44,6 +52,7 @@ struct ayaneo_ec_platform_data {
> static const struct ayaneo_ec_quirk ayaneo3 = {
> .has_fan_control = true,
> .has_charge_control = true,
> + .has_magic_modules = true,
> };
>
> static const struct dmi_system_id dmi_table[] = {
> @@ -262,6 +271,96 @@ 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)
> +{
> + bool left, right;
> + char *out;
> + int ret;
> + u8 val;
> +
> + ret = ec_read(AYANEO_MODULE_REG, &val);
> + if (ret)
> + return ret;
> +
> + left = !(val & AYANEO_MODULE_LEFT);
> + right = !(val & AYANEO_MODULE_RIGHT);
> +
> + if (left && right)
> + out = "both";
> + else if (left)
> + out = "left";
> + else if (right)
> + out = "right";
> + else
> + out = "none";
I suggest using switch/case for this so you don't need the internal
booleans at all.
BTW, I appreciate it very much this series is split per feature, it's so
much less daunting to start a review when I know I can actually finish
before the next interruption (and having to flush context from my
mind). :-)
--
i.
> +
> + return sysfs_emit(buf, "%s\n", out);
> +}
> +
> +static DEVICE_ATTR_RO(controller_modules);
> +
> +static struct attribute *aya_mm_attrs[] = {
> + &dev_attr_controller_power.attr,
> + &dev_attr_controller_modules.attr,
> + NULL
> +};
> +
> +static umode_t aya_mm_is_visible(struct kobject *kobj,
> + struct attribute *attr, int n)
> +{
> + struct device *dev = kobj_to_dev(kobj);
> + struct platform_device *pdev = to_platform_device(dev);
> + struct ayaneo_ec_platform_data *data = platform_get_drvdata(pdev);
> +
> + if (data->quirks->has_magic_modules)
> + return attr->mode;
> + return 0;
> +}
> +
> +static const struct attribute_group aya_mm_attribute_group = {
> + .is_visible = aya_mm_is_visible,
> + .attrs = aya_mm_attrs,
> +};
> +
> +static const struct attribute_group *ayaneo_ec_groups[] = {
> + &aya_mm_attribute_group,
> + NULL
> +};
> +
> static int ayaneo_ec_probe(struct platform_device *pdev)
> {
> const struct dmi_system_id *dmi_entry;
> @@ -303,6 +402,7 @@ static int ayaneo_ec_probe(struct platform_device *pdev)
> static struct platform_driver ayaneo_platform_driver = {
> .driver = {
> .name = "ayaneo-ec",
> + .dev_groups = ayaneo_ec_groups,
> },
> .probe = ayaneo_ec_probe,
> };
>
Powered by blists - more mailing lists