From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759110AbcAUJ0G (ORCPT ); Thu, 21 Jan 2016 04:26:06 -0500 Received: from mail-wm0-f44.google.com ([74.125.82.44]:34783 "EHLO mail-wm0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758564AbcAUJZ7 (ORCPT ); Thu, 21 Jan 2016 04:25:59 -0500 Subject: Re: [RFC V2 1/2] irq: Add a framework to measure interrupt timings To: Thomas Gleixner References: <1453305636-22156-1-git-send-email-daniel.lezcano@linaro.org> <1453305636-22156-2-git-send-email-daniel.lezcano@linaro.org> Cc: peterz@infradead.org, rafael@kernel.org, linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org, nicolas.pitre@linaro.org, vincent.guittot@linaro.org From: Daniel Lezcano Message-ID: <56A0A424.80306@linaro.org> Date: Thu, 21 Jan 2016 10:25:56 +0100 User-Agent: Mozilla/5.0 (X11; Linux i686; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.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 ? -- Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog