RE: [PATCH v6 01/10] OMAP3: PM: Adding voltage driver support.

Anand Sawant
2010-12-28T16:17:39Z

Thara,

Here are some comments on the voltage layer; many of them are
cosmetic in nature.

> -----Original Message-----
> From: Thara Gopinath [mailto:thara@ti.com]
> Sent: Monday, December 20, 2010 10:29 PM
> To: linux-omap@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org
> Cc: khilman@deeprootsystems.com; paul@pwsan.com; b-
> cousson@ti.com; vishwanath.bs@ti.com; sawant@ti.com;
> nm@ti.com; Thara Gopinath
> Subject: [PATCH v6 01/10] OMAP3: PM: Adding voltage driver
> support.
>
> This patch adds voltage driver support for OMAP3. The driver
> allows configuring the voltage controller and voltage
> processors during init and exports APIs to enable/disable
> voltage processors, scale voltage and reset voltage.
> The driver maintains the global voltage table on a per
> VDD basis which contains the various voltages supported by
> the
> VDD along with per voltage dependent data like smartreflex
> efuse offset, errminlimit and voltage processor errorgain.
> The driver also allows the voltage parameters dependent on
> the
> PMIC to be passed from the PMIC file through an API.
> The driver allows scaling of VDD voltages either through
> "vc bypass method" or through "vp forceupdate method" the
> choice being configurable through the board file.
>
> This patch contains code originally in linux omap pm branch
> smartreflex driver. Major contributors to this driver are
> Lesly A M, Rajendra Nayak, Kalle Jokiniemi, Paul Walmsley,
> Nishant Menon, Kevin Hilman. The separation of PMIC
> parameters
> into a separate structure which can be populated from
> the PMIC file is based on the work of Lun Chang from
> Motorola
> in an internal tree.
>
> Signed-off-by: Thara Gopinath <thara@ti.com>
> ---
> This patch has 14 checkpatch.pl above 80-chars warnings for
> the definitions of omap34xx_vddmpu_volt_data,
> omap36xx_vddmpu_volt_data,
> omap34xx_vddcore_volt_data and omap36xx_vddcore_volt_data
> structures.
> IMHO splitting of the entries in these structures affects
> readability and looks very ugly. Hence they are left as is.
>
> arch/arm/mach-omap2/Makefile | 3 +-
> arch/arm/mach-omap2/control.h | 17 +
> arch/arm/mach-omap2/pm.c | 8 +
> arch/arm/mach-omap2/voltage.c | 1226
> +++++++++++++++++++++++++++++
> arch/arm/plat-omap/include/plat/voltage.h | 134 ++++
> 5 files changed, 1387 insertions(+), 1 deletions(-)
> create mode 100644 arch/arm/mach-omap2/voltage.c
> create mode 100644 arch/arm/plat-
> omap/include/plat/voltage.h
>
> diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-
> omap2/Makefile
> index 7c79683..5034797 100644
> --- a/arch/arm/mach-omap2/Makefile
> +++ b/arch/arm/mach-omap2/Makefile
> @@ -57,7 +57,8 @@ endif
> ifeq ($(CONFIG_PM),y)
> obj-$(CONFIG_ARCH_OMAP2) += pm24xx.o
> obj-$(CONFIG_ARCH_OMAP2) += sleep24xx.o pm_bus.o
> -obj-$(CONFIG_ARCH_OMAP3) += pm34xx.o sleep34xx.o
> cpuidle34xx.o pm_bus.o
> +obj-$(CONFIG_ARCH_OMAP3) += pm34xx.o sleep34xx.o
> voltage.o \
> + cpuidle34xx.o pm_bus.o
> obj-$(CONFIG_ARCH_OMAP4) += pm44xx.o pm_bus.o
> obj-$(CONFIG_PM_DEBUG) += pm-debug.o
>
> diff --git a/arch/arm/mach-omap2/control.h b/arch/arm/mach-
> omap2/control.h
> index 5289461..9fe32dc 100644
> --- a/arch/arm/mach-omap2/control.h
> +++ b/arch/arm/mach-omap2/control.h
> @@ -148,6 +148,15 @@
> #define OMAP343X_CONTROL_TEST_KEY_11
> (OMAP2_CONTROL_GENERAL + 0x00f4)
> #define OMAP343X_CONTROL_TEST_KEY_12
> (OMAP2_CONTROL_GENERAL + 0x00f8)
> #define OMAP343X_CONTROL_TEST_KEY_13
> (OMAP2_CONTROL_GENERAL + 0x00fc)
> +#define OMAP343X_CONTROL_FUSE_OPP1_VDD1
> (OMAP2_CONTROL_GENERAL + 0x0110)
> +#define OMAP343X_CONTROL_FUSE_OPP2_VDD1
> (OMAP2_CONTROL_GENERAL + 0x0114)
> +#define OMAP343X_CONTROL_FUSE_OPP3_VDD1
> (OMAP2_CONTROL_GENERAL + 0x0118)
> +#define OMAP343X_CONTROL_FUSE_OPP4_VDD1
> (OMAP2_CONTROL_GENERAL + 0x011c)
> +#define OMAP343X_CONTROL_FUSE_OPP5_VDD1
> (OMAP2_CONTROL_GENERAL + 0x0120)
> +#define OMAP343X_CONTROL_FUSE_OPP1_VDD2
> (OMAP2_CONTROL_GENERAL + 0x0124)
> +#define OMAP343X_CONTROL_FUSE_OPP2_VDD2
> (OMAP2_CONTROL_GENERAL + 0x0128)
> +#define OMAP343X_CONTROL_FUSE_OPP3_VDD2
> (OMAP2_CONTROL_GENERAL + 0x012c)
> +#define OMAP343X_CONTROL_FUSE_SR
> (OMAP2_CONTROL_GENERAL + 0x0130)
> #define OMAP343X_CONTROL_IVA2_BOOTADDR
> (OMAP2_CONTROL_GENERAL + 0x0190)
> #define OMAP343X_CONTROL_IVA2_BOOTMOD
> (OMAP2_CONTROL_GENERAL + 0x0194)
> #define OMAP343X_CONTROL_DEBOBS(i) (OMAP2_CONTROL_GENERAL +
> 0x01B0 \
> @@ -164,6 +173,14 @@
> #define OMAP343X_CONTROL_SRAMLDO5 (OMAP2_CONTROL_GENERAL +
> 0x02C0)
> #define OMAP343X_CONTROL_CSI (OMAP2_CONTROL_GENERAL +
> 0x02C4)
>
> +/* OMAP3630 only CONTROL_GENERAL register offsets */
> +#define OMAP3630_CONTROL_FUSE_OPP1G_VDD1
> (OMAP2_CONTROL_GENERAL + 0x0110)
> +#define OMAP3630_CONTROL_FUSE_OPP50_VDD1
> (OMAP2_CONTROL_GENERAL + 0x0114)
> +#define OMAP3630_CONTROL_FUSE_OPP100_VDD1
> (OMAP2_CONTROL_GENERAL + 0x0118)
> +#define OMAP3630_CONTROL_FUSE_OPP120_VDD1
> (OMAP2_CONTROL_GENERAL + 0x0120)
> +#define OMAP3630_CONTROL_FUSE_OPP50_VDD2
> (OMAP2_CONTROL_GENERAL + 0x0128)
> +#define OMAP3630_CONTROL_FUSE_OPP100_VDD2
> (OMAP2_CONTROL_GENERAL + 0x012C)
> +
> /* AM35XX only CONTROL_GENERAL register offsets */
> #define AM35XX_CONTROL_MSUSPENDMUX_6
> (OMAP2_CONTROL_GENERAL + 0x0038)
> #define AM35XX_CONTROL_DEVCONF2
> (OMAP2_CONTROL_GENERAL + 0x0310)
> diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-
> omap2/pm.c
> index 227a211..22adfb2 100644
> --- a/arch/arm/mach-omap2/pm.c
> +++ b/arch/arm/mach-omap2/pm.c
> @@ -17,6 +17,7 @@
> #include <plat/omap-pm.h>
> #include <plat/omap_device.h>
> #include <plat/common.h>
> +#include <plat/voltage.h>
>
> #include "powerdomain.h"
> #include "clockdomain.h"
> @@ -145,3 +146,10 @@ static int __init
> omap2_common_pm_init(void)
> }
> postcore_initcall(omap2_common_pm_init);
>
> +static int __init omap2_common_pm_late_init(void)
> +{
> + omap_voltage_late_init();
> +
> + return 0;
> +}
> +late_initcall(omap2_common_pm_late_init);
> diff --git a/arch/arm/mach-omap2/voltage.c b/arch/arm/mach-
> omap2/voltage.c
> new file mode 100644
> index 0000000..875667f
> --- /dev/null
> +++ b/arch/arm/mach-omap2/voltage.c
> @@ -0,0 +1,1226 @@
> +/*
> + * OMAP3/OMAP4 Voltage Management Routines
> + *
> + * Author: Thara Gopinath <thara@ti.com>
> + *
> + * Copyright (C) 2007 Texas Instruments, Inc.
> + * Rajendra Nayak <rnayak@ti.com>
> + * Lesly A M <x0080970@ti.com>
> + *
> + * Copyright (C) 2008 Nokia Corporation
> + * Kalle Jokiniemi
> + *
> + * Copyright (C) 2010 Texas Instruments, Inc.
> + * Thara Gopinath <thara@ti.com>
> + *
> + * 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/delay.h>
> +#include <linux/io.h>
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/debugfs.h>
> +#include <linux/slab.h>
> +
> +#include <plat/common.h>
> +#include <plat/voltage.h>
> +
> +#include "prm-regbits-34xx.h"
> +#include "control.h"
> +
> +#define VP_IDLE_TIMEOUT 200
> +#define VP_TRANXDONE_TIMEOUT 300
> +#define VOLTAGE_DIR_SIZE 16

Add comments for the above #defines.

> +
> +/* Voltage processor register offsets */

Add comments for the data members.

> +struct vp_reg_offs {
> + u8 vpconfig;
> + u8 vstepmin;
> + u8 vstepmax;
> + u8 vlimitto;
> + u8 vstatus;
> + u8 voltage;
> +};
> +
> +/* Voltage Processor bit field values, shifts and masks */

Add comments for the data members.

> +struct vp_reg_val {
> + /* PRM module */
> + u16 prm_mod;
> + /* VPx_VPCONFIG */
> + u32 vpconfig_erroroffset;
> + u16 vpconfig_errorgain;
> + u32 vpconfig_errorgain_mask;
> + u8 vpconfig_errorgain_shift;
> + u32 vpconfig_initvoltage_mask;
> + u8 vpconfig_initvoltage_shift;
> + u32 vpconfig_timeouten;
> + u32 vpconfig_initvdd;
> + u32 vpconfig_forceupdate;
> + u32 vpconfig_vpenable;
> + /* VPx_VSTEPMIN */
> + u8 vstepmin_stepmin;
> + u16 vstepmin_smpswaittimemin;
> + u8 vstepmin_stepmin_shift;
> + u8 vstepmin_smpswaittimemin_shift;
> + /* VPx_VSTEPMAX */
> + u8 vstepmax_stepmax;
> + u16 vstepmax_smpswaittimemax;
> + u8 vstepmax_stepmax_shift;
> + u8 vstepmax_smpswaittimemax_shift;
> + /* VPx_VLIMITTO */
> + u8 vlimitto_vddmin;
> + u8 vlimitto_vddmax;
> + u16 vlimitto_timeout;
> + u8 vlimitto_vddmin_shift;
> + u8 vlimitto_vddmax_shift;
> + u8 vlimitto_timeout_shift;
> + /* PRM_IRQSTATUS*/
> + u32 tranxdone_status;
> +};
> +
> +/* Voltage controller registers and offsets */

Add comments for the data members.

> +struct vc_reg_info {
> + /* PRM module */
> + u16 prm_mod;
> + /* VC register offsets */
> + u8 smps_sa_reg;
> + u8 smps_volra_reg;
> + u8 bypass_val_reg;
> + u8 cmdval_reg;
> + u8 voltsetup_reg;
> + /*VC_SMPS_SA*/
> + u8 smps_sa_shift;
> + u32 smps_sa_mask;
> + /* VC_SMPS_VOL_RA */
> + u8 smps_volra_shift;
> + u32 smps_volra_mask;
> + /* VC_BYPASS_VAL */
> + u8 data_shift;
> + u8 slaveaddr_shift;
> + u8 regaddr_shift;
> + u32 valid;
> + /* VC_CMD_VAL */
> + u8 cmd_on_shift;
> + u8 cmd_onlp_shift;
> + u8 cmd_ret_shift;
> + u8 cmd_off_shift;
> + u32 cmd_on_mask;
> + /* PRM_VOLTSETUP */
> + u8 voltsetup_shift;
> + u32 voltsetup_mask;
> +};
> +
> +/**
> + * omap_vdd_info - Per Voltage Domain info
> + *
> + * @volt_data : voltage table having the distinct
> voltages supported
> + * by the domain and other associated per
> voltage data.
> + * @pmic_info : pmic specific parameters which
> should be populted by
> + * the pmic drivers.
> + * @vp_offs : structure containing the offsets
> for various
> + * vp registers
> + * @vp_reg : the register values, shifts, masks for
> various
> + * vp registers
> + * @vc_reg : structure containing various various vc
> registers,
> + * shifts, masks etc.
> + * @voltdm : pointer to the voltage domain structure
> + * @debug_dir : debug directory for this voltage
> domain.
> + * @curr_volt : current voltage for this vdd.
> + * @ocp_mod : The prm module for accessing the
> prm irqstatus reg.
> + * @prm_irqst_reg : prm irqstatus register.
> + * @vp_enabled : flag to keep track of whether vp
> is enabled or not

Add comments for read_reg & write_reg.

> + * @volt_scale : API to scale the voltage of the
> vdd.

Add more explanation about the structure - like its
significance, how it is populated and used etc.

> + */
> +struct omap_vdd_info {
> + struct omap_volt_data *volt_data;
> + struct omap_volt_pmic_info *pmic_info;
> + struct vp_reg_offs vp_offs;
> + struct vp_reg_val vp_reg;
> + struct vc_reg_info vc_reg;
> + struct voltagedomain voltdm;

Make voltdm the first member of the structure as it's a key identifier.

> + struct dentry *debug_dir;
> + u32 curr_volt;
> + u16 ocp_mod;
> + u8 prm_irqst_reg;
> + bool vp_enabled;
> + u32 (*read_reg) (u16 mod, u8 offset);
> + void (*write_reg) (u32 val, u16 mod, u8 offset);
> + int (*volt_scale) (struct omap_vdd_info *vdd,
> + unsigned long target_volt);
> +};
> +
> +static struct omap_vdd_info *vdd_info;

Add comment. It's an important pointer.

> +/*
> + * Number of scalable voltage domains.
> + */
> +static int nr_scalable_vdd;
> +
> +/* OMAP3 VDD sturctures */

Add comment to mention that partial initialization is
done here & the rest is done in omap3_vdd_data_configure()
function.

> +static struct omap_vdd_info omap3_vdd_info[] = {
> + {
> + .vp_offs = {
> + .vpconfig = OMAP3_PRM_VP1_CONFIG_OFFSET,
> + .vstepmin = OMAP3_PRM_VP1_VSTEPMIN_OFFSET,
> + .vstepmax = OMAP3_PRM_VP1_VSTEPMAX_OFFSET,
> + .vlimitto = OMAP3_PRM_VP1_VLIMITTO_OFFSET,
> + .vstatus = OMAP3_PRM_VP1_STATUS_OFFSET,
> + .voltage = OMAP3_PRM_VP1_VOLTAGE_OFFSET,
> + },

Recommend to use DEFINE_VP_REG_OFFS macro similar to
VOLT_DATA_DEFINE to make it more readable.

> + .voltdm = {
> + .name = "mpu",
> + },

Make this initialization the first initialization.

> + },
> + {
> + .vp_offs = {
> + .vpconfig = OMAP3_PRM_VP2_CONFIG_OFFSET,
> + .vstepmin = OMAP3_PRM_VP2_VSTEPMIN_OFFSET,
> + .vstepmax = OMAP3_PRM_VP2_VSTEPMAX_OFFSET,
> + .vlimitto = OMAP3_PRM_VP2_VLIMITTO_OFFSET,
> + .vstatus = OMAP3_PRM_VP2_STATUS_OFFSET,
> + .voltage = OMAP3_PRM_VP2_VOLTAGE_OFFSET,
> + },
> + .voltdm = {
> + .name = "core",
> + },
> + },
> +};
> +
> +#define OMAP3_NR_SCALABLE_VDD ARRAY_SIZE(omap3_vdd_info)
> +
> +/*
> + * Structures containing OMAP3430/OMAP3630 voltage
> supported and various
> + * voltage dependent data for each VDD.
> + */
> +#define VOLT_DATA_DEFINE(_v_nom, _efuse_offs, _errminlimit,
> _errgain) \
> +{ \
> + .volt_nominal = _v_nom,
> \
> + .sr_efuse_offs = _efuse_offs,
> \
> + .sr_errminlimit = _errminlimit,
> \
> + .vp_errgain = _errgain \
> +}
> +
> +/* VDD1 */

Add a comment stressing the need to keep
VOLT_DATA_DEFINE(0, 0, 0, 0) as always the last member.
Otherwise, the loop in omap_voltage_get_voltdata() will
not function properly.

