Re: [PATCH 02/16] OMAP3: PM: Create list to keep track of various smartreflex instances.

Fri, 26 Feb 2010 15:21:27 -0800 

Gopinath, Thara wrote:

This patch removes the pointer sr1, sr2 in smartreflex.c and
instead creatse a list for keeping track of multiple smartreflex
instances.. This makes it scalable for next gen OMAPs where there
are more than two smartreflex modules.

Signed-off-by: Thara Gopinath <th...@ti.com>
---
 arch/arm/mach-omap2/smartreflex.c |  114 ++++++++++++++++++++++++------------
 1 files changed, 76 insertions(+), 38 deletions(-)

diff --git a/arch/arm/mach-omap2/smartreflex.c 
b/arch/arm/mach-omap2/smartreflex.c
index 1c5ec37..4a9c2e2 100644
--- a/arch/arm/mach-omap2/smartreflex.c
+++ b/arch/arm/mach-omap2/smartreflex.c
@@ -26,6 +26,7 @@
 #include <linux/kobject.h>
 #include <linux/i2c/twl.h>
 #include <linux/io.h>
+#include <linux/list.h>
#include <plat/omap34xx.h> 
 #include <plat/control.h>
@@ -51,9 +52,12 @@ struct omap_sr {
        u32             opp5_nvalue;
        u32             senp_mod, senn_mod;
        void __iomem    *srbase_addr;
-       void __iomem    *vpbase_addr;
+       struct list_head        node;
 };
+/* sr_list contains all the instances of smartreflex module */ 
+static LIST_HEAD(sr_list);
+
 #define SR_REGADDR(offs)       (sr->srbase_addr + offset)
static inline void sr_write_reg(struct omap_sr *sr, unsigned offset, u32 value) 
@@ -78,6 +82,20 @@ static inline u32 sr_read_reg(struct omap_sr *sr, unsigned 
offset)
        return __raw_readl(SR_REGADDR(offset));
 }
+static struct omap_sr *_sr_lookup(int srid) 
+{
+       struct omap_sr *sr_info, *temp_sr_info;
+
+       sr_info = NULL;
+       list_for_each_entry(temp_sr_info, &sr_list, node) {
+               if (srid == temp_sr_info->srid) {
+                       sr_info = temp_sr_info;
+                       break;
+               }
+       }
+       return sr_info;
+}
+
 static int sr_clk_enable(struct omap_sr *sr)
 {
        if (clk_enable(sr->clk) != 0) {
@@ -151,11 +169,17 @@ static u8 get_vdd1_opp(void)
 {
        struct omap_opp *opp;
        unsigned long freq;
+       struct omap_sr *sr_info = _sr_lookup(SR1);
Why has the list implementation been chosen over individually registered platform devices? There seems to be a trend lately in device drivers to have a single platform device with either an array of pointers or a list implementation to track the multiple instances of the same device on a board. McBSP is another example of this approach.
Why is that? Is it a bad idea to have each SR instance be a separate platform device? Please let me know if my question is nuts. 

Mike
- if (sr1.vdd_opp_clk == NULL || IS_ERR(sr1.vdd_opp_clk)) 
+       if (!sr_info) {
+               pr_warning("omap_sr struct corresponding to SR1 not found\n");
+               return 0;
+       }
+
+       if (sr_info->vdd_opp_clk == NULL || IS_ERR(sr_info->vdd_opp_clk))
                return 0;
- freq = sr1.vdd_opp_clk->rate; 
+       freq = sr_info->vdd_opp_clk->rate;
        opp = opp_find_freq_ceil(OPP_MPU, &freq);
        if (IS_ERR(opp))
                return 0;
@@ -163,9 +187,9 @@ static u8 get_vdd1_opp(void)
         * 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 != sr1.vdd_opp_clk->rate))
+       if (unlikely(freq != sr_info->vdd_opp_clk->rate))
                pr_warning("%s: Available freq %ld != dpll freq %ld.\n",
-                          __func__, freq, sr1.vdd_opp_clk->rate);
+                          __func__, freq, sr_info->vdd_opp_clk->rate);
return opp_get_opp_id(opp); 
 }
@@ -174,11 +198,17 @@ static u8 get_vdd2_opp(void)
 {
        struct omap_opp *opp;
        unsigned long freq;
+       struct omap_sr *sr_info = _sr_lookup(SR2);
+
+       if (!sr_info) {
+               pr_warning("omap_sr struct corresponding to SR2 not found\n");
+               return 0;
+       }
- if (sr2.vdd_opp_clk == NULL || IS_ERR(sr2.vdd_opp_clk)) 
+       if (sr_info->vdd_opp_clk == NULL || IS_ERR(sr_info->vdd_opp_clk))
                return 0;
- freq = sr2.vdd_opp_clk->rate; 
+       freq = sr_info->vdd_opp_clk->rate;
        opp = opp_find_freq_ceil(OPP_L3, &freq);
        if (IS_ERR(opp))
                return 0;
@@ -187,9 +217,9 @@ static u8 get_vdd2_opp(void)
         * 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 != sr2.vdd_opp_clk->rate))
