Re: [PATCHv4 02/15] omap3+: voltage: parameter segregation

[Tero Kristo <t-kristo@ti.com>]

Hi,

Removed part of patch for easier readability of comments.

On Tue, 2011-11-29 at 12:26 -0600, Menon, Nishanth wrote:
> On Fri, Nov 25, 2011 at 09:49, Tero Kristo <t-kristo@ti.com> wrote:
<clip>

> > @@ -227,11 +207,6 @@ static struct omap_voltdm_pmic omap4_iva_pmic = {
> >  static struct omap_voltdm_pmic omap4_core_pmic = {
> >        .slew_rate              = 4000,
> >        .step_size              = 12660,
> > -       .on_volt                = 1200000,
> > -       .onlp_volt              = 1200000,
> > -       .ret_volt               = 830000,
> > -       .off_volt               = 0,
> > -       .volt_setup_time        = 0,
> >        .vp_erroroffset         = OMAP4_VP_CONFIG_ERROROFFSET,
> >        .vp_vstepmin            = OMAP4_VP_VSTEPMIN_VSTEPMIN,
> >        .vp_vstepmax            = OMAP4_VP_VSTEPMAX_VSTEPMAX,
> > diff --git a/arch/arm/mach-omap2/opp3xxx_data.c b/arch/arm/mach-omap2/opp3xxx_data.c
> > index d95f3f9..1d44df5 100644
> > --- a/arch/arm/mach-omap2/opp3xxx_data.c
> > +++ b/arch/arm/mach-omap2/opp3xxx_data.c
> > @@ -26,6 +26,10 @@
> >  #include "pm.h"
> >
> >  /* 34xx */
> > +#define OMAP3_ON_VOLTAGE_UV            1200000
> > +#define OMAP3_ONLP_VOLTAGE_UV          1000000
> > +#define OMAP3_RET_VOLTAGE_UV           975000
> > +#define OMAP3_OFF_VOLTAGE_UV           600000
> 
> this approach has a problem -> ON, ONLP and RET voltage should consider:
> a) OMAP capabiltiy as above.
> b) PMIC capability which is being removed in this patch
> 
> the framework should use the combination of both to make a decision.

So, I think we should limit these voltages based on the PMIC vddmin /
vddmax values, right? I don't think PMIC has any other limitations, and
we shouldn't define the voltage levels for all of these modes for PMIC.
PMIC limits should also probably be changed from current values (based
on OMAP defines) and changed to actual values, like vddmin = 600mV
(vsel=0), or whatever is the minimum voltage for the corresponding PMIC.

<clip>

> > +};
> > +
> >  #define OMAP4430_VDD_CORE_OPP50_UV             1025000
> >  #define OMAP4430_VDD_CORE_OPP100_UV            1200000
> >
> > @@ -64,6 +93,17 @@ struct omap_volt_data omap44xx_vdd_core_volt_data[] = {
> >        VOLT_DATA_DEFINE(0, 0, 0, 0),
> >  };
> >
> > +struct omap_vp_param omap44xx_core_vp_data = {
> > +       .vddmin                 = OMAP4_VP_CORE_VLIMITTO_VDDMIN,
> > +       .vddmax                 = OMAP4_VP_CORE_VLIMITTO_VDDMAX,
> > +};
> > +
> > +struct omap_vc_param omap44xx_core_vc_data = {
> > +       .on                     = OMAP4_ON_VOLTAGE_UV,
> > +       .onlp                   = OMAP4_ONLP_VOLTAGE_UV,
> > +       .ret                    = OMAP4_RET_VOLTAGE_UV,
> > +       .off                    = OMAP4_OFF_VOLTAGE_UV,
> > +};
> 
> NOTE: we will be reaching all combinations ahead - in time to come
> ahead we will have 4470 as well - linking this to opp data seems wrong
> to me..

They are not really that much linked to opp data, they are just defined
in this file. The actual attach to voltdm is done in
voltagedomainsxxxx_data.c file, where we can link data to whatever
voltagedomain we want to. See for example voltagedomains3xxx_data.c what
is done for omap34xx vs. omap36xx. Should we move this data over there
then...?

-Tero

<clip>