2014-10-24 05:10:40

by Wang, Yalin

[permalink] [raw]
Subject: [PATCH RFC] arm/arm64:add CONFIG_HAVE_ARCH_BITREVERSE to support rbit

this change add CONFIG_HAVE_ARCH_BITREVERSE config option,
so that we can use arm/arm64 rbit instruction to do bitrev operation
by hardware.

Signed-off-by: Yalin Wang <[email protected]>
---
arch/arm/Kconfig | 1 +
arch/arm/include/asm/bitrev.h | 21 +++++++++++++++++++++
arch/arm64/Kconfig | 1 +
arch/arm64/include/asm/bitrev.h | 21 +++++++++++++++++++++
include/linux/bitrev.h | 9 +++++++++
lib/Kconfig | 8 ++++++++
lib/bitrev.c | 2 ++
7 files changed, 63 insertions(+)
create mode 100644 arch/arm/include/asm/bitrev.h
create mode 100644 arch/arm64/include/asm/bitrev.h

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 89c4b5c..426cbcc 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -16,6 +16,7 @@ config ARM
select DCACHE_WORD_ACCESS if HAVE_EFFICIENT_UNALIGNED_ACCESS
select GENERIC_ALLOCATOR
select GENERIC_ATOMIC64 if (CPU_V7M || CPU_V6 || !CPU_32v6K || !AEABI)
+ select HAVE_ARCH_BITREVERSE if (CPU_V7M || CPU_V7)
select GENERIC_CLOCKEVENTS_BROADCAST if SMP
select GENERIC_IDLE_POLL_SETUP
select GENERIC_IRQ_PROBE
diff --git a/arch/arm/include/asm/bitrev.h b/arch/arm/include/asm/bitrev.h
new file mode 100644
index 0000000..0df5866
--- /dev/null
+++ b/arch/arm/include/asm/bitrev.h
@@ -0,0 +1,21 @@
+#ifndef __ASM_ARM_BITREV_H
+#define __ASM_ARM_BITREV_H
+
+static inline __attribute_const__ u32 __arch_bitrev32(u32 x)
+{
+ __asm__ ("rbit %0, %1" : "=r" (x) : "r" (x));
+ return x;
+}
+
+static inline __attribute_const__ u16 __arch_bitrev16(u16 x)
+{
+ return __arch_bitrev32((u32)x) >> 16;
+}
+
+static inline __attribute_const__ u8 __arch_bitrev8(u8 x)
+{
+ return __arch_bitrev32((u32)x) >> 24;
+}
+
+#endif
+
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index ac9afde..a2566d7 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -35,6 +35,7 @@ config ARM64
select HARDIRQS_SW_RESEND
select HAVE_ARCH_AUDITSYSCALL
select HAVE_ARCH_JUMP_LABEL
+ select HAVE_ARCH_BITREVERSE
select HAVE_ARCH_KGDB
select HAVE_ARCH_TRACEHOOK
select HAVE_BPF_JIT
diff --git a/arch/arm64/include/asm/bitrev.h b/arch/arm64/include/asm/bitrev.h
new file mode 100644
index 0000000..5d24c11
--- /dev/null
+++ b/arch/arm64/include/asm/bitrev.h
@@ -0,0 +1,21 @@
+#ifndef __ASM_ARM_BITREV_H
+#define __ASM_ARM_BITREV_H
+
+static inline __attribute_const__ u32 __arch_bitrev32(u32 x)
+{
+ __asm__ ("rbit %w0, %w1" : "=r" (x) : "r" (x));
+ return x;
+}
+
+static inline __attribute_const__ u16 __arch_bitrev16(u16 x)
+{
+ return __arch_bitrev32((u32)x) >> 16;
+}
+
+static inline __attribute_const__ u8 __arch_bitrev8(u8 x)
+{
+ return __arch_bitrev32((u32)x) >> 24;
+}
+
+#endif
+
diff --git a/include/linux/bitrev.h b/include/linux/bitrev.h
index 7ffe03f..ef5b2bb 100644
--- a/include/linux/bitrev.h
+++ b/include/linux/bitrev.h
@@ -3,6 +3,14 @@

#include <linux/types.h>

+#ifdef CONFIG_HAVE_ARCH_BITREVERSE
+#include <asm/bitrev.h>
+
+#define bitrev32 __arch_bitrev32
+#define bitrev16 __arch_bitrev16
+#define bitrev8 __arch_bitrev8
+
+#else
extern u8 const byte_rev_table[256];

static inline u8 bitrev8(u8 byte)
@@ -13,4 +21,5 @@ static inline u8 bitrev8(u8 byte)
extern u16 bitrev16(u16 in);
extern u32 bitrev32(u32 in);

+#endif /* CONFIG_HAVE_ARCH_BITREVERSE */
#endif /* _LINUX_BITREV_H */
diff --git a/lib/Kconfig b/lib/Kconfig
index 54cf309..e0e0453 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -13,6 +13,14 @@ config RAID6_PQ
config BITREVERSE
tristate

+config HAVE_ARCH_BITREVERSE
+ boolean
+ default n
+ help
+ This option provides an config for the architecture which have instruction
+ can do bitreverse operation, we use the hardware instruction if the architecture
+ have this capability.
+
config RATIONAL
boolean

diff --git a/lib/bitrev.c b/lib/bitrev.c
index 3956203..93d637a 100644
--- a/lib/bitrev.c
+++ b/lib/bitrev.c
@@ -1,3 +1,4 @@
+#ifndef CONFIG_HAVE_ARCH_BITREVERSE
#include <linux/types.h>
#include <linux/module.h>
#include <linux/bitrev.h>
@@ -57,3 +58,4 @@ u32 bitrev32(u32 x)
return (bitrev16(x & 0xffff) << 16) | bitrev16(x >> 16);
}
EXPORT_SYMBOL(bitrev32);
+#endif /* CONFIG_HAVE_ARCH_BITREVERSE */
--
2.1.1


2014-10-27 06:38:34

by Wang, Yalin

[permalink] [raw]
Subject: [RFC V2] arm/arm64:add CONFIG_HAVE_ARCH_BITREVERSE to support rbit instruction

this change add CONFIG_HAVE_ARCH_BITREVERSE config option,
so that we can use arm/arm64 rbit instruction to do bitrev operation
by hardware.

Signed-off-by: Yalin Wang <[email protected]>
---
arch/arm/Kconfig | 1 +
arch/arm/include/asm/bitrev.h | 21 +++++++++++++++++++++
arch/arm64/Kconfig | 1 +
arch/arm64/include/asm/bitrev.h | 21 +++++++++++++++++++++
include/linux/bitrev.h | 9 +++++++++
lib/Kconfig | 9 +++++++++
lib/bitrev.c | 2 ++
7 files changed, 64 insertions(+)
create mode 100644 arch/arm/include/asm/bitrev.h
create mode 100644 arch/arm64/include/asm/bitrev.h

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 89c4b5c..426cbcc 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -16,6 +16,7 @@ config ARM
select DCACHE_WORD_ACCESS if HAVE_EFFICIENT_UNALIGNED_ACCESS
select GENERIC_ALLOCATOR
select GENERIC_ATOMIC64 if (CPU_V7M || CPU_V6 || !CPU_32v6K || !AEABI)
+ select HAVE_ARCH_BITREVERSE if (CPU_V7M || CPU_V7)
select GENERIC_CLOCKEVENTS_BROADCAST if SMP
select GENERIC_IDLE_POLL_SETUP
select GENERIC_IRQ_PROBE
diff --git a/arch/arm/include/asm/bitrev.h b/arch/arm/include/asm/bitrev.h
new file mode 100644
index 0000000..0df5866
--- /dev/null
+++ b/arch/arm/include/asm/bitrev.h
@@ -0,0 +1,21 @@
+#ifndef __ASM_ARM_BITREV_H
+#define __ASM_ARM_BITREV_H
+
+static inline __attribute_const__ u32 __arch_bitrev32(u32 x)
+{
+ __asm__ ("rbit %0, %1" : "=r" (x) : "r" (x));
+ return x;
+}
+
+static inline __attribute_const__ u16 __arch_bitrev16(u16 x)
+{
+ return __arch_bitrev32((u32)x) >> 16;
+}
+
+static inline __attribute_const__ u8 __arch_bitrev8(u8 x)
+{
+ return __arch_bitrev32((u32)x) >> 24;
+}
+
+#endif
+
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 9532f8d..263c28c 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -36,6 +36,7 @@ config ARM64
select HARDIRQS_SW_RESEND
select HAVE_ARCH_AUDITSYSCALL
select HAVE_ARCH_JUMP_LABEL
+ select HAVE_ARCH_BITREVERSE
select HAVE_ARCH_KGDB
select HAVE_ARCH_TRACEHOOK
select HAVE_BPF_JIT
diff --git a/arch/arm64/include/asm/bitrev.h b/arch/arm64/include/asm/bitrev.h
new file mode 100644
index 0000000..5d24c11
--- /dev/null
+++ b/arch/arm64/include/asm/bitrev.h
@@ -0,0 +1,21 @@
+#ifndef __ASM_ARM_BITREV_H
+#define __ASM_ARM_BITREV_H
+
+static inline __attribute_const__ u32 __arch_bitrev32(u32 x)
+{
+ __asm__ ("rbit %w0, %w1" : "=r" (x) : "r" (x));
+ return x;
+}
+
+static inline __attribute_const__ u16 __arch_bitrev16(u16 x)
+{
+ return __arch_bitrev32((u32)x) >> 16;
+}
+
+static inline __attribute_const__ u8 __arch_bitrev8(u8 x)
+{
+ return __arch_bitrev32((u32)x) >> 24;
+}
+
+#endif
+
diff --git a/include/linux/bitrev.h b/include/linux/bitrev.h
index 7ffe03f..ef5b2bb 100644
--- a/include/linux/bitrev.h
+++ b/include/linux/bitrev.h
@@ -3,6 +3,14 @@

#include <linux/types.h>

+#ifdef CONFIG_HAVE_ARCH_BITREVERSE
+#include <asm/bitrev.h>
+
+#define bitrev32 __arch_bitrev32
+#define bitrev16 __arch_bitrev16
+#define bitrev8 __arch_bitrev8
+
+#else
extern u8 const byte_rev_table[256];

static inline u8 bitrev8(u8 byte)
@@ -13,4 +21,5 @@ static inline u8 bitrev8(u8 byte)
extern u16 bitrev16(u16 in);
extern u32 bitrev32(u32 in);

+#endif /* CONFIG_HAVE_ARCH_BITREVERSE */
#endif /* _LINUX_BITREV_H */
diff --git a/lib/Kconfig b/lib/Kconfig
index 54cf309..cd177ca 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -13,6 +13,15 @@ config RAID6_PQ
config BITREVERSE
tristate

+config HAVE_ARCH_BITREVERSE
+ boolean
+ default n
+ depends on BITREVERSE
+ help
+ This option provides an config for the architecture which have instruction
+ can do bitreverse operation, we use the hardware instruction if the architecture
+ have this capability.
+
config RATIONAL
boolean

diff --git a/lib/bitrev.c b/lib/bitrev.c
index 3956203..93d637a 100644
--- a/lib/bitrev.c
+++ b/lib/bitrev.c
@@ -1,3 +1,4 @@
+#ifndef CONFIG_HAVE_ARCH_BITREVERSE
#include <linux/types.h>
#include <linux/module.h>
#include <linux/bitrev.h>
@@ -57,3 +58,4 @@ u32 bitrev32(u32 x)
return (bitrev16(x & 0xffff) << 16) | bitrev16(x >> 16);
}
EXPORT_SYMBOL(bitrev32);
+#endif /* CONFIG_HAVE_ARCH_BITREVERSE */
--
2.1.1

2014-10-27 06:46:15

by Joe Perches

[permalink] [raw]
Subject: Re: [RFC V2] arm/arm64:add CONFIG_HAVE_ARCH_BITREVERSE to support rbit instruction

On Mon, 2014-10-27 at 14:37 +0800, Wang, Yalin wrote:
> this change add CONFIG_HAVE_ARCH_BITREVERSE config option,
> so that we can use arm/arm64 rbit instruction to do bitrev operation
> by hardware.
>
> Signed-off-by: Yalin Wang <[email protected]>
> ---
> arch/arm/Kconfig | 1 +
> arch/arm/include/asm/bitrev.h | 21 +++++++++++++++++++++
> arch/arm64/Kconfig | 1 +
> arch/arm64/include/asm/bitrev.h | 21 +++++++++++++++++++++
> include/linux/bitrev.h | 9 +++++++++
> lib/Kconfig | 9 +++++++++
> lib/bitrev.c | 2 ++
> 7 files changed, 64 insertions(+)
> create mode 100644 arch/arm/include/asm/bitrev.h
> create mode 100644 arch/arm64/include/asm/bitrev.h
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 89c4b5c..426cbcc 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -16,6 +16,7 @@ config ARM
> select DCACHE_WORD_ACCESS if HAVE_EFFICIENT_UNALIGNED_ACCESS
> select GENERIC_ALLOCATOR
> select GENERIC_ATOMIC64 if (CPU_V7M || CPU_V6 || !CPU_32v6K || !AEABI)
> + select HAVE_ARCH_BITREVERSE if (CPU_V7M || CPU_V7)
> select GENERIC_CLOCKEVENTS_BROADCAST if SMP
> select GENERIC_IDLE_POLL_SETUP
> select GENERIC_IRQ_PROBE
> diff --git a/arch/arm/include/asm/bitrev.h b/arch/arm/include/asm/bitrev.h
> new file mode 100644
> index 0000000..0df5866
> --- /dev/null
> +++ b/arch/arm/include/asm/bitrev.h
> @@ -0,0 +1,21 @@
> +#ifndef __ASM_ARM_BITREV_H
> +#define __ASM_ARM_BITREV_H
> +
> +static inline __attribute_const__ u32 __arch_bitrev32(u32 x)
> +{
> + __asm__ ("rbit %0, %1" : "=r" (x) : "r" (x));
> + return x;
> +}
> +
> +static inline __attribute_const__ u16 __arch_bitrev16(u16 x)
> +{
> + return __arch_bitrev32((u32)x) >> 16;
> +}
> +
> +static inline __attribute_const__ u8 __arch_bitrev8(u8 x)
> +{
> + return __arch_bitrev32((u32)x) >> 24;
> +}
> +
> +#endif
> +
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 9532f8d..263c28c 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -36,6 +36,7 @@ config ARM64
> select HARDIRQS_SW_RESEND
> select HAVE_ARCH_AUDITSYSCALL
> select HAVE_ARCH_JUMP_LABEL
> + select HAVE_ARCH_BITREVERSE
> select HAVE_ARCH_KGDB
> select HAVE_ARCH_TRACEHOOK
> select HAVE_BPF_JIT
> diff --git a/arch/arm64/include/asm/bitrev.h b/arch/arm64/include/asm/bitrev.h
> new file mode 100644
> index 0000000..5d24c11
> --- /dev/null
> +++ b/arch/arm64/include/asm/bitrev.h
> @@ -0,0 +1,21 @@
> +#ifndef __ASM_ARM_BITREV_H
> +#define __ASM_ARM_BITREV_H
> +
> +static inline __attribute_const__ u32 __arch_bitrev32(u32 x)
> +{
> + __asm__ ("rbit %w0, %w1" : "=r" (x) : "r" (x));
> + return x;
> +}
> +
> +static inline __attribute_const__ u16 __arch_bitrev16(u16 x)
> +{
> + return __arch_bitrev32((u32)x) >> 16;
> +}
> +
> +static inline __attribute_const__ u8 __arch_bitrev8(u8 x)
> +{
> + return __arch_bitrev32((u32)x) >> 24;
> +}
> +
> +#endif
> +
> diff --git a/include/linux/bitrev.h b/include/linux/bitrev.h
> index 7ffe03f..ef5b2bb 100644
> --- a/include/linux/bitrev.h
> +++ b/include/linux/bitrev.h
> @@ -3,6 +3,14 @@
>
> #include <linux/types.h>
>
> +#ifdef CONFIG_HAVE_ARCH_BITREVERSE
> +#include <asm/bitrev.h>
> +
> +#define bitrev32 __arch_bitrev32
> +#define bitrev16 __arch_bitrev16
> +#define bitrev8 __arch_bitrev8
> +
> +#else
> extern u8 const byte_rev_table[256];

If this is done, the direct uses of byte_rev_table in
drivers/net/wireless/ath/carl9170/phy.c and
sound/usb/6fire/firmware.c should be converted too?

2014-10-27 07:13:55

by Wang, Yalin

[permalink] [raw]
Subject: RE: [RFC V2] arm/arm64:add CONFIG_HAVE_ARCH_BITREVERSE to support rbit instruction



