Re: [PM-OPP][PATCH 2/2] omap3: opp: make independent of cpufreq
Kevin Hilman had written, on 08/12/2010 09:34 AM, the following: "Gopinath, Thara" 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 -- 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: [PM-OPP][PATCH 2/2] omap3: opp: make independent of cpufreq
"Gopinath, Thara" 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 -- 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: [PM-OPP][PATCH 2/2] omap3: opp: make independent of cpufreq
>>-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.nisha...@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
>>>>>>>> Cc: Kevin Hilman
>>>>>>>> Cc: Paul Walmsley
>>>>>>>> Cc: Rajendra Nayak
>>>>>>>> Cc: Sanjeev Premi
>>>>>>>> Cc: Thara Gopinath
>>>>>>>> Cc: Tony Lindgren
>>>>>>>>
>>>>>>>> Signed-off-by: Nishanth Menon
>>>>>>>> ---
>>>>>>>> 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
--
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: [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.nisha...@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
Cc: Kevin Hilman
Cc: Paul Walmsley
Cc: Rajendra Nayak
Cc: Sanjeev Premi
Cc: Thara Gopinath
Cc: Tony Lindgren
Signed-off-by: Nishanth Menon
---
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
--
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: [PM-OPP][PATCH 2/2] omap3: opp: make independent of cpufreq
>>-Original Message-
>>From: Nishanth Menon [mailto:menon.nisha...@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
>>>>> Cc: Kevin Hilman
>>>>> Cc: Paul Walmsley
>>>>> Cc: Rajendra Nayak
>>>>> Cc: Sanjeev Premi
>>>>> Cc: Thara Gopinath
>>>>> Cc: Tony Lindgren
>>>>>
>>>>> Signed-off-by: Nishanth Menon
>>>>> ---
>>>>> 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
>>[...]
--
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: [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
Cc: Kevin Hilman
Cc: Paul Walmsley
Cc: Rajendra Nayak
Cc: Sanjeev Premi
Cc: Thara Gopinath
Cc: Tony Lindgren
Signed-off-by: Nishanth Menon
---
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
[...]
--
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: [PM-OPP][PATCH 2/2] omap3: opp: make independent of cpufreq
>>-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
>>Cc: Kevin Hilman
>>Cc: Paul Walmsley
>>Cc: Rajendra Nayak
>>Cc: Sanjeev Premi
>>Cc: Thara Gopinath
>>Cc: Tony Lindgren
>>
>>Signed-off-by: Nishanth Menon
>>---
>>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
--
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
