On Wed, Aug 10, 2011 at 6:06 PM, Felipe Balbi <balbi@xxxxxx> wrote: > Hi, > > (you should Cc Tony, as he's OMAP maintainer) > > On Wed, Aug 10, 2011 at 05:55:20PM +0530, Keerthy wrote: >> The device file adds the device support for OMAP4 >> on die temperature sensor. >> >> Signed-off-by: Keerthy <j-keerthy@xxxxxx> >> --- >> arch/arm/mach-omap2/Makefile | 3 +- >> arch/arm/mach-omap2/temp_sensor_device.c | 85 +++++++++++++++++++ >> arch/arm/plat-omap/Kconfig | 12 +++ >> .../plat-omap/include/plat/temperature_sensor.h | 87 ++++++++++++++++++++ >> 4 files changed, 186 insertions(+), 1 deletions(-) >> create mode 100644 arch/arm/mach-omap2/temp_sensor_device.c >> create mode 100644 arch/arm/plat-omap/include/plat/temperature_sensor.h >> >> diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile >> index fb02937..5812fb4 100644 >> --- a/arch/arm/mach-omap2/Makefile >> +++ b/arch/arm/mach-omap2/Makefile >> @@ -18,6 +18,7 @@ obj-$(CONFIG_ARCH_OMAP4) += prm44xx.o $(hwmod-common) >> >> obj-$(CONFIG_OMAP_MCBSP) += mcbsp.o >> >> +obj-$(CONFIG_OMAP_TEMP_SENSOR) += temp_sensor_device.o >> obj-$(CONFIG_TWL4030_CORE) += omap_twl.o >> >> # SMP support ONLY available for OMAP4 >> @@ -86,7 +87,7 @@ obj-$(CONFIG_ARCH_OMAP3) += prcm.o cm2xxx_3xxx.o prm2xxx_3xxx.o \ >> obj-$(CONFIG_ARCH_OMAP4) += prcm.o cm2xxx_3xxx.o cminst44xx.o \ >> cm44xx.o prcm_mpu44xx.o \ >> prminst44xx.o vc44xx_data.o \ >> - vp44xx_data.o >> + vp44xx_data.o temp_sensor4460_data.o >> >> # OMAP voltage domains >> ifeq ($(CONFIG_PM),y) >> diff --git a/arch/arm/mach-omap2/temp_sensor_device.c b/arch/arm/mach-omap2/temp_sensor_device.c >> new file mode 100644 >> index 0000000..5d5e92f >> --- /dev/null >> +++ b/arch/arm/mach-omap2/temp_sensor_device.c >> @@ -0,0 +1,85 @@ >> +/* >> + * OMAP on die Temperature sensor device file >> + * >> + * Copyright (C) 2011 Texas Instruments Incorporated - http://www.ti.com/ >> + * Author: J Keerthy <j-keerthy@xxxxxx> >> + * >> + * This program is free software; you can redistribute it and/or >> + * modify it under the terms of the GNU General Public License >> + * version 2 as published by the Free Software Foundation. >> + * >> + * 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., 51 Franklin St, Fifth Floor, Boston, MA >> + * 02110-1301 USA >> + * >> + */ >> + >> +#include <linux/err.h> >> +#include <linux/slab.h> >> +#include <linux/io.h> >> +#include <linux/pm_runtime.h> >> +#include <plat/omap_device.h> >> +#include "control.h" >> +#include "pm.h" >> +#include <plat/temperature_sensor.h> >> + >> +static struct omap_device_pm_latency omap_temp_sensor_latency[] = { >> + { >> + .deactivate_func = omap_device_idle_hwmods, >> + .activate_func = omap_device_enable_hwmods, >> + .flags = OMAP_DEVICE_LATENCY_AUTO_ADJUST, >> + } > > wrong indentation. > Ok. I will correct this. >> +}; >> + >> +static int temp_sensor_dev_init(struct omap_hwmod *oh, void *user) >> +{ >> + struct omap_temp_sensor_pdata *temp_sensor_pdata; >> + struct omap_device *od; >> + struct omap_temp_sensor_dev_attr *temp_sensor_dev_attr; >> + int ret; >> + static int device_count; > > use an IDR here (see include/linux/idr.h) Ok. I will check this. > >> + ret = 0; > > initialize on the declarion and you can avoid this line. Ok > >> + temp_sensor_pdata = >> + kzalloc(sizeof(*temp_sensor_pdata), GFP_KERNEL); >> + if (!temp_sensor_pdata) { >> + dev_err(&oh->od->pdev.dev, >> + "Unable to allocate memory for temp sensor pdata\n"); >> + return -ENOMEM; >> + } >> + >> + temp_sensor_dev_attr = oh->dev_attr; >> + if (!strcmp(temp_sensor_dev_attr->name, "mpu")) >> + temp_sensor_pdata->registers = &omap_mpu_temp_sensor_registers; >> + >> + od = omap_device_build("omap_temp_sensor", device_count++, >> + oh, temp_sensor_pdata, sizeof(*temp_sensor_pdata), >> + omap_temp_sensor_latency, >> + ARRAY_SIZE(omap_temp_sensor_latency), 0); >> + if (IS_ERR(od)) { >> + dev_warn(&oh->od->pdev.dev, >> + "Could not build omap_device for %s\n", oh->name); >> + ret = PTR_ERR(od); >> + } >> + >> + kfree(temp_sensor_pdata); >> + >> + return ret; >> +} >> + >> +int __init omap_devinit_temp_sensor(void) >> +{ >> + if (!cpu_is_omap446x()) >> + return 0; >> + >> + return omap_hwmod_for_each_by_class("temperature_sensor", >> + temp_sensor_dev_init, NULL); >> +} >> + >> +arch_initcall(omap_devinit_temp_sensor); > > I really dislike people adding more and more *initcall() to their pieces > of code. But Tony is the final Judge. > >> diff --git a/arch/arm/plat-omap/Kconfig b/arch/arm/plat-omap/Kconfig >> index 6e6735f..8fd8e80 100644 >> --- a/arch/arm/plat-omap/Kconfig >> +++ b/arch/arm/plat-omap/Kconfig >> @@ -115,6 +115,18 @@ config OMAP_MCBSP >> Say Y here if you want support for the OMAP Multichannel >> Buffered Serial Port. >> >> +config OMAP_TEMP_SENSOR >> + bool "OMAP Temp Sensor Support" >> + depends on ARCH_OMAP >> + default n >> + help >> + Say Y here if you want support for the temp sensor >> + on OMAP4460. >> + >> + This provides the temperature of the MPU >> + subsystem. Only one instance of on die temperature >> + sensor is present. > > if there's only one instance, why do you use > omap_hwmod_for_each_by_class() ?? In case of OMAP5 there are multiple instances. Hence using omap_hwmod_for_each_by_class(). > >> diff --git a/arch/arm/plat-omap/include/plat/temperature_sensor.h b/arch/arm/plat-omap/include/plat/temperature_sensor.h >> new file mode 100644 >> index 0000000..692ebdc >> --- /dev/null >> +++ b/arch/arm/plat-omap/include/plat/temperature_sensor.h >> @@ -0,0 +1,87 @@ >> +/* >> + * OMAP Temperature sensor header file >> + * >> + * Copyright (C) 2011 Texas Instruments Incorporated - http://www.ti.com/ >> + * Author: J Keerthy <j-keerthy@xxxxxx> >> + * >> + * This program is free software; you can redistribute it and/or >> + * modify it under the terms of the GNU General Public License >> + * version 2 as published by the Free Software Foundation. >> + * >> + * 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., 51 Franklin St, Fifth Floor, Boston, MA >> + * 02110-1301 USA >> + * >> + */ >> + >> +#ifndef __ARCH_ARM_PLAT_OMAP_INCLUDE_PLAT_TEMPERATURE_SENSOR_H >> +#define __ARCH_ARM_PLAT_OMAP_INCLUDE_PLAT_TEMPERATURE_SENSOR_H >> + >> +/* Offsets from the base of temperature sensor registers */ >> + >> +#define OMAP4460_TEMP_SENSOR_CTRL_OFFSET 0x00 >> +#define OMAP4460_BGAP_CTRL_OFFSET 0x4c >> +#define OMAP4460_BGAP_COUNTER_OFFSET 0x50 >> +#define OMAP4460_BGAP_THRESHOLD_OFFSET 0x54 >> +#define OMAP4460_BGAP_TSHUT_OFFSET 0x58 >> +#define OMAP4460_BGAP_STATUS_OFFSET 0x5c >> +#define OMAP4460_FUSE_OPP_BGAP -0xcc >> + >> +struct omap_temp_sensor_registers { >> + u32 temp_sensor_ctrl; >> + u32 bgap_tempsoff_mask; >> + u32 bgap_soc_mask; >> + u32 bgap_eocz_mask; >> + u32 bgap_dtemp_mask; >> + >> + u32 bgap_mask_ctrl; >> + u32 mask_hot_mask; >> + u32 mask_cold_mask; >> + >> + u32 bgap_mode_ctrl; >> + u32 mode_ctrl_mask; >> + >> + u32 bgap_counter; >> + u32 counter_mask; >> + >> + u32 bgap_threshold; >> + u32 threshold_thot_mask; >> + u32 threshold_tcold_mask; >> + >> + u32 thsut_threshold; >> + u32 tshut_hot_mask; >> + u32 tshut_cold_mask; >> + >> + u32 bgap_status; >> + u32 status_clean_stop_mask; >> + u32 status_bgap_alert_mask; >> + u32 status_hot_mask; >> + u32 status_cold_mask; >> + >> + u32 bgap_efuse; >> +}; > > I find it unnecessary to pass the register map to driver using > platform_data. With multiple instances the register map to individual instances will change. So passing it via platform_data. > >> +/* >> + * @name: Name of the domain of the temperature sensor >> + */ > > comment fits in one line. Ok > > > -- > balbi > -- Regards and Thanks, Keerthy _______________________________________________ lm-sensors mailing list lm-sensors@xxxxxxxxxxxxxx http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
