Re: [PATCH v3 1/3] omap: opp: add OMAP3 OPP table data and common init

[Nishanth Menon <nm@ti.com>]

Tony Lindgren had written, on 11/16/2010 02:35 PM, the following:
> * Nishanth Menon <nm@ti.com> [101115 16:43]:
>> Thomas Petazzoni had written, on 11/15/2010 04:51 PM, the following:
>>> Hello,
>>>
>>> On Mon, 15 Nov 2010 13:27:39 -0600
>>> Nishanth Menon <nm@ti.com> wrote:
>>>
>>>> +++ b/arch/arm/mach-omap2/opp3xxx_data.h
>>>> +
>>>> +static struct omap_opp_def __initdata omap34xx_opp_def_list[] = {
>>>> +
>>>> +static struct omap_opp_def __initdata omap36xx_opp_def_list[] = {
>>> Do we really want to have structure definitions in an header file ?
>>> Unless I'm wrong, this means that if the opp3xxx_data.h file is
>>> included in two different C files, the structures will be present twice.
>> The intent here - DONT DO precisely THAT!
>>> As far as I could see, most of the kernel instantiate structure in C
>>> files instead.
>> The intent here though was that opp3xx.h and opp4xx.h are private to
>> just opp.c to prevent misuse elsewhere. hmm.. thinking a bit,
>> find drivers/ -iname "*.c"|xargs grep "#include"| grep -v "\.h"
>> shows numerous examples of .c files being included in c files. I
>> dont have an issue of renaming these headers as .c file instead (I
>> had carried them over as .h from old implementation, but we can
>> change it), main point being, I just dont want folks mucking around
>> and hacking stuff with the defines.
> 
> What usually works best is to have common opp.c, then have opp34xx.c
> that has initcall that registers the data in opp.c.
> 
> That leaves out if cpu_is_omapxxx else if stuff in opp.c and then
> adding support for new omaps is just a matter of doing oppxxxx.c.

Series rev4 already posted here:
http://marc.info/?l=linux-omap&m=128993367212640&w=2
I believe I have taken care of the comments there - do let me know if I 
screwed anything up here..
-- 
Regards,
Nishanth Menon