[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 02/12 v3] xen/arm: vpl011: Define generic vreg_reg* access functions in vreg.h



Hi Bhupinder,

On 10/05/17 15:24, Bhupinder Thakur wrote:
diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
index c838298..75c716e 100644
--- a/xen/include/asm-arm/vgic.h
+++ b/xen/include/asm-arm/vgic.h
@@ -172,18 +172,6 @@ static inline int REG_RANK_NR(int b, uint32_t n)
     }
 }

-/*
- * 64 bits registers are only supported on platform with 64-bit long.
- * This is also allow us to optimize the 32 bit case by using
- * unsigned long rather than uint64_t
- */
-#if BITS_PER_LONG == 64
-VGIC_REG_HELPERS(64, 0x7);
-#endif
-VGIC_REG_HELPERS(32, 0x3);
-
-#undef VGIC_REG_HELPERS
-

Why this is moved out here? And not in patch #1?

 enum gic_sgi_mode;

 /*
diff --git a/xen/include/asm-arm/vreg.h b/xen/include/asm-arm/vreg.h
index 1442c58..e127114 100644
--- a/xen/include/asm-arm/vreg.h
+++ b/xen/include/asm-arm/vreg.h

[...]

-{                                                                       \
-    unsigned long tmp = *reg;                                           \
-                                                                        \
-    vgic_reg_clearbits(&tmp, bits, info->gpa & offmask,                 \
-                       info->dabt.size);                                \
-                                                                        \
-    *reg = tmp;                                                         \
+#define DEFINE_VREG_REG_HELPERS(sz, offmask)                                   
 \

Nack. There are no reason to rename the name of the macro more than s/vgic/vreg/. It also makes to patch diff more difficult to read.

Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.