| Message ID | 1312979122-5896-5-git-send-email-j-keerthy@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 8/10/2011 2:48 PM, Balbi, Felipe wrote: > Hi, > > On Wed, Aug 10, 2011 at 05:41:02AM -0700, Tony Lindgren wrote: >> * Felipe Balbi<balbi@ti.com> [110810 05:31]: >>> >>> On Wed, Aug 10, 2011 at 05:55:20PM +0530, Keerthy wrote: >>>> + >>>> +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. >> >> Yes how about making this just a regular device driver and have it >> live under drivers/ somewhere? >> >> Or is there some reason why this could not be a loadable module? > > driver is loadable, this is just creating the platform_device, but still > I don't think it deserves its own arch_initcall(), it could very well be > something which is called after we know we're running at omap4, or > called by each board... Funny, because I thought we were trying to get rid of the ugly init devices from board file to use *initcall() from a dedicated device file. The advantage is that you do not have anymore a central place that everybody will change and that is thus subject to merge conflicts. The drawback is that you do not know where an when the devices are created. That being said, device-tree will provide a nice way to build all this devices without any initcall or board hacks. This is just a temporary issue :-) Benoit -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Wed, Aug 10, 2011 at 04:17:05PM +0200, Cousson, Benoit wrote: > On 8/10/2011 2:48 PM, Balbi, Felipe wrote: > >Hi, > > > >On Wed, Aug 10, 2011 at 05:41:02AM -0700, Tony Lindgren wrote: > >>* Felipe Balbi<balbi@ti.com> [110810 05:31]: > >>> > >>>On Wed, Aug 10, 2011 at 05:55:20PM +0530, Keerthy wrote: > >>>>+ > >>>>+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. > >> > >>Yes how about making this just a regular device driver and have it > >>live under drivers/ somewhere? > >> > >>Or is there some reason why this could not be a loadable module? > > > >driver is loadable, this is just creating the platform_device, but still > >I don't think it deserves its own arch_initcall(), it could very well be > >something which is called after we know we're running at omap4, or > >called by each board... > > Funny, because I thought we were trying to get rid of the ugly init > devices from board file to use *initcall() from a dedicated device > file. > The advantage is that you do not have anymore a central place that > everybody will change and that is thus subject to merge conflicts. > > The drawback is that you do not know where an when the devices are created. > > That being said, device-tree will provide a nice way to build all > this devices without any initcall or board hacks. > This is just a temporary issue :-) Temporary or not, I would rather have this device created based on CPU detection as it ought to be. But since we're moving to DT anyway, I agree it might not be worth spending the time.
On Wed, Aug 10, 2011 at 6:06 PM, Felipe Balbi <balbi@ti.com> 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@ti.com> >> --- >> 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@ti.com> >> + * >> + * 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@ti.com> >> + * >> + * 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 >
Hi, On Thu, Aug 11, 2011 at 08:10:07AM +0530, J, KEERTHY wrote: > >> 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(). that's not a reality yet, so why don't you leave it for when OMAP5 is around ? > >> 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@ti.com> > >> + * > >> + * 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. what will change is the base address, the offsets should remain the same.
On Thu, Aug 11, 2011 at 4:00 PM, Felipe Balbi <balbi@ti.com> wrote: > Hi, > > On Thu, Aug 11, 2011 at 08:10:07AM +0530, J, KEERTHY wrote: >> >> 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(). > > that's not a reality yet, so why don't you leave it for when OMAP5 is > around ? Keeping it generic so that we need not change again. We are pretty close to reality i guess. Why not keep it generic? Any specific reason for not keeping this loop? > >> >> 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@ti.com> >> >> + * >> >> + * 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. > > what will change is the base address, the offsets should remain the > same. The base address offsets and even bit fields seem to be differing across different OMAP versions. > > -- > balbi >
Hi, On Thu, Aug 11, 2011 at 04:31:50PM +0530, J, KEERTHY wrote: > On Thu, Aug 11, 2011 at 4:00 PM, Felipe Balbi <balbi@ti.com> wrote: > > Hi, > > > > On Thu, Aug 11, 2011 at 08:10:07AM +0530, J, KEERTHY wrote: > >> >> 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(). > > > > that's not a reality yet, so why don't you leave it for when OMAP5 is > > around ? > > Keeping it generic so that we need not change again. We are pretty > close to reality i guess. Why not keep it generic? Any specific reason > for not keeping this loop? Other than the loop being completely unnecessary on the only OMAP version you're supporting ? no... not really. > >> >> 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@ti.com> > >> >> + * > >> >> + * 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. > > > > what will change is the base address, the offsets should remain the > > same. > > The base address offsets and even bit fields seem to be differing across > different OMAP versions. then a comment making that clear is necessary. But as of today, you support only one OMAP version, so I'm sure it's worth the trouble for a first version of the driver.
On Thu, Aug 11, 2011 at 7:35 PM, Felipe Balbi <balbi@ti.com> wrote: > Hi, > > On Thu, Aug 11, 2011 at 04:31:50PM +0530, J, KEERTHY wrote: >> On Thu, Aug 11, 2011 at 4:00 PM, Felipe Balbi <balbi@ti.com> wrote: >> > Hi, >> > >> > On Thu, Aug 11, 2011 at 08:10:07AM +0530, J, KEERTHY wrote: >> >> >> 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(). >> > >> > that's not a reality yet, so why don't you leave it for when OMAP5 is >> > around ? >> >> Keeping it generic so that we need not change again. We are pretty >> close to reality i guess. Why not keep it generic? Any specific reason >> for not keeping this loop? > > Other than the loop being completely unnecessary on the only OMAP > version you're supporting ? no... not really. > >> >> >> 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@ti.com> >> >> >> + * >> >> >> + * 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. >> > >> > what will change is the base address, the offsets should remain the >> > same. >> >> The base address offsets and even bit fields seem to be differing across >> different OMAP versions. > > then a comment making that clear is necessary. But as of today, you > support only one OMAP version, so I'm sure it's worth the trouble for a > first version of the driver. I will add a comment. > > -- > balbi >
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@ti.com> + * + * 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, + } +}; + +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; + + ret = 0; + 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); 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. + config OMAP_MBOX_FWK tristate "Mailbox framework support" depends on ARCH_OMAP 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@ti.com> + * + * 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; +}; + +/* + * @name: Name of the domain of the temperature sensor + */ +struct omap_temp_sensor_dev_attr { + const char *name; +}; + +extern struct omap_temp_sensor_registers omap_mpu_temp_sensor_registers; + +/* + * omap_temp_sensor platform data + * @registers - pointer to register set and thier bit fields information + */ +struct omap_temp_sensor_pdata { + struct omap_temp_sensor_registers *registers; +}; + +#endif
The device file adds the device support for OMAP4 on die temperature sensor. Signed-off-by: Keerthy <j-keerthy@ti.com> --- 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