>
> If this is done, the direct uses of byte_rev_table in
> drivers/net/wireless/ath/carl9170/phy.c and sound/usb/6fire/firmware.c
> should be converted too?
>

I think use bitrev8() is safer than to use byte_rev_table[] directly.

2014-10-27 08:02:22

by Wang, Yalin

[permalink] [raw]
Subject: [RFC V3] arm/arm64:add CONFIG_HAVE_ARCH_BITREVERSE to support rbit instruction

this change add CONFIG_HAVE_ARCH_BITREVERSE config option,
so that we can use arm/arm64 rbit instruction to do bitrev operation
by hardware.

Signed-off-by: Yalin Wang <[email protected]>
---
arch/arm/Kconfig | 1 +
arch/arm/include/asm/bitrev.h | 28 ++++++++++++++++++++++++++++
arch/arm64/Kconfig | 1 +
arch/arm64/include/asm/bitrev.h | 28 ++++++++++++++++++++++++++++
include/linux/bitrev.h | 9 +++++++++
lib/Kconfig | 9 +++++++++
lib/bitrev.c | 2 ++
7 files changed, 78 insertions(+)
create mode 100644 arch/arm/include/asm/bitrev.h
create mode 100644 arch/arm64/include/asm/bitrev.h

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 89c4b5c..426cbcc 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -16,6 +16,7 @@ config ARM
select DCACHE_WORD_ACCESS if HAVE_EFFICIENT_UNALIGNED_ACCESS
select GENERIC_ALLOCATOR
select GENERIC_ATOMIC64 if (CPU_V7M || CPU_V6 || !CPU_32v6K || !AEABI)
+ select HAVE_ARCH_BITREVERSE if (CPU_V7M || CPU_V7)
select GENERIC_CLOCKEVENTS_BROADCAST if SMP
select GENERIC_IDLE_POLL_SETUP
select GENERIC_IRQ_PROBE
diff --git a/arch/arm/include/asm/bitrev.h b/arch/arm/include/asm/bitrev.h
new file mode 100644
index 0000000..c21a5f4
--- /dev/null
+++ b/arch/arm/include/asm/bitrev.h
@@ -0,0 +1,28 @@
+#ifndef __ASM_ARM_BITREV_H
+#define __ASM_ARM_BITREV_H
+
+static __always_inline __attribute_const__ u32 __arch_bitrev32(u32 x)
+{
+ if (__builtin_constant_p(x)) {
+ x = (x >> 16) | (x << 16);
+ x = ((x & 0xFF00FF00) >> 8) | ((x & 0x00FF00FF) << 8);
+ x = ((x & 0xF0F0F0F0) >> 4) | ((x & 0x0F0F0F0F) << 4);
+ x = ((x & 0xCCCCCCCC) >> 2) | ((x & 0x33333333) << 2);
+ return ((x & 0xAAAAAAAA) >> 1) | ((x & 0x55555555) << 1);
+ }
+ __asm__ ("rbit %0, %1" : "=r" (x) : "r" (x));
+ return x;
+}
+
+static __always_inline __attribute_const__ u16 __arch_bitrev16(u16 x)
+{
+ return __arch_bitrev32((u32)x) >> 16;
+}
+
+static __always_inline __attribute_const__ u8 __arch_bitrev8(u8 x)
+{
+ return __arch_bitrev32((u32)x) >> 24;
+}
+
+#endif
+
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 9532f8d..263c28c 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -36,6 +36,7 @@ config ARM64
select HARDIRQS_SW_RESEND
select HAVE_ARCH_AUDITSYSCALL
select HAVE_ARCH_JUMP_LABEL
+ select HAVE_ARCH_BITREVERSE
select HAVE_ARCH_KGDB
select HAVE_ARCH_TRACEHOOK
select HAVE_BPF_JIT
diff --git a/arch/arm64/include/asm/bitrev.h b/arch/arm64/include/asm/bitrev.h
new file mode 100644
index 0000000..f725a71
--- /dev/null
+++ b/arch/arm64/include/asm/bitrev.h
@@ -0,0 +1,28 @@
+#ifndef __ASM_ARM_BITREV_H
+#define __ASM_ARM_BITREV_H
+
+static __always_inline __attribute_const__ u32 __arch_bitrev32(u32 x)
+{
+ if (__builtin_constant_p(x)) {
+ x = (x >> 16) | (x << 16);
+ x = ((x & 0xFF00FF00) >> 8) | ((x & 0x00FF00FF) << 8);
+ x = ((x & 0xF0F0F0F0) >> 4) | ((x & 0x0F0F0F0F) << 4);
+ x = ((x & 0xCCCCCCCC) >> 2) | ((x & 0x33333333) << 2);
+ return ((x & 0xAAAAAAAA) >> 1) | ((x & 0x55555555) << 1);
+ }
+ __asm__ ("rbit %w0, %w1" : "=r" (x) : "r" (x));
+ return x;
+}
+
+static __always_inline __attribute_const__ u16 __arch_bitrev16(u16 x)
+{
+ return __arch_bitrev32((u32)x) >> 16;
+}
+
+static __always_inline __attribute_const__ u8 __arch_bitrev8(u8 x)
+{
+ return __arch_bitrev32((u32)x) >> 24;
+}
+
+#endif
+
diff --git a/include/linux/bitrev.h b/include/linux/bitrev.h
index 7ffe03f..ef5b2bb 100644
--- a/include/linux/bitrev.h
+++ b/include/linux/bitrev.h
@@ -3,6 +3,14 @@

#include <linux/types.h>

+#ifdef CONFIG_HAVE_ARCH_BITREVERSE
+#include <asm/bitrev.h>
+
+#define bitrev32 __arch_bitrev32
+#define bitrev16 __arch_bitrev16
+#define bitrev8 __arch_bitrev8
+
+#else
extern u8 const byte_rev_table[256];

static inline u8 bitrev8(u8 byte)
@@ -13,4 +21,5 @@ static inline u8 bitrev8(u8 byte)
extern u16 bitrev16(u16 in);
extern u32 bitrev32(u32 in);

+#endif /* CONFIG_HAVE_ARCH_BITREVERSE */
#endif /* _LINUX_BITREV_H */
diff --git a/lib/Kconfig b/lib/Kconfig
index 54cf309..cd177ca 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -13,6 +13,15 @@ config RAID6_PQ
config BITREVERSE
tristate

+config HAVE_ARCH_BITREVERSE
+ boolean
+ default n
+ depends on BITREVERSE
+ help
+ This option provides an config for the architecture which have instruction
+ can do bitreverse operation, we use the hardware instruction if the architecture
+ have this capability.
+
config RATIONAL
boolean

diff --git a/lib/bitrev.c b/lib/bitrev.c
index 3956203..93d637a 100644
--- a/lib/bitrev.c
+++ b/lib/bitrev.c
@@ -1,3 +1,4 @@
+#ifndef CONFIG_HAVE_ARCH_BITREVERSE
#include <linux/types.h>
#include <linux/module.h>
#include <linux/bitrev.h>
@@ -57,3 +58,4 @@ u32 bitrev32(u32 x)
return (bitrev16(x & 0xffff) << 16) | bitrev16(x >> 16);
}
EXPORT_SYMBOL(bitrev32);
+#endif /* CONFIG_HAVE_ARCH_BITREVERSE */
--
2.1.1

2014-10-27 10:49:04

by Will Deacon

[permalink] [raw]
Subject: Re: [RFC V3] arm/arm64:add CONFIG_HAVE_ARCH_BITREVERSE to support rbit instruction

