linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH 09/17] [m68k] IRQ: add handle_polled_irq() for timer based soft interrupts
       [not found] <20130131002344.1161E360095@xplor.waratah.dyndns.org>
@ 2013-02-10 10:36 ` Geert Uytterhoeven
  2013-03-20  8:54   ` Geert Uytterhoeven
  0 siblings, 1 reply; 8+ messages in thread
From: Geert Uytterhoeven @ 2013-02-10 10:36 UTC (permalink / raw)
  To: Michael Schmitz, Thomas Gleixner; +Cc: linux-m68k, linux-kernel

On Thu, Jan 31, 2013 at 1:23 AM, Michael Schmitz <schmitzmic@gmail.com> wrote:
> [PATCH 09/17] [m68k] IRQ: add handle_polled_irq() for timer based soft interrupts -
>         experimental hack to avoid unhandled interrupt timer to fire
>         on EtherNEC/NetUSBee cards that have no hardware interrupt
>         and need to be polled from a timer
>
> This patch adds a special 'polled interrupt' handler for timer based software interrupts.

Adding Thomas and lkml.

> handle_simple_irq() will respond to excessive unhandled interrupts (as are expected for a
> polling timer interrupt) by disabling the apparently unhandled interrupt source.
>
> handle_polled_irq() prevents this by setting the IRQS_POLL_INPROGRESS flag which will cause
> the unhandled interrupt events to be ignored.
>
> This is a temporary hack to allow timer based polling of the Atari ROM port network and USB
> cards only. Suggestions on how to properly handle this in the normal interrupt framework are
> most welcome.
>
> Signed-off-by: Michael Schmitz <schmitz@debian.org>
> ---
>  include/linux/irq.h |    1 +
>  kernel/irq/chip.c   |   36 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 37 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/irq.h b/include/linux/irq.h
> index fdf2c4a..39bfcff 100644
> --- a/include/linux/irq.h
> +++ b/include/linux/irq.h
> @@ -407,6 +407,7 @@ extern void handle_fasteoi_irq(unsigned int irq, struct irq_desc *desc);
>  extern void handle_edge_irq(unsigned int irq, struct irq_desc *desc);
>  extern void handle_edge_eoi_irq(unsigned int irq, struct irq_desc *desc);
>  extern void handle_simple_irq(unsigned int irq, struct irq_desc *desc);
> +extern void handle_polled_irq(unsigned int irq, struct irq_desc *desc);
>  extern void handle_percpu_irq(unsigned int irq, struct irq_desc *desc);
>  extern void handle_percpu_devid_irq(unsigned int irq, struct irq_desc *desc);
>  extern void handle_bad_irq(unsigned int irq, struct irq_desc *desc);
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index 3aca9f2..ae7ef4d 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -339,6 +339,42 @@ out_unlock:
>  }
>  EXPORT_SYMBOL_GPL(handle_simple_irq);
>
> +/**
> + *     handle_polled_irq - Simple and software-decoded IRQs.
> + *     @irq:   the interrupt number
> + *     @desc:  the interrupt description structure for this irq
> + *
> + *     Polled interrupts are sent from a demultiplexing software interrupt
> + *     handler, where no interrupt hardware control is necessary.
> + */
> +void
> +handle_polled_irq(unsigned int irq, struct irq_desc *desc)
> +{
> +       raw_spin_lock(&desc->lock);
> +
> +       if (unlikely(irqd_irq_inprogress(&desc->irq_data)))
> +               if (!irq_check_poll(desc))
> +                       goto out_unlock;
> +
> +       desc->istate &= ~(IRQS_REPLAY | IRQS_WAITING);
> +       kstat_incr_irqs_this_cpu(irq, desc);
> +
> +       if (unlikely(!desc->action || irqd_irq_disabled(&desc->irq_data))) {
> +               desc->istate |= IRQS_PENDING;
> +               goto out_unlock;
> +       }
> +
> +       desc->istate |= IRQS_POLL_INPROGRESS;
> +
> +       handle_irq_event(desc);
> +
> +       desc->istate &= ~(IRQS_POLL_INPROGRESS);
> +
> +out_unlock:
> +       raw_spin_unlock(&desc->lock);
> +}
> +EXPORT_SYMBOL_GPL(handle_polled_irq);
> +
>  /*
>   * Called unconditionally from handle_level_irq() and only for oneshot
>   * interrupts from handle_fasteoi_irq()
> --
> 1.7.0.4

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 09/17] [m68k] IRQ: add handle_polled_irq() for timer based soft interrupts
  2013-02-10 10:36 ` [PATCH 09/17] [m68k] IRQ: add handle_polled_irq() for timer based soft interrupts Geert Uytterhoeven
@ 2013-03-20  8:54   ` Geert Uytterhoeven
  2013-11-06 11:30     ` Thomas Gleixner
  0 siblings, 1 reply; 8+ messages in thread
From: Geert Uytterhoeven @ 2013-03-20  8:54 UTC (permalink / raw)
  To: Michael Schmitz, Thomas Gleixner; +Cc: linux-m68k, linux-kernel

On Sun, Feb 10, 2013 at 11:36 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Thu, Jan 31, 2013 at 1:23 AM, Michael Schmitz <schmitzmic@gmail.com> wrote:
>> [PATCH 09/17] [m68k] IRQ: add handle_polled_irq() for timer based soft interrupts -
>>         experimental hack to avoid unhandled interrupt timer to fire
>>         on EtherNEC/NetUSBee cards that have no hardware interrupt
>>         and need to be polled from a timer
>>
>> This patch adds a special 'polled interrupt' handler for timer based software interrupts.
>
> Adding Thomas and lkml.

No comments/suggestions?
Thanks again!

>> handle_simple_irq() will respond to excessive unhandled interrupts (as are expected for a
>> polling timer interrupt) by disabling the apparently unhandled interrupt source.
>>
>> handle_polled_irq() prevents this by setting the IRQS_POLL_INPROGRESS flag which will cause
>> the unhandled interrupt events to be ignored.
>>
>> This is a temporary hack to allow timer based polling of the Atari ROM port network and USB
>> cards only. Suggestions on how to properly handle this in the normal interrupt framework are
>> most welcome.
>>
>> Signed-off-by: Michael Schmitz <schmitz@debian.org>
>> ---
>>  include/linux/irq.h |    1 +
>>  kernel/irq/chip.c   |   36 ++++++++++++++++++++++++++++++++++++
>>  2 files changed, 37 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/linux/irq.h b/include/linux/irq.h
>> index fdf2c4a..39bfcff 100644
>> --- a/include/linux/irq.h
>> +++ b/include/linux/irq.h
>> @@ -407,6 +407,7 @@ extern void handle_fasteoi_irq(unsigned int irq, struct irq_desc *desc);
>>  extern void handle_edge_irq(unsigned int irq, struct irq_desc *desc);
>>  extern void handle_edge_eoi_irq(unsigned int irq, struct irq_desc *desc);
>>  extern void handle_simple_irq(unsigned int irq, struct irq_desc *desc);
>> +extern void handle_polled_irq(unsigned int irq, struct irq_desc *desc);
>>  extern void handle_percpu_irq(unsigned int irq, struct irq_desc *desc);
>>  extern void handle_percpu_devid_irq(unsigned int irq, struct irq_desc *desc);
>>  extern void handle_bad_irq(unsigned int irq, struct irq_desc *desc);
>> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
>> index 3aca9f2..ae7ef4d 100644
>> --- a/kernel/irq/chip.c
>> +++ b/kernel/irq/chip.c
>> @@ -339,6 +339,42 @@ out_unlock:
>>  }
>>  EXPORT_SYMBOL_GPL(handle_simple_irq);
>>
>> +/**
>> + *     handle_polled_irq - Simple and software-decoded IRQs.
>> + *     @irq:   the interrupt number
>> + *     @desc:  the interrupt description structure for this irq
>> + *
>> + *     Polled interrupts are sent from a demultiplexing software interrupt
>> + *     handler, where no interrupt hardware control is necessary.
>> + */
>> +void
>> +handle_polled_irq(unsigned int irq, struct irq_desc *desc)
>> +{
>> +       raw_spin_lock(&desc->lock);
>> +
>> +       if (unlikely(irqd_irq_inprogress(&desc->irq_data)))
>> +               if (!irq_check_poll(desc))
>> +                       goto out_unlock;
>> +
>> +       desc->istate &= ~(IRQS_REPLAY | IRQS_WAITING);
>> +       kstat_incr_irqs_this_cpu(irq, desc);
>> +
>> +       if (unlikely(!desc->action || irqd_irq_disabled(&desc->irq_data))) {
>> +               desc->istate |= IRQS_PENDING;
>> +               goto out_unlock;
>> +       }
>> +
>> +       desc->istate |= IRQS_POLL_INPROGRESS;
>> +
>> +       handle_irq_event(desc);
>> +
>> +       desc->istate &= ~(IRQS_POLL_INPROGRESS);
>> +
>> +out_unlock:
>> +       raw_spin_unlock(&desc->lock);
>> +}
>> +EXPORT_SYMBOL_GPL(handle_polled_irq);
>> +
>>  /*
>>   * Called unconditionally from handle_level_irq() and only for oneshot
>>   * interrupts from handle_fasteoi_irq()
>> --
>> 1.7.0.4

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 09/17] [m68k] IRQ: add handle_polled_irq() for timer based soft interrupts
  2013-03-20  8:54   ` Geert Uytterhoeven
@ 2013-11-06 11:30     ` Thomas Gleixner
  2013-11-07  7:59       ` Michael Schmitz
                         ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Thomas Gleixner @ 2013-11-06 11:30 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: Michael Schmitz, linux-m68k, linux-kernel

On Wed, 20 Mar 2013, Geert Uytterhoeven wrote:

> On Sun, Feb 10, 2013 at 11:36 AM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
> > On Thu, Jan 31, 2013 at 1:23 AM, Michael Schmitz <schmitzmic@gmail.com> wrote:
> >> [PATCH 09/17] [m68k] IRQ: add handle_polled_irq() for timer based soft interrupts -
> >>         experimental hack to avoid unhandled interrupt timer to fire
> >>         on EtherNEC/NetUSBee cards that have no hardware interrupt
> >>         and need to be polled from a timer
> >>
> >> This patch adds a special 'polled interrupt' handler for timer
> >> based software interrupts.
> >
> > Adding Thomas and lkml.
> 
> No comments/suggestions?
> Thanks again!

Sorry for answering late. This slipped out of my view completely.

> >> handle_simple_irq() will respond to excessive unhandled
> >> interrupts (as are expected for a polling timer interrupt) by
> >> disabling the apparently unhandled interrupt source.
> >> handle_polled_irq() prevents this by setting the
> >> IRQS_POLL_INPROGRESS flag which will cause the unhandled
> >> interrupt events to be ignored.

> >> This is a temporary hack to allow timer based polling of the
> >> Atari ROM port network and USB cards only. Suggestions on how to
> >> properly handle this in the normal interrupt framework are most
> >> welcome.

So you're polling devices which have no hardware interrupt from the
timer interrupt. Of course if there is no interrupt pending on one of
these devices, this will trigger the spurious detector. By setting the
POLL_INPROGRESS flag, you're preventing that.

Reading the demultiplex handler it seems you have no way to figure out
which of the sub interrupts actually triggered the mfptimer_handler,
right?

I'm not too happy about the POLL flag "abuse". I'd rather have such
interrupts explicitely marked as polled by some other interrupt. That
also excludes such interrupts from the spurious mechanism completely.

Does the following patch solve the problem? You need to call

     irq_set_status_flags(irq, IRQ_IS_POLLED);

when setting up the interrupt controller for those polled interrupt
lines.

Thanks,

	tglx
-----------------
diff --git a/include/linux/irq.h b/include/linux/irq.h
index 56bb0dc..7dc1003 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -70,6 +70,9 @@ typedef	void (*irq_preflow_handler_t)(struct irq_data *data);
  * IRQ_MOVE_PCNTXT		- Interrupt can be migrated from process context
  * IRQ_NESTED_TRHEAD		- Interrupt nests into another thread
  * IRQ_PER_CPU_DEVID		- Dev_id is a per-cpu variable
+ * IRQ_IS_POLLED		- Always polled by another interrupt. Exclude
+ *				  it from the spurious interrupt detection
+ *				  mechanism and from core side polling.
  */
 enum {
 	IRQ_TYPE_NONE		= 0x00000000,
@@ -94,12 +97,14 @@ enum {
 	IRQ_NESTED_THREAD	= (1 << 15),
 	IRQ_NOTHREAD		= (1 << 16),
 	IRQ_PER_CPU_DEVID	= (1 << 17),
+	IRQ_IS_POLLED		= (1 << 18),
 };
 
 #define IRQF_MODIFY_MASK	\
 	(IRQ_TYPE_SENSE_MASK | IRQ_NOPROBE | IRQ_NOREQUEST | \
 	 IRQ_NOAUTOEN | IRQ_MOVE_PCNTXT | IRQ_LEVEL | IRQ_NO_BALANCING | \
-	 IRQ_PER_CPU | IRQ_NESTED_THREAD | IRQ_NOTHREAD | IRQ_PER_CPU_DEVID)
+	 IRQ_PER_CPU | IRQ_NESTED_THREAD | IRQ_NOTHREAD | IRQ_PER_CPU_DEVID | \
+	 IRQ_IS_POLLED)
 
 #define IRQ_NO_BALANCING_MASK	(IRQ_PER_CPU | IRQ_NO_BALANCING)
 
diff --git a/kernel/irq/settings.h b/kernel/irq/settings.h
index 1162f10..3320b84 100644
--- a/kernel/irq/settings.h
+++ b/kernel/irq/settings.h
@@ -14,6 +14,7 @@ enum {
 	_IRQ_NO_BALANCING	= IRQ_NO_BALANCING,
 	_IRQ_NESTED_THREAD	= IRQ_NESTED_THREAD,
 	_IRQ_PER_CPU_DEVID	= IRQ_PER_CPU_DEVID,
+	_IRQ_IS_POLLED		= IRQ_IS_POLLED,
 	_IRQF_MODIFY_MASK	= IRQF_MODIFY_MASK,
 };
 
@@ -26,6 +27,7 @@ enum {
 #define IRQ_NOAUTOEN		GOT_YOU_MORON
 #define IRQ_NESTED_THREAD	GOT_YOU_MORON
 #define IRQ_PER_CPU_DEVID	GOT_YOU_MORON
+#define IRQ_IS_POLLED		GOT_YOU_MORON
 #undef IRQF_MODIFY_MASK
 #define IRQF_MODIFY_MASK	GOT_YOU_MORON
 
@@ -147,3 +149,8 @@ static inline bool irq_settings_is_nested_thread(struct irq_desc *desc)
 {
 	return desc->status_use_accessors & _IRQ_NESTED_THREAD;
 }
+
+static inline bool irq_settings_is_polled(struct irq_desc *desc)
+{
+	return desc->status_use_accessors & _IRQ_IS_POLLED;
+}
diff --git a/kernel/irq/spurious.c b/kernel/irq/spurious.c
index 7b5f012..a1d8cc6 100644
--- a/kernel/irq/spurious.c
+++ b/kernel/irq/spurious.c
@@ -67,8 +67,13 @@ static int try_one_irq(int irq, struct irq_desc *desc, bool force)
 
 	raw_spin_lock(&desc->lock);
 
-	/* PER_CPU and nested thread interrupts are never polled */
-	if (irq_settings_is_per_cpu(desc) || irq_settings_is_nested_thread(desc))
+	/*
+	 * PER_CPU, nested thread interrupts and interrupts explicitely
+	 * marked polled are excluded from polling.
+	 */
+	if (irq_settings_is_per_cpu(desc) ||
+	    irq_settings_is_nested_thread(desc) ||
+	    irq_settings_is_polled(desc))
 		goto out;
 
 	/*
@@ -268,7 +273,8 @@ try_misrouted_irq(unsigned int irq, struct irq_desc *desc,
 void note_interrupt(unsigned int irq, struct irq_desc *desc,
 		    irqreturn_t action_ret)
 {
-	if (desc->istate & IRQS_POLL_INPROGRESS)
+	if (desc->istate & IRQS_POLL_INPROGRESS ||
+	    irq_settings_is_polled(desc))
 		return;
 
 	/* we get here again via the threaded handler */




^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 09/17] [m68k] IRQ: add handle_polled_irq() for timer based soft interrupts
  2013-11-06 11:30     ` Thomas Gleixner
@ 2013-11-07  7:59       ` Michael Schmitz
  2013-11-10  8:33       ` Michael Schmitz
  2013-11-13 19:42       ` [tip:irq/urgent] genirq: Prevent spurious detection for unconditionally polled interrupts tip-bot for Thomas Gleixner
  2 siblings, 0 replies; 8+ messages in thread
