[PATCH] genirq: Provide IRQCHIP_ONESHOT_EDGE_SAFE

[PATCH] genirq: Provide IRQCHIP_ONESHOT_EDGE_SAFE

[Thomas Gleixner]

If an interrupt chip is marked IRQCHIP_ONESHOT_SAFE it signals that the
interrupt chip does not require the ONESHOT mode for threaded
interrupts. Unfortunately this is applied independent of the interrupt type
(edge/level).

The powerpc XPIC wants this functionality only for edge type
interrupts. Provide a new flag which provides the same functionality
restricted to edge type interrupts.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 include/linux/irq.h  |    2 ++
 kernel/irq/debugfs.c |    1 +
 kernel/irq/manage.c  |   34 +++++++++++++++++++++-------------
 3 files changed, 24 insertions(+), 13 deletions(-)
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -481,6 +481,7 @@ struct irq_chip {
  * IRQCHIP_SKIP_SET_WAKE:	Skip chip.irq_set_wake(), for this irq chip
  * IRQCHIP_ONESHOT_SAFE:	One shot does not require mask/unmask
  * IRQCHIP_EOI_THREADED:	Chip requires eoi() on unmask in threaded mode
+ * IRQCHIP_ONESHOT_EDGE_SAFE:	Skip the oneshot logic for edge type interrupts
  */
 enum {
 	IRQCHIP_SET_TYPE_MASKED		= (1 <<  0),
@@ -490,6 +491,7 @@ enum {
 	IRQCHIP_SKIP_SET_WAKE		= (1 <<  4),
 	IRQCHIP_ONESHOT_SAFE		= (1 <<  5),
 	IRQCHIP_EOI_THREADED		= (1 <<  6),
+	IRQCHIP_ONESHOT_EDGE_SAFE	= (1 <<  7),
 };
 
 #include <linux/irqdesc.h>
--- a/kernel/irq/debugfs.c
+++ b/kernel/irq/debugfs.c
@@ -56,6 +56,7 @@ static const struct irq_bit_descr irqchi
 	BIT_MASK_DESCR(IRQCHIP_SKIP_SET_WAKE),
 	BIT_MASK_DESCR(IRQCHIP_ONESHOT_SAFE),
 	BIT_MASK_DESCR(IRQCHIP_EOI_THREADED),
+	BIT_MASK_DESCR(IRQCHIP_ONESHOT_EDGE_SAFE),
 };
 
 static void
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -1166,18 +1166,6 @@ static int
 	}
 
 	/*
-	 * Drivers are often written to work w/o knowledge about the
-	 * underlying irq chip implementation, so a request for a
-	 * threaded irq without a primary hard irq context handler
-	 * requires the ONESHOT flag to be set. Some irq chips like
-	 * MSI based interrupts are per se one shot safe. Check the
-	 * chip flags, so we can avoid the unmask dance at the end of
-	 * the threaded handler for those.
-	 */
-	if (desc->irq_data.chip->flags & IRQCHIP_ONESHOT_SAFE)
-		new->flags &= ~IRQF_ONESHOT;
-
-	/*
 	 * Protects against a concurrent __free_irq() call which might wait
 	 * for synchronize_irq() to complete without holding the optional
 	 * chip bus lock and desc->lock.
@@ -1208,6 +1196,25 @@ static int
 	 * desc->request_mutex or the optional bus lock.
 	 */
 	raw_spin_lock_irqsave(&desc->lock, flags);
