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

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

> > +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 ?

> > +
> > +	fcur = fixup_start;
> > +	fend = fixup_end;
> > +
> > +	for (; fcur < fend; fcur++) {
> 
> 	for (fcur = fixup_start; fcur < fend; fcur++) {
> 
> > +		unsigned int *pcur, *pend;
> > +
> > +		if ((value & fcur->mask) == fcur->value)
> > +			continue;
> > +
> > +		pcur = fcur->start - offset;
> > +		pend = fcur->end - offset;
> > +		for (; pcur < pend; pcur++) {
> > +			*pcur = 0x60000000u;
> > +			asm ("dcbst 0, %0; sync; icbi 0,%0; sync; isync"
> > +			     : : "r" (pcur));
> > +		}
> 
> 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.

Thanks,
Ben.