Skip to contentSkip to conversation

Use more asm-generic headers

PATCH Superseded Rob Herring 4 participants 11 messages Jul 16, 2012 – Jul 17, 2012 Arm (32-bit)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Rob Herring

author
#0 lore raw

cover Use more asm-generic headers

From: Rob Herring <[email protected]>

Inspired by the AArgh64 claim that it should be separate from ARM and one reason was being able to use more asm-generic headers. Doing a diff of arch/arm/include/asm and include/asm-generic there are numerous asm headers which are functionally identical to their asm-generic counterparts and others which can use more of the generic versions.

Rob

Rob Herring (3):
ARM: use generic version of identical asm headers
ARM: use asm-generic version of kmap_types.h
ARM: convert asm/irqflags.h to use asm-generic/irqflags.h

 arch/arm/include/asm/Kbuild       |   18 ++++
 arch/arm/include/asm/current.h    |   15 ---
 arch/arm/include/asm/exec.h       |    6 --
 arch/arm/include/asm/ipcbuf.h     |    1 -
 arch/arm/include/asm/irqflags.h   |  131 +++++++++---------------
 arch/arm/include/asm/kmap_types.h |   33 -------
 arch/arm/include/asm/msgbuf.h     |   31 ------
 arch/arm/include/asm/param.h      |   31 ------
 arch/arm/include/asm/parport.h    |   18 ----
 arch/arm/include/asm/segment.h    |   11 ---
 arch/arm/include/asm/sembuf.h     |   25 -----
 arch/arm/include/asm/serial.h     |   19 ----
 arch/arm/include/asm/shmbuf.h     |   42 --------
 arch/arm/include/asm/socket.h     |   72 --------------
 arch/arm/include/asm/sockios.h    |   13 ---
 arch/arm/include/asm/termbits.h   |  198 -------------------------------------
 arch/arm/include/asm/termios.h    |   92 -----------------
 arch/arm/include/asm/timex.h      |    8 +-
 arch/arm/include/asm/types.h      |   16 ---
 arch/arm/include/asm/unaligned.h  |   19 ----
 20 files changed, 64 insertions(+), 735 deletions(-)
 delete mode 100644 arch/arm/include/asm/current.h
 delete mode 100644 arch/arm/include/asm/exec.h
 delete mode 100644 arch/arm/include/asm/ipcbuf.h
 delete mode 100644 arch/arm/include/asm/kmap_types.h
 delete mode 100644 arch/arm/include/asm/msgbuf.h
 delete mode 100644 arch/arm/include/asm/param.h
 delete mode 100644 arch/arm/include/asm/parport.h
 delete mode 100644 arch/arm/include/asm/segment.h
 delete mode 100644 arch/arm/include/asm/sembuf.h
 delete mode 100644 arch/arm/include/asm/serial.h
 delete mode 100644 arch/arm/include/asm/shmbuf.h
 delete mode 100644 arch/arm/include/asm/socket.h
 delete mode 100644 arch/arm/include/asm/sockios.h
 delete mode 100644 arch/arm/include/asm/termbits.h
 delete mode 100644 arch/arm/include/asm/termios.h
 delete mode 100644 arch/arm/include/asm/types.h
 delete mode 100644 arch/arm/include/asm/unaligned.h
Signature · 1 line
1.7.9.5
author
#1 lore raw

PATCH 1/3 ARM: use generic version of identical asm headers

From: Rob Herring <[email protected]>

Inspired by the AArgh64 claim that it should be separate from ARM and one reason was being able to use more asm-generic headers. Doing a diff of arch/arm/include/asm and include/asm-generic there are numerous asm headers which are functionally identical to their asm-generic counterparts. Delete the ARM version and use the generic ones.

Signed-off-by Rob Herring <[email protected]>
---
 arch/arm/include/asm/Kbuild      |   17 ++++
 arch/arm/include/asm/current.h   |   15 ---
 arch/arm/include/asm/exec.h      |    6 --
 arch/arm/include/asm/ipcbuf.h    |    1 -
 arch/arm/include/asm/msgbuf.h    |   31 ------
 arch/arm/include/asm/param.h     |   31 ------
 arch/arm/include/asm/parport.h   |   18 ----
 arch/arm/include/asm/segment.h   |   11 ---
 arch/arm/include/asm/sembuf.h    |   25 -----
 arch/arm/include/asm/serial.h    |   19 ----
 arch/arm/include/asm/shmbuf.h    |   42 --------
 arch/arm/include/asm/socket.h    |   72 --------------
 arch/arm/include/asm/sockios.h   |   13 ---
 arch/arm/include/asm/termbits.h  |  198 --------------------------------------
 arch/arm/include/asm/termios.h   |   92 ------------------
 arch/arm/include/asm/timex.h     |    8 +-
 arch/arm/include/asm/types.h     |   16 ---
 arch/arm/include/asm/unaligned.h |   19 ----
 18 files changed, 18 insertions(+), 616 deletions(-)
 delete mode 100644 arch/arm/include/asm/current.h
 delete mode 100644 arch/arm/include/asm/exec.h
 delete mode 100644 arch/arm/include/asm/ipcbuf.h
 delete mode 100644 arch/arm/include/asm/msgbuf.h
 delete mode 100644 arch/arm/include/asm/param.h
 delete mode 100644 arch/arm/include/asm/parport.h
 delete mode 100644 arch/arm/include/asm/segment.h
 delete mode 100644 arch/arm/include/asm/sembuf.h
 delete mode 100644 arch/arm/include/asm/serial.h
 delete mode 100644 arch/arm/include/asm/shmbuf.h
 delete mode 100644 arch/arm/include/asm/socket.h
 delete mode 100644 arch/arm/include/asm/sockios.h
 delete mode 100644 arch/arm/include/asm/termbits.h
 delete mode 100644 arch/arm/include/asm/termios.h
 delete mode 100644 arch/arm/include/asm/types.h
 delete mode 100644 arch/arm/include/asm/unaligned.h
