Hi Nishanth,
On Friday 13 July 2012 03:21 PM, Menon, Nishanth wrote:
On Thursday 05 July 2012, Rajendra Nayak wrote:
[..]
From 5f5e4eb342110286bf719c7d9d7c1959f53e34f9 Mon Sep 17 00:00:00 2001
From: Rajendra Nayak<rnayak@xxxxxx>
Date: Thu, 5 Jul 2012 17:33:28 +0530
Subject: [RFC] ARM: OMAP: Powerdomain: control memory and logic bits internally
Powerdomain framework exposes various apis for memory and logic
control for powerdomains, for its users to program OSWR: Open SWitch
Retention state. While in theory, there are various combinations of
memory and logic states possible which can be configured as OSWR,
in practice all OMAPs use just one combination. Logic lost, memory retained.
This can very easily be handled within the powerdomain framework itself,
without exposing all complex memory/logic control apis to upper layer
drivers like cpuidle and suspend.
To do this, introduce 2 new power domain states PWRDM_POWER_CSWR and
PWRDM_POWER_OSWR usable by the users of powerdomain framework and
make all memory/logic control apis internal to powerdomain framework.
Change all users of powerdomain framework to get rid of all usage
of memory/logic control apis and use the newly defined states for
CSWR and OSWR with the already used powerstate control apis.
Some functions (which are now made internal) are forward declared
to avoid moving functions around in the file (which can be done in a
later patch) to help keep the patch reviewable.
Signed-off-by: Rajendra Nayak<rnayak@xxxxxx>
Ref: http://marc.info/?t=133968586800004&r=1&w=2
Apologies, but i've had to copypaste the original message, so inline response
might be a bit messed up.
From an initial port to get cpuidle working on OMAP5, my experiences follow:
Thanks for the tests and the review.
a) counter handling (pm-debug.c) - we can now do better than give our
arcane RET:5
LOGIC-POWER-OFF:4 , instead we can clearly indicate OSWR, CSWR in
counter
Part of the issue also now becomes that count and time arrays are in
the range of
PWRDM_POWER_ON. They break when CSWR/OSWR is in pwrdm->state
b) pwrst handling this becomes a hard one to handle (as usual) when
comparisons of
while (!(pwrdm->pwrsts& (1<< pwrst))) {
if (pwrst == PWRDM_POWER_OFF)
goto out;
pwrst--;
}
with value 4, 5 -> pwrsts should either now use OSWR, CSWR definitions
OR we will need translate back before checks
c) in few critical places, these mentioned error checks do silent
error returns - example:
if (!(pwrdm->pwrsts_logic_ret& (1<< pwrst)))
return -EINVAL;
this bit me more than once while i tried to bring up the patch
we should be doing a patch which introduces a ratelimited WARN to kill the
bad callers.
d) we have been lazy in programming and have been using cur_pwrst<
PWRDM_POWER_ON or INACTIVE etc.. and do a set of operations based off
that. this wont work as CSWR, OSWR> POWER_INACTIVE. (e.g. pm3 code)
All are valid issues. Some I overlooked, some like the array index
issues due to CSWR/OSWR being defined post OFF, I knew but did not
handle well because it was a hastily cooked up RFC to clarify our
thoughts.
now that I have more feedback I will certainly wort on improving it.
e) similar to what Jean did, omap_set_pwrdm_state will need to move
over from pm.c to powerdomain.c
f) We probably should also will need an updated patch for
http://marc.info/?l=linux-omap&m=133968581105049&w=2
Yes, certainly, they would be needed as well.
regards,
Rajendra
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
