irq handling code consolidation, second try (ppc part)

Hi all,

attached patch is a second try of IRQ handling code consolidation. This is a ppc specific patch (compiled successfuly).

Beware, this patch removes some old(?) and crappy code:
- irq_kmalloc(), irq_kfree() removed. If ppc need to register irqs early, it should use setup_irq() as all decent people do :))
- request_irq() with NULL handler argument == free_irq(), does anyone use this kludge ?

Best regards.

Andrey Panin		| Embedded systems software developer
[email protected]	| PGP key: wwwkeys.pgp.net

diff --minimal -urN -X /usr/share/dontdiff linux-2.5.53.vanilla/arch/ppc/Kconfig linux-2.5.53/arch/ppc/Kconfig
@@ -10,6 +10,10 @@
 	bool
 	default y
 
+config GENERIC_IRQ
+	bool
+	default y
+
 config UID16
 	bool
 

diff --minimal -urN -X /usr/share/dontdiff linux-2.5.53.vanilla/include/asm-ppc/hw_irq.h linux-2.5.53/include/asm-ppc/hw_irq.h
@@ -5,6 +5,8 @@
 #ifndef _PPC_HW_IRQ_H
 #define _PPC_HW_IRQ_H
 
+//#include <linux/irq.h>
+
 extern void timer_interrupt(struct pt_regs *);
 extern void ppc_irq_dispatch_handler(struct pt_regs *regs, int irq);
 

@@ -71,6 +73,23 @@
 struct hw_interrupt_type;
 static inline void hw_resend_irq(struct hw_interrupt_type *h, unsigned int i) {}
 
+//extern irq_desc_t irq_desc[NR_IRQS];
+
+#define arch_ack_bad_irq(irq) do { } while (0)
+
+/* Return a pointer to the irq descriptor for IRQ.  */
+#define irq_desc(irq) (irq_desc + (irq))
+
+/* Check irq number */
+#define arch_check_irq(irq) ((irq) >= NR_IRQS)
+
+/* Arch specific hook for setup_irq() */
+#define arch_setup_irq(irq, desc, irqaction) do { } while (0)
+
+/* Used in setup_irq() */
+#define ARCH_NONSHARED_IRQ_MASK ~(IRQ_DISABLED | IRQ_AUTODETECT | IRQ_WAITING)
+
+#define HAVE_ARCH_IRQ_PROBE
 
 #endif /* _PPC_HW_IRQ_H */
 #endif /* __KERNEL__ */

After a quick look, things look fine some of this old PPC crap should indeed be killed now.

There's something else I would like to implement one of these days, and your consolidation work makes it easier (except for archs that won't use it...). Basically, I want some (few and rare enough though) drivers to be able to define interrupt controllers and thus create their own IRQs.

The typical example for which I need that is cardbus. I need the cardbus controller on PCI to be seen as a real cascaded controller, that is I want devices below it (either PCI or PCMCIA) calls to request/free/enable/disable_irq for their to be caught by the cardbus driver.
This is the only sane way to properly enable/disable bridging of the interrupt upon request from the driver. Without that, I still can frequent lockups with various pcmcia cards when inserted/removed due to IRQ line beeing stuck down on slot shutdown or other crappy things happening typically with legacy PCMCIA stuffs (ATA is a good example of crappy behaviour).

The "easy" way here to implement that is to make the irq_desc array larger than NR_IRQs (or rather split NR_IRQs into NR_SYS_IRQS + NR_DYNAMIC_IRQS). The additional "slots" could then easily be allocated/freed. Thus, keeping my cardbus example, the cardbus driver can allocate a couple of slots (that is IRQ
numbers) dynamically, and use it's own startup/shutdown/enable/disable/...
hooks for them, dealing itself with the cascade from the PCI irq. (Of course, this is not for systems using cardbus routed to legacy IRQs, but for systems like PPC where the PCI irq is mux'ing both the device IRQs and the controller own IRQ).

Ben.

Linearizing a space like this is always a bad idea, I think.

It would be entirely possible to just add the irq routing information to the "struct device" tree, and have a "dev_request_irq(dev, ...)", along with a few helper functions like "pci_request_irq(pci_dev, ...)".

And the old "request_irq()" would just use the system root device as the device.

Linus

I fully agree, I was just proposing a "simple" solution that would fit in a feature-frozen kernel ;)

Note that if we go the full way abstracting interrupts, then the interrupt "tree" should be separate from the device tree. The interrupt "parent" of a device may not be (and is not in a whole lot of cases I have to deal with on pmacs and embedded) the "bus" parent of a given device.

I suppose there may be similar "issues" with x86 PCI chips routed to legacy irqs, but then, I'm completely unfamiliar with the story of interrupt routing on x86's...

on pmacs, all interrupts end up in a single interrupt controller located in a "combo" ASIC along with other devices. This ASIC is a PCI device, but is really the root of the interrupt tree just below the CPU itself, regardless of the actual interrupt sources beeing located on other PCI busses or not. On embedded, the design can be as funky as a HW designer can imagine...

Do you think this is still 2.5 work ?

Benjamin Herrenschmidt <[email protected]>

I disagree. The pmac braindamage is a pmac problem, and not worth uglifying the generic device layer over. Besides, as far as I know, it is trivially solved by just making the pmac irq controller be a root controller, and that's it. There are no other irq controllers there that are worth worrying about.

No.

Linus

Right, though some other machines (CHRP for example) with cascaded legacy 8259 on top or below OpenPIC may also want more flexibility regarding interrupt routing.

It seems quite common (at least it is in embedded world) to actually wire interrupt sources rather randomly to the closest device than can act as an interrupt controller regardless of the actual bus layout ;)

But I agree this can be solved by defining the "main" PIC as root controller regardless of it's actual bus location.

Makes sense. Though the simple tweak of allocating more irq_descs in the existing array (by slightly extending the array) may be worth trying for fixing some of the pcmcia problems now. I need to experient more, it might actually be enough to just disable IRQ routing on the pcmcia bridge when the slot is shut down.

Ben.

On ppc64 irqs are sparse and can be very large (> 1000) so we have a mapping between them and slots in irq_desc[NR_IRQS]. I wonder how hard it would be to kill irq_desc and just dynamically allocate irq_desc stuff. That would solve my problem.

Anton

Similar issues on ppc64 (since it started life as a copy of the ppc32 code :) Im interested in moving to something generic.

Anton