| 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: <0daff739-f8a7-4042-af6d-e7dbe5aa6720@gmx.de>
Date: Tue, 18 Nov 2025 15:06:23 +0100
From: Armin Wolf <W_Armin@....de>
To: Antheas Kapenekakis <lkml@...heas.dev>,
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 v5 4/6] platform/x86: ayaneo-ec: Add controller power and
modules attributes
Am 18.11.25 um 13:58 schrieb Antheas Kapenekakis:
> On Tue, 18 Nov 2025 at 13:32, Ilpo Järvinen
> <ilpo.jarvinen@...ux.intel.com> wrote:
>> On Mon, 17 Nov 2025, Antheas Kapenekakis wrote:
>>> On Mon, 17 Nov 2025 at 11:30, Ilpo Järvinen
>>> <ilpo.jarvinen@...ux.intel.com> wrote:
>>>> On Thu, 13 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.
>>>> If wonder if the ejecting could/should be made to turn it off without need
>>>> for an explicit off command?
>>> Perhaps in the future, this driver leaves the possibility open for it.
>>> However, that'd require a secondary HID driver to handle the full
>>> ejection process, with a shared function hook for this driver.
>>>
>>> The eject sequence consists of sending a HID config command to start
>>> the ejection, followed by a secondary config command to turn off the
>>> ejection bits and then waiting for two ready bits to report that the
>>> ejection is finished. Then, the controller is turned off. Apart from
>>> turning off, all of this is done through HID (so HID ready races the
>>> controller_modules value).
>>>
>>> The module status of this driver is only used to check when the
>>> controller should be turned on again and to provide visual feedback
>>> about which modules are currently connected while unpowered. When
>>> powered, there is full status reporting over HID, including which
>>> specific modules are connected[1].
>>>
>>> The end-to-end sequence is currently in userspace[2]. However, the EC
>>> ports are shielded from userspace so these two specific accesses must
>>> happen through a kernel driver.
>> So can the other features be used without this part?
> Yes. The patches are independent.
>
>> I'd prefer to
>> postpone this power/modules thing and have the ejection done properly.
> The ejection process is done over vendor HID commands, not through the
> EC, so it would need a new driver. This new driver would not conflict
> with this patch, perhaps a new "auto" value for controller_power could
> be added to have that HID driver control this.
>
> But, I do not personally have plans to develop such a driver. I will
> not oppose one of course but I cannot justify the engineer effort on
> it. The userspace implementation works quite well and this is a niche
> device.
>
> Since these two registers are in protected ACPI space, they need to be
> accessed through this driver, so this patch is necessary for that.
>
> Antheas
I am OK with using a userspace application to connect the HID interface
with the EC, as the ejection process already requires userspace intervention.
Thanks,
Armin Wolf
>> --
>> i.
>>
>>> Antheas
>>>
>>> [1] https://github.com/hhd-dev/hhd/blob/8d842e547441600b8adc806bfb10ded5718e4fe3/src/hhd/device/ayaneo/base.py#L90-L117
>>> [2] https://github.com/hhd-dev/hhd/blob/8d842e547441600b8adc806bfb10ded5718e4fe3/src/hhd/device/ayaneo/base.py
>>>
>>>>> 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 | 107 ++++++++++++++++++
>>>>> 3 files changed, 127 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..a0747e7ee43a 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,18 @@
>>>>> #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)
>>>>> +#define AYANEO_MODULE_MASK (AYANEO_MODULE_LEFT | AYANEO_MODULE_RIGHT)
>>>>> +
>>>>> struct ayaneo_ec_quirk {
>>>>> bool has_fan_control;
>>>>> bool has_charge_control;
>>>>> + bool has_magic_modules;
>>>>> };
>>>>>
>>>>> struct ayaneo_ec_platform_data {
>>>>> @@ -46,6 +57,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 +278,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_MASK) {
>>>> Thanks for adding the mask.
>>>>
>>>> Now when reading this again, I also suggest changing variable name from
>>>> "val" to e.g. "unconnected_modules" as that would make the reason for
>>>> inversion more obvious.
>>>>
>>>>> + case AYANEO_MODULE_LEFT | AYANEO_MODULE_RIGHT:
>>>>> + out = "both";
>>>>> + break;
>>>>> + case AYANEO_MODULE_LEFT:
>>>>> + out = "left";
>>>>> + break;
>>>>> + case AYANEO_MODULE_RIGHT:
>>>>> + out = "right";
>>>>> + break;
>>>>> + default:
>>>>> + out = "none";
>>>>> + break;
>>>>> + }
>>>>> +
>>>>> + 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;
>>>>> @@ -307,6 +413,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,
>>>>> };
>>>>>
>>>> --
>>>> i.
>>>>
>>>>
>
Powered by blists - more mailing lists