[RFC PATCH 0/4] ARM: Unify const-swabbing and conditional asm

[RFC PATCH 0/4] ARM: Unify const-swabbing and conditional asm

[Dave Martin]

There are a few things in assembler.h that would be useful with
inline asm, but currently assembler.h refuses to be included into C
files.

This refactoring provides a place to put const swabbing helpers, which
can then be used in inline asm and other places (I recycle and share
the code from <asm/opcodes.h>).

The final patch modifies the CCI driver to use the new
__asm_cpu_to_le32() helper, based closely on Victor Kamensky's recent
fix for BE8.  This allows us to avoid some unneeded ifdeffery.


Comments?


These patches are untested, but I have build-tested the CCI driver in
LE and BE32 configurations, which produces the following change in the
generated code:

 e0400001 	sub	r0, r0, r1
 e5900004 	ldr	r0, [r0, #4]
-e3a03003 	mov	r3, #3
+e3a03403 	mov	r3, #50331648	; 0x3000000
 e5803000 	str	r3, [r0]
 e28f1024 	add	r1, pc, #36	; 0x24
 e5910000 	ldr	r0, [r1]
 e7900001 	ldr	r0, [r0, r1]
 e590100c 	ldr	r1, [r0, #12]
-e3110001 	tst	r1, #1
+e3110401 	tst	r1, #16777216	; 0x1000000
 1afffffc 	bne	288 <cci_port_not_found+0x44>
 e3a00000 	mov	r0, #0
 e12fff1e 	bx	lr


Dave Martin (4):
  ARM: Allow assembler.h to be used with inline asm
  ARM: Add common compile-time swab32 for asm code
  ARM: Add const cpu_to_le32 for asm
  drivers/bus: arm-cci: Fix CCI enable code for BE32

 arch/arm/include/asm/assembler.h |   41 ++++++++++++++++++++++++++++++++++----
 arch/arm/include/asm/opcodes.h   |    9 +++------
 drivers/bus/arm-cci.c            |    6 ++++--
 3 files changed, 44 insertions(+), 12 deletions(-)

-- 
1.7.9.5

[RFC PATCH 1/4] ARM: Allow assembler.h to be used with inline asm

[Dave Martin]

There are a few things in assembler.h that would be useful with
inline asm, but currently assembler.h refuses to be included into C
files.

In particular, the optional feature macros (PLD() and friends) can
be used sensibly with inline asm, with a string argument.

This patch enables the relevant parts of aassembler.h to be
included in C files and used in inline asm.

Since assembler.h by definition can't be included in any C file up
to now, this should not introduce any namespace clashes.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
 arch/arm/include/asm/assembler.h |   12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
index fcc1b5b..a71d117 100644
--- a/arch/arm/include/asm/assembler.h
+++ b/arch/arm/include/asm/assembler.h
@@ -16,14 +16,12 @@
 #ifndef __ASM_ASSEMBLER_H__
 #define __ASM_ASSEMBLER_H__
 
-#ifndef __ASSEMBLY__
-#error "Only include this from assembly code"
-#endif
-
 #include <asm/ptrace.h>
 #include <asm/domain.h>
 #include <asm/opcodes-virt.h>
 
+#ifdef __ASSEMBLY__
+
 #define IOMEM(x)	(x)
 
 /*
@@ -53,6 +51,8 @@
 #define put_byte_3      lsl #0
 #endif
 
+#endif /* __ASSEMBLY__ */
+
 /*
  * Data preload for architectures that support it
  */
@@ -77,6 +77,8 @@
 #define CALGN(code...)
 #endif
 
+#ifdef __ASSEMBLY__
+
 /*
  * Enable and disable interrupts
  */
@@ -376,4 +378,6 @@ THUMB(	orr	\reg , \reg , #PSR_T_BIT	)
 #endif
 	.endm
 
+#endif /* __ASSEMBLY__ */
+
 #endif /* __ASM_ASSEMBLER_H__ */
-- 
1.7.9.5

[RFC PATCH 1/4] ARM: Allow assembler.h to be used with inline asm

[Russell King - ARM Linux]

On Wed, Oct 09, 2013 at 03:29:50PM +0100, Dave Martin wrote:
> There are a few things in assembler.h that would be useful with
> inline asm, but currently assembler.h refuses to be included into C
> files.
> 
> In particular, the optional feature macros (PLD() and friends) can
> be used sensibly with inline asm, with a string argument.
> 
> This patch enables the relevant parts of aassembler.h to be
> included in C files and used in inline asm.
> 
> Since assembler.h by definition can't be included in any C file up
> to now, this should not introduce any namespace clashes.

I think it would be better to move the two macros (PLD and CALGN) out of
this file, rather than covering almost the entire file with this #ifdef.

[RFC PATCH 1/4] ARM: Allow assembler.h to be used with inline asm

[Dave Martin]

On Wed, Oct 09, 2013 at 03:42:11PM +0100, Russell King - ARM Linux wrote:
> On Wed, Oct 09, 2013 at 03:29:50PM +0100, Dave Martin wrote:
> > There are a few things in assembler.h that would be useful with
> > inline asm, but currently assembler.h refuses to be included into C
> > files.
> > 
> > In particular, the optional feature macros (PLD() and friends) can
> > be used sensibly with inline asm, with a string argument.
> > 
> > This patch enables the relevant parts of aassembler.h to be
> > included in C files and used in inline asm.
> > 
> > Since assembler.h by definition can't be included in any C file up
> > to now, this should not introduce any namespace clashes.
> 
> I think it would be better to move the two macros (PLD and CALGN) out of
> this file, rather than covering almost the entire file with this #ifdef.

Certainly we could do that.  When I started this hack, I though that a
larger proportion of the file would be relevant...

Any thoughts on a sensible name?  asm/asm-common.h, maybe?

Cheers
---Dave

[RFC PATCH 1/4] ARM: Allow assembler.h to be used with inline asm

[Nicolas Pitre]

On Wed, 9 Oct 2013, Dave Martin wrote:

> On Wed, Oct 09, 2013 at 03:42:11PM +0100, Russell King - ARM Linux wrote:
> > On Wed, Oct 09, 2013 at 03:29:50PM +0100, Dave Martin wrote:
> > > There are a few things in assembler.h that would be useful with
> > > inline asm, but currently assembler.h refuses to be included into C
> > > files.
> > > 
> > > In particular, the optional feature macros (PLD() and friends) can
> > > be used sensibly with inline asm, with a string argument.
> > > 
> > > This patch enables the relevant parts of aassembler.h to be
> > > included in C files and used in inline asm.
> > > 
> > > Since assembler.h by definition can't be included in any C file up
> > > to now, this should not introduce any namespace clashes.
> > 
> > I think it would be better to move the two macros (PLD and CALGN) out of
> > this file, rather than covering almost the entire file with this #ifdef.
> 
> Certainly we could do that.  When I started this hack, I though that a
> larger proportion of the file would be relevant...
> 
> Any thoughts on a sensible name?  asm/asm-common.h, maybe?

Since there is no need for CALGN() or PLD() in C code yet, maybe it is 
best to leave them where they are for the time being.

And then adding __asm_swab32() to opcodes.h feels quite natural with the 
purpose of that file's content.  Then including that fine to solve the 
CCI code endianness should be fine.

If anything, maybe it is the name opcodes.h which is misrepresenting the 
scope of its content.


Nicolas

[RFC PATCH 2/4] ARM: Add common compile-time swab32 for asm code

[Dave Martin]

<linux/swab.h> doesn't work for asm code, yet swabbing constants
for asm is still useful, such as when writing code which needs to
build with different target endiannesses.

This patch adds a simple asm_swab32() macro so that this operation
doesn't need to be reinvented repeatedly.

Since <asm/opcodes.h> already defines this operation, this patch
lifts the definition into assembler.h and makes opcodes.h use it.

The other swab variants defined by opcodes.h could be transferred
too, but there is no clear need for that yet.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
 arch/arm/include/asm/assembler.h |   23 +++++++++++++++++++++++
 arch/arm/include/asm/opcodes.h   |    9 +++------
 2 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
index a71d117..860256c 100644
--- a/arch/arm/include/asm/assembler.h
+++ b/arch/arm/include/asm/assembler.h
@@ -380,4 +380,27 @@ THUMB(	orr	\reg , \reg , #PSR_T_BIT	)
 
 #endif /* __ASSEMBLY__ */
 
+/*
+ * Use asm_swab32() only for compile-time swabbing of constants for
+ * assembly code.  in inline asm, the argument is a string
+ * representation of an expression which will be passed through to the
+ * assembler, so it can't contain embedded C expressions or preprocessor
+ * macros.  Macros would need to be pre-expanded using __stringify().
+ */
+#ifdef __ASSEMBLY__
+#define __asm_swab32(x) (			\
+	  (((x) >> 24) & 0x000000ff)		\
+	| (((x) >> 8)  & 0x0000ff00)		\
+	| (((x) << 8)  & 0x00ff0000)		\
+	| (((x) << 24) & 0xff000000)		\
+)
+#else /* ! __ASSEMBLY__ */
+#define __asm_swab32(x) "( "			\
+	  "(((" x ") >> 24) & 0x000000ff) "	\
+	"| (((" x ") >> 8)  & 0x0000ff00) "	\
+	"| (((" x ") << 8)  & 0x00ff0000) "	\
+	"| (((" x ") << 24) & 0xff000000) "	\
+")"
+#endif /* ! __ASSEMBLY__ */
+
 #endif /* __ASM_ASSEMBLER_H__ */
diff --git a/arch/arm/include/asm/opcodes.h b/arch/arm/include/asm/opcodes.h
index e796c59..b2e93c9 100644
--- a/arch/arm/include/asm/opcodes.h
+++ b/arch/arm/include/asm/opcodes.h
@@ -9,6 +9,8 @@
 #ifndef __ASM_ARM_OPCODES_H
 #define __ASM_ARM_OPCODES_H
 
+#include <asm/assembler.h>
+
 #ifndef __ASSEMBLY__
 #include <linux/linkage.h>
 extern asmlinkage unsigned int arm_check_condition(u32 opcode, u32 psr);
@@ -24,12 +26,7 @@ extern asmlinkage unsigned int arm_check_condition(u32 opcode, u32 psr);
  * These are only intended for use by this header: don't use them directly,
  * because they will be suboptimal in most cases.
  */
-#define ___asm_opcode_swab32(x) (	\
-	  (((x) << 24) & 0xFF000000)	\
-	| (((x) <<  8) & 0x00FF0000)	\
-	| (((x) >>  8) & 0x0000FF00)	\
-	| (((x) >> 24) & 0x000000FF)	\
-)
+#define ___asm_opcode_swab32(x) __asm_swab32(x)
 #define ___asm_opcode_swab16(x) (	\
 	  (((x) << 8) & 0xFF00)		\
 	| (((x) >> 8) & 0x00FF)		\
-- 
1.7.9.5

[RFC PATCH 3/4] ARM: Add const cpu_to_le32 for asm

[Dave Martin]

On ARM, it's common for big-endian systems to use little-endian
byte order for devices.

This patch adds a __asm_cpu_to_le32() helper, similar to
cpu_to_le32().  This is intended for preparing 32-bit constants to
be stored to a device, or compared with data loaded from a device.

Rather than add umpteen variants up front, extra variants can be
added when needed.  Accessing devices directly from asm is not the
common case.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
 arch/arm/include/asm/assembler.h |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
index 860256c..ec869e9 100644
--- a/arch/arm/include/asm/assembler.h
+++ b/arch/arm/include/asm/assembler.h
@@ -403,4 +403,10 @@ THUMB(	orr	\reg , \reg , #PSR_T_BIT	)
 ")"
 #endif /* ! __ASSEMBLY__ */
 
+#ifdef CONFIG_CPU_BIG_ENDIAN
+#define __asm_cpu_to_le32(x) __asm_swab32(x)
+#else
+#define __asm_cpu_to_le32(x) "(" x ")"
+#endif		 
+
 #endif /* __ASM_ASSEMBLER_H__ */
-- 
1.7.9.5

[RFC PATCH 4/4] drivers/bus: arm-cci: Fix CCI enable code for BE32

[Dave Martin]

The CCI control registers are always little-endian.

This patch ensures that data is transferred to/from those registers
using the correct byte order in the low-level asm code of
cci_enable_port_for_self().

This is required for CCI to work correctly with BE8 kernels.

Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
 drivers/bus/arm-cci.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
index 2009266..b205a97 100644
--- a/drivers/bus/arm-cci.c
+++ b/drivers/bus/arm-cci.c
@@ -20,11 +20,13 @@
 #include <linux/of_address.h>
 #include <linux/slab.h>
 
+#include <asm/assembler.h>
 #include <asm/cacheflush.h>
 #include <asm/smp_plat.h>
 
 #define CCI_PORT_CTRL		0x0
 #define CCI_CTRL_STATUS		0xc
+#define CCI_CHANGE_PENDING	(1 << 0)	/* change pending in CCI_CTRL_STATUS */
 
 #define CCI_ENABLE_SNOOP_REQ	0x1
 #define CCI_ENABLE_DVM_REQ	0x2
@@ -280,7 +282,7 @@ asmlinkage void __naked cci_enable_port_for_self(void)
 
 	/* Enable the CCI port */
 "	ldr	r0, [r0, %[offsetof_port_phys]] \n"
-"	mov	r3, #"__stringify(CCI_ENABLE_REQ)" \n"
+"	mov	r3, #"__asm_cpu_to_le32(__stringify(CCI_ENABLE_REQ))" \n"
 "	str	r3, [r0, #"__stringify(CCI_PORT_CTRL)"] \n"
 
 	/* poll the status reg for completion */
@@ -288,7 +290,7 @@ asmlinkage void __naked cci_enable_port_for_self(void)
 "	ldr	r0, [r1] \n"
 "	ldr	r0, [r0, r1]		@ cci_ctrl_base \n"
 "4:	ldr	r1, [r0, #"__stringify(CCI_CTRL_STATUS)"] \n"
-"	tst	r1, #1 \n"
+"	tst	r1, #"__asm_cpu_to_le32(__stringify(CCI_CHANGE_PENDING))" \n"
 "	bne	4b \n"
 
 "	mov	r0, #0 \n"
-- 
1.7.9.5

[RFC PATCH 1/4] ARM: Allow assembler.h to be used with inline asm

[Nicolas Pitre]

On Wed, 9 Oct 2013, Dave Martin wrote:

> It could make sense to add the new helper to asm/swab.h, but that might
> result in pollution when it's included via linux/swab.h.

Yet its name is specific enough that this is rather unlikely.  So maybe 
this would be the best compromize.


Nicolas

[RFC PATCH 1/4] ARM: Allow assembler.h to be used with inline asm

[Nicolas Pitre]

On Wed, 9 Oct 2013, Dave Martin wrote:

> On Wed, Oct 09, 2013 at 12:54:31PM -0400, Nicolas Pitre wrote:
> > On Wed, 9 Oct 2013, Dave Martin wrote:
> > 
> > > It could make sense to add the new helper to asm/swab.h, but that might
> > > result in pollution when it's included via linux/swab.h.
> > 
> > Yet its name is specific enough that this is rather unlikely.  So maybe 
> > this would be the best compromize.
> 
> You mean the names of the introduced macros?

Yes.


Nicolas