| Message ID | 35FD53F367049845BC99AC72306C23D103E010D18254@CNBJMBX05.corpusers.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <yalin.wang@sonymobile.com> > --- > 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?
> > 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.
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 <yalin.wang@sonymobile.com> > --- > 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
> From: Will Deacon [mailto:will.deacon@arm.com] > > +++ 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
On Tue, Oct 28, 2014 at 01:34:42AM +0000, Wang, Yalin wrote: > > From: Will Deacon [mailto:will.deacon@arm.com] > > > +++ 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
> From: Will Deacon [mailto:will.deacon@arm.com] > 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
On Mon, Oct 27, 2014 at 2:46 PM, Joe Perches <joe@perches.com> 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 <yalin.wang@sonymobile.com> >> --- >> 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
> From: Rob Herring [mailto:robherring2@gmail.com] > >> 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.
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.
> From: Joe Perches [mailto:joe@perches.com] > > 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.
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 <yalin.wang@sonymobile.com> > --- > 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
On 30 October 2014 13:01, Will Deacon <will.deacon@arm.com> 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 <yalin.wang@sonymobile.com> >> --- >> 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.
On Thu, Oct 30, 2014 at 12:26:42PM +0000, Ard Biesheuvel wrote: > On 30 October 2014 13:01, Will Deacon <will.deacon@arm.com> 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 <yalin.wang@sonymobile.com> > >> --- > >> 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
> From: Will Deacon [mailto:will.deacon@arm.com] > Sent: Thursday, October 30, 2014 8:01 PM > To: Wang, Yalin > Cc: 'Rob Herring'; 'Joe Perches'; 'Russell King - ARM Linux'; 'linux- > kernel@vger.kernel.org'; 'akinobu.mita@gmail.com'; 'linux-mm@kvack.org'; > 'linux-arm-kernel@lists.infradead.org' > 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
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.
> From: Joe Perches [mailto:joe@perches.com] > > 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
> 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 <yalin.wang@sonymobile.com> > --- > 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
On Fri, 2014-10-31 at 15:51 +0800, Wang, Yalin wrote: > > From: Joe Perches [mailto:joe@perches.com] > > > 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(); }
> From: Joe Perches [mailto:joe@perches.com] > > > > @@ -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
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 <yalin.wang@sonymobile.com> > --- > 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 <will.deacon@arm.com> Will
> From: Will Deacon [mailto:will.deacon@arm.com] > > +#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 <will.deacon@arm.com> > 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
On 3 November 2014 03:17, Wang, Yalin <Yalin.Wang@sonymobile.com> wrote: >> From: Will Deacon [mailto:will.deacon@arm.com] >> > +#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 <will.deacon@arm.com> >> > 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.
On Mon, Nov 03, 2014 at 08:47:32AM +0000, Ard Biesheuvel wrote: > On 3 November 2014 03:17, Wang, Yalin <Yalin.Wang@sonymobile.com> wrote: > >> From: Will Deacon [mailto:will.deacon@arm.com] > >> > +#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 <will.deacon@arm.com> > >> > > 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
> From: Will Deacon [mailto:will.deacon@arm.com] > > > > 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
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 <yalin.wang@sonymobile.com> > --- > 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.
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 <yalin.wang@sonymobile.com> > > --- > > 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.
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 <yalin.wang@sonymobile.com> > > > --- > > > 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.
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 <yalin.wang@sonymobile.com> > > > > --- > > > > 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
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 <yalin.wang@sonymobile.com> > > > > > --- > > > > > 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?
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.
> -----Original Message----- > From: Russell King - ARM Linux [mailto:linux@arm.linux.org.uk] > 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 <yalin.wang@sonymobile.com> > > --- > > 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 .
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 <yalin.wang@sonymobile.com> > > > --- > > > 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
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 <yalin.wang@sonymobile.com> > > > > --- > > > > 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
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 <yalin.wang@sonymobile.com> > > > > > --- > > > > > 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
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.
On Fri, Nov 14, 2014 at 10:01:34AM +0800, Wang, Yalin wrote: > > -----Original Message----- > > From: Russell King - ARM Linux [mailto:linux@arm.linux.org.uk] > > 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 <yalin.wang@sonymobile.com> > > > --- > > > 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.
> From: Russell King - ARM Linux [mailto:linux@arm.linux.org.uk] > Sent: Friday, November 14, 2014 5:58 PM > To: Wang, Yalin > Cc: 'Will Deacon'; 'Ard Biesheuvel'; 'linux-kernel@vger.kernel.org'; > 'akinobu.mita@gmail.com'; 'linux-mm@kvack.org'; 'Joe Perches'; 'linux-arm- > kernel@lists.infradead.org' > 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
On Mon, Nov 17, 2014 at 10:38:58AM +0800, Wang, Yalin wrote: > Joe has submitted patches to maintainers, > So we need wait for them to be accepted . I ran these patches through my autobuilder, and while most builds didn't seem to be a problem, the randconfigs found errors: /tmp/ccbiuDjS.s:137: Error: selected processor does not support ARM mode `rbit r3,r2' /tmp/ccbiuDjS.s:145: Error: selected processor does not support ARM mode `rbit r0,r1' make[4]: *** [drivers/iio/amplifiers/ad8366.o] Error 1 /tmp/ccFhnoO3.s:6789: Error: selected processor does not support ARM mode `rbit r2,r2' make[4]: *** [drivers/mtd/devices/docg3.o] Error 1 /tmp/cckMf2pp.s:239: Error: selected processor does not support ARM mode `rbit ip,ip' /tmp/cckMf2pp.s:241: Error: selected processor does not support ARM mode `rbit r2,r2' /tmp/cckMf2pp.s:248: Error: selected processor does not support ARM mode `rbit lr,lr' /tmp/cckMf2pp.s:250: Error: selected processor does not support ARM mode `rbit r3,r3' make[5]: *** [drivers/video/fbdev/nvidia/nvidia.o] Error 1 /tmp/ccTgULsO.s:1151: Error: selected processor does not support ARM mode `rbit r1,r1' /tmp/ccTgULsO.s:1158: Error: selected processor does not support ARM mode `rbit r0,r0' /tmp/ccTgULsO.s:1164: Error: selected processor does not support ARM mode `rbit ip,ip' /tmp/ccTgULsO.s:1166: Error: selected processor does not support ARM mode `rbit r3,r3' /tmp/ccTgULsO.s:1227: Error: selected processor does not support ARM mode `rbit r5,r5' /tmp/ccTgULsO.s:1229: Error: selected processor does not support ARM mode `rbit lr,lr' /tmp/ccTgULsO.s:1236: Error: selected processor does not support ARM mode `rbit r0,r0' /tmp/ccTgULsO.s:1238: Error: selected processor does not support ARM mode `rbit r3,r3' make[5]: *** [drivers/video/fbdev/nvidia/nv_accel.o] Error 1 The root cause is that the kernel being built is supposed to support both ARMv7 and ARMv6K CPUs. However, "rbit" is only available on ARMv6T2 (thumb2) and ARMv7, and not plain ARMv6 or ARMv6K CPUs.
> -----Original Message----- > From: Russell King - ARM Linux [mailto:linux@arm.linux.org.uk] > Sent: Friday, January 09, 2015 2:41 AM > To: Wang, Yalin > Cc: 'Will Deacon'; 'Ard Biesheuvel'; 'linux-kernel@vger.kernel.org'; > 'akinobu.mita@gmail.com'; 'linux-mm@kvack.org'; 'Joe Perches'; 'linux-arm- > kernel@lists.infradead.org' > Subject: Re: [RFC V6 2/3] arm:add bitrev.h file to support rbit instruction > > On Mon, Nov 17, 2014 at 10:38:58AM +0800, Wang, Yalin wrote: > > Joe has submitted patches to maintainers, So we need wait for them to > > be accepted . > > I ran these patches through my autobuilder, and while most builds didn't > seem to be a problem, the randconfigs found errors: > > /tmp/ccbiuDjS.s:137: Error: selected processor does not support ARM mode > `rbit r3,r2' > /tmp/ccbiuDjS.s:145: Error: selected processor does not support ARM mode > `rbit r0,r1' > make[4]: *** [drivers/iio/amplifiers/ad8366.o] Error 1 > > /tmp/ccFhnoO3.s:6789: Error: selected processor does not support ARM mode > `rbit r2,r2' > make[4]: *** [drivers/mtd/devices/docg3.o] Error 1 > > /tmp/cckMf2pp.s:239: Error: selected processor does not support ARM mode > `rbit ip,ip' > /tmp/cckMf2pp.s:241: Error: selected processor does not support ARM mode > `rbit r2,r2' > /tmp/cckMf2pp.s:248: Error: selected processor does not support ARM mode > `rbit lr,lr' > /tmp/cckMf2pp.s:250: Error: selected processor does not support ARM mode > `rbit r3,r3' > make[5]: *** [drivers/video/fbdev/nvidia/nvidia.o] Error 1 > > /tmp/ccTgULsO.s:1151: Error: selected processor does not support ARM mode > `rbit r1,r1' > /tmp/ccTgULsO.s:1158: Error: selected processor does not support ARM mode > `rbit r0,r0' > /tmp/ccTgULsO.s:1164: Error: selected processor does not support ARM mode > `rbit ip,ip' > /tmp/ccTgULsO.s:1166: Error: selected processor does not support ARM mode > `rbit r3,r3' > /tmp/ccTgULsO.s:1227: Error: selected processor does not support ARM mode > `rbit r5,r5' > /tmp/ccTgULsO.s:1229: Error: selected processor does not support ARM mode > `rbit lr,lr' > /tmp/ccTgULsO.s:1236: Error: selected processor does not support ARM mode > `rbit r0,r0' > /tmp/ccTgULsO.s:1238: Error: selected processor does not support ARM mode > `rbit r3,r3' > make[5]: *** [drivers/video/fbdev/nvidia/nv_accel.o] Error 1 > > The root cause is that the kernel being built is supposed to support both > ARMv7 and ARMv6K CPUs. However, "rbit" is only available on > ARMv6T2 (thumb2) and ARMv7, and not plain ARMv6 or ARMv6K CPUs. > In the patch that you applied: 8205/1 add bitrev.h file to support rbit instruction I have add : + select HAVE_ARCH_BITREVERSE if ((CPU_V7M || CPU_V7) && !CPU_V6) If you build kernel support ARMv6K, should CONFIG_CPU_V6=y, isn't it ? Then will not build hardware rbit instruction, isn't it ? Thanks
On Fri, Jan 09, 2015 at 10:16:32AM +0800, Wang, Yalin wrote: > > -----Original Message----- > > From: Russell King - ARM Linux [mailto:linux@arm.linux.org.uk] > > Sent: Friday, January 09, 2015 2:41 AM > > To: Wang, Yalin > > Cc: 'Will Deacon'; 'Ard Biesheuvel'; 'linux-kernel@vger.kernel.org'; > > 'akinobu.mita@gmail.com'; 'linux-mm@kvack.org'; 'Joe Perches'; 'linux-arm- > > kernel@lists.infradead.org' > > Subject: Re: [RFC V6 2/3] arm:add bitrev.h file to support rbit instruction > > > > The root cause is that the kernel being built is supposed to support both > > ARMv7 and ARMv6K CPUs. However, "rbit" is only available on > > ARMv6T2 (thumb2) and ARMv7, and not plain ARMv6 or ARMv6K CPUs. > > > In the patch that you applied: > 8205/1 add bitrev.h file to support rbit instruction > > I have add : > + select HAVE_ARCH_BITREVERSE if ((CPU_V7M || CPU_V7) && !CPU_V6) > > If you build kernel support ARMv6K, should CONFIG_CPU_V6=y, isn't it ? > Then will not build hardware rbit instruction, isn't it ? The config has: CONFIG_CPU_PJ4=y # CONFIG_CPU_V6 is not set CONFIG_CPU_V6K=y CONFIG_CPU_V7=y CONFIG_CPU_32v6=y CONFIG_CPU_32v6K=y CONFIG_CPU_32v7=y And no, the CONFIG_CPU_V* flags refer to the CPUs. The CONFIG_CPU_32v* symbols refer to the CPU architectures.
> -----Original Message----- > From: Russell King - ARM Linux [mailto:linux@arm.linux.org.uk] > Sent: Friday, January 09, 2015 7:11 PM > To: Wang, Yalin > Cc: 'Ard Biesheuvel'; 'Will Deacon'; 'linux-kernel@vger.kernel.org'; > 'akinobu.mita@gmail.com'; 'linux-mm@kvack.org'; 'Joe Perches'; 'linux-arm- > kernel@lists.infradead.org' > Subject: Re: [RFC V6 2/3] arm:add bitrev.h file to support rbit instruction > > On Fri, Jan 09, 2015 at 10:16:32AM +0800, Wang, Yalin wrote: > > > -----Original Message----- > > > From: Russell King - ARM Linux [mailto:linux@arm.linux.org.uk] > > > Sent: Friday, January 09, 2015 2:41 AM > > > To: Wang, Yalin > > > Cc: 'Will Deacon'; 'Ard Biesheuvel'; 'linux-kernel@vger.kernel.org'; > > > 'akinobu.mita@gmail.com'; 'linux-mm@kvack.org'; 'Joe Perches'; > > > 'linux-arm- kernel@lists.infradead.org' > > > Subject: Re: [RFC V6 2/3] arm:add bitrev.h file to support rbit > > > instruction > > > > > > The root cause is that the kernel being built is supposed to support > > > both > > > ARMv7 and ARMv6K CPUs. However, "rbit" is only available on > > > ARMv6T2 (thumb2) and ARMv7, and not plain ARMv6 or ARMv6K CPUs. > > > > > In the patch that you applied: > > 8205/1 add bitrev.h file to support rbit instruction > > > > I have add : > > + select HAVE_ARCH_BITREVERSE if ((CPU_V7M || CPU_V7) && !CPU_V6) > > > > If you build kernel support ARMv6K, should CONFIG_CPU_V6=y, isn't it ? > > Then will not build hardware rbit instruction, isn't it ? > > The config has: > > CONFIG_CPU_PJ4=y > # CONFIG_CPU_V6 is not set > CONFIG_CPU_V6K=y > CONFIG_CPU_V7=y > CONFIG_CPU_32v6=y > CONFIG_CPU_32v6K=y > CONFIG_CPU_32v7=y > > And no, the CONFIG_CPU_V* flags refer to the CPUs. The > CONFIG_CPU_32v* symbols refer to the CPU architectures. > Oh, I see, How about change like this: + select HAVE_ARCH_BITREVERSE if ((CPU_V7M || CPU_V7) && !CPU_V6 && !CPU_V6K) I am not sure if I also need add some older CPU types like !CPU_ARM9TDMI &&?!CPU_ARM940T ? Another solution is: + select HAVE_ARCH_BITREVERSE if ((CPU_32V7M || CPU_32V7) && !CPU_32V6 && !CPU_32V5 && !CPU_32V4 && !CPU_32V4T && !CPU_32V3) By the way, I am not clear about the difference between CPU_V6 and CPU_V6K, could you tell me? :) Thank you
On Fri, Jan 09, 2015 at 08:40:56PM +0800, Wang, Yalin wrote: > Oh, I see, > How about change like this: > + select HAVE_ARCH_BITREVERSE if ((CPU_V7M || CPU_V7) && !CPU_V6 && !CPU_V6K) > I am not sure if I also need add some older CPU types like !CPU_ARM9TDMI &&?!CPU_ARM940T ? > > Another solution is: > + select HAVE_ARCH_BITREVERSE if ((CPU_32V7M || CPU_32V7) && !CPU_32V6 && !CPU_32V5 && !CPU_32V4 && !CPU_32V4T && !CPU_32V3) > > By the way, I am not clear about the difference between CPU_V6 and CPU_V6K, could you tell me? :) I think select HAVE_ARCH_BITREVERSE if (CPU_32v7M || CPU_32v7) && !CPU_32v6 is sufficient - we don't support mixing pre-v6 and v6+ CPU architectures into a single kernel.
> -----Original Message----- > From: Russell King - ARM Linux [mailto:linux@arm.linux.org.uk] > Sent: Thursday, January 15, 2015 12:38 AM > To: Wang, Yalin > Cc: 'Ard Biesheuvel'; 'Will Deacon'; 'linux-kernel@vger.kernel.org'; > 'akinobu.mita@gmail.com'; 'linux-mm@kvack.org'; 'Joe Perches'; 'linux-arm- > kernel@lists.infradead.org' > Subject: Re: [RFC V6 2/3] arm:add bitrev.h file to support rbit instruction > > On Fri, Jan 09, 2015 at 08:40:56PM +0800, Wang, Yalin wrote: > > Oh, I see, > > How about change like this: > > + select HAVE_ARCH_BITREVERSE if ((CPU_V7M || CPU_V7) && !CPU_V6 && > > +!CPU_V6K) > > I am not sure if I also need add some older CPU types like !CPU_ARM9TDMI > &&?!CPU_ARM940T ? > > > > Another solution is: > > + select HAVE_ARCH_BITREVERSE if ((CPU_32V7M || CPU_32V7) && !CPU_32V6 > > +&& !CPU_32V5 && !CPU_32V4 && !CPU_32V4T && !CPU_32V3) > > > > By the way, I am not clear about the difference between CPU_V6 and > > CPU_V6K, could you tell me? :) > > I think > > select HAVE_ARCH_BITREVERSE if (CPU_32v7M || CPU_32v7) && !CPU_32v6 > > is sufficient - we don't support mixing pre-v6 and v6+ CPU architectures > into a single kernel. > Ok, I will re-send a patch. Thanks
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 */
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 <yalin.wang@sonymobile.com> --- 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