+
+	/*
+	 * Drivers are often written to work w/o knowledge about the
+	 * underlying irq chip implementation, so a request for a
+	 * threaded irq without a primary hard irq context handler
+	 * requires the ONESHOT flag to be set. Some irq chips like
+	 * MSI based interrupts are per se one shot safe. Check the
+	 * chip flags, so we can avoid the unmask dance at the end of
+	 * the threaded handler for those.
+	 */
+	if (desc->irq_data.chip->flags & IRQCHIP_ONESHOT_SAFE)
+		new->flags &= ~IRQF_ONESHOT;
+
+	/* Same as above, but restricted to edge type interrupts */
+	if (desc->irq_data.chip->flags & IRQCHIP_ONESHOT_EDGE_SAFE) {
+		if (!irq_settings_is_level(desc))
+			new->flags &= ~IRQF_ONESHOT;
+	}
+
 	old_ptr = &desc->action;
 	old = *old_ptr;
 	if (old) {
@@ -1281,7 +1288,8 @@ static int
 		new->thread_mask = 1 << ffz(thread_mask);
 
 	} else if (new->handler == irq_default_primary_handler &&
-		   !(desc->irq_data.chip->flags & IRQCHIP_ONESHOT_SAFE)) {
+		   !(desc->irq_data.chip->flags & IRQCHIP_ONESHOT_SAFE) &&
+		   !(desc->irq_data.chip->flags & IRQCHIP_ONESHOT_EDGE_SAFE)) {
 		/*
 		 * The interrupt was requested with handler = NULL, so
 		 * we use the default primary handler for it. But it

Re: [PATCH] genirq: Provide IRQCHIP_ONESHOT_EDGE_SAFE

[Benjamin Herrenschmidt]

On Mon, 2017-07-31 at 21:33 +0200, Thomas Gleixner wrote:
> If an interrupt chip is marked IRQCHIP_ONESHOT_SAFE it signals that the
> interrupt chip does not require the ONESHOT mode for threaded
> interrupts. Unfortunately this is applied independent of the interrupt type
> (edge/level).
> 
> The powerpc XPIC wants this functionality only for edge type
> interrupts. Provide a new flag which provides the same functionality
> restricted to edge type interrupts.

Thanks ! I'll test that asap (got pulled into another emergency so it
might take a day or two).

Cheers,
Ben.

> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  include/linux/irq.h  |    2 ++
>  kernel/irq/debugfs.c |    1 +
>  kernel/irq/manage.c  |   34 +++++++++++++++++++++-------------
>  3 files changed, 24 insertions(+), 13 deletions(-)
> --- a/include/linux/irq.h
> +++ b/include/linux/irq.h
> @@ -481,6 +481,7 @@ struct irq_chip {
>   * IRQCHIP_SKIP_SET_WAKE:	Skip chip.irq_set_wake(), for this irq chip
>   * IRQCHIP_ONESHOT_SAFE:	One shot does not require mask/unmask
>   * IRQCHIP_EOI_THREADED:	Chip requires eoi() on unmask in threaded mode
> + * IRQCHIP_ONESHOT_EDGE_SAFE:	Skip the oneshot logic for edge type interrupts
>   */
>  enum {
>  	IRQCHIP_SET_TYPE_MASKED		= (1 <<  0),
> @@ -490,6 +491,7 @@ enum {
>  	IRQCHIP_SKIP_SET_WAKE		= (1 <<  4),
>  	IRQCHIP_ONESHOT_SAFE		= (1 <<  5),
>  	IRQCHIP_EOI_THREADED		= (1 <<  6),
> +	IRQCHIP_ONESHOT_EDGE_SAFE	= (1 <<  7),
>  };
>  
>  #include <linux/irqdesc.h>
> --- a/kernel/irq/debugfs.c
> +++ b/kernel/irq/debugfs.c
> @@ -56,6 +56,7 @@ static const struct irq_bit_descr irqchi
>  	BIT_MASK_DESCR(IRQCHIP_SKIP_SET_WAKE),
>  	BIT_MASK_DESCR(IRQCHIP_ONESHOT_SAFE),
>  	BIT_MASK_DESCR(IRQCHIP_EOI_THREADED),
> +	BIT_MASK_DESCR(IRQCHIP_ONESHOT_EDGE_SAFE),
>  };
>  
>  static void
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
> @@ -1166,18 +1166,6 @@ static int
>  	}
>  
>  	/*
> -	 * Drivers are often written to work w/o knowledge about the
> -	 * underlying irq chip implementation, so a request for a
> -	 * threaded irq without a primary hard irq context handler
> -	 * requires the ONESHOT flag to be set. Some irq chips like
> -	 * MSI based interrupts are per se one shot safe. Check the
> -	 * chip flags, so we can avoid the unmask dance at the end of
> -	 * the threaded handler for those.
> -	 */
> -	if (desc->irq_data.chip->flags & IRQCHIP_ONESHOT_SAFE)
> -		new->flags &= ~IRQF_ONESHOT;
> -
> -	/*
>  	 * Protects against a concurrent __free_irq() call which might wait
>  	 * for synchronize_irq() to complete without holding the optional
>  	 * chip bus lock and desc->lock.
> @@ -1208,6 +1196,25 @@ static int
>  	 * desc->request_mutex or the optional bus lock.
>  	 */
>  	raw_spin_lock_irqsave(&desc->lock, flags);
> +
> +	/*
> +	 * Drivers are often written to work w/o knowledge about the
> +	 * underlying irq chip implementation, so a request for a
> +	 * threaded irq without a primary hard irq context handler
> +	 * requires the ONESHOT flag to be set. Some irq chips like
> +	 * MSI based interrupts are per se one shot safe. Check the
> +	 * chip flags, so we can avoid the unmask dance at the end of
> +	 * the threaded handler for those.
> +	 */
> +	if (desc->irq_data.chip->flags & IRQCHIP_ONESHOT_SAFE)
> +		new->flags &= ~IRQF_ONESHOT;
> +
> +	/* Same as above, but restricted to edge type interrupts */
> +	if (desc->irq_data.chip->flags & IRQCHIP_ONESHOT_EDGE_SAFE) {
> +		if (!irq_settings_is_level(desc))
> +			new->flags &= ~IRQF_ONESHOT;
> +	}
> +
>  	old_ptr = &desc->action;
>  	old = *old_ptr;
>  	if (old) {
> @@ -1281,7 +1288,8 @@ static int
>  		new->thread_mask = 1 << ffz(thread_mask);
>  
>  	} else if (new->handler == irq_default_primary_handler &&
> -		   !(desc->irq_data.chip->flags & IRQCHIP_ONESHOT_SAFE)) {
> +		   !(desc->irq_data.chip->flags & IRQCHIP_ONESHOT_SAFE) &&
> +		   !(desc->irq_data.chip->flags & IRQCHIP_ONESHOT_EDGE_SAFE)) {
>  		/*
>  		 * The interrupt was requested with handler = NULL, so
>  		 * we use the default primary handler for it. But it

Re: [PATCH] genirq: Provide IRQCHIP_ONESHOT_EDGE_SAFE

[Thomas Gleixner]

On Tue, 1 Aug 2017, Benjamin Herrenschmidt wrote:
> On Mon, 2017-07-31 at 21:33 +0200, Thomas Gleixner wrote:
> > If an interrupt chip is marked IRQCHIP_ONESHOT_SAFE it signals that the
> > interrupt chip does not require the ONESHOT mode for threaded
> > interrupts. Unfortunately this is applied independent of the interrupt type
> > (edge/level).
> > 
> > The powerpc XPIC wants this functionality only for edge type
> > interrupts. Provide a new flag which provides the same functionality
> > restricted to edge type interrupts.
> 
> Thanks ! I'll test that asap (got pulled into another emergency so it
> might take a day or two).

I'm not around til Aug. 14th, so feel free to route it Linuswards directly.

Thanks,

	tglx

Re: [PATCH] genirq: Provide IRQCHIP_ONESHOT_EDGE_SAFE

[Thomas Gleixner]

On Tue, 1 Aug 2017, Benjamin Herrenschmidt wrote:

> On Mon, 2017-07-31 at 21:33 +0200, Thomas Gleixner wrote:
> > If an interrupt chip is marked IRQCHIP_ONESHOT_SAFE it signals that the
> > interrupt chip does not require the ONESHOT mode for threaded
> > interrupts. Unfortunately this is applied independent of the interrupt type
> > (edge/level).
> > 
> > The powerpc XPIC wants this functionality only for edge type
> > interrupts. Provide a new flag which provides the same functionality
> > restricted to edge type interrupts.
> 
> Thanks ! I'll test that asap (got pulled into another emergency so it
> might take a day or two).

So that emergency still is on? A day or two is about 35 days ago :)