RE: [RFC][PATCH 2/3] OMAP PM: Add support for bypassing VP/VC in Voltage layer

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

> -----Original Message-----
> From: Menon, Nishanth [mailto:nm@ti.com]
> Sent: Tuesday, March 15, 2011 8:46 PM
> To: Vishwanath BS
> Cc: linux-omap@vger.kernel.org
> Subject: Re: [RFC][PATCH 2/3] OMAP PM: Add support for bypassing
> VP/VC in Voltage layer
>
> On Tue, Mar 15, 2011 at 20:03, Vishwanath BS
> <vishwanath.bs@ti.com> wrote:
> > Currently Voltage layer assumes that all PMICs use VP/VC for Voltage
> scaling.
> > There can be some instances where PMIC would want to bypass VP/VC
> for voltage
> > scaling. So make VOltage layer flexible enough to handle this.
> please fix VOltage.
>
> The commit message does'nt actually explain enough. it is too vague.
>
> >
> > Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
> > ---
> >  arch/arm/mach-omap2/omap_twl.c |    5 +++++
> >  arch/arm/mach-omap2/voltage.c  |   12 +++++++-----
> >  arch/arm/mach-omap2/voltage.h  |    6 ++++++
> >  3 files changed, 18 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/omap_twl.c b/arch/arm/mach-
> omap2/omap_twl.c
> > index f96d4b2..42f66c6 100644
> > --- a/arch/arm/mach-omap2/omap_twl.c
> > +++ b/arch/arm/mach-omap2/omap_twl.c
> > @@ -157,6 +157,7 @@ static struct omap_volt_pmic_info
> omap3_mpu_volt_info = {
> >        .onlp_cmd               = twl4030_uv_to_vsel,
> >        .ret_cmd                = twl4030_uv_to_vsel,
> >        .off_cmd                = twl4030_uv_to_vsel,
> > +       .voltage_class  = VP_VC_CLASS,
>
> Do you need this all unassigned params in a static struct is 0 which
> is basically VP_VC_CLASS.
I did not get this. You do not need to fill all these parameters if you
are not using VP/VC.
>
> >  };
> >
> >  static struct omap_volt_pmic_info omap3_core_volt_info = {
> > @@ -176,6 +177,7 @@ static struct omap_volt_pmic_info
> omap3_core_volt_info = {
> >        .onlp_cmd               = twl4030_uv_to_vsel,
> >        .ret_cmd                = twl4030_uv_to_vsel,
> >        .off_cmd                = twl4030_uv_to_vsel,
> > +       .voltage_class  = VP_VC_CLASS,
> here as well
>
> >  };
> >
> >  static struct omap_volt_pmic_info omap4_mpu_volt_info = {
> > @@ -195,6 +197,7 @@ static struct omap_volt_pmic_info
> omap4_mpu_volt_info = {
> >        .onlp_cmd               = twl4030_uv_to_vsel,
> >        .ret_cmd                = twl4030_uv_to_vsel,
> >        .off_cmd                = twl4030_uv_to_vsel,
> > +       .voltage_class  = VP_VC_CLASS,
> again
>
> >  };
> >
> >  static struct omap_volt_pmic_info omap4_iva_volt_info = {
> > @@ -214,6 +217,7 @@ static struct omap_volt_pmic_info
> omap4_iva_volt_info = {
> >        .onlp_cmd               = twl4030_uv_to_vsel,
> >        .ret_cmd                = twl4030_uv_to_vsel,
> >        .off_cmd                = twl4030_uv_to_vsel,
> > +       .voltage_class  = VP_VC_CLASS,
> and again
>
> >  };
> >
> >  static struct omap_volt_pmic_info omap4_core_volt_info = {
> > @@ -233,6 +237,7 @@ static struct omap_volt_pmic_info
> omap4_core_volt_info = {
> >        .onlp_cmd               = twl4030_uv_to_vsel,
> >        .ret_cmd                = twl4030_uv_to_vsel,
> >        .off_cmd                = twl4030_uv_to_vsel,
> > +       .voltage_class  = VP_VC_CLASS,
> and here
> >  };
> >
> >  int __init omap4_twl_init(void)
> > diff --git a/arch/arm/mach-omap2/voltage.c b/arch/arm/mach-
> omap2/voltage.c
> > index 2ac990f..73fa3ee 100644
> > --- a/arch/arm/mach-omap2/voltage.c
> > +++ b/arch/arm/mach-omap2/voltage.c
> > @@ -1194,11 +1194,13 @@ int __init omap_voltage_late_init(void)
> >                pr_err("%s: Unable to create voltage debugfs main
dir\n",
> >                        __func__);
> >        for (i = 0; i < nr_scalable_vdd; i++) {
> > -               if (omap_vdd_data_configure(vdd_info[i]))
> > -                       continue;
> > -               omap_vc_init(vdd_info[i]);
> > -               vp_init(vdd_info[i]);
> > -               vdd_debugfs_init(vdd_info[i]);
> > +               if (vdd_info[i]->pmic_info->voltage_class ==
> VP_VC_CLASS) {
> > +                       if (omap_vdd_data_configure(vdd_info[i]))
> > +                               continue;
> > +                       omap_vc_init(vdd_info[i]);
> > +                       vp_init(vdd_info[i]);
> > +                       vdd_debugfs_init(vdd_info[i]);
> > +               }
>
> >        }
> >
> >        return 0;
> > diff --git a/arch/arm/mach-omap2/voltage.h b/arch/arm/mach-
> omap2/voltage.h
> > index de4e09b..17b1e10
> > --- a/arch/arm/mach-omap2/voltage.h
> > +++ b/arch/arm/mach-omap2/voltage.h
> > @@ -23,6 +23,9 @@
> >  #define VOLTSCALE_VPFORCEUPDATE                1
> >  #define VOLTSCALE_VCBYPASS             2
> >
> > +#define VP_VC_CLASS 0
> > +#define REGULATOR_CLASS 1
> please - could you ensure namespace is proper in macro usage?
> I dont understand usage difference between VP_VC_CLASS and
> REGULATOR_CLASS
Use regulator class if you do not want to use OMAP Voltage layer.
>
> > +
> >  /**
> >  * struct omap_vfsm_instance_data - per-voltage manager FSM
> register/bitfield
> >  * data
> > @@ -87,6 +90,8 @@ struct omap_volt_data {
> >  * @onlp_cmd:  PMIC API to send onlp command instruction
> >  * @ret_cmd:   PMIC API to send ret command instruction
> >  * @off_cmd:   PMIC API to send off command instruction
> > + * @voltage_class: Voltage class used for voltage scaling (can be
> VP/VC method
> > + *                             or regulator based method for now).
> >  */
> >  struct omap_volt_pmic_info {
> >        int slew_rate;
> > @@ -105,6 +110,7 @@ struct omap_volt_pmic_info {
> >        unsigned char (*onlp_cmd)(unsigned long uV);
> >        unsigned char (*ret_cmd)(unsigned long uV);
> >        unsigned char (*off_cmd)(unsigned long uV);
> > +       u8 voltage_class;
> >  };
> >
> >  /**
> > --
> > 1.7.0.4
> overall, other than deciding to initialize or not initialize VP/VC
> based on class, I dont see how you have achieved $subject or $commit
> message in terms of bring in bypass ability to VP/VC by introducing
> class.
>
> A) the $subject is wrong. you have just introduced voltage_class - I
> suggest you state that instead.
> B) how your patch achieves support for bypassing vp/vc is not clear
> unfortunately.
I am not sure if you have really understood the patch.
The issue I am trying to address is, if someone wants to boot up OMAP +
some PMIC which does not use VP/VC for voltage scaling, how to achieve
that.

What do you mean by bypass ability?
If Voltage layer initialization is skipped (that's the case for regulator
class), then all subsequent voltage_scale calls will fail which is the
expected behavior.

Vishwa

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