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.
> };
>
> 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
> +
> /**
> * 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.
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
