Re: [RFC V2 1/2] irq: Add a framework to measure interrupt timings

[Thomas Gleixner]

On Wed, 20 Jan 2016, Daniel Lezcano wrote:
> +#ifdef CONFIG_IRQ_TIMINGS
> +/**
> + * struct irqt_ops - structure to be used by the subsystem to track
> + *                   irq timings
> + * @alloc:    called when an irqdesc is allocated
> + * @free:     called when an irqdesc is free
> + * @setup:    called when an irq is setup, this is called under lock
> + * @remove:   called when an irq is removed
> + * @handler:  called when an interrupt is handled
> + */
> +struct irqtimings_ops {
> +	int (*alloc)(unsigned int);
> +	void (*free)(unsigned int);
> +	int (*setup)(unsigned int, struct irqaction *act);
> +	void (*remove)(unsigned int, void *dev_id);
> +	irqt_handler_t handler;
> +};
> +
> +/**
> + * This macro *must* be used by the subsystem interested by the irq
> + * timing information.
> + */
> +#define DECLARE_IRQ_TIMINGS(__ops)				\
> +	const struct irqtimings_ops *__irqtimings = __ops;
> +#endif

> @@ -20,6 +20,49 @@ extern bool noirqdebug;
>  
>  extern struct irqaction chained_action;
>  
> +#ifdef CONFIG_IRQ_TIMINGS
> +
> +extern const struct irqtimings_ops *__irqtimings;
> +
> +static inline int alloc_irqtiming(unsigned int irq)
> +{
> +	if (__irqtimings->alloc)
> +		return __irqtimings->alloc(irq);

I really have a hard time to understand that indirection. __irqtimings is
statically allocated and compiled in. There can be only one user for this in
the system ever and that user has all callbacks populated.

Why can't you spare all that pointer muck and simply have:

#ifdef CONFIG_IRQ_TIMINGS
int irqtiming_alloc(usigned int irq);
....
#else
static int irqtiming_alloc(usigned int irq) { return 0; }
...
#endif

and implement those functions in your idle thingy?

Thanks,

	tglx