From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephan Linz Date: Wed, 08 Aug 2012 19:47:15 +0200 Subject: [U-Boot] [PATCH v2 3/7] microblaze: intc: Registering interrupt should return value In-Reply-To: <502222D7.9070705@monstr.eu> References: <1344239199-11445-1-git-send-email-monstr@monstr.eu> <1344239199-11445-3-git-send-email-monstr@monstr.eu> <1344370237.29456.88.camel@keto> <502222D7.9070705@monstr.eu> Message-ID: <1344448035.28216.11.camel@keto> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Am Mittwoch, den 08.08.2012, 10:27 +0200 schrieb Michal Simek: > On 08/07/2012 10:10 PM, Stephan Linz wrote: > > Am Montag, den 06.08.2012, 09:46 +0200 schrieb Michal Simek: > >> Return value to find out if un/registration was succesful. > >> > >> Signed-off-by: Michal Simek > >> > >> --- > >> v2: Add comment to header file to describe parameters and return codes > >> --- > >> arch/microblaze/cpu/interrupts.c | 16 +++++++++------- > >> arch/microblaze/include/asm/microblaze_intc.h | 11 ++++++++++- > >> 2 files changed, 19 insertions(+), 8 deletions(-) > >> > >> diff --git a/arch/microblaze/cpu/interrupts.c b/arch/microblaze/cpu/interrupts.c > >> index ee67082..08f6bad 100644 > >> --- a/arch/microblaze/cpu/interrupts.c > >> +++ b/arch/microblaze/cpu/interrupts.c > >> @@ -91,14 +91,13 @@ static void disable_one_interrupt(int irq) > >> #endif > >> } > >> > >> -/* adding new handler for interrupt */ > >> -void install_interrupt_handler (int irq, interrupt_handler_t * hdlr, void *arg) > >> +int install_interrupt_handler(int irq, interrupt_handler_t *hdlr, void *arg) > >> { > >> struct irq_action *act; > >> /* irq out of range */ > >> if ((irq < 0) || (irq > irq_no)) { > >> puts ("IRQ out of range\n"); > >> - return; > >> + return -1; > >> } > >> act = &vecs[irq]; > >> if (hdlr) { /* enable */ > >> @@ -106,11 +105,14 @@ void install_interrupt_handler (int irq, interrupt_handler_t * hdlr, void *arg) > >> act->arg = arg; > >> act->count = 0; > >> enable_one_interrupt (irq); > >> - } else { /* disable */ > >> - act->handler = (interrupt_handler_t *) def_hdlr; > >> - act->arg = (void *)irq; > >> - disable_one_interrupt (irq); > >> + return 0; > >> } > >> + > >> + /* Disable */ > >> + act->handler = (interrupt_handler_t *) def_hdlr; > >> + act->arg = (void *)irq; > >> + disable_one_interrupt(irq); > >> + return 1; > >> } > >> > >> /* initialization interrupt controller - hardware */ > >> diff --git a/arch/microblaze/include/asm/microblaze_intc.h b/arch/microblaze/include/asm/microblaze_intc.h > >> index 6142b9c..e9640f5 100644 > >> --- a/arch/microblaze/include/asm/microblaze_intc.h > >> +++ b/arch/microblaze/include/asm/microblaze_intc.h > >> @@ -39,7 +39,16 @@ struct irq_action { > >> int count; /* number of interrupt */ > >> }; > >> > >> -void install_interrupt_handler (int irq, interrupt_handler_t * hdlr, > >> +/** > >> + * Register and unregister interrupt handler rutines > >> + * > >> + * @param irq IRQ number > >> + * @param hdlr Interrupt handler rutine > >> + * @param arg Pointer to argument which is passed to int. handler rutine > >> + * @return 0 if registration pass, 1 if unregistration pass, > >> + * or an error code < 0 otherwise > >> + */ > >> +int install_interrupt_handler(int irq, interrupt_handler_t *hdlr, > >> void *arg); > > > > Hi Michal, > > > > why not two different functions here, one for registration, another one > > for unregistration? To mee it is puzzling to use a 'install' function > > for unregistration ... > > partially agree with that. Maybe we could introduce one macro for that. > > #define uninstall_interrupt_handler(irq) install_interrupt_handler(irq, NULL, NULL) yes, that is ok ... > > > > ... whatever, you should evaluate the return code in fsl_init2() too. > > Not necessary to do it in this patch. yes, it's another part ... br, Stephan