Subject: Re: [PATCH 1/4] powerpc: consolidate feature fixup code

From: Olof Johansson <[email-masked]>
Date: Thu, 12 Oct 2006 09:56:49 -0500

On Fri, 13 Oct 2006 00:47:29 +1000 Benjamin Herrenschmidt <[email-masked]> wrote:

> 
> > > +void do_feature_fixups(unsigned long offset, unsigned long value,
> > > +		       void *fixup_start, void *fixup_end)
> > > +{
> > > +	struct fixup_entry {
> > > +		unsigned long	mask;
> > > +		unsigned long	value;
> > > +		unsigned int	*start;
> > > +		unsigned int	*end;
> > > +	} *fcur, *fend;
> > 
> > Shouldn't there be a better place to keep this struct definition than
> > in the function it's used? Some header file?
> 
> It's not used anywhere else.... which header would you put it in ?

Near where the asm-corresponding ones are located (cputable.h).

-Olof

> > Ok, now it's in C, no reason to do a dcbst/icbi for every word. We did
> > it in asm for simplicity's sake. Split it in two loops, one to nop,
> > second to step per line and do the flushes. :-)
> 
> Do we care ? :) But yah, I suppose I can do that.

Close call. :-) It's just easier to do now, might as well do it. But
yeah, no big deal.

It will also be easier to graft in a branch forward instead of a large
amount of nops for large feature sections. I guess it will take some
benchmarking to see what the breakeven point is.

-Olof