Re: [PATCH v4 3/5] irqchip: Add DT binding doc for the virtual irq demuxer chip

From: Peter Zijlstra
Date: Mon Feb 16 2015 - 04:50:07 EST



Please change the Subject to start with [PATCH] again when including
patches, otherwise its too easy for them to get lost. Esp. with
excessive quoting on top.

I nearly missed the patch here, seeing nothing in the first page of
text.

On Wed, Feb 11, 2015 at 05:13:13PM +0000, Mark Rutland wrote:
> ---
> include/linux/interrupt.h | 5 +++++
> kernel/irq/pm.c | 44 ++++++++++++++++++++++++++++++++++++++++++--
> 2 files changed, 47 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
> index d9b05b5..2b8ff50 100644
> --- a/include/linux/interrupt.h
> +++ b/include/linux/interrupt.h
> @@ -57,6 +57,9 @@
> * IRQF_NO_THREAD - Interrupt cannot be threaded
> * IRQF_EARLY_RESUME - Resume IRQ early during syscore instead of at device
> * resume time.
> + * IRQF_SHARED_TIMER_OK - Interrupt is safe to be shared with a timer. The
> + * handler may be called spuriously during suspend
> + * without issue.

I feel we should do better documenting this; at the very least refer to
Documentation/power/suspend-and-interrupts.txt and ideally put a scary
note in telling people that if they use this as a bandaid to make the
warn go away, they will end up with a broken system.

Now ideally every driver that employs this should also have a comment
next to it how it does indeed behave nicely, such that we can 'quickly'
see people have indeed thought about things and not just slapped it on
to make the WARN go away.

> diff --git a/kernel/irq/pm.c b/kernel/irq/pm.c
> index 3ca5325..e4ec91a 100644
> --- a/kernel/irq/pm.c
> +++ b/kernel/irq/pm.c
> @@ -28,6 +28,47 @@ bool irq_pm_check_wakeup(struct irq_desc *desc)

> + for (action = desc->action; action; action = action->next)
> + if (!(action->flags & safe_flags))
> + return false;

In general I prefer braces around the for loop even though C does not
strictly require it. Its just too easy to confuse multi-line statements
with multiple statements. Extra braces comfort the brain in this case.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/