Re: [RFC PATCH 1/1] platform: x86: tuxi: Implement TUXEDO TUXI ACPI TFAN as thermal subsystem

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 27.11.24 um 12:21 schrieb Werner Sembach:

The TUXEDO Sirius 16 Gen1 & Gen2 have the custom TUXEDO Interface (TUXI)
ACPI interface which currently consists of the TFAN device. This has ACPI
functions to control the built in fans and monitor fan speeds and CPU and
GPU temprature.

This driver implements this TFAN device via the thermal subsystem to allow
userspace controlled, but hardware safe, custom fan curves.

Signed-off-by: Werner Sembach <wse@xxxxxxxxxxxxxxxxxxx>
---
  drivers/platform/x86/Makefile                 |   3 +
  drivers/platform/x86/tuxedo/Kbuild            |   6 +
  drivers/platform/x86/tuxedo/nbxx/Kbuild       |   8 +
  drivers/platform/x86/tuxedo/nbxx/Kconfig      |   9 +
  drivers/platform/x86/tuxedo/nbxx/acpi_tuxi.c  |  96 +++++++
  drivers/platform/x86/tuxedo/nbxx/acpi_tuxi.h  |  84 ++++++
  .../x86/tuxedo/nbxx/acpi_tuxi_thermal.c       | 241 ++++++++++++++++++
  .../x86/tuxedo/nbxx/acpi_tuxi_thermal.h       |  14 +
  8 files changed, 461 insertions(+)
  create mode 100644 drivers/platform/x86/tuxedo/Kbuild
  create mode 100644 drivers/platform/x86/tuxedo/nbxx/Kbuild
  create mode 100644 drivers/platform/x86/tuxedo/nbxx/Kconfig
  create mode 100644 drivers/platform/x86/tuxedo/nbxx/acpi_tuxi.c
  create mode 100644 drivers/platform/x86/tuxedo/nbxx/acpi_tuxi.h
  create mode 100644 drivers/platform/x86/tuxedo/nbxx/acpi_tuxi_thermal.c
  create mode 100644 drivers/platform/x86/tuxedo/nbxx/acpi_tuxi_thermal.h

diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index e1b1429470674..1562dcd7ad9a5 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -153,3 +153,6 @@ obj-$(CONFIG_WINMATE_FM07_KEYS)		+= winmate-fm07-keys.o

  # SEL
  obj-$(CONFIG_SEL3350_PLATFORM)		+= sel3350-platform.o
+
+# TUXEDO
+obj-y					+= tuxedo/
diff --git a/drivers/platform/x86/tuxedo/Kbuild b/drivers/platform/x86/tuxedo/Kbuild
new file mode 100644
index 0000000000000..16396a8923432
--- /dev/null
+++ b/drivers/platform/x86/tuxedo/Kbuild
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# TUXEDO X86 Platform Specific Drivers
+#
+
+obj-y	+= nbxx/

Hi,

please give this directory a more descriptive name, maybe call it "tuxi"?

diff --git a/drivers/platform/x86/tuxedo/nbxx/Kbuild b/drivers/platform/x86/tuxedo/nbxx/Kbuild
new file mode 100644
index 0000000000000..aa4d6d66b785f
--- /dev/null
+++ b/drivers/platform/x86/tuxedo/nbxx/Kbuild
@@ -0,0 +1,8 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# TUXEDO X86 Platform Specific Drivers
+#
+
+tuxedo_nbxx_acpi_tuxi-y			:= acpi_tuxi.o
+tuxedo_nbxx_acpi_tuxi-y			+= acpi_tuxi_thermal.o
+obj-$(CONFIG_TUXEDO_NBXX_ACPI_TUXI)	+= tuxedo_nbxx_acpi_tuxi.o
diff --git a/drivers/platform/x86/tuxedo/nbxx/Kconfig b/drivers/platform/x86/tuxedo/nbxx/Kconfig
new file mode 100644
index 0000000000000..93f7f86e803da
--- /dev/null
+++ b/drivers/platform/x86/tuxedo/nbxx/Kconfig
@@ -0,0 +1,9 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# TUXEDO X86 Platform Specific Drivers
+#
+
+config TUXEDO_NBXX_ACPI_TUXI
+	tristate "TUXEDO TUXI"
+	help
+	  TUXEDO TUXI driver

