[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
<CAGwozwFJiQeD3kCd-=vkkt4wcjwhL=ETaY7br+7+B7KdNXSL2w@mail.gmail.com>
Date: Wed, 5 Nov 2025 13:33:43 +0100
From: Antheas Kapenekakis <lkml@...heas.dev>
To: Armin Wolf <W_Armin@....de>
Cc: platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-hwmon@...r.kernel.org, Hans de Goede <hansg@...nel.org>,
Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
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 v3 4/6] platform/x86: ayaneo-ec: Add controller power and
modules attributes
On Tue, 4 Nov 2025 at 21:04, Armin Wolf <W_Armin@....de> wrote:
>
> Am 02.11.25 um 19:46 schrieb Antheas Kapenekakis:
>
> > On Sun, 2 Nov 2025 at 19:30, Armin Wolf <W_Armin@....de> wrote:
> >> Am 31.10.25 um 17:36 schrieb Antheas Kapenekakis:
> >>
> >>> 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..3c9c3580c685
> >>> --- /dev/null
> >>> +++ b/Documentation/ABI/testing/sysfs-platform-ayaneo-ec
> >>> @@ -0,0 +1,19 @@
> >>> +What: /sys/devices/platform/ayaneo-ec/controller_power
> >>> +Date: Oct 2025
> >> I think you need to update those dates.
> >>
> >>> +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: Oct 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 da9498d8cc89..b4d62ea9a926 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 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)
> >> Using GENMASK() would make sense here.
> > Only a single bit is being used though? GENMASK is used for a contiguous series?
>
> I was thinking of using GENMASK() for both bits:
>
> #define AYANEO_MODULE_MASK GENMASK(1, 0)
>
> You can then retrieve both bits using FIELD_GET() and simply use a switch statement
> together with an enum in controller_modules_show().
I will look at it closer if I revise the first 5 patches. The logic of
the register is complicated due to the flip, so I would not be eager
to.
I could do #define AYANEO_MODULE_MASK (AYANEO_MODULE_LEFT |
AYANEO_MODULE_RIGHT) for stylistic reasons.
@Ilpo: for the first 5 of this series is there something missing other
than perhaps the month? If not, I can respin the last patch on its own
later today. I would like this driver + the asus stuff to go into 6.19
if possible (there: i dealt with the asusctl bug by skipping the
quirk), I am unsure for the timeline for that. I want to say merge
window starts in two weeks?
Antheas
> Thanks,
> Armin Wolf
>
> >> With those issues being fixed:
> >> Reviewed-by: Armin Wolf <W_Armin@....de>
> >>
> >>> +
> >>> 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)) {
> >>> + 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 +412,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