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

Tony Lindgren
2010-11-16T20:35:21Z

* 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.

Regards,

Tony