On Fri, 23 Sep 2011, J, KEERTHY wrote: > On Fri, Sep 23, 2011 at 11:33 AM, Paul Walmsley <paul@xxxxxxxxx> wrote: > > > > On Thu, 22 Sep 2011, Keerthy wrote: > > > >> @@ -0,0 +1,175 @@ > >> +/* > >> + * OMAP system control module 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 > > > > You're also missing important #includes here for things like mutexes > > and kernel types that you use later on in the file. > > Those header files are included in c files. And how does that affect my comment? > >> +#define OMAP_ADC_START_VALUE 530 > >> +#define OMAP_ADC_END_VALUE 923 > > > > Are these OMAP4460, OMAP4xxx, or OMAP2+ specific? > > OMAP4460. I will pass even these values through pdata > since they differ from platform to platform. So then the macro names need to include "OMAP4460" or whatever SoC they are first valid for. > >> + > >> +/** > >> + * struct omap4460plus_scm_dev_attr - device attributes for scm > > > > There are loads of references to 'omap4460plus' when it seems to me that > > much of this driver should also apply to OMAP4430 also. Shouldn't this > > driver be named something like 'omap4430plus_scm' or even > > better 'omap4_scm' ? > > This is used by hwmod. Hence keeping it in the header file. Did you even read my comment before responding? - Paul
