[Cbe-oss-dev] arch/powerpc/platforms/cell/pmu.c fix

Michael Ellerman michael at ellerman.id.au
Tue Mar 18 12:44:34 EST 2008


On Tue, 2008-03-18 at 02:16 +0100, Arnd Bergmann wrote:
> On Monday 17 March 2008, Geoff Levand wrote:
> > On 03/17/2008 09:51 AM, Denis Joseph Barrow wrote:
> > > Hi,
> > > Just putting a bit of lipstick on no code functionality changes.
> > > 
> > > Subject: Cell Performance Measurment Unit
> > > WRITE_WO_MMIO(reg, x) macro etc. is using a cpu in the macro
> > > never defined in the parameter list, this is bad coding style.
> > > I used do it myself & other programmers got upset about it so I stopped.
> 
> Right, thanks for addressing this, I think nobody ever noticed it
> in earlier reviews.
> 
> > > Signed-off-by: Denis Joseph Barrow <denis.barrow at sonycom.com>
> > > ---
> > >  arch/powerpc/platforms/cell/pmu.c |   50 +++++++++++++++++++-------------------
> > >  1 file changed, 25 insertions(+), 25 deletions(-)
> > > 
> > > --- a/arch/powerpc/platforms/cell/pmu.c
> > > +++ b/arch/powerpc/platforms/cell/pmu.c
> > > @@ -40,7 +40,7 @@
> > >   * pmd_regs.
> > >   */
> > >  
> > > -#define WRITE_WO_MMIO(reg, x)                                        \
> > > +#define WRITE_WO_MMIO(cpu, reg, x)                           \
> > 
> > These should be static inline functions as to get proper
> > scope and type checking.
> 
> Unfortunately, this doesn't work, because the macros depend on using the
> reg argument as the name of a member in two different data structures.
> A static inline function would certainly be preferrable, but I can't
> see how we could do that here. If nobody comes up with a better idea,
> I'd just take DJ's patch as is.

You _could_ have a static inline that does the actual assign to the reg
and the shadow, and then have a macro that does the reg name pasting and
passes pointers to the reg and shadow through to the static inline.

But it's more trouble than it's worth IMHO.

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
URL: <http://lists.ozlabs.org/pipermail/cbe-oss-dev/attachments/20080318/b6a6615d/attachment.pgp>


More information about the cbe-oss-dev mailing list