Please add any dependencies and a more descriptive help text.

diff --git a/drivers/platform/x86/tuxedo/nbxx/acpi_tuxi.c b/drivers/platform/x86/tuxedo/nbxx/acpi_tuxi.c
new file mode 100644
index 0000000000000..6031cf731b449
--- /dev/null
+++ b/drivers/platform/x86/tuxedo/nbxx/acpi_tuxi.c
@@ -0,0 +1,96 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2024 Werner Sembach wse@xxxxxxxxxxxxxxxxxxx
+ */
+
+#include <linux/module.h>
+#include <linux/acpi.h>
+#include "acpi_tuxi_thermal.h"
+
+#include "acpi_tuxi.h"
+
+static int eval_intparams(acpi_handle handle, acpi_string pathname,
+			  unsigned long long *params, u32 params_count,
+			  unsigned long long *retval)
+{
+	struct acpi_object_list arguments;
+	unsigned long long data;
+	acpi_status status;
+
+	pr_debug("Eval %s\n", pathname);
+
+	if (params_count > 0) {
+		pr_debug("Params:\n");
+
+		arguments.count = params_count;
+		arguments.pointer = kzalloc(
+			params_count * sizeof(*arguments.pointer), GFP_KERNEL);
+		for (int i = 0; i < params_count; ++i) {
+			pr_debug("%llu\n", params[i]);
+
+			arguments.pointer[i].type = ACPI_TYPE_INTEGER;
+			arguments.pointer[i].integer.value = params[i];
+		}
+		status = acpi_evaluate_integer(handle, pathname, &arguments,
+					       &data);
+		kfree(arguments.pointer);
+	} else {
+		status = acpi_evaluate_integer(handle, pathname, NULL, &data);
+	}
+	if (ACPI_FAILURE(status))
+		return_ACPI_STATUS(status);

This is an internal function of ACPICA, please just return -EIO.

+
+	if (retval)
+		*retval = data;
+
+	pr_debug("RetVal: %llu\n", *retval);
+
+	return 0;
+}
+
+static int add(struct acpi_device *device) {
+	struct tuxedo_nbxx_acpi_tuxi_driver_data_t *driver_data;
+	acpi_status status;
+
+	driver_data = devm_kzalloc(&device->dev, sizeof(*driver_data),
+				   GFP_KERNEL);
+	if (!driver_data)
+		return -ENOMEM;
+
+	// Find subdevices
+	status = acpi_get_handle(device->handle, "TFAN",
+				 &driver_data->tfan_handle);
+	if (ACPI_FAILURE(status))
+		return_ACPI_STATUS(status);
+
+	driver_data->eval_intparams = &eval_intparams;
+
+	dev_set_drvdata(&device->dev, driver_data);
+
+	return tuxedo_nbxx_acpi_tuxi_thermal_add_devices(device);
+}
+
+static void remove(struct acpi_device *device) {
+	tuxedo_nbxx_acpi_tuxi_thermal_remove_devices(device);
+}
+
+static const struct acpi_device_id acpi_device_ids[] = {
+	{"TUXI0000", 0},
+	{"", 0}
+};
+MODULE_DEVICE_TABLE(acpi, acpi_device_ids);
+
+static struct acpi_driver acpi_driver = {
+	.name = "tuxedo_nbxx_acpi_tuxi",
+	.ids = acpi_device_ids,
+	.ops = {
+		.add = add,
+		.remove = remove,
+	},
+};
+
+module_acpi_driver(acpi_driver);

Please implement your driver as an platform driver.

