Thread (6 messages) 6 messages, 4 authors, 2016-05-20

[PATCH V4 6/7] iommu/msm: Use writel_relaxed and add a barrier

From: arnd@arndb.de (Arnd Bergmann)
Date: 2016-05-17 13:52:17
Also in: linux-arm-msm, linux-devicetree, linux-iommu

On Monday 16 May 2016 12:19:00 Sricharan R wrote:
quoted hunk ↗ jump to hunk
diff --git a/drivers/iommu/msm_iommu.c b/drivers/iommu/msm_iommu.c
index f7b4c11..1240a5a 100644
--- a/drivers/iommu/msm_iommu.c
+++ b/drivers/iommu/msm_iommu.c
@@ -124,6 +124,7 @@ static void msm_iommu_reset(void __iomem *base, int ncb)
                SET_TLBLKCR(base, ctx, 0);
                SET_CONTEXTIDR(base, ctx, 0);
        }
+       mb(); /* sync */
 }
 
 static void __flush_iotlb(void *cookie)
@@ -143,6 +144,7 @@ static void __flush_iotlb(void *cookie)
 
                __disable_clocks(iommu);
        }
+       mb(); /* sync */
 fail:
        return;
 }
These comments are completely useless. What is the specific race
that you are protecting against, and why are the implicit barriers
not sufficient here? Please find a better way to document what
is going on.
quoted hunk ↗ jump to hunk
diff --git a/drivers/iommu/msm_iommu_hw-8xxx.h b/drivers/iommu/msm_iommu_hw-8xxx.h
index 84ba5739..161036c 100644
--- a/drivers/iommu/msm_iommu_hw-8xxx.h
+++ b/drivers/iommu/msm_iommu_hw-8xxx.h
@@ -24,10 +24,10 @@
 #define GET_CTX_REG(reg, base, ctx) \
                                (readl((base) + (reg) + ((ctx) << CTX_SHIFT)))
 
-#define SET_GLOBAL_REG(reg, base, val) writel((val), ((base) + (reg)))
+#define SET_GLOBAL_REG(reg, base, val) writel_relaxed((val), ((base) + (reg)))
 
 #define SET_CTX_REG(reg, base, ctx, val) \
-                       writel((val), ((base) + (reg) + ((ctx) << CTX_SHIFT)))
+               writel_relaxed((val), ((base) + (reg) + ((ctx) << CTX_SHIFT)))
 
 /* Wrappers for numbered registers */
 #define SET_GLOBAL_REG_N(b, n, r, v) SET_GLOBAL_REG(b, ((r) + (n << 2)), (v))
@@ -48,7 +48,8 @@
 #define SET_FIELD(addr, mask, shift, v) \
 do { \
        int t = readl(addr); \
-       writel((t & ~((mask) << (shift))) + (((v) & (mask)) << (shift)), addr);\
+       writel_relaxed((t & ~((mask) << (shift))) + \
+                      (((v) & (mask)) << (shift)), addr);\
 } while (0)
No, please add a new macro for the relaxed accessors and then add comments
in the places where those are used, to document why you can take shortcuts
in those places.

	Arnd
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help