From: Michael Schmitz @ 2013-11-07  7:59 UTC (permalink / raw)
  To: Thomas Gleixner; +Cc: linux-m68k, linux-kernel, Geert Uytterhoeven

Thomas,

>>>> This is a temporary hack to allow timer based polling of the
>>>> Atari ROM port network and USB cards only. Suggestions on how to
>>>> properly handle this in the normal interrupt framework are most
>>>> welcome.
>
> So you're polling devices which have no hardware interrupt from the
> timer interrupt. Of course if there is no interrupt pending on one of
> these devices, this will trigger the spurious detector. By setting the
> POLL_INPROGRESS flag, you're preventing that.

That's the general idea, yes. I am aware I abuse the flag somewhat. 
This was
the least invasive fix I came up with (i.e. no risk to cause 
regressions elsewhere).
Adding a new IRQ flag would have been my preference, but that seemed a 
bit
bold at the time.

> Reading the demultiplex handler it seems you have no way to figure out
> which of the sub interrupts actually triggered the mfptimer_handler,
> right?

Correct.

> I'm not too happy about the POLL flag "abuse". I'd rather have such

Precisely why I asked for this to be reviewed, thanks,

> interrupts explicitely marked as polled by some other interrupt. That
> also excludes such interrupts from the spurious mechanism completely.
>
> Does the following patch solve the problem? You need to call
>
>      irq_set_status_flags(irq, IRQ_IS_POLLED);
>
> when setting up the interrupt controller for those polled interrupt
> lines.

