[PATCH RFC 01/27] PM / Domains: core changes for multiple states

Ulf Hansson ulf.hansson at linaro.org
Thu Dec 17 13:19:31 PST 2015


[...]

>>> +static int genpd_alloc_states_data(struct generic_pm_domain *genpd,
>>
>> In this patch I would rather just add *one* new "alloc" function and
>> name it like below.
>>
>> static int genpd_alloc_default_state(struct generic_pm_domain *genpd)
>>
>> I assume the name I suggest for it, indicates what it needs to do and
>> *not* needs to do.
>
> Hi Ulf,
>
> Thanks for your thorough review!
> i will implement your suggestions for the next spin,

Great, thanks!

>
> However I have a doubt about this comment, do you mean that i should get rid of
> genpd_alloc_states_data completly? i had a static number of states before

Yes, in this patch - but we still want to alloc the data on the heap.

Just alloc a state struct with size of one array.

> but that was droped because o wasted memory if no states were needed.

There will always be at least *one* state, so we shouldn't waste any
memory following my suggestion above.

>
> if you just meant that it should be added in a separate patch, since i
> will be sqashing

I think you need to go through review comments for each patch
separately. In the end that might very well mean that you should
completely drop some patches, rework some and even create some some
new.

> this patch with "PM / Domains: make governor select deepest state"   can
> i keep it here? maybe it should  go with the dt parsing patch from Marc?

I don't think so.

Try to keep each logical change in a separate patch. That makes it
easier to review and thus also to accept patches.

[...]

Kind regards
Uffe



More information about the linux-arm-kernel mailing list