* [PATCH] omap3: pm: cpufreq: populate l3 opp1 again
@ 2010-01-28 5:41 Nishanth Menon
2010-01-28 5:42 ` Nishanth Menon
2010-01-28 6:55 ` Romit Dasgupta
0 siblings, 2 replies; 9+ messages in thread
From: Nishanth Menon @ 2010-01-28 5:41 UTC (permalink / raw)
To: linux-omap; +Cc: Nishanth Menon, Andrew Murray, Benoit Cousson, Kevin Hilman
We had removed the frequency for OPP1 L3 when we used to use frequency
to enable/disable frequencies. It is better to populate the same
instead of confusing future readers of the code. The OPP1 remains
disabled as explained in the discussion.
Discussion: http://marc.info/?t=126453821900001&r=1&w=2
Cc: Andrew Murray <amurray@mpc-data.co.uk>
Cc: Benoit Cousson <b-cousson@ti.com>
Cc: Kevin Hilman <khilman@deeprootsystems.com>
Signed-off-by: Nishanth Menon <nm@ti.com>
---
arch/arm/mach-omap2/cpufreq34xx.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-omap2/cpufreq34xx.c b/arch/arm/mach-omap2/cpufreq34xx.c
index 209af2b..aec1ffe 100644
--- a/arch/arm/mach-omap2/cpufreq34xx.c
+++ b/arch/arm/mach-omap2/cpufreq34xx.c
@@ -43,7 +43,7 @@ static struct omap_opp_def __initdata omap34xx_mpu_rate_table[] = {
static struct omap_opp_def __initdata omap34xx_l3_rate_table[] = {
/* OPP1 */
- OMAP_OPP_DEF(false, 0, 975000),
+ OMAP_OPP_DEF(false, 41500000, 975000),
/* OPP2 */
OMAP_OPP_DEF(true, 83000000, 1050000),
/* OPP3 */
--
1.6.3.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH] omap3: pm: cpufreq: populate l3 opp1 again
2010-01-28 5:41 [PATCH] omap3: pm: cpufreq: populate l3 opp1 again Nishanth Menon
@ 2010-01-28 5:42 ` Nishanth Menon
2010-01-28 6:55 ` Romit Dasgupta
1 sibling, 0 replies; 9+ messages in thread
From: Nishanth Menon @ 2010-01-28 5:42 UTC (permalink / raw)
To: linux-omap; +Cc: Andrew Murray, Cousson, Benoit, Kevin Hilman
Menon, Nishanth had written, on 01/27/2010 11:41 PM, the following:
> We had removed the frequency for OPP1 L3 when we used to use frequency
> to enable/disable frequencies. It is better to populate the same
> instead of confusing future readers of the code. The OPP1 remains
> disabled as explained in the discussion.
>
> Discussion: http://marc.info/?t=126453821900001&r=1&w=2
Clarification: This is for pm-wip-opp branch. apologies on the spam :(
--
Regards,
Nishanth Menon
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] omap3: pm: cpufreq: populate l3 opp1 again
2010-01-28 5:41 [PATCH] omap3: pm: cpufreq: populate l3 opp1 again Nishanth Menon
2010-01-28 5:42 ` Nishanth Menon
@ 2010-01-28 6:55 ` Romit Dasgupta
2010-01-28 13:24 ` Cousson, Benoit
1 sibling, 1 reply; 9+ messages in thread
From: Romit Dasgupta @ 2010-01-28 6:55 UTC (permalink / raw)
To: Menon, Nishanth; +Cc: linux-omap, Andrew Murray, Cousson, Benoit, Kevin Hilman
Menon, Nishanth wrote:
> We had removed the frequency for OPP1 L3 when we used to use frequency
> to enable/disable frequencies. It is better to populate the same
> instead of confusing future readers of the code. The OPP1 remains
> disabled as explained in the discussion.
IMHO the best way to unconfuse readers is to remove the entry.
>
> Discussion: http://marc.info/?t=126453821900001&r=1&w=2
>
> Cc: Andrew Murray <amurray@mpc-data.co.uk>
> Cc: Benoit Cousson <b-cousson@ti.com>
> Cc: Kevin Hilman <khilman@deeprootsystems.com>
>
> Signed-off-by: Nishanth Menon <nm@ti.com>
> ---
> arch/arm/mach-omap2/cpufreq34xx.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/cpufreq34xx.c b/arch/arm/mach-omap2/cpufreq34xx.c
> index 209af2b..aec1ffe 100644
> --- a/arch/arm/mach-omap2/cpufreq34xx.c
> +++ b/arch/arm/mach-omap2/cpufreq34xx.c
> @@ -43,7 +43,7 @@ static struct omap_opp_def __initdata omap34xx_mpu_rate_table[] = {
>
> static struct omap_opp_def __initdata omap34xx_l3_rate_table[] = {
> /* OPP1 */
> - OMAP_OPP_DEF(false, 0, 975000),
> + OMAP_OPP_DEF(false, 41500000, 975000),
> /* OPP2 */
> OMAP_OPP_DEF(true, 83000000, 1050000),
> /* OPP3 */
^ permalink raw reply [flat|nested] 9+ messages in thread* RE: [PATCH] omap3: pm: cpufreq: populate l3 opp1 again
2010-01-28 6:55 ` Romit Dasgupta
@ 2010-01-28 13:24 ` Cousson, Benoit
2010-01-28 13:43 ` Dasgupta, Romit
0 siblings, 1 reply; 9+ messages in thread
From: Cousson, Benoit @ 2010-01-28 13:24 UTC (permalink / raw)
To: Dasgupta, Romit, Menon, Nishanth; +Cc: linux-omap, Andrew Murray, Kevin Hilman
Hi Nishanth,
>From: Dasgupta, Romit
>
>Menon, Nishanth wrote:
>> We had removed the frequency for OPP1 L3 when we used to use frequency
>> to enable/disable frequencies. It is better to populate the same
>> instead of confusing future readers of the code. The OPP1 remains
>> disabled as explained in the discussion.
>
>IMHO the best way to unconfuse readers is to remove the entry.
The confusion is due to a mismatch between public TRM/white paper and current code, so maybe a simple comment on top of the omap34xx_l3_rate_table will be enough.
Regards,
Benoit
>>
>> Discussion: http://marc.info/?t=126453821900001&r=1&w=2
>>
>> Cc: Andrew Murray <amurray@mpc-data.co.uk>
>> Cc: Benoit Cousson <b-cousson@ti.com>
>> Cc: Kevin Hilman <khilman@deeprootsystems.com>
>>
>> Signed-off-by: Nishanth Menon <nm@ti.com>
>> ---
>> arch/arm/mach-omap2/cpufreq34xx.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/cpufreq34xx.c b/arch/arm/mach-
>omap2/cpufreq34xx.c
>> index 209af2b..aec1ffe 100644
>> --- a/arch/arm/mach-omap2/cpufreq34xx.c
>> +++ b/arch/arm/mach-omap2/cpufreq34xx.c
>> @@ -43,7 +43,7 @@ static struct omap_opp_def __initdata
>omap34xx_mpu_rate_table[] = {
>>
>> static struct omap_opp_def __initdata omap34xx_l3_rate_table[] = {
>> /* OPP1 */
>> - OMAP_OPP_DEF(false, 0, 975000),
>> + OMAP_OPP_DEF(false, 41500000, 975000),
>> /* OPP2 */
>> OMAP_OPP_DEF(true, 83000000, 1050000),
>> /* OPP3 */
Texas Instruments France SA, 821 Avenue Jack Kilby, 06270 Villeneuve Loubet. 036 420 040 R.C.S Antibes. Capital de EUR 753.920
^ permalink raw reply [flat|nested] 9+ messages in thread* RE: [PATCH] omap3: pm: cpufreq: populate l3 opp1 again
2010-01-28 13:24 ` Cousson, Benoit
@ 2010-01-28 13:43 ` Dasgupta, Romit
2010-01-28 13:47 ` Cousson, Benoit
0 siblings, 1 reply; 9+ messages in thread
From: Dasgupta, Romit @ 2010-01-28 13:43 UTC (permalink / raw)
To: Cousson, Benoit, Menon, Nishanth; +Cc: linux-omap, Andrew Murray, Kevin Hilman
> >IMHO the best way to unconfuse readers is to remove the entry.
>
> The confusion is due to a mismatch between public TRM/white paper and
> current code, so maybe a simple comment on top of the
> omap34xx_l3_rate_table will be enough.
>
But why do we want to keep an OPP that will never be used?
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH] omap3: pm: cpufreq: populate l3 opp1 again
2010-01-28 13:43 ` Dasgupta, Romit
@ 2010-01-28 13:47 ` Cousson, Benoit
2010-01-28 14:00 ` Andrew Murray
0 siblings, 1 reply; 9+ messages in thread
From: Cousson, Benoit @ 2010-01-28 13:47 UTC (permalink / raw)
To: Dasgupta, Romit, Menon, Nishanth; +Cc: linux-omap, Andrew Murray, Kevin Hilman
>From: Dasgupta, Romit
>Sent: Thursday, January 28, 2010 2:43 PM
>To: Cousson, Benoit; Menon, Nishanth
>Cc: linux-omap; Andrew Murray; Kevin Hilman
>Subject: RE: [PATCH] omap3: pm: cpufreq: populate l3 opp1 again
>
>> >IMHO the best way to unconfuse readers is to remove the entry.
>>
>> The confusion is due to a mismatch between public TRM/white paper and
>> current code, so maybe a simple comment on top of the
>> omap34xx_l3_rate_table will be enough.
>>
>But why do we want to keep an OPP that will never be used?
I don't want to keep it. I just want to document it in order to explain why the code is not aligned with the public doc.
Andrew did ask the question so the answer might be useful for others as well, hence a small comment on top of the CORE OPP list.
Benoit
Texas Instruments France SA, 821 Avenue Jack Kilby, 06270 Villeneuve Loubet. 036 420 040 R.C.S Antibes. Capital de EUR 753.920
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH] omap3: pm: cpufreq: populate l3 opp1 again
2010-01-28 13:47 ` Cousson, Benoit
@ 2010-01-28 14:00 ` Andrew Murray
2010-01-28 15:18 ` Nishanth Menon
0 siblings, 1 reply; 9+ messages in thread
From: Andrew Murray @ 2010-01-28 14:00 UTC (permalink / raw)
To: Cousson, Benoit, Dasgupta, Romit, Menon, Nishanth
Cc: linux-omap, Kevin Hilman
> From: Cousson, Benoit [mailto:b-cousson@ti.com]
> Sent: 28 January 2010 13:48
> To: Dasgupta, Romit; Menon, Nishanth
> Cc: linux-omap; Andrew Murray; Kevin Hilman
> Subject: RE: [PATCH] omap3: pm: cpufreq: populate l3 opp1 again
> I don't want to keep it. I just want to document it in order to
explain
> why the code is not aligned with the public doc.
>
> Andrew did ask the question so the answer might be useful for others
as
> well, hence a small comment on top of the CORE OPP list.
>
> Benoit
I agree.
I don't think it matters if the OPP 'OMAP_OPP_DEF' is there or not.
Though as the OPPs differ to the hardware descriptions it should be
documented - perhaps as suggested, by a comment.
It is confusing when validating the power management software support -
and the best place to look for the rationale is in the code.
Andrew Murray
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] omap3: pm: cpufreq: populate l3 opp1 again
2010-01-28 14:00 ` Andrew Murray
@ 2010-01-28 15:18 ` Nishanth Menon
2010-01-28 17:23 ` Kevin Hilman
0 siblings, 1 reply; 9+ messages in thread
From: Nishanth Menon @ 2010-01-28 15:18 UTC (permalink / raw)
To: Andrew Murray; +Cc: Cousson, Benoit, Dasgupta, Romit, linux-omap, Kevin Hilman
Hi,
Thanks for all the comments.
Andrew Murray had written, on 01/28/2010 08:00 AM, the following:
>> From: Cousson, Benoit [mailto:b-cousson@ti.com]
>> Sent: 28 January 2010 13:48
>> To: Dasgupta, Romit; Menon, Nishanth
>> Cc: linux-omap; Andrew Murray; Kevin Hilman
>> Subject: RE: [PATCH] omap3: pm: cpufreq: populate l3 opp1 again
>
>> I don't want to keep it. I just want to document it in order to
> explain
>> why the code is not aligned with the public doc.
>>
>> Andrew did ask the question so the answer might be useful for others
> as
>> well, hence a small comment on top of the CORE OPP list.
>>
>> Benoit
>
> I agree.
>
> I don't think it matters if the OPP 'OMAP_OPP_DEF' is there or not.
> Though as the OPPs differ to the hardware descriptions it should be
> documented - perhaps as suggested, by a comment.
>
> It is confusing when validating the power management software support -
> and the best place to look for the rationale is in the code.
>
> Andrew Murray
I did consider removing it, till i did a git grep...
> $ git grep VDD2_OPP
> arch/arm/mach-omap2/pm34xx.c: resource_lock_opp(VDD2_OPP);
> arch/arm/mach-omap2/pm34xx.c: resource_unlock_opp(VDD2_OPP);
> arch/arm/mach-omap2/resource34xx.c: } else if (res == VDD2_OPP) {
> arch/arm/mach-omap2/resource34xx.c: else if (res == VDD2_OPP)
> arch/arm/mach-omap2/resource34xx.c: ret = resource_set_opp_level(VDD2_OPP, target_level, 0);
> arch/arm/mach-omap2/smartreflex.c: target_opp_no = VDD2_OPP3;
Grrrr... SR is going to break if I were to do that OR my patch can
renumber them.
> arch/arm/mach-omap2/smartreflex.c: } else if (vdd == VDD2_OPP) {
> arch/arm/mach-omap2/smartreflex.c: else if (vdd == VDD2_OPP)
> arch/arm/mach-omap2/smartreflex.h:#define PRCM_VDD2_OPP1 (OMAP(AT_3430_ES2) | OTHER_ID_TYPE(ID_OPP) | \
> arch/arm/mach-omap2/smartreflex.h:#define PRCM_VDD2_OPP2 (OMAP(AT_3430_ES2) | OTHER_ID_TYPE(ID_OPP) | \
> arch/arm/mach-omap2/smartreflex.h:#define PRCM_VDD2_OPP3 (OMAP(AT_3430_ES2) | OTHER_ID_TYPE(ID_OPP) | \
> arch/arm/mach-omap2/smartreflex.h:#define PRCM_NO_VDD2_OPPS 3
> arch/arm/plat-omap/include/plat/omap34xx.h:#define VDD2_OPP 0x2
> arch/arm/plat-omap/include/plat/omap34xx.h:#define VDD2_OPP1 0x1
> arch/arm/plat-omap/include/plat/omap34xx.h:#define VDD2_OPP2 0x2
> arch/arm/plat-omap/include/plat/omap34xx.h:#define VDD2_OPP3 0x3
All these should go away as well.. I missed these in my pm-wip-opp
patchset..
Do we want to do this now(DISCLAIMER: I am not volunteering{yet} ;) )?
OR do we want to wait till the current planned SR/SRF cleanups are complete?
--
Regards,
Nishanth Menon
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] omap3: pm: cpufreq: populate l3 opp1 again
2010-01-28 15:18 ` Nishanth Menon
@ 2010-01-28 17:23 ` Kevin Hilman
0 siblings, 0 replies; 9+ messages in thread
From: Kevin Hilman @ 2010-01-28 17:23 UTC (permalink / raw)
To: Nishanth Menon
Cc: Andrew Murray, Cousson, Benoit, Dasgupta, Romit, linux-omap
Nishanth Menon <nm@ti.com> writes:
> Hi,
> Thanks for all the comments.
> Andrew Murray had written, on 01/28/2010 08:00 AM, the following:
>>> From: Cousson, Benoit [mailto:b-cousson@ti.com]
>>> Sent: 28 January 2010 13:48
>>> To: Dasgupta, Romit; Menon, Nishanth
>>> Cc: linux-omap; Andrew Murray; Kevin Hilman
>>> Subject: RE: [PATCH] omap3: pm: cpufreq: populate l3 opp1 again
>>
>>> I don't want to keep it. I just want to document it in order to
>> explain
>>> why the code is not aligned with the public doc.
>>>
>>> Andrew did ask the question so the answer might be useful for others
>> as
>>> well, hence a small comment on top of the CORE OPP list.
>>>
>>> Benoit
>>
>> I agree.
>>
>> I don't think it matters if the OPP 'OMAP_OPP_DEF' is there or not.
>> Though as the OPPs differ to the hardware descriptions it should be
>> documented - perhaps as suggested, by a comment.
>>
>> It is confusing when validating the power management software support -
>> and the best place to look for the rationale is in the code.
>>
>> Andrew Murray
> I did consider removing it, till i did a git grep...
>> $ git grep VDD2_OPP
>> arch/arm/mach-omap2/pm34xx.c: resource_lock_opp(VDD2_OPP);
>> arch/arm/mach-omap2/pm34xx.c: resource_unlock_opp(VDD2_OPP);
>> arch/arm/mach-omap2/resource34xx.c: } else if (res == VDD2_OPP) {
>> arch/arm/mach-omap2/resource34xx.c: else if (res == VDD2_OPP)
>> arch/arm/mach-omap2/resource34xx.c: ret = resource_set_opp_level(VDD2_OPP, target_level, 0);
>> arch/arm/mach-omap2/smartreflex.c: target_opp_no = VDD2_OPP3;
> Grrrr... SR is going to break if I were to do that OR my patch can
> renumber them.
>
>> arch/arm/mach-omap2/smartreflex.c: } else if (vdd == VDD2_OPP) {
>> arch/arm/mach-omap2/smartreflex.c: else if (vdd == VDD2_OPP)
>> arch/arm/mach-omap2/smartreflex.h:#define PRCM_VDD2_OPP1 (OMAP(AT_3430_ES2) | OTHER_ID_TYPE(ID_OPP) | \
>> arch/arm/mach-omap2/smartreflex.h:#define PRCM_VDD2_OPP2 (OMAP(AT_3430_ES2) | OTHER_ID_TYPE(ID_OPP) | \
>> arch/arm/mach-omap2/smartreflex.h:#define PRCM_VDD2_OPP3 (OMAP(AT_3430_ES2) | OTHER_ID_TYPE(ID_OPP) | \
>> arch/arm/mach-omap2/smartreflex.h:#define PRCM_NO_VDD2_OPPS 3
>> arch/arm/plat-omap/include/plat/omap34xx.h:#define VDD2_OPP 0x2
>> arch/arm/plat-omap/include/plat/omap34xx.h:#define VDD2_OPP1 0x1
>> arch/arm/plat-omap/include/plat/omap34xx.h:#define VDD2_OPP2 0x2
>> arch/arm/plat-omap/include/plat/omap34xx.h:#define VDD2_OPP3 0x3
> All these should go away as well.. I missed these in my pm-wip-opp
> patchset..
>
> Do we want to do this now(DISCLAIMER: I am not volunteering{yet} ;) )?
> OR do we want to wait till the current planned SR/SRF cleanups are
> complete?
I suggest that for now we just add the value back as your original patch,
and add a comment as suggested by Benoit as well. After the SR rework
is done, we can remove the OPP and leave the comment.
Kevin
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2010-01-28 17:23 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-01-28 5:41 [PATCH] omap3: pm: cpufreq: populate l3 opp1 again Nishanth Menon
2010-01-28 5:42 ` Nishanth Menon
2010-01-28 6:55 ` Romit Dasgupta
2010-01-28 13:24 ` Cousson, Benoit
2010-01-28 13:43 ` Dasgupta, Romit
2010-01-28 13:47 ` Cousson, Benoit
2010-01-28 14:00 ` Andrew Murray
2010-01-28 15:18 ` Nishanth Menon
2010-01-28 17:23 ` Kevin Hilman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox