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

[Daniel Lezcano <daniel.lezcano@linaro.org>]

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