Patchwork [37/37] powerpc: make IRQ_NOREQUEST last to clear, first to set

login
register
mail settings
Submitter Milton Miller
Date May 11, 2011, 5:30 a.m.
Message ID <2e97f048f0a39de47c9af61c1b53fb6fba6260d6.1305092637.git.miltonm@bga.com>
Download mbox | patch
Permalink /patch/95077/
State Accepted
Commit e92623d87bc3d4c9f54c9534b379b96eab6b3e22
Delegated to: Benjamin Herrenschmidt
Headers show

Comments

Milton Miller - May 11, 2011, 5:30 a.m.
When allocating irqs, wait to clear the IRQ_NOREQUEST flag until the
host map hook has been called.

When freeing irqs, set the IRQ_NOREQUEST flag before calling the host
unmap hook.

Signed-off-by: Milton Miller <miltonm@bga.com>
---
 arch/powerpc/kernel/irq.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)
Grant Likely - May 11, 2011, 7:18 p.m.
On Wed, May 11, 2011 at 7:30 AM, Milton Miller <miltonm@bga.com> wrote:
> When allocating irqs, wait to clear the IRQ_NOREQUEST flag until the
> host map hook has been called.
>
> When freeing irqs, set the IRQ_NOREQUEST flag before calling the host
> unmap hook.

A description describing why this change is being made would be
appreciated here.

g.

>
> Signed-off-by: Milton Miller <miltonm@bga.com>
> ---
>  arch/powerpc/kernel/irq.c |   14 +++++++-------
>  1 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
> index 4368b5e..a24d37d 100644
> --- a/arch/powerpc/kernel/irq.c
> +++ b/arch/powerpc/kernel/irq.c
> @@ -586,14 +586,14 @@ struct irq_host *irq_alloc_host(struct device_node *of_node,
>                        irq_map[i].host = host;
>                        smp_wmb();
>
> -                       /* Clear norequest flags */
> -                       irq_clear_status_flags(i, IRQ_NOREQUEST);
> -
>                        /* Legacy flags are left to default at this point,
>                         * one can then use irq_create_mapping() to
>                         * explicitly change them
>                         */
>                        ops->map(host, i, i);
> +
> +                       /* Clear norequest flags */
> +                       irq_clear_status_flags(i, IRQ_NOREQUEST);
>                }
>                break;
>        case IRQ_HOST_MAP_LINEAR:
> @@ -664,8 +664,6 @@ static int irq_setup_virq(struct irq_host *host, unsigned int virq,
>                goto error;
>        }
>
> -       irq_clear_status_flags(virq, IRQ_NOREQUEST);
> -
>        /* map it */
>        smp_wmb();
>        irq_map[virq].hwirq = hwirq;
> @@ -676,6 +674,8 @@ static int irq_setup_virq(struct irq_host *host, unsigned int virq,
>                goto errdesc;
>        }
>
> +       irq_clear_status_flags(virq, IRQ_NOREQUEST);
> +
>        return 0;
>
>  errdesc:
> @@ -819,6 +819,8 @@ void irq_dispose_mapping(unsigned int virq)
>        if (host->revmap_type == IRQ_HOST_MAP_LEGACY)
>                return;
>
> +       irq_set_status_flags(virq, IRQ_NOREQUEST);
> +
>        /* remove chip and handler */
>        irq_set_chip_and_handler(virq, NULL, NULL);
>
> @@ -848,8 +850,6 @@ void irq_dispose_mapping(unsigned int virq)
>        smp_mb();
>        irq_map[virq].hwirq = host->inval_irq;
>
> -       irq_set_status_flags(virq, IRQ_NOREQUEST);
> -
>        irq_free_descs(virq, 1);
>        /* Free it */
>        irq_free_virt(virq, 1);
> --
> 1.7.0.4
>
>
Milton Miller - May 12, 2011, 8:31 a.m.
On Wed, 11 May 2011 about 21:18:11 +0200, Grant Likely wrote:
> On Wed, May 11, 2011 at 7:30 AM, Milton Miller <miltonm@bga.com> wrote:
> > When allocating irqs, wait to clear the IRQ_NOREQUEST flag until the
> > host map hook has been called.
> >
> > When freeing irqs, set the IRQ_NOREQUEST flag before calling the host
> > unmap hook.
> 
> A description describing why this change is being made would be
> appreciated here.
> 
> g.

You are right.  #insert <late addition to series but made cut>

When creating an irq, don't allow a concurent driver request until
we have caled map, which will likley call set_chip_and_handler to
change the irq_chip and its operations.

Similarly, when tearing down an IRQ, make sure no new uses come
along while we change the irq back to the nop chip and then reset
the descriptor to freed status.

If this is acceptable I'll let Ben update the changelog unless
he asks me to resend.