+
+MODULE_DESCRIPTION("TUXEDO TUXI");
+MODULE_AUTHOR("Werner Sembach <wse@xxxxxxxxxxxxxxxxxxx>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/platform/x86/tuxedo/nbxx/acpi_tuxi.h b/drivers/platform/x86/tuxedo/nbxx/acpi_tuxi.h
new file mode 100644
index 0000000000000..7ffda8e0dd68a
--- /dev/null
+++ b/drivers/platform/x86/tuxedo/nbxx/acpi_tuxi.h
@@ -0,0 +1,84 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (C) 2024 Werner Sembach wse@xxxxxxxxxxxxxxxxxxx
+ */
+
+#ifndef __PLATFORM_X86_TUXEDO_NBXX_ACPI_TUXI_H__
+#define __PLATFORM_X86_TUXEDO_NBXX_ACPI_TUXI_H__
+
+#include <linux/thermal.h>
+#include <linux/acpi.h>
+
+/*
+ * Set fan speed target
+ *
+ * Set a specific fan speed (needs manual mode)
+ *
+ * Arg0: Fan index
+ * Arg1: Fan speed as a fraction of maximum speed (0-255)
+ */
+#define TUXI_TFAN_METHOD_SET_FAN_SPEED		"SSPD"
+
+/*
+ * Get fan speed target
+ *
+ * Arg0: Fan index
+ * Returns: Current fan speed target a fraction of maximum speed (0-255)
+ */
+#define TUXI_TFAN_METHOD_GET_FAN_SPEED		"GSPD"
+
+/*
+ * Get fans count
+ *
+ * Returns: Number of individually controllable fans
+ */
+#define TUXI_TFAN_METHOD_GET_FAN_COUNT		"GCNT"
+
+/*
+ * Set fans mode
+ *
+ * Arg0: 0 = auto, 1 = manual
+ */
+#define TUXI_TFAN_METHOD_SET_FAN_MODE		"SMOD"
+
+/*
+ * Get fans mode
+ *
+ * Returns: 0 = auto, 1 = manual
+ */
+#define TUXI_TFAN_METHOD_GET_FAN_MODE		"GMOD"
+
+/*
+ * Get fan type/what the fan is pointed at
+ *
+ * Arg0: Fan index
+ * Returns: 0 = general, 1 = CPU, 2 = GPU
+ */
+#define TUXI_TFAN_METHOD_GET_FAN_TYPE		"GTYP"
+
+/*
+ * Get fan temperature/temperature of what the fan is pointed at
+ *
+ * Arg0: Fan index
+ * Returns: Temperature sensor value in 10ths of degrees kelvin
+ */
+#define TUXI_TFAN_METHOD_GET_FAN_TEMPERATURE	"GTMP"
+
+/*
+ * Get actual fan speed in RPM
+ *
+ * Arg0: Fan index
+ * Returns: Speed sensor value in revolutions per minute
+ */
+#define TUXI_TFAN_METHOD_GET_FAN_RPM		"GRPM"
+
+struct tuxedo_nbxx_acpi_tuxi_driver_data_t {
+	acpi_handle tfan_handle;
+	int (*eval_intparams)(acpi_handle, acpi_string,
+				  unsigned long long *, u32,
+				  unsigned long long *);
+	struct thermal_zone_device **tzdevs;
+	size_t tzdevs_count;
+};

Do you plan for this driver to have multiple backends? If not then
please remove this unnecessary indirection.

+
+#endif // __PLATFORM_X86_TUXEDO_NBXX_ACPI_TUXI_H__
diff --git a/drivers/platform/x86/tuxedo/nbxx/acpi_tuxi_thermal.c b/drivers/platform/x86/tuxedo/nbxx/acpi_tuxi_thermal.c
new file mode 100644
index 0000000000000..557e3fac15779
--- /dev/null
+++ b/drivers/platform/x86/tuxedo/nbxx/acpi_tuxi_thermal.c
@@ -0,0 +1,241 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2024 Werner Sembach wse@xxxxxxxxxxxxxxxxxxx
+ */
+
+#include <linux/thermal.h>
+#include <linux/minmax.h>
+#include <linux/export.h>
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/acpi.h>
+#include "acpi_tuxi.h"
+
+#include "acpi_tuxi_thermal.h"
+
+static struct thermal_trip tzdtrips[] = {
+	{
+		.type		= THERMAL_TRIP_ACTIVE,
+		.temperature	= 80000,
+		.hysteresis	= 5000,
+	},
+	{
+		.type		= THERMAL_TRIP_ACTIVE,
+		.temperature	= 90000,
+		.hysteresis	= 5000,
+	},
+	{
+		.type		= THERMAL_TRIP_HOT,
+		.temperature	= 100000,
+		.hysteresis	= 5000,
+	},
+	{
+		.type		= THERMAL_TRIP_CRITICAL,
+		.temperature	= 110000,
+	},
+};
+
+struct tz_devdata_t {
+	struct acpi_device *parent;
+	struct thermal_cooling_device *cdev;
+	size_t index;
+};
+
+static bool should_bind(struct thermal_zone_device *tzdev,
+			const struct thermal_trip __always_unused *trip,
+			struct thermal_cooling_device *cdev,
+			struct cooling_spec __always_unused *cspec)
+{
+	return cdev->devdata == tzdev;
+}
+
+static int get_temp(struct thermal_zone_device *tzdev, int *temp)
+{
+	struct tuxedo_nbxx_acpi_tuxi_driver_data_t *driver_data;
+	struct tz_devdata_t *tz_devdata;
+	unsigned long long data;
+	int ret;
+
+	tz_devdata = thermal_zone_device_priv(tzdev);
+	driver_data = dev_get_drvdata(&tz_devdata->parent->dev);
+
+	unsigned long long params[] = { tz_devdata->index };
+	ret = driver_data->eval_intparams(driver_data->tfan_handle,
+					  TUXI_TFAN_METHOD_GET_FAN_TEMPERATURE,
+					  params, ARRAY_SIZE(params), &data);
+	if (ret)
+		return ret;
+
+	*temp = data * 100 - 272000;

Please check if you can use units.h for that.

+
+	return 0;
+}
+
+static struct thermal_zone_device_ops tzdops = {
+	.should_bind = should_bind,
+	.get_temp = get_temp,
+};
+
+static struct thermal_zone_params tzdparams = {
+	.governor_name = "user_space",
+};
+
+static int get_max_state(struct thermal_cooling_device __always_unused *cdev,
+		  unsigned long *state)
+{
+	*state = 255;
+
+	return 0;
+}
+
+static int get_cur_state(struct thermal_cooling_device *cdev,
+			 unsigned long *state)
+{
+	struct tuxedo_nbxx_acpi_tuxi_driver_data_t *driver_data;
+	struct thermal_zone_device *tzdev;
+	struct tz_devdata_t *tz_devdata;
+	unsigned long long data;
+	int ret;
+
+	tzdev = cdev->devdata;
+	tz_devdata = thermal_zone_device_priv(tzdev);
+	driver_data = dev_get_drvdata(&tz_devdata->parent->dev);
+
+	unsigned long long params[] = { tz_devdata->index };
+	ret = driver_data->eval_intparams(driver_data->tfan_handle,
+					  TUXI_TFAN_METHOD_GET_FAN_SPEED,
+					  params, ARRAY_SIZE(params), &data);
+	if (ret)
+		return ret;
+
+	*state = data;
+
+	return 0;
+}
+
+static int set_cur_state(struct thermal_cooling_device *cdev,
+			 unsigned long state) {
+	struct tuxedo_nbxx_acpi_tuxi_driver_data_t *driver_data;
+	struct thermal_zone_device *tzdev;
+	struct tz_devdata_t *tz_devdata;
+	unsigned long long data;
+	int ret;
+
+	tzdev = cdev->devdata;
+	tz_devdata = thermal_zone_device_priv(tzdev);
+	driver_data = dev_get_drvdata(&tz_devdata->parent->dev);
+
+	unsigned long long params[] = { tz_devdata->index, state };

Please check if "state" is greater than 255 and return -EINVAL in such a case.

+	ret = driver_data->eval_intparams(driver_data->tfan_handle,
+					  TUXI_TFAN_METHOD_SET_FAN_SPEED,
+					  params, ARRAY_SIZE(params), &data);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static struct thermal_cooling_device_ops tcdops = {
+	.get_max_state = get_max_state,
+	.get_cur_state = get_cur_state,
+	.set_cur_state = set_cur_state,
+};
+
+int tuxedo_nbxx_acpi_tuxi_thermal_add_devices(struct acpi_device *parent)
+{
+	struct tuxedo_nbxx_acpi_tuxi_driver_data_t *driver_data;
+	struct tz_devdata_t *tz_devdata;
+	struct thermal_zone_device **tzdevs;
+	unsigned long long data;
+	size_t tzdevs_count;
+	char *type;
+	int ret;
+
+	driver_data = dev_get_drvdata(&parent->dev);
+
+	ret = driver_data->eval_intparams(driver_data->tfan_handle,
+					  TUXI_TFAN_METHOD_GET_FAN_COUNT, NULL,
+					  0, &data);
+	if (ret)
+		return ret;
+
+	tzdevs_count = data;
+	tzdevs = devm_kzalloc(&parent->dev, tzdevs_count * sizeof(*tzdevs),
+			      GFP_KERNEL);
+	if (!tzdevs)
+			return -ENOMEM;
+
+	driver_data->tzdevs = tzdevs;
+	driver_data->tzdevs_count = tzdevs_count;
+
+	for (size_t i = 0; i < tzdevs_count; ++i) {
+		tz_devdata = devm_kzalloc(&parent->dev, sizeof(*tz_devdata),
+					  GFP_KERNEL);
+		if (!tz_devdata)
+			return -ENOMEM;
+
+		tz_devdata->parent = parent;
+		tz_devdata->index = i;
+
+		unsigned long long params[] = { i };
+		ret = driver_data->eval_intparams(driver_data->tfan_handle,
+						  TUXI_TFAN_METHOD_GET_FAN_TYPE,
+						  params, ARRAY_SIZE(params),
+						  &data);
+		if (ret)
+			return ret;
+
+		switch(data) {
+		case 0:
+			type = "tuxedo_general";
+			break;
+		case 1:
+			type = "tuxedo_cpu";
+			break;
+		case 2:
+			type = "tuxedo_gpu";
+			break;
+		default:
+			return -EIO;
+		}
+
+		tzdevs[i] = thermal_zone_device_register_with_trips(
+			type, tzdtrips, ARRAY_SIZE(tzdtrips), tz_devdata, &tzdops,
+			&tzdparams, 1000, 1000);
+		if (IS_ERR(tzdevs[i]))
+			return PTR_ERR(tzdevs[i]);
+
+		tz_devdata->cdev = thermal_cooling_device_register(type,
+								   tzdevs[i],
+								   &tcdops);
+		if (IS_ERR(tz_devdata->cdev))
+			return PTR_ERR(tz_devdata->cdev);

Where do you clean up those thermal zones and cooling devices should an error occur? Better use devres for that.

Thanks,
Armin Wolf

+	}
+
+	unsigned long long params[] = { 1 };
+	driver_data->eval_intparams(driver_data->tfan_handle,
+				    TUXI_TFAN_METHOD_SET_FAN_MODE,
+				    params, ARRAY_SIZE(params), &data);
+
+	return 0;
+}
+
+void tuxedo_nbxx_acpi_tuxi_thermal_remove_devices(struct acpi_device *parent) {
+	struct tuxedo_nbxx_acpi_tuxi_driver_data_t *driver_data;
+	struct tz_devdata_t *tz_devdata;
+	unsigned long long data;
+
+	driver_data = dev_get_drvdata(&parent->dev);
+
+	unsigned long long params[] = { 0 };
+	driver_data->eval_intparams(driver_data->tfan_handle,
+				    TUXI_TFAN_METHOD_SET_FAN_MODE,
+				    params, ARRAY_SIZE(params), &data);
+
+	for (int i = 0; i < driver_data->tzdevs_count; ++i) {
+		tz_devdata = thermal_zone_device_priv(driver_data->tzdevs[i]);
+
+		thermal_cooling_device_unregister(tz_devdata->cdev);
+		thermal_zone_device_unregister(driver_data->tzdevs[i]);
+	}
+}
diff --git a/drivers/platform/x86/tuxedo/nbxx/acpi_tuxi_thermal.h b/drivers/platform/x86/tuxedo/nbxx/acpi_tuxi_thermal.h
new file mode 100644
index 0000000000000..2193fa6db0ea1
--- /dev/null
+++ b/drivers/platform/x86/tuxedo/nbxx/acpi_tuxi_thermal.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (C) 2024 Werner Sembach wse@xxxxxxxxxxxxxxxxxxx
+ */
+
+#ifndef __PLATFORM_X86_TUXEDO_NBXX_ACPI_TUXI_THERMAL_H__
+#define __PLATFORM_X86_TUXEDO_NBXX_ACPI_TUXI_THERMAL_H__
+
+#include <linux/acpi.h>
+
+int tuxedo_nbxx_acpi_tuxi_thermal_add_devices(struct acpi_device *parent);
+void tuxedo_nbxx_acpi_tuxi_thermal_remove_devices(struct acpi_device *parent);
+
+#endif // __PLATFORM_X86_TUXEDO_NBXX_ACPI_TUXI_THERMAL_H__





[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux