Message-ID: <77b944ff-2f84-3326-3901-5942739d3c43@linux.intel.com>
Date: Mon, 17 Nov 2025 12:29:26 +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 v5 4/6] platform/x86: ayaneo-ec: Add controller power
 and modules attributes

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?

> 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