This should do the trick, from what I can see. I have tried to test it, 
but recent changes
to the m68k vmlinux.lds have thrown a monkey wrench in the works, it 
seems (kernels
just won't boot at all). I'll sort that out with Geert and report back.

Much obliged,

	Michael


>
> Thanks,
>
> 	tglx
> -----------------
> diff --git a/include/linux/irq.h b/include/linux/irq.h
> index 56bb0dc..7dc1003 100644
> --- a/include/linux/irq.h
> +++ b/include/linux/irq.h
> @@ -70,6 +70,9 @@ typedef	void (*irq_preflow_handler_t)(struct 
> irq_data *data);
>   * IRQ_MOVE_PCNTXT		- Interrupt can be migrated from process context
>   * IRQ_NESTED_TRHEAD		- Interrupt nests into another thread
>   * IRQ_PER_CPU_DEVID		- Dev_id is a per-cpu variable
> + * IRQ_IS_POLLED		- Always polled by another interrupt. Exclude
> + *				  it from the spurious interrupt detection
> + *				  mechanism and from core side polling.
>   */
>  enum {
>  	IRQ_TYPE_NONE		= 0x00000000,
> @@ -94,12 +97,14 @@ enum {
>  	IRQ_NESTED_THREAD	= (1 << 15),
>  	IRQ_NOTHREAD		= (1 << 16),
>  	IRQ_PER_CPU_DEVID	= (1 << 17),
> +	IRQ_IS_POLLED		= (1 << 18),
>  };
>
>  #define IRQF_MODIFY_MASK	\
>  	(IRQ_TYPE_SENSE_MASK | IRQ_NOPROBE | IRQ_NOREQUEST | \
>  	 IRQ_NOAUTOEN | IRQ_MOVE_PCNTXT | IRQ_LEVEL | IRQ_NO_BALANCING | \
> -	 IRQ_PER_CPU | IRQ_NESTED_THREAD | IRQ_NOTHREAD | IRQ_PER_CPU_DEVID)
> +	 IRQ_PER_CPU | IRQ_NESTED_THREAD | IRQ_NOTHREAD | IRQ_PER_CPU_DEVID 
> | \
> +	 IRQ_IS_POLLED)
>
>  #define IRQ_NO_BALANCING_MASK	(IRQ_PER_CPU | IRQ_NO_BALANCING)
>
> diff --git a/kernel/irq/settings.h b/kernel/irq/settings.h
> index 1162f10..3320b84 100644
> --- a/kernel/irq/settings.h
> +++ b/kernel/irq/settings.h
> @@ -14,6 +14,7 @@ enum {
>  	_IRQ_NO_BALANCING	= IRQ_NO_BALANCING,
>  	_IRQ_NESTED_THREAD	= IRQ_NESTED_THREAD,
>  	_IRQ_PER_CPU_DEVID	= IRQ_PER_CPU_DEVID,
> +	_IRQ_IS_POLLED		= IRQ_IS_POLLED,
>  	_IRQF_MODIFY_MASK	= IRQF_MODIFY_MASK,
>  };
>
> @@ -26,6 +27,7 @@ enum {
>  #define IRQ_NOAUTOEN		GOT_YOU_MORON
>  #define IRQ_NESTED_THREAD	GOT_YOU_MORON
>  #define IRQ_PER_CPU_DEVID	GOT_YOU_MORON
> +#define IRQ_IS_POLLED		GOT_YOU_MORON
>  #undef IRQF_MODIFY_MASK
>  #define IRQF_MODIFY_MASK	GOT_YOU_MORON
>
> @@ -147,3 +149,8 @@ static inline bool 
> irq_settings_is_nested_thread(struct irq_desc *desc)
>  {
>  	return desc->status_use_accessors & _IRQ_NESTED_THREAD;
>  }
> +
> +static inline bool irq_settings_is_polled(struct irq_desc *desc)
> +{
> +	return desc->status_use_accessors & _IRQ_IS_POLLED;
> +}
> diff --git a/kernel/irq/spurious.c b/kernel/irq/spurious.c
> index 7b5f012..a1d8cc6 100644
> --- a/kernel/irq/spurious.c
> +++ b/kernel/irq/spurious.c
> @@ -67,8 +67,13 @@ static int try_one_irq(int irq, struct irq_desc 
> *desc, bool force)
>
>  	raw_spin_lock(&desc->lock);
>
> -	/* PER_CPU and nested thread interrupts are never polled */
> -	if (irq_settings_is_per_cpu(desc) || 
> irq_settings_is_nested_thread(desc))
> +	/*
> +	 * PER_CPU, nested thread interrupts and interrupts explicitely
> +	 * marked polled are excluded from polling.
> +	 */
> +	if (irq_settings_is_per_cpu(desc) ||
> +	    irq_settings_is_nested_thread(desc) ||
> +	    irq_settings_is_polled(desc))
>  		goto out;
>
>  	/*
> @@ -268,7 +273,8 @@ try_misrouted_irq(unsigned int irq, struct 
> irq_desc *desc,
>  void note_interrupt(unsigned int irq, struct irq_desc *desc,
>  		    irqreturn_t action_ret)
>  {
> -	if (desc->istate & IRQS_POLL_INPROGRESS)
> +	if (desc->istate & IRQS_POLL_INPROGRESS ||
> +	    irq_settings_is_polled(desc))
>  		return;
>
>  	/* we get here again via the threaded handler */
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-m68k" 
> in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 09/17] [m68k] IRQ: add handle_polled_irq() for timer based soft interrupts
  2013-11-06 11:30     ` Thomas Gleixner
  2013-11-07  7:59       ` Michael Schmitz
@ 2013-11-10  8:33       ` Michael Schmitz
  2013-11-12 19:08         ` Thomas Gleixner
  2013-11-13 19:42       ` [tip:irq/urgent] genirq: Prevent spurious detection for unconditionally polled interrupts tip-bot for Thomas Gleixner
  2 siblings, 1 reply; 8+ messages in thread
From: Michael Schmitz @ 2013-11-10  8:33 UTC (permalink / raw)
  To: Thomas Gleixner; +Cc: linux-m68k, linux-kernel, Geert Uytterhoeven

Thomas,

> Does the following patch solve the problem? You need to call
>
>      irq_set_status_flags(irq, IRQ_IS_POLLED);
>
> when setting up the interrupt controller for those polled interrupt
> lines.

Your patch works just fine, thanks.

I suppose setting the flag can be done in the corresponding irq startup 
function, instead of when setting up the irq controller?

Geert - I will send the patch to ataints.c implementing this as soon as 
Thomas' fix is merged.

Regards,

	Michael


> Thanks,
>
> 	tglx
> -----------------
> diff --git a/include/linux/irq.h b/include/linux/irq.h
> index 56bb0dc..7dc1003 100644
> --- a/include/linux/irq.h
> +++ b/include/linux/irq.h
> @@ -70,6 +70,9 @@ typedef	void (*irq_preflow_handler_t)(struct 
> irq_data *data);
>   * IRQ_MOVE_PCNTXT		- Interrupt can be migrated from process context
>   * IRQ_NESTED_TRHEAD		- Interrupt nests into another thread
>   * IRQ_PER_CPU_DEVID		- Dev_id is a per-cpu variable
> + * IRQ_IS_POLLED		- Always polled by another interrupt. Exclude
> + *				  it from the spurious interrupt detection
> + *				  mechanism and from core side polling.
>   */
>  enum {
>  	IRQ_TYPE_NONE		= 0x00000000,
> @@ -94,12 +97,14 @@ enum {
>  	IRQ_NESTED_THREAD	= (1 << 15),
>  	IRQ_NOTHREAD		= (1 << 16),
>  	IRQ_PER_CPU_DEVID	= (1 << 17),
> +	IRQ_IS_POLLED		= (1 << 18),
>  };
>
>  #define IRQF_MODIFY_MASK	\
>  	(IRQ_TYPE_SENSE_MASK | IRQ_NOPROBE | IRQ_NOREQUEST | \
>  	 IRQ_NOAUTOEN | IRQ_MOVE_PCNTXT | IRQ_LEVEL | IRQ_NO_BALANCING | \
> -	 IRQ_PER_CPU | IRQ_NESTED_THREAD | IRQ_NOTHREAD | IRQ_PER_CPU_DEVID)
> +	 IRQ_PER_CPU | IRQ_NESTED_THREAD | IRQ_NOTHREAD | IRQ_PER_CPU_DEVID 
> | \
> +	 IRQ_IS_POLLED)
>
>  #define IRQ_NO_BALANCING_MASK	(IRQ_PER_CPU | IRQ_NO_BALANCING)
>
> diff --git a/kernel/irq/settings.h b/kernel/irq/settings.h
> index 1162f10..3320b84 100644
> --- a/kernel/irq/settings.h
> +++ b/kernel/irq/settings.h
> @@ -14,6 +14,7 @@ enum {
>  	_IRQ_NO_BALANCING	= IRQ_NO_BALANCING,
>  	_IRQ_NESTED_THREAD	= IRQ_NESTED_THREAD,
>  	_IRQ_PER_CPU_DEVID	= IRQ_PER_CPU_DEVID,
> +	_IRQ_IS_POLLED		= IRQ_IS_POLLED,
>  	_IRQF_MODIFY_MASK	= IRQF_MODIFY_MASK,
>  };
>
> @@ -26,6 +27,7 @@ enum {
>  #define IRQ_NOAUTOEN		GOT_YOU_MORON
>  #define IRQ_NESTED_THREAD	GOT_YOU_MORON
>  #define IRQ_PER_CPU_DEVID	GOT_YOU_MORON
> +#define IRQ_IS_POLLED		GOT_YOU_MORON
>  #undef IRQF_MODIFY_MASK
>  #define IRQF_MODIFY_MASK	GOT_YOU_MORON
>
> @@ -147,3 +149,8 @@ static inline bool 
> irq_settings_is_nested_thread(struct irq_desc *desc)
>  {
>  	return desc->status_use_accessors & _IRQ_NESTED_THREAD;
>  }
> +
> +static inline bool irq_settings_is_polled(struct irq_desc *desc)
> +{
> +	return desc->status_use_accessors & _IRQ_IS_POLLED;
> +}
> diff --git a/kernel/irq/spurious.c b/kernel/irq/spurious.c
> index 7b5f012..a1d8cc6 100644
> --- a/kernel/irq/spurious.c
> +++ b/kernel/irq/spurious.c
> @@ -67,8 +67,13 @@ static int try_one_irq(int irq, struct irq_desc 
> *desc, bool force)
>
>  	raw_spin_lock(&desc->lock);
>
> -	/* PER_CPU and nested thread interrupts are never polled */
> -	if (irq_settings_is_per_cpu(desc) || 
> irq_settings_is_nested_thread(desc))
> +	/*
> +	 * PER_CPU, nested thread interrupts and interrupts explicitely
> +	 * marked polled are excluded from polling.
> +	 */
> +	if (irq_settings_is_per_cpu(desc) ||
> +	    irq_settings_is_nested_thread(desc) ||
> +	    irq_settings_is_polled(desc))
>  		goto out;
>
>  	/*
> @@ -268,7 +273,8 @@ try_misrouted_irq(unsigned int irq, struct 
> irq_desc *desc,
>  void note_interrupt(unsigned int irq, struct irq_desc *desc,
>  		    irqreturn_t action_ret)
>  {
> -	if (desc->istate & IRQS_POLL_INPROGRESS)
> +	if (desc->istate & IRQS_POLL_INPROGRESS ||
> +	    irq_settings_is_polled(desc))
>  		return;
>
>  	/* we get here again via the threaded handler */
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-m68k" 
> in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 09/17] [m68k] IRQ: add handle_polled_irq() for timer based soft interrupts
  2013-11-10  8:33       ` Michael Schmitz
@ 2013-11-12 19:08         ` Thomas Gleixner
  2013-11-13  7:57           ` Michael Schmitz
  0 siblings, 1 reply; 8+ messages in thread
From: Thomas Gleixner @ 2013-11-12 19:08 UTC (permalink / raw)
  To: Michael Schmitz; +Cc: linux-m68k, linux-kernel, Geert Uytterhoeven

On Sun, 10 Nov 2013, Michael Schmitz wrote:
> Your patch works just fine, thanks.
> 
> I suppose setting the flag can be done in the corresponding irq startup
> function, instead of when setting up the irq controller?

irq_startup() is called with irq_desc->lock held and
irq_set_status_flags() wants desc->lock as well. Deadlock

And no, you don't want to fiddle manually in the irq descriptor data
fields. See commit a6967caf00eb :)

> Geert - I will send the patch to ataints.c implementing this as soon as
> Thomas' fix is merged.

I'll expedite it for 3.13.

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 09/17] [m68k] IRQ: add handle_polled_irq() for timer based soft interrupts
  2013-11-12 19:08         ` Thomas Gleixner
@ 2013-11-13  7:57           ` Michael Schmitz
  0 siblings, 0 replies; 8+ messages in thread
From: Michael Schmitz @ 2013-11-13  7:57 UTC (permalink / raw)
  To: Thomas Gleixner; +Cc: linux-m68k, linux-kernel, Geert Uytterhoeven

Thomas,

>> I suppose setting the flag can be done in the corresponding irq 
>> startup
>> function, instead of when setting up the irq controller?
>
> irq_startup() is called with irq_desc->lock held and
> irq_set_status_flags() wants desc->lock as well. Deadlock

Thanks, point taken.

> And no, you don't want to fiddle manually in the irq descriptor data
> fields. See commit a6967caf00eb :)

I'm not that crazy :-)

>
>> Geert - I will send the patch to ataints.c implementing this as soon 
>> as
>> Thomas' fix is merged.
>
> I'll expedite it for 3.13.

Thanks, that'll help.

Cheers,

	Michael


>
> Thanks,
>
> 	tglx
> --
> To unsubscribe from this list: send the line "unsubscribe linux-m68k" 
> in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [tip:irq/urgent] genirq: Prevent spurious detection for unconditionally polled interrupts
  2013-11-06 11:30     ` Thomas Gleixner
  2013-11-07  7:59       ` Michael Schmitz
  2013-11-10  8:33       ` Michael Schmitz
@ 2013-11-13 19:42       ` tip-bot for Thomas Gleixner
  2 siblings, 0 replies; 8+ messages in thread
From: tip-bot for Thomas Gleixner @ 2013-11-13 19:42 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: linux-kernel, hpa, mingo, geert, tglx, schmitzmic

Commit-ID:  b39898cd4077f4b6ec706e717c938751c34e1dc4
Gitweb:     http://git.kernel.org/tip/b39898cd4077f4b6ec706e717c938751c34e1dc4
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Wed, 6 Nov 2013 12:30:07 +0100
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Wed, 13 Nov 2013 16:03:02 +0100

genirq: Prevent spurious detection for unconditionally polled interrupts

On a 68k platform a couple of interrupts are demultiplexed and
"polled" from a top level interrupt. Unfortunately there is no way to
determine which of the sub interrupts raised the top level interrupt,
so all of the demultiplexed interrupt handlers need to be
invoked. Given a high enough frequency this can trigger the spurious
interrupt detection mechanism, if one of the demultiplex interrupts
returns IRQ_NONE continuously. But this is a false positive as the
polling causes this behaviour and not buggy hardware/software.

Introduce IRQ_POLLED which can be set at interrupt chip setup time via
irq_set_status_flags(). The flag excludes the interrupt from the
spurious detector and from all core polling activities.

Reported-and-tested-by: Michael Schmitz <schmitzmic@gmail.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: linux-m68k@vger.kernel.org
Link: http://lkml.kernel.org/r/alpine.DEB.2.02.1311061149250.23353@ionos.tec.linutronix.de
---
 include/linux/irq.h   |  7 ++++++-
 kernel/irq/settings.h |  7 +++++++
 kernel/irq/spurious.c | 12 +++++++++---
 3 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index 56bb0dc..7dc1003 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -70,6 +70,9 @@ typedef	void (*irq_preflow_handler_t)(struct irq_data *data);
  * IRQ_MOVE_PCNTXT		- Interrupt can be migrated from process context
  * IRQ_NESTED_TRHEAD		- Interrupt nests into another thread
  * IRQ_PER_CPU_DEVID		- Dev_id is a per-cpu variable
