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@xxxxxx> 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@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. > > > > 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. -- balbi
Attachment:
signature.asc
Description: Digital signature