[PATCH 1/8] PM / Domains: Make genpd state allocation dynamic
Lina Iyer
lina.iyer at linaro.org
Thu Oct 6 08:40:52 PDT 2016
On Thu, Oct 06 2016 at 02:37 -0600, Ulf Hansson wrote:
>On 5 October 2016 at 22:31, Lina Iyer <lina.iyer at linaro.org> wrote:
>> Allow PM Domain states to be defined dynamically by the drivers. This
>> removes the limitation on the maximum number of states possible for a
>> domain.
>>
>> Cc: Axel Haslam <ahaslam+renesas at baylibre.com>
>> Suggested-by: Ulf Hansson <ulf.hansson at linaro.org>
>> Signed-off-by: Lina Iyer <lina.iyer at linaro.org>
<...>
>> -#define GENPD_MAX_NUM_STATES 8 /* Number of possible low power states */
>> -
>> enum gpd_status {
>> GPD_STATE_ACTIVE = 0, /* PM domain is active */
>> GPD_STATE_POWER_OFF, /* PM domain is off */
>> @@ -70,7 +68,7 @@ struct generic_pm_domain {
>> void (*detach_dev)(struct generic_pm_domain *domain,
>> struct device *dev);
>> unsigned int flags; /* Bit field of configs for genpd */
>> - struct genpd_power_state states[GENPD_MAX_NUM_STATES];
>> + struct genpd_power_state *states;
>> unsigned int state_count; /* number of states */
>> unsigned int state_idx; /* state that genpd will go to when off */
>>
>> --
>> 2.7.4
>>
>
>In general I like the improvement, but..
>
>This change implies that ->states may very well be NULL. This isn't
>validated by genpd's internal logic when power off/on the domain
>(genpd_power_on|off(), __default_power_down_ok()). You need to fix
>this, somehow.
>
Good point.
>Perhaps the easiest solutions is, when pm_genpd_init() finds that
>->state is NULL, that we allocate a struct genpd_power_state with
>array size of 1 and assign it to ->states. Although, doing this also
>means you need to track that genpd was responsible for the the
>allocation, so it must also free the data from within genpd_remove().
>
>Unless you have other ideas!?
>
I can think of some hacks, but they are uglier than the problem we are
trying to solve. We could drop this patch. Real world situations would
not have more than 8 states and if there is one, we can think about it
then.
Thanks,
Lina
More information about the linux-arm-kernel
mailing list