arch/arm/include/asm/Kbuild +17 -0
index 960abce..8a7196c 100644
@@ -5,16 +5,33 @@ header-y += hwcap.h
 generic-y += auxvec.h
 generic-y += bitsperlong.h
 generic-y += cputime.h
+generic-y += current.h
 generic-y += emergency-restart.h
 generic-y += errno.h
+generic-y += exec.h
 generic-y += ioctl.h
+generic-y += ipcbuf.h
 generic-y += irq_regs.h
 generic-y += kdebug.h
 generic-y += local.h
 generic-y += local64.h
+generic-y += msgbuf.h
+generic-y += param.h
+generic-y += parport.h
 generic-y += percpu.h
 generic-y += poll.h
 generic-y += resource.h
 generic-y += sections.h
+generic-y += segment.h
+generic-y += sembuf.h
+generic-y += serial.h
+generic-y += shmbuf.h
 generic-y += siginfo.h
 generic-y += sizes.h
+generic-y += socket.h
+generic-y += sockios.h
+generic-y += termbits.h
+generic-y += termios.h
+generic-y += timex.h
+generic-y += types.h
+generic-y += unaligned.h
arch/arm/include/asm/current.h deleted +0 -15
deleted file mode 100644
index 75d21e2..0000000
@@ -1,15 +0,0 @@
-#ifndef _ASMARM_CURRENT_H
-#define _ASMARM_CURRENT_H
-
-#include <linux/thread_info.h>
-
-static inline struct task_struct *get_current(void) __attribute_const__;
-
-static inline struct task_struct *get_current(void)
-{
-	return current_thread_info()->task;
-}
-
-#define current (get_current())
-
-#endif /* _ASMARM_CURRENT_H */
arch/arm/include/asm/exec.h deleted +0 -6
deleted file mode 100644
index 7c4fbef..0000000
@@ -1,6 +0,0 @@
-#ifndef __ASM_ARM_EXEC_H
-#define __ASM_ARM_EXEC_H
-
-#define arch_align_stack(x) (x)
-
-#endif /* __ASM_ARM_EXEC_H */
arch/arm/include/asm/ipcbuf.h deleted +0 -1
deleted file mode 100644
index 84c7e51..0000000
@@ -1 +0,0 @@
-#include <asm-generic/ipcbuf.h>
arch/arm/include/asm/msgbuf.h deleted +0 -31
deleted file mode 100644
index 33b35b9..0000000
@@ -1,31 +0,0 @@
-#ifndef _ASMARM_MSGBUF_H
-#define _ASMARM_MSGBUF_H
-
-/* 
- * The msqid64_ds structure for arm architecture.
- * Note extra padding because this structure is passed back and forth
- * between kernel and user space.
- *
- * Pad space is left for:
- * - 64-bit time_t to solve y2038 problem
- * - 2 miscellaneous 32-bit values
- */
-
-struct msqid64_ds {
-	struct ipc64_perm msg_perm;
-	__kernel_time_t msg_stime;	/* last msgsnd time */
-	unsigned long	__unused1;
-	__kernel_time_t msg_rtime;	/* last msgrcv time */
-	unsigned long	__unused2;
-	__kernel_time_t msg_ctime;	/* last change time */
-	unsigned long	__unused3;
-	unsigned long  msg_cbytes;	/* current number of bytes on queue */
-	unsigned long  msg_qnum;	/* number of messages in queue */
-	unsigned long  msg_qbytes;	/* max number of bytes on queue */
-	__kernel_pid_t msg_lspid;	/* pid of last msgsnd */
-	__kernel_pid_t msg_lrpid;	/* last receive pid */
-	unsigned long  __unused4;
-	unsigned long  __unused5;
-};
-
-#endif /* _ASMARM_MSGBUF_H */
arch/arm/include/asm/param.h deleted +0 -31
deleted file mode 100644
index 8b24bf94..0000000
@@ -1,31 +0,0 @@
-/*
- *  arch/arm/include/asm/param.h
- *
- *  Copyright (C) 1995-1999 Russell King
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- */
-#ifndef __ASM_PARAM_H
-#define __ASM_PARAM_H
-
-#ifdef __KERNEL__
-# define HZ		CONFIG_HZ	/* Internal kernel timer frequency */
-# define USER_HZ	100		/* User interfaces are in "ticks" */
-# define CLOCKS_PER_SEC	(USER_HZ)	/* like times() */
-#else
-# define HZ		100
-#endif
-
-#define EXEC_PAGESIZE	4096
-
-#ifndef NOGROUP
-#define NOGROUP         (-1)
-#endif
-
-/* max length of hostname */
-#define MAXHOSTNAMELEN  64
-
-#endif
-
arch/arm/include/asm/parport.h deleted +0 -18
deleted file mode 100644
index 26e94b0..0000000
@@ -1,18 +0,0 @@
-/*
- *  arch/arm/include/asm/parport.h: ARM-specific parport initialisation
- *
- *  Copyright (C) 1999, 2000  Tim Waugh <[email protected]>
- *
- * This file should only be included by drivers/parport/parport_pc.c.
- */
-
-#ifndef __ASMARM_PARPORT_H
-#define __ASMARM_PARPORT_H
-
-static int __devinit parport_pc_find_isa_ports (int autoirq, int autodma);
-static int __devinit parport_pc_find_nonpci_ports (int autoirq, int autodma)
-{
-	return parport_pc_find_isa_ports (autoirq, autodma);
-}
-
-#endif /* !(_ASMARM_PARPORT_H) */
arch/arm/include/asm/segment.h deleted +0 -11
deleted file mode 100644
index 9e24c21..0000000
@@ -1,11 +0,0 @@
-#ifndef __ASM_ARM_SEGMENT_H
-#define __ASM_ARM_SEGMENT_H
-
-#define __KERNEL_CS   0x0
-#define __KERNEL_DS   0x0
-
-#define __USER_CS     0x1
-#define __USER_DS     0x1
-
-#endif /* __ASM_ARM_SEGMENT_H */
-
arch/arm/include/asm/sembuf.h deleted +0 -25
deleted file mode 100644
index 1c02839..0000000
@@ -1,25 +0,0 @@
-#ifndef _ASMARM_SEMBUF_H
-#define _ASMARM_SEMBUF_H
-
-/* 
- * The semid64_ds structure for arm architecture.
- * Note extra padding because this structure is passed back and forth
- * between kernel and user space.
- *
- * Pad space is left for:
- * - 64-bit time_t to solve y2038 problem
- * - 2 miscellaneous 32-bit values
- */
-
-struct semid64_ds {
-	struct ipc64_perm sem_perm;		/* permissions .. see ipc.h */
-	__kernel_time_t	sem_otime;		/* last semop time */
-	unsigned long	__unused1;
-	__kernel_time_t	sem_ctime;		/* last change time */
-	unsigned long	__unused2;
-	unsigned long	sem_nsems;		/* no. of semaphores in array */
-	unsigned long	__unused3;
-	unsigned long	__unused4;
-};
-
-#endif /* _ASMARM_SEMBUF_H */
arch/arm/include/asm/serial.h deleted +0 -19
deleted file mode 100644
index ebb0490..0000000
@@ -1,19 +0,0 @@
-/*
- *  arch/arm/include/asm/serial.h
- *
- *  Copyright (C) 1996 Russell King.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- *
- *  Changelog:
- *   15-10-1996	RMK	Created
- */
-
-#ifndef __ASM_SERIAL_H
-#define __ASM_SERIAL_H
-
-#define BASE_BAUD	(1843200 / 16)
-
-#endif
arch/arm/include/asm/shmbuf.h deleted +0 -42