+ * IRQ_IS_POLLED		- Always polled by another interrupt. Exclude
+ *				  it from the spurious interrupt detection
+ *				  mechanism and from core side polling.
  */
 enum {
 	IRQ_TYPE_NONE		= 0x00000000,
@@ -94,12 +97,14 @@ enum {
 	IRQ_NESTED_THREAD	= (1 << 15),
 	IRQ_NOTHREAD		= (1 << 16),
 	IRQ_PER_CPU_DEVID	= (1 << 17),
+	IRQ_IS_POLLED		= (1 << 18),
 };
 
 #define IRQF_MODIFY_MASK	\
 	(IRQ_TYPE_SENSE_MASK | IRQ_NOPROBE | IRQ_NOREQUEST | \
 	 IRQ_NOAUTOEN | IRQ_MOVE_PCNTXT | IRQ_LEVEL | IRQ_NO_BALANCING | \
-	 IRQ_PER_CPU | IRQ_NESTED_THREAD | IRQ_NOTHREAD | IRQ_PER_CPU_DEVID)
+	 IRQ_PER_CPU | IRQ_NESTED_THREAD | IRQ_NOTHREAD | IRQ_PER_CPU_DEVID | \
+	 IRQ_IS_POLLED)
 
 #define IRQ_NO_BALANCING_MASK	(IRQ_PER_CPU | IRQ_NO_BALANCING)
 
