Thara, Here are some comments on the voltage layer; many of them are cosmetic in nature. > -----Original Message----- > From: Thara Gopinath [mailto:thara@xxxxxx] > Sent: Monday, December 20, 2010 10:29 PM > To: linux-omap@xxxxxxxxxxxxxxx; linux-arm- > kernel@xxxxxxxxxxxxxxxxxxx > Cc: khilman@xxxxxxxxxxxxxxxxxxx; paul@xxxxxxxxx; b- > cousson@xxxxxx; vishwanath.bs@xxxxxx; sawant@xxxxxx; > nm@xxxxxx; 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@xxxxxx> > --- > 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@xxxxxx> > + * > + * Copyright (C) 2007 Texas Instruments, Inc. > + * Rajendra Nayak <rnayak@xxxxxx> > + * Lesly A M <x0080970@xxxxxx> > + * > + * Copyright (C) 2008 Nokia Corporation > + * Kalle Jokiniemi > + * > + * Copyright (C) 2010 Texas Instruments, Inc. > + * Thara Gopinath <thara@xxxxxx> > + * > + * 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, > ¤t_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, > ¤t_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@xxxxxx> > + * > + * Copyright (C) 2009 Texas Instruments, Inc. > + * Thara Gopinath <thara@xxxxxx> > + * > + * 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 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