42 lines not on this page · raw message

arch/arm/include/asm/socket.h deleted +0 -72

72 lines not on this page · raw message

arch/arm/include/asm/sockios.h deleted +0 -13
deleted file mode 100644
index a2588a2..0000000
@@ -1,13 +0,0 @@
-#ifndef __ARCH_ARM_SOCKIOS_H
-#define __ARCH_ARM_SOCKIOS_H
-
-/* Socket-level I/O control calls. */
-#define FIOSETOWN 	0x8901
-#define SIOCSPGRP	0x8902
-#define FIOGETOWN	0x8903
-#define SIOCGPGRP	0x8904
-#define SIOCATMARK	0x8905
-#define SIOCGSTAMP	0x8906		/* Get stamp (timeval) */
-#define SIOCGSTAMPNS	0x8907		/* Get stamp (timespec) */
-
-#endif
arch/arm/include/asm/termbits.h deleted +0 -198

198 lines not on this page · raw message

arch/arm/include/asm/termios.h deleted +0 -92

92 lines not on this page · raw message

arch/arm/include/asm/timex.h +1 -7
index 3be8de3..a6a42d5 100644
@@ -13,12 +13,6 @@
 #define _ASMARM_TIMEX_H
 
 #include <mach/timex.h>
-
-typedef unsigned long cycles_t;
-
-static inline cycles_t get_cycles (void)
-{
-	return 0;
-}
+#include <asm-generic/timex.h>
 
 #endif
