Wed, Feb 23, 2011 at 12:24, Vishwanath Sripathy <vishwanath.bs@xxxxxx> wrote: >> -----Original Message----- >> From: Nishanth Menon [mailto:nm@xxxxxx] >> 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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
