Re: [PATCH 01/02] OMAP3 CPUidle driver

Högander Jouni
2008-06-18T07:24:29Z

"ext Kevin Hilman" <khilman@mvista.com> writes:

> "Rajendra Nayak" <rnayak@ti.com> writes:
>
>>> "Rajendra Nayak" <rnayak@ti.com> writes:
>>>
>>> > This patch adds the OMAP3 cpuidle driver. Irq enable/disable is done
>>> > in the core cpuidle driver before it queries the governor for the
>>> > next state.
>>>
>>> Can you explain why you need the IRQ/FIQ disable added to
>>> cpuidle_idle_call()
>>>
>>
>> This was done to prevent any interrupts firing in between a
>> cpuidle_curr_governor->select() and target_state->enter().
>
> I understand that, but I still don't understand exactly what you're
> trying to prevent. Did you have a specific bug that this prevented?
>
>> An interrupt in between could end up with a previously selected
>> state to be programmed.
>
> Remember that this function _is_ the idle loop, meaning when this runs
> nothing else is happening. After the select, if other system activity
> has happened (e.g. and interrupt, or thread wakeup etc.), it will run
> before the target_state->enter() because of the check for
> need_resched().

What happens if this interrupt, or thread wakeup causes change on
latency requirements? Then we are entering sleep state which was
selected using wrong latency requirement data.

>
>> Any suggestions on a better way to handle this?
>
> Just drop the IRQ/FIQ disables altogether.

At least these are needed at some point in idle loop. Otherwise we
might stepout from idle and sram in a point where it is not acceptable.

--
Jouni Högander