Messages in this thread Patch in this message | | | Date | Mon, 2 Feb 2015 15:18:24 -0800 | From | Andrew Morton <> | Subject | Re: [PATCH linux-next] lib: Kconfig: use bool instead of boolean |
| |
On Mon, 2 Feb 2015 23:05:48 +0000 Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> On Mon, Feb 02, 2015 at 02:27:32PM -0800, Andrew Morton wrote: > > On Mon, 2 Feb 2015 09:59:16 -0500 Christoph Jaeger <cj@linux.com> wrote: > > > > > Keyword 'boolean' for type definition attributes is considered > > > deprecated and, therefore, should not be used anymore. > > > > > > See http://lkml.kernel.org/r/cover.1418003065.git.cj@linux.com > > > See http://lkml.kernel.org/r/1419108071-11607-1-git-send-email-cj@linux.com > > > > > > ... > > > > > > --- a/lib/Kconfig > > > +++ b/lib/Kconfig > > > @@ -14,7 +14,7 @@ config BITREVERSE > > > tristate > > > > > > config HAVE_ARCH_BITREVERSE > > > - boolean > > > + bool > > > default n > > > depends on BITREVERSE > > > help > > > > Your patch patches 556d2f055bf6d ("ARM: 8187/1: add > > CONFIG_HAVE_ARCH_BITREVERSE to support rbit instruction") which appears > > in linux-next via the ARM tree. > > > > There are many uses of "boolean" in lib/Kconfig. Converting just one > > of them is inefficient and odd. > > > > 556d2f055bf6d is a bit of a surprise. It looks good to me from a > > non-ARM perspective - the __builtin_constant_p() optimisation is > > sensible, although bitrev on a constant probably isn't very common. > > > > I'm not sure about the ARM part though! __bitrev8() is pretty damn > > fast. Presumably an inlined rbit instruction is faster still, but not > > very much? > > > > The Kconfig help text in 556d2f055bf6d rather needs some caring for. > > The patches had already been round six iterations, and had been posted > on LKML for every iteration.
People saw "ARM" and went Zzzzz ;)
I'm a bit surprised that nobody helped out with the Kconfig text. I queued the below. Looks OK?
--- a/lib/Kconfig~a +++ a/lib/Kconfig @@ -18,9 +18,8 @@ config HAVE_ARCH_BITREVERSE 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. + This option enables the use of hardware bit-reversal instructions on + architectures which support such operations. config RATIONAL bool _
> When people started pushing to have the patches merged, there were two > dependent patches which had been merged via other random trees, so I > held them off for a cycle. At that point, I even questioned whether I > should be merging them; that question was ignored by everyone. > > Eventually, (and after some testing) I ended up giving up and merging > them because they're believed to be a net benefit for ARM,
I know the feeling ;)
> and I > couldn't locate anyone who'd be useful to ack the generic parts of the > patch.
I usually look after lib/ and hereby ack the patch! I really should do a MAINTAINERS patch but I'm shy.
> As for whether __bitrev8 is fast or not, that depends whether the table > has been speculatively prefetched and is available without having to go > out to memory - and it's not just about the table itself, there's also > the loading from the individual function's literal pool to get the > address of the table too. So that's two memory loads per rbit at > minimum. > > The rbit instruction is probably at least half the average cycles of > two dependent loads.
OK.
| |