+       if (unlikely(freq != sr_info->vdd_opp_clk->rate))
                pr_warning("%s: Available freq %ld != dpll freq %ld.\n",
-                          __func__, freq, sr2.vdd_opp_clk->rate);
+                          __func__, freq, sr_info->vdd_opp_clk->rate);
        return opp_get_opp_id(opp);
 }
@@ -694,14 +724,13 @@ static void sr_disable(struct omap_sr *sr) void sr_start_vddautocomap(int srid, u32 target_opp_no) 
 {
-       struct omap_sr *sr = NULL;
+       struct omap_sr *sr = _sr_lookup(srid);
- if (srid == SR1) 
-               sr = &sr1;
-       else if (srid == SR2)
-               sr = &sr2;
-       else
+       if (!sr) {
+               pr_warning("omap_sr struct corresponding to SR%d not found\n",
+                                                               srid);
                return;
+       }
if (sr->is_sr_reset == 1) { 
                sr_clk_enable(sr);
@@ -719,14 +748,13 @@ EXPORT_SYMBOL(sr_start_vddautocomap);
int sr_stop_vddautocomap(int srid) 
 {
-       struct omap_sr *sr = NULL;
+       struct omap_sr *sr = _sr_lookup(srid);
- if (srid == SR1) 
-               sr = &sr1;
-       else if (srid == SR2)
-               sr = &sr2;
-       else
-               return -EINVAL;
+       if (!sr) {
+               pr_warning("omap_sr struct corresponding to SR%d not found\n",
+                                                               srid);
+               return false;
+       }
if (sr->is_autocomp_active == 1) { 
                sr_disable(sr);
@@ -744,14 +772,13 @@ EXPORT_SYMBOL(sr_stop_vddautocomap);
 void enable_smartreflex(int srid)
 {
        u32 target_opp_no = 0;
-       struct omap_sr *sr = NULL;
+       struct omap_sr *sr = _sr_lookup(srid);
- if (srid == SR1) 
-               sr = &sr1;
-       else if (srid == SR2)
-               sr = &sr2;
-       else
+       if (!sr) {
+               pr_warning("omap_sr struct corresponding to SR%d not found\n",
+                                                               srid);
                return;
+       }
if (sr->is_autocomp_active == 1) { 
                if (sr->is_sr_reset == 1) {
@@ -779,15 +806,13 @@ void enable_smartreflex(int srid)
 void disable_smartreflex(int srid)
 {
        u32 i = 0;
+       struct omap_sr *sr = _sr_lookup(srid);
- struct omap_sr *sr = NULL; 
-
-       if (srid == SR1)
-               sr = &sr1;
-       else if (srid == SR2)
-               sr = &sr2;
-       else
+       if (!sr) {
+               pr_warning("omap_sr struct corresponding to SR%d not found\n",
+                                                               srid);
                return;
+       }
if (sr->is_autocomp_active == 1) { 
                if (sr->is_sr_reset == 0) {
@@ -920,7 +945,13 @@ int sr_voltagescale_vcbypass(u32 target_opp, u32 
current_opp,
 static ssize_t omap_sr_vdd1_autocomp_show(struct kobject *kobj,
                                        struct kobj_attribute *attr, char *buf)
 {
-       return sprintf(buf, "%d\n", sr1.is_autocomp_active);
+       struct omap_sr *sr_info = _sr_lookup(SR1);
+
+       if (!sr_info) {
+               pr_warning("omap_sr struct corresponding to SR1 not found\n");
+               return 0;
+       }
+       return sprintf(buf, "%d\n", sr_info->is_autocomp_active);
 }
static ssize_t omap_sr_vdd1_autocomp_store(struct kobject *kobj, 
@@ -960,7 +991,13 @@ static struct kobj_attribute sr_vdd1_autocomp = {
 static ssize_t omap_sr_vdd2_autocomp_show(struct kobject *kobj,
                                        struct kobj_attribute *attr, char *buf)
 {
-       return sprintf(buf, "%d\n", sr2.is_autocomp_active);
+       struct omap_sr *sr_info = _sr_lookup(SR2);
+
+       if (!sr_info) {
+               pr_warning("omap_sr struct corresponding to SR2 not found\n");
+               return 0;
+       }
+       return sprintf(buf, "%d\n", sr_info->is_autocomp_active);
 }
static ssize_t omap_sr_vdd2_autocomp_store(struct kobject *kobj, 
@@ -1010,7 +1047,6 @@ static int __init omap3_sr_init(void)
        RdReg |= DCDC_GLOBAL_CFG_ENABLE_SRFLX;
        ret |= twl_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, RdReg,
                                R_DCDC_GLOBAL_CFG);
-
        if (cpu_is_omap34xx()) {
                sr1.clk = clk_get(NULL, "sr1_fck");
                sr2.clk = clk_get(NULL, "sr2_fck");
@@ -1036,6 +1072,8 @@ static int __init omap3_sr_init(void)
        ret = sysfs_create_file(power_kobj, &sr_vdd2_autocomp.attr);
        if (ret)
                pr_err("sysfs_create_file failed: %d\n", ret);
+       list_add(&sr1.node, &sr_list);
+       list_add(&sr2.node, &sr_list);
return 0; 
 }


--
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

Reply via email to