[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20080117092030.a9dfe528.randy.dunlap@oracle.com>
Date: Thu, 17 Jan 2008 09:20:30 -0800
From: Randy Dunlap <randy.dunlap@...cle.com>
To: Zhang Rui <rui.zhang@...el.com>
Cc: lenb@...nel.org, linux-acpi@...r.kernel.org,
linux-pm@...ts.linux-foundation.org, linux-kernel@...r.kernel.org,
sujith.thomas@...el.com
Subject: Re: [PATCH 9/10] introduce intel_menlow platform specific driver
On Thu, 17 Jan 2008 15:51:17 +0800 Zhang Rui wrote:
> From: Thomas Sujith <sujith.thomas@...el.com>
>
> Intel menlow platform specific driver for thermal management.
>
> Signed-off-by: Thomas Sujith <sujith.thomas@...el.com>
> Signed-off-by: Zhang Rui <rui.zhang@...el.com>
> ---
> drivers/misc/Kconfig | 10
> drivers/misc/Makefile | 1
> drivers/misc/intel_menlow.c | 527 ++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 538 insertions(+)
>
> Index: linux-2.6/drivers/misc/intel_menlow.c
> ===================================================================
> --- /dev/null
> +++ linux-2.6/drivers/misc/intel_menlow.c
> @@ -0,0 +1,527 @@
> +/*
> +* intel_menlow.c - Intel menlow Driver for thermal management extension
> +*
> +* Copyright (C) 2008 Intel Corp
> +* Copyright (C) 2008 Sujith Thomas <sujith.thomas@...el.com>
> +* Copyright (C) 2008 Zhang Rui <rui.zhang@...el.com>
> +* ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +*
> +* 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.
> +*/
> +
> +static int
> +memory_get_int_max_bandwidth(struct thermal_cooling_device *cdev,
> + unsigned long *max_state)
Don't put 'static int' (return type etc.) on a line by itself.
That format is not wanted in Linux. (many places here)
> +{
> + 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;
> +}
> +
> +
> +/*
> + * 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 end;
> + result =
> + sysfs_create_link(&cdev->device.kobj, &device->dev.kobj,
> + "device");
> + if (result)
> + goto end;
> + }
> +
> + end:
Labels should begin in column 0 or 1. Only.
> + return result;
> +}
> +
> +
> +const static struct acpi_device_id intel_menlow_memory_ids[] = {
> + {"INT0002", 0},
> + {"", 0},
or just {} for the terminating entry, right?
> +};
> +/*
> + * sensor_get_auxtrip
> + * -----------------
> + * get the current auxtrip value from sensor through proprietory ACPI methods
> + * name: Thermalzone name
> + * auxtype : AUX0/AUX1
> + * buf: syfs buffer
> + */
It would be Good to use kernel-doc format if someone is going to
add function documentation at all... (ref. multiple places)
See Documentation/kernel-doc-nano-HOWTO.txt .
> +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;
> +}
> +
> +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:
Don't indent labels so much. It hides them.
> + if (status == AE_NOT_FOUND)
> + return AE_OK;
> + else
> + return status;
> +}
> Index: linux-2.6/drivers/misc/Kconfig
> ===================================================================
> --- linux-2.6.orig/drivers/misc/Kconfig
> +++ linux-2.6/drivers/misc/Kconfig
> @@ -232,4 +232,14 @@ config ATMEL_SSC
>
> If unsure, say N.
>
> +config INTEL_MENLOW
> + tristate "Thermal Management driver for Intel menlow platform"
> + depends on ACPI_THERMAL
> + default n
Documentation/CodingStyle has info on Kconfig style.
Please read & use it and be more careful.
> + ---help---
> + ACPI thermal management enhancement driver on
> + Intel Menlow platform.
> +
> + If you are not sure, say N here.
> +
> endif # MISC_DEVICES
---
~Randy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists