[...] >>> +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 -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
