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

[Vishwanath Sripathy <vishwanath.bs@ti.com>]

> -----Original Message-----
> From: Menon, Nishanth [mailto:nm@ti.com]
> Sent: Wednesday, February 23, 2011 1:48 PM
> To: Vishwanath Sripathy
> Cc: linux-omap; Tony Lindgren; Kevin Hilman
> Subject: Re: [PATCH 03/19] omap3+: voltage: remove initial voltage
>
>  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.
I never said to use random numbers. All I suggested was that instead of
relying on some others function's behavior, read the VP register to fill
in the right values. If set_init_voltage succeeds, it will anyway update
with right voltage.

Vishwa
>
> 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