milton
> 
> >
> > Signed-off-by: Milton Miller <miltonm@bga.com>
> > ---
> > arch/powerpc/kernel/irq.c |  14 +++++++-------
> > 1 files changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
> > index 4368b5e..a24d37d 100644
> > --- a/arch/powerpc/kernel/irq.c
> > +++ b/arch/powerpc/kernel/irq.c
> > @@ -586,14 +586,14 @@ struct irq_host *irq_alloc_host(struct device_node *of_node,
> >            irq_map[i].host = host;
> >            smp_wmb();
> >
> > -            /* Clear norequest flags */
> > -            irq_clear_status_flags(i, IRQ_NOREQUEST);
> > -
> >            /* Legacy flags are left to default at this point,
> >             * one can then use irq_create_mapping() to
> >             * explicitly change them
> >             */
> >            ops->map(host, i, i);
> > +
> > +            /* Clear norequest flags */
> > +            irq_clear_status_flags(i, IRQ_NOREQUEST);
> >        }
> >        break;
> >    case IRQ_HOST_MAP_LINEAR:
> > @@ -664,8 +664,6 @@ static int irq_setup_virq(struct irq_host *host, unsigned int virq,
> >        goto error;
> >    }
> >
> > -    irq_clear_status_flags(virq, IRQ_NOREQUEST);
> > -
> >    /* map it */
> >    smp_wmb();
> >    irq_map[virq].hwirq = hwirq;
> > @@ -676,6 +674,8 @@ static int irq_setup_virq(struct irq_host *host, unsigned int virq,
> >        goto errdesc;
> >    }
> >
> > +    irq_clear_status_flags(virq, IRQ_NOREQUEST);
> > +
> >    return 0;
> >
> > errdesc:
> > @@ -819,6 +819,8 @@ void irq_dispose_mapping(unsigned int virq)
> >    if (host->revmap_type == IRQ_HOST_MAP_LEGACY)
> >        return;
> >
> > +    irq_set_status_flags(virq, IRQ_NOREQUEST);
> > +
> >    /* remove chip and handler */
> >    irq_set_chip_and_handler(virq, NULL, NULL);
> >
> > @@ -848,8 +850,6 @@ void irq_dispose_mapping(unsigned int virq)
> >    smp_mb();
> >    irq_map[virq].hwirq = host->inval_irq;
> >
> > -    irq_set_status_flags(virq, IRQ_NOREQUEST);
> > -
> >    irq_free_descs(virq, 1);
> >    /* Free it */
> >    irq_free_virt(virq, 1);
> > --
> > 1.7.0.4
> >
> >

Patch

diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
index 4368b5e..a24d37d 100644
--- a/arch/powerpc/kernel/irq.c
+++ b/arch/powerpc/kernel/irq.c
@@ -586,14 +586,14 @@  struct irq_host *irq_alloc_host(struct device_node *of_node,
 			irq_map[i].host = host;
 			smp_wmb();
 
-			/* Clear norequest flags */
-			irq_clear_status_flags(i, IRQ_NOREQUEST);
-
 			/* Legacy flags are left to default at this point,
 			 * one can then use irq_create_mapping() to
 			 * explicitly change them
 			 */
 			ops->map(host, i, i);
+
+			/* Clear norequest flags */
+			irq_clear_status_flags(i, IRQ_NOREQUEST);
 		}
 		break;
 	case IRQ_HOST_MAP_LINEAR:
@@ -664,8 +664,6 @@  static int irq_setup_virq(struct irq_host *host, unsigned int virq,
 		goto error;
 	}
 
-	irq_clear_status_flags(virq, IRQ_NOREQUEST);
-
 	/* map it */
 	smp_wmb();
 	irq_map[virq].hwirq = hwirq;
@@ -676,6 +674,8 @@  static int irq_setup_virq(struct irq_host *host, unsigned int virq,
 		goto errdesc;
 	}
 
+	irq_clear_status_flags(virq, IRQ_NOREQUEST);
+
 	return 0;
 
 errdesc:
@@ -819,6 +819,8 @@  void irq_dispose_mapping(unsigned int virq)
 	if (host->revmap_type == IRQ_HOST_MAP_LEGACY)
 		return;
 
+	irq_set_status_flags(virq, IRQ_NOREQUEST);
+
 	/* remove chip and handler */
 	irq_set_chip_and_handler(virq, NULL, NULL);
 
@@ -848,8 +850,6 @@  void irq_dispose_mapping(unsigned int virq)
 	smp_mb();
 	irq_map[virq].hwirq = host->inval_irq;
 
-	irq_set_status_flags(virq, IRQ_NOREQUEST);
-
 	irq_free_descs(virq, 1);
 	/* Free it */
 	irq_free_virt(virq, 1);