Re: [PATCH 0/6] General device tree irq domain infrastructure
On Thu, 5 May 2011, Benjamin Herrenschmidt wrote: 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. 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. 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. 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. 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 :-) 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. 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
Re: [PATCH 0/6] General device tree irq domain infrastructure
On Thu, May 5, 2011 at 2:37 AM, Thomas Gleixner t...@linutronix.de wrote: On Thu, 5 May 2011, Benjamin Herrenschmidt wrote: 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. Okay, I'll take another hack at it. Unfortunately I've got a great big unmaskable interrupt in the form of UDS next week, but I'll be back on it the week after. g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 0/6] General device tree irq domain infrastructure
I completely agree that irq domains are a generically useful feature for architectures, and it should be made available. I also completely agree that it is orthogonal to device tree translations, which in a large part is why I've structured this series and the new code the way I have. In this series I'm specifically addressing device tree translation, which is the one bit that is DT specific, and is important regardless of the backend translation mechanism. In fact, the more I looked at it, the more it seems that the DT api really is orthogonal to the backend irq mapping and I don't think that the way irq_host ties them together is necessarily the best way to do it. Many of the interrupt controllers which need to gain dt irq parsing have already been converted to using the irq_alloc_desc*() apis and have all the mapping mechanism they need, but lack a method of exposing it for DT translation. But irq_alloc_desc() is purely about allocating the linux side irq descriptors. Nothing to do with HW numbers here. You still need a mapping mechanism, regardless of the device-tree, to map those linux number to your HW numbers. This is simple and eventually even 1:1 on things like x86, but the minute you start having cascaded IRQ controllers, multiple IRQ domains, and/or very large HW numbers that encode node IDs etc... this falls appart. And this is still completely orthogonal to the device-tree. Mapping is the important functionality. Whatever the actual allocator is is indeed irrelevant. 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. 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. 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. 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. 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
Re: [PATCH 0/6] General device tree irq domain infrastructure
On Tue, May 03, 2011 at 11:43:19AM +1000, Benjamin Herrenschmidt wrote: On Thu, 2011-04-28 at 14:01 -0600, Grant Likely wrote: A lot of this series ends up being fixups to powerpc code; but the 4th patch is of importance to every architecture using CONFIG_OF (except SPARC, which has its own solution). This series (finally!) factors out device tree irq domain decoding from arch/powerpc and makes it generic for all architectures. The infrastructure is quite simple. Any interrupt controller can embed the of_irq_domain structure and register it with the core code. After that, device nodes referencing interrupts on that device tree node will trigger a call to the domain's map function. This leads to an immediate reaction from me : why of_irq_domain ? The concept of interrupt domains is completely orthogonal to OF and whether you use the device-tree or not. Having a remapping mechanism allowing to deal with multiple interrupt domains without playing stupid numbering tricks is generally useful for architectures, regardless of their adoption of the device-tree. The irq domain has one and -only one- op that is related to OF which allows to map a device node to a domain, but that's 'optional' and only use if you use the OF resolving process. The whole mechanism can be (and is under some circumstances on ppc) without a device-tree relationship. We instanciate IRQs within domains manually in some cases, either because we lack proper DT informations or bcs the IRQs come from the firmware or as created out of the blue (device-tree). A domain pointer (or NULL for the default domain) is all is needed. Thus I object to tying this infrastructure generically to OF even if it's just a mater of naming of the domain structure and location of the code in the kernel tree. It should basically all go into kernel/irq/domains.c or something like that. I completely agree that irq domains are a generically useful feature for architectures, and it should be made available. I also completely agree that it is orthogonal to device tree translations, which in a large part is why I've structured this series and the new code the way I have. In this series I'm specifically addressing device tree translation, which is the one bit that is DT specific, and is important regardless of the backend translation mechanism. In fact, the more I looked at it, the more it seems that the DT api really is orthogonal to the backend irq mapping and I don't think that the way irq_host ties them together is necessarily the best way to do it. Many of the interrupt controllers which need to gain dt irq parsing have already been converted to using the irq_alloc_desc*() apis and have all the mapping mechanism they need, but lack a method of exposing it for DT translation. 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. 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. 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. Similarly, it bothers me that that radix mapping makes up a significant portion of the code, yet it has only one user. 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. Regardless, the immediate priority is to implement a mapper that will work for all architectures (or at least everything but powerpc and sparc). 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. What I /could/ have done I suppose was called it 'struct irq_domain' as you suggest, and allowed each translation mechanism to define its own match/map pair of
Re: [PATCH 0/6] General device tree irq domain infrastructure
On Thu, 2011-04-28 at 14:01 -0600, Grant Likely wrote:
A lot of this series ends up being fixups to powerpc code; but the 4th
patch is of importance to every architecture using CONFIG_OF (except
SPARC, which has its own solution).
This series (finally!) factors out device tree irq domain decoding
from arch/powerpc and makes it generic for all architectures. The
infrastructure is quite simple. Any interrupt controller can embed
the of_irq_domain structure and register it with the core code. After
that, device nodes referencing interrupts on that device tree node
will trigger a call to the domain's map function.
This leads to an immediate reaction from me : why of_irq_domain ?
The concept of interrupt domains is completely orthogonal to OF and
whether you use the device-tree or not.
Having a remapping mechanism allowing to deal with multiple interrupt
domains without playing stupid numbering tricks is generally useful for
architectures, regardless of their adoption of the device-tree.
The irq domain has one and -only one- op that is related to OF which
allows to map a device node to a domain, but that's 'optional' and only
use if you use the OF resolving process. The whole mechanism can be (and
is under some circumstances on ppc) without a device-tree relationship.
We instanciate IRQs within domains manually in some cases, either
because we lack proper DT informations or bcs the IRQs come from the
firmware or as created out of the blue (device-tree). A domain pointer
(or NULL for the default domain) is all is needed.
Thus I object to tying this infrastructure generically to OF even if
it's just a mater of naming of the domain structure and location of the
code in the kernel tree.
It should basically all go into kernel/irq/domains.c or something like
that.
Cheers,
Ben.
PowerPC and x86 have been converted to use of_irq_domain. MIPS and
Microblaze have it enabled, but nothing actually registers domains
yet, so a workaround is in place to preserve the current behaviour
until it is fixed.
I'd really like to get patches 1-4 merged into 2.6.40. Please test.
I'm also running through build testing here, and when it's complete
I'll push it out to a 'devicetree/irq-domain' branch on
git://git.secretlab.ca/git/linux-2.6
It needs testing. I've booted it on a powerpc board here without any
apparent regressions, but that isn't a very big sample. I've also
build tested on everything I think is affected.
I'd also like to get it into linux-next. Ben, if things checkout okay
over the next few days, would you be okay with me adding it to
linux-next, say around Wednesday next week? As for merging, I think
this should probably go via your powerpc tree since the that's where
the bulk of the changes are, but I'm open to other suggestions).
Patches 5 6 are follow-on cleanup work to powerpc, but patch 6 is
RFC only since there is a locking problem that I haven't fixed yet.
Cheers,
g.
---
Grant Likely (6):
powerpc: stop exporting irq_map
powerpc: make irq_{alloc,free}_virt private and remove count argument
powerpc: Make struct irq_host semi-private by moving into irqhost.h
dt: generalize of_irq_parse_and_map()
powerpc: move irq_alloc_descs_at() call into irq_alloc_virt()
powerpc: use irq_alloc_desc() to manage irq allocations
arch/microblaze/kernel/irq.c |7 -
arch/microblaze/kernel/setup.c |2
arch/mips/kernel/prom.c | 14 -
arch/powerpc/include/asm/irq.h | 88 +--
arch/powerpc/include/asm/irqhost.h | 27 ++
arch/powerpc/kernel/irq.c| 260
--
arch/powerpc/platforms/512x/mpc5121_ads_cpld.c |5
arch/powerpc/platforms/52xx/media5200.c |5
arch/powerpc/platforms/52xx/mpc52xx_gpt.c|1
arch/powerpc/platforms/52xx/mpc52xx_pic.c| 80 +--
arch/powerpc/platforms/82xx/pq2ads-pci-pic.c |5
arch/powerpc/platforms/85xx/socrates_fpga_pic.c | 26 +-
arch/powerpc/platforms/86xx/gef_pic.c| 10 -
arch/powerpc/platforms/8xx/m8xx_setup.c |2
arch/powerpc/platforms/cell/axon_msi.c | 15 +
arch/powerpc/platforms/cell/spider-pic.c | 19 +-
arch/powerpc/platforms/embedded6xx/flipper-pic.c |9 -
arch/powerpc/platforms/embedded6xx/hlwd-pic.c|9 -
arch/powerpc/platforms/embedded6xx/wii.c |6 -
arch/powerpc/platforms/iseries/irq.c | 10 -
arch/powerpc/platforms/powermac/pic.c| 12 +
arch/powerpc/platforms/pseries/ras.c |4
arch/powerpc/platforms/pseries/xics.c| 14 +
arch/powerpc/sysdev/cpm1.c |8 -
arch/powerpc/sysdev/cpm2_pic.c | 10 -
arch/powerpc/sysdev/fsl_msi.c|3
arch/powerpc/sysdev/i8259.c
Re: [PATCH 0/6] General device tree irq domain infrastructure
Grant Likely wrote: I'd really like to get patches 1-4 merged into 2.6.40. Please test. I'm also running through build testing here, and when it's complete I'll push it out to a 'devicetree/irq-domain' branch on git://git.secretlab.ca/git/linux-2.6 I pulled this, built and booted my x86-dt box and nothing exploded so far. If you merge Linus' tree then you will get a conflict (sooner or later) in ioapic_of_irq_map() in arch/x86/kernel/devicetree.c: - return io_apic_setup_irq_pin_once(*out_hwirq, cpu_to_node(0), attr); - if (io_apic_setup_irq_pin(hwirq, cpu_to_node(0), attr)) ++ if (io_apic_setup_irq_pin_once(hwirq, cpu_to_node(0), attr)) Sebastian ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 0/6] General device tree irq domain infrastructure
On Fri, Apr 29, 2011 at 06:16:33PM +0200, Sebastian Andrzej Siewior wrote: Grant Likely wrote: I'd really like to get patches 1-4 merged into 2.6.40. Please test. I'm also running through build testing here, and when it's complete I'll push it out to a 'devicetree/irq-domain' branch on git://git.secretlab.ca/git/linux-2.6 I pulled this, built and booted my x86-dt box and nothing exploded so far. If you merge Linus' tree then you will get a conflict (sooner or later) in ioapic_of_irq_map() in arch/x86/kernel/devicetree.c: - return io_apic_setup_irq_pin_once(*out_hwirq, cpu_to_node(0), attr); - if (io_apic_setup_irq_pin(hwirq, cpu_to_node(0), attr)) ++ if (io_apic_setup_irq_pin_once(hwirq, cpu_to_node(0), attr)) Sebastian Thanks Sebastian. Unless you say otherwise, I'll take that as an Acked-by. :-) g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 0/6] General device tree irq domain infrastructure
A lot of this series ends up being fixups to powerpc code; but the 4th
patch is of importance to every architecture using CONFIG_OF (except
SPARC, which has its own solution).
This series (finally!) factors out device tree irq domain decoding
from arch/powerpc and makes it generic for all architectures. The
infrastructure is quite simple. Any interrupt controller can embed
the of_irq_domain structure and register it with the core code. After
that, device nodes referencing interrupts on that device tree node
will trigger a call to the domain's map function.
PowerPC and x86 have been converted to use of_irq_domain. MIPS and
Microblaze have it enabled, but nothing actually registers domains
yet, so a workaround is in place to preserve the current behaviour
until it is fixed.
I'd really like to get patches 1-4 merged into 2.6.40. Please test.
I'm also running through build testing here, and when it's complete
I'll push it out to a 'devicetree/irq-domain' branch on
git://git.secretlab.ca/git/linux-2.6
It needs testing. I've booted it on a powerpc board here without any
apparent regressions, but that isn't a very big sample. I've also
build tested on everything I think is affected.
I'd also like to get it into linux-next. Ben, if things checkout okay
over the next few days, would you be okay with me adding it to
linux-next, say around Wednesday next week? As for merging, I think
this should probably go via your powerpc tree since the that's where
the bulk of the changes are, but I'm open to other suggestions).
Patches 5 6 are follow-on cleanup work to powerpc, but patch 6 is
RFC only since there is a locking problem that I haven't fixed yet.
Cheers,
g.
---
Grant Likely (6):
powerpc: stop exporting irq_map
powerpc: make irq_{alloc,free}_virt private and remove count argument
powerpc: Make struct irq_host semi-private by moving into irqhost.h
dt: generalize of_irq_parse_and_map()
powerpc: move irq_alloc_descs_at() call into irq_alloc_virt()
powerpc: use irq_alloc_desc() to manage irq allocations
arch/microblaze/kernel/irq.c |7 -
arch/microblaze/kernel/setup.c |2
arch/mips/kernel/prom.c | 14 -
arch/powerpc/include/asm/irq.h | 88 +--
arch/powerpc/include/asm/irqhost.h | 27 ++
arch/powerpc/kernel/irq.c| 260 --
arch/powerpc/platforms/512x/mpc5121_ads_cpld.c |5
arch/powerpc/platforms/52xx/media5200.c |5
arch/powerpc/platforms/52xx/mpc52xx_gpt.c|1
arch/powerpc/platforms/52xx/mpc52xx_pic.c| 80 +--
arch/powerpc/platforms/82xx/pq2ads-pci-pic.c |5
arch/powerpc/platforms/85xx/socrates_fpga_pic.c | 26 +-
arch/powerpc/platforms/86xx/gef_pic.c| 10 -
arch/powerpc/platforms/8xx/m8xx_setup.c |2
arch/powerpc/platforms/cell/axon_msi.c | 15 +
arch/powerpc/platforms/cell/spider-pic.c | 19 +-
arch/powerpc/platforms/embedded6xx/flipper-pic.c |9 -
arch/powerpc/platforms/embedded6xx/hlwd-pic.c|9 -
arch/powerpc/platforms/embedded6xx/wii.c |6 -
arch/powerpc/platforms/iseries/irq.c | 10 -
arch/powerpc/platforms/powermac/pic.c| 12 +
arch/powerpc/platforms/pseries/ras.c |4
arch/powerpc/platforms/pseries/xics.c| 14 +
arch/powerpc/sysdev/cpm1.c |8 -
arch/powerpc/sysdev/cpm2_pic.c | 10 -
arch/powerpc/sysdev/fsl_msi.c|3
arch/powerpc/sysdev/i8259.c |3
arch/powerpc/sysdev/ipic.c | 19 +-
arch/powerpc/sysdev/mpc8xx_pic.c | 10 -
arch/powerpc/sysdev/mpc8xxx_gpio.c | 13 +
arch/powerpc/sysdev/mpic.c | 33 +--
arch/powerpc/sysdev/mpic_msi.c |3
arch/powerpc/sysdev/mpic_pasemi_msi.c|5
arch/powerpc/sysdev/mv64x60_pic.c| 14 +
arch/powerpc/sysdev/qe_lib/qe_ic.c |9 -
arch/powerpc/sysdev/uic.c| 13 +
arch/powerpc/sysdev/xilinx_intc.c|9 -
arch/x86/include/asm/irq_controller.h| 12 -
arch/x86/include/asm/prom.h |1
arch/x86/kernel/devicetree.c | 77 +--
drivers/of/irq.c | 118 ++
include/linux/of_irq.h | 31 +++
42 files changed, 504 insertions(+), 517 deletions(-)
create mode 100644 arch/powerpc/include/asm/irqhost.h
delete mode 100644 arch/x86/include/asm/irq_controller.h
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
