Nishanth Menon <n...@ti.com> writes:
> Traditional SmartReflex AVS(Automatic Voltage Scaling) classes are:
> * Class 0 - Product test calibration
> Silicon is calibration at production floor and fused with voltages
> for each OPP
> * Class 1 - Boot time calibration
> Silicon is calibrated once at boot time and voltages are stored for
> the lifetime of operation.
> * Class 2 - Continuous s/w calibration
> SR module notifies s/w for any change in the system which is desired
> and the s/w makes runtime decisions in terms of setting the voltage,
> this mechanism could be used in the system which does not have PMIC
> capable of SR without using the voltage controller and voltage
> processor blocks.
> * Class 3 - Continuous h/w calibration
> SR module is switch on after reaching a voltage level and SR
> continuously monitors the system and makes runtime adjustments without
> s/w involvement.
>
> OMAP3430 has used SmartReflex AVS and with a a PMIC which understands the SR
> protocol, Class 3 has been used. With OMAP3630 onwards, a new SmartReflex AVS
> class of operation Class 1.5 was introduced.
> * Class 1.5 - Periodic s/w calibration
> This uses the h/w calibration loop and at the end of calibration
> stores the voltages to be used run time, periodic recalibration is
> performed as well.
>
> The operational mode is describes as the following:
> * SmartReflex AVS h/w calibration loop is essential to identify the optimal
> voltage for a given OPP.
> * Once this optimal voltage is detected, SmartReflex AVS loop is disabled in
> class 1.5 mode of operation.
> * Until there is a need for a recalibration, any further transition to an OPP
> voltage which is calibrated can use the calibrated voltage and does not
> require enabling the SR AVS h/w loop.
> * On a periodic basis (recommendation being once approximately every 24
> hours),
> software is expected to perform a recalibration to find a new optimal
> voltage which is compensated for device aging.
> - For performing this recalibration, the start voltage does not need to
> be the nominal voltage anymore. instead, the system can start with a
> voltage which is 50mV higher than the previously calibrated voltage to
> identify the new optimal voltage as the aging factor within a period of
> 1 day is not going to be anywhere close to 50mV.
> - This "new starting point" for recalibration is called a dynamic
> nominal voltage for that voltage point.
> In short, with the introduction of SmartReflex class 1.5, there are three new
> voltages possible in a system's DVFS transition:
> * Nominal Voltage - The maximum voltage needed for a worst possible device
> in the worst possible conditions. This is the voltage we choose as
> the starting point for the h/w loop to optimize for the first time
> calibration on system bootup.
> * Dynamic Nominal Voltage - Worst case voltage for a specific device in
> considering the system aging on the worst process device.
> * Calibrated Voltage - Best voltage for the current device at a given point
> of time.
>
> In terms of the implementation, doing calibration involves waiting for the
> SmartReflex h/w loop to settle down, and doing this as part of the DVFS flow
> itself is to increase the latency of DVFS transition when there is a need to
> calibrate that opp. instead, the calibration is performed "out of path" using
> a workqueue statemachine. The workqueue waits for the system stabilization,
> then enables VP interrupts to monitor for system instability interms of
> voltage
> oscillations that are reported back to the system as interrupts, in case of
> prolonged system oscillations, nominal voltage is chosen as a safe voltage and
> this event is logged in the system log for developer debug and fixing.
>
> For the recalibration, a common workqueue for all domains is started at the
> start of the class initialization and it resets the calibrated voltages
> on a periodic basis. For distros that may choose not to do the recommended
> periodic recalibration, instead choose to perform boot time calibration,
> kconfig configuration option is provided to do so.
>
> TODO:
> a) Cpuidle and suspend paths are not integrated with SmartReflex driver at
> this point.
> b) Since the SR registers are accessed and controlled in parallel to DVFS
> some sort of mechanism is necessary to be introduced along with OMAP
> DVFS layer to ensure mutual exclusivity
> c) Additional debug interfaces for vmin analysis for platform characterization
> and addition of system margin needs to be introduced from SmartReflex
> perspective.
>
> This implementation also includes the following contributors:
> Tony Lindgren for suggestion on using interrupt based mechanism instead of
> polling to detect voltage oscillations.
> Peter 'p2' De Schrijver for debating alternatives on recalibration mechanisms
> Paul Walmsey, Eduardo Valentin, Ambresh K, Igor Dmitriev and quiet a few
> others
> for patient review, testing and reporting of issues of a previous incarnation
> of this implemenation. Last, but not the least, the TI H/w team in introducing
> this new SR AVS class and patiently debating it's various facets.
>
> Cc: Ambresh K <ambr...@ti.com>
> Cc: Eduardo Valentin <eduardo.valen...@nokia.com>
> Cc: Igor Dmitriev <ext-dmitriev.i...@nokia.com>
> Cc: Paul <p...@pwsan.com>
> Cc: Peter 'p2' De Schrijver <peter.de-schrij...@nokia.com>
> Cc: Tony Lindgren <t...@atomide.com>
>
> Signed-off-by: Nishanth Menon <n...@ti.com>
You're the expert on SR1.5, but I have some comments on code and data
structure organization to improve readability. It may be because I'm
not deeply familiar with SR1.5, but I found code organization and data
structures difficult to follow and thus understand.
> ---
>
> NOTE: this patch generates a false postive checkpatch warning:
> WARNING: please write a paragraph that describes the config symbol fully
> #989: FILE: arch/arm/plat-omap/Kconfig:73:
> + help
>
> WARNING: please write a paragraph that describes the config symbol fully
> #998: FILE: arch/arm/plat-omap/Kconfig:82:
> + help
>
> There is a help documentation for each of these, but it looks like checkpatch
> aint too intelligent about it.
Agreed. I think checkpatch wants to see a certain number of lines in
the help text, and whines if they're not there.
> arch/arm/mach-omap2/Makefile | 1 +
> arch/arm/mach-omap2/smartreflex-class1p5.c | 582
> ++++++++++++++++++++++++++++
> arch/arm/mach-omap2/smartreflex-class3.c | 4 +-
> arch/arm/mach-omap2/smartreflex.c | 26 ++-
> arch/arm/mach-omap2/smartreflex.h | 10 +-
> arch/arm/mach-omap2/voltage.c | 79 ++++
> arch/arm/mach-omap2/voltage.h | 23 +-
> arch/arm/plat-omap/Kconfig | 17 +
> 8 files changed, 738 insertions(+), 4 deletions(-)
> create mode 100644 arch/arm/mach-omap2/smartreflex-class1p5.c
>
> diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
> index d566e78..1d4d2ff 100644
> --- a/arch/arm/mach-omap2/Makefile
> +++ b/arch/arm/mach-omap2/Makefile
> @@ -66,6 +66,7 @@ obj-$(CONFIG_ARCH_OMAP4) += pm44xx.o pm_bus.o
> obj-$(CONFIG_PM_DEBUG) += pm-debug.o
> obj-$(CONFIG_OMAP_SMARTREFLEX) += sr_device.o smartreflex.o
> obj-$(CONFIG_OMAP_SMARTREFLEX_CLASS3) += smartreflex-class3.o
> +obj-$(CONFIG_OMAP_SMARTREFLEX_CLASS1P5) += smartreflex-class1p5.o
Minor: personal preference here, but I don't like the 1p5 naming. I'd
rather see 1_5 or even 15, but I'll leave that to you.
> AFLAGS_sleep24xx.o :=-Wa,-march=armv6
> AFLAGS_sleep34xx.o :=-Wa,-march=armv7-a
> diff --git a/arch/arm/mach-omap2/smartreflex-class1p5.c
> b/arch/arm/mach-omap2/smartreflex-class1p5.c
> new file mode 100644
> index 0000000..fdd28f7
> --- /dev/null
> +++ b/arch/arm/mach-omap2/smartreflex-class1p5.c
> @@ -0,0 +1,582 @@
> +/*
> + * Smart reflex Class 1.5 specific implementations
> + *
> + * Copyright (C) 2010-2011 Texas Instruments, Inc.
> + * Nishanth Menon <n...@ti.com>
> + *
> + * Smart reflex class 1.5 is also called periodic SW Calibration
> + * Some of the highlights are as follows:
> + * – Host CPU triggers OPP calibration when transitioning to non calibrated
> + * OPP
> + * – SR-AVS + VP modules are used to perform calibration
> + * – Once completed, the SmartReflex-AVS module can be disabled
> + * – Enables savings based on process, supply DC accuracy and aging
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#include <linux/kernel.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/fs.h>
> +#include <linux/string.h>
> +#include <linux/uaccess.h>
Is this header needed?
> +#include <linux/kobject.h>
or this one?
> +#include <linux/workqueue.h>
> +#include <linux/opp.h>
> +
> +#include "smartreflex.h"
> +#include "voltage.h"
> +
> +#define MAX_VDDS 3
> +#define SR1P5_SAMPLING_DELAY_MS 1
> +#define SR1P5_STABLE_SAMPLES 5
> +#define SR1P5_MAX_TRIGGERS 5
> +
> +/*
> + * we expect events in 10uS, if we dont get 2wice times as much,
> + * we could kind of ignore this as a missed event.
> + */
> +#define MAX_CHECK_VPTRANS_US 20
> +
> +/**
> + * struct sr_class1p5_work_data - data meant to be used by calibration work
> + * @work: calibration work
> + * @voltdm: voltage domain for which we are triggering
> + * @vdata: voltage data we are calibrating
> + * @num_calib_triggers: number of triggers from calibration loop
> + * @num_osc_samples: number of samples collected by isr
> + * @work_active: have we scheduled a work item?
> + */
> +struct sr_class1p5_work_data {
> + struct delayed_work work;
> + struct voltagedomain *voltdm;
> + struct omap_volt_data *vdata;
> + u8 num_calib_triggers;
> + u8 num_osc_samples;
> + bool work_active;
> +};
Again, the 1p5 naming I think is a hinderance to readability.
As these are file-local, I don't think you need the sr_class1p5_ prefix.
> +#if CONFIG_OMAP_SR_CLASS1P5_RECALIBRATION_DELAY
> +/* recal_work: recalibration calibration work */
> +static struct delayed_work recal_work;
> +#endif
No Kconfig for this option please, let's make a reasonable default and
allow board code to override.
> +/**
> + * struct sr_class1p5_data - private data for class 1p5
> + * @work_data: work item data per voltage domain
> + */
> +struct sr_class1p5_data {
> + struct sr_class1p5_work_data work_data[MAX_VDDS];
> +};
> +
> +static void sr_class1p5_reset_calib(struct voltagedomain *voltdm, bool reset,
> + bool recal);
> +
> +/* our instance of class 1p5 private data */
> +static struct sr_class1p5_data class_1p5_data;
'class1p5_data' is also used below as the name of the 'struct
omap_sr_class_data'. Another name for this would help readability/grepability
> +static struct sr_class1p5_work_data *get_sr1p5_work(struct voltagedomain
> + *voltdm)
as a static function, get_work should suffice
> +{
> + int idx;
insert blank line
> + for (idx = 0; idx < MAX_VDDS; idx++) {
> + if (class_1p5_data.work_data[idx].voltdm && !strcmp
> + (class_1p5_data.work_data[idx].voltdm->name, voltdm->name))
> + return &class_1p5_data.work_data[idx];
> + }
insert blank line
Also, should do this such that you don't need to do continually do
string compares. Since you need to keep a mapping of voltdm -> SR class
data you should make something simple for it.
Using your current names, how about something like this (not even
compile tested):
static struct {
struct voltagedomain *voltdm;
struct sr_class1p5_work_data *work_data;
} voltdm_to_work[MAX_VDDS];
static struct sr_class1p5_work_data *get_sr1p5_work(struct voltagedomain
*voltdm)
{
int i;
struct sr_class1p5_work_data *work_data = NULL;
for( i = 0; i < MAX_VDDS; i++) {
struct voltagedomain *v = voltdm_to_work[i];
if (v && v == voltdm)
work_data = voltdm_to_work[i].work_data;
}
return work_data;
}
And you could probably have a 'set_work' helper as well. Maybe this can
be initialized in the class->configure hook instead of having to be done
in the ->start hook each time ?
> + return ERR_PTR(-ENODATA);
> +}
> +
> +/**
> + * sr_class1p5_notify() - isr notifier for status events
> + * @voltdm: voltage domain for which we were triggered
> + * @status: notifier event to use
> + *
> + * This basically collects data for the work to use.
> + */
> +static int sr_class1p5_notify(struct voltagedomain *voltdm, u32 status)
> +{
> + struct sr_class1p5_work_data *work_data;
> + int idx = 0;
s/idx/i/
> + if (IS_ERR_OR_NULL(voltdm)) {
on input parameters checking for NULL should suffice.
Please fix throughout this patch (and series)
> + pr_err("%s: bad parameters!\n", __func__);
> + return -EINVAL;
> + }
> +
> + work_data = get_sr1p5_work(voltdm);
> + if (unlikely(!work_data)) {
> + pr_err("%s:%s no work data!!\n", __func__, voltdm->name);
> + return -EINVAL;
> + }
> +
> + /* Wait for transdone so that we know the voltage to read */
> + do {
readablity: use:
for(i = 0; i < MAX_CHECK_VPTRANS_US; i++)
> + if (omap_vp_is_transdone(voltdm))
> + break;
> + idx++;
then drop this
> + /* get some constant delay */
comment does not help understand code, remove
> + udelay(1);
> + } while (idx < MAX_CHECK_VPTRANS_US);
and this
> + /*
> + * If we timeout, we still read the data,
> + * if we are oscillating+irq latencies are too high, we could
s/irq/IRQ/
> + * have scenarios where we miss transdone event. since
> + * we waited long enough, it is still safe to read the voltage
> + * as we would have waited long enough - still flag it..
> + */
/me no understand
> + if (idx >= MAX_CHECK_VPTRANS_US)
> + pr_warning("%s: timed out waiting for transdone!!\n", __func__);
> +
> + omap_vp_clear_transdone(voltdm);
> +
> + idx = (work_data->num_osc_samples) % SR1P5_STABLE_SAMPLES;
idx not used after this
> + work_data->num_osc_samples++;
> +
> + return 0;
> +}
> +
> +/**
> + * do_calibrate() - work which actually does the calibration
> + * @work: pointer to the work
> + *
> + * calibration routine uses the following logic:
> + * on the first trigger, we start the isr to collect sr voltages
s/isr/ISR/ (and below)
> + * wait for stabilization delay (reschdule self instead of sleeping)
"instead of sleeping" ? you'll indeed sleep until the delayed work is run
> + * after the delay, see if we collected any isr events
> + * if none, we have calibrated voltage.
> + * if there are any, we retry untill we giveup.
until we give up
> + * on retry timeout, select a voltage to use as safe voltage.
> + */
> +static void do_calibrate(struct work_struct *work)
> +{
> + struct sr_class1p5_work_data *work_data =
> + container_of(work, struct sr_class1p5_work_data, work.work);
Doesn't a work_struct have a data field? I thought it was meant for
this kind of thing, so you don't have to do a container_of()
> + unsigned long u_volt_safe = 0, u_volt_current = 0;
> + struct omap_volt_data *volt_data;
> + struct voltagedomain *voltdm;
> +
> + if (unlikely(!work_data)) {
> + pr_err("%s: ooops.. null work_data?\n", __func__);
> + return;
> + }
> +
> + /*
> + * TODO:Handle the case where we might have just been scheduled AND
> + * 1.5 disable was called. check and HOLD DVFS
> + */
> +
> + voltdm = work_data->voltdm;
> + /*
> + * In the unlikely case that we did get through when unplanned,
> + * flag and return.
> + */
> + if (unlikely(!work_data->work_active)) {
> + pr_err("%s:%s unplanned work invocation!\n", __func__,
> + voltdm->name);
> + /* TODO release the DVFS */
> + return;
> + }
> +
> + work_data->num_calib_triggers++;
> + /* if we are triggered first time, we need to start isr to sample */
> + if (work_data->num_calib_triggers == 1)
> + goto start_sampling;
> +
> + /* Stop isr from interrupting our measurements :) */
ISR.
What is the smiley supposed to mean?
> + sr_notifier_control(voltdm, false);
> +
> + volt_data = work_data->vdata;
> +
> + /* if there are no samples captured.. SR is silent, aka stability! */
Couldn't this also be zero when sampling is initially triggered. Is
there a race here?
> + if (!work_data->num_osc_samples) {
> + u_volt_safe = omap_vp_get_curr_volt(voltdm);
> + u_volt_current = u_volt_safe;
> + goto done_calib;
> + }
> + if (work_data->num_calib_triggers == SR1P5_MAX_TRIGGERS) {
> + pr_warning("%s: %s recalib timeout!\n", __func__,
> + work_data->voltdm->name);
> + goto oscillating_calib;
> + }
> +
> + /* we have potential oscillations/first sample */
> +start_sampling:
> + work_data->num_osc_samples = 0;
> + /* Clear pending events */
comment doesn't match following code
> + sr_notifier_control(voltdm, false);
> + /* Clear all transdones */
comment doesn't add anything to code
> + while (omap_vp_is_transdone(voltdm))
> + omap_vp_clear_transdone(voltdm);
> + /* trigger sampling */
> + sr_notifier_control(voltdm, true);
> + schedule_delayed_work(&work_data->work,
> + msecs_to_jiffies(SR1P5_SAMPLING_DELAY_MS *
> + SR1P5_STABLE_SAMPLES));
> + /* TODO: release DVFS */
> + return;
> +
> +oscillating_calib:
> + /* Use the nominal voltage as the safe voltage */
> + u_volt_safe = volt_data->volt_nominal;
> + /* pick up current voltage to switch if needed */
> + u_volt_current = omap_vp_get_curr_volt(voltdm);
> +
> + /* Fall through to close up common stuff */
> +done_calib:
> + omap_vp_disable(voltdm);
> + sr_disable(voltdm);
> +
> + volt_data->volt_calibrated = u_volt_safe;
> + /* Setup my dynamic voltage for the next calibration for this opp */
> + volt_data->volt_dynamic_nominal = omap_get_dyn_nominal(volt_data);
> +
> + /*
> + * if the voltage we decided as safe is not the current voltage,
> + * switch
> + */
> + if (volt_data->volt_calibrated != u_volt_current) {
> + pr_debug("%s:%s reconfiguring to voltage %d\n",
> + __func__, voltdm->name, volt_data->volt_calibrated);
> + omap_voltage_scale_vdd(voltdm, volt_data);
> + }
> +
> + /*
> + * TODO: Setup my wakeup voltage to allow immediate going to OFF and
> + * on - Pending twl and voltage layer cleanups.
> + * This is necessary, as this is not done as part of regular
> + * Dvfs flow.
> + * vc_setup_on_voltage(voltdm, volt_data->volt_calibrated);
> + */
> + work_data->work_active = false;
> + /* TODO: release DVFS */
> +}
> +
> +#if CONFIG_OMAP_SR_CLASS1P5_RECALIBRATION_DELAY
> +/**
> + * do_recalibrate() - work which actually does the calibration
> + * @work: pointer to the work
> + *
> + * on a periodic basis, we come and reset our calibration setup
> + * so that a recalibration of the OPPs take place. This takes
> + * care of aging factor in the system.
> + */
> +static void do_recalibrate(struct work_struct *work)
> +{
> + struct voltagedomain *voltdm;
> + int idx;
> + static struct sr_class1p5_work_data *work_data;
> +
> + for (idx = 0; idx < MAX_VDDS; idx++) {
> + work_data = &class_1p5_data.work_data[idx];
> + voltdm = work_data->voltdm;
> + if (voltdm) {
> + /* if sr is not enabled, we check later */
> + if (!is_sr_enabled(voltdm))
> + continue;
> + /* TODO: Pause the DVFS transitions */
> + /* if sr is not enabled, we check later */
> +
> + /* Reset and force a recalibration for current opp */
> + sr_class1p5_reset_calib(voltdm, true, true);
> +
> + /* TODO: unpause DVFS transitions */
> + }
> + }
> + /* We come back again after time the usual delay */
> + schedule_delayed_work(&recal_work,
> + msecs_to_jiffies(CONFIG_OMAP_SR_CLASS1P5_RECALIBRATION_DELAY));
> +}
> +#endif /* CONFIG_OMAP_SR_CLASS1P5_RECALIBRATION_DELAY */
> +
> +/**
> + * sr_class1p5_enable() - class 1.5 mode of enable
> + * @voltdm: voltage domain to enable SR for
> + * @volt_data: voltdata to the voltage transition taking place
> + *
> + * when this gets called, we use the h/w loop to setup our voltages
> + * to an calibrated voltage, detect any oscillations, recover from the same
> + * and finally store the optimized voltage as the calibrated voltage in the
> + * system
> + */
> +static int sr_class1p5_enable(struct voltagedomain *voltdm,
> + struct omap_volt_data *volt_data)
> +{
> + int r;
> + struct sr_class1p5_work_data *work_data;
> +
> + if (IS_ERR_OR_NULL(voltdm) || IS_ERR_OR_NULL(volt_data)) {
> + pr_err("%s: bad parameters!\n", __func__);
> + return -EINVAL;
> + }
> +
> + /* if already calibrated, nothing to do here.. */
> + if (volt_data->volt_calibrated)
> + return 0;
> +
> + work_data = get_sr1p5_work(voltdm);
> + if (unlikely(!work_data)) {
> + pr_err("%s: aieeee.. bad work data??\n", __func__);
> + return -EINVAL;
> + }
> +
> + if (work_data->work_active)
> + return 0;
> +
> + omap_vp_enable(voltdm);
> + r = sr_enable(voltdm, volt_data);
> + if (r) {
> + pr_err("%s: sr[%s] failed\n", __func__, voltdm->name);
> + omap_vp_disable(voltdm);
> + return r;
> + }
> + work_data->vdata = volt_data;
> + work_data->work_active = true;
> + work_data->num_calib_triggers = 0;
> + /* program the workqueue and leave it to calibrate offline.. */
> + schedule_delayed_work(&work_data->work,
> + msecs_to_jiffies(SR1P5_SAMPLING_DELAY_MS *
> + SR1P5_STABLE_SAMPLES));
> +
> + return 0;
> +}
> +
> +/**
> + * sr_class1p5_disable() - disable for class 1p5
> + * @voltdm: voltage domain for the sr which needs disabling
> + * @volt_data: voltagedata to disable
> + * @is_volt_reset: reset the voltage?
> + *
> + * we dont do anything if the class 1p5 is being used. this is because we
> + * already disable sr at the end of calibration and no h/w loop is actually
> + * active when this is called.
> + */
> +static int sr_class1p5_disable(struct voltagedomain *voltdm,
> + struct omap_volt_data *volt_data,
> + int is_volt_reset)
> +{
> + struct sr_class1p5_work_data *work_data;
> +
> + if (IS_ERR_OR_NULL(voltdm) || IS_ERR_OR_NULL(volt_data)) {
> + pr_err("%s: bad parameters!\n", __func__);
> + return -EINVAL;
> + }
> +
> + work_data = get_sr1p5_work(voltdm);
> + if (work_data->work_active) {
> + /* if volt reset and work is active, we dont allow this */
> + if (is_volt_reset)
> + return -EBUSY;
> + /* flag work is dead and remove the old work */
> + work_data->work_active = false;
> + cancel_delayed_work_sync(&work_data->work);
> + sr_notifier_control(voltdm, false);
> + omap_vp_disable(voltdm);
> + sr_disable(voltdm);
> + }
> +
> + /* if already calibrated, nothin special to do here.. */
> + if (volt_data->volt_calibrated)
> + return 0;
> +
> + if (is_volt_reset)
> + omap_voltage_reset(voltdm);
> + return 0;
> +}
> +
> +/**
> + * sr_class1p5_configure() - configuration function
> + * @voltdm: configure for which voltage domain
> + *
> + * we dont do much here other than setup some registers for
> + * the sr module involved.
> + */
> +static int sr_class1p5_configure(struct voltagedomain *voltdm)
> +{
> + if (IS_ERR_OR_NULL(voltdm)) {
> + pr_err("%s: bad parameters!\n", __func__);
> + return -EINVAL;
> + }
> +
> + return sr_configure_errgen(voltdm);
> +}
> +
> +/**
> + * sr_class1p5_reset_calib() - reset all calibrated voltages
> + * @voltdm: configure for which voltage domain
> + * @reset: reset voltage before we recal?
> + * @recal: should I recalibrate my current opp?
> + *
> + * if we call this, it means either periodic calibration trigger was
> + * fired(either from sysfs or other mechanisms) or we have disabled class
> 1p5,
> + * meaning we cant trust the calib voltages anymore, it is better to use
> + * nominal in the system
> + */
> +static void sr_class1p5_reset_calib(struct voltagedomain *voltdm, bool reset,
> + bool recal)
> +{
> + struct sr_class1p5_work_data *work_data;
> +
> + /* I dont need to go further if sr is not present */
> + if (!is_sr_enabled(voltdm))
> + return;
> +
> + work_data = get_sr1p5_work(voltdm);
> +
> + if (work_data->work_active)
> + sr_class1p5_disable(voltdm, work_data->vdata, 0);
> +
> + omap_voltage_calib_reset(voltdm);
> +
> + /*
> + * I should now reset the voltages to my nominal to be safe
> + */
> + if (reset)
> + omap_voltage_reset(voltdm);
> +
> + /*
> + * I should fire a recalibration for current opp if needed
> + * Note: i have just reset my calibrated voltages, and if
> + * i call sr_enable equivalent, I will cause a recalibration
> + * loop, even though the function is called sr_enable.. we
> + * are in class 1.5 ;)
> + */
> + if (reset && recal)
> + sr_class1p5_enable(voltdm, work_data->vdata);
> +}
> +
> +/**
> + * sr_class1p5_start() - class 1p5 init
> + * @voltdm: sr voltage domain
> + * @class_priv_data: private data for the class
> + *
> + * we do class specific initialization like creating sysfs/debugfs entries
> + * needed, spawning of a kthread if needed etc.
> + */
> +static int sr_class1p5_start(struct voltagedomain *voltdm,
> + void *class_priv_data)
> +{
> + struct sr_class1p5_work_data *work_data;
> + int idx;
> +
> + if (IS_ERR_OR_NULL(voltdm) || IS_ERR_OR_NULL(class_priv_data)) {
> + pr_err("%s: bad parameters!\n", __func__);
> + return -EINVAL;
> + }
> +
> + /* setup our work params */
> + work_data = get_sr1p5_work(voltdm);
> + if (!IS_ERR_OR_NULL(work_data)) {
> + pr_err("%s: ooopps.. class already initialized for %s! bug??\n",
> + __func__, voltdm->name);
> + return -EINVAL;
> + }
> + work_data = NULL;
> + /* get the next spare work_data */
> + for (idx = 0; idx < MAX_VDDS; idx++) {
> + if (!class_1p5_data.work_data[idx].voltdm) {
> + work_data = &class_1p5_data.work_data[idx];
> + break;
> + }
> + }
> + if (!work_data) {
> + pr_err("%s: no more space for work data for domains!\n",
> + __func__);
> + return -ENOMEM;
> + }
> + work_data->voltdm = voltdm;
> + INIT_DELAYED_WORK_DEFERRABLE(&work_data->work, do_calibrate);
Much of this looks to be one-time init stuff. Wouldn't that be better
in the ->configure method?
> + return 0;
> +}
> +
> +/**
> + * sr_class1p5_stop() - class 1p5 deinitialization
> + * @voltdm: voltage domain for which to do this.
> + * @class_priv_data: class private data for deinitialiation
> + *
> + * currently only resets the calibrated voltage forcing DVFS voltages
> + * to be used in the system
> + */
> +static int sr_class1p5_stop(struct voltagedomain *voltdm,
> + void *class_priv_data)
> +{
> + struct sr_class1p5_work_data *work_data;
> +
> + if (IS_ERR_OR_NULL(voltdm) || IS_ERR_OR_NULL(class_priv_data)) {
> + pr_err("%s: bad parameters!\n", __func__);
> + return -EINVAL;
> + }
> +
> + /* setup our work params */
> + work_data = get_sr1p5_work(voltdm);
> + if (IS_ERR_OR_NULL(work_data)) {
> + pr_err("%s: ooopps.. class not initialized for %s! bug??\n",
> + __func__, voltdm->name);
> + return -EINVAL;
> + }
> +
> + /*
> + * we dont have SR periodic calib anymore.. so reset calibs
> + * we are already protected by sr debugfs lock, so no lock needed
> + * here.
> + */
> + sr_class1p5_reset_calib(voltdm, true, false);
> +
> + /* reset all data for this work data */
> + memset(work_data, 0, sizeof(*work_data));
> +
> + return 0;
> +}
> +
> +/* SR class1p5 structure */
> +static struct omap_sr_class_data class1p5_data = {
> + .enable = sr_class1p5_enable,
> + .disable = sr_class1p5_disable,
> + .configure = sr_class1p5_configure,
> + .class_type = SR_CLASS1P5,
> + .start = sr_class1p5_start,
> + .stop = sr_class1p5_stop,
> + .notify = sr_class1p5_notify,
> + /*
> + * trigger for bound - this tells VP that SR has a voltage
> + * change. we should ensure transdone is set before reading
> + * vp voltage.
> + */
> + .notify_flags = SR_NOTIFY_MCUBOUND,
> + .class_priv_data = (void *)&class_1p5_data,
> +};
> +
> +/**
> + * sr_class1p5_init() - register class 1p5 as default
> + *
> + * board files call this function to use class 1p5, we register with the
> + * smartreflex subsystem
> + */
> +static int __init sr_class1p5_init(void)
> +{
> + int r;
> +
> + /* Enable this class only for OMAP3630 and OMAP4 */
> + if (!(cpu_is_omap3630() || cpu_is_omap44xx()))
> + return -EINVAL;
> +
> + r = sr_register_class(&class1p5_data);
> + if (r) {
> + pr_err("SmartReflex class 1.5 driver: "
> + "failed to register with %d\n", r);
> + } else {
> +#if CONFIG_OMAP_SR_CLASS1P5_RECALIBRATION_DELAY
> + INIT_DELAYED_WORK_DEFERRABLE(&recal_work, do_recalibrate);
> + schedule_delayed_work(&recal_work, msecs_to_jiffies(
> + CONFIG_OMAP_SR_CLASS1P5_RECALIBRATION_DELAY));
> +#endif
> + pr_info("SmartReflex class 1.5 driver: initialized (%dms)\n",
> + CONFIG_OMAP_SR_CLASS1P5_RECALIBRATION_DELAY);
> + }
> + return r;
> +}
> +late_initcall(sr_class1p5_init);
> diff --git a/arch/arm/mach-omap2/smartreflex-class3.c
> b/arch/arm/mach-omap2/smartreflex-class3.c
> index 1d3eb11..0136afb 100644
> --- a/arch/arm/mach-omap2/smartreflex-class3.c
> +++ b/arch/arm/mach-omap2/smartreflex-class3.c
> @@ -21,7 +21,9 @@ static int sr_class3_enable(struct voltagedomain *voltdm,
> return sr_enable(voltdm, volt_data);
> }
>
> -static int sr_class3_disable(struct voltagedomain *voltdm, int is_volt_reset)
> +static int sr_class3_disable(struct voltagedomain *voltdm,
> + struct omap_volt_data *vdata,
> + int is_volt_reset)
I think this change belonged in the previous patch.
> {
> omap_vp_disable(voltdm);
> sr_disable(voltdm);
> diff --git a/arch/arm/mach-omap2/smartreflex.c
> b/arch/arm/mach-omap2/smartreflex.c
> index 5c549b9..5738298 100644
> --- a/arch/arm/mach-omap2/smartreflex.c
> +++ b/arch/arm/mach-omap2/smartreflex.c
> @@ -324,7 +324,9 @@ static void sr_stop_vddautocomp(struct omap_sr *sr, bool
> class_stop,
> return;
> }
>
> - sr_class->disable(sr->voltdm, is_volt_reset);
> + sr_class->disable(sr->voltdm,
> + omap_voltage_get_nom_volt(sr->voltdm),
> + is_volt_reset);
this one too.
> if (class_stop) {
> if (sr_class->stop &&
> sr_class->stop(sr->voltdm, sr_class->class_priv_data))
> @@ -478,6 +480,28 @@ static u32 sr_retrieve_nvalue(struct omap_sr *sr, u32
> efuse_offs)
> /* Public Functions */
>
> /**
> + * is_sr_enabled() - is Smart reflex enabled for this domain?
> + * @voltdm: voltage domain to check
> + *
> + * Returns 0 if SR is enabled for this domain, else returns err
> + */
> +bool is_sr_enabled(struct voltagedomain *voltdm)
Please follow naming conventions of the rest of the SR layer.
> +{
> + struct omap_sr *sr;
insert blank line
> + if (IS_ERR_OR_NULL(voltdm)) {
> + pr_warning("%s: invalid param voltdm\n", __func__);
> + return false;
> + }
> + sr = _sr_lookup(voltdm);
> + if (IS_ERR(sr)) {
> + pr_warning("%s: omap_sr struct for sr_%s not found\n",
> + __func__, voltdm->name);
> + return false;
> + }
> + return sr->autocomp_active;
> +}
> +
> +/**
> * sr_configure_errgen() - Configures the smrtreflex to perform AVS using the
> * error generator module.
> * @voltdm: VDD pointer to which the SR module to be configured belongs to.
> diff --git a/arch/arm/mach-omap2/smartreflex.h
> b/arch/arm/mach-omap2/smartreflex.h
> index 812e86d..d1ed829 100644
> --- a/arch/arm/mach-omap2/smartreflex.h
> +++ b/arch/arm/mach-omap2/smartreflex.h
> @@ -168,6 +168,7 @@ struct omap_sr_pmic_data {
> #define SR_CLASS1 0x1
> #define SR_CLASS2 0x2
> #define SR_CLASS3 0x3
> +#define SR_CLASS1P5 0x4
>
> /**
> * struct omap_sr_class_data - Smartreflex class driver info
> @@ -188,7 +189,9 @@ struct omap_sr_pmic_data {
> struct omap_sr_class_data {
> int (*enable)(struct voltagedomain *voltdm,
> struct omap_volt_data *volt_data);
> - int (*disable)(struct voltagedomain *voltdm, int is_volt_reset);
> + int (*disable)(struct voltagedomain *voltdm,
> + struct omap_volt_data *volt_data,
> + int is_volt_reset);
This change should've been part of previous patch.
> int (*start)(struct voltagedomain *voltdm, void *class_priv_data);
> int (*stop)(struct voltagedomain *voltdm, void *class_priv_data);
> int (*configure)(struct voltagedomain *voltdm);
> @@ -250,6 +253,7 @@ int sr_configure_minmax(struct voltagedomain *voltdm);
>
> /* API to register the smartreflex class driver with the smartreflex driver
> */
> int sr_register_class(struct omap_sr_class_data *class_data);
> +bool is_sr_enabled(struct voltagedomain *voltdm);
> #else
> static inline void omap_sr_enable(struct voltagedomain *voltdm) {}
> static inline void omap_sr_disable(struct voltagedomain *voltdm) {}
> @@ -264,5 +268,9 @@ static inline void omap_sr_disable_reset_volt(
> struct voltagedomain *voltdm) {}
> static inline void omap_sr_register_pmic(
> struct omap_sr_pmic_data *pmic_data) {}
> +static inline bool is_sr_enabled(struct voltagedomain *voltdm)
> +{
> + return false;
> +}
> #endif
> #endif
> diff --git a/arch/arm/mach-omap2/voltage.c b/arch/arm/mach-omap2/voltage.c
> index 2d70d13..80db727 100644
> --- a/arch/arm/mach-omap2/voltage.c
> +++ b/arch/arm/mach-omap2/voltage.c
> @@ -182,9 +182,55 @@ static int nom_volt_debug_get(void *data, u64 *val)
> return 0;
> }
>
> +static int dyn_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;
> + }
> +
> + 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_dynamic_nominal;
> +
> + return 0;
> +}
> +
> +static int calib_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");
to be a useful debug message, should at least have a __func__ involved
to know where it came from.
> + return -EINVAL;
> + }
> +
> + 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_calibrated;
> +
> + return 0;
> +}
> +
> DEFINE_SIMPLE_ATTRIBUTE(vp_volt_debug_fops, vp_volt_debug_get, NULL,
> "%llu\n");
> DEFINE_SIMPLE_ATTRIBUTE(nom_volt_debug_fops, nom_volt_debug_get, NULL,
> "%llu\n");
> +DEFINE_SIMPLE_ATTRIBUTE(dyn_volt_debug_fops, dyn_volt_debug_get, NULL,
> + "%llu\n");
> +DEFINE_SIMPLE_ATTRIBUTE(calib_volt_debug_fops, calib_volt_debug_get, NULL,
> + "%llu\n");
> static void vp_latch_vsel(struct omap_vdd_info *vdd)
> {
> u32 vpconfig;
> @@ -306,6 +352,12 @@ static void __init vdd_debugfs_init(struct omap_vdd_info
> *vdd)
> (void) debugfs_create_file("curr_nominal_volt", S_IRUGO,
> vdd->debug_dir, (void *) vdd,
> &nom_volt_debug_fops);
> + (void) debugfs_create_file("curr_dyn_nominal_volt", S_IRUGO,
> + vdd->debug_dir, (void *) vdd,
> + &dyn_volt_debug_fops);
> + (void) debugfs_create_file("curr_calibrated_volt", S_IRUGO,
> + vdd->debug_dir, (void *) vdd,
> + &calib_volt_debug_fops);
> }
>
> /* Voltage scale and accessory APIs */
> @@ -697,6 +749,33 @@ struct omap_volt_data *omap_voltage_get_nom_volt(struct
> voltagedomain *voltdm)
> }
>
> /**
> + * omap_voltage_calib_reset() - reset the calibrated voltage entries
> + * @voltdm: voltage domain to reset the entries for
> + *
> + * when the calibrated entries are no longer valid, this api allows
> + * the calibrated voltages to be reset.
> + */
> +int omap_voltage_calib_reset(struct voltagedomain *voltdm)
> +{
> + struct omap_vdd_info *vdd;
> + struct omap_volt_data *volt_data;
> +
> + if (IS_ERR_OR_NULL(voltdm)) {
> + pr_warning("%s: VDD specified does not exist!\n", __func__);
> + return -EINVAL;
> + }
> +
> + vdd = container_of(voltdm, struct omap_vdd_info, voltdm);
> + volt_data = vdd->volt_data;
> + /* reset the calibrated voltages as 0 */
> + while (volt_data->volt_nominal) {
> + volt_data->volt_calibrated = 0;
> + volt_data++;
> + }
> + return 0;
> +}
> +
> +/**
> * omap_vp_get_curr_volt() - API to get the current vp voltage.
> * @voltdm: pointer to the VDD.
> *
> diff --git a/arch/arm/mach-omap2/voltage.h b/arch/arm/mach-omap2/voltage.h
> index 5b4e363..5e84881 100644
> --- a/arch/arm/mach-omap2/voltage.h
> +++ b/arch/arm/mach-omap2/voltage.h
> @@ -57,6 +57,8 @@ struct voltagedomain {
> char *name;
> };
>
> +#define OMAP3PLUS_DYNAMIC_NOMINAL_MARGIN_UV 50000
> +
A comment describing this value (and where it came from) would be most
useful.
> /**
> * struct omap_volt_data - Omap voltage specific data.
> * @voltage_nominal: The possible voltage value in uV
> @@ -71,6 +73,8 @@ struct voltagedomain {
> */
> struct omap_volt_data {
> u32 volt_nominal;
> + u32 volt_calibrated;
> + u32 volt_dynamic_nominal;
> u32 sr_efuse_offs;
> u8 sr_errminlimit;
> u8 vp_errgain;
> @@ -156,6 +160,7 @@ struct dentry *omap_voltage_get_dbgdir(struct
> voltagedomain *voltdm);
> int __init omap_voltage_early_init(s16 prm_mod, s16 prm_irqst_mod,
> struct omap_vdd_info *omap_vdd_array[],
> u8 omap_vdd_count);
> +int omap_voltage_calib_reset(struct voltagedomain *voltdm);
> #ifdef CONFIG_PM
> int omap_voltage_register_pmic(struct voltagedomain *voltdm,
> struct omap_volt_pmic_info *pmic_info);
> @@ -189,7 +194,23 @@ static inline unsigned long omap_get_operation_voltage(
> {
> if (IS_ERR_OR_NULL(vdata))
> return 0;
> - return vdata->volt_nominal;
> + return (vdata->volt_calibrated) ? vdata->volt_calibrated :
> + (vdata->volt_dynamic_nominal) ? vdata->volt_dynamic_nominal :
> + vdata->volt_nominal;
> }
>
> +/* what is my dynamic nominal? */
comment doesn't add anything to code
> +static inline unsigned long omap_get_dyn_nominal(struct omap_volt_data
> *vdata)
> +{
> + if (IS_ERR_OR_NULL(vdata))
> + return 0;
> + if (vdata->volt_calibrated) {
> + unsigned long v = vdata->volt_calibrated +
> + OMAP3PLUS_DYNAMIC_NOMINAL_MARGIN_UV;
> + if (v > vdata->volt_nominal)
> + return vdata->volt_nominal;
> + return v;
> + }
> + return vdata->volt_nominal;
> +}
> #endif
> diff --git a/arch/arm/plat-omap/Kconfig b/arch/arm/plat-omap/Kconfig
> index b6333ae..dba7939 100644
> --- a/arch/arm/plat-omap/Kconfig
> +++ b/arch/arm/plat-omap/Kconfig
> @@ -67,6 +67,23 @@ config OMAP_SMARTREFLEX_CLASS3
> Class 3 implementation of Smartreflex employs continuous hardware
> voltage calibration.
>
> +config OMAP_SMARTREFLEX_CLASS1P5
> + bool "Class 1.5 mode of Smartreflex Implementation"
> + depends on OMAP_SMARTREFLEX && TWL4030_CORE
> + help
> + Say Y to enable Class 1.5 implementation of Smartreflex
> + Class 1.5 implementation of Smartreflex employs software controlled
> + hardware voltage calibration.
> +
> +config OMAP_SR_CLASS1P5_RECALIBRATION_DELAY
> + int "Class 1.5 mode recalibration recalibration delay(ms)"
> + depends on OMAP_SMARTREFLEX_CLASS1P5
> + default 86400000
> + help
> + Setup the recalibration delay in milliseconds. Use 0 for never doing
> + a recalibration. Defaults to recommended recalibration every 24hrs.
> + If you do not understand this, use the default.
> +
> config OMAP_RESET_CLOCKS
> bool "Reset unused clocks during boot"
> depends on ARCH_OMAP
Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
