On Thu, 5 May 2011, Benjamin Herrenschmidt wrote:

quoted

As for the mapping, I agree that the functionality is generally
useful, I'm just not fond of the current implementation.  I think it
is more complex than it needs to be and I'm not excited about bring it
over to the other architectures as-is.

Nobody cares about the current implementation. What is important is
indeed the functionality. The basic thing I think everybody agrees is
that you need to extend the irq_desc (or data, whatever tglx prefers)
with two bits of information: Some identifier of the domain and some
identifier of the interrupt number within that domain.

irq_data because that's what is handed into the callbacks and you
probably want to have the HW number there.
 

In addition, PIC code will need a way to perform efficient reverse
mapping. You may decide that for simple IRQ controllers that handle a
small linear range of interrupts, it's kosher to simply reserve a linear
range of descriptors and use a simple offset, I'm find with that now
that we no longer live in a world constrained by NR_IRQ.

But the need for the radix tree remains for things that have massively
large potential HW numbers such as we have on powerpc.

quoted

For the majority of fixed hw interrupt controllers it is overkill.
There is no need for a map table when all the irq descs (<=32 of them)
get allocated when the irq controller is instantiated and the mapping
consists of 'virq = hw_irq + virq_base'.  For instance, with the arm
irq controllers, it's be more than sufficient to use irq_alloc_descs
to obtain a range of irq numbers and then a simple of_irq_domain
registration to handle the parsing.

That's true if and only if you make NR_IRQ a non issue and if you accept
the general wastage due to unused interrupts.

The main problem has always been that hard limit which made me chose a
more efficient mechanisms. Take a mac with 2 cascaded MPICs with 256
sources each which are mostly never used. I would need an NR_IRQs of 512
with your scheme (plus 16 because I do want to continue avoiding the
"ISA" numbers), which is a waste of space, even with SPARSE_IRQ.

Now I hope eventually NR_IRQ will go away and we'll have a more
efficient mechanism to allocate descriptors and so it will become less
of an issue.

Well, NR_IRQS in the sparse case is irrelevant already and I'm looking
into a way to remove the !SPARSE code completely.

One thing we can do to avoid allocating 512 irq descriptors for the
MAC case is to reserve the space and only allocate the descriptors you
really need to be operational.

quoted

For the cases where an interrupt controller isn't able to alloc all
the irq descs at once, like for MSI, then yes the LINEAR and RADIX
mappings are important.  What bothers me though is the way irq_host
needs to use unions and the ->revmap_type value to implement multiple
behaviours into a single type.  That's the sort of thing that can be
broken out into a library and instantiated only by the interrupt
controllers that actually need it.

But that's what it is really. You'll notice that on the fast path the
interrupt controller code calls directly into the "right" type of revmap
routine. You may want to refactor things a bit if you want, but the
union served me well simply because I didnt have to bother doing lots of
different alloc_bootmem back then. Nowadays, kmalloc is available much
earlier so it might have become a non issue too.

quoted

  Similarly, it bothers me that that
radix mapping makes up a significant portion of the code, yet it has
only one user. 

"significant" ? Seriously ? Like 3 function calls ? It's nothing. We use
an existing radix tree facility, and the amount of code in our irq.c is
actually very small.

Originally it was living in xics in fact, but I moved it out
specifically because I wanted a common interface to remapping, so for
example, I can expose the linux -> hw mapping in debugfs generically,
among others.

And there is another reverse mapping implementation in the superH
code.

quoted

 I'd be happier if each kind of mapping had its own
structure that embedded a generic irq_host/irq_domain with mapping
specific ops populated to manipulate it.

Whatever, that's just plumbing, I don't care either way, that doesn't
change the fact that the concept of domain doesn't have much to do with
OF :-)

quoted

Regardless, the immediate priority is to implement a mapper that will
work for all architectures (or at least everything but powerpc and
sparc).

I oppose the implementation of a new mapper that doesn't include
powerpc, that would be stupid. Either re-use ours or implement a new one
that encompass our needs. I can't talk for sparc but I wouldn't be
surprised if David thought about the same lines.

quoted

  x86 has already implemented a skeleton irq_domain because
there wasn't any common code for it to use.  ARM also needs the
functionality immediately, and I don't want to see yet another
arch-specific implementation.  I'd like to get the framework in place
now, and grafting in mapping features as follow on patches.  That way
the new DT users aren't blocked while waiting for us to hammer down
the features that the other architectures don't need yet.

But the mapping code exist already. It returns a device-node as the
interrupt controller. What you then need is to establish the
relationship between that device-node and a domain in linux, and have
code to decode the content of the interrupts property.

So you need the concept of domain first, generic, and -then- you can
start adding a way to bind it to OF.

Ack.
 
Thanks,

	tglx