Re: [PATCH 5/5] of/irq: merge of_irq_map_one()

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

On Thu, 2010-06-17 at 17:11 -0600, Grant Likely wrote:
> On Thu, Jun 10, 2010 at 7:17 PM, Benjamin Herrenschmidt
> <benh@kernel.crashing.org> wrote:
> > On Thu, 2010-06-10 at 17:36 -0600, Grant Likely wrote:
> >>
> >> Okay.  I had been trying to avoid #ifdefs in the common code, but
> >> you're probably right.  I'll rework.
> >
> > Not even ifdef's ... just move the quirk map there. You can always
> > #define the quirk variable to 0 on archs that have no quirks, to
> > make it compile away if you believe it represents bloat, but they
> > are simple well localized things so I doubt it matters.
> 
> They're pretty small, but powermac32 is the only code that actually
> uses the quirk facility.  Everything else parses sanely it would
> appear.  I'd rather have them localized to the powermac code and
> eliminate the quirks from the common code.

Maybe, but the way you end up gutting out some internal functions of the
parser so they can be overriden by the arch is just plain gross :-) And
more bloat than just having a localized quirk test which can easily
compile to nothing when powermac isn't built.

IE. I'd rather you then add some kind of of_irq_map_fixup() or something
like that in the generic code that can be an empty inline when powermac
isn't there, and be defined by powerpc to do the right thing then,
rather than move the core of the function to a separate weak version.

Because the day you fix a bug in the weak variant, or change a calling
convention, you'll forget to also update the "overrides" in the arch.

> If other platforms show up with bad irq mappings, then I want to take
> the approach of fixing the data rather than adding more quirks cases.
> 
> Anyway, let me try my hand at reworking to be a lot clearer and see
> what it looks like.

Ben.