Re: [RFC patch 2/5] genirq: add a quick check handler

[Steven Rostedt]

Thomas,

Nice work, thanks for doing this.

Little comments below.

On Wed, 1 Oct 2008, Thomas Gleixner wrote:
>  
>  extern irqreturn_t no_action(int cpl, void *dev_id);
> -extern int __must_check request_irq(unsigned int, irq_handler_t handler,
> +
> +extern int __must_check
> +request_irq_quickcheck(unsigned int, irq_handler_t handler,
> +		       irq_handler_t quick_check_handler,
>  		       unsigned long, const char *, void *);

It would be nice to still keep the name of the parameters here.
	unsigned long flags, const char *name, void *dev

> +
> +static inline int __must_check
> +request_irq(unsigned int irq, irq_handler_t handler, unsigned long flags,
> +	    const char *name, void *dev)
> +{
> +	return request_irq_quickcheck(irq, handler, NULL, flags, name, dev);
> +}
> +
>  extern void free_irq(unsigned int, void *);
>  
>  struct device;
> Index: linux-2.6-tip/include/linux/irqreturn.h
> ===================================================================
> --- linux-2.6-tip.orig/include/linux/irqreturn.h
> +++ linux-2.6-tip/include/linux/irqreturn.h
> @@ -5,10 +5,12 @@
>   * enum irqreturn
>   * @IRQ_NONE		interrupt was not from this device
>   * @IRQ_HANDLED		interrupt was handled by this device
> + * @IRQ_NEEDS_HANDLING	quick check handler requests to run real handler
>   */
>  enum irqreturn {
>  	IRQ_NONE,
>  	IRQ_HANDLED,
> +	IRQ_NEEDS_HANDLING,
>  };
>  
>  #define irqreturn_t	enum irqreturn
> Index: linux-2.6-tip/kernel/irq/handle.c
> ===================================================================
> --- linux-2.6-tip.orig/kernel/irq/handle.c
> +++ linux-2.6-tip/kernel/irq/handle.c
> @@ -137,9 +137,21 @@ irqreturn_t handle_IRQ_event(unsigned in
>  		local_irq_enable_in_hardirq();
>  
>  	do {
> -		ret = action->handler(irq, action->dev_id);
> -		if (ret == IRQ_HANDLED)
> -			status |= action->flags;
> +		if (action->quick_check_handler)
> +			ret = action->quick_check_handler(irq, action->dev_id);
> +		else
> +			ret = IRQ_NEEDS_HANDLING;
> +
> +		switch (ret) {
> +		default:
> +			break;
> +
> +		case IRQ_NEEDS_HANDLING:
> +			ret = action->handler(irq, action->dev_id);
> +			if (ret == IRQ_HANDLED)
> +				status |= action->flags;
> +			break;
> +		}
>  		retval |= ret;
>  		action = action->next;
>  	} while (action);
> Index: linux-2.6-tip/kernel/irq/manage.c
> ===================================================================
> --- linux-2.6-tip.orig/kernel/irq/manage.c
> +++ linux-2.6-tip/kernel/irq/manage.c
> @@ -563,9 +563,14 @@ void free_irq(unsigned int irq, void *de
>  EXPORT_SYMBOL(free_irq);
>  
>  /**
> - *	request_irq - allocate an interrupt line
> + *	request_irq_quickcheck - allocate an interrupt line
>   *	@irq: Interrupt line to allocate
> - *	@handler: Function to be called when the IRQ occurs
> + *	@handler: Function to be called when the IRQ occurs.
> + *		  Primary handler for threaded interrupts
> + *      @quick_check_handler: Function to be called when the interrupt
> + *			   before the real handler is called to check
> + *                         whether the interrupt originated from the device
> + *                         can be NULL


The above reads funny. How about something like:

	@quick_check_handler: Function called before the real interrupt
			handler. It checks if the interrupt originated
			from the device. This can be NULL.

-- Steve


>   *	@irqflags: Interrupt type flags
>   *	@devname: An ascii name for the claiming device
>   *	@dev_id: A cookie passed back to the handler function
> @@ -589,10 +594,11 @@ EXPORT_SYMBOL(free_irq);
>   *	IRQF_SHARED		Interrupt is shared
>   *	IRQF_DISABLED	Disable local interrupts while processing
>   *	IRQF_SAMPLE_RANDOM	The interrupt can be used for entropy
> - *
>   */
> -int request_irq(unsigned int irq, irq_handler_t handler,
> -		unsigned long irqflags, const char *devname, void *dev_id)
> +int request_irq_quickcheck(unsigned int irq, irq_handler_t handler,
> +			   irq_handler_t quick_check_handler,
> +			   unsigned long irqflags, const char *devname,
> +			   void *dev_id)
>  {
>  	struct irqaction *action;
>  	int retval;
> @@ -622,6 +628,7 @@ int request_irq(unsigned int irq, irq_ha
>  	if (!action)
>  		return -ENOMEM;
>  
> +	action->quick_check_handler = quick_check_handler;
>  	action->handler = handler;
>  	action->flags = irqflags;
>  	cpus_clear(action->mask);
> @@ -651,4 +658,4 @@ int request_irq(unsigned int irq, irq_ha
>  
>  	return retval;
>  }
> -EXPORT_SYMBOL(request_irq);
> +EXPORT_SYMBOL(request_irq_quickcheck);
> 
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/