All of lore.kernel.org
 help / color / mirror / Atom feed
From: adharmap@codeaurora.org (Abhijeet Dharmapurikar)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC PATCH 1/3] genirq: add support for per-cpu dev_id interrupts
Date: Sun, 18 Sep 2011 16:20:54 -0700	[thread overview]
Message-ID: <4E767CD6.6090208@codeaurora.org> (raw)
In-Reply-To: <1316105551-17505-2-git-send-email-marc.zyngier@arm.com>

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.

WARNING: multiple messages have this Message-ID (diff)
From: Abhijeet Dharmapurikar <adharmap@codeaurora.org>
To: Marc Zyngier <marc.zyngier@arm.com>
Cc: linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org,
	Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [RFC PATCH 1/3] genirq: add support for per-cpu dev_id interrupts
Date: Sun, 18 Sep 2011 16:20:54 -0700	[thread overview]
Message-ID: <4E767CD6.6090208@codeaurora.org> (raw)
In-Reply-To: <1316105551-17505-2-git-send-email-marc.zyngier@arm.com>

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.

  parent reply	other threads:[~2011-09-18 23:20 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4E767CD6.6090208@codeaurora.org \
    --to=adharmap@codeaurora.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.