Re: [PATCH V3 06/19] OMAP3+: voltage: use volt_data pointer instead values
Nishanth Menon n...@ti.com writes: Voltage values can get confusing in meaning with various SmartReflex classes being active. Depending on the class used, the actual voltage selected might be a variant. For example: With SmartReflex AVS class 3: a) If we don't have SR enabled, we will go to volt_nominal. b) If have SR enabled, we go to volt_nominal, then enable SR and expect it to adjust voltage. We don't really care about the resultant voltage. Essentially, when we ask voltage layer to scale to voltage x for OPP y, it always means x is the nominal voltage for OPP y. Now, once we introduce SmartReflex AVS class 1.5: a) If you are booting for the first time OR if you never enabled SR before, we always go to volt_nominal. b) If you enable SR and the OPP is calibrated, we will not enable SR again when that OPP is accessed anymore, but when we set voltage for an OPP, the voltage achieved will be volt_calibrated. c) If recalibration timeout triggers or SR is disabled after a calibration, the calibrated values are not valid anymore, at this point, setting the voltage will mean volt_dynamic_nominal. So, depending on which state the system is at, voltage for that OPP we are setting has not 1 single value anymore, but 3 possible valid values. For upper layers(DVFS/cpufreq OMAP SoC layers) to use voltage values, it will need to know which type of voltage AVS strategy is being used and the corresponding system state from voltage layer perspective. This would replicate the role of voltage layer, SmartReflex AVS in the upper layers and make the corresponding implementations complex. Since each voltage domain contains a set of volt_data structs representing a voltage point that is supported for that domain, volt_data is a more accurate representation of the voltage point we are interested in going to, and the actual translation of this voltage point to the voltage value is done inside the voltage layer. Doing this allows the users of the voltage layer to be blissfully ignorant of any complexity of the underneath layers and simplify the implementation of dependent layers. Signed-off-by: Nishanth Menon n...@ti.com Mostly coding style and/or readability comments below... --- arch/arm/mach-omap2/pm.c |3 +- arch/arm/mach-omap2/smartreflex-class3.c |3 +- arch/arm/mach-omap2/voltage.c| 75 +- arch/arm/mach-omap2/voltage.h| 17 +-- 4 files changed, 59 insertions(+), 39 deletions(-) diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c index 2c3a253..2372744 100644 --- a/arch/arm/mach-omap2/pm.c +++ b/arch/arm/mach-omap2/pm.c @@ -209,7 +209,8 @@ static int __init omap2_set_init_voltage(char *vdd_name, char *clk_name, goto exit; } - omap_voltage_scale_vdd(voltdm, bootup_volt); + omap_voltage_scale_vdd(voltdm, + omap_voltage_get_voltdata(voltdm, bootup_volt)); coding style: to avoid wrapping (which IMO affects readabiliyt), assign volt_data to it's own variable then pass it to scale_vdd. return 0; exit: diff --git a/arch/arm/mach-omap2/smartreflex-class3.c b/arch/arm/mach-omap2/smartreflex-class3.c index f438cf4..2ee48af 100644 --- a/arch/arm/mach-omap2/smartreflex-class3.c +++ b/arch/arm/mach-omap2/smartreflex-class3.c @@ -15,7 +15,8 @@ static int sr_class3_enable(struct voltagedomain *voltdm) { - unsigned long volt = omap_voltage_get_nom_volt(voltdm); + unsigned long volt = omap_get_operation_voltage( + omap_voltage_get_nom_volt(voltdm)); readability/wrapping Also, the name of the new function doesn't follow the naming convention of the rest of the voltage layer: e.g. omap_voltage_... if (!volt) { pr_warning(%s: Curr voltage unknown. Cannot enable sr_%s\n, diff --git a/arch/arm/mach-omap2/voltage.c b/arch/arm/mach-omap2/voltage.c index 76bcaee..a12ac1e 100644 --- a/arch/arm/mach-omap2/voltage.c +++ b/arch/arm/mach-omap2/voltage.c @@ -58,7 +58,7 @@ static struct dentry *voltage_dir; /* Init function pointers */ static int vp_forceupdate_scale_voltage(struct omap_vdd_info *vdd, - unsigned long target_volt); + struct omap_volt_data *target_volt); static u32 omap3_voltage_read_reg(u16 mod, u8 offset) { @@ -164,13 +164,20 @@ static int vp_volt_debug_get(void *data, u64 *val) static int nom_volt_debug_get(void *data, u64 *val) { struct omap_vdd_info *vdd = (struct omap_vdd_info *) data; + struct omap_volt_data *volt_data; if (!vdd) { pr_warning(Wrong paramater passed\n); return -EINVAL; } - *val = omap_voltage_get_nom_volt(vdd-voltdm); + volt_data = omap_voltage_get_nom_volt(vdd-voltdm); + if (IS_ERR_OR_NULL(volt_data)) { + pr_warning(%s: No
[PATCH V3 06/19] OMAP3+: voltage: use volt_data pointer instead values
Voltage values can get confusing in meaning with various SmartReflex classes being active. Depending on the class used, the actual voltage selected might be a variant. For example: With SmartReflex AVS class 3: a) If we don't have SR enabled, we will go to volt_nominal. b) If have SR enabled, we go to volt_nominal, then enable SR and expect it to adjust voltage. We don't really care about the resultant voltage. Essentially, when we ask voltage layer to scale to voltage x for OPP y, it always means x is the nominal voltage for OPP y. Now, once we introduce SmartReflex AVS class 1.5: a) If you are booting for the first time OR if you never enabled SR before, we always go to volt_nominal. b) If you enable SR and the OPP is calibrated, we will not enable SR again when that OPP is accessed anymore, but when we set voltage for an OPP, the voltage achieved will be volt_calibrated. c) If recalibration timeout triggers or SR is disabled after a calibration, the calibrated values are not valid anymore, at this point, setting the voltage will mean volt_dynamic_nominal. So, depending on which state the system is at, voltage for that OPP we are setting has not 1 single value anymore, but 3 possible valid values. For upper layers(DVFS/cpufreq OMAP SoC layers) to use voltage values, it will need to know which type of voltage AVS strategy is being used and the corresponding system state from voltage layer perspective. This would replicate the role of voltage layer, SmartReflex AVS in the upper layers and make the corresponding implementations complex. Since each voltage domain contains a set of volt_data structs representing a voltage point that is supported for that domain, volt_data is a more accurate representation of the voltage point we are interested in going to, and the actual translation of this voltage point to the voltage value is done inside the voltage layer. Doing this allows the users of the voltage layer to be blissfully ignorant of any complexity of the underneath layers and simplify the implementation of dependent layers. Signed-off-by: Nishanth Menon n...@ti.com --- arch/arm/mach-omap2/pm.c |3 +- arch/arm/mach-omap2/smartreflex-class3.c |3 +- arch/arm/mach-omap2/voltage.c| 75 +- arch/arm/mach-omap2/voltage.h| 17 +-- 4 files changed, 59 insertions(+), 39 deletions(-) diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c index 2c3a253..2372744 100644 --- a/arch/arm/mach-omap2/pm.c +++ b/arch/arm/mach-omap2/pm.c @@ -209,7 +209,8 @@ static int __init omap2_set_init_voltage(char *vdd_name, char *clk_name, goto exit; } - omap_voltage_scale_vdd(voltdm, bootup_volt); + omap_voltage_scale_vdd(voltdm, + omap_voltage_get_voltdata(voltdm, bootup_volt)); return 0; exit: diff --git a/arch/arm/mach-omap2/smartreflex-class3.c b/arch/arm/mach-omap2/smartreflex-class3.c index f438cf4..2ee48af 100644 --- a/arch/arm/mach-omap2/smartreflex-class3.c +++ b/arch/arm/mach-omap2/smartreflex-class3.c @@ -15,7 +15,8 @@ static int sr_class3_enable(struct voltagedomain *voltdm) { - unsigned long volt = omap_voltage_get_nom_volt(voltdm); + unsigned long volt = omap_get_operation_voltage( + omap_voltage_get_nom_volt(voltdm)); if (!volt) { pr_warning(%s: Curr voltage unknown. Cannot enable sr_%s\n, diff --git a/arch/arm/mach-omap2/voltage.c b/arch/arm/mach-omap2/voltage.c index 76bcaee..a12ac1e 100644 --- a/arch/arm/mach-omap2/voltage.c +++ b/arch/arm/mach-omap2/voltage.c @@ -58,7 +58,7 @@ static struct dentry *voltage_dir; /* Init function pointers */ static int vp_forceupdate_scale_voltage(struct omap_vdd_info *vdd, - unsigned long target_volt); + struct omap_volt_data *target_volt); static u32 omap3_voltage_read_reg(u16 mod, u8 offset) { @@ -164,13 +164,20 @@ static int vp_volt_debug_get(void *data, u64 *val) static int nom_volt_debug_get(void *data, u64 *val) { struct omap_vdd_info *vdd = (struct omap_vdd_info *) data; + struct omap_volt_data *volt_data; if (!vdd) { pr_warning(Wrong paramater passed\n); return -EINVAL; } - *val = omap_voltage_get_nom_volt(vdd-voltdm); + volt_data = omap_voltage_get_nom_volt(vdd-voltdm); + if (IS_ERR_OR_NULL(volt_data)) { + pr_warning(%s: No voltage/domain?\n, __func__); + return -ENODEV; + } + + *val = volt_data-volt_nominal; return 0; } @@ -184,7 +191,8 @@ static void vp_latch_vsel(struct omap_vdd_info *vdd) unsigned long uvdc; char vsel; - uvdc = omap_voltage_get_nom_volt(vdd-voltdm); + uvdc = omap_get_operation_voltage( + omap_voltage_get_nom_volt(vdd-voltdm)); if