> +static struct omap_volt_data omap34xx_vddmpu_volt_data[] =
> {
> + VOLT_DATA_DEFINE(OMAP3430_VDD_MPU_OPP1_UV,
> OMAP343X_CONTROL_FUSE_OPP1_VDD1, 0xf4, 0x0c),
> + VOLT_DATA_DEFINE(OMAP3430_VDD_MPU_OPP2_UV,
> OMAP343X_CONTROL_FUSE_OPP2_VDD1, 0xf4, 0x0c),
> + VOLT_DATA_DEFINE(OMAP3430_VDD_MPU_OPP3_UV,
> OMAP343X_CONTROL_FUSE_OPP3_VDD1, 0xf9, 0x18),
> + VOLT_DATA_DEFINE(OMAP3430_VDD_MPU_OPP4_UV,
> OMAP343X_CONTROL_FUSE_OPP4_VDD1, 0xf9, 0x18),
> + VOLT_DATA_DEFINE(OMAP3430_VDD_MPU_OPP5_UV,
> OMAP343X_CONTROL_FUSE_OPP5_VDD1, 0xf9, 0x18),
> + VOLT_DATA_DEFINE(0, 0, 0, 0),
> +};
> +
> +static struct omap_volt_data omap36xx_vddmpu_volt_data[] =
> {
> + VOLT_DATA_DEFINE(OMAP3630_VDD_MPU_OPP50_UV,
> OMAP3630_CONTROL_FUSE_OPP50_VDD1, 0xf4, 0x0c),
> + VOLT_DATA_DEFINE(OMAP3630_VDD_MPU_OPP100_UV,
> OMAP3630_CONTROL_FUSE_OPP100_VDD1, 0xf9, 0x16),
> + VOLT_DATA_DEFINE(OMAP3630_VDD_MPU_OPP120_UV,
> OMAP3630_CONTROL_FUSE_OPP120_VDD1, 0xfa, 0x23),
> + VOLT_DATA_DEFINE(OMAP3630_VDD_MPU_OPP1G_UV,
> OMAP3630_CONTROL_FUSE_OPP1G_VDD1, 0xfa, 0x27),
> + VOLT_DATA_DEFINE(0, 0, 0, 0),
> +};
> +
> +/* VDD2 */
> +static struct omap_volt_data omap34xx_vddcore_volt_data[] =
> {
> + VOLT_DATA_DEFINE(OMAP3430_VDD_CORE_OPP1_UV,
> OMAP343X_CONTROL_FUSE_OPP1_VDD2, 0xf4, 0x0c),
> + VOLT_DATA_DEFINE(OMAP3430_VDD_CORE_OPP2_UV,
> OMAP343X_CONTROL_FUSE_OPP2_VDD2, 0xf4, 0x0c),
> + VOLT_DATA_DEFINE(OMAP3430_VDD_CORE_OPP3_UV,
> OMAP343X_CONTROL_FUSE_OPP3_VDD2, 0xf9, 0x18),
> + VOLT_DATA_DEFINE(0, 0, 0, 0),
> +};
> +
> +static struct omap_volt_data omap36xx_vddcore_volt_data[] =
> {
> + VOLT_DATA_DEFINE(OMAP3630_VDD_CORE_OPP50_UV,
> OMAP3630_CONTROL_FUSE_OPP50_VDD2, 0xf4, 0x0c),
> + VOLT_DATA_DEFINE(OMAP3630_VDD_CORE_OPP100_UV,
> OMAP3630_CONTROL_FUSE_OPP100_VDD2, 0xf9, 0x16),
> + VOLT_DATA_DEFINE(0, 0, 0, 0),
> +};
> +
> +static struct dentry *voltage_dir;

Add comment.

> +
> +/* Init function pointers */

Add comment mentioning how they are populated.

> +static void (*vc_init) (struct omap_vdd_info *vdd);
> +static int (*vdd_data_configure) (struct omap_vdd_info
> *vdd);

Recommend to define a similar init function pointer for
vp initialization. At present, it will be initialized with
the generic vp_init(). But it will allow easy migration
in case vp initialization is different for future OMAPs.

Also, highlight the difference between init (actually writes
to module registers) and data_configure (populates data
structures; no writing to registers) for clarity.

> +
> +static u32 omap3_voltage_read_reg(u16 mod, u8 offset)
> +{
> + return omap2_prm_read_mod_reg(mod, offset);
> +}
> +
> +static void omap3_voltage_write_reg(u32 val, u16 mod, u8
> offset)
> +{
> + omap2_prm_write_mod_reg(val, mod, offset);
> +}
> +

Add comment for vp_latch_vsel.

> +static void vp_latch_vsel(struct omap_vdd_info *vdd)
> +{
> + u32 vpconfig;
> + u16 mod;
> + unsigned long uvdc;
> + char vsel;
> +
> + uvdc = omap_voltage_get_nom_volt(&vdd->voltdm);
> + if (!uvdc) {
> + pr_warning("%s: unable to find current voltage
> for vdd_%s\n",
> + __func__, vdd->voltdm.name);
> + return;
> + }
> +
> + if (!vdd->pmic_info || !vdd->pmic_info->uv_to_vsel) {
> + pr_warning("%s: PMIC function to convert voltage
> in uV to"
> + " vsel not registered\n", __func__);
> + return;
> + }
> +
> + mod = vdd->vp_reg.prm_mod;
> +
> + vsel = vdd->pmic_info->uv_to_vsel(uvdc);
> +
> + vpconfig = vdd->read_reg(mod, vdd->vp_offs.vpconfig);
> + vpconfig &= ~(vdd->vp_reg.vpconfig_initvoltage_mask |
> + vdd->vp_reg.vpconfig_initvdd);
> + vpconfig |= vsel << vdd-
> >vp_reg.vpconfig_initvoltage_shift;
> +
> + vdd->write_reg(vpconfig, mod, vdd->vp_offs.vpconfig);
> +
> + /* Trigger initVDD value copy to voltage processor */
> + vdd->write_reg((vpconfig | vdd-
> >vp_reg.vpconfig_initvdd), mod,
> + vdd->vp_offs.vpconfig);
> +

Check if any delay is needed between triggering initVDD copy
& clearing initVDD copy. Check if the two need to be atomic.

> + /* Clear initVDD copy trigger bit */
> + vdd->write_reg(vpconfig, mod, vdd->vp_offs.vpconfig);
> +}
> +
> +/* Generic voltage init functions */
> +static void __init vp_init(struct omap_vdd_info *vdd)

Recommend to change the return type to int to allow for
better error handling & diagnostic messages.

> +{
> + u32 vp_val;
> + u16 mod;
> +
> + if (!vdd->read_reg || !vdd->write_reg) {
> + pr_err("%s: No read/write API for accessing
> vdd_%s regs\n",
> + __func__, vdd->voltdm.name);
> + return;
> + }
> +
> + mod = vdd->vp_reg.prm_mod;
> +
> + vp_val = vdd->vp_reg.vpconfig_erroroffset |
> + (vdd->vp_reg.vpconfig_errorgain <<
> + vdd->vp_reg.vpconfig_errorgain_shift) |
> + vdd->vp_reg.vpconfig_timeouten;
> + vdd->write_reg(vp_val, mod, vdd->vp_offs.vpconfig);
> +
> + vp_val = ((vdd->vp_reg.vstepmin_smpswaittimemin <<
> + vdd->vp_reg.vstepmin_smpswaittimemin_shift) |
> + (vdd->vp_reg.vstepmin_stepmin <<
> + vdd->vp_reg.vstepmin_stepmin_shift));
> + vdd->write_reg(vp_val, mod, vdd->vp_offs.vstepmin);
> +
> + vp_val = ((vdd->vp_reg.vstepmax_smpswaittimemax <<
> + vdd->vp_reg.vstepmax_smpswaittimemax_shift) |
> + (vdd->vp_reg.vstepmax_stepmax <<
> + vdd->vp_reg.vstepmax_stepmax_shift));
> + vdd->write_reg(vp_val, mod, vdd->vp_offs.vstepmax);
> +
> + vp_val = ((vdd->vp_reg.vlimitto_vddmax <<
> + vdd->vp_reg.vlimitto_vddmax_shift) |
> + (vdd->vp_reg.vlimitto_vddmin <<
> + vdd->vp_reg.vlimitto_vddmin_shift) |
> + (vdd->vp_reg.vlimitto_timeout <<
> + vdd->vp_reg.vlimitto_timeout_shift));
> + vdd->write_reg(vp_val, mod, vdd->vp_offs.vlimitto);
> +}
> +

Add comment for vdd_debugfs_init.

> +static void __init vdd_debugfs_init(struct omap_vdd_info
> *vdd)
> +{
> + char *name;
> +
> + name = kzalloc(VOLTAGE_DIR_SIZE, GFP_KERNEL);
> + if (!name) {
> + pr_warning("%s: Unable to allocate memory for
> debugfs"
> + " directory name for vdd_%s",
> + __func__, vdd->voltdm.name);
> + return;
> + }
> + strcpy(name, "vdd_");
> + strcat(name, vdd->voltdm.name);
> +
> + vdd->debug_dir = debugfs_create_dir(name,
> voltage_dir);
> + if (IS_ERR(vdd->debug_dir)) {
> + pr_warning("%s: Unable to create debugfs
> directory for"
> + " vdd_%s\n", __func__, vdd->voltdm.name);
> + vdd->debug_dir = NULL;
> + }
> +}
> +
> +/* Voltage scale and accessory APIs */

Add comment for _pre_volt_scale.

> +static int _pre_volt_scale(struct omap_vdd_info *vdd,
> + unsigned long target_volt, u8 *target_vsel, u8
> *current_vsel)
> +{
> + struct omap_volt_data *volt_data;
> + u32 vc_cmdval, vp_errgain_val;
> + u16 vp_mod, vc_mod;
> +
> + /* Check if suffiecient pmic info is available for
> this vdd */
> + if (!vdd->pmic_info) {
> + pr_err("%s: Insufficient pmic info to scale the
> vdd_%s\n",
> + __func__, vdd->voltdm.name);
> + return -EINVAL;
> + }
> +
> + if (!vdd->pmic_info->uv_to_vsel) {
> + pr_err("%s: PMIC function to convert voltage in
> uV to"
> + "vsel not registered. Hence unable to
> scale voltage"
> + "for vdd_%s\n", __func__, vdd-
> >voltdm.name);
> + return -ENODATA;
> + }
> +
> + if (!vdd->read_reg || !vdd->write_reg) {
> + pr_err("%s: No read/write API for accessing
> vdd_%s regs\n",
> + __func__, vdd->voltdm.name);
> + return -EINVAL;
> + }
> +
> + vp_mod = vdd->vp_reg.prm_mod;
> + vc_mod = vdd->vc_reg.prm_mod;
> +
> + /* Get volt_data corresponding to target_volt */
> + volt_data = omap_voltage_get_voltdata(&vdd->voltdm,
> target_volt);
> + if (IS_ERR(volt_data))
> + volt_data = NULL;

If there is an error on volt_data; return an error code.
No need to continue with the rest of the processing.

> +
> + *target_vsel = vdd->pmic_info-
> >uv_to_vsel(target_volt);
> + *current_vsel = vdd->read_reg(vp_mod, vdd-
> >vp_offs.voltage);
> +
> + /* Setting the ON voltage to the new target voltage */
> + vc_cmdval = vdd->read_reg(vc_mod, vdd-
> >vc_reg.cmdval_reg);
> + vc_cmdval &= ~vdd->vc_reg.cmd_on_mask;
> + vc_cmdval |= (*target_vsel << vdd-
> >vc_reg.cmd_on_shift);
> + vdd->write_reg(vc_cmdval, vc_mod, vdd-
> >vc_reg.cmdval_reg);
> +
> + /* Setting vp errorgain based on the voltage */
> + if (volt_data) {
> + vp_errgain_val = vdd->read_reg(vp_mod,
> + vdd->vp_offs.vpconfig);
> + vdd->vp_reg.vpconfig_errorgain = volt_data-
> >vp_errgain;
> + vp_errgain_val &= ~vdd-
> >vp_reg.vpconfig_errorgain_mask;
> + vp_errgain_val |= vdd->vp_reg.vpconfig_errorgain
> <<
> + vdd-
> >vp_reg.vpconfig_errorgain_shift;
> + vdd->write_reg(vp_errgain_val, vp_mod,
> + vdd->vp_offs.vpconfig);
> + }

Recommend to take a snapshot of current vsel,
cmdval_reg & errgain before setting the new values. Think of
a mechanism to restore them in case the scaling function
(called after _pre_volt_scale()) fails midway.

Check related comments in vp_forceupdate_scale_voltage()
& vc_bypass_scale_voltage().

> +
> + return 0;
> +}
> +

Add comment for _post_volt_scale.

