Subject: Re: checkpatch nits ...

From: Wolfram Sang <[email-masked]>
Date: Wed, 27 Aug 2008 09:49:44 +0200

On Sat, Aug 23, 2008 at 10:57:21AM +0200, Arnd Bergmann wrote:

> On Saturday 23 August 2008, Kevin Diggs wrote:
> > WARNING: externs should be avoided in .c files
> > #1137: FILE: powerpc/kernel/cpu/pll_if.c:369:
> > +       __asm__ __volatile__ (
> > 
> > ??? I don't know what this is?
> > 
> > The entire block is:
> > 
> >         __asm__ __volatile__ (
> >                 "addi %0,%3,-1\n"
> >                 "andc %1,%3,%0\n"
> >                 "cntlzw %1,%1\n"
> >                 "subfic %1,%1,31\n"
> >                 "cntlzw %0,%2\n":
> >                 "=r"(cntlz), "=r"(cnttz):
> >                 "r"(tmp), "b"(cnttz)
> >         );
> > 
> 
> It's a bug in checkpatch, your code is correct (although I would write
> asm volatile, not __asm__ __volatile__, and add \t after each \n).
> Can you explain why you need that inline assembly? All you do in there
> are arithmetic operations, so you should be able to express that using
> C, or at least using the helpers we already have.
> 
> Checkpatch thinks that what you wrote is a declaration for a function
> named __volatile__ returning a variable of type __asm__, and complains
> that this declaration belongs into a header file.

I wonder if checkpatch-maintainers read this list, so I put Andy
Whitcroft to CC.

   Wolfram

-- 
  Dipl.-Ing. Wolfram Sang | http://www.pengutronix.de
 Pengutronix - Linux Solutions for Science and Industry