Re: [GIT pull] irq updates for 4.13

[Pavel Machek]

Hi!

> > This patch apparently breaks OMAP platform:
> >
> > 46e48e257360f0845fe17089713cbad4db611e70 is the first bad commit
> > commit 46e48e257360f0845fe17089713cbad4db611e70
> > Author: Thomas Gleixner <t...@linutronix.de>
> > Date:   Thu Jun 29 23:33:38 2017 +0200
> >
> >     genirq: Move irq resource handling out of spinlocked region
> >
> > Boot failure log from Droid 4:
> > [ ... snip snip ..]
> >
> > Droid 4 boots current master again after applying the patch below
> > (which is git revet of above patch, but I provide the patch, since
> > it did not revet cleanly).
> 
> Hmm. Do you actually need the full revert?
> 
> I think it's only the __setup_irq() part that looks like it may be garbage.
> 
> For example, I think it releases the resources twice if the
> __irq_set_trigger() call fails.
> 
> But it looks questionably in other ways too - notably, the change to
> make the request call be in the same context as the freeing is done is
> apparently done entirely for symmetry reasons, not for any actual
> *reason* reasons.
> 
> So I suspect just the __setup_irq() parts should be reverted, because
> they look both buggy and pointless. But the actual *real* part of the
> patch was the two-liner __free_irq() part, and that looks sane to me.
> 
> So Sebastian, can you test if it's ok to revert just the __setup_irq()
> part, but leave the smaller part in __free_irq() that just moves the
> irq_release_resources() around at freeing time?

If I understood it correctly, you wanted to test this:

And yes, this is enough to fix boot on N900 for me.

Thanks,
                                                                Pavel

commit 285358d48dec82f13fa76724bff434897883d188
Author: Pavel <pa...@ucw.cz>
Date:   Mon Jul 10 21:35:25 2017 +0200

diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index 5624b2d..528bfc3 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -1168,14 +1168,6 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, 
struct irqaction *new)
                new->flags &= ~IRQF_ONESHOT;
 
        mutex_lock(&desc->request_mutex);
-       if (!desc->action) {
-               ret = irq_request_resources(desc);
-               if (ret) {
-                       pr_err("Failed to request resources for %s (irq %d) on 
irqchip %s\n",
-                              new->name, irq, desc->irq_data.chip->name);
-                       goto out_mutex;
-               }
-       }
 
        chip_bus_lock(desc);
 
@@ -1279,6 +1271,13 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, 
struct irqaction *new)
        }
 
        if (!shared) {
+               ret = irq_request_resources(desc);
+               if (ret) {
+                       pr_err("Failed to request resources for %s (irq %d) on 
irqchip %s\n",
+                              new->name, irq, desc->irq_data.chip->name);
+                       goto out_unlock;
+               }
+
                init_waitqueue_head(&desc->wait_for_threads);
 
                /* Setup the type (level, edge polarity) if configured: */
@@ -1387,10 +1386,6 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, 
struct irqaction *new)
 
        chip_bus_sync_unlock(desc);
 
-       if (!desc->action)
-               irq_release_resources(desc);
-
-out_mutex:
        mutex_unlock(&desc->request_mutex);
 
 out_thread:


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

signature.asc
Description: Digital signature