Re: [PATCH 9/10] introduce intel_menlow platform specific driver

[Zhang Rui]

On Mon, 2008-01-21 at 13:45 +0800, Li Zefan wrote:
> >>> +static int __init intel_menlow_module_init(void)
> >>> +{
> >>> +   int result = -ENODEV;
> >>> +   acpi_status status;
> >>> +   unsigned long enable;
> >>> +
> >>> +   if (acpi_disabled)
> >>> +           return result;
> >>> +
> >>> +   /* Looking for the \_TZ.GSTS method */
> >>> +   status = acpi_evaluate_integer(NULL, BIOS_ENABLED, NULL,
> &enable);
> >>> +   if (ACPI_FAILURE(status) || !enable)
> >>> +           return -ENODEV;
> >>> +
> >>> +   /* Looking for ACPI device MEM0 with hardware id INT0002 */
> >>> +   result =
> acpi_bus_register_driver(&intel_menlow_memory_driver);
> >>> +   if (result)
> >>> +           return result;
> >>> +
> >>> +   /* Looking for sensors in each ACPI thermal zone */
> >>> +   status = acpi_walk_namespace(ACPI_TYPE_THERMAL,
> ACPI_ROOT_OBJECT,
> >>> +                                ACPI_UINT32_MAX,
> >>> +                                intel_menlow_register_sensor,
> NULL, NULL);
> >>> +   if (ACPI_FAILURE(status))
> >>> +           result = -ENODEV;
> >>> +
> >> It seems to me this should be 'return -ENODEV;'
> >>
> >> You can just eliminate variable result, because result will be
> >> -ENODEV only.
> > I disagree. "result" is required to store the return value from
> acpi_bus_register_driver which may be different from ENODEV. Otherwise
> it's all -ENODEV.
> >
> > :-Sujith
> 
> Indead, I overlooked it.
> 
> But the above 'result = -ENODEV;' should be 'return -ENODEV;', right?
Yes, please review the refreshed patches.

From: Thomas Sujith <sujith.thomas@xxxxxxxxx>

Intel menlow platform specific driver for thermal management extension.

Signed-off-by: Thomas Sujith <sujith.thomas@xxxxxxxxx>
Signed-off-by: Zhang Rui <rui.zhang@xxxxxxxxx>
---
 drivers/misc/Kconfig        |    9 
 drivers/misc/Makefile       |    1 
 drivers/misc/intel_menlow.c |  526 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 536 insertions(+)

Index: linux-2.6/drivers/misc/intel_menlow.c
===================================================================
--- /dev/null
+++ linux-2.6/drivers/misc/intel_menlow.c
@@ -0,0 +1,526 @@
+/*
+ *  intel_menlow.c - Intel menlow Driver for thermal management extension
+ *
+ *  Copyright (C) 2008 Intel Corp
+ *  Copyright (C) 2008 Sujith Thomas <sujith.thomas@xxxxxxxxx>
+ *  Copyright (C) 2008 Zhang Rui <rui.zhang@xxxxxxxxx>
+ *  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; version 2 of the License.
+ *
+ *  This program is distributed in the hope that it will be useful, but
+ *  WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *  General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License along
+ *  with this program; if not, write to the Free Software Foundation, Inc.,
+ *  59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ *  This driver creates the sys I/F for programming the sensors.
+ *  It also implements the driver for intel menlow memory controller (hardware
+ *  id is INT0002) which makes use of the platform specific ACPI methods
+ *  to get/set bandwidth.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/types.h>
+#include <linux/pci.h>
+#include <linux/pm.h>
+
+#include <linux/thermal.h>
+#include <acpi/acpi_bus.h>
+#include <acpi/acpi_drivers.h>
+
+MODULE_AUTHOR("Thomas Sujith");
+MODULE_AUTHOR("Zhang Rui");
+MODULE_DESCRIPTION("Intel Menlow platform specific driver");
+MODULE_LICENSE("GPL");
+
+/*
+ * Memory controller device control
+ */
+
+#define MEMORY_GET_BANDWIDTH "GTHS"
+#define MEMORY_SET_BANDWIDTH "STHS"
+#define MEMORY_ARG_CUR_BANDWIDTH 1
+#define MEMORY_ARG_MAX_BANDWIDTH 0
+
+static int memory_get_int_max_bandwidth(struct thermal_cooling_device *cdev,
+					unsigned long *max_state)
+{
+	struct acpi_device *device = cdev->devdata;
+	acpi_handle handle = device->handle;
+	unsigned long value;
+	struct acpi_object_list arg_list;
+	union acpi_object arg;
+	acpi_status status = AE_OK;
+
+	arg_list.count = 1;
+	arg_list.pointer = &arg;
+	arg.type = ACPI_TYPE_INTEGER;
+	arg.integer.value = MEMORY_ARG_MAX_BANDWIDTH;
+	status = acpi_evaluate_integer(handle, MEMORY_GET_BANDWIDTH,
+				       &arg_list, &value);
+	if (ACPI_FAILURE(status))
+		return -EFAULT;
+
+	*max_state = value - 1;
+	return 0;
+}
+
+static int memory_get_max_bandwidth(struct thermal_cooling_device *cdev,
+				    char *buf)
+{
+	unsigned long value;
+	if (memory_get_int_max_bandwidth(cdev, &value))
+		return -EINVAL;
+
+	return sprintf(buf, "%ld\n", value);
+}
+
+static int memory_get_cur_bandwidth(struct thermal_cooling_device *cdev,
+				    char *buf)
+{
+	struct acpi_device *device = cdev->devdata;
+	acpi_handle handle = device->handle;
+	unsigned long value;
+	struct acpi_object_list arg_list;
+	union acpi_object arg;
+	acpi_status status = AE_OK;
+
+	arg_list.count = 1;
+	arg_list.pointer = &arg;
+	arg.type = ACPI_TYPE_INTEGER;
+	arg.integer.value = MEMORY_ARG_CUR_BANDWIDTH;
+	status = acpi_evaluate_integer(handle, MEMORY_GET_BANDWIDTH,
+				       &arg_list, &value);
+	if (ACPI_FAILURE(status))
+		return -EFAULT;
+
+	return sprintf(buf, "%ld\n", value);
+}
+
+static int memory_set_cur_bandwidth(struct thermal_cooling_device *cdev,
+				    unsigned int state)
+{
+	struct acpi_device *device = cdev->devdata;
+	acpi_handle handle = device->handle;
+	struct acpi_object_list arg_list;
+	union acpi_object arg;
+	acpi_status status;
+	int temp;
+	unsigned long max_state;
+
+	if (memory_get_int_max_bandwidth(cdev, &max_state))
+		return -EFAULT;
+
+	if (max_state < 0 || state > max_state)
+		return -EINVAL;
+
+	arg_list.count = 1;
+	arg_list.pointer = &arg;
+	arg.type = ACPI_TYPE_INTEGER;
+	arg.integer.value = state;
+
+	status =
+	    acpi_evaluate_integer(handle, MEMORY_SET_BANDWIDTH, &arg_list,
+				  (unsigned long *)&temp);
+
+	printk(KERN_INFO
+	       "Bandwidth value was %d: status is %d\n", state, status);
+	if (ACPI_FAILURE(status))
+		return -EFAULT;
+
+	return 0;
+}
+
+static struct thermal_cooling_device_ops memory_cooling_ops = {
+	.get_max_state = memory_get_max_bandwidth,
+	.get_cur_state = memory_get_cur_bandwidth,
+	.set_cur_state = memory_set_cur_bandwidth,
+};
+
+/*
+ * Memory Device Management
+ */
+static int intel_menlow_memory_add(struct acpi_device *device)
+{
+	int result = -ENODEV;
+	acpi_status status = AE_OK;
+	acpi_handle dummy;
+	struct thermal_cooling_device *cdev;
+
+	if (!device)
+		return -EINVAL;
+
+	status = acpi_get_handle(device->handle, MEMORY_GET_BANDWIDTH, &dummy);
+	if (ACPI_FAILURE(status))
+		goto end;
+
+	status = acpi_get_handle(device->handle, MEMORY_SET_BANDWIDTH, &dummy);
+	if (ACPI_FAILURE(status))
+		goto end;
+
+	cdev = thermal_cooling_device_register("Memory controller", device,
+					       &memory_cooling_ops);
+	acpi_driver_data(device) = cdev;
+	if (!cdev)
+		result = -ENODEV;
+	else {
+		result = sysfs_create_link(&device->dev.kobj,
+					&cdev->device.kobj, "thermal_cooling");
+		if (result)
+			goto unregister;
+
+		result = sysfs_create_link(&cdev->device.kobj,
+					&device->dev.kobj, "device");
+		if (result) {
+			sysfs_remove_link(&device->dev.kobj, "thermal_cooling");
+			goto unregister;
+		}
+	}
+
+ end:
+	return result;
+
+ unregister:
+	thermal_cooling_device_unregister(cdev);
+	return result;
+
+}
+
+static int intel_menlow_memory_remove(struct acpi_device *device, int type)
+{
+	struct thermal_cooling_device *cdev = acpi_driver_data(device);
+
+	if (!device || !cdev)
+		return -EINVAL;
+
+	sysfs_remove_link(&device->dev.kobj, "thermal_cooling");
+	sysfs_remove_link(&cdev->device.kobj, "device");
+	thermal_cooling_device_unregister(cdev);
+
+	return 0;
+}
+
+const static struct acpi_device_id intel_menlow_memory_ids[] = {
+	{"INT0002", 0},
+	{"", 0},
+};
+
+static struct acpi_driver intel_menlow_memory_driver = {
+	.name = "intel_menlow_thermal_control",
+	.ids = intel_menlow_memory_ids,
+	.ops = {
+		.add = intel_menlow_memory_add,
+		.remove = intel_menlow_memory_remove,
+		},
+};
+
+/*
+ * Sensor control on menlow platform
+ */
+
+#define THERMAL_AUX0 0
+#define THERMAL_AUX1 1
+#define GET_AUX0 "GAX0"
+#define GET_AUX1 "GAX1"
+#define SET_AUX0 "SAX0"
+#define SET_AUX1 "SAX1"
+
+struct intel_menlow_attribute {
+	struct device_attribute attr;
+	struct device *device;
+	acpi_handle handle;
+	struct list_head node;
+};
+
+static LIST_HEAD(intel_menlow_attr_list);
+static DEFINE_MUTEX(intel_menlow_attr_lock);
+
+/*
+ * sensor_get_auxtrip - get the current auxtrip value from sensor
+ * @name: Thermalzone name
+ * @auxtype : AUX0/AUX1
+ * @buf: syfs buffer
+ */
+static int sensor_get_auxtrip(acpi_handle handle, int index, int *value)
+{
+	acpi_status status;
+
+	if ((index != 0 && index != 1) || !value)
+		return -EINVAL;
+
+	status = acpi_evaluate_integer(handle, index ? GET_AUX1 : GET_AUX0,
+				       NULL, (unsigned long *)value);
+	if (ACPI_FAILURE(status))
+		return -EIO;
+
+	return 0;
+}
+
+/*
+ * sensor_set_auxtrip - set the new auxtrip value to sensor
+ * @name: Thermalzone name
+ * @auxtype : AUX0/AUX1
+ * @buf: syfs buffer
+ */
+static int sensor_set_auxtrip(acpi_handle handle, int index, int value)
+{
+	acpi_status status;
+	union acpi_object arg = {
+		ACPI_TYPE_INTEGER
+	};
+	struct acpi_object_list args = {
+		1, &arg
+	};
+	int temp;
+
+	if (index != 0 && index != 1)
+		return -EINVAL;
+
+	status = acpi_evaluate_integer(handle, index ? GET_AUX0 : GET_AUX1,
+				       NULL, (unsigned long *)&temp);
+	if (ACPI_FAILURE(status))
+		return -EIO;
+	if ((index && value < temp) || (!index && value > temp))
+		return -EINVAL;
+
+	arg.integer.value = value;
+	status = acpi_evaluate_integer(handle, index ? SET_AUX1 : SET_AUX0,
+				       &args, (unsigned long *)&temp);
+	if (ACPI_FAILURE(status))
+		return -EIO;
+
+	/* do we need to check the return value of SAX0/SAX1 ? */
+
+	return 0;
+}
+
+#define to_intel_menlow_attr(_attr)	\
+	container_of(_attr, struct intel_menlow_attribute, attr)
+
+static ssize_t aux0_show(struct device *dev,
+			 struct device_attribute *dev_attr, char *buf)
+{
+	struct intel_menlow_attribute *attr = to_intel_menlow_attr(dev_attr);
+	int value;
+	int result;
+
+	result = sensor_get_auxtrip(attr->handle, 0, &value);
+
+	return result ? result : sprintf(buf, "%lu", KELVIN_TO_CELSIUS(value));
+}
+
+static ssize_t aux1_show(struct device *dev,
+			 struct device_attribute *dev_attr, char *buf)
+{
+	struct intel_menlow_attribute *attr = to_intel_menlow_attr(dev_attr);
+	int value;
+	int result;
+
+	result = sensor_get_auxtrip(attr->handle, 1, &value);
+
+	return result ? result : sprintf(buf, "%lu", KELVIN_TO_CELSIUS(value));
+}
+
+static ssize_t aux0_store(struct device *dev,
+			  struct device_attribute *dev_attr,
+			  const char *buf, size_t count)
+{
+	struct intel_menlow_attribute *attr = to_intel_menlow_attr(dev_attr);
+	int value;
+	int result;
+
+	/*Sanity check; should be a positive integer */
+	if (!sscanf(buf, "%d", &value))
+		return -EINVAL;
+
+	if (value < 0)
+		return -EINVAL;
+
+	result = sensor_set_auxtrip(attr->handle, 0, CELSIUS_TO_KELVIN(value));
+	return result ? result : count;
+}
+
+static ssize_t aux1_store(struct device *dev,
+			  struct device_attribute *dev_attr,
+			  const char *buf, size_t count)
+{
+	struct intel_menlow_attribute *attr = to_intel_menlow_attr(dev_attr);
+	int value;
+	int result;
+
+	/*Sanity check; should be a positive integer */
+	if (!sscanf(buf, "%d", &value))
+		return -EINVAL;
+
+	if (value < 0)
+		return -EINVAL;
+
+	result = sensor_set_auxtrip(attr->handle, 1, CELSIUS_TO_KELVIN(value));
+	return result ? result : count;
+}
+
+/* BIOS can enable/disable the thermal user application in dabney platform */
+#define BIOS_ENABLED "\\_TZ.GSTS"
+static ssize_t bios_enabled_show(struct device *dev,
+				 struct device_attribute *attr, char *buf)
+{
+	acpi_status status;
+	unsigned long bios_enabled;
+
+	status = acpi_evaluate_integer(NULL, BIOS_ENABLED, NULL, &bios_enabled);
+	if (ACPI_FAILURE(status))
+		return -ENODEV;
+
+	return sprintf(buf, "%s\n", bios_enabled ? "enabled" : "disabled");
+}
+
+static int intel_menlow_add_one_attribute(char *name, int mode, void *show,
+					  void *store, struct device *dev,
+					  acpi_handle handle)
+{
+	struct intel_menlow_attribute *attr;
+	int result;
+
+	attr = kzalloc(sizeof(struct intel_menlow_attribute), GFP_KERNEL);
+	if (!attr)
+		return -ENOMEM;
+
+	attr->attr.attr.name = name;
+	attr->attr.attr.mode = mode;
+	attr->attr.show = show;
+	attr->attr.store = store;
+	attr->device = dev;
+	attr->handle = handle;
+
+	result = device_create_file(dev, &attr->attr);
+	if (result)
+		return result;
+
+	mutex_lock(&intel_menlow_attr_lock);
+	list_add_tail(&attr->node, &intel_menlow_attr_list);
+	mutex_unlock(&intel_menlow_attr_lock);
+
+	return 0;
+}
+
+static acpi_status intel_menlow_register_sensor(acpi_handle handle, u32 lvl,
+						void *context, void **rv)
+{
+	acpi_status status;
+	acpi_handle dummy;
+	struct thermal_zone_device *thermal;
+	int result;
+
+	result = acpi_bus_get_private_data(handle, (void **)&thermal);
+	if (result)
+		return 0;
+
+	/* _TZ must have the AUX0/1 methods */
+	status = acpi_get_handle(handle, GET_AUX0, &dummy);
+	if (ACPI_FAILURE(status))
+		goto not_found;
+
+	status = acpi_get_handle(handle, SET_AUX0, &dummy);
+	if (ACPI_FAILURE(status))
+		goto not_found;
+
+	result = intel_menlow_add_one_attribute("aux0", 0644,
+						aux0_show, aux0_store,
+						&thermal->device, handle);
+	if (result)
+		return AE_ERROR;
+
+	status = acpi_get_handle(handle, GET_AUX1, &dummy);
+	if (ACPI_FAILURE(status))
+		goto not_found;
+
+	status = acpi_get_handle(handle, SET_AUX1, &dummy);
+	if (ACPI_FAILURE(status))
+		goto not_found;
+
+	result = intel_menlow_add_one_attribute("aux1", 0644,
+						aux1_show, aux1_store,
+						&thermal->device, handle);
+	if (result)
+		return AE_ERROR;
+
+	/*
+	 * create the "dabney_enabled" attribute which means the user app
+	 * should be loaded or not
+	 */
+
+	result = intel_menlow_add_one_attribute("bios_enabled", 0444,
+						bios_enabled_show, NULL,
+						&thermal->device, handle);
+	if (result)
+		return AE_ERROR;
+
+ not_found:
+	if (status == AE_NOT_FOUND)
+		return AE_OK;
+	else
+		return status;
+}
+
+static void intel_menlow_unregister_sensor(void)
+{
+	struct intel_menlow_attribute *pos, *next;
+
+	mutex_lock(&intel_menlow_attr_lock);
+	list_for_each_entry_safe(pos, next, &intel_menlow_attr_list, node) {
+		list_del(&pos->node);
+		device_remove_file(pos->device, &pos->attr);
+		kfree(pos);
+	}
+	mutex_unlock(&intel_menlow_attr_lock);
+
+	return;
+}
+
+static int __init intel_menlow_module_init(void)
+{
+	int result = -ENODEV;
+	acpi_status status;
+	unsigned long enable;
+
+	if (acpi_disabled)
+		return result;
+
+	/* Looking for the \_TZ.GSTS method */
+	status = acpi_evaluate_integer(NULL, BIOS_ENABLED, NULL, &enable);
+	if (ACPI_FAILURE(status) || !enable)
+		return -ENODEV;
+
+	/* Looking for ACPI device MEM0 with hardware id INT0002 */
+	result = acpi_bus_register_driver(&intel_menlow_memory_driver);
+	if (result)
+		return result;
+
+	/* Looking for sensors in each ACPI thermal zone */
+	status = acpi_walk_namespace(ACPI_TYPE_THERMAL, ACPI_ROOT_OBJECT,
+				     ACPI_UINT32_MAX,
+				     intel_menlow_register_sensor, NULL, NULL);
+	if (ACPI_FAILURE(status))
+		return -ENODEV;
+
+	return 0;
+}
+
+static void __exit intel_menlow_module_exit(void)
+{
+	acpi_bus_unregister_driver(&intel_menlow_memory_driver);
+	intel_menlow_unregister_sensor();
+}
+
+module_init(intel_menlow_module_init);
+module_exit(intel_menlow_module_exit);
Index: linux-2.6/drivers/misc/Kconfig
===================================================================
--- linux-2.6.orig/drivers/misc/Kconfig
+++ linux-2.6/drivers/misc/Kconfig
@@ -232,4 +232,13 @@ config ATMEL_SSC
 
 	  If unsure, say N.
 
+config INTEL_MENLOW
+	tristate "Thermal Management driver for Intel menlow platform"
+	depends on ACPI_THERMAL
+	---help---
+	  ACPI thermal management enhancement driver on
+	  Intel Menlow platform.
+
+	  If unsure, say N.
+
 endif # MISC_DEVICES
Index: linux-2.6/drivers/misc/Makefile
===================================================================
--- linux-2.6.orig/drivers/misc/Makefile
+++ linux-2.6/drivers/misc/Makefile
@@ -17,3 +17,4 @@ obj-$(CONFIG_SONY_LAPTOP)	+= sony-laptop
 obj-$(CONFIG_THINKPAD_ACPI)	+= thinkpad_acpi.o
 obj-$(CONFIG_FUJITSU_LAPTOP)	+= fujitsu-laptop.o
 obj-$(CONFIG_EEPROM_93CX6)	+= eeprom_93cx6.o
+obj-$(CONFIG_INTEL_MENLOW)	+= intel_menlow.o


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/