[U-Boot] [PATCH v2 3/7] microblaze: intc: Registering interrupt should return value

[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 <monstr at monstr.eu>
>>
>> ---
>> 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)


> ... whatever, you should evaluate the return code in fsl_init2() too.

Not necessary to do it in this patch.

Thanks,
Michal


-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian