| Message ID | 1404096760-5239-1-git-send-email-xerofoify@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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.
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.
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(); }
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(-)