[PATCH] Fixes gemmi_idle in file idle.c

[PATCH] Fixes gemmi_idle in file idle.c

[Nicholas Krause]

disables the IRQ line in gemmi_idle before calling cpu_do_idle.
In addition, according to the Fixme message in this file fixes
races that may occur due to not disabling the IRQ before calling
cpu_do_idle.

Signed-off-by: Nicholas Krause <xerofoify@gmail.com>
---
 arch/arm/mach-gemini/idle.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm/mach-gemini/idle.c b/arch/arm/mach-gemini/idle.c
index ddf8ec9..2880f5a 100644
--- a/arch/arm/mach-gemini/idle.c
+++ b/arch/arm/mach-gemini/idle.c
@@ -17,8 +17,7 @@ static void gemini_idle(void)
 	 * disabled.
 	 */
 
-	/* FIXME: Enabling interrupts here is racy! */
-	local_irq_enable();
+	local_irq_disable();
 	cpu_do_idle();
 }
 
-- 
1.9.1

[PATCH] Fixes gemmi_idle in file idle.c

[Arnd Bergmann]

On Sunday 29 June 2014 22:52:40 Nicholas Krause wrote:
> disables the IRQ line in gemmi_idle before calling cpu_do_idle.
> In addition, according to the Fixme message in this file fixes
> races that may occur due to not disabling the IRQ before calling
> cpu_do_idle.
> 
> Signed-off-by: Nicholas Krause <xerofoify@gmail.com>

Have you actually read the comment above the function?

> diff --git a/arch/arm/mach-gemini/idle.c b/arch/arm/mach-gemini/idle.c
> index ddf8ec9..2880f5a 100644
> --- a/arch/arm/mach-gemini/idle.c
> +++ b/arch/arm/mach-gemini/idle.c
> @@ -17,8 +17,7 @@ static void gemini_idle(void)
>          * disabled.
>          */
>  
> -       /* FIXME: Enabling interrupts here is racy! */
> -       local_irq_enable();
> +       local_irq_disable();
>         cpu_do_idle();
>  }

This has the same effect as deleting the entire file. Doing that
is probably the right thing now that 8182a34d85698 ("ARM: 7760/1:
cpu_fa526_do_idle: remove WFI") has been merged, but that needs
a better changeset description to explain why it was needed before
and is not needed any more.

Please submit a new patch doing that.

	Arnd

[PATCH] Fixes gemmi_idle in file idle.c

[Russell King - ARM Linux]

On Sun, Jun 29, 2014 at 10:52:40PM -0400, Nicholas Krause wrote:
> disables the IRQ line in gemmi_idle before calling cpu_do_idle.
> In addition, according to the Fixme message in this file fixes
> races that may occur due to not disabling the IRQ before calling
> cpu_do_idle.

1) the idle method is called with interrupts already disabled.
2) I wonder if you read the comment above the comment you removed, and
   if so, why it wasn't removed.
3) can you indicate how the hardware problem has been fixed, or give some
   justification why this change should be accepted.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[PATCH] Fixes gemmi_idle in file idle.c

[Nick Krause]

Thanks for the good feedback on kernel development , I will delete this file
in order to clean up race conditions caused by it.
Cheers Nick

On Mon, Jun 30, 2014 at 5:41 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Sun, Jun 29, 2014 at 10:52:40PM -0400, Nicholas Krause wrote:
>> disables the IRQ line in gemmi_idle before calling cpu_do_idle.
>> In addition, according to the Fixme message in this file fixes
>> races that may occur due to not disabling the IRQ before calling
>> cpu_do_idle.
>
> 1) the idle method is called with interrupts already disabled.
> 2) I wonder if you read the comment above the comment you removed, and
>    if so, why it wasn't removed.
> 3) can you indicate how the hardware problem has been fixed, or give some
>    justification why this change should be accepted.
>
> --
> FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
> improving, and getting towards what was expected from it.