On Fri, 2011-08-26 at 04:21 -0400, Donggeun Kim wrote:
> This patch allows to read temperature
> from TMU(Thermal Management Unit) of SAMSUNG EXYNOS4 series of SoC.
>
> Signed-off-by: Donggeun Kim <dg77.kim@xxxxxxxxxxx>
> Signed-off-by: MyungJoo Ham <myungjoo.ham@xxxxxxxxxxx>
> Signed-off-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx>
> ---
> Changes for v2
> - added six attributes for alarms
> - changed error code of EAGAIN
> - changed initialize function to return error code
>
> Documentation/hwmon/exynos4_tmu | 79 ++++
> drivers/hwmon/Kconfig | 10 +
> drivers/hwmon/Makefile | 1 +
> drivers/hwmon/exynos4_tmu.c | 556 +++++++++++++++++++++++++++++
> include/linux/platform_data/exynos4_tmu.h | 83 +++++
> 5 files changed, 729 insertions(+), 0 deletions(-)
> create mode 100644 Documentation/hwmon/exynos4_tmu
> create mode 100644 drivers/hwmon/exynos4_tmu.c
> create mode 100644 include/linux/platform_data/exynos4_tmu.h
>
> diff --git a/Documentation/hwmon/exynos4_tmu b/Documentation/hwmon/exynos4_tmu
> new file mode 100644
> index 0000000..d7aef6a
> --- /dev/null
> +++ b/Documentation/hwmon/exynos4_tmu
> @@ -0,0 +1,79 @@
> +Kernel driver exynos4_tmu
> +=================
> +
> +Supported chips:
> +* ARM SAMSUNG EXYNOS4 series of SoC
> + Prefix: 'exynos4-tmu'
> + Datasheet: Not publicly available
> +
> +Authors: Donggeun Kim <dg77.kim@xxxxxxxxxxx>
> +
> +Description
> +-----------
> +
> +This driver allows to read temperature inside SAMSUNG EXYNOS4 series of SoC.
> +
> +The chip only exposes the measured 8-bit temperature code value
> +through a register.
> +Temperature can be taken from the temperature code.
> +There are three equations converting from temperature to temperature code.
> +
> +The three equations are:
> + 1. Two point trimming
> + Tc = (T - 25) * (TI2 - TI1) / (85 - 25) + TI1
> +
> + 2. One point trimming
> + Tc = T + TI1 - 25
> +
> + 3. No trimming
> + Tc = T + 50
> +
> + Tc: Temperature code, T: Temperature,
> + TI1: Trimming info for 25 degree Celsius (stored at TRIMINFO register)
> + Temperature code measured at 25 degree Celsius which is unchanged
> + TI2: Trimming info for 85 degree Celsius (stored at TRIMINFO register)
> + Temperature code measured at 85 degree Celsius which is unchanged
> +
> +TMU(Thermal Management Unit) in EXYNOS4 generates interrupt
> +when temperature exceeds pre-defined levels.
> +The maximum number of configurable threshold is four.
> +The threshold levels are defined as follows:
> + Level_0: current temperature > trigger_level_0 + threshold
> + Level_1: current temperature > trigger_level_1 + threshold
> + Level_2: current temperature > trigger_level_2 + threshold
> + Level_3: current temperature > trigger_level_3 + threshold
> +
> + The threshold and each trigger_level are set
> + through the corresponding registers.
> +
> +When an interrupt occurs, this driver notify user space of
> +one of four threshold levels for the interrupt
> +through kobject_uevent_env and sysfs_notify functions.
> +
You might want to explain that trigger level 0 (?) is not
used/supported.
> +Sysfs Interface
> +---------------
> +name name of the temperature sensor
> + RO
> +
> +temp1_input temperature
> + RO
> +
> +temp1_max temperature for level_1 interrupt
> + RO
> +
> +temp1_crit temperature for level_2 interrupt
> + RO
> +
> +temp1_emergency temperature for level_3 interrupt
> + RO
> +
> +temp1_max_alarm alarm for level_1 interrupt
> + RO
> +
> +temp1_crit_alarm
> + alarm for level_2 interrupt
> + RO
> +
> +temp1_emergency_alarm
> + alarm for level_3 interrupt
> + RO
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 0b62c3c..c6fb761 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -303,6 +303,16 @@ config SENSORS_DS1621
> This driver can also be built as a module. If so, the module
> will be called ds1621.
>
> +config SENSORS_EXYNOS4_TMU
> + tristate "Temperature sensor on Samsung EXYNOS4"
> + depends on EXYNOS4_DEV_TMU
> + help
> + If you say yes here you get support for TMU (Thermal Managment
> + Unit) on SAMSUNG EXYNOS4 series of SoC.
> +
> + This driver can also be built as a module. If so, the module
> + will be called exynos4-tmu.
> +
> config SENSORS_I5K_AMB
> tristate "FB-DIMM AMB temperature sensor on Intel 5000 series chipsets"
> depends on PCI && EXPERIMENTAL
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 3c9ccef..dbd8963 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -47,6 +47,7 @@ obj-$(CONFIG_SENSORS_DS1621) += ds1621.o
> obj-$(CONFIG_SENSORS_EMC1403) += emc1403.o
> obj-$(CONFIG_SENSORS_EMC2103) += emc2103.o
> obj-$(CONFIG_SENSORS_EMC6W201) += emc6w201.o
> +obj-$(CONFIG_SENSORS_EXYNOS4_TMU) += exynos4_tmu.o
> obj-$(CONFIG_SENSORS_F71805F) += f71805f.o
> obj-$(CONFIG_SENSORS_F71882FG) += f71882fg.o
> obj-$(CONFIG_SENSORS_F75375S) += f75375s.o
> diff --git a/drivers/hwmon/exynos4_tmu.c b/drivers/hwmon/exynos4_tmu.c
> new file mode 100644
> index 0000000..75814aa
> --- /dev/null
> +++ b/drivers/hwmon/exynos4_tmu.c
> @@ -0,0 +1,556 @@
> +/*
> + * exynos4_tmu.c - Samsung EXYNOS4 TMU (Thermal Management Unit)
> + *
> + * Copyright (C) 2011 Samsung Electronics
> + * Donggeun Kim <dg77.kim@xxxxxxxxxxx>
> + *
> + * 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; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * 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
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/err.h>
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/platform_device.h>
> +#include <linux/interrupt.h>
> +#include <linux/clk.h>
> +#include <linux/workqueue.h>
> +#include <linux/sysfs.h>
> +#include <linux/kobject.h>
> +#include <linux/io.h>
> +
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +
> +#include <linux/platform_data/exynos4_tmu.h>
> +
> +#define EXYNOS4_TMU_REG_TRIMINFO 0x0
> +#define EXYNOS4_TMU_REG_CONTROL 0x20
> +#define EXYNOS4_TMU_REG_STATUS 0x28
> +#define EXYNOS4_TMU_REG_CURRENT_TEMP 0x40
> +#define EXYNOS4_TMU_REG_THRESHOLD_TEMP 0x44
> +#define EXYNOS4_TMU_REG_TRIG_LEVEL0 0x50
> +#define EXYNOS4_TMU_REG_TRIG_LEVEL1 0x54
> +#define EXYNOS4_TMU_REG_TRIG_LEVEL2 0x58
> +#define EXYNOS4_TMU_REG_TRIG_LEVEL3 0x5C
> +#define EXYNOS4_TMU_REG_PAST_TEMP0 0x60
> +#define EXYNOS4_TMU_REG_PAST_TEMP1 0x64
> +#define EXYNOS4_TMU_REG_PAST_TEMP2 0x68
> +#define EXYNOS4_TMU_REG_PAST_TEMP3 0x6C
> +#define EXYNOS4_TMU_REG_INTEN 0x70
> +#define EXYNOS4_TMU_REG_INTSTAT 0x74
> +#define EXYNOS4_TMU_REG_INTCLEAR 0x78
> +
> +#define EXYNOS4_TMU_GAIN_SHIFT 8
> +#define EXYNOS4_TMU_REF_VOLTAGE_SHIFT 24
> +
> +#define EXYNOS4_TMU_TRIM_TEMP_MASK 0xff
> +#define EXYNOS4_TMU_CORE_ON 3
> +#define EXYNOS4_TMU_CORE_OFF 2
> +#define EXYNOS4_TMU_DEF_CODE_TO_TEMP_OFFSET 50
> +#define EXYNOS4_TMU_TRIG_LEVEL0_MASK 0x1
> +#define EXYNOS4_TMU_TRIG_LEVEL1_MASK 0x10
> +#define EXYNOS4_TMU_TRIG_LEVEL2_MASK 0x100
> +#define EXYNOS4_TMU_TRIG_LEVEL3_MASK 0x1000
> +#define EXYNOS4_TMU_INTCLEAR_VAL 0x1111
> +
> +struct exynos4_tmu_data {
> + struct exynos4_tmu_platform_data *pdata;
> + struct device *hwmon_dev;
> + struct resource *mem;
> + void __iomem *base;
> + int irq;
> + struct work_struct irq_work;
> + struct clk *clk;
> + unsigned int interrupt_stat;
> + u8 temp_error1, temp_error2;
> +};
> +
> +/*
> + * TMU treats temperature as a mapped temperature code.
> + * The temperature is converted differently depending on the calibration type.
> + */
> +static u8 translate_temp_to_code(struct exynos4_tmu_data *data, u8 temp)
> +{
> + struct exynos4_tmu_platform_data *pdata = data->pdata;
> + unsigned int temp_code;
> +
> + switch (pdata->cal_type) {
> + case TYPE_TWO_POINT_TRIMMING:
> + temp_code = (temp - 25) *
> + (data->temp_error2 - data->temp_error1) /
> + (85 - 25) + data->temp_error1;
> + break;
> + case TYPE_ONE_POINT_TRIMMING:
> + temp_code = temp + data->temp_error1 - 25;
> + break;
> + default:
> + temp_code = temp + EXYNOS4_TMU_DEF_CODE_TO_TEMP_OFFSET;
> + break;
> + }
> +
> + return temp_code;
> +}
> +
> +/*
> + * Calculate a temperature value from a temperature code.
> + * The unit of the temperature is degree Celsius.
> + */
> +static u8 translate_code_to_temp(struct exynos4_tmu_data *data, u8 temp_code)
> +{
> + struct exynos4_tmu_platform_data *pdata = data->pdata;
> + unsigned int temp;
> +
> + switch (pdata->cal_type) {
> + case TYPE_TWO_POINT_TRIMMING:
> + temp = (temp_code - data->temp_error1) * (85 - 25) /
> + (data->temp_error2 - data->temp_error1) + 25;
> + break;
> + case TYPE_ONE_POINT_TRIMMING:
> + temp = temp_code - data->temp_error1 + 25;
> + break;
> + default:
> + temp = temp_code - EXYNOS4_TMU_DEF_CODE_TO_TEMP_OFFSET;
> + break;
> + }
> +
> + return temp;
> +}
> +
> +static int exynos4_tmu_initialize(struct platform_device *pdev)
> +{
> + struct exynos4_tmu_data *data = platform_get_drvdata(pdev);
> + struct exynos4_tmu_platform_data *pdata = data->pdata;
> + unsigned int status, trim_info, interrupt_en;
> + u8 threshold_code;
> +
> + clk_enable(data->clk);
> +
> + status = readb(data->base + EXYNOS4_TMU_REG_STATUS);
> + if (!status) {
> + clk_disable(data->clk);
> + return -EBUSY;
> + }
> +
> + /* Save trimming info in order to perform calibration */
> + trim_info = readl(data->base + EXYNOS4_TMU_REG_TRIMINFO);
> + data->temp_error1 = trim_info & EXYNOS4_TMU_TRIM_TEMP_MASK;
> + data->temp_error2 = ((trim_info >> 8) & EXYNOS4_TMU_TRIM_TEMP_MASK);
> +
> + /* Write temperature code for threshold */
> + threshold_code = translate_temp_to_code(data, pdata->threshold);
> + writeb(threshold_code,
> + data->base + EXYNOS4_TMU_REG_THRESHOLD_TEMP);
> +
> + writeb(pdata->trigger_level0,
> + data->base + EXYNOS4_TMU_REG_TRIG_LEVEL0);
> + writeb(pdata->trigger_level1,
> + data->base + EXYNOS4_TMU_REG_TRIG_LEVEL1);
> + writeb(pdata->trigger_level2,
> + data->base + EXYNOS4_TMU_REG_TRIG_LEVEL2);
> + writeb(pdata->trigger_level3,
> + data->base + EXYNOS4_TMU_REG_TRIG_LEVEL3);
> +
> + writel(EXYNOS4_TMU_INTCLEAR_VAL,
> + data->base + EXYNOS4_TMU_REG_INTCLEAR);
> +
> + interrupt_en = pdata->trigger_level3_en << 12 |
> + pdata->trigger_level2_en << 8 |
> + pdata->trigger_level1_en << 4 |
> + pdata->trigger_level0_en;
> + writel(interrupt_en, data->base + EXYNOS4_TMU_REG_INTEN);
> +
> + clk_disable(data->clk);
> +
> + return 0;
> +}
> +
> +static void exynos4_tmu_control(struct platform_device *pdev, bool on)
> +{
> + struct exynos4_tmu_data *data = platform_get_drvdata(pdev);
> + struct exynos4_tmu_platform_data *pdata = data->pdata;
> + unsigned int con;
> +
> + clk_enable(data->clk);
> +
> + con = pdata->reference_voltage << EXYNOS4_TMU_REF_VOLTAGE_SHIFT |
> + pdata->gain << EXYNOS4_TMU_GAIN_SHIFT;
> + if (on)
> + con |= EXYNOS4_TMU_CORE_ON;
> + else
> + con |= EXYNOS4_TMU_CORE_OFF;
> +
> + writel(con, data->base + EXYNOS4_TMU_REG_CONTROL);
> +
> + clk_disable(data->clk);
> +}
> +
> +static int exynos4_tmu_read(struct exynos4_tmu_data *data, u8 *temp)
> +{
Since the temperature returned is always non-negative, you don't need
the temp pointer, but instead return the temperature directly. If the
returned value is negative, there was an error, otherwise the
temperature is returned.
> + u8 temp_code;
> +
> + clk_enable(data->clk);
> +
> + temp_code = readb(data->base + EXYNOS4_TMU_REG_CURRENT_TEMP);
> + if (!temp_code) {
> + dev_err(data->hwmon_dev, "Failed to read temperature code\n");
> + clk_disable(data->clk);
> + return -ENODATA;
My concern here was not EAGAIN vs. ENODATA (Is "No data available"
correct anyway ?), but the error message itself. The user is informed
with the error; writing an error message into the log may be just noise.
Concern here is that if every error condition in the kernel would
generate an error log, the log would be completely unusable. And if
there is an error, it may be persistent, in which case the log would be
full of "Failed to read ...". Same if the error is kind of a normal
condition; actually that would be even worse since it would pretty much
guarantee the log to be filled with noise.
> + }
> + *temp = translate_code_to_temp(data, temp_code);
> +
> + clk_disable(data->clk);
> +
The chip access code is not mutex protected, yet the access sequence
always starts with clk_enable() and ends with clk_disable(). I am quite
sure this can cause race conditions if there are multiple
threads/processes trying to access the data at the same time.
> + return 0;
> +}
> +
> +static void exynos4_tmu_work(struct work_struct *work)
> +{
> + struct exynos4_tmu_data *data = container_of(work,
> + struct exynos4_tmu_data, irq_work);
> + char *envp[2];
> +
> + clk_enable(data->clk);
> +
> + data->interrupt_stat = readl(data->base + EXYNOS4_TMU_REG_INTSTAT);
> +
> + writel(EXYNOS4_TMU_INTCLEAR_VAL, data->base + EXYNOS4_TMU_REG_INTCLEAR);
> +
> + if (data->interrupt_stat & EXYNOS4_TMU_TRIG_LEVEL3_MASK) {
> + envp[0] = "TRIG_LEVEL=3";
> + sysfs_notify(&data->hwmon_dev->kobj, NULL,
> + "temp1_emergency_alarm");
> + } else if (data->interrupt_stat & EXYNOS4_TMU_TRIG_LEVEL2_MASK) {
> + envp[0] = "TRIG_LEVEL=2";
> + sysfs_notify(&data->hwmon_dev->kobj, NULL,
> + "temp1_crit_alarm");
> + } else if (data->interrupt_stat & EXYNOS4_TMU_TRIG_LEVEL1_MASK) {
> + envp[0] = "TRIG_LEVEL=1";
> + sysfs_notify(&data->hwmon_dev->kobj, NULL, "temp1_max_alarm");
> + } else
> + envp[0] = "TRIG_LEVEL=0";
> + envp[1] = NULL;
> +
> + kobject_uevent_env(&data->hwmon_dev->kobj, KOBJ_CHANGE, envp);
> +
> + enable_irq(data->irq);
> +
> + clk_disable(data->clk);
Maybe disable the clock first, then re-enable the interrupt ? Anyway,
the access will need to be mutex protected.
Another question: it seems the sysfs notification only occurs for a 0->1
transition, but not for a 1->0 transition. Is there an interrupt for a
1->0 transition, or does it only happen silently ?
If the 1->0 transition is silent (does not generate an interrrupt), you
would have to read EXYNOS4_TMU_REG_INTSTAT in the show_alarm functions
to ensure that the current status is displayed, and to ensure that the
alarm is ever cleared if the condition goes away.
Anyway, even if the 1->0 condition is silent, you might want to change
above code to not generate a notification event unconditionally if a bit
is 1, but only if that bit changed state (0->1 or 1->0) since the last
interrupt.
> +}
> +
> +static irqreturn_t exynos4_tmu_irq(int irq, void *id)
> +{
> + struct exynos4_tmu_data *data = id;
> +
> + if (!work_pending(&data->irq_work)) {
> + disable_irq_nosync(irq);
> + schedule_work(&data->irq_work);
> + } else {
> + dev_err(data->hwmon_dev, "irq_work pending\n");
> + }
> +
Kind of unusual to spit out an error message here. What harm is done if
this ever happens ?
Anyway, schedule_work() already checks if work is already scheduled and
does not reschedule it if it is. So your validation above is redundant
and can be removed.
> + return IRQ_HANDLED;
> +}
> +
> +static ssize_t exynos4_tmu_show_name(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + return sprintf(buf, "exynos4-tmu\n");
> +}
> +
> +static ssize_t exynos4_tmu_show_temp(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct exynos4_tmu_data *data = dev_get_drvdata(dev);
> + int ret;
> + u8 temp;
> +
> + ret = exynos4_tmu_read(data, &temp);
> + if (ret)
> + return ret;
> + /* convert from degree Celsius to millidegree Celsius */
> + return sprintf(buf, "%d\n", temp * 1000);
> +}
> +
> +static ssize_t exynos4_tmu_show_max_alarm(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct exynos4_tmu_data *data = dev_get_drvdata(dev);
> +
> + if (data->interrupt_stat & EXYNOS4_TMU_TRIG_LEVEL1_MASK)
> + return sprintf(buf, "%d\n", 1);
> + else
> + return sprintf(buf, "%d\n", 0);
return sprintf(buf, "%d\n",
!!(data->interrupt_stat & EXYNOS4_TMU_TRIG_LEVEL1_MASK));
would be much simpler.
You should merge all alarm and show functions into a single function and
use the sensor attribute to specify the mask. Something like:
static ssize_t exynos4_tmu_show_alarm(struct device *dev,
struct device_attribute *attr, char *buf)
{
struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
struct exynos4_tmu_data *data = dev_get_drvdata(dev);
return sprintf(buf, "%d\n", !!(data->interrupt_stat & attr->index));
}
and
static SENSOR_DEVICE_ATTR(temp1_max_alarm, S_IRUGO,
exynos4_tmu_show_alarm, NULL, EXYNOS4_TMU_TRIG_LEVEL1_MASK);
static SENSOR_DEVICE_ATTR(temp1_crit_alarm, S_IRUGO,
exynos4_tmu_show_alarm, NULL, EXYNOS4_TMU_TRIG_LEVEL2_MASK);
static SENSOR_DEVICE_ATTR(temp1_emergency_alarm, S_IRUGO,
exynos4_tmu_show_alarm, NULL, EXYNOS4_TMU_TRIG_LEVEL3_MASK);
> +}
> +
> +static ssize_t exynos4_tmu_show_crit_alarm(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct exynos4_tmu_data *data = dev_get_drvdata(dev);
> +
> + if (data->interrupt_stat & EXYNOS4_TMU_TRIG_LEVEL2_MASK)
> + return sprintf(buf, "%d\n", 1);
> + else
> + return sprintf(buf, "%d\n", 0);
> +}
> +
> +static ssize_t exynos4_tmu_show_emergency_alarm(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct exynos4_tmu_data *data = dev_get_drvdata(dev);
> +
> + if (data->interrupt_stat & EXYNOS4_TMU_TRIG_LEVEL3_MASK)
> + return sprintf(buf, "%d\n", 1);
> + else
> + return sprintf(buf, "%d\n", 0);
> +}
> +
> +static ssize_t exynos4_tmu_show_max(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct exynos4_tmu_data *data = dev_get_drvdata(dev);
> + struct exynos4_tmu_platform_data *pdata = data->pdata;
> + unsigned int max_temp = pdata->threshold + pdata->trigger_level1;
> +
> + return sprintf(buf, "%d\n", max_temp * 1000);
> +}
> +
> +static ssize_t exynos4_tmu_show_crit(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct exynos4_tmu_data *data = dev_get_drvdata(dev);
> + struct exynos4_tmu_platform_data *pdata = data->pdata;
> + unsigned int crit_temp = pdata->threshold + pdata->trigger_level2;
> +
> + return sprintf(buf, "%d\n", crit_temp * 1000);
> +}
> +
> +static ssize_t exynos4_tmu_show_emergency(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct exynos4_tmu_data *data = dev_get_drvdata(dev);
> + struct exynos4_tmu_platform_data *pdata = data->pdata;
> + unsigned int emergency_temp = pdata->threshold + pdata->trigger_level3;
> +
> + return sprintf(buf, "%d\n", emergency_temp * 1000);
> +}
> +
Same as for alarms - replace those three functions with one and use attr
to identify the level. To avoid if/else code here, you might want to
consider replacing trigger_level{0..3} with an array and put the array
index into attr->index.
> +static DEVICE_ATTR(name, S_IRUGO, exynos4_tmu_show_name, NULL);
> +static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, exynos4_tmu_show_temp, NULL, 0);
> +
> +static SENSOR_DEVICE_ATTR(temp1_max_alarm, S_IRUGO,
> + exynos4_tmu_show_max_alarm, NULL, 1);
> +static SENSOR_DEVICE_ATTR(temp1_crit_alarm, S_IRUGO,
> + exynos4_tmu_show_crit_alarm, NULL, 2);
> +static SENSOR_DEVICE_ATTR(temp1_emergency_alarm, S_IRUGO,
> + exynos4_tmu_show_emergency_alarm, NULL, 3);
> +
> +static SENSOR_DEVICE_ATTR(temp1_max, S_IRUGO, exynos4_tmu_show_max, NULL, 4);
> +static SENSOR_DEVICE_ATTR(temp1_crit, S_IRUGO, exynos4_tmu_show_crit, NULL, 5);
> +static SENSOR_DEVICE_ATTR(temp1_emergency, S_IRUGO,
> + exynos4_tmu_show_emergency, NULL, 6);
> +
> +static struct attribute *exynos4_tmu_attributes[] = {
> + &dev_attr_name.attr,
> + &sensor_dev_attr_temp1_input.dev_attr.attr,
> + &sensor_dev_attr_temp1_max_alarm.dev_attr.attr,
> + &sensor_dev_attr_temp1_crit_alarm.dev_attr.attr,
> + &sensor_dev_attr_temp1_emergency_alarm.dev_attr.attr,
> + &sensor_dev_attr_temp1_max.dev_attr.attr,
> + &sensor_dev_attr_temp1_crit.dev_attr.attr,
> + &sensor_dev_attr_temp1_emergency.dev_attr.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group exynos4_tmu_attr_group = {
> + .attrs = exynos4_tmu_attributes,
> +};
> +
> +static int __devinit exynos4_tmu_probe(struct platform_device *pdev)
> +{
> + struct exynos4_tmu_data *data;
> + struct exynos4_tmu_platform_data *pdata = pdev->dev.platform_data;
> + int ret;
> +
> + if (!pdata) {
> + dev_err(&pdev->dev, "No platform init data supplied.\n");
> + return -ENODEV;
> + }
> +
> + data = kzalloc(sizeof(struct exynos4_tmu_data), GFP_KERNEL);
> + if (!data) {
> + dev_err(&pdev->dev, "Failed to allocate driver structure\n");
> + return -ENOMEM;
> + }
> +
> + data->irq = platform_get_irq(pdev, 0);
> + if (data->irq < 0) {
> + ret = data->irq;
> + dev_err(&pdev->dev, "Failed to get platform irq\n");
> + goto err_free;
> + }
> +
> + INIT_WORK(&data->irq_work, exynos4_tmu_work);
> +
> + data->mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!data->mem) {
> + ret = -ENOENT;
> + dev_err(&pdev->dev, "Failed to get platform resource\n");
> + goto err_free;
> + }
> +
> + data->mem = request_mem_region(data->mem->start,
> + resource_size(data->mem), pdev->name);
> + if (!data->mem) {
> + ret = -EBUSY;
A conflict is only one of the possible reasons for this to fail (ENOMEM
is another), so you might want to stick with -ENODEV.
> + dev_err(&pdev->dev, "Failed to request memory region\n");
> + goto err_free;
> + }
> +
> + data->base = ioremap(data->mem->start, resource_size(data->mem));
> + if (!data->base) {
> + ret = -EBUSY;
Again, this can have multiple reasons, so -ENODEV would be more
appropriate.
> + dev_err(&pdev->dev, "Failed to ioremap memory\n");
> + goto err_mem_region;
> + }
> +
> + ret = request_irq(data->irq, exynos4_tmu_irq,
> + IRQF_DISABLED | IRQF_TRIGGER_RISING,
> + "exynos4-tmu", data);
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to request irq: %d\n", data->irq);
> + goto err_io_remap;
> + }
> +
> + data->clk = clk_get(NULL, "tmu_apbif");
> + if (IS_ERR(data->clk)) {
> + ret = -EINVAL;
ret = PTR_ERR(data->clk);
would be more appropriate.
> + dev_err(&pdev->dev, "Failed to get clock\n");
> + goto err_irq;
> + }
> +
> + data->pdata = pdata;
> + platform_set_drvdata(pdev, data);
> +
> + ret = sysfs_create_group(&pdev->dev.kobj, &exynos4_tmu_attr_group);
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to create sysfs group\n");
> + goto err_clk;
> + }
> +
> + data->hwmon_dev = hwmon_device_register(&pdev->dev);
> + if (IS_ERR(data->hwmon_dev)) {
> + ret = PTR_ERR(data->hwmon_dev);
> + dev_err(&pdev->dev, "Failed to register hwmon device\n");
> + goto err_create_group;
> + }
> +
> + ret = exynos4_tmu_initialize(pdev);
You should probably do this earlier, before registering sysfs devices.
Keep in mind that functions can be called after sysfs registration, so
the chip should be in a well known state when this happens.
If the initialization has to be here, you'll have to protect its code
with a mutex to ensure there is no race condition, and you must make
sure that all read functions don't return an error just because the chip
is not yet initialized.
One way to handle it might be to initialize the chip before sysfs
registration, separate out the interrupt enable code, and only enable
interrupts after all registrations are complete.
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to initialize TMU\n");
> + goto err_hwmon_device_register;
> + }
> + exynos4_tmu_control(pdev, true);
> +
> + return 0;
> +
> +err_hwmon_device_register:
> + hwmon_device_unregister(data->hwmon_dev);
> +err_create_group:
> + sysfs_remove_group(&pdev->dev.kobj, &exynos4_tmu_attr_group);
> +err_clk:
> + platform_set_drvdata(pdev, NULL);
> + clk_put(data->clk);
> +err_irq:
> + free_irq(data->irq, data);
> +err_io_remap:
> + iounmap(data->base);
> +err_mem_region:
> + release_mem_region(data->mem->start, resource_size(data->mem));
> +err_free:
> + kfree(data);
> +
> + return ret;
> +}
> +
> +static int __devexit exynos4_tmu_remove(struct platform_device *pdev)
> +{
> + struct exynos4_tmu_data *data = platform_get_drvdata(pdev);
> +
> + hwmon_device_unregister(data->hwmon_dev);
> + sysfs_remove_group(&pdev->dev.kobj, &exynos4_tmu_attr_group);
> +
> + clk_put(data->clk);
> +
> + free_irq(data->irq, data);
> +
Do you actually disable the HW interrupt anywhere ? If not, there might
be spurious interrupts after this.
> + iounmap(data->base);
> + release_mem_region(data->mem->start, resource_size(data->mem));
> +
> + platform_set_drvdata(pdev, NULL);
> +
> + kfree(data);
> +
> + return 0;
> +}
> +
> +#ifdef CONFIG_PM
> +static int exynos4_tmu_suspend(struct platform_device *pdev, pm_message_t state)
> +{
> + exynos4_tmu_control(pdev, false);
> +
> + return 0;
> +}
> +
> +static int exynos4_tmu_resume(struct platform_device *pdev)
> +{
> + exynos4_tmu_initialize(pdev);
> + exynos4_tmu_control(pdev, true);
> +
> + return 0;
> +}
> +#else
> +#define exynos4_tmu_suspend NULL
> +#define exynos4_tmu_resume NULL
> +#endif
> +
> +static struct platform_driver exynos4_tmu_driver = {
> + .driver = {
> + .name = "exynos4-tmu",
> + .owner = THIS_MODULE,
> + },
> + .probe = exynos4_tmu_probe,
> + .remove = __devexit_p(exynos4_tmu_remove),
> + .suspend = exynos4_tmu_suspend,
> + .resume = exynos4_tmu_resume,
> +};
> +
> +static int __init exynos4_tmu_driver_init(void)
> +{
> + return platform_driver_register(&exynos4_tmu_driver);
> +}
> +module_init(exynos4_tmu_driver_init);
> +
> +static void __exit exynos4_tmu_driver_exit(void)
> +{
> + platform_driver_unregister(&exynos4_tmu_driver);
> +}
> +module_exit(exynos4_tmu_driver_exit);
> +
> +MODULE_DESCRIPTION("EXYNOS4 TMU Driver");
> +MODULE_AUTHOR("Donggeun Kim <dg77.kim@xxxxxxxxxxx>");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:exynos4-tmu");
> diff --git a/include/linux/platform_data/exynos4_tmu.h b/include/linux/platform_data/exynos4_tmu.h
> new file mode 100644
> index 0000000..b98f024
> --- /dev/null
> +++ b/include/linux/platform_data/exynos4_tmu.h
> @@ -0,0 +1,83 @@
> +/*
> + * exynos4_tmu.h - Samsung EXYNOS4 TMU (Thermal Management Unit)
> + *
> + * Copyright (C) 2011 Samsung Electronics
> + * Donggeun Kim <dg77.kim@xxxxxxxxxxx>
> + *
> + * 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; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * 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
> + */
> +
> +#ifndef _LINUX_EXYNOS4_TMU_H
> +#define _LINUX_EXYNOS4_TMU_H
> +
> +enum calibration_type {
> + TYPE_ONE_POINT_TRIMMING,
> + TYPE_TWO_POINT_TRIMMING,
> + TYPE_NONE,
> +};
> +
> +/**
> + * struct exynos4_tmu_platform_data
> + * @threshold: basic temperature for generating interrupt
> + * @trigger_level0: terperature for trigger_level0 interrupt
> + * condition for trigger_level0 interrupt:
> + * current temperature > threshold + trigger_level0
> + * @trigger_level1: terperature for trigger_level1 interrupt
> + * condition for trigger_level1 interrupt:
> + * current temperature > threshold + trigger_level1
> + * @trigger_level2: terperature for trigger_level2 interrupt
> + * condition for trigger_level2 interrupt:
> + * current temperature > threshold + trigger_level2
> + * @trigger_level3: terperature for trigger_level3 interrupt
> + * condition for trigger_level3 interrupt:
> + * current temperature > threshold + trigger_level3
> + * @trigger_level0_en:
> + * 1 = enable trigger_level0 interrupt,
> + * 0 = disable trigger_level0 interrupt
> + * @trigger_level1_en:
> + * 1 = enable trigger_level1 interrupt,
> + * 0 = disable trigger_level1 interrupt
> + * @trigger_level2_en:
> + * 1 = enable trigger_level2 interrupt,
> + * 0 = disable trigger_level2 interrupt
> + * @trigger_level3_en:
> + * 1 = enable trigger_level3 interrupt,
> + * 0 = disable trigger_level3 interrupt
> + * @gain: gain of amplifier in the positive-TC generator block
> + * 0 <= gain <= 15
> + * @reference_voltage: reference voltage of amplifier
> + * in the positive-TC generator block
> + * 0 <= reference_voltage <= 31
> + * @cal_type: calibration type for temperature
> + *
> + * This structure is required for configuration of exynos4_tmu driver.
> + */
> +struct exynos4_tmu_platform_data {
> + u8 threshold;
> + u8 trigger_level0;
> + u8 trigger_level1;
> + u8 trigger_level2;
> + u8 trigger_level3;
> + bool trigger_level0_en;
> + bool trigger_level1_en;
> + bool trigger_level2_en;
> + bool trigger_level3_en;
> +
> + u8 gain;
> + u8 reference_voltage;
> +
> + enum calibration_type cal_type;
> +};
> +#endif /* _LINUX_EXYNOS4_TMU_H */
> --
> 1.7.4.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