On Mon, Oct 27, 2014 at 08:02:08AM +0000, Wang, Yalin wrote:
> this change add CONFIG_HAVE_ARCH_BITREVERSE config option,
> so that we can use arm/arm64 rbit instruction to do bitrev operation
> by hardware.
>
> Signed-off-by: Yalin Wang <[email protected]>
> ---
> arch/arm/Kconfig | 1 +
> arch/arm/include/asm/bitrev.h | 28 ++++++++++++++++++++++++++++
> arch/arm64/Kconfig | 1 +
> arch/arm64/include/asm/bitrev.h | 28 ++++++++++++++++++++++++++++
> include/linux/bitrev.h | 9 +++++++++
> lib/Kconfig | 9 +++++++++
> lib/bitrev.c | 2 ++
> 7 files changed, 78 insertions(+)
> create mode 100644 arch/arm/include/asm/bitrev.h
> create mode 100644 arch/arm64/include/asm/bitrev.h
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 89c4b5c..426cbcc 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -16,6 +16,7 @@ config ARM
> select DCACHE_WORD_ACCESS if HAVE_EFFICIENT_UNALIGNED_ACCESS
> select GENERIC_ALLOCATOR
> select GENERIC_ATOMIC64 if (CPU_V7M || CPU_V6 || !CPU_32v6K || !AEABI)
> + select HAVE_ARCH_BITREVERSE if (CPU_V7M || CPU_V7)
> select GENERIC_CLOCKEVENTS_BROADCAST if SMP
> select GENERIC_IDLE_POLL_SETUP
> select GENERIC_IRQ_PROBE
> diff --git a/arch/arm/include/asm/bitrev.h b/arch/arm/include/asm/bitrev.h
> new file mode 100644
> index 0000000..c21a5f4
> --- /dev/null
> +++ b/arch/arm/include/asm/bitrev.h
> @@ -0,0 +1,28 @@
> +#ifndef __ASM_ARM_BITREV_H
> +#define __ASM_ARM_BITREV_H
> +
> +static __always_inline __attribute_const__ u32 __arch_bitrev32(u32 x)
> +{
> + if (__builtin_constant_p(x)) {
> + x = (x >> 16) | (x << 16);
> + x = ((x & 0xFF00FF00) >> 8) | ((x & 0x00FF00FF) << 8);
> + x = ((x & 0xF0F0F0F0) >> 4) | ((x & 0x0F0F0F0F) << 4);
> + x = ((x & 0xCCCCCCCC) >> 2) | ((x & 0x33333333) << 2);
> + return ((x & 0xAAAAAAAA) >> 1) | ((x & 0x55555555) << 1);
> + }
> + __asm__ ("rbit %0, %1" : "=r" (x) : "r" (x));

I think you need to use %w0 and %w1 here, otherwise you bit-reverse the
64-bit register.

Will

2014-10-28 01:34:48

by Wang, Yalin

[permalink] [raw]
Subject: RE: [RFC V3] arm/arm64:add CONFIG_HAVE_ARCH_BITREVERSE to support rbit instruction

> From: Will Deacon [mailto:[email protected]]
> > +++ b/arch/arm/include/asm/bitrev.h
> > @@ -0,0 +1,28 @@
> > +#ifndef __ASM_ARM_BITREV_H
> > +#define __ASM_ARM_BITREV_H
> > +
> > +static __always_inline __attribute_const__ u32 __arch_bitrev32(u32 x)
> > +{
> > + if (__builtin_constant_p(x)) {
> > + x = (x >> 16) | (x << 16);
> > + x = ((x & 0xFF00FF00) >> 8) | ((x & 0x00FF00FF) << 8);
> > + x = ((x & 0xF0F0F0F0) >> 4) | ((x & 0x0F0F0F0F) << 4);
> > + x = ((x & 0xCCCCCCCC) >> 2) | ((x & 0x33333333) << 2);
> > + return ((x & 0xAAAAAAAA) >> 1) | ((x & 0x55555555) << 1);
> > + }
> > + __asm__ ("rbit %0, %1" : "=r" (x) : "r" (x));
>
> I think you need to use %w0 and %w1 here, otherwise you bit-reverse the 64-
> bit register.
For arm64 in arch/arm64/include/asm/bitrev.h.
I have use __asm__ ("rbit %w0, %w1" : "=r" (x) : "r" (x));
For arm , I use __asm__ ("rbit %0, %1" : "=r" (x) : "r" (x));
Am I right ?
Thanks

2014-10-28 13:59:52

by Will Deacon

[permalink] [raw]
Subject: Re: [RFC V3] arm/arm64:add CONFIG_HAVE_ARCH_BITREVERSE to support rbit instruction

On Tue, Oct 28, 2014 at 01:34:42AM +0000, Wang, Yalin wrote:
> > From: Will Deacon [mailto:[email protected]]
> > > +++ b/arch/arm/include/asm/bitrev.h
> > > @@ -0,0 +1,28 @@
> > > +#ifndef __ASM_ARM_BITREV_H
> > > +#define __ASM_ARM_BITREV_H
> > > +
> > > +static __always_inline __attribute_const__ u32 __arch_bitrev32(u32 x)
> > > +{
> > > + if (__builtin_constant_p(x)) {
> > > + x = (x >> 16) | (x << 16);
> > > + x = ((x & 0xFF00FF00) >> 8) | ((x & 0x00FF00FF) << 8);
> > > + x = ((x & 0xF0F0F0F0) >> 4) | ((x & 0x0F0F0F0F) << 4);
> > > + x = ((x & 0xCCCCCCCC) >> 2) | ((x & 0x33333333) << 2);
> > > + return ((x & 0xAAAAAAAA) >> 1) | ((x & 0x55555555) << 1);
> > > + }
> > > + __asm__ ("rbit %0, %1" : "=r" (x) : "r" (x));
> >
> > I think you need to use %w0 and %w1 here, otherwise you bit-reverse the 64-
> > bit register.
> For arm64 in arch/arm64/include/asm/bitrev.h.
> I have use __asm__ ("rbit %w0, %w1" : "=r" (x) : "r" (x));
> For arm , I use __asm__ ("rbit %0, %1" : "=r" (x) : "r" (x));
> Am I right ?

Yup, sorry, I didn't realise this patch covered both architectures. It would
probably be a good idea to split it into 3 parts: a core part, then the two
architectural bits.

Will

2014-10-28 21:19:06

by Joe Perches

[permalink] [raw]
Subject: [PATCH] carl9170: Convert byte_rev_table uses to bitrev8

Use the inline function instead of directly indexing the array.

This allows some architectures with hardware instructions
for bit reversals to eliminate the array.

Signed-off-by: Joe Perches <[email protected]>
---
On Sun, 2014-10-26 at 23:46 -0700, Joe Perches wrote:
> On Mon, 2014-10-27 at 14:37 +0800, Wang, Yalin wrote:
> > this change add CONFIG_HAVE_ARCH_BITREVERSE config option,
> > so that we can use arm/arm64 rbit instruction to do bitrev operation
> > by hardware.
[]
> > diff --git a/include/linux/bitrev.h b/include/linux/bitrev.h
> > index 7ffe03f..ef5b2bb 100644
> > --- a/include/linux/bitrev.h
> > +++ b/include/linux/bitrev.h
> > @@ -3,6 +3,14 @@
> >
> > #include <linux/types.h>
> >
> > +#ifdef CONFIG_HAVE_ARCH_BITREVERSE
> > +#include <asm/bitrev.h>
> > +
> > +#define bitrev32 __arch_bitrev32
> > +#define bitrev16 __arch_bitrev16
> > +#define bitrev8 __arch_bitrev8
> > +
> > +#else
> > extern u8 const byte_rev_table[256];

drivers/net/wireless/ath/carl9170/phy.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/carl9170/phy.c b/drivers/net/wireless/ath/carl9170/phy.c
index b80b213..dca6df1 100644
--- a/drivers/net/wireless/ath/carl9170/phy.c
+++ b/drivers/net/wireless/ath/carl9170/phy.c
@@ -994,7 +994,7 @@ static int carl9170_init_rf_bank4_pwr(struct ar9170 *ar, bool band5ghz,
refsel0 = 0;
refsel1 = 1;
}
- chansel = byte_rev_table[chansel];
+ chansel = bitrev8(chansel);
} else {
if (freq == 2484) {
chansel = 10 + (freq - 2274) / 5;
@@ -1002,7 +1002,7 @@ static int carl9170_init_rf_bank4_pwr(struct ar9170 *ar, bool band5ghz,
} else
chansel = 16 + (freq - 2272) / 5;
chansel *= 4;
- chansel = byte_rev_table[chansel];
+ chansel = bitrev8(chansel);
}

d1 = chansel;

2014-10-28 21:23:55

by Joe Perches

[permalink] [raw]
Subject: [PATCH] 6fire: Convert byte_rev_table uses to bitrev8

Use the inline function instead of directly indexing the array.

This allows some architectures with hardware instructions
for bit reversals to eliminate the array.

Signed-off-by: Joe Perches <[email protected]>
---
On Sun, 2014-10-26 at 23:46 -0700, Joe Perches wrote:
> On Mon, 2014-10-27 at 14:37 +0800, Wang, Yalin wrote:
> > this change add CONFIG_HAVE_ARCH_BITREVERSE config option,
> > so that we can use arm/arm64 rbit instruction to do bitrev operation
> > by hardware.
[]
> > diff --git a/include/linux/bitrev.h b/include/linux/bitrev.h
> > index 7ffe03f..ef5b2bb 100644
> > --- a/include/linux/bitrev.h
> > +++ b/include/linux/bitrev.h
> > @@ -3,6 +3,14 @@
> >
> > #include <linux/types.h>
> >
> > +#ifdef CONFIG_HAVE_ARCH_BITREVERSE
> > +#include <asm/bitrev.h>
> > +
> > +#define bitrev32 __arch_bitrev32
> > +#define bitrev16 __arch_bitrev16
> > +#define bitrev8 __arch_bitrev8
> > +
> > +#else
> > extern u8 const byte_rev_table[256];

sound/usb/6fire/firmware.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/usb/6fire/firmware.c b/sound/usb/6fire/firmware.c
index 3b02e54..62c25e7 100644
--- a/sound/usb/6fire/firmware.c
+++ b/sound/usb/6fire/firmware.c
@@ -316,7 +316,7 @@ static int usb6fire_fw_fpga_upload(

while (c != end) {
for (i = 0; c != end && i < FPGA_BUFSIZE; i++, c++)
- buffer[i] = byte_rev_table[(u8) *c];
+ buffer[i] = bitrev8((u8)*c);

ret = usb6fire_fw_fpga_write(device, buffer, i);
if (ret < 0) {

2014-10-29 02:42:08

by Wang, Yalin

[permalink] [raw]
Subject: RE: [PATCH] 6fire: Convert byte_rev_table uses to bitrev8

> Use the inline function instead of directly indexing the array.
>
> This allows some architectures with hardware instructions for bit reversals
> to eliminate the array.
>
> Signed-off-by: Joe Perches <[email protected]>
> ---
> On Sun, 2014-10-26 at 23:46 -0700, Joe Perches wrote:
> > On Mon, 2014-10-27 at 14:37 +0800, Wang, Yalin wrote:
> > > this change add CONFIG_HAVE_ARCH_BITREVERSE config option, so that
> > > we can use arm/arm64 rbit instruction to do bitrev operation by
> > > hardware.
> []
> > > diff --git a/include/linux/bitrev.h b/include/linux/bitrev.h index
> > > 7ffe03f..ef5b2bb 100644
> > > --- a/include/linux/bitrev.h
> > > +++ b/include/linux/bitrev.h
> > > @@ -3,6 +3,14 @@
> > >
> > > #include <linux/types.h>
> > >
> > > +#ifdef CONFIG_HAVE_ARCH_BITREVERSE
> > > +#include <asm/bitrev.h>
> > > +
> > > +#define bitrev32 __arch_bitrev32
> > > +#define bitrev16 __arch_bitrev16
> > > +#define bitrev8 __arch_bitrev8
> > > +
> > > +#else
> > > extern u8 const byte_rev_table[256];
>
> sound/usb/6fire/firmware.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/sound/usb/6fire/firmware.c b/sound/usb/6fire/firmware.c index
> 3b02e54..62c25e7 100644
> --- a/sound/usb/6fire/firmware.c
> +++ b/sound/usb/6fire/firmware.c
> @@ -316,7 +316,7 @@ static int usb6fire_fw_fpga_upload(
>
> while (c != end) {
> for (i = 0; c != end && i < FPGA_BUFSIZE; i++, c++)
> - buffer[i] = byte_rev_table[(u8) *c];
> + buffer[i] = bitrev8((u8)*c);
>
> ret = usb6fire_fw_fpga_write(device, buffer, i);
> if (ret < 0) {
>
I think the most safe way is change byte_rev_table[] to be satic,
So that no driver can access it directly,
The build error can remind the developer if they use byte_rev_table[]
Directly .

Thanks

2014-10-29 02:52:14

by Wang, Yalin

[permalink] [raw]
Subject: RE: [RFC V3] arm/arm64:add CONFIG_HAVE_ARCH_BITREVERSE to support rbit instruction

> From: Will Deacon [mailto:[email protected]]
> Yup, sorry, I didn't realise this patch covered both architectures. It
> would probably be a good idea to split it into 3 parts: a core part, then
> the two architectural bits.
>
> Will

Ok ,
I will split the patch into three parts,
And send again .

Thanks

2014-10-29 03:06:19

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] 6fire: Convert byte_rev_table uses to bitrev8

On Wed, 2014-10-29 at 10:42 +0800, Wang, Yalin wrote:
> > Use the inline function instead of directly indexing the array.
> > This allows some architectures with hardware instructions for bit reversals
> > to eliminate the array.
[]
> > On Sun, 2014-10-26 at 23:46 -0700, Joe Perches wrote:
> > > On Mon, 2014-10-27 at 14:37 +0800, Wang, Yalin wrote:
> > > > this change add CONFIG_HAVE_ARCH_BITREVERSE config option, so that
> > > > we can use arm/arm64 rbit instruction to do bitrev operation by
> > > > hardware.
[]
> I think the most safe way is change byte_rev_table[] to be satic,
> So that no driver can access it directly,
> The build error can remind the developer if they use byte_rev_table[]
> Directly .

You can do that with your later patch, but the
existing uses _must_ be converted first so you
don't break the build.


2014-10-29 03:16:17

by Wang, Yalin

[permalink] [raw]
Subject: RE: [PATCH] 6fire: Convert byte_rev_table uses to bitrev8

> From: Joe Perches [mailto:[email protected]]
> > I think the most safe way is change byte_rev_table[] to be satic, So
> > that no driver can access it directly, The build error can remind the
> > developer if they use byte_rev_table[] Directly .
>
> You can do that with your later patch, but the existing uses _must_ be
> converted first so you don't break the build.
>
>
Yeah, I agree with you,
I will add this into my later patch.

Thanks

2014-10-29 03:28:27

by Rob Herring

[permalink] [raw]
Subject: Re: [RFC V2] arm/arm64:add CONFIG_HAVE_ARCH_BITREVERSE to support rbit instruction

On Mon, Oct 27, 2014 at 2:46 PM, Joe Perches <[email protected]> wrote:
> On Mon, 2014-10-27 at 14:37 +0800, Wang, Yalin wrote:
>> this change add CONFIG_HAVE_ARCH_BITREVERSE config option,
>> so that we can use arm/arm64 rbit instruction to do bitrev operation
>> by hardware.

I don't see the original patch in my inbox, so replying here.

>>
>> Signed-off-by: Yalin Wang <[email protected]>
>> ---
>> arch/arm/Kconfig | 1 +
>> arch/arm/include/asm/bitrev.h | 21 +++++++++++++++++++++
>> arch/arm64/Kconfig | 1 +
>> arch/arm64/include/asm/bitrev.h | 21 +++++++++++++++++++++
>> include/linux/bitrev.h | 9 +++++++++
>> lib/Kconfig | 9 +++++++++
>> lib/bitrev.c | 2 ++
>> 7 files changed, 64 insertions(+)
>> create mode 100644 arch/arm/include/asm/bitrev.h
>> create mode 100644 arch/arm64/include/asm/bitrev.h
>>
>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> index 89c4b5c..426cbcc 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -16,6 +16,7 @@ config ARM
>> select DCACHE_WORD_ACCESS if HAVE_EFFICIENT_UNALIGNED_ACCESS
>> select GENERIC_ALLOCATOR
>> select GENERIC_ATOMIC64 if (CPU_V7M || CPU_V6 || !CPU_32v6K || !AEABI)
>> + select HAVE_ARCH_BITREVERSE if (CPU_V7M || CPU_V7)
>> select GENERIC_CLOCKEVENTS_BROADCAST if SMP
>> select GENERIC_IDLE_POLL_SETUP
>> select GENERIC_IRQ_PROBE

[...]

>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index 9532f8d..263c28c 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -36,6 +36,7 @@ config ARM64
>> select HARDIRQS_SW_RESEND
>> select HAVE_ARCH_AUDITSYSCALL
>> select HAVE_ARCH_JUMP_LABEL
>> + select HAVE_ARCH_BITREVERSE
>> select HAVE_ARCH_KGDB
>> select HAVE_ARCH_TRACEHOOK
>> select HAVE_BPF_JIT

The kconfig lists should be sorted.

Rob

2014-10-29 05:14:24

by Wang, Yalin

[permalink] [raw]
Subject: [RFC V4 1/3] add CONFIG_HAVE_ARCH_BITREVERSE to support rbit instruction

this change add CONFIG_HAVE_ARCH_BITREVERSE config option,
so that we can use arm/arm64 rbit instruction to do bitrev operation
by hardware.

We also change byte_rev_table[] to be static,
to make sure no drivers can access it directly.

Change bitrev16() bitrev32() to be inline function,
don't need export symbol for these tiny functions.

Signed-off-by: Yalin Wang <[email protected]>
---
include/linux/bitrev.h | 21 ++++++++++++++++++---
lib/Kconfig | 9 +++++++++
lib/bitrev.c | 19 +++----------------
3 files changed, 30 insertions(+), 19 deletions(-)

diff --git a/include/linux/bitrev.h b/include/linux/bitrev.h
index 7ffe03f..fa2682c 100644
--- a/include/linux/bitrev.h
+++ b/include/linux/bitrev.h
@@ -3,14 +3,29 @@

#include <linux/types.h>

-extern u8 const byte_rev_table[256];
+#ifdef CONFIG_HAVE_ARCH_BITREVERSE
+#include <asm/bitrev.h>
+
+#define bitrev32 __arch_bitrev32
+#define bitrev16 __arch_bitrev16
+#define bitrev8 __arch_bitrev8
+
+#else

static inline u8 bitrev8(u8 byte)
{
return byte_rev_table[byte];
}

-extern u16 bitrev16(u16 in);
-extern u32 bitrev32(u32 in);
+static inline u16 bitrev16(u16 x)
+{
+ return (bitrev8(x & 0xff) << 8) | bitrev8(x >> 8);
+}
+
+static inline u32 bitrev32(u32 x)
+{
+ return (bitrev16(x & 0xffff) << 16) | bitrev16(x >> 16);
+}

+#endif /* CONFIG_HAVE_ARCH_BITREVERSE */
#endif /* _LINUX_BITREV_H */
diff --git a/lib/Kconfig b/lib/Kconfig
index 54cf309..cd177ca 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -13,6 +13,15 @@ config RAID6_PQ
config BITREVERSE
tristate

+config HAVE_ARCH_BITREVERSE
+ boolean
+ default n
+ depends on BITREVERSE
+ help
+ This option provides an config for the architecture which have instruction
+ can do bitreverse operation, we use the hardware instruction if the architecture
+ have this capability.
+
config RATIONAL
boolean

diff --git a/lib/bitrev.c b/lib/bitrev.c
index 3956203..ba13610 100644
--- a/lib/bitrev.c
+++ b/lib/bitrev.c
@@ -1,3 +1,4 @@
+#ifndef CONFIG_HAVE_ARCH_BITREVERSE
#include <linux/types.h>
#include <linux/module.h>
#include <linux/bitrev.h>
@@ -6,7 +7,7 @@ MODULE_AUTHOR("Akinobu Mita <[email protected]>");
MODULE_DESCRIPTION("Bit ordering reversal functions");
MODULE_LICENSE("GPL");

-const u8 byte_rev_table[256] = {
+const static u8 byte_rev_table[256] = {
0x00, 0x80, 0x40, 0xc0, 0x20, 0xa0, 0x60, 0xe0,
0x10, 0x90, 0x50, 0xd0, 0x30, 0xb0, 0x70, 0xf0,
0x08, 0x88, 0x48, 0xc8, 0x28, 0xa8, 0x68, 0xe8,
@@ -42,18 +43,4 @@ const u8 byte_rev_table[256] = {
};
EXPORT_SYMBOL_GPL(byte_rev_table);

-u16 bitrev16(u16 x)
-{
- return (bitrev8(x & 0xff) << 8) | bitrev8(x >> 8);
-}
-EXPORT_SYMBOL(bitrev16);
-
-/**
- * bitrev32 - reverse the order of bits in a u32 value
- * @x: value to be bit-reversed
- */
-u32 bitrev32(u32 x)
-{
- return (bitrev16(x & 0xffff) << 16) | bitrev16(x >> 16);
-}
-EXPORT_SYMBOL(bitrev32);
+#endif /* CONFIG_HAVE_ARCH_BITREVERSE */
--
2.1.1

????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?Ý¢j"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2014-10-29 05:15:43

by Wang, Yalin

[permalink] [raw]
Subject: [RFC V4 2/3] arm:add bitrev.h file to support rbit instruction

This patch add bitrev.h file to support rbit instruction,
so that we can do bitrev operation by hardware.
Signed-off-by: Yalin Wang <[email protected]>
---
arch/arm/Kconfig | 1 +
arch/arm/include/asm/bitrev.h | 28 ++++++++++++++++++++++++++++
2 files changed, 29 insertions(+)
create mode 100644 arch/arm/include/asm/bitrev.h

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 89c4b5c..be92b3b 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -28,6 +28,7 @@ config ARM
select HANDLE_DOMAIN_IRQ
select HARDIRQS_SW_RESEND
select HAVE_ARCH_AUDITSYSCALL if (AEABI && !OABI_COMPAT)
+ select HAVE_ARCH_BITREVERSE if (CPU_V7M || CPU_V7)
select HAVE_ARCH_JUMP_LABEL if !XIP_KERNEL
select HAVE_ARCH_KGDB
select HAVE_ARCH_SECCOMP_FILTER if (AEABI && !OABI_COMPAT)
diff --git a/arch/arm/include/asm/bitrev.h b/arch/arm/include/asm/bitrev.h
new file mode 100644
index 0000000..c21a5f4
--- /dev/null
+++ b/arch/arm/include/asm/bitrev.h
@@ -0,0 +1,28 @@
+#ifndef __ASM_ARM_BITREV_H
+#define __ASM_ARM_BITREV_H
+
+static __always_inline __attribute_const__ u32 __arch_bitrev32(u32 x)
+{
+ if (__builtin_constant_p(x)) {
+ x = (x >> 16) | (x << 16);
+ x = ((x & 0xFF00FF00) >> 8) | ((x & 0x00FF00FF) << 8);
+ x = ((x & 0xF0F0F0F0) >> 4) | ((x & 0x0F0F0F0F) << 4);
+ x = ((x & 0xCCCCCCCC) >> 2) | ((x & 0x33333333) << 2);
+ return ((x & 0xAAAAAAAA) >> 1) | ((x & 0x55555555) << 1);
+ }
+ __asm__ ("rbit %0, %1" : "=r" (x) : "r" (x));
+ return x;
+}
+
+static __always_inline __attribute_const__ u16 __arch_bitrev16(u16 x)
+{
+ return __arch_bitrev32((u32)x) >> 16;
+}
+
+static __always_inline __attribute_const__ u8 __arch_bitrev8(u8 x)
+{
+ return __arch_bitrev32((u32)x) >> 24;
+}
+
+#endif
+
--
2.1.1
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?Ý¢j"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2014-10-29 05:17:02

by Wang, Yalin

[permalink] [raw]
Subject: [RFC V4 3/3] arm64:add bitrev.h file to support rbit instruction

This patch add bitrev.h file to support rbit instruction,
so that we can do bitrev operation by hardware.
Signed-off-by: Yalin Wang <[email protected]>
---
arch/arm64/Kconfig | 1 +
arch/arm64/include/asm/bitrev.h | 28 ++++++++++++++++++++++++++++
2 files changed, 29 insertions(+)
create mode 100644 arch/arm64/include/asm/bitrev.h

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 9532f8d..b1ec1dd 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -35,6 +35,7 @@ config ARM64
select HANDLE_DOMAIN_IRQ
select HARDIRQS_SW_RESEND
select HAVE_ARCH_AUDITSYSCALL
+ select HAVE_ARCH_BITREVERSE
select HAVE_ARCH_JUMP_LABEL
select HAVE_ARCH_KGDB
select HAVE_ARCH_TRACEHOOK
diff --git a/arch/arm64/include/asm/bitrev.h b/arch/arm64/include/asm/bitrev.h
new file mode 100644
index 0000000..292a5de
--- /dev/null
+++ b/arch/arm64/include/asm/bitrev.h
@@ -0,0 +1,28 @@
+#ifndef __ASM_ARM64_BITREV_H
+#define __ASM_ARM64_BITREV_H
+
+static __always_inline __attribute_const__ u32 __arch_bitrev32(u32 x)
+{
+ if (__builtin_constant_p(x)) {
+ x = (x >> 16) | (x << 16);
+ x = ((x & 0xFF00FF00) >> 8) | ((x & 0x00FF00FF) << 8);
+ x = ((x & 0xF0F0F0F0) >> 4) | ((x & 0x0F0F0F0F) << 4);
+ x = ((x & 0xCCCCCCCC) >> 2) | ((x & 0x33333333) << 2);
+ return ((x & 0xAAAAAAAA) >> 1) | ((x & 0x55555555) << 1);
+ }
+ __asm__ ("rbit %w0, %w1" : "=r" (x) : "r" (x));
+ return x;
+}
+
+static __always_inline __attribute_const__ u16 __arch_bitrev16(u16 x)
+{
+ return __arch_bitrev32((u32)x) >> 16;
+}
+
+static __always_inline __attribute_const__ u8 __arch_bitrev8(u8 x)
+{
+ return __arch_bitrev32((u32)x) >> 24;
+}
+
+#endif
+
--
2.1.1
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?Ý¢j"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2014-10-29 05:20:21

by Wang, Yalin

[permalink] [raw]
Subject: RE: [RFC V2] arm/arm64:add CONFIG_HAVE_ARCH_BITREVERSE to support rbit instruction

> From: Rob Herring [mailto:[email protected]]
> >> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index
> >> 9532f8d..263c28c 100644
> >> --- a/arch/arm64/Kconfig
> >> +++ b/arch/arm64/Kconfig
> >> @@ -36,6 +36,7 @@ config ARM64
> >> select HARDIRQS_SW_RESEND
> >> select HAVE_ARCH_AUDITSYSCALL
> >> select HAVE_ARCH_JUMP_LABEL
> >> + select HAVE_ARCH_BITREVERSE
> >> select HAVE_ARCH_KGDB
> >> select HAVE_ARCH_TRACEHOOK
> >> select HAVE_BPF_JIT
>
> The kconfig lists should be sorted.
>
> Rob

Got it ,
Thanks for your remind.
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?Ý¢j"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2014-10-29 05:21:42

by Joe Perches

[permalink] [raw]
Subject: Re: [RFC V4 1/3] add CONFIG_HAVE_ARCH_BITREVERSE to support rbit instruction

On Wed, 2014-10-29 at 13:14 +0800, Wang, Yalin wrote:
> this change add CONFIG_HAVE_ARCH_BITREVERSE config option,
> so that we can use arm/arm64 rbit instruction to do bitrev operation
> by hardware.

> We also change byte_rev_table[] to be static,
> to make sure no drivers can access it directly.

You break the build with this patch.

You can't do this until the users of the table
are converted.

So far, they are not.

I submitted patches for these uses, but those patches
are not yet applied.

Please make sure the dependencies for your patches
are explicitly stated.

2014-10-29 05:36:35

by Wang, Yalin

[permalink] [raw]
Subject: RE: [RFC V4 1/3] add CONFIG_HAVE_ARCH_BITREVERSE to support rbit instruction

> From: Joe Perches [mailto:[email protected]]
> > We also change byte_rev_table[] to be static, to make sure no drivers
> > can access it directly.
>
> You break the build with this patch.
>
> You can't do this until the users of the table are converted.
>
> So far, they are not.
>
> I submitted patches for these uses, but those patches are not yet applied.
>
> Please make sure the dependencies for your patches are explicitly stated.
>
Oh, byte_rev_table[] must be extern,
Otherwise, bitrev8() can't access it ,
I will change it.

2014-10-29 05:50:40

by Wang, Yalin

[permalink] [raw]
Subject: [RFC V5 1/3] add CONFIG_HAVE_ARCH_BITREVERSE to support rbit instruction

this change add CONFIG_HAVE_ARCH_BITREVERSE config option,
so that we can use arm/arm64 rbit instruction to do bitrev operation
by hardware.

Change bitrev16() bitrev32() to be inline function,
don't need export symbol for these tiny functions.

Signed-off-by: Yalin Wang <[email protected]>
---
include/linux/bitrev.h | 21 ++++++++++++++++++---
lib/Kconfig | 9 +++++++++
lib/bitrev.c | 17 ++---------------
3 files changed, 29 insertions(+), 18 deletions(-)

diff --git a/include/linux/bitrev.h b/include/linux/bitrev.h
index 7ffe03f..413c52c 100644
--- a/include/linux/bitrev.h
+++ b/include/linux/bitrev.h
@@ -3,14 +3,29 @@

#include <linux/types.h>

-extern u8 const byte_rev_table[256];
+#ifdef CONFIG_HAVE_ARCH_BITREVERSE
+#include <asm/bitrev.h>
+
+#define bitrev32 __arch_bitrev32
+#define bitrev16 __arch_bitrev16
+#define bitrev8 __arch_bitrev8

+#else
+extern u8 const byte_rev_table[256];
static inline u8 bitrev8(u8 byte)
{
return byte_rev_table[byte];
}

-extern u16 bitrev16(u16 in);
-extern u32 bitrev32(u32 in);
+static inline u16 bitrev16(u16 x)
+{
+ return (bitrev8(x & 0xff) << 8) | bitrev8(x >> 8);
+}
+
+static inline u32 bitrev32(u32 x)
+{
+ return (bitrev16(x & 0xffff) << 16) | bitrev16(x >> 16);
+}

+#endif /* CONFIG_HAVE_ARCH_BITREVERSE */
#endif /* _LINUX_BITREV_H */
diff --git a/lib/Kconfig b/lib/Kconfig
index 54cf309..cd177ca 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -13,6 +13,15 @@ config RAID6_PQ
config BITREVERSE
tristate

+config HAVE_ARCH_BITREVERSE
+ boolean
+ default n
+ depends on BITREVERSE
+ help
+ This option provides an config for the architecture which have instruction
+ can do bitreverse operation, we use the hardware instruction if the architecture
+ have this capability.
+
config RATIONAL
boolean

diff --git a/lib/bitrev.c b/lib/bitrev.c
index 3956203..40ffda9 100644
--- a/lib/bitrev.c
+++ b/lib/bitrev.c
@@ -1,3 +1,4 @@
+#ifndef CONFIG_HAVE_ARCH_BITREVERSE
#include <linux/types.h>
#include <linux/module.h>
#include <linux/bitrev.h>
@@ -42,18 +43,4 @@ const u8 byte_rev_table[256] = {
};
EXPORT_SYMBOL_GPL(byte_rev_table);

-u16 bitrev16(u16 x)
-{
- return (bitrev8(x & 0xff) << 8) | bitrev8(x >> 8);
-}
-EXPORT_SYMBOL(bitrev16);
-
-/**
- * bitrev32 - reverse the order of bits in a u32 value
- * @x: value to be bit-reversed
- */
-u32 bitrev32(u32 x)
-{
- return (bitrev16(x & 0xffff) << 16) | bitrev16(x >> 16);
-}
-EXPORT_SYMBOL(bitrev32);
+#endif /* CONFIG_HAVE_ARCH_BITREVERSE */
--
2.1.1
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?Ý¢j"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2014-10-29 05:51:26

by Wang, Yalin

[permalink] [raw]
Subject: [RFC V5 2/3] arm:add bitrev.h file to support rbit instruction

This patch add bitrev.h file to support rbit instruction,
so that we can do bitrev operation by hardware.
Signed-off-by: Yalin Wang <[email protected]>
---
arch/arm/Kconfig | 1 +
arch/arm/include/asm/bitrev.h | 28 ++++++++++++++++++++++++++++
2 files changed, 29 insertions(+)
create mode 100644 arch/arm/include/asm/bitrev.h

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 89c4b5c..be92b3b 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -28,6 +28,7 @@ config ARM
select HANDLE_DOMAIN_IRQ
select HARDIRQS_SW_RESEND
select HAVE_ARCH_AUDITSYSCALL if (AEABI && !OABI_COMPAT)
+ select HAVE_ARCH_BITREVERSE if (CPU_V7M || CPU_V7)
select HAVE_ARCH_JUMP_LABEL if !XIP_KERNEL
select HAVE_ARCH_KGDB
select HAVE_ARCH_SECCOMP_FILTER if (AEABI && !OABI_COMPAT)
diff --git a/arch/arm/include/asm/bitrev.h b/arch/arm/include/asm/bitrev.h
new file mode 100644
index 0000000..c21a5f4
--- /dev/null
+++ b/arch/arm/include/asm/bitrev.h
@@ -0,0 +1,28 @@
+#ifndef __ASM_ARM_BITREV_H
+#define __ASM_ARM_BITREV_H
+
+static __always_inline __attribute_const__ u32 __arch_bitrev32(u32 x)
+{
+ if (__builtin_constant_p(x)) {
+ x = (x >> 16) | (x << 16);
+ x = ((x & 0xFF00FF00) >> 8) | ((x & 0x00FF00FF) << 8);
+ x = ((x & 0xF0F0F0F0) >> 4) | ((x & 0x0F0F0F0F) << 4);
+ x = ((x & 0xCCCCCCCC) >> 2) | ((x & 0x33333333) << 2);
+ return ((x & 0xAAAAAAAA) >> 1) | ((x & 0x55555555) << 1);
+ }
+ __asm__ ("rbit %0, %1" : "=r" (x) : "r" (x));
+ return x;
+}
+
+static __always_inline __attribute_const__ u16 __arch_bitrev16(u16 x)
+{
+ return __arch_bitrev32((u32)x) >> 16;
+}
+
+static __always_inline __attribute_const__ u8 __arch_bitrev8(u8 x)
+{
+ return __arch_bitrev32((u32)x) >> 24;
+}
+
+#endif
+
--
2.1.1
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?Ý¢j"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2014-10-29 05:52:09

by Wang, Yalin

[permalink] [raw]
Subject: [RFC V5 3/3] arm64:add bitrev.h file to support rbit instruction

This patch add bitrev.h file to support rbit instruction,
so that we can do bitrev operation by hardware.
Signed-off-by: Yalin Wang <[email protected]>
---
arch/arm64/Kconfig | 1 +
arch/arm64/include/asm/bitrev.h | 28 ++++++++++++++++++++++++++++
2 files changed, 29 insertions(+)
create mode 100644 arch/arm64/include/asm/bitrev.h

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 9532f8d..b1ec1dd 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -35,6 +35,7 @@ config ARM64
select HANDLE_DOMAIN_IRQ
select HARDIRQS_SW_RESEND
select HAVE_ARCH_AUDITSYSCALL
+ select HAVE_ARCH_BITREVERSE
select HAVE_ARCH_JUMP_LABEL
select HAVE_ARCH_KGDB
select HAVE_ARCH_TRACEHOOK
diff --git a/arch/arm64/include/asm/bitrev.h b/arch/arm64/include/asm/bitrev.h
new file mode 100644
index 0000000..292a5de
--- /dev/null
+++ b/arch/arm64/include/asm/bitrev.h
@@ -0,0 +1,28 @@
+#ifndef __ASM_ARM64_BITREV_H
+#define __ASM_ARM64_BITREV_H
+
+static __always_inline __attribute_const__ u32 __arch_bitrev32(u32 x)
+{
+ if (__builtin_constant_p(x)) {
+ x = (x >> 16) | (x << 16);
+ x = ((x & 0xFF00FF00) >> 8) | ((x & 0x00FF00FF) << 8);
+ x = ((x & 0xF0F0F0F0) >> 4) | ((x & 0x0F0F0F0F) << 4);
+ x = ((x & 0xCCCCCCCC) >> 2) | ((x & 0x33333333) << 2);
+ return ((x & 0xAAAAAAAA) >> 1) | ((x & 0x55555555) << 1);
+ }
+ __asm__ ("rbit %w0, %w1" : "=r" (x) : "r" (x));
+ return x;
+}
+
+static __always_inline __attribute_const__ u16 __arch_bitrev16(u16 x)
+{
+ return __arch_bitrev32((u32)x) >> 16;
+}
+
+static __always_inline __attribute_const__ u8 __arch_bitrev8(u8 x)
+{
+ return __arch_bitrev32((u32)x) >> 24;
+}
+
+#endif
+
--
2.1.1
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?Ý¢j"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2014-10-30 12:01:34

by Will Deacon

[permalink] [raw]
Subject: Re: [RFC V5 3/3] arm64:add bitrev.h file to support rbit instruction

On Wed, Oct 29, 2014 at 05:52:00AM +0000, Wang, Yalin wrote:
> This patch add bitrev.h file to support rbit instruction,
> so that we can do bitrev operation by hardware.
> Signed-off-by: Yalin Wang <[email protected]>
> ---
> arch/arm64/Kconfig | 1 +
> arch/arm64/include/asm/bitrev.h | 28 ++++++++++++++++++++++++++++
> 2 files changed, 29 insertions(+)
> create mode 100644 arch/arm64/include/asm/bitrev.h
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 9532f8d..b1ec1dd 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -35,6 +35,7 @@ config ARM64
> select HANDLE_DOMAIN_IRQ
> select HARDIRQS_SW_RESEND
> select HAVE_ARCH_AUDITSYSCALL
> + select HAVE_ARCH_BITREVERSE
> select HAVE_ARCH_JUMP_LABEL
> select HAVE_ARCH_KGDB
> select HAVE_ARCH_TRACEHOOK
> diff --git a/arch/arm64/include/asm/bitrev.h b/arch/arm64/include/asm/bitrev.h
> new file mode 100644
> index 0000000..292a5de
> --- /dev/null
> +++ b/arch/arm64/include/asm/bitrev.h
> @@ -0,0 +1,28 @@
> +#ifndef __ASM_ARM64_BITREV_H
> +#define __ASM_ARM64_BITREV_H
> +
> +static __always_inline __attribute_const__ u32 __arch_bitrev32(u32 x)
> +{
> + if (__builtin_constant_p(x)) {
> + x = (x >> 16) | (x << 16);
> + x = ((x & 0xFF00FF00) >> 8) | ((x & 0x00FF00FF) << 8);
> + x = ((x & 0xF0F0F0F0) >> 4) | ((x & 0x0F0F0F0F) << 4);
> + x = ((x & 0xCCCCCCCC) >> 2) | ((x & 0x33333333) << 2);
> + return ((x & 0xAAAAAAAA) >> 1) | ((x & 0x55555555) << 1);

Shouldn't this part be in the generic code?

> + }
> + __asm__ ("rbit %w0, %w1" : "=r" (x) : "r" (x));

You can write this more neatly as:

asm ("rbit %w0, %w0" : "+r" (x));

Will

2014-10-30 12:26:46

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [RFC V5 3/3] arm64:add bitrev.h file to support rbit instruction

On 30 October 2014 13:01, Will Deacon <[email protected]> wrote:
> On Wed, Oct 29, 2014 at 05:52:00AM +0000, Wang, Yalin wrote:
>> This patch add bitrev.h file to support rbit instruction,
>> so that we can do bitrev operation by hardware.
>> Signed-off-by: Yalin Wang <[email protected]>
>> ---
>> arch/arm64/Kconfig | 1 +
>> arch/arm64/include/asm/bitrev.h | 28 ++++++++++++++++++++++++++++
>> 2 files changed, 29 insertions(+)
>> create mode 100644 arch/arm64/include/asm/bitrev.h
>>
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index 9532f8d..b1ec1dd 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -35,6 +35,7 @@ config ARM64
>> select HANDLE_DOMAIN_IRQ
>> select HARDIRQS_SW_RESEND
>> select HAVE_ARCH_AUDITSYSCALL
>> + select HAVE_ARCH_BITREVERSE
>> select HAVE_ARCH_JUMP_LABEL
>> select HAVE_ARCH_KGDB
>> select HAVE_ARCH_TRACEHOOK
>> diff --git a/arch/arm64/include/asm/bitrev.h b/arch/arm64/include/asm/bitrev.h
>> new file mode 100644
>> index 0000000..292a5de
>> --- /dev/null
>> +++ b/arch/arm64/include/asm/bitrev.h
>> @@ -0,0 +1,28 @@
>> +#ifndef __ASM_ARM64_BITREV_H
>> +#define __ASM_ARM64_BITREV_H
>> +
>> +static __always_inline __attribute_const__ u32 __arch_bitrev32(u32 x)
>> +{
>> + if (__builtin_constant_p(x)) {
>> + x = (x >> 16) | (x << 16);
>> + x = ((x & 0xFF00FF00) >> 8) | ((x & 0x00FF00FF) << 8);
>> + x = ((x & 0xF0F0F0F0) >> 4) | ((x & 0x0F0F0F0F) << 4);
>> + x = ((x & 0xCCCCCCCC) >> 2) | ((x & 0x33333333) << 2);
>> + return ((x & 0xAAAAAAAA) >> 1) | ((x & 0x55555555) << 1);
>
> Shouldn't this part be in the generic code?
>
>> + }
>> + __asm__ ("rbit %w0, %w1" : "=r" (x) : "r" (x));
>
> You can write this more neatly as:
>
> asm ("rbit %w0, %w0" : "+r" (x));
>

This forces GCC to use the same register as input and output, which
doesn't necessarily result in the fastest code. (e.g., if the
un-bitrev()'ed value is reused again afterwards).
On the other hand, the original notation does allow GCC to use the
same register, but doesn't force it to, so I prefer the original one.

--
Ard.

2014-10-30 13:58:03

by Will Deacon

[permalink] [raw]
Subject: Re: [RFC V5 3/3] arm64:add bitrev.h file to support rbit instruction

On Thu, Oct 30, 2014 at 12:26:42PM +0000, Ard Biesheuvel wrote:
> On 30 October 2014 13:01, Will Deacon <[email protected]> wrote:
> > On Wed, Oct 29, 2014 at 05:52:00AM +0000, Wang, Yalin wrote:
> >> This patch add bitrev.h file to support rbit instruction,
> >> so that we can do bitrev operation by hardware.
> >> Signed-off-by: Yalin Wang <[email protected]>
> >> ---
> >> arch/arm64/Kconfig | 1 +
> >> arch/arm64/include/asm/bitrev.h | 28 ++++++++++++++++++++++++++++
> >> 2 files changed, 29 insertions(+)
> >> create mode 100644 arch/arm64/include/asm/bitrev.h
> >>
> >> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> >> index 9532f8d..b1ec1dd 100644
> >> --- a/arch/arm64/Kconfig
> >> +++ b/arch/arm64/Kconfig
> >> @@ -35,6 +35,7 @@ config ARM64
> >> select HANDLE_DOMAIN_IRQ
> >> select HARDIRQS_SW_RESEND
> >> select HAVE_ARCH_AUDITSYSCALL
> >> + select HAVE_ARCH_BITREVERSE
> >> select HAVE_ARCH_JUMP_LABEL
> >> select HAVE_ARCH_KGDB
> >> select HAVE_ARCH_TRACEHOOK
> >> diff --git a/arch/arm64/include/asm/bitrev.h b/arch/arm64/include/asm/bitrev.h
> >> new file mode 100644
> >> index 0000000..292a5de
> >> --- /dev/null
> >> +++ b/arch/arm64/include/asm/bitrev.h
> >> @@ -0,0 +1,28 @@
> >> +#ifndef __ASM_ARM64_BITREV_H
> >> +#define __ASM_ARM64_BITREV_H
> >> +
> >> +static __always_inline __attribute_const__ u32 __arch_bitrev32(u32 x)
> >> +{
> >> + if (__builtin_constant_p(x)) {
> >> + x = (x >> 16) | (x << 16);
> >> + x = ((x & 0xFF00FF00) >> 8) | ((x & 0x00FF00FF) << 8);
> >> + x = ((x & 0xF0F0F0F0) >> 4) | ((x & 0x0F0F0F0F) << 4);
> >> + x = ((x & 0xCCCCCCCC) >> 2) | ((x & 0x33333333) << 2);
> >> + return ((x & 0xAAAAAAAA) >> 1) | ((x & 0x55555555) << 1);
> >
> > Shouldn't this part be in the generic code?
> >
> >> + }
> >> + __asm__ ("rbit %w0, %w1" : "=r" (x) : "r" (x));
> >
> > You can write this more neatly as:
> >
> > asm ("rbit %w0, %w0" : "+r" (x));
> >
>
> This forces GCC to use the same register as input and output, which
> doesn't necessarily result in the fastest code. (e.g., if the
> un-bitrev()'ed value is reused again afterwards).
> On the other hand, the original notation does allow GCC to use the
> same register, but doesn't force it to, so I prefer the original one.

That's a good point, especially since this is __always_inline.

Will

2014-10-31 02:03:33

by Wang, Yalin

[permalink] [raw]
Subject: RE: [RFC V5 3/3] arm64:add bitrev.h file to support rbit instruction

> From: Will Deacon [mailto:[email protected]]
> Sent: Thursday, October 30, 2014 8:01 PM
> To: Wang, Yalin
> Cc: 'Rob Herring'; 'Joe Perches'; 'Russell King - ARM Linux'; 'linux-
> [email protected]'; '[email protected]'; '[email protected]';
> '[email protected]'
> Subject: Re: [RFC V5 3/3] arm64:add bitrev.h file to support rbit
> instruction
>
> > +static __always_inline __attribute_const__ u32 __arch_bitrev32(u32 x)
> > +{
> > + if (__builtin_constant_p(x)) {
> > + x = (x >> 16) | (x << 16);
> > + x = ((x & 0xFF00FF00) >> 8) | ((x & 0x00FF00FF) << 8);
> > + x = ((x & 0xF0F0F0F0) >> 4) | ((x & 0x0F0F0F0F) << 4);
> > + x = ((x & 0xCCCCCCCC) >> 2) | ((x & 0x33333333) << 2);
> > + return ((x & 0xAAAAAAAA) >> 1) | ((x & 0x55555555) << 1);
>
> Shouldn't this part be in the generic code?

Good idea, I will change this part into linux/bitrev.h .
Thanks

2014-10-31 05:40:25

by Wang, Yalin

[permalink] [raw]
Subject: [RFC V6 1/3] add CONFIG_HAVE_ARCH_BITREVERSE to support rbit instruction

this change add CONFIG_HAVE_ARCH_BITREVERSE config option,
so that we can use some architecture's bitrev hardware instruction
to do bitrev operation.

Introduce __constant_bitrev* macro for constant bitrev operation.

Change __bitrev16() __bitrev32() to be inline function,
don't need export symbol for these tiny functions.

Signed-off-by: Yalin Wang <[email protected]>
---
include/linux/bitrev.h | 77 +++++++++++++++++++++++++++++++++++++++++++++++---
lib/Kconfig | 9 ++++++
lib/bitrev.c | 17 ++---------
3 files changed, 84 insertions(+), 19 deletions(-)

diff --git a/include/linux/bitrev.h b/include/linux/bitrev.h
index 7ffe03f..fb790b8 100644
--- a/include/linux/bitrev.h
+++ b/include/linux/bitrev.h
@@ -3,14 +3,83 @@

#include <linux/types.h>

-extern u8 const byte_rev_table[256];
+#ifdef CONFIG_HAVE_ARCH_BITREVERSE
+#include <asm/bitrev.h>
+
+#define __bitrev32 __arch_bitrev32
+#define __bitrev16 __arch_bitrev16
+#define __bitrev8 __arch_bitrev8

-static inline u8 bitrev8(u8 byte)
+#else
+extern u8 const byte_rev_table[256];
+static inline u8 __bitrev8(u8 byte)
{
return byte_rev_table[byte];
}

-extern u16 bitrev16(u16 in);
-extern u32 bitrev32(u32 in);
+static inline u16 __bitrev16(u16 x)
+{
+ return (__bitrev8(x & 0xff) << 8) | __bitrev8(x >> 8);
+}
+
+static inline u32 __bitrev32(u32 x)
+{
+ return (__bitrev16(x & 0xffff) << 16) | __bitrev16(x >> 16);
+}
+
+#endif /* CONFIG_HAVE_ARCH_BITREVERSE */
+
+#define __constant_bitrev32(x) \
+({ \
+ u32 __x = x; \
+ __x = (__x >> 16) | (__x << 16); \
+ __x = ((__x & (u32)0xFF00FF00UL) >> 8) | ((__x & (u32)0x00FF00FFUL) << 8); \
+ __x = ((__x & (u32)0xF0F0F0F0UL) >> 4) | ((__x & (u32)0x0F0F0F0FUL) << 4); \
+ __x = ((__x & (u32)0xCCCCCCCCUL) >> 2) | ((__x & (u32)0x33333333UL) << 2); \
+ __x = ((__x & (u32)0xAAAAAAAAUL) >> 1) | ((__x & (u32)0x55555555UL) << 1); \
+ __x; \
+})
+
+#define __constant_bitrev16(x) \
+({ \
+ u16 __x = x; \
+ __x = (__x >> 8) | (__x << 8); \
+ __x = ((__x & (u16)0xF0F0U) >> 4) | ((__x & (u16)0x0F0FU) << 4); \
+ __x = ((__x & (u16)0xCCCCU) >> 2) | ((__x & (u16)0x3333U) << 2); \
+ __x = ((__x & (u16)0xAAAAU) >> 1) | ((__x & (u16)0x5555U) << 1); \
+ __x; \
+})
+
+#define __constant_bitrev8(x) \
+({ \
+ u8 __x = x; \
+ __x = (__x >> 4) | (__x << 4); \
+ __x = ((__x & (u8)0xCCU) >> 2) | ((__x & (u8)0x33U) << 2); \
+ __x = ((__x & (u8)0xAAU) >> 1) | ((__x & (u8)0x55U) << 1); \
+ __x; \
+})
+
+#define bitrev32(x) \
+({ \
+ u32 __x = x; \
+ __builtin_constant_p(__x) ? \
+ __constant_bitrev32(__x) : \
+ __bitrev32(__x); \
+})
+
+#define bitrev16(x) \
+({ \
+ u16 __x = x; \
+ __builtin_constant_p(__x) ? \
+ __constant_bitrev16(__x) : \
+ __bitrev16(__x); \
+ })

+#define bitrev8(x) \
+({ \
+ u8 __x = x; \
+ __builtin_constant_p(__x) ? \
+ __constant_bitrev8(__x) : \
+ __bitrev8(__x) ; \
+ })
#endif /* _LINUX_BITREV_H */
diff --git a/lib/Kconfig b/lib/Kconfig
index 54cf309..cd177ca 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -13,6 +13,15 @@ config RAID6_PQ
config BITREVERSE
tristate

+config HAVE_ARCH_BITREVERSE
+ boolean
+ default n
+ depends on BITREVERSE
+ help
+ This option provides an config for the architecture which have instruction
+ can do bitreverse operation, we use the hardware instruction if the architecture
+ have this capability.
+
config RATIONAL
boolean

diff --git a/lib/bitrev.c b/lib/bitrev.c
index 3956203..40ffda9 100644
--- a/lib/bitrev.c
+++ b/lib/bitrev.c
@@ -1,3 +1,4 @@
+#ifndef CONFIG_HAVE_ARCH_BITREVERSE
#include <linux/types.h>
#include <linux/module.h>
#include <linux/bitrev.h>
@@ -42,18 +43,4 @@ const u8 byte_rev_table[256] = {
};
EXPORT_SYMBOL_GPL(byte_rev_table);

-u16 bitrev16(u16 x)
-{
- return (bitrev8(x & 0xff) << 8) | bitrev8(x >> 8);
-}
-EXPORT_SYMBOL(bitrev16);
-
-/**
- * bitrev32 - reverse the order of bits in a u32 value
- * @x: value to be bit-reversed
- */
-u32 bitrev32(u32 x)
-{
- return (bitrev16(x & 0xffff) << 16) | bitrev16(x >> 16);
-}
-EXPORT_SYMBOL(bitrev32);
+#endif /* CONFIG_HAVE_ARCH_BITREVERSE */
--
2.1.1

2014-10-31 05:41:00

by Wang, Yalin

[permalink] [raw]
Subject: [RFC V6 2/3] add CONFIG_HAVE_ARCH_BITREVERSE to support rbit instruction

This patch add bitrev.h file to support rbit instruction,
so that we can do bitrev operation by hardware.
Signed-off-by: Yalin Wang <[email protected]>
---
arch/arm/Kconfig | 1 +
arch/arm/include/asm/bitrev.h | 21 +++++++++++++++++++++
2 files changed, 22 insertions(+)
create mode 100644 arch/arm/include/asm/bitrev.h

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 89c4b5c..be92b3b 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -28,6 +28,7 @@ config ARM
select HANDLE_DOMAIN_IRQ
select HARDIRQS_SW_RESEND
select HAVE_ARCH_AUDITSYSCALL if (AEABI && !OABI_COMPAT)
+ select HAVE_ARCH_BITREVERSE if (CPU_V7M || CPU_V7)
select HAVE_ARCH_JUMP_LABEL if !XIP_KERNEL
select HAVE_ARCH_KGDB
select HAVE_ARCH_SECCOMP_FILTER if (AEABI && !OABI_COMPAT)
diff --git a/arch/arm/include/asm/bitrev.h b/arch/arm/include/asm/bitrev.h
new file mode 100644
index 0000000..e9b2571
--- /dev/null
+++ b/arch/arm/include/asm/bitrev.h
@@ -0,0 +1,21 @@
+#ifndef __ASM_ARM_BITREV_H
+#define __ASM_ARM_BITREV_H
+
+static __always_inline __attribute_const__ u32 __arch_bitrev32(u32 x)
+{
+ __asm__ ("rbit %0, %1" : "=r" (x) : "r" (x));
+ return x;
+}
+
+static __always_inline __attribute_const__ u16 __arch_bitrev16(u16 x)
+{
+ return __arch_bitrev32((u32)x) >> 16;
+}
+
+static __always_inline __attribute_const__ u8 __arch_bitrev8(u8 x)
+{
+ return __arch_bitrev32((u32)x) >> 24;
+}
+
+#endif
+
--
2.1.1

2014-10-31 05:41:53

by Wang, Yalin

[permalink] [raw]
Subject: [RFC V6 3/3] arm64:add bitrev.h file to support rbit instruction

This patch add bitrev.h file to support rbit instruction,
so that we can do bitrev operation by hardware.
Signed-off-by: Yalin Wang <[email protected]>
---
arch/arm64/Kconfig | 1 +
arch/arm64/include/asm/bitrev.h | 21 +++++++++++++++++++++
2 files changed, 22 insertions(+)
create mode 100644 arch/arm64/include/asm/bitrev.h

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 9532f8d..b1ec1dd 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -35,6 +35,7 @@ config ARM64
select HANDLE_DOMAIN_IRQ
select HARDIRQS_SW_RESEND
select HAVE_ARCH_AUDITSYSCALL
+ select HAVE_ARCH_BITREVERSE
select HAVE_ARCH_JUMP_LABEL
select HAVE_ARCH_KGDB
select HAVE_ARCH_TRACEHOOK
diff --git a/arch/arm64/include/asm/bitrev.h b/arch/arm64/include/asm/bitrev.h
new file mode 100644
index 0000000..706a209
--- /dev/null
+++ b/arch/arm64/include/asm/bitrev.h
@@ -0,0 +1,21 @@
+#ifndef __ASM_ARM64_BITREV_H
+#define __ASM_ARM64_BITREV_H
+
+static __always_inline __attribute_const__ u32 __arch_bitrev32(u32 x)
+{
+ __asm__ ("rbit %w0, %w1" : "=r" (x) : "r" (x));
+ return x;
+}
+
+static __always_inline __attribute_const__ u16 __arch_bitrev16(u16 x)
+{
+ return __arch_bitrev32((u32)x) >> 16;
+}
+
+static __always_inline __attribute_const__ u8 __arch_bitrev8(u8 x)
+{
+ return __arch_bitrev32((u32)x) >> 24;
+}
+
+#endif
+
--
2.1.1

2014-10-31 05:42:53

by Wang, Yalin

[permalink] [raw]
Subject: [RFC V6 2/3] arm:add bitrev.h file to support rbit instruction

This patch add bitrev.h file to support rbit instruction,
so that we can do bitrev operation by hardware.
Signed-off-by: Yalin Wang <[email protected]>
---
arch/arm/Kconfig | 1 +
arch/arm/include/asm/bitrev.h | 21 +++++++++++++++++++++
2 files changed, 22 insertions(+)
create mode 100644 arch/arm/include/asm/bitrev.h

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 89c4b5c..be92b3b 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -28,6 +28,7 @@ config ARM
select HANDLE_DOMAIN_IRQ
select HARDIRQS_SW_RESEND
select HAVE_ARCH_AUDITSYSCALL if (AEABI && !OABI_COMPAT)
+ select HAVE_ARCH_BITREVERSE if (CPU_V7M || CPU_V7)
select HAVE_ARCH_JUMP_LABEL if !XIP_KERNEL
select HAVE_ARCH_KGDB
select HAVE_ARCH_SECCOMP_FILTER if (AEABI && !OABI_COMPAT)
diff --git a/arch/arm/include/asm/bitrev.h b/arch/arm/include/asm/bitrev.h
new file mode 100644
index 0000000..e9b2571
--- /dev/null
+++ b/arch/arm/include/asm/bitrev.h
@@ -0,0 +1,21 @@
+#ifndef __ASM_ARM_BITREV_H
+#define __ASM_ARM_BITREV_H
+
+static __always_inline __attribute_const__ u32 __arch_bitrev32(u32 x)
+{
+ __asm__ ("rbit %0, %1" : "=r" (x) : "r" (x));
+ return x;
+}
+
+static __always_inline __attribute_const__ u16 __arch_bitrev16(u16 x)
+{
+ return __arch_bitrev32((u32)x) >> 16;
+}
+
+static __always_inline __attribute_const__ u8 __arch_bitrev8(u8 x)
+{
+ return __arch_bitrev32((u32)x) >> 24;
+}
+
+#endif
+
--
2.1.1

2014-10-31 07:40:51

by Wang, Yalin

[permalink] [raw]
Subject: [RFC] arm:remove clear_thread_flag(TIF_UPROBE)

This patch remove clear_thread_flag(TIF_UPROBE) in do_work_pending(),
because uprobe_notify_resume() have do this.

Signed-off-by: Yalin Wang <[email protected]>
---
arch/arm/kernel/signal.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/arm/kernel/signal.c b/arch/arm/kernel/signal.c
index bd19834..ff598f0 100644
--- a/arch/arm/kernel/signal.c
+++ b/arch/arm/kernel/signal.c
@@ -591,10 +591,9 @@ do_work_pending(struct pt_regs *regs, unsigned int thread_flags, int syscall)
return restart;
}
syscall = 0;
- } else if (thread_flags & _TIF_UPROBE) {
- clear_thread_flag(TIF_UPROBE);
+ } else if (thread_flags & _TIF_UPROBE)
uprobe_notify_resume(regs);
- } else {
+ else {
clear_thread_flag(TIF_NOTIFY_RESUME);
tracehook_notify_resume(regs);
}
--
2.1.1

2014-10-31 07:45:40

by Joe Perches

[permalink] [raw]
Subject: Re: [RFC] arm:remove clear_thread_flag(TIF_UPROBE)

On Fri, 2014-10-31 at 15:40 +0800, Wang, Yalin wrote:
> This patch remove clear_thread_flag(TIF_UPROBE) in do_work_pending(),
> because uprobe_notify_resume() have do this.
[]
> diff --git a/arch/arm/kernel/signal.c b/arch/arm/kernel/signal.c
[]
> @@ -591,10 +591,9 @@ do_work_pending(struct pt_regs *regs, unsigned int thread_flags, int syscall)
> return restart;
> }
> syscall = 0;
> - } else if (thread_flags & _TIF_UPROBE) {
> - clear_thread_flag(TIF_UPROBE);
> + } else if (thread_flags & _TIF_UPROBE)
> uprobe_notify_resume(regs);
> - } else {
> + else {
> clear_thread_flag(TIF_NOTIFY_RESUME);
> tracehook_notify_resume(regs);
> }

Please keep the braces.

2014-10-31 07:51:48

by Wang, Yalin

[permalink] [raw]
Subject: RE: [RFC] arm:remove clear_thread_flag(TIF_UPROBE)

> From: Joe Perches [mailto:[email protected]]
> > diff --git a/arch/arm/kernel/signal.c b/arch/arm/kernel/signal.c
> []
> > @@ -591,10 +591,9 @@ do_work_pending(struct pt_regs *regs, unsigned int
> thread_flags, int syscall)
> > return restart;
> > }
> > syscall = 0;
> > - } else if (thread_flags & _TIF_UPROBE) {
> > - clear_thread_flag(TIF_UPROBE);
> > + } else if (thread_flags & _TIF_UPROBE)
> > uprobe_notify_resume(regs);
> > - } else {
> > + else {
> > clear_thread_flag(TIF_NOTIFY_RESUME);
> > tracehook_notify_resume(regs);
> > }
>
> Please keep the braces.

mm.. could I know the reason ? :)

Thanks

2014-10-31 07:54:24

by Wang, Yalin

[permalink] [raw]
Subject: RE: [RFC V6 2/3] add CONFIG_HAVE_ARCH_BITREVERSE to support rbit instruction

> From: Wang, Yalin
> Subject: [RFC V6 2/3] add CONFIG_HAVE_ARCH_BITREVERSE to support rbit
> instruction
>
> This patch add bitrev.h file to support rbit instruction, so that we can do
> bitrev operation by hardware.
> Signed-off-by: Yalin Wang <[email protected]>
> ---
> arch/arm/Kconfig | 1 +
> arch/arm/include/asm/bitrev.h | 21 +++++++++++++++++++++
> 2 files changed, 22 insertions(+)
> create mode 100644 arch/arm/include/asm/bitrev.h
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 89c4b5c..be92b3b
> 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -28,6 +28,7 @@ config ARM
> select HANDLE_DOMAIN_IRQ
> select HARDIRQS_SW_RESEND
> select HAVE_ARCH_AUDITSYSCALL if (AEABI && !OABI_COMPAT)
> + select HAVE_ARCH_BITREVERSE if (CPU_V7M || CPU_V7)
> select HAVE_ARCH_JUMP_LABEL if !XIP_KERNEL
> select HAVE_ARCH_KGDB
> select HAVE_ARCH_SECCOMP_FILTER if (AEABI && !OABI_COMPAT) diff --git
> a/arch/arm/include/asm/bitrev.h b/arch/arm/include/asm/bitrev.h new file
> mode 100644 index 0000000..e9b2571
> --- /dev/null
> +++ b/arch/arm/include/asm/bitrev.h
> @@ -0,0 +1,21 @@
> +#ifndef __ASM_ARM_BITREV_H
> +#define __ASM_ARM_BITREV_H
> +
> +static __always_inline __attribute_const__ u32 __arch_bitrev32(u32 x) {
> + __asm__ ("rbit %0, %1" : "=r" (x) : "r" (x));
> + return x;
> +}
> +
> +static __always_inline __attribute_const__ u16 __arch_bitrev16(u16 x) {
> + return __arch_bitrev32((u32)x) >> 16;
> +}
> +
> +static __always_inline __attribute_const__ u8 __arch_bitrev8(u8 x) {
> + return __arch_bitrev32((u32)x) >> 24;
> +}
> +
> +#endif
> +
> --
> 2.1.1

Wrong title, please ignore this one ,
I have resend another [RFC V6 2/3] .

Thanks

2014-10-31 07:58:15

by Joe Perches

[permalink] [raw]
Subject: Re: [RFC] arm:remove clear_thread_flag(TIF_UPROBE)

On Fri, 2014-10-31 at 15:51 +0800, Wang, Yalin wrote:
> > From: Joe Perches [mailto:[email protected]]
> > > diff --git a/arch/arm/kernel/signal.c b/arch/arm/kernel/signal.c
> > []
> > > @@ -591,10 +591,9 @@ do_work_pending(struct pt_regs *regs, unsigned int
> > thread_flags, int syscall)
> > > return restart;
> > > }
> > > syscall = 0;
> > > - } else if (thread_flags & _TIF_UPROBE) {
> > > - clear_thread_flag(TIF_UPROBE);
> > > + } else if (thread_flags & _TIF_UPROBE)
> > > uprobe_notify_resume(regs);
> > > - } else {
> > > + else {
> > > clear_thread_flag(TIF_NOTIFY_RESUME);
> > > tracehook_notify_resume(regs);
> > > }
> >
> > Please keep the braces.
>
> mm.. could I know the reason ? :)

Try read Documentation/CodingStyle

Chapter 3: Placing Braces and Spaces

use braces in both branches:

if (condition) {
do_this();
do_that();
} else {
otherwise();
}

2014-10-31 07:59:42

by Wang, Yalin

[permalink] [raw]
Subject: RE: [RFC] arm:remove clear_thread_flag(TIF_UPROBE)

> From: Joe Perches [mailto:[email protected]]
> > > > @@ -591,10 +591,9 @@ do_work_pending(struct pt_regs *regs, unsigned
> int
> > > thread_flags, int syscall)
> > > > return restart;
> > > > }
> > > > syscall = 0;
> > > > - } else if (thread_flags & _TIF_UPROBE) {
> > > > - clear_thread_flag(TIF_UPROBE);
> > > > + } else if (thread_flags & _TIF_UPROBE)
> > > > uprobe_notify_resume(regs);
> > > > - } else {
> > > > + else {
> > > > clear_thread_flag(TIF_NOTIFY_RESUME);
> > > > tracehook_notify_resume(regs);
> > > > }
> > >
> > > Please keep the braces.
> >
> > mm.. could I know the reason ? :)
>
> Try read Documentation/CodingStyle
>
> Chapter 3: Placing Braces and Spaces
>
> use braces in both branches:
>
> if (condition) {
> do_this();
> do_that();
> } else {
> otherwise();
> }
>

Got it, I will resend one .
Thanks

2014-10-31 08:01:20

by Wang, Yalin

[permalink] [raw]
Subject: [RFC V2] arm:remove clear_thread_flag(TIF_UPROBE)

This patch remove clear_thread_flag(TIF_UPROBE) in do_work_pending(),
because uprobe_notify_resume() have do this.

Signed-off-by: Yalin Wang <[email protected]>
---
arch/arm/kernel/signal.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/arch/arm/kernel/signal.c b/arch/arm/kernel/signal.c
index bd19834..8aa6f1b 100644
--- a/arch/arm/kernel/signal.c
+++ b/arch/arm/kernel/signal.c
@@ -592,7 +592,6 @@ do_work_pending(struct pt_regs *regs, unsigned int thread_flags, int syscall)
}
syscall = 0;
} else if (thread_flags & _TIF_UPROBE) {
- clear_thread_flag(TIF_UPROBE);
uprobe_notify_resume(regs);
} else {
clear_thread_flag(TIF_NOTIFY_RESUME);
--
2.1.1

2014-10-31 10:43:13

by Will Deacon

[permalink] [raw]
Subject: Re: [RFC V6 3/3] arm64:add bitrev.h file to support rbit instruction

On Fri, Oct 31, 2014 at 05:41:48AM +0000, Wang, Yalin wrote:
> This patch add bitrev.h file to support rbit instruction,
> so that we can do bitrev operation by hardware.
> Signed-off-by: Yalin Wang <[email protected]>
> ---
> arch/arm64/Kconfig | 1 +
> arch/arm64/include/asm/bitrev.h | 21 +++++++++++++++++++++
> 2 files changed, 22 insertions(+)
> create mode 100644 arch/arm64/include/asm/bitrev.h
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 9532f8d..b1ec1dd 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -35,6 +35,7 @@ config ARM64
> select HANDLE_DOMAIN_IRQ
> select HARDIRQS_SW_RESEND
> select HAVE_ARCH_AUDITSYSCALL
> + select HAVE_ARCH_BITREVERSE
> select HAVE_ARCH_JUMP_LABEL
> select HAVE_ARCH_KGDB
> select HAVE_ARCH_TRACEHOOK
> diff --git a/arch/arm64/include/asm/bitrev.h b/arch/arm64/include/asm/bitrev.h
> new file mode 100644
> index 0000000..706a209
> --- /dev/null
> +++ b/arch/arm64/include/asm/bitrev.h
> @@ -0,0 +1,21 @@
> +#ifndef __ASM_ARM64_BITREV_H
> +#define __ASM_ARM64_BITREV_H

Really minor nit, but we don't tend to include 'ARM64' in our header guards,
so this should just be __ASM_BITREV_H.

With that change,

Acked-by: Will Deacon <[email protected]>

Will

2014-11-03 02:17:25

by Wang, Yalin

[permalink] [raw]
Subject: RE: [RFC V6 3/3] arm64:add bitrev.h file to support rbit instruction

> From: Will Deacon [mailto:[email protected]]
> > +#ifndef __ASM_ARM64_BITREV_H
> > +#define __ASM_ARM64_BITREV_H
>
> Really minor nit, but we don't tend to include 'ARM64' in our header guards,
> so this should just be __ASM_BITREV_H.
>
> With that change,
>
> Acked-by: Will Deacon <[email protected]>
>
I have send the patch to the patch system:
http://www.arm.linux.org.uk/developer/patches/search.php?uid=4025

8187/1 8188/1 8189/1

Just remind you that , should also cherry-pick Joe Perches's
2 patches:
[PATCH] 6fire: Convert byte_rev_table uses to bitrev8
[PATCH] carl9170: Convert byte_rev_table uses to bitrev8

To make sure there is no build error when build these 2 drivers.

Thanks

2014-11-03 08:47:37

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [RFC V6 3/3] arm64:add bitrev.h file to support rbit instruction

On 3 November 2014 03:17, Wang, Yalin <[email protected]> wrote:
>> From: Will Deacon [mailto:[email protected]]
>> > +#ifndef __ASM_ARM64_BITREV_H
>> > +#define __ASM_ARM64_BITREV_H
>>
>> Really minor nit, but we don't tend to include 'ARM64' in our header guards,
>> so this should just be __ASM_BITREV_H.
>>
>> With that change,
>>
>> Acked-by: Will Deacon <[email protected]>
>>
> I have send the patch to the patch system:
> http://www.arm.linux.org.uk/developer/patches/search.php?uid=4025
>
> 8187/1 8188/1 8189/1
>
> Just remind you that , should also cherry-pick Joe Perches's
> 2 patches:
> [PATCH] 6fire: Convert byte_rev_table uses to bitrev8
> [PATCH] carl9170: Convert byte_rev_table uses to bitrev8
>
> To make sure there is no build error when build these 2 drivers.
>

If this is the case, I suggest you update patch 8187/1 to retain the
byte_rev_table symbol, even in the accelerated case, and remove it
with a followup patch once Joe's patches have landed upstream. Also, a
link to the patches would be nice, and perhaps a bit of explanation
how/when they are expected to be merged.

--
Ard.

2014-11-03 09:51:02

by Will Deacon

[permalink] [raw]
Subject: Re: [RFC V6 3/3] arm64:add bitrev.h file to support rbit instruction

On Mon, Nov 03, 2014 at 08:47:32AM +0000, Ard Biesheuvel wrote:
> On 3 November 2014 03:17, Wang, Yalin <[email protected]> wrote:
> >> From: Will Deacon [mailto:[email protected]]
> >> > +#ifndef __ASM_ARM64_BITREV_H
> >> > +#define __ASM_ARM64_BITREV_H
> >>
> >> Really minor nit, but we don't tend to include 'ARM64' in our header guards,
> >> so this should just be __ASM_BITREV_H.
> >>
> >> With that change,
> >>
> >> Acked-by: Will Deacon <[email protected]>
> >>
> > I have send the patch to the patch system:
> > http://www.arm.linux.org.uk/developer/patches/search.php?uid=4025
> >
> > 8187/1 8188/1 8189/1
> >
> > Just remind you that , should also cherry-pick Joe Perches's
> > 2 patches:
> > [PATCH] 6fire: Convert byte_rev_table uses to bitrev8
> > [PATCH] carl9170: Convert byte_rev_table uses to bitrev8
> >
> > To make sure there is no build error when build these 2 drivers.
> >
>
> If this is the case, I suggest you update patch 8187/1 to retain the
> byte_rev_table symbol, even in the accelerated case, and remove it
> with a followup patch once Joe's patches have landed upstream. Also, a
> link to the patches would be nice, and perhaps a bit of explanation
> how/when they are expected to be merged.

Indeed, or instead put together a series with the appropriate acks so
somebody can merge it all in one go. Merging this on a piecemeal basis is
going to cause breakages (as you pointed out).

Will

2014-11-04 01:45:48

by Wang, Yalin

[permalink] [raw]
Subject: RE: [RFC V6 3/3] arm64:add bitrev.h file to support rbit instruction

> From: Will Deacon [mailto:[email protected]]
> >
> > If this is the case, I suggest you update patch 8187/1 to retain the
> > byte_rev_table symbol, even in the accelerated case, and remove it
> > with a followup patch once Joe's patches have landed upstream. Also, a
> > link to the patches would be nice, and perhaps a bit of explanation
> > how/when they are expected to be merged.
>
> Indeed, or instead put together a series with the appropriate acks so
> somebody can merge it all in one go. Merging this on a piecemeal basis is
> going to cause breakages (as you pointed out).
>
> Will

Hi Will,
Could I add you as ack-by , and submit these 2 patches into the
Patch system ?
So you can merge them together .

Thanks

2014-11-13 23:53:36

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [RFC V6 2/3] arm:add bitrev.h file to support rbit instruction

On Fri, Oct 31, 2014 at 01:42:44PM +0800, Wang, Yalin wrote:
> This patch add bitrev.h file to support rbit instruction,
> so that we can do bitrev operation by hardware.
> Signed-off-by: Yalin Wang <[email protected]>
> ---
> arch/arm/Kconfig | 1 +
> arch/arm/include/asm/bitrev.h | 21 +++++++++++++++++++++
> 2 files changed, 22 insertions(+)
> create mode 100644 arch/arm/include/asm/bitrev.h
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 89c4b5c..be92b3b 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -28,6 +28,7 @@ config ARM
> select HANDLE_DOMAIN_IRQ
> select HARDIRQS_SW_RESEND
> select HAVE_ARCH_AUDITSYSCALL if (AEABI && !OABI_COMPAT)
> + select HAVE_ARCH_BITREVERSE if (CPU_V7M || CPU_V7)

Looking at this, this is just wrong. Take a moment to consider what
happens if we build a kernel which supports both ARMv6 _and_ ARMv7 CPUs.
What happens if an ARMv6 CPU tries to execute an rbit instruction?

Second point (which isn't obvious from your submissions on-list) is that
you've loaded the patch system up with patches for other parts of the
kernel tree for which I am not responsible for. As such, I can't take
those patches without the sub-tree maintainer acking them. Also, the
commit text in those patches look weird:

6fire: Convert byte_rev_table uses to bitrev8

Use the inline function instead of directly indexing the array.

This allows some architectures with hardware instructions for bit
reversals to eliminate the array.

Signed-off-by: Joe Perches <(address hidden)>
Signed-off-by: Yalin Wang <(address hidden)>

Why is Joe signing off on these patches? As his is the first sign-off,
one assumes that he was responsible for creating the patch in the first
place, but there is no From: line marking him as the author. Shouldn't
his entry be an Acked-by: ?

Confused.

--
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.

2014-11-14 00:05:35

by Joe Perches

[permalink] [raw]
Subject: Re: [RFC V6 2/3] arm:add bitrev.h file to support rbit instruction

On Thu, 2014-11-13 at 23:53 +0000, Russell King - ARM Linux wrote:
> On Fri, Oct 31, 2014 at 01:42:44PM +0800, Wang, Yalin wrote:
> > This patch add bitrev.h file to support rbit instruction,
> > so that we can do bitrev operation by hardware.
> > Signed-off-by: Yalin Wang <[email protected]>
> > ---
> > arch/arm/Kconfig | 1 +
> > arch/arm/include/asm/bitrev.h | 21 +++++++++++++++++++++
> > 2 files changed, 22 insertions(+)
> > create mode 100644 arch/arm/include/asm/bitrev.h
> >
> > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > index 89c4b5c..be92b3b 100644
> > --- a/arch/arm/Kconfig
> > +++ b/arch/arm/Kconfig
> > @@ -28,6 +28,7 @@ config ARM
> > select HANDLE_DOMAIN_IRQ
> > select HARDIRQS_SW_RESEND
> > select HAVE_ARCH_AUDITSYSCALL if (AEABI && !OABI_COMPAT)
> > + select HAVE_ARCH_BITREVERSE if (CPU_V7M || CPU_V7)
>
> Looking at this, this is just wrong. Take a moment to consider what
> happens if we build a kernel which supports both ARMv6 _and_ ARMv7 CPUs.
> What happens if an ARMv6 CPU tries to execute an rbit instruction?
>
> Second point (which isn't obvious from your submissions on-list) is that
> you've loaded the patch system up with patches for other parts of the
> kernel tree for which I am not responsible for. As such, I can't take
> those patches without the sub-tree maintainer acking them. Also, the
> commit text in those patches look weird:
>
> 6fire: Convert byte_rev_table uses to bitrev8
>
> Use the inline function instead of directly indexing the array.
>
> This allows some architectures with hardware instructions for bit
> reversals to eliminate the array.
>
> Signed-off-by: Joe Perches <(address hidden)>
> Signed-off-by: Yalin Wang <(address hidden)>
>
> Why is Joe signing off on these patches?
> Shouldn't his entry be an Acked-by: ?

I didn't sign off on or ack the "add bitrev.h" patch.

I created 2 patches that converted direct uses of byte_rev_table
to that bitrev8 static inline. One of them is already in -next

7a1283d8f5298437a454ec477384dcd9f9f88bac carl9170: Convert byte_rev_table uses to bitrev8

The other hasn't been applied.

https://lkml.org/lkml/2014/10/28/1056

Maybe Takashi Iwai will get around to it one day.

2014-11-14 00:17:36

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [RFC V6 2/3] arm:add bitrev.h file to support rbit instruction

On Thu, Nov 13, 2014 at 04:05:30PM -0800, Joe Perches wrote:
> On Thu, 2014-11-13 at 23:53 +0000, Russell King - ARM Linux wrote:
> > On Fri, Oct 31, 2014 at 01:42:44PM +0800, Wang, Yalin wrote:
> > > This patch add bitrev.h file to support rbit instruction,
> > > so that we can do bitrev operation by hardware.
> > > Signed-off-by: Yalin Wang <[email protected]>
> > > ---
> > > arch/arm/Kconfig | 1 +
> > > arch/arm/include/asm/bitrev.h | 21 +++++++++++++++++++++
> > > 2 files changed, 22 insertions(+)
> > > create mode 100644 arch/arm/include/asm/bitrev.h
> > >
> > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > > index 89c4b5c..be92b3b 100644
> > > --- a/arch/arm/Kconfig
> > > +++ b/arch/arm/Kconfig
> > > @@ -28,6 +28,7 @@ config ARM
> > > select HANDLE_DOMAIN_IRQ
> > > select HARDIRQS_SW_RESEND
> > > select HAVE_ARCH_AUDITSYSCALL if (AEABI && !OABI_COMPAT)
> > > + select HAVE_ARCH_BITREVERSE if (CPU_V7M || CPU_V7)
> >
> > Looking at this, this is just wrong. Take a moment to consider what
> > happens if we build a kernel which supports both ARMv6 _and_ ARMv7 CPUs.
> > What happens if an ARMv6 CPU tries to execute an rbit instruction?
> >
> > Second point (which isn't obvious from your submissions on-list) is that
> > you've loaded the patch system up with patches for other parts of the
> > kernel tree for which I am not responsible for. As such, I can't take
> > those patches without the sub-tree maintainer acking them. Also, the
> > commit text in those patches look weird:
> >
> > 6fire: Convert byte_rev_table uses to bitrev8
> >
> > Use the inline function instead of directly indexing the array.
> >
> > This allows some architectures with hardware instructions for bit
> > reversals to eliminate the array.
> >
> > Signed-off-by: Joe Perches <(address hidden)>
> > Signed-off-by: Yalin Wang <(address hidden)>
> >
> > Why is Joe signing off on these patches?
> > Shouldn't his entry be an Acked-by: ?
>
> I didn't sign off on or ack the "add bitrev.h" patch.

Correct, I never said you did. Please read my message a bit more carefully
next time, huh?

> I created 2 patches that converted direct uses of byte_rev_table
> to that bitrev8 static inline. One of them is already in -next
>
> 7a1283d8f5298437a454ec477384dcd9f9f88bac carl9170: Convert byte_rev_table uses to bitrev8
>
> The other hasn't been applied.
>
> https://lkml.org/lkml/2014/10/28/1056
>
> Maybe Takashi Iwai will get around to it one day.

Great, so I can just discard these that were incorrectly submitted to me
then.

--
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.

2014-11-14 00:45:49

by Joe Perches

[permalink] [raw]
Subject: Re: [RFC V6 2/3] arm:add bitrev.h file to support rbit instruction

On Fri, 2014-11-14 at 00:17 +0000, Russell King - ARM Linux wrote:
> On Thu, Nov 13, 2014 at 04:05:30PM -0800, Joe Perches wrote:
> > On Thu, 2014-11-13 at 23:53 +0000, Russell King - ARM Linux wrote:
> > > On Fri, Oct 31, 2014 at 01:42:44PM +0800, Wang, Yalin wrote:
> > > > This patch add bitrev.h file to support rbit instruction,
> > > > so that we can do bitrev operation by hardware.
> > > > Signed-off-by: Yalin Wang <[email protected]>
> > > > ---
> > > > arch/arm/Kconfig | 1 +
> > > > arch/arm/include/asm/bitrev.h | 21 +++++++++++++++++++++
> > > > 2 files changed, 22 insertions(+)
> > > > create mode 100644 arch/arm/include/asm/bitrev.h
> > > >
> > > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > > > index 89c4b5c..be92b3b 100644
> > > > --- a/arch/arm/Kconfig
> > > > +++ b/arch/arm/Kconfig
> > > > @@ -28,6 +28,7 @@ config ARM
> > > > select HANDLE_DOMAIN_IRQ
> > > > select HARDIRQS_SW_RESEND
> > > > select HAVE_ARCH_AUDITSYSCALL if (AEABI && !OABI_COMPAT)
> > > > + select HAVE_ARCH_BITREVERSE if (CPU_V7M || CPU_V7)
> > >
> > > Looking at this, this is just wrong. Take a moment to consider what
> > > happens if we build a kernel which supports both ARMv6 _and_ ARMv7 CPUs.
> > > What happens if an ARMv6 CPU tries to execute an rbit instruction?
> > >
> > > Second point (which isn't obvious from your submissions on-list) is that
> > > you've loaded the patch system up with patches for other parts of the
> > > kernel tree for which I am not responsible for. As such, I can't take
> > > those patches without the sub-tree maintainer acking them. Also, the
> > > commit text in those patches look weird:
> > >
> > > 6fire: Convert byte_rev_table uses to bitrev8
> > >
> > > Use the inline function instead of directly indexing the array.
> > >
> > > This allows some architectures with hardware instructions for bit
> > > reversals to eliminate the array.
> > >
> > > Signed-off-by: Joe Perches <(address hidden)>
> > > Signed-off-by: Yalin Wang <(address hidden)>
> > >
> > > Why is Joe signing off on these patches?
> > > Shouldn't his entry be an Acked-by: ?
> >
> > I didn't sign off on or ack the "add bitrev.h" patch.
>
> Correct, I never said you did. Please read my message a bit more carefully
> next time, huh?

You've no reason to write that Russell.

I'm not trying to be anything other than clear and no I
didn't say you said that either.

Why not make your own writing clearer or your own memory
sharper then eh? Reply on the patch I actually wrote.
You were cc'd on it when I submitted it.

> > I created 2 patches that converted direct uses of byte_rev_table
> > to that bitrev8 static inline. One of them is already in -next
> >
> > 7a1283d8f5298437a454ec477384dcd9f9f88bac carl9170: Convert byte_rev_table uses to bitrev8
> >
> > The other hasn't been applied.
> >
> > https://lkml.org/lkml/2014/10/28/1056
> >
> > Maybe Takashi Iwai will get around to it one day.
>
> Great, so I can just discard these that were incorrectly submitted to me
> then.

I think you shouldn't apply these patches or updated
ones either until all the current uses are converted.

cheers, Joe

2014-11-14 01:18:52

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [RFC V6 2/3] arm:add bitrev.h file to support rbit instruction

On Thu, Nov 13, 2014 at 04:45:43PM -0800, Joe Perches wrote:
> On Fri, 2014-11-14 at 00:17 +0000, Russell King - ARM Linux wrote:
> > On Thu, Nov 13, 2014 at 04:05:30PM -0800, Joe Perches wrote:
> > > On Thu, 2014-11-13 at 23:53 +0000, Russell King - ARM Linux wrote:
> > > > On Fri, Oct 31, 2014 at 01:42:44PM +0800, Wang, Yalin wrote:
> > > > > This patch add bitrev.h file to support rbit instruction,
> > > > > so that we can do bitrev operation by hardware.
> > > > > Signed-off-by: Yalin Wang <[email protected]>
> > > > > ---
> > > > > arch/arm/Kconfig | 1 +
> > > > > arch/arm/include/asm/bitrev.h | 21 +++++++++++++++++++++
> > > > > 2 files changed, 22 insertions(+)
> > > > > create mode 100644 arch/arm/include/asm/bitrev.h
> > > > >
> > > > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > > > > index 89c4b5c..be92b3b 100644
> > > > > --- a/arch/arm/Kconfig
> > > > > +++ b/arch/arm/Kconfig
> > > > > @@ -28,6 +28,7 @@ config ARM
> > > > > select HANDLE_DOMAIN_IRQ
> > > > > select HARDIRQS_SW_RESEND
> > > > > select HAVE_ARCH_AUDITSYSCALL if (AEABI && !OABI_COMPAT)
> > > > > + select HAVE_ARCH_BITREVERSE if (CPU_V7M || CPU_V7)
> > > >
> > > > Looking at this, this is just wrong. Take a moment to consider what
> > > > happens if we build a kernel which supports both ARMv6 _and_ ARMv7 CPUs.
> > > > What happens if an ARMv6 CPU tries to execute an rbit instruction?
> > > >
> > > > Second point (which isn't obvious from your submissions on-list) is that
> > > > you've loaded the patch system up with patches for other parts of the
> > > > kernel tree for which I am not responsible for. As such, I can't take
> > > > those patches without the sub-tree maintainer acking them. Also, the
> > > > commit text in those patches look weird:
> > > >
> > > > 6fire: Convert byte_rev_table uses to bitrev8
> > > >
> > > > Use the inline function instead of directly indexing the array.
> > > >
> > > > This allows some architectures with hardware instructions for bit
> > > > reversals to eliminate the array.
> > > >
> > > > Signed-off-by: Joe Perches <(address hidden)>
> > > > Signed-off-by: Yalin Wang <(address hidden)>
> > > >
> > > > Why is Joe signing off on these patches?
> > > > Shouldn't his entry be an Acked-by: ?
> > >
> > > I didn't sign off on or ack the "add bitrev.h" patch.
> >
> > Correct, I never said you did. Please read my message a bit more carefully
> > next time, huh?
>
> You've no reason to write that Russell.

Absolutely I have, but I'm not going to discuss it because I'll just
end up flaming you because in my mind you are the one who is completely
mistaken with your comments.

In case it hasn't been realised, I hardly read this mailing list anymore,
or messages that I'm Cc'd on. I do read most messages that I'm in the
To: line, but generally not if they're DT changes (which always end up
being marked To: me.)

> > Great, so I can just discard these that were incorrectly submitted to me
> > then.
>
> I think you shouldn't apply these patches or updated
> ones either until all the current uses are converted.

Where are the dependencies mentioned? How do I get to know when all
the dependencies are met? Who is tracking the dependencies?

--
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.

2014-11-14 01:26:49

by Joe Perches

[permalink] [raw]
Subject: Re: [RFC V6 2/3] arm:add bitrev.h file to support rbit instruction

On Fri, 2014-11-14 at 01:18 +0000, Russell King - ARM Linux wrote:
> On Thu, Nov 13, 2014 at 04:45:43PM -0800, Joe Perches wrote:
> > I think you shouldn't apply these patches or updated
> > ones either until all the current uses are converted.
>
> Where are the dependencies mentioned?

I mentioned it when these patches (which are not
mine btw), were submitted the second time.

https://lkml.org/lkml/2014/10/27/69

> How do I get to know when all
> the dependencies are met?

No idea.

> Who is tracking the dependencies?

Not me.

2014-11-14 02:01:39

by Wang, Yalin

[permalink] [raw]
Subject: RE: [RFC V6 2/3] arm:add bitrev.h file to support rbit instruction

> -----Original Message-----
> From: Russell King - ARM Linux [mailto:[email protected]]
> Sent: Friday, November 14, 2014 7:53 AM
> To: Wang, Yalin
> > On Fri, Oct 31, 2014 at 01:42:44PM +0800, Wang, Yalin wrote:
> > This patch add bitrev.h file to support rbit instruction, so that we
> > can do bitrev operation by hardware.
> > Signed-off-by: Yalin Wang <[email protected]>
> > ---
> > arch/arm/Kconfig | 1 +
> > arch/arm/include/asm/bitrev.h | 21 +++++++++++++++++++++
> > 2 files changed, 22 insertions(+)
> > create mode 100644 arch/arm/include/asm/bitrev.h
> >
> > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index
> > 89c4b5c..be92b3b 100644
> > --- a/arch/arm/Kconfig
> > +++ b/arch/arm/Kconfig
> > @@ -28,6 +28,7 @@ config ARM
> > select HANDLE_DOMAIN_IRQ
> > select HARDIRQS_SW_RESEND
> > select HAVE_ARCH_AUDITSYSCALL if (AEABI && !OABI_COMPAT)
> > + select HAVE_ARCH_BITREVERSE if (CPU_V7M || CPU_V7)
>
> Looking at this, this is just wrong. Take a moment to consider what
> happens if we build a kernel which supports both ARMv6 _and_ ARMv7 CPUs.
> What happens if an ARMv6 CPU tries to execute an rbit instruction?

Is it possible to build a kernel that support both CPU_V6 and CPU_V7?
I mean in Kconfig, CPU_V6 = y and CPU_V7 = y ?
If there is problem like you said,
How about this solution:
select HAVE_ARCH_BITREVERSE if ((CPU_V7M || CPU_V7) && !CPU_V6)


> Second point (which isn't obvious from your submissions on-list) is that
> you've loaded the patch system up with patches for other parts of the
> kernel tree for which I am not responsible for. As such, I can't take
> those patches without the sub-tree maintainer acking them. Also, the
> commit text in those patches look weird:
>
> 6fire: Convert byte_rev_table uses to bitrev8
>
> Use the inline function instead of directly indexing the array.
>
> This allows some architectures with hardware instructions for bit reversals
> to eliminate the array.
>
> Signed-off-by: Joe Perches <(address hidden)>
> Signed-off-by: Yalin Wang <(address hidden)>
>
> Why is Joe signing off on these patches? As his is the first sign-off, one
> assumes that he was responsible for creating the patch in the first place,
> but there is no From: line marking him as the author. Shouldn't his entry
> be an Acked-by: ?
>
> Confused.
For this patch,
I just cherry-pick from Joe,
If you are not responsible for this part,
I will submit to the maintainers for these patches .
Sorry for that .

2014-11-14 06:37:18

by Takashi Iwai

[permalink] [raw]
Subject: Re: [RFC V6 2/3] arm:add bitrev.h file to support rbit instruction

At Thu, 13 Nov 2014 16:05:30 -0800,
Joe Perches wrote:
>
> On Thu, 2014-11-13 at 23:53 +0000, Russell King - ARM Linux wrote:
> > On Fri, Oct 31, 2014 at 01:42:44PM +0800, Wang, Yalin wrote:
> > > This patch add bitrev.h file to support rbit instruction,
> > > so that we can do bitrev operation by hardware.
> > > Signed-off-by: Yalin Wang <[email protected]>
> > > ---
> > > arch/arm/Kconfig | 1 +
> > > arch/arm/include/asm/bitrev.h | 21 +++++++++++++++++++++
> > > 2 files changed, 22 insertions(+)
> > > create mode 100644 arch/arm/include/asm/bitrev.h
> > >
> > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > > index 89c4b5c..be92b3b 100644
> > > --- a/arch/arm/Kconfig
> > > +++ b/arch/arm/Kconfig
> > > @@ -28,6 +28,7 @@ config ARM
> > > select HANDLE_DOMAIN_IRQ
> > > select HARDIRQS_SW_RESEND
> > > select HAVE_ARCH_AUDITSYSCALL if (AEABI && !OABI_COMPAT)
> > > + select HAVE_ARCH_BITREVERSE if (CPU_V7M || CPU_V7)
> >
> > Looking at this, this is just wrong. Take a moment to consider what
> > happens if we build a kernel which supports both ARMv6 _and_ ARMv7 CPUs.
> > What happens if an ARMv6 CPU tries to execute an rbit instruction?
> >
> > Second point (which isn't obvious from your submissions on-list) is that
> > you've loaded the patch system up with patches for other parts of the
> > kernel tree for which I am not responsible for. As such, I can't take
> > those patches without the sub-tree maintainer acking them. Also, the
> > commit text in those patches look weird:
> >
> > 6fire: Convert byte_rev_table uses to bitrev8
> >
> > Use the inline function instead of directly indexing the array.
> >
> > This allows some architectures with hardware instructions for bit
> > reversals to eliminate the array.
> >
> > Signed-off-by: Joe Perches <(address hidden)>
> > Signed-off-by: Yalin Wang <(address hidden)>
> >
> > Why is Joe signing off on these patches?
> > Shouldn't his entry be an Acked-by: ?
>
> I didn't sign off on or ack the "add bitrev.h" patch.
>
> I created 2 patches that converted direct uses of byte_rev_table
> to that bitrev8 static inline. One of them is already in -next
>
> 7a1283d8f5298437a454ec477384dcd9f9f88bac carl9170: Convert byte_rev_table uses to bitrev8
>
> The other hasn't been applied.
>
> https://lkml.org/lkml/2014/10/28/1056
>
> Maybe Takashi Iwai will get around to it one day.

It was not clear to me whether I should apply it individually from
others in the whole thread. Your description looked as if it makes
sense only with ARM's bitrev8 support.

So, again: should I apply this now to my tree?


Takashi

2014-11-14 06:55:15

by Joe Perches

[permalink] [raw]
Subject: Re: [RFC V6 2/3] arm:add bitrev.h file to support rbit instruction

On Fri, 2014-11-14 at 07:37 +0100, Takashi Iwai wrote:
> At Thu, 13 Nov 2014 16:05:30 -0800,
> Joe Perches wrote:
> >
> > On Thu, 2014-11-13 at 23:53 +0000, Russell King - ARM Linux wrote:
> > > On Fri, Oct 31, 2014 at 01:42:44PM +0800, Wang, Yalin wrote:
> > > > This patch add bitrev.h file to support rbit instruction,
> > > > so that we can do bitrev operation by hardware.
> > > > Signed-off-by: Yalin Wang <[email protected]>
> > > > ---
> > > > arch/arm/Kconfig | 1 +
> > > > arch/arm/include/asm/bitrev.h | 21 +++++++++++++++++++++
> > > > 2 files changed, 22 insertions(+)
> > > > create mode 100644 arch/arm/include/asm/bitrev.h
> > > >
> > > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > > > index 89c4b5c..be92b3b 100644
> > > > --- a/arch/arm/Kconfig
> > > > +++ b/arch/arm/Kconfig
> > > > @@ -28,6 +28,7 @@ config ARM
> > > > select HANDLE_DOMAIN_IRQ
> > > > select HARDIRQS_SW_RESEND
> > > > select HAVE_ARCH_AUDITSYSCALL if (AEABI && !OABI_COMPAT)
> > > > + select HAVE_ARCH_BITREVERSE if (CPU_V7M || CPU_V7)
> > >
> > > Looking at this, this is just wrong. Take a moment to consider what
> > > happens if we build a kernel which supports both ARMv6 _and_ ARMv7 CPUs.
> > > What happens if an ARMv6 CPU tries to execute an rbit instruction?
> > >
> > > Second point (which isn't obvious from your submissions on-list) is that
> > > you've loaded the patch system up with patches for other parts of the
> > > kernel tree for which I am not responsible for. As such, I can't take
> > > those patches without the sub-tree maintainer acking them. Also, the
> > > commit text in those patches look weird:
> > >
> > > 6fire: Convert byte_rev_table uses to bitrev8
> > >
> > > Use the inline function instead of directly indexing the array.
> > >
> > > This allows some architectures with hardware instructions for bit
> > > reversals to eliminate the array.
> > >
> > > Signed-off-by: Joe Perches <(address hidden)>
> > > Signed-off-by: Yalin Wang <(address hidden)>
> > >
> > > Why is Joe signing off on these patches?
> > > Shouldn't his entry be an Acked-by: ?
> >
> > I didn't sign off on or ack the "add bitrev.h" patch.
> >
> > I created 2 patches that converted direct uses of byte_rev_table
> > to that bitrev8 static inline. One of them is already in -next
> >
> > 7a1283d8f5298437a454ec477384dcd9f9f88bac carl9170: Convert byte_rev_table uses to bitrev8
> >
> > The other hasn't been applied.
> >
> > https://lkml.org/lkml/2014/10/28/1056
> >
> > Maybe Takashi Iwai will get around to it one day.
>
> It was not clear to me whether I should apply it individually from
> others in the whole thread. Your description looked as if it makes
> sense only with ARM's bitrev8 support.
>
> So, again: should I apply this now to my tree?

I it would be good to apply even if the
bitrev patch for arm is never applied.

$ git grep -w bitrev8 | wc -l
110

vs

this last direct use of byte_rev_table.

cheers, Joe

2014-11-14 07:03:05

by Takashi Iwai

[permalink] [raw]
Subject: Re: [RFC V6 2/3] arm:add bitrev.h file to support rbit instruction

At Thu, 13 Nov 2014 22:55:09 -0800,
Joe Perches wrote:
>
> On Fri, 2014-11-14 at 07:37 +0100, Takashi Iwai wrote:
> > At Thu, 13 Nov 2014 16:05:30 -0800,
> > Joe Perches wrote:
> > >
> > > On Thu, 2014-11-13 at 23:53 +0000, Russell King - ARM Linux wrote:
> > > > On Fri, Oct 31, 2014 at 01:42:44PM +0800, Wang, Yalin wrote:
> > > > > This patch add bitrev.h file to support rbit instruction,
> > > > > so that we can do bitrev operation by hardware.
> > > > > Signed-off-by: Yalin Wang <[email protected]>
> > > > > ---
> > > > > arch/arm/Kconfig | 1 +
> > > > > arch/arm/include/asm/bitrev.h | 21 +++++++++++++++++++++
> > > > > 2 files changed, 22 insertions(+)
> > > > > create mode 100644 arch/arm/include/asm/bitrev.h
> > > > >
> > > > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > > > > index 89c4b5c..be92b3b 100644
> > > > > --- a/arch/arm/Kconfig
> > > > > +++ b/arch/arm/Kconfig
> > > > > @@ -28,6 +28,7 @@ config ARM
> > > > > select HANDLE_DOMAIN_IRQ
> > > > > select HARDIRQS_SW_RESEND
> > > > > select HAVE_ARCH_AUDITSYSCALL if (AEABI && !OABI_COMPAT)
> > > > > + select HAVE_ARCH_BITREVERSE if (CPU_V7M || CPU_V7)
> > > >
> > > > Looking at this, this is just wrong. Take a moment to consider what
> > > > happens if we build a kernel which supports both ARMv6 _and_ ARMv7 CPUs.
> > > > What happens if an ARMv6 CPU tries to execute an rbit instruction?
> > > >
> > > > Second point (which isn't obvious from your submissions on-list) is that
> > > > you've loaded the patch system up with patches for other parts of the
> > > > kernel tree for which I am not responsible for. As such, I can't take
> > > > those patches without the sub-tree maintainer acking them. Also, the
> > > > commit text in those patches look weird:
> > > >
> > > > 6fire: Convert byte_rev_table uses to bitrev8
> > > >
> > > > Use the inline function instead of directly indexing the array.
> > > >
> > > > This allows some architectures with hardware instructions for bit
> > > > reversals to eliminate the array.
> > > >
> > > > Signed-off-by: Joe Perches <(address hidden)>
> > > > Signed-off-by: Yalin Wang <(address hidden)>
> > > >
> > > > Why is Joe signing off on these patches?
> > > > Shouldn't his entry be an Acked-by: ?
> > >
> > > I didn't sign off on or ack the "add bitrev.h" patch.
> > >
> > > I created 2 patches that converted direct uses of byte_rev_table
> > > to that bitrev8 static inline. One of them is already in -next
> > >
> > > 7a1283d8f5298437a454ec477384dcd9f9f88bac carl9170: Convert byte_rev_table uses to bitrev8
> > >
> > > The other hasn't been applied.
> > >
> > > https://lkml.org/lkml/2014/10/28/1056
> > >
> > > Maybe Takashi Iwai will get around to it one day.
> >
> > It was not clear to me whether I should apply it individually from
> > others in the whole thread. Your description looked as if it makes
> > sense only with ARM's bitrev8 support.
> >
> > So, again: should I apply this now to my tree?
>
> I it would be good to apply even if the
> bitrev patch for arm is never applied.
>
> $ git grep -w bitrev8 | wc -l
> 110
>
> vs
>
> this last direct use of byte_rev_table.

Alright, I picked up your original patch and merged.


thanks,

Takashi

2014-11-14 09:52:25

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [RFC V6 2/3] arm:add bitrev.h file to support rbit instruction

On Thu, Nov 13, 2014 at 05:26:34PM -0800, Joe Perches wrote:
> On Fri, 2014-11-14 at 01:18 +0000, Russell King - ARM Linux wrote:
> > On Thu, Nov 13, 2014 at 04:45:43PM -0800, Joe Perches wrote:
> > > I think you shouldn't apply these patches or updated
> > > ones either until all the current uses are converted.
> >
> > Where are the dependencies mentioned?
>
> I mentioned it when these patches (which are not
> mine btw), were submitted the second time.

Yes, I'm well aware that the author of the ARM patches are Yalin Wang.

> https://lkml.org/lkml/2014/10/27/69
>
> > How do I get to know when all
> > the dependencies are met?
>
> No idea.
>
> > Who is tracking the dependencies?
>
> Not me.

Right, what that means is that no one is doing that. What you've also
said in this thread now is that the ARM patches should not be applied
until all the other users are converted. As those patches are going via
other trees, that means the ARM patches can only be applied _after_ the
next merge window _if_ all maintainers pick up the previous set.

As I'm not tracking the status of what other maintainers do, I'm simply
going to avoid applying these patches until after the next merge window
and hope that the other maintainers pick the dependent patches up and get
them in during the next merge window. If not, I guess we'll see compile
breakage.

--
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.

2014-11-14 09:58:27

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [RFC V6 2/3] arm:add bitrev.h file to support rbit instruction

On Fri, Nov 14, 2014 at 10:01:34AM +0800, Wang, Yalin wrote:
> > -----Original Message-----
> > From: Russell King - ARM Linux [mailto:[email protected]]
> > Sent: Friday, November 14, 2014 7:53 AM
> > To: Wang, Yalin
> > > On Fri, Oct 31, 2014 at 01:42:44PM +0800, Wang, Yalin wrote:
> > > This patch add bitrev.h file to support rbit instruction, so that we
> > > can do bitrev operation by hardware.
> > > Signed-off-by: Yalin Wang <[email protected]>
> > > ---
> > > arch/arm/Kconfig | 1 +
> > > arch/arm/include/asm/bitrev.h | 21 +++++++++++++++++++++
> > > 2 files changed, 22 insertions(+)
> > > create mode 100644 arch/arm/include/asm/bitrev.h
> > >
> > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index
> > > 89c4b5c..be92b3b 100644
> > > --- a/arch/arm/Kconfig
> > > +++ b/arch/arm/Kconfig
> > > @@ -28,6 +28,7 @@ config ARM
> > > select HANDLE_DOMAIN_IRQ
> > > select HARDIRQS_SW_RESEND
> > > select HAVE_ARCH_AUDITSYSCALL if (AEABI && !OABI_COMPAT)
> > > + select HAVE_ARCH_BITREVERSE if (CPU_V7M || CPU_V7)
> >
> > Looking at this, this is just wrong. Take a moment to consider what
> > happens if we build a kernel which supports both ARMv6 _and_ ARMv7 CPUs.
> > What happens if an ARMv6 CPU tries to execute an rbit instruction?
>
> Is it possible to build a kernel that support both CPU_V6 and CPU_V7?

Absolutely it is.

> I mean in Kconfig, CPU_V6 = y and CPU_V7 = y ?

Yes.

> If there is problem like you said,
> How about this solution:
> select HAVE_ARCH_BITREVERSE if ((CPU_V7M || CPU_V7) && !CPU_V6)

That would work.

> For this patch,
> I just cherry-pick from Joe,
> If you are not responsible for this part,
> I will submit to the maintainers for these patches .
> Sorry for that .

I think you need to discuss with Joe how Joe would like his patches
handled. However, it seems that Joe already sent his patches to the
appropriate maintainers, and they have been applying those patches
themselves.

Since your generic ARM changes depend on these patches being accepted
first, this means is that I can't apply the generic ARM changes until
those other patches have hit mainline, otherwise things are going to
break. So, when you come to submit the latest set of patches to the
patch system, please do so only after these dependent patches have
been merged into mainline so that they don't get accidentally applied
before hand and break the two drivers that Joe mentioned.

Thanks.

--
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.

2014-11-17 02:39:08

by Wang, Yalin

[permalink] [raw]
Subject: RE: [RFC V6 2/3] arm:add bitrev.h file to support rbit instruction

> From: Russell King - ARM Linux [mailto:[email protected]]
> Sent: Friday, November 14, 2014 5:58 PM
> To: Wang, Yalin
> Cc: 'Will Deacon'; 'Ard Biesheuvel'; '[email protected]';
> '[email protected]'; '[email protected]'; 'Joe Perches'; 'linux-arm-
> [email protected]'
> Subject: Re: [RFC V6 2/3] arm:add bitrev.h file to support rbit instruction
>
> > Is it possible to build a kernel that support both CPU_V6 and CPU_V7?
>
> Absolutely it is.
>
> > I mean in Kconfig, CPU_V6 = y and CPU_V7 = y ?
>
> Yes.
>
> > If there is problem like you said,
> > How about this solution:
> > select HAVE_ARCH_BITREVERSE if ((CPU_V7M || CPU_V7) && !CPU_V6)
>
> That would work.
>
OK, I will submit a patch for this change.

> > For this patch,
> > I just cherry-pick from Joe,
> > If you are not responsible for this part, I will submit to the
> > maintainers for these patches .
> > Sorry for that .
>
> I think you need to discuss with Joe how Joe would like his patches handled.
> However, it seems that Joe already sent his patches to the appropriate
> maintainers, and they have been applying those patches themselves.
>
> Since your generic ARM changes depend on these patches being accepted first,
> this means is that I can't apply the generic ARM changes until those other
> patches have hit mainline, otherwise things are going to break. So, when
> you come to submit the latest set of patches to the patch system, please do
> so only after these dependent patches have been merged into mainline so
> that they don't get accidentally applied before hand and break the two
> drivers that Joe mentioned.

Joe has submitted patches to maintainers,
So we need wait for them to be accepted .

Thanks