diff --git a/kernel/irq/settings.h b/kernel/irq/settings.h
index 1162f10..3320b84 100644
--- a/kernel/irq/settings.h
+++ b/kernel/irq/settings.h
@@ -14,6 +14,7 @@ enum {
 	_IRQ_NO_BALANCING	= IRQ_NO_BALANCING,
 	_IRQ_NESTED_THREAD	= IRQ_NESTED_THREAD,
 	_IRQ_PER_CPU_DEVID	= IRQ_PER_CPU_DEVID,
+	_IRQ_IS_POLLED		= IRQ_IS_POLLED,
 	_IRQF_MODIFY_MASK	= IRQF_MODIFY_MASK,
 };
 
@@ -26,6 +27,7 @@ enum {
 #define IRQ_NOAUTOEN		GOT_YOU_MORON
 #define IRQ_NESTED_THREAD	GOT_YOU_MORON
 #define IRQ_PER_CPU_DEVID	GOT_YOU_MORON
+#define IRQ_IS_POLLED		GOT_YOU_MORON
 #undef IRQF_MODIFY_MASK
 #define IRQF_MODIFY_MASK	GOT_YOU_MORON
 
@@ -147,3 +149,8 @@ static inline bool irq_settings_is_nested_thread(struct irq_desc *desc)
 {
 	return desc->status_use_accessors & _IRQ_NESTED_THREAD;
 }
+
+static inline bool irq_settings_is_polled(struct irq_desc *desc)
+{
+	return desc->status_use_accessors & _IRQ_IS_POLLED;
+}
diff --git a/kernel/irq/spurious.c b/kernel/irq/spurious.c
index 7b5f012..a1d8cc6 100644
--- a/kernel/irq/spurious.c
+++ b/kernel/irq/spurious.c
@@ -67,8 +67,13 @@ static int try_one_irq(int irq, struct irq_desc *desc, bool force)
 
 	raw_spin_lock(&desc->lock);
 
-	/* PER_CPU and nested thread interrupts are never polled */
-	if (irq_settings_is_per_cpu(desc) || irq_settings_is_nested_thread(desc))
+	/*
+	 * PER_CPU, nested thread interrupts and interrupts explicitely
+	 * marked polled are excluded from polling.
+	 */
+	if (irq_settings_is_per_cpu(desc) ||
+	    irq_settings_is_nested_thread(desc) ||
+	    irq_settings_is_polled(desc))
 		goto out;
 
 	/*
@@ -268,7 +273,8 @@ try_misrouted_irq(unsigned int irq, struct irq_desc *desc,
 void note_interrupt(unsigned int irq, struct irq_desc *desc,
 		    irqreturn_t action_ret)
 {
-	if (desc->istate & IRQS_POLL_INPROGRESS)
+	if (desc->istate & IRQS_POLL_INPROGRESS ||
+	    irq_settings_is_polled(desc))
 		return;
 
 	/* we get here again via the threaded handler */

^ permalink raw reply related	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2013-11-13 19:42 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20130131002344.1161E360095@xplor.waratah.dyndns.org>
2013-02-10 10:36 ` [PATCH 09/17] [m68k] IRQ: add handle_polled_irq() for timer based soft interrupts Geert Uytterhoeven
2013-03-20  8:54   ` Geert Uytterhoeven
2013-11-06 11:30     ` Thomas Gleixner
2013-11-07  7:59       ` Michael Schmitz
2013-11-10  8:33       ` Michael Schmitz
2013-11-12 19:08         ` Thomas Gleixner
2013-11-13  7:57           ` Michael Schmitz
2013-11-13 19:42       ` [tip:irq/urgent] genirq: Prevent spurious detection for unconditionally polled interrupts tip-bot for Thomas Gleixner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).