Ramos Falcon, Ernesto had written, on 07/28/2010 09:40 AM, the following:
Remove unnecessary volatile variables; use accessor
functions __raw_readl/__raw_writel instead when applicable.

Signed-off-by: Ernesto Ramos <erne...@ti.com>
---
 drivers/staging/tidspbridge/core/tiomap3430.c |    8 ++++----
 drivers/staging/tidspbridge/dynload/tramp.c   |    4 ++--
 drivers/staging/tidspbridge/pmgr/cmm.c        |    7 ++++---
 3 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/tidspbridge/core/tiomap3430.c 
b/drivers/staging/tidspbridge/core/tiomap3430.c
index 08a2f5f..ae1f394 100644
--- a/drivers/staging/tidspbridge/core/tiomap3430.c
+++ b/drivers/staging/tidspbridge/core/tiomap3430.c
@@ -404,7 +404,7 @@ static int bridge_brd_start(struct bridge_dev_context 
*dev_ctxt,
                pr_err("%s: Illegal SM base\n", __func__);
                status = -EPERM;
        } else
-               *((volatile u32 *)dw_sync_addr) = 0xffffffff;
+               __raw_writel(0xffffffff, dw_sync_addr);
curious question: any reason not to use writel instead of __raw_writel?

if (DSP_SUCCEEDED(status)) {
                resources = dev_context->resources;
@@ -584,7 +584,7 @@ static int bridge_brd_start(struct bridge_dev_context 
*dev_ctxt,
                dev_dbg(bridge, "Waiting for Sync @ 0x%x\n", dw_sync_addr);
                dev_dbg(bridge, "DSP c_int00 Address =  0x%x\n", dsp_addr);
                if (dsp_debug)
-                       while (*((volatile u16 *)dw_sync_addr))
+                       while (__raw_readw(dw_sync_addr))
                                ;;

may be for a later on patch -> we seem to have a potential infinite loop here..

/* Wait for DSP to clear word in shared memory */
@@ -602,7 +602,7 @@ static int bridge_brd_start(struct bridge_dev_context 
*dev_ctxt,
                        /* Write the synchronization bit to indicate the
                         * completion of OPP table update to DSP
                         */
-                       *((volatile u32 *)dw_sync_addr) = 0XCAFECAFE;
+                       __raw_writel(0XCAFECAFE, dw_sync_addr);
/* update board state */
                        dev_context->dw_brd_state = BRD_RUNNING;
@@ -1852,7 +1852,7 @@ bool wait_for_start(struct bridge_dev_context 
*dev_context, u32 dw_sync_addr)
        u16 timeout = TIHELEN_ACKTIMEOUT;
/* Wait for response from board */
-       while (*((volatile u16 *)dw_sync_addr) && --timeout)
+       while (__raw_readw(dw_sync_addr) && --timeout)
                udelay(10);
/* If timed out: return false */
diff --git a/drivers/staging/tidspbridge/dynload/tramp.c 
b/drivers/staging/tidspbridge/dynload/tramp.c
index 81314d2..60d22ea 100644
--- a/drivers/staging/tidspbridge/dynload/tramp.c
+++ b/drivers/staging/tidspbridge/dynload/tramp.c
@@ -86,8 +86,8 @@ static u8 priv_h2a(u8 value)
 static void priv_tramp_sym_gen_name(u32 value, char *dst)
 {
        u32 i;
-       volatile char *prefix = TRAMP_SYM_PREFIX;
-       volatile char *dst_local = dst;
+       char *prefix = TRAMP_SYM_PREFIX;
+       char *dst_local = dst;
        u8 tmp;
/* Clear out the destination, including the ending NULL */
diff --git a/drivers/staging/tidspbridge/pmgr/cmm.c 
b/drivers/staging/tidspbridge/pmgr/cmm.c
index 874ed64..b7cba1b 100644
--- a/drivers/staging/tidspbridge/pmgr/cmm.c
+++ b/drivers/staging/tidspbridge/pmgr/cmm.c
@@ -1008,6 +1008,7 @@ void *cmm_xlator_alloc_buf(struct cmm_xlatorobject 
*xlator, void *va_buf,
 {
        struct cmm_xlator *xlator_obj = (struct cmm_xlator *)xlator;
        void *pbuf = NULL;
+       void *tmp_va_buff;
        struct cmm_attrs attrs;
DBC_REQUIRE(refs > 0);
@@ -1019,16 +1020,16 @@ void *cmm_xlator_alloc_buf(struct cmm_xlatorobject 
*xlator, void *va_buf,
if (xlator_obj) {
                attrs.ul_seg_id = xlator_obj->ul_seg_id;
-               *(volatile u32 *)va_buf = 0;
+               __raw_writel(0, va_buf);
                /* Alloc SM */
                pbuf =
                    cmm_calloc_buf(xlator_obj->hcmm_mgr, pa_size, &attrs, NULL);
                if (pbuf) {
                        /* convert to translator(node/strm) process Virtual
                         * address */
-                       *(volatile u32 **)va_buf =
-                           (u32 *) cmm_xlator_translate(xlator,
+                        tmp_va_buff = cmm_xlator_translate(xlator,
                                                         pbuf, CMM_PA2VA);
+                       __raw_writel((u32)tmp_va_buff, va_buf);

a) is the u32 cast of tmp_va_buff really necessary?
b) tmp_va_buff is used only in this branch, why not reduce the scope of the variable to just to this if branch
or an optional way could be:
--- a/drivers/staging/tidspbridge/pmgr/cmm.c
+++ b/drivers/staging/tidspbridge/pmgr/cmm.c
@@ -1022,18 +1022,17 @@ void *cmm_xlator_alloc_buf(struct cmm_xlatorobject *xlator, void *va_buf,
        DBC_REQUIRE(xlator_obj->ul_seg_id > 0);

        if (xlator_obj) {
+               void *t_buf = 0;
                attrs.ul_seg_id = xlator_obj->ul_seg_id;
-               *(volatile u32 *)va_buf = 0;
                /* Alloc SM */
                pbuf =
                    cmm_calloc_buf(xlator_obj->hcmm_mgr, pa_size, &attrs, NULL);
                if (pbuf) {
                        /* convert to translator(node/strm) process Virtual
                         * address */
-                       *(volatile u32 **)va_buf =
-                           (u32 *) cmm_xlator_translate(xlator,
-                                                        pbuf, CMM_PA2VA);
+                       t_buf = cmm_xlator_translate(xlator, pbuf, CMM_PA2VA);
                }
+               writel(t_buf, va_buf);
        }
        return pbuf;
 }


                }
        }
        return pbuf;


--
Regards,
Nishanth Menon
--
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