Patchwork [U-Boot,1/2] microblaze: Disabling interrupt should return 1 if implemented

login
register
mail settings
Submitter Michal Simek
Date Dec. 21, 2010, 12:09 p.m.
Message ID <1292933384-3032-2-git-send-email-monstr@monstr.eu>
Download mbox | patch
Permalink /patch/76284/
State Changes Requested
Headers show

Comments

Michal Simek - Dec. 21, 2010, 12:09 p.m.
Microblaze implement enable/disable interrupts through MSR
that's why disable_interrupts function should return 1.

Signed-off-by: John Linn <john.linn@xilinx.com>
Signed-off-by: Michal Simek <monstr@monstr.eu>
---
 arch/microblaze/cpu/interrupts.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Wolfgang Denk - Dec. 21, 2010, 12:24 p.m.
Dear Michal Simek,

In message <1292933384-3032-2-git-send-email-monstr@monstr.eu> you wrote:
> Microblaze implement enable/disable interrupts through MSR
> that's why disable_interrupts function should return 1.
> 
> Signed-off-by: John Linn <john.linn@xilinx.com>
> Signed-off-by: Michal Simek <monstr@monstr.eu>
> ---
>  arch/microblaze/cpu/interrupts.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/microblaze/cpu/interrupts.c b/arch/microblaze/cpu/interrupts.c
> index e9d53c1..331746c 100644
> --- a/arch/microblaze/cpu/interrupts.c
> +++ b/arch/microblaze/cpu/interrupts.c
> @@ -42,7 +42,7 @@ void enable_interrupts (void)
>  int disable_interrupts (void)
>  {
>  	MSRCLR(0x2);
> -	return 0;
> +	return 1;
>  }

I think this is wrong.  disable_interrupts() should return 1 only if
interrupts were enabled before, so code like this can make sure that
some parts are run with interrupts disabled, but then restore the
previous state, no matter if this had interrupts on or off:

	int flag = disable_interrupts();
	...
	do something
	...
	if (flag)
		enable_interrupts();

With your code, we would ALWAYS enable interrupts, even if these were
off before.

For reference, see for example arch/powerpc/lib/interrupts.c

Best regards,

Wolfgang Denk

Patch

diff --git a/arch/microblaze/cpu/interrupts.c b/arch/microblaze/cpu/interrupts.c
index e9d53c1..331746c 100644
--- a/arch/microblaze/cpu/interrupts.c
+++ b/arch/microblaze/cpu/interrupts.c
@@ -42,7 +42,7 @@  void enable_interrupts (void)
 int disable_interrupts (void)
 {
 	MSRCLR(0x2);
-	return 0;
+	return 1;
 }
 
 #ifdef CONFIG_SYS_INTC_0