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

[Daniel Lezcano]

On 01/20/2016 06:55 PM, Thomas Gleixner wrote:
> 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?

Hi Thomas,

yes sure, I can do something simpler.

Just to be sure, do you suggest to put the function declaration in 
kernel/irq/internal.h and the function definition in 
kernel/sched/idle-sched.c ?

--
 <http://www.linaro.org/> Linaro.org â Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog