Re: [PATCH 03/19] omap3+: voltage: remove initial voltage

Menon, Nishanth
2011-02-23T08:18:24Z

Wed, Feb 23, 2011 at 12:24, Vishwanath Sripathy <vishwanath.bs@ti.com> wrote:
>> -----Original Message-----
>> From: Nishanth Menon [mailto:nm@ti.com]
>> Sent: Sunday, February 20, 2011 10:42 AM
>> To: Vishwanath Sripathy
>> Cc: linux-omap; Tony Lindgren; Kevin Hilman
>> Subject: Re: [PATCH 03/19] omap3+: voltage: remove initial voltage
>>
>> Vishwanath Sripathy wrote, on 02/19/2011 06:54 PM:
>> [..]
>> >> omap2_set_init_voltage should setup the curr_volt based on which
>> OPP
>> >> the system is functioning at. Blindly setting a 1.2v setting in the
>> >> initial structure may not even match the default voltages stored in
>> >> the voltage table which are supported for the domain.
>> >>
>> >> For example, OMAP3430 core domain does not use 1.2v and ends up
>> >> generating a warning on the first transition.
>> >>
>> [..]
>> >> diff --git a/arch/arm/mach-omap2/voltage.c b/arch/arm/mach-
>> >> omap2/voltage.c
>> >> index 12be525..280ee12 100644
>> >> --- a/arch/arm/mach-omap2/voltage.c
>> >> +++ b/arch/arm/mach-omap2/voltage.c
>> [..]
>> >>    /* Generic voltage parameters */
>> >> -  vdd->curr_volt = 1200000;
>> > Where do you update this parameter upon initialization? Shouldn't you
>> read
>> > the VP register and find the actual current voltage and update this
>> param?
>>
>> The sequence is as follows:
>> a) omapx_vdd_data configure is called as part of sr init sequence.
>>       And the curr_volt with this patch is not updated at this stage.
>> b) somewhere down in the boot sequence, pm.c's
>> omap2_set_init_voltage
>> starts up. This looks up the current clk frequency from clock layer of
>> the parent device for the domain, picks up the nominal voltages stored
>> in the opp layer, then does a omap_voltage_scale_vdd to that voltage. In
>> omap_voltage_scale_vdd, The current voltage is merely picked off the vp
>> (in _pre_volt_scale). the last step it does is to setup vdd->curr_volt.
>> This can then be used by dvfs layer etc to make appropriate decisions.
>>
>> So, No, I dont think I need to update it here, it should happen as part
>> of the pm init sequence.
>>
>> Could you explain what problem do you foresee by doing this?
> What if omap_voltage_scale_vdd fails when called from
> omap2_set_init_voltage? Or omap2_set_init_voltage returns error even
> before calling omap_voltage_scale_vdd because it could not find the
> matching voltage for the frequency set by bootloader?
>
> Your assumption that curr_volt is always set by omap2_set_init_voltage
> need not be true always.

set_init_voltage's job is to set the initial voltage. if it is
incapable of doing it, I say fix that instead of hacking in some
random number as curr_volt.

The logic involved for set_init_voltage is what we need for filling in
cur_volt. just replicating the logic makes no sense to me.

Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html