> +static void _post_volt_scale(struct omap_vdd_info *vdd,
> + unsigned long target_volt, u8 target_vsel, u8
> current_vsel)
> +{
> + u32 smps_steps = 0, smps_delay = 0;
> +
> + smps_steps = abs(target_vsel - current_vsel);
> + /* SMPS slew rate / step size. 2us added as buffer. */
> + smps_delay = ((smps_steps * vdd->pmic_info->step_size)
> /
> + vdd->pmic_info->slew_rate) + 2;
> + udelay(smps_delay);
> +
> + vdd->curr_volt = target_volt;
> +}
> +
> +/* vc_bypass_scale_voltage - VC bypass method of voltage
> scaling */
> +static int vc_bypass_scale_voltage(struct omap_vdd_info
> *vdd,
> + unsigned long target_volt)
> +{
> + u32 loop_cnt = 0, retries_cnt = 0;
> + u32 vc_valid, vc_bypass_val_reg, vc_bypass_value;
> + u16 mod;
> + u8 target_vsel, current_vsel;
> + int ret;
> +
> + ret = _pre_volt_scale(vdd, target_volt, &target_vsel,
> &current_vsel);
> + if (ret)
> + return ret;

This check is ineffective to handle error in case volt_data
corresponding to the target voltage is not found as it
returns 0.

> +
> + mod = vdd->vc_reg.prm_mod;
> +
> + vc_valid = vdd->vc_reg.valid;
> + vc_bypass_val_reg = vdd->vc_reg.bypass_val_reg;
> + vc_bypass_value = (target_vsel << vdd-
> >vc_reg.data_shift) |
> + (vdd->pmic_info->pmic_reg <<
> + vdd->vc_reg.regaddr_shift) |
> + (vdd->pmic_info->i2c_slave_addr <<
> + vdd->vc_reg.slaveaddr_shift);
> +
> + vdd->write_reg(vc_bypass_value, mod,
> vc_bypass_val_reg);
> + vdd->write_reg(vc_bypass_value | vc_valid, mod,
> vc_bypass_val_reg);
> +
> + vc_bypass_value = vdd->read_reg(mod,
> vc_bypass_val_reg);
> + /*
> + * Loop till the bypass command is acknowledged from
> the SMPS.
> + * NOTE: This is legacy code. The loop count and retry
> count needs
> + * to be revisited.
> + */
> + while (!(vc_bypass_value & vc_valid)) {
> + loop_cnt++;
> +
> + if (retries_cnt > 10) {
> + pr_warning("%s: Retry count exceeded\n",
> __func__);

Recommend to restore the snapshot of vsel, cmdval_reg &
errgain in _pre_volt_scale before returning an error.

This can be bit complicated as HW's exact state is not
known.

> + return -ETIMEDOUT;
> + }
> +
> + if (loop_cnt > 50) {
> + retries_cnt++;
> + loop_cnt = 0;
> + udelay(10);
> + }
> + vc_bypass_value = vdd->read_reg(mod,
> vc_bypass_val_reg);
> + }
> +
> + _post_volt_scale(vdd, target_volt, target_vsel,
> current_vsel);
> + return 0;
> +}
> +
> +/* VP force update method of voltage scaling */
> +static int vp_forceupdate_scale_voltage(struct
> omap_vdd_info *vdd,
> + unsigned long target_volt)
> +{
> + u32 vpconfig;
> + u16 mod, ocp_mod;
> + u8 target_vsel, current_vsel, prm_irqst_reg;
> + int ret, timeout = 0;
> +
> + ret = _pre_volt_scale(vdd, target_volt, &target_vsel,
> &current_vsel);
> + if (ret)
> + return ret;

This check is ineffective to handle error in case volt_data
corresponding to the target voltage is not found as it
returns 0.

> +
> + mod = vdd->vp_reg.prm_mod;
> + ocp_mod = vdd->ocp_mod;
> + prm_irqst_reg = vdd->prm_irqst_reg;
> +
> + /*
> + * Clear all pending TransactionDone interrupt/status.
> Typical latency
> + * is <3us
> + */
> + while (timeout++ < VP_TRANXDONE_TIMEOUT) {
> + vdd->write_reg(vdd->vp_reg.tranxdone_status,
> + ocp_mod, prm_irqst_reg);
> + if (!(vdd->read_reg(ocp_mod, prm_irqst_reg) &
> + vdd->vp_reg.tranxdone_status))
> + break;
> + udelay(1);
> + }
> + if (timeout >= VP_TRANXDONE_TIMEOUT) {
> + pr_warning("%s: vdd_%s TRANXDONE timeout
> exceeded."
> + "Voltage change aborted", __func__, vdd-
> >voltdm.name);

Recommend to restore the snapshot of vsel, cmdval_reg &
errgain in _pre_volt_scale before returning an error.

This can be bit complicated as HW's exact state is not
known.

> + return -ETIMEDOUT;
> + }
> +
> + /* Configure for VP-Force Update */
> + vpconfig = vdd->read_reg(mod, vdd->vp_offs.vpconfig);
> + vpconfig &= ~(vdd->vp_reg.vpconfig_initvdd |
> + vdd->vp_reg.vpconfig_forceupdate |
> + vdd->vp_reg.vpconfig_initvoltage_mask);
> + vpconfig |= ((target_vsel <<
> + vdd->vp_reg.vpconfig_initvoltage_shift));
> + vdd->write_reg(vpconfig, mod, vdd->vp_offs.vpconfig);
> +
> + /* Trigger initVDD value copy to voltage processor */
> + vpconfig |= vdd->vp_reg.vpconfig_initvdd;
> + vdd->write_reg(vpconfig, mod, vdd->vp_offs.vpconfig);
> +
> + /* Force update of voltage */
> + vpconfig |= vdd->vp_reg.vpconfig_forceupdate;
> + vdd->write_reg(vpconfig, mod, vdd->vp_offs.vpconfig);
> +
> + /*
> + * Wait for TransactionDone. Typical latency is
> <200us.
> + * Depends on SMPSWAITTIMEMIN/MAX and voltage change
> + */
> + timeout = 0;
> + omap_test_timeout((vdd->read_reg(ocp_mod,
> prm_irqst_reg) &
> + vdd->vp_reg.tranxdone_status),
> + VP_TRANXDONE_TIMEOUT, timeout);
> + if (timeout >= VP_TRANXDONE_TIMEOUT)
> + pr_err("%s: vdd_%s TRANXDONE timeout exceeded."
> + "TRANXDONE never got set after the voltage
> update\n",
> + __func__, vdd->voltdm.name);
> +
> + _post_volt_scale(vdd, target_volt, target_vsel,
> current_vsel);

It is risky to assume voltage change has been successful in
case TRANXDONE never got set after voltage update. Recommend
to do a read of SMPS voltage for confirmation before
proceeding further.

> +
> + /*
> + * Disable TransactionDone interrupt , clear all
> status, clear
> + * control registers
> + */

Check whether it is safe to disable TRANXDONE interrupt even
if it didn't get set.

> + timeout = 0;
> + while (timeout++ < VP_TRANXDONE_TIMEOUT) {
> + vdd->write_reg(vdd->vp_reg.tranxdone_status,
> + ocp_mod, prm_irqst_reg);
> + if (!(vdd->read_reg(ocp_mod, prm_irqst_reg) &
> + vdd->vp_reg.tranxdone_status))
> + break;
> + udelay(1);
> + }
> +
> + if (timeout >= VP_TRANXDONE_TIMEOUT)
> + pr_warning("%s: vdd_%s TRANXDONE timeout
> exceeded while trying"
> + "to clear the TRANXDONE status\n",
> + __func__, vdd->voltdm.name);
> +
> + vpconfig = vdd->read_reg(mod, vdd->vp_offs.vpconfig);
> + /* Clear initVDD copy trigger bit */
> + vpconfig &= ~vdd->vp_reg.vpconfig_initvdd;;
> + vdd->write_reg(vpconfig, mod, vdd->vp_offs.vpconfig);
> + /* Clear force bit */
> + vpconfig &= ~vdd->vp_reg.vpconfig_forceupdate;
> + vdd->write_reg(vpconfig, mod, vdd->vp_offs.vpconfig);
> +
> + return 0;
> +}
> +
> +/* OMAP3 specific voltage init functions */
> +
> +/*
> + * Intializes the voltage controller registers with the
> PMIC and board
> + * specific parameters and voltage setup times for OMAP3.
> + */
> +static void __init omap3_vc_init(struct omap_vdd_info *vdd)
> +{
> + u32 vc_val;
> + u16 mod;
> + u8 on_vsel, onlp_vsel, ret_vsel, off_vsel;
> + static bool is_initialized;
> +
> + if (!vdd->pmic_info || !vdd->pmic_info->uv_to_vsel) {
> + pr_err("%s: PMIC info requried to configure vc
> for"
> + "vdd_%s not populated.Hence cannot
> initialize vc\n",
> + __func__, vdd->voltdm.name);
> + return;
> + }
> +
> + if (!vdd->read_reg || !vdd->write_reg) {
> + pr_err("%s: No read/write API for accessing
> vdd_%s regs\n",
> + __func__, vdd->voltdm.name);
> + return;
> + }
> +
> + mod = vdd->vc_reg.prm_mod;
> +
> + /* Set up the SMPS_SA(i2c slave address in VC */
> + vc_val = vdd->read_reg(mod, vdd->vc_reg.smps_sa_reg);
> + vc_val &= ~vdd->vc_reg.smps_sa_mask;
> + vc_val |= vdd->pmic_info->i2c_slave_addr << vdd-
> >vc_reg.smps_sa_shift;
> + vdd->write_reg(vc_val, mod, vdd->vc_reg.smps_sa_reg);
> +
> + /* Setup the VOLRA(pmic reg addr) in VC */
> + vc_val = vdd->read_reg(mod, vdd-
> >vc_reg.smps_volra_reg);
> + vc_val &= ~vdd->vc_reg.smps_volra_mask;
> + vc_val |= vdd->pmic_info->pmic_reg << vdd-
> >vc_reg.smps_volra_shift;
> + vdd->write_reg(vc_val, mod, vdd-
> >vc_reg.smps_volra_reg);
> +
> + /*Configure the setup times */
> + vc_val = vdd->read_reg(mod, vdd-
> >vc_reg.voltsetup_reg);
> + vc_val &= ~vdd->vc_reg.voltsetup_mask;
> + vc_val |= vdd->pmic_info->volt_setup_time <<
> + vdd->vc_reg.voltsetup_shift;
> + vdd->write_reg(vc_val, mod, vdd-
> >vc_reg.voltsetup_reg);
> +
> + /* Set up the on, inactive, retention and off voltage
> */
> + on_vsel = vdd->pmic_info->uv_to_vsel(vdd->pmic_info-
> >on_volt);
> + onlp_vsel = vdd->pmic_info->uv_to_vsel(vdd->pmic_info-
> >onlp_volt);
> + ret_vsel = vdd->pmic_info->uv_to_vsel(vdd->pmic_info-
> >ret_volt);
> + off_vsel = vdd->pmic_info->uv_to_vsel(vdd->pmic_info-
> >off_volt);
> + vc_val = ((on_vsel << vdd->vc_reg.cmd_on_shift) |
> + (onlp_vsel << vdd->vc_reg.cmd_onlp_shift) |
> + (ret_vsel << vdd->vc_reg.cmd_ret_shift) |
> + (off_vsel << vdd->vc_reg.cmd_off_shift));
> + vdd->write_reg(vc_val, mod, vdd->vc_reg.cmdval_reg);
> +
> + if (is_initialized)
> + return;
> +
> + /* Generic VC parameters init */
> + vdd->write_reg(OMAP3430_CMD1_MASK |
> OMAP3430_RAV1_MASK, mod,
> + OMAP3_PRM_VC_CH_CONF_OFFSET);
> + vdd->write_reg(OMAP3430_MCODE_SHIFT |
> OMAP3430_HSEN_MASK, mod,
> + OMAP3_PRM_VC_I2C_CFG_OFFSET);
> + vdd->write_reg(OMAP3_CLKSETUP, mod,
> OMAP3_PRM_CLKSETUP_OFFSET);
> + vdd->write_reg(OMAP3_VOLTOFFSET, mod,
> OMAP3_PRM_VOLTOFFSET_OFFSET);
> + vdd->write_reg(OMAP3_VOLTSETUP2, mod,
> OMAP3_PRM_VOLTSETUP2_OFFSET);
> + is_initialized = true;
> +}
> +
> +/* Sets up all the VDD related info for OMAP3 */
> +static int __init omap3_vdd_data_configure(struct
> omap_vdd_info *vdd)
> +{
> + struct clk *sys_ck;
> + u32 sys_clk_speed, timeout_val, waittime;
> +
> + if (!vdd->pmic_info) {
> + pr_err("%s: PMIC info requried to configure
> vdd_%s not"
> + "populated.Hence cannot initialize
> vdd_%s\n",
> + __func__, vdd->voltdm.name, vdd-
> >voltdm.name);
> + return -EINVAL;
> + }
> +
> + if (!strcmp(vdd->voltdm.name, "mpu")) {
> + if (cpu_is_omap3630())
> + vdd->volt_data =
> omap36xx_vddmpu_volt_data;
> + else
> + vdd->volt_data =
> omap34xx_vddmpu_volt_data;
> +
> + vdd->vp_reg.tranxdone_status =
> OMAP3430_VP1_TRANXDONE_ST_MASK;
> + vdd->vc_reg.cmdval_reg =
> OMAP3_PRM_VC_CMD_VAL_0_OFFSET;
> + vdd->vc_reg.smps_sa_shift =
> OMAP3430_PRM_VC_SMPS_SA_SA0_SHIFT;
> + vdd->vc_reg.smps_sa_mask =
> OMAP3430_PRM_VC_SMPS_SA_SA0_MASK;
> + vdd->vc_reg.smps_volra_shift =
> OMAP3430_VOLRA0_SHIFT;
> + vdd->vc_reg.smps_volra_mask =
> OMAP3430_VOLRA0_MASK;
> + vdd->vc_reg.voltsetup_shift =
> OMAP3430_SETUP_TIME1_SHIFT;
> + vdd->vc_reg.voltsetup_mask =
> OMAP3430_SETUP_TIME1_MASK;
> + } else if (!strcmp(vdd->voltdm.name, "core")) {
> + if (cpu_is_omap3630())
> + vdd->volt_data =
> omap36xx_vddcore_volt_data;
> + else
> + vdd->volt_data =
> omap34xx_vddcore_volt_data;
> +
> + vdd->vp_reg.tranxdone_status =
> OMAP3430_VP2_TRANXDONE_ST_MASK;
> + vdd->vc_reg.cmdval_reg =
> OMAP3_PRM_VC_CMD_VAL_1_OFFSET;
> + vdd->vc_reg.smps_sa_shift =
> OMAP3430_PRM_VC_SMPS_SA_SA1_SHIFT;
> + vdd->vc_reg.smps_sa_mask =
> OMAP3430_PRM_VC_SMPS_SA_SA1_MASK;
> + vdd->vc_reg.smps_volra_shift =
> OMAP3430_VOLRA1_SHIFT;
> + vdd->vc_reg.smps_volra_mask =
> OMAP3430_VOLRA1_MASK;
> + vdd->vc_reg.voltsetup_shift =
> OMAP3430_SETUP_TIME2_SHIFT;
> + vdd->vc_reg.voltsetup_mask =
> OMAP3430_SETUP_TIME2_MASK;
> + } else {
> + pr_warning("%s: vdd_%s does not exisit in
> OMAP3\n",
> + __func__, vdd->voltdm.name);
> + return -EINVAL;
> + }
> +
> + /*
> + * Sys clk rate is require to calculate vp timeout
> value and
> + * smpswaittimemin and smpswaittimemax.
> + */
> + sys_ck = clk_get(NULL, "sys_ck");
> + if (IS_ERR(sys_ck)) {
> + pr_warning("%s: Could not get the sys clk to
> calculate"
> + "various vdd_%s params\n", __func__, vdd-
> >voltdm.name);
> + return -EINVAL;
> + }
> + sys_clk_speed = clk_get_rate(sys_ck);
> + clk_put(sys_ck);
> + /* Divide to avoid overflow */
> + sys_clk_speed /= 1000;
> +
> + /* Generic voltage parameters */
> + vdd->curr_volt = 1200000;
> + vdd->ocp_mod = OCP_MOD;
> + vdd->prm_irqst_reg = OMAP3_PRM_IRQSTATUS_MPU_OFFSET;
> + vdd->read_reg = omap3_voltage_read_reg;
> + vdd->write_reg = omap3_voltage_write_reg;
> + vdd->volt_scale = vp_forceupdate_scale_voltage;
> + vdd->vp_enabled = false;
> +
> + /* VC parameters */
> + vdd->vc_reg.prm_mod = OMAP3430_GR_MOD;
> + vdd->vc_reg.smps_sa_reg = OMAP3_PRM_VC_SMPS_SA_OFFSET;
> + vdd->vc_reg.smps_volra_reg =
> OMAP3_PRM_VC_SMPS_VOL_RA_OFFSET;
> + vdd->vc_reg.bypass_val_reg =
> OMAP3_PRM_VC_BYPASS_VAL_OFFSET;
> + vdd->vc_reg.voltsetup_reg =
> OMAP3_PRM_VOLTSETUP1_OFFSET;
> + vdd->vc_reg.data_shift = OMAP3430_DATA_SHIFT;
> + vdd->vc_reg.slaveaddr_shift =
> OMAP3430_SLAVEADDR_SHIFT;
> + vdd->vc_reg.regaddr_shift = OMAP3430_REGADDR_SHIFT;
> + vdd->vc_reg.valid = OMAP3430_VALID_MASK;
> + vdd->vc_reg.cmd_on_shift = OMAP3430_VC_CMD_ON_SHIFT;
> + vdd->vc_reg.cmd_on_mask = OMAP3430_VC_CMD_ON_MASK;
> + vdd->vc_reg.cmd_onlp_shift =
> OMAP3430_VC_CMD_ONLP_SHIFT;
> + vdd->vc_reg.cmd_ret_shift = OMAP3430_VC_CMD_RET_SHIFT;
> + vdd->vc_reg.cmd_off_shift = OMAP3430_VC_CMD_OFF_SHIFT;
> +
> + vdd->vp_reg.prm_mod = OMAP3430_GR_MOD;
> +
> + /* VPCONFIG bit fields */
> + vdd->vp_reg.vpconfig_erroroffset = (vdd->pmic_info-
> >vp_erroroffset <<
> + OMAP3430_ERROROFFSET_SHIFT);
> + vdd->vp_reg.vpconfig_errorgain_mask =
> OMAP3430_ERRORGAIN_MASK;
> + vdd->vp_reg.vpconfig_errorgain_shift =
> OMAP3430_ERRORGAIN_SHIFT;
> + vdd->vp_reg.vpconfig_initvoltage_shift =
> OMAP3430_INITVOLTAGE_SHIFT;
> + vdd->vp_reg.vpconfig_initvoltage_mask =
> OMAP3430_INITVOLTAGE_MASK;
> + vdd->vp_reg.vpconfig_timeouten =
> OMAP3430_TIMEOUTEN_MASK;
> + vdd->vp_reg.vpconfig_initvdd = OMAP3430_INITVDD_MASK;
> + vdd->vp_reg.vpconfig_forceupdate =
> OMAP3430_FORCEUPDATE_MASK;
> + vdd->vp_reg.vpconfig_vpenable =
> OMAP3430_VPENABLE_MASK;
> +
> + /* VSTEPMIN VSTEPMAX bit fields */
> + waittime = ((vdd->pmic_info->step_size / vdd-
> >pmic_info->slew_rate) *
> + sys_clk_speed) / 1000;
> + vdd->vp_reg.vstepmin_smpswaittimemin = waittime;
> + vdd->vp_reg.vstepmax_smpswaittimemax = waittime;
> + vdd->vp_reg.vstepmin_stepmin = vdd->pmic_info-
> >vp_vstepmin;
> + vdd->vp_reg.vstepmax_stepmax = vdd->pmic_info-
> >vp_vstepmax;
> + vdd->vp_reg.vstepmin_smpswaittimemin_shift =
> + OMAP3430_SMPSWAITTIMEMIN_SHIFT;
> + vdd->vp_reg.vstepmax_smpswaittimemax_shift =
> + OMAP3430_SMPSWAITTIMEMAX_SHIFT;
> + vdd->vp_reg.vstepmin_stepmin_shift =
> OMAP3430_VSTEPMIN_SHIFT;
> + vdd->vp_reg.vstepmax_stepmax_shift =
> OMAP3430_VSTEPMAX_SHIFT;
> +
> + /* VLIMITTO bit fields */
> + timeout_val = (sys_clk_speed * vdd->pmic_info-
> >vp_timeout_us) / 1000;
> + vdd->vp_reg.vlimitto_timeout = timeout_val;
> + vdd->vp_reg.vlimitto_vddmin = vdd->pmic_info-
> >vp_vddmin;
> + vdd->vp_reg.vlimitto_vddmax = vdd->pmic_info-
> >vp_vddmax;
> + vdd->vp_reg.vlimitto_vddmin_shift =
> OMAP3430_VDDMIN_SHIFT;
> + vdd->vp_reg.vlimitto_vddmax_shift =
> OMAP3430_VDDMAX_SHIFT;
> + vdd->vp_reg.vlimitto_timeout_shift =
> OMAP3430_TIMEOUT_SHIFT;
> +
> + return 0;
> +}