arch/arm/include/asm/types.h deleted +0 -16
deleted file mode 100644
index 28beab9..0000000
@@ -1,16 +0,0 @@
-#ifndef __ASM_ARM_TYPES_H
-#define __ASM_ARM_TYPES_H
-
-#include <asm-generic/int-ll64.h>
-
-/*
- * These aren't exported outside the kernel to avoid name space clashes
- */
-#ifdef __KERNEL__
-
-#define BITS_PER_LONG 32
-
-#endif /* __KERNEL__ */
-
-#endif
-
arch/arm/include/asm/unaligned.h deleted +0 -19
deleted file mode 100644
index 44593a8..0000000
@@ -1,19 +0,0 @@
-#ifndef _ASM_ARM_UNALIGNED_H
-#define _ASM_ARM_UNALIGNED_H
-
-#include <linux/unaligned/le_byteshift.h>
-#include <linux/unaligned/be_byteshift.h>
-#include <linux/unaligned/generic.h>
-
-/*
- * Select endianness
- */
-#ifndef __ARMEB__
-#define get_unaligned	__get_unaligned_le
-#define put_unaligned	__put_unaligned_le
-#else
-#define get_unaligned	__get_unaligned_be
-#define put_unaligned	__put_unaligned_be
-#endif
-
-#endif /* _ASM_ARM_UNALIGNED_H */
Signature · 1 line
1.7.9.5
#2 lore raw

PATCH 1/3 ARM: use generic version of identical asm headers

Quoted 9 lines · Rob Herring

From: Rob Herring <[email protected]>

Inspired by the AArgh64 claim that it should be separate from ARM and one reason was being able to use more asm-generic headers. Doing a diff of arch/arm/include/asm and include/asm-generic there are numerous asm headers which are functionally identical to their asm-generic counterparts. Delete the ARM version and use the generic ones.

Signed-off-by Rob Herring <[email protected]>

Great stuff!

I had a similar series once but lost it when the kernel.org went down. I've looked at all your patches and noticed two files that are changing semantically with your patch. If we want to have the generic behavior, it's probably best to do those as separate patches.

Quoted 7 lines
arch/arm/include/asm/termios.h deleted +0 -7
-/*
- * Translate a "termio" structure into a "termios". Ugh.
- */
-#define SET_LOW_TERMIOS_BITS(termios, termio, x) {		\
-	unsigned short __tmp;					\
-	get_user(__tmp,&(termio)->x);				\
-	*(unsigned short *) &(termios)->x = __tmp;		\

arch/arm/include/asm/termios.h line 70

Quoted 10 lines
arch/arm/include/asm/termios.h deleted +0 -10
-}
-
-#define user_termio_to_kernel_termios(termios, termio) \
-({ \
-	SET_LOW_TERMIOS_BITS(termios, termio, c_iflag); \
-	SET_LOW_TERMIOS_BITS(termios, termio, c_oflag); \
-	SET_LOW_TERMIOS_BITS(termios, termio, c_cflag); \
-	SET_LOW_TERMIOS_BITS(termios, termio, c_lflag); \
-	copy_from_user((termios)->c_cc, (termio)->c_cc, NCC); \
-})

This code is not endian-safe, which means it's currently broken on big-endian machines. Using the asm-generic version fixes that problem but increases the code sizes.

arch/arm/include/asm/termios.h line 83

Quoted 12 lines
arch/arm/include/asm/termios.h deleted +0 -12
-/*
- * Translate a "termios" structure into a "termio". Ugh.
- */
-#define kernel_termios_to_user_termio(termio, termios) \
-({ \
-	put_user((termios)->c_iflag, &(termio)->c_iflag); \
-	put_user((termios)->c_oflag, &(termio)->c_oflag); \
-	put_user((termios)->c_cflag, &(termio)->c_cflag); \
-	put_user((termios)->c_lflag, &(termio)->c_lflag); \
-	put_user((termios)->c_line,  &(termio)->c_line); \
-	copy_to_user((termio)->c_cc, (termios)->c_cc, NCC); \
-})

This code is missing error handling, and again fixing this will increase the code size.

I think it's a good idea to fix these two issues, but let's use a separate patch for it.

Quoted 15 lines
arch/arm/include/asm/unaligned.h deleted +0 -9
deleted file mode 100644
index 44593a8..0000000
@@ -1,19 +0,0 @@
-#ifndef _ASM_ARM_UNALIGNED_H
-#define _ASM_ARM_UNALIGNED_H
-
-#include <linux/unaligned/le_byteshift.h>
-#include <linux/unaligned/be_byteshift.h>
-#include <linux/unaligned/generic.h>
-
-/*
- * Select endianness

arch/arm/include/asm/unaligned.h line 19

Quoted 10 lines
arch/arm/include/asm/unaligned.h deleted +0 -10
- */
-#ifndef __ARMEB__
-#define get_unaligned	__get_unaligned_le
-#define put_unaligned	__put_unaligned_le
-#else
-#define get_unaligned	__get_unaligned_be
-#define put_unaligned	__put_unaligned_be
-#endif
-
-#endif /* _ASM_ARM_UNALIGNED_H */

The asm-generic version uses the "struct" version for native-endian unaligned access and the "byteshift" version for the opposite endianess. The current ARM version however uses the "byteshift" implementation for both. I don't know what works better on ARM, but I'd say we should not change it unless we understand the effects.

Arnd

#3 lore raw

PATCH 1/3 ARM: use generic version of identical asm headers

Quoted 24 lines · Arnd Bergmann
Quoted 23 lines · Rob Herring
arch/arm/include/asm/unaligned.h deleted +0 -17
deleted file mode 100644
index 44593a8..0000000
@@ -1,19 +0,0 @@
-#ifndef _ASM_ARM_UNALIGNED_H
-#define _ASM_ARM_UNALIGNED_H
-
-#include <linux/unaligned/le_byteshift.h>
-#include <linux/unaligned/be_byteshift.h>
-#include <linux/unaligned/generic.h>
-
-/*
- * Select endianness
- */
-#ifndef __ARMEB__
-#define get_unaligned	__get_unaligned_le
-#define put_unaligned	__put_unaligned_le
-#else
-#define get_unaligned	__get_unaligned_be
-#define put_unaligned	__put_unaligned_be
-#endif
Quoted 8 lines

