Re: [PATCH 1/8] OMAP3: PM: Adding voltage driver support for OMAP3

2010-06-23 Thread Kevin Hilman
Thara Gopinath th...@ti.com writes:

 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 also 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
 n-target value, errminlimit and voltage processor errorgain.
 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.

 Signed-off-by: Thara Gopinath th...@ti.com

First some general comments:

I thought we had agreed that all the internal functions should not
need to take a VDD ID, but instead they could be just passed a
vdd_info pointer.

I would greatly improve readability to see all usage of
vdd_info[vdd_id] go away.

In the exported functions that take vdd_id as an argument, just do
something like

  struct omap_vdd_info *vdd = vdd_info[vdd_id];

at the beginning, then replace all the instances of vdd_info[vdd_id]
with 'vdd-'

In the rest of the internal functions, make them take the pointer as
the argument instead of the id.

Also, we have a bunch of stuff in the current pm-vc branch which allows
boards to override settings.  Are you planning to address that in this
series?  or is Lesly going to continue that work?

Some other comments below...

 ---
  arch/arm/mach-omap2/Makefile  |3 +-
  arch/arm/mach-omap2/voltage.c | 1059 
 +
  arch/arm/mach-omap2/voltage.h |  123 +
  3 files changed, 1184 insertions(+), 1 deletions(-)
  create mode 100644 arch/arm/mach-omap2/voltage.c
  create mode 100644 arch/arm/mach-omap2/voltage.h

 diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
 index e975b43..e4c660d 100644
 --- a/arch/arm/mach-omap2/Makefile
 +++ b/arch/arm/mach-omap2/Makefile
 @@ -46,7 +46,8 @@ obj-$(CONFIG_ARCH_OMAP2)+= sdrc2xxx.o
  ifeq ($(CONFIG_PM),y)
  obj-$(CONFIG_ARCH_OMAP2) += pm24xx.o
  obj-$(CONFIG_ARCH_OMAP2) += sleep24xx.o
 -obj-$(CONFIG_ARCH_OMAP3) += pm34xx.o sleep34xx.o cpuidle34xx.o
 +obj-$(CONFIG_ARCH_OMAP3) += pm34xx.o sleep34xx.o cpuidle34xx.o \
 +voltage.o
  obj-$(CONFIG_PM_DEBUG)   += pm-debug.o
  
  AFLAGS_sleep24xx.o   :=-Wa,-march=armv6
 diff --git a/arch/arm/mach-omap2/voltage.c b/arch/arm/mach-omap2/voltage.c
 new file mode 100644
 index 000..657e322
 --- /dev/null
 +++ b/arch/arm/mach-omap2/voltage.c
 @@ -0,0 +1,1059 @@
 +/*
 + * OMAP3/OMAP4 Voltage Management Routines
 + *
 + * Author: Thara Gopinathth...@ti.com
 + *
 + * Copyright (C) 2007 Texas Instruments, Inc.
 + * Rajendra Nayak rna...@ti.com
 + * Lesly A M x0080...@ti.com
 + *
 + * Copyright (C) 2008 Nokia Corporation
 + * Kalle Jokiniemi
 + *
 + * Copyright (C) 2010 Texas Instruments, Inc.
 + * Thara Gopinath th...@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/pm.h
 +#include linux/delay.h
 +#include linux/io.h
 +#include linux/clk.h
 +#include linux/err.h
 +
 +#include plat/omap-pm.h
 +#include plat/omap34xx.h
 +#include plat/opp.h
 +#include plat/opp_twl_tps.h
 +#include plat/clock.h
 +#include plat/common.h
 +
 +#include prm-regbits-34xx.h
 +#include voltage.h
 +
 +#define VP_IDLE_TIMEOUT  200
 +#define VP_TRANXDONE_TIMEOUT 300
 +
 +/* PRM voltage module */
 +u32 volt_mod;

should be static

 +/* Voltage processor register offsets */
 +struct vp_reg_offs {
 + u8 vpconfig_reg;
 + u8 vstepmin_reg;
 + u8 vstepmax_reg;
 + u8 vlimitto_reg;
 + u8 status_reg;
 + u8 voltage_reg;
 +};

Minor issue, but the _reg suffix is not really needed on all these
names.

[...]

 +
 +/* Generic voltage init functions */
 +static void __init init_voltageprocesor(int vp_id)

was mystified why I wasn't finding this function when grepping and
discovered there should be two 's's in processor in this function name.

[...]

 diff --git a/arch/arm/mach-omap2/voltage.h b/arch/arm/mach-omap2/voltage.h
[...]

For the naming, rather than the long names voltagecontroller,
voltageprocessor, I'd suggest just using vc and vp.  For the external
functions, they can have the omap_ prefix.

 +unsigned long omap_voltageprocessor_get_curr_volt(int vp_id);
 +void omap_voltageprocessor_enable(int vp_id);
 +void omap_voltageprocessor_disable(int vp_id);

these 

Re: [PATCH 1/8] OMAP3: PM: Adding voltage driver support for OMAP3

2010-06-23 Thread Kevin Hilman
Thara Gopinath th...@ti.com writes:

 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 also 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
 n-target value, errminlimit and voltage processor errorgain.
 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.

 Signed-off-by: Thara Gopinath th...@ti.com

[...]

 +/**
 + * get_curr_vdd_voltage : Gets the current non-auto-compensated voltage
 + * @vdd  : 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 get_curr_voltage(int vdd)
 +{
 + struct omap_opp *opp;
 + unsigned long freq;
 +
 + if (check_voltage_domain(vdd)) {
 + pr_warning(%s: VDD %d does not exist!\n, __func__, vdd);
 + return 0;
 + }
 +
 + freq = vdd_info[vdd].volt_clk-rate;
 + opp = opp_find_freq_ceil(vdd_info[vdd].opp_type, freq);
 + if (IS_ERR(opp)) {
 + pr_warning(%s: Unable to find OPP for VDD%d freq%ld\n,
 + __func__, vdd + 1, freq);
 + return 0;
 + }
 +
 + /*
 +  * Use higher freq voltage even if an exact match is not available
 +  * we are probably masking a clock framework bug, so warn
 +  */
 + if (unlikely(freq != vdd_info[vdd].volt_clk-rate))
 + pr_warning(%s: Available freq %ld != dpll freq %ld.\n,
 + __func__, freq, vdd_info[vdd].volt_clk-rate);
 +
 + return opp_get_voltage(opp);
 +}

Rather than having to do an OPP lookup each time based on current clock
rate, why not just store the current voltage into vdd_info and
update it whenever it changes.  This function could then just return
that value.

Kevin
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/8] OMAP3: PM: Adding voltage driver support for OMAP3

2010-05-29 Thread Thara Gopinath
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 also 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
n-target value, errminlimit and voltage processor errorgain.
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.

Signed-off-by: Thara Gopinath th...@ti.com
---
 arch/arm/mach-omap2/Makefile  |3 +-
 arch/arm/mach-omap2/voltage.c | 1059 +
 arch/arm/mach-omap2/voltage.h |  123 +
 3 files changed, 1184 insertions(+), 1 deletions(-)
 create mode 100644 arch/arm/mach-omap2/voltage.c
 create mode 100644 arch/arm/mach-omap2/voltage.h

diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index e975b43..e4c660d 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -46,7 +46,8 @@ obj-$(CONFIG_ARCH_OMAP2)  += sdrc2xxx.o
 ifeq ($(CONFIG_PM),y)
 obj-$(CONFIG_ARCH_OMAP2)   += pm24xx.o
 obj-$(CONFIG_ARCH_OMAP2)   += sleep24xx.o
-obj-$(CONFIG_ARCH_OMAP3)   += pm34xx.o sleep34xx.o cpuidle34xx.o
+obj-$(CONFIG_ARCH_OMAP3)   += pm34xx.o sleep34xx.o cpuidle34xx.o \
+  voltage.o
 obj-$(CONFIG_PM_DEBUG) += pm-debug.o
 
 AFLAGS_sleep24xx.o :=-Wa,-march=armv6
diff --git a/arch/arm/mach-omap2/voltage.c b/arch/arm/mach-omap2/voltage.c
new file mode 100644
index 000..657e322
--- /dev/null
+++ b/arch/arm/mach-omap2/voltage.c
@@ -0,0 +1,1059 @@
+/*
+ * OMAP3/OMAP4 Voltage Management Routines
+ *
+ * Author: Thara Gopinath  th...@ti.com
+ *
+ * Copyright (C) 2007 Texas Instruments, Inc.
+ * Rajendra Nayak rna...@ti.com
+ * Lesly A M x0080...@ti.com
+ *
+ * Copyright (C) 2008 Nokia Corporation
+ * Kalle Jokiniemi
+ *
+ * Copyright (C) 2010 Texas Instruments, Inc.
+ * Thara Gopinath th...@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/pm.h
+#include linux/delay.h
+#include linux/io.h
+#include linux/clk.h
+#include linux/err.h
+
+#include plat/omap-pm.h
+#include plat/omap34xx.h
+#include plat/opp.h
+#include plat/opp_twl_tps.h
+#include plat/clock.h
+#include plat/common.h
+
+#include prm-regbits-34xx.h
+#include voltage.h
+
+#define VP_IDLE_TIMEOUT200
+#define VP_TRANXDONE_TIMEOUT   300
+
+/* PRM voltage module */
+u32 volt_mod;
+
+/* Voltage processor register offsets */
+struct vp_reg_offs {
+   u8 vpconfig_reg;
+   u8 vstepmin_reg;
+   u8 vstepmax_reg;
+   u8 vlimitto_reg;
+   u8 status_reg;
+   u8 voltage_reg;
+};
+
+/* Voltage Processor bit field values, shifts and masks */
+struct vp_reg_val {
+   /* 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 */
+   u16 vlimitto_vddmin;
+   u16 vlimitto_vddmax;
+   u16 vlimitto_timeout;
+   u16 vlimitto_vddmin_shift;
+   u16 vlimitto_vddmax_shift;
+   u16 vlimitto_timeout_shift;
+   /* PRM_IRQSTATUS*/
+   u32 tranxdone_status;
+};
+
+/**
+ * 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.
+ * @vp_offs: structure containing the offsets for various
+ *   vp registers
+ * @vp_reg : the register values, shifts, masks for various
+ *   vp registers
+ * @volt_clk   : the clock associated with the vdd.
+ * @opp_type   : the type of OPP associated with this vdd.
+ * @volt_data_count: Number of distinct