> -----Original Message-----
> From: Menon, Nishanth
> Sent: Tuesday, November 24, 2009 7:09 AM
> To: Premi, Sanjeev
> Cc: linux-omap@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH] omap3: sr: Update ON voltage levels based on VSEL
>
> Premi, Sanjeev had written, on 11/23/2009 07:38 AM, the following:
> > The settings for ON voltage level in the registers
> > PRM_VC_CMD_VAL_0,1 was not updated based on OPP
> > table.
> >
> > Once the MPU resumes from OFF mode (during idle OR
> > suspend) the VDD1 voltage always returns to 1.2V.
> >
> > This patch programs the value based on current OPP.
>
> thanks for flagging this.
>
> >
> > Signed-off-by: Sanjeev Premi <premi@xxxxxx>
> > ---
> > arch/arm/mach-omap2/smartreflex.c | 19 ++++++++++++++++++-
> > 1 files changed, 18 insertions(+), 1 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/smartreflex.c
> b/arch/arm/mach-omap2/smartreflex.c
> > index be3a1da..4cbbd6f 100644
> > --- a/arch/arm/mach-omap2/smartreflex.c
> > +++ b/arch/arm/mach-omap2/smartreflex.c
> > @@ -313,6 +313,15 @@ static void sr_configure_vp(int srid)
> > PRM_VP1_CONFIG_TIMEOUTEN |
> > vsel << OMAP3430_INITVOLTAGE_SHIFT;
> >
> > + /*
> > + * Update the 'ON' voltage levels based on the VSEL.
> > + * (See spruf8c.pdf sec 1.5.3.1)
> > + */
> > + prm_rmw_mod_reg_bits(OMAP3430_VC_CMD_ON_MASK,
> > + (vsel << OMAP3430_VC_CMD_ON_SHIFT),
> > + OMAP3430_GR_MOD,
> > + OMAP3_PRM_VC_CMD_VAL_0_OFFSET);
> > +
> > prm_write_mod_reg(vpconfig, OMAP3430_GR_MOD,
> > OMAP3_PRM_VP1_CONFIG_OFFSET);
> > prm_write_mod_reg(PRM_VP1_VSTEPMIN_SMPSWAITTIMEMIN |
> > @@ -354,6 +363,15 @@ static void sr_configure_vp(int srid)
> > else
> > vsel = l3_opps[target_opp_no].vsel;
> >
> > + /*
> > + * Update the 'ON' voltage levels based on the VSEL.
> > + * (See spruf8c.pdf sec 1.5.3.1)
> > + */
> > + prm_rmw_mod_reg_bits(OMAP3430_VC_CMD_ON_MASK,
> > + (vsel << OMAP3430_VC_CMD_ON_SHIFT),
> > + OMAP3430_GR_MOD,
> > + OMAP3_PRM_VC_CMD_VAL_1_OFFSET);
> > +
> > vpconfig = PRM_VP2_CONFIG_ERROROFFSET |
> > PRM_VP2_CONFIG_ERRORGAIN |
> > PRM_VP2_CONFIG_TIMEOUTEN |
> > @@ -391,7 +409,6 @@ static void sr_configure_vp(int srid)
> > /* Clear force bit */
> > prm_clear_mod_reg_bits(OMAP3430_FORCEUPDATE,
> OMAP3430_GR_MOD,
> > OMAP3_PRM_VP2_CONFIG_OFFSET);
> > -
> > }
> > }
> >
>
> CMD_VAL_[01] contains:
> OFF, RET, ONLowPower, ON voltages. for VDD1 and VDD2
>
> ON voltage:
> Here is what I see:
>
> sr_voltagescale_vcbypass sets the voltage for ON mode. BUT, this is
> called as part of voltage scale logic (from
> resource34xx.c::program_opp)
> ==> invoked only as part of DVFS transitions.
>
> The trouble we have is this: IF OFF mode is hit without DVFS
> transition,
> we call disable_smartreflex, which in turn calls sr_reset_voltage().
> Unfortunately, this function is a wannabe sr_voltagescale_bypass, but
> does not do what sr_voltagebypass in it's entirety -> critical one
> being: setting ON voltage based on state it is going into.
Yes. One thing I found was that many functions have a bit of overlapping
functionality - possibly complete in completely differen respect - but
doesn't help flow.
> (ideally, it
> should set the RET/ON voltage based on the state it is going
> into - but
> that'd be an optimization thing)..
>
> Now, without your patch, the OPP voltage is based on nominal OPP at
> which the system boots up on and CMD_VAL_0::ON voltage is not
> set when
> disable_smartreflex::reset_voltage is called. ON voltage is
> some value
> someone has set in the reg.
Actually, without DVFS the mpurate argument is useful in setting the
desired OPP. This functionality was fixed few months back. Hence, the
patch is able to set correct value in this patch.
>
> For this, we come to the interesting part, we should now also look at
> arch/arm/mach-omap2/pm34xx.c::configure_vc function - which
> is another
> function who hits the ON ret and OFF voltage values based on
> prm_setup -
> this is where your bug comes from: defaults:
> 92 .vdd0_on = 0x30, /* 1.2v */
Initially, I did go the board specific way (to test the implementation);
but then the OPPs and the voltages aren't board specific. So, had to
find a good point where only one table is used.
Moving further as the new OPP layer shapes up, I believe we need
simple SR APIs:
e.g. (just some pseudo code)
#define VDD1 1
#define VDD2 2
#define VDD3 3 /* may be in future */
#define VC_VOLTS_OFF 0
#define VC_VOLTS_RET 1
#define VC_VOLTS_ON 2
#define VC_OFF_CLEAR_MASK ...
#define VC_RET_CLEAR_MASK ...
#define VC_ON_CLEAR_MASK ...
#define VC_OFF_SHIFT ...
#define VC_RET_SHIFT ...
#define VC_ON_SHIFT ...
#define V1000 1000 /* 1.00 V */
#define V1100 1100 /* 1.10 V */
#define V1200 1200 /* 1.20 V */
int vc_voltage_set(u8 domain, u8 cond, u16 volts)
{
int ret;
u32 reg, val;
u32 vcoeff;
if (domain == VDD1)
reg = OMAP3_PRM_VC_CMD_VAL_0_OFFSET;
else if (domain == VDD2)
reg = OMAP3_PRM_VC_CMD_VAL_1_OFFSET;
else
return -1 ;
/* add print as well */
/* Convert voltage to a 'coeff' based on the current OPP */
/* coeff is a generic equiv of 'vsel' in OPP table */
if (volts_to_coeff (volts, &vcoeff)) {
val = read_register (reg);
if (cond == VC_VOLTS_OFF) {
val &= VC_OFF_CLEAR_MASK;
val |= (coeff << VC_OFF_SHIFT);
write_register(reg, val);
}
/* and so on */
} else {
return -1 ;
}
return 0;
}
>
> But voila, this can be updated by omap3_pm_init_vc - e.g. see
> arch/arm/mach-omap2/board-3430sdp.c
>
> You have few options here to proceed:
> a) Modify the board file to update the ON/OFF/RET voltages you'd like
> for your board.
> b) Fix configure_vc to take boot-up OPP voltage.
> c) Given the current state of code, but I think we dont handle
> disable_smartreflex correctly @ various OPPs and states. I do
> agree that
> we badly need to clean this path up. While I work on that, could I
> request this logic be moved across to sr_resetvoltage instead as a
> temporary standin for the immediate bug and others not using
> pm_init_vc
> function from hitting this bug?
I am not looking at code currently - away from the git tree. Will take
a look at it.
~sanjeev
>
> I think (a) might be a quicker solution for you. Do let me know if I
> missed something.
>
> --
> 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
