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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Tony Lindgren had written, on 11/16/2010 02:35 PM, the following:
* Nishanth Menon <nm@xxxxxx> [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@xxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux