[PM-OPP][PATCH 0/2] OMAP: pm: opp: few additional cleanups

[PM-OPP][PATCH 0/2] OMAP: pm: opp: few additional cleanups

[Nishanth Menon]

Cleanup opp layer on pending topics:
SRF is dead, so remove the opp_id concept from opp layer, no more
deprecated functions for us.. (series is based on khilman's pm branch)

There are two things pending with opp layer as pointed in the discussion
with Sanjeev:
a) we need to make the opp data definition a little more CPUFREQ independent
 - i have taken this up in patch 2/2
b) The thing pending: how do we fit in mpurate?
Problem that Sanjeev pointed out in the thread[1] was this:
lets say a bootloader running at freq x, runs kernel at freq y, should'nt the
kernel verify if this is a valid freq for that board before setting it?	
	The call sequence currently is:
	(1) arch/arm/plat-omap/clock.c omap_clk_setup
	(2) arch/arm/mach-omap2/clock.c omap2_clk_switch_mpurate_at_boot
	(3) arch/arm/mach-omap2/opp3xxx_data.c omap3_pm_init_opp_table
mpurate is populated at (1), used at (2) and the opp table against which
it can be verified is at (3). here in lies the chicken-or-egg problem :(.
since the operating frequencies are decided based on board behavior as well,
we cant really set the frequency at (2) as we do now, and we may want to
consider the option of moving it down beyond (3).

Note: we will also have to consider the fact that we may need a higher/different
voltage for that mpu frequency as well.. which may mean this handling be
moved out of the clock.c to pm.c instead..

Does anyone have a suggestion as to how we'd want to proceed?
	
Nishanth Menon (2):
  omap: pm: opp: remove opp_id
  omap3: opp: make independent of cpufreq

 arch/arm/mach-omap2/Makefile                       |    5 +--
 .../mach-omap2/{cpufreq34xx.c => opp3xxx_data.c}   |    0
 arch/arm/plat-omap/include/plat/opp.h              |   15 ------
 arch/arm/plat-omap/opp.c                           |   47 --------------------
 4 files changed, 1 insertions(+), 66 deletions(-)
 rename arch/arm/mach-omap2/{cpufreq34xx.c => opp3xxx_data.c} (100%)


Cc: Eduardo Valentin <eduardo.valentin@nokia.com>
Cc: Kevin Hilman <khilman@deeprootsystems.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Rajendra Nayak <rnayak@ti.com>
Cc: Sanjeev Premi <premi@ti.com>
Cc: Thara Gopinath <thara@ti.com>
Cc: Tony Lindgren <tony@atomide.com>

Regards,
Nishanth Menon
Ref:
[1]: http://marc.info/?t=127495972700003&r=1&w=2

[PM-OPP][PATCH 1/2] omap: pm: opp: remove opp_id

[Nishanth Menon]

Remove the concept of opp_id now that SRF is gone from pm branch
the new framework should exist on silicon variants without the
notion of opp ids

This also removes the deprecated functions opp_get_opp_id and
opp_find_by_opp_id.

Cc: Eduardo Valentin <eduardo.valentin@nokia.com>
Cc: Kevin Hilman <khilman@deeprootsystems.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Rajendra Nayak <rnayak@ti.com>
Cc: Sanjeev Premi <premi@ti.com>
Cc: Thara Gopinath <thara@ti.com>
Cc: Tony Lindgren <tony@atomide.com>

Signed-off-by: Nishanth Menon <nm@ti.com>
---

NOTE: for those desiring to live with SRF will have to revert this
patch

 arch/arm/plat-omap/include/plat/opp.h |   15 ----------
 arch/arm/plat-omap/opp.c              |   47 ---------------------------------
 2 files changed, 0 insertions(+), 62 deletions(-)

diff --git a/arch/arm/plat-omap/include/plat/opp.h b/arch/arm/plat-omap/include/plat/opp.h
index f9feb8d..997b56e 100644
--- a/arch/arm/plat-omap/include/plat/opp.h
+++ b/arch/arm/plat-omap/include/plat/opp.h
@@ -82,10 +82,6 @@ int opp_enable(struct omap_opp *opp);
 
 int opp_disable(struct omap_opp *opp);
 
-struct omap_opp *__deprecated opp_find_by_opp_id(struct device *dev,
-						  u8 opp_id);
-u8 __deprecated opp_get_opp_id(struct omap_opp *opp);
-
 void opp_init_cpufreq_table(struct device *dev,
 			    struct cpufreq_frequency_table **table);
 #else
@@ -139,17 +135,6 @@ static inline int opp_disable(struct omap_opp *opp)
 	return 0;
 }
 
-static inline struct omap_opp *__deprecated
-opp_find_by_opp_id(struct omap_opp *opps, u8 opp_id)
-{
-	return ERR_PTR(-EINVAL);
-}
-
-static inline u8 __deprecated opp_get_opp_id(struct omap_opp *opp)
-{
-	return 0;
-}
-
 static inline
 void opp_init_cpufreq_table(struct omap_opp *opps,
 			    struct cpufreq_frequency_table **table)
diff --git a/arch/arm/plat-omap/opp.c b/arch/arm/plat-omap/opp.c
index b9b7bda..a246bb9 100644
--- a/arch/arm/plat-omap/opp.c
+++ b/arch/arm/plat-omap/opp.c
@@ -27,7 +27,6 @@
  * @enabled:	true/false - marking this OPP as enabled/disabled
  * @rate:	Frequency in hertz
  * @u_volt:	Nominal voltage in microvolts corresponding to this OPP
- * @opp_id:	opp identifier (deprecated)
  * @dev_opp:	contains the device_opp struct
  *
  * This structure stores the OPP information for a given domain.
@@ -38,7 +37,6 @@ struct omap_opp {
 	bool enabled;
 	unsigned long rate;
 	unsigned long u_volt;
-	u8 opp_id;
 
 	struct device_opp *dev_opp;
 };
@@ -132,50 +130,6 @@ unsigned long opp_get_freq(const struct omap_opp *opp)
 }
 
 /**
- * opp_find_by_opp_id - look up OPP by OPP ID (deprecated)
- * @opp_type:	OPP type where we want the look up to happen.
- * @opp_id:	OPP ID to search for
- *
- * Returns the struct omap_opp pointer corresponding to the given OPP
- * ID @opp_id, or returns NULL on error.
- */
-struct omap_opp * __deprecated opp_find_by_opp_id(struct device *dev,
-						  u8 opp_id)
-{
-	struct device_opp *dev_opp;
-	struct omap_opp *temp_opp, *opp = ERR_PTR(-ENODEV);
-
-	dev_opp = find_device_opp(dev);
-	if (IS_ERR(dev_opp))
-		return opp;
-
-	list_for_each_entry(temp_opp, &dev_opp->opp_list, node) {
-		if (temp_opp->enabled && temp_opp->opp_id == opp_id) {
-			opp = temp_opp;
-			break;
-		}
-	}
-
-	return opp;
-}
-
-/**
- * opp_get_opp_id() - Provide OPP ID corresponding to an OPP (deprecated)
- * @opp:	opp for which frequency has to be returned for
- *
- * Returns an OPP ID for the OPP required, if error, returns 0
- */
-u8 __deprecated opp_get_opp_id(struct omap_opp *opp)
-{
-	if (unlikely(!opp || IS_ERR(opp)) || !opp->enabled) {
-		pr_err("%s: Invalid parameter being passed\n", __func__);
-		return 0;
-	}
-
-	return opp->opp_id;
-}
-
-/**
  * opp_get_opp_count() - Get number of opps enabled in the opp list
  * @opp_type:	OPP type we want to count
  *
@@ -413,7 +367,6 @@ int opp_add(const struct omap_opp_def *opp_def)
 	/* renumber (deprecated) OPP IDs based on new order */
 	i = 0;
 	list_for_each_entry(opp, &dev_opp->opp_list, node)
-		opp->opp_id = i++;
 
 	return 0;
 }
-- 
1.6.3.3

[PM-OPP][PATCH 2/2] omap3: opp: make independent of cpufreq

[Nishanth Menon]

Make opp3xx data which is registered with the opp layer
dependent purely on CONFIG_PM as opp layer and pm.c users
are CONFIG_PM dependent not cpufreq dependent.
so we rename the data definition to opp3xxx_data.c (inline with what
we have for omap2), also move the build definition to be under
the existing CONFIG_PM build instead of CPUFREQ.

Cc: Eduardo Valentin <eduardo.valentin@nokia.com>
Cc: Kevin Hilman <khilman@deeprootsystems.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Rajendra Nayak <rnayak@ti.com>
Cc: Sanjeev Premi <premi@ti.com>
Cc: Thara Gopinath <thara@ti.com>
Cc: Tony Lindgren <tony@atomide.com>

Signed-off-by: Nishanth Menon <nm@ti.com>
---
Note:
This takes care of the discussion on opp file renaming and making
it independent of cpufreq, unless I missed something else

 arch/arm/mach-omap2/Makefile                       |    5 +----
 .../mach-omap2/{cpufreq34xx.c => opp3xxx_data.c}   |    0
 2 files changed, 1 insertions(+), 4 deletions(-)
 rename arch/arm/mach-omap2/{cpufreq34xx.c => opp3xxx_data.c} (100%)

diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index 0b188fa..43d7372 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -58,11 +58,8 @@ obj-$(CONFIG_OMAP_SMARTREFLEX_CLASS3)	+= smartreflex-class3.o
 AFLAGS_sleep24xx.o			:=-Wa,-march=armv6
 AFLAGS_sleep34xx.o			:=-Wa,-march=armv7-a
 
-endif
+obj-$(CONFIG_ARCH_OMAP3)		+= opp3xxx_data.o
 
-# CPU Frequency
-ifeq ($(CONFIG_CPU_FREQ),y)
-obj-$(CONFIG_ARCH_OMAP3)		+= cpufreq34xx.o
 endif
 
 # PRCM
diff --git a/arch/arm/mach-omap2/cpufreq34xx.c b/arch/arm/mach-omap2/opp3xxx_data.c
similarity index 100%
rename from arch/arm/mach-omap2/cpufreq34xx.c
rename to arch/arm/mach-omap2/opp3xxx_data.c
-- 
1.6.3.3

RE: [PM-OPP][PATCH 2/2] omap3: opp: make independent of cpufreq

[Gopinath, Thara]

>>-----Original Message-----
>>From: Menon, Nishanth
>>Sent: Wednesday, August 11, 2010 7:47 AM
>>To: linux-omap
>>Cc: Menon, Nishanth; Eduardo Valentin; Kevin Hilman; Paul Walmsley; Nayak, Rajendra; Premi, Sanjeev;
>>Gopinath, Thara; Tony Lindgren
>>Subject: [PM-OPP][PATCH 2/2] omap3: opp: make independent of cpufreq
>>
>>Make opp3xx data which is registered with the opp layer
>>dependent purely on CONFIG_PM as opp layer and pm.c users
>>are CONFIG_PM dependent not cpufreq dependent.
>>so we rename the data definition to opp3xxx_data.c (inline with what
>>we have for omap2), also move the build definition to be under
>>the existing CONFIG_PM build instead of CPUFREQ.
>>
>>Cc: Eduardo Valentin <eduardo.valentin@nokia.com>
>>Cc: Kevin Hilman <khilman@deeprootsystems.com>
>>Cc: Paul Walmsley <paul@pwsan.com>
>>Cc: Rajendra Nayak <rnayak@ti.com>
>>Cc: Sanjeev Premi <premi@ti.com>
>>Cc: Thara Gopinath <thara@ti.com>
>>Cc: Tony Lindgren <tony@atomide.com>
>>
>>Signed-off-by: Nishanth Menon <nm@ti.com>
>>---
>>Note:
>>This takes care of the discussion on opp file renaming and making
>>it independent of cpufreq, unless I missed something else
>>
>> arch/arm/mach-omap2/Makefile                       |    5 +----
>> .../mach-omap2/{cpufreq34xx.c => opp3xxx_data.c}   |    0
>> 2 files changed, 1 insertions(+), 4 deletions(-)
>> rename arch/arm/mach-omap2/{cpufreq34xx.c => opp3xxx_data.c} (100%)

Is this part of PM-OPP branch? Also I was thinking of reusing the same file for OMAP4.
No reason why we should have a different file for OMAP4. So a better name than opp3xxx_data.c?

Regards
Thara
>>
>>diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
>>index 0b188fa..43d7372 100644
>>--- a/arch/arm/mach-omap2/Makefile
>>+++ b/arch/arm/mach-omap2/Makefile
>>@@ -58,11 +58,8 @@ obj-$(CONFIG_OMAP_SMARTREFLEX_CLASS3)	+= smartreflex-class3.o
>> AFLAGS_sleep24xx.o			:=-Wa,-march=armv6
>> AFLAGS_sleep34xx.o			:=-Wa,-march=armv7-a
>>
>>-endif
>>+obj-$(CONFIG_ARCH_OMAP3)		+= opp3xxx_data.o
>>
>>-# CPU Frequency
>>-ifeq ($(CONFIG_CPU_FREQ),y)
>>-obj-$(CONFIG_ARCH_OMAP3)		+= cpufreq34xx.o
>> endif
>>
>> # PRCM
>>diff --git a/arch/arm/mach-omap2/cpufreq34xx.c b/arch/arm/mach-omap2/opp3xxx_data.c
>>similarity index 100%
>>rename from arch/arm/mach-omap2/cpufreq34xx.c
>>rename to arch/arm/mach-omap2/opp3xxx_data.c
>>--
>>1.6.3.3

Re: [PM-OPP][PATCH 2/2] omap3: opp: make independent of cpufreq

[Nishanth Menon]

On 08/11/2010 04:12 AM, Gopinath, Thara wrote:
>
>
>>> -----Original Message-----
>>> From: Menon, Nishanth
>>> Sent: Wednesday, August 11, 2010 7:47 AM
>>> To: linux-omap
>>> Cc: Menon, Nishanth; Eduardo Valentin; Kevin Hilman; Paul Walmsley; Nayak, Rajendra; Premi, Sanjeev;
>>> Gopinath, Thara; Tony Lindgren
>>> Subject: [PM-OPP][PATCH 2/2] omap3: opp: make independent of cpufreq
>>>
>>> Make opp3xx data which is registered with the opp layer
>>> dependent purely on CONFIG_PM as opp layer and pm.c users
>>> are CONFIG_PM dependent not cpufreq dependent.
>>> so we rename the data definition to opp3xxx_data.c (inline with what
>>> we have for omap2), also move the build definition to be under
>>> the existing CONFIG_PM build instead of CPUFREQ.
>>>
>>> Cc: Eduardo Valentin<eduardo.valentin@nokia.com>
>>> Cc: Kevin Hilman<khilman@deeprootsystems.com>
>>> Cc: Paul Walmsley<paul@pwsan.com>
>>> Cc: Rajendra Nayak<rnayak@ti.com>
>>> Cc: Sanjeev Premi<premi@ti.com>
>>> Cc: Thara Gopinath<thara@ti.com>
>>> Cc: Tony Lindgren<tony@atomide.com>
>>>
>>> Signed-off-by: Nishanth Menon<nm@ti.com>
>>> ---
>>> Note:
>>> This takes care of the discussion on opp file renaming and making
>>> it independent of cpufreq, unless I missed something else
>>>
>>> arch/arm/mach-omap2/Makefile                       |    5 +----
>>> .../mach-omap2/{cpufreq34xx.c =>  opp3xxx_data.c}   |    0
>>> 2 files changed, 1 insertions(+), 4 deletions(-)
>>> rename arch/arm/mach-omap2/{cpufreq34xx.c =>  opp3xxx_data.c} (100%)
>
> Is this part of PM-OPP branch? Also I was thinking of reusing the same file for OMAP4.
this defines the opp data base and would be part of pm-opp branch. the 
idea of rename was this:
a) be clear that this is not dependent on cpufreq alone.
b) use the same convention in arch/arm/mach-omap2/ like omap2's opp data 
files which could be converted to use the opp layer now instead of 
having it's own opp layer. and maybe hopefully omap1 as well..
c) when we do specific product build, it makes sense to have arch 
specific files as it makes not much reason to carry the omap4/2 
definitions(even if init_data).

> No reason why we should have a different file for OMAP4. So a better name than opp3xxx_data.c?
why do we need to have it in the same file? Remember, 3630,3430 are 
under OMAP3 family, but omap4 is considered a different arch.

Regards,
Nishanth Menon
[...]

RE: [PM-OPP][PATCH 2/2] omap3: opp: make independent of cpufreq

[Gopinath, Thara]

>>-----Original Message-----
>>From: Nishanth Menon [mailto:menon.nishanth@gmail.com]
>>Sent: Wednesday, August 11, 2010 4:14 PM
>>To: Gopinath, Thara
>>Cc: Menon, Nishanth; linux-omap; Eduardo Valentin; Kevin Hilman; Paul Walmsley; Nayak, Rajendra;
>>Premi, Sanjeev; Tony Lindgren
>>Subject: Re: [PM-OPP][PATCH 2/2] omap3: opp: make independent of cpufreq
>>
>>On 08/11/2010 04:12 AM, Gopinath, Thara wrote:
>>>
>>>
>>>>> -----Original Message-----
>>>>> From: Menon, Nishanth
>>>>> Sent: Wednesday, August 11, 2010 7:47 AM
>>>>> To: linux-omap
>>>>> Cc: Menon, Nishanth; Eduardo Valentin; Kevin Hilman; Paul Walmsley; Nayak, Rajendra; Premi,
>>Sanjeev;
>>>>> Gopinath, Thara; Tony Lindgren
>>>>> Subject: [PM-OPP][PATCH 2/2] omap3: opp: make independent of cpufreq
>>>>>
>>>>> Make opp3xx data which is registered with the opp layer
>>>>> dependent purely on CONFIG_PM as opp layer and pm.c users
>>>>> are CONFIG_PM dependent not cpufreq dependent.
>>>>> so we rename the data definition to opp3xxx_data.c (inline with what
>>>>> we have for omap2), also move the build definition to be under
>>>>> the existing CONFIG_PM build instead of CPUFREQ.
>>>>>
>>>>> Cc: Eduardo Valentin<eduardo.valentin@nokia.com>
>>>>> Cc: Kevin Hilman<khilman@deeprootsystems.com>
>>>>> Cc: Paul Walmsley<paul@pwsan.com>
>>>>> Cc: Rajendra Nayak<rnayak@ti.com>
>>>>> Cc: Sanjeev Premi<premi@ti.com>
>>>>> Cc: Thara Gopinath<thara@ti.com>
>>>>> Cc: Tony Lindgren<tony@atomide.com>
>>>>>
>>>>> Signed-off-by: Nishanth Menon<nm@ti.com>
>>>>> ---
>>>>> Note:
>>>>> This takes care of the discussion on opp file renaming and making
>>>>> it independent of cpufreq, unless I missed something else
>>>>>
>>>>> arch/arm/mach-omap2/Makefile                       |    5 +----
>>>>> .../mach-omap2/{cpufreq34xx.c =>  opp3xxx_data.c}   |    0
>>>>> 2 files changed, 1 insertions(+), 4 deletions(-)
>>>>> rename arch/arm/mach-omap2/{cpufreq34xx.c =>  opp3xxx_data.c} (100%)
>>>
>>> Is this part of PM-OPP branch? Also I was thinking of reusing the same file for OMAP4.
>>this defines the opp data base and would be part of pm-opp branch. the
>>idea of rename was this:
>>a) be clear that this is not dependent on cpufreq alone.

I do not understand this. This files is not present in PM-OPP branch. But you have a patch modifying it against PM-OPP branch. Am I looking at a wrong version of PM-OPP branch?

>>b) use the same convention in arch/arm/mach-omap2/ like omap2's opp data
>>files which could be converted to use the opp layer now instead of
>>having it's own opp layer. and maybe hopefully omap1 as well..
>>c) when we do specific product build, it makes sense to have arch
>>specific files as it makes not much reason to carry the omap4/2
>>definitions(even if init_data).
>>
>>> No reason why we should have a different file for OMAP4. So a better name than opp3xxx_data.c?
>>why do we need to have it in the same file? Remember, 3630,3430 are
>>under OMAP3 family, but omap4 is considered a different arch.

Code is more or less the same. Is that not a sufficient reason to reuse a  file ?
>>
>>Regards,
>>Nishanth Menon
>>[...]

Re: [PM-OPP][PATCH 2/2] omap3: opp: make independent of cpufreq

[Nishanth Menon]

On 08/11/2010 06:23 AM, Gopinath, Thara wrote:
>
>
>>> -----Original Message-----
>>> From: Nishanth Menon [mailto:menon.nishanth@gmail.com]
>>> Sent: Wednesday, August 11, 2010 4:14 PM
>>> To: Gopinath, Thara
>>> Cc: Menon, Nishanth; linux-omap; Eduardo Valentin; Kevin Hilman; Paul Walmsley; Nayak, Rajendra;
>>> Premi, Sanjeev; Tony Lindgren
>>> Subject: Re: [PM-OPP][PATCH 2/2] omap3: opp: make independent of cpufreq
>>>
>>> On 08/11/2010 04:12 AM, Gopinath, Thara wrote:
>>>>
>>>>
>>>>>> -----Original Message-----
>>>>>> From: Menon, Nishanth
>>>>>> Sent: Wednesday, August 11, 2010 7:47 AM
>>>>>> To: linux-omap
>>>>>> Cc: Menon, Nishanth; Eduardo Valentin; Kevin Hilman; Paul Walmsley; Nayak, Rajendra; Premi,
>>> Sanjeev;
>>>>>> Gopinath, Thara; Tony Lindgren
>>>>>> Subject: [PM-OPP][PATCH 2/2] omap3: opp: make independent of cpufreq
>>>>>>
>>>>>> Make opp3xx data which is registered with the opp layer
>>>>>> dependent purely on CONFIG_PM as opp layer and pm.c users
>>>>>> are CONFIG_PM dependent not cpufreq dependent.
>>>>>> so we rename the data definition to opp3xxx_data.c (inline with what
>>>>>> we have for omap2), also move the build definition to be under
>>>>>> the existing CONFIG_PM build instead of CPUFREQ.
>>>>>>
>>>>>> Cc: Eduardo Valentin<eduardo.valentin@nokia.com>
>>>>>> Cc: Kevin Hilman<khilman@deeprootsystems.com>
>>>>>> Cc: Paul Walmsley<paul@pwsan.com>
>>>>>> Cc: Rajendra Nayak<rnayak@ti.com>
>>>>>> Cc: Sanjeev Premi<premi@ti.com>
>>>>>> Cc: Thara Gopinath<thara@ti.com>
>>>>>> Cc: Tony Lindgren<tony@atomide.com>
>>>>>>
>>>>>> Signed-off-by: Nishanth Menon<nm@ti.com>
>>>>>> ---
>>>>>> Note:
>>>>>> This takes care of the discussion on opp file renaming and making
>>>>>> it independent of cpufreq, unless I missed something else
>>>>>>
>>>>>> arch/arm/mach-omap2/Makefile                       |    5 +----
>>>>>> .../mach-omap2/{cpufreq34xx.c =>   opp3xxx_data.c}   |    0
>>>>>> 2 files changed, 1 insertions(+), 4 deletions(-)
>>>>>> rename arch/arm/mach-omap2/{cpufreq34xx.c =>   opp3xxx_data.c} (100%)
>>>>
>>>> Is this part of PM-OPP branch? Also I was thinking of reusing the same file for OMAP4.
>>> this defines the opp data base and would be part of pm-opp branch. the
>>> idea of rename was this:
>>> a) be clear that this is not dependent on cpufreq alone.
>
> I do not understand this. This files is not present in PM-OPP branch. But you have a patch modifying it against PM-OPP branch. Am I looking at a wrong version of PM-OPP branch?
you got me curious as well, my apologies, I had assumed things were how 
they were before :( Looks like Kevin shuffled things around and the data 
by itself is in cpufreq branch:
http://git.kernel.org/?p=linux/kernel/git/khilman/linux-omap-pm.git;a=shortlog;h=refs/heads/pm-cpufreq

ergo, Kevin, do we need this cpufreq branch to contain the opp data:
http://git.kernel.org/?p=linux/kernel/git/khilman/linux-omap-pm.git;a=commitdiff;h=9f6847282f65cdcd26d740e6ae6afadc3ee00233
and related changes could potentially be pulled into the same pm-opp series?

>
>>> b) use the same convention in arch/arm/mach-omap2/ like omap2's opp data
>>> files which could be converted to use the opp layer now instead of
>>> having it's own opp layer. and maybe hopefully omap1 as well..
>>> c) when we do specific product build, it makes sense to have arch
>>> specific files as it makes not much reason to carry the omap4/2
>>> definitions(even if init_data).
>>>
>>>> No reason why we should have a different file for OMAP4. So a better name than opp3xxx_data.c?
>>> why do we need to have it in the same file? Remember, 3630,3430 are
>>> under OMAP3 family, but omap4 is considered a different arch.
>
> Code is more or less the same. Is that not a sufficient reason to reuse a  file ?
I dont really care as long as opp layer is usable by mpurate without 
depending on cpufreq and it is maintainable without going to if else 
nightmare. But personally, I dont see really reusuable code in that file 
(other than doing an opp addition in a loop) it could result eventually 
in a large amount of code redundancy and maintenance nightmare with 
OMAP4 variants coming in.

Regards,
Nishanth Menon

RE: [PM-OPP][PATCH 2/2] omap3: opp: make independent of cpufreq

[Gopinath, Thara]

>>-----Original Message-----
>>From: Menon, Nishanth
>>Sent: Wednesday, August 11, 2010 5:08 PM
>>To: Gopinath, Thara
>>Cc: Nishanth Menon; linux-omap; Eduardo Valentin; Kevin Hilman; Paul Walmsley; Nayak, Rajendra;
>>Premi, Sanjeev; Tony Lindgren
>>Subject: Re: [PM-OPP][PATCH 2/2] omap3: opp: make independent of cpufreq
>>
>>On 08/11/2010 06:23 AM, Gopinath, Thara wrote:
>>>
>>>
>>>>> -----Original Message-----
>>>>> From: Nishanth Menon [mailto:menon.nishanth@gmail.com]
>>>>> Sent: Wednesday, August 11, 2010 4:14 PM
>>>>> To: Gopinath, Thara
>>>>> Cc: Menon, Nishanth; linux-omap; Eduardo Valentin; Kevin Hilman; Paul Walmsley; Nayak, Rajendra;
>>>>> Premi, Sanjeev; Tony Lindgren
>>>>> Subject: Re: [PM-OPP][PATCH 2/2] omap3: opp: make independent of cpufreq
>>>>>
>>>>> On 08/11/2010 04:12 AM, Gopinath, Thara wrote:
>>>>>>
>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Menon, Nishanth
>>>>>>>> Sent: Wednesday, August 11, 2010 7:47 AM
>>>>>>>> To: linux-omap
>>>>>>>> Cc: Menon, Nishanth; Eduardo Valentin; Kevin Hilman; Paul Walmsley; Nayak, Rajendra; Premi,
>>>>> Sanjeev;
>>>>>>>> Gopinath, Thara; Tony Lindgren
>>>>>>>> Subject: [PM-OPP][PATCH 2/2] omap3: opp: make independent of cpufreq
>>>>>>>>
>>>>>>>> Make opp3xx data which is registered with the opp layer
>>>>>>>> dependent purely on CONFIG_PM as opp layer and pm.c users
>>>>>>>> are CONFIG_PM dependent not cpufreq dependent.
>>>>>>>> so we rename the data definition to opp3xxx_data.c (inline with what
>>>>>>>> we have for omap2), also move the build definition to be under
>>>>>>>> the existing CONFIG_PM build instead of CPUFREQ.
>>>>>>>>
>>>>>>>> Cc: Eduardo Valentin<eduardo.valentin@nokia.com>
>>>>>>>> Cc: Kevin Hilman<khilman@deeprootsystems.com>
>>>>>>>> Cc: Paul Walmsley<paul@pwsan.com>
>>>>>>>> Cc: Rajendra Nayak<rnayak@ti.com>
>>>>>>>> Cc: Sanjeev Premi<premi@ti.com>
>>>>>>>> Cc: Thara Gopinath<thara@ti.com>
>>>>>>>> Cc: Tony Lindgren<tony@atomide.com>
>>>>>>>>
>>>>>>>> Signed-off-by: Nishanth Menon<nm@ti.com>
>>>>>>>> ---
>>>>>>>> Note:
>>>>>>>> This takes care of the discussion on opp file renaming and making
>>>>>>>> it independent of cpufreq, unless I missed something else
>>>>>>>>
>>>>>>>> arch/arm/mach-omap2/Makefile                       |    5 +----
>>>>>>>> .../mach-omap2/{cpufreq34xx.c =>   opp3xxx_data.c}   |    0
>>>>>>>> 2 files changed, 1 insertions(+), 4 deletions(-)
>>>>>>>> rename arch/arm/mach-omap2/{cpufreq34xx.c =>   opp3xxx_data.c} (100%)
>>>>>>
>>>>>> Is this part of PM-OPP branch? Also I was thinking of reusing the same file for OMAP4.
>>>>> this defines the opp data base and would be part of pm-opp branch. the
>>>>> idea of rename was this:
>>>>> a) be clear that this is not dependent on cpufreq alone.
>>>
>>> I do not understand this. This files is not present in PM-OPP branch. But you have a patch
>>modifying it against PM-OPP branch. Am I looking at a wrong version of PM-OPP branch?
>>you got me curious as well, my apologies, I had assumed things were how
>>they were before :( Looks like Kevin shuffled things around and the data
>>by itself is in cpufreq branch:
>>http://git.kernel.org/?p=linux/kernel/git/khilman/linux-omap-pm.git;a=shortlog;h=refs/heads/pm-
>>cpufreq
>>
>>ergo, Kevin, do we need this cpufreq branch to contain the opp data:
>>http://git.kernel.org/?p=linux/kernel/git/khilman/linux-omap-
>>pm.git;a=commitdiff;h=9f6847282f65cdcd26d740e6ae6afadc3ee00233
>>and related changes could potentially be pulled into the same pm-opp series?
>>
>>>
>>>>> b) use the same convention in arch/arm/mach-omap2/ like omap2's opp data
>>>>> files which could be converted to use the opp layer now instead of
>>>>> having it's own opp layer. and maybe hopefully omap1 as well..
>>>>> c) when we do specific product build, it makes sense to have arch
>>>>> specific files as it makes not much reason to carry the omap4/2
>>>>> definitions(even if init_data).
>>>>>
>>>>>> No reason why we should have a different file for OMAP4. So a better name than opp3xxx_data.c?
>>>>> why do we need to have it in the same file? Remember, 3630,3430 are
>>>>> under OMAP3 family, but omap4 is considered a different arch.
>>>
>>> Code is more or less the same. Is that not a sufficient reason to reuse a  file ?
>>I dont really care as long as opp layer is usable by mpurate without
>>depending on cpufreq and it is maintainable without going to if else
>>nightmare. But personally, I dont see really reusuable code in that file
>>(other than doing an opp addition in a loop) it could result eventually
>>in a large amount of code redundancy and maintenance nightmare with
>>OMAP4 variants coming in.

Why do you say maintenance nightmare? It is going to be one opp table per SoC. Anyways, Kevin what is your take on this?

>>
>>Regards,
>>Nishanth Menon

Re: [PM-OPP][PATCH 2/2] omap3: opp: make independent of cpufreq

[Kevin Hilman]

"Gopinath, Thara" <thara@ti.com> writes:

[...]

>>>>>>
>>>>>>> No reason why we should have a different file for OMAP4. So a better name than opp3xxx_data.c?
>>>>>> why do we need to have it in the same file? Remember, 3630,3430 are
>>>>>> under OMAP3 family, but omap4 is considered a different arch.
>>>>
>>>> Code is more or less the same. Is that not a sufficient reason to reuse a  file ?
>>>I dont really care as long as opp layer is usable by mpurate without
>>>depending on cpufreq and it is maintainable without going to if else
>>>nightmare. But personally, I dont see really reusuable code in that file
>>>(other than doing an opp addition in a loop) it could result eventually
>>>in a large amount of code redundancy and maintenance nightmare with
>>>OMAP4 variants coming in.
>
> Why do you say maintenance nightmare? It is going to be one opp table
> per SoC. Anyways, Kevin what is your take on this?
>

I think we should keep separate files for each SoC listing the OPP data,
and in those files should be *only* data.

The init functions across these files will be basically the same, so
maybe the common code should be pulled out into a separate file (pm.c?),
and the data files have a very simple init function (device_initcall) that just registers
their data.

Kevin

Re: [PM-OPP][PATCH 2/2] omap3: opp: make independent of cpufreq

[Nishanth Menon]

Kevin Hilman had written, on 08/12/2010 09:34 AM, the following:
> "Gopinath, Thara" <thara@ti.com> writes:
> 
> [...]
> 
>>>>>>>> No reason why we should have a different file for OMAP4. So a better name than opp3xxx_data.c?
>>>>>>> why do we need to have it in the same file? Remember, 3630,3430 are
>>>>>>> under OMAP3 family, but omap4 is considered a different arch.
>>>>> Code is more or less the same. Is that not a sufficient reason to reuse a  file ?
>>>> I dont really care as long as opp layer is usable by mpurate without
>>>> depending on cpufreq and it is maintainable without going to if else
>>>> nightmare. But personally, I dont see really reusuable code in that file
>>>> (other than doing an opp addition in a loop) it could result eventually
>>>> in a large amount of code redundancy and maintenance nightmare with
>>>> OMAP4 variants coming in.
>> Why do you say maintenance nightmare? It is going to be one opp table
>> per SoC. Anyways, Kevin what is your take on this?
>>
> 
> I think we should keep separate files for each SoC listing the OPP data,
> and in those files should be *only* data.
> 
> The init functions across these files will be basically the same, so
> maybe the common code should be pulled out into a separate file (pm.c?),
> and the data files have a very simple init function (device_initcall) that just registers
> their data.
> 
yep, this sounds like a good idea, let me try something on this line and 
post a new rev..

-- 
Regards,
Nishanth Menon