arch/arm/include/asm/unaligned.h line 19

Quoted 2 lines
arch/arm/include/asm/unaligned.h deleted +0 -2
-
-#endif /* _ASM_ARM_UNALIGNED_H */

The asm-generic version uses the "struct" version for native-endian unaligned access and the "byteshift" version for the opposite endianess. The current ARM version however uses the "byteshift" implementation for both. I don't know what works better on ARM, but I'd say we should not change it unless we understand the effects.

Test case:

int foo (int *x) { return get_unaligned(x); } long long bar (long long *x) { return get_unaligned(x); }

With the current ARM version:

foo:

	ldrb	r3, [r0, #2]	@ zero_extendqisi2	@ MEM[(const u8 *)x_1(D) + 2B], MEM[(const u8 *)x_1(D) + 2B]
	ldrb	r1, [r0, #1]	@ zero_extendqisi2	@ MEM[(const u8 *)x_1(D) + 1B], MEM[(const u8 *)x_1(D) + 1B]
	ldrb	r2, [r0, #0]	@ zero_extendqisi2	@ MEM[(const u8 *)x_1(D)], MEM[(const u8 *)x_1(D)]
	mov	r3, r3, asl #16	@ tmp154, MEM[(const u8 *)x_1(D) + 2B],
	ldrb	r0, [r0, #3]	@ zero_extendqisi2	@ MEM[(const u8 *)x_1(D) + 3B], MEM[(const u8 *)x_1(D) + 3B]
	orr	r3, r3, r1, asl #8	@, tmp155, tmp154, MEM[(const u8 *)x_1(D) + 1B],
	orr	r3, r3, r2	@ tmp157, tmp155, MEM[(const u8 *)x_1(D)]
	orr	r0, r3, r0, asl #24	@,, tmp157, MEM[(const u8 *)x_1(D) + 3B],
	bx	lr	@

bar:

	stmfd	sp!, {r4, r5, r6, r7}	@,
	mov	r2, #0	@ tmp184,
	ldrb	r5, [r0, #6]	@ zero_extendqisi2	@ MEM[(const u8 *)x_1(D) + 6B], MEM[(const u8 *)x_1(D) + 6B]
	ldrb	r4, [r0, #5]	@ zero_extendqisi2	@ MEM[(const u8 *)x_1(D) + 5B], MEM[(const u8 *)x_1(D) + 5B]
	ldrb	ip, [r0, #2]	@ zero_extendqisi2	@ MEM[(const u8 *)x_1(D) + 2B], MEM[(const u8 *)x_1(D) + 2B]
	ldrb	r1, [r0, #4]	@ zero_extendqisi2	@ MEM[(const u8 *)x_1(D) + 4B], MEM[(const u8 *)x_1(D) + 4B]
	mov	r5, r5, asl #16	@ tmp175, MEM[(const u8 *)x_1(D) + 6B],
	ldrb	r7, [r0, #1]	@ zero_extendqisi2	@ MEM[(const u8 *)x_1(D) + 1B], MEM[(const u8 *)x_1(D) + 1B]
	orr	r5, r5, r4, asl #8	@, tmp176, tmp175, MEM[(const u8 *)x_1(D) + 5B],
	ldrb	r6, [r0, #7]	@ zero_extendqisi2	@ MEM[(const u8 *)x_1(D) + 7B], MEM[(const u8 *)x_1(D) + 7B]
	orr	r5, r5, r1	@ tmp178, tmp176, MEM[(const u8 *)x_1(D) + 4B]
	ldrb	r4, [r0, #0]	@ zero_extendqisi2	@ MEM[(const u8 *)x_1(D)], MEM[(const u8 *)x_1(D)]
	mov	ip, ip, asl #16	@ tmp188, MEM[(const u8 *)x_1(D) + 2B],
	ldrb	r1, [r0, #3]	@ zero_extendqisi2	@ MEM[(const u8 *)x_1(D) + 3B], MEM[(const u8 *)x_1(D) + 3B]
	orr	ip, ip, r7, asl #8	@, tmp189, tmp188, MEM[(const u8 *)x_1(D) + 1B],
	orr	r3, r5, r6, asl #24	@,, tmp178, MEM[(const u8 *)x_1(D) + 7B],
	orr	ip, ip, r4	@ tmp191, tmp189, MEM[(const u8 *)x_1(D)]
	orr	ip, ip, r1, asl #24	@, tmp194, tmp191, MEM[(const u8 *)x_1(D) + 3B],
	mov	r1, r3	@,
	orr	r0, r2, ip	@ tmp171, tmp184, tmp194
	ldmfd	sp!, {r4, r5, r6, r7}
	bx	lr

In both cases the code is slightly suboptimal. One may wonder why wasting r2 with the constant 0 in the second case for example. And all the mov's could be folded in subsequent orr's, etc.

Now with the asm-generic version:

foo:

	ldr	r0, [r0, #0]	@ unaligned	@,* x
	bx	lr	@

bar:

	mov	r3, r0	@ x, x
	ldr	r0, [r0, #0]	@ unaligned	@,* x
	ldr	r1, [r3, #4]	@ unaligned	@,
	bx	lr	@

This is way better of course, but only because this was compiled for ARMv7. In this case the compiler knows that the hardware can do unaligned word access. This isn't that obvious for foo(), but if we remove the get_unaligned() from bar as follows:

long long bar (long long *x) {return *x; }

then the resulting code is:

bar:

	ldmia	r0, {r0, r1}	@ x,,
	bx	lr	@

So this proves that the presumed aligned vs unaligned cases does have influence on the instructions the compiler may use and that the above unaligned code results are not just an accident.

Still... this isn't fully conclusive without at least looking at the resulting assembly fron a pre ARMv6 compilation. Let's see with an ARMv5 target:

foo:

	ldrb	r3, [r0, #0]	@ zero_extendqisi2	@ tmp139,* x
	ldrb	r1, [r0, #1]	@ zero_extendqisi2	@ tmp140,
	ldrb	r2, [r0, #2]	@ zero_extendqisi2	@ tmp143,
	ldrb	r0, [r0, #3]	@ zero_extendqisi2	@ tmp146,
	orr	r3, r3, r1, asl #8	@, tmp142, tmp139, tmp140,
	orr	r3, r3, r2, asl #16	@, tmp145, tmp142, tmp143,
	orr	r0, r3, r0, asl #24	@,, tmp145, tmp146,
	bx	lr	@

bar:

	stmfd	sp!, {r4, r5, r6, r7}	@,
	ldrb	r2, [r0, #0]	@ zero_extendqisi2	@ tmp139,* x
	ldrb	r7, [r0, #1]	@ zero_extendqisi2	@ tmp140,
	ldrb	r3, [r0, #4]	@ zero_extendqisi2	@ tmp149,
	ldrb	r6, [r0, #5]	@ zero_extendqisi2	@ tmp150,
	ldrb	r5, [r0, #2]	@ zero_extendqisi2	@ tmp143,
	ldrb	r4, [r0, #6]	@ zero_extendqisi2	@ tmp153,
	ldrb	r1, [r0, #7]	@ zero_extendqisi2	@ tmp156,
	ldrb	ip, [r0, #3]	@ zero_extendqisi2	@ tmp146,
	orr	r2, r2, r7, asl #8	@, tmp142, tmp139, tmp140,
	orr	r3, r3, r6, asl #8	@, tmp152, tmp149, tmp150,
	orr	r2, r2, r5, asl #16	@, tmp145, tmp142, tmp143,
	orr	r3, r3, r4, asl #16	@, tmp155, tmp152, tmp153,
	orr	r0, r2, ip, asl #24	@,, tmp145, tmp146,
	orr	r1, r3, r1, asl #24	@,, tmp155, tmp156,
	ldmfd	sp!, {r4, r5, r6, r7}
	bx	lr

Compared to the initial results, this is really nicely optimized and I couldn't do much better if I were to hand code it myself.

Conclusion: Please go ahead with this change. This is really beneficial, especially for ARMv6+. However, given this does change the generated code (significantly for ARMv6+) I'd ask for this to be in a patch of its own to help bisect potential issues if they were to happen.

Then you can add:

Reviewed-by Nicolas Pitre <[email protected]>

Nicolas

author
#4 lore raw

PATCH 2/3 ARM: use asm-generic version of kmap_types.h

In reply to Rob Herring

From: Rob Herring <[email protected]>

kmap_types.h is virtually unused now with the stack based kmap_atomic, so we can just use the generic one. This does bump KM_TYPE_NR from 16 to 20, but there is still plenty of space left in the fixmap area.

Signed-off-by Rob Herring <[email protected]>
---
 arch/arm/include/asm/Kbuild       |    1 +
 arch/arm/include/asm/kmap_types.h |   33 ---------------------------------
 2 files changed, 1 insertion(+), 33 deletions(-)
 delete mode 100644 arch/arm/include/asm/kmap_types.h
arch/arm/include/asm/Kbuild +1 -0
index 8a7196c..c296c75 100644
@@ -13,6 +13,7 @@ generic-y += ioctl.h
 generic-y += ipcbuf.h
 generic-y += irq_regs.h
 generic-y += kdebug.h
+generic-y += kmap_types.h
 generic-y += local.h
 generic-y += local64.h
 generic-y += msgbuf.h
arch/arm/include/asm/kmap_types.h deleted +0 -33
deleted file mode 100644
index e51b1e8..0000000
@@ -1,33 +0,0 @@
-#ifndef __ARM_KMAP_TYPES_H
-#define __ARM_KMAP_TYPES_H
-
-/*
- * This is the "bare minimum".  AIO seems to require this.
- */
-enum km_type {
-	KM_BOUNCE_READ,
-	KM_SKB_SUNRPC_DATA,
-	KM_SKB_DATA_SOFTIRQ,
-	KM_USER0,
-	KM_USER1,
-	KM_BIO_SRC_IRQ,
-	KM_BIO_DST_IRQ,
-	KM_PTE0,
-	KM_PTE1,
-	KM_IRQ0,
-	KM_IRQ1,
-	KM_SOFTIRQ0,
-	KM_SOFTIRQ1,
-	KM_L1_CACHE,
-	KM_L2_CACHE,
-	KM_KDB,
-	KM_TYPE_NR
-};
-
-#ifdef CONFIG_DEBUG_HIGHMEM
-#define KM_NMI		(-1)
-#define KM_NMI_PTE	(-1)
-#define KM_IRQ_PTE	(-1)
-#endif
-
-#endif
Signature · 1 line
1.7.9.5
#5 lore raw

PATCH 2/3 ARM: use asm-generic version of kmap_types.h

Quoted 9 lines · Rob Herring

From: Rob Herring <[email protected]>

kmap_types.h is virtually unused now with the stack based kmap_atomic, so we can just use the generic one. This does bump KM_TYPE_NR from 16 to 20, but there is still plenty of space left in the fixmap area.

Signed-off-by Rob Herring <[email protected]>

---

I would suggest you drop this one. There is a series for removing kmap_types.h on all architectures already, so let's avoid the conflict.

Arnd

author
#6 lore raw

PATCH 2/3 ARM: use asm-generic version of kmap_types.h

Quoted 8 lines · Arnd Bergmann
Quoted 7 lines · Rob Herring

From: Rob Herring <[email protected]>

kmap_types.h is virtually unused now with the stack based kmap_atomic, so we can just use the generic one. This does bump KM_TYPE_NR from 16 to 20, but there is still plenty of space left in the fixmap area.

Quoted 5 lines
Quoted 2 lines
Signed-off-by Rob Herring <[email protected]>

---

I would suggest you drop this one. There is a series for removing kmap_types.h on all architectures already, so let's avoid the conflict.

Agreed. I was wondering why that hadn't happened...

Rob

author
#7 lore raw

PATCH 3/3 ARM: convert asm/irqflags.h to use asm-generic/irqflags.h

In reply to Rob Herring

From: Rob Herring <[email protected]>

By implmenting arch_local_irq_restore with constant flags, we can use asm-generic/irqflags.h and remove some of the ARM version of irqflags.h.

Verified the code is equivalent by checking the disassembled code.

Signed-off-by Rob Herring <[email protected]>
---
 arch/arm/include/asm/irqflags.h |  131 ++++++++++++++-------------------------
 1 file changed, 45 insertions(+), 86 deletions(-)
arch/arm/include/asm/irqflags.h +45 -86
index 1e6cca5..67bfb53 100644
@@ -8,88 +8,68 @@
 /*
  * CPU interrupt mask handling.
  */
-#if __LINUX_ARM_ARCH__ >= 6
+#define ARCH_IRQ_DISABLED	PSR_I_BIT
+#define ARCH_IRQ_ENABLED	0
 
-static inline unsigned long arch_local_irq_save(void)
+static inline unsigned long arch_local_save_flags(void)
 {
 	unsigned long flags;
 
 	asm volatile(
-		"	mrs	%0, cpsr	@ arch_local_irq_save\n"
-		"	cpsid	i"
+		"	mrs	%0, cpsr	@ arch_local_save_flags\n"
 		: "=r" (flags) : : "memory", "cc");
 	return flags;
 }
 
-static inline void arch_local_irq_enable(void)
+static inline void constant_arch_local_irq_restore(unsigned long flags)
 {
-	asm volatile(
-		"	cpsie i			@ arch_local_irq_enable"
-		:
-		:
-		: "memory", "cc");
+#if __LINUX_ARM_ARCH__ >= 6
+	if (flags == ARCH_IRQ_ENABLED)
+		asm volatile(
+			"	cpsie i			@ constant_arch_local_irq_restore"
+			: : : "memory", "cc");
+	else if (flags == ARCH_IRQ_DISABLED)
+		asm volatile(
+			"	cpsid i			@ constant_arch_local_irq_restore"
+			: : : "memory", "cc");
+#else
+	unsigned long temp;
+	if (flags == ARCH_IRQ_ENABLED)
+		asm volatile(
+			"	mrs	%0, cpsr	@ constant_arch_local_irq_restore\n"
+			"	bic	%0, %0, #128\n"
+			"	msr	cpsr_c, %0"
+			: "=r" (temp)
+			:
+			: "memory", "cc");
+	else if (flags == ARCH_IRQ_DISABLED)
+		asm volatile(
+			"	mrs	%0, cpsr	@ constant_arch_local_irq_restore\n"
+			"	orr	%0, %0, #128\n"
+			"	msr	cpsr_c, %0"
+			: "=r" (temp)
+			:
+			: "memory", "cc");
+#endif
 }
 
-static inline void arch_local_irq_disable(void)
+static inline void _arch_local_irq_restore(unsigned long flags)
 {
-	asm volatile(
-		"	cpsid i			@ arch_local_irq_disable"
-		:
+		asm volatile(
+		"	msr	cpsr_c, %0	@ _arch_local_irq_restore"
 		:
+		: "r" (flags)
 		: "memory", "cc");
 }
 
+#define arch_local_irq_restore(flags) \
+	(__builtin_constant_p(flags) ? constant_arch_local_irq_restore(flags) : \
+	_arch_local_irq_restore(flags))
+
+#if __LINUX_ARM_ARCH__ >= 6
 #define local_fiq_enable()  __asm__("cpsie f	@ __stf" : : : "memory", "cc")
 #define local_fiq_disable() __asm__("cpsid f	@ __clf" : : : "memory", "cc")
 #else
-
-/*
- * Save the current interrupt enable state & disable IRQs
- */
-static inline unsigned long arch_local_irq_save(void)
-{
-	unsigned long flags, temp;
-
-	asm volatile(
-		"	mrs	%0, cpsr	@ arch_local_irq_save\n"
-		"	orr	%1, %0, #128\n"
-		"	msr	cpsr_c, %1"
-		: "=r" (flags), "=r" (temp)
-		:
-		: "memory", "cc");
-	return flags;
-}
-
-/*
- * Enable IRQs
- */
-static inline void arch_local_irq_enable(void)
-{
-	unsigned long temp;
-	asm volatile(
-		"	mrs	%0, cpsr	@ arch_local_irq_enable\n"
-		"	bic	%0, %0, #128\n"
-		"	msr	cpsr_c, %0"
-		: "=r" (temp)
-		:
-		: "memory", "cc");
-}
-
-/*
- * Disable IRQs
- */
-static inline void arch_local_irq_disable(void)
-{
-	unsigned long temp;
-	asm volatile(
-		"	mrs	%0, cpsr	@ arch_local_irq_disable\n"
-		"	orr	%0, %0, #128\n"
-		"	msr	cpsr_c, %0"
-		: "=r" (temp)
-		:
-		: "memory", "cc");
-}
-
 /*
  * Enable FIQs
  */

@@ -122,34 +102,13 @@ static inline void arch_local_irq_disable(void)
 
 #endif
 
-/*
- * Save the current interrupt enable state.
- */
-static inline unsigned long arch_local_save_flags(void)
-{
-	unsigned long flags;
-	asm volatile(
-		"	mrs	%0, cpsr	@ local_save_flags"
-		: "=r" (flags) : : "memory", "cc");
-	return flags;
-}
-
-/*
- * restore saved IRQ & FIQ state
- */
-static inline void arch_local_irq_restore(unsigned long flags)
-{
-	asm volatile(
-		"	msr	cpsr_c, %0	@ local_irq_restore"
-		:
-		: "r" (flags)
-		: "memory", "cc");
-}
-
 static inline int arch_irqs_disabled_flags(unsigned long flags)
 {
 	return flags & PSR_I_BIT;
 }
+#define arch_irqs_disabled_flags arch_irqs_disabled_flags
+
+#include <asm-generic/irqflags.h>
 
 #endif
 #endif
Signature · 1 line
1.7.9.5
#8 lore raw

PATCH 3/3 ARM: convert asm/irqflags.h to use asm-generic/irqflags.h

Quoted 6 lines · Rob Herring

From: Rob Herring <[email protected]>

By implmenting arch_local_irq_restore with constant flags, we can use asm-generic/irqflags.h and remove some of the ARM version of irqflags.h.

[...]

arch/arm/include/asm/irqflags.h static inline void arch_local_irq_disable(void) · line 110

Quoted 11 lines
arch/arm/include/asm/irqflags.h +2 -4
-		: "r" (flags)
-		: "memory", "cc");
-}
-
 static inline int arch_irqs_disabled_flags(unsigned long flags)
 {
 	return flags & PSR_I_BIT;
 }
+#define arch_irqs_disabled_flags arch_irqs_disabled_flags
+

Why don't directly define arch_irqs_disabled_flags like that:

#define arch_irqs_disabled_flags(flags) (flags & PSR_I_BIT)

Quoted 5 lines

+#include <asm-generic/irqflags.h>

#endif
#endif

author
#9 lore raw

PATCH 3/3 ARM: convert asm/irqflags.h to use asm-generic/irqflags.h

Quoted 21 lines · Hui Wang
Quoted 6 lines · Rob Herring

From: Rob Herring <[email protected]>

By implmenting arch_local_irq_restore with constant flags, we can use asm-generic/irqflags.h and remove some of the ARM version of irqflags.h.

[...]

arch/arm/include/asm/irqflags.h static inline void arch_local_irq_disable(void) · line 110

Quoted 11 lines
arch/arm/include/asm/irqflags.h +2 -4
-        : "r" (flags)
-        : "memory", "cc");
-}
-
 static inline int arch_irqs_disabled_flags(unsigned long flags)
 {
     return flags & PSR_I_BIT;
 }
+#define arch_irqs_disabled_flags arch_irqs_disabled_flags
+

Why don't directly define arch_irqs_disabled_flags like that:

Quoted 1 line

#define arch_irqs_disabled_flags(flags) (flags & PSR_I_BIT)

In most cases, I think inlines are preferred as they have type checking. Doesn't really matter to me.

Rob

Quoted 7 lines
Quoted 6 lines

+#include <asm-generic/irqflags.h>

#endif
#endif

#10 lore raw

PATCH 3/3 ARM: convert asm/irqflags.h to use asm-generic/irqflags.h

Quoted 4 lines

In most cases, I think inlines are preferred as they have type checking.

Make sense.

Quoted 2 lines

Doesn't really matter to me.

Just a small case. :-)

Regards,
Hui.

Quoted 13 lines

Rob

Quoted 6 lines
Quoted 6 lines

+#include <asm-generic/irqflags.h>

#endif
#endif