Big function. Recommend to simplify it by creating two data
configure functions, one for vp and one for vc. And call them
from omap3_vdd_data_configure().

> +
> +/* Public functions */
> +/**
> + * omap_voltage_get_nom_volt() - Gets the current non-auto-
> compensated voltage
> + * @voltdm: pointer to the VDD for which current
> voltage info is needed
> + *
> + * API to get the current non-auto-compensated voltage for
> a VDD.
> + * Returns 0 in case of error else returns the current
> voltage for the VDD.
> + */
> +unsigned long omap_voltage_get_nom_volt(struct
> voltagedomain *voltdm)
> +{
> + struct omap_vdd_info *vdd;
> +
> + if (!voltdm || IS_ERR(voltdm)) {
> + pr_warning("%s: VDD specified does not
> exist!\n", __func__);
> + return 0;
> + }
> +
> + vdd = container_of(voltdm, struct omap_vdd_info,
> voltdm);
> +
> + return vdd->curr_volt;
> +}
> +
> +/**
> + * omap_vp_get_curr_volt() - API to get the current vp
> voltage.
> + * @voltdm: pointer to the VDD.
> + *
> + * This API returns the current voltage for the specified
> voltage processor
> + */
> +unsigned long omap_vp_get_curr_volt(struct voltagedomain
> *voltdm)
> +{
> + struct omap_vdd_info *vdd;
> + u8 curr_vsel;
> +
> + if (!voltdm || IS_ERR(voltdm)) {
> + pr_warning("%s: VDD specified does not
> exist!\n", __func__);
> + return 0;
> + }
> +
> + vdd = container_of(voltdm, struct omap_vdd_info,
> voltdm);
> + if (!vdd->read_reg) {
> + pr_err("%s: No read API for reading vdd_%s
> regs\n",
> + __func__, voltdm->name);
> + return 0;
> + }
> +
> + curr_vsel = vdd->read_reg(vdd->vp_reg.prm_mod,
> + vdd->vp_offs.voltage);
> +
> + if (!vdd->pmic_info || !vdd->pmic_info->vsel_to_uv) {
> + pr_warning("%s: PMIC function to convert vsel to
> voltage"
> + "in uV not registerd\n", __func__);
> + return 0;
> + }
> +
> + return vdd->pmic_info->vsel_to_uv(curr_vsel);
> +}
> +
> +/**
> + * omap_vp_enable() - API to enable a particular VP
> + * @voltdm: pointer to the VDD whose VP is to be
> enabled.
> + *
> + * This API enables a particular voltage processor. Needed
> by the smartreflex
> + * class drivers.
> + */
> +void omap_vp_enable(struct voltagedomain *voltdm)
> +{
> + struct omap_vdd_info *vdd;
> + u32 vpconfig;
> + u16 mod;
> +
> + if (!voltdm || IS_ERR(voltdm)) {
> + pr_warning("%s: VDD specified does not
> exist!\n", __func__);
> + return;
> + }
> +
> + vdd = container_of(voltdm, struct omap_vdd_info,
> voltdm);
> + if (!vdd->read_reg || !vdd->write_reg) {
> + pr_err("%s: No read/write API for accessing
> vdd_%s regs\n",
> + __func__, voltdm->name);
> + return;
> + }
> +
> + mod = vdd->vp_reg.prm_mod;
> +
> + /* If VP is already enabled, do nothing. Return */
> + if (vdd->vp_enabled)
> + return;
> +
> + vp_latch_vsel(vdd);
> +
> + /* Enable VP */
> + vpconfig = vdd->read_reg(mod, vdd->vp_offs.vpconfig);
> + vpconfig |= vdd->vp_reg.vpconfig_vpenable;
> + vdd->write_reg(vpconfig, mod, vdd->vp_offs.vpconfig);

Check if there is a need for delay or status check (similar to
omap_vp_disable) before flipping the status to true.

> + vdd->vp_enabled = true;
> +}
> +
> +/**
> + * omap_vp_disable() - API to disable a particular VP
> + * @voltdm: pointer to the VDD whose VP is to be
> disabled.
> + *
> + * This API disables a particular voltage processor. Needed
> by the smartreflex
> + * class drivers.
> + */
> +void omap_vp_disable(struct voltagedomain *voltdm)
> +{
> + struct omap_vdd_info *vdd;
> + u32 vpconfig;
> + u16 mod;
> + int timeout;
> +
> + if (!voltdm || IS_ERR(voltdm)) {
> + pr_warning("%s: VDD specified does not
> exist!\n", __func__);
> + return;
> + }
> +
> + vdd = container_of(voltdm, struct omap_vdd_info,
> voltdm);
> + if (!vdd->read_reg || !vdd->write_reg) {
> + pr_err("%s: No read/write API for accessing
> vdd_%s regs\n",
> + __func__, voltdm->name);
> + return;
> + }
> +
> + mod = vdd->vp_reg.prm_mod;
> +
> + /* If VP is already disabled, do nothing. Return */
> + if (!vdd->vp_enabled) {
> + pr_warning("%s: Trying to disable VP for vdd_%s
> when"
> + "it is already disabled\n", __func__,
> voltdm->name);
> + return;
> + }
> +
> + /* Disable VP */
> + vpconfig = vdd->read_reg(mod, vdd->vp_offs.vpconfig);
> + vpconfig &= ~vdd->vp_reg.vpconfig_vpenable;
> + vdd->write_reg(vpconfig, mod, vdd->vp_offs.vpconfig);
> +
> + /*
> + * Wait for VP idle Typical latency is <2us. Maximum
> latency is ~100us
> + */
> + omap_test_timeout((vdd->read_reg(mod, vdd-
> >vp_offs.vstatus)),
> + VP_IDLE_TIMEOUT, timeout);
> +
> + if (timeout >= VP_IDLE_TIMEOUT)
> + pr_warning("%s: vdd_%s idle timedout\n",
> + __func__, voltdm->name);
> +
> + vdd->vp_enabled = false;
> +
> + return;
> +}
> +
> +/**
> + * omap_voltage_scale_vdd() - API to scale voltage of a
> particular
> + * voltage domain.
> + * @voltdm: pointer to the VDD which is to be scaled.
> + * @target_volt: The target voltage of the voltage domain
> + *
> + * This API should be called by the kernel to do the
> voltage scaling
> + * for a particular voltage domain during dvfs or any other
> situation.
> + */
> +int omap_voltage_scale_vdd(struct voltagedomain *voltdm,
> + unsigned long target_volt)
> +{
> + struct omap_vdd_info *vdd;
> +
> + if (!voltdm || IS_ERR(voltdm)) {
> + pr_warning("%s: VDD specified does not
> exist!\n", __func__);
> + return -EINVAL;
> + }
> +
> + vdd = container_of(voltdm, struct omap_vdd_info,
> voltdm);
> +
> + if (!vdd->volt_scale) {
> + pr_err("%s: No voltage scale API registered for
> vdd_%s\n",
> + __func__, voltdm->name);
> + return -ENODATA;
> + }
> +
> + return vdd->volt_scale(vdd, target_volt);
> +}
> +
> +/**
> + * omap_voltage_reset() - Resets the voltage of a
> particular voltage domain
> + * to that of the current OPP.
> + * @voltdm: pointer to the VDD whose voltage is to be
> reset.
> + *
> + * This API finds out the correct voltage the voltage
> domain is supposed
> + * to be at and resets the voltage to that level. Should be
> used expecially
> + * while disabling any voltage compensation modules.
> + */
> +void omap_voltage_reset(struct voltagedomain *voltdm)
> +{
> + unsigned long target_uvdc;
> +
> + if (!voltdm || IS_ERR(voltdm)) {
> + pr_warning("%s: VDD specified does not
> exist!\n", __func__);
> + return;
> + }
> +
> + target_uvdc = omap_voltage_get_nom_volt(voltdm);
> + if (!target_uvdc) {
> + pr_err("%s: unable to find current voltage for
> vdd_%s\n",
> + __func__, voltdm->name);
> + return;
> + }
> +
> + omap_voltage_scale_vdd(voltdm, target_uvdc);
> +}
> +
> +/**
> + * omap_voltage_get_volttable() - API to get the voltage
> table associated with a
> + * particular voltage domain.
> + * @voltdm: pointer to the VDD for which the voltage
> table is required
> + * @volt_data: the voltage table for the particular vdd
> which is to be
> + * populated by this API
> + *
> + * This API populates the voltage table associated with a
> VDD into the
> + * passed parameter pointer. Returns the count of distinct
> voltages
> + * supported by this vdd.
> + *
> + */
> +void omap_voltage_get_volttable(struct voltagedomain
> *voltdm,
> + struct omap_volt_data **volt_data)
> +{
> + struct omap_vdd_info *vdd;
> +
> + if (!voltdm || IS_ERR(voltdm)) {
> + pr_warning("%s: VDD specified does not
> exist!\n", __func__);
> + return;
> + }
> +
> + vdd = container_of(voltdm, struct omap_vdd_info,
> voltdm);
> +
> + *volt_data = vdd->volt_data;
> +}
> +
> +/**
> + * omap_voltage_get_voltdata() - API to get the voltage
> table entry for a
> + * particular voltage
> + * @voltdm: pointer to the VDD whose voltage table has
> to be searched
> + * @volt: the voltage to be searched in the voltage table
> + *
> + * This API searches through the voltage table for the
> required voltage
> + * domain and tries to find a matching entry for the passed
> voltage volt.
> + * If a matching entry is found volt_data is populated with
> that entry.
> + * This API searches only through the non-compensated
> voltages int the
> + * voltage table.
> + * Returns pointer to the voltage table entry corresponding
> to volt on
> + * sucess. Returns -ENODATA if no voltage table exisits for
> the passed voltage
> + * domain or if there is no matching entry.
> + */
> +struct omap_volt_data *omap_voltage_get_voltdata(struct
> voltagedomain *voltdm,
> + unsigned long volt)
> +{
> + struct omap_vdd_info *vdd;
> + int i;
> +
> + if (!voltdm || IS_ERR(voltdm)) {
> + pr_warning("%s: VDD specified does not
> exist!\n", __func__);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + vdd = container_of(voltdm, struct omap_vdd_info,
> voltdm);
> +
> + if (!vdd->volt_data) {
> + pr_warning("%s: voltage table does not exist for
> vdd_%s\n",
> + __func__, voltdm->name);
> + return ERR_PTR(-ENODATA);
> + }
> +
> + for (i = 0; vdd->volt_data[i].volt_nominal != 0; i++)
> {
> + if (vdd->volt_data[i].volt_nominal == volt)
> + return &vdd->volt_data[i];
> + }
> +
> + pr_notice("%s: Unable to match the current voltage
> with the voltage"
> + "table for vdd_%s\n", __func__, voltdm->name);
> +
> + return ERR_PTR(-ENODATA);
> +}
> +
> +/**
> + * omap_voltage_register_pmic() - API to register PMIC
> specific data
> + * @voltdm: pointer to the VDD for which the PMIC
> specific data is
> + * to be registered
> + * @pmic_info: the structure containing pmic info
> + *
> + * This API is to be called by the SOC/PMIC file to specify
> the
> + * pmic specific info as present in omap_volt_pmic_info
> structure.
> + */
> +int omap_voltage_register_pmic(struct voltagedomain
> *voltdm,
> + struct omap_volt_pmic_info *pmic_info)
> +{
> + struct omap_vdd_info *vdd;
> +
> + if (!voltdm || IS_ERR(voltdm)) {
> + pr_warning("%s: VDD specified does not
> exist!\n", __func__);
> + return -EINVAL;
> + }
> +
> + vdd = container_of(voltdm, struct omap_vdd_info,
> voltdm);
> +
> + vdd->pmic_info = pmic_info;
> +
> + return 0;
> +}
> +
> +/**
> + * omap_voltage_get_dbgdir() - API to get pointer to the
> debugfs directory
> + * corresponding to a voltage domain.
> + *
> + * @voltdm: pointer to the VDD whose debug directory
> is required.
> + *
> + * This API returns pointer to the debugfs directory
> corresponding
> + * to the voltage domain. Should be used by drivers
> requiring to
> + * add any debug entry for a particular voltage domain.
> Returns NULL
> + * in case of error.
> + */
> +struct dentry *omap_voltage_get_dbgdir(struct voltagedomain
> *voltdm)
> +{
> + struct omap_vdd_info *vdd;
> +
> + if (!voltdm || IS_ERR(voltdm)) {
> + pr_warning("%s: VDD specified does not
> exist!\n", __func__);
> + return NULL;
> + }
> +
> + vdd = container_of(voltdm, struct omap_vdd_info,
> voltdm);
> +
> + return vdd->debug_dir;
> +}
> +
> +/**
> + * omap_change_voltscale_method() - API to change the
> voltage scaling method.
> + * @voltdm: pointer to the VDD whose voltage scaling
> method
> + * has to be changed.
> + * @voltscale_method: the method to be used for voltage
> scaling.
> + *
> + * This API can be used by the board files to change the
> method of voltage
> + * scaling between vpforceupdate and vcbypass. The
> parameter values are
> + * defined in voltage.h
> + */
> +void omap_change_voltscale_method(struct voltagedomain
> *voltdm,
> + int voltscale_method)

Recommend to change the return type to int for better error
handling.

> +{
> + struct omap_vdd_info *vdd;
> +
> + if (!voltdm || IS_ERR(voltdm)) {
> + pr_warning("%s: VDD specified does not
> exist!\n", __func__);
> + return;
> + }
> +
> + vdd = container_of(voltdm, struct omap_vdd_info,
> voltdm);
> +
> + switch (voltscale_method) {
> + case VOLTSCALE_VPFORCEUPDATE:
> + vdd->volt_scale = vp_forceupdate_scale_voltage;
> + return;
> + case VOLTSCALE_VCBYPASS:
> + vdd->volt_scale = vc_bypass_scale_voltage;
> + return;
> + default:
> + pr_warning("%s: Trying to change the method of
> voltage scaling"
> + "to an unsupported one!\n", __func__);
> + }
> +}
> +
> +/**
> + * omap_voltage_domain_lookup() - API to get the voltage
> domain pointer
> + * @name: Name of the voltage domain
> + *
> + * This API looks up in the global vdd_info struct for the
> + * existence of voltage domain <name>. If it exists, the
> API returns
> + * a pointer to the voltage domain structure corresponding
> to the
> + * VDD<name>. Else retuns error pointer.
> + */
> +struct voltagedomain *omap_voltage_domain_lookup(char
> *name)
> +{
> + int i;
> +
> + if (!vdd_info) {
> + pr_err("%s: Voltage driver init not yet
> happened.Faulting!\n",
> + __func__);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + if (!name) {
> + pr_err("%s: No name to get the votage
> domain!\n", __func__);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + for (i = 0; i < nr_scalable_vdd; i++) {
> + if (!(strcmp(name, vdd_info[i].voltdm.name)))
> + return &vdd_info[i].voltdm;
> + }

Print an error before returning.

> +
> + return ERR_PTR(-EINVAL);
> +}
> +
> +/**
> + * omap_voltage_late_init() - Init the various voltage
> parameters
> + *
> + * This API is to be called in the later stages of the
> + * system boot to init the voltage controller and
> + * voltage processors.
> + */
> +int __init omap_voltage_late_init(void)
> +{
> + int i;
> +
> + if (!vdd_info) {
> + pr_err("%s: Voltage driver support not added\n",
> + __func__);
> + return -EINVAL;
> + }
> +
> + voltage_dir = debugfs_create_dir("voltage", NULL);
> + if (IS_ERR(voltage_dir))
> + pr_err("%s: Unable to create voltage debugfs
> main dir\n",
> + __func__);
> + for (i = 0; i < nr_scalable_vdd; i++) {
> + if (vdd_data_configure(&vdd_info[i]))
> + continue;

Do error checking and print success/ fail message (especially
fail) for each of vc_init, vp_init & vdd_debugfs_init.
May need to change the return types of those functions.

A failure should stop further initialization.

> + vc_init(&vdd_info[i]);
> + vp_init(&vdd_info[i]);
> + vdd_debugfs_init(&vdd_info[i]);
> + }
> +
> + return 0;
> +}
> +
> +/**
> + * omap_voltage_early_init()- Volatage driver early init
> + */
> +static int __init omap_voltage_early_init(void)
> +{
> + if (cpu_is_omap34xx()) {
> + vdd_info = omap3_vdd_info;
> + nr_scalable_vdd = OMAP3_NR_SCALABLE_VDD;
> + vc_init = omap3_vc_init;
> + vdd_data_configure = omap3_vdd_data_configure;
> + } else {
> + pr_warning("%s: voltage driver support not
> added\n", __func__);
> + }
> +
> + return 0;
> +}

Recommend to reorder the functions based on logical grouping
of related functionalities e.g. all init/ configure functions
can be listed together. A comment explaining these different
groups and their constituent functions (located near the
beginning) will be useful.

> +core_initcall(omap_voltage_early_init);
> diff --git a/arch/arm/plat-omap/include/plat/voltage.h
> b/arch/arm/plat-omap/include/plat/voltage.h
> new file mode 100644
> index 0000000..2f4f59a
> --- /dev/null
> +++ b/arch/arm/plat-omap/include/plat/voltage.h
> @@ -0,0 +1,134 @@
> +/*
> + * OMAP Voltage Management Routines
> + *
> + * Author: Thara Gopinath <thara@ti.com>
> + *
> + * Copyright (C) 2009 Texas Instruments, Inc.
> + * Thara Gopinath <thara@ti.com>
> + *
> + * 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.
> + */
> +
> +#ifndef __ARCH_ARM_MACH_OMAP2_VOLTAGE_H
> +#define __ARCH_ARM_MACH_OMAP2_VOLTAGE_H
> +
> +#define VOLTSCALE_VPFORCEUPDATE 1
> +#define VOLTSCALE_VCBYPASS 2

Add comment explaining the above #defines

> +
> +/*
> + * OMAP3 GENERIC setup times. Revisit to see if these needs
> to be
> + * passed from board or PMIC file
> + */
> +#define OMAP3_CLKSETUP 0xff
> +#define OMAP3_VOLTOFFSET 0xff
> +#define OMAP3_VOLTSETUP2 0xff
> +
> +/* Voltage value defines */
> +#define OMAP3430_VDD_MPU_OPP1_UV 975000
> +#define OMAP3430_VDD_MPU_OPP2_UV 1075000
> +#define OMAP3430_VDD_MPU_OPP3_UV 1200000
> +#define OMAP3430_VDD_MPU_OPP4_UV 1270000
> +#define OMAP3430_VDD_MPU_OPP5_UV 1350000
> +
> +#define OMAP3430_VDD_CORE_OPP1_UV 975000
> +#define OMAP3430_VDD_CORE_OPP2_UV 1050000
> +#define OMAP3430_VDD_CORE_OPP3_UV 1150000
> +
> +#define OMAP3630_VDD_MPU_OPP50_UV 1012500
> +#define OMAP3630_VDD_MPU_OPP100_UV 1200000
> +#define OMAP3630_VDD_MPU_OPP120_UV 1325000
> +#define OMAP3630_VDD_MPU_OPP1G_UV 1375000
> +
> +#define OMAP3630_VDD_CORE_OPP50_UV 1000000
> +#define OMAP3630_VDD_CORE_OPP100_UV 1200000
> +
> +/**
> + * struct voltagedomain - omap voltage domain global
> structure.
> + * @name: Name of the voltage domain which can be used as
> a unique
> + * identifier.
> + */
> +struct voltagedomain {
> + char *name;
> +};
> +
> +/* API to get the voltagedomain pointer */
> +struct voltagedomain *omap_voltage_domain_lookup(char
> *name);
> +
> +/**
> + * struct omap_volt_data - Omap voltage specific data.
> + * @voltage_nominal: The possible voltage value in uV
> + * @sr_efuse_offs: The offset of the efuse
> register(from system
> + * control module base address) from where to
> read
> + * the n-target value for the smartreflex
> module.
> + * @sr_errminlimit: Error min limit value for
> smartreflex. This value
> + * differs at differnet opp and thus is
> linked
> + * with voltage.
> + * @vp_errorgain: Error gain value for the voltage
> processor. This
> + * field also differs according to the
> voltage/opp.
> + */
> +struct omap_volt_data {
> + u32 volt_nominal;
> + u32 sr_efuse_offs;
> + u8 sr_errminlimit;
> + u8 vp_errgain;
> +};
> +
> +/**
> + * struct omap_volt_pmic_info - PMIC specific data required
> by voltage driver.
> + * @slew_rate: PMIC slew rate (in uv/us)
> + * @step_size: PMIC voltage step size (in uv)
> + * @vsel_to_uv: PMIC API to convert vsel value to actual
> voltage in uV.
> + * @uv_to_vsel: PMIC API to convert voltage in uV to vsel
> value.

Add comments for the remaining data members.

Best Regards,

Anand Sawant

> + */
> +struct omap_volt_pmic_info {
> + int slew_rate;
> + int step_size;
> + u32 on_volt;
> + u32 onlp_volt;
> + u32 ret_volt;
> + u32 off_volt;
> + u16 volt_setup_time;
> + u8 vp_erroroffset;
> + u8 vp_vstepmin;
> + u8 vp_vstepmax;
> + u8 vp_vddmin;
> + u8 vp_vddmax;
> + u8 vp_timeout_us;
> + u8 i2c_slave_addr;
> + u8 pmic_reg;
> + unsigned long (*vsel_to_uv) (const u8 vsel);
> + u8 (*uv_to_vsel) (unsigned long uV);
> +};
> +
> +unsigned long omap_vp_get_curr_volt(struct voltagedomain
> *voltdm);
> +void omap_vp_enable(struct voltagedomain *voltdm);
> +void omap_vp_disable(struct voltagedomain *voltdm);
> +int omap_voltage_scale_vdd(struct voltagedomain *voltdm,
> + unsigned long target_volt);
> +void omap_voltage_reset(struct voltagedomain *voltdm);
> +void omap_voltage_get_volttable(struct voltagedomain
> *voltdm,
> + struct omap_volt_data **volt_data);
> +struct omap_volt_data *omap_voltage_get_voltdata(struct
> voltagedomain *voltdm,
> + unsigned long volt);
> +unsigned long omap_voltage_get_nom_volt(struct
> voltagedomain *voltdm);
> +struct dentry *omap_voltage_get_dbgdir(struct voltagedomain
> *voltdm);
> +#ifdef CONFIG_PM
> +int omap_voltage_register_pmic(struct voltagedomain
> *voltdm,
> + struct omap_volt_pmic_info *pmic_info);
> +void omap_change_voltscale_method(struct voltagedomain
> *voltdm,
> + int voltscale_method);
> +int omap_voltage_late_init(void);
> +#else
> +static inline int omap_voltage_register_pmic(struct
> voltagedomain *voltdm,
> + struct omap_volt_pmic_info *pmic_info) {}
> +static inline void omap_change_voltscale_method(struct
> voltagedomain *voltdm,
> + int voltscale_method) {}
> +static inline int omap_voltage_late_init(void)
> +{
> + return -EINVAL;
> +}
> +#endif
> +
> +#endif
> --
> 1.7.0.4