* [RFC PATCH 0/3] genirq: handling GIC per-cpu interrupts
@ 2011-09-15 16:52 ` Marc Zyngier
0 siblings, 0 replies; 38+ messages in thread
From: Marc Zyngier @ 2011-09-15 16:52 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel
Cc: Bryan Huntsman, David Brown, Kukjin Kim, Magnus Damm, Paul Mundt,
Thomas Gleixner, Tony Lindgren
The current GIC per-cpu interrupts (aka PPIs) suffer from a number of
problems:
- They use a completely separate scheme to handle the interrupts,
mostly because the PPI concept doesn't really match the kernel view
of an interrupt.
- PPIs can only be used by the timer code, unless we add more low-level
assembly code.
- The local timer code can only be used by devices generating PPIs,
and not SPIs.
- At least one platform (msm) has started implementing its own
alternative scheme.
- Some low-level code gets duplicated, as usual...
The proposed solution is to handle the PPIs using the same path as
SPIs. A new core API is added to deal with per-cpu interrupts in a
less awkward way. The local timer code is updated to reflect these
changes.
The core API changes are based on an initial patch by Thomas Gleixner.
Tested on ARM Versatile Express (Cortex A5 and A15), ARM RealView
PB11MP, OMAP4 (Panda) and Tegra (Harmony). Patch series against
next-20110831.
Marc Zyngier (3):
genirq: add support for per-cpu dev_id interrupts
ARM: gic: consolidate PPI handling
ARM: gic, local timers: use the request_percpu_irq() interface
arch/arm/common/Kconfig | 1 +
arch/arm/common/gic.c | 38 +++-
arch/arm/include/asm/entry-macro-multi.S | 7 -
arch/arm/include/asm/hardirq.h | 3 -
arch/arm/include/asm/hardware/entry-macro-gic.S | 19 +--
arch/arm/include/asm/hardware/gic.h | 1 -
arch/arm/include/asm/localtimer.h | 19 +-
arch/arm/include/asm/smp.h | 5 -
arch/arm/include/asm/smp_twd.h | 2 +-
arch/arm/kernel/irq.c | 3 -
arch/arm/kernel/smp.c | 33 +---
arch/arm/kernel/smp_twd.c | 47 +++++-
arch/arm/mach-exynos4/include/mach/entry-macro.S | 6 +-
arch/arm/mach-exynos4/mct.c | 5 -
arch/arm/mach-msm/board-msm8x60.c | 11 -
arch/arm/mach-msm/include/mach/entry-macro-qgic.S | 73 +-------
arch/arm/mach-msm/timer.c | 69 ++++---
arch/arm/mach-omap2/include/mach/entry-macro.S | 14 +--
arch/arm/mach-shmobile/entry-intc.S | 3 -
arch/arm/mach-shmobile/include/mach/entry-macro.S | 3 -
include/linux/interrupt.h | 40 +++-
include/linux/irq.h | 25 +++-
include/linux/irqdesc.h | 3 +
kernel/irq/Kconfig | 4 +
kernel/irq/chip.c | 58 ++++++
kernel/irq/internals.h | 2 +
kernel/irq/manage.c | 209 ++++++++++++++++++++-
kernel/irq/settings.h | 7 +
28 files changed, 461 insertions(+), 249 deletions(-)
^ permalink raw reply [flat|nested] 38+ messages in thread
* [RFC PATCH 0/3] genirq: handling GIC per-cpu interrupts
@ 2011-09-15 16:52 ` Marc Zyngier
0 siblings, 0 replies; 38+ messages in thread
From: Marc Zyngier @ 2011-09-15 16:52 UTC (permalink / raw)
To: linux-arm-kernel
The current GIC per-cpu interrupts (aka PPIs) suffer from a number of
problems:
- They use a completely separate scheme to handle the interrupts,
mostly because the PPI concept doesn't really match the kernel view
of an interrupt.
- PPIs can only be used by the timer code, unless we add more low-level
assembly code.
- The local timer code can only be used by devices generating PPIs,
and not SPIs.
- At least one platform (msm) has started implementing its own
alternative scheme.
- Some low-level code gets duplicated, as usual...
The proposed solution is to handle the PPIs using the same path as
SPIs. A new core API is added to deal with per-cpu interrupts in a
less awkward way. The local timer code is updated to reflect these
changes.
The core API changes are based on an initial patch by Thomas Gleixner.
Tested on ARM Versatile Express (Cortex A5 and A15), ARM RealView
PB11MP, OMAP4 (Panda) and Tegra (Harmony). Patch series against
next-20110831.
Marc Zyngier (3):
genirq: add support for per-cpu dev_id interrupts
ARM: gic: consolidate PPI handling
ARM: gic, local timers: use the request_percpu_irq() interface
arch/arm/common/Kconfig | 1 +
arch/arm/common/gic.c | 38 +++-
arch/arm/include/asm/entry-macro-multi.S | 7 -
arch/arm/include/asm/hardirq.h | 3 -
arch/arm/include/asm/hardware/entry-macro-gic.S | 19 +--
arch/arm/include/asm/hardware/gic.h | 1 -
arch/arm/include/asm/localtimer.h | 19 +-
arch/arm/include/asm/smp.h | 5 -
arch/arm/include/asm/smp_twd.h | 2 +-
arch/arm/kernel/irq.c | 3 -
arch/arm/kernel/smp.c | 33 +---
arch/arm/kernel/smp_twd.c | 47 +++++-
arch/arm/mach-exynos4/include/mach/entry-macro.S | 6 +-
arch/arm/mach-exynos4/mct.c | 5 -
arch/arm/mach-msm/board-msm8x60.c | 11 -
arch/arm/mach-msm/include/mach/entry-macro-qgic.S | 73 +-------
arch/arm/mach-msm/timer.c | 69 ++++---
arch/arm/mach-omap2/include/mach/entry-macro.S | 14 +--
arch/arm/mach-shmobile/entry-intc.S | 3 -
arch/arm/mach-shmobile/include/mach/entry-macro.S | 3 -
include/linux/interrupt.h | 40 +++-
include/linux/irq.h | 25 +++-
include/linux/irqdesc.h | 3 +
kernel/irq/Kconfig | 4 +
kernel/irq/chip.c | 58 ++++++
kernel/irq/internals.h | 2 +
kernel/irq/manage.c | 209 ++++++++++++++++++++-
kernel/irq/settings.h | 7 +
28 files changed, 461 insertions(+), 249 deletions(-)
^ permalink raw reply [flat|nested] 38+ messages in thread
* [RFC PATCH 1/3] genirq: add support for per-cpu dev_id interrupts
2011-09-15 16:52 ` Marc Zyngier
@ 2011-09-15 16:52 ` Marc Zyngier
-1 siblings, 0 replies; 38+ messages in thread
From: Marc Zyngier @ 2011-09-15 16:52 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel; +Cc: Thomas Gleixner
The ARM GIC interrupt controller offers per CPU interrupts (PPIs),
which are usually used to connect local timers to each core.
Each CPU has its own private interface to the GIC,
and only sees the PPIs that are directly connect to it.
While these timers are separate devices and have a separate
interrupt line to a core, they all use the same IRQ number.
For these devices, request_irq() is not the right API as it
assumes that an IRQ number is visible by a number of CPUs
(through the affinity setting), but makes it very awkward to
express that an IRQ number can be handled by all CPUs, and
yet be a different interrupt line on each CPU, requiring a
different dev_id cookie to be passed back to the handler.
The *_percpu_irq() functions is designed to overcome these
limitations, by providing a per-cpu dev_id vector:
int request_percpu_irq(unsigned int irq, irq_handler_t handler,
const char *devname, void __percpu *percpu_dev_id);
void free_percpu_irq(unsigned int, void __percpu *);
int setup_percpu_irq(unsigned int irq, struct irqaction *new);
void remove_percpu_irq(unsigned int irq, struct irqaction *act);
void enable_percpu_irq(unsigned int irq);
void disable_percpu_irq(unsigned int irq);
The API has a number of limitations:
- no interrupt sharing
- no threading
- common handler across all the CPUs
Once the interrupt is requested using setup_percpu_irq() or
request_percpu_irq(), it must be enabled by each core that wishes
its local interrupt to be delivered.
Based on an initial patch by Thomas Gleixner.
Cc: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
include/linux/interrupt.h | 40 ++++++---
include/linux/irq.h | 25 +++++-
include/linux/irqdesc.h | 3 +
kernel/irq/Kconfig | 4 +
kernel/irq/chip.c | 58 +++++++++++++
kernel/irq/internals.h | 2 +
kernel/irq/manage.c | 209 ++++++++++++++++++++++++++++++++++++++++++++-
kernel/irq/settings.h | 7 ++
8 files changed, 332 insertions(+), 16 deletions(-)
diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index a103732..f9b7fa3 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -95,6 +95,7 @@ typedef irqreturn_t (*irq_handler_t)(int, void *);
* @flags: flags (see IRQF_* above)
* @name: name of the device
* @dev_id: cookie to identify the device
+ * @percpu_dev_id: cookie to identify the device
* @next: pointer to the next irqaction for shared interrupts
* @irq: interrupt number
* @dir: pointer to the proc/irq/NN/name entry
@@ -104,17 +105,20 @@ typedef irqreturn_t (*irq_handler_t)(int, void *);
* @thread_mask: bitmask for keeping track of @thread activity
*/
struct irqaction {
- irq_handler_t handler;
- unsigned long flags;
- void *dev_id;
- struct irqaction *next;
- int irq;
- irq_handler_t thread_fn;
- struct task_struct *thread;
- unsigned long thread_flags;
- unsigned long thread_mask;
- const char *name;
- struct proc_dir_entry *dir;
+ irq_handler_t handler;
+ unsigned long flags;
+ void *dev_id;
+#ifdef CONFIG_IRQ_PERCPU_DEVID
+ void __percpu *percpu_dev_id;
+#endif
+ struct irqaction *next;
+ int irq;
+ irq_handler_t thread_fn;
+ struct task_struct *thread;
+ unsigned long thread_flags;
+ unsigned long thread_mask;
+ const char *name;
+ struct proc_dir_entry *dir;
} ____cacheline_internodealigned_in_smp;
extern irqreturn_t no_action(int cpl, void *dev_id);
@@ -136,6 +140,10 @@ extern int __must_check
request_any_context_irq(unsigned int irq, irq_handler_t handler,
unsigned long flags, const char *name, void *dev_id);
+extern int __must_check
+request_percpu_irq(unsigned int irq, irq_handler_t handler,
+ const char *devname, void __percpu *percpu_dev_id);
+
extern void exit_irq_thread(void);
#else
@@ -164,10 +172,18 @@ request_any_context_irq(unsigned int irq, irq_handler_t handler,
return request_irq(irq, handler, flags, name, dev_id);
}
+static inline int __must_check
+request_percpu_irq(unsigned int irq, irq_handler_t handler,
+ const char *devname, void __percpu *percpu_dev_id)
+{
+ return request_irq(irq, handler, 0, name, dev_id);
+}
+
static inline void exit_irq_thread(void) { }
#endif
extern void free_irq(unsigned int, void *);
+extern void free_percpu_irq(unsigned int, void __percpu *);
struct device;
@@ -207,7 +223,9 @@ extern void devm_free_irq(struct device *dev, unsigned int irq, void *dev_id);
extern void disable_irq_nosync(unsigned int irq);
extern void disable_irq(unsigned int irq);
+extern void disable_percpu_irq(unsigned int irq);
extern void enable_irq(unsigned int irq);
+extern void enable_percpu_irq(unsigned int irq);
/* The following three functions are for the core kernel use only. */
#ifdef CONFIG_GENERIC_HARDIRQS
diff --git a/include/linux/irq.h b/include/linux/irq.h
index 5951730..1e14fd1 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -66,6 +66,7 @@ typedef void (*irq_preflow_handler_t)(struct irq_data *data);
* IRQ_NO_BALANCING - Interrupt cannot be balanced (affinity set)
* 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
*/
enum {
IRQ_TYPE_NONE = 0x00000000,
@@ -88,12 +89,13 @@ enum {
IRQ_MOVE_PCNTXT = (1 << 14),
IRQ_NESTED_THREAD = (1 << 15),
IRQ_NOTHREAD = (1 << 16),
+ IRQ_PER_CPU_DEVID = (1 << 17),
};
#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_PER_CPU | IRQ_NESTED_THREAD | IRQ_NOTHREAD | IRQ_PER_CPU_DEVID)
#define IRQ_NO_BALANCING_MASK (IRQ_PER_CPU | IRQ_NO_BALANCING)
@@ -365,6 +367,8 @@ enum {
struct irqaction;
extern int setup_irq(unsigned int irq, struct irqaction *new);
extern void remove_irq(unsigned int irq, struct irqaction *act);
+extern int setup_percpu_irq(unsigned int irq, struct irqaction *new);
+extern void remove_percpu_irq(unsigned int irq, struct irqaction *act);
extern void irq_cpu_online(void);
extern void irq_cpu_offline(void);
@@ -392,6 +396,7 @@ 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_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);
extern void handle_nested_irq(unsigned int irq);
@@ -481,6 +486,24 @@ static inline void irq_set_nested_thread(unsigned int irq, bool nest)
irq_clear_status_flags(irq, IRQ_NESTED_THREAD);
}
+#ifdef CONFIG_IRQ_PERCPU_DEVID
+static inline int irq_set_percpu_devid(unsigned int irq)
+{
+ struct irq_desc *desc = irq_to_desc(irq);
+
+ if (!desc)
+ return -EINVAL;
+
+ if (!zalloc_cpumask_var(&desc->percpu_enabled, GFP_KERNEL))
+ return -ENOMEM;
+
+ irq_set_status_flags(irq,
+ IRQ_NOAUTOEN | IRQ_PER_CPU | IRQ_NOTHREAD |
+ IRQ_NOPROBE | IRQ_PER_CPU_DEVID);
+ return 0;
+}
+#endif
+
/* Handle dynamic irq creation and destruction */
extern unsigned int create_irq_nr(unsigned int irq_want, int node);
extern int create_irq(void);
diff --git a/include/linux/irqdesc.h b/include/linux/irqdesc.h
index 150134a..0b4419a 100644
--- a/include/linux/irqdesc.h
+++ b/include/linux/irqdesc.h
@@ -53,6 +53,9 @@ struct irq_desc {
unsigned long last_unhandled; /* Aging timer for unhandled count */
unsigned int irqs_unhandled;
raw_spinlock_t lock;
+#ifdef CONFIG_IRQ_PERCPU_DEVID
+ cpumask_var_t percpu_enabled;
+#endif
#ifdef CONFIG_SMP
const struct cpumask *affinity_hint;
struct irq_affinity_notify *affinity_notify;
diff --git a/kernel/irq/Kconfig b/kernel/irq/Kconfig
index 5a38bf4..75c0631 100644
--- a/kernel/irq/Kconfig
+++ b/kernel/irq/Kconfig
@@ -60,6 +60,10 @@ config IRQ_DOMAIN
config IRQ_FORCED_THREADING
bool
+# Support per CPU dev id
+config IRQ_PERCPU_DEVID
+ bool
+
config SPARSE_IRQ
bool "Support sparse irq numbering"
depends on HAVE_SPARSE_IRQ
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index d5a3009..d65b23f 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -204,6 +204,30 @@ void irq_disable(struct irq_desc *desc)
}
}
+#ifdef CONFIG_IRQ_PERCPU_DEVID
+void irq_percpu_enable(struct irq_desc *desc)
+{
+ unsigned int cpu = get_cpu();
+ if (desc->irq_data.chip->irq_enable)
+ desc->irq_data.chip->irq_enable(&desc->irq_data);
+ else
+ desc->irq_data.chip->irq_unmask(&desc->irq_data);
+ cpumask_set_cpu(cpu, desc->percpu_enabled);
+ put_cpu();
+}
+
+void irq_percpu_disable(struct irq_desc *desc)
+{
+ unsigned int cpu = get_cpu();
+ if (desc->irq_data.chip->irq_disable) {
+ desc->irq_data.chip->irq_disable(&desc->irq_data);
+ irq_state_set_masked(desc);
+ }
+ cpumask_clear_cpu(cpu, desc->percpu_enabled);
+ put_cpu();
+}
+#endif
+
static inline void mask_ack_irq(struct irq_desc *desc)
{
if (desc->irq_data.chip->irq_mask_ack)
@@ -544,6 +568,40 @@ handle_percpu_irq(unsigned int irq, struct irq_desc *desc)
chip->irq_eoi(&desc->irq_data);
}
+#ifdef CONFIG_IRQ_PERCPU_DEVID
+/**
+ * handle_percpu_devid_irq - Per CPU local irq handler with per cpu dev ids
+ * @irq: the interrupt number
+ * @desc: the interrupt description structure for this irq
+ *
+ * Per CPU interrupts on SMP machines without locking requirements. Same as
+ * handle_percpu_irq() above but with the following extras:
+ *
+ * action->percpu_dev_id is a pointer to percpu variables which
+ * contain the real device id for the cpu on which this handler is
+ * called
+ */
+void handle_percpu_devid_irq(unsigned int irq, struct irq_desc *desc)
+{
+ struct irq_chip *chip = irq_desc_get_chip(desc);
+ struct irqaction *action = desc->action;
+ void *dev_id = __this_cpu_ptr(action->percpu_dev_id);
+ irqreturn_t res;
+
+ kstat_incr_irqs_this_cpu(irq, desc);
+
+ if (chip->irq_ack)
+ chip->irq_ack(&desc->irq_data);
+
+ trace_irq_handler_entry(irq, action);
+ res = action->handler(irq, dev_id);
+ trace_irq_handler_exit(irq, action, res);
+
+ if (chip->irq_eoi)
+ chip->irq_eoi(&desc->irq_data);
+}
+#endif
+
void
__irq_set_handler(unsigned int irq, irq_flow_handler_t handle, int is_chained,
const char *name)
diff --git a/kernel/irq/internals.h b/kernel/irq/internals.h
index 6546431..a57fd3b 100644
--- a/kernel/irq/internals.h
+++ b/kernel/irq/internals.h
@@ -71,6 +71,8 @@ extern int irq_startup(struct irq_desc *desc);
extern void irq_shutdown(struct irq_desc *desc);
extern void irq_enable(struct irq_desc *desc);
extern void irq_disable(struct irq_desc *desc);
+extern void irq_percpu_enable(struct irq_desc *desc);
+extern void irq_percpu_disable(struct irq_desc *desc);
extern void mask_irq(struct irq_desc *desc);
extern void unmask_irq(struct irq_desc *desc);
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index 9b956fa..9f10b07 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -1118,6 +1118,8 @@ int setup_irq(unsigned int irq, struct irqaction *act)
int retval;
struct irq_desc *desc = irq_to_desc(irq);
+ if (irq_settings_is_per_cpu_devid(desc))
+ return -EINVAL;
chip_bus_lock(desc);
retval = __setup_irq(irq, desc, act);
chip_bus_sync_unlock(desc);
@@ -1126,7 +1128,7 @@ int setup_irq(unsigned int irq, struct irqaction *act)
}
EXPORT_SYMBOL_GPL(setup_irq);
- /*
+/*
* Internal function to unregister an irqaction - used to free
* regular and special interrupts that are part of the architecture.
*/
@@ -1224,7 +1226,10 @@ static struct irqaction *__free_irq(unsigned int irq, void *dev_id)
*/
void remove_irq(unsigned int irq, struct irqaction *act)
{
- __free_irq(irq, act->dev_id);
+ struct irq_desc *desc = irq_to_desc(irq);
+
+ if (desc && !irq_settings_is_per_cpu_devid(desc))
+ __free_irq(irq, act->dev_id);
}
EXPORT_SYMBOL_GPL(remove_irq);
@@ -1246,7 +1251,7 @@ void free_irq(unsigned int irq, void *dev_id)
{
struct irq_desc *desc = irq_to_desc(irq);
- if (!desc)
+ if (!desc || irq_settings_is_per_cpu_devid(desc))
return;
#ifdef CONFIG_SMP
@@ -1324,7 +1329,8 @@ int request_threaded_irq(unsigned int irq, irq_handler_t handler,
if (!desc)
return -EINVAL;
- if (!irq_settings_can_request(desc))
+ if (!irq_settings_can_request(desc) ||
+ irq_settings_is_per_cpu_devid(desc))
return -EINVAL;
if (!handler) {
@@ -1409,3 +1415,198 @@ int request_any_context_irq(unsigned int irq, irq_handler_t handler,
return !ret ? IRQC_IS_HARDIRQ : ret;
}
EXPORT_SYMBOL_GPL(request_any_context_irq);
+
+#ifdef CONFIG_IRQ_PERCPU_DEVID
+void enable_percpu_irq(unsigned int irq)
+{
+ unsigned long flags;
+ struct irq_desc *desc = irq_get_desc_buslock(irq, &flags);
+
+ if (!desc)
+ return;
+
+ irq_percpu_enable(desc);
+ irq_put_desc_busunlock(desc, flags);
+}
+EXPORT_SYMBOL(enable_percpu_irq);
+
+void disable_percpu_irq(unsigned int irq)
+{
+ unsigned long flags;
+ struct irq_desc *desc = irq_get_desc_buslock(irq, &flags);
+
+ if (!desc)
+ return;
+
+ irq_percpu_disable(desc);
+ irq_put_desc_busunlock(desc, flags);
+}
+EXPORT_SYMBOL(disable_percpu_irq);
+
+/*
+ * Internal function to unregister a percpu irqaction.
+ */
+static struct irqaction *__free_percpu_irq(unsigned int irq, void __percpu *dev_id)
+{
+ struct irq_desc *desc = irq_to_desc(irq);
+ struct irqaction *action;
+ unsigned long flags;
+
+ WARN(in_interrupt(), "Trying to free IRQ %d from IRQ context!\n", irq);
+
+ if (!desc)
+ return NULL;
+
+ raw_spin_lock_irqsave(&desc->lock, flags);
+
+ action = desc->action;
+ if (!action || action->percpu_dev_id != dev_id) {
+ WARN(1, "Trying to free already-free IRQ %d\n", irq);
+ raw_spin_unlock_irqrestore(&desc->lock, flags);
+ return NULL;
+ }
+
+ /* Found it - now remove it from the list of entries: */
+ WARN(!cpumask_empty(desc->percpu_enabled),
+ "percpu IRQ %d still enabled on CPU%d!\n",
+ irq, cpumask_first(desc->percpu_enabled));
+ desc->action = NULL;
+
+#ifdef CONFIG_SMP
+ /* make sure affinity_hint is cleaned up */
+ if (WARN_ON_ONCE(desc->affinity_hint))
+ desc->affinity_hint = NULL;
+#endif
+
+ raw_spin_unlock_irqrestore(&desc->lock, flags);
+
+ unregister_handler_proc(irq, action);
+
+ /* Make sure it's not being used on another CPU: */
+ synchronize_irq(irq);
+
+ module_put(desc->owner);
+ return action;
+}
+
+/**
+ * remove_percpu_irq - free a per-cpu interrupt
+ * @irq: Interrupt line to free
+ * @act: irqaction for the interrupt
+ *
+ * Used to remove interrupts statically setup by the early boot process.
+ */
+void remove_percpu_irq(unsigned int irq, struct irqaction *act)
+{
+ struct irq_desc *desc = irq_to_desc(irq);
+
+ if (desc && irq_settings_is_per_cpu_devid(desc))
+ __free_percpu_irq(irq, act->percpu_dev_id);
+}
+EXPORT_SYMBOL_GPL(remove_percpu_irq);
+
+/**
+ * free_percpu_irq - free an interrupt allocated with request_percpu_irq
+ * @irq: Interrupt line to free
+ * @dev_id: Device identity to free
+ *
+ * Remove a percpu interrupt handler. The handler is removed, but
+ * the interrupt line is not disabled. This must be done on each
+ * CPU before calling this function. The function does not return
+ * until any executing interrupts for this IRQ have completed.
+ *
+ * This function must not be called from interrupt context.
+ */
+void free_percpu_irq(unsigned int irq, void __percpu *dev_id)
+{
+ struct irq_desc *desc = irq_to_desc(irq);
+
+ if (!desc || !irq_settings_is_per_cpu_devid(desc))
+ return;
+
+#ifdef CONFIG_SMP
+ if (WARN_ON(desc->affinity_notify))
+ desc->affinity_notify = NULL;
+#endif
+
+ chip_bus_lock(desc);
+ kfree(__free_percpu_irq(irq, dev_id));
+ chip_bus_sync_unlock(desc);
+}
+EXPORT_SYMBOL(free_percpu_irq);
+
+/**
+ * setup_percpu_irq - setup a per-cpu interrupt
+ * @irq: Interrupt line to setup
+ * @act: irqaction for the interrupt
+ *
+ * Used to statically setup per-cpu interrupts in the early boot process.
+ */
+int setup_percpu_irq(unsigned int irq, struct irqaction *act)
+{
+ int retval;
+ struct irq_desc *desc = irq_to_desc(irq);
+
+ if (!irq_settings_is_per_cpu_devid(desc))
+ return -EINVAL;
+ chip_bus_lock(desc);
+ retval = __setup_irq(irq, desc, act);
+ chip_bus_sync_unlock(desc);
+
+ return retval;
+}
+EXPORT_SYMBOL_GPL(setup_percpu_irq);
+
+/**
+ * request_percpu_irq - allocate a percpu interrupt line
+ * @irq: Interrupt line to allocate
+ * @handler: Function to be called when the IRQ occurs.
+ * Primary handler for threaded interrupts
+ * If NULL and thread_fn != NULL the default
+ * primary handler is installed
+ * @devname: An ascii name for the claiming device
+ * @dev_id: A percpu cookie passed back to the handler function
+ *
+ * This call allocates interrupt resources, but doesn't
+ * automatically enable the interrupt. It has to be done on each
+ * CPU using enable_percpu_irq().
+ *
+ * Dev_id must be globally unique. It is a per-cpu variable, and
+ * the handler gets called with the interrupted CPU's instance of
+ * that variable.
+ */
+int request_percpu_irq(unsigned int irq, irq_handler_t handler,
+ const char *devname, void __percpu *dev_id)
+{
+ struct irqaction *action;
+ struct irq_desc *desc;
+ int retval;
+
+ if (!dev_id)
+ return -EINVAL;
+
+ desc = irq_to_desc(irq);
+ if (!desc || !irq_settings_can_request(desc) ||
+ !irq_settings_is_per_cpu_devid(desc))
+ return -EINVAL;
+
+ action = kzalloc(sizeof(struct irqaction), GFP_KERNEL);
+ if (!action)
+ return -ENOMEM;
+
+ action->handler = handler;
+ action->flags = IRQF_PERCPU;
+ action->name = devname;
+ action->percpu_dev_id = dev_id;
+
+ chip_bus_lock(desc);
+ retval = __setup_irq(irq, desc, action);
+ chip_bus_sync_unlock(desc);
+
+ if (retval)
+ kfree(action);
+
+ return retval;
+}
+EXPORT_SYMBOL(request_percpu_irq);
+#endif
diff --git a/kernel/irq/settings.h b/kernel/irq/settings.h
index f166783..1162f10 100644
--- a/kernel/irq/settings.h
+++ b/kernel/irq/settings.h
@@ -13,6 +13,7 @@ enum {
_IRQ_MOVE_PCNTXT = IRQ_MOVE_PCNTXT,
_IRQ_NO_BALANCING = IRQ_NO_BALANCING,
_IRQ_NESTED_THREAD = IRQ_NESTED_THREAD,
+ _IRQ_PER_CPU_DEVID = IRQ_PER_CPU_DEVID,
_IRQF_MODIFY_MASK = IRQF_MODIFY_MASK,
};
@@ -24,6 +25,7 @@ enum {
#define IRQ_NOTHREAD GOT_YOU_MORON
#define IRQ_NOAUTOEN GOT_YOU_MORON
#define IRQ_NESTED_THREAD GOT_YOU_MORON
+#define IRQ_PER_CPU_DEVID GOT_YOU_MORON
#undef IRQF_MODIFY_MASK
#define IRQF_MODIFY_MASK GOT_YOU_MORON
@@ -39,6 +41,11 @@ static inline bool irq_settings_is_per_cpu(struct irq_desc *desc)
return desc->status_use_accessors & _IRQ_PER_CPU;
}
+static inline bool irq_settings_is_per_cpu_devid(struct irq_desc *desc)
+{
+ return desc->status_use_accessors & _IRQ_PER_CPU_DEVID;
+}
+
static inline void irq_settings_set_per_cpu(struct irq_desc *desc)
{
desc->status_use_accessors |= _IRQ_PER_CPU;
--
1.7.0.4
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [RFC PATCH 1/3] genirq: add support for per-cpu dev_id interrupts
@ 2011-09-15 16:52 ` Marc Zyngier
0 siblings, 0 replies; 38+ messages in thread
From: Marc Zyngier @ 2011-09-15 16:52 UTC (permalink / raw)
To: linux-arm-kernel
The ARM GIC interrupt controller offers per CPU interrupts (PPIs),
which are usually used to connect local timers to each core.
Each CPU has its own private interface to the GIC,
and only sees the PPIs that are directly connect to it.
While these timers are separate devices and have a separate
interrupt line to a core, they all use the same IRQ number.
For these devices, request_irq() is not the right API as it
assumes that an IRQ number is visible by a number of CPUs
(through the affinity setting), but makes it very awkward to
express that an IRQ number can be handled by all CPUs, and
yet be a different interrupt line on each CPU, requiring a
different dev_id cookie to be passed back to the handler.
The *_percpu_irq() functions is designed to overcome these
limitations, by providing a per-cpu dev_id vector:
int request_percpu_irq(unsigned int irq, irq_handler_t handler,
const char *devname, void __percpu *percpu_dev_id);
void free_percpu_irq(unsigned int, void __percpu *);
int setup_percpu_irq(unsigned int irq, struct irqaction *new);
void remove_percpu_irq(unsigned int irq, struct irqaction *act);
void enable_percpu_irq(unsigned int irq);
void disable_percpu_irq(unsigned int irq);
The API has a number of limitations:
- no interrupt sharing
- no threading
- common handler across all the CPUs
Once the interrupt is requested using setup_percpu_irq() or
request_percpu_irq(), it must be enabled by each core that wishes
its local interrupt to be delivered.
Based on an initial patch by Thomas Gleixner.
Cc: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
include/linux/interrupt.h | 40 ++++++---
include/linux/irq.h | 25 +++++-
include/linux/irqdesc.h | 3 +
kernel/irq/Kconfig | 4 +
kernel/irq/chip.c | 58 +++++++++++++
kernel/irq/internals.h | 2 +
kernel/irq/manage.c | 209 ++++++++++++++++++++++++++++++++++++++++++++-
kernel/irq/settings.h | 7 ++
8 files changed, 332 insertions(+), 16 deletions(-)
diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index a103732..f9b7fa3 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -95,6 +95,7 @@ typedef irqreturn_t (*irq_handler_t)(int, void *);
* @flags: flags (see IRQF_* above)
* @name: name of the device
* @dev_id: cookie to identify the device
+ * @percpu_dev_id: cookie to identify the device
* @next: pointer to the next irqaction for shared interrupts
* @irq: interrupt number
* @dir: pointer to the proc/irq/NN/name entry
@@ -104,17 +105,20 @@ typedef irqreturn_t (*irq_handler_t)(int, void *);
* @thread_mask: bitmask for keeping track of @thread activity
*/
struct irqaction {
- irq_handler_t handler;
- unsigned long flags;
- void *dev_id;
- struct irqaction *next;
- int irq;
- irq_handler_t thread_fn;
- struct task_struct *thread;
- unsigned long thread_flags;
- unsigned long thread_mask;
- const char *name;
- struct proc_dir_entry *dir;
+ irq_handler_t handler;
+ unsigned long flags;
+ void *dev_id;
+#ifdef CONFIG_IRQ_PERCPU_DEVID
+ void __percpu *percpu_dev_id;
+#endif
+ struct irqaction *next;
+ int irq;
+ irq_handler_t thread_fn;
+ struct task_struct *thread;
+ unsigned long thread_flags;
+ unsigned long thread_mask;
+ const char *name;
+ struct proc_dir_entry *dir;
} ____cacheline_internodealigned_in_smp;
extern irqreturn_t no_action(int cpl, void *dev_id);
@@ -136,6 +140,10 @@ extern int __must_check
request_any_context_irq(unsigned int irq, irq_handler_t handler,
unsigned long flags, const char *name, void *dev_id);
+extern int __must_check
+request_percpu_irq(unsigned int irq, irq_handler_t handler,
+ const char *devname, void __percpu *percpu_dev_id);
+
extern void exit_irq_thread(void);
#else
@@ -164,10 +172,18 @@ request_any_context_irq(unsigned int irq, irq_handler_t handler,
return request_irq(irq, handler, flags, name, dev_id);
}
+static inline int __must_check
+request_percpu_irq(unsigned int irq, irq_handler_t handler,
+ const char *devname, void __percpu *percpu_dev_id)
+{
+ return request_irq(irq, handler, 0, name, dev_id);
+}
+
static inline void exit_irq_thread(void) { }
#endif
extern void free_irq(unsigned int, void *);
+extern void free_percpu_irq(unsigned int, void __percpu *);
struct device;
@@ -207,7 +223,9 @@ extern void devm_free_irq(struct device *dev, unsigned int irq, void *dev_id);
extern void disable_irq_nosync(unsigned int irq);
extern void disable_irq(unsigned int irq);
+extern void disable_percpu_irq(unsigned int irq);
extern void enable_irq(unsigned int irq);
+extern void enable_percpu_irq(unsigned int irq);
/* The following three functions are for the core kernel use only. */
#ifdef CONFIG_GENERIC_HARDIRQS
diff --git a/include/linux/irq.h b/include/linux/irq.h
index 5951730..1e14fd1 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -66,6 +66,7 @@ typedef void (*irq_preflow_handler_t)(struct irq_data *data);
* IRQ_NO_BALANCING - Interrupt cannot be balanced (affinity set)
* 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
*/
enum {
IRQ_TYPE_NONE = 0x00000000,
@@ -88,12 +89,13 @@ enum {
IRQ_MOVE_PCNTXT = (1 << 14),
IRQ_NESTED_THREAD = (1 << 15),
IRQ_NOTHREAD = (1 << 16),
+ IRQ_PER_CPU_DEVID = (1 << 17),
};
#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_PER_CPU | IRQ_NESTED_THREAD | IRQ_NOTHREAD | IRQ_PER_CPU_DEVID)
#define IRQ_NO_BALANCING_MASK (IRQ_PER_CPU | IRQ_NO_BALANCING)
@@ -365,6 +367,8 @@ enum {
struct irqaction;
extern int setup_irq(unsigned int irq, struct irqaction *new);
extern void remove_irq(unsigned int irq, struct irqaction *act);
+extern int setup_percpu_irq(unsigned int irq, struct irqaction *new);
+extern void remove_percpu_irq(unsigned int irq, struct irqaction *act);
extern void irq_cpu_online(void);
extern void irq_cpu_offline(void);
@@ -392,6 +396,7 @@ 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_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);
extern void handle_nested_irq(unsigned int irq);
@@ -481,6 +486,24 @@ static inline void irq_set_nested_thread(unsigned int irq, bool nest)
irq_clear_status_flags(irq, IRQ_NESTED_THREAD);
}
+#ifdef CONFIG_IRQ_PERCPU_DEVID
+static inline int irq_set_percpu_devid(unsigned int irq)
+{
+ struct irq_desc *desc = irq_to_desc(irq);
+
+ if (!desc)
+ return -EINVAL;
+
+ if (!zalloc_cpumask_var(&desc->percpu_enabled, GFP_KERNEL))
+ return -ENOMEM;
+
+ irq_set_status_flags(irq,
+ IRQ_NOAUTOEN | IRQ_PER_CPU | IRQ_NOTHREAD |
+ IRQ_NOPROBE | IRQ_PER_CPU_DEVID);
+ return 0;
+}
+#endif
+
/* Handle dynamic irq creation and destruction */
extern unsigned int create_irq_nr(unsigned int irq_want, int node);
extern int create_irq(void);
diff --git a/include/linux/irqdesc.h b/include/linux/irqdesc.h
index 150134a..0b4419a 100644
--- a/include/linux/irqdesc.h
+++ b/include/linux/irqdesc.h
@@ -53,6 +53,9 @@ struct irq_desc {
unsigned long last_unhandled; /* Aging timer for unhandled count */
unsigned int irqs_unhandled;
raw_spinlock_t lock;
+#ifdef CONFIG_IRQ_PERCPU_DEVID
+ cpumask_var_t percpu_enabled;
+#endif
#ifdef CONFIG_SMP
const struct cpumask *affinity_hint;
struct irq_affinity_notify *affinity_notify;
diff --git a/kernel/irq/Kconfig b/kernel/irq/Kconfig
index 5a38bf4..75c0631 100644
--- a/kernel/irq/Kconfig
+++ b/kernel/irq/Kconfig
@@ -60,6 +60,10 @@ config IRQ_DOMAIN
config IRQ_FORCED_THREADING
bool
+# Support per CPU dev id
+config IRQ_PERCPU_DEVID
+ bool
+
config SPARSE_IRQ
bool "Support sparse irq numbering"
depends on HAVE_SPARSE_IRQ
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index d5a3009..d65b23f 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -204,6 +204,30 @@ void irq_disable(struct irq_desc *desc)
}
}
+#ifdef CONFIG_IRQ_PERCPU_DEVID
+void irq_percpu_enable(struct irq_desc *desc)
+{
+ unsigned int cpu = get_cpu();
+ if (desc->irq_data.chip->irq_enable)
+ desc->irq_data.chip->irq_enable(&desc->irq_data);
+ else
+ desc->irq_data.chip->irq_unmask(&desc->irq_data);
+ cpumask_set_cpu(cpu, desc->percpu_enabled);
+ put_cpu();
+}
+
+void irq_percpu_disable(struct irq_desc *desc)
+{
+ unsigned int cpu = get_cpu();
+ if (desc->irq_data.chip->irq_disable) {
+ desc->irq_data.chip->irq_disable(&desc->irq_data);
+ irq_state_set_masked(desc);
+ }
+ cpumask_clear_cpu(cpu, desc->percpu_enabled);
+ put_cpu();
+}
+#endif
+
static inline void mask_ack_irq(struct irq_desc *desc)
{
if (desc->irq_data.chip->irq_mask_ack)
@@ -544,6 +568,40 @@ handle_percpu_irq(unsigned int irq, struct irq_desc *desc)
chip->irq_eoi(&desc->irq_data);
}
+#ifdef CONFIG_IRQ_PERCPU_DEVID
+/**
+ * handle_percpu_devid_irq - Per CPU local irq handler with per cpu dev ids
+ * @irq: the interrupt number
+ * @desc: the interrupt description structure for this irq
+ *
+ * Per CPU interrupts on SMP machines without locking requirements. Same as
+ * handle_percpu_irq() above but with the following extras:
+ *
+ * action->percpu_dev_id is a pointer to percpu variables which
+ * contain the real device id for the cpu on which this handler is
+ * called
+ */
+void handle_percpu_devid_irq(unsigned int irq, struct irq_desc *desc)
+{
+ struct irq_chip *chip = irq_desc_get_chip(desc);
+ struct irqaction *action = desc->action;
+ void *dev_id = __this_cpu_ptr(action->percpu_dev_id);
+ irqreturn_t res;
+
+ kstat_incr_irqs_this_cpu(irq, desc);
+
+ if (chip->irq_ack)
+ chip->irq_ack(&desc->irq_data);
+
+ trace_irq_handler_entry(irq, action);
+ res = action->handler(irq, dev_id);
+ trace_irq_handler_exit(irq, action, res);
+
+ if (chip->irq_eoi)
+ chip->irq_eoi(&desc->irq_data);
+}
+#endif
+
void
__irq_set_handler(unsigned int irq, irq_flow_handler_t handle, int is_chained,
const char *name)
diff --git a/kernel/irq/internals.h b/kernel/irq/internals.h
index 6546431..a57fd3b 100644
--- a/kernel/irq/internals.h
+++ b/kernel/irq/internals.h
@@ -71,6 +71,8 @@ extern int irq_startup(struct irq_desc *desc);
extern void irq_shutdown(struct irq_desc *desc);
extern void irq_enable(struct irq_desc *desc);
extern void irq_disable(struct irq_desc *desc);
+extern void irq_percpu_enable(struct irq_desc *desc);
+extern void irq_percpu_disable(struct irq_desc *desc);
extern void mask_irq(struct irq_desc *desc);
extern void unmask_irq(struct irq_desc *desc);
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index 9b956fa..9f10b07 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -1118,6 +1118,8 @@ int setup_irq(unsigned int irq, struct irqaction *act)
int retval;
struct irq_desc *desc = irq_to_desc(irq);
+ if (irq_settings_is_per_cpu_devid(desc))
+ return -EINVAL;
chip_bus_lock(desc);
retval = __setup_irq(irq, desc, act);
chip_bus_sync_unlock(desc);
@@ -1126,7 +1128,7 @@ int setup_irq(unsigned int irq, struct irqaction *act)
}
EXPORT_SYMBOL_GPL(setup_irq);
- /*
+/*
* Internal function to unregister an irqaction - used to free
* regular and special interrupts that are part of the architecture.
*/
@@ -1224,7 +1226,10 @@ static struct irqaction *__free_irq(unsigned int irq, void *dev_id)
*/
void remove_irq(unsigned int irq, struct irqaction *act)
{
- __free_irq(irq, act->dev_id);
+ struct irq_desc *desc = irq_to_desc(irq);
+
+ if (desc && !irq_settings_is_per_cpu_devid(desc))
+ __free_irq(irq, act->dev_id);
}
EXPORT_SYMBOL_GPL(remove_irq);
@@ -1246,7 +1251,7 @@ void free_irq(unsigned int irq, void *dev_id)
{
struct irq_desc *desc = irq_to_desc(irq);
- if (!desc)
+ if (!desc || irq_settings_is_per_cpu_devid(desc))
return;
#ifdef CONFIG_SMP
@@ -1324,7 +1329,8 @@ int request_threaded_irq(unsigned int irq, irq_handler_t handler,
if (!desc)
return -EINVAL;
- if (!irq_settings_can_request(desc))
+ if (!irq_settings_can_request(desc) ||
+ irq_settings_is_per_cpu_devid(desc))
return -EINVAL;
if (!handler) {
@@ -1409,3 +1415,198 @@ int request_any_context_irq(unsigned int irq, irq_handler_t handler,
return !ret ? IRQC_IS_HARDIRQ : ret;
}
EXPORT_SYMBOL_GPL(request_any_context_irq);
+
+#ifdef CONFIG_IRQ_PERCPU_DEVID
+void enable_percpu_irq(unsigned int irq)
+{
+ unsigned long flags;
+ struct irq_desc *desc = irq_get_desc_buslock(irq, &flags);
+
+ if (!desc)
+ return;
+
+ irq_percpu_enable(desc);
+ irq_put_desc_busunlock(desc, flags);
+}
+EXPORT_SYMBOL(enable_percpu_irq);
+
+void disable_percpu_irq(unsigned int irq)
+{
+ unsigned long flags;
+ struct irq_desc *desc = irq_get_desc_buslock(irq, &flags);
+
+ if (!desc)
+ return;
+
+ irq_percpu_disable(desc);
+ irq_put_desc_busunlock(desc, flags);
+}
+EXPORT_SYMBOL(disable_percpu_irq);
+
+/*
+ * Internal function to unregister a percpu irqaction.
+ */
+static struct irqaction *__free_percpu_irq(unsigned int irq, void __percpu *dev_id)
+{
+ struct irq_desc *desc = irq_to_desc(irq);
+ struct irqaction *action;
+ unsigned long flags;
+
+ WARN(in_interrupt(), "Trying to free IRQ %d from IRQ context!\n", irq);
+
+ if (!desc)
+ return NULL;
+
+ raw_spin_lock_irqsave(&desc->lock, flags);
+
+ action = desc->action;
+ if (!action || action->percpu_dev_id != dev_id) {
+ WARN(1, "Trying to free already-free IRQ %d\n", irq);
+ raw_spin_unlock_irqrestore(&desc->lock, flags);
+ return NULL;
+ }
+
+ /* Found it - now remove it from the list of entries: */
+ WARN(!cpumask_empty(desc->percpu_enabled),
+ "percpu IRQ %d still enabled on CPU%d!\n",
+ irq, cpumask_first(desc->percpu_enabled));
+ desc->action = NULL;
+
+#ifdef CONFIG_SMP
+ /* make sure affinity_hint is cleaned up */
+ if (WARN_ON_ONCE(desc->affinity_hint))
+ desc->affinity_hint = NULL;
+#endif
+
+ raw_spin_unlock_irqrestore(&desc->lock, flags);
+
+ unregister_handler_proc(irq, action);
+
+ /* Make sure it's not being used on another CPU: */
+ synchronize_irq(irq);
+
+ module_put(desc->owner);
+ return action;
+}
+
+/**
+ * remove_percpu_irq - free a per-cpu interrupt
+ * @irq: Interrupt line to free
+ * @act: irqaction for the interrupt
+ *
+ * Used to remove interrupts statically setup by the early boot process.
+ */
+void remove_percpu_irq(unsigned int irq, struct irqaction *act)
+{
+ struct irq_desc *desc = irq_to_desc(irq);
+
+ if (desc && irq_settings_is_per_cpu_devid(desc))
+ __free_percpu_irq(irq, act->percpu_dev_id);
+}
+EXPORT_SYMBOL_GPL(remove_percpu_irq);
+
+/**
+ * free_percpu_irq - free an interrupt allocated with request_percpu_irq
+ * @irq: Interrupt line to free
+ * @dev_id: Device identity to free
+ *
+ * Remove a percpu interrupt handler. The handler is removed, but
+ * the interrupt line is not disabled. This must be done on each
+ * CPU before calling this function. The function does not return
+ * until any executing interrupts for this IRQ have completed.
+ *
+ * This function must not be called from interrupt context.
+ */
+void free_percpu_irq(unsigned int irq, void __percpu *dev_id)
+{
+ struct irq_desc *desc = irq_to_desc(irq);
+
+ if (!desc || !irq_settings_is_per_cpu_devid(desc))
+ return;
+
+#ifdef CONFIG_SMP
+ if (WARN_ON(desc->affinity_notify))
+ desc->affinity_notify = NULL;
+#endif
+
+ chip_bus_lock(desc);
+ kfree(__free_percpu_irq(irq, dev_id));
+ chip_bus_sync_unlock(desc);
+}
+EXPORT_SYMBOL(free_percpu_irq);
+
+/**
+ * setup_percpu_irq - setup a per-cpu interrupt
+ * @irq: Interrupt line to setup
+ * @act: irqaction for the interrupt
+ *
+ * Used to statically setup per-cpu interrupts in the early boot process.
+ */
+int setup_percpu_irq(unsigned int irq, struct irqaction *act)
+{
+ int retval;
+ struct irq_desc *desc = irq_to_desc(irq);
+
+ if (!irq_settings_is_per_cpu_devid(desc))
+ return -EINVAL;
+ chip_bus_lock(desc);
+ retval = __setup_irq(irq, desc, act);
+ chip_bus_sync_unlock(desc);
+
+ return retval;
+}
+EXPORT_SYMBOL_GPL(setup_percpu_irq);
+
+/**
+ * request_percpu_irq - allocate a percpu interrupt line
+ * @irq: Interrupt line to allocate
+ * @handler: Function to be called when the IRQ occurs.
+ * Primary handler for threaded interrupts
+ * If NULL and thread_fn != NULL the default
+ * primary handler is installed
+ * @devname: An ascii name for the claiming device
+ * @dev_id: A percpu cookie passed back to the handler function
+ *
+ * This call allocates interrupt resources, but doesn't
+ * automatically enable the interrupt. It has to be done on each
+ * CPU using enable_percpu_irq().
+ *
+ * Dev_id must be globally unique. It is a per-cpu variable, and
+ * the handler gets called with the interrupted CPU's instance of
+ * that variable.
+ */
+int request_percpu_irq(unsigned int irq, irq_handler_t handler,
+ const char *devname, void __percpu *dev_id)
+{
+ struct irqaction *action;
+ struct irq_desc *desc;
+ int retval;
+
+ if (!dev_id)
+ return -EINVAL;
+
+ desc = irq_to_desc(irq);
+ if (!desc || !irq_settings_can_request(desc) ||
+ !irq_settings_is_per_cpu_devid(desc))
+ return -EINVAL;
+
+ action = kzalloc(sizeof(struct irqaction), GFP_KERNEL);
+ if (!action)
+ return -ENOMEM;
+
+ action->handler = handler;
+ action->flags = IRQF_PERCPU;
+ action->name = devname;
+ action->percpu_dev_id = dev_id;
+
+ chip_bus_lock(desc);
+ retval = __setup_irq(irq, desc, action);
+ chip_bus_sync_unlock(desc);
+
+ if (retval)
+ kfree(action);
+
+ return retval;
+}
+EXPORT_SYMBOL(request_percpu_irq);
+#endif
diff --git a/kernel/irq/settings.h b/kernel/irq/settings.h
index f166783..1162f10 100644
--- a/kernel/irq/settings.h
+++ b/kernel/irq/settings.h
@@ -13,6 +13,7 @@ enum {
_IRQ_MOVE_PCNTXT = IRQ_MOVE_PCNTXT,
_IRQ_NO_BALANCING = IRQ_NO_BALANCING,
_IRQ_NESTED_THREAD = IRQ_NESTED_THREAD,
+ _IRQ_PER_CPU_DEVID = IRQ_PER_CPU_DEVID,
_IRQF_MODIFY_MASK = IRQF_MODIFY_MASK,
};
@@ -24,6 +25,7 @@ enum {
#define IRQ_NOTHREAD GOT_YOU_MORON
#define IRQ_NOAUTOEN GOT_YOU_MORON
#define IRQ_NESTED_THREAD GOT_YOU_MORON
+#define IRQ_PER_CPU_DEVID GOT_YOU_MORON
#undef IRQF_MODIFY_MASK
#define IRQF_MODIFY_MASK GOT_YOU_MORON
@@ -39,6 +41,11 @@ static inline bool irq_settings_is_per_cpu(struct irq_desc *desc)
return desc->status_use_accessors & _IRQ_PER_CPU;
}
+static inline bool irq_settings_is_per_cpu_devid(struct irq_desc *desc)
+{
+ return desc->status_use_accessors & _IRQ_PER_CPU_DEVID;
+}
+
static inline void irq_settings_set_per_cpu(struct irq_desc *desc)
{
desc->status_use_accessors |= _IRQ_PER_CPU;
--
1.7.0.4
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [RFC PATCH 2/3] ARM: gic: consolidate PPI handling
2011-09-15 16:52 ` Marc Zyngier
@ 2011-09-15 16:52 ` Marc Zyngier
-1 siblings, 0 replies; 38+ messages in thread
From: Marc Zyngier @ 2011-09-15 16:52 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel
Cc: Kukjin Kim, David Brown, Bryan Huntsman, Tony Lindgren,
Paul Mundt, Magnus Damm
PPI handling is a bit of an odd beast. It uses its own low level
handling code and is hardwired to the local timers (hence lacking
a registration interface).
Instead, switch the low handling to the normal SPI handling code.
PPIs are handled by the handle_percpu_devid_irq flow.
This also allows the removal of some duplicated code.
Cc: Kukjin Kim <kgene.kim@samsung.com>
Cc: David Brown <davidb@codeaurora.org>
Cc: Bryan Huntsman <bryanh@codeaurora.org>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Paul Mundt <lethal@linux-sh.org>
Cc: Magnus Damm <magnus.damm@gmail.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
arch/arm/common/Kconfig | 1 +
arch/arm/common/gic.c | 70 +++++++++++++++++++-
arch/arm/include/asm/entry-macro-multi.S | 7 --
arch/arm/include/asm/hardirq.h | 3 -
arch/arm/include/asm/hardware/entry-macro-gic.S | 19 +-----
arch/arm/include/asm/localtimer.h | 6 +-
arch/arm/include/asm/smp.h | 5 --
arch/arm/kernel/irq.c | 3 -
arch/arm/kernel/smp.c | 27 ++------
arch/arm/mach-exynos4/include/mach/entry-macro.S | 6 +--
arch/arm/mach-msm/board-msm8x60.c | 11 ---
arch/arm/mach-msm/include/mach/entry-macro-qgic.S | 73 +--------------------
arch/arm/mach-omap2/include/mach/entry-macro.S | 14 +----
arch/arm/mach-shmobile/entry-intc.S | 3 -
arch/arm/mach-shmobile/include/mach/entry-macro.S | 3 -
15 files changed, 84 insertions(+), 167 deletions(-)
diff --git a/arch/arm/common/Kconfig b/arch/arm/common/Kconfig
index 4b71766..114a432 100644
--- a/arch/arm/common/Kconfig
+++ b/arch/arm/common/Kconfig
@@ -1,4 +1,5 @@
config ARM_GIC
+ select IRQ_PERCPU_DEVID
bool
config ARM_VIC
diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
index 666b278..4bbcce8 100644
--- a/arch/arm/common/gic.c
+++ b/arch/arm/common/gic.c
@@ -28,10 +28,14 @@
#include <linux/smp.h>
#include <linux/cpumask.h>
#include <linux/io.h>
+#include <linux/interrupt.h>
+#include <linux/percpu.h>
+#include <linux/slab.h>
#include <asm/irq.h>
#include <asm/mach/irq.h>
#include <asm/hardware/gic.h>
+#include <asm/localtimer.h>
static DEFINE_SPINLOCK(irq_controller_lock);
@@ -255,6 +259,32 @@ void __init gic_cascade_irq(unsigned int gic_nr, unsigned int irq)
irq_set_chained_handler(irq, gic_handle_cascade_irq);
}
+#ifdef CONFIG_LOCAL_TIMERS
+#define gic_ppi_handler percpu_timer_handler
+#else
+static irqreturn_t gic_ppi_handler(int irq, void *dev_id)
+{
+ return IRQ_NONE;
+}
+#endif
+
+#define PPI_IRQACT(nr) \
+ { \
+ .handler = gic_ppi_handler, \
+ .flags = IRQF_PERCPU | IRQF_TIMER, \
+ .irq = nr, \
+ .name = "PPI-" # nr, \
+ }
+
+static struct irqaction ppi_irqaction_template[16] __initdata = {
+ PPI_IRQACT(0), PPI_IRQACT(1), PPI_IRQACT(2), PPI_IRQACT(3),
+ PPI_IRQACT(4), PPI_IRQACT(5), PPI_IRQACT(6), PPI_IRQACT(7),
+ PPI_IRQACT(8), PPI_IRQACT(9), PPI_IRQACT(10), PPI_IRQACT(11),
+ PPI_IRQACT(12), PPI_IRQACT(13), PPI_IRQACT(14), PPI_IRQACT(15),
+};
+
+static struct irqaction *ppi_irqaction;
+
static void __init gic_dist_init(struct gic_chip_data *gic,
unsigned int irq_start)
{
@@ -262,6 +292,7 @@ static void __init gic_dist_init(struct gic_chip_data *gic,
u32 cpumask;
void __iomem *base = gic->dist_base;
u32 cpu = 0;
+ u32 nrppis = 0, ppi_base = 0;
#ifdef CONFIG_SMP
cpu = cpu_logical_map(smp_processor_id());
@@ -283,6 +314,28 @@ static void __init gic_dist_init(struct gic_chip_data *gic,
gic_irqs = 1020;
/*
+ * Nobody would be insane enough to use PPIs on a secondary
+ * GIC, right?
+ */
+ if (gic == &gic_data[0]) {
+ nrppis = 16 - (irq_start & 15);
+ ppi_base = gic->irq_offset + 32 - nrppis;
+
+ ppi_irqaction = kmemdup(&ppi_irqaction_template[16 - nrppis],
+ sizeof(*ppi_irqaction) * nrppis,
+ GFP_KERNEL);
+
+ if (nrppis && !ppi_irqaction) {
+ pr_err("GIC: Can't allocate PPI memory");
+ nrppis = 0;
+ ppi_base = 0;
+ }
+ }
+
+ pr_info("Configuring GIC with %d sources (%d PPIs)\n",
+ gic_irqs, (gic == &gic_data[0]) ? nrppis : 0);
+
+ /*
* Set all global interrupts to be level triggered, active low.
*/
for (i = 32; i < gic_irqs; i += 16)
@@ -317,7 +370,22 @@ static void __init gic_dist_init(struct gic_chip_data *gic,
/*
* Setup the Linux IRQ subsystem.
*/
- for (i = irq_start; i < irq_limit; i++) {
+ for (i = 0; i < nrppis; i++) {
+ int ppi = i + ppi_base;
+ int err;
+
+ irq_set_percpu_devid(ppi);
+ irq_set_chip_and_handler(ppi, &gic_chip,
+ handle_percpu_devid_irq);
+ irq_set_chip_data(ppi, gic);
+ set_irq_flags(ppi, IRQF_VALID | IRQF_NOAUTOEN);
+
+ err = setup_percpu_irq(ppi, &ppi_irqaction[i]);
+ if (err)
+ pr_err("GIC: can't setup PPI%d (%d)\n", ppi, err);
+ }
+
+ for (i = irq_start + nrppis; i < irq_limit; i++) {
irq_set_chip_and_handler(i, &gic_chip, handle_fasteoi_irq);
irq_set_chip_data(i, gic);
set_irq_flags(i, IRQF_VALID | IRQF_PROBE);
diff --git a/arch/arm/include/asm/entry-macro-multi.S b/arch/arm/include/asm/entry-macro-multi.S
index 2f1e209..88d6181 100644
--- a/arch/arm/include/asm/entry-macro-multi.S
+++ b/arch/arm/include/asm/entry-macro-multi.S
@@ -25,13 +25,6 @@
movne r1, sp
adrne lr, BSYM(1b)
bne do_IPI
-
-#ifdef CONFIG_LOCAL_TIMERS
- test_for_ltirq r0, r2, r6, lr
- movne r0, sp
- adrne lr, BSYM(1b)
- bne do_local_timer
-#endif
#endif
9997:
.endm
diff --git a/arch/arm/include/asm/hardirq.h b/arch/arm/include/asm/hardirq.h
index 89ad180..ddf07a9 100644
--- a/arch/arm/include/asm/hardirq.h
+++ b/arch/arm/include/asm/hardirq.h
@@ -9,9 +9,6 @@
typedef struct {
unsigned int __softirq_pending;
-#ifdef CONFIG_LOCAL_TIMERS
- unsigned int local_timer_irqs;
-#endif
#ifdef CONFIG_SMP
unsigned int ipi_irqs[NR_IPI];
#endif
diff --git a/arch/arm/include/asm/hardware/entry-macro-gic.S b/arch/arm/include/asm/hardware/entry-macro-gic.S
index c115b82..74ebc80 100644
--- a/arch/arm/include/asm/hardware/entry-macro-gic.S
+++ b/arch/arm/include/asm/hardware/entry-macro-gic.S
@@ -22,15 +22,11 @@
* interrupt controller spec. To wit:
*
* Interrupts 0-15 are IPI
- * 16-28 are reserved
- * 29-31 are local. We allow 30 to be used for the watchdog.
+ * 16-31 are local. We allow 30 to be used for the watchdog.
* 32-1020 are global
* 1021-1022 are reserved
* 1023 is "spurious" (no interrupt)
*
- * For now, we ignore all local interrupts so only return an interrupt if it's
- * between 30 and 1020. The test_for_ipi routine below will pick up on IPIs.
- *
* A simple read from the controller will tell us the number of the highest
* priority enabled interrupt. We then just need to check whether it is in the
* valid range for an IRQ (30-1020 inclusive).
@@ -43,7 +39,7 @@
ldr \tmp, =1021
bic \irqnr, \irqstat, #0x1c00
- cmp \irqnr, #29
+ cmp \irqnr, #15
cmpcc \irqnr, \irqnr
cmpne \irqnr, \tmp
cmpcs \irqnr, \irqnr
@@ -62,14 +58,3 @@
strcc \irqstat, [\base, #GIC_CPU_EOI]
cmpcs \irqnr, \irqnr
.endm
-
-/* As above, this assumes that irqstat and base are preserved.. */
-
- .macro test_for_ltirq, irqnr, irqstat, base, tmp
- bic \irqnr, \irqstat, #0x1c00
- mov \tmp, #0
- cmp \irqnr, #29
- moveq \tmp, #1
- streq \irqstat, [\base, #GIC_CPU_EOI]
- cmp \tmp, #0
- .endm
diff --git a/arch/arm/include/asm/localtimer.h b/arch/arm/include/asm/localtimer.h
index 080d74f..e3663f7 100644
--- a/arch/arm/include/asm/localtimer.h
+++ b/arch/arm/include/asm/localtimer.h
@@ -10,6 +10,8 @@
#ifndef __ASM_ARM_LOCALTIMER_H
#define __ASM_ARM_LOCALTIMER_H
+#include <linux/interrupt.h>
+
struct clock_event_device;
/*
@@ -18,9 +20,9 @@ struct clock_event_device;
void percpu_timer_setup(void);
/*
- * Called from assembly, this is the local timer IRQ handler
+ * Per-cpu timer IRQ handler
*/
-asmlinkage void do_local_timer(struct pt_regs *);
+irqreturn_t percpu_timer_handler(int irq, void *dev_id);
#ifdef CONFIG_LOCAL_TIMERS
diff --git a/arch/arm/include/asm/smp.h b/arch/arm/include/asm/smp.h
index 674ebcd..7c2299f 100644
--- a/arch/arm/include/asm/smp.h
+++ b/arch/arm/include/asm/smp.h
@@ -94,9 +94,4 @@ extern void platform_cpu_enable(unsigned int cpu);
extern void arch_send_call_function_single_ipi(int cpu);
extern void arch_send_call_function_ipi_mask(const struct cpumask *mask);
-/*
- * show local interrupt info
- */
-extern void show_local_irqs(struct seq_file *, int);
-
#endif /* ifndef __ASM_ARM_SMP_H */
diff --git a/arch/arm/kernel/irq.c b/arch/arm/kernel/irq.c
index e20a5d0..8e744f6 100644
--- a/arch/arm/kernel/irq.c
+++ b/arch/arm/kernel/irq.c
@@ -58,9 +58,6 @@ int arch_show_interrupts(struct seq_file *p, int prec)
#ifdef CONFIG_SMP
show_ipi_list(p, prec);
#endif
-#ifdef CONFIG_LOCAL_TIMERS
- show_local_irqs(p, prec);
-#endif
seq_printf(p, "%*s: %10lu\n", prec, "Err", irq_err_count);
return 0;
}
diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index 04d7b80..e3dbd99 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -504,10 +504,6 @@ u64 smp_irq_stat_cpu(unsigned int cpu)
for (i = 0; i < NR_IPI; i++)
sum += __get_irq_stat(cpu, ipi_irqs[i]);
-#ifdef CONFIG_LOCAL_TIMERS
- sum += __get_irq_stat(cpu, local_timer_irqs);
-#endif
-
return sum;
}
@@ -525,29 +521,16 @@ static void ipi_timer(void)
}
#ifdef CONFIG_LOCAL_TIMERS
-asmlinkage void __exception_irq_entry do_local_timer(struct pt_regs *regs)
+irqreturn_t percpu_timer_handler(int irq, void *dev_id)
{
- struct pt_regs *old_regs = set_irq_regs(regs);
- int cpu = smp_processor_id();
+ struct clock_event_device *evt = &__get_cpu_var(percpu_clockevent);
if (local_timer_ack()) {
- __inc_irq_stat(cpu, local_timer_irqs);
- ipi_timer();
+ evt->event_handler(evt);
+ return IRQ_HANDLED;
}
- set_irq_regs(old_regs);
-}
-
-void show_local_irqs(struct seq_file *p, int prec)
-{
- unsigned int cpu;
-
- seq_printf(p, "%*s: ", prec, "LOC");
-
- for_each_present_cpu(cpu)
- seq_printf(p, "%10u ", __get_irq_stat(cpu, local_timer_irqs));
-
- seq_printf(p, " Local timer interrupts\n");
+ return IRQ_NONE;
}
#endif
diff --git a/arch/arm/mach-exynos4/include/mach/entry-macro.S b/arch/arm/mach-exynos4/include/mach/entry-macro.S
index d7a1e28..807d05d 100644
--- a/arch/arm/mach-exynos4/include/mach/entry-macro.S
+++ b/arch/arm/mach-exynos4/include/mach/entry-macro.S
@@ -55,7 +55,7 @@
bic \irqnr, \irqstat, #0x1c00
- cmp \irqnr, #29
+ cmp \irqnr, #15
cmpcc \irqnr, \irqnr
cmpne \irqnr, \tmp
cmpcs \irqnr, \irqnr
@@ -77,7 +77,3 @@
cmpcs \irqnr, \irqnr
.endm
- /* As above, this assumes that irqstat and base are preserved.. */
-
- .macro test_for_ltirq, irqnr, irqstat, base, tmp
- .endm
diff --git a/arch/arm/mach-msm/board-msm8x60.c b/arch/arm/mach-msm/board-msm8x60.c
index 9221f54..106170f 100644
--- a/arch/arm/mach-msm/board-msm8x60.c
+++ b/arch/arm/mach-msm/board-msm8x60.c
@@ -53,8 +53,6 @@ static void __init msm8x60_map_io(void)
static void __init msm8x60_init_irq(void)
{
- unsigned int i;
-
gic_init(0, GIC_PPI_START, MSM_QGIC_DIST_BASE,
(void *)MSM_QGIC_CPU_BASE);
@@ -66,15 +64,6 @@ static void __init msm8x60_init_irq(void)
*/
if (!machine_is_msm8x60_sim())
writel(0x0000FFFF, MSM_QGIC_DIST_BASE + GIC_DIST_ENABLE_SET);
-
- /* FIXME: Not installing AVS_SVICINT and AVS_SVICINTSWDONE yet
- * as they are configured as level, which does not play nice with
- * handle_percpu_irq.
- */
- for (i = GIC_PPI_START; i < GIC_SPI_START; i++) {
- if (i != AVS_SVICINT && i != AVS_SVICINTSWDONE)
- irq_set_handler(i, handle_percpu_irq);
- }
}
static void __init msm8x60_init(void)
diff --git a/arch/arm/mach-msm/include/mach/entry-macro-qgic.S b/arch/arm/mach-msm/include/mach/entry-macro-qgic.S
index 1246715..717076f 100644
--- a/arch/arm/mach-msm/include/mach/entry-macro-qgic.S
+++ b/arch/arm/mach-msm/include/mach/entry-macro-qgic.S
@@ -8,81 +8,10 @@
* warranty of any kind, whether express or implied.
*/
-#include <mach/hardware.h>
-#include <asm/hardware/gic.h>
+#include <asm/hardware/entry-macro-gic.S>
.macro disable_fiq
.endm
- .macro get_irqnr_preamble, base, tmp
- ldr \base, =gic_cpu_base_addr
- ldr \base, [\base]
- .endm
-
.macro arch_ret_to_user, tmp1, tmp2
.endm
-
- /*
- * The interrupt numbering scheme is defined in the
- * interrupt controller spec. To wit:
- *
- * Migrated the code from ARM MP port to be more consistent
- * with interrupt processing , the following still holds true
- * however, all interrupts are treated the same regardless of
- * if they are local IPI or PPI
- *
- * Interrupts 0-15 are IPI
- * 16-31 are PPI
- * (16-18 are the timers)
- * 32-1020 are global
- * 1021-1022 are reserved
- * 1023 is "spurious" (no interrupt)
- *
- * A simple read from the controller will tell us the number of the
- * highest priority enabled interrupt. We then just need to check
- * whether it is in the valid range for an IRQ (0-1020 inclusive).
- *
- * Base ARM code assumes that the local (private) peripheral interrupts
- * are not valid, we treat them differently, in that the privates are
- * handled like normal shared interrupts with the exception that only
- * one processor can register the interrupt and the handler must be
- * the same for all processors.
- */
-
- .macro get_irqnr_and_base, irqnr, irqstat, base, tmp
-
- ldr \irqstat, [\base, #GIC_CPU_INTACK] /* bits 12-10 =srcCPU,
- 9-0 =int # */
-
- bic \irqnr, \irqstat, #0x1c00 @mask src
- cmp \irqnr, #15
- ldr \tmp, =1021
- cmpcc \irqnr, \irqnr
- cmpne \irqnr, \tmp
- cmpcs \irqnr, \irqnr
-
- .endm
-
- /* We assume that irqstat (the raw value of the IRQ acknowledge
- * register) is preserved from the macro above.
- * If there is an IPI, we immediately signal end of interrupt on the
- * controller, since this requires the original irqstat value which
- * we won't easily be able to recreate later.
- */
- .macro test_for_ipi, irqnr, irqstat, base, tmp
- bic \irqnr, \irqstat, #0x1c00
- cmp \irqnr, #16
- strcc \irqstat, [\base, #GIC_CPU_EOI]
- cmpcs \irqnr, \irqnr
- .endm
-
- /* As above, this assumes that irqstat and base are preserved.. */
-
- .macro test_for_ltirq, irqnr, irqstat, base, tmp
- bic \irqnr, \irqstat, #0x1c00
- mov \tmp, #0
- cmp \irqnr, #16
- moveq \tmp, #1
- streq \irqstat, [\base, #GIC_CPU_EOI]
- cmp \tmp, #0
- .endm
diff --git a/arch/arm/mach-omap2/include/mach/entry-macro.S b/arch/arm/mach-omap2/include/mach/entry-macro.S
index ceb8b7e..feb90a1 100644
--- a/arch/arm/mach-omap2/include/mach/entry-macro.S
+++ b/arch/arm/mach-omap2/include/mach/entry-macro.S
@@ -78,7 +78,7 @@
4401: ldr \irqstat, [\base, #GIC_CPU_INTACK]
ldr \tmp, =1021
bic \irqnr, \irqstat, #0x1c00
- cmp \irqnr, #29
+ cmp \irqnr, #15
cmpcc \irqnr, \irqnr
cmpne \irqnr, \tmp
cmpcs \irqnr, \irqnr
@@ -101,18 +101,6 @@
it cs
cmpcs \irqnr, \irqnr
.endm
-
- /* As above, this assumes that irqstat and base are preserved */
-
- .macro test_for_ltirq, irqnr, irqstat, base, tmp
- bic \irqnr, \irqstat, #0x1c00
- mov \tmp, #0
- cmp \irqnr, #29
- itt eq
- moveq \tmp, #1
- streq \irqstat, [\base, #GIC_CPU_EOI]
- cmp \tmp, #0
- .endm
#endif /* CONFIG_SMP */
#else /* MULTI_OMAP2 */
diff --git a/arch/arm/mach-shmobile/entry-intc.S b/arch/arm/mach-shmobile/entry-intc.S
index cac0a7a..1a1c00c 100644
--- a/arch/arm/mach-shmobile/entry-intc.S
+++ b/arch/arm/mach-shmobile/entry-intc.S
@@ -51,7 +51,4 @@
.macro test_for_ipi, irqnr, irqstat, base, tmp
.endm
- .macro test_for_ltirq, irqnr, irqstat, base, tmp
- .endm
-
arch_irq_handler shmobile_handle_irq_intc
diff --git a/arch/arm/mach-shmobile/include/mach/entry-macro.S b/arch/arm/mach-shmobile/include/mach/entry-macro.S
index d791f10..8d4a416 100644
--- a/arch/arm/mach-shmobile/include/mach/entry-macro.S
+++ b/arch/arm/mach-shmobile/include/mach/entry-macro.S
@@ -27,8 +27,5 @@
.macro test_for_ipi, irqnr, irqstat, base, tmp
.endm
- .macro test_for_ltirq, irqnr, irqstat, base, tmp
- .endm
-
.macro arch_ret_to_user, tmp1, tmp2
.endm
--
1.7.0.4
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [RFC PATCH 2/3] ARM: gic: consolidate PPI handling
@ 2011-09-15 16:52 ` Marc Zyngier
0 siblings, 0 replies; 38+ messages in thread
From: Marc Zyngier @ 2011-09-15 16:52 UTC (permalink / raw)
To: linux-arm-kernel
PPI handling is a bit of an odd beast. It uses its own low level
handling code and is hardwired to the local timers (hence lacking
a registration interface).
Instead, switch the low handling to the normal SPI handling code.
PPIs are handled by the handle_percpu_devid_irq flow.
This also allows the removal of some duplicated code.
Cc: Kukjin Kim <kgene.kim@samsung.com>
Cc: David Brown <davidb@codeaurora.org>
Cc: Bryan Huntsman <bryanh@codeaurora.org>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Paul Mundt <lethal@linux-sh.org>
Cc: Magnus Damm <magnus.damm@gmail.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
arch/arm/common/Kconfig | 1 +
arch/arm/common/gic.c | 70 +++++++++++++++++++-
arch/arm/include/asm/entry-macro-multi.S | 7 --
arch/arm/include/asm/hardirq.h | 3 -
arch/arm/include/asm/hardware/entry-macro-gic.S | 19 +-----
arch/arm/include/asm/localtimer.h | 6 +-
arch/arm/include/asm/smp.h | 5 --
arch/arm/kernel/irq.c | 3 -
arch/arm/kernel/smp.c | 27 ++------
arch/arm/mach-exynos4/include/mach/entry-macro.S | 6 +--
arch/arm/mach-msm/board-msm8x60.c | 11 ---
arch/arm/mach-msm/include/mach/entry-macro-qgic.S | 73 +--------------------
arch/arm/mach-omap2/include/mach/entry-macro.S | 14 +----
arch/arm/mach-shmobile/entry-intc.S | 3 -
arch/arm/mach-shmobile/include/mach/entry-macro.S | 3 -
15 files changed, 84 insertions(+), 167 deletions(-)
diff --git a/arch/arm/common/Kconfig b/arch/arm/common/Kconfig
index 4b71766..114a432 100644
--- a/arch/arm/common/Kconfig
+++ b/arch/arm/common/Kconfig
@@ -1,4 +1,5 @@
config ARM_GIC
+ select IRQ_PERCPU_DEVID
bool
config ARM_VIC
diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
index 666b278..4bbcce8 100644
--- a/arch/arm/common/gic.c
+++ b/arch/arm/common/gic.c
@@ -28,10 +28,14 @@
#include <linux/smp.h>
#include <linux/cpumask.h>
#include <linux/io.h>
+#include <linux/interrupt.h>
+#include <linux/percpu.h>
+#include <linux/slab.h>
#include <asm/irq.h>
#include <asm/mach/irq.h>
#include <asm/hardware/gic.h>
+#include <asm/localtimer.h>
static DEFINE_SPINLOCK(irq_controller_lock);
@@ -255,6 +259,32 @@ void __init gic_cascade_irq(unsigned int gic_nr, unsigned int irq)
irq_set_chained_handler(irq, gic_handle_cascade_irq);
}
+#ifdef CONFIG_LOCAL_TIMERS
+#define gic_ppi_handler percpu_timer_handler
+#else
+static irqreturn_t gic_ppi_handler(int irq, void *dev_id)
+{
+ return IRQ_NONE;
+}
+#endif
+
+#define PPI_IRQACT(nr) \
+ { \
+ .handler = gic_ppi_handler, \
+ .flags = IRQF_PERCPU | IRQF_TIMER, \
+ .irq = nr, \
+ .name = "PPI-" # nr, \
+ }
+
+static struct irqaction ppi_irqaction_template[16] __initdata = {
+ PPI_IRQACT(0), PPI_IRQACT(1), PPI_IRQACT(2), PPI_IRQACT(3),
+ PPI_IRQACT(4), PPI_IRQACT(5), PPI_IRQACT(6), PPI_IRQACT(7),
+ PPI_IRQACT(8), PPI_IRQACT(9), PPI_IRQACT(10), PPI_IRQACT(11),
+ PPI_IRQACT(12), PPI_IRQACT(13), PPI_IRQACT(14), PPI_IRQACT(15),
+};
+
+static struct irqaction *ppi_irqaction;
+
static void __init gic_dist_init(struct gic_chip_data *gic,
unsigned int irq_start)
{
@@ -262,6 +292,7 @@ static void __init gic_dist_init(struct gic_chip_data *gic,
u32 cpumask;
void __iomem *base = gic->dist_base;
u32 cpu = 0;
+ u32 nrppis = 0, ppi_base = 0;
#ifdef CONFIG_SMP
cpu = cpu_logical_map(smp_processor_id());
@@ -283,6 +314,28 @@ static void __init gic_dist_init(struct gic_chip_data *gic,
gic_irqs = 1020;
/*
+ * Nobody would be insane enough to use PPIs on a secondary
+ * GIC, right?
+ */
+ if (gic == &gic_data[0]) {
+ nrppis = 16 - (irq_start & 15);
+ ppi_base = gic->irq_offset + 32 - nrppis;
+
+ ppi_irqaction = kmemdup(&ppi_irqaction_template[16 - nrppis],
+ sizeof(*ppi_irqaction) * nrppis,
+ GFP_KERNEL);
+
+ if (nrppis && !ppi_irqaction) {
+ pr_err("GIC: Can't allocate PPI memory");
+ nrppis = 0;
+ ppi_base = 0;
+ }
+ }
+
+ pr_info("Configuring GIC with %d sources (%d PPIs)\n",
+ gic_irqs, (gic == &gic_data[0]) ? nrppis : 0);
+
+ /*
* Set all global interrupts to be level triggered, active low.
*/
for (i = 32; i < gic_irqs; i += 16)
@@ -317,7 +370,22 @@ static void __init gic_dist_init(struct gic_chip_data *gic,
/*
* Setup the Linux IRQ subsystem.
*/
- for (i = irq_start; i < irq_limit; i++) {
+ for (i = 0; i < nrppis; i++) {
+ int ppi = i + ppi_base;
+ int err;
+
+ irq_set_percpu_devid(ppi);
+ irq_set_chip_and_handler(ppi, &gic_chip,
+ handle_percpu_devid_irq);
+ irq_set_chip_data(ppi, gic);
+ set_irq_flags(ppi, IRQF_VALID | IRQF_NOAUTOEN);
+
+ err = setup_percpu_irq(ppi, &ppi_irqaction[i]);
+ if (err)
+ pr_err("GIC: can't setup PPI%d (%d)\n", ppi, err);
+ }
+
+ for (i = irq_start + nrppis; i < irq_limit; i++) {
irq_set_chip_and_handler(i, &gic_chip, handle_fasteoi_irq);
irq_set_chip_data(i, gic);
set_irq_flags(i, IRQF_VALID | IRQF_PROBE);
diff --git a/arch/arm/include/asm/entry-macro-multi.S b/arch/arm/include/asm/entry-macro-multi.S
index 2f1e209..88d6181 100644
--- a/arch/arm/include/asm/entry-macro-multi.S
+++ b/arch/arm/include/asm/entry-macro-multi.S
@@ -25,13 +25,6 @@
movne r1, sp
adrne lr, BSYM(1b)
bne do_IPI
-
-#ifdef CONFIG_LOCAL_TIMERS
- test_for_ltirq r0, r2, r6, lr
- movne r0, sp
- adrne lr, BSYM(1b)
- bne do_local_timer
-#endif
#endif
9997:
.endm
diff --git a/arch/arm/include/asm/hardirq.h b/arch/arm/include/asm/hardirq.h
index 89ad180..ddf07a9 100644
--- a/arch/arm/include/asm/hardirq.h
+++ b/arch/arm/include/asm/hardirq.h
@@ -9,9 +9,6 @@
typedef struct {
unsigned int __softirq_pending;
-#ifdef CONFIG_LOCAL_TIMERS
- unsigned int local_timer_irqs;
-#endif
#ifdef CONFIG_SMP
unsigned int ipi_irqs[NR_IPI];
#endif
diff --git a/arch/arm/include/asm/hardware/entry-macro-gic.S b/arch/arm/include/asm/hardware/entry-macro-gic.S
index c115b82..74ebc80 100644
--- a/arch/arm/include/asm/hardware/entry-macro-gic.S
+++ b/arch/arm/include/asm/hardware/entry-macro-gic.S
@@ -22,15 +22,11 @@
* interrupt controller spec. To wit:
*
* Interrupts 0-15 are IPI
- * 16-28 are reserved
- * 29-31 are local. We allow 30 to be used for the watchdog.
+ * 16-31 are local. We allow 30 to be used for the watchdog.
* 32-1020 are global
* 1021-1022 are reserved
* 1023 is "spurious" (no interrupt)
*
- * For now, we ignore all local interrupts so only return an interrupt if it's
- * between 30 and 1020. The test_for_ipi routine below will pick up on IPIs.
- *
* A simple read from the controller will tell us the number of the highest
* priority enabled interrupt. We then just need to check whether it is in the
* valid range for an IRQ (30-1020 inclusive).
@@ -43,7 +39,7 @@
ldr \tmp, =1021
bic \irqnr, \irqstat, #0x1c00
- cmp \irqnr, #29
+ cmp \irqnr, #15
cmpcc \irqnr, \irqnr
cmpne \irqnr, \tmp
cmpcs \irqnr, \irqnr
@@ -62,14 +58,3 @@
strcc \irqstat, [\base, #GIC_CPU_EOI]
cmpcs \irqnr, \irqnr
.endm
-
-/* As above, this assumes that irqstat and base are preserved.. */
-
- .macro test_for_ltirq, irqnr, irqstat, base, tmp
- bic \irqnr, \irqstat, #0x1c00
- mov \tmp, #0
- cmp \irqnr, #29
- moveq \tmp, #1
- streq \irqstat, [\base, #GIC_CPU_EOI]
- cmp \tmp, #0
- .endm
diff --git a/arch/arm/include/asm/localtimer.h b/arch/arm/include/asm/localtimer.h
index 080d74f..e3663f7 100644
--- a/arch/arm/include/asm/localtimer.h
+++ b/arch/arm/include/asm/localtimer.h
@@ -10,6 +10,8 @@
#ifndef __ASM_ARM_LOCALTIMER_H
#define __ASM_ARM_LOCALTIMER_H
+#include <linux/interrupt.h>
+
struct clock_event_device;
/*
@@ -18,9 +20,9 @@ struct clock_event_device;
void percpu_timer_setup(void);
/*
- * Called from assembly, this is the local timer IRQ handler
+ * Per-cpu timer IRQ handler
*/
-asmlinkage void do_local_timer(struct pt_regs *);
+irqreturn_t percpu_timer_handler(int irq, void *dev_id);
#ifdef CONFIG_LOCAL_TIMERS
diff --git a/arch/arm/include/asm/smp.h b/arch/arm/include/asm/smp.h
index 674ebcd..7c2299f 100644
--- a/arch/arm/include/asm/smp.h
+++ b/arch/arm/include/asm/smp.h
@@ -94,9 +94,4 @@ extern void platform_cpu_enable(unsigned int cpu);
extern void arch_send_call_function_single_ipi(int cpu);
extern void arch_send_call_function_ipi_mask(const struct cpumask *mask);
-/*
- * show local interrupt info
- */
-extern void show_local_irqs(struct seq_file *, int);
-
#endif /* ifndef __ASM_ARM_SMP_H */
diff --git a/arch/arm/kernel/irq.c b/arch/arm/kernel/irq.c
index e20a5d0..8e744f6 100644
--- a/arch/arm/kernel/irq.c
+++ b/arch/arm/kernel/irq.c
@@ -58,9 +58,6 @@ int arch_show_interrupts(struct seq_file *p, int prec)
#ifdef CONFIG_SMP
show_ipi_list(p, prec);
#endif
-#ifdef CONFIG_LOCAL_TIMERS
- show_local_irqs(p, prec);
-#endif
seq_printf(p, "%*s: %10lu\n", prec, "Err", irq_err_count);
return 0;
}
diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index 04d7b80..e3dbd99 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -504,10 +504,6 @@ u64 smp_irq_stat_cpu(unsigned int cpu)
for (i = 0; i < NR_IPI; i++)
sum += __get_irq_stat(cpu, ipi_irqs[i]);
-#ifdef CONFIG_LOCAL_TIMERS
- sum += __get_irq_stat(cpu, local_timer_irqs);
-#endif
-
return sum;
}
@@ -525,29 +521,16 @@ static void ipi_timer(void)
}
#ifdef CONFIG_LOCAL_TIMERS
-asmlinkage void __exception_irq_entry do_local_timer(struct pt_regs *regs)
+irqreturn_t percpu_timer_handler(int irq, void *dev_id)
{
- struct pt_regs *old_regs = set_irq_regs(regs);
- int cpu = smp_processor_id();
+ struct clock_event_device *evt = &__get_cpu_var(percpu_clockevent);
if (local_timer_ack()) {
- __inc_irq_stat(cpu, local_timer_irqs);
- ipi_timer();
+ evt->event_handler(evt);
+ return IRQ_HANDLED;
}
- set_irq_regs(old_regs);
-}
-
-void show_local_irqs(struct seq_file *p, int prec)
-{
- unsigned int cpu;
-
- seq_printf(p, "%*s: ", prec, "LOC");
-
- for_each_present_cpu(cpu)
- seq_printf(p, "%10u ", __get_irq_stat(cpu, local_timer_irqs));
-
- seq_printf(p, " Local timer interrupts\n");
+ return IRQ_NONE;
}
#endif
diff --git a/arch/arm/mach-exynos4/include/mach/entry-macro.S b/arch/arm/mach-exynos4/include/mach/entry-macro.S
index d7a1e28..807d05d 100644
--- a/arch/arm/mach-exynos4/include/mach/entry-macro.S
+++ b/arch/arm/mach-exynos4/include/mach/entry-macro.S
@@ -55,7 +55,7 @@
bic \irqnr, \irqstat, #0x1c00
- cmp \irqnr, #29
+ cmp \irqnr, #15
cmpcc \irqnr, \irqnr
cmpne \irqnr, \tmp
cmpcs \irqnr, \irqnr
@@ -77,7 +77,3 @@
cmpcs \irqnr, \irqnr
.endm
- /* As above, this assumes that irqstat and base are preserved.. */
-
- .macro test_for_ltirq, irqnr, irqstat, base, tmp
- .endm
diff --git a/arch/arm/mach-msm/board-msm8x60.c b/arch/arm/mach-msm/board-msm8x60.c
index 9221f54..106170f 100644
--- a/arch/arm/mach-msm/board-msm8x60.c
+++ b/arch/arm/mach-msm/board-msm8x60.c
@@ -53,8 +53,6 @@ static void __init msm8x60_map_io(void)
static void __init msm8x60_init_irq(void)
{
- unsigned int i;
-
gic_init(0, GIC_PPI_START, MSM_QGIC_DIST_BASE,
(void *)MSM_QGIC_CPU_BASE);
@@ -66,15 +64,6 @@ static void __init msm8x60_init_irq(void)
*/
if (!machine_is_msm8x60_sim())
writel(0x0000FFFF, MSM_QGIC_DIST_BASE + GIC_DIST_ENABLE_SET);
-
- /* FIXME: Not installing AVS_SVICINT and AVS_SVICINTSWDONE yet
- * as they are configured as level, which does not play nice with
- * handle_percpu_irq.
- */
- for (i = GIC_PPI_START; i < GIC_SPI_START; i++) {
- if (i != AVS_SVICINT && i != AVS_SVICINTSWDONE)
- irq_set_handler(i, handle_percpu_irq);
- }
}
static void __init msm8x60_init(void)
diff --git a/arch/arm/mach-msm/include/mach/entry-macro-qgic.S b/arch/arm/mach-msm/include/mach/entry-macro-qgic.S
index 1246715..717076f 100644
--- a/arch/arm/mach-msm/include/mach/entry-macro-qgic.S
+++ b/arch/arm/mach-msm/include/mach/entry-macro-qgic.S
@@ -8,81 +8,10 @@
* warranty of any kind, whether express or implied.
*/
-#include <mach/hardware.h>
-#include <asm/hardware/gic.h>
+#include <asm/hardware/entry-macro-gic.S>
.macro disable_fiq
.endm
- .macro get_irqnr_preamble, base, tmp
- ldr \base, =gic_cpu_base_addr
- ldr \base, [\base]
- .endm
-
.macro arch_ret_to_user, tmp1, tmp2
.endm
-
- /*
- * The interrupt numbering scheme is defined in the
- * interrupt controller spec. To wit:
- *
- * Migrated the code from ARM MP port to be more consistent
- * with interrupt processing , the following still holds true
- * however, all interrupts are treated the same regardless of
- * if they are local IPI or PPI
- *
- * Interrupts 0-15 are IPI
- * 16-31 are PPI
- * (16-18 are the timers)
- * 32-1020 are global
- * 1021-1022 are reserved
- * 1023 is "spurious" (no interrupt)
- *
- * A simple read from the controller will tell us the number of the
- * highest priority enabled interrupt. We then just need to check
- * whether it is in the valid range for an IRQ (0-1020 inclusive).
- *
- * Base ARM code assumes that the local (private) peripheral interrupts
- * are not valid, we treat them differently, in that the privates are
- * handled like normal shared interrupts with the exception that only
- * one processor can register the interrupt and the handler must be
- * the same for all processors.
- */
-
- .macro get_irqnr_and_base, irqnr, irqstat, base, tmp
-
- ldr \irqstat, [\base, #GIC_CPU_INTACK] /* bits 12-10 =srcCPU,
- 9-0 =int # */
-
- bic \irqnr, \irqstat, #0x1c00 @mask src
- cmp \irqnr, #15
- ldr \tmp, =1021
- cmpcc \irqnr, \irqnr
- cmpne \irqnr, \tmp
- cmpcs \irqnr, \irqnr
-
- .endm
-
- /* We assume that irqstat (the raw value of the IRQ acknowledge
- * register) is preserved from the macro above.
- * If there is an IPI, we immediately signal end of interrupt on the
- * controller, since this requires the original irqstat value which
- * we won't easily be able to recreate later.
- */
- .macro test_for_ipi, irqnr, irqstat, base, tmp
- bic \irqnr, \irqstat, #0x1c00
- cmp \irqnr, #16
- strcc \irqstat, [\base, #GIC_CPU_EOI]
- cmpcs \irqnr, \irqnr
- .endm
-
- /* As above, this assumes that irqstat and base are preserved.. */
-
- .macro test_for_ltirq, irqnr, irqstat, base, tmp
- bic \irqnr, \irqstat, #0x1c00
- mov \tmp, #0
- cmp \irqnr, #16
- moveq \tmp, #1
- streq \irqstat, [\base, #GIC_CPU_EOI]
- cmp \tmp, #0
- .endm
diff --git a/arch/arm/mach-omap2/include/mach/entry-macro.S b/arch/arm/mach-omap2/include/mach/entry-macro.S
index ceb8b7e..feb90a1 100644
--- a/arch/arm/mach-omap2/include/mach/entry-macro.S
+++ b/arch/arm/mach-omap2/include/mach/entry-macro.S
@@ -78,7 +78,7 @@
4401: ldr \irqstat, [\base, #GIC_CPU_INTACK]
ldr \tmp, =1021
bic \irqnr, \irqstat, #0x1c00
- cmp \irqnr, #29
+ cmp \irqnr, #15
cmpcc \irqnr, \irqnr
cmpne \irqnr, \tmp
cmpcs \irqnr, \irqnr
@@ -101,18 +101,6 @@
it cs
cmpcs \irqnr, \irqnr
.endm
-
- /* As above, this assumes that irqstat and base are preserved */
-
- .macro test_for_ltirq, irqnr, irqstat, base, tmp
- bic \irqnr, \irqstat, #0x1c00
- mov \tmp, #0
- cmp \irqnr, #29
- itt eq
- moveq \tmp, #1
- streq \irqstat, [\base, #GIC_CPU_EOI]
- cmp \tmp, #0
- .endm
#endif /* CONFIG_SMP */
#else /* MULTI_OMAP2 */
diff --git a/arch/arm/mach-shmobile/entry-intc.S b/arch/arm/mach-shmobile/entry-intc.S
index cac0a7a..1a1c00c 100644
--- a/arch/arm/mach-shmobile/entry-intc.S
+++ b/arch/arm/mach-shmobile/entry-intc.S
@@ -51,7 +51,4 @@
.macro test_for_ipi, irqnr, irqstat, base, tmp
.endm
- .macro test_for_ltirq, irqnr, irqstat, base, tmp
- .endm
-
arch_irq_handler shmobile_handle_irq_intc
diff --git a/arch/arm/mach-shmobile/include/mach/entry-macro.S b/arch/arm/mach-shmobile/include/mach/entry-macro.S
index d791f10..8d4a416 100644
--- a/arch/arm/mach-shmobile/include/mach/entry-macro.S
+++ b/arch/arm/mach-shmobile/include/mach/entry-macro.S
@@ -27,8 +27,5 @@
.macro test_for_ipi, irqnr, irqstat, base, tmp
.endm
- .macro test_for_ltirq, irqnr, irqstat, base, tmp
- .endm
-
.macro arch_ret_to_user, tmp1, tmp2
.endm
--
1.7.0.4
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [RFC PATCH 3/3] ARM: gic, local timers: use the request_percpu_irq() interface
2011-09-15 16:52 ` Marc Zyngier
@ 2011-09-15 16:52 ` Marc Zyngier
-1 siblings, 0 replies; 38+ messages in thread
From: Marc Zyngier @ 2011-09-15 16:52 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel; +Cc: David Brown
This patch remove the hardcoded link between local timers and PPIs,
and convert the PPI users (TWD and MSM timers) to the new *_percpu_irq
interface.
PPIs are now useable for more than just the local timers.
Cc: David Brown <davidb@codeaurora.org>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
arch/arm/common/gic.c | 52 --------------------------
arch/arm/include/asm/hardware/gic.h | 1 -
arch/arm/include/asm/localtimer.h | 17 ++++-----
arch/arm/include/asm/smp_twd.h | 2 +-
arch/arm/kernel/smp.c | 16 +--------
arch/arm/kernel/smp_twd.c | 47 +++++++++++++++++++++++-
arch/arm/mach-exynos4/mct.c | 5 ---
arch/arm/mach-msm/timer.c | 69 ++++++++++++++++++++---------------
8 files changed, 94 insertions(+), 115 deletions(-)
diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
index 4bbcce8..8e2f0c3 100644
--- a/arch/arm/common/gic.c
+++ b/arch/arm/common/gic.c
@@ -35,7 +35,6 @@
#include <asm/irq.h>
#include <asm/mach/irq.h>
#include <asm/hardware/gic.h>
-#include <asm/localtimer.h>
static DEFINE_SPINLOCK(irq_controller_lock);
@@ -259,32 +258,6 @@ void __init gic_cascade_irq(unsigned int gic_nr, unsigned int irq)
irq_set_chained_handler(irq, gic_handle_cascade_irq);
}
-#ifdef CONFIG_LOCAL_TIMERS
-#define gic_ppi_handler percpu_timer_handler
-#else
-static irqreturn_t gic_ppi_handler(int irq, void *dev_id)
-{
- return IRQ_NONE;
-}
-#endif
-
-#define PPI_IRQACT(nr) \
- { \
- .handler = gic_ppi_handler, \
- .flags = IRQF_PERCPU | IRQF_TIMER, \
- .irq = nr, \
- .name = "PPI-" # nr, \
- }
-
-static struct irqaction ppi_irqaction_template[16] __initdata = {
- PPI_IRQACT(0), PPI_IRQACT(1), PPI_IRQACT(2), PPI_IRQACT(3),
- PPI_IRQACT(4), PPI_IRQACT(5), PPI_IRQACT(6), PPI_IRQACT(7),
- PPI_IRQACT(8), PPI_IRQACT(9), PPI_IRQACT(10), PPI_IRQACT(11),
- PPI_IRQACT(12), PPI_IRQACT(13), PPI_IRQACT(14), PPI_IRQACT(15),
-};
-
-static struct irqaction *ppi_irqaction;
-
static void __init gic_dist_init(struct gic_chip_data *gic,
unsigned int irq_start)
{
@@ -320,16 +293,6 @@ static void __init gic_dist_init(struct gic_chip_data *gic,
if (gic == &gic_data[0]) {
nrppis = 16 - (irq_start & 15);
ppi_base = gic->irq_offset + 32 - nrppis;
-
- ppi_irqaction = kmemdup(&ppi_irqaction_template[16 - nrppis],
- sizeof(*ppi_irqaction) * nrppis,
- GFP_KERNEL);
-
- if (nrppis && !ppi_irqaction) {
- pr_err("GIC: Can't allocate PPI memory");
- nrppis = 0;
- ppi_base = 0;
- }
}
pr_info("Configuring GIC with %d sources (%d PPIs)\n",
@@ -372,17 +335,12 @@ static void __init gic_dist_init(struct gic_chip_data *gic,
*/
for (i = 0; i < nrppis; i++) {
int ppi = i + ppi_base;
- int err;
irq_set_percpu_devid(ppi);
irq_set_chip_and_handler(ppi, &gic_chip,
handle_percpu_devid_irq);
irq_set_chip_data(ppi, gic);
set_irq_flags(ppi, IRQF_VALID | IRQF_NOAUTOEN);
-
- err = setup_percpu_irq(ppi, &ppi_irqaction[i]);
- if (err)
- pr_err("GIC: can't setup PPI%d (%d)\n", ppi, err);
}
for (i = irq_start + nrppis; i < irq_limit; i++) {
@@ -443,16 +401,6 @@ void __cpuinit gic_secondary_init(unsigned int gic_nr)
gic_cpu_init(&gic_data[gic_nr]);
}
-void __cpuinit gic_enable_ppi(unsigned int irq)
-{
- unsigned long flags;
-
- local_irq_save(flags);
- irq_set_status_flags(irq, IRQ_NOPROBE);
- gic_unmask_irq(irq_get_irq_data(irq));
- local_irq_restore(flags);
-}
-
#ifdef CONFIG_SMP
void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
{
diff --git a/arch/arm/include/asm/hardware/gic.h b/arch/arm/include/asm/hardware/gic.h
index 435d3f8..2dadd50 100644
--- a/arch/arm/include/asm/hardware/gic.h
+++ b/arch/arm/include/asm/hardware/gic.h
@@ -40,7 +40,6 @@ void gic_init(unsigned int, unsigned int, void __iomem *, void __iomem *);
void gic_secondary_init(unsigned int);
void gic_cascade_irq(unsigned int gic_nr, unsigned int irq);
void gic_raise_softirq(const struct cpumask *mask, unsigned int irq);
-void gic_enable_ppi(unsigned int);
struct gic_chip_data {
unsigned int irq_offset;
diff --git a/arch/arm/include/asm/localtimer.h b/arch/arm/include/asm/localtimer.h
index e3663f7..f5e1cec 100644
--- a/arch/arm/include/asm/localtimer.h
+++ b/arch/arm/include/asm/localtimer.h
@@ -19,27 +19,20 @@ struct clock_event_device;
*/
void percpu_timer_setup(void);
-/*
- * Per-cpu timer IRQ handler
- */
-irqreturn_t percpu_timer_handler(int irq, void *dev_id);
-
-
#ifdef CONFIG_LOCAL_TIMERS
#ifdef CONFIG_HAVE_ARM_TWD
#include "smp_twd.h"
-#define local_timer_ack() twd_timer_ack()
+#define local_timer_stop(c) twd_timer_stop((c))
#else
/*
- * Platform provides this to acknowledge a local timer IRQ.
- * Returns true if the local timer IRQ is to be processed.
+ * Stop the local timer
*/
-int local_timer_ack(void);
+void local_timer_stop(struct clock_event_device *);
#endif
@@ -54,6 +47,10 @@ static inline int local_timer_setup(struct clock_event_device *evt)
{
return -ENXIO;
}
+
+static inline void local_timer_stop(struct clock_event_device *evt)
+{
+}
#endif
#endif
diff --git a/arch/arm/include/asm/smp_twd.h b/arch/arm/include/asm/smp_twd.h
index fed9981..ef9ffba 100644
--- a/arch/arm/include/asm/smp_twd.h
+++ b/arch/arm/include/asm/smp_twd.h
@@ -22,7 +22,7 @@ struct clock_event_device;
extern void __iomem *twd_base;
-int twd_timer_ack(void);
void twd_timer_setup(struct clock_event_device *);
+void twd_timer_stop(struct clock_event_device *);
#endif
diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index e3dbd99..c206aed 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -520,20 +520,6 @@ static void ipi_timer(void)
irq_exit();
}
-#ifdef CONFIG_LOCAL_TIMERS
-irqreturn_t percpu_timer_handler(int irq, void *dev_id)
-{
- struct clock_event_device *evt = &__get_cpu_var(percpu_clockevent);
-
- if (local_timer_ack()) {
- evt->event_handler(evt);
- return IRQ_HANDLED;
- }
-
- return IRQ_NONE;
-}
-#endif
-
#ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
static void smp_timer_broadcast(const struct cpumask *mask)
{
@@ -584,7 +570,7 @@ static void percpu_timer_stop(void)
unsigned int cpu = smp_processor_id();
struct clock_event_device *evt = &per_cpu(percpu_clockevent, cpu);
- evt->set_mode(CLOCK_EVT_MODE_UNUSED, evt);
+ local_timer_stop(evt);
}
#endif
diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
index 01c1862..566c0fe 100644
--- a/arch/arm/kernel/smp_twd.c
+++ b/arch/arm/kernel/smp_twd.c
@@ -19,6 +19,7 @@
#include <linux/io.h>
#include <asm/smp_twd.h>
+#include <asm/localtimer.h>
#include <asm/hardware/gic.h>
/* set up by the platform code */
@@ -26,6 +27,8 @@ void __iomem *twd_base;
static unsigned long twd_timer_rate;
+static struct clock_event_device __percpu **twd_evt;
+
static void twd_set_mode(enum clock_event_mode mode,
struct clock_event_device *clk)
{
@@ -80,6 +83,12 @@ int twd_timer_ack(void)
return 0;
}
+void twd_timer_stop(struct clock_event_device *clk)
+{
+ twd_set_mode(CLOCK_EVT_MODE_UNUSED, clk);
+ disable_percpu_irq(clk->irq);
+}
+
static void __cpuinit twd_calibrate_rate(void)
{
unsigned long count;
@@ -119,11 +128,43 @@ static void __cpuinit twd_calibrate_rate(void)
}
}
+static irqreturn_t twd_handler(int irq, void *dev_id)
+{
+ struct clock_event_device *evt = *(struct clock_event_device **)dev_id;
+
+ if (twd_timer_ack()) {
+ evt->event_handler(evt);
+ return IRQ_HANDLED;
+ }
+
+ return IRQ_NONE;
+}
+
/*
* Setup the local clock events for a CPU.
*/
void __cpuinit twd_timer_setup(struct clock_event_device *clk)
{
+ struct clock_event_device **this_cpu_clk;
+
+ if (!twd_evt) {
+ int err;
+
+ twd_evt = alloc_percpu(struct clock_event_device *);
+ if (!twd_evt) {
+ pr_err("twd: can't allocate memory\n");
+ return;
+ }
+
+ err = request_percpu_irq(clk->irq, twd_handler,
+ "twd", twd_evt);
+ if (err) {
+ pr_err("twd: can't register interrupt %d (%d)\n",
+ clk->irq, err);
+ return;
+ }
+ }
+
twd_calibrate_rate();
clk->name = "local_timer";
@@ -137,8 +178,10 @@ void __cpuinit twd_timer_setup(struct clock_event_device *clk)
clk->max_delta_ns = clockevent_delta2ns(0xffffffff, clk);
clk->min_delta_ns = clockevent_delta2ns(0xf, clk);
+ this_cpu_clk = __this_cpu_ptr(twd_evt);
+ *this_cpu_clk = clk;
+
clockevents_register_device(clk);
- /* Make sure our local interrupt controller has this enabled */
- gic_enable_ppi(clk->irq);
+ enable_percpu_irq(clk->irq);
}
diff --git a/arch/arm/mach-exynos4/mct.c b/arch/arm/mach-exynos4/mct.c
index f3638fa..3a1e4f3 100644
--- a/arch/arm/mach-exynos4/mct.c
+++ b/arch/arm/mach-exynos4/mct.c
@@ -396,11 +396,6 @@ int __cpuinit local_timer_setup(struct clock_event_device *evt)
return 0;
}
-int local_timer_ack(void)
-{
- return 0;
-}
-
#endif /* CONFIG_LOCAL_TIMERS */
static void __init exynos4_timer_resources(void)
diff --git a/arch/arm/mach-msm/timer.c b/arch/arm/mach-msm/timer.c
index 63621f1..701d5e6 100644
--- a/arch/arm/mach-msm/timer.c
+++ b/arch/arm/mach-msm/timer.c
@@ -71,12 +71,16 @@ enum timer_location {
struct msm_clock {
struct clock_event_device clockevent;
struct clocksource clocksource;
- struct irqaction irq;
+ unsigned int irq;
void __iomem *regbase;
uint32_t freq;
uint32_t shift;
void __iomem *global_counter;
void __iomem *local_counter;
+ union {
+ struct clock_event_device *evt;
+ struct clock_event_device __percpu **percpu_evt;
+ };
};
enum {
@@ -87,13 +91,10 @@ enum {
static struct msm_clock msm_clocks[];
-static struct clock_event_device *local_clock_event;
static irqreturn_t msm_timer_interrupt(int irq, void *dev_id)
{
- struct clock_event_device *evt = dev_id;
- if (smp_processor_id() != 0)
- evt = local_clock_event;
+ struct clock_event_device *evt = *(struct clock_event_device **)dev_id;
if (evt->event_handler == NULL)
return IRQ_HANDLED;
evt->event_handler(evt);
@@ -171,13 +172,7 @@ static struct msm_clock msm_clocks[] = {
.mask = CLOCKSOURCE_MASK(32),
.flags = CLOCK_SOURCE_IS_CONTINUOUS,
},
- .irq = {
- .name = "gp_timer",
- .flags = IRQF_DISABLED | IRQF_TIMER | IRQF_TRIGGER_RISING,
- .handler = msm_timer_interrupt,
- .dev_id = &msm_clocks[0].clockevent,
- .irq = INT_GP_TIMER_EXP
- },
+ .irq = INT_GP_TIMER_EXP,
.freq = GPT_HZ,
},
[MSM_CLOCK_DGT] = {
@@ -196,13 +191,7 @@ static struct msm_clock msm_clocks[] = {
.mask = CLOCKSOURCE_MASK((32 - MSM_DGT_SHIFT)),
.flags = CLOCK_SOURCE_IS_CONTINUOUS,
},
- .irq = {
- .name = "dg_timer",
- .flags = IRQF_DISABLED | IRQF_TIMER | IRQF_TRIGGER_RISING,
- .handler = msm_timer_interrupt,
- .dev_id = &msm_clocks[1].clockevent,
- .irq = INT_DEBUG_TIMER_EXP
- },
+ .irq = INT_DEBUG_TIMER_EXP,
.freq = DGT_HZ >> MSM_DGT_SHIFT,
.shift = MSM_DGT_SHIFT,
}
@@ -261,10 +250,30 @@ static void __init msm_timer_init(void)
printk(KERN_ERR "msm_timer_init: clocksource_register "
"failed for %s\n", cs->name);
- res = setup_irq(clock->irq.irq, &clock->irq);
+ ce->irq = clock->irq;
+ if (cpu_is_msm8x60() || cpu_is_msm8960()) {
+ clock->percpu_evt = alloc_percpu(struct clock_event_device *);
+ if (!clock->percpu_evt) {
+ pr_err("msm_timer_init: memory allocation "
+ "failed for %s\n", ce->name);
+ continue;
+ }
+
+ *__this_cpu_ptr(clock->percpu_evt) = ce;
+ res = request_percpu_irq(ce->irq, msm_timer_interrupt,
+ ce->name, clock->percpu_evt);
+ if (!res)
+ enable_percpu_irq(ce->irq);
+ } else {
+ clock->evt = ce;
+ res = request_irq(ce->irq, msm_timer_interrupt,
+ IRQF_TIMER | IRQF_NOBALANCING | IRQF_TRIGGER_RISING,
+ ce->name, &clock->evt);
+ }
+
if (res)
- printk(KERN_ERR "msm_timer_init: setup_irq "
- "failed for %s\n", cs->name);
+ pr_err("msm_timer_init: request_irq failed for %s\n",
+ ce->name);
clockevents_register_device(ce);
}
@@ -273,6 +282,7 @@ static void __init msm_timer_init(void)
#ifdef CONFIG_SMP
int __cpuinit local_timer_setup(struct clock_event_device *evt)
{
+ static bool local_timer_inited;
struct msm_clock *clock = &msm_clocks[MSM_GLOBAL_TIMER];
/* Use existing clock_event for cpu 0 */
@@ -281,12 +291,13 @@ int __cpuinit local_timer_setup(struct clock_event_device *evt)
writel(DGT_CLK_CTL_DIV_4, MSM_TMR_BASE + DGT_CLK_CTL);
- if (!local_clock_event) {
+ if (!local_timer_inited) {
writel(0, clock->regbase + TIMER_ENABLE);
writel(0, clock->regbase + TIMER_CLEAR);
writel(~0, clock->regbase + TIMER_MATCH_VAL);
+ local_timer_inited = true;
}
- evt->irq = clock->irq.irq;
+ evt->irq = clock->irq;
evt->name = "local_timer";
evt->features = CLOCK_EVT_FEAT_ONESHOT;
evt->rating = clock->clockevent.rating;
@@ -298,17 +309,17 @@ int __cpuinit local_timer_setup(struct clock_event_device *evt)
clockevent_delta2ns(0xf0000000 >> clock->shift, evt);
evt->min_delta_ns = clockevent_delta2ns(4, evt);
- local_clock_event = evt;
-
- gic_enable_ppi(clock->irq.irq);
+ *__this_cpu_ptr(clock->percpu_evt) = evt;
+ enable_percpu_irq(evt->irq);
clockevents_register_device(evt);
return 0;
}
-inline int local_timer_ack(void)
+void local_timer_stop(struct clock_event_device *evt)
{
- return 1;
+ evt->set_mode(CLOCK_EVT_MODE_UNUSED, evt);
+ disable_percpu_irq(evt->irq);
}
#endif
--
1.7.0.4
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [RFC PATCH 3/3] ARM: gic, local timers: use the request_percpu_irq() interface
@ 2011-09-15 16:52 ` Marc Zyngier
0 siblings, 0 replies; 38+ messages in thread
From: Marc Zyngier @ 2011-09-15 16:52 UTC (permalink / raw)
To: linux-arm-kernel
This patch remove the hardcoded link between local timers and PPIs,
and convert the PPI users (TWD and MSM timers) to the new *_percpu_irq
interface.
PPIs are now useable for more than just the local timers.
Cc: David Brown <davidb@codeaurora.org>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
arch/arm/common/gic.c | 52 --------------------------
arch/arm/include/asm/hardware/gic.h | 1 -
arch/arm/include/asm/localtimer.h | 17 ++++-----
arch/arm/include/asm/smp_twd.h | 2 +-
arch/arm/kernel/smp.c | 16 +--------
arch/arm/kernel/smp_twd.c | 47 +++++++++++++++++++++++-
arch/arm/mach-exynos4/mct.c | 5 ---
arch/arm/mach-msm/timer.c | 69 ++++++++++++++++++++---------------
8 files changed, 94 insertions(+), 115 deletions(-)
diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
index 4bbcce8..8e2f0c3 100644
--- a/arch/arm/common/gic.c
+++ b/arch/arm/common/gic.c
@@ -35,7 +35,6 @@
#include <asm/irq.h>
#include <asm/mach/irq.h>
#include <asm/hardware/gic.h>
-#include <asm/localtimer.h>
static DEFINE_SPINLOCK(irq_controller_lock);
@@ -259,32 +258,6 @@ void __init gic_cascade_irq(unsigned int gic_nr, unsigned int irq)
irq_set_chained_handler(irq, gic_handle_cascade_irq);
}
-#ifdef CONFIG_LOCAL_TIMERS
-#define gic_ppi_handler percpu_timer_handler
-#else
-static irqreturn_t gic_ppi_handler(int irq, void *dev_id)
-{
- return IRQ_NONE;
-}
-#endif
-
-#define PPI_IRQACT(nr) \
- { \
- .handler = gic_ppi_handler, \
- .flags = IRQF_PERCPU | IRQF_TIMER, \
- .irq = nr, \
- .name = "PPI-" # nr, \
- }
-
-static struct irqaction ppi_irqaction_template[16] __initdata = {
- PPI_IRQACT(0), PPI_IRQACT(1), PPI_IRQACT(2), PPI_IRQACT(3),
- PPI_IRQACT(4), PPI_IRQACT(5), PPI_IRQACT(6), PPI_IRQACT(7),
- PPI_IRQACT(8), PPI_IRQACT(9), PPI_IRQACT(10), PPI_IRQACT(11),
- PPI_IRQACT(12), PPI_IRQACT(13), PPI_IRQACT(14), PPI_IRQACT(15),
-};
-
-static struct irqaction *ppi_irqaction;
-
static void __init gic_dist_init(struct gic_chip_data *gic,
unsigned int irq_start)
{
@@ -320,16 +293,6 @@ static void __init gic_dist_init(struct gic_chip_data *gic,
if (gic == &gic_data[0]) {
nrppis = 16 - (irq_start & 15);
ppi_base = gic->irq_offset + 32 - nrppis;
-
- ppi_irqaction = kmemdup(&ppi_irqaction_template[16 - nrppis],
- sizeof(*ppi_irqaction) * nrppis,
- GFP_KERNEL);
-
- if (nrppis && !ppi_irqaction) {
- pr_err("GIC: Can't allocate PPI memory");
- nrppis = 0;
- ppi_base = 0;
- }
}
pr_info("Configuring GIC with %d sources (%d PPIs)\n",
@@ -372,17 +335,12 @@ static void __init gic_dist_init(struct gic_chip_data *gic,
*/
for (i = 0; i < nrppis; i++) {
int ppi = i + ppi_base;
- int err;
irq_set_percpu_devid(ppi);
irq_set_chip_and_handler(ppi, &gic_chip,
handle_percpu_devid_irq);
irq_set_chip_data(ppi, gic);
set_irq_flags(ppi, IRQF_VALID | IRQF_NOAUTOEN);
-
- err = setup_percpu_irq(ppi, &ppi_irqaction[i]);
- if (err)
- pr_err("GIC: can't setup PPI%d (%d)\n", ppi, err);
}
for (i = irq_start + nrppis; i < irq_limit; i++) {
@@ -443,16 +401,6 @@ void __cpuinit gic_secondary_init(unsigned int gic_nr)
gic_cpu_init(&gic_data[gic_nr]);
}
-void __cpuinit gic_enable_ppi(unsigned int irq)
-{
- unsigned long flags;
-
- local_irq_save(flags);
- irq_set_status_flags(irq, IRQ_NOPROBE);
- gic_unmask_irq(irq_get_irq_data(irq));
- local_irq_restore(flags);
-}
-
#ifdef CONFIG_SMP
void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
{
diff --git a/arch/arm/include/asm/hardware/gic.h b/arch/arm/include/asm/hardware/gic.h
index 435d3f8..2dadd50 100644
--- a/arch/arm/include/asm/hardware/gic.h
+++ b/arch/arm/include/asm/hardware/gic.h
@@ -40,7 +40,6 @@ void gic_init(unsigned int, unsigned int, void __iomem *, void __iomem *);
void gic_secondary_init(unsigned int);
void gic_cascade_irq(unsigned int gic_nr, unsigned int irq);
void gic_raise_softirq(const struct cpumask *mask, unsigned int irq);
-void gic_enable_ppi(unsigned int);
struct gic_chip_data {
unsigned int irq_offset;
diff --git a/arch/arm/include/asm/localtimer.h b/arch/arm/include/asm/localtimer.h
index e3663f7..f5e1cec 100644
--- a/arch/arm/include/asm/localtimer.h
+++ b/arch/arm/include/asm/localtimer.h
@@ -19,27 +19,20 @@ struct clock_event_device;
*/
void percpu_timer_setup(void);
-/*
- * Per-cpu timer IRQ handler
- */
-irqreturn_t percpu_timer_handler(int irq, void *dev_id);
-
-
#ifdef CONFIG_LOCAL_TIMERS
#ifdef CONFIG_HAVE_ARM_TWD
#include "smp_twd.h"
-#define local_timer_ack() twd_timer_ack()
+#define local_timer_stop(c) twd_timer_stop((c))
#else
/*
- * Platform provides this to acknowledge a local timer IRQ.
- * Returns true if the local timer IRQ is to be processed.
+ * Stop the local timer
*/
-int local_timer_ack(void);
+void local_timer_stop(struct clock_event_device *);
#endif
@@ -54,6 +47,10 @@ static inline int local_timer_setup(struct clock_event_device *evt)
{
return -ENXIO;
}
+
+static inline void local_timer_stop(struct clock_event_device *evt)
+{
+}
#endif
#endif
diff --git a/arch/arm/include/asm/smp_twd.h b/arch/arm/include/asm/smp_twd.h
index fed9981..ef9ffba 100644
--- a/arch/arm/include/asm/smp_twd.h
+++ b/arch/arm/include/asm/smp_twd.h
@@ -22,7 +22,7 @@ struct clock_event_device;
extern void __iomem *twd_base;
-int twd_timer_ack(void);
void twd_timer_setup(struct clock_event_device *);
+void twd_timer_stop(struct clock_event_device *);
#endif
diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index e3dbd99..c206aed 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -520,20 +520,6 @@ static void ipi_timer(void)
irq_exit();
}
-#ifdef CONFIG_LOCAL_TIMERS
-irqreturn_t percpu_timer_handler(int irq, void *dev_id)
-{
- struct clock_event_device *evt = &__get_cpu_var(percpu_clockevent);
-
- if (local_timer_ack()) {
- evt->event_handler(evt);
- return IRQ_HANDLED;
- }
-
- return IRQ_NONE;
-}
-#endif
-
#ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
static void smp_timer_broadcast(const struct cpumask *mask)
{
@@ -584,7 +570,7 @@ static void percpu_timer_stop(void)
unsigned int cpu = smp_processor_id();
struct clock_event_device *evt = &per_cpu(percpu_clockevent, cpu);
- evt->set_mode(CLOCK_EVT_MODE_UNUSED, evt);
+ local_timer_stop(evt);
}
#endif
diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
index 01c1862..566c0fe 100644
--- a/arch/arm/kernel/smp_twd.c
+++ b/arch/arm/kernel/smp_twd.c
@@ -19,6 +19,7 @@
#include <linux/io.h>
#include <asm/smp_twd.h>
+#include <asm/localtimer.h>
#include <asm/hardware/gic.h>
/* set up by the platform code */
@@ -26,6 +27,8 @@ void __iomem *twd_base;
static unsigned long twd_timer_rate;
+static struct clock_event_device __percpu **twd_evt;
+
static void twd_set_mode(enum clock_event_mode mode,
struct clock_event_device *clk)
{
@@ -80,6 +83,12 @@ int twd_timer_ack(void)
return 0;
}
+void twd_timer_stop(struct clock_event_device *clk)
+{
+ twd_set_mode(CLOCK_EVT_MODE_UNUSED, clk);
+ disable_percpu_irq(clk->irq);
+}
+
static void __cpuinit twd_calibrate_rate(void)
{
unsigned long count;
@@ -119,11 +128,43 @@ static void __cpuinit twd_calibrate_rate(void)
}
}
+static irqreturn_t twd_handler(int irq, void *dev_id)
+{
+ struct clock_event_device *evt = *(struct clock_event_device **)dev_id;
+
+ if (twd_timer_ack()) {
+ evt->event_handler(evt);
+ return IRQ_HANDLED;
+ }
+
+ return IRQ_NONE;
+}
+
/*
* Setup the local clock events for a CPU.
*/
void __cpuinit twd_timer_setup(struct clock_event_device *clk)
{
+ struct clock_event_device **this_cpu_clk;
+
+ if (!twd_evt) {
+ int err;
+
+ twd_evt = alloc_percpu(struct clock_event_device *);
+ if (!twd_evt) {
+ pr_err("twd: can't allocate memory\n");
+ return;
+ }
+
+ err = request_percpu_irq(clk->irq, twd_handler,
+ "twd", twd_evt);
+ if (err) {
+ pr_err("twd: can't register interrupt %d (%d)\n",
+ clk->irq, err);
+ return;
+ }
+ }
+
twd_calibrate_rate();
clk->name = "local_timer";
@@ -137,8 +178,10 @@ void __cpuinit twd_timer_setup(struct clock_event_device *clk)
clk->max_delta_ns = clockevent_delta2ns(0xffffffff, clk);
clk->min_delta_ns = clockevent_delta2ns(0xf, clk);
+ this_cpu_clk = __this_cpu_ptr(twd_evt);
+ *this_cpu_clk = clk;
+
clockevents_register_device(clk);
- /* Make sure our local interrupt controller has this enabled */
- gic_enable_ppi(clk->irq);
+ enable_percpu_irq(clk->irq);
}
diff --git a/arch/arm/mach-exynos4/mct.c b/arch/arm/mach-exynos4/mct.c
index f3638fa..3a1e4f3 100644
--- a/arch/arm/mach-exynos4/mct.c
+++ b/arch/arm/mach-exynos4/mct.c
@@ -396,11 +396,6 @@ int __cpuinit local_timer_setup(struct clock_event_device *evt)
return 0;
}
-int local_timer_ack(void)
-{
- return 0;
-}
-
#endif /* CONFIG_LOCAL_TIMERS */
static void __init exynos4_timer_resources(void)
diff --git a/arch/arm/mach-msm/timer.c b/arch/arm/mach-msm/timer.c
index 63621f1..701d5e6 100644
--- a/arch/arm/mach-msm/timer.c
+++ b/arch/arm/mach-msm/timer.c
@@ -71,12 +71,16 @@ enum timer_location {
struct msm_clock {
struct clock_event_device clockevent;
struct clocksource clocksource;
- struct irqaction irq;
+ unsigned int irq;
void __iomem *regbase;
uint32_t freq;
uint32_t shift;
void __iomem *global_counter;
void __iomem *local_counter;
+ union {
+ struct clock_event_device *evt;
+ struct clock_event_device __percpu **percpu_evt;
+ };
};
enum {
@@ -87,13 +91,10 @@ enum {
static struct msm_clock msm_clocks[];
-static struct clock_event_device *local_clock_event;
static irqreturn_t msm_timer_interrupt(int irq, void *dev_id)
{
- struct clock_event_device *evt = dev_id;
- if (smp_processor_id() != 0)
- evt = local_clock_event;
+ struct clock_event_device *evt = *(struct clock_event_device **)dev_id;
if (evt->event_handler == NULL)
return IRQ_HANDLED;
evt->event_handler(evt);
@@ -171,13 +172,7 @@ static struct msm_clock msm_clocks[] = {
.mask = CLOCKSOURCE_MASK(32),
.flags = CLOCK_SOURCE_IS_CONTINUOUS,
},
- .irq = {
- .name = "gp_timer",
- .flags = IRQF_DISABLED | IRQF_TIMER | IRQF_TRIGGER_RISING,
- .handler = msm_timer_interrupt,
- .dev_id = &msm_clocks[0].clockevent,
- .irq = INT_GP_TIMER_EXP
- },
+ .irq = INT_GP_TIMER_EXP,
.freq = GPT_HZ,
},
[MSM_CLOCK_DGT] = {
@@ -196,13 +191,7 @@ static struct msm_clock msm_clocks[] = {
.mask = CLOCKSOURCE_MASK((32 - MSM_DGT_SHIFT)),
.flags = CLOCK_SOURCE_IS_CONTINUOUS,
},
- .irq = {
- .name = "dg_timer",
- .flags = IRQF_DISABLED | IRQF_TIMER | IRQF_TRIGGER_RISING,
- .handler = msm_timer_interrupt,
- .dev_id = &msm_clocks[1].clockevent,
- .irq = INT_DEBUG_TIMER_EXP
- },
+ .irq = INT_DEBUG_TIMER_EXP,
.freq = DGT_HZ >> MSM_DGT_SHIFT,
.shift = MSM_DGT_SHIFT,
}
@@ -261,10 +250,30 @@ static void __init msm_timer_init(void)
printk(KERN_ERR "msm_timer_init: clocksource_register "
"failed for %s\n", cs->name);
- res = setup_irq(clock->irq.irq, &clock->irq);
+ ce->irq = clock->irq;
+ if (cpu_is_msm8x60() || cpu_is_msm8960()) {
+ clock->percpu_evt = alloc_percpu(struct clock_event_device *);
+ if (!clock->percpu_evt) {
+ pr_err("msm_timer_init: memory allocation "
+ "failed for %s\n", ce->name);
+ continue;
+ }
+
+ *__this_cpu_ptr(clock->percpu_evt) = ce;
+ res = request_percpu_irq(ce->irq, msm_timer_interrupt,
+ ce->name, clock->percpu_evt);
+ if (!res)
+ enable_percpu_irq(ce->irq);
+ } else {
+ clock->evt = ce;
+ res = request_irq(ce->irq, msm_timer_interrupt,
+ IRQF_TIMER | IRQF_NOBALANCING | IRQF_TRIGGER_RISING,
+ ce->name, &clock->evt);
+ }
+
if (res)
- printk(KERN_ERR "msm_timer_init: setup_irq "
- "failed for %s\n", cs->name);
+ pr_err("msm_timer_init: request_irq failed for %s\n",
+ ce->name);
clockevents_register_device(ce);
}
@@ -273,6 +282,7 @@ static void __init msm_timer_init(void)
#ifdef CONFIG_SMP
int __cpuinit local_timer_setup(struct clock_event_device *evt)
{
+ static bool local_timer_inited;
struct msm_clock *clock = &msm_clocks[MSM_GLOBAL_TIMER];
/* Use existing clock_event for cpu 0 */
@@ -281,12 +291,13 @@ int __cpuinit local_timer_setup(struct clock_event_device *evt)
writel(DGT_CLK_CTL_DIV_4, MSM_TMR_BASE + DGT_CLK_CTL);
- if (!local_clock_event) {
+ if (!local_timer_inited) {
writel(0, clock->regbase + TIMER_ENABLE);
writel(0, clock->regbase + TIMER_CLEAR);
writel(~0, clock->regbase + TIMER_MATCH_VAL);
+ local_timer_inited = true;
}
- evt->irq = clock->irq.irq;
+ evt->irq = clock->irq;
evt->name = "local_timer";
evt->features = CLOCK_EVT_FEAT_ONESHOT;
evt->rating = clock->clockevent.rating;
@@ -298,17 +309,17 @@ int __cpuinit local_timer_setup(struct clock_event_device *evt)
clockevent_delta2ns(0xf0000000 >> clock->shift, evt);
evt->min_delta_ns = clockevent_delta2ns(4, evt);
- local_clock_event = evt;
-
- gic_enable_ppi(clock->irq.irq);
+ *__this_cpu_ptr(clock->percpu_evt) = evt;
+ enable_percpu_irq(evt->irq);
clockevents_register_device(evt);
return 0;
}
-inline int local_timer_ack(void)
+void local_timer_stop(struct clock_event_device *evt)
{
- return 1;
+ evt->set_mode(CLOCK_EVT_MODE_UNUSED, evt);
+ disable_percpu_irq(evt->irq);
}
#endif
--
1.7.0.4
^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 1/3] genirq: add support for per-cpu dev_id interrupts
2011-09-15 16:52 ` Marc Zyngier
@ 2011-09-15 21:36 ` Michał Mirosław
-1 siblings, 0 replies; 38+ messages in thread
From: Michał Mirosław @ 2011-09-15 21:36 UTC (permalink / raw)
To: Marc Zyngier; +Cc: linux-arm-kernel, linux-kernel, Thomas Gleixner
2011/9/15 Marc Zyngier <marc.zyngier@arm.com>:
[...]
> diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
> index a103732..f9b7fa3 100644
> --- a/include/linux/interrupt.h
> +++ b/include/linux/interrupt.h
> @@ -95,6 +95,7 @@ typedef irqreturn_t (*irq_handler_t)(int, void *);
> * @flags: flags (see IRQF_* above)
> * @name: name of the device
> * @dev_id: cookie to identify the device
> + * @percpu_dev_id: cookie to identify the device
> * @next: pointer to the next irqaction for shared interrupts
> * @irq: interrupt number
> * @dir: pointer to the proc/irq/NN/name entry
> @@ -104,17 +105,20 @@ typedef irqreturn_t (*irq_handler_t)(int, void *);
> * @thread_mask: bitmask for keeping track of @thread activity
> */
> struct irqaction {
[...]
> + void *dev_id;
> +#ifdef CONFIG_IRQ_PERCPU_DEVID
> + void __percpu *percpu_dev_id;
> +#endif
Those two can share the memory (in a anonymous union), if I read the
idea correctly.
Best Regards,
Michał Mirosław
^ permalink raw reply [flat|nested] 38+ messages in thread
* [RFC PATCH 1/3] genirq: add support for per-cpu dev_id interrupts
@ 2011-09-15 21:36 ` Michał Mirosław
0 siblings, 0 replies; 38+ messages in thread
From: Michał Mirosław @ 2011-09-15 21:36 UTC (permalink / raw)
To: linux-arm-kernel
2011/9/15 Marc Zyngier <marc.zyngier@arm.com>:
[...]
> diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
> index a103732..f9b7fa3 100644
> --- a/include/linux/interrupt.h
> +++ b/include/linux/interrupt.h
> @@ -95,6 +95,7 @@ typedef irqreturn_t (*irq_handler_t)(int, void *);
> ?* @flags: ? ? flags (see IRQF_* above)
> ?* @name: ? ? ?name of the device
> ?* @dev_id: ? ?cookie to identify the device
> + * @percpu_dev_id: ? ? cookie to identify the device
> ?* @next: ? ? ?pointer to the next irqaction for shared interrupts
> ?* @irq: ? ? ? interrupt number
> ?* @dir: ? ? ? pointer to the proc/irq/NN/name entry
> @@ -104,17 +105,20 @@ typedef irqreturn_t (*irq_handler_t)(int, void *);
> ?* @thread_mask: ? ? ? bitmask for keeping track of @thread activity
> ?*/
> ?struct irqaction {
[...]
> + ? ? ? void ? ? ? ? ? ? ? ? ? ?*dev_id;
> +#ifdef CONFIG_IRQ_PERCPU_DEVID
> + ? ? ? void __percpu ? ? ? ? ? *percpu_dev_id;
> +#endif
Those two can share the memory (in a anonymous union), if I read the
idea correctly.
Best Regards,
Micha? Miros?aw
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 1/3] genirq: add support for per-cpu dev_id interrupts
2011-09-15 16:52 ` Marc Zyngier
@ 2011-09-15 22:49 ` Thomas Gleixner
-1 siblings, 0 replies; 38+ messages in thread
From: Thomas Gleixner @ 2011-09-15 22:49 UTC (permalink / raw)
To: Marc Zyngier; +Cc: linux-arm-kernel, linux-kernel
Marc,
On Thu, 15 Sep 2011, Marc Zyngier wrote:
>
> Based on an initial patch by Thomas Gleixner.
Was more an idea than a patch :)
> + void *dev_id;
> +#ifdef CONFIG_IRQ_PERCPU_DEVID
> + void __percpu *percpu_dev_id;
> +#endif
Can we please avoid that ifdef and use an anon union ?
> +#ifdef CONFIG_IRQ_PERCPU_DEVID
> +static inline int irq_set_percpu_devid(unsigned int irq)
Real function in kernel/irq/ please
> +{
> + struct irq_desc *desc = irq_to_desc(irq);
> +
> + if (!desc)
> + return -EINVAL;
> +
> + if (!zalloc_cpumask_var(&desc->percpu_enabled, GFP_KERNEL))
> + return -ENOMEM;
What prevents that allocation happening more than once ?
> + irq_set_status_flags(irq,
> + IRQ_NOAUTOEN | IRQ_PER_CPU | IRQ_NOTHREAD |
> + IRQ_NOPROBE | IRQ_PER_CPU_DEVID);
> + return 0;
> +}
> +#endif
> +
> /* Handle dynamic irq creation and destruction */
> extern unsigned int create_irq_nr(unsigned int irq_want, int node);
> extern int create_irq(void);
> diff --git a/include/linux/irqdesc.h b/include/linux/irqdesc.h
> index 150134a..0b4419a 100644
> --- a/include/linux/irqdesc.h
> +++ b/include/linux/irqdesc.h
> @@ -53,6 +53,9 @@ struct irq_desc {
> unsigned long last_unhandled; /* Aging timer for unhandled count */
> unsigned int irqs_unhandled;
> raw_spinlock_t lock;
> +#ifdef CONFIG_IRQ_PERCPU_DEVID
Can we just make that a pointer and get rid of the config option ?
> +#ifdef CONFIG_IRQ_PERCPU_DEVID
> +void irq_percpu_enable(struct irq_desc *desc)
> +{
> + unsigned int cpu = get_cpu();
get_cpu() is overkill here. That's called with desc->lock held and
interrupts disabled. No way that the cpu can go away :)
I'd rather have a check in the calling function, which makes sure that
enable_percpu_irq() is called from proper per cpu context. See comment
below.
> void remove_irq(unsigned int irq, struct irqaction *act)
> {
> - __free_irq(irq, act->dev_id);
> + struct irq_desc *desc = irq_to_desc(irq);
> +
> + if (desc && !irq_settings_is_per_cpu_devid(desc))
That probably should be
if (!WARN_ON(....))
> + __free_irq(irq, act->dev_id);
> }
> EXPORT_SYMBOL_GPL(remove_irq);
>
> @@ -1246,7 +1251,7 @@ void free_irq(unsigned int irq, void *dev_id)
> {
> struct irq_desc *desc = irq_to_desc(irq);
>
> - if (!desc)
> + if (!desc || irq_settings_is_per_cpu_devid(desc))
> return;
Please make these percpu_devid checks WARN_ON so we can avoid stupid
questions on the mailing lists.
> #ifdef CONFIG_SMP
> @@ -1324,7 +1329,8 @@ int request_threaded_irq(unsigned int irq, irq_handler_t handler,
> if (!desc)
> return -EINVAL;
>
> - if (!irq_settings_can_request(desc))
> + if (!irq_settings_can_request(desc) ||
> + irq_settings_is_per_cpu_devid(desc))
> return -EINVAL;
That's fine.
> if (!handler) {
> @@ -1409,3 +1415,198 @@ int request_any_context_irq(unsigned int irq, irq_handler_t handler,
> return !ret ? IRQC_IS_HARDIRQ : ret;
> }
> EXPORT_SYMBOL_GPL(request_any_context_irq);
> +
> +#ifdef CONFIG_IRQ_PERCPU_DEVID
> +void enable_percpu_irq(unsigned int irq)
> +{
> + unsigned long flags;
> + struct irq_desc *desc = irq_get_desc_buslock(irq, &flags);
> +
> + if (!desc)
> + return;
> +
> + irq_percpu_enable(desc);
> + irq_put_desc_busunlock(desc, flags);
To make this luser proof and better debuggable I'd like to have that as:
int cpu = smp_processsor_id();
unsigned long flags;
....
irq_percpu_enable(desc, cpu);
That way we catch at least lusers who call that from random thread
context - assuming that they enabled some debug options .....
> +}
> +EXPORT_SYMBOL(enable_percpu_irq);
_GPL please if at all. Not sure whether this needs to be available for
modules.
> +/*
> + * Internal function to unregister a percpu irqaction.
> + */
> +static struct irqaction *__free_percpu_irq(unsigned int irq, void __percpu *dev_id)
> +{
> + struct irq_desc *desc = irq_to_desc(irq);
> + struct irqaction *action;
> + unsigned long flags;
> +
> + WARN(in_interrupt(), "Trying to free IRQ %d from IRQ context!\n", irq);
> +
> + if (!desc)
> + return NULL;
> +
> + raw_spin_lock_irqsave(&desc->lock, flags);
> +
> + action = desc->action;
> + if (!action || action->percpu_dev_id != dev_id) {
> + WARN(1, "Trying to free already-free IRQ %d\n", irq);
> + raw_spin_unlock_irqrestore(&desc->lock, flags);
> + return NULL;
> + }
> +
> + /* Found it - now remove it from the list of entries: */
> + WARN(!cpumask_empty(desc->percpu_enabled),
> + "percpu IRQ %d still enabled on CPU%d!\n",
> + irq, cpumask_first(desc->percpu_enabled));
> + desc->action = NULL;
> +
> +#ifdef CONFIG_SMP
> + /* make sure affinity_hint is cleaned up */
> + if (WARN_ON_ONCE(desc->affinity_hint))
> + desc->affinity_hint = NULL;
> +#endif
We don't want to have an affinity hint for percpu interrupts. That's
pretty useless AFACIT :)
> +
> + raw_spin_unlock_irqrestore(&desc->lock, flags);
> +
> + unregister_handler_proc(irq, action);
> +
> + /* Make sure it's not being used on another CPU: */
> + synchronize_irq(irq);
That's not helping w/o making synchronize_irq() aware of the percpu
stuff. Also there is the question whether we need the ability to
remove such interrupts in the first place. The target users are low
level arch interrupts not some random device drivers.
Otherwise this is a pretty cool patch!
Thanks,
tglx
^ permalink raw reply [flat|nested] 38+ messages in thread
* [RFC PATCH 1/3] genirq: add support for per-cpu dev_id interrupts
@ 2011-09-15 22:49 ` Thomas Gleixner
0 siblings, 0 replies; 38+ messages in thread
From: Thomas Gleixner @ 2011-09-15 22:49 UTC (permalink / raw)
To: linux-arm-kernel
Marc,
On Thu, 15 Sep 2011, Marc Zyngier wrote:
>
> Based on an initial patch by Thomas Gleixner.
Was more an idea than a patch :)
> + void *dev_id;
> +#ifdef CONFIG_IRQ_PERCPU_DEVID
> + void __percpu *percpu_dev_id;
> +#endif
Can we please avoid that ifdef and use an anon union ?
> +#ifdef CONFIG_IRQ_PERCPU_DEVID
> +static inline int irq_set_percpu_devid(unsigned int irq)
Real function in kernel/irq/ please
> +{
> + struct irq_desc *desc = irq_to_desc(irq);
> +
> + if (!desc)
> + return -EINVAL;
> +
> + if (!zalloc_cpumask_var(&desc->percpu_enabled, GFP_KERNEL))
> + return -ENOMEM;
What prevents that allocation happening more than once ?
> + irq_set_status_flags(irq,
> + IRQ_NOAUTOEN | IRQ_PER_CPU | IRQ_NOTHREAD |
> + IRQ_NOPROBE | IRQ_PER_CPU_DEVID);
> + return 0;
> +}
> +#endif
> +
> /* Handle dynamic irq creation and destruction */
> extern unsigned int create_irq_nr(unsigned int irq_want, int node);
> extern int create_irq(void);
> diff --git a/include/linux/irqdesc.h b/include/linux/irqdesc.h
> index 150134a..0b4419a 100644
> --- a/include/linux/irqdesc.h
> +++ b/include/linux/irqdesc.h
> @@ -53,6 +53,9 @@ struct irq_desc {
> unsigned long last_unhandled; /* Aging timer for unhandled count */
> unsigned int irqs_unhandled;
> raw_spinlock_t lock;
> +#ifdef CONFIG_IRQ_PERCPU_DEVID
Can we just make that a pointer and get rid of the config option ?
> +#ifdef CONFIG_IRQ_PERCPU_DEVID
> +void irq_percpu_enable(struct irq_desc *desc)
> +{
> + unsigned int cpu = get_cpu();
get_cpu() is overkill here. That's called with desc->lock held and
interrupts disabled. No way that the cpu can go away :)
I'd rather have a check in the calling function, which makes sure that
enable_percpu_irq() is called from proper per cpu context. See comment
below.
> void remove_irq(unsigned int irq, struct irqaction *act)
> {
> - __free_irq(irq, act->dev_id);
> + struct irq_desc *desc = irq_to_desc(irq);
> +
> + if (desc && !irq_settings_is_per_cpu_devid(desc))
That probably should be
if (!WARN_ON(....))
> + __free_irq(irq, act->dev_id);
> }
> EXPORT_SYMBOL_GPL(remove_irq);
>
> @@ -1246,7 +1251,7 @@ void free_irq(unsigned int irq, void *dev_id)
> {
> struct irq_desc *desc = irq_to_desc(irq);
>
> - if (!desc)
> + if (!desc || irq_settings_is_per_cpu_devid(desc))
> return;
Please make these percpu_devid checks WARN_ON so we can avoid stupid
questions on the mailing lists.
> #ifdef CONFIG_SMP
> @@ -1324,7 +1329,8 @@ int request_threaded_irq(unsigned int irq, irq_handler_t handler,
> if (!desc)
> return -EINVAL;
>
> - if (!irq_settings_can_request(desc))
> + if (!irq_settings_can_request(desc) ||
> + irq_settings_is_per_cpu_devid(desc))
> return -EINVAL;
That's fine.
> if (!handler) {
> @@ -1409,3 +1415,198 @@ int request_any_context_irq(unsigned int irq, irq_handler_t handler,
> return !ret ? IRQC_IS_HARDIRQ : ret;
> }
> EXPORT_SYMBOL_GPL(request_any_context_irq);
> +
> +#ifdef CONFIG_IRQ_PERCPU_DEVID
> +void enable_percpu_irq(unsigned int irq)
> +{
> + unsigned long flags;
> + struct irq_desc *desc = irq_get_desc_buslock(irq, &flags);
> +
> + if (!desc)
> + return;
> +
> + irq_percpu_enable(desc);
> + irq_put_desc_busunlock(desc, flags);
To make this luser proof and better debuggable I'd like to have that as:
int cpu = smp_processsor_id();
unsigned long flags;
....
irq_percpu_enable(desc, cpu);
That way we catch at least lusers who call that from random thread
context - assuming that they enabled some debug options .....
> +}
> +EXPORT_SYMBOL(enable_percpu_irq);
_GPL please if at all. Not sure whether this needs to be available for
modules.
> +/*
> + * Internal function to unregister a percpu irqaction.
> + */
> +static struct irqaction *__free_percpu_irq(unsigned int irq, void __percpu *dev_id)
> +{
> + struct irq_desc *desc = irq_to_desc(irq);
> + struct irqaction *action;
> + unsigned long flags;
> +
> + WARN(in_interrupt(), "Trying to free IRQ %d from IRQ context!\n", irq);
> +
> + if (!desc)
> + return NULL;
> +
> + raw_spin_lock_irqsave(&desc->lock, flags);
> +
> + action = desc->action;
> + if (!action || action->percpu_dev_id != dev_id) {
> + WARN(1, "Trying to free already-free IRQ %d\n", irq);
> + raw_spin_unlock_irqrestore(&desc->lock, flags);
> + return NULL;
> + }
> +
> + /* Found it - now remove it from the list of entries: */
> + WARN(!cpumask_empty(desc->percpu_enabled),
> + "percpu IRQ %d still enabled on CPU%d!\n",
> + irq, cpumask_first(desc->percpu_enabled));
> + desc->action = NULL;
> +
> +#ifdef CONFIG_SMP
> + /* make sure affinity_hint is cleaned up */
> + if (WARN_ON_ONCE(desc->affinity_hint))
> + desc->affinity_hint = NULL;
> +#endif
We don't want to have an affinity hint for percpu interrupts. That's
pretty useless AFACIT :)
> +
> + raw_spin_unlock_irqrestore(&desc->lock, flags);
> +
> + unregister_handler_proc(irq, action);
> +
> + /* Make sure it's not being used on another CPU: */
> + synchronize_irq(irq);
That's not helping w/o making synchronize_irq() aware of the percpu
stuff. Also there is the question whether we need the ability to
remove such interrupts in the first place. The target users are low
level arch interrupts not some random device drivers.
Otherwise this is a pretty cool patch!
Thanks,
tglx
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 1/3] genirq: add support for per-cpu dev_id interrupts
2011-09-15 22:49 ` Thomas Gleixner
@ 2011-09-15 23:29 ` Russell King - ARM Linux
-1 siblings, 0 replies; 38+ messages in thread
From: Russell King - ARM Linux @ 2011-09-15 23:29 UTC (permalink / raw)
To: Thomas Gleixner; +Cc: Marc Zyngier, linux-kernel, linux-arm-kernel
On Fri, Sep 16, 2011 at 12:49:10AM +0200, Thomas Gleixner wrote:
> Marc,
>
> On Thu, 15 Sep 2011, Marc Zyngier wrote:
> > +
> > + raw_spin_unlock_irqrestore(&desc->lock, flags);
> > +
> > + unregister_handler_proc(irq, action);
> > +
> > + /* Make sure it's not being used on another CPU: */
> > + synchronize_irq(irq);
>
> That's not helping w/o making synchronize_irq() aware of the percpu
> stuff. Also there is the question whether we need the ability to
> remove such interrupts in the first place. The target users are low
> level arch interrupts not some random device drivers.
You do - think local timers which go away on hotunplug and come back
on hotplug. The alternative is requiring every local timer code to
remember whether it registered its per-cpu handler on each CPU or not,
and that just gets more messy than having them unregister on hotunplug.
Not only would that be more prone to bugs but it will also mean extra
complexity in arch code.
^ permalink raw reply [flat|nested] 38+ messages in thread
* [RFC PATCH 1/3] genirq: add support for per-cpu dev_id interrupts
@ 2011-09-15 23:29 ` Russell King - ARM Linux
0 siblings, 0 replies; 38+ messages in thread
From: Russell King - ARM Linux @ 2011-09-15 23:29 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Sep 16, 2011 at 12:49:10AM +0200, Thomas Gleixner wrote:
> Marc,
>
> On Thu, 15 Sep 2011, Marc Zyngier wrote:
> > +
> > + raw_spin_unlock_irqrestore(&desc->lock, flags);
> > +
> > + unregister_handler_proc(irq, action);
> > +
> > + /* Make sure it's not being used on another CPU: */
> > + synchronize_irq(irq);
>
> That's not helping w/o making synchronize_irq() aware of the percpu
> stuff. Also there is the question whether we need the ability to
> remove such interrupts in the first place. The target users are low
> level arch interrupts not some random device drivers.
You do - think local timers which go away on hotunplug and come back
on hotplug. The alternative is requiring every local timer code to
remember whether it registered its per-cpu handler on each CPU or not,
and that just gets more messy than having them unregister on hotunplug.
Not only would that be more prone to bugs but it will also mean extra
complexity in arch code.
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 1/3] genirq: add support for per-cpu dev_id interrupts
2011-09-15 23:29 ` Russell King - ARM Linux
@ 2011-09-15 23:41 ` Thomas Gleixner
-1 siblings, 0 replies; 38+ messages in thread
From: Thomas Gleixner @ 2011-09-15 23:41 UTC (permalink / raw)
To: Russell King - ARM Linux; +Cc: Marc Zyngier, linux-kernel, linux-arm-kernel
On Fri, 16 Sep 2011, Russell King - ARM Linux wrote:
> On Fri, Sep 16, 2011 at 12:49:10AM +0200, Thomas Gleixner wrote:
> > Marc,
> >
> > On Thu, 15 Sep 2011, Marc Zyngier wrote:
> > > +
> > > + raw_spin_unlock_irqrestore(&desc->lock, flags);
> > > +
> > > + unregister_handler_proc(irq, action);
> > > +
> > > + /* Make sure it's not being used on another CPU: */
> > > + synchronize_irq(irq);
> >
> > That's not helping w/o making synchronize_irq() aware of the percpu
> > stuff. Also there is the question whether we need the ability to
> > remove such interrupts in the first place. The target users are low
> > level arch interrupts not some random device drivers.
>
> You do - think local timers which go away on hotunplug and come back
> on hotplug. The alternative is requiring every local timer code to
> remember whether it registered its per-cpu handler on each CPU or not,
> and that just gets more messy than having them unregister on hotunplug.
> Not only would that be more prone to bugs but it will also mean extra
> complexity in arch code.
Yikes! That code is removing the GLOBAL action, so all users are going
to hell.
The point of the percpu_irq stuff is to have a single action with a
percpu dev_id and a per cpu enable/disable. So when you unplug your
cpu that very cpu calls the disable function and therefor removes
itself w/o causing the other cpus to die on action = NULL
Thanks,
tglx
^ permalink raw reply [flat|nested] 38+ messages in thread
* [RFC PATCH 1/3] genirq: add support for per-cpu dev_id interrupts
@ 2011-09-15 23:41 ` Thomas Gleixner
0 siblings, 0 replies; 38+ messages in thread
From: Thomas Gleixner @ 2011-09-15 23:41 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, 16 Sep 2011, Russell King - ARM Linux wrote:
> On Fri, Sep 16, 2011 at 12:49:10AM +0200, Thomas Gleixner wrote:
> > Marc,
> >
> > On Thu, 15 Sep 2011, Marc Zyngier wrote:
> > > +
> > > + raw_spin_unlock_irqrestore(&desc->lock, flags);
> > > +
> > > + unregister_handler_proc(irq, action);
> > > +
> > > + /* Make sure it's not being used on another CPU: */
> > > + synchronize_irq(irq);
> >
> > That's not helping w/o making synchronize_irq() aware of the percpu
> > stuff. Also there is the question whether we need the ability to
> > remove such interrupts in the first place. The target users are low
> > level arch interrupts not some random device drivers.
>
> You do - think local timers which go away on hotunplug and come back
> on hotplug. The alternative is requiring every local timer code to
> remember whether it registered its per-cpu handler on each CPU or not,
> and that just gets more messy than having them unregister on hotunplug.
> Not only would that be more prone to bugs but it will also mean extra
> complexity in arch code.
Yikes! That code is removing the GLOBAL action, so all users are going
to hell.
The point of the percpu_irq stuff is to have a single action with a
percpu dev_id and a per cpu enable/disable. So when you unplug your
cpu that very cpu calls the disable function and therefor removes
itself w/o causing the other cpus to die on action = NULL
Thanks,
tglx
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 1/3] genirq: add support for per-cpu dev_id interrupts
2011-09-15 21:36 ` Michał Mirosław
@ 2011-09-16 8:20 ` Marc Zyngier
-1 siblings, 0 replies; 38+ messages in thread
From: Marc Zyngier @ 2011-09-16 8:20 UTC (permalink / raw)
To: Michał Mirosław; +Cc: linux-arm-kernel, linux-kernel, Thomas Gleixner
Hi Michał,
On 15/09/11 22:36, Michał Mirosław wrote:
> 2011/9/15 Marc Zyngier <marc.zyngier@arm.com>:
> [...]
>> diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
>> index a103732..f9b7fa3 100644
>> --- a/include/linux/interrupt.h
>> +++ b/include/linux/interrupt.h
>> @@ -95,6 +95,7 @@ typedef irqreturn_t (*irq_handler_t)(int, void *);
>> * @flags: flags (see IRQF_* above)
>> * @name: name of the device
>> * @dev_id: cookie to identify the device
>> + * @percpu_dev_id: cookie to identify the device
>> * @next: pointer to the next irqaction for shared interrupts
>> * @irq: interrupt number
>> * @dir: pointer to the proc/irq/NN/name entry
>> @@ -104,17 +105,20 @@ typedef irqreturn_t (*irq_handler_t)(int, void *);
>> * @thread_mask: bitmask for keeping track of @thread activity
>> */
>> struct irqaction {
> [...]
>> + void *dev_id;
>> +#ifdef CONFIG_IRQ_PERCPU_DEVID
>> + void __percpu *percpu_dev_id;
>> +#endif
>
> Those two can share the memory (in a anonymous union), if I read the
> idea correctly.
That was the initial implementation, and everything was fine until I
tried gcc 4.4.1. Having an anonymous union breaks static initialization
of the structure.
Try the following:
$ cat x.c
struct foo {
int a;
union {
int b1;
void * b2;
};
int c;
};
struct foo bar = {
.a = 1,
.b1 = 0,
.c = 2,
};
$ gcc -c -Wall x.c
x.c:13: error: unknown field ‘b1’ specified in initializer
x.c:13: warning: missing braces around initializer
x.c:13: warning: (near initialization for ‘bar.<anonymous>’)
GCC 4.6 seem fine though. Haven't tried anything in between.
A possible solution would be to name the union and fix all the accesses
to .dev_id. Ugly at best...
Cheers,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 38+ messages in thread
* [RFC PATCH 1/3] genirq: add support for per-cpu dev_id interrupts
@ 2011-09-16 8:20 ` Marc Zyngier
0 siblings, 0 replies; 38+ messages in thread
From: Marc Zyngier @ 2011-09-16 8:20 UTC (permalink / raw)
To: linux-arm-kernel
Hi Micha?,
On 15/09/11 22:36, Micha? Miros?aw wrote:
> 2011/9/15 Marc Zyngier <marc.zyngier@arm.com>:
> [...]
>> diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
>> index a103732..f9b7fa3 100644
>> --- a/include/linux/interrupt.h
>> +++ b/include/linux/interrupt.h
>> @@ -95,6 +95,7 @@ typedef irqreturn_t (*irq_handler_t)(int, void *);
>> * @flags: flags (see IRQF_* above)
>> * @name: name of the device
>> * @dev_id: cookie to identify the device
>> + * @percpu_dev_id: cookie to identify the device
>> * @next: pointer to the next irqaction for shared interrupts
>> * @irq: interrupt number
>> * @dir: pointer to the proc/irq/NN/name entry
>> @@ -104,17 +105,20 @@ typedef irqreturn_t (*irq_handler_t)(int, void *);
>> * @thread_mask: bitmask for keeping track of @thread activity
>> */
>> struct irqaction {
> [...]
>> + void *dev_id;
>> +#ifdef CONFIG_IRQ_PERCPU_DEVID
>> + void __percpu *percpu_dev_id;
>> +#endif
>
> Those two can share the memory (in a anonymous union), if I read the
> idea correctly.
That was the initial implementation, and everything was fine until I
tried gcc 4.4.1. Having an anonymous union breaks static initialization
of the structure.
Try the following:
$ cat x.c
struct foo {
int a;
union {
int b1;
void * b2;
};
int c;
};
struct foo bar = {
.a = 1,
.b1 = 0,
.c = 2,
};
$ gcc -c -Wall x.c
x.c:13: error: unknown field ?b1? specified in initializer
x.c:13: warning: missing braces around initializer
x.c:13: warning: (near initialization for ?bar.<anonymous>?)
GCC 4.6 seem fine though. Haven't tried anything in between.
A possible solution would be to name the union and fix all the accesses
to .dev_id. Ugly at best...
Cheers,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 1/3] genirq: add support for per-cpu dev_id interrupts
2011-09-15 22:49 ` Thomas Gleixner
@ 2011-09-16 9:37 ` Marc Zyngier
-1 siblings, 0 replies; 38+ messages in thread
From: Marc Zyngier @ 2011-09-16 9:37 UTC (permalink / raw)
To: Thomas Gleixner; +Cc: linux-arm-kernel, linux-kernel
On 15/09/11 23:49, Thomas Gleixner wrote:
> Marc,
>
> On Thu, 15 Sep 2011, Marc Zyngier wrote:
>>
>> Based on an initial patch by Thomas Gleixner.
>
> Was more an idea than a patch :)
Almost the same thing :)
>
>> + void *dev_id;
>> +#ifdef CONFIG_IRQ_PERCPU_DEVID
>> + void __percpu *percpu_dev_id;
>> +#endif
>
> Can we please avoid that ifdef and use an anon union ?
See my reply to Michał on the same subject. Doing so breaks existing
code because of what looks like a GCC problem.
>
>> +#ifdef CONFIG_IRQ_PERCPU_DEVID
>> +static inline int irq_set_percpu_devid(unsigned int irq)
>
> Real function in kernel/irq/ please
I will then have to split it somehow, as anything that includes irq.h
and refers to a IRQ_* flag falls into the "GOT_YOU_MORON" trap...
>> +{
>> + struct irq_desc *desc = irq_to_desc(irq);
>> +
>> + if (!desc)
>> + return -EINVAL;
>> +
>> + if (!zalloc_cpumask_var(&desc->percpu_enabled, GFP_KERNEL))
>> + return -ENOMEM;
>
> What prevents that allocation happening more than once ?
Will fix.
>> + irq_set_status_flags(irq,
>> + IRQ_NOAUTOEN | IRQ_PER_CPU | IRQ_NOTHREAD |
>> + IRQ_NOPROBE | IRQ_PER_CPU_DEVID);
>> + return 0;
>> +}
>> +#endif
>> +
>> /* Handle dynamic irq creation and destruction */
>> extern unsigned int create_irq_nr(unsigned int irq_want, int node);
>> extern int create_irq(void);
>> diff --git a/include/linux/irqdesc.h b/include/linux/irqdesc.h
>> index 150134a..0b4419a 100644
>> --- a/include/linux/irqdesc.h
>> +++ b/include/linux/irqdesc.h
>> @@ -53,6 +53,9 @@ struct irq_desc {
>> unsigned long last_unhandled; /* Aging timer for unhandled count */
>> unsigned int irqs_unhandled;
>> raw_spinlock_t lock;
>> +#ifdef CONFIG_IRQ_PERCPU_DEVID
>
> Can we just make that a pointer and get rid of the config option ?
Yes. Will add the necessary allocation to irq_set_percpu_devid();
>
>> +#ifdef CONFIG_IRQ_PERCPU_DEVID
>> +void irq_percpu_enable(struct irq_desc *desc)
>> +{
>> + unsigned int cpu = get_cpu();
>
> get_cpu() is overkill here. That's called with desc->lock held and
> interrupts disabled. No way that the cpu can go away :)
Ah, that's only me getting paranoid... ;-)
> I'd rather have a check in the calling function, which makes sure that
> enable_percpu_irq() is called from proper per cpu context. See comment
> below.
>
>> void remove_irq(unsigned int irq, struct irqaction *act)
>> {
>> - __free_irq(irq, act->dev_id);
>> + struct irq_desc *desc = irq_to_desc(irq);
>> +
>> + if (desc && !irq_settings_is_per_cpu_devid(desc))
>
> That probably should be
>
> if (!WARN_ON(....))
Right.
>> + __free_irq(irq, act->dev_id);
>> }
>> EXPORT_SYMBOL_GPL(remove_irq);
>>
>> @@ -1246,7 +1251,7 @@ void free_irq(unsigned int irq, void *dev_id)
>> {
>> struct irq_desc *desc = irq_to_desc(irq);
>>
>> - if (!desc)
>> + if (!desc || irq_settings_is_per_cpu_devid(desc))
>> return;
>
> Please make these percpu_devid checks WARN_ON so we can avoid stupid
> questions on the mailing lists.
OK.
>> #ifdef CONFIG_SMP
>> @@ -1324,7 +1329,8 @@ int request_threaded_irq(unsigned int irq, irq_handler_t handler,
>> if (!desc)
>> return -EINVAL;
>>
>> - if (!irq_settings_can_request(desc))
>> + if (!irq_settings_can_request(desc) ||
>> + irq_settings_is_per_cpu_devid(desc))
>> return -EINVAL;
>
> That's fine.
>
>> if (!handler) {
>> @@ -1409,3 +1415,198 @@ int request_any_context_irq(unsigned int irq, irq_handler_t handler,
>> return !ret ? IRQC_IS_HARDIRQ : ret;
>> }
>> EXPORT_SYMBOL_GPL(request_any_context_irq);
>> +
>> +#ifdef CONFIG_IRQ_PERCPU_DEVID
>> +void enable_percpu_irq(unsigned int irq)
>> +{
>> + unsigned long flags;
>> + struct irq_desc *desc = irq_get_desc_buslock(irq, &flags);
>> +
>> + if (!desc)
>> + return;
>> +
>> + irq_percpu_enable(desc);
>> + irq_put_desc_busunlock(desc, flags);
>
> To make this luser proof and better debuggable I'd like to have that as:
>
> int cpu = smp_processsor_id();
> unsigned long flags;
> ....
> irq_percpu_enable(desc, cpu);
>
> That way we catch at least lusers who call that from random thread
> context - assuming that they enabled some debug options .....
Ah, nice trick.
>> +}
>> +EXPORT_SYMBOL(enable_percpu_irq);
>
> _GPL please if at all. Not sure whether this needs to be available for
> modules.
Well, At the moment, the only potential users of that code are timers,
which cannot be modules. I'd be happy to drop this altogether.
>> +/*
>> + * Internal function to unregister a percpu irqaction.
>> + */
>> +static struct irqaction *__free_percpu_irq(unsigned int irq, void __percpu *dev_id)
>> +{
>> + struct irq_desc *desc = irq_to_desc(irq);
>> + struct irqaction *action;
>> + unsigned long flags;
>> +
>> + WARN(in_interrupt(), "Trying to free IRQ %d from IRQ context!\n", irq);
>> +
>> + if (!desc)
>> + return NULL;
>> +
>> + raw_spin_lock_irqsave(&desc->lock, flags);
>> +
>> + action = desc->action;
>> + if (!action || action->percpu_dev_id != dev_id) {
>> + WARN(1, "Trying to free already-free IRQ %d\n", irq);
>> + raw_spin_unlock_irqrestore(&desc->lock, flags);
>> + return NULL;
>> + }
>> +
>> + /* Found it - now remove it from the list of entries: */
>> + WARN(!cpumask_empty(desc->percpu_enabled),
>> + "percpu IRQ %d still enabled on CPU%d!\n",
>> + irq, cpumask_first(desc->percpu_enabled));
>> + desc->action = NULL;
>> +
>> +#ifdef CONFIG_SMP
>> + /* make sure affinity_hint is cleaned up */
>> + if (WARN_ON_ONCE(desc->affinity_hint))
>> + desc->affinity_hint = NULL;
>> +#endif
>
> We don't want to have an affinity hint for percpu interrupts. That's
> pretty useless AFACIT :)
Copy paste issue. Will fix :)
>> +
>> + raw_spin_unlock_irqrestore(&desc->lock, flags);
>> +
>> + unregister_handler_proc(irq, action);
>> +
>> + /* Make sure it's not being used on another CPU: */
>> + synchronize_irq(irq);
>
> That's not helping w/o making synchronize_irq() aware of the percpu
> stuff. Also there is the question whether we need the ability to
> remove such interrupts in the first place. The target users are low
> level arch interrupts not some random device drivers.
Again, there is no need for this at the moment (the timer code runs
running forever), and this is only there for completeness.
I'll see if I can come up with a synchronize_percpu_irq() without adding
too much bloat to irqdesc.
Thanks for reviewing!
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 38+ messages in thread
* [RFC PATCH 1/3] genirq: add support for per-cpu dev_id interrupts
@ 2011-09-16 9:37 ` Marc Zyngier
0 siblings, 0 replies; 38+ messages in thread
From: Marc Zyngier @ 2011-09-16 9:37 UTC (permalink / raw)
To: linux-arm-kernel
On 15/09/11 23:49, Thomas Gleixner wrote:
> Marc,
>
> On Thu, 15 Sep 2011, Marc Zyngier wrote:
>>
>> Based on an initial patch by Thomas Gleixner.
>
> Was more an idea than a patch :)
Almost the same thing :)
>
>> + void *dev_id;
>> +#ifdef CONFIG_IRQ_PERCPU_DEVID
>> + void __percpu *percpu_dev_id;
>> +#endif
>
> Can we please avoid that ifdef and use an anon union ?
See my reply to Micha? on the same subject. Doing so breaks existing
code because of what looks like a GCC problem.
>
>> +#ifdef CONFIG_IRQ_PERCPU_DEVID
>> +static inline int irq_set_percpu_devid(unsigned int irq)
>
> Real function in kernel/irq/ please
I will then have to split it somehow, as anything that includes irq.h
and refers to a IRQ_* flag falls into the "GOT_YOU_MORON" trap...
>> +{
>> + struct irq_desc *desc = irq_to_desc(irq);
>> +
>> + if (!desc)
>> + return -EINVAL;
>> +
>> + if (!zalloc_cpumask_var(&desc->percpu_enabled, GFP_KERNEL))
>> + return -ENOMEM;
>
> What prevents that allocation happening more than once ?
Will fix.
>> + irq_set_status_flags(irq,
>> + IRQ_NOAUTOEN | IRQ_PER_CPU | IRQ_NOTHREAD |
>> + IRQ_NOPROBE | IRQ_PER_CPU_DEVID);
>> + return 0;
>> +}
>> +#endif
>> +
>> /* Handle dynamic irq creation and destruction */
>> extern unsigned int create_irq_nr(unsigned int irq_want, int node);
>> extern int create_irq(void);
>> diff --git a/include/linux/irqdesc.h b/include/linux/irqdesc.h
>> index 150134a..0b4419a 100644
>> --- a/include/linux/irqdesc.h
>> +++ b/include/linux/irqdesc.h
>> @@ -53,6 +53,9 @@ struct irq_desc {
>> unsigned long last_unhandled; /* Aging timer for unhandled count */
>> unsigned int irqs_unhandled;
>> raw_spinlock_t lock;
>> +#ifdef CONFIG_IRQ_PERCPU_DEVID
>
> Can we just make that a pointer and get rid of the config option ?
Yes. Will add the necessary allocation to irq_set_percpu_devid();
>
>> +#ifdef CONFIG_IRQ_PERCPU_DEVID
>> +void irq_percpu_enable(struct irq_desc *desc)
>> +{
>> + unsigned int cpu = get_cpu();
>
> get_cpu() is overkill here. That's called with desc->lock held and
> interrupts disabled. No way that the cpu can go away :)
Ah, that's only me getting paranoid... ;-)
> I'd rather have a check in the calling function, which makes sure that
> enable_percpu_irq() is called from proper per cpu context. See comment
> below.
>
>> void remove_irq(unsigned int irq, struct irqaction *act)
>> {
>> - __free_irq(irq, act->dev_id);
>> + struct irq_desc *desc = irq_to_desc(irq);
>> +
>> + if (desc && !irq_settings_is_per_cpu_devid(desc))
>
> That probably should be
>
> if (!WARN_ON(....))
Right.
>> + __free_irq(irq, act->dev_id);
>> }
>> EXPORT_SYMBOL_GPL(remove_irq);
>>
>> @@ -1246,7 +1251,7 @@ void free_irq(unsigned int irq, void *dev_id)
>> {
>> struct irq_desc *desc = irq_to_desc(irq);
>>
>> - if (!desc)
>> + if (!desc || irq_settings_is_per_cpu_devid(desc))
>> return;
>
> Please make these percpu_devid checks WARN_ON so we can avoid stupid
> questions on the mailing lists.
OK.
>> #ifdef CONFIG_SMP
>> @@ -1324,7 +1329,8 @@ int request_threaded_irq(unsigned int irq, irq_handler_t handler,
>> if (!desc)
>> return -EINVAL;
>>
>> - if (!irq_settings_can_request(desc))
>> + if (!irq_settings_can_request(desc) ||
>> + irq_settings_is_per_cpu_devid(desc))
>> return -EINVAL;
>
> That's fine.
>
>> if (!handler) {
>> @@ -1409,3 +1415,198 @@ int request_any_context_irq(unsigned int irq, irq_handler_t handler,
>> return !ret ? IRQC_IS_HARDIRQ : ret;
>> }
>> EXPORT_SYMBOL_GPL(request_any_context_irq);
>> +
>> +#ifdef CONFIG_IRQ_PERCPU_DEVID
>> +void enable_percpu_irq(unsigned int irq)
>> +{
>> + unsigned long flags;
>> + struct irq_desc *desc = irq_get_desc_buslock(irq, &flags);
>> +
>> + if (!desc)
>> + return;
>> +
>> + irq_percpu_enable(desc);
>> + irq_put_desc_busunlock(desc, flags);
>
> To make this luser proof and better debuggable I'd like to have that as:
>
> int cpu = smp_processsor_id();
> unsigned long flags;
> ....
> irq_percpu_enable(desc, cpu);
>
> That way we catch at least lusers who call that from random thread
> context - assuming that they enabled some debug options .....
Ah, nice trick.
>> +}
>> +EXPORT_SYMBOL(enable_percpu_irq);
>
> _GPL please if at all. Not sure whether this needs to be available for
> modules.
Well, At the moment, the only potential users of that code are timers,
which cannot be modules. I'd be happy to drop this altogether.
>> +/*
>> + * Internal function to unregister a percpu irqaction.
>> + */
>> +static struct irqaction *__free_percpu_irq(unsigned int irq, void __percpu *dev_id)
>> +{
>> + struct irq_desc *desc = irq_to_desc(irq);
>> + struct irqaction *action;
>> + unsigned long flags;
>> +
>> + WARN(in_interrupt(), "Trying to free IRQ %d from IRQ context!\n", irq);
>> +
>> + if (!desc)
>> + return NULL;
>> +
>> + raw_spin_lock_irqsave(&desc->lock, flags);
>> +
>> + action = desc->action;
>> + if (!action || action->percpu_dev_id != dev_id) {
>> + WARN(1, "Trying to free already-free IRQ %d\n", irq);
>> + raw_spin_unlock_irqrestore(&desc->lock, flags);
>> + return NULL;
>> + }
>> +
>> + /* Found it - now remove it from the list of entries: */
>> + WARN(!cpumask_empty(desc->percpu_enabled),
>> + "percpu IRQ %d still enabled on CPU%d!\n",
>> + irq, cpumask_first(desc->percpu_enabled));
>> + desc->action = NULL;
>> +
>> +#ifdef CONFIG_SMP
>> + /* make sure affinity_hint is cleaned up */
>> + if (WARN_ON_ONCE(desc->affinity_hint))
>> + desc->affinity_hint = NULL;
>> +#endif
>
> We don't want to have an affinity hint for percpu interrupts. That's
> pretty useless AFACIT :)
Copy paste issue. Will fix :)
>> +
>> + raw_spin_unlock_irqrestore(&desc->lock, flags);
>> +
>> + unregister_handler_proc(irq, action);
>> +
>> + /* Make sure it's not being used on another CPU: */
>> + synchronize_irq(irq);
>
> That's not helping w/o making synchronize_irq() aware of the percpu
> stuff. Also there is the question whether we need the ability to
> remove such interrupts in the first place. The target users are low
> level arch interrupts not some random device drivers.
Again, there is no need for this at the moment (the timer code runs
running forever), and this is only there for completeness.
I'll see if I can come up with a synchronize_percpu_irq() without adding
too much bloat to irqdesc.
Thanks for reviewing!
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 1/3] genirq: add support for per-cpu dev_id interrupts
2011-09-16 8:20 ` Marc Zyngier
@ 2011-09-16 9:37 ` Thomas Gleixner
-1 siblings, 0 replies; 38+ messages in thread
From: Thomas Gleixner @ 2011-09-16 9:37 UTC (permalink / raw)
To: Marc Zyngier; +Cc: Michał Mirosław, linux-arm-kernel, linux-kernel
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1579 bytes --]
On Fri, 16 Sep 2011, Marc Zyngier wrote:
> Hi Michał,
>
> On 15/09/11 22:36, Michał Mirosław wrote:
> > 2011/9/15 Marc Zyngier <marc.zyngier@arm.com>:
> > [...]
> >> diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
> >> index a103732..f9b7fa3 100644
> >> --- a/include/linux/interrupt.h
> >> +++ b/include/linux/interrupt.h
> >> @@ -95,6 +95,7 @@ typedef irqreturn_t (*irq_handler_t)(int, void *);
> >> * @flags: flags (see IRQF_* above)
> >> * @name: name of the device
> >> * @dev_id: cookie to identify the device
> >> + * @percpu_dev_id: cookie to identify the device
> >> * @next: pointer to the next irqaction for shared interrupts
> >> * @irq: interrupt number
> >> * @dir: pointer to the proc/irq/NN/name entry
> >> @@ -104,17 +105,20 @@ typedef irqreturn_t (*irq_handler_t)(int, void *);
> >> * @thread_mask: bitmask for keeping track of @thread activity
> >> */
> >> struct irqaction {
> > [...]
> >> + void *dev_id;
> >> +#ifdef CONFIG_IRQ_PERCPU_DEVID
> >> + void __percpu *percpu_dev_id;
> >> +#endif
> >
> > Those two can share the memory (in a anonymous union), if I read the
> > idea correctly.
>
> That was the initial implementation, and everything was fine until I
> tried gcc 4.4.1. Having an anonymous union breaks static initialization
> of the structure.
Bah, right. It wants to have brackets around it. So we can use a
separate pointer to get this going and then run coccinelle over it to
fix that up just before 3.2-rc1.
Thanks,
tglx
^ permalink raw reply [flat|nested] 38+ messages in thread
* [RFC PATCH 1/3] genirq: add support for per-cpu dev_id interrupts
@ 2011-09-16 9:37 ` Thomas Gleixner
0 siblings, 0 replies; 38+ messages in thread
From: Thomas Gleixner @ 2011-09-16 9:37 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, 16 Sep 2011, Marc Zyngier wrote:
> Hi Micha?,
>
> On 15/09/11 22:36, Micha? Miros?aw wrote:
> > 2011/9/15 Marc Zyngier <marc.zyngier@arm.com>:
> > [...]
> >> diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
> >> index a103732..f9b7fa3 100644
> >> --- a/include/linux/interrupt.h
> >> +++ b/include/linux/interrupt.h
> >> @@ -95,6 +95,7 @@ typedef irqreturn_t (*irq_handler_t)(int, void *);
> >> * @flags: flags (see IRQF_* above)
> >> * @name: name of the device
> >> * @dev_id: cookie to identify the device
> >> + * @percpu_dev_id: cookie to identify the device
> >> * @next: pointer to the next irqaction for shared interrupts
> >> * @irq: interrupt number
> >> * @dir: pointer to the proc/irq/NN/name entry
> >> @@ -104,17 +105,20 @@ typedef irqreturn_t (*irq_handler_t)(int, void *);
> >> * @thread_mask: bitmask for keeping track of @thread activity
> >> */
> >> struct irqaction {
> > [...]
> >> + void *dev_id;
> >> +#ifdef CONFIG_IRQ_PERCPU_DEVID
> >> + void __percpu *percpu_dev_id;
> >> +#endif
> >
> > Those two can share the memory (in a anonymous union), if I read the
> > idea correctly.
>
> That was the initial implementation, and everything was fine until I
> tried gcc 4.4.1. Having an anonymous union breaks static initialization
> of the structure.
Bah, right. It wants to have brackets around it. So we can use a
separate pointer to get this going and then run coccinelle over it to
fix that up just before 3.2-rc1.
Thanks,
tglx
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 1/3] genirq: add support for per-cpu dev_id interrupts
2011-09-16 9:37 ` Marc Zyngier
@ 2011-09-16 9:41 ` Thomas Gleixner
-1 siblings, 0 replies; 38+ messages in thread
From: Thomas Gleixner @ 2011-09-16 9:41 UTC (permalink / raw)
To: Marc Zyngier; +Cc: linux-arm-kernel, linux-kernel
On Fri, 16 Sep 2011, Marc Zyngier wrote:
> On 15/09/11 23:49, Thomas Gleixner wrote:
> >> +
> >> + raw_spin_unlock_irqrestore(&desc->lock, flags);
> >> +
> >> + unregister_handler_proc(irq, action);
> >> +
> >> + /* Make sure it's not being used on another CPU: */
> >> + synchronize_irq(irq);
> >
> > That's not helping w/o making synchronize_irq() aware of the percpu
> > stuff. Also there is the question whether we need the ability to
> > remove such interrupts in the first place. The target users are low
> > level arch interrupts not some random device drivers.
>
> Again, there is no need for this at the moment (the timer code runs
> running forever), and this is only there for completeness.
>
> I'll see if I can come up with a synchronize_percpu_irq() without adding
> too much bloat to irqdesc.
You'd need a PROGRESS flag per cpu, which is overkill. What you can do
is to check whether the percpu enabled cpumask is completely empty and
return with a WARN when not.
Thanks,
tglx
^ permalink raw reply [flat|nested] 38+ messages in thread
* [RFC PATCH 1/3] genirq: add support for per-cpu dev_id interrupts
@ 2011-09-16 9:41 ` Thomas Gleixner
0 siblings, 0 replies; 38+ messages in thread
From: Thomas Gleixner @ 2011-09-16 9:41 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, 16 Sep 2011, Marc Zyngier wrote:
> On 15/09/11 23:49, Thomas Gleixner wrote:
> >> +
> >> + raw_spin_unlock_irqrestore(&desc->lock, flags);
> >> +
> >> + unregister_handler_proc(irq, action);
> >> +
> >> + /* Make sure it's not being used on another CPU: */
> >> + synchronize_irq(irq);
> >
> > That's not helping w/o making synchronize_irq() aware of the percpu
> > stuff. Also there is the question whether we need the ability to
> > remove such interrupts in the first place. The target users are low
> > level arch interrupts not some random device drivers.
>
> Again, there is no need for this at the moment (the timer code runs
> running forever), and this is only there for completeness.
>
> I'll see if I can come up with a synchronize_percpu_irq() without adding
> too much bloat to irqdesc.
You'd need a PROGRESS flag per cpu, which is overkill. What you can do
is to check whether the percpu enabled cpumask is completely empty and
return with a WARN when not.
Thanks,
tglx
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 1/3] genirq: add support for per-cpu dev_id interrupts
2011-09-15 16:52 ` Marc Zyngier
@ 2011-09-18 23:20 ` Abhijeet Dharmapurikar
-1 siblings, 0 replies; 38+ messages in thread
From: Abhijeet Dharmapurikar @ 2011-09-18 23:20 UTC (permalink / raw)
To: Marc Zyngier; +Cc: linux-arm-kernel, linux-kernel, Thomas Gleixner
On 09/15/2011 09:52 AM, Marc Zyngier wrote:
> The ARM GIC interrupt controller offers per CPU interrupts (PPIs),
> which are usually used to connect local timers to each core.
> Each CPU has its own private interface to the GIC,
> and only sees the PPIs that are directly connect to it.
>
> While these timers are separate devices and have a separate
> interrupt line to a core, they all use the same IRQ number.
>
> For these devices, request_irq() is not the right API as it
> assumes that an IRQ number is visible by a number of CPUs
> (through the affinity setting), but makes it very awkward to
> express that an IRQ number can be handled by all CPUs, and
> yet be a different interrupt line on each CPU, requiring a
> different dev_id cookie to be passed back to the handler.
>
> The *_percpu_irq() functions is designed to overcome these
> limitations, by providing a per-cpu dev_id vector:
>
> int request_percpu_irq(unsigned int irq, irq_handler_t handler,
> const char *devname, void __percpu *percpu_dev_id);
> void free_percpu_irq(unsigned int, void __percpu *);
> int setup_percpu_irq(unsigned int irq, struct irqaction *new);
> void remove_percpu_irq(unsigned int irq, struct irqaction *act);
> void enable_percpu_irq(unsigned int irq);
> void disable_percpu_irq(unsigned int irq);
>
> The API has a number of limitations:
> - no interrupt sharing
> - no threading
> - common handler across all the CPUs
>
> Once the interrupt is requested using setup_percpu_irq() or
> request_percpu_irq(), it must be enabled by each core that wishes
> its local interrupt to be delivered.
>
> Based on an initial patch by Thomas Gleixner.
>
> Cc: Thomas Gleixner<tglx@linutronix.de>
> Signed-off-by: Marc Zyngier<marc.zyngier@arm.com>
> ---
> include/linux/interrupt.h | 40 ++++++---
> include/linux/irq.h | 25 +++++-
> include/linux/irqdesc.h | 3 +
> kernel/irq/Kconfig | 4 +
> kernel/irq/chip.c | 58 +++++++++++++
> kernel/irq/internals.h | 2 +
> kernel/irq/manage.c | 209
++++++++++++++++++++++++++++++++++++++++++++-
> kernel/irq/settings.h | 7 ++
> 8 files changed, 332 insertions(+), 16 deletions(-)
>
> diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
> index a103732..f9b7fa3 100644
> --- a/include/linux/interrupt.h
> +++ b/include/linux/interrupt.h
> @@ -95,6 +95,7 @@ typedef irqreturn_t (*irq_handler_t)(int, void *);
> * @flags: flags (see IRQF_* above)
> * @name: name of the device
> * @dev_id: cookie to identify the device
> + * @percpu_dev_id: cookie to identify the device
> * @next: pointer to the next irqaction for shared interrupts
> * @irq: interrupt number
> * @dir: pointer to the proc/irq/NN/name entry
> @@ -104,17 +105,20 @@ typedef irqreturn_t (*irq_handler_t)(int, void *);
> * @thread_mask: bitmask for keeping track of @thread activity
> */
> struct irqaction {
> - irq_handler_t handler;
> - unsigned long flags;
> - void *dev_id;
> - struct irqaction *next;
> - int irq;
> - irq_handler_t thread_fn;
> - struct task_struct *thread;
> - unsigned long thread_flags;
> - unsigned long thread_mask;
> - const char *name;
> - struct proc_dir_entry *dir;
> + irq_handler_t handler;
> + unsigned long flags;
> + void *dev_id;
> +#ifdef CONFIG_IRQ_PERCPU_DEVID
> + void __percpu *percpu_dev_id;
> +#endif
> + struct irqaction *next;
> + int irq;
> + irq_handler_t thread_fn;
> + struct task_struct *thread;
> + unsigned long thread_flags;
> + unsigned long thread_mask;
> + const char *name;
> + struct proc_dir_entry *dir;
> } ____cacheline_internodealigned_in_smp;
>
> extern irqreturn_t no_action(int cpl, void *dev_id);
> @@ -136,6 +140,10 @@ extern int __must_check
> request_any_context_irq(unsigned int irq, irq_handler_t handler,
> unsigned long flags, const char *name, void *dev_id);
>
> +extern int __must_check
> +request_percpu_irq(unsigned int irq, irq_handler_t handler,
> + const char *devname, void __percpu *percpu_dev_id);
> +
> extern void exit_irq_thread(void);
> #else
>
> @@ -164,10 +172,18 @@ request_any_context_irq(unsigned int irq,
irq_handler_t handler,
> return request_irq(irq, handler, flags, name, dev_id);
> }
>
> +static inline int __must_check
> +request_percpu_irq(unsigned int irq, irq_handler_t handler,
> + const char *devname, void __percpu *percpu_dev_id)
> +{
> + return request_irq(irq, handler, 0, name, dev_id);
you probably meant devname here instead of name and also percpu_dev_id
instead of dev_id.
> +}
> +
> static inline void exit_irq_thread(void) { }
> #endif
>
> extern void free_irq(unsigned int, void *);
> +extern void free_percpu_irq(unsigned int, void __percpu *);
>
> struct device;
>
> @@ -207,7 +223,9 @@ extern void devm_free_irq(struct device *dev,
unsigned int irq, void *dev_id);
>
> extern void disable_irq_nosync(unsigned int irq);
> extern void disable_irq(unsigned int irq);
> +extern void disable_percpu_irq(unsigned int irq);
> extern void enable_irq(unsigned int irq);
> +extern void enable_percpu_irq(unsigned int irq);
>
> /* The following three functions are for the core kernel use only. */
> #ifdef CONFIG_GENERIC_HARDIRQS
> diff --git a/include/linux/irq.h b/include/linux/irq.h
> index 5951730..1e14fd1 100644
> --- a/include/linux/irq.h
> +++ b/include/linux/irq.h
> @@ -66,6 +66,7 @@ typedef void (*irq_preflow_handler_t)(struct
irq_data *data);
> * IRQ_NO_BALANCING - Interrupt cannot be balanced (affinity set)
> * 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
> */
> enum {
> IRQ_TYPE_NONE = 0x00000000,
> @@ -88,12 +89,13 @@ enum {
> IRQ_MOVE_PCNTXT = (1<< 14),
> IRQ_NESTED_THREAD = (1<< 15),
> IRQ_NOTHREAD = (1<< 16),
> + IRQ_PER_CPU_DEVID = (1<< 17),
> };
>
> #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_PER_CPU | IRQ_NESTED_THREAD | IRQ_NOTHREAD | IRQ_PER_CPU_DEVID)
>
> #define IRQ_NO_BALANCING_MASK (IRQ_PER_CPU | IRQ_NO_BALANCING)
>
> @@ -365,6 +367,8 @@ enum {
> struct irqaction;
> extern int setup_irq(unsigned int irq, struct irqaction *new);
> extern void remove_irq(unsigned int irq, struct irqaction *act);
> +extern int setup_percpu_irq(unsigned int irq, struct irqaction *new);
> +extern void remove_percpu_irq(unsigned int irq, struct irqaction *act);
>
> extern void irq_cpu_online(void);
> extern void irq_cpu_offline(void);
> @@ -392,6 +396,7 @@ 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_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);
> extern void handle_nested_irq(unsigned int irq);
>
> @@ -481,6 +486,24 @@ static inline void
irq_set_nested_thread(unsigned int irq, bool nest)
> irq_clear_status_flags(irq, IRQ_NESTED_THREAD);
> }
>
> +#ifdef CONFIG_IRQ_PERCPU_DEVID
> +static inline int irq_set_percpu_devid(unsigned int irq)
> +{
> + struct irq_desc *desc = irq_to_desc(irq);
> +
> + if (!desc)
> + return -EINVAL;
> +
> + if (!zalloc_cpumask_var(&desc->percpu_enabled, GFP_KERNEL))
> + return -ENOMEM;
> +
> + irq_set_status_flags(irq,
> + IRQ_NOAUTOEN | IRQ_PER_CPU | IRQ_NOTHREAD |
> + IRQ_NOPROBE | IRQ_PER_CPU_DEVID);
> + return 0;
> +}
> +#endif
> +
> /* Handle dynamic irq creation and destruction */
> extern unsigned int create_irq_nr(unsigned int irq_want, int node);
> extern int create_irq(void);
> diff --git a/include/linux/irqdesc.h b/include/linux/irqdesc.h
> index 150134a..0b4419a 100644
> --- a/include/linux/irqdesc.h
> +++ b/include/linux/irqdesc.h
> @@ -53,6 +53,9 @@ struct irq_desc {
> unsigned long last_unhandled; /* Aging timer for unhandled count */
> unsigned int irqs_unhandled;
> raw_spinlock_t lock;
> +#ifdef CONFIG_IRQ_PERCPU_DEVID
> + cpumask_var_t percpu_enabled;
> +#endif
> #ifdef CONFIG_SMP
> const struct cpumask *affinity_hint;
> struct irq_affinity_notify *affinity_notify;
> diff --git a/kernel/irq/Kconfig b/kernel/irq/Kconfig
> index 5a38bf4..75c0631 100644
> --- a/kernel/irq/Kconfig
> +++ b/kernel/irq/Kconfig
> @@ -60,6 +60,10 @@ config IRQ_DOMAIN
> config IRQ_FORCED_THREADING
> bool
>
> +# Support per CPU dev id
> +config IRQ_PERCPU_DEVID
> + bool
> +
> config SPARSE_IRQ
> bool "Support sparse irq numbering"
> depends on HAVE_SPARSE_IRQ
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index d5a3009..d65b23f 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -204,6 +204,30 @@ void irq_disable(struct irq_desc *desc)
> }
> }
>
> +#ifdef CONFIG_IRQ_PERCPU_DEVID
> +void irq_percpu_enable(struct irq_desc *desc)
> +{
> + unsigned int cpu = get_cpu();
> + if (desc->irq_data.chip->irq_enable)
> + desc->irq_data.chip->irq_enable(&desc->irq_data);
> + else
> + desc->irq_data.chip->irq_unmask(&desc->irq_data);
> + cpumask_set_cpu(cpu, desc->percpu_enabled);
> + put_cpu();
> +}
> +
> +void irq_percpu_disable(struct irq_desc *desc)
> +{
> + unsigned int cpu = get_cpu();
> + if (desc->irq_data.chip->irq_disable) {
> + desc->irq_data.chip->irq_disable(&desc->irq_data);
> + irq_state_set_masked(desc);
> + }
> + cpumask_clear_cpu(cpu, desc->percpu_enabled);
> + put_cpu();
> +}
> +#endif
> +
> static inline void mask_ack_irq(struct irq_desc *desc)
> {
> if (desc->irq_data.chip->irq_mask_ack)
> @@ -544,6 +568,40 @@ handle_percpu_irq(unsigned int irq, struct
irq_desc *desc)
> chip->irq_eoi(&desc->irq_data);
> }
>
> +#ifdef CONFIG_IRQ_PERCPU_DEVID
> +/**
> + * handle_percpu_devid_irq - Per CPU local irq handler with per cpu
dev ids
> + * @irq: the interrupt number
> + * @desc: the interrupt description structure for this irq
> + *
> + * Per CPU interrupts on SMP machines without locking requirements.
Same as
> + * handle_percpu_irq() above but with the following extras:
> + *
> + * action->percpu_dev_id is a pointer to percpu variables which
> + * contain the real device id for the cpu on which this handler is
> + * called
> + */
> +void handle_percpu_devid_irq(unsigned int irq, struct irq_desc *desc)
> +{
> + struct irq_chip *chip = irq_desc_get_chip(desc);
> + struct irqaction *action = desc->action;
> + void *dev_id = __this_cpu_ptr(action->percpu_dev_id);
> + irqreturn_t res;
> +
> + kstat_incr_irqs_this_cpu(irq, desc);
> +
> + if (chip->irq_ack)
> + chip->irq_ack(&desc->irq_data);
> +
> + trace_irq_handler_entry(irq, action);
> + res = action->handler(irq, dev_id);
> + trace_irq_handler_exit(irq, action, res);
> +
> + if (chip->irq_eoi)
> + chip->irq_eoi(&desc->irq_data);
> +}
> +#endif
> +
> void
> __irq_set_handler(unsigned int irq, irq_flow_handler_t handle, int
is_chained,
> const char *name)
> diff --git a/kernel/irq/internals.h b/kernel/irq/internals.h
> index 6546431..a57fd3b 100644
> --- a/kernel/irq/internals.h
> +++ b/kernel/irq/internals.h
> @@ -71,6 +71,8 @@ extern int irq_startup(struct irq_desc *desc);
> extern void irq_shutdown(struct irq_desc *desc);
> extern void irq_enable(struct irq_desc *desc);
> extern void irq_disable(struct irq_desc *desc);
> +extern void irq_percpu_enable(struct irq_desc *desc);
> +extern void irq_percpu_disable(struct irq_desc *desc);
> extern void mask_irq(struct irq_desc *desc);
> extern void unmask_irq(struct irq_desc *desc);
>
> diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> index 9b956fa..9f10b07 100644
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
> @@ -1118,6 +1118,8 @@ int setup_irq(unsigned int irq, struct
irqaction *act)
> int retval;
> struct irq_desc *desc = irq_to_desc(irq);
>
> + if (irq_settings_is_per_cpu_devid(desc))
> + return -EINVAL;
> chip_bus_lock(desc);
> retval = __setup_irq(irq, desc, act);
> chip_bus_sync_unlock(desc);
> @@ -1126,7 +1128,7 @@ int setup_irq(unsigned int irq, struct
irqaction *act)
> }
> EXPORT_SYMBOL_GPL(setup_irq);
>
> - /*
> +/*
> * Internal function to unregister an irqaction - used to free
> * regular and special interrupts that are part of the architecture.
> */
> @@ -1224,7 +1226,10 @@ static struct irqaction *__free_irq(unsigned
int irq, void *dev_id)
> */
> void remove_irq(unsigned int irq, struct irqaction *act)
> {
> - __free_irq(irq, act->dev_id);
> + struct irq_desc *desc = irq_to_desc(irq);
> +
> + if (desc&& !irq_settings_is_per_cpu_devid(desc))
> + __free_irq(irq, act->dev_id);
> }
> EXPORT_SYMBOL_GPL(remove_irq);
>
> @@ -1246,7 +1251,7 @@ void free_irq(unsigned int irq, void *dev_id)
> {
> struct irq_desc *desc = irq_to_desc(irq);
>
> - if (!desc)
> + if (!desc || irq_settings_is_per_cpu_devid(desc))
> return;
>
> #ifdef CONFIG_SMP
> @@ -1324,7 +1329,8 @@ int request_threaded_irq(unsigned int irq,
irq_handler_t handler,
> if (!desc)
> return -EINVAL;
>
> - if (!irq_settings_can_request(desc))
> + if (!irq_settings_can_request(desc) ||
> + irq_settings_is_per_cpu_devid(desc))
> return -EINVAL;
>
> if (!handler) {
> @@ -1409,3 +1415,198 @@ int request_any_context_irq(unsigned int irq,
irq_handler_t handler,
> return !ret ? IRQC_IS_HARDIRQ : ret;
> }
> EXPORT_SYMBOL_GPL(request_any_context_irq);
> +
> +#ifdef CONFIG_IRQ_PERCPU_DEVID
> +void enable_percpu_irq(unsigned int irq)
> +{
> + unsigned long flags;
> + struct irq_desc *desc = irq_get_desc_buslock(irq,&flags);
> +
> + if (!desc)
> + return;
> +
> + irq_percpu_enable(desc);
> + irq_put_desc_busunlock(desc, flags);
> +}
> +EXPORT_SYMBOL(enable_percpu_irq);
> +
> +void disable_percpu_irq(unsigned int irq)
> +{
> + unsigned long flags;
> + struct irq_desc *desc = irq_get_desc_buslock(irq,&flags);
> +
> + if (!desc)
> + return;
> +
> + irq_percpu_disable(desc);
> + irq_put_desc_busunlock(desc, flags);
> +}
> +EXPORT_SYMBOL(disable_percpu_irq);
> +
> +/*
> + * Internal function to unregister a percpu irqaction.
> + */
> +static struct irqaction *__free_percpu_irq(unsigned int irq, void
__percpu *dev_id)
> +{
> + struct irq_desc *desc = irq_to_desc(irq);
> + struct irqaction *action;
> + unsigned long flags;
> +
> + WARN(in_interrupt(), "Trying to free IRQ %d from IRQ context!\n", irq);
> +
> + if (!desc)
> + return NULL;
> +
> + raw_spin_lock_irqsave(&desc->lock, flags);
> +
> + action = desc->action;
> + if (!action || action->percpu_dev_id != dev_id) {
> + WARN(1, "Trying to free already-free IRQ %d\n", irq);
> + raw_spin_unlock_irqrestore(&desc->lock, flags);
> + return NULL;
> + }
> +
> + /* Found it - now remove it from the list of entries: */
> + WARN(!cpumask_empty(desc->percpu_enabled),
> + "percpu IRQ %d still enabled on CPU%d!\n",
> + irq, cpumask_first(desc->percpu_enabled));
> + desc->action = NULL;
> +
> +#ifdef CONFIG_SMP
> + /* make sure affinity_hint is cleaned up */
> + if (WARN_ON_ONCE(desc->affinity_hint))
> + desc->affinity_hint = NULL;
> +#endif
> +
> + raw_spin_unlock_irqrestore(&desc->lock, flags);
> +
> + unregister_handler_proc(irq, action);
> +
> + /* Make sure it's not being used on another CPU: */
> + synchronize_irq(irq);
> +
> + module_put(desc->owner);
Not sure why is this required. Where is the corresponding try_module_get()?
> + return action;
> +}
> +
> +/**
> + * remove_percpu_irq - free a per-cpu interrupt
> + * @irq: Interrupt line to free
> + * @act: irqaction for the interrupt
> + *
> + * Used to remove interrupts statically setup by the early boot process.
> + */
> +void remove_percpu_irq(unsigned int irq, struct irqaction *act)
> +{
> + struct irq_desc *desc = irq_to_desc(irq);
> +
> + if (desc&& irq_settings_is_per_cpu_devid(desc))
> + __free_percpu_irq(irq, act->percpu_dev_id);
> +}
> +EXPORT_SYMBOL_GPL(remove_percpu_irq);
> +
> +/**
> + * free_percpu_irq - free an interrupt allocated with request_percpu_irq
> + * @irq: Interrupt line to free
> + * @dev_id: Device identity to free
> + *
> + * Remove a percpu interrupt handler. The handler is removed, but
> + * the interrupt line is not disabled. This must be done on each
> + * CPU before calling this function. The function does not return
> + * until any executing interrupts for this IRQ have completed.
> + *
> + * This function must not be called from interrupt context.
> + */
> +void free_percpu_irq(unsigned int irq, void __percpu *dev_id)
> +{
> + struct irq_desc *desc = irq_to_desc(irq);
> +
> + if (!desc || !irq_settings_is_per_cpu_devid(desc))
> + return;
> +
> +#ifdef CONFIG_SMP
> + if (WARN_ON(desc->affinity_notify))
> + desc->affinity_notify = NULL;
> +#endif
> +
> + chip_bus_lock(desc);
> + kfree(__free_percpu_irq(irq, dev_id));
> + chip_bus_sync_unlock(desc);
> +}
> +EXPORT_SYMBOL(free_percpu_irq);
> +
> +/**
> + * setup_percpu_irq - setup a per-cpu interrupt
> + * @irq: Interrupt line to setup
> + * @act: irqaction for the interrupt
> + *
> + * Used to statically setup per-cpu interrupts in the early boot
process.
> + */
> +int setup_percpu_irq(unsigned int irq, struct irqaction *act)
> +{
> + int retval;
> + struct irq_desc *desc = irq_to_desc(irq);
> +
> + if (!irq_settings_is_per_cpu_devid(desc))
> + return -EINVAL;
> + chip_bus_lock(desc);
> + retval = __setup_irq(irq, desc, act);
> + chip_bus_sync_unlock(desc);
> +
> + return retval;
> +}
> +EXPORT_SYMBOL_GPL(setup_percpu_irq);
> +
> +/**
> + * request_percpu_irq - allocate a percpu interrupt line
> + * @irq: Interrupt line to allocate
> + * @handler: Function to be called when the IRQ occurs.
> + * Primary handler for threaded interrupts
> + * If NULL and thread_fn != NULL the default
> + * primary handler is installed
The patch doesnt support threaded percpu interrupts - please set the
comment accordingly?
> + * @devname: An ascii name for the claiming device
> + * @dev_id: A percpu cookie passed back to the handler function
> + *
> + * This call allocates interrupt resources, but doesn't
> + * automatically enable the interrupt. It has to be done on each
> + * CPU using enable_percpu_irq().
> + *
> + * Dev_id must be globally unique. It is a per-cpu variable, and
> + * the handler gets called with the interrupted CPU's instance of
> + * that variable.
> + */
> +int request_percpu_irq(unsigned int irq, irq_handler_t handler,
> + const char *devname, void __percpu *dev_id)
Can we add irqflags argument. I think it will be useful to pass flags,
at least the IRQF_TRIGGER_MASK since it ends up calling __setup_irq().
The chip could use a set_type callback for ppi's too.
> +{
> + struct irqaction *action;
> + struct irq_desc *desc;
> + int retval;
> +
> + if (!dev_id)
> + return -EINVAL;
> +
> + desc = irq_to_desc(irq);
> + if (!desc || !irq_settings_can_request(desc) ||
> + !irq_settings_is_per_cpu_devid(desc))
> + return -EINVAL;
> +
> + action = kzalloc(sizeof(struct irqaction), GFP_KERNEL);
> + if (!action)
> + return -ENOMEM;
> +
> + action->handler = handler;
> + action->flags = IRQF_PERCPU;
continuing my previous comment this will then be changed to
action->flags = irqflags | IRQF_PERCPU
> + action->name = devname;
> + action->percpu_dev_id = dev_id;
> +
> + chip_bus_lock(desc);
> + retval = __setup_irq(irq, desc, action);
> + chip_bus_sync_unlock(desc);
> +
> + if (retval)
> + kfree(action);
> +
> + return retval;
> +}
> +EXPORT_SYMBOL(request_percpu_irq);
> +#endif
> diff --git a/kernel/irq/settings.h b/kernel/irq/settings.h
> index f166783..1162f10 100644
> --- a/kernel/irq/settings.h
> +++ b/kernel/irq/settings.h
> @@ -13,6 +13,7 @@ enum {
> _IRQ_MOVE_PCNTXT = IRQ_MOVE_PCNTXT,
> _IRQ_NO_BALANCING = IRQ_NO_BALANCING,
> _IRQ_NESTED_THREAD = IRQ_NESTED_THREAD,
> + _IRQ_PER_CPU_DEVID = IRQ_PER_CPU_DEVID,
> _IRQF_MODIFY_MASK = IRQF_MODIFY_MASK,
> };
>
> @@ -24,6 +25,7 @@ enum {
> #define IRQ_NOTHREAD GOT_YOU_MORON
> #define IRQ_NOAUTOEN GOT_YOU_MORON
> #define IRQ_NESTED_THREAD GOT_YOU_MORON
> +#define IRQ_PER_CPU_DEVID GOT_YOU_MORON
> #undef IRQF_MODIFY_MASK
> #define IRQF_MODIFY_MASK GOT_YOU_MORON
>
> @@ -39,6 +41,11 @@ static inline bool irq_settings_is_per_cpu(struct
irq_desc *desc)
> return desc->status_use_accessors& _IRQ_PER_CPU;
> }
>
> +static inline bool irq_settings_is_per_cpu_devid(struct irq_desc *desc)
> +{
> + return desc->status_use_accessors& _IRQ_PER_CPU_DEVID;
> +}
> +
> static inline void irq_settings_set_per_cpu(struct irq_desc *desc)
> {
> desc->status_use_accessors |= _IRQ_PER_CPU;
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply [flat|nested] 38+ messages in thread
* [RFC PATCH 1/3] genirq: add support for per-cpu dev_id interrupts
@ 2011-09-18 23:20 ` Abhijeet Dharmapurikar
0 siblings, 0 replies; 38+ messages in thread
From: Abhijeet Dharmapurikar @ 2011-09-18 23:20 UTC (permalink / raw)
To: linux-arm-kernel
On 09/15/2011 09:52 AM, Marc Zyngier wrote:
> The ARM GIC interrupt controller offers per CPU interrupts (PPIs),
> which are usually used to connect local timers to each core.
> Each CPU has its own private interface to the GIC,
> and only sees the PPIs that are directly connect to it.
>
> While these timers are separate devices and have a separate
> interrupt line to a core, they all use the same IRQ number.
>
> For these devices, request_irq() is not the right API as it
> assumes that an IRQ number is visible by a number of CPUs
> (through the affinity setting), but makes it very awkward to
> express that an IRQ number can be handled by all CPUs, and
> yet be a different interrupt line on each CPU, requiring a
> different dev_id cookie to be passed back to the handler.
>
> The *_percpu_irq() functions is designed to overcome these
> limitations, by providing a per-cpu dev_id vector:
>
> int request_percpu_irq(unsigned int irq, irq_handler_t handler,
> const char *devname, void __percpu *percpu_dev_id);
> void free_percpu_irq(unsigned int, void __percpu *);
> int setup_percpu_irq(unsigned int irq, struct irqaction *new);
> void remove_percpu_irq(unsigned int irq, struct irqaction *act);
> void enable_percpu_irq(unsigned int irq);
> void disable_percpu_irq(unsigned int irq);
>
> The API has a number of limitations:
> - no interrupt sharing
> - no threading
> - common handler across all the CPUs
>
> Once the interrupt is requested using setup_percpu_irq() or
> request_percpu_irq(), it must be enabled by each core that wishes
> its local interrupt to be delivered.
>
> Based on an initial patch by Thomas Gleixner.
>
> Cc: Thomas Gleixner<tglx@linutronix.de>
> Signed-off-by: Marc Zyngier<marc.zyngier@arm.com>
> ---
> include/linux/interrupt.h | 40 ++++++---
> include/linux/irq.h | 25 +++++-
> include/linux/irqdesc.h | 3 +
> kernel/irq/Kconfig | 4 +
> kernel/irq/chip.c | 58 +++++++++++++
> kernel/irq/internals.h | 2 +
> kernel/irq/manage.c | 209
++++++++++++++++++++++++++++++++++++++++++++-
> kernel/irq/settings.h | 7 ++
> 8 files changed, 332 insertions(+), 16 deletions(-)
>
> diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
> index a103732..f9b7fa3 100644
> --- a/include/linux/interrupt.h
> +++ b/include/linux/interrupt.h
> @@ -95,6 +95,7 @@ typedef irqreturn_t (*irq_handler_t)(int, void *);
> * @flags: flags (see IRQF_* above)
> * @name: name of the device
> * @dev_id: cookie to identify the device
> + * @percpu_dev_id: cookie to identify the device
> * @next: pointer to the next irqaction for shared interrupts
> * @irq: interrupt number
> * @dir: pointer to the proc/irq/NN/name entry
> @@ -104,17 +105,20 @@ typedef irqreturn_t (*irq_handler_t)(int, void *);
> * @thread_mask: bitmask for keeping track of @thread activity
> */
> struct irqaction {
> - irq_handler_t handler;
> - unsigned long flags;
> - void *dev_id;
> - struct irqaction *next;
> - int irq;
> - irq_handler_t thread_fn;
> - struct task_struct *thread;
> - unsigned long thread_flags;
> - unsigned long thread_mask;
> - const char *name;
> - struct proc_dir_entry *dir;
> + irq_handler_t handler;
> + unsigned long flags;
> + void *dev_id;
> +#ifdef CONFIG_IRQ_PERCPU_DEVID
> + void __percpu *percpu_dev_id;
> +#endif
> + struct irqaction *next;
> + int irq;
> + irq_handler_t thread_fn;
> + struct task_struct *thread;
> + unsigned long thread_flags;
> + unsigned long thread_mask;
> + const char *name;
> + struct proc_dir_entry *dir;
> } ____cacheline_internodealigned_in_smp;
>
> extern irqreturn_t no_action(int cpl, void *dev_id);
> @@ -136,6 +140,10 @@ extern int __must_check
> request_any_context_irq(unsigned int irq, irq_handler_t handler,
> unsigned long flags, const char *name, void *dev_id);
>
> +extern int __must_check
> +request_percpu_irq(unsigned int irq, irq_handler_t handler,
> + const char *devname, void __percpu *percpu_dev_id);
> +
> extern void exit_irq_thread(void);
> #else
>
> @@ -164,10 +172,18 @@ request_any_context_irq(unsigned int irq,
irq_handler_t handler,
> return request_irq(irq, handler, flags, name, dev_id);
> }
>
> +static inline int __must_check
> +request_percpu_irq(unsigned int irq, irq_handler_t handler,
> + const char *devname, void __percpu *percpu_dev_id)
> +{
> + return request_irq(irq, handler, 0, name, dev_id);
you probably meant devname here instead of name and also percpu_dev_id
instead of dev_id.
> +}
> +
> static inline void exit_irq_thread(void) { }
> #endif
>
> extern void free_irq(unsigned int, void *);
> +extern void free_percpu_irq(unsigned int, void __percpu *);
>
> struct device;
>
> @@ -207,7 +223,9 @@ extern void devm_free_irq(struct device *dev,
unsigned int irq, void *dev_id);
>
> extern void disable_irq_nosync(unsigned int irq);
> extern void disable_irq(unsigned int irq);
> +extern void disable_percpu_irq(unsigned int irq);
> extern void enable_irq(unsigned int irq);
> +extern void enable_percpu_irq(unsigned int irq);
>
> /* The following three functions are for the core kernel use only. */
> #ifdef CONFIG_GENERIC_HARDIRQS
> diff --git a/include/linux/irq.h b/include/linux/irq.h
> index 5951730..1e14fd1 100644
> --- a/include/linux/irq.h
> +++ b/include/linux/irq.h
> @@ -66,6 +66,7 @@ typedef void (*irq_preflow_handler_t)(struct
irq_data *data);
> * IRQ_NO_BALANCING - Interrupt cannot be balanced (affinity set)
> * 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
> */
> enum {
> IRQ_TYPE_NONE = 0x00000000,
> @@ -88,12 +89,13 @@ enum {
> IRQ_MOVE_PCNTXT = (1<< 14),
> IRQ_NESTED_THREAD = (1<< 15),
> IRQ_NOTHREAD = (1<< 16),
> + IRQ_PER_CPU_DEVID = (1<< 17),
> };
>
> #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_PER_CPU | IRQ_NESTED_THREAD | IRQ_NOTHREAD | IRQ_PER_CPU_DEVID)
>
> #define IRQ_NO_BALANCING_MASK (IRQ_PER_CPU | IRQ_NO_BALANCING)
>
> @@ -365,6 +367,8 @@ enum {
> struct irqaction;
> extern int setup_irq(unsigned int irq, struct irqaction *new);
> extern void remove_irq(unsigned int irq, struct irqaction *act);
> +extern int setup_percpu_irq(unsigned int irq, struct irqaction *new);
> +extern void remove_percpu_irq(unsigned int irq, struct irqaction *act);
>
> extern void irq_cpu_online(void);
> extern void irq_cpu_offline(void);
> @@ -392,6 +396,7 @@ 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_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);
> extern void handle_nested_irq(unsigned int irq);
>
> @@ -481,6 +486,24 @@ static inline void
irq_set_nested_thread(unsigned int irq, bool nest)
> irq_clear_status_flags(irq, IRQ_NESTED_THREAD);
> }
>
> +#ifdef CONFIG_IRQ_PERCPU_DEVID
> +static inline int irq_set_percpu_devid(unsigned int irq)
> +{
> + struct irq_desc *desc = irq_to_desc(irq);
> +
> + if (!desc)
> + return -EINVAL;
> +
> + if (!zalloc_cpumask_var(&desc->percpu_enabled, GFP_KERNEL))
> + return -ENOMEM;
> +
> + irq_set_status_flags(irq,
> + IRQ_NOAUTOEN | IRQ_PER_CPU | IRQ_NOTHREAD |
> + IRQ_NOPROBE | IRQ_PER_CPU_DEVID);
> + return 0;
> +}
> +#endif
> +
> /* Handle dynamic irq creation and destruction */
> extern unsigned int create_irq_nr(unsigned int irq_want, int node);
> extern int create_irq(void);
> diff --git a/include/linux/irqdesc.h b/include/linux/irqdesc.h
> index 150134a..0b4419a 100644
> --- a/include/linux/irqdesc.h
> +++ b/include/linux/irqdesc.h
> @@ -53,6 +53,9 @@ struct irq_desc {
> unsigned long last_unhandled; /* Aging timer for unhandled count */
> unsigned int irqs_unhandled;
> raw_spinlock_t lock;
> +#ifdef CONFIG_IRQ_PERCPU_DEVID
> + cpumask_var_t percpu_enabled;
> +#endif
> #ifdef CONFIG_SMP
> const struct cpumask *affinity_hint;
> struct irq_affinity_notify *affinity_notify;
> diff --git a/kernel/irq/Kconfig b/kernel/irq/Kconfig
> index 5a38bf4..75c0631 100644
> --- a/kernel/irq/Kconfig
> +++ b/kernel/irq/Kconfig
> @@ -60,6 +60,10 @@ config IRQ_DOMAIN
> config IRQ_FORCED_THREADING
> bool
>
> +# Support per CPU dev id
> +config IRQ_PERCPU_DEVID
> + bool
> +
> config SPARSE_IRQ
> bool "Support sparse irq numbering"
> depends on HAVE_SPARSE_IRQ
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index d5a3009..d65b23f 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -204,6 +204,30 @@ void irq_disable(struct irq_desc *desc)
> }
> }
>
> +#ifdef CONFIG_IRQ_PERCPU_DEVID
> +void irq_percpu_enable(struct irq_desc *desc)
> +{
> + unsigned int cpu = get_cpu();
> + if (desc->irq_data.chip->irq_enable)
> + desc->irq_data.chip->irq_enable(&desc->irq_data);
> + else
> + desc->irq_data.chip->irq_unmask(&desc->irq_data);
> + cpumask_set_cpu(cpu, desc->percpu_enabled);
> + put_cpu();
> +}
> +
> +void irq_percpu_disable(struct irq_desc *desc)
> +{
> + unsigned int cpu = get_cpu();
> + if (desc->irq_data.chip->irq_disable) {
> + desc->irq_data.chip->irq_disable(&desc->irq_data);
> + irq_state_set_masked(desc);
> + }
> + cpumask_clear_cpu(cpu, desc->percpu_enabled);
> + put_cpu();
> +}
> +#endif
> +
> static inline void mask_ack_irq(struct irq_desc *desc)
> {
> if (desc->irq_data.chip->irq_mask_ack)
> @@ -544,6 +568,40 @@ handle_percpu_irq(unsigned int irq, struct
irq_desc *desc)
> chip->irq_eoi(&desc->irq_data);
> }
>
> +#ifdef CONFIG_IRQ_PERCPU_DEVID
> +/**
> + * handle_percpu_devid_irq - Per CPU local irq handler with per cpu
dev ids
> + * @irq: the interrupt number
> + * @desc: the interrupt description structure for this irq
> + *
> + * Per CPU interrupts on SMP machines without locking requirements.
Same as
> + * handle_percpu_irq() above but with the following extras:
> + *
> + * action->percpu_dev_id is a pointer to percpu variables which
> + * contain the real device id for the cpu on which this handler is
> + * called
> + */
> +void handle_percpu_devid_irq(unsigned int irq, struct irq_desc *desc)
> +{
> + struct irq_chip *chip = irq_desc_get_chip(desc);
> + struct irqaction *action = desc->action;
> + void *dev_id = __this_cpu_ptr(action->percpu_dev_id);
> + irqreturn_t res;
> +
> + kstat_incr_irqs_this_cpu(irq, desc);
> +
> + if (chip->irq_ack)
> + chip->irq_ack(&desc->irq_data);
> +
> + trace_irq_handler_entry(irq, action);
> + res = action->handler(irq, dev_id);
> + trace_irq_handler_exit(irq, action, res);
> +
> + if (chip->irq_eoi)
> + chip->irq_eoi(&desc->irq_data);
> +}
> +#endif
> +
> void
> __irq_set_handler(unsigned int irq, irq_flow_handler_t handle, int
is_chained,
> const char *name)
> diff --git a/kernel/irq/internals.h b/kernel/irq/internals.h
> index 6546431..a57fd3b 100644
> --- a/kernel/irq/internals.h
> +++ b/kernel/irq/internals.h
> @@ -71,6 +71,8 @@ extern int irq_startup(struct irq_desc *desc);
> extern void irq_shutdown(struct irq_desc *desc);
> extern void irq_enable(struct irq_desc *desc);
> extern void irq_disable(struct irq_desc *desc);
> +extern void irq_percpu_enable(struct irq_desc *desc);
> +extern void irq_percpu_disable(struct irq_desc *desc);
> extern void mask_irq(struct irq_desc *desc);
> extern void unmask_irq(struct irq_desc *desc);
>
> diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> index 9b956fa..9f10b07 100644
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
> @@ -1118,6 +1118,8 @@ int setup_irq(unsigned int irq, struct
irqaction *act)
> int retval;
> struct irq_desc *desc = irq_to_desc(irq);
>
> + if (irq_settings_is_per_cpu_devid(desc))
> + return -EINVAL;
> chip_bus_lock(desc);
> retval = __setup_irq(irq, desc, act);
> chip_bus_sync_unlock(desc);
> @@ -1126,7 +1128,7 @@ int setup_irq(unsigned int irq, struct
irqaction *act)
> }
> EXPORT_SYMBOL_GPL(setup_irq);
>
> - /*
> +/*
> * Internal function to unregister an irqaction - used to free
> * regular and special interrupts that are part of the architecture.
> */
> @@ -1224,7 +1226,10 @@ static struct irqaction *__free_irq(unsigned
int irq, void *dev_id)
> */
> void remove_irq(unsigned int irq, struct irqaction *act)
> {
> - __free_irq(irq, act->dev_id);
> + struct irq_desc *desc = irq_to_desc(irq);
> +
> + if (desc&& !irq_settings_is_per_cpu_devid(desc))
> + __free_irq(irq, act->dev_id);
> }
> EXPORT_SYMBOL_GPL(remove_irq);
>
> @@ -1246,7 +1251,7 @@ void free_irq(unsigned int irq, void *dev_id)
> {
> struct irq_desc *desc = irq_to_desc(irq);
>
> - if (!desc)
> + if (!desc || irq_settings_is_per_cpu_devid(desc))
> return;
>
> #ifdef CONFIG_SMP
> @@ -1324,7 +1329,8 @@ int request_threaded_irq(unsigned int irq,
irq_handler_t handler,
> if (!desc)
> return -EINVAL;
>
> - if (!irq_settings_can_request(desc))
> + if (!irq_settings_can_request(desc) ||
> + irq_settings_is_per_cpu_devid(desc))
> return -EINVAL;
>
> if (!handler) {
> @@ -1409,3 +1415,198 @@ int request_any_context_irq(unsigned int irq,
irq_handler_t handler,
> return !ret ? IRQC_IS_HARDIRQ : ret;
> }
> EXPORT_SYMBOL_GPL(request_any_context_irq);
> +
> +#ifdef CONFIG_IRQ_PERCPU_DEVID
> +void enable_percpu_irq(unsigned int irq)
> +{
> + unsigned long flags;
> + struct irq_desc *desc = irq_get_desc_buslock(irq,&flags);
> +
> + if (!desc)
> + return;
> +
> + irq_percpu_enable(desc);
> + irq_put_desc_busunlock(desc, flags);
> +}
> +EXPORT_SYMBOL(enable_percpu_irq);
> +
> +void disable_percpu_irq(unsigned int irq)
> +{
> + unsigned long flags;
> + struct irq_desc *desc = irq_get_desc_buslock(irq,&flags);
> +
> + if (!desc)
> + return;
> +
> + irq_percpu_disable(desc);
> + irq_put_desc_busunlock(desc, flags);
> +}
> +EXPORT_SYMBOL(disable_percpu_irq);
> +
> +/*
> + * Internal function to unregister a percpu irqaction.
> + */
> +static struct irqaction *__free_percpu_irq(unsigned int irq, void
__percpu *dev_id)
> +{
> + struct irq_desc *desc = irq_to_desc(irq);
> + struct irqaction *action;
> + unsigned long flags;
> +
> + WARN(in_interrupt(), "Trying to free IRQ %d from IRQ context!\n", irq);
> +
> + if (!desc)
> + return NULL;
> +
> + raw_spin_lock_irqsave(&desc->lock, flags);
> +
> + action = desc->action;
> + if (!action || action->percpu_dev_id != dev_id) {
> + WARN(1, "Trying to free already-free IRQ %d\n", irq);
> + raw_spin_unlock_irqrestore(&desc->lock, flags);
> + return NULL;
> + }
> +
> + /* Found it - now remove it from the list of entries: */
> + WARN(!cpumask_empty(desc->percpu_enabled),
> + "percpu IRQ %d still enabled on CPU%d!\n",
> + irq, cpumask_first(desc->percpu_enabled));
> + desc->action = NULL;
> +
> +#ifdef CONFIG_SMP
> + /* make sure affinity_hint is cleaned up */
> + if (WARN_ON_ONCE(desc->affinity_hint))
> + desc->affinity_hint = NULL;
> +#endif
> +
> + raw_spin_unlock_irqrestore(&desc->lock, flags);
> +
> + unregister_handler_proc(irq, action);
> +
> + /* Make sure it's not being used on another CPU: */
> + synchronize_irq(irq);
> +
> + module_put(desc->owner);
Not sure why is this required. Where is the corresponding try_module_get()?
> + return action;
> +}
> +
> +/**
> + * remove_percpu_irq - free a per-cpu interrupt
> + * @irq: Interrupt line to free
> + * @act: irqaction for the interrupt
> + *
> + * Used to remove interrupts statically setup by the early boot process.
> + */
> +void remove_percpu_irq(unsigned int irq, struct irqaction *act)
> +{
> + struct irq_desc *desc = irq_to_desc(irq);
> +
> + if (desc&& irq_settings_is_per_cpu_devid(desc))
> + __free_percpu_irq(irq, act->percpu_dev_id);
> +}
> +EXPORT_SYMBOL_GPL(remove_percpu_irq);
> +
> +/**
> + * free_percpu_irq - free an interrupt allocated with request_percpu_irq
> + * @irq: Interrupt line to free
> + * @dev_id: Device identity to free
> + *
> + * Remove a percpu interrupt handler. The handler is removed, but
> + * the interrupt line is not disabled. This must be done on each
> + * CPU before calling this function. The function does not return
> + * until any executing interrupts for this IRQ have completed.
> + *
> + * This function must not be called from interrupt context.
> + */
> +void free_percpu_irq(unsigned int irq, void __percpu *dev_id)
> +{
> + struct irq_desc *desc = irq_to_desc(irq);
> +
> + if (!desc || !irq_settings_is_per_cpu_devid(desc))
> + return;
> +
> +#ifdef CONFIG_SMP
> + if (WARN_ON(desc->affinity_notify))
> + desc->affinity_notify = NULL;
> +#endif
> +
> + chip_bus_lock(desc);
> + kfree(__free_percpu_irq(irq, dev_id));
> + chip_bus_sync_unlock(desc);
> +}
> +EXPORT_SYMBOL(free_percpu_irq);
> +
> +/**
> + * setup_percpu_irq - setup a per-cpu interrupt
> + * @irq: Interrupt line to setup
> + * @act: irqaction for the interrupt
> + *
> + * Used to statically setup per-cpu interrupts in the early boot
process.
> + */
> +int setup_percpu_irq(unsigned int irq, struct irqaction *act)
> +{
> + int retval;
> + struct irq_desc *desc = irq_to_desc(irq);
> +
> + if (!irq_settings_is_per_cpu_devid(desc))
> + return -EINVAL;
> + chip_bus_lock(desc);
> + retval = __setup_irq(irq, desc, act);
> + chip_bus_sync_unlock(desc);
> +
> + return retval;
> +}
> +EXPORT_SYMBOL_GPL(setup_percpu_irq);
> +
> +/**
> + * request_percpu_irq - allocate a percpu interrupt line
> + * @irq: Interrupt line to allocate
> + * @handler: Function to be called when the IRQ occurs.
> + * Primary handler for threaded interrupts
> + * If NULL and thread_fn != NULL the default
> + * primary handler is installed
The patch doesnt support threaded percpu interrupts - please set the
comment accordingly?
> + * @devname: An ascii name for the claiming device
> + * @dev_id: A percpu cookie passed back to the handler function
> + *
> + * This call allocates interrupt resources, but doesn't
> + * automatically enable the interrupt. It has to be done on each
> + * CPU using enable_percpu_irq().
> + *
> + * Dev_id must be globally unique. It is a per-cpu variable, and
> + * the handler gets called with the interrupted CPU's instance of
> + * that variable.
> + */
> +int request_percpu_irq(unsigned int irq, irq_handler_t handler,
> + const char *devname, void __percpu *dev_id)
Can we add irqflags argument. I think it will be useful to pass flags,
at least the IRQF_TRIGGER_MASK since it ends up calling __setup_irq().
The chip could use a set_type callback for ppi's too.
> +{
> + struct irqaction *action;
> + struct irq_desc *desc;
> + int retval;
> +
> + if (!dev_id)
> + return -EINVAL;
> +
> + desc = irq_to_desc(irq);
> + if (!desc || !irq_settings_can_request(desc) ||
> + !irq_settings_is_per_cpu_devid(desc))
> + return -EINVAL;
> +
> + action = kzalloc(sizeof(struct irqaction), GFP_KERNEL);
> + if (!action)
> + return -ENOMEM;
> +
> + action->handler = handler;
> + action->flags = IRQF_PERCPU;
continuing my previous comment this will then be changed to
action->flags = irqflags | IRQF_PERCPU
> + action->name = devname;
> + action->percpu_dev_id = dev_id;
> +
> + chip_bus_lock(desc);
> + retval = __setup_irq(irq, desc, action);
> + chip_bus_sync_unlock(desc);
> +
> + if (retval)
> + kfree(action);
> +
> + return retval;
> +}
> +EXPORT_SYMBOL(request_percpu_irq);
> +#endif
> diff --git a/kernel/irq/settings.h b/kernel/irq/settings.h
> index f166783..1162f10 100644
> --- a/kernel/irq/settings.h
> +++ b/kernel/irq/settings.h
> @@ -13,6 +13,7 @@ enum {
> _IRQ_MOVE_PCNTXT = IRQ_MOVE_PCNTXT,
> _IRQ_NO_BALANCING = IRQ_NO_BALANCING,
> _IRQ_NESTED_THREAD = IRQ_NESTED_THREAD,
> + _IRQ_PER_CPU_DEVID = IRQ_PER_CPU_DEVID,
> _IRQF_MODIFY_MASK = IRQF_MODIFY_MASK,
> };
>
> @@ -24,6 +25,7 @@ enum {
> #define IRQ_NOTHREAD GOT_YOU_MORON
> #define IRQ_NOAUTOEN GOT_YOU_MORON
> #define IRQ_NESTED_THREAD GOT_YOU_MORON
> +#define IRQ_PER_CPU_DEVID GOT_YOU_MORON
> #undef IRQF_MODIFY_MASK
> #define IRQF_MODIFY_MASK GOT_YOU_MORON
>
> @@ -39,6 +41,11 @@ static inline bool irq_settings_is_per_cpu(struct
irq_desc *desc)
> return desc->status_use_accessors& _IRQ_PER_CPU;
> }
>
> +static inline bool irq_settings_is_per_cpu_devid(struct irq_desc *desc)
> +{
> + return desc->status_use_accessors& _IRQ_PER_CPU_DEVID;
> +}
> +
> static inline void irq_settings_set_per_cpu(struct irq_desc *desc)
> {
> desc->status_use_accessors |= _IRQ_PER_CPU;
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 1/3] genirq: add support for per-cpu dev_id interrupts
2011-09-18 23:20 ` Abhijeet Dharmapurikar
@ 2011-09-19 9:28 ` Marc Zyngier
-1 siblings, 0 replies; 38+ messages in thread
From: Marc Zyngier @ 2011-09-19 9:28 UTC (permalink / raw)
To: Abhijeet Dharmapurikar; +Cc: linux-arm-kernel, linux-kernel, Thomas Gleixner
On 19/09/11 00:20, Abhijeet Dharmapurikar wrote:
> On 09/15/2011 09:52 AM, Marc Zyngier wrote:
> > The ARM GIC interrupt controller offers per CPU interrupts (PPIs),
> > which are usually used to connect local timers to each core.
> > Each CPU has its own private interface to the GIC,
> > and only sees the PPIs that are directly connect to it.
> >
> > While these timers are separate devices and have a separate
> > interrupt line to a core, they all use the same IRQ number.
> >
> > For these devices, request_irq() is not the right API as it
> > assumes that an IRQ number is visible by a number of CPUs
> > (through the affinity setting), but makes it very awkward to
> > express that an IRQ number can be handled by all CPUs, and
> > yet be a different interrupt line on each CPU, requiring a
> > different dev_id cookie to be passed back to the handler.
> >
> > The *_percpu_irq() functions is designed to overcome these
> > limitations, by providing a per-cpu dev_id vector:
> >
> > int request_percpu_irq(unsigned int irq, irq_handler_t handler,
> > const char *devname, void __percpu *percpu_dev_id);
> > void free_percpu_irq(unsigned int, void __percpu *);
> > int setup_percpu_irq(unsigned int irq, struct irqaction *new);
> > void remove_percpu_irq(unsigned int irq, struct irqaction *act);
> > void enable_percpu_irq(unsigned int irq);
> > void disable_percpu_irq(unsigned int irq);
> >
> > The API has a number of limitations:
> > - no interrupt sharing
> > - no threading
> > - common handler across all the CPUs
> >
> > Once the interrupt is requested using setup_percpu_irq() or
> > request_percpu_irq(), it must be enabled by each core that wishes
> > its local interrupt to be delivered.
> >
> > Based on an initial patch by Thomas Gleixner.
> >
> > Cc: Thomas Gleixner<tglx@linutronix.de>
> > Signed-off-by: Marc Zyngier<marc.zyngier@arm.com>
> > ---
> > include/linux/interrupt.h | 40 ++++++---
> > include/linux/irq.h | 25 +++++-
> > include/linux/irqdesc.h | 3 +
> > kernel/irq/Kconfig | 4 +
> > kernel/irq/chip.c | 58 +++++++++++++
> > kernel/irq/internals.h | 2 +
> > kernel/irq/manage.c | 209
> ++++++++++++++++++++++++++++++++++++++++++++-
> > kernel/irq/settings.h | 7 ++
> > 8 files changed, 332 insertions(+), 16 deletions(-)
> >
> > diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
> > index a103732..f9b7fa3 100644
> > --- a/include/linux/interrupt.h
> > +++ b/include/linux/interrupt.h
> > @@ -95,6 +95,7 @@ typedef irqreturn_t (*irq_handler_t)(int, void *);
> > * @flags: flags (see IRQF_* above)
> > * @name: name of the device
> > * @dev_id: cookie to identify the device
> > + * @percpu_dev_id: cookie to identify the device
> > * @next: pointer to the next irqaction for shared interrupts
> > * @irq: interrupt number
> > * @dir: pointer to the proc/irq/NN/name entry
> > @@ -104,17 +105,20 @@ typedef irqreturn_t (*irq_handler_t)(int, void *);
> > * @thread_mask: bitmask for keeping track of @thread activity
> > */
> > struct irqaction {
> > - irq_handler_t handler;
> > - unsigned long flags;
> > - void *dev_id;
> > - struct irqaction *next;
> > - int irq;
> > - irq_handler_t thread_fn;
> > - struct task_struct *thread;
> > - unsigned long thread_flags;
> > - unsigned long thread_mask;
> > - const char *name;
> > - struct proc_dir_entry *dir;
> > + irq_handler_t handler;
> > + unsigned long flags;
> > + void *dev_id;
> > +#ifdef CONFIG_IRQ_PERCPU_DEVID
> > + void __percpu *percpu_dev_id;
> > +#endif
> > + struct irqaction *next;
> > + int irq;
> > + irq_handler_t thread_fn;
> > + struct task_struct *thread;
> > + unsigned long thread_flags;
> > + unsigned long thread_mask;
> > + const char *name;
> > + struct proc_dir_entry *dir;
> > } ____cacheline_internodealigned_in_smp;
> >
> > extern irqreturn_t no_action(int cpl, void *dev_id);
> > @@ -136,6 +140,10 @@ extern int __must_check
> > request_any_context_irq(unsigned int irq, irq_handler_t handler,
> > unsigned long flags, const char *name, void *dev_id);
> >
> > +extern int __must_check
> > +request_percpu_irq(unsigned int irq, irq_handler_t handler,
> > + const char *devname, void __percpu *percpu_dev_id);
> > +
> > extern void exit_irq_thread(void);
> > #else
> >
> > @@ -164,10 +172,18 @@ request_any_context_irq(unsigned int irq,
> irq_handler_t handler,
> > return request_irq(irq, handler, flags, name, dev_id);
> > }
> >
> > +static inline int __must_check
> > +request_percpu_irq(unsigned int irq, irq_handler_t handler,
> > + const char *devname, void __percpu *percpu_dev_id)
> > +{
> > + return request_irq(irq, handler, 0, name, dev_id);
>
> you probably meant devname here instead of name and also percpu_dev_id
> instead of dev_id.
Doh. Indeed, thanks for noticing this.
> > +/*
> > + * Internal function to unregister a percpu irqaction.
> > + */
> > +static struct irqaction *__free_percpu_irq(unsigned int irq, void
> __percpu *dev_id)
> > +{
> > + struct irq_desc *desc = irq_to_desc(irq);
> > + struct irqaction *action;
> > + unsigned long flags;
> > +
> > + WARN(in_interrupt(), "Trying to free IRQ %d from IRQ context!\n", irq);
> > +
> > + if (!desc)
> > + return NULL;
> > +
> > + raw_spin_lock_irqsave(&desc->lock, flags);
> > +
> > + action = desc->action;
> > + if (!action || action->percpu_dev_id != dev_id) {
> > + WARN(1, "Trying to free already-free IRQ %d\n", irq);
> > + raw_spin_unlock_irqrestore(&desc->lock, flags);
> > + return NULL;
> > + }
> > +
> > + /* Found it - now remove it from the list of entries: */
> > + WARN(!cpumask_empty(desc->percpu_enabled),
> > + "percpu IRQ %d still enabled on CPU%d!\n",
> > + irq, cpumask_first(desc->percpu_enabled));
> > + desc->action = NULL;
> > +
> > +#ifdef CONFIG_SMP
> > + /* make sure affinity_hint is cleaned up */
> > + if (WARN_ON_ONCE(desc->affinity_hint))
> > + desc->affinity_hint = NULL;
> > +#endif
> > +
> > + raw_spin_unlock_irqrestore(&desc->lock, flags);
> > +
> > + unregister_handler_proc(irq, action);
> > +
> > + /* Make sure it's not being used on another CPU: */
> > + synchronize_irq(irq);
> > +
> > + module_put(desc->owner);
>
> Not sure why is this required. Where is the corresponding try_module_get()?
A few lines into __setup_irq().
> > +/**
> > + * request_percpu_irq - allocate a percpu interrupt line
> > + * @irq: Interrupt line to allocate
> > + * @handler: Function to be called when the IRQ occurs.
> > + * Primary handler for threaded interrupts
> > + * If NULL and thread_fn != NULL the default
> > + * primary handler is installed
>
> The patch doesnt support threaded percpu interrupts - please set the
> comment accordingly?
Yup, will fix.
> > + * @devname: An ascii name for the claiming device
> > + * @dev_id: A percpu cookie passed back to the handler function
> > + *
> > + * This call allocates interrupt resources, but doesn't
> > + * automatically enable the interrupt. It has to be done on each
> > + * CPU using enable_percpu_irq().
> > + *
> > + * Dev_id must be globally unique. It is a per-cpu variable, and
> > + * the handler gets called with the interrupted CPU's instance of
> > + * that variable.
> > + */
> > +int request_percpu_irq(unsigned int irq, irq_handler_t handler,
> > + const char *devname, void __percpu *dev_id)
>
> Can we add irqflags argument. I think it will be useful to pass flags,
> at least the IRQF_TRIGGER_MASK since it ends up calling __setup_irq().
> The chip could use a set_type callback for ppi's too.
We're entering dangerous territory here. While this would work with the
GIC (the interrupt type is at the distributor level), you could easily
imagine an interrupt controller with the PPI configuration at the CPU
interface level... In that case, calling set_type from __setup_irq()
would end up doing the wrong thing, and I'd hate the API to give the
idea it can do things it may not do in the end...
Furthermore, do we actually have a GIC implementation where PPI
configuration isn't read-only? I only know about the ARM implementation,
and the Qualcomm may well be different (the spec says it's
implementation defined).
Cheers,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 38+ messages in thread
* [RFC PATCH 1/3] genirq: add support for per-cpu dev_id interrupts
@ 2011-09-19 9:28 ` Marc Zyngier
0 siblings, 0 replies; 38+ messages in thread
From: Marc Zyngier @ 2011-09-19 9:28 UTC (permalink / raw)
To: linux-arm-kernel
On 19/09/11 00:20, Abhijeet Dharmapurikar wrote:
> On 09/15/2011 09:52 AM, Marc Zyngier wrote:
> > The ARM GIC interrupt controller offers per CPU interrupts (PPIs),
> > which are usually used to connect local timers to each core.
> > Each CPU has its own private interface to the GIC,
> > and only sees the PPIs that are directly connect to it.
> >
> > While these timers are separate devices and have a separate
> > interrupt line to a core, they all use the same IRQ number.
> >
> > For these devices, request_irq() is not the right API as it
> > assumes that an IRQ number is visible by a number of CPUs
> > (through the affinity setting), but makes it very awkward to
> > express that an IRQ number can be handled by all CPUs, and
> > yet be a different interrupt line on each CPU, requiring a
> > different dev_id cookie to be passed back to the handler.
> >
> > The *_percpu_irq() functions is designed to overcome these
> > limitations, by providing a per-cpu dev_id vector:
> >
> > int request_percpu_irq(unsigned int irq, irq_handler_t handler,
> > const char *devname, void __percpu *percpu_dev_id);
> > void free_percpu_irq(unsigned int, void __percpu *);
> > int setup_percpu_irq(unsigned int irq, struct irqaction *new);
> > void remove_percpu_irq(unsigned int irq, struct irqaction *act);
> > void enable_percpu_irq(unsigned int irq);
> > void disable_percpu_irq(unsigned int irq);
> >
> > The API has a number of limitations:
> > - no interrupt sharing
> > - no threading
> > - common handler across all the CPUs
> >
> > Once the interrupt is requested using setup_percpu_irq() or
> > request_percpu_irq(), it must be enabled by each core that wishes
> > its local interrupt to be delivered.
> >
> > Based on an initial patch by Thomas Gleixner.
> >
> > Cc: Thomas Gleixner<tglx@linutronix.de>
> > Signed-off-by: Marc Zyngier<marc.zyngier@arm.com>
> > ---
> > include/linux/interrupt.h | 40 ++++++---
> > include/linux/irq.h | 25 +++++-
> > include/linux/irqdesc.h | 3 +
> > kernel/irq/Kconfig | 4 +
> > kernel/irq/chip.c | 58 +++++++++++++
> > kernel/irq/internals.h | 2 +
> > kernel/irq/manage.c | 209
> ++++++++++++++++++++++++++++++++++++++++++++-
> > kernel/irq/settings.h | 7 ++
> > 8 files changed, 332 insertions(+), 16 deletions(-)
> >
> > diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
> > index a103732..f9b7fa3 100644
> > --- a/include/linux/interrupt.h
> > +++ b/include/linux/interrupt.h
> > @@ -95,6 +95,7 @@ typedef irqreturn_t (*irq_handler_t)(int, void *);
> > * @flags: flags (see IRQF_* above)
> > * @name: name of the device
> > * @dev_id: cookie to identify the device
> > + * @percpu_dev_id: cookie to identify the device
> > * @next: pointer to the next irqaction for shared interrupts
> > * @irq: interrupt number
> > * @dir: pointer to the proc/irq/NN/name entry
> > @@ -104,17 +105,20 @@ typedef irqreturn_t (*irq_handler_t)(int, void *);
> > * @thread_mask: bitmask for keeping track of @thread activity
> > */
> > struct irqaction {
> > - irq_handler_t handler;
> > - unsigned long flags;
> > - void *dev_id;
> > - struct irqaction *next;
> > - int irq;
> > - irq_handler_t thread_fn;
> > - struct task_struct *thread;
> > - unsigned long thread_flags;
> > - unsigned long thread_mask;
> > - const char *name;
> > - struct proc_dir_entry *dir;
> > + irq_handler_t handler;
> > + unsigned long flags;
> > + void *dev_id;
> > +#ifdef CONFIG_IRQ_PERCPU_DEVID
> > + void __percpu *percpu_dev_id;
> > +#endif
> > + struct irqaction *next;
> > + int irq;
> > + irq_handler_t thread_fn;
> > + struct task_struct *thread;
> > + unsigned long thread_flags;
> > + unsigned long thread_mask;
> > + const char *name;
> > + struct proc_dir_entry *dir;
> > } ____cacheline_internodealigned_in_smp;
> >
> > extern irqreturn_t no_action(int cpl, void *dev_id);
> > @@ -136,6 +140,10 @@ extern int __must_check
> > request_any_context_irq(unsigned int irq, irq_handler_t handler,
> > unsigned long flags, const char *name, void *dev_id);
> >
> > +extern int __must_check
> > +request_percpu_irq(unsigned int irq, irq_handler_t handler,
> > + const char *devname, void __percpu *percpu_dev_id);
> > +
> > extern void exit_irq_thread(void);
> > #else
> >
> > @@ -164,10 +172,18 @@ request_any_context_irq(unsigned int irq,
> irq_handler_t handler,
> > return request_irq(irq, handler, flags, name, dev_id);
> > }
> >
> > +static inline int __must_check
> > +request_percpu_irq(unsigned int irq, irq_handler_t handler,
> > + const char *devname, void __percpu *percpu_dev_id)
> > +{
> > + return request_irq(irq, handler, 0, name, dev_id);
>
> you probably meant devname here instead of name and also percpu_dev_id
> instead of dev_id.
Doh. Indeed, thanks for noticing this.
> > +/*
> > + * Internal function to unregister a percpu irqaction.
> > + */
> > +static struct irqaction *__free_percpu_irq(unsigned int irq, void
> __percpu *dev_id)
> > +{
> > + struct irq_desc *desc = irq_to_desc(irq);
> > + struct irqaction *action;
> > + unsigned long flags;
> > +
> > + WARN(in_interrupt(), "Trying to free IRQ %d from IRQ context!\n", irq);
> > +
> > + if (!desc)
> > + return NULL;
> > +
> > + raw_spin_lock_irqsave(&desc->lock, flags);
> > +
> > + action = desc->action;
> > + if (!action || action->percpu_dev_id != dev_id) {
> > + WARN(1, "Trying to free already-free IRQ %d\n", irq);
> > + raw_spin_unlock_irqrestore(&desc->lock, flags);
> > + return NULL;
> > + }
> > +
> > + /* Found it - now remove it from the list of entries: */
> > + WARN(!cpumask_empty(desc->percpu_enabled),
> > + "percpu IRQ %d still enabled on CPU%d!\n",
> > + irq, cpumask_first(desc->percpu_enabled));
> > + desc->action = NULL;
> > +
> > +#ifdef CONFIG_SMP
> > + /* make sure affinity_hint is cleaned up */
> > + if (WARN_ON_ONCE(desc->affinity_hint))
> > + desc->affinity_hint = NULL;
> > +#endif
> > +
> > + raw_spin_unlock_irqrestore(&desc->lock, flags);
> > +
> > + unregister_handler_proc(irq, action);
> > +
> > + /* Make sure it's not being used on another CPU: */
> > + synchronize_irq(irq);
> > +
> > + module_put(desc->owner);
>
> Not sure why is this required. Where is the corresponding try_module_get()?
A few lines into __setup_irq().
> > +/**
> > + * request_percpu_irq - allocate a percpu interrupt line
> > + * @irq: Interrupt line to allocate
> > + * @handler: Function to be called when the IRQ occurs.
> > + * Primary handler for threaded interrupts
> > + * If NULL and thread_fn != NULL the default
> > + * primary handler is installed
>
> The patch doesnt support threaded percpu interrupts - please set the
> comment accordingly?
Yup, will fix.
> > + * @devname: An ascii name for the claiming device
> > + * @dev_id: A percpu cookie passed back to the handler function
> > + *
> > + * This call allocates interrupt resources, but doesn't
> > + * automatically enable the interrupt. It has to be done on each
> > + * CPU using enable_percpu_irq().
> > + *
> > + * Dev_id must be globally unique. It is a per-cpu variable, and
> > + * the handler gets called with the interrupted CPU's instance of
> > + * that variable.
> > + */
> > +int request_percpu_irq(unsigned int irq, irq_handler_t handler,
> > + const char *devname, void __percpu *dev_id)
>
> Can we add irqflags argument. I think it will be useful to pass flags,
> at least the IRQF_TRIGGER_MASK since it ends up calling __setup_irq().
> The chip could use a set_type callback for ppi's too.
We're entering dangerous territory here. While this would work with the
GIC (the interrupt type is at the distributor level), you could easily
imagine an interrupt controller with the PPI configuration at the CPU
interface level... In that case, calling set_type from __setup_irq()
would end up doing the wrong thing, and I'd hate the API to give the
idea it can do things it may not do in the end...
Furthermore, do we actually have a GIC implementation where PPI
configuration isn't read-only? I only know about the ARM implementation,
and the Qualcomm may well be different (the spec says it's
implementation defined).
Cheers,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 1/3] genirq: add support for per-cpu dev_id interrupts
2011-09-19 9:28 ` Marc Zyngier
@ 2011-09-19 15:00 ` Marc Zyngier
-1 siblings, 0 replies; 38+ messages in thread
From: Marc Zyngier @ 2011-09-19 15:00 UTC (permalink / raw)
To: Abhijeet Dharmapurikar; +Cc: linux-arm-kernel, linux-kernel, Thomas Gleixner
On 19/09/11 10:28, Marc Zyngier wrote:
> On 19/09/11 00:20, Abhijeet Dharmapurikar wrote:
>> > + * @devname: An ascii name for the claiming device
>> > + * @dev_id: A percpu cookie passed back to the handler function
>> > + *
>> > + * This call allocates interrupt resources, but doesn't
>> > + * automatically enable the interrupt. It has to be done on each
>> > + * CPU using enable_percpu_irq().
>> > + *
>> > + * Dev_id must be globally unique. It is a per-cpu variable, and
>> > + * the handler gets called with the interrupted CPU's instance of
>> > + * that variable.
>> > + */
>> > +int request_percpu_irq(unsigned int irq, irq_handler_t handler,
>> > + const char *devname, void __percpu *dev_id)
>>
>> Can we add irqflags argument. I think it will be useful to pass flags,
>> at least the IRQF_TRIGGER_MASK since it ends up calling __setup_irq().
>> The chip could use a set_type callback for ppi's too.
>
> We're entering dangerous territory here. While this would work with the
> GIC (the interrupt type is at the distributor level), you could easily
> imagine an interrupt controller with the PPI configuration at the CPU
> interface level... In that case, calling set_type from __setup_irq()
> would end up doing the wrong thing, and I'd hate the API to give the
> idea it can do things it may not do in the end...
>
> Furthermore, do we actually have a GIC implementation where PPI
> configuration isn't read-only? I only know about the ARM implementation,
> and the Qualcomm may well be different (the spec says it's
> implementation defined).
Replying to myself after a quick investigation... Looks like the Qualcomm
implementation does exactly what is mentioned above:
arch/arm/mach-msm/platsmp.c:
void __cpuinit platform_secondary_init(unsigned int cpu)
{
/* Configure edge-triggered PPIs */
writel(GIC_PPI_EDGE_MASK, MSM_QGIC_DIST_BASE + GIC_DIST_CONFIG + 4);
[...]
The way I understand it, this "MSM_QGIC_DIST_BASE + GIC_DIST_CONFIG + 4"
is a banked register (otherwise we would not do it in platform_secondary_init(),
right?) So doing a set_type() from __setup_irq() would be just wrong. It really
needs to be done on a per-CPU basis.
Do you agree?
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 38+ messages in thread
* [RFC PATCH 1/3] genirq: add support for per-cpu dev_id interrupts
@ 2011-09-19 15:00 ` Marc Zyngier
0 siblings, 0 replies; 38+ messages in thread
From: Marc Zyngier @ 2011-09-19 15:00 UTC (permalink / raw)
To: linux-arm-kernel
On 19/09/11 10:28, Marc Zyngier wrote:
> On 19/09/11 00:20, Abhijeet Dharmapurikar wrote:
>> > + * @devname: An ascii name for the claiming device
>> > + * @dev_id: A percpu cookie passed back to the handler function
>> > + *
>> > + * This call allocates interrupt resources, but doesn't
>> > + * automatically enable the interrupt. It has to be done on each
>> > + * CPU using enable_percpu_irq().
>> > + *
>> > + * Dev_id must be globally unique. It is a per-cpu variable, and
>> > + * the handler gets called with the interrupted CPU's instance of
>> > + * that variable.
>> > + */
>> > +int request_percpu_irq(unsigned int irq, irq_handler_t handler,
>> > + const char *devname, void __percpu *dev_id)
>>
>> Can we add irqflags argument. I think it will be useful to pass flags,
>> at least the IRQF_TRIGGER_MASK since it ends up calling __setup_irq().
>> The chip could use a set_type callback for ppi's too.
>
> We're entering dangerous territory here. While this would work with the
> GIC (the interrupt type is at the distributor level), you could easily
> imagine an interrupt controller with the PPI configuration at the CPU
> interface level... In that case, calling set_type from __setup_irq()
> would end up doing the wrong thing, and I'd hate the API to give the
> idea it can do things it may not do in the end...
>
> Furthermore, do we actually have a GIC implementation where PPI
> configuration isn't read-only? I only know about the ARM implementation,
> and the Qualcomm may well be different (the spec says it's
> implementation defined).
Replying to myself after a quick investigation... Looks like the Qualcomm
implementation does exactly what is mentioned above:
arch/arm/mach-msm/platsmp.c:
void __cpuinit platform_secondary_init(unsigned int cpu)
{
/* Configure edge-triggered PPIs */
writel(GIC_PPI_EDGE_MASK, MSM_QGIC_DIST_BASE + GIC_DIST_CONFIG + 4);
[...]
The way I understand it, this "MSM_QGIC_DIST_BASE + GIC_DIST_CONFIG + 4"
is a banked register (otherwise we would not do it in platform_secondary_init(),
right?) So doing a set_type() from __setup_irq() would be just wrong. It really
needs to be done on a per-CPU basis.
Do you agree?
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 1/3] genirq: add support for per-cpu dev_id interrupts
2011-09-19 15:00 ` Marc Zyngier
@ 2011-09-19 15:05 ` Russell King - ARM Linux
-1 siblings, 0 replies; 38+ messages in thread
From: Russell King - ARM Linux @ 2011-09-19 15:05 UTC (permalink / raw)
To: Marc Zyngier
Cc: Abhijeet Dharmapurikar, Thomas Gleixner, linux-kernel, linux-arm-kernel
On Mon, Sep 19, 2011 at 04:00:34PM +0100, Marc Zyngier wrote:
> Replying to myself after a quick investigation... Looks like the Qualcomm
> implementation does exactly what is mentioned above:
>
> arch/arm/mach-msm/platsmp.c:
> void __cpuinit platform_secondary_init(unsigned int cpu)
> {
> /* Configure edge-triggered PPIs */
> writel(GIC_PPI_EDGE_MASK, MSM_QGIC_DIST_BASE + GIC_DIST_CONFIG + 4);
> [...]
>
> The way I understand it, this "MSM_QGIC_DIST_BASE + GIC_DIST_CONFIG + 4"
> is a banked register (otherwise we would not do it in platform_secondary_init(),
> right?) So doing a set_type() from __setup_irq() would be just wrong. It really
> needs to be done on a per-CPU basis.
All the registers to do with the first 32 interrupts in the distributer
are banked - the enable, configuration, and priority registers are all
only accessible to the specific CPU which owns the PPIs and SGIs.
^ permalink raw reply [flat|nested] 38+ messages in thread
* [RFC PATCH 1/3] genirq: add support for per-cpu dev_id interrupts
@ 2011-09-19 15:05 ` Russell King - ARM Linux
0 siblings, 0 replies; 38+ messages in thread
From: Russell King - ARM Linux @ 2011-09-19 15:05 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Sep 19, 2011 at 04:00:34PM +0100, Marc Zyngier wrote:
> Replying to myself after a quick investigation... Looks like the Qualcomm
> implementation does exactly what is mentioned above:
>
> arch/arm/mach-msm/platsmp.c:
> void __cpuinit platform_secondary_init(unsigned int cpu)
> {
> /* Configure edge-triggered PPIs */
> writel(GIC_PPI_EDGE_MASK, MSM_QGIC_DIST_BASE + GIC_DIST_CONFIG + 4);
> [...]
>
> The way I understand it, this "MSM_QGIC_DIST_BASE + GIC_DIST_CONFIG + 4"
> is a banked register (otherwise we would not do it in platform_secondary_init(),
> right?) So doing a set_type() from __setup_irq() would be just wrong. It really
> needs to be done on a per-CPU basis.
All the registers to do with the first 32 interrupts in the distributer
are banked - the enable, configuration, and priority registers are all
only accessible to the specific CPU which owns the PPIs and SGIs.
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 1/3] genirq: add support for per-cpu dev_id interrupts
2011-09-19 15:05 ` Russell King - ARM Linux
@ 2011-09-19 15:24 ` Marc Zyngier
-1 siblings, 0 replies; 38+ messages in thread
From: Marc Zyngier @ 2011-09-19 15:24 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Abhijeet Dharmapurikar, Thomas Gleixner, linux-kernel, linux-arm-kernel
On 19/09/11 16:05, Russell King - ARM Linux wrote:
> On Mon, Sep 19, 2011 at 04:00:34PM +0100, Marc Zyngier wrote:
>> Replying to myself after a quick investigation... Looks like the Qualcomm
>> implementation does exactly what is mentioned above:
>>
>> arch/arm/mach-msm/platsmp.c:
>> void __cpuinit platform_secondary_init(unsigned int cpu)
>> {
>> /* Configure edge-triggered PPIs */
>> writel(GIC_PPI_EDGE_MASK, MSM_QGIC_DIST_BASE + GIC_DIST_CONFIG + 4);
>> [...]
>>
>> The way I understand it, this "MSM_QGIC_DIST_BASE + GIC_DIST_CONFIG + 4"
>> is a banked register (otherwise we would not do it in platform_secondary_init(),
>> right?) So doing a set_type() from __setup_irq() would be just wrong. It really
>> needs to be done on a per-CPU basis.
>
> All the registers to do with the first 32 interrupts in the distributer
> are banked - the enable, configuration, and priority registers are all
> only accessible to the specific CPU which owns the PPIs and SGIs.
Indeed. The major difference is that the configuration registers for the
PPIs seem to be writable on MSM (11MP and A9 have them RO).
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 38+ messages in thread
* [RFC PATCH 1/3] genirq: add support for per-cpu dev_id interrupts
@ 2011-09-19 15:24 ` Marc Zyngier
0 siblings, 0 replies; 38+ messages in thread
From: Marc Zyngier @ 2011-09-19 15:24 UTC (permalink / raw)
To: linux-arm-kernel
On 19/09/11 16:05, Russell King - ARM Linux wrote:
> On Mon, Sep 19, 2011 at 04:00:34PM +0100, Marc Zyngier wrote:
>> Replying to myself after a quick investigation... Looks like the Qualcomm
>> implementation does exactly what is mentioned above:
>>
>> arch/arm/mach-msm/platsmp.c:
>> void __cpuinit platform_secondary_init(unsigned int cpu)
>> {
>> /* Configure edge-triggered PPIs */
>> writel(GIC_PPI_EDGE_MASK, MSM_QGIC_DIST_BASE + GIC_DIST_CONFIG + 4);
>> [...]
>>
>> The way I understand it, this "MSM_QGIC_DIST_BASE + GIC_DIST_CONFIG + 4"
>> is a banked register (otherwise we would not do it in platform_secondary_init(),
>> right?) So doing a set_type() from __setup_irq() would be just wrong. It really
>> needs to be done on a per-CPU basis.
>
> All the registers to do with the first 32 interrupts in the distributer
> are banked - the enable, configuration, and priority registers are all
> only accessible to the specific CPU which owns the PPIs and SGIs.
Indeed. The major difference is that the configuration registers for the
PPIs seem to be writable on MSM (11MP and A9 have them RO).
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 1/3] genirq: add support for per-cpu dev_id interrupts
2011-09-19 9:28 ` Marc Zyngier
@ 2011-09-26 1:31 ` Abhijeet Dharmapurikar
-1 siblings, 0 replies; 38+ messages in thread
From: Abhijeet Dharmapurikar @ 2011-09-26 1:31 UTC (permalink / raw)
To: Marc Zyngier; +Cc: Thomas Gleixner, linux-kernel, linux-arm-kernel
On 09/19/2011 02:28 AM, Marc Zyngier wrote:
> On 19/09/11 00:20, Abhijeet Dharmapurikar wrote:
>> On 09/15/2011 09:52 AM, Marc Zyngier wrote:
...
>> > + * @devname: An ascii name for the claiming device
>> > + * @dev_id: A percpu cookie passed back to the handler function
>> > + *
>> > + * This call allocates interrupt resources, but doesn't
>> > + * automatically enable the interrupt. It has to be done on each
>> > + * CPU using enable_percpu_irq().
>> > + *
>> > + * Dev_id must be globally unique. It is a per-cpu variable, and
>> > + * the handler gets called with the interrupted CPU's instance of
>> > + * that variable.
>> > + */
>> > +int request_percpu_irq(unsigned int irq, irq_handler_t handler,
>> > + const char *devname, void __percpu *dev_id)
>>
>> Can we add irqflags argument. I think it will be useful to pass flags,
>> at least the IRQF_TRIGGER_MASK since it ends up calling __setup_irq().
>> The chip could use a set_type callback for ppi's too.
>
> We're entering dangerous territory here. While this would work with the
> GIC (the interrupt type is at the distributor level), you could easily
> imagine an interrupt controller with the PPI configuration at the CPU
> interface level... In that case, calling set_type from __setup_irq()
> would end up doing the wrong thing, and I'd hate the API to give the
> idea it can do things it may not do in the end...
>
> Furthermore, do we actually have a GIC implementation where PPI
> configuration isn't read-only? I only know about the ARM implementation,
> and the Qualcomm may well be different (the spec says it's
> implementation defined).
Yes, you are exactly right, Qualcomm's GIC has configurable PPIs. The
default configuration for PPI's is level triggered, but we change the
timer PPI to edge trigger. Without this we wont even boot (no timer
interrupts). We do this trigger type setting in board specific code.
Although I agree with your concern, I would still request to provide a
facility to set the trigger flags. All the PPI's request will have that
argument set to zero, except for msm timer (and few other msm
interrupts). Additionally we can add that concern as a comment in
request_percpu_irq so the user of request_percpu_irq is aware of it.
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply [flat|nested] 38+ messages in thread
* [RFC PATCH 1/3] genirq: add support for per-cpu dev_id interrupts
@ 2011-09-26 1:31 ` Abhijeet Dharmapurikar
0 siblings, 0 replies; 38+ messages in thread
From: Abhijeet Dharmapurikar @ 2011-09-26 1:31 UTC (permalink / raw)
To: linux-arm-kernel
On 09/19/2011 02:28 AM, Marc Zyngier wrote:
> On 19/09/11 00:20, Abhijeet Dharmapurikar wrote:
>> On 09/15/2011 09:52 AM, Marc Zyngier wrote:
...
>> > + * @devname: An ascii name for the claiming device
>> > + * @dev_id: A percpu cookie passed back to the handler function
>> > + *
>> > + * This call allocates interrupt resources, but doesn't
>> > + * automatically enable the interrupt. It has to be done on each
>> > + * CPU using enable_percpu_irq().
>> > + *
>> > + * Dev_id must be globally unique. It is a per-cpu variable, and
>> > + * the handler gets called with the interrupted CPU's instance of
>> > + * that variable.
>> > + */
>> > +int request_percpu_irq(unsigned int irq, irq_handler_t handler,
>> > + const char *devname, void __percpu *dev_id)
>>
>> Can we add irqflags argument. I think it will be useful to pass flags,
>> at least the IRQF_TRIGGER_MASK since it ends up calling __setup_irq().
>> The chip could use a set_type callback for ppi's too.
>
> We're entering dangerous territory here. While this would work with the
> GIC (the interrupt type is at the distributor level), you could easily
> imagine an interrupt controller with the PPI configuration at the CPU
> interface level... In that case, calling set_type from __setup_irq()
> would end up doing the wrong thing, and I'd hate the API to give the
> idea it can do things it may not do in the end...
>
> Furthermore, do we actually have a GIC implementation where PPI
> configuration isn't read-only? I only know about the ARM implementation,
> and the Qualcomm may well be different (the spec says it's
> implementation defined).
Yes, you are exactly right, Qualcomm's GIC has configurable PPIs. The
default configuration for PPI's is level triggered, but we change the
timer PPI to edge trigger. Without this we wont even boot (no timer
interrupts). We do this trigger type setting in board specific code.
Although I agree with your concern, I would still request to provide a
facility to set the trigger flags. All the PPI's request will have that
argument set to zero, except for msm timer (and few other msm
interrupts). Additionally we can add that concern as a comment in
request_percpu_irq so the user of request_percpu_irq is aware of it.
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 1/3] genirq: add support for per-cpu dev_id interrupts
2011-09-26 1:31 ` Abhijeet Dharmapurikar
@ 2011-09-26 1:58 ` Abhijeet Dharmapurikar
-1 siblings, 0 replies; 38+ messages in thread
From: Abhijeet Dharmapurikar @ 2011-09-26 1:58 UTC (permalink / raw)
To: Marc Zyngier; +Cc: Thomas Gleixner, linux-kernel, linux-arm-kernel
On 09/25/2011 06:31 PM, Abhijeet Dharmapurikar wrote:
> On 09/19/2011 02:28 AM, Marc Zyngier wrote:
>> On 19/09/11 00:20, Abhijeet Dharmapurikar wrote:
>>> On 09/15/2011 09:52 AM, Marc Zyngier wrote:
> ...
>>> > + * @devname: An ascii name for the claiming device
>>> > + * @dev_id: A percpu cookie passed back to the handler function
>>> > + *
>>> > + * This call allocates interrupt resources, but doesn't
>>> > + * automatically enable the interrupt. It has to be done on each
>>> > + * CPU using enable_percpu_irq().
>>> > + *
>>> > + * Dev_id must be globally unique. It is a per-cpu variable, and
>>> > + * the handler gets called with the interrupted CPU's instance of
>>> > + * that variable.
>>> > + */
>>> > +int request_percpu_irq(unsigned int irq, irq_handler_t handler,
>>> > + const char *devname, void __percpu *dev_id)
>>>
>>> Can we add irqflags argument. I think it will be useful to pass flags,
>>> at least the IRQF_TRIGGER_MASK since it ends up calling __setup_irq().
>>> The chip could use a set_type callback for ppi's too.
>>
>> We're entering dangerous territory here. While this would work with the
>> GIC (the interrupt type is at the distributor level), you could easily
>> imagine an interrupt controller with the PPI configuration at the CPU
>> interface level... In that case, calling set_type from __setup_irq()
>> would end up doing the wrong thing, and I'd hate the API to give the
>> idea it can do things it may not do in the end...
>>
>> Furthermore, do we actually have a GIC implementation where PPI
>> configuration isn't read-only? I only know about the ARM implementation,
>> and the Qualcomm may well be different (the spec says it's
>> implementation defined).
>
> Yes, you are exactly right, Qualcomm's GIC has configurable PPIs. The
> default configuration for PPI's is level triggered, but we change the
> timer PPI to edge trigger. Without this we wont even boot (no timer
> interrupts). We do this trigger type setting in board specific code.
>
> Although I agree with your concern, I would still request to provide a
> facility to set the trigger flags. All the PPI's request will have that
> argument set to zero, except for msm timer (and few other msm
> interrupts). Additionally we can add that concern as a comment in
> request_percpu_irq so the user of request_percpu_irq is aware of it.
>
I need to correct myself a tad bit. As Russell King pointed in the other
email, the trigger type register in the GIC is banked per cpu for PPI
interrupts. So, on those lines, enable_percpu_irq should take this
irqflags parameter (and call set_type on the chip) instead of
request_percpu_irq.
Thanks,
Abhijeet
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply [flat|nested] 38+ messages in thread
* [RFC PATCH 1/3] genirq: add support for per-cpu dev_id interrupts
@ 2011-09-26 1:58 ` Abhijeet Dharmapurikar
0 siblings, 0 replies; 38+ messages in thread
From: Abhijeet Dharmapurikar @ 2011-09-26 1:58 UTC (permalink / raw)
To: linux-arm-kernel
On 09/25/2011 06:31 PM, Abhijeet Dharmapurikar wrote:
> On 09/19/2011 02:28 AM, Marc Zyngier wrote:
>> On 19/09/11 00:20, Abhijeet Dharmapurikar wrote:
>>> On 09/15/2011 09:52 AM, Marc Zyngier wrote:
> ...
>>> > + * @devname: An ascii name for the claiming device
>>> > + * @dev_id: A percpu cookie passed back to the handler function
>>> > + *
>>> > + * This call allocates interrupt resources, but doesn't
>>> > + * automatically enable the interrupt. It has to be done on each
>>> > + * CPU using enable_percpu_irq().
>>> > + *
>>> > + * Dev_id must be globally unique. It is a per-cpu variable, and
>>> > + * the handler gets called with the interrupted CPU's instance of
>>> > + * that variable.
>>> > + */
>>> > +int request_percpu_irq(unsigned int irq, irq_handler_t handler,
>>> > + const char *devname, void __percpu *dev_id)
>>>
>>> Can we add irqflags argument. I think it will be useful to pass flags,
>>> at least the IRQF_TRIGGER_MASK since it ends up calling __setup_irq().
>>> The chip could use a set_type callback for ppi's too.
>>
>> We're entering dangerous territory here. While this would work with the
>> GIC (the interrupt type is at the distributor level), you could easily
>> imagine an interrupt controller with the PPI configuration at the CPU
>> interface level... In that case, calling set_type from __setup_irq()
>> would end up doing the wrong thing, and I'd hate the API to give the
>> idea it can do things it may not do in the end...
>>
>> Furthermore, do we actually have a GIC implementation where PPI
>> configuration isn't read-only? I only know about the ARM implementation,
>> and the Qualcomm may well be different (the spec says it's
>> implementation defined).
>
> Yes, you are exactly right, Qualcomm's GIC has configurable PPIs. The
> default configuration for PPI's is level triggered, but we change the
> timer PPI to edge trigger. Without this we wont even boot (no timer
> interrupts). We do this trigger type setting in board specific code.
>
> Although I agree with your concern, I would still request to provide a
> facility to set the trigger flags. All the PPI's request will have that
> argument set to zero, except for msm timer (and few other msm
> interrupts). Additionally we can add that concern as a comment in
> request_percpu_irq so the user of request_percpu_irq is aware of it.
>
I need to correct myself a tad bit. As Russell King pointed in the other
email, the trigger type register in the GIC is banked per cpu for PPI
interrupts. So, on those lines, enable_percpu_irq should take this
irqflags parameter (and call set_type on the chip) instead of
request_percpu_irq.
Thanks,
Abhijeet
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply [flat|nested] 38+ messages in thread
end of thread, other threads:[~2011-09-26 1:58 UTC | newest]
Thread overview: 38+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-15 16:52 [RFC PATCH 0/3] genirq: handling GIC per-cpu interrupts Marc Zyngier
2011-09-15 16:52 ` Marc Zyngier
2011-09-15 16:52 ` [RFC PATCH 1/3] genirq: add support for per-cpu dev_id interrupts Marc Zyngier
2011-09-15 16:52 ` Marc Zyngier
2011-09-15 21:36 ` Michał Mirosław
2011-09-15 21:36 ` Michał Mirosław
2011-09-16 8:20 ` Marc Zyngier
2011-09-16 8:20 ` Marc Zyngier
2011-09-16 9:37 ` Thomas Gleixner
2011-09-16 9:37 ` Thomas Gleixner
2011-09-15 22:49 ` Thomas Gleixner
2011-09-15 22:49 ` Thomas Gleixner
2011-09-15 23:29 ` Russell King - ARM Linux
2011-09-15 23:29 ` Russell King - ARM Linux
2011-09-15 23:41 ` Thomas Gleixner
2011-09-15 23:41 ` Thomas Gleixner
2011-09-16 9:37 ` Marc Zyngier
2011-09-16 9:37 ` Marc Zyngier
2011-09-16 9:41 ` Thomas Gleixner
2011-09-16 9:41 ` Thomas Gleixner
2011-09-18 23:20 ` Abhijeet Dharmapurikar
2011-09-18 23:20 ` Abhijeet Dharmapurikar
2011-09-19 9:28 ` Marc Zyngier
2011-09-19 9:28 ` Marc Zyngier
2011-09-19 15:00 ` Marc Zyngier
2011-09-19 15:00 ` Marc Zyngier
2011-09-19 15:05 ` Russell King - ARM Linux
2011-09-19 15:05 ` Russell King - ARM Linux
2011-09-19 15:24 ` Marc Zyngier
2011-09-19 15:24 ` Marc Zyngier
2011-09-26 1:31 ` Abhijeet Dharmapurikar
2011-09-26 1:31 ` Abhijeet Dharmapurikar
2011-09-26 1:58 ` Abhijeet Dharmapurikar
2011-09-26 1:58 ` Abhijeet Dharmapurikar
2011-09-15 16:52 ` [RFC PATCH 2/3] ARM: gic: consolidate PPI handling Marc Zyngier
2011-09-15 16:52 ` Marc Zyngier
2011-09-15 16:52 ` [RFC PATCH 3/3] ARM: gic, local timers: use the request_percpu_irq() interface Marc Zyngier
2011-09-15 16:52 ` Marc Zyngier
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.