On Thu, Aug 11, 2011 at 7:35 PM, Felipe Balbi <ba...@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 <ba...@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-keer...@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 > -- Regards and Thanks, Keerthy -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
