Re: [PATCH v3 1/3] omap: opp: add OMAP3 OPP table data and common init
Thomas Petazzoni had written, on 11/16/2010 07:20 AM, the following: would prevent you from having no OPP table (the case where a NULL OPP table is passed is tested *before* in omapX_init_opp()). HUH?? NULL table to a static function - what code are you talking about?? why are you so behind BUG_ON, when there are valid reasons for reentry into code. In the current design, yes, there are indeed valid reasons for reentry into the omapX_init_opp() function, and that's exactly the point I'm critizicing here. how about: if (omap_table_init) return -EEXIST; does that make it better? it still retains the ability to handle both kinds of platforms as well as not BUG out. -- Regards, Nishanth Menon -- 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
Re: [PATCH v3 1/3] omap: opp: add OMAP3 OPP table data and common init
On Tue, 16 Nov 2010 08:16:07 -0800 Kevin Hilman khil...@deeprootsystems.com wrote: Yes, I'm not a big fan of the init function called multiple times either, but I really want to minimize what board files have to do. Historically, we tend to add all the init functions to every board file, and this is getting cumbersome to understand and maintain. What we need is for common code to take care of sensible defaults for all boards, and then only boards with non-default behavior have to have custom code. The other way is to have the board code register its customization into the OPP subsystem, and then when the OPP subsystem is initialized, it takes those customizations into account. But in that specific case, it's not clear how those customizations could easily be expressed, so maybe that multiple call strategy is the simplest solution. Thomas -- Thomas Petazzoni, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com -- 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
Re: [PATCH v3 1/3] omap: opp: add OMAP3 OPP table data and common init
Hello, On Mon, 15 Nov 2010 13:27:39 -0600 Nishanth Menon n...@ti.com wrote: + /* + * Allow multiple calls, but initialize only if not already initalized Minor: s/initalized/initialized/. + * even if the previous call failed, coz, no reason we'd succeed again + */ + if (omap_table_init) + return 0; + omap_table_init = 1; Do we really need this ? I personaly don't really like this quite of Hey, I'm already initialized, let's do nothing silently then. Unless there are strong reasons for which this function could be called twice, I'd rather not have this, or turn this into a BUG_ON(omap_table_init == 1). Regards, Thomas -- Thomas Petazzoni, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com -- 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
Re: [PATCH v3 1/3] omap: opp: add OMAP3 OPP table data and common init
Thomas Petazzoni wrote, on 11/16/2010 05:21 AM: Hello, On Mon, 15 Nov 2010 13:27:39 -0600 Nishanth Menonn...@ti.com wrote: + /* +* Allow multiple calls, but initialize only if not already initalized Minor: s/initalized/initialized/. aah thanks :) +* even if the previous call failed, coz, no reason we'd succeed again +*/ + if (omap_table_init) + return 0; + omap_table_init = 1; Do we really need this ? I personaly don't really like this quite of Hey, I'm already initialized, let's do nothing silently then. Unless there are strong reasons for which this function could be called twice, I'd rather not have this, or turn this into a BUG_ON(omap_table_init == 1). Yes, it is needed. The intent here is different. See the documentation that I put along with this patch - At times, board files may need to do customization to opps - like enable 1GHz on that platform alone - it can do it *only if* the defaults are registered, following which it can call opp_enable. when device_initcall follows this at a later point, it is still valid. btw, BUG_ON is a strict NO NO for me here - if I dont have OPP table, ok fine, system can still survive without cpufreq, no need to stop system operations because of that. -- Regards, Nishanth Menon -- 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
Re: [PATCH v3 1/3] omap: opp: add OMAP3 OPP table data and common init
Hello, On Tue, 16 Nov 2010 05:54:50 -0600 Nishanth Menon n...@ti.com wrote: Do we really need this ? I personaly don't really like this quite of Hey, I'm already initialized, let's do nothing silently then. Unless there are strong reasons for which this function could be called twice, I'd rather not have this, or turn this into a BUG_ON(omap_table_init == 1). Yes, it is needed. The intent here is different. See the documentation that I put along with this patch - At times, board files may need to do customization to opps - like enable 1GHz on that platform alone - it can do it *only if* the defaults are registered, following which it can call opp_enable. when device_initcall follows this at a later point, it is still valid. Ah, right, I didn't catch that omapX_init_opp() could be called first from the board init function, and then later on as a device_initcall() callback. But, sorry, I find this even uglier than I thought it was :) What about adding the obligation to boards file to call the omapX_init_opp() function and then do their customization (if needed), then no call to omapX_init_opp() would be needed as a device_initcall() callback. Or, add a mechanism that allows the board file to register its customizations, that are later taken into account by the omapX_init_opp() function. Maybe it's just a matter of personal taste, but I really don't like this kind of let's call this function once, do some stuff, then call it again, since it'll know that it shouldn't do anything. btw, BUG_ON is a strict NO NO for me here - if I dont have OPP table, ok fine, system can still survive without cpufreq, no need to stop system operations because of that. I don't see why replacing: + if (omap_table_init) + return 0; + omap_table_init = 1; by BUG_ON(omap_table_init == 1); omap_table_init = 1; would prevent you from having no OPP table (the case where a NULL OPP table is passed is tested *before* in omapX_init_opp()). Regards, Thomas -- Thomas Petazzoni, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com -- 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
Re: [PATCH v3 1/3] omap: opp: add OMAP3 OPP table data and common init
Thomas Petazzoni wrote, on 11/16/2010 06:42 AM: Hello, On Tue, 16 Nov 2010 05:54:50 -0600 Nishanth Menonn...@ti.com wrote: Do we really need this ? I personaly don't really like this quite of Hey, I'm already initialized, let's do nothing silently then. Unless there are strong reasons for which this function could be called twice, I'd rather not have this, or turn this into a BUG_ON(omap_table_init == 1). Yes, it is needed. The intent here is different. See the documentation that I put along with this patch - At times, board files may need to do customization to opps - like enable 1GHz on that platform alone - it can do it *only if* the defaults are registered, following which it can call opp_enable. when device_initcall follows this at a later point, it is still valid. Ah, right, I didn't catch that omapX_init_opp() could be called first from the board init function, and then later on as a device_initcall() callback. But, sorry, I find this even uglier than I thought it was :) What about adding the obligation to boards file to call the omapX_init_opp() function and then do their customization (if needed), then no call to omapX_init_opp() would be needed as a device_initcall() callback. Or, add a mechanism that allows the board file to register its customizations, that are later taken into account by the omapX_init_opp() function. Maybe it's just a matter of personal taste, but I really don't like this kind of let's call this function once, do some stuff, then call it again, since it'll know that it shouldn't do anything., I feel you may have misunderstood the code, we DONOT oblige all boards to *have* to call omapX_init_opp. It is a device_initcall - so for the boards that dont call it, device_initcall will trigger and initialzie it. the hooks for the customization of the OPPs is in OPP layer itself. the need we satisfy is this: if you need to support two sets of boards: a) boards that are happy with the defaults - most of the boards - dont do anything in the board file. (device_init_call with auto register the defaults) b) boards that need customization - these guys need to call omapX_init_opp(to register the defaults) before customizing the defaults. Does this explain the code and reason for the logic? if you do have a better mechanism, lets know. btw, BUG_ON is a strict NO NO for me here - if I dont have OPP table, ok fine, system can still survive without cpufreq, no need to stop system operations because of that. I don't see why replacing: + if (omap_table_init) + return 0; + omap_table_init = 1; by BUG_ON(omap_table_init == 1); omap_table_init = 1; would prevent you from having no OPP table (the case where a NULL OPP table is passed is tested *before* in omapX_init_opp()). HUH?? NULL table to a static function - what code are you talking about?? why are you so behind BUG_ON, when there are valid reasons for reentry into code. -- Regards, Nishanth Menon -- 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
Re: [PATCH v3 1/3] omap: opp: add OMAP3 OPP table data and common init
Hello, On Tue, 16 Nov 2010 07:10:36 -0600 Nishanth Menon n...@ti.com wrote: I feel you may have misunderstood the code, we DONOT oblige all boards to *have* to call omapX_init_opp. It is a device_initcall - so for the boards that dont call it, device_initcall will trigger and initialzie it. the hooks for the customization of the OPPs is in OPP layer itself. This is exactly what I have understood. the need we satisfy is this: if you need to support two sets of boards: a) boards that are happy with the defaults - most of the boards - dont do anything in the board file. (device_init_call with auto register the defaults) b) boards that need customization - these guys need to call omapX_init_opp(to register the defaults) before customizing the defaults. Does this explain the code and reason for the logic? if you do have a better mechanism, lets know. Yes, it explains the code and reason for the logic, but still doesn't make it pretty :-) would prevent you from having no OPP table (the case where a NULL OPP table is passed is tested *before* in omapX_init_opp()). HUH?? NULL table to a static function - what code are you talking about?? why are you so behind BUG_ON, when there are valid reasons for reentry into code. In the current design, yes, there are indeed valid reasons for reentry into the omapX_init_opp() function, and that's exactly the point I'm critizicing here. Regards! Thomas -- Thomas Petazzoni, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com -- 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
Re: [PATCH v3 1/3] omap: opp: add OMAP3 OPP table data and common init
Thomas Petazzoni had written, on 11/16/2010 06:42 AM, the following: But, sorry, I find this even uglier than I thought it was :) What about adding the obligation to boards file to call the omapX_init_opp() function and then do their customization (if needed), then no call to I knew I had discussed this before! Apologies, I should have dug this thread up earlier in the discussion. my initial implementation had forced board files to call the opp_init_table, then changed that here: http://marc.info/?l=linux-omapm=127431810922704w=2 Kevin seemed happy with the change here: http://marc.info/?l=linux-omapm=127507237109393w=2 -- Regards, Nishanth Menon -- 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
Re: [PATCH v3 1/3] omap: opp: add OMAP3 OPP table data and common init
On Tue, 16 Nov 2010 09:23:06 -0600 Nishanth Menon n...@ti.com wrote: my initial implementation had forced board files to call the opp_init_table, then changed that here: http://marc.info/?l=linux-omapm=127431810922704w=2 Kevin seemed happy with the change here: http://marc.info/?l=linux-omapm=127507237109393w=2 Ok, if Kevin is happy with this solution, fair enough. Sorry for the noise, and thanks for your answers. Regards, Thomas -- Thomas Petazzoni, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com -- 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
Re: [PATCH v3 1/3] omap: opp: add OMAP3 OPP table data and common init
Thomas Petazzoni had written, on 11/16/2010 09:50 AM, the following: On Tue, 16 Nov 2010 09:23:06 -0600 Nishanth Menon n...@ti.com wrote: my initial implementation had forced board files to call the opp_init_table, then changed that here: http://marc.info/?l=linux-omapm=127431810922704w=2 Kevin seemed happy with the change here: http://marc.info/?l=linux-omapm=127507237109393w=2 Ok, if Kevin is happy with this solution, fair enough. Sorry for the noise, and thanks for your answers. Thanks for taking the time to review and the comments. It is always a good idea to evolve to a better solution. If there are no further comments, I will post a v4 later today incorporating comments: a) return error instead of 0 if opp table is already initialized b) change the .h to .c -- Regards, Nishanth Menon -- 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
Re: [PATCH v3 1/3] omap: opp: add OMAP3 OPP table data and common init
Thomas Petazzoni thomas.petazz...@free-electrons.com writes: On Tue, 16 Nov 2010 09:23:06 -0600 Nishanth Menon n...@ti.com wrote: my initial implementation had forced board files to call the opp_init_table, then changed that here: http://marc.info/?l=linux-omapm=127431810922704w=2 Kevin seemed happy with the change here: http://marc.info/?l=linux-omapm=127507237109393w=2 Ok, if Kevin is happy with this solution, fair enough. Sorry for the noise, and thanks for your answers. Yes, I'm not a big fan of the init function called multiple times either, but I really want to minimize what board files have to do. Historically, we tend to add all the init functions to every board file, and this is getting cumbersome to understand and maintain. What we need is for common code to take care of sensible defaults for all boards, and then only boards with non-default behavior have to have custom code. Kevin -- 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
Re: [PATCH v3 1/3] omap: opp: add OMAP3 OPP table data and common init
* Nishanth Menon n...@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 n...@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 opp.c.
Regards,
Tony
--
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
Re: [PATCH v3 1/3] omap: opp: add OMAP3 OPP table data and common init
* Kevin Hilman khil...@deeprootsystems.com [101116 08:06]: Thomas Petazzoni thomas.petazz...@free-electrons.com writes: On Tue, 16 Nov 2010 09:23:06 -0600 Nishanth Menon n...@ti.com wrote: my initial implementation had forced board files to call the opp_init_table, then changed that here: http://marc.info/?l=linux-omapm=127431810922704w=2 Kevin seemed happy with the change here: http://marc.info/?l=linux-omapm=127507237109393w=2 Ok, if Kevin is happy with this solution, fair enough. Sorry for the noise, and thanks for your answers. Yes, I'm not a big fan of the init function called multiple times either, but I really want to minimize what board files have to do. Historically, we tend to add all the init functions to every board file, and this is getting cumbersome to understand and maintain. What we need is for common code to take care of sensible defaults for all boards, and then only boards with non-default behavior have to have custom code. Yeah. The initial comment from Thomas with data in .h file should be fixed though. Tony -- 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
Re: [PATCH v3 1/3] omap: opp: add OMAP3 OPP table data and common init
Tony Lindgren had written, on 11/16/2010 02:35 PM, the following:
* Nishanth Menon n...@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 n...@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 opp.c.
Series rev4 already posted here:
http://marc.info/?l=linux-omapm=128993367212640w=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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/3] omap: opp: add OMAP3 OPP table data and common init
Add OPP data for OMAP34xx and OMAP36xx and initialization functions
to populate OPP tables based on current SoC.
introduce an OMAP generic opp initialization routine which OMAP3
and OMAP4+ SoCs can use to register their OPP definitions.
Signed-off-by: Kevin Hilman khil...@deeprootsystems.com
Signed-off-by: Nishanth Menon n...@ti.com
---
Warning: http://lkml.org/lkml/2010/11/9/389
Introduces ARCH_HAS_OPP which needs to be enabled as well
for OMAP3 in the Kconfig
v3:
* added documentation for custom opp modification
by board files
* switched to using device_initcall to autoinitialize the
opp tables
v2: https://patchwork.kernel.org/patch/266911/
Documentation/arm/OMAP/omap_pm | 26 ++
arch/arm/mach-omap2/Kconfig|1 +
arch/arm/mach-omap2/Makefile |2 +
arch/arm/mach-omap2/opp.c | 154
arch/arm/mach-omap2/opp3xxx_data.h | 82 +++
arch/arm/mach-omap2/pm.h |9 ++
6 files changed, 274 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/mach-omap2/opp.c
create mode 100644 arch/arm/mach-omap2/opp3xxx_data.h
diff --git a/Documentation/arm/OMAP/omap_pm b/Documentation/arm/OMAP/omap_pm
index 5389440..88341f0 100644
--- a/Documentation/arm/OMAP/omap_pm
+++ b/Documentation/arm/OMAP/omap_pm
@@ -127,3 +127,29 @@ implementation needs:
10. (*pdata-cpu_set_freq)(unsigned long f)
11. (*pdata-cpu_get_freq)(void)
+
+Customizing OPP for platform
+
+Defining CONFIG_PM should enable OPP layer for the silicon
+and the registration of OPP table should take place automatically.
+However, in special cases, the default OPP table may need to be
+tweaked, for e.g.:
+ * enable default OPPs which are disabled by default, but which
+ could be enabled on a platform
+ * Disable an unsupported OPP on the platform
+ * Define and add a custom opp table entry
+in these cases, the board file needs to do additional steps as follows:
+arch/arm/mach-omapx/board-xyz.c
+ #include pm.h
+
+ static void __init omap_xyz_init_irq(void)
+ {
+
+ /* Initialize the default table */
+ omapx_opp_init();
+ /* Do customization to the defaults */
+
+ }
+NOTE: omapx_opp_init will be omap3_opp_init or as required
+based on the omap family.
+
diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
index ab784bf..93a91ff 100644
--- a/arch/arm/mach-omap2/Kconfig
+++ b/arch/arm/mach-omap2/Kconfig
@@ -35,6 +35,7 @@ config ARCH_OMAP3
select CPU_V7
select USB_ARCH_HAS_EHCI
select ARM_L1_CACHE_SHIFT_6 if !ARCH_OMAP4
+ select PM_OPP if PM
config ARCH_OMAP4
bool TI OMAP4
diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index 60e51bc..1650a62 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -64,6 +64,8 @@ endif
endif
+obj-$(CONFIG_PM_OPP) += opp.o
+
# PRCM
obj-$(CONFIG_ARCH_OMAP2) += cm.o
obj-$(CONFIG_ARCH_OMAP3) += cm.o
diff --git a/arch/arm/mach-omap2/opp.c b/arch/arm/mach-omap2/opp.c
new file mode 100644
index 000..79c711c
--- /dev/null
+++ b/arch/arm/mach-omap2/opp.c
@@ -0,0 +1,154 @@
+/*
+ * OMAP SoC specific OPP wrapper function
+ *
+ * Copyright (C) 2009 - 2010 Texas Instruments Incorporated.
+ * Nishanth Menon
+ * Copyright (C) 2009 - 2010 Deep Root Systems, LLC.
+ * Kevin Hilman
+ * Copyright (C) 2010 Nokia Corporation.
+ * Eduardo Valentin
+ *
+ * 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.
+ */
+
+#include linux/module.h
+#include linux/err.h
+#include linux/opp.h
+
+#include plat/cpu.h
+#include plat/omap_device.h
+
+#include pm.h
+
+/**
+ * struct omap_opp_def - OMAP OPP Definition
+ * @hwmod_name:Name of the hwmod for this domain
+ * @freq: Frequency in hertz corresponding to this OPP
+ * @u_volt:Nominal voltage in microvolts corresponding to this OPP
+ * @enabled: True/false - is this OPP enabled/disabled by default
+ *
+ * OMAP SOCs have a standard set of tuples consisting of frequency and voltage
+ * pairs that the device will support per voltage domain. This is called
+ * Operating Points or OPP. The actual definitions of OMAP Operating Points
+ * varies over silicon within the same family of devices. For a specific
+ * domain, you can have a set of {frequency, voltage} pairs and this is denoted
+ * by an array of omap_opp_def. As the kernel boots and more information is
+ * available, a set of these are activated based on the precise nature of
+ * device the kernel boots up on. It is interesting to remember that each IP
+ * which belongs to a voltage domain may define their own set of OPPs on top
+ * of this - but this is
Re: [PATCH v3 1/3] omap: opp: add OMAP3 OPP table data and common init
Hello,
On Mon, 15 Nov 2010 13:27:39 -0600
Nishanth Menon n...@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.
As far as I could see, most of the kernel instantiate structure in C
files instead.
Regards,
Thomas
--
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
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
Re: [PATCH v3 1/3] omap: opp: add OMAP3 OPP table data and common init
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 n...@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.
--
Regards,
Nishanth Menon
--
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
