search

raspberrypi / linux

Kernel source tree for Raspberry Pi-provided kernel builds. Issues unrelated to the linux kernel should be posted on the community forum at https://forums.raspberrypi.com/
Other
11.03k stars 4.96k forks source link

irq_alloc_descs() always fails on rpi #743

Closed chaosjug closed 9 years ago

chaosjug commented 9 years ago

All modules trying to use irq_alloc_descs() fail on rpi because all irqs are allocated in arch/arm/mach-bmc2708/armctrl.c in armctrl_init() This e.g. prevents usage of IIO drivers with external trigger.

The problem is, that irq_set_chip() which in turn calls irq_reserve_irq() is called for all irqs (0 to NR_IRQS)

This fixes the issue for me, but as I am not sure what the spare irqs are for, I don't know if this breaks something else:

diff --git a/arch/arm/mach-bcm2708/armctrl.c b/arch/arm/mach-bcm2708/armctrl.c
index 42f5e1c..a4bea99 100644
--- a/arch/arm/mach-bcm2708/armctrl.c
+++ b/arch/arm/mach-bcm2708/armctrl.c
@@ -298,7 +298,7 @@ int __init armctrl_init(void __iomem * base, unsigned int 
irq_start,
 {
        unsigned int irq;

-       for (irq = 0; irq < NR_IRQS; irq++) {
+       for (irq = 0; irq < (NR_IRQS-SPARE_IRQS); irq++) {
                unsigned int data = irq;
                if (irq >= INTERRUPT_JPEG && irq <= INTERRUPT_ARASANSDIO)
                        data = remap_irqs[irq - INTERRUPT_JPEG];
notro commented 9 years ago

SPARE_IRQS was added so that drivers/input/touchscreen/stmpe-ts.c could request an irq. The pitft display by Adafruit uses this touch controller. PR: https://github.com/raspberrypi/linux/pull/568

popcornmix commented 9 years ago

Does it make sense to initialise only some of the SPARE IRQS? e.g.

for (irq = 0; irq < NR_IRQS-SPARE_IRQS/2; irq++) {
pelwell commented 9 years ago

Is it sensible that there are two interrupt allocation mechanisms in competition?

notro commented 9 years ago

I would guess that a driver acting as an interrupt controller would need to allocate irq descriptors.

These are the two allocation call chains, which end up doing the same to mark the descriptor in use (we don't have CONFIG_SPARSE_IRQ set, right?): irq_set_chip->irq_reserve_irq->irq_reserve_irqs:

                bitmap_set(allocated_irqs, start, cnt);

irq_alloc_descs->__irq_alloc_descs:

        bitmap_set(allocated_irqs, start, cnt);

@popcornmix AFAICT this shouldn't break stmpe-ts. It looks a bit hackish though. Maybe something like this to make it more descriptive?

arch/arm/mach-bcm2708/include/mach/irqs.h

#define SPARE_ALLOC_IRQS      32
#define BCM2708_ALLOC_IRQS    (HARD_IRQS+FIQ_IRQS+GPIO_IRQS+SPARE_ALLOC_IRQS)
#define FREE_IRQS             32
#define NR_IRQS               (BCM2708_ALLOC_IRQS+FREE_IRQS)

arch/arm/mach-bcm2708/armctrl.c

    for (irq = 0; irq < BCM2708_ALLOC_IRQS; irq++) {
notro commented 9 years ago

Some months ago I tried to get the stmpe touch controller to work with Device Tree, but gave up after a few rounds. With the help of this issue I succeeded using this patch:

diff --git a/arch/arm/mach-bcm2708/armctrl.c b/arch/arm/mach-bcm2708/armctrl.c
index 42f5e1c..5e2f9a6 100644
--- a/arch/arm/mach-bcm2708/armctrl.c
+++ b/arch/arm/mach-bcm2708/armctrl.c
@@ -167,7 +167,8 @@ void __init armctrl_dt_init(void)
    if (!np)
        return;

-        domain = irq_domain_add_legacy(np, NR_IRQS, IRQ_ARMCTRL_START, 0,
+        domain = irq_domain_add_legacy(np, BCM2708_ALLOC_IRQS,
+                      IRQ_ARMCTRL_START, 0,
                    &armctrl_ops, NULL);
         WARN_ON(!domain);
 }
@@ -298,7 +299,7 @@ int __init armctrl_init(void __iomem * base, unsigned int irq_start,
 {
    unsigned int irq;

-   for (irq = 0; irq < NR_IRQS; irq++) {
+   for (irq = 0; irq < BCM2708_ALLOC_IRQS; irq++) {
        unsigned int data = irq;
        if (irq >= INTERRUPT_JPEG && irq <= INTERRUPT_ARASANSDIO)
            data = remap_irqs[irq - INTERRUPT_JPEG];
diff --git a/arch/arm/mach-bcm2708/include/mach/irqs.h b/arch/arm/mach-bcm2708/include/mach/irqs.h
index 4299054..5b4f678 100644
--- a/arch/arm/mach-bcm2708/include/mach/irqs.h
+++ b/arch/arm/mach-bcm2708/include/mach/irqs.h
@@ -191,7 +191,9 @@
 #define FIQ_IRQS        (64 + 21)
 #define GPIO_IRQ_START  (HARD_IRQS + FIQ_IRQS)
 #define GPIO_IRQS        (32*5)
-#define SPARE_IRQS       (64)
-#define NR_IRQS              (HARD_IRQS+FIQ_IRQS+GPIO_IRQS+SPARE_IRQS)
+#define SPARE_ALLOC_IRQS      32
+#define BCM2708_ALLOC_IRQS    (HARD_IRQS+FIQ_IRQS+GPIO_IRQS+SPARE_ALLOC_IRQS)
+#define FREE_IRQS             32
+#define NR_IRQS               (BCM2708_ALLOC_IRQS+FREE_IRQS)

 #endif /* _BCM2708_IRQS_H_ */
notro commented 9 years ago

PR https://github.com/raspberrypi/linux/pull/748 contains among other things, a proposed solution to this problem: https://github.com/notro/linux/commit/971f7fcdb9619ac620b62ab5a3d619fab993d57b

notro commented 9 years ago

PR is now applied, so this issue should be fixed. Thanks for tracking this down, it has been really helpful in many respects.

popcornmix commented 9 years ago

@chaosjug okay to close?

chaosjug commented 9 years ago

I didn't have the time to test this as I'm not using a 3.18 kernel at the moment, but this seems fine to me.