Re: [PATCH] omap3: pm: cpufreq: populate l3 opp1 again

[Kevin Hilman <khilman@deeprootsystems.com>]

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