> -----Original Message----- > From: Menon, Nishanth [mailto:nm@xxxxxx] > Sent: Tuesday, March 15, 2011 8:46 PM > To: Vishwanath BS > Cc: linux-omap@xxxxxxxxxxxxxxx > 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@xxxxxx> 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@xxxxxx> > > --- > > 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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
